| Message ID | 20210820142002.152994-3-lvivier@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtio-net failover cleanup and new features | expand |
Laurent Vivier <lvivier@redhat.com> wrote: > Commit f3a850565693 ("qdev/qbus: add hidden device support") has > introduced a generic way to hide a device but it has modified > qdev_device_add() to check a specific option of the failover device, > "failover_pair_id", before calling the generic mechanism. > > It's not needed (and not generic) to do that in qdev_device_add() because > this is also checked by the failover_hide_primary_device() function that > uses the generic mechanism to hide the device. > > Cc: Jens Freimann <jfreimann@redhat.com> > Signed-off-by: Laurent Vivier <lvivier@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> I see why you want this change. It was done the other way to make sure that we only tried to hide the divec is there is a failover_pair_id property. You can't have both.
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 542f9e167eb4..0c5ec930356b 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -3288,6 +3288,13 @@ static bool failover_hide_primary_device(DeviceListener *listener, return false; } standby_id = qemu_opt_get(device_opts, "failover_pair_id"); + if (standby_id == NULL) { + return false; + } + if (device_opts->id == NULL) { + error_setg(errp, "Device with failover_pair_id don't have id"); + return true; + } if (g_strcmp0(standby_id, n->netclient_name) != 0) { return false; } diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c index 7adf0d22beb1..5c92dbed3139 100644 --- a/softmmu/qdev-monitor.c +++ b/softmmu/qdev-monitor.c @@ -622,17 +622,11 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) } } - if (qemu_opt_get(opts, "failover_pair_id")) { - if (!opts->id) { - error_setg(errp, "Device with failover_pair_id don't have id"); - return NULL; - } - if (qdev_should_hide_device(opts, errp)) { - if (errp && !*errp && bus && !qbus_is_hotpluggable(bus)) { - error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name); - } - return NULL; + if (qdev_should_hide_device(opts, errp)) { + if (errp && !*errp && bus && !qbus_is_hotpluggable(bus)) { + error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name); } + return NULL; } if (phase_check(PHASE_MACHINE_READY) && bus && !qbus_is_hotpluggable(bus)) {
Commit f3a850565693 ("qdev/qbus: add hidden device support") has introduced a generic way to hide a device but it has modified qdev_device_add() to check a specific option of the failover device, "failover_pair_id", before calling the generic mechanism. It's not needed (and not generic) to do that in qdev_device_add() because this is also checked by the failover_hide_primary_device() function that uses the generic mechanism to hide the device. Cc: Jens Freimann <jfreimann@redhat.com> Signed-off-by: Laurent Vivier <lvivier@redhat.com> --- hw/net/virtio-net.c | 7 +++++++ softmmu/qdev-monitor.c | 14 ++++---------- 2 files changed, 11 insertions(+), 10 deletions(-)