[Qemu-devel] [PATCH 2/3] migration: Allow "device add" options to only add migratable devices
Introduce checks for the unmigratable flag in the VMStateDescription structs of respective devices when user attempts to add them. If the "--only-migratable" was specified, all unmigratable devices will rightly fail to add. This feature is made compatible for both "-device" and "-usbdevice" command line options and covers their hmp and qmp counterparts as well. Signed-off-by: Ashijeet Acharya --- hw/usb/bus.c | 15 +++ qdev-monitor.c | 9 + 2 files changed, 24 insertions(+) diff --git a/hw/usb/bus.c b/hw/usb/bus.c index 25913ad..8796788 100644 --- a/hw/usb/bus.c +++ b/hw/usb/bus.c @@ -8,6 +8,7 @@ #include "monitor/monitor.h" #include "trace.h" #include "qemu/cutils.h" +#include "migration/migration.h" static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent); @@ -686,6 +687,8 @@ USBDevice *usbdevice_create(const char *cmdline) const char *params; int len; USBDevice *dev; +ObjectClass *klass; +DeviceClass *dc; params = strchr(cmdline,':'); if (params) { @@ -720,6 +723,18 @@ USBDevice *usbdevice_create(const char *cmdline) return NULL; } +klass = object_class_by_name(f->name); + +dc = DEVICE_CLASS(klass); + +if (only_migratable) { +if (dc->vmsd->unmigratable) { +error_report("Device %s is not migratable, but --only-migratable " + "was specified", f->name); +return NULL; +} +} + if (f->usbdevice_init) { dev = f->usbdevice_init(bus, params); } else { diff --git a/qdev-monitor.c b/qdev-monitor.c index c73410c..81d01df 100644 --- a/qdev-monitor.c +++ b/qdev-monitor.c @@ -29,6 +29,7 @@ #include "qemu/error-report.h" #include "qemu/help_option.h" #include "sysemu/block-backend.h" +#include "migration/migration.h" /* * Aliases were a bad idea from the start. Let's keep them @@ -577,6 +578,14 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) return NULL; } +if (only_migratable) { +if (dc->vmsd->unmigratable) { +error_setg(errp, "Device %s is not migratable, but " + "--only-migratable was specified", driver); +return NULL; +} +} + /* find bus */ path = qemu_opt_get(opts, "bus"); if (path != NULL) { -- 2.6.2
Re: [Qemu-devel] [PATCH 2/3] migration: Allow "device add" options to only add migratable devices
* Ashijeet Acharya (ashijeetacha...@gmail.com) wrote: > Introduce checks for the unmigratable flag in the VMStateDescription > structs of respective devices when user attempts to add them. If the > "--only-migratable" was specified, all unmigratable devices will > rightly fail to add. This feature is made compatible for both "-device" > and "-usbdevice" command line options and covers their hmp and qmp > counterparts as well. > > Signed-off-by: Ashijeet Acharya > --- > hw/usb/bus.c | 15 +++ > qdev-monitor.c | 9 + > 2 files changed, 24 insertions(+) > > diff --git a/hw/usb/bus.c b/hw/usb/bus.c > index 25913ad..8796788 100644 > --- a/hw/usb/bus.c > +++ b/hw/usb/bus.c > @@ -8,6 +8,7 @@ > #include "monitor/monitor.h" > #include "trace.h" > #include "qemu/cutils.h" > +#include "migration/migration.h" > > static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent); > > @@ -686,6 +687,8 @@ USBDevice *usbdevice_create(const char *cmdline) > const char *params; > int len; > USBDevice *dev; > +ObjectClass *klass; > +DeviceClass *dc; > > params = strchr(cmdline,':'); > if (params) { > @@ -720,6 +723,18 @@ USBDevice *usbdevice_create(const char *cmdline) > return NULL; > } > > +klass = object_class_by_name(f->name); > +dc = DEVICE_CLASS(klass); Would qdev_get_device_class work here instead of that pair? (I was thinking you needed to check klass to make sure it wasn't null) > +if (only_migratable) { > +if (dc->vmsd->unmigratable) { > +error_report("Device %s is not migratable, but --only-migratable > " > + "was specified", f->name); > +return NULL; > +} > +} > + > if (f->usbdevice_init) { > dev = f->usbdevice_init(bus, params); > } else { > diff --git a/qdev-monitor.c b/qdev-monitor.c > index c73410c..81d01df 100644 > --- a/qdev-monitor.c > +++ b/qdev-monitor.c > @@ -29,6 +29,7 @@ > #include "qemu/error-report.h" > #include "qemu/help_option.h" > #include "sysemu/block-backend.h" > +#include "migration/migration.h" > > /* > * Aliases were a bad idea from the start. Let's keep them > @@ -577,6 +578,14 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error > **errp) > return NULL; > } > > +if (only_migratable) { > +if (dc->vmsd->unmigratable) { > +error_setg(errp, "Device %s is not migratable, but " > + "--only-migratable was specified", driver); > +return NULL; > +} > +} > + > /* find bus */ > path = qemu_opt_get(opts, "bus"); > if (path != NULL) { > -- > 2.6.2 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 2/3] migration: Allow "device add" options to only add migratable devices
* Ashijeet Acharya (ashijeetacha...@gmail.com) wrote: > On Thu, Dec 15, 2016 at 9:35 PM, Dr. David Alan Gilbert > wrote: > > * Ashijeet Acharya (ashijeetacha...@gmail.com) wrote: > >> Introduce checks for the unmigratable flag in the VMStateDescription > >> structs of respective devices when user attempts to add them. If the > >> "--only-migratable" was specified, all unmigratable devices will > >> rightly fail to add. This feature is made compatible for both "-device" > >> and "-usbdevice" command line options and covers their hmp and qmp > >> counterparts as well. > >> > >> Signed-off-by: Ashijeet Acharya > >> --- > >> hw/usb/bus.c | 15 +++ > >> qdev-monitor.c | 9 + > >> 2 files changed, 24 insertions(+) > >> > >> diff --git a/hw/usb/bus.c b/hw/usb/bus.c > >> index 25913ad..8796788 100644 > >> --- a/hw/usb/bus.c > >> +++ b/hw/usb/bus.c > >> @@ -8,6 +8,7 @@ > >> #include "monitor/monitor.h" > >> #include "trace.h" > >> #include "qemu/cutils.h" > >> +#include "migration/migration.h" > >> > >> static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int > >> indent); > >> > >> @@ -686,6 +687,8 @@ USBDevice *usbdevice_create(const char *cmdline) > >> const char *params; > >> int len; > >> USBDevice *dev; > >> +ObjectClass *klass; > >> +DeviceClass *dc; > >> > >> params = strchr(cmdline,':'); > >> if (params) { > >> @@ -720,6 +723,18 @@ USBDevice *usbdevice_create(const char *cmdline) > >> return NULL; > >> } > >> > >> +klass = object_class_by_name(f->name); > > > >> +dc = DEVICE_CLASS(klass); > > > > Would qdev_get_device_class work here instead of that pair? > > No, because I believe that is defined as static in qdev_monitor.c > which was one of the reasons I faced a difficulty with this one. Hmm, I wonder if Markus can suggest a better way; qdev_get_device_class seems a lot more complex dealing iwth aliases and stuff, if we need that type of stuff. Dave > > (I was thinking you needed to check klass to make sure it wasn't null) > > Right. I will include the check in v2. > > Ashijeet > > > >> +if (only_migratable) { > >> +if (dc->vmsd->unmigratable) { > >> +error_report("Device %s is not migratable, but > >> --only-migratable " > >> + "was specified", f->name); > >> +return NULL; > >> +} > >> +} > >> + > >> if (f->usbdevice_init) { > >> dev = f->usbdevice_init(bus, params); > >> } else { > >> diff --git a/qdev-monitor.c b/qdev-monitor.c > >> index c73410c..81d01df 100644 > >> --- a/qdev-monitor.c > >> +++ b/qdev-monitor.c > >> @@ -29,6 +29,7 @@ > >> #include "qemu/error-report.h" > >> #include "qemu/help_option.h" > >> #include "sysemu/block-backend.h" > >> +#include "migration/migration.h" > >> > >> /* > >> * Aliases were a bad idea from the start. Let's keep them > >> @@ -577,6 +578,14 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error > >> **errp) > >> return NULL; > >> } > >> > >> +if (only_migratable) { > >> +if (dc->vmsd->unmigratable) { > >> +error_setg(errp, "Device %s is not migratable, but " > >> + "--only-migratable was specified", driver); > >> +return NULL; > >> +} > >> +} > >> + > >> /* find bus */ > >> path = qemu_opt_get(opts, "bus"); > >> if (path != NULL) { > >> -- > >> 2.6.2 > >> > > -- > > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 2/3] migration: Allow "device add" options to only add migratable devices
On Thu, Dec 15, 2016 at 9:35 PM, Dr. David Alan Gilbert wrote: > * Ashijeet Acharya (ashijeetacha...@gmail.com) wrote: >> Introduce checks for the unmigratable flag in the VMStateDescription >> structs of respective devices when user attempts to add them. If the >> "--only-migratable" was specified, all unmigratable devices will >> rightly fail to add. This feature is made compatible for both "-device" >> and "-usbdevice" command line options and covers their hmp and qmp >> counterparts as well. >> >> Signed-off-by: Ashijeet Acharya >> --- >> hw/usb/bus.c | 15 +++ >> qdev-monitor.c | 9 + >> 2 files changed, 24 insertions(+) >> >> diff --git a/hw/usb/bus.c b/hw/usb/bus.c >> index 25913ad..8796788 100644 >> --- a/hw/usb/bus.c >> +++ b/hw/usb/bus.c >> @@ -8,6 +8,7 @@ >> #include "monitor/monitor.h" >> #include "trace.h" >> #include "qemu/cutils.h" >> +#include "migration/migration.h" >> >> static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent); >> >> @@ -686,6 +687,8 @@ USBDevice *usbdevice_create(const char *cmdline) >> const char *params; >> int len; >> USBDevice *dev; >> +ObjectClass *klass; >> +DeviceClass *dc; >> >> params = strchr(cmdline,':'); >> if (params) { >> @@ -720,6 +723,18 @@ USBDevice *usbdevice_create(const char *cmdline) >> return NULL; >> } >> >> +klass = object_class_by_name(f->name); > >> +dc = DEVICE_CLASS(klass); > > Would qdev_get_device_class work here instead of that pair? No, because I believe that is defined as static in qdev_monitor.c which was one of the reasons I faced a difficulty with this one. > (I was thinking you needed to check klass to make sure it wasn't null) Right. I will include the check in v2. Ashijeet > >> +if (only_migratable) { >> +if (dc->vmsd->unmigratable) { >> +error_report("Device %s is not migratable, but >> --only-migratable " >> + "was specified", f->name); >> +return NULL; >> +} >> +} >> + >> if (f->usbdevice_init) { >> dev = f->usbdevice_init(bus, params); >> } else { >> diff --git a/qdev-monitor.c b/qdev-monitor.c >> index c73410c..81d01df 100644 >> --- a/qdev-monitor.c >> +++ b/qdev-monitor.c >> @@ -29,6 +29,7 @@ >> #include "qemu/error-report.h" >> #include "qemu/help_option.h" >> #include "sysemu/block-backend.h" >> +#include "migration/migration.h" >> >> /* >> * Aliases were a bad idea from the start. Let's keep them >> @@ -577,6 +578,14 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error >> **errp) >> return NULL; >> } >> >> +if (only_migratable) { >> +if (dc->vmsd->unmigratable) { >> +error_setg(errp, "Device %s is not migratable, but " >> + "--only-migratable was specified", driver); >> +return NULL; >> +} >> +} >> + >> /* find bus */ >> path = qemu_opt_get(opts, "bus"); >> if (path != NULL) { >> -- >> 2.6.2 >> > -- > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 2/3] migration: Allow "device add" options to only add migratable devices
On Thu, Dec 15, 2016 at 9:49 PM, Dr. David Alan Gilbert wrote: > * Ashijeet Acharya (ashijeetacha...@gmail.com) wrote: >> On Thu, Dec 15, 2016 at 9:35 PM, Dr. David Alan Gilbert >> wrote: >> > * Ashijeet Acharya (ashijeetacha...@gmail.com) wrote: >> >> Introduce checks for the unmigratable flag in the VMStateDescription >> >> structs of respective devices when user attempts to add them. If the >> >> "--only-migratable" was specified, all unmigratable devices will >> >> rightly fail to add. This feature is made compatible for both "-device" >> >> and "-usbdevice" command line options and covers their hmp and qmp >> >> counterparts as well. >> >> >> >> Signed-off-by: Ashijeet Acharya >> >> --- >> >> hw/usb/bus.c | 15 +++ >> >> qdev-monitor.c | 9 + >> >> 2 files changed, 24 insertions(+) >> >> >> >> diff --git a/hw/usb/bus.c b/hw/usb/bus.c >> >> index 25913ad..8796788 100644 >> >> --- a/hw/usb/bus.c >> >> +++ b/hw/usb/bus.c >> >> @@ -8,6 +8,7 @@ >> >> #include "monitor/monitor.h" >> >> #include "trace.h" >> >> #include "qemu/cutils.h" >> >> +#include "migration/migration.h" >> >> >> >> static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int >> >> indent); >> >> >> >> @@ -686,6 +687,8 @@ USBDevice *usbdevice_create(const char *cmdline) >> >> const char *params; >> >> int len; >> >> USBDevice *dev; >> >> +ObjectClass *klass; >> >> +DeviceClass *dc; >> >> >> >> params = strchr(cmdline,':'); >> >> if (params) { >> >> @@ -720,6 +723,18 @@ USBDevice *usbdevice_create(const char *cmdline) >> >> return NULL; >> >> } >> >> >> >> +klass = object_class_by_name(f->name); >> > >> >> +dc = DEVICE_CLASS(klass); >> > >> > Would qdev_get_device_class work here instead of that pair? >> >> No, because I believe that is defined as static in qdev_monitor.c >> which was one of the reasons I faced a difficulty with this one. > > Hmm, I wonder if Markus can suggest a better way; qdev_get_device_class > seems a lot more complex dealing iwth aliases and stuff, if we need that > type of stuff. Okay. I have grepped around for other uses of the macro DEVICE_CLASS which is mostly during the initialization of the various devices and we are not dealing with aliases probably because the device name is rightly there. But similarly here, I am trusting the device name stored in "f->name" more so after following the function call flow to the lowest level and finding a similar call to object_class_by_name() to get the ObjectClass of the device which passes the same name as stored in f->name. Ashijeet > > Dave > >> > (I was thinking you needed to check klass to make sure it wasn't null) >> >> Right. I will include the check in v2. >> >> Ashijeet >> > >> >> +if (only_migratable) { >> >> +if (dc->vmsd->unmigratable) { >> >> +error_report("Device %s is not migratable, but >> >> --only-migratable " >> >> + "was specified", f->name); >> >> +return NULL; >> >> +} >> >> +} >> >> + >> >> if (f->usbdevice_init) { >> >> dev = f->usbdevice_init(bus, params); >> >> } else { >> >> diff --git a/qdev-monitor.c b/qdev-monitor.c >> >> index c73410c..81d01df 100644 >> >> --- a/qdev-monitor.c >> >> +++ b/qdev-monitor.c >> >> @@ -29,6 +29,7 @@ >> >> #include "qemu/error-report.h" >> >> #include "qemu/help_option.h" >> >> #include "sysemu/block-backend.h" >> >> +#include "migration/migration.h" >> >> >> >> /* >> >> * Aliases were a bad idea from the start. Let's keep them >> >> @@ -577,6 +578,14 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error >> >> **errp) >> >> return NULL; >> >> } >> >> >> >> +if (only_migratable) { >> >> +if (dc->vmsd->unmigratable) { >> >> +error_setg(errp, "Device %s is not migratable, but " >> >> + "--only-migratable was specified", driver); >> >> +return NULL; >> >> +} >> >> +} >> >> + >> >> /* find bus */ >> >> path = qemu_opt_get(opts, "bus"); >> >> if (path != NULL) { >> >> -- >> >> 2.6.2 >> >> >> > -- >> > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK > -- > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK