| Message ID | 1481742422-15969-3-git-send-email-ashijeetacharya@gmail.com |
|---|---|
| State | New |
| Headers | show |
* Ashijeet Acharya (ashijeetacharya@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 <ashijeetacharya@gmail.com> > --- > 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 / dgilbert@redhat.com / Manchester, UK
On Thu, Dec 15, 2016 at 9:35 PM, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote: > * Ashijeet Acharya (ashijeetacharya@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 <ashijeetacharya@gmail.com> >> --- >> 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 / dgilbert@redhat.com / Manchester, UK
* Ashijeet Acharya (ashijeetacharya@gmail.com) wrote: > On Thu, Dec 15, 2016 at 9:35 PM, Dr. David Alan Gilbert > <dgilbert@redhat.com> wrote: > > * Ashijeet Acharya (ashijeetacharya@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 <ashijeetacharya@gmail.com> > >> --- > >> 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 / dgilbert@redhat.com / Manchester, UK -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Thu, Dec 15, 2016 at 9:49 PM, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote: > * Ashijeet Acharya (ashijeetacharya@gmail.com) wrote: >> On Thu, Dec 15, 2016 at 9:35 PM, Dr. David Alan Gilbert >> <dgilbert@redhat.com> wrote: >> > * Ashijeet Acharya (ashijeetacharya@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 <ashijeetacharya@gmail.com> >> >> --- >> >> 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 / dgilbert@redhat.com / Manchester, UK > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
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) {
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 <ashijeetacharya@gmail.com> --- hw/usb/bus.c | 15 +++++++++++++++ qdev-monitor.c | 9 +++++++++ 2 files changed, 24 insertions(+)