[Qemu-devel] [PATCH 00/11] virtio: cleanup and fix hot-unplug

[Qemu-devel] [PATCH 00/11] virtio: cleanup and fix hot-unplug

[Paolo Bonzini]

This series fixes hot-unplug of virtio devices, which can crash due to
dangling pointer accesses.

The current implementation supports guest-initiated hot-unplug via the
virtio_bus_destroy_device function, but not hot-unplugging the virtio
device by virtue of unplugging its parent container device.

The problem is that the callback for the bus implementation to cleanup
is placed in the wrong place; it is in virtio_bus_destroy_device, which
should be called by the bus, instead of being somewhere in device code.
We need to have the callback in device code (for example in dc->exit),
so that we invoke it on every unplug action, no matter who starts it.

Thus, the series cleans up plugging and unplugging of virtio devices
so that it does not need any help from the bus (patches 1-4).  It then
stops the virtio devices' overriding of dc->exit, moving their cleanup
code to the new exit callback in VirtioDeviceClass (patches 5-10).
Finally, patch 11 can make virtio-pci implement the device_unplugged
callback.

Something similar is probably needed in virtio-ccw too.  However,
virtio-ccw needs more surgery because it does not include a device_plugged
callback either, so I did not touch it.

Paolo Bonzini (11):
  virtio-bus: remove vdev field
  virtio-pci: remove vdev field
  virtio-ccw: remove vdev field
  virtio-bus: cleanup plug/unplug interface
  virtio-blk: switch exit callback to VirtioDeviceClass
  virtio-serial: switch exit callback to VirtioDeviceClass
  virtio-net: switch exit callback to VirtioDeviceClass
  virtio-scsi: switch exit callback to VirtioDeviceClass
  virtio-balloon: switch exit callback to VirtioDeviceClass
  virtio-rng: switch exit callback to VirtioDeviceClass
  virtio-pci: add device_unplugged callback

 hw/block/virtio-blk.c           |  10 ++--
 hw/char/virtio-serial-bus.c     |  10 ++--
 hw/net/virtio-net.c             |  11 ++--
 hw/s390x/virtio-ccw.c           |  80 +++++++++++++++------------
 hw/s390x/virtio-ccw.h           |   1 -
 hw/scsi/vhost-scsi.c            |  11 ++--
 hw/scsi/virtio-scsi.c           |  15 +++--
 hw/virtio/virtio-balloon.c      |  10 ++--
 hw/virtio/virtio-bus.c          |  81 +++++++++++++++------------
 hw/virtio/virtio-mmio.c         |   9 +--
 hw/virtio/virtio-pci.c          | 119 ++++++++++++++++++++++++----------------
 hw/virtio/virtio-pci.h          |   1 -
 hw/virtio/virtio-rng.c          |  10 ++--
 hw/virtio/virtio.c              |   7 ++-
 include/hw/virtio/virtio-bus.h  |  22 +++++---
 include/hw/virtio/virtio-scsi.h |   2 +-
 include/hw/virtio/virtio.h      |   1 +
 17 files changed, 223 insertions(+), 177 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 01/11] virtio-bus: remove vdev field

[Paolo Bonzini]

The vdev field is complicated to synchronize.  Just access the
BusState's list of children.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio/virtio-bus.c         | 67 ++++++++++++++++++++++++------------------
 hw/virtio/virtio-mmio.c        |  9 +++---
 hw/virtio/virtio-pci.c         |  2 +-
 include/hw/virtio/virtio-bus.h | 16 +++++++---
 4 files changed, 57 insertions(+), 37 deletions(-)

diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index 6849a01..669ce38 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -46,8 +46,6 @@ int virtio_bus_plug_device(VirtIODevice *vdev)
     VirtioBusClass *klass = VIRTIO_BUS_GET_CLASS(bus);
     DPRINTF("%s: plug device.\n", qbus->name);
 
-    bus->vdev = vdev;
-
     if (klass->device_plugged != NULL) {
         klass->device_plugged(qbus->parent);
     }
