[RFC PATCH v2 0/8] virtio-net failover cleanup and new features

[RFC PATCH v2 0/8] virtio-net failover cleanup and new features

[Laurent Vivier]

v2: add helpers to manage the list of hidden devices rather
    than relying on the command line parameters
    Hide VFIO device if it is plugged before the virtio-net one

This series moves the code used by virtio-net failover from the
virtio-net device to the PCI subsystem.

Doing that, we can use failover with a regular QEMU PCI device
(we can add the function call to unregister the ROM vmstate) and we
can also use this code to unplug a PCI card before migration
and plug it back after migration without using a failover
device (of course, connectivity is lost during all the migration).
In contrary of failover, this does not need support from the
guest system to work.

Laurent Vivier (8):
  qdev: add an Error parameter to the DeviceListener hide_device()
    function
  qdev/qbus: remove failover specific code
  failover: virtio-net: remove failover_primary_hidden flag
  failover: pci: move failover hotplug/unplug code into pci subsystem
  failover: hide the PCI device if the virtio-net device is not present
  failover: pci: unregister ROM on unplug
  pci: automatically unplug a PCI card before migration
  failover: qemu-opts: manage hidden device list

 include/hw/pci/pci.h           |   5 +
 include/hw/qdev-core.h         |   6 +-
 include/hw/virtio/virtio-net.h |   4 -
 include/hw/virtio/virtio.h     |   1 -
 include/qemu/option.h          |   4 +
 hw/core/qdev.c                 |   5 +-
 hw/net/virtio-net.c            | 149 +-------------------
 hw/pci/pci.c                   | 242 +++++++++++++++++++++++++++++++--
 hw/vfio/pci.c                  |   2 +-
 softmmu/qdev-monitor.c         |  14 +-
 util/qemu-option.c             |  82 +++++++++++
 11 files changed, 338 insertions(+), 176 deletions(-)

-- 
2.31.1

[RFC PATCH v2 1/8] qdev: add an Error parameter to the DeviceListener hide_device() function

[Laurent Vivier]

This allows an error to be reported to the caller of qdev_device_add()

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 include/hw/qdev-core.h | 6 ++++--
 hw/core/qdev.c         | 4 ++--
 hw/net/virtio-net.c    | 2 +-
 softmmu/qdev-monitor.c | 4 ++--
 4 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index bafc311bfa1b..e23b23a2f8d6 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -202,7 +202,8 @@ struct DeviceListener {
      * hide a failover device depending for example on the device
      * opts.
      */
-    bool (*hide_device)(DeviceListener *listener, QemuOpts *device_opts);
+    bool (*hide_device)(DeviceListener *listener, QemuOpts *device_opts,
+                        Error **errp);
     QTAILQ_ENTRY(DeviceListener) link;
 };
 
@@ -804,12 +805,13 @@ void device_listener_unregister(DeviceListener *listener);
 /**
  * @qdev_should_hide_device:
  * @opts: QemuOpts as passed on cmdline.
+ * @errp: pointer to error object
  *
  * Check if a device should be added.
  * When a device is added via qdev_device_add() this will be called,
  * and return if the device should be added now or not.
  */
-bool qdev_should_hide_device(QemuOpts *opts);
+bool qdev_should_hide_device(QemuOpts *opts, Error **errp);
 
 typedef enum MachineInitPhase {
     /* current_machine is NULL.  */
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index cefc5eaa0a92..13f4c1e696bf 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -211,13 +211,13 @@ void device_listener_unregister(DeviceListener *listener)
     QTAILQ_REMOVE(&device_listeners, listener, link);
 }
 
