From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753691AbcESGfP (ORCPT ); Thu, 19 May 2016 02:35:15 -0400 Received: from metis.ext.4.pengutronix.de ([92.198.50.35]:58731 "EHLO metis.ext.4.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753508AbcESGfK (ORCPT ); Thu, 19 May 2016 02:35:10 -0400 Date: Thu, 19 May 2016 08:35:03 +0200 From: Markus Pargmann To: Wouter Verhelst Cc: Ratna Manoj , nbd-general@lists.sourceforge.net, Vinod Jayaraman , jack@suse.cz, linux-kernel@vger.kernel.org, Gou Rao , pbonzini@redhat.com Subject: Re: [Nbd] [PATCH] NBD: replace kill_bdev() with __invalidate_device() Message-ID: <20160519063503.GE19642@pengutronix.de> References: <56E711D1.9090708@gmail.com> <1809196.i4ZeAM11iZ@adelgunde> <20160428162734.GA3082@grep.be> <1733802.dNuTe5BCE4@adelgunde> <20160515125539.GA14784@grep.be> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="EXKGNeO8l0xGFBjy" Content-Disposition: inline In-Reply-To: <20160515125539.GA14784@grep.be> X-Sent-From: Pengutronix Hildesheim X-URL: http://www.pengutronix.de/ X-IRC: #ptxdist @freenode X-Accept-Language: de,en X-Accept-Content-Type: text/plain X-Uptime: 08:24:05 up 18 days, 15:25, 115 users, load average: 1.16, 1.56, 1.16 User-Agent: Mutt/1.5.24 (2015-08-30) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::7 X-SA-Exim-Mail-From: mpa@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --EXKGNeO8l0xGFBjy Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi Wouter, On Sun, May 15, 2016 at 02:55:39PM +0200, Wouter Verhelst wrote: > Hi Markus, >=20 > On Thu, May 12, 2016 at 11:53:01AM +0200, Markus Pargmann wrote: > > On Thursday 28 April 2016 18:27:34 Wouter Verhelst wrote: > > > However, at some point I agreed with Paul (your predecessor) that when > > > this happens due to an error condition (as opposed to it being due to= an > > > explicit disconnect), the kernel would block all reads from or writes= to > > > the device, and the client may try to reconnect *from the same > > > PID* (i.e., it may not fork()). If that succeeds, the next NBD_DO_IT = is > > > assumed to be connected to the same server; if instead the process > > > exits, then the block device is assumed to be dead, will be reset, and > > > all pending reads or writes would error. > > >=20 > > > In principle, this allows for a proper reconnect from userspace if it > > > can be done. However, I'm not sure whether this ever worked well or > > > whether it was documented, so it's probably fine if you think it shou= ld > > > be replaced with something else. > >=20 > > At least I was not aware of this possibility. As far as I know the > > previous code even had issues with the signals used to kill on timeouts > > which also killed the userspace program sometimes. > >=20 > > Currently I can't see a code path that supports reconnects. But I may > > have removed that accidently in the past. >=20 > Right. Like I said, I'm not sure if it ever worked well. The user space > client has a -persist option that tries to implement it, but I've been > getting some bug reports from people who've tried it (although that may > have been my fault rather than the kernel's). >=20 > > > (obviously, userspace reconnecting the device to a different device is > > > wrong and should not be done, but that's a case of "if you break it, = you > > > get to keep both pieces) > > >=20 > > > At any rate, I think it makes sense for userspace to be given a chance > > > to *attempt* to reconnect a device when the connection drops > > > unexpectedly. > >=20 > > Perhaps it would be better to setup the kernel driver explicitly for > > that. Perhaps some flag to let the kernel driver know that the client > > would like to keep the block device open? In that case the client could > > excplicitly use NBD_CLEAR_SOCK to cleanup everything. >=20 > I'm not sure what you mean by this. Can you clarify? I meant that it might be better to have a separate way for NBD_DO_IT. Something where the client software can directly instruct the kernel to keep everything opened in case of an error so that the client may reconnect afterwards. This could be a new ioctl that sets it up, for example 'NBD_PERSISTENT'. The NBD_DO_IT afterwards would keep everything up and running in case of a connection error so that the client could set a new socket using NBD_SET_SOCK and reenter using NBD_DO_IT. For all clients that are not capable of this mechanism or don't use it, NBD_DO_IT would clean up everything properly on any error. >=20 > > Or perhaps a completely new ioctl that can transmit back some more > > information about what failures were seen and whether the blockdevice > > was closed or not? >=20 > The intent was that ioctl(NBD_DO_IT) would return an error when the > disconnect was not requested, and would return 0 when the connection > dropped due to userspace doing ioctl(NBD_DISCONNECT), since dropping the > connection when userspace explicitly asks for it is not an error. >=20 > drivers/block/nbd.c contains the following: >=20 > static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, > unsigned int cmd, unsigned long arg) > { > [...] > case NBD_DO_IT: { > [...] > if (nbd->disconnect) /* user requested, ignore socket err= ors */ > return 0; > return error; > } > [...] >=20 > so the signalling part of it is at least still there. Whether it works, > I haven't tested. I just looked up the kernel code from 4.0. This code was there as well. But the socket and blockdevice were both destroyed before leaving the NBD_DO_IT ioctl. So it seems to have never been really persistent. Filesystems would have still been killed. So for a persistent nbd device there is some more code necessary to do it. Best Regards, Markus --=20 Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | --EXKGNeO8l0xGFBjy Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXPV6XAAoJEEpcgKtcEGQQ3f8P/02SJhUTt+3C3pE0EdDoaXdt G/tR6pKHDlUIRhDzSXEXU1dPBx3cu7UMjsD3xS+paiTS4FvT2KdxQl6IN/bWm4B/ tCAIQRxzIZM2PTyLCLK6dcfmVgaa2kq9Mr3lSdeyBQp7r49p7vrRzAqh0DHBr3Eq xSUnY+ndR+IIl66uJUvuXGOQS64fBk+yxTpjSPbF+21lxickH92zSNpX524THyQ4 i8xyyf3fI9ykgyS0R0AEVRtEarS2PR18eh+QtkW0HDnbyA9qOV+uG9iGutZ8IrMf xy9ecBCADQF4/VZO8SnKM/m5V8vA+8mMpcG3OF+Phy5XcHRoAsQ8z+NyFD3FnxWa ljH5bUwZA5tzaWcXeXguq2/a/eIi+pSEBxzm4aiJlXhkMvZCluLoyxUwF5VBqVb7 d8C7IyLPl6hylQV6DlGIc+fw5hwjbCoIGiUUKloGzGQ2TvtgPUhEq35t/QOJsx06 PuF9s5BEMyLXWYRhdVju7cLqfws8vOF3v+HdzmI6CJmpdi3roKE3wEOq48aR6RN5 RXqemnonbR2P+Ygj15/fDFsT28Pb6BgK+aWRUg1HQdDlEJ06B6W1eZkK5HSfS9jp RykWKgCsoIAvfNYwj6mppteqHlP6iJ6cXPcwnoPHlMgRNYJoBR5dqON1Uef2I+2l GkItjxnPSQZkA/QO77mE =RTl8 -----END PGP SIGNATURE----- --EXKGNeO8l0xGFBjy--