[PATCH v6 0/6] tests/qtest: add some tests for virtio-net failover

[PATCH v6 0/6] tests/qtest: add some tests for virtio-net failover

[Laurent Vivier]

This series adds a qtest entry to test virtio-net failover feature.

We check following error cases:

- check missing id on device with failover_pair_id triggers an error
- check a primary device plugged on a bus that doesn't support hotplug
  triggers an error

We check the status of the machine before and after hotplugging cards and
feature negotiation:

- check we don't see the primary device at boot if failover is on
- check we see the primary device at boot if failover is off
- check we don't see the primary device if failover is on
  but failover_pair_id is not the one with on (I think this should be changed)
- check the primary device is plugged after the feature negotiation
- check the result if the primary device is plugged before standby device and
  vice-versa
- check the if the primary device is coldplugged and the standy device
  hotplugged and vice-versa
- check the migration triggers the unplug and the hotplug

There is one preliminary patch in the series:

- PATCH 1 introduces a function to enable PCI bridge.
  Failover needs to be plugged on a pcie-root-port and while
  the root port is not configured the cards behind it are not
  available

v6:
- manage more than 2 root ports
- add a function to check if a card is available or not
- check migration state
- add cancelled migration test cases
- rename tests

v5:
- re-add the wait-unplug test that has been removed from v4 by mistake.

v4:
- rely on query-migrate status to know the migration state rather than
  to wait the STOP event.
- remove the patch to add time out to qtest_qmp_eventwait()

v3:
- fix a bug with ACPI unplug and add the related test

v2:
- remove PATCH 1 that introduced a function that can be replaced by
  qobject_to_json_pretty() (Markus)
- Add migration to a file and from the file to check the card is
  correctly unplugged on the source, and hotplugged on the dest
- Add an ACPI call to eject the card as the kernel would do

Laurent Vivier (6):
  qtest/libqos: add a function to initialize secondary PCI buses
  tests/qtest: add some tests for virtio-net failover
  failover: fix unplug pending detection
  tests/libqtest: update virtio-net failover test
  test/libqtest: add some virtio-net failover migration cancelling tests
  tests/libqtest: add a migration test with two couples of failover
    devices

 hw/acpi/pcihp.c                   |   30 +-
 include/hw/pci/pci_bridge.h       |    8 +
 tests/qtest/libqos/pci.c          |  118 +++
 tests/qtest/libqos/pci.h          |    1 +
 tests/qtest/meson.build           |    3 +
 tests/qtest/virtio-net-failover.c | 1294 +++++++++++++++++++++++++++++
 6 files changed, 1451 insertions(+), 3 deletions(-)
 create mode 100644 tests/qtest/virtio-net-failover.c

-- 
2.33.1

[PATCH v6 1/6] qtest/libqos: add a function to initialize secondary PCI buses

[Laurent Vivier]

Scan the PCI devices to find bridge and set PCI_SECONDARY_BUS and
PCI_SUBORDINATE_BUS (algorithm from seabios)

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 include/hw/pci/pci_bridge.h |   8 +++
 tests/qtest/libqos/pci.c    | 118 ++++++++++++++++++++++++++++++++++++
 tests/qtest/libqos/pci.h    |   1 +
 3 files changed, 127 insertions(+)

diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h
index a94d350034bf..30691a6e5728 100644
--- a/include/hw/pci/pci_bridge.h
+++ b/include/hw/pci/pci_bridge.h
@@ -138,6 +138,7 @@ typedef struct PCIBridgeQemuCap {
     uint64_t mem_pref_64; /* Prefetchable memory to reserve (64-bit MMIO) */
 } PCIBridgeQemuCap;
 