-bool qdev_should_hide_device(QemuOpts *opts)
+bool qdev_should_hide_device(QemuOpts *opts, Error **errp)
 {
     DeviceListener *listener;
 
     QTAILQ_FOREACH(listener, &device_listeners, link) {
         if (listener->hide_device) {
-            if (listener->hide_device(listener, opts)) {
+            if (listener->hide_device(listener, opts, errp)) {
                 return true;
             }
         }
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 16d20cdee52a..542f9e167eb4 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3279,7 +3279,7 @@ static void virtio_net_migration_state_notifier(Notifier *notifier, void *data)
 }
 
 static bool failover_hide_primary_device(DeviceListener *listener,
-                                         QemuOpts *device_opts)
+                                         QemuOpts *device_opts, Error **errp)
 {
     VirtIONet *n = container_of(listener, VirtIONet, primary_listener);
     const char *standby_id;
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index 721dec2d8200..7adf0d22beb1 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -627,8 +627,8 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
             error_setg(errp, "Device with failover_pair_id don't have id");
             return NULL;
         }
-        if (qdev_should_hide_device(opts)) {
-            if (bus && !qbus_is_hotpluggable(bus)) {
+        if (qdev_should_hide_device(opts, errp)) {
+            if (errp && !*errp && bus && !qbus_is_hotpluggable(bus)) {
                 error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name);
             }
             return NULL;
-- 
2.31.1

[RFC PATCH v2 2/8] qdev/qbus: remove failover specific code

[Laurent Vivier]

Commit f3a850565693 ("qdev/qbus: add hidden device support") has
introduced a generic way to hide a device but it has modified
qdev_device_add() to check a specific option of the failover device,
"failover_pair_id", before calling the generic mechanism.

It's not needed (and not generic) to do that in qdev_device_add() because
this is also checked by the failover_hide_primary_device() function that
uses the generic mechanism to hide the device.

Cc: Jens Freimann <jfreimann@redhat.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 hw/net/virtio-net.c    |  7 +++++++
 softmmu/qdev-monitor.c | 14 ++++----------
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 542f9e167eb4..0c5ec930356b 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3288,6 +3288,13 @@ static bool failover_hide_primary_device(DeviceListener *listener,
         return false;
     }
     standby_id = qemu_opt_get(device_opts, "failover_pair_id");
+    if (standby_id == NULL) {
+        return false;
+    }
+    if (device_opts->id == NULL) {
+        error_setg(errp, "Device with failover_pair_id don't have id");
+        return true;
+    }
     if (g_strcmp0(standby_id, n->netclient_name) != 0) {
         return false;
     }
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index 7adf0d22beb1..5c92dbed3139 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -622,17 +622,11 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
         }
     }
 
-    if (qemu_opt_get(opts, "failover_pair_id")) {
-        if (!opts->id) {
-            error_setg(errp, "Device with failover_pair_id don't have id");
-            return NULL;
-        }
-        if (qdev_should_hide_device(opts, errp)) {
-            if (errp && !*errp && bus && !qbus_is_hotpluggable(bus)) {
-                error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name);
-            }
-            return NULL;
+    if (qdev_should_hide_device(opts, errp)) {
+        if (errp && !*errp && bus && !qbus_is_hotpluggable(bus)) {
+            error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name);
         }
+        return NULL;
     }
 
     if (phase_check(PHASE_MACHINE_READY) && bus && !qbus_is_hotpluggable(bus)) {
-- 
2.31.1

[RFC PATCH v2 3/8] failover: virtio-net: remove failover_primary_hidden flag

[Laurent Vivier]

We dont't need a flag to know if the primary device must be hidden, we
can rely on the machine state:
Device is hidden if the machine is in prelaunch state (src) or
in inmigrate state with migration status set to none (dst).
We don't need to check the flag in virtio_net_handle_migration_primary()
because this function is only registered if the failover is enabled
and the flag also set to false. This function also sets it back to
true after a successful unplug during the migration but we don't
need to know it is hidden because nothing will try to plug it back
after the migration (except in case of error but the flag is set
back to false).

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 include/hw/virtio/virtio-net.h |  2 --
 hw/net/virtio-net.c            | 25 +++++++++++++++----------
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index 824a69c23f06..21d8c3aa5f3a 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -205,8 +205,6 @@ struct VirtIONet {
     AnnounceTimer announce_timer;
     bool needs_vnet_hdr_swap;
     bool mtu_bypass_backend;
-    /* primary failover device is hidden*/
-    bool failover_primary_hidden;
     bool failover;
     DeviceListener primary_listener;
     Notifier migration_state;
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 0c5ec930356b..c81466f6efb7 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -36,8 +36,10 @@
 #include "qapi/qapi-types-migration.h"
 #include "qapi/qapi-events-migration.h"
 #include "hw/virtio/virtio-access.h"
+#include "migration/migration.h"
 #include "migration/misc.h"
 #include "standard-headers/linux/ethtool.h"
+#include "sysemu/runstate.h"
 #include "sysemu/sysemu.h"
 #include "trace.h"
 #include "monitor/qdev.h"
@@ -937,7 +939,6 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
 
     if (virtio_has_feature(features, VIRTIO_NET_F_STANDBY)) {
         qapi_event_send_failover_negotiated(n->netclient_name);
-        qatomic_set(&n->failover_primary_hidden, false);
         failover_add_primary(n, &err);
         if (err) {
             warn_report_err(err);
@@ -3225,7 +3226,6 @@ static bool failover_replug_primary(VirtIONet *n, DeviceState *dev,
         return false;
     }
     qdev_set_parent_bus(dev, primary_bus, &error_abort);
-    qatomic_set(&n->failover_primary_hidden, false);
     hotplug_ctrl = qdev_get_hotplug_handler(dev);
     if (hotplug_ctrl) {
         hotplug_handler_pre_plug(hotplug_ctrl, dev, &err);
@@ -3243,7 +3243,6 @@ out:
 
 static void virtio_net_handle_migration_primary(VirtIONet *n, MigrationState *s)
 {
-    bool should_be_hidden;
     Error *err = NULL;
     DeviceState *dev = failover_find_primary_device(n);
 
@@ -3251,13 +3250,10 @@ static void virtio_net_handle_migration_primary(VirtIONet *n, MigrationState *s)
         return;
     }
 
-    should_be_hidden = qatomic_read(&n->failover_primary_hidden);
-
-    if (migration_in_setup(s) && !should_be_hidden) {
+    if (migration_in_setup(s)) {
         if (failover_unplug_primary(n, dev)) {
             vmstate_unregister(VMSTATE_IF(dev), qdev_get_vmsd(dev), dev);
             qapi_event_send_unplug_primary(dev->id);
-            qatomic_set(&n->failover_primary_hidden, true);
         } else {
             warn_report("couldn't unplug primary device");
         }
@@ -3299,8 +3295,18 @@ static bool failover_hide_primary_device(DeviceListener *listener,
         return false;
     }
 
-    /* failover_primary_hidden is set during feature negotiation */
-    return qatomic_read(&n->failover_primary_hidden);
+    if (runstate_check(RUN_STATE_PRELAUNCH)) {
+        /* hide the failover primary on src on startup */
+        return true;
+    }
+
+    if (runstate_check(RUN_STATE_INMIGRATE) &&
+        migration_incoming_get_current()->state == MIGRATION_STATUS_NONE) {
+        /* hide the failover primary on dest on startup */
+        return true;
+    }
+
+    return false;
 }
 
 static void virtio_net_device_realize(DeviceState *dev, Error **errp)
@@ -3338,7 +3344,6 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
 
     if (n->failover) {
         n->primary_listener.hide_device = failover_hide_primary_device;
-        qatomic_set(&n->failover_primary_hidden, true);
         device_listener_register(&n->primary_listener);
         n->migration_state.notify = virtio_net_migration_state_notifier;
         add_migration_state_change_notifier(&n->migration_state);
-- 
2.31.1

[RFC PATCH v2 4/8] failover: pci: move failover hotplug/unplug code into pci subsystem

[Laurent Vivier]

failover allows a VFIO device to be unplugged on migration to switch
to the standby device, a virtio-net device.

Failover relies on PCI ability to hotplug/unplug a card (the VFIO
one) but all the code is implemented in virtio-net device that
is not a PCI device (even not in virtio-net-pci that is the bridge
between the PCI bus and the virtio-net device)

This patch moves all the failover PCI related code to the PCI
sub-system and keeps only in virtio-net the code related to the
failover (that is implemented in the virtio-net driver in the kernel)

There are several parts involved in the PCI device hotplug/unplug:

- the migration state notifier

  originally virtio_net_handle_migration_primary(), moved to hw/pci/pci.c
  as pci_dev_handle_migration(). On source, this function unplugs the card
  on migration setup and plugs it back in case of failure.
  (helpers are pci_dev_migration_unplug() and pci_dev_migration_replug(),
   previously failover_unplug_primary() and failover_replug_primary()).

- the hide device listener

  originally failover_hide_primary_device(), moved to hw/pci/pci.c
  as pci_dev_hide_device().
  This function is called by qdev_device_add() to know if a device
  must be hidden or not. To do that, the function checks the command line,
  if the device doesn't have a "failover_pair_id" it is not hidden. Otherwise
  it is hidden at startup on source and destination and it is only plugged by
  a compatible virtio-net device).

- the unplug pending checking functions

  originally primary_unplug_pending() and dev_unplug_pending(),
  moved to hw/pci/pci.c as bus_unplug_pending() and
  pci_device_unplug_pending()

  bus_unplug_pending() is called during the migration to check
  if the PCI device has been unplugged before starting to migrate
  data. It has been moved from vmstate_virtio_net to vmstate_pcibus.
  We can't move it to vmstate_pci_device because the device vmstate is
  removed before to start the migration (we don't migrate the device we
  unplug).

  pci_device_unplug_pending() is called by bus_unplug_pending() to
  check if an unplug is pending for a given device (this is a class
  function) of the bus.

  For a given PCI bus, bus_unplug_pending() will call
  pci_device_unplug_pending() for each available bus slot.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 include/hw/pci/pci.h           |   4 +
 include/hw/virtio/virtio-net.h |   2 -
 include/hw/virtio/virtio.h     |   1 -
 hw/net/virtio-net.c            | 156 -------------------------------
 hw/pci/pci.c                   | 166 ++++++++++++++++++++++++++++++++-
 5 files changed, 169 insertions(+), 160 deletions(-)

diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index d0f4266e3725..d35214144d1b 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -241,6 +241,9 @@ struct PCIDeviceClass {
 
     /* rom bar */
     const char *romfile;
+
+    DeviceListener listener;
+    bool (*dev_unplug_pending)(void *opaque);
 };
 
 typedef void (*PCIINTxRoutingNotifier)(PCIDevice *dev);
@@ -359,6 +362,7 @@ struct PCIDevice {
 
     /* ID of standby device in net_failover pair */
     char *failover_pair_id;
+    Notifier migration_state;
     uint32_t acpi_index;
 };
 
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index 21d8c3aa5f3a..d8161b61cc9e 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -206,8 +206,6 @@ struct VirtIONet {
     bool needs_vnet_hdr_swap;
     bool mtu_bypass_backend;
     bool failover;
-    DeviceListener primary_listener;
-    Notifier migration_state;
     VirtioNetRssData rss_data;
     struct NetRxPkt *rx_pkt;
     struct EBPFRSSContext ebpf_rss;
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 8bab9cfb7507..c9d26d5daa43 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -159,7 +159,6 @@ struct VirtioDeviceClass {
      */
     int (*post_load)(VirtIODevice *vdev);
     const VMStateDescription *vmsd;
-    bool (*primary_unplug_pending)(void *opaque);
 };
 
 void virtio_instance_init_common(Object *proxy_obj, void *data,
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index c81466f6efb7..35e3d024f8d6 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -36,10 +36,8 @@
 #include "qapi/qapi-types-migration.h"
 #include "qapi/qapi-events-migration.h"
 #include "hw/virtio/virtio-access.h"
-#include "migration/migration.h"
 #include "migration/misc.h"
 #include "standard-headers/linux/ethtool.h"
-#include "sysemu/runstate.h"
 #include "sysemu/sysemu.h"
 #include "trace.h"
 #include "monitor/qdev.h"
@@ -3188,127 +3186,6 @@ void virtio_net_set_netclient_name(VirtIONet *n, const char *name,
     n->netclient_type = g_strdup(type);
 }
 
-static bool failover_unplug_primary(VirtIONet *n, DeviceState *dev)
-{
-    HotplugHandler *hotplug_ctrl;
-    PCIDevice *pci_dev;
-    Error *err = NULL;
-
-    hotplug_ctrl = qdev_get_hotplug_handler(dev);
-    if (hotplug_ctrl) {
-        pci_dev = PCI_DEVICE(dev);
-        pci_dev->partially_hotplugged = true;
-        hotplug_handler_unplug_request(hotplug_ctrl, dev, &err);
-        if (err) {
-            error_report_err(err);
-            return false;
-        }
-    } else {
-        return false;
-    }
-    return true;
-}
-
-static bool failover_replug_primary(VirtIONet *n, DeviceState *dev,
-                                    Error **errp)
-{
-    Error *err = NULL;
-    HotplugHandler *hotplug_ctrl;
-    PCIDevice *pdev = PCI_DEVICE(dev);
-    BusState *primary_bus;
-
-    if (!pdev->partially_hotplugged) {
-        return true;
-    }
-    primary_bus = dev->parent_bus;
-    if (!primary_bus) {
-        error_setg(errp, "virtio_net: couldn't find primary bus");
-        return false;
-    }
-    qdev_set_parent_bus(dev, primary_bus, &error_abort);
-    hotplug_ctrl = qdev_get_hotplug_handler(dev);
-    if (hotplug_ctrl) {
-        hotplug_handler_pre_plug(hotplug_ctrl, dev, &err);
-        if (err) {
-            goto out;
-        }
-        hotplug_handler_plug(hotplug_ctrl, dev, &err);
-    }
-    pdev->partially_hotplugged = false;
-
-out:
-    error_propagate(errp, err);
-    return !err;
-}
-
-static void virtio_net_handle_migration_primary(VirtIONet *n, MigrationState *s)
-{
-    Error *err = NULL;
-    DeviceState *dev = failover_find_primary_device(n);
-
-    if (!dev) {
-        return;
-    }
-
-    if (migration_in_setup(s)) {
-        if (failover_unplug_primary(n, dev)) {
-            vmstate_unregister(VMSTATE_IF(dev), qdev_get_vmsd(dev), dev);
-            qapi_event_send_unplug_primary(dev->id);
-        } else {
-            warn_report("couldn't unplug primary device");
-        }
-    } else if (migration_has_failed(s)) {
-        /* We already unplugged the device let's plug it back */
-        if (!failover_replug_primary(n, dev, &err)) {
-            if (err) {
-                error_report_err(err);
-            }
-        }
-    }
-}
-
-static void virtio_net_migration_state_notifier(Notifier *notifier, void *data)
-{
-    MigrationState *s = data;
-    VirtIONet *n = container_of(notifier, VirtIONet, migration_state);
-    virtio_net_handle_migration_primary(n, s);
-}
-
-static bool failover_hide_primary_device(DeviceListener *listener,
-                                         QemuOpts *device_opts, Error **errp)
-{
-    VirtIONet *n = container_of(listener, VirtIONet, primary_listener);
-    const char *standby_id;
-
-    if (!device_opts) {
-        return false;
-    }
-    standby_id = qemu_opt_get(device_opts, "failover_pair_id");
-    if (standby_id == NULL) {
-        return false;
-    }
-    if (device_opts->id == NULL) {
-        error_setg(errp, "Device with failover_pair_id don't have id");
-        return true;
-    }
-    if (g_strcmp0(standby_id, n->netclient_name) != 0) {
-        return false;
-    }
-
-    if (runstate_check(RUN_STATE_PRELAUNCH)) {
-        /* hide the failover primary on src on startup */
-        return true;
-    }
-
-    if (runstate_check(RUN_STATE_INMIGRATE) &&
-        migration_incoming_get_current()->state == MIGRATION_STATUS_NONE) {
-        /* hide the failover primary on dest on startup */
-        return true;
-    }
-
-    return false;
-}
-
 static void virtio_net_device_realize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
@@ -3343,10 +3220,6 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
     }
 
     if (n->failover) {
-        n->primary_listener.hide_device = failover_hide_primary_device;
-        device_listener_register(&n->primary_listener);
-        n->migration_state.notify = virtio_net_migration_state_notifier;
-        add_migration_state_change_notifier(&n->migration_state);
         n->host_features |= (1ULL << VIRTIO_NET_F_STANDBY);
     }
 
@@ -3492,11 +3365,6 @@ static void virtio_net_device_unrealize(DeviceState *dev)
     g_free(n->mac_table.macs);
     g_free(n->vlans);
 
-    if (n->failover) {
-        device_listener_unregister(&n->primary_listener);
-        remove_migration_state_change_notifier(&n->migration_state);
-    }
-
     max_queues = n->multiqueue ? n->max_queues : 1;
     for (i = 0; i < max_queues; i++) {
         virtio_net_del_queue(n, i);
@@ -3539,28 +3407,6 @@ static int virtio_net_pre_save(void *opaque)
     return 0;
 }
 
-static bool primary_unplug_pending(void *opaque)
-{
-    DeviceState *dev = opaque;
-    DeviceState *primary;
-    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-    VirtIONet *n = VIRTIO_NET(vdev);
-
-    if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_STANDBY)) {
-        return false;
-    }
-    primary = failover_find_primary_device(n);
-    return primary ? primary->pending_deleted_event : false;
-}
-
-static bool dev_unplug_pending(void *opaque)
-{
-    DeviceState *dev = opaque;
-    VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(dev);
-
-    return vdc->primary_unplug_pending(dev);
-}
-
 static const VMStateDescription vmstate_virtio_net = {
     .name = "virtio-net",
     .minimum_version_id = VIRTIO_NET_VM_VERSION,
@@ -3570,7 +3416,6 @@ static const VMStateDescription vmstate_virtio_net = {
         VMSTATE_END_OF_LIST()
     },
     .pre_save = virtio_net_pre_save,
-    .dev_unplug_pending = dev_unplug_pending,
 };
 
 static Property virtio_net_properties[] = {
@@ -3662,7 +3507,6 @@ static void virtio_net_class_init(ObjectClass *klass, void *data)
     vdc->legacy_features |= (0x1 << VIRTIO_NET_F_GSO);
     vdc->post_load = virtio_net_post_load_virtio;
     vdc->vmsd = &vmstate_virtio_net_device;
-    vdc->primary_unplug_pending = primary_unplug_pending;
 }
 
 static const TypeInfo virtio_net_info = {
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 23d2ae2ab232..f849945e2819 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -33,11 +33,15 @@
 #include "hw/pci/pci_host.h"
 #include "hw/qdev-properties.h"
 #include "hw/qdev-properties-system.h"
+#include "migration/migration.h"
+#include "migration/misc.h"
 #include "migration/qemu-file-types.h"
 #include "migration/vmstate.h"
 #include "monitor/monitor.h"
+#include "monitor/qdev.h"
 #include "net/net.h"
 #include "sysemu/numa.h"
+#include "sysemu/runstate.h"
 #include "sysemu/sysemu.h"
 #include "hw/loader.h"
 #include "qemu/error-report.h"
@@ -47,8 +51,10 @@
 #include "hw/pci/msix.h"
 #include "hw/hotplug.h"
 #include "hw/boards.h"
+#include "hw/virtio/virtio-net.h" /* for failover */
 #include "qapi/error.h"
 #include "qapi/qapi-commands-pci.h"
+#include "qapi/qapi-events-migration.h"
 #include "qemu/cutils.h"
 
 //#define DEBUG_PCI
@@ -82,6 +88,28 @@ static Property pci_props[] = {
     DEFINE_PROP_END_OF_LIST()
 };
 
+static bool bus_unplug_pending(void *opaque)
+{
+    DeviceState *dev = opaque;
+    PCIBus *bus = PCI_BUS(dev);
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
+        PCIDeviceClass *pc;
+
+        if (bus->devices[i] == NULL) {
+            continue;
+        }
+
+        pc = PCI_DEVICE_GET_CLASS(bus->devices[i]);
+        if (pc->dev_unplug_pending &&
+            pc->dev_unplug_pending(DEVICE(bus->devices[i]))) {
+            return true;
+        }
+    }
+    return false;
+}
+
 static const VMStateDescription vmstate_pcibus = {
     .name = "PCIBUS",
     .version_id = 1,
@@ -92,7 +120,8 @@ static const VMStateDescription vmstate_pcibus = {
                              nirq, 0, vmstate_info_int32,
                              int32_t),
         VMSTATE_END_OF_LIST()
-    }
+    },
+    .dev_unplug_pending = bus_unplug_pending,
 };
 
 static void pci_init_bus_master(PCIDevice *pci_dev)
@@ -1171,6 +1200,10 @@ static void pci_qdev_unrealize(DeviceState *dev)
     PCIDevice *pci_dev = PCI_DEVICE(dev);
     PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
 
+    if (pci_dev->failover_pair_id) {
+        remove_migration_state_change_notifier(&pci_dev->migration_state);
+    }
+
     pci_unregister_io_regions(pci_dev);
     pci_del_option_rom(pci_dev);
 
@@ -2116,6 +2149,123 @@ PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn)
     return bus->devices[devfn];
 }
 
+static bool pci_dev_migration_unplug(PCIDevice *pci_dev)
+{
+    HotplugHandler *hotplug_ctrl;
+    DeviceState *dev = &pci_dev->qdev;
+    Error *err = NULL;
+
+    hotplug_ctrl = qdev_get_hotplug_handler(dev);
+    if (hotplug_ctrl) {
+        pci_dev->partially_hotplugged = true;
+        hotplug_handler_unplug_request(hotplug_ctrl, dev, &err);
+        if (err) {
+            error_report_err(err);
+            return false;
+        }
+    } else {
+        return false;
+    }
+    return true;
+}
+
+static bool pci_dev_migration_replug(PCIDevice *pci_dev, Error **errp)
+{
+    Error *err = NULL;
+    HotplugHandler *hotplug_ctrl;
+    DeviceState *dev = &pci_dev->qdev;
+    BusState *primary_bus;
+
+    if (!pci_dev->partially_hotplugged) {
+        return true;
+    }
+    primary_bus = dev->parent_bus;
+    if (!primary_bus) {
+        error_setg(errp, "virtio_net: couldn't find primary bus");
+        return false;
+    }
+    qdev_set_parent_bus(dev, primary_bus, &error_abort);
+    hotplug_ctrl = qdev_get_hotplug_handler(dev);
+    if (hotplug_ctrl) {
+        hotplug_handler_pre_plug(hotplug_ctrl, dev, &err);
+        if (err) {
+            goto out;
+        }
+        hotplug_handler_plug(hotplug_ctrl, dev, &err);
+    }
+    pci_dev->partially_hotplugged = false;
+
+out:
+    error_propagate(errp, err);
+    return !err;
+}
+
+static void pci_dev_handle_migration(PCIDevice *pci_dev, MigrationState *s)
+{
+    Error *err = NULL;
+    DeviceState *dev = &pci_dev->qdev;
+
+    if (migration_in_setup(s)) {
+        if (pci_dev_migration_unplug(pci_dev)) {
+            vmstate_unregister(VMSTATE_IF(dev), qdev_get_vmsd(dev), dev);
+            qapi_event_send_unplug_primary(dev->id);
+        } else {
+            warn_report("couldn't unplug primary device");
+        }
+    } else if (migration_has_failed(s)) {
+        /* We already unplugged the device let's plug it back */
+        if (!pci_dev_migration_replug(pci_dev, &err)) {
+            if (err) {
+                error_report_err(err);
+            }
+        }
+    }
+}
+
+static void pci_dev_migration_state_notifier(Notifier *notifier, void *data)
+{
+    MigrationState *s = data;
+    PCIDevice *pci_dev = container_of(notifier, PCIDevice, migration_state);
+    pci_dev_handle_migration(pci_dev, s);
+}
+
+static bool pci_dev_hide_device(DeviceListener *listener,
+                                QemuOpts *device_opts, Error **errp)
+{
+    const char *opt;
+    DeviceState *d;
+
+    opt = qemu_opt_get(device_opts, "failover_pair_id");
+    if (opt) {
+        if (device_opts->id == NULL) {
+            error_setg(errp, "Device with failover_pair_id don't have id");
+            return true;
+        }
+        d =  qdev_find_recursive(sysbus_get_default(), opt);
+        if (d == NULL) {
+            /*
+             * if the the virtio-net device is not plugged it can be
+             * plugged later, and this device will be added to the failover
+             */
+            return false;
+        }
+
+        if (runstate_check(RUN_STATE_PRELAUNCH)) {
+            /* hide the failover primary on src on startup */
+            return true;
+        }
+
+        if (runstate_check(RUN_STATE_INMIGRATE) &&
+            migration_incoming_get_current()->state == MIGRATION_STATUS_NONE) {
+            /* hide the failover primary on dest on startup */
+            return true;
+        }
+        return false;
+    }
+
+    return false;
+}
+
 static void pci_qdev_realize(DeviceState *qdev, Error **errp)
 {
     PCIDevice *pci_dev = (PCIDevice *)qdev;
@@ -2175,6 +2325,8 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
             return;
         }
         qdev->allow_unplug_during_migration = true;
+        pci_dev->migration_state.notify = pci_dev_migration_state_notifier;
+        add_migration_state_change_notifier(&pci_dev->migration_state);
     }
 
     /* rom loading */
@@ -2684,14 +2836,26 @@ MemoryRegion *pci_address_space_io(PCIDevice *dev)
     return pci_get_bus(dev)->address_space_io;
 }
 
