On Fri, Dec 16, 2016 at 4:07 PM, Greg Kurz <gr...@kaod.org> wrote:
> On Fri, 16 Dec 2016 15:23:43 +0530
> Ashijeet Acharya <ashijeetacha...@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 <js...@redhat.com>
>> Signed-off-by: Ashijeet Acharya <ashijeetacha...@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 | 16 ++++++++++++----
>> 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, 192 insertions(+), 75 deletions(-)
>>
>> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
>> index faebd91..e72ef43 100644
>> --- a/hw/9pfs/9p.c
>> +++ b/hw/9pfs/9p.c
>> @@ -1013,20 +1013,28 @@ 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) {
>> + int ret;
>> s->root_fid = fid;
>
> Since we may fail now, I guess ^^ should move...
>
>> 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);
>
> Please don't rely on put_fid() to rollback what was done in this
> function. You need to do the following here:
> - error_free(s->migration_blocker)
> - s->migration_blocker = NULL
Okay, I was not aware of this. I have added these two now.
>
>> + goto out;
>> + }
>
> s->root_fid = fid;
>
> ... here.
Right. I missed out on this one. I have moved it now.
Ashijeet
>
>> }
>> +
>> + 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:
