On 12/06/2017 10:55 AM, Michael Roth wrote: > Quoting Eric Blake (2017-11-17 09:05:55) >> The NBD spec says that a server may fail any transmission request >> with ESHUTDOWN when it is apparent that no further request from >> the client can be successfully honored. The client is supposed >> to then initiate a soft shutdown (wait for all remaining in-flight >> requests to be answered, then send NBD_CMD_DISC). However, since >> qemu's server never uses ESHUTDOWN errors, this code was mostly >> untested since its introduction in commit b6f5d3b5. >> >> More recently, I learned that nbdkit as the NBD server is able to >> send ESHUTDOWN errors, so I finally tested this code, and noticed >> that our client was special-casing ESHUTDOWN to cause a hard >> shutdown (immediate disconnect, with no NBD_CMD_DISC), but only >> if the server sends this error as a simple reply. Further >> investigation found that commit d2febedb introduced a regression >> where structured replies behave differently than simple replies - >> but that the structured reply behavior is more in line with the >> spec (even if we still lack code in nbd-client.c to properly quit >> sending further requests). So this patch reverts the portion of >> b6f5d3b5 that introduced an improper hard-disconnect special-case >> at the lower level, and leaves the future enhancement of a nicer >> soft-disconnect at the higher level for another day. >> >> CC: [email protected] > > Since 2.10.x lacks d2febedb, the patch would look something like: > > diff --git a/nbd/client.c b/nbd/client.c > index 4caff77119..f04e95542f 100644 > --- a/nbd/client.c > +++ b/nbd/client.c > @@ -945,12 +945,6 @@ ssize_t nbd_receive_reply(QIOChannel *ioc, NBDReply > *reply, Error **errp) > reply->handle = ldq_be_p(buf + 8); > > reply->error = nbd_errno_to_system_errno(reply->error); > - > - if (reply->error == ESHUTDOWN) { > - /* This works even on mingw which lacks a native ESHUTDOWN */ > - error_setg(errp, "server shutting down"); > - return -EINVAL; > - } > trace_nbd_receive_reply(magic, reply->error, reply->handle); > > if (magic != NBD_REPLY_MAGIC) { > > If that seems reasonable I can apply it as such.
Your backport looks correct to me; thanks. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
