* [Qemu-devel] [PATCH 1/5] qdev: qdev_hotplug is really a bool
2017-04-06 13:13 [Qemu-devel] [PATCH v2 0/5] Disable hotplug during migration Juan Quintela
@ 2017-04-06 13:13 ` Juan Quintela
2017-04-06 13:46 ` Eric Blake
2017-04-06 16:39 ` Philippe Mathieu-Daudé
2017-04-06 13:13 ` [Qemu-devel] [PATCH 2/5] qdev: Export qdev_hot_removed Juan Quintela
` (4 subsequent siblings)
5 siblings, 2 replies; 16+ messages in thread
From: Juan Quintela @ 2017-04-06 13:13 UTC (permalink / raw)
To: qemu-devel; +Cc: dgilbert
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
hw/core/qdev.c | 4 ++--
include/hw/qdev-core.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 1e7fb33..6fa46b5 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -39,7 +39,7 @@
#include "qapi-event.h"
#include "migration/migration.h"
-int qdev_hotplug = 0;
+bool qdev_hotplug = false;
static bool qdev_hot_added = false;
static bool qdev_hot_removed = false;
@@ -385,7 +385,7 @@ void qdev_machine_creation_done(void)
* ok, initial machine setup is done, starting from now we can
* only create hotpluggable devices
*/
- qdev_hotplug = 1;
+ qdev_hotplug = true;
}
bool qdev_machine_modified(void)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index b44b476..a96a913 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -386,7 +386,7 @@ Object *qdev_get_machine(void);
/* FIXME: make this a link<> */
void qdev_set_parent_bus(DeviceState *dev, BusState *bus);
-extern int qdev_hotplug;
+extern bool qdev_hotplug;
char *qdev_get_dev_path(DeviceState *dev);
--
2.9.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 1/5] qdev: qdev_hotplug is really a bool
2017-04-06 13:13 ` [Qemu-devel] [PATCH 1/5] qdev: qdev_hotplug is really a bool Juan Quintela
@ 2017-04-06 13:46 ` Eric Blake
2017-04-06 16:39 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 16+ messages in thread
From: Eric Blake @ 2017-04-06 13:46 UTC (permalink / raw)
To: Juan Quintela, qemu-devel; +Cc: dgilbert
[-- Attachment #1: Type: text/plain, Size: 388 bytes --]
On 04/06/2017 08:13 AM, Juan Quintela wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> hw/core/qdev.c | 4 ++--
> include/hw/qdev-core.h | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 1/5] qdev: qdev_hotplug is really a bool
2017-04-06 13:13 ` [Qemu-devel] [PATCH 1/5] qdev: qdev_hotplug is really a bool Juan Quintela
2017-04-06 13:46 ` Eric Blake
@ 2017-04-06 16:39 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-04-06 16:39 UTC (permalink / raw)
To: Juan Quintela, qemu-devel; +Cc: dgilbert
On 04/06/2017 10:13 AM, Juan Quintela wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/core/qdev.c | 4 ++--
> include/hw/qdev-core.h | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 1e7fb33..6fa46b5 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -39,7 +39,7 @@
> #include "qapi-event.h"
> #include "migration/migration.h"
>
> -int qdev_hotplug = 0;
> +bool qdev_hotplug = false;
> static bool qdev_hot_added = false;
> static bool qdev_hot_removed = false;
>
> @@ -385,7 +385,7 @@ void qdev_machine_creation_done(void)
> * ok, initial machine setup is done, starting from now we can
> * only create hotpluggable devices
> */
> - qdev_hotplug = 1;
> + qdev_hotplug = true;
> }
>
> bool qdev_machine_modified(void)
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index b44b476..a96a913 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -386,7 +386,7 @@ Object *qdev_get_machine(void);
> /* FIXME: make this a link<> */
> void qdev_set_parent_bus(DeviceState *dev, BusState *bus);
>
> -extern int qdev_hotplug;
> +extern bool qdev_hotplug;
>
> char *qdev_get_dev_path(DeviceState *dev);
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 2/5] qdev: Export qdev_hot_removed
2017-04-06 13:13 [Qemu-devel] [PATCH v2 0/5] Disable hotplug during migration Juan Quintela
2017-04-06 13:13 ` [Qemu-devel] [PATCH 1/5] qdev: qdev_hotplug is really a bool Juan Quintela
@ 2017-04-06 13:13 ` Juan Quintela
2017-04-06 14:05 ` Eric Blake
2017-04-11 11:27 ` Markus Armbruster
2017-04-06 13:13 ` [Qemu-devel] [PATCH 3/5] qdev: Move qdev_unplug() to qdev-monitor.c Juan Quintela
` (3 subsequent siblings)
5 siblings, 2 replies; 16+ messages in thread
From: Juan Quintela @ 2017-04-06 13:13 UTC (permalink / raw)
To: qemu-devel; +Cc: dgilbert
I need to move qdev_unplug to qdev-monitor in the following patch, and
it needs access to this variable.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
hw/core/qdev.c | 2 +-
include/hw/qdev-core.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 6fa46b5..c26cf84 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -41,7 +41,7 @@
bool qdev_hotplug = false;
static bool qdev_hot_added = false;
-static bool qdev_hot_removed = false;
+bool qdev_hot_removed = false;
const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
{
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index a96a913..f09b6b7 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -387,6 +387,7 @@ Object *qdev_get_machine(void);
void qdev_set_parent_bus(DeviceState *dev, BusState *bus);
extern bool qdev_hotplug;
+extern bool qdev_hot_removed;
char *qdev_get_dev_path(DeviceState *dev);
--
2.9.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] qdev: Export qdev_hot_removed
2017-04-06 13:13 ` [Qemu-devel] [PATCH 2/5] qdev: Export qdev_hot_removed Juan Quintela
@ 2017-04-06 14:05 ` Eric Blake
2017-04-11 11:27 ` Markus Armbruster
1 sibling, 0 replies; 16+ messages in thread
From: Eric Blake @ 2017-04-06 14:05 UTC (permalink / raw)
To: Juan Quintela, qemu-devel; +Cc: dgilbert
[-- Attachment #1: Type: text/plain, Size: 503 bytes --]
On 04/06/2017 08:13 AM, Juan Quintela wrote:
> I need to move qdev_unplug to qdev-monitor in the following patch, and
> it needs access to this variable.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> hw/core/qdev.c | 2 +-
> include/hw/qdev-core.h | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] qdev: Export qdev_hot_removed
2017-04-06 13:13 ` [Qemu-devel] [PATCH 2/5] qdev: Export qdev_hot_removed Juan Quintela
2017-04-06 14:05 ` Eric Blake
@ 2017-04-11 11:27 ` Markus Armbruster
1 sibling, 0 replies; 16+ messages in thread
From: Markus Armbruster @ 2017-04-11 11:27 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel, dgilbert, Paolo Bonzini
Cc: Paolo for additional qdev expertise.
Juan Quintela <quintela@redhat.com> writes:
> I need to move qdev_unplug to qdev-monitor in the following patch, and
> it needs access to this variable.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> hw/core/qdev.c | 2 +-
> include/hw/qdev-core.h | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 6fa46b5..c26cf84 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -41,7 +41,7 @@
>
> bool qdev_hotplug = false;
> static bool qdev_hot_added = false;
> -static bool qdev_hot_removed = false;
> +bool qdev_hot_removed = false;
Makes qdev_hot_added and qdev_hot_removed differently static, which is
weird.
The reason for this asymmetry is the asymmetry in how they get set:
* qdev_hot_added gets set .instance_init() method in device_initfn()
when it runs after qdev_machine_creation_done(). It's called by
qdev_device_add() via object_new()... then.
* qdev_hot_removed gets set directly in qdev_unplug(), not in the
.instance_finalize() method device_finalize().
Note that for some devices, qdev_unplug() only requests unplug.
Actual unplug happens later, or even not at all. device_finalize()
runs on actual unplug.
Questions:
* Is setting qdev_hot_removed on unplug *requests* correct? Or should
it be set only on *actual* unplug?
* Can we avoid the asymmetry? It's a bit ugly.
> const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
> {
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index a96a913..f09b6b7 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -387,6 +387,7 @@ Object *qdev_get_machine(void);
> void qdev_set_parent_bus(DeviceState *dev, BusState *bus);
>
> extern bool qdev_hotplug;
> +extern bool qdev_hot_removed;
>
> char *qdev_get_dev_path(DeviceState *dev);
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 3/5] qdev: Move qdev_unplug() to qdev-monitor.c
2017-04-06 13:13 [Qemu-devel] [PATCH v2 0/5] Disable hotplug during migration Juan Quintela
2017-04-06 13:13 ` [Qemu-devel] [PATCH 1/5] qdev: qdev_hotplug is really a bool Juan Quintela
2017-04-06 13:13 ` [Qemu-devel] [PATCH 2/5] qdev: Export qdev_hot_removed Juan Quintela
@ 2017-04-06 13:13 ` Juan Quintela
2017-04-06 14:07 ` Eric Blake
2017-04-11 11:29 ` Markus Armbruster
2017-04-06 13:13 ` [Qemu-devel] [PATCH 4/5] migration: Disable hotplug/unplug during migration Juan Quintela
` (2 subsequent siblings)
5 siblings, 2 replies; 16+ messages in thread
From: Juan Quintela @ 2017-04-06 13:13 UTC (permalink / raw)
To: qemu-devel; +Cc: dgilbert
It is not used by linux-user, otherwise I need to to create one stub
for migration_is_idle() on following patch.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
hw/core/qdev.c | 34 ----------------------------------
qdev-monitor.c | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index c26cf84..0df0050 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -271,40 +271,6 @@ HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
return hotplug_ctrl;
}
-void qdev_unplug(DeviceState *dev, Error **errp)
-{
- DeviceClass *dc = DEVICE_GET_CLASS(dev);
- HotplugHandler *hotplug_ctrl;
- HotplugHandlerClass *hdc;
-
- if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) {
- error_setg(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
- return;
- }
-
- if (!dc->hotpluggable) {
- error_setg(errp, QERR_DEVICE_NO_HOTPLUG,
- object_get_typename(OBJECT(dev)));
- return;
- }
-
- qdev_hot_removed = true;
-
- hotplug_ctrl = qdev_get_hotplug_handler(dev);
- /* hotpluggable device MUST have HotplugHandler, if it doesn't
- * then something is very wrong with it */
- g_assert(hotplug_ctrl);
-
- /* If device supports async unplug just request it to be done,
- * otherwise just remove it synchronously */
- hdc = HOTPLUG_HANDLER_GET_CLASS(hotplug_ctrl);
- if (hdc->unplug_request) {
- hotplug_handler_unplug_request(hotplug_ctrl, dev, errp);
- } else {
- hotplug_handler_unplug(hotplug_ctrl, dev, errp);
- }
-}
-
static int qdev_reset_one(DeviceState *dev, void *opaque)
{
device_reset(dev);
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 5f2fcdf..bb3d8ba 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -836,6 +836,40 @@ static DeviceState *find_device_state(const char *id, Error **errp)
return DEVICE(obj);
}
+void qdev_unplug(DeviceState *dev, Error **errp)
+{
+ DeviceClass *dc = DEVICE_GET_CLASS(dev);
+ HotplugHandler *hotplug_ctrl;
+ HotplugHandlerClass *hdc;
+
+ if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) {
+ error_setg(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
+ return;
+ }
+
+ if (!dc->hotpluggable) {
+ error_setg(errp, QERR_DEVICE_NO_HOTPLUG,
+ object_get_typename(OBJECT(dev)));
+ return;
+ }
+
+ qdev_hot_removed = true;
+
+ hotplug_ctrl = qdev_get_hotplug_handler(dev);
+ /* hotpluggable device MUST have HotplugHandler, if it doesn't
+ * then something is very wrong with it */
+ g_assert(hotplug_ctrl);
+
+ /* If device supports async unplug just request it to be done,
+ * otherwise just remove it synchronously */
+ hdc = HOTPLUG_HANDLER_GET_CLASS(hotplug_ctrl);
+ if (hdc->unplug_request) {
+ hotplug_handler_unplug_request(hotplug_ctrl, dev, errp);
+ } else {
+ hotplug_handler_unplug(hotplug_ctrl, dev, errp);
+ }
+}
+
void qmp_device_del(const char *id, Error **errp)
{
DeviceState *dev = find_device_state(id, errp);
--
2.9.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 3/5] qdev: Move qdev_unplug() to qdev-monitor.c
2017-04-06 13:13 ` [Qemu-devel] [PATCH 3/5] qdev: Move qdev_unplug() to qdev-monitor.c Juan Quintela
@ 2017-04-06 14:07 ` Eric Blake
2017-04-11 11:29 ` Markus Armbruster
1 sibling, 0 replies; 16+ messages in thread
From: Eric Blake @ 2017-04-06 14:07 UTC (permalink / raw)
To: Juan Quintela, qemu-devel; +Cc: dgilbert
[-- Attachment #1: Type: text/plain, Size: 561 bytes --]
On 04/06/2017 08:13 AM, Juan Quintela wrote:
> It is not used by linux-user, otherwise I need to to create one stub
> for migration_is_idle() on following patch.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> hw/core/qdev.c | 34 ----------------------------------
> qdev-monitor.c | 34 ++++++++++++++++++++++++++++++++++
> 2 files changed, 34 insertions(+), 34 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 3/5] qdev: Move qdev_unplug() to qdev-monitor.c
2017-04-06 13:13 ` [Qemu-devel] [PATCH 3/5] qdev: Move qdev_unplug() to qdev-monitor.c Juan Quintela
2017-04-06 14:07 ` Eric Blake
@ 2017-04-11 11:29 ` Markus Armbruster
1 sibling, 0 replies; 16+ messages in thread
From: Markus Armbruster @ 2017-04-11 11:29 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel, dgilbert
Juan Quintela <quintela@redhat.com> writes:
> It is not used by linux-user, otherwise I need to to create one stub
> for migration_is_idle() on following patch.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> hw/core/qdev.c | 34 ----------------------------------
> qdev-monitor.c | 34 ++++++++++++++++++++++++++++++++++
> 2 files changed, 34 insertions(+), 34 deletions(-)
Makes sense because its buddy qdev_device_add() is already in
qdev-monitor.c.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 4/5] migration: Disable hotplug/unplug during migration
2017-04-06 13:13 [Qemu-devel] [PATCH v2 0/5] Disable hotplug during migration Juan Quintela
` (2 preceding siblings ...)
2017-04-06 13:13 ` [Qemu-devel] [PATCH 3/5] qdev: Move qdev_unplug() to qdev-monitor.c Juan Quintela
@ 2017-04-06 13:13 ` Juan Quintela
2017-04-06 14:09 ` Eric Blake
2017-04-06 13:13 ` [Qemu-devel] [PATCH 5/5] ram: Remove migration_bitmap_extend() Juan Quintela
2017-04-10 8:22 ` [Qemu-devel] [PATCH v2 0/5] Disable hotplug during migration Hailiang Zhang
5 siblings, 1 reply; 16+ messages in thread
From: Juan Quintela @ 2017-04-06 13:13 UTC (permalink / raw)
To: qemu-devel; +Cc: dgilbert
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>
--
- Fix typo. Thanks Thomas.
- Delay migration check after we have checked that we can hotplug that
device.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
qdev-monitor.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/qdev-monitor.c b/qdev-monitor.c
index bb3d8ba..752c362 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
@@ -603,6 +604,11 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
return NULL;
}
+ if (!migration_is_idle()) {
+ error_setg(errp, "device_add not allowed while migrating");
+ return NULL;
+ }
+
/* create device */
dev = DEVICE(object_new(driver));
@@ -853,6 +859,11 @@ void qdev_unplug(DeviceState *dev, Error **errp)
return;
}
+ if (!migration_is_idle()) {
+ error_setg(errp, "device_add not allowed while migrating");
+ return;
+ }
+
qdev_hot_removed = true;
hotplug_ctrl = qdev_get_hotplug_handler(dev);
--
2.9.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 4/5] migration: Disable hotplug/unplug during migration
2017-04-06 13:13 ` [Qemu-devel] [PATCH 4/5] migration: Disable hotplug/unplug during migration Juan Quintela
@ 2017-04-06 14:09 ` Eric Blake
2017-04-18 19:30 ` Juan Quintela
0 siblings, 1 reply; 16+ messages in thread
From: Eric Blake @ 2017-04-06 14:09 UTC (permalink / raw)
To: Juan Quintela, qemu-devel; +Cc: dgilbert
[-- Attachment #1: Type: text/plain, Size: 1227 bytes --]
On 04/06/2017 08:13 AM, Juan Quintela wrote:
> Until we have reviewed what can/can't be hotplug during migration,
s/hotplug/hotplugged/
> disable it. We can enable it later for the things that we know that
> work. For instance, memory hotplug during postcopy don't work
s/don't/doesn't/
> currently.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
>
> --
>
> @@ -603,6 +604,11 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
> return NULL;
> }
>
> + if (!migration_is_idle()) {
> + error_setg(errp, "device_add not allowed while migrating");
> + return NULL;
> + }
> +
> /* create device */
> dev = DEVICE(object_new(driver));
>
> @@ -853,6 +859,11 @@ void qdev_unplug(DeviceState *dev, Error **errp)
> return;
> }
>
> + if (!migration_is_idle()) {
> + error_setg(errp, "device_add not allowed while migrating");
s/device_add/device_del/ ?
> + return;
> + }
> +
> qdev_hot_removed = true;
>
> hotplug_ctrl = qdev_get_hotplug_handler(dev);
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 4/5] migration: Disable hotplug/unplug during migration
2017-04-06 14:09 ` Eric Blake
@ 2017-04-18 19:30 ` Juan Quintela
0 siblings, 0 replies; 16+ messages in thread
From: Juan Quintela @ 2017-04-18 19:30 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, dgilbert
Eric Blake <eblake@redhat.com> wrote:
> On 04/06/2017 08:13 AM, Juan Quintela wrote:
>> Until we have reviewed what can/can't be hotplug during migration,
>
> s/hotplug/hotplugged/
>
>> disable it. We can enable it later for the things that we know that
>> work. For instance, memory hotplug during postcopy don't work
>
> s/don't/doesn't/
Fixed the three typos, thanks.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 5/5] ram: Remove migration_bitmap_extend()
2017-04-06 13:13 [Qemu-devel] [PATCH v2 0/5] Disable hotplug during migration Juan Quintela
` (3 preceding siblings ...)
2017-04-06 13:13 ` [Qemu-devel] [PATCH 4/5] migration: Disable hotplug/unplug during migration Juan Quintela
@ 2017-04-06 13:13 ` Juan Quintela
2017-04-06 14:09 ` Eric Blake
2017-04-10 8:22 ` [Qemu-devel] [PATCH v2 0/5] Disable hotplug during migration Hailiang Zhang
5 siblings, 1 reply; 16+ messages in thread
From: Juan Quintela @ 2017-04-06 13:13 UTC (permalink / raw)
To: qemu-devel; +Cc: dgilbert
We have disabled memory hotplug, so we don't need to handle
migration_bitamp there.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
exec.c | 1 -
include/exec/ram_addr.h | 2 --
migration/ram.c | 34 ----------------------------------
3 files changed, 37 deletions(-)
diff --git a/exec.c b/exec.c
index de843f4..c2def9e 100644
--- a/exec.c
+++ b/exec.c
@@ -1758,7 +1758,6 @@ static void ram_block_add(RAMBlock *new_block, Error **errp)
new_ram_size = MAX(old_ram_size,
(new_block->offset + new_block->max_length) >> TARGET_PAGE_BITS);
if (new_ram_size > old_ram_size) {
- migration_bitmap_extend(old_ram_size, new_ram_size);
dirty_memory_extend(old_ram_size, new_ram_size);
}
/* Keep the list sorted from biggest to smallest block. Unlike QTAILQ,
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index a8411c7..c9ddcd0 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -413,7 +413,5 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(unsigned long *dest,
return num_dirty;
}
-
-void migration_bitmap_extend(ram_addr_t old, ram_addr_t new);
#endif
#endif
diff --git a/migration/ram.c b/migration/ram.c
index 92d6ff7..f05080f 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1496,40 +1496,6 @@ static void ram_state_reset(RAMState *rs)
#define MAX_WAIT 50 /* ms, half buffered_file limit */
-void migration_bitmap_extend(ram_addr_t old, ram_addr_t new)
-{
- RAMState *rs = &ram_state;
-
- /* called in qemu main thread, so there is
- * no writing race against this migration_bitmap
- */
- if (rs->ram_bitmap) {
- RAMBitmap *old_bitmap = rs->ram_bitmap, *bitmap;
- bitmap = g_new(RAMBitmap, 1);
- bitmap->bmap = bitmap_new(new);
-
- /* prevent migration_bitmap content from being set bit
- * by migration_bitmap_sync_range() at the same time.
- * it is safe to migration if migration_bitmap is cleared bit
- * at the same time.
- */
- qemu_mutex_lock(&rs->bitmap_mutex);
- bitmap_copy(bitmap->bmap, old_bitmap->bmap, old);
- bitmap_set(bitmap->bmap, old, new - old);
-
- /* We don't have a way to safely extend the sentmap
- * with RCU; so mark it as missing, entry to postcopy
- * will fail.
- */
- bitmap->unsentmap = NULL;
-
- atomic_rcu_set(&rs->ram_bitmap, bitmap);
- qemu_mutex_unlock(&rs->bitmap_mutex);
- rs->migration_dirty_pages += new - old;
- call_rcu(old_bitmap, migration_bitmap_free, rcu);
- }
-}
-
/*
* 'expected' is the value you expect the bitmap mostly to be full
* of; it won't bother printing lines that are all this value.
--
2.9.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/5] Disable hotplug during migration
2017-04-06 13:13 [Qemu-devel] [PATCH v2 0/5] Disable hotplug during migration Juan Quintela
` (4 preceding siblings ...)
2017-04-06 13:13 ` [Qemu-devel] [PATCH 5/5] ram: Remove migration_bitmap_extend() Juan Quintela
@ 2017-04-10 8:22 ` Hailiang Zhang
5 siblings, 0 replies; 16+ messages in thread
From: Hailiang Zhang @ 2017-04-10 8:22 UTC (permalink / raw)
To: Juan Quintela, qemu-devel; +Cc: dgilbert
On 2017/4/6 21:13, Juan Quintela wrote:
> Hi
>
> This updates patches with all the comments received.
> I move qdev_unplug() to make linux-user compile.
>
> Please, review.
>
>
> [RFC - v1]
> This series disable hotplug/unplug during migration. Thank to Markus
> for explaining where I had to put the checks. Why? Because during
> migration we will fail if there are changes. For instance, in
> postcopy, if we add a memory region, we would failing. Same for other
> devices if they are not setup exactly the same on destination.
>
> Iidea would be to disable it, andthen enable for the thing that we know that work.
>
> This series are on top of my previous RAMState v2 serie.
>
> Commets, please?
Make sense, this will benefit COLO too :)
After the types found by Eric be fixed in patch 5,
Reviewed-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Thanks, Juan.
>
>
> *** BLURB HERE ***
>
> Juan Quintela (5):
> qdev: qdev_hotplug is really a bool
> qdev: Export qdev_hot_removed
> qdev: Move qdev_unplug() to qdev-monitor.c
> migration: Disable hotplug/unplug during migration
> ram: Remove migration_bitmap_extend()
>
> exec.c | 1 -
> hw/core/qdev.c | 40 +++-------------------------------------
> include/exec/ram_addr.h | 2 --
> include/hw/qdev-core.h | 3 ++-
> migration/ram.c | 34 ----------------------------------
> qdev-monitor.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 50 insertions(+), 75 deletions(-)
>
^ permalink raw reply [flat|nested] 16+ messages in thread