Commit b6f5d3b5 authored by Eric Blake's avatar Eric Blake Committed by Paolo Bonzini
Browse files

nbd: Improve server handling of shutdown requests 



NBD commit 6d34500b clarified how clients and servers are supposed
to behave before closing a connection. It added NBD_REP_ERR_SHUTDOWN
(for the server to announce it is about to go away during option
haggling, so the client should quit sending NBD_OPT_* other than
NBD_OPT_ABORT) and ESHUTDOWN (for the server to announce it is about
to go away during transmission, so the client should quit sending
NBD_CMD_* other than NBD_CMD_DISC).  It also clarified that
NBD_OPT_ABORT gets a reply, while NBD_CMD_DISC does not.

This patch merely adds the missing reply to NBD_OPT_ABORT and teaches
the client to recognize server errors.  Actually teaching the server
to send NBD_REP_ERR_SHUTDOWN or ESHUTDOWN would require knowing that
the server has been requested to shut down soon (maybe we could do
that by installing a SIGINT handler in qemu-nbd, which transitions
from RUNNING to a new state that waits for the client to react,
rather than just out-right quitting - but that's a bigger task for
another day).

Signed-off-by: default avatarEric Blake <eblake@redhat.com>
Message-Id: <1476469998-28592-15-git-send-email-eblake@redhat.com>
[Move dummy ESHUTDOWN to include/qemu/osdep.h. - Paolo]
Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
parent 8b34a9db

include/block/nbd.h

+9 −4
Original line number Diff line number Diff line
@@ -83,12 +83,17 @@ typedef struct NBDReply NBDReply; 
#define NBD_FLAG_C_NO_ZEROES      (1 << 1) /* End handshake without zeroes. */



/* Reply types. */

#define NBD_REP_ERR(value) ((UINT32_C(1) << 31) | (value))



#define NBD_REP_ACK             (1)             /* Data sending finished. */

#define NBD_REP_SERVER          (2)             /* Export description. */

#define NBD_REP_ERR_UNSUP       ((UINT32_C(1) << 31) | 1) /* Unknown option. */

#define NBD_REP_ERR_POLICY      ((UINT32_C(1) << 31) | 2) /* Server denied */

#define NBD_REP_ERR_INVALID     ((UINT32_C(1) << 31) | 3) /* Invalid length. */

#define NBD_REP_ERR_TLS_REQD    ((UINT32_C(1) << 31) | 5) /* TLS required */



#define NBD_REP_ERR_UNSUP       NBD_REP_ERR(1)  /* Unknown option */

#define NBD_REP_ERR_POLICY      NBD_REP_ERR(2)  /* Server denied */

#define NBD_REP_ERR_INVALID     NBD_REP_ERR(3)  /* Invalid length */

#define NBD_REP_ERR_PLATFORM    NBD_REP_ERR(4)  /* Not compiled in */

#define NBD_REP_ERR_TLS_REQD    NBD_REP_ERR(5)  /* TLS required */

#define NBD_REP_ERR_SHUTDOWN    NBD_REP_ERR(7)  /* Server shutting down */



/* Request flags, sent from client to server during transmission phase */

#define NBD_CMD_FLAG_FUA        (1 << 0)

include/qemu/osdep.h

+3 −0
Original line number Diff line number Diff line
@@ -128,6 +128,9 @@ extern int daemon(int, int); 
#if !defined(EMEDIUMTYPE)

#define EMEDIUMTYPE 4098

#endif

#if !defined(ESHUTDOWN)

#define ESHUTDOWN 4099

#endif

#ifndef TIME_MAX

#define TIME_MAX LONG_MAX

#endif

nbd/client.c

+18 −0
Original line number Diff line number Diff line
@@ -40,6 +40,9 @@ static int nbd_errno_to_system_errno(int err) 
    case NBD_ENOSPC:

        ret = ENOSPC;

        break;

    case NBD_ESHUTDOWN:

        ret = ESHUTDOWN;

        break;

    default:

        TRACE("Squashing unexpected error %d to EINVAL", err);

        /* fallthrough */
@@ -239,11 +242,21 @@ static int nbd_handle_reply_err(QIOChannel *ioc, nbd_opt_reply *reply, 
                   reply->option);

        break;



    case NBD_REP_ERR_PLATFORM:

        error_setg(errp, "Server lacks support for option %" PRIx32,

                   reply->option);

        break;



    case NBD_REP_ERR_TLS_REQD:

        error_setg(errp, "TLS negotiation required before option %" PRIx32,

                   reply->option);

        break;



    case NBD_REP_ERR_SHUTDOWN:

        error_setg(errp, "Server shutting down before option %" PRIx32,

                   reply->option);

        break;



    default:

        error_setg(errp, "Unknown error code when asking for option %" PRIx32,

                   reply->option);
@@ -785,6 +798,11 @@ ssize_t nbd_receive_reply(QIOChannel *ioc, NBDReply *reply) 


    reply->error = nbd_errno_to_system_errno(reply->error);



    if (reply->error == ESHUTDOWN) {

        /* This works even on mingw which lacks a native ESHUTDOWN */

        LOG("server shutting down");

        return -EINVAL;

    }

    TRACE("Got reply: { magic = 0x%" PRIx32 ", .error = % " PRId32

          ", handle = %" PRIu64" }",

          magic, reply->error, reply->handle);

nbd/nbd-internal.h

+1 −0
Original line number Diff line number Diff line
@@ -92,6 +92,7 @@ 
#define NBD_ENOMEM     12

#define NBD_EINVAL     22

#define NBD_ENOSPC     28

#define NBD_ESHUTDOWN  108



static inline ssize_t read_sync(QIOChannel *ioc, void *buffer, size_t size)

{

nbd/server.c

+10 −0
Original line number Diff line number Diff line
@@ -39,6 +39,8 @@ static int system_errno_to_nbd_errno(int err) 
    case EFBIG:

    case ENOSPC:

        return NBD_ENOSPC;

    case ESHUTDOWN:

        return NBD_ESHUTDOWN;

    case EINVAL:

    default:

        return NBD_EINVAL;
@@ -527,6 +529,10 @@ static int nbd_negotiate_options(NBDClient *client) 
                if (ret < 0) {

                    return ret;

                }

                /* Let the client keep trying, unless they asked to quit */

                if (clientflags == NBD_OPT_ABORT) {

                    return -EINVAL;

                }

                break;

            }

        } else if (fixedNewstyle) {
@@ -539,6 +545,10 @@ static int nbd_negotiate_options(NBDClient *client) 
                break;



            case NBD_OPT_ABORT:

                /* NBD spec says we must try to reply before

                 * disconnecting, but that we must also tolerate

                 * guests that don't wait for our reply. */

                nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, clientflags);

                return -EINVAL;



            case NBD_OPT_EXPORT_NAME: