qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: Alex Bligh <alex@alex.org.uk>
Cc: Wouter Verhelst <w@uter.be>,
	"nbd-general@lists.sourceforge.net"
	<nbd-general@lists.sourceforge.net>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [Nbd] NBD_CMD_DISC
Date: Tue, 12 Apr 2016 11:34:06 +0100	[thread overview]
Message-ID: <20160412103406.GA28370@redhat.com> (raw)
In-Reply-To: <20160412094820.GA8122@redhat.com>

On Tue, Apr 12, 2016 at 10:48:20AM +0100, Daniel P. Berrange wrote:
> On Sun, Apr 10, 2016 at 10:49:00AM +0100, Alex Bligh wrote:
> > (Daniel: if you want to replicate the issue, just run qemu-img info
> > against my gonbdserver with TLS. Every fifth NBD_CMD_DISC doesn't
> > get through before the TCP session closes).
> 
> Hmm, I'll have a look at this - I'm not sure its caused by
> the lack of gnutls_bye, as opposed to incorrect handling
> of EAGAIN when the block layer sends CMD_DISC

I have tried to reproduce with your server yet, but I did look at the
code and identified one potential flaw that might cause the behaviour
you mention. Can you try testing with the following change applied
to QEMU:

diff --git a/nbd/common.c b/nbd/common.c
index 8ddb2dd..47757b6 100644
--- a/nbd/common.c
+++ b/nbd/common.c
@@ -50,7 +50,7 @@ ssize_t nbd_wr_syncv(QIOChannel *ioc,
                  * qio_channel_yield() that works with AIO contexts
                  * and consider using that in this branch */
                 qemu_coroutine_yield();
-            } else if (done) {
+            } else if (done || !do_read) {
                 /* XXX this is needed by nbd_reply_ready.  */
                 qio_channel_wait(ioc,
                                  do_read ? G_IO_IN : G_IO_OUT);


The current code will cause it to silently not send CMD_DISC if the socket
returns EAGAIN on send(). This change will cause it to do a poll to wait
and retry the send(). That said I'd be pretty surprised if we do actually
get EAGAIN in this scenario, as I'd expect the outgoing TCP buffers to be
empty at the point in which we close the client connection, but this fix
is worth a try.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

  parent reply	other threads:[~2016-04-12 10:34 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <D4F43553-381E-42EE-838E-AC8729E3D356@alex.org.uk>
     [not found] ` <5705727B.1000802@redhat.com>
     [not found]   ` <20160409091634.GB19023@grep.be>
     [not found]     ` <C0E9BDD0-C8B6-412E-9835-A696A1279935@alex.org.uk>
     [not found]       ` <57097020.7030003@redhat.com>
     [not found]         ` <56CF18E3-554D-4CF2-9DC3-6831D739A846@alex.org.uk>
2016-04-09 23:23           ` [Qemu-devel] [Nbd] NBD_CMD_DISC Eric Blake
     [not found]       ` <20160409102833.GL19023@grep.be>
     [not found]         ` <FDFBD171-A214-4780-B3E8-7E25FB41FACB@alex.org.uk>
     [not found]           ` <20160409113225.GR19023@grep.be>
     [not found]             ` <F405A775-6A76-4776-9182-2431293A3A60@alex.org.uk>
     [not found]               ` <20160410082710.GA28660@grep.be>
     [not found]                 ` <91E6DE09-E8F3-489E-B1F2-B3ECC358BB75@alex.org.uk>
     [not found]                   ` <20160412094820.GA8122@redhat.com>
2016-04-12 10:34                     ` Daniel P. Berrange [this message]
2016-04-12 11:14                       ` Alex Bligh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160412103406.GA28370@redhat.com \
    --to=berrange@redhat.com \
    --cc=alex@alex.org.uk \
    --cc=nbd-general@lists.sourceforge.net \
    --cc=qemu-devel@nongnu.org \
    --cc=w@uter.be \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).