From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33651) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQcdY-00031h-70 for qemu-devel@nongnu.org; Tue, 23 Oct 2012 07:27:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TQcdX-0006F7-4x for qemu-devel@nongnu.org; Tue, 23 Oct 2012 07:27:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46024) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQcdW-0006F2-Rp for qemu-devel@nongnu.org; Tue, 23 Oct 2012 07:27:07 -0400 Message-ID: <50867EFB.8030207@redhat.com> Date: Tue, 23 Oct 2012 13:26:51 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <5086728B.1010809@redhat.com> <1350990519.16343.255.camel@eboracum.office.bytemark.co.uk> In-Reply-To: <1350990519.16343.255.camel@eboracum.office.bytemark.co.uk> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/3] nbd: Only try to send flush/discard commands if connected to the NBD server List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nicholas Thomas Cc: pbonzini@redhat.com, qemu-devel@nongnu.org Am 23.10.2012 13:08, schrieb Nicholas Thomas: > On Tue, 2012-10-23 at 12:33 +0200, Kevin Wolf wrote: >> Am 22.10.2012 13:09, schrieb nick@bytemark.co.uk: >>> >>> This is unlikely to come up now, but is a necessary prerequisite for reconnection >>> behaviour. >>> >>> Signed-off-by: Nick Thomas >>> --- >>> block/nbd.c | 13 +++++++++++-- >>> 1 files changed, 11 insertions(+), 2 deletions(-) >> >> What's the real requirement here? Silently ignoring a flush and >> returning success for it feels wrong. Why is it correct? >> >> Kevin > > I just needed to avoid socket operations while s->sock == -1, and > extending the existing case of "can't do the command, so pretend I did > it" to "can't do the command right now, so pretend..." seemed like an > easy way out. The difference is that NBD_FLAG_SEND_FLUSH is only clear if the server is configured like that, i.e. it's completely in the control of the user, and it ignores flushes consistently (for example because it already uses a writethrough mode so that flushes aren't required). Network outage usually isn't in the control of the user and ignoring a flush request when the server actually asked for flushes is completely surprising and dangerous. > In the Bytemark case, the NBD server always opens the file O_SYNC, so > nbd_co_flush could check in_flight == 0 and return 0/1 based on that; > but I'd be surprised if that's true for all NBD servers. Should we be > returning 1 here for both "not supported" and "can't do it right now", > instead? The best return value is probably -ENOTCONN or -EIO if you must return an error. But maybe using the same logic as for writes would be appropriate, i.e. try to reestablish the connection before you return an error. Kevin