From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:41734)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <famz@redhat.com>) id 1W0mTQ-0003Aj-HR
	for qemu-devel@nongnu.org; Wed, 08 Jan 2014 01:18:46 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <famz@redhat.com>) id 1W0mTK-0003Wu-I9
	for qemu-devel@nongnu.org; Wed, 08 Jan 2014 01:18:40 -0500
Received: from mx1.redhat.com ([209.132.183.28]:37175)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <famz@redhat.com>) id 1W0mTK-0003Wq-89
	for qemu-devel@nongnu.org; Wed, 08 Jan 2014 01:18:34 -0500
Message-ID: <52CCEDA4.9050300@redhat.com>
Date: Wed, 08 Jan 2014 14:18:12 +0800
From: Fam Zheng <famz@redhat.com>
MIME-Version: 1.0
References: <1386920120-2651-1-git-send-email-famz@redhat.com>
	<1386920120-2651-9-git-send-email-famz@redhat.com>
	<20140103091901.GC1483@stefanha-thinkpad.redhat.com>
In-Reply-To: <20140103091901.GC1483@stefanha-thinkpad.redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] [PATCH v8 08/12] block: Parse "backing" option to
 reference existing BDS
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: Stefan Hajnoczi <stefanha@gmail.com>
Cc: kwolf@redhat.com, rjones@redhat.com, armbru@redhat.com, qemu-devel@nongnu.org, imain@redhat.com, stefanha@redhat.com, pbonzini@redhat.com

On 2014=E5=B9=B401=E6=9C=8803=E6=97=A5 17:19, Stefan Hajnoczi wrote:
>>       }
>>
>> @@ -1682,7 +1696,6 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDr=
iverState *bs_old)
>>       assert(QLIST_EMPTY(&bs_new->dirty_bitmaps));
>>       assert(bs_new->job =3D=3D NULL);
>>       assert(bs_new->dev =3D=3D NULL);
>> -    assert(bdrv_op_blocker_is_empty(bs_new));
>>       assert(bs_new->io_limits_enabled =3D=3D false);
>>       assert(!throttle_have_timer(&bs_new->throttle_state));
>>
>> @@ -1701,7 +1714,6 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDr=
iverState *bs_old)
>>       /* Check a few fields that should remain attached to the device =
*/
>>       assert(bs_new->dev =3D=3D NULL);
>>       assert(bs_new->job =3D=3D NULL);
>> -    assert(bdrv_op_blocker_is_empty(bs_new));
>>       assert(bs_new->io_limits_enabled =3D=3D false);
>>       assert(!throttle_have_timer(&bs_new->throttle_state));
>
> Why are these hunks part of this patch?  I guess it makes sense *not* t=
o
> check for blockers in bdrv_swap().  Instead the high-level functions in
> blockdev.c and elsewhere should check blockers.
>

The two checks are here because of the mechanical replace of in_use.=20
They are removed because it is no longer true for some valid cases, e.g=20
with "block-commit". So we need these hunks here, or do this as a=20
preceding in the series.

Will update the commit message and keep it as is.

Fam