Re: [Qemu-devel] [PATCH 07/10] snapshot: create bdrv_all_create_snapshot helper

From: Stefan Hajnoczi <stefanha@redhat.com>
To: "Denis V. Lunev" <den@openvz.org>
Cc: Kevin Wolf <kwolf@redhat.com>,
	qemu-devel@nongnu.org, Juan Quintela <quintela@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 07/10] snapshot: create bdrv_all_create_snapshot helper
Date: Mon, 9 Nov 2015 17:33:07 +0000	[thread overview]
Message-ID: <20151109173307.GC29494@stefanha-x1.localdomain> (raw)
In-Reply-To: <1446911700-16803-8-git-send-email-den@openvz.org>

[-- Attachment #1: Type: text/plain, Size: 953 bytes --]

On Sat, Nov 07, 2015 at 06:54:57PM +0300, Denis V. Lunev wrote:
> +int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn, BlockDriverState **bad)
> +{
> +    int err = 0;
> +    BlockDriverState *bs = NULL;
> +
> +    while (err == 0 && (bs = bdrv_next(bs))) {
> +        AioContext *ctx = bdrv_get_aio_context(bs);
> +
> +        aio_context_acquire(ctx);
> +        if (bdrv_can_snapshot(bs)) {
> +            err = bdrv_snapshot_create(bs, sn);
> +            /* Tricky part here. First image contains VM state. The behavior
> +             * is matched one in bdrv_all_find_vmstate_bs */
> +            sn->vm_state_size = 0;

Please avoid the tricky part by passing in vm_state_bs and
vm_state_size.  Then this function can do:

  /* Write VM state size only to the image that contains the state */
  sn->vm_state_size = (bs == bs1 ? vm_state_size : 0);

without making assumptions about the algorithm for choosing the device
to store vmstate data on.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]