From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:55236)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pl@dlhnet.de>) id 1UDJ3Y-0002Iw-QP
	for qemu-devel@nongnu.org; Wed, 06 Mar 2013 13:27:16 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pl@dlhnet.de>) id 1UDJ3U-0001Cy-0K
	for qemu-devel@nongnu.org; Wed, 06 Mar 2013 13:27:12 -0500
Received: from ssl.dlhnet.de ([91.198.192.8]:60931 helo=ssl.dlh.net)
	by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from <pl@dlhnet.de>)
	id 1UDJ3T-0001Cn-QS
	for qemu-devel@nongnu.org; Wed, 06 Mar 2013 13:27:07 -0500
Message-ID: <51378A7A.1060208@dlhnet.de>
Date: Wed, 06 Mar 2013 19:27:06 +0100
From: Peter Lieven <pl@dlhnet.de>
MIME-Version: 1.0
References: <cover.1362580930.git.jcody@redhat.com>
	<a56a55df5012c863f79cdb45ce2ba5a886ac37f4.1362580930.git.jcody@redhat.com>
	<51378200.5010705@dlhnet.de> <513785A0.1010001@redhat.com>
In-Reply-To: <513785A0.1010001@redhat.com>
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 1/7] block: only force IO completion in
 .bdrv_truncate if we are shrinking
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: Paolo Bonzini <pbonzini@redhat.com>
Cc: kwolf@redhat.com, sw@weilnetz.de, Jeff Cody <jcody@redhat.com>, qemu-devel@nongnu.org, stefanha@redhat.com

Am 06.03.2013 19:06, schrieb Paolo Bonzini:
> Il 06/03/2013 18:50, Peter Lieven ha scritto:
>>>> Commit 9a665b2b made bdrv_truncate() call bdrv_drain_all(), but this breaks
>>>> QCOW images, as well other future image formats (such as VHDX) that may call
>>>> bdrv_truncate(bs->file) from within a read/write operation.  For example, QCOW
>>>> will cause an assert, due to tracked_requests not being empty (since the
>>>> read/write that called bdrv_truncate() is still in progress).
> 
> I'm not sure such bdrv_truncate calls are necessary.  QCOW2 doesn't have
> them (almost; there is one in qcow2_write_compressed, I'm not even sure
> that one is necessary though), and I think QCOW's breaks using it with a
> block device as a backing file.

I think we have to check the sense of bs->growable nevertheless. It is set
to 1 for all drivers which can't be right?!

int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags)
{
    BlockDriverState *bs;
    BlockDriver *drv;
    int ret;

    drv = bdrv_find_protocol(filename);
    if (!drv) {
        return -ENOENT;
    }

    bs = bdrv_new("");
    ret = bdrv_open_common(bs, NULL, filename, flags, drv);
    if (ret < 0) {
        bdrv_delete(bs);
        return ret;
    }
    bs->growable = 1;
    *pbs = bs;
    return 0;
}

I think each driver should set it accordingly on its own.

Peter


> 
> Paolo
>