From: Laurent Vivier <lvivier@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>, qemu-devel@nongnu.org
Cc: damien.hedde@greensocs.com, pkrempa@redhat.com,
berrange@redhat.com, ehabkost@redhat.com, qemu-block@nongnu.org,
mst@redhat.com, libvir-list@redhat.com, jasowang@redhat.com,
quintela@redhat.com, armbru@redhat.com, vsementsov@virtuozzo.com,
its@irrelevant.dk, pbonzini@redhat.com, eblake@redhat.com
Subject: Re: [PATCH v2 12/15] virtio-net: Store failover primary opts pointer locally
Date: Tue, 19 Oct 2021 10:06:35 +0200 [thread overview]
Message-ID: <3be47c0d-817b-b5e6-fa7f-234a417ec2c5@redhat.com> (raw)
In-Reply-To: <20211008133442.141332-13-kwolf@redhat.com>
On 08/10/2021 15:34, Kevin Wolf wrote:
> Instead of accessing the global QemuOptsList, which really belong to the
> command line parser and shouldn't be accessed from devices, store a
> pointer to the QemuOpts in a new VirtIONet field.
>
> This is not the final state, but just an intermediate step to get rid of
> QemuOpts in devices. It will later be replaced with an options QDict.
>
> Before this patch, two "primary" devices could be hidden for the same
> standby device, but only one of them would actually be enabled and the
> other one would be kept hidden forever, so this doesn't make sense.
> After this patch, configuring a second primary device is an error.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> include/hw/virtio/virtio-net.h | 1 +
> hw/net/virtio-net.c | 26 ++++++++++++++++++--------
> 2 files changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
> index 824a69c23f..d118c95f69 100644
> --- a/include/hw/virtio/virtio-net.h
> +++ b/include/hw/virtio/virtio-net.h
> @@ -209,6 +209,7 @@ struct VirtIONet {
> bool failover_primary_hidden;
> bool failover;
> DeviceListener primary_listener;
> + QemuOpts *primary_opts;
> Notifier migration_state;
> VirtioNetRssData rss_data;
> struct NetRxPkt *rx_pkt;
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index a17d5739fc..ed9a9012e9 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -858,27 +858,24 @@ static DeviceState *failover_find_primary_device(VirtIONet *n)
> static void failover_add_primary(VirtIONet *n, Error **errp)
> {
> Error *err = NULL;
> - QemuOpts *opts;
> - char *id;
> DeviceState *dev = failover_find_primary_device(n);
>
> if (dev) {
> return;
> }
>
> - id = failover_find_primary_device_id(n);
> - if (!id) {
> + if (!n->primary_opts) {
> error_setg(errp, "Primary device not found");
> error_append_hint(errp, "Virtio-net failover will not work. Make "
> "sure primary device has parameter"
> " failover_pair_id=%s\n", n->netclient_name);
> return;
> }
> - opts = qemu_opts_find(qemu_find_opts("device"), id);
> - g_assert(opts); /* cannot be NULL because id was found using opts list */
> - dev = qdev_device_add(opts, &err);
> +
> + dev = qdev_device_add(n->primary_opts, &err);
> if (err) {
> - qemu_opts_del(opts);
> + qemu_opts_del(n->primary_opts);
> + n->primary_opts = NULL;
> } else {
> object_unref(OBJECT(dev));
> }
> @@ -3317,6 +3314,19 @@ static bool failover_hide_primary_device(DeviceListener *listener,
> return false;
> }
>
> + if (n->primary_opts) {
> + error_setg(errp, "Cannot attach more than one primary device to '%s'",
> + n->netclient_name);
> + return false;
> + }
> +
This part has introduced a regression, I've sent a patch to fix that.
https://patchew.org/QEMU/20211019071532.682717-1-lvivier@redhat.com/
Thanks,
Laurent
next prev parent reply other threads:[~2021-10-19 8:19 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-08 13:34 [PATCH v2 00/15] qdev: Add JSON -device Kevin Wolf
2021-10-08 13:34 ` [PATCH v2 01/15] net: Introduce NetClientInfo.check_peer_type() Kevin Wolf
2021-10-13 13:28 ` Damien Hedde
2021-10-14 3:22 ` Jason Wang
2021-10-08 13:34 ` [PATCH v2 02/15] net/vhost-user: Fix device compatibility check Kevin Wolf
2021-10-13 13:30 ` Damien Hedde
2021-10-14 3:23 ` Jason Wang
2021-10-08 13:34 ` [PATCH v2 03/15] net/vhost-vdpa: " Kevin Wolf
2021-10-13 13:30 ` Damien Hedde
2021-10-14 3:24 ` Jason Wang
2021-10-08 13:34 ` [PATCH v2 04/15] qom: Reduce use of error_propagate() Kevin Wolf
2021-10-11 15:07 ` Markus Armbruster
2021-10-08 13:34 ` [PATCH v2 05/15] iotests/245: Fix type for iothread property Kevin Wolf
2021-10-08 13:34 ` [PATCH v2 06/15] iotests/051: Fix typo Kevin Wolf
2021-10-08 13:34 ` [PATCH v2 07/15] qdev: Avoid using string visitor for properties Kevin Wolf
2021-10-08 13:34 ` [PATCH v2 08/15] qdev: Make DeviceState.id independent of QemuOpts Kevin Wolf
2021-10-13 13:41 ` Damien Hedde
2021-10-08 13:34 ` [PATCH v2 09/15] softmmu/qdev-monitor: add error handling in qdev_set_id Kevin Wolf
2021-10-11 21:00 ` Eric Blake
2021-10-13 13:10 ` Damien Hedde
2021-10-13 21:37 ` Eric Blake
2021-10-15 7:24 ` Kevin Wolf
2021-10-08 13:34 ` [PATCH v2 10/15] qemu-option: Allow deleting opts during qemu_opts_foreach() Kevin Wolf
2021-10-08 13:34 ` [PATCH v2 11/15] qdev: Add Error parameter to hide_device() callbacks Kevin Wolf
2021-10-08 13:34 ` [PATCH v2 12/15] virtio-net: Store failover primary opts pointer locally Kevin Wolf
2021-10-19 8:06 ` Laurent Vivier [this message]
2021-10-19 10:57 ` Kevin Wolf
2021-10-08 13:34 ` [PATCH v2 13/15] virtio-net: Avoid QemuOpts in failover_find_primary_device() Kevin Wolf
2021-10-08 13:34 ` [PATCH v2 14/15] qdev: Base object creation on QDict rather than QemuOpts Kevin Wolf
2021-10-08 15:17 ` Laurent Vivier
2021-10-08 13:34 ` [PATCH v2 15/15] vl: Enable JSON syntax for -device Kevin Wolf
2021-10-13 15:30 ` [PATCH v2 00/15] qdev: Add JSON -device Michael S. Tsirkin
2021-10-13 16:53 ` Kevin Wolf
2021-10-15 13:24 ` Peter Krempa
2021-10-15 14:38 ` Kevin Wolf
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=3be47c0d-817b-b5e6-fa7f-234a417ec2c5@redhat.com \
--to=lvivier@redhat.com \
--cc=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=damien.hedde@greensocs.com \
--cc=eblake@redhat.com \
--cc=ehabkost@redhat.com \
--cc=its@irrelevant.dk \
--cc=jasowang@redhat.com \
--cc=kwolf@redhat.com \
--cc=libvir-list@redhat.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=pkrempa@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=quintela@redhat.com \
--cc=vsementsov@virtuozzo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).