| Message ID | 1483533149-12807-5-git-send-email-ashijeetacharya@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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; > } >
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; > > } > > > >
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; > > > } > > > > > > >
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
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; }
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(-)