| Message ID | 20170323205025.12113-2-quintela@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 23.03.2017 21:50, Juan Quintela wrote: > Until we have reviewed what can/can't be hotplug during migration, > disable it. We can enable it later for the things that we know that > work. For instance, memory hotplug during postcopy don't work > currently. > > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > hw/core/qdev.c | 5 +++++ > qdev-monitor.c | 7 ++++++- > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index 1e7fb33..8c4a3f3 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -277,6 +277,11 @@ void qdev_unplug(DeviceState *dev, Error **errp) > HotplugHandler *hotplug_ctrl; > HotplugHandlerClass *hdc; > > + if (!migration_is_idle()) { > + error_setg(errp, "device_add not allowed while migrating"); > + return; > + } > + > if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) { > error_setg(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name); > return; > diff --git a/qdev-monitor.c b/qdev-monitor.c > index 5f2fcdf..e0622b4 100644 > --- a/qdev-monitor.c > +++ b/qdev-monitor.c > @@ -29,7 +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 > * from spreading further. Maybe better keep an empty line between the include statement and the comment? Thomas > @@ -566,6 +566,11 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) > BusState *bus = NULL; > Error *err = NULL; > > + if (!migration_is_idle()) { > + error_setg(errp, "device_add not allowed while migrating"); > + return NULL; > + } > + > driver = qemu_opt_get(opts, "driver"); > if (!driver) { > error_setg(errp, QERR_MISSING_PARAMETER, "driver"); >
Thomas Huth <thuth@redhat.com> wrote: > On 23.03.2017 21:50, Juan Quintela wrote: >> Until we have reviewed what can/can't be hotplug during migration, >> disable it. We can enable it later for the things that we know that >> work. For instance, memory hotplug during postcopy don't work >> currently. >> >> Signed-off-by: Juan Quintela <quintela@redhat.com> >> --- >> hw/core/qdev.c | 5 +++++ >> qdev-monitor.c | 7 ++++++- >> 2 files changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/hw/core/qdev.c b/hw/core/qdev.c >> index 1e7fb33..8c4a3f3 100644 >> --- a/hw/core/qdev.c >> +++ b/hw/core/qdev.c >> @@ -277,6 +277,11 @@ void qdev_unplug(DeviceState *dev, Error **errp) >> HotplugHandler *hotplug_ctrl; >> HotplugHandlerClass *hdc; >> >> + if (!migration_is_idle()) { >> + error_setg(errp, "device_add not allowed while migrating"); >> + return; >> + } >> + >> if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) { >> error_setg(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name); >> return; >> diff --git a/qdev-monitor.c b/qdev-monitor.c >> index 5f2fcdf..e0622b4 100644 >> --- a/qdev-monitor.c >> +++ b/qdev-monitor.c >> @@ -29,7 +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 >> * from spreading further. > > Maybe better keep an empty line between the include statement and the > comment? Oops, sure. > > Thomas > > >> @@ -566,6 +566,11 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) >> BusState *bus = NULL; >> Error *err = NULL; >> >> + if (!migration_is_idle()) { >> + error_setg(errp, "device_add not allowed while migrating"); >> + return NULL; >> + } >> + >> driver = qemu_opt_get(opts, "driver"); >> if (!driver) { >> error_setg(errp, QERR_MISSING_PARAMETER, "driver"); >>
On Thu, 23 Mar 2017 21:50:24 +0100 Juan Quintela <quintela@redhat.com> wrote: > Until we have reviewed what can/can't be hotplug during migration, > disable it. We can enable it later for the things that we know that > work. For instance, memory hotplug during postcopy don't work > currently. > > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > hw/core/qdev.c | 5 +++++ > qdev-monitor.c | 7 ++++++- > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index 1e7fb33..8c4a3f3 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -277,6 +277,11 @@ void qdev_unplug(DeviceState *dev, Error **errp) > HotplugHandler *hotplug_ctrl; > HotplugHandlerClass *hdc; > > + if (!migration_is_idle()) { > + error_setg(errp, "device_add not allowed while migrating"); s/device_add/device_del/ I'd put this check after "if (!dc->hotpluggable)" block, so error message won't hide wrong upluggable device error cases > + return; > + } > + > if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) { > error_setg(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name); > return; > diff --git a/qdev-monitor.c b/qdev-monitor.c > index 5f2fcdf..e0622b4 100644 > --- a/qdev-monitor.c > +++ b/qdev-monitor.c > @@ -29,7 +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 > * from spreading further. > @@ -566,6 +566,11 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) > BusState *bus = NULL; > Error *err = NULL; > > + if (!migration_is_idle()) { > + error_setg(errp, "device_add not allowed while migrating"); > + return NULL; > + } the same here, do it right before "/* create device */" > driver = qemu_opt_get(opts, "driver"); > if (!driver) { > error_setg(errp, QERR_MISSING_PARAMETER, "driver");
Igor Mammedov <imammedo@redhat.com> wrote: > On Thu, 23 Mar 2017 21:50:24 +0100 > Juan Quintela <quintela@redhat.com> wrote: > >> Until we have reviewed what can/can't be hotplug during migration, >> disable it. We can enable it later for the things that we know that >> work. For instance, memory hotplug during postcopy don't work >> currently. >> >> Signed-off-by: Juan Quintela <quintela@redhat.com> >> --- >> hw/core/qdev.c | 5 +++++ >> qdev-monitor.c | 7 ++++++- >> 2 files changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/hw/core/qdev.c b/hw/core/qdev.c >> index 1e7fb33..8c4a3f3 100644 >> --- a/hw/core/qdev.c >> +++ b/hw/core/qdev.c >> @@ -277,6 +277,11 @@ void qdev_unplug(DeviceState *dev, Error **errp) >> HotplugHandler *hotplug_ctrl; >> HotplugHandlerClass *hdc; >> >> + if (!migration_is_idle()) { >> + error_setg(errp, "device_add not allowed while migrating"); > s/device_add/device_del/ > > > I'd put this check after "if (!dc->hotpluggable)" block, so error > message won't hide wrong upluggable device error cases Thanks. I did both changes. Not sure yet what *should* came 1st: - device is not plugable/unplugable - you can't do it while on migration But I don't care, and as you seemed to preffer that other way, changed. Later, Juan. >> + return; >> + } >> + >> if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) { >> error_setg(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name); >> return; >> diff --git a/qdev-monitor.c b/qdev-monitor.c >> index 5f2fcdf..e0622b4 100644 >> --- a/qdev-monitor.c >> +++ b/qdev-monitor.c >> @@ -29,7 +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 >> * from spreading further. >> @@ -566,6 +566,11 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) >> BusState *bus = NULL; >> Error *err = NULL; >> >> + if (!migration_is_idle()) { >> + error_setg(errp, "device_add not allowed while migrating"); >> + return NULL; >> + } > the same here, do it right before "/* create device */" > >> driver = qemu_opt_get(opts, "driver"); >> if (!driver) { >> error_setg(errp, QERR_MISSING_PARAMETER, "driver");
diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 1e7fb33..8c4a3f3 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -277,6 +277,11 @@ void qdev_unplug(DeviceState *dev, Error **errp) HotplugHandler *hotplug_ctrl; HotplugHandlerClass *hdc; + if (!migration_is_idle()) { + error_setg(errp, "device_add not allowed while migrating"); + return; + } + if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) { error_setg(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name); return; diff --git a/qdev-monitor.c b/qdev-monitor.c index 5f2fcdf..e0622b4 100644 --- a/qdev-monitor.c +++ b/qdev-monitor.c @@ -29,7 +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 * from spreading further. @@ -566,6 +566,11 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) BusState *bus = NULL; Error *err = NULL; + if (!migration_is_idle()) { + error_setg(errp, "device_add not allowed while migrating"); + return NULL; + } + driver = qemu_opt_get(opts, "driver"); if (!driver) { error_setg(errp, QERR_MISSING_PARAMETER, "driver");
Until we have reviewed what can/can't be hotplug during migration, disable it. We can enable it later for the things that we know that work. For instance, memory hotplug during postcopy don't work currently. Signed-off-by: Juan Quintela <quintela@redhat.com> --- hw/core/qdev.c | 5 +++++ qdev-monitor.c | 7 ++++++- 2 files changed, 11 insertions(+), 1 deletion(-)