From 7d61808206cc651ae42024ab6def827afa2f807b Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 8 Oct 2021 15:34:38 +0200 Subject: [PATCH] qdev: Add Error parameter to hide_device() callbacks hide_device() is used for virtio-net failover, where the standby virtio device delays creation of the primary device. It only makes sense to have a single primary device for each standby device. Adding a second one should result in an error instead of hiding it and never using it afterwards. Prepare for this by adding an Error parameter to the hide_device() callback where virtio-net is informed about adding a primary device. Signed-off-by: Kevin Wolf Message-Id: <20211008133442.141332-12-kwolf@redhat.com> Reviewed-by: Michael S. Tsirkin Tested-by: Peter Krempa Signed-off-by: Kevin Wolf --- hw/core/qdev.c | 7 +++++-- hw/net/virtio-net.c | 2 +- include/hw/qdev-core.h | 8 ++++++-- softmmu/qdev-monitor.c | 5 ++++- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index d918b50a1d..c3a021c444 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -211,14 +211,17 @@ void device_listener_unregister(DeviceListener *listener) QTAILQ_REMOVE(&device_listeners, listener, link); } -bool qdev_should_hide_device(QemuOpts *opts) +bool qdev_should_hide_device(QemuOpts *opts, Error **errp) { + ERRP_GUARD(); DeviceListener *listener; QTAILQ_FOREACH(listener, &device_listeners, link) { if (listener->hide_device) { - if (listener->hide_device(listener, opts)) { + if (listener->hide_device(listener, opts, errp)) { return true; + } else if (*errp) { + return false; } } } diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index f205331dcf..a17d5739fc 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -3304,7 +3304,7 @@ static void virtio_net_migration_state_notifier(Notifier *notifier, void *data) } static bool failover_hide_primary_device(DeviceListener *listener, - QemuOpts *device_opts) + QemuOpts *device_opts, Error **errp) { VirtIONet *n = container_of(listener, VirtIONet, primary_listener); const char *standby_id; diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 5a073fc368..74d8b614a7 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -201,8 +201,12 @@ struct DeviceListener { * informs qdev if a device should be visible or hidden. We can * hide a failover device depending for example on the device * opts. + * + * On errors, it returns false and errp is set. Device creation + * should fail in this case. */ - bool (*hide_device)(DeviceListener *listener, QemuOpts *device_opts); + bool (*hide_device)(DeviceListener *listener, QemuOpts *device_opts, + Error **errp); QTAILQ_ENTRY(DeviceListener) link; }; @@ -837,7 +841,7 @@ void device_listener_unregister(DeviceListener *listener); * When a device is added via qdev_device_add() this will be called, * and return if the device should be added now or not. */ -bool qdev_should_hide_device(QemuOpts *opts); +bool qdev_should_hide_device(QemuOpts *opts, Error **errp); typedef enum MachineInitPhase { /* current_machine is NULL. */ diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c index 0b6833cc57..ea737db028 100644 --- a/softmmu/qdev-monitor.c +++ b/softmmu/qdev-monitor.c @@ -626,6 +626,7 @@ const char *qdev_set_id(DeviceState *dev, char *id, Error **errp) DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) { + ERRP_GUARD(); DeviceClass *dc; const char *driver, *path; DeviceState *dev = NULL; @@ -669,11 +670,13 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) error_setg(errp, "Device with failover_pair_id don't have id"); return NULL; } - if (qdev_should_hide_device(opts)) { + if (qdev_should_hide_device(opts, errp)) { if (bus && !qbus_is_hotpluggable(bus)) { error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name); } return NULL; + } else if (*errp) { + return NULL; } }