From 925fe64ae7b487fdb7bd56fcab63e2f87653c226 Mon Sep 17 00:00:00 2001 From: Alex Williamson Date: Tue, 11 May 2010 06:44:21 -0400 Subject: [PATCH 1/4] pci: cleanly backout of pci_qdev_init() If the init function of a device fails, as might happen with device assignment, we never undo the work done by do_pci_register_device(). This not only causes a bit of a memory leak, but also leaves a bogus pointer in the bus devices array that can cause a segfault or garbage data from 'info pci'. Signed-off-by: Alex Williamson Signed-off-by: Michael S. Tsirkin --- hw/pci.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/hw/pci.c b/hw/pci.c index f167436edc..3ca5f0953f 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -625,6 +625,13 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, return pci_dev; } +static void do_pci_unregister_device(PCIDevice *pci_dev) +{ + qemu_free_irqs(pci_dev->irq); + pci_dev->bus->devices[pci_dev->devfn] = NULL; + pci_config_free(pci_dev); +} + PCIDevice *pci_register_device(PCIBus *bus, const char *name, int instance_size, int devfn, PCIConfigReadFunc *config_read, @@ -680,10 +687,7 @@ static int pci_unregister_device(DeviceState *dev) return ret; pci_unregister_io_regions(pci_dev); - - qemu_free_irqs(pci_dev->irq); - pci_dev->bus->devices[pci_dev->devfn] = NULL; - pci_config_free(pci_dev); + do_pci_unregister_device(pci_dev); return 0; } @@ -1652,8 +1656,10 @@ static int pci_qdev_init(DeviceState *qdev, DeviceInfo *base) if (pci_dev == NULL) return -1; rc = info->init(pci_dev); - if (rc != 0) + if (rc != 0) { + do_pci_unregister_device(pci_dev); return rc; + } /* rom loading */ if (pci_dev->romfile == NULL && info->romfile != NULL) From c3f8f61157625d0bb5bfc135047573de48fdc675 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Sun, 9 May 2010 19:15:16 +0300 Subject: [PATCH 2/4] pci: irq_state vmstate breakage Code for saving irq_state got vm_state macros wrong, passing in the wrong parameter. As a result, we both saved a wrong value and restored it to a wrong offset. This leads to device and bus irq counts getting out of sync, which in turn leads to interrupts getting lost or never cleared, such as https://bugzilla.redhat.com/show_bug.cgi?id=588133 Signed-off-by: Michael S. Tsirkin Acked-by: Juan Quintela --- hw/pci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/pci.c b/hw/pci.c index 3ca5f0953f..1e143d9bf0 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -322,7 +322,7 @@ static VMStateInfo vmstate_info_pci_config = { static int get_pci_irq_state(QEMUFile *f, void *pv, size_t size) { - PCIDevice *s = container_of(pv, PCIDevice, config); + PCIDevice *s = container_of(pv, PCIDevice, irq_state); uint32_t irq_state[PCI_NUM_PINS]; int i; for (i = 0; i < PCI_NUM_PINS; ++i) { @@ -344,7 +344,7 @@ static int get_pci_irq_state(QEMUFile *f, void *pv, size_t size) static void put_pci_irq_state(QEMUFile *f, void *pv, size_t size) { int i; - PCIDevice *s = container_of(pv, PCIDevice, config); + PCIDevice *s = container_of(pv, PCIDevice, irq_state); for (i = 0; i < PCI_NUM_PINS; ++i) { qemu_put_be32(f, pci_irq_state(s, i)); From fae054b070143a60ce40671470292efc2dc58a49 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Sun, 9 May 2010 19:42:09 +0300 Subject: [PATCH 3/4] virtio: invoke set_features on load After migration, vhost was not getting features acked because set_features callback was never invoked. The fix is just to invoke that callback. Reported-by: David L Stevens Signed-off-by: Michael S. Tsirkin Tested-by: David L Stevens --- hw/virtio.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/virtio.c b/hw/virtio.c index e7657ae8aa..4475bb3e44 100644 --- a/hw/virtio.c +++ b/hw/virtio.c @@ -677,6 +677,8 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f) features, supported_features); return -1; } + if (vdev->set_features) + vdev->set_features(vdev, features); vdev->guest_features = features; vdev->config_len = qemu_get_be32(f); qemu_get_buffer(f, vdev->config, vdev->config_len); From 57c3229ba1c5cecae277301b8b16577fbf2de98b Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Sun, 9 May 2010 14:35:43 +0300 Subject: [PATCH 4/4] virtio-net: return with value in void function virtio-net has return with value in a void function. No idea why does it compile with gcc, but this isn't standard C. Signed-off-by: Michael S. Tsirkin --- hw/virtio-net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/virtio-net.c b/hw/virtio-net.c index d602c56c25..cb664e6207 100644 --- a/hw/virtio-net.c +++ b/hw/virtio-net.c @@ -227,7 +227,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features) if (!tap_get_vhost_net(n->nic->nc.peer)) { return; } - return vhost_net_ack_features(tap_get_vhost_net(n->nic->nc.peer), features); + vhost_net_ack_features(tap_get_vhost_net(n->nic->nc.peer), features); } static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,