qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: qemu-devel@nongnu.org
Cc: Laurent Vivier <lvivier@redhat.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	Jason Wang <jasowang@redhat.com>,
	kwolf@redhat.com
Subject: [PULL 29/29] failover: fix a regression introduced by JSON'ification of -device
Date: Tue, 19 Oct 2021 07:21:03 -0400	[thread overview]
Message-ID: <20211019111923.679826-30-mst@redhat.com> (raw)
In-Reply-To: <20211019111923.679826-1-mst@redhat.com>

From: Laurent Vivier <lvivier@redhat.com>

The hide_device helper can be called several times for the same
devices as it shouldn't change any state and should only return an
information.

But not to rely anymore on QemuOpts we have introduced a new field
to store the parameters of the device and don't allow to update it
once it is done.

And as the function is called several times, we ends with:

  warning: Cannot attach more than one primary device to 'virtio0'

That is not only a warning as it prevents to hide the device and breaks
failover.

Fix that by checking the device id.

Now, we fail only if the virtio-net device is really used by two different
devices, for instance:

   -device virtio-net-pci,id=virtio0,failover=on,... \
   -device vfio-pci,id=hostdev0,failover_pair_id=virtio0,... \
   -device e1000e,id=e1000e0,failover_pair_id=virtio0,... \

will exit with:

  Cannot attach more than one primary device to 'virtio0': 'hostdev0' and 'e1000e0'

Fixes: 259a10dbcb4f ("virtio-net: Store failover primary opts pointer locally")
Cc: kwolf@redhat.com
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Message-Id: <20211019071532.682717-2-lvivier@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/net/virtio-net.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 09e173a558..83642c85b2 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3304,15 +3304,27 @@ static bool failover_hide_primary_device(DeviceListener *listener,
         return false;
     }
 
+    /*
+     * The hide helper can be called several times for a given device.
+     * Check there is only one primary for a virtio-net device but
+     * don't duplicate the qdict several times if it's called for the same
+     * device.
+     */
     if (n->primary_opts) {
-        error_setg(errp, "Cannot attach more than one primary device to '%s'",
-                   n->netclient_name);
-        return false;
+        const char *old, *new;
+        /* devices with failover_pair_id always have an id */
+        old = qdict_get_str(n->primary_opts, "id");
+        new = qdict_get_str(device_opts, "id");
+        if (strcmp(old, new) != 0) {
+            error_setg(errp, "Cannot attach more than one primary device to "
+                       "'%s': '%s' and '%s'", n->netclient_name, old, new);
+            return false;
+        }
+    } else {
+        n->primary_opts = qdict_clone_shallow(device_opts);
+        n->primary_opts_from_json = from_json;
     }
 
-    n->primary_opts = qdict_clone_shallow(device_opts);
-    n->primary_opts_from_json = from_json;
-
     /* failover_primary_hidden is set during feature negotiation */
     return qatomic_read(&n->failover_primary_hidden);
 }