@@ -58,75 +56,84 @@ int virtio_bus_plug_device(VirtIODevice *vdev)
 /* Reset the virtio_bus */
 void virtio_bus_reset(VirtioBusState *bus)
 {
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
+
     DPRINTF("%s: reset device.\n", qbus->name);
-    if (bus->vdev != NULL) {
-        virtio_reset(bus->vdev);
+    if (vdev != NULL) {
+        virtio_reset(vdev);
     }
 }
 
 /* Destroy the VirtIODevice */
 void virtio_bus_destroy_device(VirtioBusState *bus)
 {
-    DeviceState *qdev;
     BusState *qbus = BUS(bus);
     VirtioBusClass *klass = VIRTIO_BUS_GET_CLASS(bus);
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
+
     DPRINTF("%s: remove device.\n", qbus->name);
 
-    if (bus->vdev != NULL) {
+    if (vdev != NULL) {
         if (klass->device_unplug != NULL) {
             klass->device_unplug(qbus->parent);
         }
-        qdev = DEVICE(bus->vdev);
-        qdev_free(qdev);
-        bus->vdev = NULL;
+        qdev_free(DEVICE(vdev));
     }
 }
 
 /* Get the device id of the plugged device. */
 uint16_t virtio_bus_get_vdev_id(VirtioBusState *bus)
 {
-    assert(bus->vdev != NULL);
-    return bus->vdev->device_id;
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
+    assert(vdev != NULL);
+    return vdev->device_id;
 }
 
 /* Get the config_len field of the plugged device. */
 size_t virtio_bus_get_vdev_config_len(VirtioBusState *bus)
 {
-    assert(bus->vdev != NULL);
-    return bus->vdev->config_len;
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
+    assert(vdev != NULL);
+    return vdev->config_len;
 }
 
 /* Get the features of the plugged device. */
 uint32_t virtio_bus_get_vdev_features(VirtioBusState *bus,
                                     uint32_t requested_features)
 {
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
     VirtioDeviceClass *k;
-    assert(bus->vdev != NULL);
-    k = VIRTIO_DEVICE_GET_CLASS(bus->vdev);
+
+    assert(vdev != NULL);
+    k = VIRTIO_DEVICE_GET_CLASS(vdev);
     assert(k->get_features != NULL);
-    return k->get_features(bus->vdev, requested_features);
+    return k->get_features(vdev, requested_features);
 }
 
 /* Set the features of the plugged device. */
 void virtio_bus_set_vdev_features(VirtioBusState *bus,
                                       uint32_t requested_features)
 {
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
     VirtioDeviceClass *k;
-    assert(bus->vdev != NULL);
-    k = VIRTIO_DEVICE_GET_CLASS(bus->vdev);
+
+    assert(vdev != NULL);
+    k = VIRTIO_DEVICE_GET_CLASS(vdev);
     if (k->set_features != NULL) {
-        k->set_features(bus->vdev, requested_features);
+        k->set_features(vdev, requested_features);
     }
 }
 
 /* Get bad features of the plugged device. */
 uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus)
 {
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
     VirtioDeviceClass *k;
-    assert(bus->vdev != NULL);
-    k = VIRTIO_DEVICE_GET_CLASS(bus->vdev);
+
+    assert(vdev != NULL);
+    k = VIRTIO_DEVICE_GET_CLASS(vdev);
     if (k->bad_features != NULL) {
-        return k->bad_features(bus->vdev);
+        return k->bad_features(vdev);
     } else {
         return 0;
     }
@@ -135,22 +142,26 @@ uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus)
 /* Get config of the plugged device. */
 void virtio_bus_get_vdev_config(VirtioBusState *bus, uint8_t *config)
 {
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
     VirtioDeviceClass *k;
-    assert(bus->vdev != NULL);
-    k = VIRTIO_DEVICE_GET_CLASS(bus->vdev);
+
+    assert(vdev != NULL);
+    k = VIRTIO_DEVICE_GET_CLASS(vdev);
     if (k->get_config != NULL) {
-        k->get_config(bus->vdev, config);
+        k->get_config(vdev, config);
     }
 }
 
 /* Set config of the plugged device. */
 void virtio_bus_set_vdev_config(VirtioBusState *bus, uint8_t *config)
 {
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
     VirtioDeviceClass *k;
-    assert(bus->vdev != NULL);
-    k = VIRTIO_DEVICE_GET_CLASS(bus->vdev);
+
+    assert(vdev != NULL);
+    k = VIRTIO_DEVICE_GET_CLASS(vdev);
     if (k->set_config != NULL) {
-        k->set_config(bus->vdev, config);
+        k->set_config(vdev, config);
     }
 }
 
diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 29cf284..8829eb0 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -95,7 +95,7 @@ static void virtio_mmio_bus_new(VirtioBusState *bus, size_t bus_size,
 static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size)
 {
     VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
-    VirtIODevice *vdev = proxy->bus.vdev;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
 
     DPRINTF("virtio_mmio_read offset 0x%x\n", (int)offset);
 
@@ -185,7 +185,7 @@ static void virtio_mmio_write(void *opaque, hwaddr offset, uint64_t value,
                               unsigned size)
 {
     VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
-    VirtIODevice *vdev = proxy->bus.vdev;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
 
     DPRINTF("virtio_mmio_write offset 0x%x value 0x%" PRIx64 "\n",
             (int)offset, value);
@@ -298,12 +298,13 @@ static const MemoryRegionOps virtio_mem_ops = {
 static void virtio_mmio_update_irq(DeviceState *opaque, uint16_t vector)
 {
     VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     int level;
 
-    if (!proxy->bus.vdev) {
+    if (!vdev) {
         return;
     }
-    level = (proxy->bus.vdev->isr != 0);
+    level = (vdev->isr != 0);
     DPRINTF("virtio_mmio setting IRQ %d\n", level);
     qemu_set_irq(proxy->irq, level);
 }
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 4825802..07708a3 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -943,7 +943,7 @@ static void virtio_pci_device_plugged(DeviceState *d)
     uint8_t *config;
     uint32_t size;
 
-    proxy->vdev = bus->vdev;
+    proxy->vdev = virtio_bus_get_device(bus);
 
     config = proxy->pci_dev.config;
     if (proxy->class_code) {
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index 9217f85..48afb7c 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -72,10 +72,6 @@ typedef struct VirtioBusClass {
 
 struct VirtioBusState {
     BusState parent_obj;
-    /*
-     * Only one VirtIODevice can be plugged on the bus.
-     */
-    VirtIODevice *vdev;
 };
 
 int virtio_bus_plug_device(VirtIODevice *vdev);
@@ -98,4 +94,16 @@ void virtio_bus_get_vdev_config(VirtioBusState *bus, uint8_t *config);
 /* Set config of the plugged device. */
 void virtio_bus_set_vdev_config(VirtioBusState *bus, uint8_t *config);
 
+static inline VirtIODevice *virtio_bus_get_device(VirtioBusState *bus)
+{
+    BusState *qbus = &bus->parent_obj;
+    BusChild *kid = QTAILQ_FIRST(&qbus->children);
+    DeviceState *qdev = kid ? kid->child : NULL;
+
+    /* This is used on the data path, the cast is guaranteed
+     * to succeed by the qdev machinery.
+     */
+    return (VirtIODevice *)qdev;
+}
+
 #endif /* VIRTIO_BUS_H */
-- 
1.8.3.1

[Qemu-devel] [PATCH 02/11] virtio-pci: remove vdev field

[Paolo Bonzini]

The vdev field is complicated to synchronize.  Just access the
BusState's list of children.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/s390x/virtio-ccw.h  |   1 -
 hw/virtio/virtio-pci.c | 107 +++++++++++++++++++++++++++++--------------------
 hw/virtio/virtio-pci.h |   1 -
 3 files changed, 63 insertions(+), 46 deletions(-)

diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
index 96d6f5d..00932c7 100644
--- a/hw/s390x/virtio-ccw.h
+++ b/hw/s390x/virtio-ccw.h
@@ -77,7 +77,6 @@ typedef struct VirtIOCCWDeviceClass {
 struct VirtioCcwDevice {
     DeviceState parent_obj;
     SubchDev *sch;
-    VirtIODevice *vdev;
     char *bus_id;
     uint32_t host_features[VIRTIO_CCW_FEATURE_SIZE];
     VirtioBusState bus;
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 07708a3..c8d8b99 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -113,31 +113,39 @@ static inline VirtIOPCIProxy *to_virtio_pci_proxy_fast(DeviceState *d)
 static void virtio_pci_notify(DeviceState *d, uint16_t vector)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy_fast(d);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+
     if (msix_enabled(&proxy->pci_dev))
         msix_notify(&proxy->pci_dev, vector);
     else
-        qemu_set_irq(proxy->pci_dev.irq[0], proxy->vdev->isr & 1);
+        qemu_set_irq(proxy->pci_dev.irq[0], vdev->isr & 1);
 }
 
 static void virtio_pci_save_config(DeviceState *d, QEMUFile *f)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+
     pci_device_save(&proxy->pci_dev, f);
     msix_save(&proxy->pci_dev, f);
     if (msix_present(&proxy->pci_dev))
-        qemu_put_be16(f, proxy->vdev->config_vector);
+        qemu_put_be16(f, vdev->config_vector);
 }
 
 static void virtio_pci_save_queue(DeviceState *d, int n, QEMUFile *f)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+
     if (msix_present(&proxy->pci_dev))
-        qemu_put_be16(f, virtio_queue_vector(proxy->vdev, n));
+        qemu_put_be16(f, virtio_queue_vector(vdev, n));
 }
 
 static int virtio_pci_load_config(DeviceState *d, QEMUFile *f)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+
     int ret;
     ret = pci_device_load(&proxy->pci_dev, f);
     if (ret) {
@@ -146,12 +154,12 @@ static int virtio_pci_load_config(DeviceState *d, QEMUFile *f)
     msix_unuse_all_vectors(&proxy->pci_dev);
     msix_load(&proxy->pci_dev, f);
     if (msix_present(&proxy->pci_dev)) {
-        qemu_get_be16s(f, &proxy->vdev->config_vector);
+        qemu_get_be16s(f, &vdev->config_vector);
     } else {
-        proxy->vdev->config_vector = VIRTIO_NO_VECTOR;
+        vdev->config_vector = VIRTIO_NO_VECTOR;
     }
-    if (proxy->vdev->config_vector != VIRTIO_NO_VECTOR) {
-        return msix_vector_use(&proxy->pci_dev, proxy->vdev->config_vector);
+    if (vdev->config_vector != VIRTIO_NO_VECTOR) {
+        return msix_vector_use(&proxy->pci_dev, vdev->config_vector);
     }
     return 0;
 }
@@ -159,13 +167,15 @@ static int virtio_pci_load_config(DeviceState *d, QEMUFile *f)
 static int virtio_pci_load_queue(DeviceState *d, int n, QEMUFile *f)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+
     uint16_t vector;
     if (msix_present(&proxy->pci_dev)) {
         qemu_get_be16s(f, &vector);
     } else {
         vector = VIRTIO_NO_VECTOR;
     }
-    virtio_queue_set_vector(proxy->vdev, n, vector);
+    virtio_queue_set_vector(vdev, n, vector);
     if (vector != VIRTIO_NO_VECTOR) {
         return msix_vector_use(&proxy->pci_dev, vector);
     }
@@ -175,7 +185,8 @@ static int virtio_pci_load_queue(DeviceState *d, int n, QEMUFile *f)
 static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
                                                  int n, bool assign, bool set_handler)
 {
-    VirtQueue *vq = virtio_get_queue(proxy->vdev, n);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    VirtQueue *vq = virtio_get_queue(vdev, n);
     EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
     int r = 0;
 
@@ -200,6 +211,7 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
 
 static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
 {
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     int n, r;
 
     if (!(proxy->flags & VIRTIO_PCI_FLAG_USE_IOEVENTFD) ||
@@ -209,7 +221,7 @@ static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
     }
 
     for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
-        if (!virtio_queue_get_num(proxy->vdev, n)) {
+        if (!virtio_queue_get_num(vdev, n)) {
             continue;
         }
 
@@ -223,7 +235,7 @@ static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
 
 assign_error:
     while (--n >= 0) {
-        if (!virtio_queue_get_num(proxy->vdev, n)) {
+        if (!virtio_queue_get_num(vdev, n)) {
             continue;
         }
 
@@ -236,6 +248,7 @@ assign_error:
 
 static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy *proxy)
 {
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     int r;
     int n;
 
@@ -244,7 +257,7 @@ static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy *proxy)
     }
 
     for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
-        if (!virtio_queue_get_num(proxy->vdev, n)) {
+        if (!virtio_queue_get_num(vdev, n)) {
             continue;
         }
 
@@ -257,7 +270,7 @@ static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy *proxy)
 static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
 {
     VirtIOPCIProxy *proxy = opaque;
-    VirtIODevice *vdev = proxy->vdev;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     hwaddr pa;
 
     switch (addr) {
@@ -272,7 +285,7 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
         pa = (hwaddr)val << VIRTIO_PCI_QUEUE_ADDR_SHIFT;
         if (pa == 0) {
             virtio_pci_stop_ioeventfd(proxy);
-            virtio_reset(proxy->vdev);
+            virtio_reset(vdev);
             msix_unuse_all_vectors(&proxy->pci_dev);
         }
         else
@@ -299,7 +312,7 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
         }
 
         if (vdev->status == 0) {
-            virtio_reset(proxy->vdev);
+            virtio_reset(vdev);
             msix_unuse_all_vectors(&proxy->pci_dev);
         }
 
@@ -335,7 +348,7 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
 
 static uint32_t virtio_ioport_read(VirtIOPCIProxy *proxy, uint32_t addr)
 {
-    VirtIODevice *vdev = proxy->vdev;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     uint32_t ret = 0xFFFFFFFF;
 
     switch (addr) {
@@ -381,6 +394,7 @@ static uint64_t virtio_pci_config_read(void *opaque, hwaddr addr,
                                        unsigned size)
 {
     VirtIOPCIProxy *proxy = opaque;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
     uint64_t val = 0;
     if (addr < config) {
@@ -390,16 +404,16 @@ static uint64_t virtio_pci_config_read(void *opaque, hwaddr addr,
 
     switch (size) {
     case 1:
-        val = virtio_config_readb(proxy->vdev, addr);
+        val = virtio_config_readb(vdev, addr);
         break;
     case 2:
-        val = virtio_config_readw(proxy->vdev, addr);
+        val = virtio_config_readw(vdev, addr);
         if (virtio_is_big_endian()) {
             val = bswap16(val);
         }
         break;
     case 4:
-        val = virtio_config_readl(proxy->vdev, addr);
+        val = virtio_config_readl(vdev, addr);
         if (virtio_is_big_endian()) {
             val = bswap32(val);
         }
@@ -413,6 +427,7 @@ static void virtio_pci_config_write(void *opaque, hwaddr addr,
 {
     VirtIOPCIProxy *proxy = opaque;
     uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     if (addr < config) {
         virtio_ioport_write(proxy, addr, val);
         return;
@@ -424,19 +439,19 @@ static void virtio_pci_config_write(void *opaque, hwaddr addr,
      */
     switch (size) {
     case 1:
-        virtio_config_writeb(proxy->vdev, addr, val);
+        virtio_config_writeb(vdev, addr, val);
         break;
     case 2:
         if (virtio_is_big_endian()) {
             val = bswap16(val);
         }
-        virtio_config_writew(proxy->vdev, addr, val);
+        virtio_config_writew(vdev, addr, val);
         break;
     case 4:
         if (virtio_is_big_endian()) {
             val = bswap32(val);
         }
-        virtio_config_writel(proxy->vdev, addr, val);
+        virtio_config_writel(vdev, addr, val);
         break;
     }
 }
@@ -455,6 +470,7 @@ static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
                                 uint32_t val, int len)
 {
     VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
 
     pci_default_write_config(pci_dev, address, val, len);
 
@@ -462,8 +478,7 @@ static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
         !(pci_dev->config[PCI_COMMAND] & PCI_COMMAND_MASTER) &&
         !(proxy->flags & VIRTIO_PCI_FLAG_BUS_MASTER_BUG)) {
         virtio_pci_stop_ioeventfd(proxy);
-        virtio_set_status(proxy->vdev,
-                          proxy->vdev->status & ~VIRTIO_CONFIG_S_DRIVER_OK);
+        virtio_set_status(vdev, vdev->status & ~VIRTIO_CONFIG_S_DRIVER_OK);
     }
 }
 
@@ -506,7 +521,8 @@ static int kvm_virtio_pci_irqfd_use(VirtIOPCIProxy *proxy,
                                  unsigned int vector)
 {
     VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector];
-    VirtQueue *vq = virtio_get_queue(proxy->vdev, queue_no);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    VirtQueue *vq = virtio_get_queue(vdev, queue_no);
     EventNotifier *n = virtio_queue_get_guest_notifier(vq);
     int ret;
     ret = kvm_irqchip_add_irqfd_notifier(kvm_state, n, NULL, irqfd->virq);
@@ -517,7 +533,8 @@ static void kvm_virtio_pci_irqfd_release(VirtIOPCIProxy *proxy,
                                       unsigned int queue_no,
                                       unsigned int vector)
 {
-    VirtQueue *vq = virtio_get_queue(proxy->vdev, queue_no);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    VirtQueue *vq = virtio_get_queue(vdev, queue_no);
     EventNotifier *n = virtio_queue_get_guest_notifier(vq);
     VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector];
     int ret;
@@ -529,7 +546,7 @@ static void kvm_virtio_pci_irqfd_release(VirtIOPCIProxy *proxy,
 static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
 {
     PCIDevice *dev = &proxy->pci_dev;
-    VirtIODevice *vdev = proxy->vdev;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
     unsigned int vector;
     int ret, queue_no;
@@ -578,7 +595,7 @@ undo:
 static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
 {
     PCIDevice *dev = &proxy->pci_dev;
-    VirtIODevice *vdev = proxy->vdev;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     unsigned int vector;
     int queue_no;
     VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
@@ -606,8 +623,9 @@ static int virtio_pci_vq_vector_unmask(VirtIOPCIProxy *proxy,
                                        unsigned int vector,
                                        MSIMessage msg)
 {
-    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(proxy->vdev);
-    VirtQueue *vq = virtio_get_queue(proxy->vdev, queue_no);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+    VirtQueue *vq = virtio_get_queue(vdev, queue_no);
     EventNotifier *n = virtio_queue_get_guest_notifier(vq);
     VirtIOIRQFD *irqfd;
     int ret = 0;
@@ -626,10 +644,10 @@ static int virtio_pci_vq_vector_unmask(VirtIOPCIProxy *proxy,
      * Otherwise, set it up now.
      */
     if (k->guest_notifier_mask) {
-        k->guest_notifier_mask(proxy->vdev, queue_no, false);
+        k->guest_notifier_mask(vdev, queue_no, false);
         /* Test after unmasking to avoid losing events. */
         if (k->guest_notifier_pending &&
-            k->guest_notifier_pending(proxy->vdev, queue_no)) {
+            k->guest_notifier_pending(vdev, queue_no)) {
             event_notifier_set(n);
         }
     } else {
@@ -642,13 +660,14 @@ static void virtio_pci_vq_vector_mask(VirtIOPCIProxy *proxy,
                                              unsigned int queue_no,
                                              unsigned int vector)
 {
-    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(proxy->vdev);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
 
     /* If guest supports masking, keep irqfd but mask it.
      * Otherwise, clean it up now.
      */ 
     if (k->guest_notifier_mask) {
-        k->guest_notifier_mask(proxy->vdev, queue_no, true);
+        k->guest_notifier_mask(vdev, queue_no, true);
     } else {
         kvm_virtio_pci_irqfd_release(proxy, queue_no, vector);
     }
@@ -658,7 +677,7 @@ static int virtio_pci_vector_unmask(PCIDevice *dev, unsigned vector,
                                     MSIMessage msg)
 {
     VirtIOPCIProxy *proxy = container_of(dev, VirtIOPCIProxy, pci_dev);
-    VirtIODevice *vdev = proxy->vdev;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     int ret, queue_no;
 
     for (queue_no = 0; queue_no < proxy->nvqs_with_notifiers; queue_no++) {
@@ -688,7 +707,7 @@ undo:
 static void virtio_pci_vector_mask(PCIDevice *dev, unsigned vector)
 {
     VirtIOPCIProxy *proxy = container_of(dev, VirtIOPCIProxy, pci_dev);
-    VirtIODevice *vdev = proxy->vdev;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     int queue_no;
 
     for (queue_no = 0; queue_no < proxy->nvqs_with_notifiers; queue_no++) {
@@ -707,7 +726,7 @@ static void virtio_pci_vector_poll(PCIDevice *dev,
                                    unsigned int vector_end)
 {
     VirtIOPCIProxy *proxy = container_of(dev, VirtIOPCIProxy, pci_dev);
-    VirtIODevice *vdev = proxy->vdev;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
     int queue_no;
     unsigned int vector;
@@ -739,8 +758,9 @@ static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
                                          bool with_irqfd)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
-    VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(proxy->vdev);
-    VirtQueue *vq = virtio_get_queue(proxy->vdev, n);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
+    VirtQueue *vq = virtio_get_queue(vdev, n);
     EventNotifier *notifier = virtio_queue_get_guest_notifier(vq);
 
     if (assign) {
@@ -755,7 +775,7 @@ static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
     }
 
     if (!msix_enabled(&proxy->pci_dev) && vdc->guest_notifier_mask) {
-        vdc->guest_notifier_mask(proxy->vdev, n, !assign);
+        vdc->guest_notifier_mask(vdev, n, !assign);
     }
 
     return 0;
@@ -770,7 +790,7 @@ static bool virtio_pci_query_guest_notifiers(DeviceState *d)
 static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
-    VirtIODevice *vdev = proxy->vdev;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
     int r, n;
     bool with_irqfd = msix_enabled(&proxy->pci_dev) &&
@@ -864,11 +884,12 @@ static int virtio_pci_set_host_notifier(DeviceState *d, int n, bool assign)
 static void virtio_pci_vmstate_change(DeviceState *d, bool running)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
 
     if (running) {
         /* Try to find out if the guest has bus master disabled, but is
            in ready state. Then we have a buggy guest OS. */
-        if ((proxy->vdev->status & VIRTIO_CONFIG_S_DRIVER_OK) &&
+        if ((vdev->status & VIRTIO_CONFIG_S_DRIVER_OK) &&
             !(proxy->pci_dev.config[PCI_COMMAND] & PCI_COMMAND_MASTER)) {
             proxy->flags |= VIRTIO_PCI_FLAG_BUS_MASTER_BUG;
         }
@@ -943,8 +964,6 @@ static void virtio_pci_device_plugged(DeviceState *d)
     uint8_t *config;
     uint32_t size;
 
-    proxy->vdev = virtio_bus_get_device(bus);
-
     config = proxy->pci_dev.config;
     if (proxy->class_code) {
         pci_config_set_class(config, proxy->class_code);
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index 917bcc5..dc332ae 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -82,7 +82,6 @@ typedef struct VirtioPCIClass {
 
 struct VirtIOPCIProxy {
     PCIDevice pci_dev;
-    VirtIODevice *vdev;
     MemoryRegion bar;
     uint32_t flags;
     uint32_t class_code;
-- 
1.8.3.1

[Qemu-devel] [PATCH 03/11] virtio-ccw: remove vdev field

[Paolo Bonzini]

The vdev field is complicated to synchronize.  Just access the
BusState's list of children.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/s390x/virtio-ccw.c | 80 ++++++++++++++++++++++++++++-----------------------
 1 file changed, 44 insertions(+), 36 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index cd67db5..9742a4c 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -57,9 +57,10 @@ static const TypeInfo virtual_css_bus_info = {
 VirtIODevice *virtio_ccw_get_vdev(SubchDev *sch)
 {
     VirtIODevice *vdev = NULL;
+    VirtioCcwDevice *dev = sch->driver_data;
 
-    if (sch->driver_data) {
-        vdev = ((VirtioCcwDevice *)sch->driver_data)->vdev;
+    if (dev) {
+        vdev = virtio_bus_get_device(&dev->bus);
     }
     return vdev;
 }
@@ -67,7 +68,8 @@ VirtIODevice *virtio_ccw_get_vdev(SubchDev *sch)
 static int virtio_ccw_set_guest2host_notifier(VirtioCcwDevice *dev, int n,
                                               bool assign, bool set_handler)
 {
-    VirtQueue *vq = virtio_get_queue(dev->vdev, n);
+    VirtIODevice *vdev = virtio_bus_get_device(&dev->bus);
+    VirtQueue *vq = virtio_get_queue(vdev, n);
     EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
     int r = 0;
     SubchDev *sch = dev->sch;
@@ -97,6 +99,7 @@ static int virtio_ccw_set_guest2host_notifier(VirtioCcwDevice *dev, int n,
 
 static void virtio_ccw_start_ioeventfd(VirtioCcwDevice *dev)
 {
+    VirtIODevice *vdev;
     int n, r;
 
     if (!(dev->flags & VIRTIO_CCW_FLAG_USE_IOEVENTFD) ||
@@ -104,8 +107,9 @@ static void virtio_ccw_start_ioeventfd(VirtioCcwDevice *dev)
         dev->ioeventfd_started) {
         return;
     }
+    vdev = virtio_bus_get_device(&dev->bus);
     for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
-        if (!virtio_queue_get_num(dev->vdev, n)) {
+        if (!virtio_queue_get_num(vdev, n)) {
             continue;
         }
         r = virtio_ccw_set_guest2host_notifier(dev, n, true, true);
@@ -118,7 +122,7 @@ static void virtio_ccw_start_ioeventfd(VirtioCcwDevice *dev)
 
   assign_error:
     while (--n >= 0) {
-        if (!virtio_queue_get_num(dev->vdev, n)) {
+        if (!virtio_queue_get_num(vdev, n)) {
             continue;
         }
         r = virtio_ccw_set_guest2host_notifier(dev, n, false, false);
@@ -132,13 +136,15 @@ static void virtio_ccw_start_ioeventfd(VirtioCcwDevice *dev)
 
 static void virtio_ccw_stop_ioeventfd(VirtioCcwDevice *dev)
 {
+    VirtIODevice *vdev;
     int n, r;
 
     if (!dev->ioeventfd_started) {
         return;
     }
+    vdev = virtio_bus_get_device(&dev->bus);
     for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
-        if (!virtio_queue_get_num(dev->vdev, n)) {
+        if (!virtio_queue_get_num(vdev, n)) {
             continue;
         }
         r = virtio_ccw_set_guest2host_notifier(dev, n, false, false);
@@ -189,7 +195,7 @@ typedef struct VirtioFeatDesc {
 static int virtio_ccw_set_vqs(SubchDev *sch, uint64_t addr, uint32_t align,
                               uint16_t index, uint16_t num)
 {
-    VirtioCcwDevice *dev = sch->driver_data;
+    VirtIODevice *vdev = virtio_ccw_get_vdev(sch);
 
     if (index > VIRTIO_PCI_QUEUE_MAX) {
         return -EINVAL;
@@ -200,23 +206,23 @@ static int virtio_ccw_set_vqs(SubchDev *sch, uint64_t addr, uint32_t align,
         return -EINVAL;
     }
 
-    if (!dev) {
+    if (!vdev) {
         return -EINVAL;
     }
 
-    virtio_queue_set_addr(dev->vdev, index, addr);
+    virtio_queue_set_addr(vdev, index, addr);
     if (!addr) {
-        virtio_queue_set_vector(dev->vdev, index, 0);
+        virtio_queue_set_vector(vdev, index, 0);
     } else {
         /* Fail if we don't have a big enough queue. */
         /* TODO: Add interface to handle vring.num changing */
-        if (virtio_queue_get_num(dev->vdev, index) > num) {
+        if (virtio_queue_get_num(vdev, index) > num) {
             return -EINVAL;
         }
-        virtio_queue_set_vector(dev->vdev, index, index);
+        virtio_queue_set_vector(vdev, index, index);
     }
     /* tell notify handler in case of config change */
-    dev->vdev->config_vector = VIRTIO_PCI_QUEUE_MAX;
+    vdev->config_vector = VIRTIO_PCI_QUEUE_MAX;
     return 0;
 }
 
@@ -230,6 +236,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
     hwaddr indicators;
     VqConfigBlock vq_config;
     VirtioCcwDevice *dev = sch->driver_data;
+    VirtIODevice *vdev = virtio_bus_get_device(&dev->bus);
     bool check_len;
     int len;
     hwaddr hw_len;
@@ -272,7 +279,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
         break;
     case CCW_CMD_VDEV_RESET:
         virtio_ccw_stop_ioeventfd(dev);
-        virtio_reset(dev->vdev);
+        virtio_reset(vdev);
         ret = 0;
         break;
     case CCW_CMD_READ_FEAT:
@@ -319,7 +326,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
             features.features = ldl_le_phys(ccw.cda);
             if (features.index < ARRAY_SIZE(dev->host_features)) {
                 virtio_bus_set_vdev_features(&dev->bus, features.features);
-                dev->vdev->guest_features = features.features;
+                vdev->guest_features = features.features;
             } else {
                 /*
                  * If the guest supports more feature bits, assert that it
@@ -337,30 +344,30 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
         break;
     case CCW_CMD_READ_CONF:
         if (check_len) {
-            if (ccw.count > dev->vdev->config_len) {
+            if (ccw.count > vdev->config_len) {
                 ret = -EINVAL;
                 break;
             }
         }
-        len = MIN(ccw.count, dev->vdev->config_len);
+        len = MIN(ccw.count, vdev->config_len);
         if (!ccw.cda) {
             ret = -EFAULT;
         } else {
-            virtio_bus_get_vdev_config(&dev->bus, dev->vdev->config);
+            virtio_bus_get_vdev_config(&dev->bus, vdev->config);
             /* XXX config space endianness */
-            cpu_physical_memory_write(ccw.cda, dev->vdev->config, len);
+            cpu_physical_memory_write(ccw.cda, vdev->config, len);
             sch->curr_status.scsw.count = ccw.count - len;
             ret = 0;
         }
         break;
     case CCW_CMD_WRITE_CONF:
         if (check_len) {
-            if (ccw.count > dev->vdev->config_len) {
+            if (ccw.count > vdev->config_len) {
                 ret = -EINVAL;
                 break;
             }
         }
-        len = MIN(ccw.count, dev->vdev->config_len);
+        len = MIN(ccw.count, vdev->config_len);
         hw_len = len;
         if (!ccw.cda) {
             ret = -EFAULT;
@@ -371,9 +378,9 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
             } else {
                 len = hw_len;
                 /* XXX config space endianness */
-                memcpy(dev->vdev->config, config, len);
+                memcpy(vdev->config, config, len);
                 cpu_physical_memory_unmap(config, hw_len, 0, hw_len);
-                virtio_bus_set_vdev_config(&dev->bus, dev->vdev->config);
+                virtio_bus_set_vdev_config(&dev->bus, vdev->config);
                 sch->curr_status.scsw.count = ccw.count - len;
                 ret = 0;
             }
@@ -397,9 +404,9 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
             if (!(status & VIRTIO_CONFIG_S_DRIVER_OK)) {
                 virtio_ccw_stop_ioeventfd(dev);
             }
-            virtio_set_status(dev->vdev, status);
-            if (dev->vdev->status == 0) {
-                virtio_reset(dev->vdev);
+            virtio_set_status(vdev, status);
+            if (vdev->status == 0) {
+                virtio_reset(vdev);
             }
             if (status & VIRTIO_CONFIG_S_DRIVER_OK) {
                 virtio_ccw_start_ioeventfd(dev);
@@ -463,7 +470,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
             ret = -EFAULT;
         } else {
             vq_config.index = lduw_phys(ccw.cda);
-            vq_config.num_max = virtio_queue_get_num(dev->vdev,
+            vq_config.num_max = virtio_queue_get_num(vdev,
                                                      vq_config.index);
             stw_phys(ccw.cda + sizeof(vq_config.index), vq_config.num_max);
             sch->curr_status.scsw.count = ccw.count - sizeof(vq_config);
@@ -495,7 +502,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
     sch->driver_data = dev;
     dev->sch = sch;
 
-    dev->vdev = vdev;
     dev->indicators = 0;
 
     /* Initialize subchannel structure. */
@@ -608,7 +614,7 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
     memset(&sch->id, 0, sizeof(SenseId));
     sch->id.reserved = 0xff;
     sch->id.cu_type = VIRTIO_CCW_CU_TYPE;
-    sch->id.cu_model = dev->vdev->device_id;
+    sch->id.cu_model = vdev->device_id;
 
     /* Only the first 32 feature bits are used. */
     dev->host_features[0] = virtio_bus_get_vdev_features(&dev->bus,
@@ -892,9 +898,10 @@ static unsigned virtio_ccw_get_features(DeviceState *d)
 static void virtio_ccw_reset(DeviceState *d)
 {
     VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
+    VirtIODevice *vdev = virtio_bus_get_device(&dev->bus);
 
     virtio_ccw_stop_ioeventfd(dev);
-    virtio_reset(dev->vdev);
+    virtio_reset(vdev);
     css_reset_sch(dev->sch);
     dev->indicators = 0;
     dev->indicators2 = 0;
@@ -934,9 +941,10 @@ static int virtio_ccw_set_host_notifier(DeviceState *d, int n, bool assign)
 static int virtio_ccw_set_guest_notifier(VirtioCcwDevice *dev, int n,
                                          bool assign, bool with_irqfd)
 {
-    VirtQueue *vq = virtio_get_queue(dev->vdev, n);
+    VirtIODevice *vdev = virtio_bus_get_device(&dev->bus);
+    VirtQueue *vq = virtio_get_queue(vdev, n);
     EventNotifier *notifier = virtio_queue_get_guest_notifier(vq);
-    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(dev->vdev);
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
 
     if (assign) {
         int r = event_notifier_init(notifier, 0);
@@ -952,16 +960,16 @@ static int virtio_ccw_set_guest_notifier(VirtioCcwDevice *dev, int n,
          * land in qemu (and only the irq fd) in this code.
          */
         if (k->guest_notifier_mask) {
-            k->guest_notifier_mask(dev->vdev, n, false);
+            k->guest_notifier_mask(vdev, n, false);
         }
         /* get lost events and re-inject */
         if (k->guest_notifier_pending &&
-            k->guest_notifier_pending(dev->vdev, n)) {
+            k->guest_notifier_pending(vdev, n)) {
             event_notifier_set(notifier);
         }
     } else {
         if (k->guest_notifier_mask) {
-            k->guest_notifier_mask(dev->vdev, n, true);
+            k->guest_notifier_mask(vdev, n, true);
         }
         virtio_queue_set_guest_notifier_fd_handler(vq, false, with_irqfd);
         event_notifier_cleanup(notifier);
@@ -973,7 +981,7 @@ static int virtio_ccw_set_guest_notifiers(DeviceState *d, int nvqs,
                                           bool assigned)
 {
     VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
-    VirtIODevice *vdev = dev->vdev;
+    VirtIODevice *vdev = virtio_bus_get_device(&dev->bus);
     int r, n;
 
     for (n = 0; n < nvqs; n++) {
-- 
1.8.3.1

[Qemu-devel] [PATCH 04/11] virtio-bus: cleanup plug/unplug interface

[Paolo Bonzini]

Right now we have these pairs:

- virtio_bus_plug_device/virtio_bus_destroy_device.  The first
  takes a VirtIODevice, the second takes a VirtioBusState

- device_plugged/device_unplug callbacks in the VirtioBusClass
  (here it's just the naming that is inconsistent)

- virtio_bus_destroy_device is not called by anyone (and since
  it calls qdev_free, it would be called by the proxies---but
  then the callback is useless since the proxies can do whatever
  they want before calling virtio_bus_destroy_device)

And there is a k->init but no k->exit, hence virtio_device_exit is
overwritten by subclasses (except virtio-9p).  This cleans it up by:

- renaming the device_unplug callback to device_unplugged

- renaming virtio_bus_plug_device to virtio_bus_device_plugged,
  matching the callback name

- renaming virtio_bus_destroy_device to virtio_bus_device_unplugged,
  removing the qdev_free, making it take a VirtIODevice and calling it
  from virtio_device_exit

- adding a k->exit callback

virtio_device_exit is still overwritten, the next patches will fix that.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio/virtio-bus.c         | 18 +++++++++---------
 hw/virtio/virtio.c             |  7 ++++++-
 include/hw/virtio/virtio-bus.h |  6 +++---
 include/hw/virtio/virtio.h     |  1 +
 4 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index 669ce38..7aed6a4 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -37,8 +37,8 @@ do { printf("virtio_bus: " fmt , ## __VA_ARGS__); } while (0)
 #define DPRINTF(fmt, ...) do { } while (0)
 #endif
 
-/* Plug the VirtIODevice */
-int virtio_bus_plug_device(VirtIODevice *vdev)
+/* A VirtIODevice is being plugged */
+int virtio_bus_device_plugged(VirtIODevice *vdev)
 {
     DeviceState *qdev = DEVICE(vdev);
     BusState *qbus = BUS(qdev_get_parent_bus(qdev));
@@ -64,20 +64,20 @@ void virtio_bus_reset(VirtioBusState *bus)
     }
 }
 
-/* Destroy the VirtIODevice */
-void virtio_bus_destroy_device(VirtioBusState *bus)
+/* A VirtIODevice is being unplugged */
+void virtio_bus_device_unplugged(VirtIODevice *vdev)
 {
-    BusState *qbus = BUS(bus);
+    DeviceState *qdev = DEVICE(vdev);
+    BusState *qbus = BUS(qdev_get_parent_bus(qdev));
+    VirtioBusState *bus = VIRTIO_BUS(qbus);
     VirtioBusClass *klass = VIRTIO_BUS_GET_CLASS(bus);
-    VirtIODevice *vdev = virtio_bus_get_device(bus);
 
     DPRINTF("%s: remove device.\n", qbus->name);
 
     if (vdev != NULL) {
-        if (klass->device_unplug != NULL) {
-            klass->device_unplug(qbus->parent);
+        if (klass->device_unplugged != NULL) {
+            klass->device_unplugged(qbus->parent);
         }
-        qdev_free(DEVICE(vdev));
     }
 }
 
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 2f1e73b..965b2c0 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1158,14 +1158,19 @@ static int virtio_device_init(DeviceState *qdev)
     if (k->init(vdev) < 0) {
         return -1;
     }
-    virtio_bus_plug_device(vdev);
+    virtio_bus_device_plugged(vdev);
     return 0;
 }
 
 static int virtio_device_exit(DeviceState *qdev)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(qdev);
 
+    virtio_bus_device_unplugged(vdev);
+    if (k->exit) {
+        k->exit(vdev);
+    }
     if (vdev->bus_name) {
         g_free(vdev->bus_name);
         vdev->bus_name = NULL;
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index 48afb7c..0d59315 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -61,7 +61,7 @@ typedef struct VirtioBusClass {
      * transport independent exit function.
      * This is called by virtio-bus just before the device is unplugged.
      */
-    void (*device_unplug)(DeviceState *d);
+    void (*device_unplugged)(DeviceState *d);
     /*
      * Does the transport have variable vring alignment?
      * (ie can it ever call virtio_queue_set_align()?)
@@ -74,9 +74,9 @@ struct VirtioBusState {
     BusState parent_obj;
 };
 
-int virtio_bus_plug_device(VirtIODevice *vdev);
+int virtio_bus_device_plugged(VirtIODevice *vdev);
 void virtio_bus_reset(VirtioBusState *bus);
-void virtio_bus_destroy_device(VirtioBusState *bus);
+void virtio_bus_device_unplugged(VirtIODevice *bus);
 /* Get the device id of the plugged device. */
 uint16_t virtio_bus_get_vdev_id(VirtioBusState *bus);
 /* Get the config_len field of the plugged device. */
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index a90522d..59756c2 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -127,6 +127,7 @@ typedef struct VirtioDeviceClass {
     /* This is what a VirtioDevice must implement */
     DeviceClass parent;
     int (*init)(VirtIODevice *vdev);
+    void (*exit)(VirtIODevice *vdev);
     uint32_t (*get_features)(VirtIODevice *vdev, uint32_t requested_features);
     uint32_t (*bad_features)(VirtIODevice *vdev);
     void (*set_features)(VirtIODevice *vdev, uint32_t val);
-- 
1.8.3.1

[Qemu-devel] [PATCH 05/11] virtio-blk: switch exit callback to VirtioDeviceClass

[Paolo Bonzini]

This ensures hot-unplug is handled properly by the proxy.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/block/virtio-blk.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index e2f55cc..5f99bfa 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -709,20 +709,18 @@ static int virtio_blk_device_init(VirtIODevice *vdev)
     return 0;
 }
 
-static int virtio_blk_device_exit(DeviceState *dev)
+static void virtio_blk_device_exit(VirtIODevice *vdev)
 {
-    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-    VirtIOBlock *s = VIRTIO_BLK(dev);
+    VirtIOBlock *s = VIRTIO_BLK(vdev);
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
     remove_migration_state_change_notifier(&s->migration_state_notifier);
     virtio_blk_data_plane_destroy(s->dataplane);
     s->dataplane = NULL;
 #endif
     qemu_del_vm_change_state_handler(s->change);
-    unregister_savevm(dev, "virtio-blk", s);
+    unregister_savevm(DEVICE(vdev), "virtio-blk", s);
     blockdev_mark_auto_del(s->bs);
     virtio_cleanup(vdev);
-    return 0;
 }
 
 static Property virtio_blk_properties[] = {
@@ -734,10 +732,10 @@ static void virtio_blk_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_blk_device_exit;
     dc->props = virtio_blk_properties;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
     vdc->init = virtio_blk_device_init;
+    vdc->exit = virtio_blk_device_exit;
     vdc->get_config = virtio_blk_update_config;
     vdc->set_config = virtio_blk_set_config;
     vdc->get_features = virtio_blk_get_features;
-- 
1.8.3.1

[Qemu-devel] [PATCH 06/11] virtio-serial: switch exit callback to VirtioDeviceClass

[Paolo Bonzini]

This ensures hot-unplug is handled properly by the proxy.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/char/virtio-serial-bus.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 703f026..a7ede90 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -987,12 +987,11 @@ static const TypeInfo virtio_serial_port_type_info = {
     .class_init = virtio_serial_port_class_init,
 };
 
-static int virtio_serial_device_exit(DeviceState *dev)
+static void virtio_serial_device_exit(VirtIODevice *vdev)
 {
-    VirtIOSerial *vser = VIRTIO_SERIAL(dev);
-    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIOSerial *vser = VIRTIO_SERIAL(vdev);
 
-    unregister_savevm(dev, "virtio-console", vser);
+    unregister_savevm(DEVICE(vdev), "virtio-console", vser);
 
     g_free(vser->ivqs);
     g_free(vser->ovqs);
@@ -1004,7 +1003,6 @@ static int virtio_serial_device_exit(DeviceState *dev)
         g_free(vser->post_load);
     }
     virtio_cleanup(vdev);
-    return 0;
 }
 
 static Property virtio_serial_properties[] = {
@@ -1016,10 +1014,10 @@ static void virtio_serial_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_serial_device_exit;
     dc->props = virtio_serial_properties;
     set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
     vdc->init = virtio_serial_device_init;
+    vdc->exit = virtio_serial_device_exit;
     vdc->get_features = get_features;
     vdc->get_config = get_config;
     vdc->set_config = set_config;
-- 
1.8.3.1

[Qemu-devel] [PATCH 07/11] virtio-net: switch exit callback to VirtioDeviceClass

[Paolo Bonzini]

This ensures hot-unplug is handled properly by the proxy.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/net/virtio-net.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index dd41008..40f9f61 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1568,16 +1568,15 @@ static int virtio_net_device_init(VirtIODevice *vdev)
     return 0;
 }
 
-static int virtio_net_device_exit(DeviceState *qdev)
+static void virtio_net_device_exit(VirtIODevice *vdev)
 {
-    VirtIONet *n = VIRTIO_NET(qdev);
-    VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
+    VirtIONet *n = VIRTIO_NET(vdev);
     int i;
 
     /* This will stop vhost backend if appropriate. */
     virtio_net_set_status(vdev, 0);
 
-    unregister_savevm(qdev, "virtio-net", n);
+    unregister_savevm(DEVICE(vdev), "virtio-net", n);
 
     if (n->netclient_name) {
         g_free(n->netclient_name);
@@ -1608,8 +1607,6 @@ static int virtio_net_device_exit(DeviceState *qdev)
     g_free(n->vqs);
     qemu_del_nic(n->nic);
     virtio_cleanup(vdev);
-
-    return 0;
 }
 
 static void virtio_net_instance_init(Object *obj)
@@ -1636,10 +1633,10 @@ static void virtio_net_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_net_device_exit;
     dc->props = virtio_net_properties;
     set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
     vdc->init = virtio_net_device_init;
+    vdc->exit = virtio_net_device_exit;
     vdc->get_config = virtio_net_get_config;
     vdc->set_config = virtio_net_set_config;
     vdc->get_features = virtio_net_get_features;
-- 
1.8.3.1

[Qemu-devel] [PATCH 08/11] virtio-scsi: switch exit callback to VirtioDeviceClass

[Paolo Bonzini]

This ensures hot-unplug is handled properly by the proxy.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/vhost-scsi.c            | 11 +++++------
 hw/scsi/virtio-scsi.c           | 15 +++++++--------
 include/hw/virtio/virtio-scsi.h |  2 +-
 3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index 9e770fb..5e3cc61 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -240,11 +240,10 @@ static int vhost_scsi_init(VirtIODevice *vdev)
     return 0;
 }
 
-static int vhost_scsi_exit(DeviceState *qdev)
+static void vhost_scsi_exit(VirtIODevice *vdev)
 {
-    VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
-    VHostSCSI *s = VHOST_SCSI(qdev);
-    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(qdev);
+    VHostSCSI *s = VHOST_SCSI(vdev);
+    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
 
     migrate_del_blocker(s->migration_blocker);
     error_free(s->migration_blocker);
@@ -253,7 +252,7 @@ static int vhost_scsi_exit(DeviceState *qdev)
     vhost_scsi_set_status(vdev, 0);
 
     g_free(s->dev.vqs);
-    return virtio_scsi_common_exit(vs);
+    virtio_scsi_common_exit(vs);
 }
 
 static Property vhost_scsi_properties[] = {
@@ -265,10 +264,10 @@ static void vhost_scsi_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = vhost_scsi_exit;
     dc->props = vhost_scsi_properties;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
     vdc->init = vhost_scsi_init;
+    vdc->exit = vhost_scsi_exit;
     vdc->get_features = vhost_scsi_get_features;
     vdc->set_config = vhost_scsi_set_config;
     vdc->set_status = vhost_scsi_set_status;
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 3bd690d..5512ded 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -644,22 +644,21 @@ static int virtio_scsi_device_init(VirtIODevice *vdev)
     return 0;
 }
 
-int virtio_scsi_common_exit(VirtIOSCSICommon *vs)
+void virtio_scsi_common_exit(VirtIOSCSICommon *vs)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(vs);
 
     g_free(vs->cmd_vqs);
     virtio_cleanup(vdev);
-    return 0;
 }
 
-static int virtio_scsi_device_exit(DeviceState *qdev)
+static void virtio_scsi_device_exit(VirtIODevice *vdev)
 {
-    VirtIOSCSI *s = VIRTIO_SCSI(qdev);
-    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(qdev);
+    VirtIOSCSI *s = VIRTIO_SCSI(vdev);
+    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
 
-    unregister_savevm(qdev, "virtio-scsi", s);
-    return virtio_scsi_common_exit(vs);
+    unregister_savevm(DEVICE(vdev), "virtio-scsi", s);
+    virtio_scsi_common_exit(vs);
 }
 
 static Property virtio_scsi_properties[] = {
@@ -680,10 +679,10 @@ static void virtio_scsi_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_scsi_device_exit;
     dc->props = virtio_scsi_properties;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
     vdc->init = virtio_scsi_device_init;
+    vdc->exit = virtio_scsi_device_exit;
     vdc->set_config = virtio_scsi_set_config;
     vdc->get_features = virtio_scsi_get_features;
     vdc->reset = virtio_scsi_reset;
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index 9a98540..206c61d 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -187,6 +187,6 @@ typedef struct {
                                             VIRTIO_SCSI_F_CHANGE, true)
 
 int virtio_scsi_common_init(VirtIOSCSICommon *vs);
-int virtio_scsi_common_exit(VirtIOSCSICommon *vs);
+void virtio_scsi_common_exit(VirtIOSCSICommon *vs);
 
 #endif /* _QEMU_VIRTIO_SCSI_H */
-- 
1.8.3.1

[Qemu-devel] [PATCH 09/11] virtio-balloon: switch exit callback to VirtioDeviceClass

[Paolo Bonzini]

This ensures hot-unplug is handled properly by the proxy.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio/virtio-balloon.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 9504877..d7a392d 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -370,16 +370,14 @@ static int virtio_balloon_device_init(VirtIODevice *vdev)
     return 0;
 }
 
-static int virtio_balloon_device_exit(DeviceState *qdev)
+static void virtio_balloon_device_exit(VirtIODevice *vdev)
 {
-    VirtIOBalloon *s = VIRTIO_BALLOON(qdev);
-    VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
+    VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
 
     balloon_stats_destroy_timer(s);
     qemu_remove_balloon_handler(s);
-    unregister_savevm(qdev, "virtio-balloon", s);
+    unregister_savevm(DEVICE(vdev), "virtio-balloon", s);
     virtio_cleanup(vdev);
-    return 0;
 }
 
 static Property virtio_balloon_properties[] = {
@@ -390,10 +388,10 @@ static void virtio_balloon_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_balloon_device_exit;
     dc->props = virtio_balloon_properties;
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
     vdc->init = virtio_balloon_device_init;
+    vdc->exit = virtio_balloon_device_exit;
     vdc->get_config = virtio_balloon_get_config;
     vdc->set_config = virtio_balloon_set_config;
     vdc->get_features = virtio_balloon_get_features;
-- 
1.8.3.1

[Qemu-devel] [PATCH 10/11] virtio-rng: switch exit callback to VirtioDeviceClass

[Paolo Bonzini]

This ensures hot-unplug is handled properly by the proxy.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio/virtio-rng.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index 314e393..18cbfac 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -184,16 +184,14 @@ static int virtio_rng_device_init(VirtIODevice *vdev)
     return 0;
 }
 
-static int virtio_rng_device_exit(DeviceState *qdev)
+static void virtio_rng_device_exit(VirtIODevice *vdev)
 {
-    VirtIORNG *vrng = VIRTIO_RNG(qdev);
-    VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
+    VirtIORNG *vrng = VIRTIO_RNG(vdev);
 
     timer_del(vrng->rate_limit_timer);
     timer_free(vrng->rate_limit_timer);
-    unregister_savevm(qdev, "virtio-rng", vrng);
+    unregister_savevm(DEVICE(vdev), "virtio-rng", vrng);
     virtio_cleanup(vdev);
-    return 0;
 }
 
 static Property virtio_rng_properties[] = {
@@ -205,10 +203,10 @@ static void virtio_rng_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_rng_device_exit;
     dc->props = virtio_rng_properties;
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
     vdc->init = virtio_rng_device_init;
+    vdc->exit = virtio_rng_device_exit;
     vdc->get_features = get_features;
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 11/11] virtio-pci: add device_unplugged callback

[Paolo Bonzini]

This fixes a crash in hot-unplug of virtio-pci devices behind a PCIe
switch.  The crash happens because the ioeventfd is still set whent the
child is destroyed (destruction happens in postorder).  Then the proxy
tries to unset to ioeventfd, but the virtqueue structure that holds the
EventNotifier has been trashed in the meanwhile.  kvm_set_ioeventfd_pio
does not expect failure and aborts.

The fix is simply to move parts of uninitialization to a new
device_unplugged callback, which is called before the child is destroyed.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio/virtio-pci.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index c8d8b99..1a179ea 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1001,6 +1001,15 @@ static void virtio_pci_device_plugged(DeviceState *d)
                                                       proxy->host_features);
 }
 
+static void virtio_pci_device_unplugged(DeviceState *d)
+{
+    PCIDevice *pci_dev = PCI_DEVICE(d);
+    VirtIOPCIProxy *proxy = VIRTIO_PCI(d);
+
+    virtio_pci_stop_ioeventfd(proxy);
+    msix_uninit_exclusive_bar(pci_dev);
+}
+
 static int virtio_pci_init(PCIDevice *pci_dev)
 {
     VirtIOPCIProxy *dev = VIRTIO_PCI(pci_dev);
@@ -1015,9 +1024,7 @@ static int virtio_pci_init(PCIDevice *pci_dev)
 static void virtio_pci_exit(PCIDevice *pci_dev)
 {
     VirtIOPCIProxy *proxy = VIRTIO_PCI(pci_dev);
-    virtio_pci_stop_ioeventfd(proxy);
     memory_region_destroy(&proxy->bar);
-    msix_uninit_exclusive_bar(pci_dev);
 }
 
 static void virtio_pci_reset(DeviceState *qdev)
@@ -1552,6 +1559,7 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
     k->set_guest_notifiers = virtio_pci_set_guest_notifiers;
     k->vmstate_change = virtio_pci_vmstate_change;
     k->device_plugged = virtio_pci_device_plugged;
+    k->device_unplugged = virtio_pci_device_unplugged;
 }
 
 static const TypeInfo virtio_pci_bus_info = {
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 00/11] virtio: cleanup and fix hot-unplug

[Michael S. Tsirkin]

On Fri, Sep 20, 2013 at 04:57:49PM +0200, Paolo Bonzini wrote:
> This series fixes hot-unplug of virtio devices, which can crash due to
> dangling pointer accesses.

Could you please describe the sequence of steps that makes
qemu crash?

> The current implementation supports guest-initiated hot-unplug via the
> virtio_bus_destroy_device function, but not hot-unplugging the virtio
> device by virtue of unplugging its parent container device.
> 
> The problem is that the callback for the bus implementation to cleanup
> is placed in the wrong place; it is in virtio_bus_destroy_device, which
> should be called by the bus, instead of being somewhere in device code.
> We need to have the callback in device code (for example in dc->exit),
> so that we invoke it on every unplug action, no matter who starts it.
> 
> Thus, the series cleans up plugging and unplugging of virtio devices
> so that it does not need any help from the bus (patches 1-4).  It then
> stops the virtio devices' overriding of dc->exit, moving their cleanup
> code to the new exit callback in VirtioDeviceClass (patches 5-10).
> Finally, patch 11 can make virtio-pci implement the device_unplugged
> callback.
> 
> Something similar is probably needed in virtio-ccw too.  However,
> virtio-ccw needs more surgery because it does not include a device_plugged
> callback either, so I did not touch it.
> 
> Paolo Bonzini (11):
>   virtio-bus: remove vdev field
>   virtio-pci: remove vdev field
>   virtio-ccw: remove vdev field
>   virtio-bus: cleanup plug/unplug interface
>   virtio-blk: switch exit callback to VirtioDeviceClass
>   virtio-serial: switch exit callback to VirtioDeviceClass
>   virtio-net: switch exit callback to VirtioDeviceClass
>   virtio-scsi: switch exit callback to VirtioDeviceClass
>   virtio-balloon: switch exit callback to VirtioDeviceClass
>   virtio-rng: switch exit callback to VirtioDeviceClass
>   virtio-pci: add device_unplugged callback
> 
>  hw/block/virtio-blk.c           |  10 ++--
>  hw/char/virtio-serial-bus.c     |  10 ++--
>  hw/net/virtio-net.c             |  11 ++--
>  hw/s390x/virtio-ccw.c           |  80 +++++++++++++++------------
>  hw/s390x/virtio-ccw.h           |   1 -
>  hw/scsi/vhost-scsi.c            |  11 ++--
>  hw/scsi/virtio-scsi.c           |  15 +++--
>  hw/virtio/virtio-balloon.c      |  10 ++--
>  hw/virtio/virtio-bus.c          |  81 +++++++++++++++------------
>  hw/virtio/virtio-mmio.c         |   9 +--
>  hw/virtio/virtio-pci.c          | 119 ++++++++++++++++++++++++----------------
>  hw/virtio/virtio-pci.h          |   1 -
>  hw/virtio/virtio-rng.c          |  10 ++--
>  hw/virtio/virtio.c              |   7 ++-
>  include/hw/virtio/virtio-bus.h  |  22 +++++---
>  include/hw/virtio/virtio-scsi.h |   2 +-
>  include/hw/virtio/virtio.h      |   1 +
>  17 files changed, 223 insertions(+), 177 deletions(-)
> 
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PATCH 00/11] virtio: cleanup and fix hot-unplug

[Paolo Bonzini]

Il 21/09/2013 21:17, Michael S. Tsirkin ha scritto:
> On Fri, Sep 20, 2013 at 04:57:49PM +0200, Paolo Bonzini wrote:
>> This series fixes hot-unplug of virtio devices, which can crash due to
>> dangling pointer accesses.
> 
> Could you please describe the sequence of steps that makes
> qemu crash?

See patch 11.  I didn't find out why it fails with PCIe but not PCI,
probably a difference in how malloc reuses freed blocks.

Paolo

Re: [Qemu-devel] [PATCH 00/11] virtio: cleanup and fix hot-unplug

[Paolo Bonzini]

Il 22/09/2013 10:08, Paolo Bonzini ha scritto:
> Il 21/09/2013 21:17, Michael S. Tsirkin ha scritto:
>> On Fri, Sep 20, 2013 at 04:57:49PM +0200, Paolo Bonzini wrote:
>>> This series fixes hot-unplug of virtio devices, which can crash due to
>>> dangling pointer accesses.
>>
>> Could you please describe the sequence of steps that makes
>> qemu crash?
> 
> See patch 11.  I didn't find out why it fails with PCIe but not PCI,
> probably a difference in how malloc reuses freed blocks.

Ping?

Paolo

Re: [Qemu-devel] [PATCH 00/11] virtio: cleanup and fix hot-unplug

[Paolo Bonzini]

Il 20/09/2013 16:57, Paolo Bonzini ha scritto:
> This series fixes hot-unplug of virtio devices, which can crash due to
> dangling pointer accesses.
> 
> The current implementation supports guest-initiated hot-unplug via the
> virtio_bus_destroy_device function, but not hot-unplugging the virtio
> device by virtue of unplugging its parent container device.
> 
> The problem is that the callback for the bus implementation to cleanup
> is placed in the wrong place; it is in virtio_bus_destroy_device, which
> should be called by the bus, instead of being somewhere in device code.
> We need to have the callback in device code (for example in dc->exit),
> so that we invoke it on every unplug action, no matter who starts it.
> 
> Thus, the series cleans up plugging and unplugging of virtio devices
> so that it does not need any help from the bus (patches 1-4).  It then
> stops the virtio devices' overriding of dc->exit, moving their cleanup
> code to the new exit callback in VirtioDeviceClass (patches 5-10).
> Finally, patch 11 can make virtio-pci implement the device_unplugged
> callback.
> 
> Something similar is probably needed in virtio-ccw too.  However,
> virtio-ccw needs more surgery because it does not include a device_plugged
> callback either, so I did not touch it.

Michael, I prepared a rebase of
http://permalink.gmane.org/gmane.comp.emulators.qemu/225985 on top of
these patches and sent it to Andreas.  My understanding is that he will
send them to qemu-devel.

Let me know if you want to handle these patches yourself, or I can send
a pull request for both directly with your Acked-bys.

As to review, I think it can be usefully split as follows:

- 4-10 for Andreas

- 3 for Alex

- 2 11 for you

- 1 for either you or Andreas

Paolo

> 
> Paolo Bonzini (11):
>   virtio-bus: remove vdev field
>   virtio-pci: remove vdev field
>   virtio-ccw: remove vdev field
>   virtio-bus: cleanup plug/unplug interface
>   virtio-blk: switch exit callback to VirtioDeviceClass
>   virtio-serial: switch exit callback to VirtioDeviceClass
>   virtio-net: switch exit callback to VirtioDeviceClass
>   virtio-scsi: switch exit callback to VirtioDeviceClass
>   virtio-balloon: switch exit callback to VirtioDeviceClass
>   virtio-rng: switch exit callback to VirtioDeviceClass
>   virtio-pci: add device_unplugged callback
> 
>  hw/block/virtio-blk.c           |  10 ++--
>  hw/char/virtio-serial-bus.c     |  10 ++--
>  hw/net/virtio-net.c             |  11 ++--
>  hw/s390x/virtio-ccw.c           |  80 +++++++++++++++------------
>  hw/s390x/virtio-ccw.h           |   1 -
>  hw/scsi/vhost-scsi.c            |  11 ++--
>  hw/scsi/virtio-scsi.c           |  15 +++--
>  hw/virtio/virtio-balloon.c      |  10 ++--
>  hw/virtio/virtio-bus.c          |  81 +++++++++++++++------------
>  hw/virtio/virtio-mmio.c         |   9 +--
>  hw/virtio/virtio-pci.c          | 119 ++++++++++++++++++++++++----------------
>  hw/virtio/virtio-pci.h          |   1 -
>  hw/virtio/virtio-rng.c          |  10 ++--
>  hw/virtio/virtio.c              |   7 ++-
>  include/hw/virtio/virtio-bus.h  |  22 +++++---
>  include/hw/virtio/virtio-scsi.h |   2 +-
>  include/hw/virtio/virtio.h      |   1 +
>  17 files changed, 223 insertions(+), 177 deletions(-)
> 

[Qemu-devel] [PATCH 12/11] virtio-pci: avoid extra pointer dereferences on fast path

[Paolo Bonzini]

We do not need to access vdev on the MSI-X fast path of virtio_pci_notify.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
	It is possible to get rid of BusChild altogether, but even then
	this would be one less pointer dereference, and it's a simpler
	patch.  So let's do this instead for 1.7.

 hw/virtio/virtio-pci.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 1a179ea..a191c24 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -113,12 +113,13 @@ static inline VirtIOPCIProxy *to_virtio_pci_proxy_fast(DeviceState *d)
 static void virtio_pci_notify(DeviceState *d, uint16_t vector)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy_fast(d);
-    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
 
     if (msix_enabled(&proxy->pci_dev))
         msix_notify(&proxy->pci_dev, vector);
-    else
+    else {
+        VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
         qemu_set_irq(proxy->pci_dev.irq[0], vdev->isr & 1);
+    }
 }
 
 static void virtio_pci_save_config(DeviceState *d, QEMUFile *f)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 12/11] virtio-pci: avoid extra pointer dereferences on fast path

[Frederic Konrad]

Hi Paolo,

This looks ok, but I don't find the branch where it applies?

Seems it's already fixed on master.

Thanks,
Fred

On 14/10/2013 18:23, Paolo Bonzini wrote:
> We do not need to access vdev on the MSI-X fast path of virtio_pci_notify.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> 	It is possible to get rid of BusChild altogether, but even then
> 	this would be one less pointer dereference, and it's a simpler
> 	patch.  So let's do this instead for 1.7.
>
>   hw/virtio/virtio-pci.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 1a179ea..a191c24 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -113,12 +113,13 @@ static inline VirtIOPCIProxy *to_virtio_pci_proxy_fast(DeviceState *d)
>   static void virtio_pci_notify(DeviceState *d, uint16_t vector)
>   {
>       VirtIOPCIProxy *proxy = to_virtio_pci_proxy_fast(d);
> -    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
>   
>       if (msix_enabled(&proxy->pci_dev))
>           msix_notify(&proxy->pci_dev, vector);
> -    else
> +    else {
> +        VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
>           qemu_set_irq(proxy->pci_dev.irq[0], vdev->isr & 1);
> +    }
>   }
>   
>   static void virtio_pci_save_config(DeviceState *d, QEMUFile *f)

Re: [Qemu-devel] [PATCH 12/11] virtio-pci: avoid extra pointer dereferences on fast path

[Paolo Bonzini]

Il 15/10/2013 14:13, Frederic Konrad ha scritto:
> Hi Paolo,
> 
> This looks ok, but I don't find the branch where it applies?
> 
> Seems it's already fixed on master.

It's on top of the previous 11 patches.

Paolo

Re: [Qemu-devel] [PATCH 00/11] virtio: cleanup and fix hot-unplug

[Michael S. Tsirkin]

On Tue, Oct 08, 2013 at 07:02:44PM +0200, Paolo Bonzini wrote:
> Il 20/09/2013 16:57, Paolo Bonzini ha scritto:
> > This series fixes hot-unplug of virtio devices, which can crash due to
> > dangling pointer accesses.
> > 
> > The current implementation supports guest-initiated hot-unplug via the
> > virtio_bus_destroy_device function, but not hot-unplugging the virtio
> > device by virtue of unplugging its parent container device.
> > 
> > The problem is that the callback for the bus implementation to cleanup
> > is placed in the wrong place; it is in virtio_bus_destroy_device, which
> > should be called by the bus, instead of being somewhere in device code.
> > We need to have the callback in device code (for example in dc->exit),
> > so that we invoke it on every unplug action, no matter who starts it.
> > 
> > Thus, the series cleans up plugging and unplugging of virtio devices
> > so that it does not need any help from the bus (patches 1-4).  It then
> > stops the virtio devices' overriding of dc->exit, moving their cleanup
> > code to the new exit callback in VirtioDeviceClass (patches 5-10).
> > Finally, patch 11 can make virtio-pci implement the device_unplugged
> > callback.
> > 
> > Something similar is probably needed in virtio-ccw too.  However,
> > virtio-ccw needs more surgery because it does not include a device_plugged
> > callback either, so I did not touch it.
> 
> Michael, I prepared a rebase of
> http://permalink.gmane.org/gmane.comp.emulators.qemu/225985 on top of
> these patches and sent it to Andreas.  My understanding is that he will
> send them to qemu-devel.
> 
> Let me know if you want to handle these patches yourself, or I can send
> a pull request for both directly with your Acked-bys.
> 
> As to review, I think it can be usefully split as follows:
> 
> - 4-10 for Andreas
> 
> - 3 for Alex
> 
> - 2 11 for you
> 
> - 1 for either you or Andreas
> 
> Paolo

OK so I can merge this but need Acks from Alex and Andreas, right?

> > 
> > Paolo Bonzini (11):
> >   virtio-bus: remove vdev field
> >   virtio-pci: remove vdev field
> >   virtio-ccw: remove vdev field
> >   virtio-bus: cleanup plug/unplug interface
> >   virtio-blk: switch exit callback to VirtioDeviceClass
> >   virtio-serial: switch exit callback to VirtioDeviceClass
> >   virtio-net: switch exit callback to VirtioDeviceClass
> >   virtio-scsi: switch exit callback to VirtioDeviceClass
> >   virtio-balloon: switch exit callback to VirtioDeviceClass
> >   virtio-rng: switch exit callback to VirtioDeviceClass
> >   virtio-pci: add device_unplugged callback
> > 
> >  hw/block/virtio-blk.c           |  10 ++--
> >  hw/char/virtio-serial-bus.c     |  10 ++--
> >  hw/net/virtio-net.c             |  11 ++--
> >  hw/s390x/virtio-ccw.c           |  80 +++++++++++++++------------
> >  hw/s390x/virtio-ccw.h           |   1 -
> >  hw/scsi/vhost-scsi.c            |  11 ++--
> >  hw/scsi/virtio-scsi.c           |  15 +++--
> >  hw/virtio/virtio-balloon.c      |  10 ++--
> >  hw/virtio/virtio-bus.c          |  81 +++++++++++++++------------
> >  hw/virtio/virtio-mmio.c         |   9 +--
> >  hw/virtio/virtio-pci.c          | 119 ++++++++++++++++++++++++----------------
> >  hw/virtio/virtio-pci.h          |   1 -
> >  hw/virtio/virtio-rng.c          |  10 ++--
> >  hw/virtio/virtio.c              |   7 ++-
> >  include/hw/virtio/virtio-bus.h  |  22 +++++---
> >  include/hw/virtio/virtio-scsi.h |   2 +-
> >  include/hw/virtio/virtio.h      |   1 +
> >  17 files changed, 223 insertions(+), 177 deletions(-)
> > 

[Qemu-devel] [PATCH v3 00/12] virtio: cleanup and fix hot-unplug

[Paolo Bonzini]

This series fixes hot-unplug of virtio devices, which can crash due to
dangling pointer accesses.

The current implementation supports guest-initiated hot-unplug via the
virtio_bus_destroy_device function, but not hot-unplugging the virtio
device by virtue of unplugging its parent container device.

The problem is that the callback for the bus implementation to cleanup
is placed in the wrong place; it is in virtio_bus_destroy_device, which
should be called by the bus, instead of being somewhere in device code.
We need to have the callback in device code (for example in dc->exit),
so that we invoke it on every unplug action, no matter who starts it.

Thus, the series cleans up plugging and unplugging of virtio devices
so that it does not need any help from the bus (patches 2-5).  It then
stops the virtio devices' overriding of dc->exit, moving their cleanup
code to the new exit callback in VirtioDeviceClass (patches 6-11).
Finally, patch 12 can make virtio-pci implement the device_unplugged
callback.

A similar dangling-pointer bug is exposed by this change in virtio-ccw.
Patch 1 avoids this; it is kept at the beginning to ensure bisectability.

v2->v3: fix to s390 patch; added Reviewed-by and Cced patch 1 to
	qemu-stable.

v1->v2: remove useless pointer chasing in virtio_pci_notify, add
	patch 1 to fix breakage reported by Cornelia.

Paolo Bonzini (12):
  virtio-ccw: move virtio_ccw_stop_ioeventfd to virtio_ccw_busdev_unplug
  virtio-bus: remove vdev field
  virtio-ccw: remove vdev field
  virtio-pci: remove vdev field
  virtio-bus: cleanup plug/unplug interface
  virtio-blk: switch exit callback to VirtioDeviceClass
  virtio-serial: switch exit callback to VirtioDeviceClass
  virtio-net: switch exit callback to VirtioDeviceClass
  virtio-scsi: switch exit callback to VirtioDeviceClass
  virtio-balloon: switch exit callback to VirtioDeviceClass
  virtio-rng: switch exit callback to VirtioDeviceClass
  virtio-pci: add device_unplugged callback

 hw/block/virtio-blk.c           |  10 ++--
 hw/char/virtio-serial-bus.c     |  10 ++--
 hw/net/virtio-net.c             |  11 ++--
 hw/s390x/virtio-ccw.c           |  83 +++++++++++++++------------
 hw/s390x/virtio-ccw.h           |   1 -
 hw/scsi/vhost-scsi.c            |  11 ++--
 hw/scsi/virtio-scsi.c           |  15 +++--
 hw/virtio/virtio-balloon.c      |  10 ++--
 hw/virtio/virtio-bus.c          |  81 ++++++++++++++------------
 hw/virtio/virtio-mmio.c         |   9 +--
 hw/virtio/virtio-pci.c          | 122 ++++++++++++++++++++++++----------------
 hw/virtio/virtio-pci.h          |   1 -
 hw/virtio/virtio-rng.c          |  10 ++--
 hw/virtio/virtio.c              |   7 ++-
 include/hw/virtio/virtio-bus.h  |  22 +++++---
 include/hw/virtio/virtio-scsi.h |   2 +-
 include/hw/virtio/virtio.h      |   1 +
 17 files changed, 227 insertions(+), 179 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v3 01/12] virtio-ccw: move virtio_ccw_stop_ioeventfd to virtio_ccw_busdev_unplug

[Paolo Bonzini]

Similar to the PCI bug that prompted these patches, virtio-ccw will
segfault after the reworking of hotplug/hot-unplug.  Prepare for
this by moving virtio_ccw_stop_ioeventfd to before the freeing
of the proxy device.

A better place for this could be the device_unplugged callback
for the virtio-ccw bus.  However, we do not yet have a callback
that works: this patch avoids the problem while leaving the tree
bisectable.

Cc: qemu-stable@nongnu.org
Reported-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Suggested-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/s390x/virtio-ccw.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index cd67db5..a6accdf 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -631,7 +631,6 @@ static int virtio_ccw_exit(VirtioCcwDevice *dev)
 {
     SubchDev *sch = dev->sch;
 
-    virtio_ccw_stop_ioeventfd(dev);
     if (sch) {
         css_subch_assign(sch->cssid, sch->ssid, sch->schid, sch->devno, NULL);
         g_free(sch);
@@ -1228,6 +1227,8 @@ static int virtio_ccw_busdev_unplug(DeviceState *dev)
     VirtioCcwDevice *_dev = (VirtioCcwDevice *)dev;
     SubchDev *sch = _dev->sch;
 
+    virtio_ccw_stop_ioeventfd(_dev);
+
     /*
      * We should arrive here only for device_del, since we don't support
      * direct hot(un)plug of channels, but only through virtio.
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 02/12] virtio-bus: remove vdev field

[Paolo Bonzini]

The vdev field is complicated to synchronize.  Just access the
BusState's list of children.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio/virtio-bus.c         | 65 +++++++++++++++++++++++++-----------------
 hw/virtio/virtio-mmio.c        |  9 +++---
 hw/virtio/virtio-pci.c         |  2 +-
 include/hw/virtio/virtio-bus.h | 16 ++++++++---
 4 files changed, 57 insertions(+), 35 deletions(-)

diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index 6849a01..10dfa2c 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -46,8 +46,6 @@ int virtio_bus_plug_device(VirtIODevice *vdev)
     VirtioBusClass *klass = VIRTIO_BUS_GET_CLASS(bus);
     DPRINTF("%s: plug device.\n", qbus->name);
 
-    bus->vdev = vdev;
-
     if (klass->device_plugged != NULL) {
         klass->device_plugged(qbus->parent);
     }
@@ -58,9 +56,11 @@ int virtio_bus_plug_device(VirtIODevice *vdev)
 /* Reset the virtio_bus */
 void virtio_bus_reset(VirtioBusState *bus)
 {
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
+
     DPRINTF("%s: reset device.\n", qbus->name);
-    if (bus->vdev != NULL) {
-        virtio_reset(bus->vdev);
+    if (vdev != NULL) {
+        virtio_reset(vdev);
     }
 }
 
@@ -70,63 +70,72 @@ void virtio_bus_destroy_device(VirtioBusState *bus)
     DeviceState *qdev;
     BusState *qbus = BUS(bus);
     VirtioBusClass *klass = VIRTIO_BUS_GET_CLASS(bus);
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
+
     DPRINTF("%s: remove device.\n", qbus->name);
 
-    if (bus->vdev != NULL) {
+    if (vdev != NULL) {
         if (klass->device_unplug != NULL) {
             klass->device_unplug(qbus->parent);
         }
-        qdev = DEVICE(bus->vdev);
+        qdev = DEVICE(vdev);
         qdev_free(qdev);
-        bus->vdev = NULL;
     }
 }
 
 /* Get the device id of the plugged device. */
 uint16_t virtio_bus_get_vdev_id(VirtioBusState *bus)
 {
-    assert(bus->vdev != NULL);
-    return bus->vdev->device_id;
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
+    assert(vdev != NULL);
+    return vdev->device_id;
 }
 
 /* Get the config_len field of the plugged device. */
 size_t virtio_bus_get_vdev_config_len(VirtioBusState *bus)
 {
-    assert(bus->vdev != NULL);
-    return bus->vdev->config_len;
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
+    assert(vdev != NULL);
+    return vdev->config_len;
 }
 
 /* Get the features of the plugged device. */
 uint32_t virtio_bus_get_vdev_features(VirtioBusState *bus,
                                     uint32_t requested_features)
 {
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
     VirtioDeviceClass *k;
-    assert(bus->vdev != NULL);
-    k = VIRTIO_DEVICE_GET_CLASS(bus->vdev);
+
+    assert(vdev != NULL);
+    k = VIRTIO_DEVICE_GET_CLASS(vdev);
     assert(k->get_features != NULL);
-    return k->get_features(bus->vdev, requested_features);
+    return k->get_features(vdev, requested_features);
 }
 
 /* Set the features of the plugged device. */
 void virtio_bus_set_vdev_features(VirtioBusState *bus,
                                       uint32_t requested_features)
 {
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
     VirtioDeviceClass *k;
-    assert(bus->vdev != NULL);
-    k = VIRTIO_DEVICE_GET_CLASS(bus->vdev);
+
+    assert(vdev != NULL);
+    k = VIRTIO_DEVICE_GET_CLASS(vdev);
     if (k->set_features != NULL) {
-        k->set_features(bus->vdev, requested_features);
+        k->set_features(vdev, requested_features);
     }
 }
 
 /* Get bad features of the plugged device. */
 uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus)
 {
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
     VirtioDeviceClass *k;
-    assert(bus->vdev != NULL);
-    k = VIRTIO_DEVICE_GET_CLASS(bus->vdev);
+
+    assert(vdev != NULL);
+    k = VIRTIO_DEVICE_GET_CLASS(vdev);
     if (k->bad_features != NULL) {
-        return k->bad_features(bus->vdev);
+        return k->bad_features(vdev);
     } else {
         return 0;
     }
@@ -135,22 +144,26 @@ uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus)
 /* Get config of the plugged device. */
 void virtio_bus_get_vdev_config(VirtioBusState *bus, uint8_t *config)
 {
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
     VirtioDeviceClass *k;
-    assert(bus->vdev != NULL);
-    k = VIRTIO_DEVICE_GET_CLASS(bus->vdev);
+
+    assert(vdev != NULL);
+    k = VIRTIO_DEVICE_GET_CLASS(vdev);
     if (k->get_config != NULL) {
-        k->get_config(bus->vdev, config);
+        k->get_config(vdev, config);
     }
 }
 
 /* Set config of the plugged device. */
 void virtio_bus_set_vdev_config(VirtioBusState *bus, uint8_t *config)
 {
+    VirtIODevice *vdev = virtio_bus_get_device(bus);
     VirtioDeviceClass *k;
-    assert(bus->vdev != NULL);
-    k = VIRTIO_DEVICE_GET_CLASS(bus->vdev);
+
+    assert(vdev != NULL);
+    k = VIRTIO_DEVICE_GET_CLASS(vdev);
     if (k->set_config != NULL) {
-        k->set_config(bus->vdev, config);
+        k->set_config(vdev, config);
     }
 }
 
diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 29cf284..8829eb0 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -95,7 +95,7 @@ static void virtio_mmio_bus_new(VirtioBusState *bus, size_t bus_size,
 static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size)
 {
     VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
-    VirtIODevice *vdev = proxy->bus.vdev;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
 
     DPRINTF("virtio_mmio_read offset 0x%x\n", (int)offset);
 
@@ -185,7 +185,7 @@ static void virtio_mmio_write(void *opaque, hwaddr offset, uint64_t value,
                               unsigned size)
 {
     VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
-    VirtIODevice *vdev = proxy->bus.vdev;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
 
     DPRINTF("virtio_mmio_write offset 0x%x value 0x%" PRIx64 "\n",
             (int)offset, value);
@@ -298,12 +298,13 @@ static const MemoryRegionOps virtio_mem_ops = {
 static void virtio_mmio_update_irq(DeviceState *opaque, uint16_t vector)
 {
     VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     int level;
 
-    if (!proxy->bus.vdev) {
+    if (!vdev) {
         return;
     }
-    level = (proxy->bus.vdev->isr != 0);
+    level = (vdev->isr != 0);
     DPRINTF("virtio_mmio setting IRQ %d\n", level);
     qemu_set_irq(proxy->irq, level);
 }
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 4825802..07708a3 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -943,7 +943,7 @@ static void virtio_pci_device_plugged(DeviceState *d)
     uint8_t *config;
     uint32_t size;
 
-    proxy->vdev = bus->vdev;
+    proxy->vdev = virtio_bus_get_device(bus);
 
     config = proxy->pci_dev.config;
     if (proxy->class_code) {
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index 9217f85..ba0f86a 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -72,10 +72,6 @@ typedef struct VirtioBusClass {
 
 struct VirtioBusState {
     BusState parent_obj;
-    /*
-     * Only one VirtIODevice can be plugged on the bus.
-     */
-    VirtIODevice *vdev;
 };
 
 int virtio_bus_plug_device(VirtIODevice *vdev);
@@ -98,4 +94,16 @@ void virtio_bus_get_vdev_config(VirtioBusState *bus, uint8_t *config);
 /* Set config of the plugged device. */
 void virtio_bus_set_vdev_config(VirtioBusState *bus, uint8_t *config);
 
+static inline VirtIODevice *virtio_bus_get_device(VirtioBusState *bus)
+{
+    BusState *qbus = &bus->parent_obj;
+    BusChild *kid = QTAILQ_FIRST(&qbus->children);
+    DeviceState *qdev = kid ? kid->child : NULL;
+
+    /* This is used on the data path, the cast is guaranteed
+     * to succeed by the qdev machinery.
+     */
+    return (VirtIODevice *)qdev;
+}
+
 #endif /* VIRTIO_BUS_H */
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 03/12] virtio-ccw: remove vdev field

[Paolo Bonzini]

The vdev field is complicated to synchronize.  Just access the
BusState's list of children.

Cc: qemu-stable@nongnu.org
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/s390x/virtio-ccw.c | 80 ++++++++++++++++++++++++++++-----------------------
 hw/s390x/virtio-ccw.h |  1 -
 2 files changed, 44 insertions(+), 37 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index a6accdf..89758c8 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -57,9 +57,10 @@ static const TypeInfo virtual_css_bus_info = {
 VirtIODevice *virtio_ccw_get_vdev(SubchDev *sch)
 {
     VirtIODevice *vdev = NULL;
+    VirtioCcwDevice *dev = sch->driver_data;
 
-    if (sch->driver_data) {
-        vdev = ((VirtioCcwDevice *)sch->driver_data)->vdev;
+    if (dev) {
+        vdev = virtio_bus_get_device(&dev->bus);
     }
     return vdev;
 }
@@ -67,7 +68,8 @@ VirtIODevice *virtio_ccw_get_vdev(SubchDev *sch)
 static int virtio_ccw_set_guest2host_notifier(VirtioCcwDevice *dev, int n,
                                               bool assign, bool set_handler)
 {
-    VirtQueue *vq = virtio_get_queue(dev->vdev, n);
+    VirtIODevice *vdev = virtio_bus_get_device(&dev->bus);
+    VirtQueue *vq = virtio_get_queue(vdev, n);
     EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
     int r = 0;
     SubchDev *sch = dev->sch;
@@ -97,6 +99,7 @@ static int virtio_ccw_set_guest2host_notifier(VirtioCcwDevice *dev, int n,
 
 static void virtio_ccw_start_ioeventfd(VirtioCcwDevice *dev)
 {
+    VirtIODevice *vdev;
     int n, r;
 
     if (!(dev->flags & VIRTIO_CCW_FLAG_USE_IOEVENTFD) ||
@@ -104,8 +107,9 @@ static void virtio_ccw_start_ioeventfd(VirtioCcwDevice *dev)
         dev->ioeventfd_started) {
         return;
     }
+    vdev = virtio_bus_get_device(&dev->bus);
     for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
-        if (!virtio_queue_get_num(dev->vdev, n)) {
+        if (!virtio_queue_get_num(vdev, n)) {
             continue;
         }
         r = virtio_ccw_set_guest2host_notifier(dev, n, true, true);
@@ -118,7 +122,7 @@ static void virtio_ccw_start_ioeventfd(VirtioCcwDevice *dev)
 
   assign_error:
     while (--n >= 0) {
-        if (!virtio_queue_get_num(dev->vdev, n)) {
+        if (!virtio_queue_get_num(vdev, n)) {
             continue;
         }
         r = virtio_ccw_set_guest2host_notifier(dev, n, false, false);
@@ -132,13 +136,15 @@ static void virtio_ccw_start_ioeventfd(VirtioCcwDevice *dev)
 
 static void virtio_ccw_stop_ioeventfd(VirtioCcwDevice *dev)
 {
+    VirtIODevice *vdev;
     int n, r;
 
     if (!dev->ioeventfd_started) {
         return;
     }
+    vdev = virtio_bus_get_device(&dev->bus);
     for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
-        if (!virtio_queue_get_num(dev->vdev, n)) {
+        if (!virtio_queue_get_num(vdev, n)) {
             continue;
         }
         r = virtio_ccw_set_guest2host_notifier(dev, n, false, false);
@@ -189,7 +195,7 @@ typedef struct VirtioFeatDesc {
 static int virtio_ccw_set_vqs(SubchDev *sch, uint64_t addr, uint32_t align,
                               uint16_t index, uint16_t num)
 {
-    VirtioCcwDevice *dev = sch->driver_data;
+    VirtIODevice *vdev = virtio_ccw_get_vdev(sch);
 
     if (index > VIRTIO_PCI_QUEUE_MAX) {
         return -EINVAL;
@@ -200,23 +206,23 @@ static int virtio_ccw_set_vqs(SubchDev *sch, uint64_t addr, uint32_t align,
         return -EINVAL;
     }
 
-    if (!dev) {
+    if (!vdev) {
         return -EINVAL;
     }
 
-    virtio_queue_set_addr(dev->vdev, index, addr);
+    virtio_queue_set_addr(vdev, index, addr);
     if (!addr) {
-        virtio_queue_set_vector(dev->vdev, index, 0);
+        virtio_queue_set_vector(vdev, index, 0);
     } else {
         /* Fail if we don't have a big enough queue. */
         /* TODO: Add interface to handle vring.num changing */
-        if (virtio_queue_get_num(dev->vdev, index) > num) {
+        if (virtio_queue_get_num(vdev, index) > num) {
             return -EINVAL;
         }
-        virtio_queue_set_vector(dev->vdev, index, index);
+        virtio_queue_set_vector(vdev, index, index);
     }
     /* tell notify handler in case of config change */
-    dev->vdev->config_vector = VIRTIO_PCI_QUEUE_MAX;
+    vdev->config_vector = VIRTIO_PCI_QUEUE_MAX;
     return 0;
 }
 
@@ -230,6 +236,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
     hwaddr indicators;
     VqConfigBlock vq_config;
     VirtioCcwDevice *dev = sch->driver_data;
+    VirtIODevice *vdev = virtio_ccw_get_vdev(sch);
     bool check_len;
     int len;
     hwaddr hw_len;
@@ -272,7 +279,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
         break;
     case CCW_CMD_VDEV_RESET:
         virtio_ccw_stop_ioeventfd(dev);
-        virtio_reset(dev->vdev);
+        virtio_reset(vdev);
         ret = 0;
         break;
     case CCW_CMD_READ_FEAT:
@@ -319,7 +326,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
             features.features = ldl_le_phys(ccw.cda);
             if (features.index < ARRAY_SIZE(dev->host_features)) {
                 virtio_bus_set_vdev_features(&dev->bus, features.features);
-                dev->vdev->guest_features = features.features;
+                vdev->guest_features = features.features;
             } else {
                 /*
                  * If the guest supports more feature bits, assert that it
@@ -337,30 +344,30 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
         break;
     case CCW_CMD_READ_CONF:
         if (check_len) {
-            if (ccw.count > dev->vdev->config_len) {
+            if (ccw.count > vdev->config_len) {
                 ret = -EINVAL;
                 break;
             }
         }
-        len = MIN(ccw.count, dev->vdev->config_len);
+        len = MIN(ccw.count, vdev->config_len);
         if (!ccw.cda) {
             ret = -EFAULT;
         } else {
-            virtio_bus_get_vdev_config(&dev->bus, dev->vdev->config);
+            virtio_bus_get_vdev_config(&dev->bus, vdev->config);
             /* XXX config space endianness */
-            cpu_physical_memory_write(ccw.cda, dev->vdev->config, len);
+            cpu_physical_memory_write(ccw.cda, vdev->config, len);
             sch->curr_status.scsw.count = ccw.count - len;
             ret = 0;
         }
         break;
     case CCW_CMD_WRITE_CONF:
         if (check_len) {
-            if (ccw.count > dev->vdev->config_len) {
+            if (ccw.count > vdev->config_len) {
                 ret = -EINVAL;
                 break;
             }
         }
-        len = MIN(ccw.count, dev->vdev->config_len);
+        len = MIN(ccw.count, vdev->config_len);
         hw_len = len;
         if (!ccw.cda) {
             ret = -EFAULT;
@@ -371,9 +378,9 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
             } else {
                 len = hw_len;
                 /* XXX config space endianness */
-                memcpy(dev->vdev->config, config, len);
+                memcpy(vdev->config, config, len);
                 cpu_physical_memory_unmap(config, hw_len, 0, hw_len);
-                virtio_bus_set_vdev_config(&dev->bus, dev->vdev->config);
+                virtio_bus_set_vdev_config(&dev->bus, vdev->config);
                 sch->curr_status.scsw.count = ccw.count - len;
                 ret = 0;
             }
@@ -397,9 +404,9 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
             if (!(status & VIRTIO_CONFIG_S_DRIVER_OK)) {
                 virtio_ccw_stop_ioeventfd(dev);
             }
-            virtio_set_status(dev->vdev, status);
-            if (dev->vdev->status == 0) {
-                virtio_reset(dev->vdev);
+            virtio_set_status(vdev, status);
+            if (vdev->status == 0) {
+                virtio_reset(vdev);
             }
             if (status & VIRTIO_CONFIG_S_DRIVER_OK) {
                 virtio_ccw_start_ioeventfd(dev);
@@ -463,7 +470,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
             ret = -EFAULT;
         } else {
             vq_config.index = lduw_phys(ccw.cda);
-            vq_config.num_max = virtio_queue_get_num(dev->vdev,
+            vq_config.num_max = virtio_queue_get_num(vdev,
                                                      vq_config.index);
             stw_phys(ccw.cda + sizeof(vq_config.index), vq_config.num_max);
             sch->curr_status.scsw.count = ccw.count - sizeof(vq_config);
@@ -495,7 +502,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
     sch->driver_data = dev;
     dev->sch = sch;
 
-    dev->vdev = vdev;
     dev->indicators = 0;
 
     /* Initialize subchannel structure. */
@@ -608,7 +614,7 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
     memset(&sch->id, 0, sizeof(SenseId));
     sch->id.reserved = 0xff;
     sch->id.cu_type = VIRTIO_CCW_CU_TYPE;
-    sch->id.cu_model = dev->vdev->device_id;
+    sch->id.cu_model = vdev->device_id;
 
     /* Only the first 32 feature bits are used. */
     dev->host_features[0] = virtio_bus_get_vdev_features(&dev->bus,
@@ -891,9 +897,10 @@ static unsigned virtio_ccw_get_features(DeviceState *d)
 static void virtio_ccw_reset(DeviceState *d)
 {
     VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
+    VirtIODevice *vdev = virtio_bus_get_device(&dev->bus);
 
     virtio_ccw_stop_ioeventfd(dev);
-    virtio_reset(dev->vdev);
+    virtio_reset(vdev);
     css_reset_sch(dev->sch);
     dev->indicators = 0;
     dev->indicators2 = 0;
@@ -933,9 +940,10 @@ static int virtio_ccw_set_host_notifier(DeviceState *d, int n, bool assign)
 static int virtio_ccw_set_guest_notifier(VirtioCcwDevice *dev, int n,
                                          bool assign, bool with_irqfd)
 {
-    VirtQueue *vq = virtio_get_queue(dev->vdev, n);
+    VirtIODevice *vdev = virtio_bus_get_device(&dev->bus);
+    VirtQueue *vq = virtio_get_queue(vdev, n);
     EventNotifier *notifier = virtio_queue_get_guest_notifier(vq);
-    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(dev->vdev);
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
 
     if (assign) {
         int r = event_notifier_init(notifier, 0);
@@ -951,16 +959,16 @@ static int virtio_ccw_set_guest_notifier(VirtioCcwDevice *dev, int n,
          * land in qemu (and only the irq fd) in this code.
          */
         if (k->guest_notifier_mask) {
-            k->guest_notifier_mask(dev->vdev, n, false);
+            k->guest_notifier_mask(vdev, n, false);
         }
         /* get lost events and re-inject */
         if (k->guest_notifier_pending &&
-            k->guest_notifier_pending(dev->vdev, n)) {
+            k->guest_notifier_pending(vdev, n)) {
             event_notifier_set(notifier);
         }
     } else {
         if (k->guest_notifier_mask) {
-            k->guest_notifier_mask(dev->vdev, n, true);
+            k->guest_notifier_mask(vdev, n, true);
         }
         virtio_queue_set_guest_notifier_fd_handler(vq, false, with_irqfd);
         event_notifier_cleanup(notifier);
@@ -972,7 +980,7 @@ static int virtio_ccw_set_guest_notifiers(DeviceState *d, int nvqs,
                                           bool assigned)
 {
     VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
-    VirtIODevice *vdev = dev->vdev;
+    VirtIODevice *vdev = virtio_bus_get_device(&dev->bus);
     int r, n;
 
     for (n = 0; n < nvqs; n++) {
diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
index 96d6f5d..00932c7 100644
--- a/hw/s390x/virtio-ccw.h
+++ b/hw/s390x/virtio-ccw.h
@@ -77,7 +77,6 @@ typedef struct VirtIOCCWDeviceClass {
 struct VirtioCcwDevice {
     DeviceState parent_obj;
     SubchDev *sch;
-    VirtIODevice *vdev;
     char *bus_id;
     uint32_t host_features[VIRTIO_CCW_FEATURE_SIZE];
     VirtioBusState bus;
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 04/12] virtio-pci: remove vdev field

[Paolo Bonzini]

The vdev field is complicated to synchronize.  Just access the
BusState's list of children.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio/virtio-pci.c | 110 +++++++++++++++++++++++++++++--------------------
 hw/virtio/virtio-pci.h |   1 -
 2 files changed, 65 insertions(+), 46 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 07708a3..89af295 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -113,31 +113,40 @@ static inline VirtIOPCIProxy *to_virtio_pci_proxy_fast(DeviceState *d)
 static void virtio_pci_notify(DeviceState *d, uint16_t vector)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy_fast(d);
+
     if (msix_enabled(&proxy->pci_dev))
         msix_notify(&proxy->pci_dev, vector);
-    else
-        qemu_set_irq(proxy->pci_dev.irq[0], proxy->vdev->isr & 1);
+    else {
+        VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+        qemu_set_irq(proxy->pci_dev.irq[0], vdev->isr & 1);
+    }
 }
 
 static void virtio_pci_save_config(DeviceState *d, QEMUFile *f)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+
     pci_device_save(&proxy->pci_dev, f);
     msix_save(&proxy->pci_dev, f);
     if (msix_present(&proxy->pci_dev))
-        qemu_put_be16(f, proxy->vdev->config_vector);
+        qemu_put_be16(f, vdev->config_vector);
 }
 
 static void virtio_pci_save_queue(DeviceState *d, int n, QEMUFile *f)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+
     if (msix_present(&proxy->pci_dev))
-        qemu_put_be16(f, virtio_queue_vector(proxy->vdev, n));
+        qemu_put_be16(f, virtio_queue_vector(vdev, n));
 }
 
 static int virtio_pci_load_config(DeviceState *d, QEMUFile *f)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+
     int ret;
     ret = pci_device_load(&proxy->pci_dev, f);
     if (ret) {
@@ -146,12 +155,12 @@ static int virtio_pci_load_config(DeviceState *d, QEMUFile *f)
     msix_unuse_all_vectors(&proxy->pci_dev);
     msix_load(&proxy->pci_dev, f);
     if (msix_present(&proxy->pci_dev)) {
-        qemu_get_be16s(f, &proxy->vdev->config_vector);
+        qemu_get_be16s(f, &vdev->config_vector);
     } else {
-        proxy->vdev->config_vector = VIRTIO_NO_VECTOR;
+        vdev->config_vector = VIRTIO_NO_VECTOR;
     }
-    if (proxy->vdev->config_vector != VIRTIO_NO_VECTOR) {
-        return msix_vector_use(&proxy->pci_dev, proxy->vdev->config_vector);
+    if (vdev->config_vector != VIRTIO_NO_VECTOR) {
+        return msix_vector_use(&proxy->pci_dev, vdev->config_vector);
     }
     return 0;
 }
@@ -159,13 +168,15 @@ static int virtio_pci_load_config(DeviceState *d, QEMUFile *f)
 static int virtio_pci_load_queue(DeviceState *d, int n, QEMUFile *f)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+
     uint16_t vector;
     if (msix_present(&proxy->pci_dev)) {
         qemu_get_be16s(f, &vector);
     } else {
         vector = VIRTIO_NO_VECTOR;
     }
-    virtio_queue_set_vector(proxy->vdev, n, vector);
+    virtio_queue_set_vector(vdev, n, vector);
     if (vector != VIRTIO_NO_VECTOR) {
         return msix_vector_use(&proxy->pci_dev, vector);
     }
@@ -175,7 +186,8 @@ static int virtio_pci_load_queue(DeviceState *d, int n, QEMUFile *f)
 static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
                                                  int n, bool assign, bool set_handler)
 {
-    VirtQueue *vq = virtio_get_queue(proxy->vdev, n);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    VirtQueue *vq = virtio_get_queue(vdev, n);
     EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
     int r = 0;
 
@@ -200,6 +212,7 @@ static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
 
 static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
 {
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     int n, r;
 
     if (!(proxy->flags & VIRTIO_PCI_FLAG_USE_IOEVENTFD) ||
@@ -209,7 +222,7 @@ static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
     }
 
     for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
-        if (!virtio_queue_get_num(proxy->vdev, n)) {
+        if (!virtio_queue_get_num(vdev, n)) {
             continue;
         }
 
@@ -223,7 +236,7 @@ static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
 
 assign_error:
     while (--n >= 0) {
-        if (!virtio_queue_get_num(proxy->vdev, n)) {
+        if (!virtio_queue_get_num(vdev, n)) {
             continue;
         }
 
@@ -236,6 +249,7 @@ assign_error:
 
 static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy *proxy)
 {
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     int r;
     int n;
 
@@ -244,7 +258,7 @@ static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy *proxy)
     }
 
     for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
-        if (!virtio_queue_get_num(proxy->vdev, n)) {
+        if (!virtio_queue_get_num(vdev, n)) {
             continue;
         }
 
@@ -257,7 +271,7 @@ static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy *proxy)
 static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
 {
     VirtIOPCIProxy *proxy = opaque;
-    VirtIODevice *vdev = proxy->vdev;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     hwaddr pa;
 
     switch (addr) {
@@ -272,7 +286,7 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
         pa = (hwaddr)val << VIRTIO_PCI_QUEUE_ADDR_SHIFT;
         if (pa == 0) {
             virtio_pci_stop_ioeventfd(proxy);
-            virtio_reset(proxy->vdev);
+            virtio_reset(vdev);
             msix_unuse_all_vectors(&proxy->pci_dev);
         }
         else
@@ -299,7 +313,7 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
         }
 
         if (vdev->status == 0) {
-            virtio_reset(proxy->vdev);
+            virtio_reset(vdev);
             msix_unuse_all_vectors(&proxy->pci_dev);
         }
 
@@ -335,7 +349,7 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
 
 static uint32_t virtio_ioport_read(VirtIOPCIProxy *proxy, uint32_t addr)
 {
-    VirtIODevice *vdev = proxy->vdev;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     uint32_t ret = 0xFFFFFFFF;
 
     switch (addr) {
@@ -381,6 +395,7 @@ static uint64_t virtio_pci_config_read(void *opaque, hwaddr addr,
                                        unsigned size)
 {
     VirtIOPCIProxy *proxy = opaque;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
     uint64_t val = 0;
     if (addr < config) {
@@ -390,16 +405,16 @@ static uint64_t virtio_pci_config_read(void *opaque, hwaddr addr,
 
     switch (size) {
     case 1:
-        val = virtio_config_readb(proxy->vdev, addr);
+        val = virtio_config_readb(vdev, addr);
         break;
     case 2:
-        val = virtio_config_readw(proxy->vdev, addr);
+        val = virtio_config_readw(vdev, addr);
         if (virtio_is_big_endian()) {
             val = bswap16(val);
         }
         break;
     case 4:
-        val = virtio_config_readl(proxy->vdev, addr);
+        val = virtio_config_readl(vdev, addr);
         if (virtio_is_big_endian()) {
             val = bswap32(val);
         }
@@ -413,6 +428,7 @@ static void virtio_pci_config_write(void *opaque, hwaddr addr,
 {
     VirtIOPCIProxy *proxy = opaque;
     uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     if (addr < config) {
         virtio_ioport_write(proxy, addr, val);
         return;
@@ -424,19 +440,19 @@ static void virtio_pci_config_write(void *opaque, hwaddr addr,
      */
     switch (size) {
     case 1:
-        virtio_config_writeb(proxy->vdev, addr, val);
+        virtio_config_writeb(vdev, addr, val);
         break;
     case 2:
         if (virtio_is_big_endian()) {
             val = bswap16(val);
         }
-        virtio_config_writew(proxy->vdev, addr, val);
+        virtio_config_writew(vdev, addr, val);
         break;
     case 4:
         if (virtio_is_big_endian()) {
             val = bswap32(val);
         }
-        virtio_config_writel(proxy->vdev, addr, val);
+        virtio_config_writel(vdev, addr, val);
         break;
     }
 }
@@ -455,6 +471,7 @@ static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
                                 uint32_t val, int len)
 {
     VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
 
     pci_default_write_config(pci_dev, address, val, len);
 
@@ -462,8 +479,7 @@ static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
         !(pci_dev->config[PCI_COMMAND] & PCI_COMMAND_MASTER) &&
         !(proxy->flags & VIRTIO_PCI_FLAG_BUS_MASTER_BUG)) {
         virtio_pci_stop_ioeventfd(proxy);
-        virtio_set_status(proxy->vdev,
-                          proxy->vdev->status & ~VIRTIO_CONFIG_S_DRIVER_OK);
+        virtio_set_status(vdev, vdev->status & ~VIRTIO_CONFIG_S_DRIVER_OK);
     }
 }
 
@@ -506,7 +522,8 @@ static int kvm_virtio_pci_irqfd_use(VirtIOPCIProxy *proxy,
                                  unsigned int vector)
 {
     VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector];
-    VirtQueue *vq = virtio_get_queue(proxy->vdev, queue_no);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    VirtQueue *vq = virtio_get_queue(vdev, queue_no);
     EventNotifier *n = virtio_queue_get_guest_notifier(vq);
     int ret;
     ret = kvm_irqchip_add_irqfd_notifier(kvm_state, n, NULL, irqfd->virq);
@@ -517,7 +534,8 @@ static void kvm_virtio_pci_irqfd_release(VirtIOPCIProxy *proxy,
                                       unsigned int queue_no,
                                       unsigned int vector)
 {
-    VirtQueue *vq = virtio_get_queue(proxy->vdev, queue_no);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    VirtQueue *vq = virtio_get_queue(vdev, queue_no);
     EventNotifier *n = virtio_queue_get_guest_notifier(vq);
     VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector];
     int ret;
@@ -529,7 +547,7 @@ static void kvm_virtio_pci_irqfd_release(VirtIOPCIProxy *proxy,
 static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
 {
     PCIDevice *dev = &proxy->pci_dev;
-    VirtIODevice *vdev = proxy->vdev;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
     unsigned int vector;
     int ret, queue_no;
@@ -578,7 +596,7 @@ undo:
 static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
 {
     PCIDevice *dev = &proxy->pci_dev;
-    VirtIODevice *vdev = proxy->vdev;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     unsigned int vector;
     int queue_no;
     VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
@@ -606,8 +624,9 @@ static int virtio_pci_vq_vector_unmask(VirtIOPCIProxy *proxy,
                                        unsigned int vector,
                                        MSIMessage msg)
 {
-    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(proxy->vdev);
-    VirtQueue *vq = virtio_get_queue(proxy->vdev, queue_no);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+    VirtQueue *vq = virtio_get_queue(vdev, queue_no);
     EventNotifier *n = virtio_queue_get_guest_notifier(vq);
     VirtIOIRQFD *irqfd;
     int ret = 0;
@@ -626,10 +645,10 @@ static int virtio_pci_vq_vector_unmask(VirtIOPCIProxy *proxy,
      * Otherwise, set it up now.
      */
     if (k->guest_notifier_mask) {
-        k->guest_notifier_mask(proxy->vdev, queue_no, false);
+        k->guest_notifier_mask(vdev, queue_no, false);
         /* Test after unmasking to avoid losing events. */
         if (k->guest_notifier_pending &&
-            k->guest_notifier_pending(proxy->vdev, queue_no)) {
+            k->guest_notifier_pending(vdev, queue_no)) {
             event_notifier_set(n);
         }
     } else {
@@ -642,13 +661,14 @@ static void virtio_pci_vq_vector_mask(VirtIOPCIProxy *proxy,
                                              unsigned int queue_no,
                                              unsigned int vector)
 {
-    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(proxy->vdev);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
 
     /* If guest supports masking, keep irqfd but mask it.
      * Otherwise, clean it up now.
      */ 
     if (k->guest_notifier_mask) {
-        k->guest_notifier_mask(proxy->vdev, queue_no, true);
+        k->guest_notifier_mask(vdev, queue_no, true);
     } else {
         kvm_virtio_pci_irqfd_release(proxy, queue_no, vector);
     }
@@ -658,7 +678,7 @@ static int virtio_pci_vector_unmask(PCIDevice *dev, unsigned vector,
                                     MSIMessage msg)
 {
     VirtIOPCIProxy *proxy = container_of(dev, VirtIOPCIProxy, pci_dev);
-    VirtIODevice *vdev = proxy->vdev;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     int ret, queue_no;
 
     for (queue_no = 0; queue_no < proxy->nvqs_with_notifiers; queue_no++) {
@@ -688,7 +708,7 @@ undo:
 static void virtio_pci_vector_mask(PCIDevice *dev, unsigned vector)
 {
     VirtIOPCIProxy *proxy = container_of(dev, VirtIOPCIProxy, pci_dev);
-    VirtIODevice *vdev = proxy->vdev;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     int queue_no;
 
     for (queue_no = 0; queue_no < proxy->nvqs_with_notifiers; queue_no++) {
@@ -707,7 +727,7 @@ static void virtio_pci_vector_poll(PCIDevice *dev,
                                    unsigned int vector_end)
 {
     VirtIOPCIProxy *proxy = container_of(dev, VirtIOPCIProxy, pci_dev);
-    VirtIODevice *vdev = proxy->vdev;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
     int queue_no;
     unsigned int vector;
@@ -739,8 +759,9 @@ static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
                                          bool with_irqfd)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
-    VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(proxy->vdev);
-    VirtQueue *vq = virtio_get_queue(proxy->vdev, n);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
+    VirtQueue *vq = virtio_get_queue(vdev, n);
     EventNotifier *notifier = virtio_queue_get_guest_notifier(vq);
 
     if (assign) {
@@ -755,7 +776,7 @@ static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
     }
 
     if (!msix_enabled(&proxy->pci_dev) && vdc->guest_notifier_mask) {
-        vdc->guest_notifier_mask(proxy->vdev, n, !assign);
+        vdc->guest_notifier_mask(vdev, n, !assign);
     }
 
     return 0;
@@ -770,7 +791,7 @@ static bool virtio_pci_query_guest_notifiers(DeviceState *d)
 static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
-    VirtIODevice *vdev = proxy->vdev;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
     int r, n;
     bool with_irqfd = msix_enabled(&proxy->pci_dev) &&
@@ -864,11 +885,12 @@ static int virtio_pci_set_host_notifier(DeviceState *d, int n, bool assign)
 static void virtio_pci_vmstate_change(DeviceState *d, bool running)
 {
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
 
     if (running) {
         /* Try to find out if the guest has bus master disabled, but is
            in ready state. Then we have a buggy guest OS. */
-        if ((proxy->vdev->status & VIRTIO_CONFIG_S_DRIVER_OK) &&
+        if ((vdev->status & VIRTIO_CONFIG_S_DRIVER_OK) &&
             !(proxy->pci_dev.config[PCI_COMMAND] & PCI_COMMAND_MASTER)) {
             proxy->flags |= VIRTIO_PCI_FLAG_BUS_MASTER_BUG;
         }
@@ -943,8 +965,6 @@ static void virtio_pci_device_plugged(DeviceState *d)
     uint8_t *config;
     uint32_t size;
 
-    proxy->vdev = virtio_bus_get_device(bus);
-
     config = proxy->pci_dev.config;
     if (proxy->class_code) {
         pci_config_set_class(config, proxy->class_code);
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index 917bcc5..dc332ae 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -82,7 +82,6 @@ typedef struct VirtioPCIClass {
 
 struct VirtIOPCIProxy {
     PCIDevice pci_dev;
-    VirtIODevice *vdev;
     MemoryRegion bar;
     uint32_t flags;
     uint32_t class_code;
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 05/12] virtio-bus: cleanup plug/unplug interface

[Paolo Bonzini]

Right now we have these pairs:

- virtio_bus_plug_device/virtio_bus_destroy_device.  The first
  takes a VirtIODevice, the second takes a VirtioBusState

- device_plugged/device_unplug callbacks in the VirtioBusClass
  (here it's just the naming that is inconsistent)

- virtio_bus_destroy_device is not called by anyone (and since
  it calls qdev_free, it would be called by the proxies---but
  then the callback is useless since the proxies can do whatever
  they want before calling virtio_bus_destroy_device)

And there is a k->init but no k->exit, hence virtio_device_exit is
overwritten by subclasses (except virtio-9p).  This cleans it up by:

- renaming the device_unplug callback to device_unplugged

- renaming virtio_bus_plug_device to virtio_bus_device_plugged,
  matching the callback name

- renaming virtio_bus_destroy_device to virtio_bus_device_unplugged,
  removing the qdev_free, making it take a VirtIODevice and calling it
  from virtio_device_exit

- adding a k->exit callback

virtio_device_exit is still overwritten, the next patches will fix that.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio/virtio-bus.c         | 20 +++++++++-----------
 hw/virtio/virtio.c             |  7 ++++++-
 include/hw/virtio/virtio-bus.h |  6 +++---
 include/hw/virtio/virtio.h     |  1 +
 4 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index 10dfa2c..7aed6a4 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -37,8 +37,8 @@ do { printf("virtio_bus: " fmt , ## __VA_ARGS__); } while (0)
 #define DPRINTF(fmt, ...) do { } while (0)
 #endif
 
-/* Plug the VirtIODevice */
-int virtio_bus_plug_device(VirtIODevice *vdev)
+/* A VirtIODevice is being plugged */
+int virtio_bus_device_plugged(VirtIODevice *vdev)
 {
     DeviceState *qdev = DEVICE(vdev);
     BusState *qbus = BUS(qdev_get_parent_bus(qdev));
@@ -64,22 +64,20 @@ void virtio_bus_reset(VirtioBusState *bus)
     }
 }
 
-/* Destroy the VirtIODevice */
-void virtio_bus_destroy_device(VirtioBusState *bus)
+/* A VirtIODevice is being unplugged */
+void virtio_bus_device_unplugged(VirtIODevice *vdev)
 {
-    DeviceState *qdev;
-    BusState *qbus = BUS(bus);
+    DeviceState *qdev = DEVICE(vdev);
+    BusState *qbus = BUS(qdev_get_parent_bus(qdev));
+    VirtioBusState *bus = VIRTIO_BUS(qbus);
     VirtioBusClass *klass = VIRTIO_BUS_GET_CLASS(bus);
-    VirtIODevice *vdev = virtio_bus_get_device(bus);
 
     DPRINTF("%s: remove device.\n", qbus->name);
 
     if (vdev != NULL) {
-        if (klass->device_unplug != NULL) {
-            klass->device_unplug(qbus->parent);
+        if (klass->device_unplugged != NULL) {
+            klass->device_unplugged(qbus->parent);
         }
-        qdev = DEVICE(vdev);
-        qdev_free(qdev);
     }
 }
 
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 2f1e73b..965b2c0 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1158,14 +1158,19 @@ static int virtio_device_init(DeviceState *qdev)
     if (k->init(vdev) < 0) {
         return -1;
     }
-    virtio_bus_plug_device(vdev);
+    virtio_bus_device_plugged(vdev);
     return 0;
 }
 
 static int virtio_device_exit(DeviceState *qdev)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
+    VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(qdev);
 
+    virtio_bus_device_unplugged(vdev);
+    if (k->exit) {
+        k->exit(vdev);
+    }
     if (vdev->bus_name) {
         g_free(vdev->bus_name);
         vdev->bus_name = NULL;
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index ba0f86a..0756545 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -61,7 +61,7 @@ typedef struct VirtioBusClass {
      * transport independent exit function.
      * This is called by virtio-bus just before the device is unplugged.
      */
-    void (*device_unplug)(DeviceState *d);
+    void (*device_unplugged)(DeviceState *d);
     /*
      * Does the transport have variable vring alignment?
      * (ie can it ever call virtio_queue_set_align()?)
@@ -74,9 +74,9 @@ struct VirtioBusState {
     BusState parent_obj;
 };
 
-int virtio_bus_plug_device(VirtIODevice *vdev);
+int virtio_bus_device_plugged(VirtIODevice *vdev);
 void virtio_bus_reset(VirtioBusState *bus);
-void virtio_bus_destroy_device(VirtioBusState *bus);
+void virtio_bus_device_unplugged(VirtIODevice *bus);
 /* Get the device id of the plugged device. */
 uint16_t virtio_bus_get_vdev_id(VirtioBusState *bus);
 /* Get the config_len field of the plugged device. */
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index a90522d..59756c2 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -127,6 +127,7 @@ typedef struct VirtioDeviceClass {
     /* This is what a VirtioDevice must implement */
     DeviceClass parent;
     int (*init)(VirtIODevice *vdev);
+    void (*exit)(VirtIODevice *vdev);
     uint32_t (*get_features)(VirtIODevice *vdev, uint32_t requested_features);
     uint32_t (*bad_features)(VirtIODevice *vdev);
     void (*set_features)(VirtIODevice *vdev, uint32_t val);
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 06/12] virtio-blk: switch exit callback to VirtioDeviceClass

[Paolo Bonzini]

This ensures hot-unplug is handled properly by the proxy.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/block/virtio-blk.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 49a23c3..aa37cc9 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -729,20 +729,18 @@ static int virtio_blk_device_init(VirtIODevice *vdev)
     return 0;
 }
 
-static int virtio_blk_device_exit(DeviceState *dev)
+static void virtio_blk_device_exit(VirtIODevice *vdev)
 {
-    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-    VirtIOBlock *s = VIRTIO_BLK(dev);
+    VirtIOBlock *s = VIRTIO_BLK(vdev);
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
     remove_migration_state_change_notifier(&s->migration_state_notifier);
     virtio_blk_data_plane_destroy(s->dataplane);
     s->dataplane = NULL;
 #endif
     qemu_del_vm_change_state_handler(s->change);
-    unregister_savevm(dev, "virtio-blk", s);
+    unregister_savevm(DEVICE(vdev), "virtio-blk", s);
     blockdev_mark_auto_del(s->bs);
     virtio_cleanup(vdev);
-    return 0;
 }
 
 static Property virtio_blk_properties[] = {
@@ -754,10 +752,10 @@ static void virtio_blk_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_blk_device_exit;
     dc->props = virtio_blk_properties;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
     vdc->init = virtio_blk_device_init;
+    vdc->exit = virtio_blk_device_exit;
     vdc->get_config = virtio_blk_update_config;
     vdc->set_config = virtio_blk_set_config;
     vdc->get_features = virtio_blk_get_features;
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 07/12] virtio-serial: switch exit callback to VirtioDeviceClass

[Paolo Bonzini]

This ensures hot-unplug is handled properly by the proxy.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/char/virtio-serial-bus.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 703f026..a7ede90 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -987,12 +987,11 @@ static const TypeInfo virtio_serial_port_type_info = {
     .class_init = virtio_serial_port_class_init,
 };
 
-static int virtio_serial_device_exit(DeviceState *dev)
+static void virtio_serial_device_exit(VirtIODevice *vdev)
 {
-    VirtIOSerial *vser = VIRTIO_SERIAL(dev);
-    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIOSerial *vser = VIRTIO_SERIAL(vdev);
 
-    unregister_savevm(dev, "virtio-console", vser);
+    unregister_savevm(DEVICE(vdev), "virtio-console", vser);
 
     g_free(vser->ivqs);
     g_free(vser->ovqs);
@@ -1004,7 +1003,6 @@ static int virtio_serial_device_exit(DeviceState *dev)
         g_free(vser->post_load);
     }
     virtio_cleanup(vdev);
-    return 0;
 }
 
 static Property virtio_serial_properties[] = {
@@ -1016,10 +1014,10 @@ static void virtio_serial_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_serial_device_exit;
     dc->props = virtio_serial_properties;
     set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
     vdc->init = virtio_serial_device_init;
+    vdc->exit = virtio_serial_device_exit;
     vdc->get_features = get_features;
     vdc->get_config = get_config;
     vdc->set_config = set_config;
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 08/12] virtio-net: switch exit callback to VirtioDeviceClass

[Paolo Bonzini]

This ensures hot-unplug is handled properly by the proxy.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/net/virtio-net.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 22dbd05..0911096 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1569,16 +1569,15 @@ static int virtio_net_device_init(VirtIODevice *vdev)
     return 0;
 }
 
-static int virtio_net_device_exit(DeviceState *qdev)
+static void virtio_net_device_exit(VirtIODevice *vdev)
 {
-    VirtIONet *n = VIRTIO_NET(qdev);
-    VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
+    VirtIONet *n = VIRTIO_NET(vdev);
     int i;
 
     /* This will stop vhost backend if appropriate. */
     virtio_net_set_status(vdev, 0);
 
-    unregister_savevm(qdev, "virtio-net", n);
+    unregister_savevm(DEVICE(vdev), "virtio-net", n);
 
     if (n->netclient_name) {
         g_free(n->netclient_name);
@@ -1609,8 +1608,6 @@ static int virtio_net_device_exit(DeviceState *qdev)
     g_free(n->vqs);
     qemu_del_nic(n->nic);
     virtio_cleanup(vdev);
-
-    return 0;
 }
 
 static void virtio_net_instance_init(Object *obj)
@@ -1637,10 +1634,10 @@ static void virtio_net_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_net_device_exit;
     dc->props = virtio_net_properties;
     set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
     vdc->init = virtio_net_device_init;
+    vdc->exit = virtio_net_device_exit;
     vdc->get_config = virtio_net_get_config;
     vdc->set_config = virtio_net_set_config;
     vdc->get_features = virtio_net_get_features;
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 09/12] virtio-scsi: switch exit callback to VirtioDeviceClass

[Paolo Bonzini]

This ensures hot-unplug is handled properly by the proxy.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/vhost-scsi.c            | 11 +++++------
 hw/scsi/virtio-scsi.c           | 15 +++++++--------
 include/hw/virtio/virtio-scsi.h |  2 +-
 3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index 9e770fb..5e3cc61 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -240,11 +240,10 @@ static int vhost_scsi_init(VirtIODevice *vdev)
     return 0;
 }
 
-static int vhost_scsi_exit(DeviceState *qdev)
+static void vhost_scsi_exit(VirtIODevice *vdev)
 {
-    VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
-    VHostSCSI *s = VHOST_SCSI(qdev);
-    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(qdev);
+    VHostSCSI *s = VHOST_SCSI(vdev);
+    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
 
     migrate_del_blocker(s->migration_blocker);
     error_free(s->migration_blocker);
@@ -253,7 +252,7 @@ static int vhost_scsi_exit(DeviceState *qdev)
     vhost_scsi_set_status(vdev, 0);
 
     g_free(s->dev.vqs);
-    return virtio_scsi_common_exit(vs);
+    virtio_scsi_common_exit(vs);
 }
 
 static Property vhost_scsi_properties[] = {
@@ -265,10 +264,10 @@ static void vhost_scsi_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = vhost_scsi_exit;
     dc->props = vhost_scsi_properties;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
     vdc->init = vhost_scsi_init;
+    vdc->exit = vhost_scsi_exit;
     vdc->get_features = vhost_scsi_get_features;
     vdc->set_config = vhost_scsi_set_config;
     vdc->set_status = vhost_scsi_set_status;
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 26d95a1..83344ea 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -644,22 +644,21 @@ static int virtio_scsi_device_init(VirtIODevice *vdev)
     return 0;
 }
 
-int virtio_scsi_common_exit(VirtIOSCSICommon *vs)
+void virtio_scsi_common_exit(VirtIOSCSICommon *vs)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(vs);
 
     g_free(vs->cmd_vqs);
     virtio_cleanup(vdev);
-    return 0;
 }
 
-static int virtio_scsi_device_exit(DeviceState *qdev)
+static void virtio_scsi_device_exit(VirtIODevice *vdev)
 {
-    VirtIOSCSI *s = VIRTIO_SCSI(qdev);
-    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(qdev);
+    VirtIOSCSI *s = VIRTIO_SCSI(vdev);
+    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
 
-    unregister_savevm(qdev, "virtio-scsi", s);
-    return virtio_scsi_common_exit(vs);
+    unregister_savevm(DEVICE(vdev), "virtio-scsi", s);
+    virtio_scsi_common_exit(vs);
 }
 
 static Property virtio_scsi_properties[] = {
@@ -680,10 +679,10 @@ static void virtio_scsi_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_scsi_device_exit;
     dc->props = virtio_scsi_properties;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
     vdc->init = virtio_scsi_device_init;
+    vdc->exit = virtio_scsi_device_exit;
     vdc->set_config = virtio_scsi_set_config;
     vdc->get_features = virtio_scsi_get_features;
     vdc->reset = virtio_scsi_reset;
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index 9a98540..206c61d 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -187,6 +187,6 @@ typedef struct {
                                             VIRTIO_SCSI_F_CHANGE, true)
 
 int virtio_scsi_common_init(VirtIOSCSICommon *vs);
-int virtio_scsi_common_exit(VirtIOSCSICommon *vs);
+void virtio_scsi_common_exit(VirtIOSCSICommon *vs);
 
 #endif /* _QEMU_VIRTIO_SCSI_H */
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 10/12] virtio-balloon: switch exit callback to VirtioDeviceClass

[Paolo Bonzini]

This ensures hot-unplug is handled properly by the proxy.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio/virtio-balloon.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 9504877..d7a392d 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -370,16 +370,14 @@ static int virtio_balloon_device_init(VirtIODevice *vdev)
     return 0;
 }
 
-static int virtio_balloon_device_exit(DeviceState *qdev)
+static void virtio_balloon_device_exit(VirtIODevice *vdev)
 {
-    VirtIOBalloon *s = VIRTIO_BALLOON(qdev);
-    VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
+    VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
 
     balloon_stats_destroy_timer(s);
     qemu_remove_balloon_handler(s);
-    unregister_savevm(qdev, "virtio-balloon", s);
+    unregister_savevm(DEVICE(vdev), "virtio-balloon", s);
     virtio_cleanup(vdev);
-    return 0;
 }
 
 static Property virtio_balloon_properties[] = {
@@ -390,10 +388,10 @@ static void virtio_balloon_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_balloon_device_exit;
     dc->props = virtio_balloon_properties;
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
     vdc->init = virtio_balloon_device_init;
+    vdc->exit = virtio_balloon_device_exit;
     vdc->get_config = virtio_balloon_get_config;
     vdc->set_config = virtio_balloon_set_config;
     vdc->get_features = virtio_balloon_get_features;
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 11/12] virtio-rng: switch exit callback to VirtioDeviceClass

[Paolo Bonzini]

This ensures hot-unplug is handled properly by the proxy.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio/virtio-rng.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index 314e393..18cbfac 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -184,16 +184,14 @@ static int virtio_rng_device_init(VirtIODevice *vdev)
     return 0;
 }
 
-static int virtio_rng_device_exit(DeviceState *qdev)
+static void virtio_rng_device_exit(VirtIODevice *vdev)
 {
-    VirtIORNG *vrng = VIRTIO_RNG(qdev);
-    VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
+    VirtIORNG *vrng = VIRTIO_RNG(vdev);
 
     timer_del(vrng->rate_limit_timer);
     timer_free(vrng->rate_limit_timer);
-    unregister_savevm(qdev, "virtio-rng", vrng);
+    unregister_savevm(DEVICE(vdev), "virtio-rng", vrng);
     virtio_cleanup(vdev);
-    return 0;
 }
 
 static Property virtio_rng_properties[] = {
@@ -205,10 +203,10 @@ static void virtio_rng_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-    dc->exit = virtio_rng_device_exit;
     dc->props = virtio_rng_properties;
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
     vdc->init = virtio_rng_device_init;
+    vdc->exit = virtio_rng_device_exit;
     vdc->get_features = get_features;
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 12/12] virtio-pci: add device_unplugged callback

[Paolo Bonzini]

This fixes a crash in hot-unplug of virtio-pci devices behind a PCIe
switch.  The crash happens because the ioeventfd is still set whent the
child is destroyed (destruction happens in postorder).  Then the proxy
tries to unset to ioeventfd, but the virtqueue structure that holds the
EventNotifier has been trashed in the meanwhile.  kvm_set_ioeventfd_pio
does not expect failure and aborts.

The fix is simply to move parts of uninitialization to a new
device_unplugged callback, which is called before the child is destroyed.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio/virtio-pci.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 89af295..a191c24 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1002,6 +1002,15 @@ static void virtio_pci_device_plugged(DeviceState *d)
                                                       proxy->host_features);
 }
 
+static void virtio_pci_device_unplugged(DeviceState *d)
+{
+    PCIDevice *pci_dev = PCI_DEVICE(d);
+    VirtIOPCIProxy *proxy = VIRTIO_PCI(d);
+
+    virtio_pci_stop_ioeventfd(proxy);
+    msix_uninit_exclusive_bar(pci_dev);
+}
+
 static int virtio_pci_init(PCIDevice *pci_dev)
 {
     VirtIOPCIProxy *dev = VIRTIO_PCI(pci_dev);
@@ -1016,9 +1025,7 @@ static int virtio_pci_init(PCIDevice *pci_dev)
 static void virtio_pci_exit(PCIDevice *pci_dev)
 {
     VirtIOPCIProxy *proxy = VIRTIO_PCI(pci_dev);
-    virtio_pci_stop_ioeventfd(proxy);
     memory_region_destroy(&proxy->bar);
-    msix_uninit_exclusive_bar(pci_dev);
 }
 
 static void virtio_pci_reset(DeviceState *qdev)
@@ -1553,6 +1560,7 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
     k->set_guest_notifiers = virtio_pci_set_guest_notifiers;
     k->vmstate_change = virtio_pci_vmstate_change;
     k->device_plugged = virtio_pci_device_plugged;
+    k->device_unplugged = virtio_pci_device_unplugged;
 }
 
 static const TypeInfo virtio_pci_bus_info = {
-- 
1.8.3.1