Am 19.10.2021 um 10:06 hat Laurent Vivier geschrieben:
> 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@xxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxx/
Thanks for catching this! The fix looks good to me.
Kevin