-- 
MST



  parent reply	other threads:[~2021-10-19 11:50 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20211019140622.738008-1-mst@redhat.com>
2021-10-19 11:19 ` [PULL 00/29] pc,pci,virtio: features, fixes Michael S. Tsirkin
2021-10-19 11:19   ` [PULL 01/29] tests: acpi: dump table with failed checksum Michael S. Tsirkin
2021-10-19 11:19   ` [PULL 02/29] tests: qtest: add qtest_has_accel() to check if tested binary supports accelerator Michael S. Tsirkin
2021-10-19 11:19   ` [PULL 03/29] tests: acpi: whitelist expected tables for acpi/q35/xapic testcase Michael S. Tsirkin
2021-10-19 11:19   ` [PULL 04/29] tests: acpi: q35: test for x2APIC entries in SRAT Michael S. Tsirkin
2021-10-19 11:19   ` [PULL 05/29] tests: acpi: update expected tables blobs Michael S. Tsirkin
2021-10-19 11:19   ` [PULL 06/29] tests: acpi: whitelist new expected table tests/data/acpi/q35/DMAR.dmar Michael S. Tsirkin
2021-10-19 11:19   ` [PULL 07/29] tests: acpi: add testcase for intel_iommu (DMAR table) Michael S. Tsirkin
2021-10-19 11:20   ` [PULL 08/29] tests: acpi: add expected blob for DMAR table Michael S. Tsirkin
2021-10-19 11:20   ` [PULL 09/29] tests: acpi: whitelist expected blobs for new acpi/q35/ivrs testcase Michael S. Tsirkin
2021-10-19 11:20   ` [PULL 10/29] tests: acpi: add testcase for amd-iommu (IVRS table) Michael S. Tsirkin
2021-10-19 11:20   ` [PULL 11/29] tests: acpi: update expected blobs Michael S. Tsirkin
2021-10-19 11:20   ` [PULL 12/29] tests: acpi: arm/virt: drop redundant test_acpi_one() in test_acpi_virt_tcg() Michael S. Tsirkin
2021-10-19 11:20   ` [PULL 13/29] tests: arm-cpu-features: use qtest_has_kvm() API Michael S. Tsirkin
2021-10-19 11:20   ` [PULL 14/29] tests: migration-test: use qtest_has_accel() API Michael S. Tsirkin
2021-10-19 11:20   ` [PULL 15/29] tests: bios-tables-test: use qtest_has_accel() API to register TCG only tests Michael S. Tsirkin
2021-10-19 11:20   ` [PULL 16/29] vdpa: Skip protected ram IOMMU mappings Michael S. Tsirkin
2021-10-19 11:20   ` [PULL 17/29] vdpa: Add vhost_vdpa_section_end Michael S. Tsirkin
2021-10-19 11:20   ` [PULL 18/29] vdpa: Check for iova range at mappings changes Michael S. Tsirkin
2021-10-19 11:20   ` [PULL 19/29] virtio-iommu: Remove the non transitional name Michael S. Tsirkin
2021-10-19 11:20   ` [PULL 20/29] virtio-iommu: Drop base_name and change generic_name Michael S. Tsirkin
2021-10-19 11:20   ` [PULL 21/29] libvhost-user: fix VHOST_USER_REM_MEM_REG skipping mmap_addr Michael S. Tsirkin
2021-10-19 11:20   ` [PULL 22/29] vhost-user-rng: Add vhost-user-rng implementation Michael S. Tsirkin
2021-10-19 11:20   ` [PULL 23/29] vhost-user-rng-pci: Add vhost-user-rng-pci implementation Michael S. Tsirkin
2021-10-19 11:20   ` [PULL 24/29] docs: Add documentation for vhost based RNG implementation Michael S. Tsirkin
2021-10-19 11:20   ` [PULL 25/29] vhost-user: fix duplicated notifier MR init Michael S. Tsirkin
2021-10-19 11:20   ` [PULL 26/29] tests/acpi/bios-tables-test: add and allow changes to a new q35 DSDT table blob Michael S. Tsirkin
2021-10-19 11:20   ` [PULL 27/29] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35 Michael S. Tsirkin
2021-10-19 11:21   ` [PULL 28/29] tests/acpi/bios-tables-test: update DSDT blob for multifunction bridge test Michael S. Tsirkin
2021-10-19 11:21   ` Michael S. Tsirkin [this message]
2021-10-19 14:07   ` [PULL 00/29] pc,pci,virtio: features, fixes Michael S. Tsirkin
2021-10-19 14:08   ` [PULL 30/29] vhost-user-blk-test: pass vhost-user socket fds to QSD Michael S. Tsirkin
2021-10-19 18:38   ` [PULL 00/29] pc,pci,virtio: features, fixes Richard Henderson
2021-10-20  8:39     ` Igor Mammedov
2021-10-20  9:39       ` Thomas Huth
2021-10-20  9:41         ` Michael S. Tsirkin
2021-10-20  9:46           ` Thomas Huth
2021-10-20 15:34           ` Marcel Apfelbaum

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=20211019111923.679826-30-mst@redhat.com \
    --to=mst@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=lvivier@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

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

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