| Message ID | 20101229145245.GA8847@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Dec 29, 2010 at 04:52:46PM +0200, Michael S. Tsirkin wrote: > This is on top of the ioeventfd patches, and > fixes two potential issues when ioeventfd is mixed with vhost-net: > - ioeventfd could start running then get stopped for vhost-net. > For example, vm state change handlers run in no particular order. > This would be hard to debug. Prevent this by always running state > callback before ioeventfd start and after ioeventfd stop. > - Separate the flags for user-requested ioeventfd and for ioeventfd > being overridden by vhost backend. > This will make it possible to enable/disable vhost depending on > guest behaviour. > > I'll probably split this patch in two, and merge into the > appropriate patches in the ioeventfd series. > > Compile-tested only so far, would appreciate feedback/test reports. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Stefan - just to clarify, I'm waiting for you ack to merge this + the ioeventfd patches. > diff --git a/hw/vhost.c b/hw/vhost.c > index 6082da2..1d09ed0 100644 > --- a/hw/vhost.c > +++ b/hw/vhost.c > @@ -630,6 +630,17 @@ void vhost_dev_cleanup(struct vhost_dev *hdev) > int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev) > { > int i, r; > + /* A binding must support vmstate notifiers (for migration to work) */ > + if (!vdev->binding->vmstate_change) { > + fprintf(stderr, "binding does not support vmstate notifiers\n"); > + r = -ENOSYS; > + goto fail; > + } > + if (!vdev->binding->set_guest_notifiers) { > + fprintf(stderr, "binding does not support guest notifiers\n"); > + r = -ENOSYS; > + goto fail; > + } > if (!vdev->binding->set_guest_notifiers) { > fprintf(stderr, "binding does not support guest notifiers\n"); > r = -ENOSYS; > diff --git a/hw/virtio-net.c b/hw/virtio-net.c > index ec1bf8d..ccb3e63 100644 > --- a/hw/virtio-net.c > +++ b/hw/virtio-net.c > @@ -54,8 +54,6 @@ typedef struct VirtIONet > uint8_t nouni; > uint8_t nobcast; > uint8_t vhost_started; > - bool vm_running; > - VMChangeStateEntry *vmstate; > struct { > int in_use; > int first_multi; > @@ -102,7 +100,7 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config) > static bool virtio_net_started(VirtIONet *n, uint8_t status) > { > return (status & VIRTIO_CONFIG_S_DRIVER_OK) && > - (n->status & VIRTIO_NET_S_LINK_UP) && n->vm_running; > + (n->status & VIRTIO_NET_S_LINK_UP) && n->vdev.vm_running; > } > > static void virtio_net_vhost_status(VirtIONet *n, uint8_t status) > @@ -453,7 +451,7 @@ static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq) > static int virtio_net_can_receive(VLANClientState *nc) > { > VirtIONet *n = DO_UPCAST(NICState, nc, nc)->opaque; > - if (!n->vm_running) { > + if (!n->vdev.vm_running) { > return 0; > } > > @@ -708,7 +706,7 @@ static int32_t virtio_net_flush_tx(VirtIONet *n, VirtQueue *vq) > return num_packets; > } > > - assert(n->vm_running); > + assert(n->vdev.vm_running); > > if (n->async_tx.elem.out_num) { > virtio_queue_set_notification(n->tx_vq, 0); > @@ -769,7 +767,7 @@ static void virtio_net_handle_tx_timer(VirtIODevice *vdev, VirtQueue *vq) > VirtIONet *n = to_virtio_net(vdev); > > /* This happens when device was stopped but VCPU wasn't. */ > - if (!n->vm_running) { > + if (!n->vdev.vm_running) { > n->tx_waiting = 1; > return; > } > @@ -796,7 +794,7 @@ static void virtio_net_handle_tx_bh(VirtIODevice *vdev, VirtQueue *vq) > } > n->tx_waiting = 1; > /* This happens when device was stopped but VCPU wasn't. */ > - if (!n->vm_running) { > + if (!n->vdev.vm_running) { > return; > } > virtio_queue_set_notification(vq, 0); > @@ -806,7 +804,7 @@ static void virtio_net_handle_tx_bh(VirtIODevice *vdev, VirtQueue *vq) > static void virtio_net_tx_timer(void *opaque) > { > VirtIONet *n = opaque; > - assert(n->vm_running); > + assert(n->vdev.vm_running); > > n->tx_waiting = 0; > > @@ -823,7 +821,7 @@ static void virtio_net_tx_bh(void *opaque) > VirtIONet *n = opaque; > int32_t ret; > > - assert(n->vm_running); > + assert(n->vdev.vm_running); > > n->tx_waiting = 0; > > @@ -988,16 +986,6 @@ static NetClientInfo net_virtio_info = { > .link_status_changed = virtio_net_set_link_status, > }; > > -static void virtio_net_vmstate_change(void *opaque, int running, int reason) > -{ > - VirtIONet *n = opaque; > - n->vm_running = running; > - /* This is called when vm is started/stopped, > - * it will start/stop vhost backend if appropriate > - * e.g. after migration. */ > - virtio_net_set_status(&n->vdev, n->vdev.status); > -} > - > VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf, > virtio_net_conf *net) > { > @@ -1052,7 +1040,6 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf, > n->qdev = dev; > register_savevm(dev, "virtio-net", -1, VIRTIO_NET_VM_VERSION, > virtio_net_save, virtio_net_load, n); > - n->vmstate = qemu_add_vm_change_state_handler(virtio_net_vmstate_change, n); > > add_boot_device_path(conf->bootindex, dev, "/ethernet-phy@0"); > > @@ -1062,7 +1049,6 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf, > void virtio_net_exit(VirtIODevice *vdev) > { > VirtIONet *n = DO_UPCAST(VirtIONet, vdev, vdev); > - qemu_del_vm_change_state_handler(n->vmstate); > > /* This will stop vhost backend if appropriate. */ > virtio_net_set_status(vdev, 0); > diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c > index b2181fc..75b5e53 100644 > --- a/hw/virtio-pci.c > +++ b/hw/virtio-pci.c > @@ -112,8 +112,8 @@ typedef struct { > /* Max. number of ports we can have for a the virtio-serial device */ > uint32_t max_virtserial_ports; > virtio_net_conf net; > + bool ioeventfd_disabled; > bool ioeventfd_started; > - VMChangeStateEntry *vm_change_state_entry; > } VirtIOPCIProxy; > > /* virtio device */ > @@ -251,6 +251,7 @@ static int virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy) > int n, r; > > if (!(proxy->flags & VIRTIO_PCI_FLAG_USE_IOEVENTFD) || > + proxy->ioeventfd_disabled || > proxy->ioeventfd_started) { > return 0; > } > @@ -280,7 +281,7 @@ assign_error: > virtio_pci_set_host_notifier_internal(proxy, n, false); > } > proxy->ioeventfd_started = false; > - proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD; > + proxy->ioeventfd_disabled = true; > return r; > } > > @@ -350,13 +351,16 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val) > virtio_queue_notify(vdev, val); > break; > case VIRTIO_PCI_STATUS: > - if (val & VIRTIO_CONFIG_S_DRIVER_OK) { > - virtio_pci_start_ioeventfd(proxy); > - } else { > + if (!(val & VIRTIO_CONFIG_S_DRIVER_OK)) { > virtio_pci_stop_ioeventfd(proxy); > } > > virtio_set_status(vdev, val & 0xFF); > + > + if (val & VIRTIO_CONFIG_S_DRIVER_OK) { > + virtio_pci_start_ioeventfd(proxy); > + } > + > if (vdev->status == 0) { > virtio_reset(proxy->vdev); > msix_unuse_all_vectors(&proxy->pci_dev); > @@ -618,22 +622,24 @@ static int virtio_pci_set_host_notifier(void *opaque, int n, bool assign) > /* Stop using ioeventfd for virtqueue kick if the device starts using host > * notifiers. This makes it easy to avoid stepping on each others' toes. > */ > - if (proxy->ioeventfd_started) { > + proxy->ioeventfd_disabled = assign; > + if (assign) { > virtio_pci_stop_ioeventfd(proxy); > - proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD; > } > > return virtio_pci_set_host_notifier_internal(proxy, n, assign); > } > > -static void virtio_pci_vm_change_state_handler(void *opaque, int running, int reason) > +static void virtio_pci_vmstate_change(void *opaque, bool running) > { > VirtIOPCIProxy *proxy = opaque; > > if (running && (proxy->vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) { > + virtio_set_status(proxy->vdev, proxy->vdev->status); > virtio_pci_start_ioeventfd(proxy); > } else { > virtio_pci_stop_ioeventfd(proxy); > + virtio_set_status(proxy->vdev, proxy->vdev->status); > } > } > > @@ -646,6 +652,7 @@ static const VirtIOBindings virtio_pci_bindings = { > .get_features = virtio_pci_get_features, > .set_host_notifier = virtio_pci_set_host_notifier, > .set_guest_notifiers = virtio_pci_set_guest_notifiers, > + .vmstate_change = virtio_pci_vmstate_change, > }; > > static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev, > @@ -699,9 +706,6 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev, > proxy->host_features |= 0x1 << VIRTIO_F_BAD_FEATURE; > proxy->host_features = vdev->get_features(vdev, proxy->host_features); > > - proxy->vm_change_state_entry = qemu_add_vm_change_state_handler( > - virtio_pci_vm_change_state_handler, > - proxy); > } > > static int virtio_blk_init_pci(PCIDevice *pci_dev) > @@ -729,9 +733,6 @@ static int virtio_blk_init_pci(PCIDevice *pci_dev) > > static int virtio_exit_pci(PCIDevice *pci_dev) > { > - VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev); > - > - qemu_del_vm_change_state_handler(proxy->vm_change_state_entry); > return msix_uninit(pci_dev); > } > > diff --git a/hw/virtio.c b/hw/virtio.c > index e40296a..13c3373 100644 > --- a/hw/virtio.c > +++ b/hw/virtio.c > @@ -751,11 +751,21 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f) > > void virtio_cleanup(VirtIODevice *vdev) > { > + qemu_del_vm_change_state_handler(vdev->vmstate); > if (vdev->config) > qemu_free(vdev->config); > qemu_free(vdev->vq); > } > > +static void virtio_vmstate_change(void *opaque, int running, int reason) > +{ > + VirtIODevice *vdev = opaque; > + vdev->vm_running = running; > + if (vdev->binding->vmstate_change) { > + vdev->binding->vmstate_change(vdev->binding_opaque, running); > + } > +} > + > VirtIODevice *virtio_common_init(const char *name, uint16_t device_id, > size_t config_size, size_t struct_size) > { > @@ -782,6 +792,8 @@ VirtIODevice *virtio_common_init(const char *name, uint16_t device_id, > else > vdev->config = NULL; > > + vdev->vmstate = qemu_add_vm_change_state_handler(virtio_vmstate_change, vdev); > + > return vdev; > } > > diff --git a/hw/virtio.h b/hw/virtio.h > index 5ae521c..d8546d5 100644 > --- a/hw/virtio.h > +++ b/hw/virtio.h > @@ -95,6 +95,7 @@ typedef struct { > unsigned (*get_features)(void * opaque); > int (*set_guest_notifiers)(void * opaque, bool assigned); > int (*set_host_notifier)(void * opaque, int n, bool assigned); > + void (*vmstate_change)(void * opaque, bool running); > } VirtIOBindings; > > #define VIRTIO_PCI_QUEUE_MAX 64 > @@ -123,6 +124,8 @@ struct VirtIODevice > const VirtIOBindings *binding; > void *binding_opaque; > uint16_t device_id; > + bool vm_running; > + VMChangeStateEntry *vmstate; > }; > > static inline void virtio_set_status(VirtIODevice *vdev, uint8_t val)
On Mon, Jan 03, 2011 at 06:38:53PM +0200, Michael S. Tsirkin wrote: > On Wed, Dec 29, 2010 at 04:52:46PM +0200, Michael S. Tsirkin wrote: > > This is on top of the ioeventfd patches, and > > fixes two potential issues when ioeventfd is mixed with vhost-net: > > - ioeventfd could start running then get stopped for vhost-net. > > For example, vm state change handlers run in no particular order. > > This would be hard to debug. Prevent this by always running state > > callback before ioeventfd start and after ioeventfd stop. > > - Separate the flags for user-requested ioeventfd and for ioeventfd > > being overridden by vhost backend. > > This will make it possible to enable/disable vhost depending on > > guest behaviour. > > > > I'll probably split this patch in two, and merge into the > > appropriate patches in the ioeventfd series. > > > > Compile-tested only so far, would appreciate feedback/test reports. > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > Stefan - just to clarify, I'm waiting for you ack > to merge this + the ioeventfd patches. Back from the holidays today. Will send feedback/ack later today. Stefan
On Wed, Dec 29, 2010 at 2:52 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > I'll probably split this patch in two, and merge into the > appropriate patches in the ioeventfd series. > > Compile-tested only so far, would appreciate feedback/test reports. virtio-ioeventfd works as expected. > diff --git a/hw/vhost.c b/hw/vhost.c > index 6082da2..1d09ed0 100644 > --- a/hw/vhost.c > +++ b/hw/vhost.c > @@ -630,6 +630,17 @@ void vhost_dev_cleanup(struct vhost_dev *hdev) > int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev) > { > int i, r; > + /* A binding must support vmstate notifiers (for migration to work) */ > + if (!vdev->binding->vmstate_change) { > + fprintf(stderr, "binding does not support vmstate notifiers\n"); > + r = -ENOSYS; > + goto fail; > + } > + if (!vdev->binding->set_guest_notifiers) { > + fprintf(stderr, "binding does not support guest notifiers\n"); > + r = -ENOSYS; > + goto fail; > + } Merge error? The set_guest_notifiers check is already present. Stefan
On Tue, Jan 04, 2011 at 04:57:43PM +0000, Stefan Hajnoczi wrote: > On Wed, Dec 29, 2010 at 2:52 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > > I'll probably split this patch in two, and merge into the > > appropriate patches in the ioeventfd series. > > > > Compile-tested only so far, would appreciate feedback/test reports. > > virtio-ioeventfd works as expected. > > > diff --git a/hw/vhost.c b/hw/vhost.c > > index 6082da2..1d09ed0 100644 > > --- a/hw/vhost.c > > +++ b/hw/vhost.c > > @@ -630,6 +630,17 @@ void vhost_dev_cleanup(struct vhost_dev *hdev) > > int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev) > > { > > int i, r; > > + /* A binding must support vmstate notifiers (for migration to work) */ > > + if (!vdev->binding->vmstate_change) { > > + fprintf(stderr, "binding does not support vmstate notifiers\n"); > > + r = -ENOSYS; > > + goto fail; > > + } > > + if (!vdev->binding->set_guest_notifiers) { > > + fprintf(stderr, "binding does not support guest notifiers\n"); > > + r = -ENOSYS; > > + goto fail; > > + } > > Merge error? The set_guest_notifiers check is already present. > > Stefan OK I need to test with/without vhost-net and merge then. Thanks!
diff --git a/hw/vhost.c b/hw/vhost.c index 6082da2..1d09ed0 100644 --- a/hw/vhost.c +++ b/hw/vhost.c @@ -630,6 +630,17 @@ void vhost_dev_cleanup(struct vhost_dev *hdev) int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev) { int i, r; + /* A binding must support vmstate notifiers (for migration to work) */ + if (!vdev->binding->vmstate_change) { + fprintf(stderr, "binding does not support vmstate notifiers\n"); + r = -ENOSYS; + goto fail; + } + if (!vdev->binding->set_guest_notifiers) { + fprintf(stderr, "binding does not support guest notifiers\n"); + r = -ENOSYS; + goto fail; + } if (!vdev->binding->set_guest_notifiers) { fprintf(stderr, "binding does not support guest notifiers\n"); r = -ENOSYS; diff --git a/hw/virtio-net.c b/hw/virtio-net.c index ec1bf8d..ccb3e63 100644 --- a/hw/virtio-net.c +++ b/hw/virtio-net.c @@ -54,8 +54,6 @@ typedef struct VirtIONet uint8_t nouni; uint8_t nobcast; uint8_t vhost_started; - bool vm_running; - VMChangeStateEntry *vmstate; struct { int in_use; int first_multi; @@ -102,7 +100,7 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config) static bool virtio_net_started(VirtIONet *n, uint8_t status) { return (status & VIRTIO_CONFIG_S_DRIVER_OK) && - (n->status & VIRTIO_NET_S_LINK_UP) && n->vm_running; + (n->status & VIRTIO_NET_S_LINK_UP) && n->vdev.vm_running; } static void virtio_net_vhost_status(VirtIONet *n, uint8_t status) @@ -453,7 +451,7 @@ static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq) static int virtio_net_can_receive(VLANClientState *nc) { VirtIONet *n = DO_UPCAST(NICState, nc, nc)->opaque; - if (!n->vm_running) { + if (!n->vdev.vm_running) { return 0; } @@ -708,7 +706,7 @@ static int32_t virtio_net_flush_tx(VirtIONet *n, VirtQueue *vq) return num_packets; } - assert(n->vm_running); + assert(n->vdev.vm_running); if (n->async_tx.elem.out_num) { virtio_queue_set_notification(n->tx_vq, 0); @@ -769,7 +767,7 @@ static void virtio_net_handle_tx_timer(VirtIODevice *vdev, VirtQueue *vq) VirtIONet *n = to_virtio_net(vdev); /* This happens when device was stopped but VCPU wasn't. */ - if (!n->vm_running) { + if (!n->vdev.vm_running) { n->tx_waiting = 1; return; } @@ -796,7 +794,7 @@ static void virtio_net_handle_tx_bh(VirtIODevice *vdev, VirtQueue *vq) } n->tx_waiting = 1; /* This happens when device was stopped but VCPU wasn't. */ - if (!n->vm_running) { + if (!n->vdev.vm_running) { return; } virtio_queue_set_notification(vq, 0); @@ -806,7 +804,7 @@ static void virtio_net_handle_tx_bh(VirtIODevice *vdev, VirtQueue *vq) static void virtio_net_tx_timer(void *opaque) { VirtIONet *n = opaque; - assert(n->vm_running); + assert(n->vdev.vm_running); n->tx_waiting = 0; @@ -823,7 +821,7 @@ static void virtio_net_tx_bh(void *opaque) VirtIONet *n = opaque; int32_t ret; - assert(n->vm_running); + assert(n->vdev.vm_running); n->tx_waiting = 0; @@ -988,16 +986,6 @@ static NetClientInfo net_virtio_info = { .link_status_changed = virtio_net_set_link_status, }; -static void virtio_net_vmstate_change(void *opaque, int running, int reason) -{ - VirtIONet *n = opaque; - n->vm_running = running; - /* This is called when vm is started/stopped, - * it will start/stop vhost backend if appropriate - * e.g. after migration. */ - virtio_net_set_status(&n->vdev, n->vdev.status); -} - VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf, virtio_net_conf *net) { @@ -1052,7 +1040,6 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf, n->qdev = dev; register_savevm(dev, "virtio-net", -1, VIRTIO_NET_VM_VERSION, virtio_net_save, virtio_net_load, n); - n->vmstate = qemu_add_vm_change_state_handler(virtio_net_vmstate_change, n); add_boot_device_path(conf->bootindex, dev, "/ethernet-phy@0"); @@ -1062,7 +1049,6 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf, void virtio_net_exit(VirtIODevice *vdev) { VirtIONet *n = DO_UPCAST(VirtIONet, vdev, vdev); - qemu_del_vm_change_state_handler(n->vmstate); /* This will stop vhost backend if appropriate. */ virtio_net_set_status(vdev, 0); diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index b2181fc..75b5e53 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -112,8 +112,8 @@ typedef struct { /* Max. number of ports we can have for a the virtio-serial device */ uint32_t max_virtserial_ports; virtio_net_conf net; + bool ioeventfd_disabled; bool ioeventfd_started; - VMChangeStateEntry *vm_change_state_entry; } VirtIOPCIProxy; /* virtio device */ @@ -251,6 +251,7 @@ static int virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy) int n, r; if (!(proxy->flags & VIRTIO_PCI_FLAG_USE_IOEVENTFD) || + proxy->ioeventfd_disabled || proxy->ioeventfd_started) { return 0; } @@ -280,7 +281,7 @@ assign_error: virtio_pci_set_host_notifier_internal(proxy, n, false); } proxy->ioeventfd_started = false; - proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD; + proxy->ioeventfd_disabled = true; return r; } @@ -350,13 +351,16 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val) virtio_queue_notify(vdev, val); break; case VIRTIO_PCI_STATUS: - if (val & VIRTIO_CONFIG_S_DRIVER_OK) { - virtio_pci_start_ioeventfd(proxy); - } else { + if (!(val & VIRTIO_CONFIG_S_DRIVER_OK)) { virtio_pci_stop_ioeventfd(proxy); } virtio_set_status(vdev, val & 0xFF); + + if (val & VIRTIO_CONFIG_S_DRIVER_OK) { + virtio_pci_start_ioeventfd(proxy); + } + if (vdev->status == 0) { virtio_reset(proxy->vdev); msix_unuse_all_vectors(&proxy->pci_dev); @@ -618,22 +622,24 @@ static int virtio_pci_set_host_notifier(void *opaque, int n, bool assign) /* Stop using ioeventfd for virtqueue kick if the device starts using host * notifiers. This makes it easy to avoid stepping on each others' toes. */ - if (proxy->ioeventfd_started) { + proxy->ioeventfd_disabled = assign; + if (assign) { virtio_pci_stop_ioeventfd(proxy); - proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD; } return virtio_pci_set_host_notifier_internal(proxy, n, assign); } -static void virtio_pci_vm_change_state_handler(void *opaque, int running, int reason) +static void virtio_pci_vmstate_change(void *opaque, bool running) { VirtIOPCIProxy *proxy = opaque; if (running && (proxy->vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) { + virtio_set_status(proxy->vdev, proxy->vdev->status); virtio_pci_start_ioeventfd(proxy); } else { virtio_pci_stop_ioeventfd(proxy); + virtio_set_status(proxy->vdev, proxy->vdev->status); } } @@ -646,6 +652,7 @@ static const VirtIOBindings virtio_pci_bindings = { .get_features = virtio_pci_get_features, .set_host_notifier = virtio_pci_set_host_notifier, .set_guest_notifiers = virtio_pci_set_guest_notifiers, + .vmstate_change = virtio_pci_vmstate_change, }; static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev, @@ -699,9 +706,6 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev, proxy->host_features |= 0x1 << VIRTIO_F_BAD_FEATURE; proxy->host_features = vdev->get_features(vdev, proxy->host_features); - proxy->vm_change_state_entry = qemu_add_vm_change_state_handler( - virtio_pci_vm_change_state_handler, - proxy); } static int virtio_blk_init_pci(PCIDevice *pci_dev) @@ -729,9 +733,6 @@ static int virtio_blk_init_pci(PCIDevice *pci_dev) static int virtio_exit_pci(PCIDevice *pci_dev) { - VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev); - - qemu_del_vm_change_state_handler(proxy->vm_change_state_entry); return msix_uninit(pci_dev); } diff --git a/hw/virtio.c b/hw/virtio.c index e40296a..13c3373 100644 --- a/hw/virtio.c +++ b/hw/virtio.c @@ -751,11 +751,21 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f) void virtio_cleanup(VirtIODevice *vdev) { + qemu_del_vm_change_state_handler(vdev->vmstate); if (vdev->config) qemu_free(vdev->config); qemu_free(vdev->vq); } +static void virtio_vmstate_change(void *opaque, int running, int reason) +{ + VirtIODevice *vdev = opaque; + vdev->vm_running = running; + if (vdev->binding->vmstate_change) { + vdev->binding->vmstate_change(vdev->binding_opaque, running); + } +} + VirtIODevice *virtio_common_init(const char *name, uint16_t device_id, size_t config_size, size_t struct_size) { @@ -782,6 +792,8 @@ VirtIODevice *virtio_common_init(const char *name, uint16_t device_id, else vdev->config = NULL; + vdev->vmstate = qemu_add_vm_change_state_handler(virtio_vmstate_change, vdev); + return vdev; } diff --git a/hw/virtio.h b/hw/virtio.h index 5ae521c..d8546d5 100644 --- a/hw/virtio.h +++ b/hw/virtio.h @@ -95,6 +95,7 @@ typedef struct { unsigned (*get_features)(void * opaque); int (*set_guest_notifiers)(void * opaque, bool assigned); int (*set_host_notifier)(void * opaque, int n, bool assigned); + void (*vmstate_change)(void * opaque, bool running); } VirtIOBindings; #define VIRTIO_PCI_QUEUE_MAX 64 @@ -123,6 +124,8 @@ struct VirtIODevice const VirtIOBindings *binding; void *binding_opaque; uint16_t device_id; + bool vm_running; + VMChangeStateEntry *vmstate; }; static inline void virtio_set_status(VirtIODevice *vdev, uint8_t val)
This is on top of the ioeventfd patches, and fixes two potential issues when ioeventfd is mixed with vhost-net: - ioeventfd could start running then get stopped for vhost-net. For example, vm state change handlers run in no particular order. This would be hard to debug. Prevent this by always running state callback before ioeventfd start and after ioeventfd stop. - Separate the flags for user-requested ioeventfd and for ioeventfd being overridden by vhost backend. This will make it possible to enable/disable vhost depending on guest behaviour. I'll probably split this patch in two, and merge into the appropriate patches in the ioeventfd series. Compile-tested only so far, would appreciate feedback/test reports. Signed-off-by: Michael S. Tsirkin <mst@redhat.com>