From: Joe Jin <joe.jin@oracle.com>
Subject: Re: [PATCH -v2 0/3] xen-blkback: refactor vbd
	remove/disconnect.
Date: Thu, 04 Aug 2011 14:56:09 +0800
Message-ID: <4E3A4289.4060403@oracle.com>
References: <4E38E4A2.3070003@oracle.com> <20110803214947.GA12168@dumpdata.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Return-path: <xen-devel-bounces@lists.xensource.com>
In-Reply-To: <20110803214947.GA12168@dumpdata.com>
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>, Jens Axboe <jaxboe@fusionio.com>, Greg Marsden <greg.marsden@oracle.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Ian Campbell <Ian.Campbell@eu.citrix.com>, Annie Li <annie.li@oracle.com>, Kurt C Hackel <KURT.HACKEL@oracle.com>, Daniel Stodden <daniel.stodden@citrix.com>
List-Id: xen-devel@lists.xenproject.org

On 2011=E5=B9=B408=E6=9C=8804=E6=97=A5 05:49, Konrad Rzeszutek Wilk wrote=
:
> On Wed, Aug 03, 2011 at 02:03:14PM +0800, Joe Jin wrote:
>> This patchset is a backport and original patch author is Daniel Stodde=
n:
>> http://xenbits.xen.org/hg/XCP/linux-2.6.32.pq.hg/file/tip/CA-7672-blkb=
ack-shutdown.patch
>>
>> Initial issue:
>>   When we do block device attach/detach test with below steps, umount =
hang
>>   in guest and the guest unable to shutdown:
>=20
> So the patchset looks good and it fixes the guest hanging.. but
>>  =20
>>   1. start guest with the latest kernel.
>>   2. attach new block device by xm block-attach in Dom0
>=20
> So I think your patch while it fixes this problem it introduces a bug:
>=20
> I did this in Dom0:
>=20
> 18:10:23 # 5 :~/
>> xm block-attach 1 phy:/dev/sda xvda w
>=20
> and did _not_ attach the disk in the guest. Then I did
>=20
>=20
> 18:10:35 # 6 :~/
>> xm block-list 1
> Vdev  BE handle state evt-ch ring-ref BE-path
> 51712  0    0     4      18     770   /local/domain/0/backend/vbd/1/517=
12
>=20
> 18:10:39 # 7 :~/
>> xm block-detach 1 51712
>=20
> 18:10:46 # 8 :~/
>> xm block-list 1
>=20
>=20
>=20
> If I try the same sequence of events with your patch, I get this:
>=20
> 1:28:06 # 1 :~/=20
>> xm list
> Name                                        ID   Mem VCPUs      State  =
 Time(s)
> Domain-0                                     0  1500     4     r-----  =
 1246.6
> sda                                          2  2048     2     -b----  =
 1034.7
> sdb                                          6  2048     2     -b----  =
    3.4
> 21:28:09 # 2 :~/=20
>> xm block-list 6
>=20
> 21:28:22 # 4 :~/=20
>> xm block-attach 6 phy:/dev/sdb xvda w
>=20
> [did not do anything in the guest]
> 21:28:33 # 5 :~/=20
>> xm block-list 6
> Vdev  BE handle state evt-ch ring-ref BE-path
> 51712  0    0     4      18     770   /local/domain/0/backend/vbd/6/517=
12 =20
>=20
> 21:28:37 # 6 :~/=20
>> xm block-detach 6 51712
> Error: Device 51712 (vbd) could not be disconnected.
> Usage: xm block-detach <Domain> <DevId> [-f|--force]
>=20
> Destroy a domain's virtual block device.
>=20
> 21:30:30 # 7 :~/
>=20
> Any ideas?
Konrad,

Thanks for the finding.

Review the patch looked like it caused by below piece of codes in patch3:
        case XenbusStateClosed:
-               xenbus_switch_state(dev, XenbusStateClosed);
-               if (xenbus_dev_is_online(dev))
-                       break;
-               /* fall through if not online */
+               if (!xenvbd_kthread_remove(be))
+                       xenvbd_signal_shutdown(be);
+               break;
+
        case XenbusStateUnknown:
-               /* implies blkif_disconnect() via blkback_remove() */
+               /* implies xen_blkif_disconnect() via blkback_remove() */
                device_unregister(&dev->dev);
                break;

When device's state switched to XenbusStateClosed, did not unregister the=
 device.
Will send new patches for this.

Regards,
Joe

>>   3. mount new disk in guest
>>   4. execute xm block-detach to detach the block device in dom0 until =
timeout
>>   5. try to unmount the disk in guest, umount hung. at here, any IOs t=
o the=20
>>      device will hang.
>>  =20
>> Root cause:
>>   This caused by 'xm block-detach' in Dom0 set backend device's state =
to
>>   'XenbusStateClosing', frontend received the notification and=20
>>   blkfront_closing() be called, at the moment, the disk still using by=
 guest,
>>   so frontend refused to close. In the blkfront_closing(), frontend se=
nd a
>>   notification to backend said that the its state switched to 'Closing=
', when
>>   backend got the event, it will disconnect from real device, at here =
any IO
>>   request will be stuck, even tried to release the disk by umount.
>>
>>   So this may fix either frontend or backend, I have send a fix for fr=
ontend:
>>   https://lkml.org/lkml/2011/7/8/159
>>   Ian think we should fix it from backend and he pointed out Daniel St=
odden have
>>   submitted a patch(see above link) for xen-blkback, I tried it and it=
 works=20
>>   well.
>>
>> Changes:
>>   v2:
>>     - Reformat code style.
>>     - Per Knoard suggestions, change some int defines to bool.
>>
>>  drivers/block/xen-blkback/blkback.c |   10 +--
>>  drivers/block/xen-blkback/common.h  |    5 +
>>  drivers/block/xen-blkback/xenbus.c  |  203 ++++++++++++++++++++++++++=
+++++++++++++++++++++++++++++-------
>>  3 files changed, 192 insertions(+), 26 deletions(-)


--=20
Oracle <http://www.oracle.com>
Joe Jin | Team Leader, Software Development | +8610.6106.5624
ORACLE | Linux and Virtualization
No. 24 Zhongguancun Software Park, Haidian District | 100193 Beijing=20