+#define REDHAT_PCI_CAP_TYPE_OFFSET      3
 #define REDHAT_PCI_CAP_RESOURCE_RESERVE 1
 
 /*
@@ -152,6 +153,13 @@ typedef struct PCIResReserve {
     uint64_t mem_pref_64;
 } PCIResReserve;
 
+#define REDHAT_PCI_CAP_RES_RESERVE_BUS_RES     4
+#define REDHAT_PCI_CAP_RES_RESERVE_IO          8
+#define REDHAT_PCI_CAP_RES_RESERVE_MEM         16
+#define REDHAT_PCI_CAP_RES_RESERVE_PREF_MEM_32 20
+#define REDHAT_PCI_CAP_RES_RESERVE_PREF_MEM_64 24
+#define REDHAT_PCI_CAP_RES_RESERVE_CAP_SIZE    32
+
 int pci_bridge_qemu_reserve_cap_init(PCIDevice *dev, int cap_offset,
                                PCIResReserve res_reserve, Error **errp);
 
diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c
index e1e96189c821..3f0b18f4750b 100644
--- a/tests/qtest/libqos/pci.c
+++ b/tests/qtest/libqos/pci.c
@@ -13,6 +13,8 @@
 #include "qemu/osdep.h"
 #include "pci.h"
 
+#include "hw/pci/pci.h"
+#include "hw/pci/pci_bridge.h"
 #include "hw/pci/pci_regs.h"
 #include "qemu/host-utils.h"
 #include "qgraph.h"
@@ -99,6 +101,122 @@ void qpci_device_init(QPCIDevice *dev, QPCIBus *bus, QPCIAddress *addr)
     g_assert(!addr->device_id || device_id == addr->device_id);
 }
 
+static uint8_t qpci_find_resource_reserve_capability(QPCIDevice *dev)
+{
+    uint16_t device_id;
+    uint8_t cap = 0;
+
+    if (qpci_config_readw(dev, PCI_VENDOR_ID) != PCI_VENDOR_ID_REDHAT) {
+        return 0;
+    }
+
+    device_id = qpci_config_readw(dev, PCI_DEVICE_ID);
+
+    if (device_id != PCI_DEVICE_ID_REDHAT_PCIE_RP &&
+        device_id != PCI_DEVICE_ID_REDHAT_BRIDGE) {
+        return 0;
+    }
+
+    do {
+        cap = qpci_find_capability(dev, PCI_CAP_ID_VNDR, cap);
+    } while (cap &&
+             qpci_config_readb(dev, cap + REDHAT_PCI_CAP_TYPE_OFFSET) !=
+             REDHAT_PCI_CAP_RESOURCE_RESERVE);
+    if (cap) {
+        uint8_t cap_len = qpci_config_readb(dev, cap + PCI_CAP_FLAGS);
+        if (cap_len < REDHAT_PCI_CAP_RES_RESERVE_CAP_SIZE) {
+            return 0;
+        }
+    }
+    return cap;
+}
+
+static void qpci_secondary_buses_rec(QPCIBus *qbus, int bus, int *pci_bus)
+{
+    QPCIDevice *dev;
+    uint16_t class;
+    uint8_t pribus, secbus, subbus;
+    int i;
+
+    for (i = 0; i < 32; i++) {
+        dev = qpci_device_find(qbus, QPCI_DEVFN(bus + i, 0));
+        if (dev == NULL) {
+            continue;
+        }
+        class = qpci_config_readw(dev, PCI_CLASS_DEVICE);
+        if (class == PCI_CLASS_BRIDGE_PCI) {
+            qpci_config_writeb(dev, PCI_SECONDARY_BUS, 255);
+            qpci_config_writeb(dev, PCI_SUBORDINATE_BUS, 0);
+        }
+        g_free(dev);
+    }
+
+    for (i = 0; i < 32; i++) {
+        dev = qpci_device_find(qbus, QPCI_DEVFN(bus + i, 0));
+        if (dev == NULL) {
+            continue;
+        }
+        class = qpci_config_readw(dev, PCI_CLASS_DEVICE);
+        if (class != PCI_CLASS_BRIDGE_PCI) {
+            continue;
+        }
+
+        pribus = qpci_config_readb(dev, PCI_PRIMARY_BUS);
+        if (pribus != bus) {
+            qpci_config_writeb(dev, PCI_PRIMARY_BUS, bus);
+        }
+
+        secbus = qpci_config_readb(dev, PCI_SECONDARY_BUS);
+        (*pci_bus)++;
+        if (*pci_bus != secbus) {
+            secbus = *pci_bus;
+            qpci_config_writeb(dev, PCI_SECONDARY_BUS, secbus);
+        }
+
+        subbus = qpci_config_readb(dev, PCI_SUBORDINATE_BUS);
+        qpci_config_writeb(dev, PCI_SUBORDINATE_BUS, 255);
+
+        qpci_secondary_buses_rec(qbus, secbus << 5, pci_bus);
+
+        if (subbus != *pci_bus) {
+            uint8_t res_bus = *pci_bus;
+            uint8_t cap = qpci_find_resource_reserve_capability(dev);
+
+            if (cap) {
+                uint32_t tmp_res_bus;
+
+                tmp_res_bus = qpci_config_readl(dev, cap +
+                                            REDHAT_PCI_CAP_RES_RESERVE_BUS_RES);
+                if (tmp_res_bus != (uint32_t)-1) {
+                    res_bus = tmp_res_bus & 0xFF;
+                    if ((uint8_t)(res_bus + secbus) < secbus ||
+                        (uint8_t)(res_bus + secbus) < res_bus) {
+                        res_bus = 0;
+                    }
+                    if (secbus + res_bus > *pci_bus) {
+                        res_bus = secbus + res_bus;
+                    }
+                }
+            }
+            subbus = res_bus;
+            *pci_bus = res_bus;
+        }
+
+        qpci_config_writeb(dev, PCI_SUBORDINATE_BUS, subbus);
+        g_free(dev);
+    }
+}
+
+int qpci_secondary_buses_init(QPCIBus *bus)
+{
+    int last_bus = 0;
+
+    qpci_secondary_buses_rec(bus, 0, &last_bus);
+
+    return last_bus;
+}
+
+
 void qpci_device_enable(QPCIDevice *dev)
 {
     uint16_t cmd;
diff --git a/tests/qtest/libqos/pci.h b/tests/qtest/libqos/pci.h
index ee64fdecbda8..becb800f9e6a 100644
--- a/tests/qtest/libqos/pci.h
+++ b/tests/qtest/libqos/pci.h
@@ -81,6 +81,7 @@ void qpci_device_foreach(QPCIBus *bus, int vendor_id, int device_id,
                          void *data);
 QPCIDevice *qpci_device_find(QPCIBus *bus, int devfn);
 void qpci_device_init(QPCIDevice *dev, QPCIBus *bus, QPCIAddress *addr);
+int qpci_secondary_buses_init(QPCIBus *bus);
 
 bool qpci_has_buggy_msi(QPCIDevice *dev);
 bool qpci_check_buggy_msi(QPCIDevice *dev);
-- 
2.33.1

[PATCH v6 2/6] tests/qtest: add some tests for virtio-net failover

[Laurent Vivier]

Add test cases to test several error cases that must be
generated by invalid failover configuration.

Add a combination of coldplug and hotplug test cases to be
sure the primary is correctly managed according the
presence or not of the STANDBY feature.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 tests/qtest/meson.build           |   3 +
 tests/qtest/virtio-net-failover.c | 690 ++++++++++++++++++++++++++++++
 2 files changed, 693 insertions(+)
 create mode 100644 tests/qtest/virtio-net-failover.c

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index c9d8458062ff..6d66bf522156 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -22,6 +22,9 @@ qtests_generic = \
   (config_all_devices.has_key('CONFIG_VIRTIO_SCSI') ? ['fuzz-virtio-scsi-test'] : []) + \
   (config_all_devices.has_key('CONFIG_SB16') ? ['fuzz-sb16-test'] : []) + \
   (config_all_devices.has_key('CONFIG_SDHCI_PCI') ? ['fuzz-sdcard-test'] : []) + \
+  (config_all_devices.has_key('CONFIG_VIRTIO_NET') and \
+   config_all_devices.has_key('CONFIG_Q35') and \
+   config_all_devices.has_key('CONFIG_VIRTIO_PCI') ? ['virtio-net-failover'] : []) + \
   [
   'cdrom-test',
   'device-introspect-test',
diff --git a/tests/qtest/virtio-net-failover.c b/tests/qtest/virtio-net-failover.c
new file mode 100644
index 000000000000..f8f5fbb3c7fe
--- /dev/null
+++ b/tests/qtest/virtio-net-failover.c
@@ -0,0 +1,690 @@
+#include "qemu/osdep.h"
+#include "libqos/libqtest.h"
+#include "libqos/pci.h"
+#include "libqos/pci-pc.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qlist.h"
+#include "qapi/qmp/qjson.h"
+#include "libqos/malloc-pc.h"
+#include "libqos/virtio-pci.h"
+#include "hw/pci/pci.h"
+
+#define ACPI_PCIHP_ADDR_ICH9    0x0cc0
+#define PCI_EJ_BASE             0x0008
+
+#define BASE_MACHINE "-M q35 -nodefaults " \
+    "-device pcie-root-port,id=root0,addr=0x1,bus=pcie.0,chassis=1 " \
+    "-device pcie-root-port,id=root1,addr=0x2,bus=pcie.0,chassis=2 "
+
+#define MAC_PRIMARY0 "52:54:00:11:11:11"
+#define MAC_STANDBY0 "52:54:00:22:22:22"
+
+static QGuestAllocator guest_malloc;
+static QPCIBus *pcibus;
+
+static QTestState *machine_start(const char *args, int numbus)
+{
+    QTestState *qts;
+    QPCIDevice *dev;
+    int i;
+
+    qts = qtest_init(args);
+
+    pc_alloc_init(&guest_malloc, qts, 0);
+    pcibus = qpci_new_pc(qts, &guest_malloc);
+    g_assert(qpci_secondary_buses_init(pcibus) == numbus);
+
+    for (i = 0; i < numbus; i++) {
+        dev = qpci_device_find(pcibus, QPCI_DEVFN(i + 1, 0));
+        g_assert_nonnull(dev);
+
+        qpci_device_enable(dev);
+        qpci_iomap(dev, 4, NULL);
+
+        g_free(dev);
+    }
+
+    return qts;
+}
+
+static void machine_stop(QTestState *qts)
+{
+    qpci_free_pc(pcibus);
+    alloc_destroy(&guest_malloc);
+    qtest_quit(qts);
+}
+
+static void test_error_id(void)
+{
+    QTestState *qts;
+    QDict *resp;
+    QDict *err;
+
+    qts = machine_start(BASE_MACHINE
+                        "-device virtio-net,bus=root0,id=standby0,failover=on",
+                        2);
+
+    resp = qtest_qmp(qts, "{'execute': 'device_add',"
+                          "'arguments': {"
+                          "'driver': 'virtio-net',"
+                          "'bus': 'root1',"
+                          "'failover_pair_id': 'standby0'"
+                          "} }");
+    g_assert(qdict_haskey(resp, "error"));
+
+    err = qdict_get_qdict(resp, "error");
+    g_assert(qdict_haskey(err, "desc"));
+
+    g_assert_cmpstr(qdict_get_str(err, "desc"), ==,
+                    "Device with failover_pair_id needs to have id");
+
+    qobject_unref(resp);
+
+    machine_stop(qts);
+}
+
+static void test_error_pcie(void)
+{
+    QTestState *qts;
+    QDict *resp;
+    QDict *err;
+
+    qts = machine_start(BASE_MACHINE
+                        "-device virtio-net,bus=root0,id=standby0,failover=on",
+                        2);
+
+    resp = qtest_qmp(qts, "{'execute': 'device_add',"
+                          "'arguments': {"
+                          "'driver': 'virtio-net',"
+                          "'id': 'primary0',"
+                          "'bus': 'pcie.0',"
+                          "'failover_pair_id': 'standby0'"
+                          "} }");
+    g_assert(qdict_haskey(resp, "error"));
+
+    err = qdict_get_qdict(resp, "error");
+    g_assert(qdict_haskey(err, "desc"));
+
+    g_assert_cmpstr(qdict_get_str(err, "desc"), ==,
+                    "Bus 'pcie.0' does not support hotplugging");
+
+    qobject_unref(resp);
+
+    machine_stop(qts);
+}
+
+static QDict *find_device(QDict *bus, const char *name)
+{
+    const QObject *obj;
+    QList *devices;
+    QList *list;
+
+    devices = qdict_get_qlist(bus, "devices");
+    if (devices == NULL) {
+        return NULL;
+    }
+
+    list = qlist_copy(devices);
+    while ((obj = qlist_pop(list))) {
+        QDict *device;
+
+        device = qobject_to(QDict, obj);
+
+        if (qdict_haskey(device, "pci_bridge")) {
+            QDict *bridge;
+            QDict *bridge_device;
+
+            bridge = qdict_get_qdict(device, "pci_bridge");
+
+            if (qdict_haskey(bridge, "devices")) {
+                bridge_device = find_device(bridge, name);
+                if (bridge_device) {
+                    qobject_unref(list);
+                    return bridge_device;
+                }
+            }
+        }
+
+        if (!qdict_haskey(device, "qdev_id")) {
+            continue;
+        }
+
+        if (strcmp(qdict_get_str(device, "qdev_id"), name) == 0) {
+            qobject_ref(device);
+            qobject_unref(list);
+            return device;
+        }
+    }
+    qobject_unref(list);
+
+    return NULL;
+}
+
+static QDict *get_bus(QTestState *qts, int num)
+{
+    QObject *obj;
+    QDict *resp;
+    QList *ret;
+
+    resp = qtest_qmp(qts, "{ 'execute': 'query-pci' }");
+    g_assert(qdict_haskey(resp, "return"));
+
+    ret = qdict_get_qlist(resp, "return");
+    g_assert_nonnull(ret);
+
+    while ((obj = qlist_pop(ret))) {
+        QDict *bus;
+
+        bus = qobject_to(QDict, obj);
+        if (!qdict_haskey(bus, "bus")) {
+            continue;
+        }
+        if (qdict_get_int(bus, "bus") == num) {
+            qobject_ref(bus);
+            qobject_unref(resp);
+            return bus;
+        }
+    }
+    qobject_unref(resp);
+
+    return NULL;
+}
+
+static char *get_mac(QTestState *qts, const char *name)
+{
+    QDict *resp;
+    char *mac;
+
+    resp = qtest_qmp(qts, "{ 'execute': 'qom-get', "
+                     "'arguments': { "
+                     "'path': %s, "
+                     "'property': 'mac' } }", name);
+
+    g_assert(qdict_haskey(resp, "return"));
+
+    mac = g_strdup( qdict_get_str(resp, "return"));
+
+    qobject_unref(resp);
+
+    return mac;
+}
+
+static void check_one_card(QTestState *qts, bool present,
+                           const char *id, const char *mac)
+{
+    QDict *device;
+    QDict *bus;
+    char *addr;
+
+    bus = get_bus(qts, 0);
+    device = find_device(bus, id);
+    if (present) {
+        char *path;
+
+        g_assert_nonnull(device);
+        qobject_unref(device);
+
+        path = g_strdup_printf("/machine/peripheral/%s", id);
+        addr = get_mac(qts, path);
+        g_free(path);
+        g_assert_cmpstr(mac, ==, addr);
+        g_free(addr);
+    } else {
+       g_assert_null(device);
+    }
+
+    qobject_unref(bus);
+}
+
+static void test_on(void)
+{
+    QTestState *qts;
+
+    qts = machine_start(BASE_MACHINE
+                        "-netdev user,id=hs0 "
+                        "-device virtio-net,bus=root0,id=standby0,"
+                        "failover=on,netdev=hs0,mac="MAC_STANDBY0" "
+                        "-device virtio-net,bus=root1,id=primary0,"
+                        "failover_pair_id=standby0,netdev=hs1,mac="MAC_PRIMARY0,
+                        2);
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+
+    machine_stop(qts);
+}
+
+static void test_on_mismatch(void)
+{
+    QTestState *qts;
+
+    qts = machine_start(BASE_MACHINE
+                     "-netdev user,id=hs0 "
+                     "-device virtio-net,bus=root0,id=standby0,"
+                     "failover=on,netdev=hs0,mac="MAC_STANDBY0" "
+                     "-netdev user,id=hs1 "
+                     "-device virtio-net,bus=root1,id=primary0,"
+                     "failover_pair_id=standby1,netdev=hs1,mac="MAC_PRIMARY0,
+                     2);
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+
+    machine_stop(qts);
+}
+
+static void test_off(void)
+{
+    QTestState *qts;
+
+    qts = machine_start(BASE_MACHINE
+                     "-netdev user,id=hs0 "
+                     "-device virtio-net,bus=root0,id=standby0,"
+                     "failover=off,netdev=hs0,mac="MAC_STANDBY0" "
+                     "-netdev user,id=hs1 "
+                     "-device virtio-net,bus=root1,id=primary0,"
+                     "failover_pair_id=standby0,netdev=hs1,mac="MAC_PRIMARY0,
+                     2);
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+
+    machine_stop(qts);
+}
+
+static void start_virtio_net(QTestState *qts, int bus, int slot,
+                             const char *id)
+{
+    QVirtioPCIDevice *dev;
+    uint64_t features;
+    QPCIAddress addr;
+    QDict *resp;
+    QDict *data;
+
+    addr.devfn = QPCI_DEVFN((bus << 5) + slot, 0);
+    dev = virtio_pci_new(pcibus, &addr);
+    g_assert_nonnull(dev);
+    qvirtio_pci_device_enable(dev);
+    qvirtio_start_device(&dev->vdev);
+    features = qvirtio_get_features(&dev->vdev);
+    features = features & ~(QVIRTIO_F_BAD_FEATURE |
+                            (1ull << VIRTIO_RING_F_INDIRECT_DESC) |
+                            (1ull << VIRTIO_RING_F_EVENT_IDX));
+    qvirtio_set_features(&dev->vdev, features);
+    qvirtio_set_driver_ok(&dev->vdev);
+
+    resp = qtest_qmp_eventwait_ref(qts, "FAILOVER_NEGOTIATED");
+    g_assert(qdict_haskey(resp, "data"));
+
+    data = qdict_get_qdict(resp, "data");
+    g_assert(qdict_haskey(data, "device-id"));
+    g_assert_cmpstr(qdict_get_str(data, "device-id"), ==, id);
+
+    qobject_unref(resp);
+}
+
+static void test_enabled(void)
+{
+    QTestState *qts;
+
+    qts = machine_start(BASE_MACHINE
+                     "-netdev user,id=hs0 "
+                     "-device virtio-net,bus=root0,id=standby0,"
+                     "failover=on,netdev=hs0,mac="MAC_STANDBY0" "
+                     "-netdev user,id=hs1 "
+                     "-device virtio-net,bus=root1,id=primary0,"
+                     "failover_pair_id=standby0,netdev=hs1,mac="MAC_PRIMARY0" ",
+                     2);
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+
+    start_virtio_net(qts, 1, 0, "standby0");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+
+    machine_stop(qts);
+}
+
+static void test_hotplug_1(void)
+{
+    QTestState *qts;
+
+    qts = machine_start(BASE_MACHINE
+                     "-netdev user,id=hs0 "
+                     "-device virtio-net,bus=root0,id=standby0,"
+                     "failover=on,netdev=hs0,mac="MAC_STANDBY0" "
+                     "-netdev user,id=hs1 ", 2);
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+
+    start_virtio_net(qts, 1, 0, "standby0");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+
+    qtest_qmp_device_add(qts, "virtio-net", "primary0",
+                         "{'bus': 'root1',"
+                         "'failover_pair_id': 'standby0',"
+                         "'netdev': 'hs1',"
+                         "'mac': '"MAC_PRIMARY0"'}");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+
+    machine_stop(qts);
+}
+
+static void test_hotplug_1_reverse(void)
+{
+    QTestState *qts;
+
+    qts = machine_start(BASE_MACHINE
+                     "-netdev user,id=hs0 "
+                     "-netdev user,id=hs1 "
+                     "-device virtio-net,bus=root1,id=primary0,"
+                     "failover_pair_id=standby0,netdev=hs1,mac="MAC_PRIMARY0" ",
+                     2);
+
+    check_one_card(qts, false, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+
+    qtest_qmp_device_add(qts, "virtio-net", "standby0",
+                         "{'bus': 'root0',"
+                         "'failover': 'on',"
+                         "'netdev': 'hs0',"
+                         "'mac': '"MAC_STANDBY0"'}");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+
+    start_virtio_net(qts, 1, 0, "standby0");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+
+    machine_stop(qts);
+}
+
+static void test_hotplug_2(void)
+{
+    QTestState *qts;
+
+    qts = machine_start(BASE_MACHINE
+                     "-netdev user,id=hs0 "
+                     "-netdev user,id=hs1 ",
+                     2);
+
+    check_one_card(qts, false, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+
+    qtest_qmp_device_add(qts, "virtio-net", "standby0",
+                         "{'bus': 'root0',"
+                         "'failover': 'on',"
+                         "'netdev': 'hs0',"
+                         "'mac': '"MAC_STANDBY0"'}");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+
+    start_virtio_net(qts, 1, 0, "standby0");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+
+    qtest_qmp_device_add(qts, "virtio-net", "primary0",
+                         "{'bus': 'root1',"
+                         "'failover_pair_id': 'standby0',"
+                         "'netdev': 'hs1',"
+                         "'mac': '"MAC_PRIMARY0"'}");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+
+    machine_stop(qts);
+}
+
+static void test_hotplug_2_reverse(void)
+{
+    QTestState *qts;
+
+    qts = machine_start(BASE_MACHINE
+                     "-netdev user,id=hs0 "
+                     "-netdev user,id=hs1 ",
+                     2);
+
+    check_one_card(qts, false, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+
+    qtest_qmp_device_add(qts, "virtio-net", "primary0",
+                         "{'bus': 'root1',"
+                         "'failover_pair_id': 'standby0',"
+                         "'netdev': 'hs1',"
+                         "'mac': '"MAC_PRIMARY0"'}");
+
+    check_one_card(qts, false, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+
+    qtest_qmp_device_add(qts, "virtio-net", "standby0",
+                         "{'bus': 'root0',"
+                         "'failover': 'on',"
+                         "'netdev': 'hs0',"
+                         "'rombar': 0,"
+                         "'romfile': '',"
+                         "'mac': '"MAC_STANDBY0"'}");
+
+    /* XXX: sounds like a bug */
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+
+    start_virtio_net(qts, 1, 0, "standby0");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+
+    machine_stop(qts);
+}
+
+static QDict *migrate_status(QTestState *qts)
+{
+    QDict *resp, *ret;
+
+    resp = qtest_qmp(qts, "{ 'execute': 'query-migrate' }");
+    g_assert(qdict_haskey(resp, "return"));
+
+    ret = qdict_get_qdict(resp, "return");
+    g_assert(qdict_haskey(ret, "status"));
+    qobject_ref(ret);
+    qobject_unref(resp);
+
+    return ret;
+}
+
+static void test_migrate_out(gconstpointer opaque)
+{
+    QTestState *qts;
+    QDict *resp, *args, *data, *ret;
+    g_autofree gchar *uri = g_strdup_printf("exec: cat > %s", (gchar *)opaque);
+    const gchar *status;
+
+    qts = machine_start(BASE_MACHINE
+                     "-netdev user,id=hs0 "
+                     "-netdev user,id=hs1 ",
+                     2);
+
+    check_one_card(qts, false, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+
+    qtest_qmp_device_add(qts, "virtio-net", "standby0",
+                         "{'bus': 'root0',"
+                         "'failover': 'on',"
+                         "'netdev': 'hs0',"
+                         "'mac': '"MAC_STANDBY0"'}");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+
+    start_virtio_net(qts, 1, 0, "standby0");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+
+    qtest_qmp_device_add(qts, "virtio-net", "primary0",
+                         "{'bus': 'root1',"
+                         "'failover_pair_id': 'standby0',"
+                         "'netdev': 'hs1',"
+                         "'rombar': 0,"
+                         "'romfile': '',"
+                         "'mac': '"MAC_PRIMARY0"'}");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+
+    args = qdict_from_jsonf_nofail("{}");
+    g_assert_nonnull(args);
+    qdict_put_str(args, "uri", uri);
+
+    resp = qtest_qmp(qts, "{ 'execute': 'migrate', 'arguments': %p}", args);
+    g_assert(qdict_haskey(resp, "return"));
+    qobject_unref(resp);
+
+    /* the event is sent whan QEMU asks the OS to unplug the card */
+    resp = qtest_qmp_eventwait_ref(qts, "UNPLUG_PRIMARY");
+    g_assert(qdict_haskey(resp, "data"));
+
+    data = qdict_get_qdict(resp, "data");
+    g_assert(qdict_haskey(data, "device-id"));
+    g_assert_cmpstr(qdict_get_str(data, "device-id"), ==, "primary0");
+
+    qobject_unref(resp);
+
+    qtest_outl(qts, ACPI_PCIHP_ADDR_ICH9 + PCI_EJ_BASE, 1);
+
+    while (true) {
+        ret = migrate_status(qts);
+
+        status = qdict_get_str(ret, "status");
+        if (strcmp(status, "completed") == 0) {
+            break;
+        }
+        g_assert_cmpstr(status, !=, "failed");
+        g_assert_cmpstr(status, !=, "cancelling");
+        g_assert_cmpstr(status, !=, "cancelled");
+        qobject_unref(ret);
+    }
+    qobject_unref(ret);
+
+    qtest_qmp_eventwait(qts, "STOP");
+
+    /*
+     * in fact, the card is ejected from the point of view of kernel
+     * but not really from QEMU to be able to hotplug it back if
+     * migration fails. So we can't check that:
+     *   check_one_card(qts, true, "standby0", MAC_STANDBY0);
+     *   check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+     */
+
+    machine_stop(qts);
+}
+
+static void test_migrate_in(gconstpointer opaque)
+{
+    QTestState *qts;
+    QDict *resp, *args, *ret;
+    g_autofree gchar *uri = g_strdup_printf("exec: cat %s", (gchar *)opaque);
+
+    qts = machine_start(BASE_MACHINE
+                     "-netdev user,id=hs0 "
+                     "-netdev user,id=hs1 "
+                     "-incoming defer ",
+                     2);
+
+    check_one_card(qts, false, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+
+    qtest_qmp_device_add(qts, "virtio-net", "standby0",
+                         "{'bus': 'root0',"
+                         "'failover': 'on',"
+                         "'netdev': 'hs0',"
+                         "'mac': '"MAC_STANDBY0"'}");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+
+    qtest_qmp_device_add(qts, "virtio-net", "primary0",
+                         "{'bus': 'root1',"
+                         "'failover_pair_id': 'standby0',"
+                         "'netdev': 'hs1',"
+                         "'rombar': 0,"
+                         "'romfile': '',"
+                         "'mac': '"MAC_PRIMARY0"'}");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+
+    args = qdict_from_jsonf_nofail("{}");
+    g_assert_nonnull(args);
+    qdict_put_str(args, "uri", uri);
+
+    resp = qtest_qmp(qts, "{ 'execute': 'migrate-incoming', 'arguments': %p}",
+                     args);
+    g_assert(qdict_haskey(resp, "return"));
+    qobject_unref(resp);
+
+    qtest_qmp_eventwait(qts, "MIGRATION");
+    qtest_qmp_eventwait(qts, "FAILOVER_NEGOTIATED");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+
+    qtest_qmp_eventwait(qts, "RESUME");
+
+    ret = migrate_status(qts);
+    g_assert_cmpstr(qdict_get_str(ret, "status"), ==, "completed");
+    qobject_unref(ret);
+
+    machine_stop(qts);
+}
+
+int main(int argc, char **argv)
+{
+    gchar *tmpfile = g_strdup_printf("/tmp/failover_test_migrate-%u-%u",
+                                     getpid(), g_test_rand_int());
+    const char *arch;
+    int ret;
+
+    g_test_init(&argc, &argv, NULL);
+
+    arch = qtest_get_arch();
+    if (strcmp(arch, "i386") && strcmp(arch, "x86_64")) {
+        g_test_message("Skipping test for non-x86");
+        return g_test_run();
+    }
+
+    qtest_add_func("failover-virtio-net/params/error/id", test_error_id);
+    qtest_add_func("failover-virtio-net/params/error/pcie", test_error_pcie);
+    qtest_add_func("failover-virtio-net/params/on", test_on);
+    qtest_add_func("failover-virtio-net/params/on_mismatch",
+                   test_on_mismatch);
+    qtest_add_func("failover-virtio-net/params/off", test_off);
+    qtest_add_func("failover-virtio-net/params/enabled", test_enabled);
+    qtest_add_func("failover-virtio-net/hotplug_1", test_hotplug_1);
+    qtest_add_func("failover-virtio-net/hotplug_1_reverse",
+                   test_hotplug_1_reverse);
+    qtest_add_func("failover-virtio-net/hotplug_2", test_hotplug_2);
+    qtest_add_func("failover-virtio-net/hotplug_2_reverse",
+                   test_hotplug_2_reverse);
+    qtest_add_data_func("failover-virtio-net/migrate/out", tmpfile,
+                        test_migrate_out);
+    qtest_add_data_func("failover-virtio-net/migrate/in", tmpfile,
+                        test_migrate_in);
+
+    ret = g_test_run();
+
+    unlink(tmpfile);
+    g_free(tmpfile);
+
+    return ret;
+}
-- 
2.33.1

[PATCH v6 3/6] failover: fix unplug pending detection

[Laurent Vivier]

Failover needs to detect the end of the PCI unplug to start migration
after the VFIO card has been unplugged.

To do that, a flag is set in pcie_cap_slot_unplug_request_cb() and reset in
pcie_unplug_device().

But since
    17858a169508 ("hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35")
we have switched to ACPI unplug and these functions are not called anymore
and the flag not set. So failover migration is not able to detect if card
is really unplugged and acts as it's done as soon as it's started. So it
doesn't wait the end of the unplug to start the migration. We don't see any
problem when we test that because ACPI unplug is faster than PCIe native
hotplug and when the migration really starts the unplug operation is
already done.

See c000a9bd06ea ("pci: mark device having guest unplug request pending")
    a99c4da9fc2a ("pci: mark devices partially unplugged")

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Ani Sinha <ani@anisinha.ca>
---
 hw/acpi/pcihp.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index f610a25d2ef9..30405b5113d7 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -222,9 +222,27 @@ static void acpi_pcihp_eject_slot(AcpiPciHpState *s, unsigned bsel, unsigned slo
         PCIDevice *dev = PCI_DEVICE(qdev);
         if (PCI_SLOT(dev->devfn) == slot) {
             if (!acpi_pcihp_pc_no_hotplug(s, dev)) {
-                hotplug_ctrl = qdev_get_hotplug_handler(qdev);
-                hotplug_handler_unplug(hotplug_ctrl, qdev, &error_abort);
-                object_unparent(OBJECT(qdev));
+                /*
+                 * partially_hotplugged is used by virtio-net failover:
+                 * failover has asked the guest OS to unplug the device
+                 * but we need to keep some references to the device
+                 * to be able to plug it back in case of failure so
+                 * we don't execute hotplug_handler_unplug().
+                 */
+                if (dev->partially_hotplugged) {
+                    /*
+                     * pending_deleted_event is set to true when
+                     * virtio-net failover asks to unplug the device,
+                     * and set to false here when the operation is done
+                     * This is used by the migration loop to detect the
+                     * end of the operation and really start the migration.
+                     */
+                    qdev->pending_deleted_event = false;
+                } else {
+                    hotplug_ctrl = qdev_get_hotplug_handler(qdev);
+                    hotplug_handler_unplug(hotplug_ctrl, qdev, &error_abort);
+                    object_unparent(OBJECT(qdev));
+                }
             }
         }
     }
@@ -396,6 +414,12 @@ void acpi_pcihp_device_unplug_request_cb(HotplugHandler *hotplug_dev,
         return;
     }
 
+    /*
+     * pending_deleted_event is used by virtio-net failover to detect the
+     * end of the unplug operation, the flag is set to false in
+     * acpi_pcihp_eject_slot() when the operation is completed.
+     */
+    pdev->qdev.pending_deleted_event = true;
     s->acpi_pcihp_pci_status[bsel].down |= (1U << slot);
     acpi_send_event(DEVICE(hotplug_dev), ACPI_PCI_HOTPLUG_STATUS);
 }
-- 
2.33.1

[PATCH v6 4/6] tests/libqtest: update virtio-net failover test

[Laurent Vivier]

Update the migration test to check we correctly wait the end
of the card unplug before doing the migration.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 tests/qtest/virtio-net-failover.c | 34 +++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/tests/qtest/virtio-net-failover.c b/tests/qtest/virtio-net-failover.c
index f8f5fbb3c7fe..c88f8ddec39a 100644
--- a/tests/qtest/virtio-net-failover.c
+++ b/tests/qtest/virtio-net-failover.c
@@ -560,6 +560,40 @@ static void test_migrate_out(gconstpointer opaque)
 
     qobject_unref(resp);
 
+    /* wait the end of the migration setup phase */
+    while (true) {
+        ret = migrate_status(qts);
+
+        status = qdict_get_str(ret, "status");
+        if (strcmp(status, "wait-unplug") == 0) {
+            break;
+        }
+
+        /* The migration must not start if the card is not ejected */
+        g_assert_cmpstr(status, !=, "active");
+        g_assert_cmpstr(status, !=, "completed");
+        g_assert_cmpstr(status, !=, "failed");
+        g_assert_cmpstr(status, !=, "cancelling");
+        g_assert_cmpstr(status, !=, "cancelled");
+
+        qobject_unref(ret);
+    }
+    qobject_unref(ret);
+
+    if (g_test_slow()) {
+        /* check we stay in wait-unplug while the card is not ejected */
+        int i;
+
+        for (i = 0; i < 10; i++) {
+            sleep(1);
+            ret = migrate_status(qts);
+            status = qdict_get_str(ret, "status");
+            g_assert_cmpstr(status, ==, "wait-unplug");
+            qobject_unref(ret);
+        }
+    }
+
+    /* OS unplugs the cards, QEMU can move from wait-unplug state */
     qtest_outl(qts, ACPI_PCIHP_ADDR_ICH9 + PCI_EJ_BASE, 1);
 
     while (true) {
-- 
2.33.1

[PATCH v6 5/6] test/libqtest: add some virtio-net failover migration cancelling tests

[Laurent Vivier]

Add some tests to check the state of the machine if the migration
is cancelled while we are using virtio-net failover.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 tests/qtest/virtio-net-failover.c | 291 ++++++++++++++++++++++++++++++
 1 file changed, 291 insertions(+)

diff --git a/tests/qtest/virtio-net-failover.c b/tests/qtest/virtio-net-failover.c
index c88f8ddec39a..57abb99e7f6e 100644
--- a/tests/qtest/virtio-net-failover.c
+++ b/tests/qtest/virtio-net-failover.c
@@ -682,6 +682,289 @@ static void test_migrate_in(gconstpointer opaque)
     machine_stop(qts);
 }
 
+static void test_migrate_abort_wait_unplug(gconstpointer opaque)
+{
+    QTestState *qts;
+    QDict *resp, *args, *data, *ret;
+    g_autofree gchar *uri = g_strdup_printf("exec: cat > %s", (gchar *)opaque);
+    const gchar *status;
+
+    qts = machine_start(BASE_MACHINE
+                     "-netdev user,id=hs0 "
+                     "-netdev user,id=hs1 ",
+                     2);
+
+    check_one_card(qts, false, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+
+    qtest_qmp_device_add(qts, "virtio-net", "standby0",
+                         "{'bus': 'root0',"
+                         "'failover': 'on',"
+                         "'netdev': 'hs0',"
+                         "'mac': '"MAC_STANDBY0"'}");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+
+    start_virtio_net(qts, 1, 0, "standby0");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+
+    qtest_qmp_device_add(qts, "virtio-net", "primary0",
+                         "{'bus': 'root1',"
+                         "'failover_pair_id': 'standby0',"
+                         "'netdev': 'hs1',"
+                         "'rombar': 0,"
+                         "'romfile': '',"
+                         "'mac': '"MAC_PRIMARY0"'}");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+
+    args = qdict_from_jsonf_nofail("{}");
+    g_assert_nonnull(args);
+    qdict_put_str(args, "uri", uri);
+
+    resp = qtest_qmp(qts, "{ 'execute': 'migrate', 'arguments': %p}", args);
+    g_assert(qdict_haskey(resp, "return"));
+    qobject_unref(resp);
+
+    /* the event is sent whan QEMU asks the OS to unplug the card */
+    resp = qtest_qmp_eventwait_ref(qts, "UNPLUG_PRIMARY");
+    g_assert(qdict_haskey(resp, "data"));
+
+    data = qdict_get_qdict(resp, "data");
+    g_assert(qdict_haskey(data, "device-id"));
+    g_assert_cmpstr(qdict_get_str(data, "device-id"), ==, "primary0");
+
+    qobject_unref(resp);
+
+    resp = qtest_qmp(qts, "{ 'execute': 'migrate_cancel' }");
+    g_assert(qdict_haskey(resp, "return"));
+    qobject_unref(resp);
+
+    /* migration has been cancelled while the unplug was in progress */
+
+    /* while the card is not ejected, we must be in "cancelling" state */
+    ret = migrate_status(qts);
+
+    status = qdict_get_str(ret, "status");
+    g_assert_cmpstr(status, ==, "cancelling");
+    qobject_unref(ret);
+
+    /* OS unplugs the cards, QEMU can move from wait-unplug state */
+    qtest_outl(qts, ACPI_PCIHP_ADDR_ICH9 + PCI_EJ_BASE, 1);
+
+    while (true) {
+        ret = migrate_status(qts);
+
+        status = qdict_get_str(ret, "status");
+        if (strcmp(status, "cancelled") == 0) {
+            break;
+        }
+        g_assert_cmpstr(status, !=, "failed");
+        g_assert_cmpstr(status, !=, "active");
+        qobject_unref(ret);
+    }
+    qobject_unref(ret);
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+
+    machine_stop(qts);
+}
+
+static void test_migrate_abort_active(gconstpointer opaque)
+{
+    QTestState *qts;
+    QDict *resp, *args, *data, *ret;
+    g_autofree gchar *uri = g_strdup_printf("exec: cat > %s", (gchar *)opaque);
+    const gchar *status;
+
+    qts = machine_start(BASE_MACHINE
+                     "-netdev user,id=hs0 "
+                     "-netdev user,id=hs1 ",
+                     2);
+
+    check_one_card(qts, false, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+
+    qtest_qmp_device_add(qts, "virtio-net", "standby0",
+                         "{'bus': 'root0',"
+                         "'failover': 'on',"
+                         "'netdev': 'hs0',"
+                         "'mac': '"MAC_STANDBY0"'}");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+
+    start_virtio_net(qts, 1, 0, "standby0");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+
+    qtest_qmp_device_add(qts, "virtio-net", "primary0",
+                         "{'bus': 'root1',"
+                         "'failover_pair_id': 'standby0',"
+                         "'netdev': 'hs1',"
+                         "'rombar': 0,"
+                         "'romfile': '',"
+                         "'mac': '"MAC_PRIMARY0"'}");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+
+    args = qdict_from_jsonf_nofail("{}");
+    g_assert_nonnull(args);
+    qdict_put_str(args, "uri", uri);
+
+    resp = qtest_qmp(qts, "{ 'execute': 'migrate', 'arguments': %p}", args);
+    g_assert(qdict_haskey(resp, "return"));
+    qobject_unref(resp);
+
+    /* the event is sent whan QEMU asks the OS to unplug the card */
+    resp = qtest_qmp_eventwait_ref(qts, "UNPLUG_PRIMARY");
+    g_assert(qdict_haskey(resp, "data"));
+
+    data = qdict_get_qdict(resp, "data");
+    g_assert(qdict_haskey(data, "device-id"));
+    g_assert_cmpstr(qdict_get_str(data, "device-id"), ==, "primary0");
+
+    qobject_unref(resp);
+
+    /* OS unplugs the cards, QEMU can move from wait-unplug state */
+    qtest_outl(qts, ACPI_PCIHP_ADDR_ICH9 + PCI_EJ_BASE, 1);
+
+    while (true) {
+        ret = migrate_status(qts);
+
+        status = qdict_get_str(ret, "status");
+        if (strcmp(status, "wait-unplug") != 0) {
+            break;
+        }
+        g_assert_cmpstr(status, !=, "failed");
+        qobject_unref(ret);
+    }
+    qobject_unref(resp);
+
+    resp = qtest_qmp(qts, "{ 'execute': 'migrate_cancel' }");
+    g_assert(qdict_haskey(resp, "return"));
+    qobject_unref(resp);
+
+    while (true) {
+        ret = migrate_status(qts);
+
+        status = qdict_get_str(ret, "status");
+        if (strcmp(status, "cancelled") == 0) {
+            break;
+        }
+        g_assert_cmpstr(status, !=, "failed");
+        g_assert_cmpstr(status, !=, "active");
+        qobject_unref(ret);
+    }
+    qobject_unref(ret);
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+
+    machine_stop(qts);
+}
+
+static void test_migrate_abort_timeout(gconstpointer opaque)
+{
+    QTestState *qts;
+    QDict *resp, *args, *data, *ret;
+    g_autofree gchar *uri = g_strdup_printf("exec: cat > %s", (gchar *)opaque);
+    const gchar *status;
+    int total;
+
+    qts = machine_start(BASE_MACHINE
+                     "-netdev user,id=hs0 "
+                     "-netdev user,id=hs1 ",
+                     2);
+
+    check_one_card(qts, false, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+
+    qtest_qmp_device_add(qts, "virtio-net", "standby0",
+                         "{'bus': 'root0',"
+                         "'failover': 'on',"
+                         "'netdev': 'hs0',"
+                         "'mac': '"MAC_STANDBY0"'}");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+
+    start_virtio_net(qts, 1, 0, "standby0");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+
+    qtest_qmp_device_add(qts, "virtio-net", "primary0",
+                         "{'bus': 'root1',"
+                         "'failover_pair_id': 'standby0',"
+                         "'netdev': 'hs1',"
+                         "'rombar': 0,"
+                         "'romfile': '',"
+                         "'mac': '"MAC_PRIMARY0"'}");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+
+    args = qdict_from_jsonf_nofail("{}");
+    g_assert_nonnull(args);
+    qdict_put_str(args, "uri", uri);
+
+    resp = qtest_qmp(qts, "{ 'execute': 'migrate', 'arguments': %p}", args);
+    g_assert(qdict_haskey(resp, "return"));
+    qobject_unref(resp);
+
+    /* the event is sent whan QEMU asks the OS to unplug the card */
+    resp = qtest_qmp_eventwait_ref(qts, "UNPLUG_PRIMARY");
+    g_assert(qdict_haskey(resp, "data"));
+
+    data = qdict_get_qdict(resp, "data");
+    g_assert(qdict_haskey(data, "device-id"));
+    g_assert_cmpstr(qdict_get_str(data, "device-id"), ==, "primary0");
+
+    qobject_unref(resp);
+
+    resp = qtest_qmp(qts, "{ 'execute': 'migrate_cancel' }");
+    g_assert(qdict_haskey(resp, "return"));
+    qobject_unref(resp);
+
+    /* migration has been cancelled while the unplug was in progress */
+
+    /* while the card is not ejected, we must be in "cancelling" state */
+
+    total = 0;
+    while (true) {
+        ret = migrate_status(qts);
+
+        status = qdict_get_str(ret, "status");
+        if (strcmp(status, "cancelled") == 0) {
+            break;
+        }
+        g_assert_cmpstr(status, ==, "cancelling");
+        g_assert(qdict_haskey(ret, "total-time"));
+        total = qdict_get_int(ret, "total-time");
+        qobject_unref(ret);
+    }
+    qobject_unref(ret);
+
+    /*
+     * migration timeout in this case is 30 seconds
+     * check we exit on the timeout (ms)
+     */
+    g_assert_cmpint(total, >, 30000);
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+
+    machine_stop(qts);
+}
+
 int main(int argc, char **argv)
 {
     gchar *tmpfile = g_strdup_printf("/tmp/failover_test_migrate-%u-%u",
@@ -714,6 +997,14 @@ int main(int argc, char **argv)
                         test_migrate_out);
     qtest_add_data_func("failover-virtio-net/migrate/in", tmpfile,
                         test_migrate_in);
+    qtest_add_data_func("failover-virtio-net/migrate/abort/wait-unplug",
+                        tmpfile, test_migrate_abort_wait_unplug);
+    qtest_add_data_func("failover-virtio-net/migrate/abort/active", tmpfile,
+                        test_migrate_abort_active);
+    if (g_test_slow()) {
+        qtest_add_data_func("failover-virtio-net/migrate/abort/timeout",
+                            tmpfile, test_migrate_abort_timeout);
+    }
 
     ret = g_test_run();
 
-- 
2.33.1

[PATCH v6 6/6] tests/libqtest: add a migration test with two couples of failover devices

[Laurent Vivier]

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 tests/qtest/virtio-net-failover.c | 279 ++++++++++++++++++++++++++++++
 1 file changed, 279 insertions(+)

diff --git a/tests/qtest/virtio-net-failover.c b/tests/qtest/virtio-net-failover.c
index 57abb99e7f6e..ace9001894af 100644
--- a/tests/qtest/virtio-net-failover.c
+++ b/tests/qtest/virtio-net-failover.c
@@ -11,6 +11,7 @@
 
 #define ACPI_PCIHP_ADDR_ICH9    0x0cc0
 #define PCI_EJ_BASE             0x0008
+#define PCI_SEL_BASE            0x0010
 
 #define BASE_MACHINE "-M q35 -nodefaults " \
     "-device pcie-root-port,id=root0,addr=0x1,bus=pcie.0,chassis=1 " \
@@ -18,6 +19,8 @@
 
 #define MAC_PRIMARY0 "52:54:00:11:11:11"
 #define MAC_STANDBY0 "52:54:00:22:22:22"
+#define MAC_PRIMARY1 "52:54:00:33:33:33"
+#define MAC_STANDBY1 "52:54:00:44:44:44"
 
 static QGuestAllocator guest_malloc;
 static QPCIBus *pcibus;
@@ -965,6 +968,278 @@ static void test_migrate_abort_timeout(gconstpointer opaque)
     machine_stop(qts);
 }
 
+static void test_multi_out(gconstpointer opaque)
+{
+    QTestState *qts;
+    QDict *resp, *args, *data, *ret;
+    g_autofree gchar *uri = g_strdup_printf("exec: cat > %s", (gchar *)opaque);
+    const gchar *status, *expected;
+
+    qts = machine_start(BASE_MACHINE
+                "-device pcie-root-port,id=root2,addr=0x3,bus=pcie.0,chassis=3 "
+                "-device pcie-root-port,id=root3,addr=0x4,bus=pcie.0,chassis=4 "
+                "-netdev user,id=hs0 "
+                "-netdev user,id=hs1 "
+                "-netdev user,id=hs2 "
+                "-netdev user,id=hs3 ",
+                4);
+
+    check_one_card(qts, false, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+    check_one_card(qts, false, "standby1", MAC_STANDBY1);
+    check_one_card(qts, false, "primary1", MAC_PRIMARY1);
+
+    qtest_qmp_device_add(qts, "virtio-net", "standby0",
+                         "{'bus': 'root0',"
+                         "'failover': 'on',"
+                         "'netdev': 'hs0',"
+                         "'mac': '"MAC_STANDBY0"'}");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+    check_one_card(qts, false, "standby1", MAC_STANDBY1);
+    check_one_card(qts, false, "primary1", MAC_PRIMARY1);
+
+    qtest_qmp_device_add(qts, "virtio-net", "primary0",
+                         "{'bus': 'root1',"
+                         "'failover_pair_id': 'standby0',"
+                         "'netdev': 'hs1',"
+                         "'rombar': 0,"
+                         "'romfile': '',"
+                         "'mac': '"MAC_PRIMARY0"'}");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+    check_one_card(qts, false, "standby1", MAC_STANDBY1);
+    check_one_card(qts, false, "primary1", MAC_PRIMARY1);
+
+    start_virtio_net(qts, 1, 0, "standby0");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+    check_one_card(qts, false, "standby1", MAC_STANDBY1);
+    check_one_card(qts, false, "primary1", MAC_PRIMARY1);
+
+    qtest_qmp_device_add(qts, "virtio-net", "standby1",
+                         "{'bus': 'root2',"
+                         "'failover': 'on',"
+                         "'netdev': 'hs2',"
+                         "'mac': '"MAC_STANDBY1"'}");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+    check_one_card(qts, true, "standby1", MAC_STANDBY1);
+    check_one_card(qts, false, "primary1", MAC_PRIMARY1);
+
+    qtest_qmp_device_add(qts, "virtio-net", "primary1",
+                         "{'bus': 'root3',"
+                         "'failover_pair_id': 'standby1',"
+                         "'netdev': 'hs3',"
+                         "'rombar': 0,"
+                         "'romfile': '',"
+                         "'mac': '"MAC_PRIMARY1"'}");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+    check_one_card(qts, true, "standby1", MAC_STANDBY1);
+    check_one_card(qts, false, "primary1", MAC_PRIMARY1);
+
+    start_virtio_net(qts, 3, 0, "standby1");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+    check_one_card(qts, true, "standby1", MAC_STANDBY1);
+    check_one_card(qts, true, "primary1", MAC_PRIMARY1);
+
+    args = qdict_from_jsonf_nofail("{}");
+    g_assert_nonnull(args);
+    qdict_put_str(args, "uri", uri);
+
+    resp = qtest_qmp(qts, "{ 'execute': 'migrate', 'arguments': %p}", args);
+    g_assert(qdict_haskey(resp, "return"));
+    qobject_unref(resp);
+
+    /* the event is sent whan QEMU asks the OS to unplug the card */
+    resp = qtest_qmp_eventwait_ref(qts, "UNPLUG_PRIMARY");
+    g_assert(qdict_haskey(resp, "data"));
+
+    data = qdict_get_qdict(resp, "data");
+    g_assert(qdict_haskey(data, "device-id"));
+    if (strcmp(qdict_get_str(data, "device-id"), "primary0") == 0) {
+        expected = "primary1";
+    } else if (strcmp(qdict_get_str(data, "device-id"), "primary1") == 0) {
+        expected = "primary0";
+    } else {
+        g_assert_not_reached();
+    }
+    qobject_unref(resp);
+
+    resp = qtest_qmp_eventwait_ref(qts, "UNPLUG_PRIMARY");
+    g_assert(qdict_haskey(resp, "data"));
+
+    data = qdict_get_qdict(resp, "data");
+    g_assert(qdict_haskey(data, "device-id"));
+    g_assert_cmpstr(qdict_get_str(data, "device-id"), ==, expected);
+    qobject_unref(resp);
+
+    /* wait the end of the migration setup phase */
+    while (true) {
+        ret = migrate_status(qts);
+
+        status = qdict_get_str(ret, "status");
+        if (strcmp(status, "wait-unplug") == 0) {
+            break;
+        }
+
+        /* The migration must not start if the card is not ejected */
+        g_assert_cmpstr(status, !=, "active");
+        g_assert_cmpstr(status, !=, "completed");
+        g_assert_cmpstr(status, !=, "failed");
+        g_assert_cmpstr(status, !=, "cancelling");
+        g_assert_cmpstr(status, !=, "cancelled");
+
+        qobject_unref(ret);
+    }
+    qobject_unref(ret);
+
+    /* OS unplugs primary1, but we must wait the second */
+    qtest_outl(qts, ACPI_PCIHP_ADDR_ICH9 + PCI_EJ_BASE, 1);
+
+    ret = migrate_status(qts);
+    status = qdict_get_str(ret, "status");
+    g_assert_cmpstr(status, ==, "wait-unplug");
+    qobject_unref(ret);
+
+    if (g_test_slow()) {
+        /* check we stay in wait-unplug while the card is not ejected */
+        int i;
+
+        for (i = 0; i < 10; i++) {
+            sleep(1);
+            ret = migrate_status(qts);
+            status = qdict_get_str(ret, "status");
+            g_assert_cmpstr(status, ==, "wait-unplug");
+            qobject_unref(ret);
+        }
+    }
+
+    /* OS unplugs primary0, QEMU can move from wait-unplug state */
+    qtest_outl(qts, ACPI_PCIHP_ADDR_ICH9 + PCI_SEL_BASE, 2);
+    qtest_outl(qts, ACPI_PCIHP_ADDR_ICH9 + PCI_EJ_BASE, 1);
+
+    while (true) {
+        ret = migrate_status(qts);
+
+        status = qdict_get_str(ret, "status");
+        if (strcmp(status, "completed") == 0) {
+            break;
+        }
+        g_assert_cmpstr(status, !=, "failed");
+        g_assert_cmpstr(status, !=, "cancelling");
+        g_assert_cmpstr(status, !=, "cancelled");
+    }
+
+    qtest_qmp_eventwait(qts, "STOP");
+
+    machine_stop(qts);
+}
+
+static void test_multi_in(gconstpointer opaque)
+{
+    QTestState *qts;
+    QDict *resp, *args, *ret;
+    g_autofree gchar *uri = g_strdup_printf("exec: cat %s", (gchar *)opaque);
+
+    qts = machine_start(BASE_MACHINE
+                "-device pcie-root-port,id=root2,addr=0x3,bus=pcie.0,chassis=3 "
+                "-device pcie-root-port,id=root3,addr=0x4,bus=pcie.0,chassis=4 "
+                "-netdev user,id=hs0 "
+                "-netdev user,id=hs1 "
+                "-netdev user,id=hs2 "
+                "-netdev user,id=hs3 "
+                "-incoming defer ",
+                4);
+
+    check_one_card(qts, false, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+    check_one_card(qts, false, "standby1", MAC_STANDBY1);
+    check_one_card(qts, false, "primary1", MAC_PRIMARY1);
+
+    qtest_qmp_device_add(qts, "virtio-net", "standby0",
+                         "{'bus': 'root0',"
+                         "'failover': 'on',"
+                         "'netdev': 'hs0',"
+                         "'mac': '"MAC_STANDBY0"'}");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+    check_one_card(qts, false, "standby1", MAC_STANDBY1);
+    check_one_card(qts, false, "primary1", MAC_PRIMARY1);
+
+    qtest_qmp_device_add(qts, "virtio-net", "primary0",
+                         "{'bus': 'root1',"
+                         "'failover_pair_id': 'standby0',"
+                         "'netdev': 'hs1',"
+                         "'rombar': 0,"
+                         "'romfile': '',"
+                         "'mac': '"MAC_PRIMARY0"'}");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+    check_one_card(qts, false, "standby1", MAC_STANDBY1);
+    check_one_card(qts, false, "primary1", MAC_PRIMARY1);
+
+    qtest_qmp_device_add(qts, "virtio-net", "standby1",
+                         "{'bus': 'root2',"
+                         "'failover': 'on',"
+                         "'netdev': 'hs2',"
+                         "'mac': '"MAC_STANDBY1"'}");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+    check_one_card(qts, true, "standby1", MAC_STANDBY1);
+    check_one_card(qts, false, "primary1", MAC_PRIMARY1);
+
+    qtest_qmp_device_add(qts, "virtio-net", "primary1",
+                         "{'bus': 'root3',"
+                         "'failover_pair_id': 'standby1',"
+                         "'netdev': 'hs3',"
+                         "'rombar': 0,"
+                         "'romfile': '',"
+                         "'mac': '"MAC_PRIMARY1"'}");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
+    check_one_card(qts, true, "standby1", MAC_STANDBY1);
+    check_one_card(qts, false, "primary1", MAC_PRIMARY1);
+
+    args = qdict_from_jsonf_nofail("{}");
+    g_assert_nonnull(args);
+    qdict_put_str(args, "uri", uri);
+
+    resp = qtest_qmp(qts, "{ 'execute': 'migrate-incoming', 'arguments': %p}",
+                     args);
+    g_assert(qdict_haskey(resp, "return"));
+    qobject_unref(resp);
+
+    qtest_qmp_eventwait(qts, "MIGRATION");
+    qtest_qmp_eventwait(qts, "FAILOVER_NEGOTIATED");
+    qtest_qmp_eventwait(qts, "FAILOVER_NEGOTIATED");
+
+    check_one_card(qts, true, "standby0", MAC_STANDBY0);
+    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
+    check_one_card(qts, true, "standby1", MAC_STANDBY1);
+    check_one_card(qts, true, "primary1", MAC_PRIMARY1);
+
+    qtest_qmp_eventwait(qts, "RESUME");
+
+    ret = migrate_status(qts);
+    g_assert_cmpstr(qdict_get_str(ret, "status"), ==, "completed");
+    qobject_unref(ret);
+
+    machine_stop(qts);
+}
+
 int main(int argc, char **argv)
 {
     gchar *tmpfile = g_strdup_printf("/tmp/failover_test_migrate-%u-%u",
@@ -1005,6 +1280,10 @@ int main(int argc, char **argv)
         qtest_add_data_func("failover-virtio-net/migrate/abort/timeout",
                             tmpfile, test_migrate_abort_timeout);
     }
+    qtest_add_data_func("failover-virtio-net/multi/out",
+                        tmpfile, test_multi_out);
+    qtest_add_data_func("failover-virtio-net/multi/in",
+                   tmpfile, test_multi_in);
 
     ret = g_test_run();
 
-- 
2.33.1

Re: [PATCH v6 3/6] failover: fix unplug pending detection

[Ani Sinha]

On Mon, 6 Dec 2021, Laurent Vivier wrote:

> Failover needs to detect the end of the PCI unplug to start migration
> after the VFIO card has been unplugged.
>
> To do that, a flag is set in pcie_cap_slot_unplug_request_cb() and reset in
> pcie_unplug_device().
>
> But since
>     17858a169508 ("hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35")
> we have switched to ACPI unplug and these functions are not called anymore
> and the flag not set. So failover migration is not able to detect if card
> is really unplugged and acts as it's done as soon as it's started. So it
> doesn't wait the end of the unplug to start the migration. We don't see any
> problem when we test that because ACPI unplug is faster than PCIe native
> hotplug and when the migration really starts the unplug operation is
> already done.
>
> See c000a9bd06ea ("pci: mark device having guest unplug request pending")
>     a99c4da9fc2a ("pci: mark devices partially unplugged")
>

This has already been merged upstream:
https://git.qemu.org/?p=qemu.git;a=commit;h=9323f892b39d133eb69b301484bf7b2f3f49737d

Not sure why you resent this one.

> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> Reviewed-by: Ani Sinha <ani@anisinha.ca>
> ---
>  hw/acpi/pcihp.c | 30 +++++++++++++++++++++++++++---
>  1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> index f610a25d2ef9..30405b5113d7 100644
> --- a/hw/acpi/pcihp.c
> +++ b/hw/acpi/pcihp.c
> @@ -222,9 +222,27 @@ static void acpi_pcihp_eject_slot(AcpiPciHpState *s, unsigned bsel, unsigned slo
>          PCIDevice *dev = PCI_DEVICE(qdev);
>          if (PCI_SLOT(dev->devfn) == slot) {
>              if (!acpi_pcihp_pc_no_hotplug(s, dev)) {
> -                hotplug_ctrl = qdev_get_hotplug_handler(qdev);
> -                hotplug_handler_unplug(hotplug_ctrl, qdev, &error_abort);
> -                object_unparent(OBJECT(qdev));
> +                /*
> +                 * partially_hotplugged is used by virtio-net failover:
> +                 * failover has asked the guest OS to unplug the device
> +                 * but we need to keep some references to the device
> +                 * to be able to plug it back in case of failure so
> +                 * we don't execute hotplug_handler_unplug().
> +                 */
> +                if (dev->partially_hotplugged) {
> +                    /*
> +                     * pending_deleted_event is set to true when
> +                     * virtio-net failover asks to unplug the device,
> +                     * and set to false here when the operation is done
> +                     * This is used by the migration loop to detect the
> +                     * end of the operation and really start the migration.
> +                     */
> +                    qdev->pending_deleted_event = false;
> +                } else {
> +                    hotplug_ctrl = qdev_get_hotplug_handler(qdev);
> +                    hotplug_handler_unplug(hotplug_ctrl, qdev, &error_abort);
> +                    object_unparent(OBJECT(qdev));
> +                }
>              }
>          }
>      }
> @@ -396,6 +414,12 @@ void acpi_pcihp_device_unplug_request_cb(HotplugHandler *hotplug_dev,
>          return;
>      }
>
> +    /*
> +     * pending_deleted_event is used by virtio-net failover to detect the
> +     * end of the unplug operation, the flag is set to false in
> +     * acpi_pcihp_eject_slot() when the operation is completed.
> +     */
> +    pdev->qdev.pending_deleted_event = true;
>      s->acpi_pcihp_pci_status[bsel].down |= (1U << slot);
>      acpi_send_event(DEVICE(hotplug_dev), ACPI_PCI_HOTPLUG_STATUS);
>  }
> --
> 2.33.1
>
>

Re: [PATCH v6 3/6] failover: fix unplug pending detection

[Laurent Vivier]

On 07/12/2021 05:13, Ani Sinha wrote:
> 
> 
> On Mon, 6 Dec 2021, Laurent Vivier wrote:
> 
>> Failover needs to detect the end of the PCI unplug to start migration
>> after the VFIO card has been unplugged.
>>
>> To do that, a flag is set in pcie_cap_slot_unplug_request_cb() and reset in
>> pcie_unplug_device().
>>
>> But since
>>      17858a169508 ("hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35")
>> we have switched to ACPI unplug and these functions are not called anymore
>> and the flag not set. So failover migration is not able to detect if card
>> is really unplugged and acts as it's done as soon as it's started. So it
>> doesn't wait the end of the unplug to start the migration. We don't see any
>> problem when we test that because ACPI unplug is faster than PCIe native
>> hotplug and when the migration really starts the unplug operation is
>> already done.
>>
>> See c000a9bd06ea ("pci: mark device having guest unplug request pending")
>>      a99c4da9fc2a ("pci: mark devices partially unplugged")
>>
> 
> This has already been merged upstream:
> https://git.qemu.org/?p=qemu.git;a=commit;h=9323f892b39d133eb69b301484bf7b2f3f49737d

Sorry, I forgot to rebase my series before sending.

Thank you for the remark.

Laurent

Re: [PATCH v6 1/6] qtest/libqos: add a function to initialize secondary PCI buses

[Thomas Huth]

On 06/12/2021 23.20, Laurent Vivier wrote:
> Scan the PCI devices to find bridge and set PCI_SECONDARY_BUS and
> PCI_SUBORDINATE_BUS (algorithm from seabios)
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>   include/hw/pci/pci_bridge.h |   8 +++
>   tests/qtest/libqos/pci.c    | 118 ++++++++++++++++++++++++++++++++++++
>   tests/qtest/libqos/pci.h    |   1 +
>   3 files changed, 127 insertions(+)
> 
> diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h
> index a94d350034bf..30691a6e5728 100644
> --- a/include/hw/pci/pci_bridge.h
> +++ b/include/hw/pci/pci_bridge.h
> @@ -138,6 +138,7 @@ typedef struct PCIBridgeQemuCap {
>       uint64_t mem_pref_64; /* Prefetchable memory to reserve (64-bit MMIO) */
>   } PCIBridgeQemuCap;
>   
> +#define REDHAT_PCI_CAP_TYPE_OFFSET      3
>   #define REDHAT_PCI_CAP_RESOURCE_RESERVE 1
>   
>   /*
> @@ -152,6 +153,13 @@ typedef struct PCIResReserve {
>       uint64_t mem_pref_64;
>   } PCIResReserve;
>   
> +#define REDHAT_PCI_CAP_RES_RESERVE_BUS_RES     4
> +#define REDHAT_PCI_CAP_RES_RESERVE_IO          8
> +#define REDHAT_PCI_CAP_RES_RESERVE_MEM         16
> +#define REDHAT_PCI_CAP_RES_RESERVE_PREF_MEM_32 20
> +#define REDHAT_PCI_CAP_RES_RESERVE_PREF_MEM_64 24
> +#define REDHAT_PCI_CAP_RES_RESERVE_CAP_SIZE    32
> +
>   int pci_bridge_qemu_reserve_cap_init(PCIDevice *dev, int cap_offset,
>                                  PCIResReserve res_reserve, Error **errp);
>   
> diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c
> index e1e96189c821..3f0b18f4750b 100644
> --- a/tests/qtest/libqos/pci.c
> +++ b/tests/qtest/libqos/pci.c
> @@ -13,6 +13,8 @@
>   #include "qemu/osdep.h"
>   #include "pci.h"
>   
> +#include "hw/pci/pci.h"
> +#include "hw/pci/pci_bridge.h"
>   #include "hw/pci/pci_regs.h"
>   #include "qemu/host-utils.h"
>   #include "qgraph.h"
> @@ -99,6 +101,122 @@ void qpci_device_init(QPCIDevice *dev, QPCIBus *bus, QPCIAddress *addr)
>       g_assert(!addr->device_id || device_id == addr->device_id);
>   }
>   
> +static uint8_t qpci_find_resource_reserve_capability(QPCIDevice *dev)
> +{
> +    uint16_t device_id;
> +    uint8_t cap = 0;
> +
> +    if (qpci_config_readw(dev, PCI_VENDOR_ID) != PCI_VENDOR_ID_REDHAT) {
> +        return 0;
> +    }
> +
> +    device_id = qpci_config_readw(dev, PCI_DEVICE_ID);
> +
> +    if (device_id != PCI_DEVICE_ID_REDHAT_PCIE_RP &&
> +        device_id != PCI_DEVICE_ID_REDHAT_BRIDGE) {
> +        return 0;
> +    }
> +
> +    do {
> +        cap = qpci_find_capability(dev, PCI_CAP_ID_VNDR, cap);
> +    } while (cap &&
> +             qpci_config_readb(dev, cap + REDHAT_PCI_CAP_TYPE_OFFSET) !=
> +             REDHAT_PCI_CAP_RESOURCE_RESERVE);
> +    if (cap) {
> +        uint8_t cap_len = qpci_config_readb(dev, cap + PCI_CAP_FLAGS);
> +        if (cap_len < REDHAT_PCI_CAP_RES_RESERVE_CAP_SIZE) {
> +            return 0;
> +        }
> +    }
> +    return cap;
> +}
> +
> +static void qpci_secondary_buses_rec(QPCIBus *qbus, int bus, int *pci_bus)
> +{
> +    QPCIDevice *dev;
> +    uint16_t class;
> +    uint8_t pribus, secbus, subbus;
> +    int i;

<nit>I'd maybe use a better name instead of "i" here.</nit>

> +    for (i = 0; i < 32; i++) {
> +        dev = qpci_device_find(qbus, QPCI_DEVFN(bus + i, 0));
> +        if (dev == NULL) {
> +            continue;
> +        }
> +        class = qpci_config_readw(dev, PCI_CLASS_DEVICE);
> +        if (class == PCI_CLASS_BRIDGE_PCI) {
> +            qpci_config_writeb(dev, PCI_SECONDARY_BUS, 255);
> +            qpci_config_writeb(dev, PCI_SUBORDINATE_BUS, 0);
> +        }
> +        g_free(dev);
> +    }
> +
> +    for (i = 0; i < 32; i++) {
> +        dev = qpci_device_find(qbus, QPCI_DEVFN(bus + i, 0));
> +        if (dev == NULL) {
> +            continue;
> +        }
> +        class = qpci_config_readw(dev, PCI_CLASS_DEVICE);
> +        if (class != PCI_CLASS_BRIDGE_PCI) {
> +            continue;
> +        }
> +
> +        pribus = qpci_config_readb(dev, PCI_PRIMARY_BUS);
> +        if (pribus != bus) {
> +            qpci_config_writeb(dev, PCI_PRIMARY_BUS, bus);
> +        }
> +
> +        secbus = qpci_config_readb(dev, PCI_SECONDARY_BUS);
> +        (*pci_bus)++;
> +        if (*pci_bus != secbus) {
> +            secbus = *pci_bus;
> +            qpci_config_writeb(dev, PCI_SECONDARY_BUS, secbus);
> +        }
> +
> +        subbus = qpci_config_readb(dev, PCI_SUBORDINATE_BUS);
> +        qpci_config_writeb(dev, PCI_SUBORDINATE_BUS, 255);
> +
> +        qpci_secondary_buses_rec(qbus, secbus << 5, pci_bus);
> +
> +        if (subbus != *pci_bus) {
> +            uint8_t res_bus = *pci_bus;
> +            uint8_t cap = qpci_find_resource_reserve_capability(dev);
> +
> +            if (cap) {
> +                uint32_t tmp_res_bus;
> +
> +                tmp_res_bus = qpci_config_readl(dev, cap +
> +                                            REDHAT_PCI_CAP_RES_RESERVE_BUS_RES);
> +                if (tmp_res_bus != (uint32_t)-1) {
> +                    res_bus = tmp_res_bus & 0xFF;
> +                    if ((uint8_t)(res_bus + secbus) < secbus ||
> +                        (uint8_t)(res_bus + secbus) < res_bus) {
> +                        res_bus = 0;
> +                    }
> +                    if (secbus + res_bus > *pci_bus) {
> +                        res_bus = secbus + res_bus;
> +                    }
> +                }
> +            }
> +            subbus = res_bus;
> +            *pci_bus = res_bus;
> +        }
> +
> +        qpci_config_writeb(dev, PCI_SUBORDINATE_BUS, subbus);
> +        g_free(dev);
> +    }
> +}
> +
> +int qpci_secondary_buses_init(QPCIBus *bus)
> +{
> +    int last_bus = 0;
> +
> +    qpci_secondary_buses_rec(bus, 0, &last_bus);
> +
> +    return last_bus;
> +}
> +
> +
>   void qpci_device_enable(QPCIDevice *dev)
>   {
>       uint16_t cmd;
> diff --git a/tests/qtest/libqos/pci.h b/tests/qtest/libqos/pci.h
> index ee64fdecbda8..becb800f9e6a 100644
> --- a/tests/qtest/libqos/pci.h
> +++ b/tests/qtest/libqos/pci.h
> @@ -81,6 +81,7 @@ void qpci_device_foreach(QPCIBus *bus, int vendor_id, int device_id,
>                            void *data);
>   QPCIDevice *qpci_device_find(QPCIBus *bus, int devfn);
>   void qpci_device_init(QPCIDevice *dev, QPCIBus *bus, QPCIAddress *addr);
> +int qpci_secondary_buses_init(QPCIBus *bus);
>   
>   bool qpci_has_buggy_msi(QPCIDevice *dev);
>   bool qpci_check_buggy_msi(QPCIDevice *dev);
> 

Acked-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v6 2/6] tests/qtest: add some tests for virtio-net failover

[Thomas Huth]

On 06/12/2021 23.20, Laurent Vivier wrote:
> Add test cases to test several error cases that must be
> generated by invalid failover configuration.
> 
> Add a combination of coldplug and hotplug test cases to be
> sure the primary is correctly managed according the
> presence or not of the STANDBY feature.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>   tests/qtest/meson.build           |   3 +
>   tests/qtest/virtio-net-failover.c | 690 ++++++++++++++++++++++++++++++
>   2 files changed, 693 insertions(+)
>   create mode 100644 tests/qtest/virtio-net-failover.c
> 
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index c9d8458062ff..6d66bf522156 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -22,6 +22,9 @@ qtests_generic = \
>     (config_all_devices.has_key('CONFIG_VIRTIO_SCSI') ? ['fuzz-virtio-scsi-test'] : []) + \
>     (config_all_devices.has_key('CONFIG_SB16') ? ['fuzz-sb16-test'] : []) + \
>     (config_all_devices.has_key('CONFIG_SDHCI_PCI') ? ['fuzz-sdcard-test'] : []) + \
> +  (config_all_devices.has_key('CONFIG_VIRTIO_NET') and \
> +   config_all_devices.has_key('CONFIG_Q35') and \
> +   config_all_devices.has_key('CONFIG_VIRTIO_PCI') ? ['virtio-net-failover'] : []) + \

I think you should only add this to qtests_i386 for now, since you later add 
a check to skip on non-x86 architectures.

>     [
>     'cdrom-test',
>     'device-introspect-test',
> diff --git a/tests/qtest/virtio-net-failover.c b/tests/qtest/virtio-net-failover.c
> new file mode 100644
> index 000000000000..f8f5fbb3c7fe
> --- /dev/null
> +++ b/tests/qtest/virtio-net-failover.c
> @@ -0,0 +1,690 @@

Please add a short header with at least a one-liner what this is all about 
and at least a SPDX license information here.

> +#include "qemu/osdep.h"
> +#include "libqos/libqtest.h"
> +#include "libqos/pci.h"
> +#include "libqos/pci-pc.h"
> +#include "qapi/qmp/qdict.h"
> +#include "qapi/qmp/qlist.h"
> +#include "qapi/qmp/qjson.h"
> +#include "libqos/malloc-pc.h"
> +#include "libqos/virtio-pci.h"
> +#include "hw/pci/pci.h"
> +
> +#define ACPI_PCIHP_ADDR_ICH9    0x0cc0
> +#define PCI_EJ_BASE             0x0008
> +
> +#define BASE_MACHINE "-M q35 -nodefaults " \
> +    "-device pcie-root-port,id=root0,addr=0x1,bus=pcie.0,chassis=1 " \
> +    "-device pcie-root-port,id=root1,addr=0x2,bus=pcie.0,chassis=2 "
> +
> +#define MAC_PRIMARY0 "52:54:00:11:11:11"
> +#define MAC_STANDBY0 "52:54:00:22:22:22"
> +
> +static QGuestAllocator guest_malloc;
> +static QPCIBus *pcibus;
> +
> +static QTestState *machine_start(const char *args, int numbus)
> +{
> +    QTestState *qts;
> +    QPCIDevice *dev;
> +    int i;

Nit: Use a more descriptive name instead of "i" - like "bus"?

> +    qts = qtest_init(args);
> +
> +    pc_alloc_init(&guest_malloc, qts, 0);
> +    pcibus = qpci_new_pc(qts, &guest_malloc);
> +    g_assert(qpci_secondary_buses_init(pcibus) == numbus);
> +
> +    for (i = 0; i < numbus; i++) {
> +        dev = qpci_device_find(pcibus, QPCI_DEVFN(i + 1, 0));
> +        g_assert_nonnull(dev);
> +
> +        qpci_device_enable(dev);
> +        qpci_iomap(dev, 4, NULL);
> +
> +        g_free(dev);
> +    }
> +
> +    return qts;
> +}
> +
> +static void machine_stop(QTestState *qts)
> +{
> +    qpci_free_pc(pcibus);
> +    alloc_destroy(&guest_malloc);
> +    qtest_quit(qts);
> +}
> +
> +static void test_error_id(void)
> +{
> +    QTestState *qts;
> +    QDict *resp;
> +    QDict *err;
> +
> +    qts = machine_start(BASE_MACHINE
> +                        "-device virtio-net,bus=root0,id=standby0,failover=on",
> +                        2);
> +
> +    resp = qtest_qmp(qts, "{'execute': 'device_add',"
> +                          "'arguments': {"
> +                          "'driver': 'virtio-net',"
> +                          "'bus': 'root1',"
> +                          "'failover_pair_id': 'standby0'"
> +                          "} }");
> +    g_assert(qdict_haskey(resp, "error"));
> +
> +    err = qdict_get_qdict(resp, "error");
> +    g_assert(qdict_haskey(err, "desc"));
> +
> +    g_assert_cmpstr(qdict_get_str(err, "desc"), ==,
> +                    "Device with failover_pair_id needs to have id");
> +
> +    qobject_unref(resp);
> +
> +    machine_stop(qts);
> +}
> +
> +static void test_error_pcie(void)
> +{
> +    QTestState *qts;
> +    QDict *resp;
> +    QDict *err;
> +
> +    qts = machine_start(BASE_MACHINE
> +                        "-device virtio-net,bus=root0,id=standby0,failover=on",
> +                        2);
> +
> +    resp = qtest_qmp(qts, "{'execute': 'device_add',"
> +                          "'arguments': {"
> +                          "'driver': 'virtio-net',"
> +                          "'id': 'primary0',"
> +                          "'bus': 'pcie.0',"
> +                          "'failover_pair_id': 'standby0'"
> +                          "} }");
> +    g_assert(qdict_haskey(resp, "error"));
> +
> +    err = qdict_get_qdict(resp, "error");
> +    g_assert(qdict_haskey(err, "desc"));
> +
> +    g_assert_cmpstr(qdict_get_str(err, "desc"), ==,
> +                    "Bus 'pcie.0' does not support hotplugging");
> +
> +    qobject_unref(resp);
> +
> +    machine_stop(qts);
> +}
> +
> +static QDict *find_device(QDict *bus, const char *name)
> +{
> +    const QObject *obj;
> +    QList *devices;
> +    QList *list;
> +
> +    devices = qdict_get_qlist(bus, "devices");
> +    if (devices == NULL) {
> +        return NULL;
> +    }
> +
> +    list = qlist_copy(devices);
> +    while ((obj = qlist_pop(list))) {
> +        QDict *device;
> +
> +        device = qobject_to(QDict, obj);
> +
> +        if (qdict_haskey(device, "pci_bridge")) {
> +            QDict *bridge;
> +            QDict *bridge_device;
> +
> +            bridge = qdict_get_qdict(device, "pci_bridge");
> +
> +            if (qdict_haskey(bridge, "devices")) {
> +                bridge_device = find_device(bridge, name);
> +                if (bridge_device) {
> +                    qobject_unref(list);
> +                    return bridge_device;
> +                }
> +            }
> +        }
> +
> +        if (!qdict_haskey(device, "qdev_id")) {
> +            continue;
> +        }
> +
> +        if (strcmp(qdict_get_str(device, "qdev_id"), name) == 0) {
> +            qobject_ref(device);
> +            qobject_unref(list);
> +            return device;
> +        }
> +    }
> +    qobject_unref(list);
> +
> +    return NULL;
> +}
> +
> +static QDict *get_bus(QTestState *qts, int num)
> +{
> +    QObject *obj;
> +    QDict *resp;
> +    QList *ret;
> +
> +    resp = qtest_qmp(qts, "{ 'execute': 'query-pci' }");
> +    g_assert(qdict_haskey(resp, "return"));
> +
> +    ret = qdict_get_qlist(resp, "return");
> +    g_assert_nonnull(ret);
> +
> +    while ((obj = qlist_pop(ret))) {
> +        QDict *bus;
> +
> +        bus = qobject_to(QDict, obj);
> +        if (!qdict_haskey(bus, "bus")) {
> +            continue;
> +        }
> +        if (qdict_get_int(bus, "bus") == num) {
> +            qobject_ref(bus);
> +            qobject_unref(resp);
> +            return bus;
> +        }
> +    }
> +    qobject_unref(resp);
> +
> +    return NULL;
> +}
> +
> +static char *get_mac(QTestState *qts, const char *name)
> +{
> +    QDict *resp;
> +    char *mac;
> +
> +    resp = qtest_qmp(qts, "{ 'execute': 'qom-get', "
> +                     "'arguments': { "
> +                     "'path': %s, "
> +                     "'property': 'mac' } }", name);
> +
> +    g_assert(qdict_haskey(resp, "return"));
> +
> +    mac = g_strdup( qdict_get_str(resp, "return"));
> +
> +    qobject_unref(resp);
> +
> +    return mac;
> +}
> +
> +static void check_one_card(QTestState *qts, bool present,
> +                           const char *id, const char *mac)
> +{
> +    QDict *device;
> +    QDict *bus;
> +    char *addr;
> +
> +    bus = get_bus(qts, 0);
> +    device = find_device(bus, id);
> +    if (present) {
> +        char *path;
> +
> +        g_assert_nonnull(device);
> +        qobject_unref(device);
> +
> +        path = g_strdup_printf("/machine/peripheral/%s", id);
> +        addr = get_mac(qts, path);
> +        g_free(path);
> +        g_assert_cmpstr(mac, ==, addr);
> +        g_free(addr);
> +    } else {
> +       g_assert_null(device);
> +    }
> +
> +    qobject_unref(bus);
> +}
> +
> +static void test_on(void)
> +{
> +    QTestState *qts;
> +
> +    qts = machine_start(BASE_MACHINE
> +                        "-netdev user,id=hs0 "
> +                        "-device virtio-net,bus=root0,id=standby0,"
> +                        "failover=on,netdev=hs0,mac="MAC_STANDBY0" "
> +                        "-device virtio-net,bus=root1,id=primary0,"
> +                        "failover_pair_id=standby0,netdev=hs1,mac="MAC_PRIMARY0,
> +                        2);
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    machine_stop(qts);
> +}
> +
> +static void test_on_mismatch(void)
> +{
> +    QTestState *qts;
> +
> +    qts = machine_start(BASE_MACHINE
> +                     "-netdev user,id=hs0 "
> +                     "-device virtio-net,bus=root0,id=standby0,"
> +                     "failover=on,netdev=hs0,mac="MAC_STANDBY0" "
> +                     "-netdev user,id=hs1 "
> +                     "-device virtio-net,bus=root1,id=primary0,"
> +                     "failover_pair_id=standby1,netdev=hs1,mac="MAC_PRIMARY0,
> +                     2);
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    machine_stop(qts);
> +}
> +
> +static void test_off(void)
> +{
> +    QTestState *qts;
> +
> +    qts = machine_start(BASE_MACHINE
> +                     "-netdev user,id=hs0 "
> +                     "-device virtio-net,bus=root0,id=standby0,"
> +                     "failover=off,netdev=hs0,mac="MAC_STANDBY0" "
> +                     "-netdev user,id=hs1 "
> +                     "-device virtio-net,bus=root1,id=primary0,"
> +                     "failover_pair_id=standby0,netdev=hs1,mac="MAC_PRIMARY0,
> +                     2);
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    machine_stop(qts);
> +}
> +
> +static void start_virtio_net(QTestState *qts, int bus, int slot,
> +                             const char *id)
> +{
> +    QVirtioPCIDevice *dev;
> +    uint64_t features;
> +    QPCIAddress addr;
> +    QDict *resp;
> +    QDict *data;
> +
> +    addr.devfn = QPCI_DEVFN((bus << 5) + slot, 0);
> +    dev = virtio_pci_new(pcibus, &addr);
> +    g_assert_nonnull(dev);
> +    qvirtio_pci_device_enable(dev);
> +    qvirtio_start_device(&dev->vdev);
> +    features = qvirtio_get_features(&dev->vdev);
> +    features = features & ~(QVIRTIO_F_BAD_FEATURE |
> +                            (1ull << VIRTIO_RING_F_INDIRECT_DESC) |
> +                            (1ull << VIRTIO_RING_F_EVENT_IDX));
> +    qvirtio_set_features(&dev->vdev, features);
> +    qvirtio_set_driver_ok(&dev->vdev);
> +
> +    resp = qtest_qmp_eventwait_ref(qts, "FAILOVER_NEGOTIATED");
> +    g_assert(qdict_haskey(resp, "data"));
> +
> +    data = qdict_get_qdict(resp, "data");
> +    g_assert(qdict_haskey(data, "device-id"));
> +    g_assert_cmpstr(qdict_get_str(data, "device-id"), ==, id);
> +
> +    qobject_unref(resp);
> +}
> +
> +static void test_enabled(void)
> +{
> +    QTestState *qts;
> +
> +    qts = machine_start(BASE_MACHINE
> +                     "-netdev user,id=hs0 "
> +                     "-device virtio-net,bus=root0,id=standby0,"
> +                     "failover=on,netdev=hs0,mac="MAC_STANDBY0" "
> +                     "-netdev user,id=hs1 "
> +                     "-device virtio-net,bus=root1,id=primary0,"
> +                     "failover_pair_id=standby0,netdev=hs1,mac="MAC_PRIMARY0" ",
> +                     2);
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    start_virtio_net(qts, 1, 0, "standby0");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    machine_stop(qts);
> +}
> +
> +static void test_hotplug_1(void)
> +{
> +    QTestState *qts;
> +
> +    qts = machine_start(BASE_MACHINE
> +                     "-netdev user,id=hs0 "
> +                     "-device virtio-net,bus=root0,id=standby0,"
> +                     "failover=on,netdev=hs0,mac="MAC_STANDBY0" "
> +                     "-netdev user,id=hs1 ", 2);
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    start_virtio_net(qts, 1, 0, "standby0");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "primary0",
> +                         "{'bus': 'root1',"
> +                         "'failover_pair_id': 'standby0',"
> +                         "'netdev': 'hs1',"
> +                         "'mac': '"MAC_PRIMARY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    machine_stop(qts);
> +}
> +
> +static void test_hotplug_1_reverse(void)
> +{
> +    QTestState *qts;
> +
> +    qts = machine_start(BASE_MACHINE
> +                     "-netdev user,id=hs0 "
> +                     "-netdev user,id=hs1 "
> +                     "-device virtio-net,bus=root1,id=primary0,"
> +                     "failover_pair_id=standby0,netdev=hs1,mac="MAC_PRIMARY0" ",
> +                     2);
> +
> +    check_one_card(qts, false, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "standby0",
> +                         "{'bus': 'root0',"
> +                         "'failover': 'on',"
> +                         "'netdev': 'hs0',"
> +                         "'mac': '"MAC_STANDBY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    start_virtio_net(qts, 1, 0, "standby0");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    machine_stop(qts);
> +}
> +
> +static void test_hotplug_2(void)
> +{
> +    QTestState *qts;
> +
> +    qts = machine_start(BASE_MACHINE
> +                     "-netdev user,id=hs0 "
> +                     "-netdev user,id=hs1 ",
> +                     2);
> +
> +    check_one_card(qts, false, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "standby0",
> +                         "{'bus': 'root0',"
> +                         "'failover': 'on',"
> +                         "'netdev': 'hs0',"
> +                         "'mac': '"MAC_STANDBY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    start_virtio_net(qts, 1, 0, "standby0");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "primary0",
> +                         "{'bus': 'root1',"
> +                         "'failover_pair_id': 'standby0',"
> +                         "'netdev': 'hs1',"
> +                         "'mac': '"MAC_PRIMARY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    machine_stop(qts);
> +}
> +
> +static void test_hotplug_2_reverse(void)
> +{
> +    QTestState *qts;
> +
> +    qts = machine_start(BASE_MACHINE
> +                     "-netdev user,id=hs0 "
> +                     "-netdev user,id=hs1 ",
> +                     2);
> +
> +    check_one_card(qts, false, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "primary0",
> +                         "{'bus': 'root1',"
> +                         "'failover_pair_id': 'standby0',"
> +                         "'netdev': 'hs1',"
> +                         "'mac': '"MAC_PRIMARY0"'}");
> +
> +    check_one_card(qts, false, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "standby0",
> +                         "{'bus': 'root0',"
> +                         "'failover': 'on',"
> +                         "'netdev': 'hs0',"
> +                         "'rombar': 0,"
> +                         "'romfile': '',"
> +                         "'mac': '"MAC_STANDBY0"'}");
> +
> +    /* XXX: sounds like a bug */

Could you elaborate?

> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    start_virtio_net(qts, 1, 0, "standby0");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    machine_stop(qts);
> +}
> +
> +static QDict *migrate_status(QTestState *qts)
> +{
> +    QDict *resp, *ret;
> +
> +    resp = qtest_qmp(qts, "{ 'execute': 'query-migrate' }");
> +    g_assert(qdict_haskey(resp, "return"));
> +
> +    ret = qdict_get_qdict(resp, "return");
> +    g_assert(qdict_haskey(ret, "status"));
> +    qobject_ref(ret);
> +    qobject_unref(resp);
> +
> +    return ret;
> +}
> +
> +static void test_migrate_out(gconstpointer opaque)
> +{
> +    QTestState *qts;
> +    QDict *resp, *args, *data, *ret;
> +    g_autofree gchar *uri = g_strdup_printf("exec: cat > %s", (gchar *)opaque);
> +    const gchar *status;
> +
> +    qts = machine_start(BASE_MACHINE
> +                     "-netdev user,id=hs0 "
> +                     "-netdev user,id=hs1 ",
> +                     2);
> +
> +    check_one_card(qts, false, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "standby0",
> +                         "{'bus': 'root0',"
> +                         "'failover': 'on',"
> +                         "'netdev': 'hs0',"
> +                         "'mac': '"MAC_STANDBY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    start_virtio_net(qts, 1, 0, "standby0");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "primary0",
> +                         "{'bus': 'root1',"
> +                         "'failover_pair_id': 'standby0',"
> +                         "'netdev': 'hs1',"
> +                         "'rombar': 0,"
> +                         "'romfile': '',"
> +                         "'mac': '"MAC_PRIMARY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    args = qdict_from_jsonf_nofail("{}");
> +    g_assert_nonnull(args);
> +    qdict_put_str(args, "uri", uri);
> +
> +    resp = qtest_qmp(qts, "{ 'execute': 'migrate', 'arguments': %p}", args);
> +    g_assert(qdict_haskey(resp, "return"));
> +    qobject_unref(resp);
> +
> +    /* the event is sent whan QEMU asks the OS to unplug the card */
> +    resp = qtest_qmp_eventwait_ref(qts, "UNPLUG_PRIMARY");
> +    g_assert(qdict_haskey(resp, "data"));
> +
> +    data = qdict_get_qdict(resp, "data");
> +    g_assert(qdict_haskey(data, "device-id"));
> +    g_assert_cmpstr(qdict_get_str(data, "device-id"), ==, "primary0");
> +
> +    qobject_unref(resp);
> +
> +    qtest_outl(qts, ACPI_PCIHP_ADDR_ICH9 + PCI_EJ_BASE, 1);
> +
> +    while (true) {
> +        ret = migrate_status(qts);
> +
> +        status = qdict_get_str(ret, "status");
> +        if (strcmp(status, "completed") == 0) {
> +            break;
> +        }
> +        g_assert_cmpstr(status, !=, "failed");
> +        g_assert_cmpstr(status, !=, "cancelling");
> +        g_assert_cmpstr(status, !=, "cancelled");
> +        qobject_unref(ret);
> +    }
> +    qobject_unref(ret);
> +
> +    qtest_qmp_eventwait(qts, "STOP");
> +
> +    /*
> +     * in fact, the card is ejected from the point of view of kernel
> +     * but not really from QEMU to be able to hotplug it back if
> +     * migration fails. So we can't check that:
> +     *   check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +     *   check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +     */
> +
> +    machine_stop(qts);
> +}
> +
> +static void test_migrate_in(gconstpointer opaque)
> +{
> +    QTestState *qts;
> +    QDict *resp, *args, *ret;
> +    g_autofree gchar *uri = g_strdup_printf("exec: cat %s", (gchar *)opaque);
> +
> +    qts = machine_start(BASE_MACHINE
> +                     "-netdev user,id=hs0 "
> +                     "-netdev user,id=hs1 "
> +                     "-incoming defer ",
> +                     2);
> +
> +    check_one_card(qts, false, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "standby0",
> +                         "{'bus': 'root0',"
> +                         "'failover': 'on',"
> +                         "'netdev': 'hs0',"
> +                         "'mac': '"MAC_STANDBY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "primary0",
> +                         "{'bus': 'root1',"
> +                         "'failover_pair_id': 'standby0',"
> +                         "'netdev': 'hs1',"
> +                         "'rombar': 0,"
> +                         "'romfile': '',"
> +                         "'mac': '"MAC_PRIMARY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    args = qdict_from_jsonf_nofail("{}");
> +    g_assert_nonnull(args);
> +    qdict_put_str(args, "uri", uri);
> +
> +    resp = qtest_qmp(qts, "{ 'execute': 'migrate-incoming', 'arguments': %p}",
> +                     args);
> +    g_assert(qdict_haskey(resp, "return"));
> +    qobject_unref(resp);
> +
> +    qtest_qmp_eventwait(qts, "MIGRATION");
> +    qtest_qmp_eventwait(qts, "FAILOVER_NEGOTIATED");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    qtest_qmp_eventwait(qts, "RESUME");
> +
> +    ret = migrate_status(qts);
> +    g_assert_cmpstr(qdict_get_str(ret, "status"), ==, "completed");
> +    qobject_unref(ret);
> +
> +    machine_stop(qts);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +    gchar *tmpfile = g_strdup_printf("/tmp/failover_test_migrate-%u-%u",
> +                                     getpid(), g_test_rand_int());

Could you please use g_get_tmp_dir() instead of hard-coding /tmp?

> +    const char *arch;
> +    int ret;
> +
> +    g_test_init(&argc, &argv, NULL);
> +
> +    arch = qtest_get_arch();
> +    if (strcmp(arch, "i386") && strcmp(arch, "x86_64")) {
> +        g_test_message("Skipping test for non-x86");
> +        return g_test_run();
> +    }

You could skip that architecture test if you only add the test to 
qtests_i386 in meson.build.

> +    qtest_add_func("failover-virtio-net/params/error/id", test_error_id);
> +    qtest_add_func("failover-virtio-net/params/error/pcie", test_error_pcie);
> +    qtest_add_func("failover-virtio-net/params/on", test_on);
> +    qtest_add_func("failover-virtio-net/params/on_mismatch",
> +                   test_on_mismatch);
> +    qtest_add_func("failover-virtio-net/params/off", test_off);
> +    qtest_add_func("failover-virtio-net/params/enabled", test_enabled);
> +    qtest_add_func("failover-virtio-net/hotplug_1", test_hotplug_1);
> +    qtest_add_func("failover-virtio-net/hotplug_1_reverse",
> +                   test_hotplug_1_reverse);
> +    qtest_add_func("failover-virtio-net/hotplug_2", test_hotplug_2);
> +    qtest_add_func("failover-virtio-net/hotplug_2_reverse",
> +                   test_hotplug_2_reverse);
> +    qtest_add_data_func("failover-virtio-net/migrate/out", tmpfile,
> +                        test_migrate_out);
> +    qtest_add_data_func("failover-virtio-net/migrate/in", tmpfile,
> +                        test_migrate_in);
> +
> +    ret = g_test_run();
> +
> +    unlink(tmpfile);
> +    g_free(tmpfile);
> +
> +    return ret;
> +}
> 

  Thomas

Re: [PATCH v6 4/6] tests/libqtest: update virtio-net failover test

[Thomas Huth]

On 06/12/2021 23.20, Laurent Vivier wrote:
> Update the migration test to check we correctly wait the end
> of the card unplug before doing the migration.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>   tests/qtest/virtio-net-failover.c | 34 +++++++++++++++++++++++++++++++
>   1 file changed, 34 insertions(+)
> 
> diff --git a/tests/qtest/virtio-net-failover.c b/tests/qtest/virtio-net-failover.c
> index f8f5fbb3c7fe..c88f8ddec39a 100644
> --- a/tests/qtest/virtio-net-failover.c
> +++ b/tests/qtest/virtio-net-failover.c
> @@ -560,6 +560,40 @@ static void test_migrate_out(gconstpointer opaque)
>   
>       qobject_unref(resp);
>   
> +    /* wait the end of the migration setup phase */
> +    while (true) {
> +        ret = migrate_status(qts);
> +
> +        status = qdict_get_str(ret, "status");
> +        if (strcmp(status, "wait-unplug") == 0) {
> +            break;
> +        }
> +
> +        /* The migration must not start if the card is not ejected */
> +        g_assert_cmpstr(status, !=, "active");
> +        g_assert_cmpstr(status, !=, "completed");
> +        g_assert_cmpstr(status, !=, "failed");
> +        g_assert_cmpstr(status, !=, "cancelling");
> +        g_assert_cmpstr(status, !=, "cancelled");
> +
> +        qobject_unref(ret);
> +    }
> +    qobject_unref(ret);
> +
> +    if (g_test_slow()) {
> +        /* check we stay in wait-unplug while the card is not ejected */
> +        int i;
> +
> +        for (i = 0; i < 10; i++) {

10 seconds is quite long already, even for slow mode... I wouldn't expect 
any difference after 2 or 3 seconds anymore anyway, so maybe just wait for 5 
seconds?

> +            sleep(1);
> +            ret = migrate_status(qts);
> +            status = qdict_get_str(ret, "status");
> +            g_assert_cmpstr(status, ==, "wait-unplug");
> +            qobject_unref(ret);
> +        }
> +    }
> +
> +    /* OS unplugs the cards, QEMU can move from wait-unplug state */
>       qtest_outl(qts, ACPI_PCIHP_ADDR_ICH9 + PCI_EJ_BASE, 1);
>   
>       while (true) {
> 

Acked-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v6 5/6] test/libqtest: add some virtio-net failover migration cancelling tests

[Thomas Huth]

On 06/12/2021 23.20, Laurent Vivier wrote:
> Add some tests to check the state of the machine if the migration
> is cancelled while we are using virtio-net failover.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>   tests/qtest/virtio-net-failover.c | 291 ++++++++++++++++++++++++++++++
>   1 file changed, 291 insertions(+)
> 
> diff --git a/tests/qtest/virtio-net-failover.c b/tests/qtest/virtio-net-failover.c
> index c88f8ddec39a..57abb99e7f6e 100644
> --- a/tests/qtest/virtio-net-failover.c
> +++ b/tests/qtest/virtio-net-failover.c
> @@ -682,6 +682,289 @@ static void test_migrate_in(gconstpointer opaque)
>       machine_stop(qts);
>   }
>   
> +static void test_migrate_abort_wait_unplug(gconstpointer opaque)
> +{
> +    QTestState *qts;
> +    QDict *resp, *args, *data, *ret;
> +    g_autofree gchar *uri = g_strdup_printf("exec: cat > %s", (gchar *)opaque);
> +    const gchar *status;
> +
> +    qts = machine_start(BASE_MACHINE
> +                     "-netdev user,id=hs0 "
> +                     "-netdev user,id=hs1 ",
> +                     2);
> +
> +    check_one_card(qts, false, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "standby0",
> +                         "{'bus': 'root0',"
> +                         "'failover': 'on',"
> +                         "'netdev': 'hs0',"
> +                         "'mac': '"MAC_STANDBY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    start_virtio_net(qts, 1, 0, "standby0");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "primary0",
> +                         "{'bus': 'root1',"
> +                         "'failover_pair_id': 'standby0',"
> +                         "'netdev': 'hs1',"
> +                         "'rombar': 0,"
> +                         "'romfile': '',"
> +                         "'mac': '"MAC_PRIMARY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    args = qdict_from_jsonf_nofail("{}");
> +    g_assert_nonnull(args);
> +    qdict_put_str(args, "uri", uri);
> +
> +    resp = qtest_qmp(qts, "{ 'execute': 'migrate', 'arguments': %p}", args);
> +    g_assert(qdict_haskey(resp, "return"));
> +    qobject_unref(resp);
> +
> +    /* the event is sent whan QEMU asks the OS to unplug the card */
> +    resp = qtest_qmp_eventwait_ref(qts, "UNPLUG_PRIMARY");
> +    g_assert(qdict_haskey(resp, "data"));
> +
> +    data = qdict_get_qdict(resp, "data");
> +    g_assert(qdict_haskey(data, "device-id"));
> +    g_assert_cmpstr(qdict_get_str(data, "device-id"), ==, "primary0");
> +
> +    qobject_unref(resp);
> +
> +    resp = qtest_qmp(qts, "{ 'execute': 'migrate_cancel' }");
> +    g_assert(qdict_haskey(resp, "return"));
> +    qobject_unref(resp);
> +
> +    /* migration has been cancelled while the unplug was in progress */
> +
> +    /* while the card is not ejected, we must be in "cancelling" state */
> +    ret = migrate_status(qts);
> +
> +    status = qdict_get_str(ret, "status");
> +    g_assert_cmpstr(status, ==, "cancelling");
> +    qobject_unref(ret);
> +
> +    /* OS unplugs the cards, QEMU can move from wait-unplug state */
> +    qtest_outl(qts, ACPI_PCIHP_ADDR_ICH9 + PCI_EJ_BASE, 1);
> +
> +    while (true) {
> +        ret = migrate_status(qts);
> +
> +        status = qdict_get_str(ret, "status");
> +        if (strcmp(status, "cancelled") == 0) {
> +            break;
> +        }
> +        g_assert_cmpstr(status, !=, "failed");
> +        g_assert_cmpstr(status, !=, "active");
> +        qobject_unref(ret);
> +    }
> +    qobject_unref(ret);
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    machine_stop(qts);
> +}
> +
> +static void test_migrate_abort_active(gconstpointer opaque)
> +{
> +    QTestState *qts;
> +    QDict *resp, *args, *data, *ret;
> +    g_autofree gchar *uri = g_strdup_printf("exec: cat > %s", (gchar *)opaque);
> +    const gchar *status;
> +
> +    qts = machine_start(BASE_MACHINE
> +                     "-netdev user,id=hs0 "
> +                     "-netdev user,id=hs1 ",
> +                     2);
> +
> +    check_one_card(qts, false, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "standby0",
> +                         "{'bus': 'root0',"
> +                         "'failover': 'on',"
> +                         "'netdev': 'hs0',"
> +                         "'mac': '"MAC_STANDBY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    start_virtio_net(qts, 1, 0, "standby0");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "primary0",
> +                         "{'bus': 'root1',"
> +                         "'failover_pair_id': 'standby0',"
> +                         "'netdev': 'hs1',"
> +                         "'rombar': 0,"
> +                         "'romfile': '',"
> +                         "'mac': '"MAC_PRIMARY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    args = qdict_from_jsonf_nofail("{}");
> +    g_assert_nonnull(args);
> +    qdict_put_str(args, "uri", uri);
> +
> +    resp = qtest_qmp(qts, "{ 'execute': 'migrate', 'arguments': %p}", args);
> +    g_assert(qdict_haskey(resp, "return"));
> +    qobject_unref(resp);
> +
> +    /* the event is sent whan QEMU asks the OS to unplug the card */
> +    resp = qtest_qmp_eventwait_ref(qts, "UNPLUG_PRIMARY");
> +    g_assert(qdict_haskey(resp, "data"));
> +
> +    data = qdict_get_qdict(resp, "data");
> +    g_assert(qdict_haskey(data, "device-id"));
> +    g_assert_cmpstr(qdict_get_str(data, "device-id"), ==, "primary0");
> +
> +    qobject_unref(resp);
> +
> +    /* OS unplugs the cards, QEMU can move from wait-unplug state */
> +    qtest_outl(qts, ACPI_PCIHP_ADDR_ICH9 + PCI_EJ_BASE, 1);
> +
> +    while (true) {
> +        ret = migrate_status(qts);
> +
> +        status = qdict_get_str(ret, "status");
> +        if (strcmp(status, "wait-unplug") != 0) {
> +            break;
> +        }
> +        g_assert_cmpstr(status, !=, "failed");
> +        qobject_unref(ret);
> +    }
> +    qobject_unref(resp);
> +
> +    resp = qtest_qmp(qts, "{ 'execute': 'migrate_cancel' }");
> +    g_assert(qdict_haskey(resp, "return"));
> +    qobject_unref(resp);
> +
> +    while (true) {
> +        ret = migrate_status(qts);
> +
> +        status = qdict_get_str(ret, "status");
> +        if (strcmp(status, "cancelled") == 0) {
> +            break;
> +        }
> +        g_assert_cmpstr(status, !=, "failed");
> +        g_assert_cmpstr(status, !=, "active");
> +        qobject_unref(ret);
> +    }
> +    qobject_unref(ret);
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    machine_stop(qts);
> +}
> +
> +static void test_migrate_abort_timeout(gconstpointer opaque)
> +{
> +    QTestState *qts;
> +    QDict *resp, *args, *data, *ret;
> +    g_autofree gchar *uri = g_strdup_printf("exec: cat > %s", (gchar *)opaque);
> +    const gchar *status;
> +    int total;
> +
> +    qts = machine_start(BASE_MACHINE
> +                     "-netdev user,id=hs0 "
> +                     "-netdev user,id=hs1 ",
> +                     2);
> +
> +    check_one_card(qts, false, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "standby0",
> +                         "{'bus': 'root0',"
> +                         "'failover': 'on',"
> +                         "'netdev': 'hs0',"
> +                         "'mac': '"MAC_STANDBY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    start_virtio_net(qts, 1, 0, "standby0");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "primary0",
> +                         "{'bus': 'root1',"
> +                         "'failover_pair_id': 'standby0',"
> +                         "'netdev': 'hs1',"
> +                         "'rombar': 0,"
> +                         "'romfile': '',"
> +                         "'mac': '"MAC_PRIMARY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    args = qdict_from_jsonf_nofail("{}");
> +    g_assert_nonnull(args);
> +    qdict_put_str(args, "uri", uri);
> +
> +    resp = qtest_qmp(qts, "{ 'execute': 'migrate', 'arguments': %p}", args);
> +    g_assert(qdict_haskey(resp, "return"));
> +    qobject_unref(resp);
> +
> +    /* the event is sent whan QEMU asks the OS to unplug the card */
> +    resp = qtest_qmp_eventwait_ref(qts, "UNPLUG_PRIMARY");
> +    g_assert(qdict_haskey(resp, "data"));
> +
> +    data = qdict_get_qdict(resp, "data");
> +    g_assert(qdict_haskey(data, "device-id"));
> +    g_assert_cmpstr(qdict_get_str(data, "device-id"), ==, "primary0");
> +
> +    qobject_unref(resp);
> +
> +    resp = qtest_qmp(qts, "{ 'execute': 'migrate_cancel' }");
> +    g_assert(qdict_haskey(resp, "return"));
> +    qobject_unref(resp);
> +
> +    /* migration has been cancelled while the unplug was in progress */
> +
> +    /* while the card is not ejected, we must be in "cancelling" state */
> +
> +    total = 0;
> +    while (true) {
> +        ret = migrate_status(qts);
> +
> +        status = qdict_get_str(ret, "status");
> +        if (strcmp(status, "cancelled") == 0) {
> +            break;
> +        }
> +        g_assert_cmpstr(status, ==, "cancelling");
> +        g_assert(qdict_haskey(ret, "total-time"));
> +        total = qdict_get_int(ret, "total-time");
> +        qobject_unref(ret);
> +    }
> +    qobject_unref(ret);
> +
> +    /*
> +     * migration timeout in this case is 30 seconds
> +     * check we exit on the timeout (ms)
> +     */
> +    g_assert_cmpint(total, >, 30000);
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    machine_stop(qts);
> +}
> +
>   int main(int argc, char **argv)
>   {
>       gchar *tmpfile = g_strdup_printf("/tmp/failover_test_migrate-%u-%u",
> @@ -714,6 +997,14 @@ int main(int argc, char **argv)
>                           test_migrate_out);
>       qtest_add_data_func("failover-virtio-net/migrate/in", tmpfile,
>                           test_migrate_in);
> +    qtest_add_data_func("failover-virtio-net/migrate/abort/wait-unplug",
> +                        tmpfile, test_migrate_abort_wait_unplug);
> +    qtest_add_data_func("failover-virtio-net/migrate/abort/active", tmpfile,
> +                        test_migrate_abort_active);
> +    if (g_test_slow()) {
> +        qtest_add_data_func("failover-virtio-net/migrate/abort/timeout",
> +                            tmpfile, test_migrate_abort_timeout);
> +    }
>   
>       ret = g_test_run();

Acked-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v6 6/6] tests/libqtest: add a migration test with two couples of failover devices

[Thomas Huth]

On 06/12/2021 23.20, Laurent Vivier wrote:
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>   tests/qtest/virtio-net-failover.c | 279 ++++++++++++++++++++++++++++++
>   1 file changed, 279 insertions(+)
> 
> diff --git a/tests/qtest/virtio-net-failover.c b/tests/qtest/virtio-net-failover.c
> index 57abb99e7f6e..ace9001894af 100644
> --- a/tests/qtest/virtio-net-failover.c
> +++ b/tests/qtest/virtio-net-failover.c
> @@ -11,6 +11,7 @@
>   
>   #define ACPI_PCIHP_ADDR_ICH9    0x0cc0
>   #define PCI_EJ_BASE             0x0008
> +#define PCI_SEL_BASE            0x0010
>   
>   #define BASE_MACHINE "-M q35 -nodefaults " \
>       "-device pcie-root-port,id=root0,addr=0x1,bus=pcie.0,chassis=1 " \
> @@ -18,6 +19,8 @@
>   
>   #define MAC_PRIMARY0 "52:54:00:11:11:11"
>   #define MAC_STANDBY0 "52:54:00:22:22:22"
> +#define MAC_PRIMARY1 "52:54:00:33:33:33"
> +#define MAC_STANDBY1 "52:54:00:44:44:44"
>   
>   static QGuestAllocator guest_malloc;
>   static QPCIBus *pcibus;
> @@ -965,6 +968,278 @@ static void test_migrate_abort_timeout(gconstpointer opaque)
>       machine_stop(qts);
>   }
>   
> +static void test_multi_out(gconstpointer opaque)
> +{
> +    QTestState *qts;
> +    QDict *resp, *args, *data, *ret;
> +    g_autofree gchar *uri = g_strdup_printf("exec: cat > %s", (gchar *)opaque);
> +    const gchar *status, *expected;
> +
> +    qts = machine_start(BASE_MACHINE
> +                "-device pcie-root-port,id=root2,addr=0x3,bus=pcie.0,chassis=3 "
> +                "-device pcie-root-port,id=root3,addr=0x4,bus=pcie.0,chassis=4 "
> +                "-netdev user,id=hs0 "
> +                "-netdev user,id=hs1 "
> +                "-netdev user,id=hs2 "
> +                "-netdev user,id=hs3 ",
> +                4);
> +
> +    check_one_card(qts, false, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +    check_one_card(qts, false, "standby1", MAC_STANDBY1);
> +    check_one_card(qts, false, "primary1", MAC_PRIMARY1);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "standby0",
> +                         "{'bus': 'root0',"
> +                         "'failover': 'on',"
> +                         "'netdev': 'hs0',"
> +                         "'mac': '"MAC_STANDBY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +    check_one_card(qts, false, "standby1", MAC_STANDBY1);
> +    check_one_card(qts, false, "primary1", MAC_PRIMARY1);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "primary0",
> +                         "{'bus': 'root1',"
> +                         "'failover_pair_id': 'standby0',"
> +                         "'netdev': 'hs1',"
> +                         "'rombar': 0,"
> +                         "'romfile': '',"
> +                         "'mac': '"MAC_PRIMARY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +    check_one_card(qts, false, "standby1", MAC_STANDBY1);
> +    check_one_card(qts, false, "primary1", MAC_PRIMARY1);
> +
> +    start_virtio_net(qts, 1, 0, "standby0");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +    check_one_card(qts, false, "standby1", MAC_STANDBY1);
> +    check_one_card(qts, false, "primary1", MAC_PRIMARY1);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "standby1",
> +                         "{'bus': 'root2',"
> +                         "'failover': 'on',"
> +                         "'netdev': 'hs2',"
> +                         "'mac': '"MAC_STANDBY1"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +    check_one_card(qts, true, "standby1", MAC_STANDBY1);
> +    check_one_card(qts, false, "primary1", MAC_PRIMARY1);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "primary1",
> +                         "{'bus': 'root3',"
> +                         "'failover_pair_id': 'standby1',"
> +                         "'netdev': 'hs3',"
> +                         "'rombar': 0,"
> +                         "'romfile': '',"
> +                         "'mac': '"MAC_PRIMARY1"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +    check_one_card(qts, true, "standby1", MAC_STANDBY1);
> +    check_one_card(qts, false, "primary1", MAC_PRIMARY1);
> +
> +    start_virtio_net(qts, 3, 0, "standby1");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +    check_one_card(qts, true, "standby1", MAC_STANDBY1);
> +    check_one_card(qts, true, "primary1", MAC_PRIMARY1);
> +
> +    args = qdict_from_jsonf_nofail("{}");
> +    g_assert_nonnull(args);
> +    qdict_put_str(args, "uri", uri);
> +
> +    resp = qtest_qmp(qts, "{ 'execute': 'migrate', 'arguments': %p}", args);
> +    g_assert(qdict_haskey(resp, "return"));
> +    qobject_unref(resp);
> +
> +    /* the event is sent whan QEMU asks the OS to unplug the card */
> +    resp = qtest_qmp_eventwait_ref(qts, "UNPLUG_PRIMARY");
> +    g_assert(qdict_haskey(resp, "data"));
> +
> +    data = qdict_get_qdict(resp, "data");
> +    g_assert(qdict_haskey(data, "device-id"));
> +    if (strcmp(qdict_get_str(data, "device-id"), "primary0") == 0) {
> +        expected = "primary1";
> +    } else if (strcmp(qdict_get_str(data, "device-id"), "primary1") == 0) {
> +        expected = "primary0";
> +    } else {
> +        g_assert_not_reached();
> +    }
> +    qobject_unref(resp);
> +
> +    resp = qtest_qmp_eventwait_ref(qts, "UNPLUG_PRIMARY");
> +    g_assert(qdict_haskey(resp, "data"));
> +
> +    data = qdict_get_qdict(resp, "data");
> +    g_assert(qdict_haskey(data, "device-id"));
> +    g_assert_cmpstr(qdict_get_str(data, "device-id"), ==, expected);
> +    qobject_unref(resp);
> +
> +    /* wait the end of the migration setup phase */
> +    while (true) {
> +        ret = migrate_status(qts);
> +
> +        status = qdict_get_str(ret, "status");
> +        if (strcmp(status, "wait-unplug") == 0) {
> +            break;
> +        }
> +
> +        /* The migration must not start if the card is not ejected */
> +        g_assert_cmpstr(status, !=, "active");
> +        g_assert_cmpstr(status, !=, "completed");
> +        g_assert_cmpstr(status, !=, "failed");
> +        g_assert_cmpstr(status, !=, "cancelling");
> +        g_assert_cmpstr(status, !=, "cancelled");
> +
> +        qobject_unref(ret);
> +    }
> +    qobject_unref(ret);
> +
> +    /* OS unplugs primary1, but we must wait the second */
> +    qtest_outl(qts, ACPI_PCIHP_ADDR_ICH9 + PCI_EJ_BASE, 1);
> +
> +    ret = migrate_status(qts);
> +    status = qdict_get_str(ret, "status");
> +    g_assert_cmpstr(status, ==, "wait-unplug");
> +    qobject_unref(ret);
> +
> +    if (g_test_slow()) {
> +        /* check we stay in wait-unplug while the card is not ejected */
> +        int i;
> +
> +        for (i = 0; i < 10; i++) {

We're using gnu11 mode these days, so you could also write
  "for (int i = 0; ...)".
Maybe also consider to limit the test to 5 seconds.

> +            sleep(1);
> +            ret = migrate_status(qts);
> +            status = qdict_get_str(ret, "status");
> +            g_assert_cmpstr(status, ==, "wait-unplug");
> +            qobject_unref(ret);
> +        }
> +    }
> +
> +    /* OS unplugs primary0, QEMU can move from wait-unplug state */
> +    qtest_outl(qts, ACPI_PCIHP_ADDR_ICH9 + PCI_SEL_BASE, 2);
> +    qtest_outl(qts, ACPI_PCIHP_ADDR_ICH9 + PCI_EJ_BASE, 1);
> +
> +    while (true) {
> +        ret = migrate_status(qts);
> +
> +        status = qdict_get_str(ret, "status");
> +        if (strcmp(status, "completed") == 0) {
> +            break;
> +        }
> +        g_assert_cmpstr(status, !=, "failed");
> +        g_assert_cmpstr(status, !=, "cancelling");
> +        g_assert_cmpstr(status, !=, "cancelled");
> +    }
> +
> +    qtest_qmp_eventwait(qts, "STOP");
> +
> +    machine_stop(qts);
> +}
> +
> +static void test_multi_in(gconstpointer opaque)
> +{
> +    QTestState *qts;
> +    QDict *resp, *args, *ret;
> +    g_autofree gchar *uri = g_strdup_printf("exec: cat %s", (gchar *)opaque);
> +
> +    qts = machine_start(BASE_MACHINE
> +                "-device pcie-root-port,id=root2,addr=0x3,bus=pcie.0,chassis=3 "
> +                "-device pcie-root-port,id=root3,addr=0x4,bus=pcie.0,chassis=4 "
> +                "-netdev user,id=hs0 "
> +                "-netdev user,id=hs1 "
> +                "-netdev user,id=hs2 "
> +                "-netdev user,id=hs3 "
> +                "-incoming defer ",
> +                4);
> +
> +    check_one_card(qts, false, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +    check_one_card(qts, false, "standby1", MAC_STANDBY1);
> +    check_one_card(qts, false, "primary1", MAC_PRIMARY1);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "standby0",
> +                         "{'bus': 'root0',"
> +                         "'failover': 'on',"
> +                         "'netdev': 'hs0',"
> +                         "'mac': '"MAC_STANDBY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +    check_one_card(qts, false, "standby1", MAC_STANDBY1);
> +    check_one_card(qts, false, "primary1", MAC_PRIMARY1);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "primary0",
> +                         "{'bus': 'root1',"
> +                         "'failover_pair_id': 'standby0',"
> +                         "'netdev': 'hs1',"
> +                         "'rombar': 0,"
> +                         "'romfile': '',"
> +                         "'mac': '"MAC_PRIMARY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +    check_one_card(qts, false, "standby1", MAC_STANDBY1);
> +    check_one_card(qts, false, "primary1", MAC_PRIMARY1);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "standby1",
> +                         "{'bus': 'root2',"
> +                         "'failover': 'on',"
> +                         "'netdev': 'hs2',"
> +                         "'mac': '"MAC_STANDBY1"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +    check_one_card(qts, true, "standby1", MAC_STANDBY1);
> +    check_one_card(qts, false, "primary1", MAC_PRIMARY1);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "primary1",
> +                         "{'bus': 'root3',"
> +                         "'failover_pair_id': 'standby1',"
> +                         "'netdev': 'hs3',"
> +                         "'rombar': 0,"
> +                         "'romfile': '',"
> +                         "'mac': '"MAC_PRIMARY1"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +    check_one_card(qts, true, "standby1", MAC_STANDBY1);
> +    check_one_card(qts, false, "primary1", MAC_PRIMARY1);
> +
> +    args = qdict_from_jsonf_nofail("{}");
> +    g_assert_nonnull(args);
> +    qdict_put_str(args, "uri", uri);
> +
> +    resp = qtest_qmp(qts, "{ 'execute': 'migrate-incoming', 'arguments': %p}",
> +                     args);
> +    g_assert(qdict_haskey(resp, "return"));
> +    qobject_unref(resp);
> +
> +    qtest_qmp_eventwait(qts, "MIGRATION");
> +    qtest_qmp_eventwait(qts, "FAILOVER_NEGOTIATED");
> +    qtest_qmp_eventwait(qts, "FAILOVER_NEGOTIATED");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +    check_one_card(qts, true, "standby1", MAC_STANDBY1);
> +    check_one_card(qts, true, "primary1", MAC_PRIMARY1);
> +
> +    qtest_qmp_eventwait(qts, "RESUME");
> +
> +    ret = migrate_status(qts);
> +    g_assert_cmpstr(qdict_get_str(ret, "status"), ==, "completed");
> +    qobject_unref(ret);
> +
> +    machine_stop(qts);
> +}
> +
>   int main(int argc, char **argv)
>   {
>       gchar *tmpfile = g_strdup_printf("/tmp/failover_test_migrate-%u-%u",
> @@ -1005,6 +1280,10 @@ int main(int argc, char **argv)
>           qtest_add_data_func("failover-virtio-net/migrate/abort/timeout",
>                               tmpfile, test_migrate_abort_timeout);
>       }
> +    qtest_add_data_func("failover-virtio-net/multi/out",
> +                        tmpfile, test_multi_out);
> +    qtest_add_data_func("failover-virtio-net/multi/in",
> +                   tmpfile, test_multi_in);
>   
>       ret = g_test_run();
>   
> 

Acked-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v6 5/6] test/libqtest: add some virtio-net failover migration cancelling tests

[Michael S. Tsirkin]

On Mon, Dec 06, 2021 at 11:20:39PM +0100, Laurent Vivier wrote:
> Add some tests to check the state of the machine if the migration
> is cancelled while we are using virtio-net failover.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

So this one I think is needed for the release. Thomas, are you
merging it there or should I?

> ---
>  tests/qtest/virtio-net-failover.c | 291 ++++++++++++++++++++++++++++++
>  1 file changed, 291 insertions(+)
> 
> diff --git a/tests/qtest/virtio-net-failover.c b/tests/qtest/virtio-net-failover.c
> index c88f8ddec39a..57abb99e7f6e 100644
> --- a/tests/qtest/virtio-net-failover.c
> +++ b/tests/qtest/virtio-net-failover.c
> @@ -682,6 +682,289 @@ static void test_migrate_in(gconstpointer opaque)
>      machine_stop(qts);
>  }
>  
> +static void test_migrate_abort_wait_unplug(gconstpointer opaque)
> +{
> +    QTestState *qts;
> +    QDict *resp, *args, *data, *ret;
> +    g_autofree gchar *uri = g_strdup_printf("exec: cat > %s", (gchar *)opaque);
> +    const gchar *status;
> +
> +    qts = machine_start(BASE_MACHINE
> +                     "-netdev user,id=hs0 "
> +                     "-netdev user,id=hs1 ",
> +                     2);
> +
> +    check_one_card(qts, false, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "standby0",
> +                         "{'bus': 'root0',"
> +                         "'failover': 'on',"
> +                         "'netdev': 'hs0',"
> +                         "'mac': '"MAC_STANDBY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    start_virtio_net(qts, 1, 0, "standby0");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "primary0",
> +                         "{'bus': 'root1',"
> +                         "'failover_pair_id': 'standby0',"
> +                         "'netdev': 'hs1',"
> +                         "'rombar': 0,"
> +                         "'romfile': '',"
> +                         "'mac': '"MAC_PRIMARY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    args = qdict_from_jsonf_nofail("{}");
> +    g_assert_nonnull(args);
> +    qdict_put_str(args, "uri", uri);
> +
> +    resp = qtest_qmp(qts, "{ 'execute': 'migrate', 'arguments': %p}", args);
> +    g_assert(qdict_haskey(resp, "return"));
> +    qobject_unref(resp);
> +
> +    /* the event is sent whan QEMU asks the OS to unplug the card */
> +    resp = qtest_qmp_eventwait_ref(qts, "UNPLUG_PRIMARY");
> +    g_assert(qdict_haskey(resp, "data"));
> +
> +    data = qdict_get_qdict(resp, "data");
> +    g_assert(qdict_haskey(data, "device-id"));
> +    g_assert_cmpstr(qdict_get_str(data, "device-id"), ==, "primary0");
> +
> +    qobject_unref(resp);
> +
> +    resp = qtest_qmp(qts, "{ 'execute': 'migrate_cancel' }");
> +    g_assert(qdict_haskey(resp, "return"));
> +    qobject_unref(resp);
> +
> +    /* migration has been cancelled while the unplug was in progress */
> +
> +    /* while the card is not ejected, we must be in "cancelling" state */
> +    ret = migrate_status(qts);
> +
> +    status = qdict_get_str(ret, "status");
> +    g_assert_cmpstr(status, ==, "cancelling");
> +    qobject_unref(ret);
> +
> +    /* OS unplugs the cards, QEMU can move from wait-unplug state */
> +    qtest_outl(qts, ACPI_PCIHP_ADDR_ICH9 + PCI_EJ_BASE, 1);
> +
> +    while (true) {
> +        ret = migrate_status(qts);
> +
> +        status = qdict_get_str(ret, "status");
> +        if (strcmp(status, "cancelled") == 0) {
> +            break;
> +        }
> +        g_assert_cmpstr(status, !=, "failed");
> +        g_assert_cmpstr(status, !=, "active");
> +        qobject_unref(ret);
> +    }
> +    qobject_unref(ret);
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    machine_stop(qts);
> +}
> +
> +static void test_migrate_abort_active(gconstpointer opaque)
> +{
> +    QTestState *qts;
> +    QDict *resp, *args, *data, *ret;
> +    g_autofree gchar *uri = g_strdup_printf("exec: cat > %s", (gchar *)opaque);
> +    const gchar *status;
> +
> +    qts = machine_start(BASE_MACHINE
> +                     "-netdev user,id=hs0 "
> +                     "-netdev user,id=hs1 ",
> +                     2);
> +
> +    check_one_card(qts, false, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "standby0",
> +                         "{'bus': 'root0',"
> +                         "'failover': 'on',"
> +                         "'netdev': 'hs0',"
> +                         "'mac': '"MAC_STANDBY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    start_virtio_net(qts, 1, 0, "standby0");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "primary0",
> +                         "{'bus': 'root1',"
> +                         "'failover_pair_id': 'standby0',"
> +                         "'netdev': 'hs1',"
> +                         "'rombar': 0,"
> +                         "'romfile': '',"
> +                         "'mac': '"MAC_PRIMARY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    args = qdict_from_jsonf_nofail("{}");
> +    g_assert_nonnull(args);
> +    qdict_put_str(args, "uri", uri);
> +
> +    resp = qtest_qmp(qts, "{ 'execute': 'migrate', 'arguments': %p}", args);
> +    g_assert(qdict_haskey(resp, "return"));
> +    qobject_unref(resp);
> +
> +    /* the event is sent whan QEMU asks the OS to unplug the card */
> +    resp = qtest_qmp_eventwait_ref(qts, "UNPLUG_PRIMARY");
> +    g_assert(qdict_haskey(resp, "data"));
> +
> +    data = qdict_get_qdict(resp, "data");
> +    g_assert(qdict_haskey(data, "device-id"));
> +    g_assert_cmpstr(qdict_get_str(data, "device-id"), ==, "primary0");
> +
> +    qobject_unref(resp);
> +
> +    /* OS unplugs the cards, QEMU can move from wait-unplug state */
> +    qtest_outl(qts, ACPI_PCIHP_ADDR_ICH9 + PCI_EJ_BASE, 1);
> +
> +    while (true) {
> +        ret = migrate_status(qts);
> +
> +        status = qdict_get_str(ret, "status");
> +        if (strcmp(status, "wait-unplug") != 0) {
> +            break;
> +        }
> +        g_assert_cmpstr(status, !=, "failed");
> +        qobject_unref(ret);
> +    }
> +    qobject_unref(resp);
> +
> +    resp = qtest_qmp(qts, "{ 'execute': 'migrate_cancel' }");
> +    g_assert(qdict_haskey(resp, "return"));
> +    qobject_unref(resp);
> +
> +    while (true) {
> +        ret = migrate_status(qts);
> +
> +        status = qdict_get_str(ret, "status");
> +        if (strcmp(status, "cancelled") == 0) {
> +            break;
> +        }
> +        g_assert_cmpstr(status, !=, "failed");
> +        g_assert_cmpstr(status, !=, "active");
> +        qobject_unref(ret);
> +    }
> +    qobject_unref(ret);
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    machine_stop(qts);
> +}
> +
> +static void test_migrate_abort_timeout(gconstpointer opaque)
> +{
> +    QTestState *qts;
> +    QDict *resp, *args, *data, *ret;
> +    g_autofree gchar *uri = g_strdup_printf("exec: cat > %s", (gchar *)opaque);
> +    const gchar *status;
> +    int total;
> +
> +    qts = machine_start(BASE_MACHINE
> +                     "-netdev user,id=hs0 "
> +                     "-netdev user,id=hs1 ",
> +                     2);
> +
> +    check_one_card(qts, false, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "standby0",
> +                         "{'bus': 'root0',"
> +                         "'failover': 'on',"
> +                         "'netdev': 'hs0',"
> +                         "'mac': '"MAC_STANDBY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    start_virtio_net(qts, 1, 0, "standby0");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, false, "primary0", MAC_PRIMARY0);
> +
> +    qtest_qmp_device_add(qts, "virtio-net", "primary0",
> +                         "{'bus': 'root1',"
> +                         "'failover_pair_id': 'standby0',"
> +                         "'netdev': 'hs1',"
> +                         "'rombar': 0,"
> +                         "'romfile': '',"
> +                         "'mac': '"MAC_PRIMARY0"'}");
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    args = qdict_from_jsonf_nofail("{}");
> +    g_assert_nonnull(args);
> +    qdict_put_str(args, "uri", uri);
> +
> +    resp = qtest_qmp(qts, "{ 'execute': 'migrate', 'arguments': %p}", args);
> +    g_assert(qdict_haskey(resp, "return"));
> +    qobject_unref(resp);
> +
> +    /* the event is sent whan QEMU asks the OS to unplug the card */
> +    resp = qtest_qmp_eventwait_ref(qts, "UNPLUG_PRIMARY");
> +    g_assert(qdict_haskey(resp, "data"));
> +
> +    data = qdict_get_qdict(resp, "data");
> +    g_assert(qdict_haskey(data, "device-id"));
> +    g_assert_cmpstr(qdict_get_str(data, "device-id"), ==, "primary0");
> +
> +    qobject_unref(resp);
> +
> +    resp = qtest_qmp(qts, "{ 'execute': 'migrate_cancel' }");
> +    g_assert(qdict_haskey(resp, "return"));
> +    qobject_unref(resp);
> +
> +    /* migration has been cancelled while the unplug was in progress */
> +
> +    /* while the card is not ejected, we must be in "cancelling" state */
> +
> +    total = 0;
> +    while (true) {
> +        ret = migrate_status(qts);
> +
> +        status = qdict_get_str(ret, "status");
> +        if (strcmp(status, "cancelled") == 0) {
> +            break;
> +        }
> +        g_assert_cmpstr(status, ==, "cancelling");
> +        g_assert(qdict_haskey(ret, "total-time"));
> +        total = qdict_get_int(ret, "total-time");
> +        qobject_unref(ret);
> +    }
> +    qobject_unref(ret);
> +
> +    /*
> +     * migration timeout in this case is 30 seconds
> +     * check we exit on the timeout (ms)
> +     */
> +    g_assert_cmpint(total, >, 30000);
> +
> +    check_one_card(qts, true, "standby0", MAC_STANDBY0);
> +    check_one_card(qts, true, "primary0", MAC_PRIMARY0);
> +
> +    machine_stop(qts);
> +}
> +
>  int main(int argc, char **argv)
>  {
>      gchar *tmpfile = g_strdup_printf("/tmp/failover_test_migrate-%u-%u",
> @@ -714,6 +997,14 @@ int main(int argc, char **argv)
>                          test_migrate_out);
>      qtest_add_data_func("failover-virtio-net/migrate/in", tmpfile,
>                          test_migrate_in);
> +    qtest_add_data_func("failover-virtio-net/migrate/abort/wait-unplug",
> +                        tmpfile, test_migrate_abort_wait_unplug);
> +    qtest_add_data_func("failover-virtio-net/migrate/abort/active", tmpfile,
> +                        test_migrate_abort_active);
> +    if (g_test_slow()) {
> +        qtest_add_data_func("failover-virtio-net/migrate/abort/timeout",
> +                            tmpfile, test_migrate_abort_timeout);
> +    }
>  
>      ret = g_test_run();
>  
> -- 
> 2.33.1
> 
> 
> 

Re: [PATCH v6 0/6] tests/qtest: add some tests for virtio-net failover

[Michael S. Tsirkin]

On Mon, Dec 06, 2021 at 11:20:34PM +0100, Laurent Vivier wrote:
> This series adds a qtest entry to test virtio-net failover feature.

I think it's a good idea to CC me and Jason on the next version.
Thanks!


> We check following error cases:
> 
> - check missing id on device with failover_pair_id triggers an error
> - check a primary device plugged on a bus that doesn't support hotplug
>   triggers an error
> 
> We check the status of the machine before and after hotplugging cards and
> feature negotiation:
> 
> - check we don't see the primary device at boot if failover is on
> - check we see the primary device at boot if failover is off
> - check we don't see the primary device if failover is on
>   but failover_pair_id is not the one with on (I think this should be changed)
> - check the primary device is plugged after the feature negotiation
> - check the result if the primary device is plugged before standby device and
>   vice-versa
> - check the if the primary device is coldplugged and the standy device
>   hotplugged and vice-versa
> - check the migration triggers the unplug and the hotplug
> 
> There is one preliminary patch in the series:
> 
> - PATCH 1 introduces a function to enable PCI bridge.
>   Failover needs to be plugged on a pcie-root-port and while
>   the root port is not configured the cards behind it are not
>   available
> 
> v6:
> - manage more than 2 root ports
> - add a function to check if a card is available or not
> - check migration state
> - add cancelled migration test cases
> - rename tests
> 
> v5:
> - re-add the wait-unplug test that has been removed from v4 by mistake.
> 
> v4:
> - rely on query-migrate status to know the migration state rather than
>   to wait the STOP event.
> - remove the patch to add time out to qtest_qmp_eventwait()
> 
> v3:
> - fix a bug with ACPI unplug and add the related test
> 
> v2:
> - remove PATCH 1 that introduced a function that can be replaced by
>   qobject_to_json_pretty() (Markus)
> - Add migration to a file and from the file to check the card is
>   correctly unplugged on the source, and hotplugged on the dest
> - Add an ACPI call to eject the card as the kernel would do
> 
> Laurent Vivier (6):
>   qtest/libqos: add a function to initialize secondary PCI buses
>   tests/qtest: add some tests for virtio-net failover
>   failover: fix unplug pending detection
>   tests/libqtest: update virtio-net failover test
>   test/libqtest: add some virtio-net failover migration cancelling tests
>   tests/libqtest: add a migration test with two couples of failover
>     devices
> 
>  hw/acpi/pcihp.c                   |   30 +-
>  include/hw/pci/pci_bridge.h       |    8 +
>  tests/qtest/libqos/pci.c          |  118 +++
>  tests/qtest/libqos/pci.h          |    1 +
>  tests/qtest/meson.build           |    3 +
>  tests/qtest/virtio-net-failover.c | 1294 +++++++++++++++++++++++++++++
>  6 files changed, 1451 insertions(+), 3 deletions(-)
>  create mode 100644 tests/qtest/virtio-net-failover.c
> 
> -- 
> 2.33.1
> 
> 
> 
> 

Re: [PATCH v6 5/6] test/libqtest: add some virtio-net failover migration cancelling tests

[Thomas Huth]

On 08/12/2021 08.44, Michael S. Tsirkin wrote:
> On Mon, Dec 06, 2021 at 11:20:39PM +0100, Laurent Vivier wrote:
>> Add some tests to check the state of the machine if the migration
>> is cancelled while we are using virtio-net failover.
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> 
> So this one I think is needed for the release. Thomas, are you
> merging it there or should I?

rc4 has already been tagged yesterday. I don't think that Richard will still 
allow another PR at this point in time unless it fixes a really really 
critical problem. Laurent's series only adds a new qtest, so this certainly 
does not qualify, AFAIK.

  Thomas

Re: [PATCH v6 5/6] test/libqtest: add some virtio-net failover migration cancelling tests

[Thomas Huth]

On 08/12/2021 08.48, Thomas Huth wrote:
> On 08/12/2021 08.44, Michael S. Tsirkin wrote:
>> On Mon, Dec 06, 2021 at 11:20:39PM +0100, Laurent Vivier wrote:
>>> Add some tests to check the state of the machine if the migration
>>> is cancelled while we are using virtio-net failover.
>>>
>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>
>> So this one I think is needed for the release. Thomas, are you
>> merging it there or should I?
> 
> rc4 has already been tagged yesterday. I don't think that Richard will still 
> allow another PR at this point in time unless it fixes a really really 
> critical problem. Laurent's series only adds a new qtest, so this certainly 
> does not qualify, AFAIK.

Never mind, I had patch series v7 before my eyes when I hit the reply button 
here. The patch that touches the code outside of the test folder (3/6) has 
already been merged, so we should be fine for the release.

  Thomas