From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:48825)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1TEhUv-0005HD-C0
	for qemu-devel@nongnu.org; Thu, 20 Sep 2012 10:13:07 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1TEhUu-0003X5-15
	for qemu-devel@nongnu.org; Thu, 20 Sep 2012 10:12:57 -0400
Received: from mx1.redhat.com ([209.132.183.28]:25232)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1TEhUt-0003Wp-Nh
	for qemu-devel@nongnu.org; Thu, 20 Sep 2012 10:12:55 -0400
Message-ID: <505B2461.7040707@redhat.com>
Date: Thu, 20 Sep 2012 16:12:49 +0200
From: Kevin Wolf <kwolf@redhat.com>
MIME-Version: 1.0
References: <cover.1347993885.git.jcody@redhat.com>
	<8a055b1732489a90b50fcf941691ef2a14bb4514.1347993885.git.jcody@redhat.com>
In-Reply-To: <8a055b1732489a90b50fcf941691ef2a14bb4514.1347993885.git.jcody@redhat.com>
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v3 14/19] block: vmdk image file reopen
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Jeff Cody <jcody@redhat.com>
Cc: stefanha@gmail.com, pbonzini@redhat.com, eblake@redhat.com, qemu-devel@nongnu.org, supriyak@linux.vnet.ibm.com

Am 18.09.2012 20:53, schrieb Jeff Cody:
> This patch supports reopen for VMDK image files.  VMDK extents are added
> to the existing reopen queue, so that the transactional model of reopen
> is maintained with multiple image files.
> 
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  block/vmdk.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/block/vmdk.c b/block/vmdk.c
> index bba4c61..f2e861b 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -300,6 +300,40 @@ static int vmdk_is_cid_valid(BlockDriverState *bs)
>      return 1;
>  }
>  
> +/* Queue extents, if any, for reopen() */
> +static int vmdk_reopen_prepare(BDRVReopenState *state,
> +                               BlockReopenQueue *queue, Error **errp)
> +{
> +    BDRVVmdkState *s;
> +    int ret = -1;
> +    int i;
> +    VmdkExtent *e;
> +
> +    assert(state != NULL);
> +    assert(state->bs != NULL);
> +
> +    if (queue == NULL) {
> +        error_set(errp, ERROR_CLASS_GENERIC_ERROR,
> +                 "No reopen queue for VMDK extents");
> +        goto exit;
> +    }
> +
> +    s = state->bs->opaque;
> +
> +    assert(s != NULL);
> +
> +    for (i = 0; i < s->num_extents; i++) {
> +        e = &s->extents[i];
> +        if (e->file != state->bs->file) {
> +            bdrv_reopen_queue(queue, e->file, state->flags);

This is wrong, you can't abort the transaction if you do it like this.

VMDK needs to separately pass through each of prepare/commit/abort to
all extents, it can't use the all-in-one function.

Kevin