* [Qemu-devel] [PATCH v3 0/4] Introduce a new --only-migratable option
@ 2017-01-04 12:32 Ashijeet Acharya
2017-01-04 12:32 ` [Qemu-devel] [PATCH v3 1/4] migration: Add a new option to enable only-migratable Ashijeet Acharya
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Ashijeet Acharya @ 2017-01-04 12:32 UTC (permalink / raw)
To: dgilbert
Cc: jsnow, amit.shah, pbonzini, kwolf, armbru, quintela, mst,
marcandre.lureau, groug, aneesh.kumar, peter.maydell, qemu-devel,
Ashijeet Acharya
Previously posted series patches:
http://lists.nongnu.org/archive/html/qemu-devel/2016-12/msg02391.html
http://lists.nongnu.org/archive/html/qemu-devel/2016-12/msg02062.html
This series adds a new command line option "--only-migratable" which will only
allow addition of those devices to a QEMU instance which are migratable and do
not abruptly fail QEMU after migration.
Patch 1 adds the new option "-only-migratable".
Patch 2 adds compatibility for various "device adding" options for both via
command line and hotplug methods.
Patch 3 helps to fail the migration blocker if the migration is already in
progress and thus cannot be blocked.
Note: This patch was originally written by John Snow and I have only made few
changes.
Patch 4 handles the special case of devices which become unmigratable
dynamically
by making call to "migrate_add_blocker". Here we fail the particular action of
the
device which results in an unmigratable VM.
Eg: 9pfs fails to mount the filesystem.
Note: I have not been able to test and compile the ARM drivers for KVM. They
are:
hw/intc/arm_gic_kvm.c
hw/intc/arm_gicv3_its_kvm.c
hw/intc/arm_gicv3_kvm.c
Changes in v3:
- set s->root_fid after migrate_add_blocker
- free migration_blocker inside v9fs_attach()
- change back ret<0 to just ret
- free local_err
Changes in v2:
- change the documentation for the new option
- add a NULL check for ObjectClass
- break patch 3 into patch 3 and 4
- use error_append_hint
- return -EACCES for only-migratable
- fix the error messages
Ashijeet Acharya (4):
migration: Add a new option to enable only-migratable
migration: Allow "device add" options to only add migratable devices
migration: disallow migrate_add_blocker during migration
migration: Fail migration blocker for --only-migratble
block/qcow.c | 9 ++++++++-
block/vdi.c | 9 ++++++++-
block/vhdx.c | 18 ++++++++++++------
block/vmdk.c | 10 +++++++++-
block/vpc.c | 13 ++++++++++---
block/vvfat.c | 22 ++++++++++++++--------
hw/9pfs/9p.c | 26 +++++++++++++++++++++-----
hw/display/virtio-gpu.c | 34 +++++++++++++++++++++-------------
hw/intc/arm_gic_kvm.c | 19 +++++++++++++------
hw/intc/arm_gicv3_its_kvm.c | 21 ++++++++++++++-------
hw/intc/arm_gicv3_kvm.c | 22 +++++++++++++++-------
hw/misc/ivshmem.c | 17 +++++++++++++----
hw/scsi/vhost-scsi.c | 27 +++++++++++++++++++++------
hw/usb/bus.c | 18 ++++++++++++++++++
hw/virtio/vhost.c | 10 +++++++++-
include/migration/migration.h | 9 ++++++++-
migration/migration.c | 42 ++++++++++++++++++++++++++++++++++++++++--
qdev-monitor.c | 9 +++++++++
qemu-options.hx | 9 +++++++++
stubs/migr-blocker.c | 3 ++-
target-i386/kvm.c | 19 ++++++++++++++++---
vl.c | 4 ++++
22 files changed, 294 insertions(+), 76 deletions(-)
--
2.6.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v3 1/4] migration: Add a new option to enable only-migratable
2017-01-04 12:32 [Qemu-devel] [PATCH v3 0/4] Introduce a new --only-migratable option Ashijeet Acharya
@ 2017-01-04 12:32 ` Ashijeet Acharya
2017-01-04 12:32 ` [Qemu-devel] [PATCH v3 2/4] migration: Allow "device add" options to only add migratable devices Ashijeet Acharya
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Ashijeet Acharya @ 2017-01-04 12:32 UTC (permalink / raw)
To: dgilbert
Cc: jsnow, amit.shah, pbonzini, kwolf, armbru, quintela, mst,
marcandre.lureau, groug, aneesh.kumar, peter.maydell, qemu-devel,
Ashijeet Acharya
Add a new option "--only-migratable" in qemu which will allow to add
only those devices which will not fail qemu after migration. Devices
set with the flag 'unmigratable' cannot be added when this option will
be used.
Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
---
include/migration/migration.h | 3 +++
qemu-options.hx | 9 +++++++++
vl.c | 4 ++++
3 files changed, 16 insertions(+)
diff --git a/include/migration/migration.h b/include/migration/migration.h
index c309d23..40b3697 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -38,6 +38,9 @@
#define QEMU_VM_COMMAND 0x08
#define QEMU_VM_SECTION_FOOTER 0x7e
+/* for vl.c */
+extern int only_migratable;
+
struct MigrationParams {
bool blk;
bool shared;
diff --git a/qemu-options.hx b/qemu-options.hx
index c534a2f..1e16ae8 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3574,6 +3574,15 @@ be used to change settings (such as migration parameters) prior to issuing
the migrate_incoming to allow the migration to begin.
ETEXI
+DEF("only-migratable", 0, QEMU_OPTION_only_migratable, \
+ "-only-migratable allow only migratable devices\n", QEMU_ARCH_ALL)
+STEXI
+@item -only-migratable
+@findex -only-migratable
+Only allow migratable devices. Devices will not be allowed to enter an
+unmigratable state.
+ETEXI
+
DEF("nodefaults", 0, QEMU_OPTION_nodefaults, \
"-nodefaults don't create default devices\n", QEMU_ARCH_ALL)
STEXI
diff --git a/vl.c b/vl.c
index d77dd86..82bffb9 100644
--- a/vl.c
+++ b/vl.c
@@ -180,6 +180,7 @@ bool boot_strict;
uint8_t *boot_splash_filedata;
size_t boot_splash_filedata_size;
uint8_t qemu_extra_params_fw[2];
+int only_migratable = 0; /* turn it off unless user states otherwise */
int icount_align_option;
@@ -3914,6 +3915,9 @@ int main(int argc, char **argv, char **envp)
}
incoming = optarg;
break;
+ case QEMU_OPTION_only_migratable:
+ only_migratable = 1;
+ break;
case QEMU_OPTION_nodefaults:
has_defaults = 0;
break;
--
2.6.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v3 2/4] migration: Allow "device add" options to only add migratable devices
2017-01-04 12:32 [Qemu-devel] [PATCH v3 0/4] Introduce a new --only-migratable option Ashijeet Acharya
2017-01-04 12:32 ` [Qemu-devel] [PATCH v3 1/4] migration: Add a new option to enable only-migratable Ashijeet Acharya
@ 2017-01-04 12:32 ` Ashijeet Acharya
2017-01-04 12:32 ` [Qemu-devel] [PATCH v3 3/4] migration: disallow migrate_add_blocker during migration Ashijeet Acharya
` (2 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Ashijeet Acharya @ 2017-01-04 12:32 UTC (permalink / raw)
To: dgilbert
Cc: jsnow, amit.shah, pbonzini, kwolf, armbru, quintela, mst,
marcandre.lureau, groug, aneesh.kumar, peter.maydell, qemu-devel,
Ashijeet Acharya
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 | 18 ++++++++++++++++++
qdev-monitor.c | 9 +++++++++
2 files changed, 27 insertions(+)
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 25913ad..3d26103 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,21 @@ USBDevice *usbdevice_create(const char *cmdline)
return NULL;
}
+ klass = object_class_by_name(f->name);
+ if (klass == NULL) {
+ return NULL;
+ }
+
+ 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v3 3/4] migration: disallow migrate_add_blocker during migration
2017-01-04 12:32 [Qemu-devel] [PATCH v3 0/4] Introduce a new --only-migratable option Ashijeet Acharya
2017-01-04 12:32 ` [Qemu-devel] [PATCH v3 1/4] migration: Add a new option to enable only-migratable Ashijeet Acharya
2017-01-04 12:32 ` [Qemu-devel] [PATCH v3 2/4] migration: Allow "device add" options to only add migratable devices Ashijeet Acharya
@ 2017-01-04 12:32 ` Ashijeet Acharya
2017-01-06 16:18 ` Greg Kurz
2017-01-04 12:32 ` [Qemu-devel] [PATCH v3 4/4] migration: Fail migration blocker for --only-migratble Ashijeet Acharya
2017-01-04 12:41 ` [Qemu-devel] [PATCH v3 0/4] Introduce a new --only-migratable option no-reply
4 siblings, 1 reply; 13+ messages in thread
From: Ashijeet Acharya @ 2017-01-04 12:32 UTC (permalink / raw)
To: dgilbert
Cc: jsnow, amit.shah, pbonzini, kwolf, armbru, quintela, mst,
marcandre.lureau, groug, aneesh.kumar, peter.maydell, qemu-devel,
Ashijeet Acharya
If a migration is already in progress and somebody attempts
to add a migration blocker, this should rightly fail.
Add an errp parameter and a retcode return value to migrate_add_blocker.
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
---
block/qcow.c | 6 +++++-
block/vdi.c | 6 +++++-
block/vhdx.c | 16 ++++++++++------
block/vmdk.c | 7 ++++++-
block/vpc.c | 10 +++++++---
block/vvfat.c | 20 ++++++++++++--------
hw/9pfs/9p.c | 20 +++++++++++++++-----
hw/display/virtio-gpu.c | 29 ++++++++++++++++-------------
hw/intc/arm_gic_kvm.c | 16 ++++++++++------
hw/intc/arm_gicv3_its_kvm.c | 18 +++++++++++-------
hw/intc/arm_gicv3_kvm.c | 19 ++++++++++++-------
hw/misc/ivshmem.c | 11 +++++++----
hw/scsi/vhost-scsi.c | 25 +++++++++++++++++++------
hw/virtio/vhost.c | 8 +++++++-
include/migration/migration.h | 6 +++++-
migration/migration.c | 35 +++++++++++++++++++++++++++++++++--
stubs/migr-blocker.c | 3 ++-
target-i386/kvm.c | 16 +++++++++++++---
18 files changed, 195 insertions(+), 76 deletions(-)
diff --git a/block/qcow.c b/block/qcow.c
index 7540f43..11526a1 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -252,7 +252,11 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
error_setg(&s->migration_blocker, "The qcow format used by node '%s' "
"does not support live migration",
bdrv_get_device_or_node_name(bs));
- migrate_add_blocker(s->migration_blocker);
+ ret = migrate_add_blocker(s->migration_blocker, errp);
+ if (ret < 0) {
+ error_free(s->migration_blocker);
+ goto fail;
+ }
qemu_co_mutex_init(&s->lock);
return 0;
diff --git a/block/vdi.c b/block/vdi.c
index 96b78d5..2b56f52 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -471,7 +471,11 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
error_setg(&s->migration_blocker, "The vdi format used by node '%s' "
"does not support live migration",
bdrv_get_device_or_node_name(bs));
- migrate_add_blocker(s->migration_blocker);
+ ret = migrate_add_blocker(s->migration_blocker, errp);
+ if (ret < 0) {
+ error_free(s->migration_blocker);
+ goto fail_free_bmap;
+ }
qemu_co_mutex_init(&s->write_lock);
diff --git a/block/vhdx.c b/block/vhdx.c
index 0ba2f0a..d81941b 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -991,6 +991,16 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
}
}
+ /* Disable migration when VHDX images are used */
+ error_setg(&s->migration_blocker, "The vhdx format used by node '%s' "
+ "does not support live migration",
+ bdrv_get_device_or_node_name(bs));
+ ret = migrate_add_blocker(s->migration_blocker, errp);
+ if (ret < 0) {
+ error_free(s->migration_blocker);
+ goto fail;
+ }
+
if (flags & BDRV_O_RDWR) {
ret = vhdx_update_headers(bs, s, false, NULL);
if (ret < 0) {
@@ -1000,12 +1010,6 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
/* TODO: differencing files */
- /* Disable migration when VHDX images are used */
- error_setg(&s->migration_blocker, "The vhdx format used by node '%s' "
- "does not support live migration",
- bdrv_get_device_or_node_name(bs));
- migrate_add_blocker(s->migration_blocker);
-
return 0;
fail:
vhdx_close(bs);
diff --git a/block/vmdk.c b/block/vmdk.c
index a11c27a..4a45a83 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -976,7 +976,12 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
error_setg(&s->migration_blocker, "The vmdk format used by node '%s' "
"does not support live migration",
bdrv_get_device_or_node_name(bs));
- migrate_add_blocker(s->migration_blocker);
+ ret = migrate_add_blocker(s->migration_blocker, errp);
+ if (ret < 0) {
+ error_free(s->migration_blocker);
+ goto fail;
+ }
+
g_free(buf);
return 0;
diff --git a/block/vpc.c b/block/vpc.c
index 8d5886f..299a8c8 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -422,13 +422,17 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
#endif
}
- qemu_co_mutex_init(&s->lock);
-
/* Disable migration when VHD images are used */
error_setg(&s->migration_blocker, "The vpc format used by node '%s' "
"does not support live migration",
bdrv_get_device_or_node_name(bs));
- migrate_add_blocker(s->migration_blocker);
+ ret = migrate_add_blocker(s->migration_blocker, errp);
+ if (ret < 0) {
+ error_free(s->migration_blocker);
+ goto fail;
+ }
+
+ qemu_co_mutex_init(&s->lock);
return 0;
diff --git a/block/vvfat.c b/block/vvfat.c
index ded2109..3de3320 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1185,22 +1185,26 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
s->sector_count = s->faked_sectors + s->sectors_per_cluster*s->cluster_count;
- if (s->first_sectors_number == 0x40) {
- init_mbr(s, cyls, heads, secs);
- }
-
- // assert(is_consistent(s));
- qemu_co_mutex_init(&s->lock);
-
/* Disable migration when vvfat is used rw */
if (s->qcow) {
error_setg(&s->migration_blocker,
"The vvfat (rw) format used by node '%s' "
"does not support live migration",
bdrv_get_device_or_node_name(bs));
- migrate_add_blocker(s->migration_blocker);
+ ret = migrate_add_blocker(s->migration_blocker, errp);
+ if (ret < 0) {
+ error_free(s->migration_blocker);
+ goto fail;
+ }
+ }
+
+ if (s->first_sectors_number == 0x40) {
+ init_mbr(s, cyls, heads, secs);
}
+ // assert(is_consistent(s));
+ qemu_co_mutex_init(&s->lock);
+
ret = 0;
fail:
qemu_opts_del(opts);
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index faebd91..43cb065 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1013,20 +1013,30 @@ static void coroutine_fn v9fs_attach(void *opaque)
goto out;
}
err += offset;
- memcpy(&s->root_qid, &qid, sizeof(qid));
- trace_v9fs_attach_return(pdu->tag, pdu->id,
- qid.type, qid.version, qid.path);
+
/*
* disable migration if we haven't done already.
* attach could get called multiple times for the same export.
*/
if (!s->migration_blocker) {
- s->root_fid = fid;
+ int ret;
error_setg(&s->migration_blocker,
"Migration is disabled when VirtFS export path '%s' is mounted in the guest using mount_tag '%s'",
s->ctx.fs_root ? s->ctx.fs_root : "NULL", s->tag);
- migrate_add_blocker(s->migration_blocker);
+ ret = migrate_add_blocker(s->migration_blocker, NULL);
+ if (ret < 0) {
+ err = ret;
+ clunk_fid(s, fid);
+ error_free(s->migration_blocker);
+ s->migration_blocker = NULL;
+ goto out;
+ }
+ s->root_fid = fid;
}
+
+ memcpy(&s->root_qid, &qid, sizeof(qid));
+ trace_v9fs_attach_return(pdu->tag, pdu->id,
+ qid.type, qid.version, qid.path);
out:
put_fid(pdu, fidp);
out_nofid:
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 5f32e1a..6e60b8c 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1108,14 +1108,6 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
return;
}
- g->config_size = sizeof(struct virtio_gpu_config);
- g->virtio_config.num_scanouts = g->conf.max_outputs;
- virtio_init(VIRTIO_DEVICE(g), "virtio-gpu", VIRTIO_ID_GPU,
- g->config_size);
-
- g->req_state[0].width = 1024;
- g->req_state[0].height = 768;
-
g->use_virgl_renderer = false;
#if !defined(CONFIG_VIRGL) || defined(HOST_WORDS_BIGENDIAN)
have_virgl = false;
@@ -1127,6 +1119,22 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
}
if (virtio_gpu_virgl_enabled(g->conf)) {
+ error_setg(&g->migration_blocker, "virgl is not yet migratable");
+ if (migrate_add_blocker(g->migration_blocker, errp) < 0) {
+ error_free(g->migration_blocker);
+ return;
+ }
+ }
+
+ g->config_size = sizeof(struct virtio_gpu_config);
+ g->virtio_config.num_scanouts = g->conf.max_outputs;
+ virtio_init(VIRTIO_DEVICE(g), "virtio-gpu", VIRTIO_ID_GPU,
+ g->config_size);
+
+ g->req_state[0].width = 1024;
+ g->req_state[0].height = 768;
+
+ if (virtio_gpu_virgl_enabled(g->conf)) {
/* use larger control queue in 3d mode */
g->ctrl_vq = virtio_add_queue(vdev, 256, virtio_gpu_handle_ctrl_cb);
g->cursor_vq = virtio_add_queue(vdev, 16, virtio_gpu_handle_cursor_cb);
@@ -1152,11 +1160,6 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
dpy_gfx_replace_surface(g->scanout[i].con, NULL);
}
}
-
- if (virtio_gpu_virgl_enabled(g->conf)) {
- error_setg(&g->migration_blocker, "virgl is not yet migratable");
- migrate_add_blocker(g->migration_blocker);
- }
}
static void virtio_gpu_device_unrealize(DeviceState *qdev, Error **errp)
diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index 11729ee..3614daa 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -510,6 +510,16 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp)
return;
}
+ if (!kvm_arm_gic_can_save_restore(s)) {
+ error_setg(&s->migration_blocker, "This operating system kernel does "
+ "not support vGICv2 migration");
+ ret = migrate_add_blocker(s->migration_blocker, errp);
+ if (ret < 0) {
+ error_free(s->migration_blocker);
+ return;
+ }
+ }
+
gic_init_irqs_and_mmio(s, kvm_arm_gicv2_set_irq, NULL);
for (i = 0; i < s->num_irq - GIC_INTERNAL; i++) {
@@ -558,12 +568,6 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp)
KVM_VGIC_V2_ADDR_TYPE_CPU,
s->dev_fd);
- if (!kvm_arm_gic_can_save_restore(s)) {
- error_setg(&s->migration_blocker, "This operating system kernel does "
- "not support vGICv2 migration");
- migrate_add_blocker(s->migration_blocker);
- }
-
if (kvm_has_gsi_routing()) {
/* set up irq routing */
kvm_init_irq_routing(kvm_state);
diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
index fc246e0..950a02f 100644
--- a/hw/intc/arm_gicv3_its_kvm.c
+++ b/hw/intc/arm_gicv3_its_kvm.c
@@ -63,6 +63,17 @@ static void kvm_arm_its_realize(DeviceState *dev, Error **errp)
return;
}
+ /*
+ * Block migration of a KVM GICv3 ITS device: the API for saving and
+ * restoring the state in the kernel is not yet available
+ */
+ error_setg(&s->migration_blocker, "vITS migration is not implemented");
+ ret = migrate_add_blocker(s->migration_blocker, errp);
+ if (ret < 0) {
+ error_free(s->migration_blocker);
+ return;
+ }
+
/* explicit init of the ITS */
kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
KVM_DEV_ARM_VGIC_CTRL_INIT, NULL, true);
@@ -73,13 +84,6 @@ static void kvm_arm_its_realize(DeviceState *dev, Error **errp)
gicv3_its_init_mmio(s, NULL);
- /*
- * Block migration of a KVM GICv3 ITS device: the API for saving and
- * restoring the state in the kernel is not yet available
- */
- error_setg(&s->migration_blocker, "vITS migration is not implemented");
- migrate_add_blocker(s->migration_blocker);
-
kvm_msi_use_devid = true;
kvm_gsi_direct_mapping = false;
kvm_msi_via_irqfd_allowed = kvm_irqfds_enabled();
diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index 199a439..06f6f30 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -86,6 +86,7 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
KVMARMGICv3Class *kgc = KVM_ARM_GICV3_GET_CLASS(s);
Error *local_err = NULL;
int i;
+ int ret;
DPRINTF("kvm_arm_gicv3_realize\n");
@@ -110,6 +111,17 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
return;
}
+ /* Block migration of a KVM GICv3 device: the API for saving and restoring
+ * the state in the kernel is not yet finalised in the kernel or
+ * implemented in QEMU.
+ */
+ error_setg(&s->migration_blocker, "vGICv3 migration is not implemented");
+ ret = migrate_add_blocker(s->migration_blocker, errp);
+ if (ret < 0) {
+ error_free(s->migration_blocker);
+ return;
+ }
+
kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_NR_IRQS,
0, &s->num_irq, true);
@@ -122,13 +134,6 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
kvm_arm_register_device(&s->iomem_redist, -1, KVM_DEV_ARM_VGIC_GRP_ADDR,
KVM_VGIC_V3_ADDR_TYPE_REDIST, s->dev_fd);
- /* Block migration of a KVM GICv3 device: the API for saving and restoring
- * the state in the kernel is not yet finalised in the kernel or
- * implemented in QEMU.
- */
- error_setg(&s->migration_blocker, "vGICv3 migration is not implemented");
- migrate_add_blocker(s->migration_blocker);
-
if (kvm_has_gsi_routing()) {
/* set up irq routing */
kvm_init_irq_routing(kvm_state);
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index abeaf3d..585cc5b 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -903,9 +903,6 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
}
}
- vmstate_register_ram(s->ivshmem_bar2, DEVICE(s));
- pci_register_bar(PCI_DEVICE(s), 2, attr, s->ivshmem_bar2);
-
if (s->master == ON_OFF_AUTO_AUTO) {
s->master = s->vm_id == 0 ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
}
@@ -913,8 +910,14 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
if (!ivshmem_is_master(s)) {
error_setg(&s->migration_blocker,
"Migration is disabled when using feature 'peer mode' in device 'ivshmem'");
- migrate_add_blocker(s->migration_blocker);
+ if (migrate_add_blocker(s->migration_blocker, errp) < 0) {
+ error_free(s->migration_blocker);
+ return;
+ }
}
+
+ vmstate_register_ram(s->ivshmem_bar2, DEVICE(s));
+ pci_register_bar(PCI_DEVICE(s), 2, attr, s->ivshmem_bar2);
}
static void ivshmem_exit(PCIDevice *dev)
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index 5b26946..ff503d0 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -238,8 +238,14 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
vhost_dummy_handle_output);
if (err != NULL) {
error_propagate(errp, err);
- close(vhostfd);
- return;
+ goto close_fd;
+ }
+
+ error_setg(&s->migration_blocker,
+ "vhost-scsi does not support migration");
+ ret = migrate_add_blocker(s->migration_blocker, errp);
+ if (ret < 0) {
+ goto free_blocker;
}
s->dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs->conf.num_queues;
@@ -252,7 +258,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
if (ret < 0) {
error_setg(errp, "vhost-scsi: vhost initialization failed: %s",
strerror(-ret));
- return;
+ goto free_vqs;
}
/* At present, channel and lun both are 0 for bootable vhost-scsi disk */
@@ -261,9 +267,16 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
/* Note: we can also get the minimum tpgt from kernel */
s->target = vs->conf.boot_tpgt;
- error_setg(&s->migration_blocker,
- "vhost-scsi does not support migration");
- migrate_add_blocker(s->migration_blocker);
+ return;
+
+ free_vqs:
+ migrate_del_blocker(s->migration_blocker);
+ g_free(s->dev.vqs);
+ free_blocker:
+ error_free(s->migration_blocker);
+ close_fd:
+ close(vhostfd);
+ return;
}
static void vhost_scsi_unrealize(DeviceState *dev, Error **errp)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index f7f7023..416fada 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1157,7 +1157,13 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
}
if (hdev->migration_blocker != NULL) {
- migrate_add_blocker(hdev->migration_blocker);
+ Error *local_err;
+ r = migrate_add_blocker(hdev->migration_blocker, &local_err);
+ if (r < 0) {
+ error_report_err(local_err);
+ error_free(hdev->migration_blocker);
+ goto fail_busyloop;
+ }
}
hdev->mem = g_malloc0(offsetof(struct vhost_memory, regions));
diff --git a/include/migration/migration.h b/include/migration/migration.h
index 40b3697..46a4bb5 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -243,6 +243,7 @@ void remove_migration_state_change_notifier(Notifier *notify);
MigrationState *migrate_init(const MigrationParams *params);
bool migration_is_blocked(Error **errp);
bool migration_in_setup(MigrationState *);
+bool migration_is_idle(MigrationState *s);
bool migration_has_finished(MigrationState *);
bool migration_has_failed(MigrationState *);
/* True if outgoing migration has entered postcopy phase */
@@ -287,8 +288,11 @@ int ram_postcopy_incoming_init(MigrationIncomingState *mis);
* @migrate_add_blocker - prevent migration from proceeding
*
* @reason - an error to be returned whenever migration is attempted
+ * @errp - [out] The reason (if any) we cannot block migration right now.
+ *
+ * @returns - 0 on success, -EBUSY on failure, with errp set.
*/
-void migrate_add_blocker(Error *reason);
+int migrate_add_blocker(Error *reason, Error **errp);
/**
* @migrate_del_blocker - remove a blocking error from migration
diff --git a/migration/migration.c b/migration/migration.c
index f498ab8..713e012 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1044,6 +1044,31 @@ bool migration_in_postcopy_after_devices(MigrationState *s)
return migration_in_postcopy(s) && s->postcopy_after_devices;
}
+bool migration_is_idle(MigrationState *s)
+{
+ if (!s) {
+ s = migrate_get_current();
+ }
+
+ switch (s->state) {
+ case MIGRATION_STATUS_NONE:
+ case MIGRATION_STATUS_CANCELLED:
+ case MIGRATION_STATUS_COMPLETED:
+ case MIGRATION_STATUS_FAILED:
+ return true;
+ case MIGRATION_STATUS_SETUP:
+ case MIGRATION_STATUS_CANCELLING:
+ case MIGRATION_STATUS_ACTIVE:
+ case MIGRATION_STATUS_POSTCOPY_ACTIVE:
+ case MIGRATION_STATUS_COLO:
+ return false;
+ case MIGRATION_STATUS__MAX:
+ g_assert_not_reached();
+ }
+
+ return false;
+}
+
MigrationState *migrate_init(const MigrationParams *params)
{
MigrationState *s = migrate_get_current();
@@ -1086,9 +1111,15 @@ MigrationState *migrate_init(const MigrationParams *params)
static GSList *migration_blockers;
-void migrate_add_blocker(Error *reason)
+int migrate_add_blocker(Error *reason, Error **errp)
{
- migration_blockers = g_slist_prepend(migration_blockers, reason);
+ if (migration_is_idle(NULL)) {
+ migration_blockers = g_slist_prepend(migration_blockers, reason);
+ return 0;
+ }
+
+ error_setg(errp, "Cannot block a migration already in-progress");
+ return -EBUSY;
}
void migrate_del_blocker(Error *reason)
diff --git a/stubs/migr-blocker.c b/stubs/migr-blocker.c
index 8ab3604..a5ba18f 100644
--- a/stubs/migr-blocker.c
+++ b/stubs/migr-blocker.c
@@ -2,8 +2,9 @@
#include "qemu-common.h"
#include "migration/migration.h"
-void migrate_add_blocker(Error *reason)
+int migrate_add_blocker(Error *reason, Error **errp)
{
+ return 0;
}
void migrate_del_blocker(Error *reason)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index f62264a..6031a1c 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -961,7 +961,11 @@ int kvm_arch_init_vcpu(CPUState *cs)
error_setg(&invtsc_mig_blocker,
"State blocked by non-migratable CPU device"
" (invtsc flag)");
- migrate_add_blocker(invtsc_mig_blocker);
+ r = migrate_add_blocker(invtsc_mig_blocker, NULL);
+ if (r < 0) {
+ error_report("kvm: migrate_add_blocker failed");
+ goto efail;
+ }
/* for savevm */
vmstate_x86_cpu.unmigratable = 1;
}
@@ -969,12 +973,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
cpuid_data.cpuid.padding = 0;
r = kvm_vcpu_ioctl(cs, KVM_SET_CPUID2, &cpuid_data);
if (r) {
- return r;
+ goto fail;
}
r = kvm_arch_set_tsc_khz(cs);
if (r < 0) {
- return r;
+ goto fail;
}
/* vcpu's TSC frequency is either specified by user, or following
@@ -1001,6 +1005,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
}
return 0;
+
+ fail:
+ migrate_del_blocker(invtsc_mig_blocker);
+ efail:
+ error_free(invtsc_mig_blocker);
+ return r;
}
void kvm_arch_reset_vcpu(X86CPU *cpu)
--
2.6.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v3 4/4] migration: Fail migration blocker for --only-migratble
2017-01-04 12:32 [Qemu-devel] [PATCH v3 0/4] Introduce a new --only-migratable option Ashijeet Acharya
` (2 preceding siblings ...)
2017-01-04 12:32 ` [Qemu-devel] [PATCH v3 3/4] migration: disallow migrate_add_blocker during migration Ashijeet Acharya
@ 2017-01-04 12:32 ` Ashijeet Acharya
2017-01-06 16:40 ` Greg Kurz
2017-01-04 12:41 ` [Qemu-devel] [PATCH v3 0/4] Introduce a new --only-migratable option no-reply
4 siblings, 1 reply; 13+ messages in thread
From: Ashijeet Acharya @ 2017-01-04 12:32 UTC (permalink / raw)
To: dgilbert
Cc: jsnow, amit.shah, pbonzini, kwolf, armbru, quintela, mst,
marcandre.lureau, groug, aneesh.kumar, peter.maydell, qemu-devel,
Ashijeet Acharya
migrate_add_blocker should rightly fail if the '--only-migratable'
option was specified and the device in use should not be able to
perform the action which results in an unmigratable VM.
Make migrate_add_blocker return -EACCES in this case.
Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
---
block/qcow.c | 5 ++++-
block/vdi.c | 5 ++++-
block/vhdx.c | 6 ++++--
block/vmdk.c | 5 ++++-
block/vpc.c | 5 ++++-
block/vvfat.c | 6 ++++--
hw/9pfs/9p.c | 8 +++++++-
hw/display/virtio-gpu.c | 9 +++++++--
hw/intc/arm_gic_kvm.c | 5 ++++-
hw/intc/arm_gicv3_its_kvm.c | 5 ++++-
hw/intc/arm_gicv3_kvm.c | 5 ++++-
hw/misc/ivshmem.c | 10 ++++++++--
hw/scsi/vhost-scsi.c | 8 +++++---
hw/virtio/vhost.c | 6 ++++--
migration/migration.c | 7 +++++++
target-i386/kvm.c | 5 ++++-
16 files changed, 78 insertions(+), 22 deletions(-)
diff --git a/block/qcow.c b/block/qcow.c
index 11526a1..bdc6446 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -254,7 +254,10 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
bdrv_get_device_or_node_name(bs));
ret = migrate_add_blocker(s->migration_blocker, errp);
if (ret < 0) {
- error_free(s->migration_blocker);
+ if (ret == -EACCES) {
+ error_append_hint(errp, "Cannot use a node with qcow format as "
+ "it does not support live migration");
+ }
goto fail;
}
diff --git a/block/vdi.c b/block/vdi.c
index 2b56f52..0b011ea 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -473,7 +473,10 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
bdrv_get_device_or_node_name(bs));
ret = migrate_add_blocker(s->migration_blocker, errp);
if (ret < 0) {
- error_free(s->migration_blocker);
+ if (ret == -EACCES) {
+ error_append_hint(errp, "Cannot use a node with vdi format as "
+ "it does not support live migration");
+ }
goto fail_free_bmap;
}
diff --git a/block/vhdx.c b/block/vhdx.c
index d81941b..b8d53ec 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -997,10 +997,12 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
bdrv_get_device_or_node_name(bs));
ret = migrate_add_blocker(s->migration_blocker, errp);
if (ret < 0) {
- error_free(s->migration_blocker);
+ if (ret == -EACCES) {
+ error_append_hint(errp, "Cannot use a node with vhdx format as "
+ "it does not support live migration");
+ }
goto fail;
}
-
if (flags & BDRV_O_RDWR) {
ret = vhdx_update_headers(bs, s, false, NULL);
if (ret < 0) {
diff --git a/block/vmdk.c b/block/vmdk.c
index 4a45a83..defe400 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -978,7 +978,10 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
bdrv_get_device_or_node_name(bs));
ret = migrate_add_blocker(s->migration_blocker, errp);
if (ret < 0) {
- error_free(s->migration_blocker);
+ if (ret == -EACCES) {
+ error_append_hint(errp, "Cannot use a node with vmdk format as "
+ "it does not support live migration");
+ }
goto fail;
}
diff --git a/block/vpc.c b/block/vpc.c
index 299a8c8..5e58d77 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -428,7 +428,10 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
bdrv_get_device_or_node_name(bs));
ret = migrate_add_blocker(s->migration_blocker, errp);
if (ret < 0) {
- error_free(s->migration_blocker);
+ if (ret == -EACCES) {
+ error_append_hint(errp, "Cannot use a node with vpc format as "
+ "it does not support live migration");
+ }
goto fail;
}
diff --git a/block/vvfat.c b/block/vvfat.c
index 3de3320..5a6e038 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1193,7 +1193,10 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
bdrv_get_device_or_node_name(bs));
ret = migrate_add_blocker(s->migration_blocker, errp);
if (ret < 0) {
- error_free(s->migration_blocker);
+ if (ret == -EACCES) {
+ error_append_hint(errp, "Cannot use a node with vvfat format "
+ "as it does not support live migration");
+ }
goto fail;
}
}
@@ -1202,7 +1205,6 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
init_mbr(s, cyls, heads, secs);
}
- // assert(is_consistent(s));
qemu_co_mutex_init(&s->lock);
ret = 0;
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 43cb065..3b4fd24 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1020,17 +1020,23 @@ static void coroutine_fn v9fs_attach(void *opaque)
*/
if (!s->migration_blocker) {
int ret;
+ Error *local_err;
error_setg(&s->migration_blocker,
"Migration is disabled when VirtFS export path '%s' is mounted in the guest using mount_tag '%s'",
s->ctx.fs_root ? s->ctx.fs_root : "NULL", s->tag);
- ret = migrate_add_blocker(s->migration_blocker, NULL);
+ ret = migrate_add_blocker(s->migration_blocker, &local_err);
if (ret < 0) {
+ if (ret == -EACCES) {
+ error_append_hint(&local_err, "Failed to mount VirtFS as it "
+ "does not support live migration");
+ }
err = ret;
clunk_fid(s, fid);
error_free(s->migration_blocker);
s->migration_blocker = NULL;
goto out;
}
+ error_free(local_err);
s->root_fid = fid;
}
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 6e60b8c..fad95bf 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1102,6 +1102,7 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
VirtIOGPU *g = VIRTIO_GPU(qdev);
bool have_virgl;
int i;
+ int ret;
if (g->conf.max_outputs > VIRTIO_GPU_MAX_SCANOUTS) {
error_setg(errp, "invalid max_outputs > %d", VIRTIO_GPU_MAX_SCANOUTS);
@@ -1120,8 +1121,12 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
if (virtio_gpu_virgl_enabled(g->conf)) {
error_setg(&g->migration_blocker, "virgl is not yet migratable");
- if (migrate_add_blocker(g->migration_blocker, errp) < 0) {
- error_free(g->migration_blocker);
+ ret = migrate_add_blocker(g->migration_blocker, errp);
+ if (ret < 0) {
+ if (ret == -EACCES) {
+ error_append_hint(errp, "Cannot use virgl as it does not "
+ "support live migration yet");
+ }
return;
}
}
diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index 3614daa..c2ef236 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -515,7 +515,10 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp)
"not support vGICv2 migration");
ret = migrate_add_blocker(s->migration_blocker, errp);
if (ret < 0) {
- error_free(s->migration_blocker);
+ if (ret == -EACCES) {
+ error_append_hint(errp, "Failed to realize vGICv2 as its"
+ " migrataion is not implemented yet");
+ }
return;
}
}
diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
index 950a02f..3474642 100644
--- a/hw/intc/arm_gicv3_its_kvm.c
+++ b/hw/intc/arm_gicv3_its_kvm.c
@@ -70,7 +70,10 @@ static void kvm_arm_its_realize(DeviceState *dev, Error **errp)
error_setg(&s->migration_blocker, "vITS migration is not implemented");
ret = migrate_add_blocker(s->migration_blocker, errp);
if (ret < 0) {
- error_free(s->migration_blocker);
+ if (ret == -EACCES) {
+ error_append_hint(errp, "Failed to realize vITS as its migration "
+ "is not implemented yet");
+ }
return;
}
diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index 06f6f30..be7b97c 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -118,7 +118,10 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
error_setg(&s->migration_blocker, "vGICv3 migration is not implemented");
ret = migrate_add_blocker(s->migration_blocker, errp);
if (ret < 0) {
- error_free(s->migration_blocker);
+ if (ret == -EACCES) {
+ error_append_hint(errp, "Failed to realize vGICv3 as its migration"
+ "is not implemented yet");
+ }
return;
}
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 585cc5b..14ed60d 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -837,6 +837,7 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
{
IVShmemState *s = IVSHMEM_COMMON(dev);
Error *err = NULL;
+ int ret;
uint8_t *pci_conf;
uint8_t attr = PCI_BASE_ADDRESS_SPACE_MEMORY |
PCI_BASE_ADDRESS_MEM_PREFETCH;
@@ -910,8 +911,13 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
if (!ivshmem_is_master(s)) {
error_setg(&s->migration_blocker,
"Migration is disabled when using feature 'peer mode' in device 'ivshmem'");
- if (migrate_add_blocker(s->migration_blocker, errp) < 0) {
- error_free(s->migration_blocker);
+ ret = migrate_add_blocker(s->migration_blocker, errp);
+ if (ret < 0) {
+ if (ret == -EACCES) {
+ error_append_hint(errp, "Cannot use the 'peer mode' feature "
+ "in device 'ivshmem' as it does not "
+ "support live migration");
+ }
return;
}
}
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index ff503d0..bb731b2 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -245,7 +245,11 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
"vhost-scsi does not support migration");
ret = migrate_add_blocker(s->migration_blocker, errp);
if (ret < 0) {
- goto free_blocker;
+ if (ret == -EACCES) {
+ error_append_hint(errp, "Cannot use vhost-scsi as it does not "
+ "support live migration");
+ }
+ goto close_fd;
}
s->dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs->conf.num_queues;
@@ -272,8 +276,6 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
free_vqs:
migrate_del_blocker(s->migration_blocker);
g_free(s->dev.vqs);
- free_blocker:
- error_free(s->migration_blocker);
close_fd:
close(vhostfd);
return;
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 416fada..6d2375a 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1160,8 +1160,10 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
Error *local_err;
r = migrate_add_blocker(hdev->migration_blocker, &local_err);
if (r < 0) {
- error_report_err(local_err);
- error_free(hdev->migration_blocker);
+ if (r == -EACCES) {
+ error_append_hint(&local_err, "Cannot use vhost drivers as "
+ "it does not support live migration");
+ }
goto fail_busyloop;
}
}
diff --git a/migration/migration.c b/migration/migration.c
index 713e012..ab34328 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1115,9 +1115,16 @@ int migrate_add_blocker(Error *reason, Error **errp)
{
if (migration_is_idle(NULL)) {
migration_blockers = g_slist_prepend(migration_blockers, reason);
+ error_free(reason);
return 0;
}
+ if (only_migratable) {
+ error_setg(errp, "Failed to add migration blocker: --only-migratable "
+ "was specified");
+ return -EACCES;
+ }
+
error_setg(errp, "Cannot block a migration already in-progress");
return -EBUSY;
}
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 6031a1c..f2c35d8 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -963,6 +963,10 @@ int kvm_arch_init_vcpu(CPUState *cs)
" (invtsc flag)");
r = migrate_add_blocker(invtsc_mig_blocker, NULL);
if (r < 0) {
+ if (r == -EACCES) {
+ error_report("kvm: non-migratable CPU device but"
+ " --only-migratable was specified");
+ }
error_report("kvm: migrate_add_blocker failed");
goto efail;
}
@@ -1009,7 +1013,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
fail:
migrate_del_blocker(invtsc_mig_blocker);
efail:
- error_free(invtsc_mig_blocker);
return r;
}
--
2.6.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/4] Introduce a new --only-migratable option
2017-01-04 12:32 [Qemu-devel] [PATCH v3 0/4] Introduce a new --only-migratable option Ashijeet Acharya
` (3 preceding siblings ...)
2017-01-04 12:32 ` [Qemu-devel] [PATCH v3 4/4] migration: Fail migration blocker for --only-migratble Ashijeet Acharya
@ 2017-01-04 12:41 ` no-reply
4 siblings, 0 replies; 13+ messages in thread
From: no-reply @ 2017-01-04 12:41 UTC (permalink / raw)
To: ashijeetacharya
Cc: famz, dgilbert, kwolf, peter.maydell, quintela, qemu-devel, mst,
armbru, groug, aneesh.kumar, marcandre.lureau, amit.shah,
pbonzini, jsnow
Hi,
Your series seems to have some coding style problems. See output below for
more information:
Subject: [Qemu-devel] [PATCH v3 0/4] Introduce a new --only-migratable option
Type: series
Message-id: 1483533149-12807-1-git-send-email-ashijeetacharya@gmail.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/1483533149-12807-1-git-send-email-ashijeetacharya@gmail.com -> patchew/1483533149-12807-1-git-send-email-ashijeetacharya@gmail.com
Switched to a new branch 'test'
265fa5a migration: Fail migration blocker for --only-migratble
d98d956 migration: disallow migrate_add_blocker during migration
8581792 migration: Allow "device add" options to only add migratable devices
0c977ab migration: Add a new option to enable only-migratable
=== OUTPUT BEGIN ===
Checking PATCH 1/4: migration: Add a new option to enable only-migratable...
ERROR: do not initialise globals to 0 or NULL
#56: FILE: vl.c:183:
+int only_migratable = 0; /* turn it off unless user states otherwise */
total: 1 errors, 0 warnings, 40 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/4: migration: Allow "device add" options to only add migratable devices...
Checking PATCH 3/4: migration: disallow migrate_add_blocker during migration...
ERROR: do not use C99 // comments
#159: FILE: block/vvfat.c:1205:
+ // assert(is_consistent(s));
total: 1 errors, 0 warnings, 492 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 4/4: migration: Fail migration blocker for --only-migratble...
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v3 3/4] migration: disallow migrate_add_blocker during migration
2017-01-04 12:32 ` [Qemu-devel] [PATCH v3 3/4] migration: disallow migrate_add_blocker during migration Ashijeet Acharya
@ 2017-01-06 16:18 ` Greg Kurz
2017-01-08 7:34 ` [Qemu-devel] [PATCH v2 " Ashijeet Acharya
0 siblings, 1 reply; 13+ messages in thread
From: Greg Kurz @ 2017-01-06 16:18 UTC (permalink / raw)
To: Ashijeet Acharya
Cc: dgilbert, jsnow, amit.shah, pbonzini, kwolf, armbru, quintela,
mst, marcandre.lureau, aneesh.kumar, peter.maydell, qemu-devel
Hi Ashijeet,
I didn't think hard enough while reviewing the changes for hw/9pfs/9p.c... I have
some more remarks, sorry... :-/
On Wed, 4 Jan 2017 18:02:28 +0530
Ashijeet Acharya <ashijeetacharya@gmail.com> wrote:
> If a migration is already in progress and somebody attempts
> to add a migration blocker, this should rightly fail.
>
> Add an errp parameter and a retcode return value to migrate_add_blocker.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
> ---
> block/qcow.c | 6 +++++-
> block/vdi.c | 6 +++++-
> block/vhdx.c | 16 ++++++++++------
> block/vmdk.c | 7 ++++++-
> block/vpc.c | 10 +++++++---
> block/vvfat.c | 20 ++++++++++++--------
> hw/9pfs/9p.c | 20 +++++++++++++++-----
> hw/display/virtio-gpu.c | 29 ++++++++++++++++-------------
> hw/intc/arm_gic_kvm.c | 16 ++++++++++------
> hw/intc/arm_gicv3_its_kvm.c | 18 +++++++++++-------
> hw/intc/arm_gicv3_kvm.c | 19 ++++++++++++-------
> hw/misc/ivshmem.c | 11 +++++++----
> hw/scsi/vhost-scsi.c | 25 +++++++++++++++++++------
> hw/virtio/vhost.c | 8 +++++++-
> include/migration/migration.h | 6 +++++-
> migration/migration.c | 35 +++++++++++++++++++++++++++++++++--
> stubs/migr-blocker.c | 3 ++-
> target-i386/kvm.c | 16 +++++++++++++---
> 18 files changed, 195 insertions(+), 76 deletions(-)
>
> diff --git a/block/qcow.c b/block/qcow.c
> index 7540f43..11526a1 100644
> --- a/block/qcow.c
> +++ b/block/qcow.c
> @@ -252,7 +252,11 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
> error_setg(&s->migration_blocker, "The qcow format used by node '%s' "
> "does not support live migration",
> bdrv_get_device_or_node_name(bs));
> - migrate_add_blocker(s->migration_blocker);
> + ret = migrate_add_blocker(s->migration_blocker, errp);
> + if (ret < 0) {
> + error_free(s->migration_blocker);
> + goto fail;
> + }
>
> qemu_co_mutex_init(&s->lock);
> return 0;
> diff --git a/block/vdi.c b/block/vdi.c
> index 96b78d5..2b56f52 100644
> --- a/block/vdi.c
> +++ b/block/vdi.c
> @@ -471,7 +471,11 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
> error_setg(&s->migration_blocker, "The vdi format used by node '%s' "
> "does not support live migration",
> bdrv_get_device_or_node_name(bs));
> - migrate_add_blocker(s->migration_blocker);
> + ret = migrate_add_blocker(s->migration_blocker, errp);
> + if (ret < 0) {
> + error_free(s->migration_blocker);
> + goto fail_free_bmap;
> + }
>
> qemu_co_mutex_init(&s->write_lock);
>
> diff --git a/block/vhdx.c b/block/vhdx.c
> index 0ba2f0a..d81941b 100644
> --- a/block/vhdx.c
> +++ b/block/vhdx.c
> @@ -991,6 +991,16 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
> }
> }
>
> + /* Disable migration when VHDX images are used */
> + error_setg(&s->migration_blocker, "The vhdx format used by node '%s' "
> + "does not support live migration",
> + bdrv_get_device_or_node_name(bs));
> + ret = migrate_add_blocker(s->migration_blocker, errp);
> + if (ret < 0) {
> + error_free(s->migration_blocker);
> + goto fail;
> + }
> +
> if (flags & BDRV_O_RDWR) {
> ret = vhdx_update_headers(bs, s, false, NULL);
> if (ret < 0) {
> @@ -1000,12 +1010,6 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
>
> /* TODO: differencing files */
>
> - /* Disable migration when VHDX images are used */
> - error_setg(&s->migration_blocker, "The vhdx format used by node '%s' "
> - "does not support live migration",
> - bdrv_get_device_or_node_name(bs));
> - migrate_add_blocker(s->migration_blocker);
> -
> return 0;
> fail:
> vhdx_close(bs);
> diff --git a/block/vmdk.c b/block/vmdk.c
> index a11c27a..4a45a83 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -976,7 +976,12 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
> error_setg(&s->migration_blocker, "The vmdk format used by node '%s' "
> "does not support live migration",
> bdrv_get_device_or_node_name(bs));
> - migrate_add_blocker(s->migration_blocker);
> + ret = migrate_add_blocker(s->migration_blocker, errp);
> + if (ret < 0) {
> + error_free(s->migration_blocker);
> + goto fail;
> + }
> +
> g_free(buf);
> return 0;
>
> diff --git a/block/vpc.c b/block/vpc.c
> index 8d5886f..299a8c8 100644
> --- a/block/vpc.c
> +++ b/block/vpc.c
> @@ -422,13 +422,17 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
> #endif
> }
>
> - qemu_co_mutex_init(&s->lock);
> -
> /* Disable migration when VHD images are used */
> error_setg(&s->migration_blocker, "The vpc format used by node '%s' "
> "does not support live migration",
> bdrv_get_device_or_node_name(bs));
> - migrate_add_blocker(s->migration_blocker);
> + ret = migrate_add_blocker(s->migration_blocker, errp);
> + if (ret < 0) {
> + error_free(s->migration_blocker);
> + goto fail;
> + }
> +
> + qemu_co_mutex_init(&s->lock);
>
> return 0;
>
> diff --git a/block/vvfat.c b/block/vvfat.c
> index ded2109..3de3320 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -1185,22 +1185,26 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
>
> s->sector_count = s->faked_sectors + s->sectors_per_cluster*s->cluster_count;
>
> - if (s->first_sectors_number == 0x40) {
> - init_mbr(s, cyls, heads, secs);
> - }
> -
> - // assert(is_consistent(s));
> - qemu_co_mutex_init(&s->lock);
> -
> /* Disable migration when vvfat is used rw */
> if (s->qcow) {
> error_setg(&s->migration_blocker,
> "The vvfat (rw) format used by node '%s' "
> "does not support live migration",
> bdrv_get_device_or_node_name(bs));
> - migrate_add_blocker(s->migration_blocker);
> + ret = migrate_add_blocker(s->migration_blocker, errp);
> + if (ret < 0) {
> + error_free(s->migration_blocker);
> + goto fail;
> + }
> + }
> +
> + if (s->first_sectors_number == 0x40) {
> + init_mbr(s, cyls, heads, secs);
> }
>
> + // assert(is_consistent(s));
> + qemu_co_mutex_init(&s->lock);
> +
> ret = 0;
> fail:
> qemu_opts_del(opts);
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index faebd91..43cb065 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -1013,20 +1013,30 @@ static void coroutine_fn v9fs_attach(void *opaque)
> goto out;
> }
> err += offset;
> - memcpy(&s->root_qid, &qid, sizeof(qid));
> - trace_v9fs_attach_return(pdu->tag, pdu->id,
> - qid.type, qid.version, qid.path);
> +
> /*
> * disable migration if we haven't done already.
> * attach could get called multiple times for the same export.
> */
> if (!s->migration_blocker) {
> - s->root_fid = fid;
> + int ret;
> error_setg(&s->migration_blocker,
> "Migration is disabled when VirtFS export path '%s' is mounted in the guest using mount_tag '%s'",
> s->ctx.fs_root ? s->ctx.fs_root : "NULL", s->tag);
> - migrate_add_blocker(s->migration_blocker);
> + ret = migrate_add_blocker(s->migration_blocker, NULL);
> + if (ret < 0) {
> + err = ret;
> + clunk_fid(s, fid);
> + error_free(s->migration_blocker);
> + s->migration_blocker = NULL;
It's better to rollback things in reverse order (i.e. clunk_fid() should
be called here).
> + goto out;
> + }
> + s->root_fid = fid;
> }
I now realize that all the migration blocker stuff should be done before
we call pdu_marshal() since we may now return an error to the guest. And
thus, you can drop ret and use err directly.
Cheers.
--
Greg
> +
> + memcpy(&s->root_qid, &qid, sizeof(qid));
> + trace_v9fs_attach_return(pdu->tag, pdu->id,
> + qid.type, qid.version, qid.path);
> out:
> put_fid(pdu, fidp);
> out_nofid:
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 5f32e1a..6e60b8c 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -1108,14 +1108,6 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
> return;
> }
>
> - g->config_size = sizeof(struct virtio_gpu_config);
> - g->virtio_config.num_scanouts = g->conf.max_outputs;
> - virtio_init(VIRTIO_DEVICE(g), "virtio-gpu", VIRTIO_ID_GPU,
> - g->config_size);
> -
> - g->req_state[0].width = 1024;
> - g->req_state[0].height = 768;
> -
> g->use_virgl_renderer = false;
> #if !defined(CONFIG_VIRGL) || defined(HOST_WORDS_BIGENDIAN)
> have_virgl = false;
> @@ -1127,6 +1119,22 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
> }
>
> if (virtio_gpu_virgl_enabled(g->conf)) {
> + error_setg(&g->migration_blocker, "virgl is not yet migratable");
> + if (migrate_add_blocker(g->migration_blocker, errp) < 0) {
> + error_free(g->migration_blocker);
> + return;
> + }
> + }
> +
> + g->config_size = sizeof(struct virtio_gpu_config);
> + g->virtio_config.num_scanouts = g->conf.max_outputs;
> + virtio_init(VIRTIO_DEVICE(g), "virtio-gpu", VIRTIO_ID_GPU,
> + g->config_size);
> +
> + g->req_state[0].width = 1024;
> + g->req_state[0].height = 768;
> +
> + if (virtio_gpu_virgl_enabled(g->conf)) {
> /* use larger control queue in 3d mode */
> g->ctrl_vq = virtio_add_queue(vdev, 256, virtio_gpu_handle_ctrl_cb);
> g->cursor_vq = virtio_add_queue(vdev, 16, virtio_gpu_handle_cursor_cb);
> @@ -1152,11 +1160,6 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
> dpy_gfx_replace_surface(g->scanout[i].con, NULL);
> }
> }
> -
> - if (virtio_gpu_virgl_enabled(g->conf)) {
> - error_setg(&g->migration_blocker, "virgl is not yet migratable");
> - migrate_add_blocker(g->migration_blocker);
> - }
> }
>
> static void virtio_gpu_device_unrealize(DeviceState *qdev, Error **errp)
> diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
> index 11729ee..3614daa 100644
> --- a/hw/intc/arm_gic_kvm.c
> +++ b/hw/intc/arm_gic_kvm.c
> @@ -510,6 +510,16 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp)
> return;
> }
>
> + if (!kvm_arm_gic_can_save_restore(s)) {
> + error_setg(&s->migration_blocker, "This operating system kernel does "
> + "not support vGICv2 migration");
> + ret = migrate_add_blocker(s->migration_blocker, errp);
> + if (ret < 0) {
> + error_free(s->migration_blocker);
> + return;
> + }
> + }
> +
> gic_init_irqs_and_mmio(s, kvm_arm_gicv2_set_irq, NULL);
>
> for (i = 0; i < s->num_irq - GIC_INTERNAL; i++) {
> @@ -558,12 +568,6 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp)
> KVM_VGIC_V2_ADDR_TYPE_CPU,
> s->dev_fd);
>
> - if (!kvm_arm_gic_can_save_restore(s)) {
> - error_setg(&s->migration_blocker, "This operating system kernel does "
> - "not support vGICv2 migration");
> - migrate_add_blocker(s->migration_blocker);
> - }
> -
> if (kvm_has_gsi_routing()) {
> /* set up irq routing */
> kvm_init_irq_routing(kvm_state);
> diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
> index fc246e0..950a02f 100644
> --- a/hw/intc/arm_gicv3_its_kvm.c
> +++ b/hw/intc/arm_gicv3_its_kvm.c
> @@ -63,6 +63,17 @@ static void kvm_arm_its_realize(DeviceState *dev, Error **errp)
> return;
> }
>
> + /*
> + * Block migration of a KVM GICv3 ITS device: the API for saving and
> + * restoring the state in the kernel is not yet available
> + */
> + error_setg(&s->migration_blocker, "vITS migration is not implemented");
> + ret = migrate_add_blocker(s->migration_blocker, errp);
> + if (ret < 0) {
> + error_free(s->migration_blocker);
> + return;
> + }
> +
> /* explicit init of the ITS */
> kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
> KVM_DEV_ARM_VGIC_CTRL_INIT, NULL, true);
> @@ -73,13 +84,6 @@ static void kvm_arm_its_realize(DeviceState *dev, Error **errp)
>
> gicv3_its_init_mmio(s, NULL);
>
> - /*
> - * Block migration of a KVM GICv3 ITS device: the API for saving and
> - * restoring the state in the kernel is not yet available
> - */
> - error_setg(&s->migration_blocker, "vITS migration is not implemented");
> - migrate_add_blocker(s->migration_blocker);
> -
> kvm_msi_use_devid = true;
> kvm_gsi_direct_mapping = false;
> kvm_msi_via_irqfd_allowed = kvm_irqfds_enabled();
> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
> index 199a439..06f6f30 100644
> --- a/hw/intc/arm_gicv3_kvm.c
> +++ b/hw/intc/arm_gicv3_kvm.c
> @@ -86,6 +86,7 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
> KVMARMGICv3Class *kgc = KVM_ARM_GICV3_GET_CLASS(s);
> Error *local_err = NULL;
> int i;
> + int ret;
>
> DPRINTF("kvm_arm_gicv3_realize\n");
>
> @@ -110,6 +111,17 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
> return;
> }
>
> + /* Block migration of a KVM GICv3 device: the API for saving and restoring
> + * the state in the kernel is not yet finalised in the kernel or
> + * implemented in QEMU.
> + */
> + error_setg(&s->migration_blocker, "vGICv3 migration is not implemented");
> + ret = migrate_add_blocker(s->migration_blocker, errp);
> + if (ret < 0) {
> + error_free(s->migration_blocker);
> + return;
> + }
> +
> kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_NR_IRQS,
> 0, &s->num_irq, true);
>
> @@ -122,13 +134,6 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
> kvm_arm_register_device(&s->iomem_redist, -1, KVM_DEV_ARM_VGIC_GRP_ADDR,
> KVM_VGIC_V3_ADDR_TYPE_REDIST, s->dev_fd);
>
> - /* Block migration of a KVM GICv3 device: the API for saving and restoring
> - * the state in the kernel is not yet finalised in the kernel or
> - * implemented in QEMU.
> - */
> - error_setg(&s->migration_blocker, "vGICv3 migration is not implemented");
> - migrate_add_blocker(s->migration_blocker);
> -
> if (kvm_has_gsi_routing()) {
> /* set up irq routing */
> kvm_init_irq_routing(kvm_state);
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index abeaf3d..585cc5b 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -903,9 +903,6 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
> }
> }
>
> - vmstate_register_ram(s->ivshmem_bar2, DEVICE(s));
> - pci_register_bar(PCI_DEVICE(s), 2, attr, s->ivshmem_bar2);
> -
> if (s->master == ON_OFF_AUTO_AUTO) {
> s->master = s->vm_id == 0 ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
> }
> @@ -913,8 +910,14 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
> if (!ivshmem_is_master(s)) {
> error_setg(&s->migration_blocker,
> "Migration is disabled when using feature 'peer mode' in device 'ivshmem'");
> - migrate_add_blocker(s->migration_blocker);
> + if (migrate_add_blocker(s->migration_blocker, errp) < 0) {
> + error_free(s->migration_blocker);
> + return;
> + }
> }
> +
> + vmstate_register_ram(s->ivshmem_bar2, DEVICE(s));
> + pci_register_bar(PCI_DEVICE(s), 2, attr, s->ivshmem_bar2);
> }
>
> static void ivshmem_exit(PCIDevice *dev)
> diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
> index 5b26946..ff503d0 100644
> --- a/hw/scsi/vhost-scsi.c
> +++ b/hw/scsi/vhost-scsi.c
> @@ -238,8 +238,14 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
> vhost_dummy_handle_output);
> if (err != NULL) {
> error_propagate(errp, err);
> - close(vhostfd);
> - return;
> + goto close_fd;
> + }
> +
> + error_setg(&s->migration_blocker,
> + "vhost-scsi does not support migration");
> + ret = migrate_add_blocker(s->migration_blocker, errp);
> + if (ret < 0) {
> + goto free_blocker;
> }
>
> s->dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs->conf.num_queues;
> @@ -252,7 +258,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
> if (ret < 0) {
> error_setg(errp, "vhost-scsi: vhost initialization failed: %s",
> strerror(-ret));
> - return;
> + goto free_vqs;
> }
>
> /* At present, channel and lun both are 0 for bootable vhost-scsi disk */
> @@ -261,9 +267,16 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
> /* Note: we can also get the minimum tpgt from kernel */
> s->target = vs->conf.boot_tpgt;
>
> - error_setg(&s->migration_blocker,
> - "vhost-scsi does not support migration");
> - migrate_add_blocker(s->migration_blocker);
> + return;
> +
> + free_vqs:
> + migrate_del_blocker(s->migration_blocker);
> + g_free(s->dev.vqs);
> + free_blocker:
> + error_free(s->migration_blocker);
> + close_fd:
> + close(vhostfd);
> + return;
> }
>
> static void vhost_scsi_unrealize(DeviceState *dev, Error **errp)
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index f7f7023..416fada 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1157,7 +1157,13 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> }
>
> if (hdev->migration_blocker != NULL) {
> - migrate_add_blocker(hdev->migration_blocker);
> + Error *local_err;
> + r = migrate_add_blocker(hdev->migration_blocker, &local_err);
> + if (r < 0) {
> + error_report_err(local_err);
> + error_free(hdev->migration_blocker);
> + goto fail_busyloop;
> + }
> }
>
> hdev->mem = g_malloc0(offsetof(struct vhost_memory, regions));
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index 40b3697..46a4bb5 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -243,6 +243,7 @@ void remove_migration_state_change_notifier(Notifier *notify);
> MigrationState *migrate_init(const MigrationParams *params);
> bool migration_is_blocked(Error **errp);
> bool migration_in_setup(MigrationState *);
> +bool migration_is_idle(MigrationState *s);
> bool migration_has_finished(MigrationState *);
> bool migration_has_failed(MigrationState *);
> /* True if outgoing migration has entered postcopy phase */
> @@ -287,8 +288,11 @@ int ram_postcopy_incoming_init(MigrationIncomingState *mis);
> * @migrate_add_blocker - prevent migration from proceeding
> *
> * @reason - an error to be returned whenever migration is attempted
> + * @errp - [out] The reason (if any) we cannot block migration right now.
> + *
> + * @returns - 0 on success, -EBUSY on failure, with errp set.
> */
> -void migrate_add_blocker(Error *reason);
> +int migrate_add_blocker(Error *reason, Error **errp);
>
> /**
> * @migrate_del_blocker - remove a blocking error from migration
> diff --git a/migration/migration.c b/migration/migration.c
> index f498ab8..713e012 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1044,6 +1044,31 @@ bool migration_in_postcopy_after_devices(MigrationState *s)
> return migration_in_postcopy(s) && s->postcopy_after_devices;
> }
>
> +bool migration_is_idle(MigrationState *s)
> +{
> + if (!s) {
> + s = migrate_get_current();
> + }
> +
> + switch (s->state) {
> + case MIGRATION_STATUS_NONE:
> + case MIGRATION_STATUS_CANCELLED:
> + case MIGRATION_STATUS_COMPLETED:
> + case MIGRATION_STATUS_FAILED:
> + return true;
> + case MIGRATION_STATUS_SETUP:
> + case MIGRATION_STATUS_CANCELLING:
> + case MIGRATION_STATUS_ACTIVE:
> + case MIGRATION_STATUS_POSTCOPY_ACTIVE:
> + case MIGRATION_STATUS_COLO:
> + return false;
> + case MIGRATION_STATUS__MAX:
> + g_assert_not_reached();
> + }
> +
> + return false;
> +}
> +
> MigrationState *migrate_init(const MigrationParams *params)
> {
> MigrationState *s = migrate_get_current();
> @@ -1086,9 +1111,15 @@ MigrationState *migrate_init(const MigrationParams *params)
>
> static GSList *migration_blockers;
>
> -void migrate_add_blocker(Error *reason)
> +int migrate_add_blocker(Error *reason, Error **errp)
> {
> - migration_blockers = g_slist_prepend(migration_blockers, reason);
> + if (migration_is_idle(NULL)) {
> + migration_blockers = g_slist_prepend(migration_blockers, reason);
> + return 0;
> + }
> +
> + error_setg(errp, "Cannot block a migration already in-progress");
> + return -EBUSY;
> }
>
> void migrate_del_blocker(Error *reason)
> diff --git a/stubs/migr-blocker.c b/stubs/migr-blocker.c
> index 8ab3604..a5ba18f 100644
> --- a/stubs/migr-blocker.c
> +++ b/stubs/migr-blocker.c
> @@ -2,8 +2,9 @@
> #include "qemu-common.h"
> #include "migration/migration.h"
>
> -void migrate_add_blocker(Error *reason)
> +int migrate_add_blocker(Error *reason, Error **errp)
> {
> + return 0;
> }
>
> void migrate_del_blocker(Error *reason)
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index f62264a..6031a1c 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -961,7 +961,11 @@ int kvm_arch_init_vcpu(CPUState *cs)
> error_setg(&invtsc_mig_blocker,
> "State blocked by non-migratable CPU device"
> " (invtsc flag)");
> - migrate_add_blocker(invtsc_mig_blocker);
> + r = migrate_add_blocker(invtsc_mig_blocker, NULL);
> + if (r < 0) {
> + error_report("kvm: migrate_add_blocker failed");
> + goto efail;
> + }
> /* for savevm */
> vmstate_x86_cpu.unmigratable = 1;
> }
> @@ -969,12 +973,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
> cpuid_data.cpuid.padding = 0;
> r = kvm_vcpu_ioctl(cs, KVM_SET_CPUID2, &cpuid_data);
> if (r) {
> - return r;
> + goto fail;
> }
>
> r = kvm_arch_set_tsc_khz(cs);
> if (r < 0) {
> - return r;
> + goto fail;
> }
>
> /* vcpu's TSC frequency is either specified by user, or following
> @@ -1001,6 +1005,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
> }
>
> return 0;
> +
> + fail:
> + migrate_del_blocker(invtsc_mig_blocker);
> + efail:
> + error_free(invtsc_mig_blocker);
> + return r;
> }
>
> void kvm_arch_reset_vcpu(X86CPU *cpu)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v3 4/4] migration: Fail migration blocker for --only-migratble
2017-01-04 12:32 ` [Qemu-devel] [PATCH v3 4/4] migration: Fail migration blocker for --only-migratble Ashijeet Acharya
@ 2017-01-06 16:40 ` Greg Kurz
2017-01-08 7:25 ` [Qemu-devel] [PATCH v2 " Ashijeet Acharya
0 siblings, 1 reply; 13+ messages in thread
From: Greg Kurz @ 2017-01-06 16:40 UTC (permalink / raw)
To: Ashijeet Acharya
Cc: dgilbert, jsnow, amit.shah, pbonzini, kwolf, armbru, quintela,
mst, marcandre.lureau, aneesh.kumar, peter.maydell, qemu-devel
On Wed, 4 Jan 2017 18:02:29 +0530
Ashijeet Acharya <ashijeetacharya@gmail.com> wrote:
> migrate_add_blocker should rightly fail if the '--only-migratable'
> option was specified and the device in use should not be able to
> perform the action which results in an unmigratable VM.
>
> Make migrate_add_blocker return -EACCES in this case.
>
> Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
> ---
> block/qcow.c | 5 ++++-
> block/vdi.c | 5 ++++-
> block/vhdx.c | 6 ++++--
> block/vmdk.c | 5 ++++-
> block/vpc.c | 5 ++++-
> block/vvfat.c | 6 ++++--
> hw/9pfs/9p.c | 8 +++++++-
> hw/display/virtio-gpu.c | 9 +++++++--
> hw/intc/arm_gic_kvm.c | 5 ++++-
> hw/intc/arm_gicv3_its_kvm.c | 5 ++++-
> hw/intc/arm_gicv3_kvm.c | 5 ++++-
> hw/misc/ivshmem.c | 10 ++++++++--
> hw/scsi/vhost-scsi.c | 8 +++++---
> hw/virtio/vhost.c | 6 ++++--
> migration/migration.c | 7 +++++++
> target-i386/kvm.c | 5 ++++-
> 16 files changed, 78 insertions(+), 22 deletions(-)
>
> diff --git a/block/qcow.c b/block/qcow.c
> index 11526a1..bdc6446 100644
> --- a/block/qcow.c
> +++ b/block/qcow.c
> @@ -254,7 +254,10 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
> bdrv_get_device_or_node_name(bs));
> ret = migrate_add_blocker(s->migration_blocker, errp);
> if (ret < 0) {
> - error_free(s->migration_blocker);
> + if (ret == -EACCES) {
> + error_append_hint(errp, "Cannot use a node with qcow format as "
> + "it does not support live migration");
> + }
> goto fail;
> }
>
> diff --git a/block/vdi.c b/block/vdi.c
> index 2b56f52..0b011ea 100644
> --- a/block/vdi.c
> +++ b/block/vdi.c
> @@ -473,7 +473,10 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
> bdrv_get_device_or_node_name(bs));
> ret = migrate_add_blocker(s->migration_blocker, errp);
> if (ret < 0) {
> - error_free(s->migration_blocker);
> + if (ret == -EACCES) {
> + error_append_hint(errp, "Cannot use a node with vdi format as "
> + "it does not support live migration");
> + }
> goto fail_free_bmap;
> }
>
> diff --git a/block/vhdx.c b/block/vhdx.c
> index d81941b..b8d53ec 100644
> --- a/block/vhdx.c
> +++ b/block/vhdx.c
> @@ -997,10 +997,12 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
> bdrv_get_device_or_node_name(bs));
> ret = migrate_add_blocker(s->migration_blocker, errp);
> if (ret < 0) {
> - error_free(s->migration_blocker);
> + if (ret == -EACCES) {
> + error_append_hint(errp, "Cannot use a node with vhdx format as "
> + "it does not support live migration");
> + }
> goto fail;
> }
> -
> if (flags & BDRV_O_RDWR) {
> ret = vhdx_update_headers(bs, s, false, NULL);
> if (ret < 0) {
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 4a45a83..defe400 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -978,7 +978,10 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
> bdrv_get_device_or_node_name(bs));
> ret = migrate_add_blocker(s->migration_blocker, errp);
> if (ret < 0) {
> - error_free(s->migration_blocker);
> + if (ret == -EACCES) {
> + error_append_hint(errp, "Cannot use a node with vmdk format as "
> + "it does not support live migration");
> + }
> goto fail;
> }
>
> diff --git a/block/vpc.c b/block/vpc.c
> index 299a8c8..5e58d77 100644
> --- a/block/vpc.c
> +++ b/block/vpc.c
> @@ -428,7 +428,10 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
> bdrv_get_device_or_node_name(bs));
> ret = migrate_add_blocker(s->migration_blocker, errp);
> if (ret < 0) {
> - error_free(s->migration_blocker);
> + if (ret == -EACCES) {
> + error_append_hint(errp, "Cannot use a node with vpc format as "
> + "it does not support live migration");
> + }
> goto fail;
> }
>
> diff --git a/block/vvfat.c b/block/vvfat.c
> index 3de3320..5a6e038 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -1193,7 +1193,10 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
> bdrv_get_device_or_node_name(bs));
> ret = migrate_add_blocker(s->migration_blocker, errp);
> if (ret < 0) {
> - error_free(s->migration_blocker);
> + if (ret == -EACCES) {
> + error_append_hint(errp, "Cannot use a node with vvfat format "
> + "as it does not support live migration");
> + }
> goto fail;
> }
> }
> @@ -1202,7 +1205,6 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
> init_mbr(s, cyls, heads, secs);
> }
>
> - // assert(is_consistent(s));
> qemu_co_mutex_init(&s->lock);
>
> ret = 0;
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 43cb065..3b4fd24 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -1020,17 +1020,23 @@ static void coroutine_fn v9fs_attach(void *opaque)
> */
> if (!s->migration_blocker) {
> int ret;
> + Error *local_err;
> error_setg(&s->migration_blocker,
> "Migration is disabled when VirtFS export path '%s' is mounted in the guest using mount_tag '%s'",
> s->ctx.fs_root ? s->ctx.fs_root : "NULL", s->tag);
> - ret = migrate_add_blocker(s->migration_blocker, NULL);
> + ret = migrate_add_blocker(s->migration_blocker, &local_err);
> if (ret < 0) {
> + if (ret == -EACCES) {
> + error_append_hint(&local_err, "Failed to mount VirtFS as it "
> + "does not support live migration");
> + }
> err = ret;
> clunk_fid(s, fid);
> error_free(s->migration_blocker);
> s->migration_blocker = NULL;
> goto out;
> }
> + error_free(local_err);
Thinking again, I suggest you simply drop these changes: v9fs_attach() is
triggered by the guest and doesn't cause QEMU to fail, so we don't really
care.
Cheers.
--
Greg
> s->root_fid = fid;
> }
>
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 6e60b8c..fad95bf 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -1102,6 +1102,7 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
> VirtIOGPU *g = VIRTIO_GPU(qdev);
> bool have_virgl;
> int i;
> + int ret;
>
> if (g->conf.max_outputs > VIRTIO_GPU_MAX_SCANOUTS) {
> error_setg(errp, "invalid max_outputs > %d", VIRTIO_GPU_MAX_SCANOUTS);
> @@ -1120,8 +1121,12 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
>
> if (virtio_gpu_virgl_enabled(g->conf)) {
> error_setg(&g->migration_blocker, "virgl is not yet migratable");
> - if (migrate_add_blocker(g->migration_blocker, errp) < 0) {
> - error_free(g->migration_blocker);
> + ret = migrate_add_blocker(g->migration_blocker, errp);
> + if (ret < 0) {
> + if (ret == -EACCES) {
> + error_append_hint(errp, "Cannot use virgl as it does not "
> + "support live migration yet");
> + }
> return;
> }
> }
> diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
> index 3614daa..c2ef236 100644
> --- a/hw/intc/arm_gic_kvm.c
> +++ b/hw/intc/arm_gic_kvm.c
> @@ -515,7 +515,10 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp)
> "not support vGICv2 migration");
> ret = migrate_add_blocker(s->migration_blocker, errp);
> if (ret < 0) {
> - error_free(s->migration_blocker);
> + if (ret == -EACCES) {
> + error_append_hint(errp, "Failed to realize vGICv2 as its"
> + " migrataion is not implemented yet");
> + }
> return;
> }
> }
> diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
> index 950a02f..3474642 100644
> --- a/hw/intc/arm_gicv3_its_kvm.c
> +++ b/hw/intc/arm_gicv3_its_kvm.c
> @@ -70,7 +70,10 @@ static void kvm_arm_its_realize(DeviceState *dev, Error **errp)
> error_setg(&s->migration_blocker, "vITS migration is not implemented");
> ret = migrate_add_blocker(s->migration_blocker, errp);
> if (ret < 0) {
> - error_free(s->migration_blocker);
> + if (ret == -EACCES) {
> + error_append_hint(errp, "Failed to realize vITS as its migration "
> + "is not implemented yet");
> + }
> return;
> }
>
> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
> index 06f6f30..be7b97c 100644
> --- a/hw/intc/arm_gicv3_kvm.c
> +++ b/hw/intc/arm_gicv3_kvm.c
> @@ -118,7 +118,10 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
> error_setg(&s->migration_blocker, "vGICv3 migration is not implemented");
> ret = migrate_add_blocker(s->migration_blocker, errp);
> if (ret < 0) {
> - error_free(s->migration_blocker);
> + if (ret == -EACCES) {
> + error_append_hint(errp, "Failed to realize vGICv3 as its migration"
> + "is not implemented yet");
> + }
> return;
> }
>
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index 585cc5b..14ed60d 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -837,6 +837,7 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
> {
> IVShmemState *s = IVSHMEM_COMMON(dev);
> Error *err = NULL;
> + int ret;
> uint8_t *pci_conf;
> uint8_t attr = PCI_BASE_ADDRESS_SPACE_MEMORY |
> PCI_BASE_ADDRESS_MEM_PREFETCH;
> @@ -910,8 +911,13 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
> if (!ivshmem_is_master(s)) {
> error_setg(&s->migration_blocker,
> "Migration is disabled when using feature 'peer mode' in device 'ivshmem'");
> - if (migrate_add_blocker(s->migration_blocker, errp) < 0) {
> - error_free(s->migration_blocker);
> + ret = migrate_add_blocker(s->migration_blocker, errp);
> + if (ret < 0) {
> + if (ret == -EACCES) {
> + error_append_hint(errp, "Cannot use the 'peer mode' feature "
> + "in device 'ivshmem' as it does not "
> + "support live migration");
> + }
> return;
> }
> }
> diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
> index ff503d0..bb731b2 100644
> --- a/hw/scsi/vhost-scsi.c
> +++ b/hw/scsi/vhost-scsi.c
> @@ -245,7 +245,11 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
> "vhost-scsi does not support migration");
> ret = migrate_add_blocker(s->migration_blocker, errp);
> if (ret < 0) {
> - goto free_blocker;
> + if (ret == -EACCES) {
> + error_append_hint(errp, "Cannot use vhost-scsi as it does not "
> + "support live migration");
> + }
> + goto close_fd;
> }
>
> s->dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs->conf.num_queues;
> @@ -272,8 +276,6 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
> free_vqs:
> migrate_del_blocker(s->migration_blocker);
> g_free(s->dev.vqs);
> - free_blocker:
> - error_free(s->migration_blocker);
> close_fd:
> close(vhostfd);
> return;
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 416fada..6d2375a 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1160,8 +1160,10 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> Error *local_err;
> r = migrate_add_blocker(hdev->migration_blocker, &local_err);
> if (r < 0) {
> - error_report_err(local_err);
> - error_free(hdev->migration_blocker);
> + if (r == -EACCES) {
> + error_append_hint(&local_err, "Cannot use vhost drivers as "
> + "it does not support live migration");
> + }
> goto fail_busyloop;
> }
> }
> diff --git a/migration/migration.c b/migration/migration.c
> index 713e012..ab34328 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1115,9 +1115,16 @@ int migrate_add_blocker(Error *reason, Error **errp)
> {
> if (migration_is_idle(NULL)) {
> migration_blockers = g_slist_prepend(migration_blockers, reason);
> + error_free(reason);
> return 0;
> }
>
> + if (only_migratable) {
> + error_setg(errp, "Failed to add migration blocker: --only-migratable "
> + "was specified");
> + return -EACCES;
> + }
> +
> error_setg(errp, "Cannot block a migration already in-progress");
> return -EBUSY;
> }
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 6031a1c..f2c35d8 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -963,6 +963,10 @@ int kvm_arch_init_vcpu(CPUState *cs)
> " (invtsc flag)");
> r = migrate_add_blocker(invtsc_mig_blocker, NULL);
> if (r < 0) {
> + if (r == -EACCES) {
> + error_report("kvm: non-migratable CPU device but"
> + " --only-migratable was specified");
> + }
> error_report("kvm: migrate_add_blocker failed");
> goto efail;
> }
> @@ -1009,7 +1013,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
> fail:
> migrate_del_blocker(invtsc_mig_blocker);
> efail:
> - error_free(invtsc_mig_blocker);
> return r;
> }
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 4/4] migration: Fail migration blocker for --only-migratble
2017-01-06 16:40 ` Greg Kurz
@ 2017-01-08 7:25 ` Ashijeet Acharya
2017-01-09 8:09 ` Greg Kurz
0 siblings, 1 reply; 13+ messages in thread
From: Ashijeet Acharya @ 2017-01-08 7:25 UTC (permalink / raw)
To: Greg Kurz
Cc: dgilbert, jsnow, amit.shah, pbonzini, kwolf, armbru, quintela,
mst, marcandre.lureau, aneesh.kumar, peter.maydell, qemu-devel
On Friday, January 6, 2017, Greg Kurz <groug@kaod.org
<javascript:_e(%7B%7D,'cvml','groug@kaod.org');>> wrote:
> On Wed, 4 Jan 2017 18:02:29 +0530
> Ashijeet Acharya <ashijeetacharya@gmail.com> wrote:
>
> > migrate_add_blocker should rightly fail if the '--only-migratable'
> > option was specified and the device in use should not be able to
> > perform the action which results in an unmigratable VM.
> >
> > Make migrate_add_blocker return -EACCES in this case.
> >
> > Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
> > ---
> > block/qcow.c | 5 ++++-
> > block/vdi.c | 5 ++++-
> > block/vhdx.c | 6 ++++--
> > block/vmdk.c | 5 ++++-
> > block/vpc.c | 5 ++++-
> > block/vvfat.c | 6 ++++--
> > hw/9pfs/9p.c | 8 +++++++-
> > hw/display/virtio-gpu.c | 9 +++++++--
> > hw/intc/arm_gic_kvm.c | 5 ++++-
> > hw/intc/arm_gicv3_its_kvm.c | 5 ++++-
> > hw/intc/arm_gicv3_kvm.c | 5 ++++-
> > hw/misc/ivshmem.c | 10 ++++++++--
> > hw/scsi/vhost-scsi.c | 8 +++++---
> > hw/virtio/vhost.c | 6 ++++--
> > migration/migration.c | 7 +++++++
> > target-i386/kvm.c | 5 ++++-
> > 16 files changed, 78 insertions(+), 22 deletions(-)
> >
> > diff --git a/block/qcow.c b/block/qcow.c
> > index 11526a1..bdc6446 100644
> > --- a/block/qcow.c
> > +++ b/block/qcow.c
> > @@ -254,7 +254,10 @@ static int qcow_open(BlockDriverState *bs, QDict
> *options, int flags,
> > bdrv_get_device_or_node_name(bs));
> > ret = migrate_add_blocker(s->migration_blocker, errp);
> > if (ret < 0) {
> > - error_free(s->migration_blocker);
> > + if (ret == -EACCES) {
> > + error_append_hint(errp, "Cannot use a node with qcow format
> as "
> > + "it does not support live migration");
> > + }
> > goto fail;
> > }
> >
> > diff --git a/block/vdi.c b/block/vdi.c
> > index 2b56f52..0b011ea 100644
> > --- a/block/vdi.c
> > +++ b/block/vdi.c
> > @@ -473,7 +473,10 @@ static int vdi_open(BlockDriverState *bs, QDict
> *options, int flags,
> > bdrv_get_device_or_node_name(bs));
> > ret = migrate_add_blocker(s->migration_blocker, errp);
> > if (ret < 0) {
> > - error_free(s->migration_blocker);
> > + if (ret == -EACCES) {
> > + error_append_hint(errp, "Cannot use a node with vdi format
> as "
> > + "it does not support live migration");
> > + }
> > goto fail_free_bmap;
> > }
> >
> > diff --git a/block/vhdx.c b/block/vhdx.c
> > index d81941b..b8d53ec 100644
> > --- a/block/vhdx.c
> > +++ b/block/vhdx.c
> > @@ -997,10 +997,12 @@ static int vhdx_open(BlockDriverState *bs, QDict
> *options, int flags,
> > bdrv_get_device_or_node_name(bs));
> > ret = migrate_add_blocker(s->migration_blocker, errp);
> > if (ret < 0) {
> > - error_free(s->migration_blocker);
> > + if (ret == -EACCES) {
> > + error_append_hint(errp, "Cannot use a node with vhdx format
> as "
> > + "it does not support live migration");
> > + }
> > goto fail;
> > }
> > -
> > if (flags & BDRV_O_RDWR) {
> > ret = vhdx_update_headers(bs, s, false, NULL);
> > if (ret < 0) {
> > diff --git a/block/vmdk.c b/block/vmdk.c
> > index 4a45a83..defe400 100644
> > --- a/block/vmdk.c
> > +++ b/block/vmdk.c
> > @@ -978,7 +978,10 @@ static int vmdk_open(BlockDriverState *bs, QDict
> *options, int flags,
> > bdrv_get_device_or_node_name(bs));
> > ret = migrate_add_blocker(s->migration_blocker, errp);
> > if (ret < 0) {
> > - error_free(s->migration_blocker);
> > + if (ret == -EACCES) {
> > + error_append_hint(errp, "Cannot use a node with vmdk format
> as "
> > + "it does not support live migration");
> > + }
> > goto fail;
> > }
> >
> > diff --git a/block/vpc.c b/block/vpc.c
> > index 299a8c8..5e58d77 100644
> > --- a/block/vpc.c
> > +++ b/block/vpc.c
> > @@ -428,7 +428,10 @@ static int vpc_open(BlockDriverState *bs, QDict
> *options, int flags,
> > bdrv_get_device_or_node_name(bs));
> > ret = migrate_add_blocker(s->migration_blocker, errp);
> > if (ret < 0) {
> > - error_free(s->migration_blocker);
> > + if (ret == -EACCES) {
> > + error_append_hint(errp, "Cannot use a node with vpc format
> as "
> > + "it does not support live migration");
> > + }
> > goto fail;
> > }
> >
> > diff --git a/block/vvfat.c b/block/vvfat.c
> > index 3de3320..5a6e038 100644
> > --- a/block/vvfat.c
> > +++ b/block/vvfat.c
> > @@ -1193,7 +1193,10 @@ static int vvfat_open(BlockDriverState *bs, QDict
> *options, int flags,
> > bdrv_get_device_or_node_name(bs));
> > ret = migrate_add_blocker(s->migration_blocker, errp);
> > if (ret < 0) {
> > - error_free(s->migration_blocker);
> > + if (ret == -EACCES) {
> > + error_append_hint(errp, "Cannot use a node with vvfat
> format "
> > + "as it does not support live
> migration");
> > + }
> > goto fail;
> > }
> > }
> > @@ -1202,7 +1205,6 @@ static int vvfat_open(BlockDriverState *bs, QDict
> *options, int flags,
> > init_mbr(s, cyls, heads, secs);
> > }
> >
> > - // assert(is_consistent(s));
> > qemu_co_mutex_init(&s->lock);
> >
> > ret = 0;
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index 43cb065..3b4fd24 100644
> > --- a/hw/9pfs/9p.c
> > +++ b/hw/9pfs/9p.c
> > @@ -1020,17 +1020,23 @@ static void coroutine_fn v9fs_attach(void
> *opaque)
> > */
> > if (!s->migration_blocker) {
> > int ret;
> > + Error *local_err;
> > error_setg(&s->migration_blocker,
> > "Migration is disabled when VirtFS export path '%s'
> is mounted in the guest using mount_tag '%s'",
> > s->ctx.fs_root ? s->ctx.fs_root : "NULL", s->tag);
> > - ret = migrate_add_blocker(s->migration_blocker, NULL);
> > + ret = migrate_add_blocker(s->migration_blocker, &local_err);
> > if (ret < 0) {
> > + if (ret == -EACCES) {
> > + error_append_hint(&local_err, "Failed to mount VirtFS
> as it "
> > + "does not support live migration");
> > + }
> > err = ret;
> > clunk_fid(s, fid);
> > error_free(s->migration_blocker);
> > s->migration_blocker = NULL;
> > goto out;
> > }
> > + error_free(local_err);
>
> Thinking again, I suggest you simply drop these changes: v9fs_attach() is
> triggered by the guest and doesn't cause QEMU to fail, so we don't really
> care.
>
> This might be naive but I didn't quite understand which changes you are
implying here. V2 to V3 or the entire diff in 9pfs?
Ashijeet
> Cheers.
>
> --
> Greg
>
> > s->root_fid = fid;
> > }
> >
> > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> > index 6e60b8c..fad95bf 100644
> > --- a/hw/display/virtio-gpu.c
> > +++ b/hw/display/virtio-gpu.c
> > @@ -1102,6 +1102,7 @@ static void virtio_gpu_device_realize(DeviceState
> *qdev, Error **errp)
> > VirtIOGPU *g = VIRTIO_GPU(qdev);
> > bool have_virgl;
> > int i;
> > + int ret;
> >
> > if (g->conf.max_outputs > VIRTIO_GPU_MAX_SCANOUTS) {
> > error_setg(errp, "invalid max_outputs > %d",
> VIRTIO_GPU_MAX_SCANOUTS);
> > @@ -1120,8 +1121,12 @@ static void virtio_gpu_device_realize(DeviceState
> *qdev, Error **errp)
> >
> > if (virtio_gpu_virgl_enabled(g->conf)) {
> > error_setg(&g->migration_blocker, "virgl is not yet
> migratable");
> > - if (migrate_add_blocker(g->migration_blocker, errp) < 0) {
> > - error_free(g->migration_blocker);
> > + ret = migrate_add_blocker(g->migration_blocker, errp);
> > + if (ret < 0) {
> > + if (ret == -EACCES) {
> > + error_append_hint(errp, "Cannot use virgl as it does
> not "
> > + "support live migration yet");
> > + }
> > return;
> > }
> > }
> > diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
> > index 3614daa..c2ef236 100644
> > --- a/hw/intc/arm_gic_kvm.c
> > +++ b/hw/intc/arm_gic_kvm.c
> > @@ -515,7 +515,10 @@ static void kvm_arm_gic_realize(DeviceState *dev,
> Error **errp)
> > "not support vGICv2
> migration");
> > ret = migrate_add_blocker(s->migration_blocker, errp);
> > if (ret < 0) {
> > - error_free(s->migration_blocker);
> > + if (ret == -EACCES) {
> > + error_append_hint(errp, "Failed to realize vGICv2 as
> its"
> > + " migrataion is not implemented yet");
> > + }
> > return;
> > }
> > }
> > diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
> > index 950a02f..3474642 100644
> > --- a/hw/intc/arm_gicv3_its_kvm.c
> > +++ b/hw/intc/arm_gicv3_its_kvm.c
> > @@ -70,7 +70,10 @@ static void kvm_arm_its_realize(DeviceState *dev,
> Error **errp)
> > error_setg(&s->migration_blocker, "vITS migration is not
> implemented");
> > ret = migrate_add_blocker(s->migration_blocker, errp);
> > if (ret < 0) {
> > - error_free(s->migration_blocker);
> > + if (ret == -EACCES) {
> > + error_append_hint(errp, "Failed to realize vITS as its
> migration "
> > + "is not implemented yet");
> > + }
> > return;
> > }
> >
> > diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
> > index 06f6f30..be7b97c 100644
> > --- a/hw/intc/arm_gicv3_kvm.c
> > +++ b/hw/intc/arm_gicv3_kvm.c
> > @@ -118,7 +118,10 @@ static void kvm_arm_gicv3_realize(DeviceState
> *dev, Error **errp)
> > error_setg(&s->migration_blocker, "vGICv3 migration is not
> implemented");
> > ret = migrate_add_blocker(s->migration_blocker, errp);
> > if (ret < 0) {
> > - error_free(s->migration_blocker);
> > + if (ret == -EACCES) {
> > + error_append_hint(errp, "Failed to realize vGICv3 as its
> migration"
> > + "is not implemented yet");
> > + }
> > return;
> > }
> >
> > diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> > index 585cc5b..14ed60d 100644
> > --- a/hw/misc/ivshmem.c
> > +++ b/hw/misc/ivshmem.c
> > @@ -837,6 +837,7 @@ static void ivshmem_common_realize(PCIDevice *dev,
> Error **errp)
> > {
> > IVShmemState *s = IVSHMEM_COMMON(dev);
> > Error *err = NULL;
> > + int ret;
> > uint8_t *pci_conf;
> > uint8_t attr = PCI_BASE_ADDRESS_SPACE_MEMORY |
> > PCI_BASE_ADDRESS_MEM_PREFETCH;
> > @@ -910,8 +911,13 @@ static void ivshmem_common_realize(PCIDevice *dev,
> Error **errp)
> > if (!ivshmem_is_master(s)) {
> > error_setg(&s->migration_blocker,
> > "Migration is disabled when using feature 'peer
> mode' in device 'ivshmem'");
> > - if (migrate_add_blocker(s->migration_blocker, errp) < 0) {
> > - error_free(s->migration_blocker);
> > + ret = migrate_add_blocker(s->migration_blocker, errp);
> > + if (ret < 0) {
> > + if (ret == -EACCES) {
> > + error_append_hint(errp, "Cannot use the 'peer mode'
> feature "
> > + "in device 'ivshmem' as it does not "
> > + "support live migration");
> > + }
> > return;
> > }
> > }
> > diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
> > index ff503d0..bb731b2 100644
> > --- a/hw/scsi/vhost-scsi.c
> > +++ b/hw/scsi/vhost-scsi.c
> > @@ -245,7 +245,11 @@ static void vhost_scsi_realize(DeviceState *dev,
> Error **errp)
> > "vhost-scsi does not support migration");
> > ret = migrate_add_blocker(s->migration_blocker, errp);
> > if (ret < 0) {
> > - goto free_blocker;
> > + if (ret == -EACCES) {
> > + error_append_hint(errp, "Cannot use vhost-scsi as it does
> not "
> > + "support live migration");
> > + }
> > + goto close_fd;
> > }
> >
> > s->dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs->conf.num_queues;
> > @@ -272,8 +276,6 @@ static void vhost_scsi_realize(DeviceState *dev,
> Error **errp)
> > free_vqs:
> > migrate_del_blocker(s->migration_blocker);
> > g_free(s->dev.vqs);
> > - free_blocker:
> > - error_free(s->migration_blocker);
> > close_fd:
> > close(vhostfd);
> > return;
> > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > index 416fada..6d2375a 100644
> > --- a/hw/virtio/vhost.c
> > +++ b/hw/virtio/vhost.c
> > @@ -1160,8 +1160,10 @@ int vhost_dev_init(struct vhost_dev *hdev, void
> *opaque,
> > Error *local_err;
> > r = migrate_add_blocker(hdev->migration_blocker, &local_err);
> > if (r < 0) {
> > - error_report_err(local_err);
> > - error_free(hdev->migration_blocker);
> > + if (r == -EACCES) {
> > + error_append_hint(&local_err, "Cannot use vhost drivers
> as "
> > + "it does not support live migration");
> > + }
> > goto fail_busyloop;
> > }
> > }
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 713e012..ab34328 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -1115,9 +1115,16 @@ int migrate_add_blocker(Error *reason, Error
> **errp)
> > {
> > if (migration_is_idle(NULL)) {
> > migration_blockers = g_slist_prepend(migration_blockers,
> reason);
> > + error_free(reason);
> > return 0;
> > }
> >
> > + if (only_migratable) {
> > + error_setg(errp, "Failed to add migration blocker:
> --only-migratable "
> > + "was specified");
> > + return -EACCES;
> > + }
> > +
> > error_setg(errp, "Cannot block a migration already in-progress");
> > return -EBUSY;
> > }
> > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > index 6031a1c..f2c35d8 100644
> > --- a/target-i386/kvm.c
> > +++ b/target-i386/kvm.c
> > @@ -963,6 +963,10 @@ int kvm_arch_init_vcpu(CPUState *cs)
> > " (invtsc flag)");
> > r = migrate_add_blocker(invtsc_mig_blocker, NULL);
> > if (r < 0) {
> > + if (r == -EACCES) {
> > + error_report("kvm: non-migratable CPU device but"
> > + " --only-migratable was specified");
> > + }
> > error_report("kvm: migrate_add_blocker failed");
> > goto efail;
> > }
> > @@ -1009,7 +1013,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
> > fail:
> > migrate_del_blocker(invtsc_mig_blocker);
> > efail:
> > - error_free(invtsc_mig_blocker);
> > return r;
> > }
> >
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] migration: disallow migrate_add_blocker during migration
2017-01-06 16:18 ` Greg Kurz
@ 2017-01-08 7:34 ` Ashijeet Acharya
2017-01-09 7:54 ` Greg Kurz
0 siblings, 1 reply; 13+ messages in thread
From: Ashijeet Acharya @ 2017-01-08 7:34 UTC (permalink / raw)
To: Greg Kurz
Cc: dgilbert, jsnow, amit.shah, pbonzini, kwolf, armbru, quintela,
mst, marcandre.lureau, aneesh.kumar, peter.maydell, qemu-devel
On Friday, January 6, 2017, Greg Kurz <groug@kaod.org> wrote:
> Hi Ashijeet,
>
Hello Greg,
> I didn't think hard enough while reviewing the changes for hw/9pfs/9p.c...
> I have
> some more remarks, sorry... :-/
>
> No problem, I will send an updated v4 for these.
On Wed, 4 Jan 2017 18:02:28 +0530
> Ashijeet Acharya <ashijeetacharya@gmail.com <javascript:;>> wrote:
>
> > If a migration is already in progress and somebody attempts
> > to add a migration blocker, this should rightly fail.
> >
> > Add an errp parameter and a retcode return value to migrate_add_blocker.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com <javascript:;>>
> > Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com
> <javascript:;>>
> > ---
> > block/qcow.c | 6 +++++-
> > block/vdi.c | 6 +++++-
> > block/vhdx.c | 16 ++++++++++------
> > block/vmdk.c | 7 ++++++-
> > block/vpc.c | 10 +++++++---
> > block/vvfat.c | 20 ++++++++++++--------
> > hw/9pfs/9p.c | 20 +++++++++++++++-----
> > hw/display/virtio-gpu.c | 29 ++++++++++++++++-------------
> > hw/intc/arm_gic_kvm.c | 16 ++++++++++------
> > hw/intc/arm_gicv3_its_kvm.c | 18 +++++++++++-------
> > hw/intc/arm_gicv3_kvm.c | 19 ++++++++++++-------
> > hw/misc/ivshmem.c | 11 +++++++----
> > hw/scsi/vhost-scsi.c | 25 +++++++++++++++++++------
> > hw/virtio/vhost.c | 8 +++++++-
> > include/migration/migration.h | 6 +++++-
> > migration/migration.c | 35 +++++++++++++++++++++++++++++++++--
> > stubs/migr-blocker.c | 3 ++-
> > target-i386/kvm.c | 16 +++++++++++++---
> > 18 files changed, 195 insertions(+), 76 deletions(-)
> >
> > diff --git a/block/qcow.c b/block/qcow.c
> > index 7540f43..11526a1 100644
> > --- a/block/qcow.c
> > +++ b/block/qcow.c
> > @@ -252,7 +252,11 @@ static int qcow_open(BlockDriverState *bs, QDict
> *options, int flags,
> > error_setg(&s->migration_blocker, "The qcow format used by node
> '%s' "
> > "does not support live migration",
> > bdrv_get_device_or_node_name(bs));
> > - migrate_add_blocker(s->migration_blocker);
> > + ret = migrate_add_blocker(s->migration_blocker, errp);
> > + if (ret < 0) {
> > + error_free(s->migration_blocker);
> > + goto fail;
> > + }
> >
> > qemu_co_mutex_init(&s->lock);
> > return 0;
> > diff --git a/block/vdi.c b/block/vdi.c
> > index 96b78d5..2b56f52 100644
> > --- a/block/vdi.c
> > +++ b/block/vdi.c
> > @@ -471,7 +471,11 @@ static int vdi_open(BlockDriverState *bs, QDict
> *options, int flags,
> > error_setg(&s->migration_blocker, "The vdi format used by node
> '%s' "
> > "does not support live migration",
> > bdrv_get_device_or_node_name(bs));
> > - migrate_add_blocker(s->migration_blocker);
> > + ret = migrate_add_blocker(s->migration_blocker, errp);
> > + if (ret < 0) {
> > + error_free(s->migration_blocker);
> > + goto fail_free_bmap;
> > + }
> >
> > qemu_co_mutex_init(&s->write_lock);
> >
> > diff --git a/block/vhdx.c b/block/vhdx.c
> > index 0ba2f0a..d81941b 100644
> > --- a/block/vhdx.c
> > +++ b/block/vhdx.c
> > @@ -991,6 +991,16 @@ static int vhdx_open(BlockDriverState *bs, QDict
> *options, int flags,
> > }
> > }
> >
> > + /* Disable migration when VHDX images are used */
> > + error_setg(&s->migration_blocker, "The vhdx format used by node
> '%s' "
> > + "does not support live migration",
> > + bdrv_get_device_or_node_name(bs));
> > + ret = migrate_add_blocker(s->migration_blocker, errp);
> > + if (ret < 0) {
> > + error_free(s->migration_blocker);
> > + goto fail;
> > + }
> > +
> > if (flags & BDRV_O_RDWR) {
> > ret = vhdx_update_headers(bs, s, false, NULL);
> > if (ret < 0) {
> > @@ -1000,12 +1010,6 @@ static int vhdx_open(BlockDriverState *bs, QDict
> *options, int flags,
> >
> > /* TODO: differencing files */
> >
> > - /* Disable migration when VHDX images are used */
> > - error_setg(&s->migration_blocker, "The vhdx format used by node
> '%s' "
> > - "does not support live migration",
> > - bdrv_get_device_or_node_name(bs));
> > - migrate_add_blocker(s->migration_blocker);
> > -
> > return 0;
> > fail:
> > vhdx_close(bs);
> > diff --git a/block/vmdk.c b/block/vmdk.c
> > index a11c27a..4a45a83 100644
> > --- a/block/vmdk.c
> > +++ b/block/vmdk.c
> > @@ -976,7 +976,12 @@ static int vmdk_open(BlockDriverState *bs, QDict
> *options, int flags,
> > error_setg(&s->migration_blocker, "The vmdk format used by node
> '%s' "
> > "does not support live migration",
> > bdrv_get_device_or_node_name(bs));
> > - migrate_add_blocker(s->migration_blocker);
> > + ret = migrate_add_blocker(s->migration_blocker, errp);
> > + if (ret < 0) {
> > + error_free(s->migration_blocker);
> > + goto fail;
> > + }
> > +
> > g_free(buf);
> > return 0;
> >
> > diff --git a/block/vpc.c b/block/vpc.c
> > index 8d5886f..299a8c8 100644
> > --- a/block/vpc.c
> > +++ b/block/vpc.c
> > @@ -422,13 +422,17 @@ static int vpc_open(BlockDriverState *bs, QDict
> *options, int flags,
> > #endif
> > }
> >
> > - qemu_co_mutex_init(&s->lock);
> > -
> > /* Disable migration when VHD images are used */
> > error_setg(&s->migration_blocker, "The vpc format used by node
> '%s' "
> > "does not support live migration",
> > bdrv_get_device_or_node_name(bs));
> > - migrate_add_blocker(s->migration_blocker);
> > + ret = migrate_add_blocker(s->migration_blocker, errp);
> > + if (ret < 0) {
> > + error_free(s->migration_blocker);
> > + goto fail;
> > + }
> > +
> > + qemu_co_mutex_init(&s->lock);
> >
> > return 0;
> >
> > diff --git a/block/vvfat.c b/block/vvfat.c
> > index ded2109..3de3320 100644
> > --- a/block/vvfat.c
> > +++ b/block/vvfat.c
> > @@ -1185,22 +1185,26 @@ static int vvfat_open(BlockDriverState *bs,
> QDict *options, int flags,
> >
> > s->sector_count = s->faked_sectors + s->sectors_per_cluster*s->
> cluster_count;
> >
> > - if (s->first_sectors_number == 0x40) {
> > - init_mbr(s, cyls, heads, secs);
> > - }
> > -
> > - // assert(is_consistent(s));
> > - qemu_co_mutex_init(&s->lock);
> > -
> > /* Disable migration when vvfat is used rw */
> > if (s->qcow) {
> > error_setg(&s->migration_blocker,
> > "The vvfat (rw) format used by node '%s' "
> > "does not support live migration",
> > bdrv_get_device_or_node_name(bs));
> > - migrate_add_blocker(s->migration_blocker);
> > + ret = migrate_add_blocker(s->migration_blocker, errp);
> > + if (ret < 0) {
> > + error_free(s->migration_blocker);
> > + goto fail;
> > + }
> > + }
> > +
> > + if (s->first_sectors_number == 0x40) {
> > + init_mbr(s, cyls, heads, secs);
> > }
> >
> > + // assert(is_consistent(s));
> > + qemu_co_mutex_init(&s->lock);
> > +
> > ret = 0;
> > fail:
> > qemu_opts_del(opts);
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index faebd91..43cb065 100644
> > --- a/hw/9pfs/9p.c
> > +++ b/hw/9pfs/9p.c
> > @@ -1013,20 +1013,30 @@ static void coroutine_fn v9fs_attach(void
> *opaque)
> > goto out;
> > }
> > err += offset;
> > - memcpy(&s->root_qid, &qid, sizeof(qid));
> > - trace_v9fs_attach_return(pdu->tag, pdu->id,
> > - qid.type, qid.version, qid.path);
> > +
> > /*
> > * disable migration if we haven't done already.
> > * attach could get called multiple times for the same export.
> > */
> > if (!s->migration_blocker) {
> > - s->root_fid = fid;
> > + int ret;
> > error_setg(&s->migration_blocker,
> > "Migration is disabled when VirtFS export path '%s'
> is mounted in the guest using mount_tag '%s'",
> > s->ctx.fs_root ? s->ctx.fs_root : "NULL", s->tag);
> > - migrate_add_blocker(s->migration_blocker);
> > + ret = migrate_add_blocker(s->migration_blocker, NULL);
> > + if (ret < 0) {
> > + err = ret;
> > + clunk_fid(s, fid);
> > + error_free(s->migration_blocker);
> > + s->migration_blocker = NULL;
>
> It's better to rollback things in reverse order (i.e. clunk_fid() should
> be called here).
Done.
>
> > + goto out;
> > + }
> > + s->root_fid = fid;
> > }
>
> I now realize that all the migration blocker stuff should be done before
> we call pdu_marshal() since we may now return an error to the guest. And
Yes, migration blocker needs to be allowed to do its bit first and it might
fail and return an error.
thus, you can drop ret and use err directly.
Alright. I will drop it.
Also you suggested to use EBUSY instead of EACCES but then it will be
difficult to differentiate which case failed migration blocker as both will
end up returning the same error number.
Ashijeet
> Cheers.
>
> --
> Greg
>
> > +
> > + memcpy(&s->root_qid, &qid, sizeof(qid));
> > + trace_v9fs_attach_return(pdu->tag, pdu->id,
> > + qid.type, qid.version, qid.path);
> > out:
> > put_fid(pdu, fidp);
> > out_nofid:
> > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> > index 5f32e1a..6e60b8c 100644
> > --- a/hw/display/virtio-gpu.c
> > +++ b/hw/display/virtio-gpu.c
> > @@ -1108,14 +1108,6 @@ static void virtio_gpu_device_realize(DeviceState
> *qdev, Error **errp)
> > return;
> > }
> >
> > - g->config_size = sizeof(struct virtio_gpu_config);
> > - g->virtio_config.num_scanouts = g->conf.max_outputs;
> > - virtio_init(VIRTIO_DEVICE(g), "virtio-gpu", VIRTIO_ID_GPU,
> > - g->config_size);
> > -
> > - g->req_state[0].width = 1024;
> > - g->req_state[0].height = 768;
> > -
> > g->use_virgl_renderer = false;
> > #if !defined(CONFIG_VIRGL) || defined(HOST_WORDS_BIGENDIAN)
> > have_virgl = false;
> > @@ -1127,6 +1119,22 @@ static void virtio_gpu_device_realize(DeviceState
> *qdev, Error **errp)
> > }
> >
> > if (virtio_gpu_virgl_enabled(g->conf)) {
> > + error_setg(&g->migration_blocker, "virgl is not yet
> migratable");
> > + if (migrate_add_blocker(g->migration_blocker, errp) < 0) {
> > + error_free(g->migration_blocker);
> > + return;
> > + }
> > + }
> > +
> > + g->config_size = sizeof(struct virtio_gpu_config);
> > + g->virtio_config.num_scanouts = g->conf.max_outputs;
> > + virtio_init(VIRTIO_DEVICE(g), "virtio-gpu", VIRTIO_ID_GPU,
> > + g->config_size);
> > +
> > + g->req_state[0].width = 1024;
> > + g->req_state[0].height = 768;
> > +
> > + if (virtio_gpu_virgl_enabled(g->conf)) {
> > /* use larger control queue in 3d mode */
> > g->ctrl_vq = virtio_add_queue(vdev, 256,
> virtio_gpu_handle_ctrl_cb);
> > g->cursor_vq = virtio_add_queue(vdev, 16,
> virtio_gpu_handle_cursor_cb);
> > @@ -1152,11 +1160,6 @@ static void virtio_gpu_device_realize(DeviceState
> *qdev, Error **errp)
> > dpy_gfx_replace_surface(g->scanout[i].con, NULL);
> > }
> > }
> > -
> > - if (virtio_gpu_virgl_enabled(g->conf)) {
> > - error_setg(&g->migration_blocker, "virgl is not yet
> migratable");
> > - migrate_add_blocker(g->migration_blocker);
> > - }
> > }
> >
> > static void virtio_gpu_device_unrealize(DeviceState *qdev, Error
> **errp)
> > diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
> > index 11729ee..3614daa 100644
> > --- a/hw/intc/arm_gic_kvm.c
> > +++ b/hw/intc/arm_gic_kvm.c
> > @@ -510,6 +510,16 @@ static void kvm_arm_gic_realize(DeviceState *dev,
> Error **errp)
> > return;
> > }
> >
> > + if (!kvm_arm_gic_can_save_restore(s)) {
> > + error_setg(&s->migration_blocker, "This operating system
> kernel does "
> > + "not support vGICv2
> migration");
> > + ret = migrate_add_blocker(s->migration_blocker, errp);
> > + if (ret < 0) {
> > + error_free(s->migration_blocker);
> > + return;
> > + }
> > + }
> > +
> > gic_init_irqs_and_mmio(s, kvm_arm_gicv2_set_irq, NULL);
> >
> > for (i = 0; i < s->num_irq - GIC_INTERNAL; i++) {
> > @@ -558,12 +568,6 @@ static void kvm_arm_gic_realize(DeviceState *dev,
> Error **errp)
> > KVM_VGIC_V2_ADDR_TYPE_CPU,
> > s->dev_fd);
> >
> > - if (!kvm_arm_gic_can_save_restore(s)) {
> > - error_setg(&s->migration_blocker, "This operating system
> kernel does "
> > - "not support vGICv2
> migration");
> > - migrate_add_blocker(s->migration_blocker);
> > - }
> > -
> > if (kvm_has_gsi_routing()) {
> > /* set up irq routing */
> > kvm_init_irq_routing(kvm_state);
> > diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
> > index fc246e0..950a02f 100644
> > --- a/hw/intc/arm_gicv3_its_kvm.c
> > +++ b/hw/intc/arm_gicv3_its_kvm.c
> > @@ -63,6 +63,17 @@ static void kvm_arm_its_realize(DeviceState *dev,
> Error **errp)
> > return;
> > }
> >
> > + /*
> > + * Block migration of a KVM GICv3 ITS device: the API for saving and
> > + * restoring the state in the kernel is not yet available
> > + */
> > + error_setg(&s->migration_blocker, "vITS migration is not
> implemented");
> > + ret = migrate_add_blocker(s->migration_blocker, errp);
> > + if (ret < 0) {
> > + error_free(s->migration_blocker);
> > + return;
> > + }
> > +
> > /* explicit init of the ITS */
> > kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
> > KVM_DEV_ARM_VGIC_CTRL_INIT, NULL, true);
> > @@ -73,13 +84,6 @@ static void kvm_arm_its_realize(DeviceState *dev,
> Error **errp)
> >
> > gicv3_its_init_mmio(s, NULL);
> >
> > - /*
> > - * Block migration of a KVM GICv3 ITS device: the API for saving and
> > - * restoring the state in the kernel is not yet available
> > - */
> > - error_setg(&s->migration_blocker, "vITS migration is not
> implemented");
> > - migrate_add_blocker(s->migration_blocker);
> > -
> > kvm_msi_use_devid = true;
> > kvm_gsi_direct_mapping = false;
> > kvm_msi_via_irqfd_allowed = kvm_irqfds_enabled();
> > diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
> > index 199a439..06f6f30 100644
> > --- a/hw/intc/arm_gicv3_kvm.c
> > +++ b/hw/intc/arm_gicv3_kvm.c
> > @@ -86,6 +86,7 @@ static void kvm_arm_gicv3_realize(DeviceState *dev,
> Error **errp)
> > KVMARMGICv3Class *kgc = KVM_ARM_GICV3_GET_CLASS(s);
> > Error *local_err = NULL;
> > int i;
> > + int ret;
> >
> > DPRINTF("kvm_arm_gicv3_realize\n");
> >
> > @@ -110,6 +111,17 @@ static void kvm_arm_gicv3_realize(DeviceState
> *dev, Error **errp)
> > return;
> > }
> >
> > + /* Block migration of a KVM GICv3 device: the API for saving and
> restoring
> > + * the state in the kernel is not yet finalised in the kernel or
> > + * implemented in QEMU.
> > + */
> > + error_setg(&s->migration_blocker, "vGICv3 migration is not
> implemented");
> > + ret = migrate_add_blocker(s->migration_blocker, errp);
> > + if (ret < 0) {
> > + error_free(s->migration_blocker);
> > + return;
> > + }
> > +
> > kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_NR_IRQS,
> > 0, &s->num_irq, true);
> >
> > @@ -122,13 +134,6 @@ static void kvm_arm_gicv3_realize(DeviceState
> *dev, Error **errp)
> > kvm_arm_register_device(&s->iomem_redist, -1,
> KVM_DEV_ARM_VGIC_GRP_ADDR,
> > KVM_VGIC_V3_ADDR_TYPE_REDIST, s->dev_fd);
> >
> > - /* Block migration of a KVM GICv3 device: the API for saving and
> restoring
> > - * the state in the kernel is not yet finalised in the kernel or
> > - * implemented in QEMU.
> > - */
> > - error_setg(&s->migration_blocker, "vGICv3 migration is not
> implemented");
> > - migrate_add_blocker(s->migration_blocker);
> > -
> > if (kvm_has_gsi_routing()) {
> > /* set up irq routing */
> > kvm_init_irq_routing(kvm_state);
> > diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> > index abeaf3d..585cc5b 100644
> > --- a/hw/misc/ivshmem.c
> > +++ b/hw/misc/ivshmem.c
> > @@ -903,9 +903,6 @@ static void ivshmem_common_realize(PCIDevice *dev,
> Error **errp)
> > }
> > }
> >
> > - vmstate_register_ram(s->ivshmem_bar2, DEVICE(s));
> > - pci_register_bar(PCI_DEVICE(s), 2, attr, s->ivshmem_bar2);
> > -
> > if (s->master == ON_OFF_AUTO_AUTO) {
> > s->master = s->vm_id == 0 ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
> > }
> > @@ -913,8 +910,14 @@ static void ivshmem_common_realize(PCIDevice *dev,
> Error **errp)
> > if (!ivshmem_is_master(s)) {
> > error_setg(&s->migration_blocker,
> > "Migration is disabled when using feature 'peer
> mode' in device 'ivshmem'");
> > - migrate_add_blocker(s->migration_blocker);
> > + if (migrate_add_blocker(s->migration_blocker, errp) < 0) {
> > + error_free(s->migration_blocker);
> > + return;
> > + }
> > }
> > +
> > + vmstate_register_ram(s->ivshmem_bar2, DEVICE(s));
> > + pci_register_bar(PCI_DEVICE(s), 2, attr, s->ivshmem_bar2);
> > }
> >
> > static void ivshmem_exit(PCIDevice *dev)
> > diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
> > index 5b26946..ff503d0 100644
> > --- a/hw/scsi/vhost-scsi.c
> > +++ b/hw/scsi/vhost-scsi.c
> > @@ -238,8 +238,14 @@ static void vhost_scsi_realize(DeviceState *dev,
> Error **errp)
> > vhost_dummy_handle_output);
> > if (err != NULL) {
> > error_propagate(errp, err);
> > - close(vhostfd);
> > - return;
> > + goto close_fd;
> > + }
> > +
> > + error_setg(&s->migration_blocker,
> > + "vhost-scsi does not support migration");
> > + ret = migrate_add_blocker(s->migration_blocker, errp);
> > + if (ret < 0) {
> > + goto free_blocker;
> > }
> >
> > s->dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs->conf.num_queues;
> > @@ -252,7 +258,7 @@ static void vhost_scsi_realize(DeviceState *dev,
> Error **errp)
> > if (ret < 0) {
> > error_setg(errp, "vhost-scsi: vhost initialization failed: %s",
> > strerror(-ret));
> > - return;
> > + goto free_vqs;
> > }
> >
> > /* At present, channel and lun both are 0 for bootable vhost-scsi
> disk */
> > @@ -261,9 +267,16 @@ static void vhost_scsi_realize(DeviceState *dev,
> Error **errp)
> > /* Note: we can also get the minimum tpgt from kernel */
> > s->target = vs->conf.boot_tpgt;
> >
> > - error_setg(&s->migration_blocker,
> > - "vhost-scsi does not support migration");
> > - migrate_add_blocker(s->migration_blocker);
> > + return;
> > +
> > + free_vqs:
> > + migrate_del_blocker(s->migration_blocker);
> > + g_free(s->dev.vqs);
> > + free_blocker:
> > + error_free(s->migration_blocker);
> > + close_fd:
> > + close(vhostfd);
> > + return;
> > }
> >
> > static void vhost_scsi_unrealize(DeviceState *dev, Error **errp)
> > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > index f7f7023..416fada 100644
> > --- a/hw/virtio/vhost.c
> > +++ b/hw/virtio/vhost.c
> > @@ -1157,7 +1157,13 @@ int vhost_dev_init(struct vhost_dev *hdev, void
> *opaque,
> > }
> >
> > if (hdev->migration_blocker != NULL) {
> > - migrate_add_blocker(hdev->migration_blocker);
> > + Error *local_err;
> > + r = migrate_add_blocker(hdev->migration_blocker, &local_err);
> > + if (r < 0) {
> > + error_report_err(local_err);
> > + error_free(hdev->migration_blocker);
> > + goto fail_busyloop;
> > + }
> > }
> >
> > hdev->mem = g_malloc0(offsetof(struct vhost_memory, regions));
> > diff --git a/include/migration/migration.h
> b/include/migration/migration.h
> > index 40b3697..46a4bb5 100644
> > --- a/include/migration/migration.h
> > +++ b/include/migration/migration.h
> > @@ -243,6 +243,7 @@ void remove_migration_state_change_notifier(Notifier
> *notify);
> > MigrationState *migrate_init(const MigrationParams *params);
> > bool migration_is_blocked(Error **errp);
> > bool migration_in_setup(MigrationState *);
> > +bool migration_is_idle(MigrationState *s);
> > bool migration_has_finished(MigrationState *);
> > bool migration_has_failed(MigrationState *);
> > /* True if outgoing migration has entered postcopy phase */
> > @@ -287,8 +288,11 @@ int ram_postcopy_incoming_init(MigrationIncomingState
> *mis);
> > * @migrate_add_blocker - prevent migration from proceeding
> > *
> > * @reason - an error to be returned whenever migration is attempted
> > + * @errp - [out] The reason (if any) we cannot block migration right
> now.
> > + *
> > + * @returns - 0 on success, -EBUSY on failure, with errp set.
> > */
> > -void migrate_add_blocker(Error *reason);
> > +int migrate_add_blocker(Error *reason, Error **errp);
> >
> > /**
> > * @migrate_del_blocker - remove a blocking error from migration
> > diff --git a/migration/migration.c b/migration/migration.c
> > index f498ab8..713e012 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -1044,6 +1044,31 @@ bool migration_in_postcopy_after_devices(MigrationState
> *s)
> > return migration_in_postcopy(s) && s->postcopy_after_devices;
> > }
> >
> > +bool migration_is_idle(MigrationState *s)
> > +{
> > + if (!s) {
> > + s = migrate_get_current();
> > + }
> > +
> > + switch (s->state) {
> > + case MIGRATION_STATUS_NONE:
> > + case MIGRATION_STATUS_CANCELLED:
> > + case MIGRATION_STATUS_COMPLETED:
> > + case MIGRATION_STATUS_FAILED:
> > + return true;
> > + case MIGRATION_STATUS_SETUP:
> > + case MIGRATION_STATUS_CANCELLING:
> > + case MIGRATION_STATUS_ACTIVE:
> > + case MIGRATION_STATUS_POSTCOPY_ACTIVE:
> > + case MIGRATION_STATUS_COLO:
> > + return false;
> > + case MIGRATION_STATUS__MAX:
> > + g_assert_not_reached();
> > + }
> > +
> > + return false;
> > +}
> > +
> > MigrationState *migrate_init(const MigrationParams *params)
> > {
> > MigrationState *s = migrate_get_current();
> > @@ -1086,9 +1111,15 @@ MigrationState *migrate_init(const
> MigrationParams *params)
> >
> > static GSList *migration_blockers;
> >
> > -void migrate_add_blocker(Error *reason)
> > +int migrate_add_blocker(Error *reason, Error **errp)
> > {
> > - migration_blockers = g_slist_prepend(migration_blockers, reason);
> > + if (migration_is_idle(NULL)) {
> > + migration_blockers = g_slist_prepend(migration_blockers,
> reason);
> > + return 0;
> > + }
> > +
> > + error_setg(errp, "Cannot block a migration already in-progress");
> > + return -EBUSY;
> > }
> >
> > void migrate_del_blocker(Error *reason)
> > diff --git a/stubs/migr-blocker.c b/stubs/migr-blocker.c
> > index 8ab3604..a5ba18f 100644
> > --- a/stubs/migr-blocker.c
> > +++ b/stubs/migr-blocker.c
> > @@ -2,8 +2,9 @@
> > #include "qemu-common.h"
> > #include "migration/migration.h"
> >
> > -void migrate_add_blocker(Error *reason)
> > +int migrate_add_blocker(Error *reason, Error **errp)
> > {
> > + return 0;
> > }
> >
> > void migrate_del_blocker(Error *reason)
> > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > index f62264a..6031a1c 100644
> > --- a/target-i386/kvm.c
> > +++ b/target-i386/kvm.c
> > @@ -961,7 +961,11 @@ int kvm_arch_init_vcpu(CPUState *cs)
> > error_setg(&invtsc_mig_blocker,
> > "State blocked by non-migratable CPU device"
> > " (invtsc flag)");
> > - migrate_add_blocker(invtsc_mig_blocker);
> > + r = migrate_add_blocker(invtsc_mig_blocker, NULL);
> > + if (r < 0) {
> > + error_report("kvm: migrate_add_blocker failed");
> > + goto efail;
> > + }
> > /* for savevm */
> > vmstate_x86_cpu.unmigratable = 1;
> > }
> > @@ -969,12 +973,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
> > cpuid_data.cpuid.padding = 0;
> > r = kvm_vcpu_ioctl(cs, KVM_SET_CPUID2, &cpuid_data);
> > if (r) {
> > - return r;
> > + goto fail;
> > }
> >
> > r = kvm_arch_set_tsc_khz(cs);
> > if (r < 0) {
> > - return r;
> > + goto fail;
> > }
> >
> > /* vcpu's TSC frequency is either specified by user, or following
> > @@ -1001,6 +1005,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
> > }
> >
> > return 0;
> > +
> > + fail:
> > + migrate_del_blocker(invtsc_mig_blocker);
> > + efail:
> > + error_free(invtsc_mig_blocker);
> > + return r;
> > }
> >
> > void kvm_arch_reset_vcpu(X86CPU *cpu)
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] migration: disallow migrate_add_blocker during migration
2017-01-08 7:34 ` [Qemu-devel] [PATCH v2 " Ashijeet Acharya
@ 2017-01-09 7:54 ` Greg Kurz
0 siblings, 0 replies; 13+ messages in thread
From: Greg Kurz @ 2017-01-09 7:54 UTC (permalink / raw)
To: Ashijeet Acharya
Cc: dgilbert, jsnow, amit.shah, pbonzini, kwolf, armbru, quintela,
mst, marcandre.lureau, aneesh.kumar, peter.maydell, qemu-devel
Funny... this was v3 and now it is v2 :)
On Sun, 8 Jan 2017 13:04:47 +0530
Ashijeet Acharya <ashijeetacharya@gmail.com> wrote:
> On Friday, January 6, 2017, Greg Kurz <groug@kaod.org> wrote:
>
> > Hi Ashijeet,
> >
>
> Hello Greg,
>
>
> > I didn't think hard enough while reviewing the changes for hw/9pfs/9p.c...
> > I have
> > some more remarks, sorry... :-/
> >
> > No problem, I will send an updated v4 for these.
>
> On Wed, 4 Jan 2017 18:02:28 +0530
> > Ashijeet Acharya <ashijeetacharya@gmail.com <javascript:;>> wrote:
> >
> > > If a migration is already in progress and somebody attempts
> > > to add a migration blocker, this should rightly fail.
> > >
> > > Add an errp parameter and a retcode return value to migrate_add_blocker.
> > >
> > > Signed-off-by: John Snow <jsnow@redhat.com <javascript:;>>
> > > Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com
> > <javascript:;>>
> > > ---
> > > block/qcow.c | 6 +++++-
> > > block/vdi.c | 6 +++++-
> > > block/vhdx.c | 16 ++++++++++------
> > > block/vmdk.c | 7 ++++++-
> > > block/vpc.c | 10 +++++++---
> > > block/vvfat.c | 20 ++++++++++++--------
> > > hw/9pfs/9p.c | 20 +++++++++++++++-----
> > > hw/display/virtio-gpu.c | 29 ++++++++++++++++-------------
> > > hw/intc/arm_gic_kvm.c | 16 ++++++++++------
> > > hw/intc/arm_gicv3_its_kvm.c | 18 +++++++++++-------
> > > hw/intc/arm_gicv3_kvm.c | 19 ++++++++++++-------
> > > hw/misc/ivshmem.c | 11 +++++++----
> > > hw/scsi/vhost-scsi.c | 25 +++++++++++++++++++------
> > > hw/virtio/vhost.c | 8 +++++++-
> > > include/migration/migration.h | 6 +++++-
> > > migration/migration.c | 35 +++++++++++++++++++++++++++++++++--
> > > stubs/migr-blocker.c | 3 ++-
> > > target-i386/kvm.c | 16 +++++++++++++---
> > > 18 files changed, 195 insertions(+), 76 deletions(-)
> > >
> > > diff --git a/block/qcow.c b/block/qcow.c
> > > index 7540f43..11526a1 100644
> > > --- a/block/qcow.c
> > > +++ b/block/qcow.c
> > > @@ -252,7 +252,11 @@ static int qcow_open(BlockDriverState *bs, QDict
> > *options, int flags,
> > > error_setg(&s->migration_blocker, "The qcow format used by node
> > '%s' "
> > > "does not support live migration",
> > > bdrv_get_device_or_node_name(bs));
> > > - migrate_add_blocker(s->migration_blocker);
> > > + ret = migrate_add_blocker(s->migration_blocker, errp);
> > > + if (ret < 0) {
> > > + error_free(s->migration_blocker);
> > > + goto fail;
> > > + }
> > >
> > > qemu_co_mutex_init(&s->lock);
> > > return 0;
> > > diff --git a/block/vdi.c b/block/vdi.c
> > > index 96b78d5..2b56f52 100644
> > > --- a/block/vdi.c
> > > +++ b/block/vdi.c
> > > @@ -471,7 +471,11 @@ static int vdi_open(BlockDriverState *bs, QDict
> > *options, int flags,
> > > error_setg(&s->migration_blocker, "The vdi format used by node
> > '%s' "
> > > "does not support live migration",
> > > bdrv_get_device_or_node_name(bs));
> > > - migrate_add_blocker(s->migration_blocker);
> > > + ret = migrate_add_blocker(s->migration_blocker, errp);
> > > + if (ret < 0) {
> > > + error_free(s->migration_blocker);
> > > + goto fail_free_bmap;
> > > + }
> > >
> > > qemu_co_mutex_init(&s->write_lock);
> > >
> > > diff --git a/block/vhdx.c b/block/vhdx.c
> > > index 0ba2f0a..d81941b 100644
> > > --- a/block/vhdx.c
> > > +++ b/block/vhdx.c
> > > @@ -991,6 +991,16 @@ static int vhdx_open(BlockDriverState *bs, QDict
> > *options, int flags,
> > > }
> > > }
> > >
> > > + /* Disable migration when VHDX images are used */
> > > + error_setg(&s->migration_blocker, "The vhdx format used by node
> > '%s' "
> > > + "does not support live migration",
> > > + bdrv_get_device_or_node_name(bs));
> > > + ret = migrate_add_blocker(s->migration_blocker, errp);
> > > + if (ret < 0) {
> > > + error_free(s->migration_blocker);
> > > + goto fail;
> > > + }
> > > +
> > > if (flags & BDRV_O_RDWR) {
> > > ret = vhdx_update_headers(bs, s, false, NULL);
> > > if (ret < 0) {
> > > @@ -1000,12 +1010,6 @@ static int vhdx_open(BlockDriverState *bs, QDict
> > *options, int flags,
> > >
> > > /* TODO: differencing files */
> > >
> > > - /* Disable migration when VHDX images are used */
> > > - error_setg(&s->migration_blocker, "The vhdx format used by node
> > '%s' "
> > > - "does not support live migration",
> > > - bdrv_get_device_or_node_name(bs));
> > > - migrate_add_blocker(s->migration_blocker);
> > > -
> > > return 0;
> > > fail:
> > > vhdx_close(bs);
> > > diff --git a/block/vmdk.c b/block/vmdk.c
> > > index a11c27a..4a45a83 100644
> > > --- a/block/vmdk.c
> > > +++ b/block/vmdk.c
> > > @@ -976,7 +976,12 @@ static int vmdk_open(BlockDriverState *bs, QDict
> > *options, int flags,
> > > error_setg(&s->migration_blocker, "The vmdk format used by node
> > '%s' "
> > > "does not support live migration",
> > > bdrv_get_device_or_node_name(bs));
> > > - migrate_add_blocker(s->migration_blocker);
> > > + ret = migrate_add_blocker(s->migration_blocker, errp);
> > > + if (ret < 0) {
> > > + error_free(s->migration_blocker);
> > > + goto fail;
> > > + }
> > > +
> > > g_free(buf);
> > > return 0;
> > >
> > > diff --git a/block/vpc.c b/block/vpc.c
> > > index 8d5886f..299a8c8 100644
> > > --- a/block/vpc.c
> > > +++ b/block/vpc.c
> > > @@ -422,13 +422,17 @@ static int vpc_open(BlockDriverState *bs, QDict
> > *options, int flags,
> > > #endif
> > > }
> > >
> > > - qemu_co_mutex_init(&s->lock);
> > > -
> > > /* Disable migration when VHD images are used */
> > > error_setg(&s->migration_blocker, "The vpc format used by node
> > '%s' "
> > > "does not support live migration",
> > > bdrv_get_device_or_node_name(bs));
> > > - migrate_add_blocker(s->migration_blocker);
> > > + ret = migrate_add_blocker(s->migration_blocker, errp);
> > > + if (ret < 0) {
> > > + error_free(s->migration_blocker);
> > > + goto fail;
> > > + }
> > > +
> > > + qemu_co_mutex_init(&s->lock);
> > >
> > > return 0;
> > >
> > > diff --git a/block/vvfat.c b/block/vvfat.c
> > > index ded2109..3de3320 100644
> > > --- a/block/vvfat.c
> > > +++ b/block/vvfat.c
> > > @@ -1185,22 +1185,26 @@ static int vvfat_open(BlockDriverState *bs,
> > QDict *options, int flags,
> > >
> > > s->sector_count = s->faked_sectors + s->sectors_per_cluster*s->
> > cluster_count;
> > >
> > > - if (s->first_sectors_number == 0x40) {
> > > - init_mbr(s, cyls, heads, secs);
> > > - }
> > > -
> > > - // assert(is_consistent(s));
> > > - qemu_co_mutex_init(&s->lock);
> > > -
> > > /* Disable migration when vvfat is used rw */
> > > if (s->qcow) {
> > > error_setg(&s->migration_blocker,
> > > "The vvfat (rw) format used by node '%s' "
> > > "does not support live migration",
> > > bdrv_get_device_or_node_name(bs));
> > > - migrate_add_blocker(s->migration_blocker);
> > > + ret = migrate_add_blocker(s->migration_blocker, errp);
> > > + if (ret < 0) {
> > > + error_free(s->migration_blocker);
> > > + goto fail;
> > > + }
> > > + }
> > > +
> > > + if (s->first_sectors_number == 0x40) {
> > > + init_mbr(s, cyls, heads, secs);
> > > }
> > >
> > > + // assert(is_consistent(s));
> > > + qemu_co_mutex_init(&s->lock);
> > > +
> > > ret = 0;
> > > fail:
> > > qemu_opts_del(opts);
> > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > > index faebd91..43cb065 100644
> > > --- a/hw/9pfs/9p.c
> > > +++ b/hw/9pfs/9p.c
> > > @@ -1013,20 +1013,30 @@ static void coroutine_fn v9fs_attach(void
> > *opaque)
> > > goto out;
> > > }
> > > err += offset;
> > > - memcpy(&s->root_qid, &qid, sizeof(qid));
> > > - trace_v9fs_attach_return(pdu->tag, pdu->id,
> > > - qid.type, qid.version, qid.path);
> > > +
> > > /*
> > > * disable migration if we haven't done already.
> > > * attach could get called multiple times for the same export.
> > > */
> > > if (!s->migration_blocker) {
> > > - s->root_fid = fid;
> > > + int ret;
> > > error_setg(&s->migration_blocker,
> > > "Migration is disabled when VirtFS export path '%s'
> > is mounted in the guest using mount_tag '%s'",
> > > s->ctx.fs_root ? s->ctx.fs_root : "NULL", s->tag);
> > > - migrate_add_blocker(s->migration_blocker);
> > > + ret = migrate_add_blocker(s->migration_blocker, NULL);
> > > + if (ret < 0) {
> > > + err = ret;
> > > + clunk_fid(s, fid);
> > > + error_free(s->migration_blocker);
> > > + s->migration_blocker = NULL;
> >
> > It's better to rollback things in reverse order (i.e. clunk_fid() should
> > be called here).
>
>
> Done.
>
>
> >
> > > + goto out;
> > > + }
> > > + s->root_fid = fid;
> > > }
> >
> > I now realize that all the migration blocker stuff should be done before
> > we call pdu_marshal() since we may now return an error to the guest. And
>
>
>
> Yes, migration blocker needs to be allowed to do its bit first and it might
> fail and return an error.
>
> thus, you can drop ret and use err directly.
>
>
> Alright. I will drop it.
>
> Also you suggested to use EBUSY instead of EACCES but then it will be
> difficult to differentiate which case failed migration blocker as both will
> end up returning the same error number.
>
IIRC this was a suggestion for patch 4/4 but I now retract :) It makes
sense that mount(2) in the guest fails with EBUSY while migration is in
progress: the guest may retry later and it will hopefully succeed. In
the --only-migratable case, mount will always fail so EACCES is ok.
> Ashijeet
>
>
> > Cheers.
> >
> > --
> > Greg
> >
> > > +
> > > + memcpy(&s->root_qid, &qid, sizeof(qid));
> > > + trace_v9fs_attach_return(pdu->tag, pdu->id,
> > > + qid.type, qid.version, qid.path);
> > > out:
> > > put_fid(pdu, fidp);
> > > out_nofid:
> > > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> > > index 5f32e1a..6e60b8c 100644
> > > --- a/hw/display/virtio-gpu.c
> > > +++ b/hw/display/virtio-gpu.c
> > > @@ -1108,14 +1108,6 @@ static void virtio_gpu_device_realize(DeviceState
> > *qdev, Error **errp)
> > > return;
> > > }
> > >
> > > - g->config_size = sizeof(struct virtio_gpu_config);
> > > - g->virtio_config.num_scanouts = g->conf.max_outputs;
> > > - virtio_init(VIRTIO_DEVICE(g), "virtio-gpu", VIRTIO_ID_GPU,
> > > - g->config_size);
> > > -
> > > - g->req_state[0].width = 1024;
> > > - g->req_state[0].height = 768;
> > > -
> > > g->use_virgl_renderer = false;
> > > #if !defined(CONFIG_VIRGL) || defined(HOST_WORDS_BIGENDIAN)
> > > have_virgl = false;
> > > @@ -1127,6 +1119,22 @@ static void virtio_gpu_device_realize(DeviceState
> > *qdev, Error **errp)
> > > }
> > >
> > > if (virtio_gpu_virgl_enabled(g->conf)) {
> > > + error_setg(&g->migration_blocker, "virgl is not yet
> > migratable");
> > > + if (migrate_add_blocker(g->migration_blocker, errp) < 0) {
> > > + error_free(g->migration_blocker);
> > > + return;
> > > + }
> > > + }
> > > +
> > > + g->config_size = sizeof(struct virtio_gpu_config);
> > > + g->virtio_config.num_scanouts = g->conf.max_outputs;
> > > + virtio_init(VIRTIO_DEVICE(g), "virtio-gpu", VIRTIO_ID_GPU,
> > > + g->config_size);
> > > +
> > > + g->req_state[0].width = 1024;
> > > + g->req_state[0].height = 768;
> > > +
> > > + if (virtio_gpu_virgl_enabled(g->conf)) {
> > > /* use larger control queue in 3d mode */
> > > g->ctrl_vq = virtio_add_queue(vdev, 256,
> > virtio_gpu_handle_ctrl_cb);
> > > g->cursor_vq = virtio_add_queue(vdev, 16,
> > virtio_gpu_handle_cursor_cb);
> > > @@ -1152,11 +1160,6 @@ static void virtio_gpu_device_realize(DeviceState
> > *qdev, Error **errp)
> > > dpy_gfx_replace_surface(g->scanout[i].con, NULL);
> > > }
> > > }
> > > -
> > > - if (virtio_gpu_virgl_enabled(g->conf)) {
> > > - error_setg(&g->migration_blocker, "virgl is not yet
> > migratable");
> > > - migrate_add_blocker(g->migration_blocker);
> > > - }
> > > }
> > >
> > > static void virtio_gpu_device_unrealize(DeviceState *qdev, Error
> > **errp)
> > > diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
> > > index 11729ee..3614daa 100644
> > > --- a/hw/intc/arm_gic_kvm.c
> > > +++ b/hw/intc/arm_gic_kvm.c
> > > @@ -510,6 +510,16 @@ static void kvm_arm_gic_realize(DeviceState *dev,
> > Error **errp)
> > > return;
> > > }
> > >
> > > + if (!kvm_arm_gic_can_save_restore(s)) {
> > > + error_setg(&s->migration_blocker, "This operating system
> > kernel does "
> > > + "not support vGICv2
> > migration");
> > > + ret = migrate_add_blocker(s->migration_blocker, errp);
> > > + if (ret < 0) {
> > > + error_free(s->migration_blocker);
> > > + return;
> > > + }
> > > + }
> > > +
> > > gic_init_irqs_and_mmio(s, kvm_arm_gicv2_set_irq, NULL);
> > >
> > > for (i = 0; i < s->num_irq - GIC_INTERNAL; i++) {
> > > @@ -558,12 +568,6 @@ static void kvm_arm_gic_realize(DeviceState *dev,
> > Error **errp)
> > > KVM_VGIC_V2_ADDR_TYPE_CPU,
> > > s->dev_fd);
> > >
> > > - if (!kvm_arm_gic_can_save_restore(s)) {
> > > - error_setg(&s->migration_blocker, "This operating system
> > kernel does "
> > > - "not support vGICv2
> > migration");
> > > - migrate_add_blocker(s->migration_blocker);
> > > - }
> > > -
> > > if (kvm_has_gsi_routing()) {
> > > /* set up irq routing */
> > > kvm_init_irq_routing(kvm_state);
> > > diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
> > > index fc246e0..950a02f 100644
> > > --- a/hw/intc/arm_gicv3_its_kvm.c
> > > +++ b/hw/intc/arm_gicv3_its_kvm.c
> > > @@ -63,6 +63,17 @@ static void kvm_arm_its_realize(DeviceState *dev,
> > Error **errp)
> > > return;
> > > }
> > >
> > > + /*
> > > + * Block migration of a KVM GICv3 ITS device: the API for saving and
> > > + * restoring the state in the kernel is not yet available
> > > + */
> > > + error_setg(&s->migration_blocker, "vITS migration is not
> > implemented");
> > > + ret = migrate_add_blocker(s->migration_blocker, errp);
> > > + if (ret < 0) {
> > > + error_free(s->migration_blocker);
> > > + return;
> > > + }
> > > +
> > > /* explicit init of the ITS */
> > > kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
> > > KVM_DEV_ARM_VGIC_CTRL_INIT, NULL, true);
> > > @@ -73,13 +84,6 @@ static void kvm_arm_its_realize(DeviceState *dev,
> > Error **errp)
> > >
> > > gicv3_its_init_mmio(s, NULL);
> > >
> > > - /*
> > > - * Block migration of a KVM GICv3 ITS device: the API for saving and
> > > - * restoring the state in the kernel is not yet available
> > > - */
> > > - error_setg(&s->migration_blocker, "vITS migration is not
> > implemented");
> > > - migrate_add_blocker(s->migration_blocker);
> > > -
> > > kvm_msi_use_devid = true;
> > > kvm_gsi_direct_mapping = false;
> > > kvm_msi_via_irqfd_allowed = kvm_irqfds_enabled();
> > > diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
> > > index 199a439..06f6f30 100644
> > > --- a/hw/intc/arm_gicv3_kvm.c
> > > +++ b/hw/intc/arm_gicv3_kvm.c
> > > @@ -86,6 +86,7 @@ static void kvm_arm_gicv3_realize(DeviceState *dev,
> > Error **errp)
> > > KVMARMGICv3Class *kgc = KVM_ARM_GICV3_GET_CLASS(s);
> > > Error *local_err = NULL;
> > > int i;
> > > + int ret;
> > >
> > > DPRINTF("kvm_arm_gicv3_realize\n");
> > >
> > > @@ -110,6 +111,17 @@ static void kvm_arm_gicv3_realize(DeviceState
> > *dev, Error **errp)
> > > return;
> > > }
> > >
> > > + /* Block migration of a KVM GICv3 device: the API for saving and
> > restoring
> > > + * the state in the kernel is not yet finalised in the kernel or
> > > + * implemented in QEMU.
> > > + */
> > > + error_setg(&s->migration_blocker, "vGICv3 migration is not
> > implemented");
> > > + ret = migrate_add_blocker(s->migration_blocker, errp);
> > > + if (ret < 0) {
> > > + error_free(s->migration_blocker);
> > > + return;
> > > + }
> > > +
> > > kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_NR_IRQS,
> > > 0, &s->num_irq, true);
> > >
> > > @@ -122,13 +134,6 @@ static void kvm_arm_gicv3_realize(DeviceState
> > *dev, Error **errp)
> > > kvm_arm_register_device(&s->iomem_redist, -1,
> > KVM_DEV_ARM_VGIC_GRP_ADDR,
> > > KVM_VGIC_V3_ADDR_TYPE_REDIST, s->dev_fd);
> > >
> > > - /* Block migration of a KVM GICv3 device: the API for saving and
> > restoring
> > > - * the state in the kernel is not yet finalised in the kernel or
> > > - * implemented in QEMU.
> > > - */
> > > - error_setg(&s->migration_blocker, "vGICv3 migration is not
> > implemented");
> > > - migrate_add_blocker(s->migration_blocker);
> > > -
> > > if (kvm_has_gsi_routing()) {
> > > /* set up irq routing */
> > > kvm_init_irq_routing(kvm_state);
> > > diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> > > index abeaf3d..585cc5b 100644
> > > --- a/hw/misc/ivshmem.c
> > > +++ b/hw/misc/ivshmem.c
> > > @@ -903,9 +903,6 @@ static void ivshmem_common_realize(PCIDevice *dev,
> > Error **errp)
> > > }
> > > }
> > >
> > > - vmstate_register_ram(s->ivshmem_bar2, DEVICE(s));
> > > - pci_register_bar(PCI_DEVICE(s), 2, attr, s->ivshmem_bar2);
> > > -
> > > if (s->master == ON_OFF_AUTO_AUTO) {
> > > s->master = s->vm_id == 0 ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
> > > }
> > > @@ -913,8 +910,14 @@ static void ivshmem_common_realize(PCIDevice *dev,
> > Error **errp)
> > > if (!ivshmem_is_master(s)) {
> > > error_setg(&s->migration_blocker,
> > > "Migration is disabled when using feature 'peer
> > mode' in device 'ivshmem'");
> > > - migrate_add_blocker(s->migration_blocker);
> > > + if (migrate_add_blocker(s->migration_blocker, errp) < 0) {
> > > + error_free(s->migration_blocker);
> > > + return;
> > > + }
> > > }
> > > +
> > > + vmstate_register_ram(s->ivshmem_bar2, DEVICE(s));
> > > + pci_register_bar(PCI_DEVICE(s), 2, attr, s->ivshmem_bar2);
> > > }
> > >
> > > static void ivshmem_exit(PCIDevice *dev)
> > > diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
> > > index 5b26946..ff503d0 100644
> > > --- a/hw/scsi/vhost-scsi.c
> > > +++ b/hw/scsi/vhost-scsi.c
> > > @@ -238,8 +238,14 @@ static void vhost_scsi_realize(DeviceState *dev,
> > Error **errp)
> > > vhost_dummy_handle_output);
> > > if (err != NULL) {
> > > error_propagate(errp, err);
> > > - close(vhostfd);
> > > - return;
> > > + goto close_fd;
> > > + }
> > > +
> > > + error_setg(&s->migration_blocker,
> > > + "vhost-scsi does not support migration");
> > > + ret = migrate_add_blocker(s->migration_blocker, errp);
> > > + if (ret < 0) {
> > > + goto free_blocker;
> > > }
> > >
> > > s->dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs->conf.num_queues;
> > > @@ -252,7 +258,7 @@ static void vhost_scsi_realize(DeviceState *dev,
> > Error **errp)
> > > if (ret < 0) {
> > > error_setg(errp, "vhost-scsi: vhost initialization failed: %s",
> > > strerror(-ret));
> > > - return;
> > > + goto free_vqs;
> > > }
> > >
> > > /* At present, channel and lun both are 0 for bootable vhost-scsi
> > disk */
> > > @@ -261,9 +267,16 @@ static void vhost_scsi_realize(DeviceState *dev,
> > Error **errp)
> > > /* Note: we can also get the minimum tpgt from kernel */
> > > s->target = vs->conf.boot_tpgt;
> > >
> > > - error_setg(&s->migration_blocker,
> > > - "vhost-scsi does not support migration");
> > > - migrate_add_blocker(s->migration_blocker);
> > > + return;
> > > +
> > > + free_vqs:
> > > + migrate_del_blocker(s->migration_blocker);
> > > + g_free(s->dev.vqs);
> > > + free_blocker:
> > > + error_free(s->migration_blocker);
> > > + close_fd:
> > > + close(vhostfd);
> > > + return;
> > > }
> > >
> > > static void vhost_scsi_unrealize(DeviceState *dev, Error **errp)
> > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > > index f7f7023..416fada 100644
> > > --- a/hw/virtio/vhost.c
> > > +++ b/hw/virtio/vhost.c
> > > @@ -1157,7 +1157,13 @@ int vhost_dev_init(struct vhost_dev *hdev, void
> > *opaque,
> > > }
> > >
> > > if (hdev->migration_blocker != NULL) {
> > > - migrate_add_blocker(hdev->migration_blocker);
> > > + Error *local_err;
> > > + r = migrate_add_blocker(hdev->migration_blocker, &local_err);
> > > + if (r < 0) {
> > > + error_report_err(local_err);
> > > + error_free(hdev->migration_blocker);
> > > + goto fail_busyloop;
> > > + }
> > > }
> > >
> > > hdev->mem = g_malloc0(offsetof(struct vhost_memory, regions));
> > > diff --git a/include/migration/migration.h
> > b/include/migration/migration.h
> > > index 40b3697..46a4bb5 100644
> > > --- a/include/migration/migration.h
> > > +++ b/include/migration/migration.h
> > > @@ -243,6 +243,7 @@ void remove_migration_state_change_notifier(Notifier
> > *notify);
> > > MigrationState *migrate_init(const MigrationParams *params);
> > > bool migration_is_blocked(Error **errp);
> > > bool migration_in_setup(MigrationState *);
> > > +bool migration_is_idle(MigrationState *s);
> > > bool migration_has_finished(MigrationState *);
> > > bool migration_has_failed(MigrationState *);
> > > /* True if outgoing migration has entered postcopy phase */
> > > @@ -287,8 +288,11 @@ int ram_postcopy_incoming_init(MigrationIncomingState
> > *mis);
> > > * @migrate_add_blocker - prevent migration from proceeding
> > > *
> > > * @reason - an error to be returned whenever migration is attempted
> > > + * @errp - [out] The reason (if any) we cannot block migration right
> > now.
> > > + *
> > > + * @returns - 0 on success, -EBUSY on failure, with errp set.
> > > */
> > > -void migrate_add_blocker(Error *reason);
> > > +int migrate_add_blocker(Error *reason, Error **errp);
> > >
> > > /**
> > > * @migrate_del_blocker - remove a blocking error from migration
> > > diff --git a/migration/migration.c b/migration/migration.c
> > > index f498ab8..713e012 100644
> > > --- a/migration/migration.c
> > > +++ b/migration/migration.c
> > > @@ -1044,6 +1044,31 @@ bool migration_in_postcopy_after_devices(MigrationState
> > *s)
> > > return migration_in_postcopy(s) && s->postcopy_after_devices;
> > > }
> > >
> > > +bool migration_is_idle(MigrationState *s)
> > > +{
> > > + if (!s) {
> > > + s = migrate_get_current();
> > > + }
> > > +
> > > + switch (s->state) {
> > > + case MIGRATION_STATUS_NONE:
> > > + case MIGRATION_STATUS_CANCELLED:
> > > + case MIGRATION_STATUS_COMPLETED:
> > > + case MIGRATION_STATUS_FAILED:
> > > + return true;
> > > + case MIGRATION_STATUS_SETUP:
> > > + case MIGRATION_STATUS_CANCELLING:
> > > + case MIGRATION_STATUS_ACTIVE:
> > > + case MIGRATION_STATUS_POSTCOPY_ACTIVE:
> > > + case MIGRATION_STATUS_COLO:
> > > + return false;
> > > + case MIGRATION_STATUS__MAX:
> > > + g_assert_not_reached();
> > > + }
> > > +
> > > + return false;
> > > +}
> > > +
> > > MigrationState *migrate_init(const MigrationParams *params)
> > > {
> > > MigrationState *s = migrate_get_current();
> > > @@ -1086,9 +1111,15 @@ MigrationState *migrate_init(const
> > MigrationParams *params)
> > >
> > > static GSList *migration_blockers;
> > >
> > > -void migrate_add_blocker(Error *reason)
> > > +int migrate_add_blocker(Error *reason, Error **errp)
> > > {
> > > - migration_blockers = g_slist_prepend(migration_blockers, reason);
> > > + if (migration_is_idle(NULL)) {
> > > + migration_blockers = g_slist_prepend(migration_blockers,
> > reason);
> > > + return 0;
> > > + }
> > > +
> > > + error_setg(errp, "Cannot block a migration already in-progress");
> > > + return -EBUSY;
> > > }
> > >
> > > void migrate_del_blocker(Error *reason)
> > > diff --git a/stubs/migr-blocker.c b/stubs/migr-blocker.c
> > > index 8ab3604..a5ba18f 100644
> > > --- a/stubs/migr-blocker.c
> > > +++ b/stubs/migr-blocker.c
> > > @@ -2,8 +2,9 @@
> > > #include "qemu-common.h"
> > > #include "migration/migration.h"
> > >
> > > -void migrate_add_blocker(Error *reason)
> > > +int migrate_add_blocker(Error *reason, Error **errp)
> > > {
> > > + return 0;
> > > }
> > >
> > > void migrate_del_blocker(Error *reason)
> > > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > > index f62264a..6031a1c 100644
> > > --- a/target-i386/kvm.c
> > > +++ b/target-i386/kvm.c
> > > @@ -961,7 +961,11 @@ int kvm_arch_init_vcpu(CPUState *cs)
> > > error_setg(&invtsc_mig_blocker,
> > > "State blocked by non-migratable CPU device"
> > > " (invtsc flag)");
> > > - migrate_add_blocker(invtsc_mig_blocker);
> > > + r = migrate_add_blocker(invtsc_mig_blocker, NULL);
> > > + if (r < 0) {
> > > + error_report("kvm: migrate_add_blocker failed");
> > > + goto efail;
> > > + }
> > > /* for savevm */
> > > vmstate_x86_cpu.unmigratable = 1;
> > > }
> > > @@ -969,12 +973,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
> > > cpuid_data.cpuid.padding = 0;
> > > r = kvm_vcpu_ioctl(cs, KVM_SET_CPUID2, &cpuid_data);
> > > if (r) {
> > > - return r;
> > > + goto fail;
> > > }
> > >
> > > r = kvm_arch_set_tsc_khz(cs);
> > > if (r < 0) {
> > > - return r;
> > > + goto fail;
> > > }
> > >
> > > /* vcpu's TSC frequency is either specified by user, or following
> > > @@ -1001,6 +1005,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
> > > }
> > >
> > > return 0;
> > > +
> > > + fail:
> > > + migrate_del_blocker(invtsc_mig_blocker);
> > > + efail:
> > > + error_free(invtsc_mig_blocker);
> > > + return r;
> > > }
> > >
> > > void kvm_arch_reset_vcpu(X86CPU *cpu)
> >
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/4] migration: Fail migration blocker for --only-migratble
2017-01-08 7:25 ` [Qemu-devel] [PATCH v2 " Ashijeet Acharya
@ 2017-01-09 8:09 ` Greg Kurz
2017-01-09 11:36 ` Ashijeet Acharya
0 siblings, 1 reply; 13+ messages in thread
From: Greg Kurz @ 2017-01-09 8:09 UTC (permalink / raw)
To: Ashijeet Acharya
Cc: dgilbert, jsnow, amit.shah, pbonzini, kwolf, armbru, quintela,
mst, marcandre.lureau, aneesh.kumar, peter.maydell, qemu-devel
Again v3 turned into v2... what happened ?
On Sun, 8 Jan 2017 12:55:51 +0530
Ashijeet Acharya <ashijeetacharya@gmail.com> wrote:
> On Friday, January 6, 2017, Greg Kurz <groug@kaod.org
> <javascript:_e(%7B%7D,'cvml','groug@kaod.org');>> wrote:
>
> > On Wed, 4 Jan 2017 18:02:29 +0530
> > Ashijeet Acharya <ashijeetacharya@gmail.com> wrote:
> >
> > > migrate_add_blocker should rightly fail if the '--only-migratable'
> > > option was specified and the device in use should not be able to
> > > perform the action which results in an unmigratable VM.
> > >
> > > Make migrate_add_blocker return -EACCES in this case.
> > >
> > > Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
> > > ---
> > > block/qcow.c | 5 ++++-
> > > block/vdi.c | 5 ++++-
> > > block/vhdx.c | 6 ++++--
> > > block/vmdk.c | 5 ++++-
> > > block/vpc.c | 5 ++++-
> > > block/vvfat.c | 6 ++++--
> > > hw/9pfs/9p.c | 8 +++++++-
> > > hw/display/virtio-gpu.c | 9 +++++++--
> > > hw/intc/arm_gic_kvm.c | 5 ++++-
> > > hw/intc/arm_gicv3_its_kvm.c | 5 ++++-
> > > hw/intc/arm_gicv3_kvm.c | 5 ++++-
> > > hw/misc/ivshmem.c | 10 ++++++++--
> > > hw/scsi/vhost-scsi.c | 8 +++++---
> > > hw/virtio/vhost.c | 6 ++++--
> > > migration/migration.c | 7 +++++++
> > > target-i386/kvm.c | 5 ++++-
> > > 16 files changed, 78 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/block/qcow.c b/block/qcow.c
> > > index 11526a1..bdc6446 100644
> > > --- a/block/qcow.c
> > > +++ b/block/qcow.c
> > > @@ -254,7 +254,10 @@ static int qcow_open(BlockDriverState *bs, QDict
> > *options, int flags,
> > > bdrv_get_device_or_node_name(bs));
> > > ret = migrate_add_blocker(s->migration_blocker, errp);
> > > if (ret < 0) {
> > > - error_free(s->migration_blocker);
> > > + if (ret == -EACCES) {
> > > + error_append_hint(errp, "Cannot use a node with qcow format
> > as "
> > > + "it does not support live migration");
> > > + }
> > > goto fail;
> > > }
> > >
> > > diff --git a/block/vdi.c b/block/vdi.c
> > > index 2b56f52..0b011ea 100644
> > > --- a/block/vdi.c
> > > +++ b/block/vdi.c
> > > @@ -473,7 +473,10 @@ static int vdi_open(BlockDriverState *bs, QDict
> > *options, int flags,
> > > bdrv_get_device_or_node_name(bs));
> > > ret = migrate_add_blocker(s->migration_blocker, errp);
> > > if (ret < 0) {
> > > - error_free(s->migration_blocker);
> > > + if (ret == -EACCES) {
> > > + error_append_hint(errp, "Cannot use a node with vdi format
> > as "
> > > + "it does not support live migration");
> > > + }
> > > goto fail_free_bmap;
> > > }
> > >
> > > diff --git a/block/vhdx.c b/block/vhdx.c
> > > index d81941b..b8d53ec 100644
> > > --- a/block/vhdx.c
> > > +++ b/block/vhdx.c
> > > @@ -997,10 +997,12 @@ static int vhdx_open(BlockDriverState *bs, QDict
> > *options, int flags,
> > > bdrv_get_device_or_node_name(bs));
> > > ret = migrate_add_blocker(s->migration_blocker, errp);
> > > if (ret < 0) {
> > > - error_free(s->migration_blocker);
> > > + if (ret == -EACCES) {
> > > + error_append_hint(errp, "Cannot use a node with vhdx format
> > as "
> > > + "it does not support live migration");
> > > + }
> > > goto fail;
> > > }
> > > -
> > > if (flags & BDRV_O_RDWR) {
> > > ret = vhdx_update_headers(bs, s, false, NULL);
> > > if (ret < 0) {
> > > diff --git a/block/vmdk.c b/block/vmdk.c
> > > index 4a45a83..defe400 100644
> > > --- a/block/vmdk.c
> > > +++ b/block/vmdk.c
> > > @@ -978,7 +978,10 @@ static int vmdk_open(BlockDriverState *bs, QDict
> > *options, int flags,
> > > bdrv_get_device_or_node_name(bs));
> > > ret = migrate_add_blocker(s->migration_blocker, errp);
> > > if (ret < 0) {
> > > - error_free(s->migration_blocker);
> > > + if (ret == -EACCES) {
> > > + error_append_hint(errp, "Cannot use a node with vmdk format
> > as "
> > > + "it does not support live migration");
> > > + }
> > > goto fail;
> > > }
> > >
> > > diff --git a/block/vpc.c b/block/vpc.c
> > > index 299a8c8..5e58d77 100644
> > > --- a/block/vpc.c
> > > +++ b/block/vpc.c
> > > @@ -428,7 +428,10 @@ static int vpc_open(BlockDriverState *bs, QDict
> > *options, int flags,
> > > bdrv_get_device_or_node_name(bs));
> > > ret = migrate_add_blocker(s->migration_blocker, errp);
> > > if (ret < 0) {
> > > - error_free(s->migration_blocker);
> > > + if (ret == -EACCES) {
> > > + error_append_hint(errp, "Cannot use a node with vpc format
> > as "
> > > + "it does not support live migration");
> > > + }
> > > goto fail;
> > > }
> > >
> > > diff --git a/block/vvfat.c b/block/vvfat.c
> > > index 3de3320..5a6e038 100644
> > > --- a/block/vvfat.c
> > > +++ b/block/vvfat.c
> > > @@ -1193,7 +1193,10 @@ static int vvfat_open(BlockDriverState *bs, QDict
> > *options, int flags,
> > > bdrv_get_device_or_node_name(bs));
> > > ret = migrate_add_blocker(s->migration_blocker, errp);
> > > if (ret < 0) {
> > > - error_free(s->migration_blocker);
> > > + if (ret == -EACCES) {
> > > + error_append_hint(errp, "Cannot use a node with vvfat
> > format "
> > > + "as it does not support live
> > migration");
> > > + }
> > > goto fail;
> > > }
> > > }
> > > @@ -1202,7 +1205,6 @@ static int vvfat_open(BlockDriverState *bs, QDict
> > *options, int flags,
> > > init_mbr(s, cyls, heads, secs);
> > > }
> > >
> > > - // assert(is_consistent(s));
> > > qemu_co_mutex_init(&s->lock);
> > >
> > > ret = 0;
> > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > > index 43cb065..3b4fd24 100644
> > > --- a/hw/9pfs/9p.c
> > > +++ b/hw/9pfs/9p.c
> > > @@ -1020,17 +1020,23 @@ static void coroutine_fn v9fs_attach(void
> > *opaque)
> > > */
> > > if (!s->migration_blocker) {
> > > int ret;
> > > + Error *local_err;
> > > error_setg(&s->migration_blocker,
> > > "Migration is disabled when VirtFS export path '%s'
> > is mounted in the guest using mount_tag '%s'",
> > > s->ctx.fs_root ? s->ctx.fs_root : "NULL", s->tag);
> > > - ret = migrate_add_blocker(s->migration_blocker, NULL);
> > > + ret = migrate_add_blocker(s->migration_blocker, &local_err);
> > > if (ret < 0) {
> > > + if (ret == -EACCES) {
> > > + error_append_hint(&local_err, "Failed to mount VirtFS
> > as it "
> > > + "does not support live migration");
> > > + }
> > > err = ret;
> > > clunk_fid(s, fid);
> > > error_free(s->migration_blocker);
> > > s->migration_blocker = NULL;
> > > goto out;
> > > }
> > > + error_free(local_err);
> >
> > Thinking again, I suggest you simply drop these changes: v9fs_attach() is
> > triggered by the guest and doesn't cause QEMU to fail, so we don't really
> > care.
> >
> > This might be naive but I didn't quite understand which changes you are
> implying here. V2 to V3 or the entire diff in 9pfs?
>
The entire diff of 4/4 in 9pfs, which has no effect actually since local_err
is not even passed to error_report_err() :) and anyway, this would allow
a guest to fill the QEMU log with error messages if it deviced to call
mount(2) repeatedly.
> Ashijeet
>
> > Cheers.
> >
> > --
> > Greg
> >
> > > s->root_fid = fid;
> > > }
> > >
> > > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> > > index 6e60b8c..fad95bf 100644
> > > --- a/hw/display/virtio-gpu.c
> > > +++ b/hw/display/virtio-gpu.c
> > > @@ -1102,6 +1102,7 @@ static void virtio_gpu_device_realize(DeviceState
> > *qdev, Error **errp)
> > > VirtIOGPU *g = VIRTIO_GPU(qdev);
> > > bool have_virgl;
> > > int i;
> > > + int ret;
> > >
> > > if (g->conf.max_outputs > VIRTIO_GPU_MAX_SCANOUTS) {
> > > error_setg(errp, "invalid max_outputs > %d",
> > VIRTIO_GPU_MAX_SCANOUTS);
> > > @@ -1120,8 +1121,12 @@ static void virtio_gpu_device_realize(DeviceState
> > *qdev, Error **errp)
> > >
> > > if (virtio_gpu_virgl_enabled(g->conf)) {
> > > error_setg(&g->migration_blocker, "virgl is not yet
> > migratable");
> > > - if (migrate_add_blocker(g->migration_blocker, errp) < 0) {
> > > - error_free(g->migration_blocker);
> > > + ret = migrate_add_blocker(g->migration_blocker, errp);
> > > + if (ret < 0) {
> > > + if (ret == -EACCES) {
> > > + error_append_hint(errp, "Cannot use virgl as it does
> > not "
> > > + "support live migration yet");
> > > + }
> > > return;
> > > }
> > > }
> > > diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
> > > index 3614daa..c2ef236 100644
> > > --- a/hw/intc/arm_gic_kvm.c
> > > +++ b/hw/intc/arm_gic_kvm.c
> > > @@ -515,7 +515,10 @@ static void kvm_arm_gic_realize(DeviceState *dev,
> > Error **errp)
> > > "not support vGICv2
> > migration");
> > > ret = migrate_add_blocker(s->migration_blocker, errp);
> > > if (ret < 0) {
> > > - error_free(s->migration_blocker);
> > > + if (ret == -EACCES) {
> > > + error_append_hint(errp, "Failed to realize vGICv2 as
> > its"
> > > + " migrataion is not implemented yet");
> > > + }
> > > return;
> > > }
> > > }
> > > diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
> > > index 950a02f..3474642 100644
> > > --- a/hw/intc/arm_gicv3_its_kvm.c
> > > +++ b/hw/intc/arm_gicv3_its_kvm.c
> > > @@ -70,7 +70,10 @@ static void kvm_arm_its_realize(DeviceState *dev,
> > Error **errp)
> > > error_setg(&s->migration_blocker, "vITS migration is not
> > implemented");
> > > ret = migrate_add_blocker(s->migration_blocker, errp);
> > > if (ret < 0) {
> > > - error_free(s->migration_blocker);
> > > + if (ret == -EACCES) {
> > > + error_append_hint(errp, "Failed to realize vITS as its
> > migration "
> > > + "is not implemented yet");
> > > + }
> > > return;
> > > }
> > >
> > > diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
> > > index 06f6f30..be7b97c 100644
> > > --- a/hw/intc/arm_gicv3_kvm.c
> > > +++ b/hw/intc/arm_gicv3_kvm.c
> > > @@ -118,7 +118,10 @@ static void kvm_arm_gicv3_realize(DeviceState
> > *dev, Error **errp)
> > > error_setg(&s->migration_blocker, "vGICv3 migration is not
> > implemented");
> > > ret = migrate_add_blocker(s->migration_blocker, errp);
> > > if (ret < 0) {
> > > - error_free(s->migration_blocker);
> > > + if (ret == -EACCES) {
> > > + error_append_hint(errp, "Failed to realize vGICv3 as its
> > migration"
> > > + "is not implemented yet");
> > > + }
> > > return;
> > > }
> > >
> > > diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> > > index 585cc5b..14ed60d 100644
> > > --- a/hw/misc/ivshmem.c
> > > +++ b/hw/misc/ivshmem.c
> > > @@ -837,6 +837,7 @@ static void ivshmem_common_realize(PCIDevice *dev,
> > Error **errp)
> > > {
> > > IVShmemState *s = IVSHMEM_COMMON(dev);
> > > Error *err = NULL;
> > > + int ret;
> > > uint8_t *pci_conf;
> > > uint8_t attr = PCI_BASE_ADDRESS_SPACE_MEMORY |
> > > PCI_BASE_ADDRESS_MEM_PREFETCH;
> > > @@ -910,8 +911,13 @@ static void ivshmem_common_realize(PCIDevice *dev,
> > Error **errp)
> > > if (!ivshmem_is_master(s)) {
> > > error_setg(&s->migration_blocker,
> > > "Migration is disabled when using feature 'peer
> > mode' in device 'ivshmem'");
> > > - if (migrate_add_blocker(s->migration_blocker, errp) < 0) {
> > > - error_free(s->migration_blocker);
> > > + ret = migrate_add_blocker(s->migration_blocker, errp);
> > > + if (ret < 0) {
> > > + if (ret == -EACCES) {
> > > + error_append_hint(errp, "Cannot use the 'peer mode'
> > feature "
> > > + "in device 'ivshmem' as it does not "
> > > + "support live migration");
> > > + }
> > > return;
> > > }
> > > }
> > > diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
> > > index ff503d0..bb731b2 100644
> > > --- a/hw/scsi/vhost-scsi.c
> > > +++ b/hw/scsi/vhost-scsi.c
> > > @@ -245,7 +245,11 @@ static void vhost_scsi_realize(DeviceState *dev,
> > Error **errp)
> > > "vhost-scsi does not support migration");
> > > ret = migrate_add_blocker(s->migration_blocker, errp);
> > > if (ret < 0) {
> > > - goto free_blocker;
> > > + if (ret == -EACCES) {
> > > + error_append_hint(errp, "Cannot use vhost-scsi as it does
> > not "
> > > + "support live migration");
> > > + }
> > > + goto close_fd;
> > > }
> > >
> > > s->dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs->conf.num_queues;
> > > @@ -272,8 +276,6 @@ static void vhost_scsi_realize(DeviceState *dev,
> > Error **errp)
> > > free_vqs:
> > > migrate_del_blocker(s->migration_blocker);
> > > g_free(s->dev.vqs);
> > > - free_blocker:
> > > - error_free(s->migration_blocker);
> > > close_fd:
> > > close(vhostfd);
> > > return;
> > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > > index 416fada..6d2375a 100644
> > > --- a/hw/virtio/vhost.c
> > > +++ b/hw/virtio/vhost.c
> > > @@ -1160,8 +1160,10 @@ int vhost_dev_init(struct vhost_dev *hdev, void
> > *opaque,
> > > Error *local_err;
> > > r = migrate_add_blocker(hdev->migration_blocker, &local_err);
> > > if (r < 0) {
> > > - error_report_err(local_err);
> > > - error_free(hdev->migration_blocker);
> > > + if (r == -EACCES) {
> > > + error_append_hint(&local_err, "Cannot use vhost drivers
> > as "
> > > + "it does not support live migration");
> > > + }
> > > goto fail_busyloop;
> > > }
> > > }
> > > diff --git a/migration/migration.c b/migration/migration.c
> > > index 713e012..ab34328 100644
> > > --- a/migration/migration.c
> > > +++ b/migration/migration.c
> > > @@ -1115,9 +1115,16 @@ int migrate_add_blocker(Error *reason, Error
> > **errp)
> > > {
> > > if (migration_is_idle(NULL)) {
> > > migration_blockers = g_slist_prepend(migration_blockers,
> > reason);
> > > + error_free(reason);
> > > return 0;
> > > }
> > >
> > > + if (only_migratable) {
> > > + error_setg(errp, "Failed to add migration blocker:
> > --only-migratable "
> > > + "was specified");
> > > + return -EACCES;
> > > + }
> > > +
> > > error_setg(errp, "Cannot block a migration already in-progress");
> > > return -EBUSY;
> > > }
> > > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > > index 6031a1c..f2c35d8 100644
> > > --- a/target-i386/kvm.c
> > > +++ b/target-i386/kvm.c
> > > @@ -963,6 +963,10 @@ int kvm_arch_init_vcpu(CPUState *cs)
> > > " (invtsc flag)");
> > > r = migrate_add_blocker(invtsc_mig_blocker, NULL);
> > > if (r < 0) {
> > > + if (r == -EACCES) {
> > > + error_report("kvm: non-migratable CPU device but"
> > > + " --only-migratable was specified");
> > > + }
> > > error_report("kvm: migrate_add_blocker failed");
> > > goto efail;
> > > }
> > > @@ -1009,7 +1013,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
> > > fail:
> > > migrate_del_blocker(invtsc_mig_blocker);
> > > efail:
> > > - error_free(invtsc_mig_blocker);
> > > return r;
> > > }
> > >
> >
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/4] migration: Fail migration blocker for --only-migratble
2017-01-09 8:09 ` Greg Kurz
@ 2017-01-09 11:36 ` Ashijeet Acharya
0 siblings, 0 replies; 13+ messages in thread
From: Ashijeet Acharya @ 2017-01-09 11:36 UTC (permalink / raw)
To: Greg Kurz
Cc: dgilbert, jsnow, amit.shah, pbonzini, kwolf, armbru, quintela,
mst, marcandre.lureau, aneesh.kumar, peter.maydell, qemu-devel
On Monday, 9 January 2017, Greg Kurz <groug@kaod.org> wrote:
> Again v3 turned into v2... what happened ?
I am suspecting that my email client is at fault for this :(
>
> On Sun, 8 Jan 2017 12:55:51 +0530
> Ashijeet Acharya <ashijeetacharya@gmail.com <javascript:;>> wrote:
>
> > On Friday, January 6, 2017, Greg Kurz <groug@kaod.org <javascript:;>
> > <javascript:_e(%7B%7D,'cvml','groug@kaod.org <javascript:;>');>> wrote:
> >
> > > On Wed, 4 Jan 2017 18:02:29 +0530
> > > Ashijeet Acharya <ashijeetacharya@gmail.com <javascript:;>> wrote:
> > >
> > > > migrate_add_blocker should rightly fail if the '--only-migratable'
> > > > option was specified and the device in use should not be able to
> > > > perform the action which results in an unmigratable VM.
> > > >
> > > > Make migrate_add_blocker return -EACCES in this case.
> > > >
> > > > Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com
> <javascript:;>>
> > > > ---
> > > > block/qcow.c | 5 ++++-
> > > > block/vdi.c | 5 ++++-
> > > > block/vhdx.c | 6 ++++--
> > > > block/vmdk.c | 5 ++++-
> > > > block/vpc.c | 5 ++++-
> > > > block/vvfat.c | 6 ++++--
> > > > hw/9pfs/9p.c | 8 +++++++-
> > > > hw/display/virtio-gpu.c | 9 +++++++--
> > > > hw/intc/arm_gic_kvm.c | 5 ++++-
> > > > hw/intc/arm_gicv3_its_kvm.c | 5 ++++-
> > > > hw/intc/arm_gicv3_kvm.c | 5 ++++-
> > > > hw/misc/ivshmem.c | 10 ++++++++--
> > > > hw/scsi/vhost-scsi.c | 8 +++++---
> > > > hw/virtio/vhost.c | 6 ++++--
> > > > migration/migration.c | 7 +++++++
> > > > target-i386/kvm.c | 5 ++++-
> > > > 16 files changed, 78 insertions(+), 22 deletions(-)
> > > >
> > > > diff --git a/block/qcow.c b/block/qcow.c
> > > > index 11526a1..bdc6446 100644
> > > > --- a/block/qcow.c
> > > > +++ b/block/qcow.c
> > > > @@ -254,7 +254,10 @@ static int qcow_open(BlockDriverState *bs, QDict
> > > *options, int flags,
> > > > bdrv_get_device_or_node_name(bs));
> > > > ret = migrate_add_blocker(s->migration_blocker, errp);
> > > > if (ret < 0) {
> > > > - error_free(s->migration_blocker);
> > > > + if (ret == -EACCES) {
> > > > + error_append_hint(errp, "Cannot use a node with qcow
> format
> > > as "
> > > > + "it does not support live migration");
> > > > + }
> > > > goto fail;
> > > > }
> > > >
> > > > diff --git a/block/vdi.c b/block/vdi.c
> > > > index 2b56f52..0b011ea 100644
> > > > --- a/block/vdi.c
> > > > +++ b/block/vdi.c
> > > > @@ -473,7 +473,10 @@ static int vdi_open(BlockDriverState *bs, QDict
> > > *options, int flags,
> > > > bdrv_get_device_or_node_name(bs));
> > > > ret = migrate_add_blocker(s->migration_blocker, errp);
> > > > if (ret < 0) {
> > > > - error_free(s->migration_blocker);
> > > > + if (ret == -EACCES) {
> > > > + error_append_hint(errp, "Cannot use a node with vdi
> format
> > > as "
> > > > + "it does not support live migration");
> > > > + }
> > > > goto fail_free_bmap;
> > > > }
> > > >
> > > > diff --git a/block/vhdx.c b/block/vhdx.c
> > > > index d81941b..b8d53ec 100644
> > > > --- a/block/vhdx.c
> > > > +++ b/block/vhdx.c
> > > > @@ -997,10 +997,12 @@ static int vhdx_open(BlockDriverState *bs,
> QDict
> > > *options, int flags,
> > > > bdrv_get_device_or_node_name(bs));
> > > > ret = migrate_add_blocker(s->migration_blocker, errp);
> > > > if (ret < 0) {
> > > > - error_free(s->migration_blocker);
> > > > + if (ret == -EACCES) {
> > > > + error_append_hint(errp, "Cannot use a node with vhdx
> format
> > > as "
> > > > + "it does not support live migration");
> > > > + }
> > > > goto fail;
> > > > }
> > > > -
> > > > if (flags & BDRV_O_RDWR) {
> > > > ret = vhdx_update_headers(bs, s, false, NULL);
> > > > if (ret < 0) {
> > > > diff --git a/block/vmdk.c b/block/vmdk.c
> > > > index 4a45a83..defe400 100644
> > > > --- a/block/vmdk.c
> > > > +++ b/block/vmdk.c
> > > > @@ -978,7 +978,10 @@ static int vmdk_open(BlockDriverState *bs, QDict
> > > *options, int flags,
> > > > bdrv_get_device_or_node_name(bs));
> > > > ret = migrate_add_blocker(s->migration_blocker, errp);
> > > > if (ret < 0) {
> > > > - error_free(s->migration_blocker);
> > > > + if (ret == -EACCES) {
> > > > + error_append_hint(errp, "Cannot use a node with vmdk
> format
> > > as "
> > > > + "it does not support live migration");
> > > > + }
> > > > goto fail;
> > > > }
> > > >
> > > > diff --git a/block/vpc.c b/block/vpc.c
> > > > index 299a8c8..5e58d77 100644
> > > > --- a/block/vpc.c
> > > > +++ b/block/vpc.c
> > > > @@ -428,7 +428,10 @@ static int vpc_open(BlockDriverState *bs, QDict
> > > *options, int flags,
> > > > bdrv_get_device_or_node_name(bs));
> > > > ret = migrate_add_blocker(s->migration_blocker, errp);
> > > > if (ret < 0) {
> > > > - error_free(s->migration_blocker);
> > > > + if (ret == -EACCES) {
> > > > + error_append_hint(errp, "Cannot use a node with vpc
> format
> > > as "
> > > > + "it does not support live migration");
> > > > + }
> > > > goto fail;
> > > > }
> > > >
> > > > diff --git a/block/vvfat.c b/block/vvfat.c
> > > > index 3de3320..5a6e038 100644
> > > > --- a/block/vvfat.c
> > > > +++ b/block/vvfat.c
> > > > @@ -1193,7 +1193,10 @@ static int vvfat_open(BlockDriverState *bs,
> QDict
> > > *options, int flags,
> > > > bdrv_get_device_or_node_name(bs));
> > > > ret = migrate_add_blocker(s->migration_blocker, errp);
> > > > if (ret < 0) {
> > > > - error_free(s->migration_blocker);
> > > > + if (ret == -EACCES) {
> > > > + error_append_hint(errp, "Cannot use a node with
> vvfat
> > > format "
> > > > + "as it does not support live
> > > migration");
> > > > + }
> > > > goto fail;
> > > > }
> > > > }
> > > > @@ -1202,7 +1205,6 @@ static int vvfat_open(BlockDriverState *bs,
> QDict
> > > *options, int flags,
> > > > init_mbr(s, cyls, heads, secs);
> > > > }
> > > >
> > > > - // assert(is_consistent(s));
> > > > qemu_co_mutex_init(&s->lock);
> > > >
> > > > ret = 0;
> > > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > > > index 43cb065..3b4fd24 100644
> > > > --- a/hw/9pfs/9p.c
> > > > +++ b/hw/9pfs/9p.c
> > > > @@ -1020,17 +1020,23 @@ static void coroutine_fn v9fs_attach(void
> > > *opaque)
> > > > */
> > > > if (!s->migration_blocker) {
> > > > int ret;
> > > > + Error *local_err;
> > > > error_setg(&s->migration_blocker,
> > > > "Migration is disabled when VirtFS export path
> '%s'
> > > is mounted in the guest using mount_tag '%s'",
> > > > s->ctx.fs_root ? s->ctx.fs_root : "NULL",
> s->tag);
> > > > - ret = migrate_add_blocker(s->migration_blocker, NULL);
> > > > + ret = migrate_add_blocker(s->migration_blocker,
> &local_err);
> > > > if (ret < 0) {
> > > > + if (ret == -EACCES) {
> > > > + error_append_hint(&local_err, "Failed to mount
> VirtFS
> > > as it "
> > > > + "does not support live
> migration");
> > > > + }
> > > > err = ret;
> > > > clunk_fid(s, fid);
> > > > error_free(s->migration_blocker);
> > > > s->migration_blocker = NULL;
> > > > goto out;
> > > > }
> > > > + error_free(local_err);
> > >
> > > Thinking again, I suggest you simply drop these changes: v9fs_attach()
> is
> > > triggered by the guest and doesn't cause QEMU to fail, so we don't
> really
> > > care.
> > >
> > > This might be naive but I didn't quite understand which changes you are
> > implying here. V2 to V3 or the entire diff in 9pfs?
> >
>
> The entire diff of 4/4 in 9pfs, which has no effect actually since
> local_err
> is not even passed to error_report_err() :) and anyway, this would allow
> a guest to fill the QEMU log with error messages if it deviced to call
> mount(2) repeatedly.
Alright, I will drop these changes. Thanks for clearing that up.
Ashijeet
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-01-09 11:36 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-04 12:32 [Qemu-devel] [PATCH v3 0/4] Introduce a new --only-migratable option Ashijeet Acharya
2017-01-04 12:32 ` [Qemu-devel] [PATCH v3 1/4] migration: Add a new option to enable only-migratable Ashijeet Acharya
2017-01-04 12:32 ` [Qemu-devel] [PATCH v3 2/4] migration: Allow "device add" options to only add migratable devices Ashijeet Acharya
2017-01-04 12:32 ` [Qemu-devel] [PATCH v3 3/4] migration: disallow migrate_add_blocker during migration Ashijeet Acharya
2017-01-06 16:18 ` Greg Kurz
2017-01-08 7:34 ` [Qemu-devel] [PATCH v2 " Ashijeet Acharya
2017-01-09 7:54 ` Greg Kurz
2017-01-04 12:32 ` [Qemu-devel] [PATCH v3 4/4] migration: Fail migration blocker for --only-migratble Ashijeet Acharya
2017-01-06 16:40 ` Greg Kurz
2017-01-08 7:25 ` [Qemu-devel] [PATCH v2 " Ashijeet Acharya
2017-01-09 8:09 ` Greg Kurz
2017-01-09 11:36 ` Ashijeet Acharya
2017-01-04 12:41 ` [Qemu-devel] [PATCH v3 0/4] Introduce a new --only-migratable option no-reply
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.