virtio: support dynamic device add - #103
Conversation
|
@Inquisitor-201 @ForeverYolo Could you help review this PR when you have time? This PR mainly addresses #57 by adding dynamic Virtio device add support. Thanks! |
agicy
left a comment
There was a problem hiding this comment.
This PR is ~45 commits behind upstream main. Since your branch was created, upstream has merged several things that touch the same files:
- PR #102 (perf/virtio-net): rewrote the net RX path (
process_descriptor_chain_buf,update_used_ring_batch,IFF_VNET_HDR), addedstatus_changedhook - PR #100 (refactor/zone-mem-struct): changed zone memory struct layout
- PR #84 (feat-virtio-scmi): SCMI protocol support in both driver and tools
After rebase, the net event handler, SCMI handling, and status_changed hook will all be consistent with current upstream. The per-device close hardening and the staging pattern (unpublished -> publish) will still apply cleanly - those changes don't conflict with upstream.
|
Looking at the code, there are 4 separate switch-case / if-else blocks that dispatch on device type:
Plus I think we should consolidate into ops table: // Runtime dispatch - used throughout the daemon's lifetime
struct virtio_dev_ops {
VirtioDeviceType type;
uint64_t features;
uint32_t num_queues;
uint32_t queue_max_size;
int (*init)(VirtIODevice *vdev, void *params);
void (*close)(VirtIODevice *vdev);
void (*reset)(VirtIODevice *vdev);
void (*status_changed)(VirtIODevice *vdev, uint32_t status);
int (*notify_handlers[VIRTIO_MAX_VQUEUES])(VirtIODevice *, VirtQueue *);
};
// Config parsing - only used during virtio_start / virtio_add JSON parsing
struct virtio_config_ops {
int (*parse)(cJSON *json, void **params_out);
void (*free_params)(void *params);
};With a clear contract:
Then the switch-case blocks collapse to: // create_virtio_device_unpublished
const struct virtio_dev_ops *ops = lookup_dev_ops(dev_type);
vdev->regs.dev_feature = ops->features;
if (ops->init(vdev, params) != 0) goto err;
if (init_virtio_queue(vdev, ops) != 0) goto err;
// destroy_unpublished_virtio_device
vdev->virtio_close(vdev); // each device sets this = ops->close
// init_virtio_queue
vdev->vqs_len = ops->num_queues;
for (i = 0; i < ops->num_queues; i++) {
vqs[i].queue_num_max = ops->queue_max_size;
vqs[i].notify_handler = ops->notify_handlers[i];
...
}
// JSON parsing in virtio_start / virtio_add path
const struct virtio_config_ops *cfg_ops = lookup_config_ops(dev_type);
void *params = NULL;
if (cfg_ops->parse(device_json, ¶ms) != 0) return -1;
// ... later: cfg_ops->free_params(params);Your staging architecture - I already have a draft of this on a branch. If the direction looks right to you, I'll push it so you can rebase on top of it. Curious what you think. |
|
@agicy Thanks for the detailed review. The latest upstream changes and the control-thread shutdown fix have been pushed to the PR branch. I am reviewing the updated diff and rerunning the tests now. I will reply to each point with the detailed results after the review is complete. |
|
@Jaxtonmax Thanks for your quick fix. The ops-table draft is up at #112. It currently includes the commits we discussed - init/close contract, lifecycle ops table, and the That PR depends on |
|
Thanks for the clarification. I understand that the ops-table and related lifecycle/config refactoring are being handled in PR #112. I will keep PR #103 focused on the dynamic-add implementation, staged creation, failure cleanup, and control-thread shutdown fix. The QEMU AArch64 validation for PR #103 covers the blk + console paths, including failed add rollback, successful add, zone startup, legacy virtio start, and daemon shutdown. Net and SCMI runtime validation are not covered by the current QEMU configuration because they require separate TAP/bridge and SCMI platform setups. I have completed the review of the updated diff and will keep the remaining changes scoped to PR #103. Please re-review it when convenient. |
ae462b7 to
7464cfd
Compare
virtio: support dynamic device add
virtio: support dynamic device add
virtio: support dynamic device add
Summary
Closes #57.
This PR adds a
virtio addflow so new Virtio device configurations can be sent to an already running hvisor-tool Virtio backend daemon.Main changes:
hvisor virtio add <virtio.json>CLI support./run/hvisor-virtio.sock.virtio addact as a client while the running daemon performs the actual state mutation.virtio startJSON parsing, memory mapping, validation, and device creation path for later add requests.vdevs[].README.mdandREADME-zh.md.New Workflow
The existing one-shot startup flow is still supported:
nohup ./hvisor virtio start virtio_cfg.json & ./hvisor zone start zone1_linux.jsonThe new flow can add another Virtio configuration to the running daemon:
nohup ./hvisor virtio start virtio_start_empty.json & ./hvisor virtio add virtio_add_console_blk.json ./hvisor zone start zone1_linux.jsonFor MMIO Virtio devices,
virtio addshould be run before starting the zone that will use the newly added devices, unless full guest runtime hotplug is separately verified.Implementation Notes
virtio addconnects to the daemon through/run/hvisor-virtio.sock.vdevs[],zone_mem[], event monitor state, and device-specific resources.VDEV_MUTEXprotects global device table publication and MMIO address publication.ZONE_MEM_MUTEXprotects zone memory mapping records.vdevs[]togetherValidation
Runtime validation performed on QEMU AArch64 with QEMU 9.0.1:
virtio addwithconsole + wrong blk:virtio add failed/dev/ptsvirtio addwithconsole + valid blk:virtio add succeededvirtio startstyle flow after updating the hvisor side.Static/local checks performed during the branch work:
Notes