qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Laurent Vivier <lvivier@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Juan Quintela" <quintela@redhat.com>,
	"Jason Wang" <jasowang@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
	"Igor Mammedov" <imammedo@redhat.com>,
	"Jens Freimann" <jfreimann@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: [PATCH v4] failover: unregister ROM on unplug
Date: Wed, 21 Jul 2021 18:09:05 +0200	[thread overview]
Message-ID: <20210721160905.234915-1-lvivier@redhat.com> (raw)

The intend of failover is to allow to migrate a VM with a VFIO
networking card without disrupting the network operation by switching
to a virtio-net device during the migration.

This simple change allows to test failover with a simulated device
like e1000e rather than a vfio device, even if it's useless in real
life it can help to debug failover.

This is interesting to developers that want to test failover on
a system with no vfio device. Moreover it simplifies host networking
configuration as we can use the same bridge for virtio-net and
the other failover networking device.

Without this change the migration of a system configured with failover
fails with:

  ...
  -device virtio-net-pci,id=virtionet0,failover=on,...  \
  -device e1000,failover_pair_id=virtionet0,... \
  ...

  (qemu) migrate ...

  Unknown ramblock "0000:00:01.1:00.0/e1000e.rom", cannot accept migration
  error while loading state for instance 0x0 of device 'ram'
  load of migration failed: Invalid argument

This happens because QEMU correctly unregisters the interface vmstate but
not the ROM one. This patch fixes that.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---

Notes:
    v4:
      export and use pci_del_option_rom()
    
    v3:
      remove useless space before comma
    
    v2:
      reset has_rom to false
      update commit log message

 include/hw/pci/pci.h | 2 ++
 hw/net/virtio-net.c  | 1 +
 hw/pci/pci.c         | 3 +--
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index d0f4266e3725..84707034cbf8 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -369,6 +369,8 @@ void pci_register_vga(PCIDevice *pci_dev, MemoryRegion *mem,
 void pci_unregister_vga(PCIDevice *pci_dev);
 pcibus_t pci_get_bar_addr(PCIDevice *pci_dev, int region_num);
 
+void pci_del_option_rom(PCIDevice *pdev);
+
 int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
                        uint8_t offset, uint8_t size,
                        Error **errp);
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 16d20cdee52a..d6f03633f1b3 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3256,6 +3256,7 @@ static void virtio_net_handle_migration_primary(VirtIONet *n, MigrationState *s)
     if (migration_in_setup(s) && !should_be_hidden) {
         if (failover_unplug_primary(n, dev)) {
             vmstate_unregister(VMSTATE_IF(dev), qdev_get_vmsd(dev), dev);
+            pci_del_option_rom(PCI_DEVICE(dev));
             qapi_event_send_unplug_primary(dev->id);
             qatomic_set(&n->failover_primary_hidden, true);
         } else {
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 23d2ae2ab232..c210d92b5ba7 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -228,7 +228,6 @@ static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
 static void pci_update_mappings(PCIDevice *d);
 static void pci_irq_handler(void *opaque, int irq_num, int level);
 static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom, Error **);
-static void pci_del_option_rom(PCIDevice *pdev);
 
 static uint16_t pci_default_sub_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET;
 static uint16_t pci_default_sub_device_id = PCI_SUBDEVICE_ID_QEMU;
@@ -2429,7 +2428,7 @@ static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom,
     pci_register_bar(pdev, PCI_ROM_SLOT, 0, &pdev->rom);
 }
 
-static void pci_del_option_rom(PCIDevice *pdev)
+void pci_del_option_rom(PCIDevice *pdev)
 {
     if (!pdev->has_rom)
         return;
-- 
2.31.1



             reply	other threads:[~2021-07-21 16:10 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-21 16:09 Laurent Vivier [this message]
2021-07-21 16:19 ` [PATCH v4] failover: unregister ROM on unplug Philippe Mathieu-Daudé
2021-07-21 16:19 ` Michael S. Tsirkin
2021-07-21 17:01   ` Laurent Vivier
2021-07-23  8:55   ` Laurent Vivier
2021-08-03 14:04 ` Michael S. Tsirkin
2021-08-03 14:49   ` Laurent Vivier
2021-08-11  6:50 ` Michael S. Tsirkin
2021-08-11  9:51   ` Laurent Vivier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210721160905.234915-1-lvivier@redhat.com \
    --to=lvivier@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=jfreimann@redhat.com \
    --cc=mst@redhat.com \
    --cc=philmd@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).