[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v6 13/15] nbd: Improve server handling of shutdown r
From: |
Eric Blake |
Subject: |
[Qemu-devel] [PATCH v6 13/15] nbd: Improve server handling of shutdown requests |
Date: |
Thu, 13 Oct 2016 15:58:53 -0500 |
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: Eric Blake <address@hidden>
---
v6: rebase
v5: no change
v4: new patch
---
include/block/nbd.h | 13 +++++++++----
nbd/nbd-internal.h | 1 +
nbd/client.c | 16 ++++++++++++++++
nbd/server.c | 10 ++++++++++
4 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index d326308..eea7ef0 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -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)
diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h
index dd57e18..eee20ab 100644
--- a/nbd/nbd-internal.h
+++ b/nbd/nbd-internal.h
@@ -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)
{
diff --git a/nbd/client.c b/nbd/client.c
index f5e4c74..b33ae46 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -34,6 +34,8 @@ static int nbd_errno_to_system_errno(int err)
return ENOMEM;
case NBD_ENOSPC:
return ENOSPC;
+ case NBD_ESHUTDOWN:
+ return ESHUTDOWN;
default:
TRACE("Squashing unexpected error %d to EINVAL", err);
/* fallthrough */
@@ -231,11 +233,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);
@@ -784,6 +796,10 @@ ssize_t nbd_receive_reply(QIOChannel *ioc, NBDReply *reply)
LOG("invalid magic (got 0x%" PRIx32 ")", magic);
return -EINVAL;
}
+ if (reply->error == ESHUTDOWN) {
+ LOG("server shutting down");
+ return -EINVAL;
+ }
return 0;
}
diff --git a/nbd/server.c b/nbd/server.c
index 20f1086..a7aa2ba 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -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;
@@ -526,6 +528,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) {
@@ -538,6 +544,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:
--
2.7.4
- [Qemu-devel] [PATCH v6 09/15] nbd: Let server know when client gives up negotiation, (continued)
- [Qemu-devel] [PATCH v6 09/15] nbd: Let server know when client gives up negotiation, Eric Blake, 2016/10/13
- [Qemu-devel] [PATCH v6 10/15] nbd: Let client skip portions of server reply, Eric Blake, 2016/10/13
- [Qemu-devel] [PATCH v6 11/15] nbd: Less allocation during NBD_OPT_LIST, Eric Blake, 2016/10/13
- [Qemu-devel] [PATCH v6 12/15] nbd: Support shorter handshake, Eric Blake, 2016/10/13
- [Qemu-devel] [PATCH v6 01/15] nbd: Add qemu-nbd -D for human-readable description, Eric Blake, 2016/10/13
- [Qemu-devel] [PATCH v6 15/15] nbd: Implement NBD_CMD_WRITE_ZEROES on client, Eric Blake, 2016/10/13
- [Qemu-devel] [PATCH v6 05/15] nbd: Rename struct nbd_request and nbd_reply, Eric Blake, 2016/10/13
- [Qemu-devel] [PATCH v6 06/15] nbd: Share common reply-sending code in server, Eric Blake, 2016/10/13
- [Qemu-devel] [PATCH v6 02/15] nbd: Treat flags vs. command type as separate fields, Eric Blake, 2016/10/13
- [Qemu-devel] [PATCH v6 14/15] nbd: Implement NBD_CMD_WRITE_ZEROES on server, Eric Blake, 2016/10/13
- [Qemu-devel] [PATCH v6 13/15] nbd: Improve server handling of shutdown requests,
Eric Blake <=
- [Qemu-devel] [PATCH v6 08/15] nbd: Share common option-sending code in client, Eric Blake, 2016/10/13
- Re: [Qemu-devel] [PATCH v6 00/15] nbd: efficient write zeroes, no-reply, 2016/10/13
- Re: [Qemu-devel] [PATCH v6 00/15] nbd: efficient write zeroes, no-reply, 2016/10/14