[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/3] nbd: Add exports listing support.
|
From: |
Hani Benhabiles |
|
Subject: |
Re: [Qemu-devel] [PATCH 2/3] nbd: Add exports listing support. |
|
Date: |
Wed, 4 Jun 2014 23:35:52 +0100 |
|
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Tue, Jun 03, 2014 at 01:36:35PM +0200, Paolo Bonzini wrote:
> Il 31/05/2014 23:39, Hani Benhabiles ha scritto:
> >This is added by handling the NBD_OPT_LIST and NBD_OPT_ABORT options.
> >
> >NBD_REP_ERR_UNSUP is also sent for unknown NBD option values.
> >
> >Signed-off-by: Hani Benhabiles <address@hidden>
> >---
> > include/block/nbd.h | 5 ++
> > nbd.c | 197
> > +++++++++++++++++++++++++++++++++++++++++-----------
> > 2 files changed, 162 insertions(+), 40 deletions(-)
> >
> >diff --git a/include/block/nbd.h b/include/block/nbd.h
> >index 95d52ab..fd7e057 100644
> >--- a/include/block/nbd.h
> >+++ b/include/block/nbd.h
> >@@ -51,6 +51,11 @@ struct nbd_reply {
> > /* New-style client flags. */
> > #define NBD_FLAG_C_FIXED_NEWSTYLE (1 << 0) /* Fixed newstyle protocol.
> > */
> >
> >+/* Reply types. */
> >+#define NBD_REP_ACK (1) /* Data sending finished. */
> >+#define NBD_REP_SERVER (2) /* Export description. */
> >+#define NBD_REP_ERR_UNSUP ((1 << 31) | 1) /* Unknown option. */
> >+
> > #define NBD_CMD_MASK_COMMAND 0x0000ffff
> > #define NBD_CMD_FLAG_FUA (1 << 16)
> >
> >diff --git a/nbd.c b/nbd.c
> >index aa2e552..012e5da 100644
> >--- a/nbd.c
> >+++ b/nbd.c
> >@@ -64,6 +64,7 @@
> > #define NBD_REPLY_MAGIC 0x67446698
> > #define NBD_OPTS_MAGIC 0x49484156454F5054LL
> > #define NBD_CLIENT_MAGIC 0x0000420281861253LL
> >+#define NBD_REP_MAGIC 0x3e889045565a9LL
> >
> > #define NBD_SET_SOCK _IO(0xab, 0)
> > #define NBD_SET_BLKSIZE _IO(0xab, 1)
> >@@ -77,7 +78,9 @@
> > #define NBD_SET_TIMEOUT _IO(0xab, 9)
> > #define NBD_SET_FLAGS _IO(0xab, 10)
> >
> >-#define NBD_OPT_EXPORT_NAME (1 << 0)
> >+#define NBD_OPT_EXPORT_NAME (1)
> >+#define NBD_OPT_ABORT (2)
> >+#define NBD_OPT_LIST (3)
> >
> > /* Definitions for opaque data types */
> >
> >@@ -215,54 +218,98 @@ static ssize_t write_sync(int fd, void *buffer, size_t
> >size)
> >
> > */
> >
> >-static int nbd_receive_options(NBDClient *client)
> >+static int nbd_send_rep(int csock, uint32_t type, uint32_t opt)
> > {
> >- int csock = client->sock;
> >- char name[256];
> >- uint32_t tmp, length;
> > uint64_t magic;
> >- int rc;
> >-
> >- /* Client sends:
> >- [ 0 .. 3] client flags
> >- [ 4 .. 11] NBD_OPTS_MAGIC
> >- [12 .. 15] NBD_OPT_EXPORT_NAME
> >- [16 .. 19] length
> >- [20 .. xx] export name (length bytes)
> >- */
> >+ uint32_t len;
> >
> >- rc = -EINVAL;
> >- if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {
> >- LOG("read failed");
> >- goto fail;
> >+ magic = cpu_to_be64(NBD_REP_MAGIC);
> >+ if (write_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
> >+ LOG("write failed (rep magic)");
> >+ return -EINVAL;
> > }
> >- TRACE("Checking client flags");
> >- tmp = be32_to_cpu(tmp);
> >- if (tmp != 0 && tmp != NBD_FLAG_C_FIXED_NEWSTYLE) {
> >- LOG("Bad client flags received");
> >- goto fail;
> >+ opt = cpu_to_be32(opt);
> >+ if (write_sync(csock, &opt, sizeof(opt)) != sizeof(opt)) {
> >+ LOG("write failed (rep opt)");
> >+ return -EINVAL;
> > }
> >-
> >- if (read_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
> >- LOG("read failed");
> >- goto fail;
> >+ type = cpu_to_be32(type);
> >+ if (write_sync(csock, &type, sizeof(type)) != sizeof(type)) {
> >+ LOG("write failed (rep type)");
> >+ return -EINVAL;
> > }
> >- TRACE("Checking opts magic");
> >- if (magic != be64_to_cpu(NBD_OPTS_MAGIC)) {
> >- LOG("Bad magic received");
> >- goto fail;
> >+ len = cpu_to_be32(0);
> >+ if (write_sync(csock, &len, sizeof(len)) != sizeof(len)) {
> >+ LOG("write failed (rep data length)");
> >+ return -EINVAL;
> > }
> >+ return 0;
> >+}
> >
> >- if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {
> >- LOG("read failed");
> >- goto fail;
> >+static int nbd_send_rep_list(int csock, NBDExport *exp)
> >+{
> >+ uint64_t magic, name_len;
> >+ uint32_t opt, type, len;
> >+
> >+ name_len = strlen(exp->name);
> >+ magic = cpu_to_be64(NBD_REP_MAGIC);
> >+ if (write_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
> >+ LOG("write failed (magic)");
> >+ return -EINVAL;
> > }
> >- TRACE("Checking option");
> >- if (tmp != be32_to_cpu(NBD_OPT_EXPORT_NAME)) {
> >- LOG("Bad option received");
> >- goto fail;
> >+ opt = cpu_to_be32(NBD_OPT_LIST);
> >+ if (write_sync(csock, &opt, sizeof(opt)) != sizeof(opt)) {
> >+ LOG("write failed (opt)");
> >+ return -EINVAL;
> >+ }
> >+ type = cpu_to_be32(NBD_REP_SERVER);
> >+ if (write_sync(csock, &type, sizeof(type)) != sizeof(type)) {
> >+ LOG("write failed (reply type)");
> >+ return -EINVAL;
> >+ }
> >+ len = cpu_to_be32(name_len + sizeof(len));
> >+ if (write_sync(csock, &len, sizeof(len)) != sizeof(len)) {
> >+ LOG("write failed (length)");
> >+ return -EINVAL;
> >+ }
> >+ len = cpu_to_be32(name_len);
> >+ if (write_sync(csock, &len, sizeof(len)) != sizeof(len)) {
> >+ LOG("write failed (length)");
> >+ return -EINVAL;
> >+ }
> >+ if (write_sync(csock, exp->name, name_len) != name_len) {
> >+ LOG("write failed (buffer)");
> >+ return -EINVAL;
> >+ }
> >+ return 0;
> >+}
> >+
> >+static int nbd_send_list(NBDClient *client)
> >+{
> >+ int csock;
> >+ NBDExport *exp;
> >+
> >+ csock = client->sock;
> >+ /* For each export, send a NBD_REP_SERVER reply. */
> >+ QTAILQ_FOREACH(exp, &exports, next) {
> >+ if (nbd_send_rep_list(csock, exp)) {
> >+ return -EINVAL;
> >+ }
> > }
> >+ /* Finish with a NBD_REP_ACK. */
> >+ return nbd_send_rep(csock, NBD_REP_ACK, NBD_OPT_LIST);
> >+}
> >+
> >+static int nbd_handle_export_name(NBDClient *client)
> >+{
> >+ int rc = -EINVAL, csock = client->sock;
> >+ char name[256];
> >+ uint32_t length;
> >
> >+ /* Client sends:
> >+ [16 .. 19] length
> >+ [20 .. xx] export name (length bytes)
> >+ */
> > if (read_sync(csock, &length, sizeof(length)) != sizeof(length)) {
> > LOG("read failed");
> > goto fail;
> >@@ -287,8 +334,76 @@ static int nbd_receive_options(NBDClient *client)
> >
> > QTAILQ_INSERT_TAIL(&client->exp->clients, client, next);
> > nbd_export_get(client->exp);
> >+ rc = 0;
> >+fail:
> >+ return rc;
> >+}
> >+
> >+static int nbd_receive_options(NBDClient *client)
> >+{
> >+ int rc = -EINVAL;
> >
> >- TRACE("Option negotiation succeeded.");
> >+ while (1) {
> >+ int csock = client->sock;
> >+ uint32_t tmp;
> >+ uint64_t magic;
> >+
> >+ /* Client sends:
> >+ [ 0 .. 3] client flags
> >+ [ 4 .. 11] NBD_OPTS_MAGIC
> >+ [12 .. 15] NBD option
> >+ ... Rest of request
> >+ */
> >+
> >+ if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {
> >+ LOG("read failed");
> >+ goto fail;
> >+ }
> >+ TRACE("Checking client flags");
> >+ tmp = be32_to_cpu(tmp);
> >+ if (tmp != 0 && tmp != NBD_FLAG_C_FIXED_NEWSTYLE) {
> >+ LOG("Bad client flags received");
> >+ goto fail;
> >+ }
> >+
> >+ if (read_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
> >+ LOG("read failed");
> >+ goto fail;
> >+ }
> >+ TRACE("Checking opts magic");
> >+ if (magic != be64_to_cpu(NBD_OPTS_MAGIC)) {
> >+ LOG("Bad magic received");
> >+ goto fail;
> >+ }
> >+
> >+ if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {
> >+ LOG("read failed");
> >+ goto fail;
> >+ }
> >+
> >+ TRACE("Checking option");
> >+ switch (be32_to_cpu(tmp)) {
> >+ case NBD_OPT_LIST:
> >+ if (nbd_send_list(client) < 0) {
> >+ return -EINVAL;
> >+ }
> >+ break;
>
> You can just do "return nbd_send_list(client);" here. You probably should
> also rename the function to nbd_handle_list.
>
Ack.
> >+ case NBD_OPT_ABORT:
> >+ return 1;
> >+
> >+ case NBD_OPT_EXPORT_NAME:
> >+ return nbd_handle_export_name(client);
>
> Part of this patch belongs in patch 1. This one should only add
> nbd_send_rep_list, nbd_handle_list and the NBD_OPT_LIST case.
>
Alright, I will resend, making the changes you and Stefan suggested.
Thanks.