On Monday, 9 January 2017, Greg Kurz <gr...@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 <ashijeetacha...@gmail.com <javascript:;>> wrote: > > > On Friday, January 6, 2017, Greg Kurz <gr...@kaod.org <javascript:;> > > <javascript:_e(%7B%7D,'cvml','gr...@kaod.org <javascript:;>');>> wrote: > > > > > On Wed, 4 Jan 2017 18:02:29 +0530 > > > Ashijeet Acharya <ashijeetacha...@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 <ashijeetacha...@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