+static bool pci_device_unplug_pending(void *opaque)
+{
+    DeviceState *dev = opaque;
+
+    return dev->pending_deleted_event;
+}
+
 static void pci_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
+    PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
 
     k->realize = pci_qdev_realize;
     k->unrealize = pci_qdev_unrealize;
     k->bus_type = TYPE_PCI_BUS;
     device_class_set_props(k, pci_props);
+    pc->dev_unplug_pending = pci_device_unplug_pending;
+
+    pc->listener.hide_device = pci_dev_hide_device;
+    device_listener_register(&pc->listener);
 }
 
 static void pci_device_class_base_init(ObjectClass *klass, void *data)
-- 
2.31.1

[RFC PATCH v2 5/8] failover: hide the PCI device if the virtio-net device is not present

[Laurent Vivier]

We can plug either the virtio-net device first or the PCI device
first.

If we plug the PCI device first, QEMU checks the failover_pair_id
but doesn't hide the device if the virtio-net device is not present.

This is a problem because if the virtio-net device is not plugged
and the machine is migrated the VFIO device doesn't prevent anymore
the migration and the machine is migrated with the VFIO device.

And of course, on destination the guest reboots because VFIO device
cannot be migrated.

We can fix the problem by hiding the PCI device with a failover_pair_id
device that is not present. In this case, if the machine is migrated the
VFIO device is not migrated, and if the virtio-net device is plugged, the
PCI device is plugged and can be migrated using the failover mechanism.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 hw/pci/pci.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index f849945e2819..dd5e95216abf 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2244,10 +2244,11 @@ static bool pci_dev_hide_device(DeviceListener *listener,
         d =  qdev_find_recursive(sysbus_get_default(), opt);
         if (d == NULL) {
             /*
-             * if the the virtio-net device is not plugged it can be
-             * plugged later, and this device will be added to the failover
+             * PCI device has a pair id, but the virtio-net device is not
+             * plugged: hide it, it will be plugged later when the virtio-net
+             * device will be plugged
              */
-            return false;
+            return true;
         }
 
         if (runstate_check(RUN_STATE_PRELAUNCH)) {
-- 
2.31.1

[RFC PATCH v2 6/8] failover: pci: unregister ROM on unplug

[Laurent Vivier]

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

This simple change allows a simulated device like e1000e to be tested
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>
---
 hw/pci/pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index dd5e95216abf..10b875fdfad1 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2208,6 +2208,7 @@ static void pci_dev_handle_migration(PCIDevice *pci_dev, MigrationState *s)
     if (migration_in_setup(s)) {
         if (pci_dev_migration_unplug(pci_dev)) {
             vmstate_unregister(VMSTATE_IF(dev), qdev_get_vmsd(dev), dev);
+            pci_del_option_rom(pci_dev);
             qapi_event_send_unplug_primary(dev->id);
         } else {
             warn_report("couldn't unplug primary device");
-- 
2.31.1

[RFC PATCH v2 7/8] pci: automatically unplug a PCI card before migration

[Laurent Vivier]

We have moved all the functions needed by failover to unplug a card to the
PCI subsystem.

A side effect of this change is we can implement automatic hotplug/unplug
of any PCI card during migration without using a failover virtio-net card.
For that, we need to introduce a new PCI device property,
"unplug-on-migration", we can set to "true" or "on" if we want QEMU unplugs
the card before the migration and plugs it back on the destination side
after the migration.

We modify the pci_dev_hide_device() function to check for the
"unplug-on-migration" property on the command line.
If it is present, the device is hidden on startup only on the destination
side and it will be unplugged before the migration.

To implement the "unplug-on-migration" property, we add a post_load
function in vmstate_pcibus to hotplug the card after the migration
(bus_post_load() and pci_dev_replug_on_migration()). This is not
needed with virtio-net failover because the device is plugged back
by the virtio-net device during the features migration
(VIRTIO_NET_F_STANDBY)

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 include/hw/pci/pci.h |  1 +
 hw/pci/pci.c         | 76 ++++++++++++++++++++++++++++++++++++++------
 hw/vfio/pci.c        |  2 +-
 3 files changed, 69 insertions(+), 10 deletions(-)

diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index d35214144d1b..e02d965c064f 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -362,6 +362,7 @@ struct PCIDevice {
 
     /* ID of standby device in net_failover pair */
     char *failover_pair_id;
+    bool unplug_on_migration;
     Notifier migration_state;
     uint32_t acpi_index;
 };
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 10b875fdfad1..31ab80b81b6b 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -82,6 +82,8 @@ static Property pci_props[] = {
                     QEMU_PCIE_LNKSTA_DLLLA_BITNR, true),
     DEFINE_PROP_BIT("x-pcie-extcap-init", PCIDevice, cap_present,
                     QEMU_PCIE_EXTCAP_INIT_BITNR, true),
+    DEFINE_PROP_BOOL("unplug-on-migration", PCIDevice,
+                     unplug_on_migration, false),
     DEFINE_PROP_STRING("failover_pair_id", PCIDevice,
                        failover_pair_id),
     DEFINE_PROP_UINT32("acpi-index",  PCIDevice, acpi_index, 0),
@@ -110,6 +112,45 @@ static bool bus_unplug_pending(void *opaque)
     return false;
 }
 
+static int pci_dev_replug_on_migration(void *opaque, QemuOpts *opts,
+                                       Error **errp)
+{
+    Error *err = NULL;
+    const char *bus_name = opaque;
+    const char *opt;
+    DeviceState *dev;
+
+    if (g_strcmp0(qemu_opt_get(opts, "bus"), bus_name)) {
+        return 0;
+    }
+
+    opt = qemu_opt_get(opts, "unplug-on-migration");
+    if (g_strcmp0(opt, "on") && g_strcmp0(opt, "true")) {
+        return 0;
+    }
+    dev = qdev_device_add(opts, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return 1;
+    }
+    object_unref(OBJECT(dev));
+    return 0;
+}
+
+static int bus_post_load(void *opaque, int version_id)
+{
+    Error *err = NULL;
+    PCIBus *bus = opaque;
+
+    if (qemu_opts_foreach(qemu_find_opts("device"),
+                          pci_dev_replug_on_migration, bus->qbus.name, &err)) {
+        error_report_err(err);
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
 static const VMStateDescription vmstate_pcibus = {
     .name = "PCIBUS",
     .version_id = 1,
@@ -122,6 +163,7 @@ static const VMStateDescription vmstate_pcibus = {
         VMSTATE_END_OF_LIST()
     },
     .dev_unplug_pending = bus_unplug_pending,
+    .post_load = bus_post_load,
 };
 
 static void pci_init_bus_master(PCIDevice *pci_dev)
@@ -1200,7 +1242,7 @@ static void pci_qdev_unrealize(DeviceState *dev)
     PCIDevice *pci_dev = PCI_DEVICE(dev);
     PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
 
-    if (pci_dev->failover_pair_id) {
+    if (pci_dev->unplug_on_migration) {
         remove_migration_state_change_notifier(&pci_dev->migration_state);
     }
 
@@ -2265,6 +2307,15 @@ static bool pci_dev_hide_device(DeviceListener *listener,
         return false;
     }
 
+    opt = qemu_opt_get(device_opts, "unplug-on-migration");
+    if (g_strcmp0(opt, "on") == 0 || g_strcmp0(opt, "true") == 0) {
+        if (runstate_check(RUN_STATE_INMIGRATE)) {
+            return migration_incoming_get_current()->state !=
+                   MIGRATION_STATUS_ACTIVE;
+        }
+        return false;
+    }
+
     return false;
 }
 
@@ -2290,6 +2341,10 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
         pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
     }
 
+    if (pci_dev->failover_pair_id) {
+        pci_dev->unplug_on_migration = true;
+    }
+
     pci_dev = do_pci_register_device(pci_dev,
                                      object_get_typename(OBJECT(qdev)),
                                      pci_dev->devfn, errp);
@@ -2306,12 +2361,6 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
     }
 
     if (pci_dev->failover_pair_id) {
-        if (!pci_bus_is_express(pci_get_bus(pci_dev))) {
-            error_setg(errp, "failover primary device must be on "
-                             "PCIExpress bus");
-            pci_qdev_unrealize(DEVICE(pci_dev));
-            return;
-        }
         class_id = pci_get_word(pci_dev->config + PCI_CLASS_DEVICE);
         if (class_id != PCI_CLASS_NETWORK_ETHERNET) {
             error_setg(errp, "failover primary device is not an "
@@ -2319,10 +2368,19 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
             pci_qdev_unrealize(DEVICE(pci_dev));
             return;
         }
+    }
+
+    if (pci_dev->unplug_on_migration) {
+        if (!pci_bus_is_express(pci_get_bus(pci_dev))) {
+            error_setg(errp, "Unplugged device on migration must be on "
+                             "PCIExpress bus");
+            pci_qdev_unrealize(DEVICE(pci_dev));
+            return;
+        }
         if ((pci_dev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION)
             || (PCI_FUNC(pci_dev->devfn) != 0)) {
-            error_setg(errp, "failover: primary device must be in its own "
-                              "PCI slot");
+            error_setg(errp, "Unplugged device on migration must be in its "
+                              "own PCI slot");
             pci_qdev_unrealize(DEVICE(pci_dev));
             return;
         }
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index e1ea1d8a23b5..187e1b58a4d9 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3080,7 +3080,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
         }
     }
 
-    if (!pdev->failover_pair_id) {
+    if (!pdev->unplug_on_migration) {
         ret = vfio_migration_probe(&vdev->vbasedev, errp);
         if (ret) {
             error_report("%s: Migration disabled", vdev->vbasedev.name);
-- 
2.31.1

[RFC PATCH v2 8/8] failover: qemu-opts: manage hidden device list

[Laurent Vivier]

failover relies on the command line parameters to store and detect
failover devices because while the device is hidden it doesn't appears
in qdev objects and so we don't have the list anywhere else.

But this doesn't work if the the device is hotplugged because it is
not added to the qemu opts list (and the opts list memory is released
after the qdev_device_add() if the device object is not created as it is
the case when it is hidden).

It seems cleaner to manage our own list of hidden devices.

To do that, this patch adds some qemu_opts functions to store the opts
list of the device when it is hidden. This list will be used to actually
plug the device when it will be enabled for the guest.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 include/qemu/option.h |  4 +++
 hw/core/qdev.c        |  1 +
 hw/net/virtio-net.c   |  5 ++-
 hw/pci/pci.c          |  4 +--
 util/qemu-option.c    | 82 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 91 insertions(+), 5 deletions(-)

diff --git a/include/qemu/option.h b/include/qemu/option.h
index 306bf0757509..d44550f02542 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -150,4 +150,8 @@ QDict *keyval_parse(const char *params, const char *implied_key,
                     bool *help, Error **errp);
 void keyval_merge(QDict *old, const QDict *new, Error **errp);
 
+int qemu_opts_hidden_device_foreach(qemu_opts_loopfunc func,
+                                    void *opaque, Error **errp);
+QemuOpts *qemu_opts_hidden_device_find(const char *id);
+void qemu_opts_store_hidden_device(QemuOpts *opts);
 #endif
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 13f4c1e696bf..f402309a3af9 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -218,6 +218,7 @@ bool qdev_should_hide_device(QemuOpts *opts, Error **errp)
     QTAILQ_FOREACH(listener, &device_listeners, link) {
         if (listener->hide_device) {
             if (listener->hide_device(listener, opts, errp)) {
+                qemu_opts_store_hidden_device(opts);
                 return true;
             }
         }
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 35e3d024f8d6..dc971bc9885b 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -831,8 +831,7 @@ static char *failover_find_primary_device_id(VirtIONet *n)
     FailoverId fid;
 
     fid.n = n;
-    if (!qemu_opts_foreach(qemu_find_opts("device"),
-                           failover_set_primary, &fid, &err)) {
+    if (!qemu_opts_hidden_device_foreach(failover_set_primary, &fid, &err)) {
         return NULL;
     }
     return fid.id;
@@ -874,7 +873,7 @@ static void failover_add_primary(VirtIONet *n, Error **errp)
                           " failover_pair_id=%s\n", n->netclient_name);
         return;
     }
-    opts = qemu_opts_find(qemu_find_opts("device"), id);
+    opts = qemu_opts_hidden_device_find(id);
     g_assert(opts); /* cannot be NULL because id was found using opts list */
     dev = qdev_device_add(opts, &err);
     if (err) {
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 31ab80b81b6b..d222623a68b2 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -142,8 +142,8 @@ static int bus_post_load(void *opaque, int version_id)
     Error *err = NULL;
     PCIBus *bus = opaque;
 
-    if (qemu_opts_foreach(qemu_find_opts("device"),
-                          pci_dev_replug_on_migration, bus->qbus.name, &err)) {
+    if (qemu_opts_hidden_device_foreach(pci_dev_replug_on_migration,
+                                        bus->qbus.name, &err)) {
         error_report_err(err);
         return -EINVAL;
     }
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 61cb4a97bdb6..90bdec030624 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -37,6 +37,8 @@
 #include "qemu/id.h"
 #include "qemu/help_option.h"
 
+static QTAILQ_HEAD(, QemuOpts) hidden_devices =
+                               QTAILQ_HEAD_INITIALIZER(hidden_devices);
 /*
  * Extracts the name of an option from the parameter string (@p points at the
  * first byte of the option name)
@@ -1224,3 +1226,83 @@ QemuOptsList *qemu_opts_append(QemuOptsList *dst,
 
     return dst;
 }
+
+/* create a copy of an opts */
+static QemuOpts *qemu_opts_duplicate(QemuOpts *opts)
+{
+    QemuOpts *new;
+    QemuOpt *opt;
+
+    new = g_malloc0(sizeof(*new));
+    new->id = g_strdup(opts->id);
+    new->list = opts->list;
+
+    QTAILQ_INIT(&new->head);
+
+    QTAILQ_FOREACH_REVERSE(opt, &opts->head, next) {
+        QemuOpt *new_opt = g_malloc0(sizeof(*new_opt));
+
+        new_opt->name = g_strdup(opt->name);
+        new_opt->str = g_strdup(opt->str);
+        new_opt->opts = new;
+        QTAILQ_INSERT_TAIL(&new->head, new_opt, next);
+    }
+
+    QTAILQ_INSERT_TAIL(&new->list->head, new, next);
+
+    return new;
+}
+
+/*
+ * For each member of the hidded device list,
+ * call @func(@opaque, name, value, @errp).
+ * @func() may store an Error through @errp, but must return non-zero then.
+ * When @func() returns non-zero, break the loop and return that value.
+ * Return zero when the loop completes.
+ */
+int qemu_opts_hidden_device_foreach(qemu_opts_loopfunc func,
+                                    void *opaque, Error **errp)
+{
+    QemuOpts *hidden;
+    int rc = 0;
+
+    QTAILQ_FOREACH(hidden, &hidden_devices, next) {
+        rc = func(opaque, hidden, errp);
+        if (rc) {
+            break;
+        }
+    }
+    return rc;
+}
+
+/* scan the list of hidden devices to find opts for the one with id @id */
+QemuOpts *qemu_opts_hidden_device_find(const char *id)
+{
+    QemuOpts *hidden;
+
+    QTAILQ_FOREACH(hidden, &hidden_devices, next) {
+        if (g_strcmp0(id, hidden->id) == 0) {
+            return hidden;
+        }
+    }
+
+    return NULL;
+}
+
+/* add the @opts to the list of hidden devices */
+void qemu_opts_store_hidden_device(QemuOpts *opts)
+{
+    QemuOpts *copy;
+
+    if (qemu_opts_hidden_device_find(opts->id)) {
+        return;
+    }
+
+    /*
+     * we need to duplicate opts because qmp_device_add() calls
+     * qemu_opts_del(opts) if the device is not added,
+     * and this is what happens when it is hidden
+     */
+    copy = qemu_opts_duplicate(opts);
+    QTAILQ_INSERT_TAIL(&hidden_devices, copy, next);
+}
-- 
2.31.1

Re: [RFC PATCH v2 1/8] qdev: add an Error parameter to the DeviceListener hide_device() function

[Juan Quintela]

Laurent Vivier <lvivier@redhat.com> wrote:
D> This allows an error to be reported to the caller of qdev_device_add()
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

Re: [RFC PATCH v2 2/8] qdev/qbus: remove failover specific code

[Juan Quintela]

Laurent Vivier <lvivier@redhat.com> wrote:
> Commit f3a850565693 ("qdev/qbus: add hidden device support") has
> introduced a generic way to hide a device but it has modified
> qdev_device_add() to check a specific option of the failover device,
> "failover_pair_id", before calling the generic mechanism.
>
> It's not needed (and not generic) to do that in qdev_device_add() because
> this is also checked by the failover_hide_primary_device() function that
> uses the generic mechanism to hide the device.
>
> Cc: Jens Freimann <jfreimann@redhat.com>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

I see why you want this change.  It was done the other way to make sure
that we only tried to hide the divec is there is a failover_pair_id
property.  You can't have both.

Re: [RFC PATCH v2 3/8] failover: virtio-net: remove failover_primary_hidden flag

[Juan Quintela]

Laurent Vivier <lvivier@redhat.com> wrote:
> We dont't need a flag to know if the primary device must be hidden, we
> can rely on the machine state:
> Device is hidden if the machine is in prelaunch state (src) or
> in inmigrate state with migration status set to none (dst).
> We don't need to check the flag in virtio_net_handle_migration_primary()
> because this function is only registered if the failover is enabled
> and the flag also set to false. This function also sets it back to
> true after a successful unplug during the migration but we don't
> need to know it is hidden because nothing will try to plug it back
> after the migration (except in case of error but the flag is set
> back to false).
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>


Much clear.  I was removing all the global failover variables, but
didn't finish the job.

Well done.

Re: [RFC PATCH v2 6/8] failover: pci: unregister ROM on unplug

[Juan Quintela]

Laurent Vivier <lvivier@redhat.com> wrote:
> The intend of failover is to allow a VM with a VFIO networking card to
> be migrated without disrupting the network operation by switching
> to a virtio-net device during the migration.
>
> This simple change allows a simulated device like e1000e to be tested
> 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>

Reviewed-by: Juan Quintela <quintela@redhat.com>

One could even defend that unpluging the device and *NOT* unpluging the
ROM is a bug, independently of failover, no?

Later, Juan.