| Message ID | 1253907769-1067-17-git-send-email-kraxel@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
Gerd Hoffmann <kraxel@redhat.com> writes: > From: Gerd Hoffmann <kraxel@redhat.com> > Subject: [Qemu-devel] [PATCH 16/24] qdev hotplug: infrastructure and monitor > commands. > To: qemu-devel@nongnu.org > Cc: Gerd Hoffmann <kraxel@redhat.com> > Date: Fri, 25 Sep 2009 21:42:41 +0200 > > Adds device_add and device_del commands. device_add accepts accepts > the same syntax like the -device command line switch. device_del > expects a device id. So you should tag your devices with ids if you > want to remove them later on, like this: > > device_add pci-ohci,id=ohci > device_del ohci > > Unplugging via pci_del or usb_del works too. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > hw/qdev.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > hw/qdev.h | 12 +++++++- > qemu-monitor.hx | 16 +++++++++++ > vl.c | 2 + > 4 files changed, 107 insertions(+), 2 deletions(-) > > diff --git a/hw/qdev.c b/hw/qdev.c > index a25245a..00cb849 100644 > --- a/hw/qdev.c > +++ b/hw/qdev.c > @@ -31,6 +31,8 @@ > #include "monitor.h" > > /* This is a nasty hack to allow passing a NULL bus to qdev_create. */ > +static int qdev_hotplug = 0; > + I see the "nasty hack" part, but I don't see how qdev_create() accepts a null bus now: > static BusState *main_system_bus; > > static DeviceInfo *device_info_list; > @@ -102,6 +104,10 @@ DeviceState *qdev_create(BusState *bus, const char *name) > qdev_prop_set_defaults(dev, dev->parent_bus->info->props); > qdev_prop_set_compat(dev); > QLIST_INSERT_HEAD(&bus->children, dev, sibling); It still derefences bus. > + if (qdev_hotplug) { > + assert(bus->allow_hotplug); > + dev->hotplugged = 1; > + } > dev->state = DEV_STATE_CREATED; > return dev; > } > @@ -192,6 +198,11 @@ DeviceState *qdev_device_add(QemuOpts *opts) > path ? path : info->bus_info->name, info->name); > return NULL; > } > + if (qdev_hotplug && !bus->allow_hotplug) { > + qemu_error("Bus %s does not support hotplugging\n", > + bus->name); > + return NULL; > + } > > /* create device, set properties */ > qdev = qdev_create(bus, driver); [...] As far as I can see, all qdev_hotplug does is telling qdev_device_add() and qdev_create() that this is a hotplug. What about something like: DeviceState *qdev_device_add(QemuOpts *opts, int hotplug) { [...] if (hotplug && !bus->allow_hotplug) { qemu_error("Bus %s does not support hotplugging\n", bus->name); return NULL; } /* create device, set properties */ qdev = qdev_create(bus, driver); if (hotplug) { dev->hotplugged = 1; [...] }
>> /* This is a nasty hack to allow passing a NULL bus to qdev_create. */ >> +static int qdev_hotplug = 0; >> + > > I see the "nasty hack" part, but I don't see how qdev_create() accepts a > null bus now: > >> static BusState *main_system_bus; The (existing) "nasty hack" comment belongs to the main_system_bus variable and should stay there of course to avoid confusion ... >> /* create device, set properties */ >> qdev = qdev_create(bus, driver); > [...] > > As far as I can see, all qdev_hotplug does is telling qdev_device_add() > and qdev_create() that this is a hotplug. Yes. > What about something like: > > DeviceState *qdev_device_add(QemuOpts *opts, int hotplug) > { > [...] > if (hotplug&& !bus->allow_hotplug) { > qemu_error("Bus %s does not support hotplugging\n", > bus->name); > return NULL; > } > > /* create device, set properties */ > qdev = qdev_create(bus, driver); > if (hotplug) { > dev->hotplugged = 1; I started that way. Doesn't fly. Not every device creation goes through qdev_device_add(). Thus you'll have to do this in qdev_create(), which in turn means that you would have to add a hotplug parameter to tons of functions just to pass it down to qdev_create ... cheers, Gerd
Gerd Hoffmann <kraxel@redhat.com> writes: [...] >> >> As far as I can see, all qdev_hotplug does is telling qdev_device_add() >> and qdev_create() that this is a hotplug. > > Yes. > >> What about something like: >> >> DeviceState *qdev_device_add(QemuOpts *opts, int hotplug) >> { >> [...] >> if (hotplug&& !bus->allow_hotplug) { >> qemu_error("Bus %s does not support hotplugging\n", >> bus->name); >> return NULL; >> } >> >> /* create device, set properties */ >> qdev = qdev_create(bus, driver); >> if (hotplug) { >> dev->hotplugged = 1; > > I started that way. Doesn't fly. Not every device creation goes > through qdev_device_add(). Thus you'll have to do this in > qdev_create(), which in turn means that you would have to add a > hotplug parameter to tons of functions just to pass it down to > qdev_create ... Isn't it sufficient if every *hotplug* device creation goes through qdev_device_add()?
Hi, >> I started that way. Doesn't fly. Not every device creation goes >> through qdev_device_add(). Thus you'll have to do this in >> qdev_create(), which in turn means that you would have to add a >> hotplug parameter to tons of functions just to pass it down to >> qdev_create ... > > Isn't it sufficient if every *hotplug* device creation goes through > qdev_device_add()? That isn't the case too. cheers, Gerd
diff --git a/hw/qdev.c b/hw/qdev.c index a25245a..00cb849 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -31,6 +31,8 @@ #include "monitor.h" /* This is a nasty hack to allow passing a NULL bus to qdev_create. */ +static int qdev_hotplug = 0; + static BusState *main_system_bus; static DeviceInfo *device_info_list; @@ -102,6 +104,10 @@ DeviceState *qdev_create(BusState *bus, const char *name) qdev_prop_set_defaults(dev, dev->parent_bus->info->props); qdev_prop_set_compat(dev); QLIST_INSERT_HEAD(&bus->children, dev, sibling); + if (qdev_hotplug) { + assert(bus->allow_hotplug); + dev->hotplugged = 1; + } dev->state = DEV_STATE_CREATED; return dev; } @@ -192,6 +198,11 @@ DeviceState *qdev_device_add(QemuOpts *opts) path ? path : info->bus_info->name, info->name); return NULL; } + if (qdev_hotplug && !bus->allow_hotplug) { + qemu_error("Bus %s does not support hotplugging\n", + bus->name); + return NULL; + } /* create device, set properties */ qdev = qdev_create(bus, driver); @@ -229,6 +240,24 @@ int qdev_init(DeviceState *dev) return 0; } +int qdev_unplug(DeviceState *dev) +{ + if (!dev->parent_bus->allow_hotplug) { + qemu_error("Bus %s does not support hotplugging\n", + dev->parent_bus->name); + return -1; + } + return dev->info->unplug(dev); +} + +/* can be used as ->unplug() callback for the simple cases */ +int qdev_simple_unplug_cb(DeviceState *dev) +{ + /* just zap it */ + qdev_free(dev); + return 0; +} + /* Unlink device from bus and free the structure. */ void qdev_free(DeviceState *dev) { @@ -252,6 +281,15 @@ void qdev_free(DeviceState *dev) qemu_free(dev); } +void qdev_machine_creation_done(void) +{ + /* + * ok, initial machine setup is done, starting from now we can + * only create hotpluggable devices + */ + qdev_hotplug = 1; +} + /* Get a character (serial) device interface. */ CharDriverState *qdev_init_chardev(DeviceState *dev) { @@ -370,6 +408,24 @@ static BusState *qbus_find_recursive(BusState *bus, const char *name, return NULL; } +static DeviceState *qdev_find_recursive(BusState *bus, const char *id) +{ + DeviceState *dev, *ret; + BusState *child; + + QLIST_FOREACH(dev, &bus->children, sibling) { + if (dev->id && strcmp(dev->id, id) == 0) + return dev; + QLIST_FOREACH(child, &dev->child_bus, sibling) { + ret = qdev_find_recursive(child, id); + if (ret) { + return ret; + } + } + } + return NULL; +} + static void qbus_list_bus(DeviceState *dev, char *dest, int len) { BusState *child; @@ -647,3 +703,26 @@ void do_info_qdm(Monitor *mon) monitor_printf(mon, "%s\n", msg); } } + +void do_device_add(Monitor *mon, const QDict *qdict) +{ + QemuOpts *opts; + + opts = qemu_opts_parse(&qemu_device_opts, + qdict_get_str(qdict, "config"), "driver"); + if (opts) + qdev_device_add(opts); +} + +void do_device_del(Monitor *mon, const QDict *qdict) +{ + const char *id = qdict_get_str(qdict, "id"); + DeviceState *dev; + + dev = qdev_find_recursive(main_system_bus, id); + if (NULL == dev) { + qemu_error("Device '%s' not found\n", id); + return; + } + qdev_unplug(dev); +} diff --git a/hw/qdev.h b/hw/qdev.h index 0db2d32..8b6e4fe 100644 --- a/hw/qdev.h +++ b/hw/qdev.h @@ -29,6 +29,7 @@ enum DevState { struct DeviceState { const char *id; enum DevState state; + int hotplugged; DeviceInfo *info; BusState *parent_bus; int num_gpio_out; @@ -53,6 +54,7 @@ struct BusState { DeviceState *parent; BusInfo *info; const char *name; + int allow_hotplug; int qdev_allocated; QLIST_HEAD(, DeviceState) children; QLIST_ENTRY(BusState) sibling; @@ -97,7 +99,10 @@ struct CompatProperty { DeviceState *qdev_create(BusState *bus, const char *name); DeviceState *qdev_device_add(QemuOpts *opts); int qdev_init(DeviceState *dev); +int qdev_unplug(DeviceState *dev); void qdev_free(DeviceState *dev); +int qdev_simple_unplug_cb(DeviceState *dev); +void qdev_machine_creation_done(void); qemu_irq qdev_get_gpio_in(DeviceState *dev, int n); void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin); @@ -107,7 +112,7 @@ BusState *qdev_get_child_bus(DeviceState *dev, const char *name); /*** Device API. ***/ typedef int (*qdev_initfn)(DeviceState *dev, DeviceInfo *info); -typedef int (*qdev_exitfn)(DeviceState *dev); +typedef int (*qdev_event)(DeviceState *dev); struct DeviceInfo { const char *name; @@ -125,7 +130,8 @@ struct DeviceInfo { /* Private to qdev / bus. */ qdev_initfn init; - qdev_exitfn exit; + qdev_event unplug; + qdev_event exit; BusInfo *bus_info; struct DeviceInfo *next; }; @@ -164,6 +170,8 @@ void qbus_free(BusState *bus); void do_info_qtree(Monitor *mon); void do_info_qdm(Monitor *mon); +void do_device_add(Monitor *mon, const QDict *qdict); +void do_device_del(Monitor *mon, const QDict *qdict); /*** qdev-properties.c ***/ diff --git a/qemu-monitor.hx b/qemu-monitor.hx index 9f91873..e4afe62 100644 --- a/qemu-monitor.hx +++ b/qemu-monitor.hx @@ -369,6 +369,22 @@ hub. @var{devname} has the syntax @code{bus.addr}. Use the monitor command @code{info usb} to see the devices you can remove. ETEXI + { "device_add", "config:s", do_device_add, + "device", "add device, like -device on the command line" }, +STEXI +@item device_add @var{config} + +Add device. +ETEXI + + { "device_del", "id:s", do_device_del, + "device", "remove device" }, +STEXI +@item device_del @var{id} + +Remove device @var{id}. +ETEXI + { "cpu", "index:i", do_cpu_set, "index", "set the default CPU" }, STEXI diff --git a/vl.c b/vl.c index eb01da7..7df9328 100644 --- a/vl.c +++ b/vl.c @@ -5869,6 +5869,8 @@ int main(int argc, char **argv, char **envp) exit(1); } + qdev_machine_creation_done(); + if (loadvm) { if (load_vmstate(cur_mon, loadvm) < 0) { autostart = 0;
Adds device_add and device_del commands. device_add accepts accepts the same syntax like the -device command line switch. device_del expects a device id. So you should tag your devices with ids if you want to remove them later on, like this: device_add pci-ohci,id=ohci device_del ohci Unplugging via pci_del or usb_del works too. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- hw/qdev.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ hw/qdev.h | 12 +++++++- qemu-monitor.hx | 16 +++++++++++ vl.c | 2 + 4 files changed, 107 insertions(+), 2 deletions(-)