Re: [Qemu-devel] [PATCH 2/6 for-3.1] nbd: stop waiting for a NBD response with NBD_CMD_DISC

[Eric Blake]

On 11/16/18 9:53 AM, Daniel P. Berrangé wrote:
> When sending a NBD_CMD_DISC message there is no reply expected,
> however, the nbd_read_eof() coroutine is still waiting for a reply.
> In a plain NBD connection this doesn't matter as it will just get an
> EOF, however, on a TLS connection it will get an interrupted TLS data
> packet. The nbd_read_eof() will then print an error message on the
> console due to missing reply to NBD_CMD_DISC.
>
> This can be seen with qemu-img
>
>    $ qemu-img info \
>        --object tls-creds-x509,dir=tlsdir,id=tls0,endpoint=client \
>        --image-opts driver=nbd,host=127.0.0.1,port=9000,tls-creds=tls0
>    qemu-img: Cannot read from TLS channel: Input/output error
>    image: nbd://127.0.0.1:9000
>    file format: nbd
>    virtual size: 10M (10485760 bytes)
>    disk size: unavailable
>
> Simply setting the 'quit' flag after sending NBD_CMD_DISC is enough to
> get the coroutine to stop waiting for a reply and thus supress the error
> message.

Actually, it's not quite enough - once you actually start performing 
I/O, enough coroutines are kicked off that the error still happens:

$  qemu-io -c 'r 1m 1m' -c 'w -P 0x22 1m 1m' --image-opts \
--object tls-creds-x509,dir=scratch/tls/client1,endpoint=client,id=tls0\
 driver=nbd,host=localhost,port=10809,tls-creds=tls0
read 1048576/1048576 bytes at offset 1048576
1 MiB, 1 ops; 0.0430 sec (23.204 MiB/sec and 23.2040 ops/sec)
wrote 1048576/1048576 bytes at offset 1048576
1 MiB, 1 ops; 0.0152 sec (65.479 MiB/sec and 65.4793 ops/sec)
Cannot read from TLS channel: Input/output error

Squashing this in on top of your patch helps, though:

diff --git i/block/nbd-client.c w/block/nbd-client.c
index 5f63e4b8f15..e7916c78996 100644
--- i/block/nbd-client.c
+++ w/block/nbd-client.c
@@ -79,7 +79,14 @@ static coroutine_fn void nbd_read_reply_entry(void 
*opaque)
         assert(s->reply.handle == 0);
         ret = nbd_receive_reply(s->ioc, &s->reply, &local_err);
         if (local_err) {
-            error_report_err(local_err);
+            /* If we are already quitting, either another error has
+             * already been reported, or we requested NBD_CMD_DISC and
+             * don't need to report anything further.  */
+            if (!s->quit) {
+                error_report_err(local_err);
+            } else {
+                error_free(local_err);
+            }
         }
         if (ret <= 0) {
             break;

But I want to do more testing to make sure I'm not missing out on 
reporting an actual error if I add that.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org