From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53470) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ayzyy-0003jY-Ja for qemu-devel@nongnu.org; Sat, 07 May 2016 07:01:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ayzyt-00077d-Fk for qemu-devel@nongnu.org; Sat, 07 May 2016 07:01:12 -0400 Received: from mail-am1on0116.outbound.protection.outlook.com ([157.56.112.116]:56240 helo=emea01-am1-obe.outbound.protection.outlook.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ayzys-00077G-4q for qemu-devel@nongnu.org; Sat, 07 May 2016 07:01:07 -0400 References: <1452770941-21582-1-git-send-email-den@openvz.org> <1452770941-21582-7-git-send-email-den@openvz.org> From: "Denis V. Lunev" Message-ID: <572DC72C.40302@openvz.org> Date: Sat, 7 May 2016 13:45:00 +0300 MIME-Version: 1.0 In-Reply-To: <1452770941-21582-7-git-send-email-den@openvz.org> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 6/8] migration, block: better select BDS for VM state loading List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: qemu-devel@nongnu.org, Kevin Wolf , Juan Quintela , Amit Shah , Markus Armbruster , Eric Blake On 01/14/2016 02:28 PM, Denis V. Lunev wrote: > This patch does 2 things: > - it merges all snapshot validity checks for load_vmstate into one function > - it now selects BDS to load VM state by availability of the state in the > snapshot > > This commit is preparatory to allow to select BDS to save snapshot for QMP. > > Signed-off-by: Denis V. Lunev > CC: Kevin Wolf > CC: Juan Quintela > CC: Amit Shah > CC: Markus Armbruster > CC: Eric Blake > --- > block/snapshot.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++ > include/block/snapshot.h | 1 + > migration/savevm.c | 33 ++------------------------ > 3 files changed, 64 insertions(+), 31 deletions(-) > > diff --git a/block/snapshot.c b/block/snapshot.c > index 2d86b88..77affbc 100644 > --- a/block/snapshot.c > +++ b/block/snapshot.c > @@ -489,3 +489,64 @@ BlockDriverState *bdrv_all_find_vmstate_bs(void) > } > return bs; > } > + > + > +static bool validate_bs(BlockDriverState *bs, BlockDriverState **vmstate_bs, > + const char *name, Error **errp) > +{ > + QEMUSnapshotInfo sn; > + > + if (!bdrv_can_snapshot(bs)) { > + if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) { > + return true; > + } > + error_setg(errp, > + "Device '%s' is writable but does not support snapshots", > + bdrv_get_device_name(bs)); > + return false; > + } > + > + if (bdrv_snapshot_find(bs, &sn, name) < 0) { > + error_setg(errp, > + "Device '%s' does not have the requested snapshot '%s'", > + bdrv_get_device_name(bs), name); > + return false; > + } > + > + if (sn.vm_state_size == 0) { > + return true; > + } > + if (*vmstate_bs != NULL) { > + error_setg(errp, "Devices '%s' and '%s' both has vmstate with the " > + "requested snapshot '%s'", bdrv_get_device_name(bs), > + bdrv_get_device_name(*vmstate_bs), name); > + return false; > + } > + *vmstate_bs = bs; > + > + return true; > +} > + > +BlockDriverState *bdrv_all_validate_snapshot(const char *name, Error **errp) > +{ > + BlockDriverState *bs = NULL; > + BlockDriverState *vmstate_bs = NULL; > + > + while ((bs = bdrv_next(bs))) { > + AioContext *ctx = bdrv_get_aio_context(bs); > + bool ok; > + > + aio_context_acquire(ctx); > + ok = validate_bs(bs, &vmstate_bs, name, errp); > + aio_context_release(ctx); > + > + if (!ok) { > + return NULL; > + } > + } > + > + if (vmstate_bs == NULL) { > + error_setg(errp, "VM state for the snapshot '%s' not found", name); > + } > + return vmstate_bs; > +} > diff --git a/include/block/snapshot.h b/include/block/snapshot.h > index c6910da6..a8506e9 100644 > --- a/include/block/snapshot.h > +++ b/include/block/snapshot.h > @@ -92,5 +92,6 @@ int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn, > BlockDriverState **first_bad_bs); > > BlockDriverState *bdrv_all_find_vmstate_bs(void); > +BlockDriverState *bdrv_all_validate_snapshot(const char *name, Error **errp); > > #endif > diff --git a/migration/savevm.c b/migration/savevm.c > index 6b34017..fed8664 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -2022,45 +2022,16 @@ void qmp_xen_save_devices_state(const char *filename, Error **errp) > int load_vmstate(const char *name, Error **errp) > { > BlockDriverState *bs, *bs_vm_state; > - QEMUSnapshotInfo sn; > QEMUFile *f; > int ret; > AioContext *aio_context; > > - if (!bdrv_all_can_snapshot(&bs)) { > - error_setg(errp, > - "Device '%s' is writable but does not support snapshots", > - bdrv_get_device_name(bs)); > - return -ENOTSUP; > - } > - ret = bdrv_all_find_snapshot(name, &bs); > - if (ret < 0) { > - error_setg(errp, > - "Device '%s' does not have the requested snapshot '%s'", > - bdrv_get_device_name(bs), name); > - return ret; > - } > - > - bs_vm_state = bdrv_all_find_vmstate_bs(); > - if (!bs_vm_state) { > - error_setg(errp, "No block device supports snapshots"); > + bs_vm_state = bdrv_all_validate_snapshot(name, errp); > + if (bs == NULL) { we should have bs_vm_state here checked...