[Qemu-devel] [PATCH] Introduce NBD named exports.

[Qemu-devel] [PATCH] Introduce NBD named exports.

[Laurent Vivier]

This patch allows to connect Qemu using NBD protocol to an nbd-server
using named exports.

For instance, if on the host "isoserver", in /etc/nbd-server/config, you have:

[generic]
[debian-500-ppc-netinst]
        exportname = /ISO/debian-500-powerpc-netinst.iso
[Fedora-10-ppc-netinst]
        exportname = /ISO/Fedora-10-ppc-netinst.iso

You can connect to it, using:

    qemu -cdrom nbd:isoserver:exportname=debian-500-ppc-netinst
    qemu -cdrom nbd:isoserver:exportname=Fedora-10-ppc-netinst

NOTE: you need at least nbd-server 2.9.18
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 block/nbd.c   |   57 +++++++++++++++++++--------
 nbd.c         |  119 ++++++++++++++++++++++++++++++++++++++++++++++++++------
 nbd.h         |    5 ++-
 qemu-doc.texi |    7 +++
 qemu-nbd.c    |    2 +-
 5 files changed, 158 insertions(+), 32 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index a1ec123..ee22b47 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -42,55 +42,78 @@ typedef struct BDRVNBDState {
 static int nbd_open(BlockDriverState *bs, const char* filename, int flags)
 {
     BDRVNBDState *s = bs->opaque;
+
+    #define EN_OPTSTR ":exportname="
+    char *file;
+    char *name;
     const char *host;
     const char *unixpath;
     int sock;
     off_t size;
     size_t blocksize;
     int ret;
+    int err = -EINVAL;
+
+    file = qemu_strdup(filename);
 
-    if (!strstart(filename, "nbd:", &host))
-        return -EINVAL;
+    name = strstr(file, EN_OPTSTR);
+    if (name) {
+        if (name[sizeof(EN_OPTSTR) - 1] == 0)
+            goto out;
+        name[0] = 0;
+        name += sizeof(EN_OPTSTR) - 1;
+    }
+
+    if (!strstart(file, "nbd:", &host))
+        goto out;
 
     if (strstart(host, "unix:", &unixpath)) {
 
         if (unixpath[0] != '/')
-            return -EINVAL;
+            goto out;
 
         sock = unix_socket_outgoing(unixpath);
 
     } else {
-        uint16_t port;
+        uint16_t port = NBD_DEFAULT_PORT;
         char *p, *r;
         char hostname[128];
 
         pstrcpy(hostname, 128, host);
 
         p = strchr(hostname, ':');
-        if (p == NULL)
-            return -EINVAL;
+        if (p != NULL) {
+            *p = '\0';
+            p++;
 
-        *p = '\0';
-        p++;
+            port = strtol(p, &r, 0);
+            if (r == p)
+                goto out;
+        } else if (name == NULL)
+                goto out;
 
-        port = strtol(p, &r, 0);
-        if (r == p)
-            return -EINVAL;
         sock = tcp_socket_outgoing(hostname, port);
     }
 
-    if (sock == -1)
-        return -errno;
+    if (sock == -1) {
+        err = -errno;
+        goto out;
+    }
 
-    ret = nbd_receive_negotiate(sock, &size, &blocksize);
-    if (ret == -1)
-        return -errno;
+    ret = nbd_receive_negotiate(sock, name, &size, &blocksize);
+    if (ret == -1) {
+        err = -errno;
+        goto out;
+    }
 
     s->sock = sock;
     s->size = size;
     s->blocksize = blocksize;
+    err = 0;
 
-    return 0;
+out:
+    qemu_free(file);
+    return err;
 }
 
 static int nbd_read(BlockDriverState *bs, int64_t sector_num,
diff --git a/nbd.c b/nbd.c
index a9f295f..755613a 100644
--- a/nbd.c
+++ b/nbd.c
@@ -62,6 +62,8 @@
 #define NBD_SET_SIZE_BLOCKS	_IO(0xab, 7)
 #define NBD_DISCONNECT          _IO(0xab, 8)
 
+#define NBD_OPT_EXPORT_NAME	(1 << 0)
+
 /* That's all folks */
 
 #define read_sync(fd, buffer, size) nbd_wr_sync(fd, buffer, size, true)
@@ -296,22 +298,26 @@ int nbd_negotiate(int csock, off_t size)
 	return 0;
 }
 
-int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize)
+int nbd_receive_negotiate(int csock, const char *name,
+                          off_t *size, size_t *blocksize)
 {
-	char buf[8 + 8 + 8 + 128];
-	uint64_t magic;
+	char buf[256] = "\0\0\0\0\0\0\0\0\0";
+	uint64_t magic, s;
+	uint16_t tmp;
+	uint32_t flags;
 
 	TRACE("Receiving negotation.");
 
-	if (read_sync(csock, buf, sizeof(buf)) != sizeof(buf)) {
+	if (read_sync(csock, buf, 8) != 8) {
 		LOG("read failed");
 		errno = EINVAL;
 		return -1;
 	}
-
-	magic = be64_to_cpup((uint64_t*)(buf + 8));
-	*size = be64_to_cpup((uint64_t*)(buf + 16));
-	*blocksize = 1024;
+	if (strlen(buf) == 0) {
+		LOG("server connection closed");
+		errno = EINVAL;
+		return -1;
+	}
 
 	TRACE("Magic is %c%c%c%c%c%c%c%c",
 	      qemu_isprint(buf[0]) ? buf[0] : '.',
@@ -322,8 +328,6 @@ int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize)
 	      qemu_isprint(buf[5]) ? buf[5] : '.',
 	      qemu_isprint(buf[6]) ? buf[6] : '.',
 	      qemu_isprint(buf[7]) ? buf[7] : '.');
-	TRACE("Magic is 0x%" PRIx64, magic);
-	TRACE("Size is %" PRIu64, *size);
 
 	if (memcmp(buf, "NBDMAGIC", 8) != 0) {
 		LOG("Invalid magic received");
@@ -331,10 +335,99 @@ int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize)
 		return -1;
 	}
 
-	TRACE("Checking magic");
+	if (read_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
+		LOG("read failed");
+		errno = EINVAL;
+		return -1;
+	}
+	magic = be64_to_cpu(magic);
+	TRACE("Magic is 0x%" PRIx64, magic);
 
-	if (magic != 0x00420281861253LL) {
-		LOG("Bad magic received");
+	if (name) {
+		uint32_t reserved = 0;
+		uint32_t opt;
+		uint32_t namesize;
+
+		TRACE("Checking magic (opts_magic)");
+		if (magic != 0x49484156454F5054LL) {
+			LOG("Bad magic received");
+			errno = EINVAL;
+			return -1;
+		}
+		if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {
+			LOG("flags read failed");
+			errno = EINVAL;
+			return -1;
+		}
+		flags = be16_to_cpu(tmp) << 16;
+		/* reserved for future use */
+		if (write_sync(csock, &reserved, sizeof(reserved)) !=
+		    sizeof(reserved)) {
+			LOG("write failed (reserved)");
+			errno = EINVAL;
+			return -1;
+		}
+		/* write the export name */
+		magic = cpu_to_be64(magic);
+		if (write_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
+			LOG("write failed (magic)");
+			errno = EINVAL;
+			return -1;
+		}
+		opt = cpu_to_be32(NBD_OPT_EXPORT_NAME);
+		if (write_sync(csock, &opt, sizeof(opt)) != sizeof(opt)) {
+			LOG("write failed (opt)");
+			errno = EINVAL;
+			return -1;
+		}
+		namesize = cpu_to_be32(strlen(name));
+		if (write_sync(csock, &namesize, sizeof(namesize)) !=
+		    sizeof(namesize)) {
+			LOG("write failed (namesize)");
+			errno = EINVAL;
+			return -1;
+		}
+		if (write_sync(csock, (char*)name, strlen(name)) != strlen(name)) {
+			LOG("write failed (name)");
+			errno = EINVAL;
+			return -1;
+		}
+	} else {
+		TRACE("Checking magic (cli_magic)");
+
+		if (magic != 0x00420281861253LL) {
+			LOG("Bad magic received");
+			errno = EINVAL;
+			return -1;
+		}
+	}
+
+	if (read_sync(csock, &s, sizeof(s)) != sizeof(s)) {
+		LOG("read failed");
+		errno = EINVAL;
+		return -1;
+	}
+	*size = be64_to_cpu(s);
+	*blocksize = 1024;
+	TRACE("Size is %" PRIu64, *size);
+
+	if (!name) {
+		if (read_sync(csock, &flags, sizeof(flags)) != sizeof(flags)) {
+			LOG("read failed (flags)");
+			errno = EINVAL;
+			return -1;
+		}
+		flags = be64_to_cpu(flags);
+	} else {
+		if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {
+			LOG("read failed (tmp)");
+			errno = EINVAL;
+			return -1;
+		}
+		flags |= be32_to_cpu(tmp);
+	}
+	if (read_sync(csock, &buf, 124) != 124) {
+		LOG("read failed (buf)");
 		errno = EINVAL;
 		return -1;
 	}
diff --git a/nbd.h b/nbd.h
index 5a1fbdf..17be0fa 100644
--- a/nbd.h
+++ b/nbd.h
@@ -42,6 +42,8 @@ enum {
     NBD_CMD_DISC = 2
 };
 
+#define NBD_DEFAULT_PORT	10809
+
 size_t nbd_wr_sync(int fd, void *buffer, size_t size, bool do_read);
 int tcp_socket_outgoing(const char *address, uint16_t port);
 int tcp_socket_incoming(const char *address, uint16_t port);
@@ -49,7 +51,8 @@ int unix_socket_outgoing(const char *path);
 int unix_socket_incoming(const char *path);
 
 int nbd_negotiate(int csock, off_t size);
-int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize);
+int nbd_receive_negotiate(int csock, const char *name,
+                          off_t *size, size_t *blocksize);
 int nbd_init(int fd, int csock, off_t size, size_t blocksize);
 int nbd_send_request(int csock, struct nbd_request *request);
 int nbd_receive_reply(int csock, struct nbd_reply *reply);
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 55a966f..ca5bf53 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -620,6 +620,13 @@ qemu linux1.img -hdb nbd:unix:/tmp/my_socket
 qemu linux2.img -hdb nbd:unix:/tmp/my_socket
 @end example
 
+If the nbd-server uses named exports (since NBD 2.9.18) , you must use the
+"exportname" option:
+@example
+qemu -cdrom nbd:localhost:exportname=debian-500-ppc-netinst
+qemu -cdrom nbd:localhost:exportname=openSUSE-11.1-ppc-netinst
+@end example
+
 @node pcsys_network
 @section Network emulation
 
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 4e607cf..c28ac2f 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -398,7 +398,7 @@ int main(int argc, char **argv)
                 goto out;
             }
 
-            ret = nbd_receive_negotiate(sock, &size, &blocksize);
+            ret = nbd_receive_negotiate(sock, NULL, &size, &blocksize);
             if (ret == -1) {
                 ret = 1;
                 goto out;
-- 
1.7.0.4

Re: [Qemu-devel] [PATCH] Introduce NBD named exports.

[Kevin Wolf]

Am 24.08.2010 03:04, schrieb Laurent Vivier:
> This patch allows to connect Qemu using NBD protocol to an nbd-server
> using named exports.
> 
> For instance, if on the host "isoserver", in /etc/nbd-server/config, you have:
> 
> [generic]
> [debian-500-ppc-netinst]
>         exportname = /ISO/debian-500-powerpc-netinst.iso
> [Fedora-10-ppc-netinst]
>         exportname = /ISO/Fedora-10-ppc-netinst.iso
> 
> You can connect to it, using:
> 
>     qemu -cdrom nbd:isoserver:exportname=debian-500-ppc-netinst
>     qemu -cdrom nbd:isoserver:exportname=Fedora-10-ppc-netinst
> 
> NOTE: you need at least nbd-server 2.9.18
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  block/nbd.c   |   57 +++++++++++++++++++--------
>  nbd.c         |  119 ++++++++++++++++++++++++++++++++++++++++++++++++++------
>  nbd.h         |    5 ++-
>  qemu-doc.texi |    7 +++
>  qemu-nbd.c    |    2 +-
>  5 files changed, 158 insertions(+), 32 deletions(-)
> 
> diff --git a/block/nbd.c b/block/nbd.c
> index a1ec123..ee22b47 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -42,55 +42,78 @@ typedef struct BDRVNBDState {
>  static int nbd_open(BlockDriverState *bs, const char* filename, int flags)
>  {
>      BDRVNBDState *s = bs->opaque;
> +
> +    #define EN_OPTSTR ":exportname="

Hm, this makes EN_OPTSTR look like a local variable, but it's actually a
macro which is valid after the end of the function. So maybe move it out?

> +    char *file;
> +    char *name;
>      const char *host;
>      const char *unixpath;
>      int sock;
>      off_t size;
>      size_t blocksize;
>      int ret;
> +    int err = -EINVAL;
> +
> +    file = qemu_strdup(filename);
>  
> -    if (!strstart(filename, "nbd:", &host))
> -        return -EINVAL;
> +    name = strstr(file, EN_OPTSTR);
> +    if (name) {
> +        if (name[sizeof(EN_OPTSTR) - 1] == 0)
> +            goto out;

Please add braces here and throughout the patch (see CODING_STYLE)

Maybe using strlen(EN_OPTSTR) would be clearer? At first I missed the
fact that sizeof includes the null byte, but maybe it's just me.

> +        name[0] = 0;
> +        name += sizeof(EN_OPTSTR) - 1;
> +    }
> +
> +    if (!strstart(file, "nbd:", &host))
> +        goto out;
>  
>      if (strstart(host, "unix:", &unixpath)) {
>  
>          if (unixpath[0] != '/')
> -            return -EINVAL;
> +            goto out;
>  
>          sock = unix_socket_outgoing(unixpath);
>  
>      } else {
> -        uint16_t port;
> +        uint16_t port = NBD_DEFAULT_PORT;
>          char *p, *r;
>          char hostname[128];
>  
>          pstrcpy(hostname, 128, host);
>  
>          p = strchr(hostname, ':');
> -        if (p == NULL)
> -            return -EINVAL;
> +        if (p != NULL) {
> +            *p = '\0';
> +            p++;
>  
> -        *p = '\0';
> -        p++;
> +            port = strtol(p, &r, 0);
> +            if (r == p)
> +                goto out;
> +        } else if (name == NULL)
> +                goto out;

Wrong indentation level (and missing braces again)

>  
> -        port = strtol(p, &r, 0);
> -        if (r == p)
> -            return -EINVAL;
>          sock = tcp_socket_outgoing(hostname, port);
>      }
>  
> -    if (sock == -1)
> -        return -errno;
> +    if (sock == -1) {
> +        err = -errno;
> +        goto out;
> +    }
>  
> -    ret = nbd_receive_negotiate(sock, &size, &blocksize);
> -    if (ret == -1)
> -        return -errno;
> +    ret = nbd_receive_negotiate(sock, name, &size, &blocksize);
> +    if (ret == -1) {
> +        err = -errno;
> +        goto out;
> +    }
>  
>      s->sock = sock;
>      s->size = size;
>      s->blocksize = blocksize;
> +    err = 0;
>  
> -    return 0;
> +out:
> +    qemu_free(file);
> +    return err;
>  }
>  
>  static int nbd_read(BlockDriverState *bs, int64_t sector_num,
> diff --git a/nbd.c b/nbd.c
> index a9f295f..755613a 100644
> --- a/nbd.c
> +++ b/nbd.c
> @@ -62,6 +62,8 @@
>  #define NBD_SET_SIZE_BLOCKS	_IO(0xab, 7)
>  #define NBD_DISCONNECT          _IO(0xab, 8)
>  
> +#define NBD_OPT_EXPORT_NAME	(1 << 0)
> +
>  /* That's all folks */
>  
>  #define read_sync(fd, buffer, size) nbd_wr_sync(fd, buffer, size, true)
> @@ -296,22 +298,26 @@ int nbd_negotiate(int csock, off_t size)
>  	return 0;
>  }
>  
> -int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize)
> +int nbd_receive_negotiate(int csock, const char *name,
> +                          off_t *size, size_t *blocksize)
>  {
> -	char buf[8 + 8 + 8 + 128];
> -	uint64_t magic;
> +	char buf[256] = "\0\0\0\0\0\0\0\0\0";
> +	uint64_t magic, s;
> +	uint16_t tmp;
> +	uint32_t flags;

Hm, where are the flags actually used? There are a few places where they
are assigned, but I can't see any use.

>  
>  	TRACE("Receiving negotation.");
>  
> -	if (read_sync(csock, buf, sizeof(buf)) != sizeof(buf)) {
> +	if (read_sync(csock, buf, 8) != 8) {
>  		LOG("read failed");
>  		errno = EINVAL;
>  		return -1;
>  	}

I think this makes it easy to miss that you rely on getting null
termination by the initialisation of buf. Why not do the obvious thing
and just do a buf[8] = '\0' after the read?

> -
> -	magic = be64_to_cpup((uint64_t*)(buf + 8));
> -	*size = be64_to_cpup((uint64_t*)(buf + 16));
> -	*blocksize = 1024;
> +	if (strlen(buf) == 0) {
> +		LOG("server connection closed");
> +		errno = EINVAL;
> +		return -1;
> +	}
>  
>  	TRACE("Magic is %c%c%c%c%c%c%c%c",
>  	      qemu_isprint(buf[0]) ? buf[0] : '.',
> @@ -322,8 +328,6 @@ int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize)
>  	      qemu_isprint(buf[5]) ? buf[5] : '.',
>  	      qemu_isprint(buf[6]) ? buf[6] : '.',
>  	      qemu_isprint(buf[7]) ? buf[7] : '.');
> -	TRACE("Magic is 0x%" PRIx64, magic);
> -	TRACE("Size is %" PRIu64, *size);
>  
>  	if (memcmp(buf, "NBDMAGIC", 8) != 0) {
>  		LOG("Invalid magic received");
> @@ -331,10 +335,99 @@ int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize)
>  		return -1;
>  	}
>  
> -	TRACE("Checking magic");
> +	if (read_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
> +		LOG("read failed");
> +		errno = EINVAL;
> +		return -1;
> +	}
> +	magic = be64_to_cpu(magic);
> +	TRACE("Magic is 0x%" PRIx64, magic);
>  
> -	if (magic != 0x00420281861253LL) {
> -		LOG("Bad magic received");
> +	if (name) {
> +		uint32_t reserved = 0;
> +		uint32_t opt;
> +		uint32_t namesize;
> +
> +		TRACE("Checking magic (opts_magic)");
> +		if (magic != 0x49484156454F5054LL) {
> +			LOG("Bad magic received");
> +			errno = EINVAL;
> +			return -1;
> +		}
> +		if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {
> +			LOG("flags read failed");
> +			errno = EINVAL;
> +			return -1;
> +		}
> +		flags = be16_to_cpu(tmp) << 16;
> +		/* reserved for future use */
> +		if (write_sync(csock, &reserved, sizeof(reserved)) !=
> +		    sizeof(reserved)) {
> +			LOG("write failed (reserved)");
> +			errno = EINVAL;
> +			return -1;
> +		}
> +		/* write the export name */
> +		magic = cpu_to_be64(magic);
> +		if (write_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
> +			LOG("write failed (magic)");
> +			errno = EINVAL;
> +			return -1;
> +		}
> +		opt = cpu_to_be32(NBD_OPT_EXPORT_NAME);
> +		if (write_sync(csock, &opt, sizeof(opt)) != sizeof(opt)) {
> +			LOG("write failed (opt)");
> +			errno = EINVAL;
> +			return -1;
> +		}
> +		namesize = cpu_to_be32(strlen(name));
> +		if (write_sync(csock, &namesize, sizeof(namesize)) !=
> +		    sizeof(namesize)) {
> +			LOG("write failed (namesize)");
> +			errno = EINVAL;
> +			return -1;
> +		}
> +		if (write_sync(csock, (char*)name, strlen(name)) != strlen(name)) {
> +			LOG("write failed (name)");
> +			errno = EINVAL;
> +			return -1;
> +		}
> +	} else {
> +		TRACE("Checking magic (cli_magic)");
> +
> +		if (magic != 0x00420281861253LL) {
> +			LOG("Bad magic received");
> +			errno = EINVAL;
> +			return -1;
> +		}
> +	}
> +
> +	if (read_sync(csock, &s, sizeof(s)) != sizeof(s)) {
> +		LOG("read failed");
> +		errno = EINVAL;
> +		return -1;
> +	}
> +	*size = be64_to_cpu(s);
> +	*blocksize = 1024;
> +	TRACE("Size is %" PRIu64, *size);
> +
> +	if (!name) {
> +		if (read_sync(csock, &flags, sizeof(flags)) != sizeof(flags)) {
> +			LOG("read failed (flags)");
> +			errno = EINVAL;
> +			return -1;
> +		}
> +		flags = be64_to_cpu(flags);
> +	} else {
> +		if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {
> +			LOG("read failed (tmp)");
> +			errno = EINVAL;
> +			return -1;
> +		}
> +		flags |= be32_to_cpu(tmp);
> +	}
> +	if (read_sync(csock, &buf, 124) != 124) {
> +		LOG("read failed (buf)");
>  		errno = EINVAL;
>  		return -1;
>  	}
> diff --git a/nbd.h b/nbd.h
> index 5a1fbdf..17be0fa 100644
> --- a/nbd.h
> +++ b/nbd.h
> @@ -42,6 +42,8 @@ enum {
>      NBD_CMD_DISC = 2
>  };
>  
> +#define NBD_DEFAULT_PORT	10809
> +
>  size_t nbd_wr_sync(int fd, void *buffer, size_t size, bool do_read);
>  int tcp_socket_outgoing(const char *address, uint16_t port);
>  int tcp_socket_incoming(const char *address, uint16_t port);
> @@ -49,7 +51,8 @@ int unix_socket_outgoing(const char *path);
>  int unix_socket_incoming(const char *path);
>  
>  int nbd_negotiate(int csock, off_t size);
> -int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize);
> +int nbd_receive_negotiate(int csock, const char *name,
> +                          off_t *size, size_t *blocksize);
>  int nbd_init(int fd, int csock, off_t size, size_t blocksize);
>  int nbd_send_request(int csock, struct nbd_request *request);
>  int nbd_receive_reply(int csock, struct nbd_reply *reply);
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index 55a966f..ca5bf53 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -620,6 +620,13 @@ qemu linux1.img -hdb nbd:unix:/tmp/my_socket
>  qemu linux2.img -hdb nbd:unix:/tmp/my_socket
>  @end example
>  
> +If the nbd-server uses named exports (since NBD 2.9.18) , you must use the

Please remove the space before the comma.

I don't have a new enough ndbserver to test if it actually works (and I
also haven't checked the spec for the new parts), so I'll have to trust
your tests there.

Kevin

Re: [Qemu-devel] [PATCH] Introduce NBD named exports.

[Laurent Vivier]

Le mardi 24 août 2010 à 10:59 +0200, Kevin Wolf a écrit :
> Am 24.08.2010 03:04, schrieb Laurent Vivier:
> > This patch allows to connect Qemu using NBD protocol to an nbd-server
> > using named exports.
> > 
> > For instance, if on the host "isoserver", in /etc/nbd-server/config, you have:
> > 
> > [generic]
> > [debian-500-ppc-netinst]
> >         exportname = /ISO/debian-500-powerpc-netinst.iso
> > [Fedora-10-ppc-netinst]
> >         exportname = /ISO/Fedora-10-ppc-netinst.iso
> > 
> > You can connect to it, using:
> > 
> >     qemu -cdrom nbd:isoserver:exportname=debian-500-ppc-netinst
> >     qemu -cdrom nbd:isoserver:exportname=Fedora-10-ppc-netinst
> > 
> > NOTE: you need at least nbd-server 2.9.18
> > Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> > ---
> >  block/nbd.c   |   57 +++++++++++++++++++--------
> >  nbd.c         |  119 ++++++++++++++++++++++++++++++++++++++++++++++++++------
> >  nbd.h         |    5 ++-
> >  qemu-doc.texi |    7 +++
> >  qemu-nbd.c    |    2 +-
> >  5 files changed, 158 insertions(+), 32 deletions(-)
> > 
> > diff --git a/block/nbd.c b/block/nbd.c
> > index a1ec123..ee22b47 100644
> > --- a/block/nbd.c
> > +++ b/block/nbd.c
> > @@ -42,55 +42,78 @@ typedef struct BDRVNBDState {
> >  static int nbd_open(BlockDriverState *bs, const char* filename, int flags)
> >  {
> >      BDRVNBDState *s = bs->opaque;
> > +
> > +    #define EN_OPTSTR ":exportname="
> 
> Hm, this makes EN_OPTSTR look like a local variable, but it's actually a
> macro which is valid after the end of the function. So maybe move it out?

I agree, but have a look at curl_open().

> 
> > +    char *file;
> > +    char *name;
> >      const char *host;
> >      const char *unixpath;
> >      int sock;
> >      off_t size;
> >      size_t blocksize;
> >      int ret;
> > +    int err = -EINVAL;
> > +
> > +    file = qemu_strdup(filename);
> >  
> > -    if (!strstart(filename, "nbd:", &host))
> > -        return -EINVAL;
> > +    name = strstr(file, EN_OPTSTR);
> > +    if (name) {
> > +        if (name[sizeof(EN_OPTSTR) - 1] == 0)
> > +            goto out;
> 
> Please add braces here and throughout the patch (see CODING_STYLE)

OK.

> Maybe using strlen(EN_OPTSTR) would be clearer? At first I missed the
> fact that sizeof includes the null byte, but maybe it's just me.

I think sizeof() is resolved at compile time whereas strlen() it is at
runtime. For me, it is better, it like to have a constant.

> 
> > +        name[0] = 0;
> > +        name += sizeof(EN_OPTSTR) - 1;
> > +    }
> > +
> > +    if (!strstart(file, "nbd:", &host))
> > +        goto out;
> >  
> >      if (strstart(host, "unix:", &unixpath)) {
> >  
> >          if (unixpath[0] != '/')
> > -            return -EINVAL;
> > +            goto out;
> >  
> >          sock = unix_socket_outgoing(unixpath);
> >  
> >      } else {
> > -        uint16_t port;
> > +        uint16_t port = NBD_DEFAULT_PORT;
> >          char *p, *r;
> >          char hostname[128];
> >  
> >          pstrcpy(hostname, 128, host);
> >  
> >          p = strchr(hostname, ':');
> > -        if (p == NULL)
> > -            return -EINVAL;
> > +        if (p != NULL) {
> > +            *p = '\0';
> > +            p++;
> >  
> > -        *p = '\0';
> > -        p++;
> > +            port = strtol(p, &r, 0);
> > +            if (r == p)
> > +                goto out;
> > +        } else if (name == NULL)
> > +                goto out;
> 
> Wrong indentation level (and missing braces again)

OK

> 
> >  
> > -        port = strtol(p, &r, 0);
> > -        if (r == p)
> > -            return -EINVAL;
> >          sock = tcp_socket_outgoing(hostname, port);
> >      }
> >  
> > -    if (sock == -1)
> > -        return -errno;
> > +    if (sock == -1) {
> > +        err = -errno;
> > +        goto out;
> > +    }
> >  
> > -    ret = nbd_receive_negotiate(sock, &size, &blocksize);
> > -    if (ret == -1)
> > -        return -errno;
> > +    ret = nbd_receive_negotiate(sock, name, &size, &blocksize);
> > +    if (ret == -1) {
> > +        err = -errno;
> > +        goto out;
> > +    }
> >  
> >      s->sock = sock;
> >      s->size = size;
> >      s->blocksize = blocksize;
> > +    err = 0;
> >  
> > -    return 0;
> > +out:
> > +    qemu_free(file);
> > +    return err;
> >  }
> >  
> >  static int nbd_read(BlockDriverState *bs, int64_t sector_num,
> > diff --git a/nbd.c b/nbd.c
> > index a9f295f..755613a 100644
> > --- a/nbd.c
> > +++ b/nbd.c
> > @@ -62,6 +62,8 @@
> >  #define NBD_SET_SIZE_BLOCKS	_IO(0xab, 7)
> >  #define NBD_DISCONNECT          _IO(0xab, 8)
> >  
> > +#define NBD_OPT_EXPORT_NAME	(1 << 0)
> > +
> >  /* That's all folks */
> >  
> >  #define read_sync(fd, buffer, size) nbd_wr_sync(fd, buffer, size, true)
> > @@ -296,22 +298,26 @@ int nbd_negotiate(int csock, off_t size)
> >  	return 0;
> >  }
> >  
> > -int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize)
> > +int nbd_receive_negotiate(int csock, const char *name,
> > +                          off_t *size, size_t *blocksize)
> >  {
> > -	char buf[8 + 8 + 8 + 128];
> > -	uint64_t magic;
> > +	char buf[256] = "\0\0\0\0\0\0\0\0\0";
> > +	uint64_t magic, s;
> > +	uint16_t tmp;
> > +	uint32_t flags;
> 
> Hm, where are the flags actually used? There are a few places where they
> are assigned, but I can't see any use.

It is never used. But as it is in the file, I take it. But I'll modify
the function to return flags as it is done in nbd-client (the reference
implementation).

> 
> >  
> >  	TRACE("Receiving negotation.");
> >  
> > -	if (read_sync(csock, buf, sizeof(buf)) != sizeof(buf)) {
> > +	if (read_sync(csock, buf, 8) != 8) {
> >  		LOG("read failed");
> >  		errno = EINVAL;
> >  		return -1;
> >  	}
> 
> I think this makes it easy to miss that you rely on getting null
> termination by the initialisation of buf. Why not do the obvious thing
> and just do a buf[8] = '\0' after the read?

because it is done like that in nbd-client.c... but I agree.

> 
> > -
> > -	magic = be64_to_cpup((uint64_t*)(buf + 8));
> > -	*size = be64_to_cpup((uint64_t*)(buf + 16));
> > -	*blocksize = 1024;
> > +	if (strlen(buf) == 0) {
> > +		LOG("server connection closed");
> > +		errno = EINVAL;
> > +		return -1;
> > +	}
> >  
> >  	TRACE("Magic is %c%c%c%c%c%c%c%c",
> >  	      qemu_isprint(buf[0]) ? buf[0] : '.',
> > @@ -322,8 +328,6 @@ int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize)
> >  	      qemu_isprint(buf[5]) ? buf[5] : '.',
> >  	      qemu_isprint(buf[6]) ? buf[6] : '.',
> >  	      qemu_isprint(buf[7]) ? buf[7] : '.');
> > -	TRACE("Magic is 0x%" PRIx64, magic);
> > -	TRACE("Size is %" PRIu64, *size);
> >  
> >  	if (memcmp(buf, "NBDMAGIC", 8) != 0) {
> >  		LOG("Invalid magic received");
> > @@ -331,10 +335,99 @@ int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize)
> >  		return -1;
> >  	}
> >  
> > -	TRACE("Checking magic");
> > +	if (read_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
> > +		LOG("read failed");
> > +		errno = EINVAL;
> > +		return -1;
> > +	}
> > +	magic = be64_to_cpu(magic);
> > +	TRACE("Magic is 0x%" PRIx64, magic);
> >  
> > -	if (magic != 0x00420281861253LL) {
> > -		LOG("Bad magic received");
> > +	if (name) {
> > +		uint32_t reserved = 0;
> > +		uint32_t opt;
> > +		uint32_t namesize;
> > +
> > +		TRACE("Checking magic (opts_magic)");
> > +		if (magic != 0x49484156454F5054LL) {
> > +			LOG("Bad magic received");
> > +			errno = EINVAL;
> > +			return -1;
> > +		}
> > +		if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {
> > +			LOG("flags read failed");
> > +			errno = EINVAL;
> > +			return -1;
> > +		}
> > +		flags = be16_to_cpu(tmp) << 16;
> > +		/* reserved for future use */
> > +		if (write_sync(csock, &reserved, sizeof(reserved)) !=
> > +		    sizeof(reserved)) {
> > +			LOG("write failed (reserved)");
> > +			errno = EINVAL;
> > +			return -1;
> > +		}
> > +		/* write the export name */
> > +		magic = cpu_to_be64(magic);
> > +		if (write_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
> > +			LOG("write failed (magic)");
> > +			errno = EINVAL;
> > +			return -1;
> > +		}
> > +		opt = cpu_to_be32(NBD_OPT_EXPORT_NAME);
> > +		if (write_sync(csock, &opt, sizeof(opt)) != sizeof(opt)) {
> > +			LOG("write failed (opt)");
> > +			errno = EINVAL;
> > +			return -1;
> > +		}
> > +		namesize = cpu_to_be32(strlen(name));
> > +		if (write_sync(csock, &namesize, sizeof(namesize)) !=
> > +		    sizeof(namesize)) {
> > +			LOG("write failed (namesize)");
> > +			errno = EINVAL;
> > +			return -1;
> > +		}
> > +		if (write_sync(csock, (char*)name, strlen(name)) != strlen(name)) {
> > +			LOG("write failed (name)");
> > +			errno = EINVAL;
> > +			return -1;
> > +		}
> > +	} else {
> > +		TRACE("Checking magic (cli_magic)");
> > +
> > +		if (magic != 0x00420281861253LL) {
> > +			LOG("Bad magic received");
> > +			errno = EINVAL;
> > +			return -1;
> > +		}
> > +	}
> > +
> > +	if (read_sync(csock, &s, sizeof(s)) != sizeof(s)) {
> > +		LOG("read failed");
> > +		errno = EINVAL;
> > +		return -1;
> > +	}
> > +	*size = be64_to_cpu(s);
> > +	*blocksize = 1024;
> > +	TRACE("Size is %" PRIu64, *size);
> > +
> > +	if (!name) {
> > +		if (read_sync(csock, &flags, sizeof(flags)) != sizeof(flags)) {
> > +			LOG("read failed (flags)");
> > +			errno = EINVAL;
> > +			return -1;
> > +		}
> > +		flags = be64_to_cpu(flags);
> > +	} else {
> > +		if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {
> > +			LOG("read failed (tmp)");
> > +			errno = EINVAL;
> > +			return -1;
> > +		}
> > +		flags |= be32_to_cpu(tmp);
> > +	}
> > +	if (read_sync(csock, &buf, 124) != 124) {
> > +		LOG("read failed (buf)");
> >  		errno = EINVAL;
> >  		return -1;
> >  	}
> > diff --git a/nbd.h b/nbd.h
> > index 5a1fbdf..17be0fa 100644
> > --- a/nbd.h
> > +++ b/nbd.h
> > @@ -42,6 +42,8 @@ enum {
> >      NBD_CMD_DISC = 2
> >  };
> >  
> > +#define NBD_DEFAULT_PORT	10809
> > +
> >  size_t nbd_wr_sync(int fd, void *buffer, size_t size, bool do_read);
> >  int tcp_socket_outgoing(const char *address, uint16_t port);
> >  int tcp_socket_incoming(const char *address, uint16_t port);
> > @@ -49,7 +51,8 @@ int unix_socket_outgoing(const char *path);
> >  int unix_socket_incoming(const char *path);
> >  
> >  int nbd_negotiate(int csock, off_t size);
> > -int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize);
> > +int nbd_receive_negotiate(int csock, const char *name,
> > +                          off_t *size, size_t *blocksize);
> >  int nbd_init(int fd, int csock, off_t size, size_t blocksize);
> >  int nbd_send_request(int csock, struct nbd_request *request);
> >  int nbd_receive_reply(int csock, struct nbd_reply *reply);
> > diff --git a/qemu-doc.texi b/qemu-doc.texi
> > index 55a966f..ca5bf53 100644
> > --- a/qemu-doc.texi
> > +++ b/qemu-doc.texi
> > @@ -620,6 +620,13 @@ qemu linux1.img -hdb nbd:unix:/tmp/my_socket
> >  qemu linux2.img -hdb nbd:unix:/tmp/my_socket
> >  @end example
> >  
> > +If the nbd-server uses named exports (since NBD 2.9.18) , you must use the
> 
> Please remove the space before the comma.

OK

> I don't have a new enough ndbserver to test if it actually works (and I

You can test against an old nbdserver to check it is always working ;-)
(or qemu-nbd).

> also haven't checked the spec for the new parts), so I'll have to trust
> your tests there.

There is no spec (what is a spec ;-) ?), only a reference implementation
in nbd-client.c and nbd-server.c from http://nbd.sf.net.

Thank you for the review.

Patch follows nearly...

> 
> Kevin

Laurent
-- 
--------------------- laurent@vivier.eu ----------------------
"Tout ce qui est impossible reste à accomplir"    Jules Verne
"Things are only impossible until they're not" Jean-Luc Picard

[Qemu-devel] [PATCH][v2] Introduce NBD named exports.

[Laurent Vivier]

This patch allows to connect Qemu using NBD protocol to an nbd-server
using named exports.

For instance, if on the host "isoserver", in /etc/nbd-server/config, you have:

[generic]
[debian-500-ppc-netinst]
        exportname = /ISO/debian-500-powerpc-netinst.iso
[Fedora-10-ppc-netinst]
        exportname = /ISO/Fedora-10-ppc-netinst.iso

You can connect to it, using:

    qemu -cdrom nbd:isoserver:exportname=debian-500-ppc-netinst
    qemu -cdrom nbd:isoserver:exportname=Fedora-10-ppc-netinst

NOTE: you need at least nbd-server 2.9.18

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
v2: address Kevin comments (CODING STYLE)

 block/nbd.c   |   68 +++++++++++++++++++++++---------
 nbd.c         |  118 +++++++++++++++++++++++++++++++++++++++++++++++++++------
 nbd.h         |    5 ++-
 qemu-doc.texi |    7 +++
 qemu-nbd.c    |    4 +-
 5 files changed, 169 insertions(+), 33 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index a1ec123..5012b70 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -33,6 +33,8 @@
 #include <sys/types.h>
 #include <unistd.h>
 
+#define EN_OPTSTR ":exportname="
+
 typedef struct BDRVNBDState {
     int sock;
     off_t size;
@@ -42,55 +44,83 @@ typedef struct BDRVNBDState {
 static int nbd_open(BlockDriverState *bs, const char* filename, int flags)
 {
     BDRVNBDState *s = bs->opaque;
+    uint32_t nbdflags;
+
+    char *file;
+    char *name;
     const char *host;
     const char *unixpath;
     int sock;
     off_t size;
     size_t blocksize;
     int ret;
+    int err = -EINVAL;
+
+    file = qemu_strdup(filename);
 
-    if (!strstart(filename, "nbd:", &host))
-        return -EINVAL;
+    name = strstr(file, EN_OPTSTR);
+    if (name) {
+        if (name[sizeof(EN_OPTSTR) - 1] == 0) {
+            goto out;
+        }
+        name[0] = 0;
+        name += sizeof(EN_OPTSTR) - 1;
+    }
+
+    if (!strstart(file, "nbd:", &host)) {
+        goto out;
+    }
 
     if (strstart(host, "unix:", &unixpath)) {
 
-        if (unixpath[0] != '/')
-            return -EINVAL;
+        if (unixpath[0] != '/') {
+            goto out;
+        }
 
         sock = unix_socket_outgoing(unixpath);
 
     } else {
-        uint16_t port;
+        uint16_t port = NBD_DEFAULT_PORT;
         char *p, *r;
         char hostname[128];
 
         pstrcpy(hostname, 128, host);
 
         p = strchr(hostname, ':');
-        if (p == NULL)
-            return -EINVAL;
+        if (p != NULL) {
+            *p = '\0';
+            p++;
+
+            port = strtol(p, &r, 0);
+            if (r == p) {
+                goto out;
+            }
+        } else if (name == NULL) {
+            goto out;
+        }
 
-        *p = '\0';
-        p++;
-
-        port = strtol(p, &r, 0);
-        if (r == p)
-            return -EINVAL;
         sock = tcp_socket_outgoing(hostname, port);
     }
 
-    if (sock == -1)
-        return -errno;
+    if (sock == -1) {
+        err = -errno;
+        goto out;
+    }
 
-    ret = nbd_receive_negotiate(sock, &size, &blocksize);
-    if (ret == -1)
-        return -errno;
+    ret = nbd_receive_negotiate(sock, name, &nbdflags, &size, &blocksize);
+    if (ret == -1) {
+        err = -errno;
+        goto out;
+    }
 
     s->sock = sock;
     s->size = size;
     s->blocksize = blocksize;
+    err = 0;
 
-    return 0;
+out:
+    qemu_free(file);
+    return err;
 }
 
 static int nbd_read(BlockDriverState *bs, int64_t sector_num,
diff --git a/nbd.c b/nbd.c
index a9f295f..30dec8d 100644
--- a/nbd.c
+++ b/nbd.c
@@ -62,6 +62,8 @@
 #define NBD_SET_SIZE_BLOCKS	_IO(0xab, 7)
 #define NBD_DISCONNECT          _IO(0xab, 8)
 
+#define NBD_OPT_EXPORT_NAME	(1 << 0)
+
 /* That's all folks */
 
 #define read_sync(fd, buffer, size) nbd_wr_sync(fd, buffer, size, true)
@@ -296,22 +298,27 @@ int nbd_negotiate(int csock, off_t size)
 	return 0;
 }
 
-int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize)
+int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags,
+                          off_t *size, size_t *blocksize)
 {
-	char buf[8 + 8 + 8 + 128];
-	uint64_t magic;
+	char buf[256];
+	uint64_t magic, s;
+	uint16_t tmp;
 
 	TRACE("Receiving negotation.");
 
-	if (read_sync(csock, buf, sizeof(buf)) != sizeof(buf)) {
+	if (read_sync(csock, buf, 8) != 8) {
 		LOG("read failed");
 		errno = EINVAL;
 		return -1;
 	}
 
-	magic = be64_to_cpup((uint64_t*)(buf + 8));
-	*size = be64_to_cpup((uint64_t*)(buf + 16));
-	*blocksize = 1024;
+	buf[8] = '\0';
+	if (strlen(buf) == 0) {
+		LOG("server connection closed");
+		errno = EINVAL;
+		return -1;
+	}
 
 	TRACE("Magic is %c%c%c%c%c%c%c%c",
 	      qemu_isprint(buf[0]) ? buf[0] : '.',
@@ -322,8 +329,6 @@ int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize)
 	      qemu_isprint(buf[5]) ? buf[5] : '.',
 	      qemu_isprint(buf[6]) ? buf[6] : '.',
 	      qemu_isprint(buf[7]) ? buf[7] : '.');
-	TRACE("Magic is 0x%" PRIx64, magic);
-	TRACE("Size is %" PRIu64, *size);
 
 	if (memcmp(buf, "NBDMAGIC", 8) != 0) {
 		LOG("Invalid magic received");
@@ -331,10 +336,99 @@ int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize)
 		return -1;
 	}
 
-	TRACE("Checking magic");
+	if (read_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
+		LOG("read failed");
+		errno = EINVAL;
+		return -1;
+	}
+	magic = be64_to_cpu(magic);
+	TRACE("Magic is 0x%" PRIx64, magic);
+
+	if (name) {
+		uint32_t reserved = 0;
+		uint32_t opt;
+		uint32_t namesize;
+
+		TRACE("Checking magic (opts_magic)");
+		if (magic != 0x49484156454F5054LL) {
+			LOG("Bad magic received");
+			errno = EINVAL;
+			return -1;
+		}
+		if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {
+			LOG("flags read failed");
+			errno = EINVAL;
+			return -1;
+		}
+		*flags = be16_to_cpu(tmp) << 16;
+		/* reserved for future use */
+		if (write_sync(csock, &reserved, sizeof(reserved)) !=
+		    sizeof(reserved)) {
+			LOG("write failed (reserved)");
+			errno = EINVAL;
+			return -1;
+		}
+		/* write the export name */
+		magic = cpu_to_be64(magic);
+		if (write_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
+			LOG("write failed (magic)");
+			errno = EINVAL;
+			return -1;
+		}
+		opt = cpu_to_be32(NBD_OPT_EXPORT_NAME);
+		if (write_sync(csock, &opt, sizeof(opt)) != sizeof(opt)) {
+			LOG("write failed (opt)");
+			errno = EINVAL;
+			return -1;
+		}
+		namesize = cpu_to_be32(strlen(name));
+		if (write_sync(csock, &namesize, sizeof(namesize)) !=
+		    sizeof(namesize)) {
+			LOG("write failed (namesize)");
+			errno = EINVAL;
+			return -1;
+		}
+		if (write_sync(csock, (char*)name, strlen(name)) != strlen(name)) {
+			LOG("write failed (name)");
+			errno = EINVAL;
+			return -1;
+		}
+	} else {
+		TRACE("Checking magic (cli_magic)");
+
+		if (magic != 0x00420281861253LL) {
+			LOG("Bad magic received");
+			errno = EINVAL;
+			return -1;
+		}
+	}
+
+	if (read_sync(csock, &s, sizeof(s)) != sizeof(s)) {
+		LOG("read failed");
+		errno = EINVAL;
+		return -1;
+	}
+	*size = be64_to_cpu(s);
+	*blocksize = 1024;
+	TRACE("Size is %" PRIu64, *size);
 
-	if (magic != 0x00420281861253LL) {
-		LOG("Bad magic received");
+	if (!name) {
+		if (read_sync(csock, flags, sizeof(*flags)) != sizeof(*flags)) {
+			LOG("read failed (flags)");
+			errno = EINVAL;
+			return -1;
+		}
+		*flags = be32_to_cpup(flags);
+	} else {
+		if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {
+			LOG("read failed (tmp)");
+			errno = EINVAL;
+			return -1;
+		}
+		*flags |= be32_to_cpu(tmp);
+	}
+	if (read_sync(csock, &buf, 124) != 124) {
+		LOG("read failed (buf)");
 		errno = EINVAL;
 		return -1;
 	}
diff --git a/nbd.h b/nbd.h
index 5a1fbdf..8ff65a1 100644
--- a/nbd.h
+++ b/nbd.h
@@ -42,6 +42,8 @@ enum {
     NBD_CMD_DISC = 2
 };
 
+#define NBD_DEFAULT_PORT	10809
+
 size_t nbd_wr_sync(int fd, void *buffer, size_t size, bool do_read);
 int tcp_socket_outgoing(const char *address, uint16_t port);
 int tcp_socket_incoming(const char *address, uint16_t port);
@@ -49,7 +51,8 @@ int unix_socket_outgoing(const char *path);
 int unix_socket_incoming(const char *path);
 
 int nbd_negotiate(int csock, off_t size);
-int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize);
+int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags,
+                          off_t *size, size_t *blocksize);
 int nbd_init(int fd, int csock, off_t size, size_t blocksize);
 int nbd_send_request(int csock, struct nbd_request *request);
 int nbd_receive_reply(int csock, struct nbd_reply *reply);
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 55a966f..d7d760f 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -620,6 +620,13 @@ qemu linux1.img -hdb nbd:unix:/tmp/my_socket
 qemu linux2.img -hdb nbd:unix:/tmp/my_socket
 @end example
 
+If the nbd-server uses named exports (since NBD 2.9.18), you must use the
+"exportname" option:
+@example
+qemu -cdrom nbd:localhost:exportname=debian-500-ppc-netinst
+qemu -cdrom nbd:localhost:exportname=openSUSE-11.1-ppc-netinst
+@end example
+
 @node pcsys_network
 @section Network emulation
 
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 4e607cf..41e5c5d 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -230,6 +230,7 @@ int main(int argc, char **argv)
     int nb_fds = 0;
     int max_fd;
     int persistent = 0;
+    uint32_t nbdflags;
 
     while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
         switch (ch) {
@@ -398,7 +399,8 @@ int main(int argc, char **argv)
                 goto out;
             }
 
-            ret = nbd_receive_negotiate(sock, &size, &blocksize);
+            ret = nbd_receive_negotiate(sock, NULL, &nbdflags,
+					&size, &blocksize);
             if (ret == -1) {
                 ret = 1;
                 goto out;
-- 
1.7.0.4

Re: [Qemu-devel] [PATCH] Introduce NBD named exports.

[Kevin Wolf]

Am 25.08.2010 00:21, schrieb Laurent Vivier:
> Le mardi 24 août 2010 à 10:59 +0200, Kevin Wolf a écrit :
>> Am 24.08.2010 03:04, schrieb Laurent Vivier:
>>> This patch allows to connect Qemu using NBD protocol to an nbd-server
>>> using named exports.
>>>
>>> For instance, if on the host "isoserver", in /etc/nbd-server/config, you have:
>>>
>>> [generic]
>>> [debian-500-ppc-netinst]
>>>         exportname = /ISO/debian-500-powerpc-netinst.iso
>>> [Fedora-10-ppc-netinst]
>>>         exportname = /ISO/Fedora-10-ppc-netinst.iso
>>>
>>> You can connect to it, using:
>>>
>>>     qemu -cdrom nbd:isoserver:exportname=debian-500-ppc-netinst
>>>     qemu -cdrom nbd:isoserver:exportname=Fedora-10-ppc-netinst
>>>
>>> NOTE: you need at least nbd-server 2.9.18
>>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>>> ---
>>>  block/nbd.c   |   57 +++++++++++++++++++--------
>>>  nbd.c         |  119 ++++++++++++++++++++++++++++++++++++++++++++++++++------
>>>  nbd.h         |    5 ++-
>>>  qemu-doc.texi |    7 +++
>>>  qemu-nbd.c    |    2 +-
>>>  5 files changed, 158 insertions(+), 32 deletions(-)
>>>
>>> diff --git a/block/nbd.c b/block/nbd.c
>>> index a1ec123..ee22b47 100644
>>> --- a/block/nbd.c
>>> +++ b/block/nbd.c
>>> @@ -42,55 +42,78 @@ typedef struct BDRVNBDState {
>>>  static int nbd_open(BlockDriverState *bs, const char* filename, int flags)
>>>  {
>>>      BDRVNBDState *s = bs->opaque;
>>> +
>>> +    #define EN_OPTSTR ":exportname="
>>
>> Hm, this makes EN_OPTSTR look like a local variable, but it's actually a
>> macro which is valid after the end of the function. So maybe move it out?
> 
> I agree, but have a look at curl_open().

Seems I should have reviewed the curl patch when it came in. :-)

>> Maybe using strlen(EN_OPTSTR) would be clearer? At first I missed the
>> fact that sizeof includes the null byte, but maybe it's just me.
> 
> I think sizeof() is resolved at compile time whereas strlen() it is at
> runtime. For me, it is better, it like to have a constant.

gcc is smart enough to make strlen of a literal string a constant, too.
Besides, opening a connection isn't really performance critical.

I guess it's a matter of taste, though. So if you like it better as it
is, just leave it.

>> I don't have a new enough ndbserver to test if it actually works (and I
> 
> You can test against an old nbdserver to check it is always working ;-)
> (or qemu-nbd).

Sure, I can test for regressions in the old thing.

>> also haven't checked the spec for the new parts), so I'll have to trust
>> your tests there.
> 
> There is no spec (what is a spec ;-) ?), only a reference implementation
> in nbd-client.c and nbd-server.c from http://nbd.sf.net.

Oh right, I forgot, the world outside qemu is just as bad. :-)

Kevin

[Qemu-devel] [PATCH][v3] Introduce NBD named exports.

[Laurent Vivier]

This patch allows to connect Qemu using NBD protocol to an nbd-server
using named exports.

For instance, if on the host "isoserver", in /etc/nbd-server/config, you have:

[generic]
[debian-500-ppc-netinst]
        exportname = /ISO/debian-500-powerpc-netinst.iso
[Fedora-10-ppc-netinst]
        exportname = /ISO/Fedora-10-ppc-netinst.iso

You can connect to it, using:

    qemu -cdrom nbd:isoserver:exportname=debian-500-ppc-netinst
    qemu -cdrom nbd:isoserver:exportname=Fedora-10-ppc-netinst

NOTE: you need at least nbd-server 2.9.18

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
v3: change sizeof() by strlen()
v2: address Kevin comments (CODING STYLE)

 block/nbd.c   |   68 +++++++++++++++++++++++---------
 nbd.c         |  118 +++++++++++++++++++++++++++++++++++++++++++++++++++------
 nbd.h         |    5 ++-
 qemu-doc.texi |    7 +++
 qemu-nbd.c    |    4 +-
 5 files changed, 169 insertions(+), 33 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index a1ec123..5e9d6cb 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -33,6 +33,8 @@
 #include <sys/types.h>
 #include <unistd.h>
 
+#define EN_OPTSTR ":exportname="
+
 typedef struct BDRVNBDState {
     int sock;
     off_t size;
@@ -42,55 +44,83 @@ typedef struct BDRVNBDState {
 static int nbd_open(BlockDriverState *bs, const char* filename, int flags)
 {
     BDRVNBDState *s = bs->opaque;
+    uint32_t nbdflags;
+
+    char *file;
+    char *name;
     const char *host;
     const char *unixpath;
     int sock;
     off_t size;
     size_t blocksize;
     int ret;
+    int err = -EINVAL;
+
+    file = qemu_strdup(filename);
 
-    if (!strstart(filename, "nbd:", &host))
-        return -EINVAL;
+    name = strstr(file, EN_OPTSTR);
+    if (name) {
+        if (name[strlen(EN_OPTSTR)] == 0) {
+            goto out;
+        }
+        name[0] = 0;
+        name += strlen(EN_OPTSTR);
+    }
+
+    if (!strstart(file, "nbd:", &host)) {
+        goto out;
+    }
 
     if (strstart(host, "unix:", &unixpath)) {
 
-        if (unixpath[0] != '/')
-            return -EINVAL;
+        if (unixpath[0] != '/') {
+            goto out;
+        }
 
         sock = unix_socket_outgoing(unixpath);
 
     } else {
-        uint16_t port;
+        uint16_t port = NBD_DEFAULT_PORT;
         char *p, *r;
         char hostname[128];
 
         pstrcpy(hostname, 128, host);
 
         p = strchr(hostname, ':');
-        if (p == NULL)
-            return -EINVAL;
+        if (p != NULL) {
+            *p = '\0';
+            p++;
+
+            port = strtol(p, &r, 0);
+            if (r == p) {
+                goto out;
+            }
+        } else if (name == NULL) {
+            goto out;
+        }
 
-        *p = '\0';
-        p++;
-
-        port = strtol(p, &r, 0);
-        if (r == p)
-            return -EINVAL;
         sock = tcp_socket_outgoing(hostname, port);
     }
 
-    if (sock == -1)
-        return -errno;
+    if (sock == -1) {
+        err = -errno;
+        goto out;
+    }
 
-    ret = nbd_receive_negotiate(sock, &size, &blocksize);
-    if (ret == -1)
-        return -errno;
+    ret = nbd_receive_negotiate(sock, name, &nbdflags, &size, &blocksize);
+    if (ret == -1) {
+        err = -errno;
+        goto out;
+    }
 
     s->sock = sock;
     s->size = size;
     s->blocksize = blocksize;
+    err = 0;
 
-    return 0;
+out:
+    qemu_free(file);
+    return err;
 }
 
 static int nbd_read(BlockDriverState *bs, int64_t sector_num,
diff --git a/nbd.c b/nbd.c
index a9f295f..30dec8d 100644
--- a/nbd.c
+++ b/nbd.c
@@ -62,6 +62,8 @@
 #define NBD_SET_SIZE_BLOCKS	_IO(0xab, 7)
 #define NBD_DISCONNECT          _IO(0xab, 8)
 
+#define NBD_OPT_EXPORT_NAME	(1 << 0)
+
 /* That's all folks */
 
 #define read_sync(fd, buffer, size) nbd_wr_sync(fd, buffer, size, true)
@@ -296,22 +298,27 @@ int nbd_negotiate(int csock, off_t size)
 	return 0;
 }
 
-int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize)
+int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags,
+                          off_t *size, size_t *blocksize)
 {
-	char buf[8 + 8 + 8 + 128];
-	uint64_t magic;
+	char buf[256];
+	uint64_t magic, s;
+	uint16_t tmp;
 
 	TRACE("Receiving negotation.");
 
-	if (read_sync(csock, buf, sizeof(buf)) != sizeof(buf)) {
+	if (read_sync(csock, buf, 8) != 8) {
 		LOG("read failed");
 		errno = EINVAL;
 		return -1;
 	}
 
-	magic = be64_to_cpup((uint64_t*)(buf + 8));
-	*size = be64_to_cpup((uint64_t*)(buf + 16));
-	*blocksize = 1024;
+	buf[8] = '\0';
+	if (strlen(buf) == 0) {
+		LOG("server connection closed");
+		errno = EINVAL;
+		return -1;
+	}
 
 	TRACE("Magic is %c%c%c%c%c%c%c%c",
 	      qemu_isprint(buf[0]) ? buf[0] : '.',
@@ -322,8 +329,6 @@ int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize)
 	      qemu_isprint(buf[5]) ? buf[5] : '.',
 	      qemu_isprint(buf[6]) ? buf[6] : '.',
 	      qemu_isprint(buf[7]) ? buf[7] : '.');
-	TRACE("Magic is 0x%" PRIx64, magic);
-	TRACE("Size is %" PRIu64, *size);
 
 	if (memcmp(buf, "NBDMAGIC", 8) != 0) {
 		LOG("Invalid magic received");
@@ -331,10 +336,99 @@ int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize)
 		return -1;
 	}
 
-	TRACE("Checking magic");
+	if (read_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
+		LOG("read failed");
+		errno = EINVAL;
+		return -1;
+	}
+	magic = be64_to_cpu(magic);
+	TRACE("Magic is 0x%" PRIx64, magic);
+
+	if (name) {
+		uint32_t reserved = 0;
+		uint32_t opt;
+		uint32_t namesize;
+
+		TRACE("Checking magic (opts_magic)");
+		if (magic != 0x49484156454F5054LL) {
+			LOG("Bad magic received");
+			errno = EINVAL;
+			return -1;
+		}
+		if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {
+			LOG("flags read failed");
+			errno = EINVAL;
+			return -1;
+		}
+		*flags = be16_to_cpu(tmp) << 16;
+		/* reserved for future use */
+		if (write_sync(csock, &reserved, sizeof(reserved)) !=
+		    sizeof(reserved)) {
+			LOG("write failed (reserved)");
+			errno = EINVAL;
+			return -1;
+		}
+		/* write the export name */
+		magic = cpu_to_be64(magic);
+		if (write_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
+			LOG("write failed (magic)");
+			errno = EINVAL;
+			return -1;
+		}
+		opt = cpu_to_be32(NBD_OPT_EXPORT_NAME);
+		if (write_sync(csock, &opt, sizeof(opt)) != sizeof(opt)) {
+			LOG("write failed (opt)");
+			errno = EINVAL;
+			return -1;
+		}
+		namesize = cpu_to_be32(strlen(name));
+		if (write_sync(csock, &namesize, sizeof(namesize)) !=
+		    sizeof(namesize)) {
+			LOG("write failed (namesize)");
+			errno = EINVAL;
+			return -1;
+		}
+		if (write_sync(csock, (char*)name, strlen(name)) != strlen(name)) {
+			LOG("write failed (name)");
+			errno = EINVAL;
+			return -1;
+		}
+	} else {
+		TRACE("Checking magic (cli_magic)");
+
+		if (magic != 0x00420281861253LL) {
+			LOG("Bad magic received");
+			errno = EINVAL;
+			return -1;
+		}
+	}
+
+	if (read_sync(csock, &s, sizeof(s)) != sizeof(s)) {
+		LOG("read failed");
+		errno = EINVAL;
+		return -1;
+	}
+	*size = be64_to_cpu(s);
+	*blocksize = 1024;
+	TRACE("Size is %" PRIu64, *size);
 
-	if (magic != 0x00420281861253LL) {
-		LOG("Bad magic received");
+	if (!name) {
+		if (read_sync(csock, flags, sizeof(*flags)) != sizeof(*flags)) {
+			LOG("read failed (flags)");
+			errno = EINVAL;
+			return -1;
+		}
+		*flags = be32_to_cpup(flags);
+	} else {
+		if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {
+			LOG("read failed (tmp)");
+			errno = EINVAL;
+			return -1;
+		}
+		*flags |= be32_to_cpu(tmp);
+	}
+	if (read_sync(csock, &buf, 124) != 124) {
+		LOG("read failed (buf)");
 		errno = EINVAL;
 		return -1;
 	}
diff --git a/nbd.h b/nbd.h
index 5a1fbdf..8ff65a1 100644
--- a/nbd.h
+++ b/nbd.h
@@ -42,6 +42,8 @@ enum {
     NBD_CMD_DISC = 2
 };
 
+#define NBD_DEFAULT_PORT	10809
+
 size_t nbd_wr_sync(int fd, void *buffer, size_t size, bool do_read);
 int tcp_socket_outgoing(const char *address, uint16_t port);
 int tcp_socket_incoming(const char *address, uint16_t port);
@@ -49,7 +51,8 @@ int unix_socket_outgoing(const char *path);
 int unix_socket_incoming(const char *path);
 
 int nbd_negotiate(int csock, off_t size);
-int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize);
+int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags,
+                          off_t *size, size_t *blocksize);
 int nbd_init(int fd, int csock, off_t size, size_t blocksize);
 int nbd_send_request(int csock, struct nbd_request *request);
 int nbd_receive_reply(int csock, struct nbd_reply *reply);
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 55a966f..d7d760f 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -620,6 +620,13 @@ qemu linux1.img -hdb nbd:unix:/tmp/my_socket
 qemu linux2.img -hdb nbd:unix:/tmp/my_socket
 @end example
 
+If the nbd-server uses named exports (since NBD 2.9.18), you must use the
+"exportname" option:
+@example
+qemu -cdrom nbd:localhost:exportname=debian-500-ppc-netinst
+qemu -cdrom nbd:localhost:exportname=openSUSE-11.1-ppc-netinst
+@end example
+
 @node pcsys_network
 @section Network emulation
 
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 4e607cf..41e5c5d 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -230,6 +230,7 @@ int main(int argc, char **argv)
     int nb_fds = 0;
     int max_fd;
     int persistent = 0;
+    uint32_t nbdflags;
 
     while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
         switch (ch) {
@@ -398,7 +399,8 @@ int main(int argc, char **argv)
                 goto out;
             }
 
-            ret = nbd_receive_negotiate(sock, &size, &blocksize);
+            ret = nbd_receive_negotiate(sock, NULL, &nbdflags,
+					&size, &blocksize);
             if (ret == -1) {
                 ret = 1;
                 goto out;
-- 
1.7.0.4

Re: [Qemu-devel] [PATCH][v3] Introduce NBD named exports.

[Anthony Liguori]

On 08/25/2010 03:48 PM, Laurent Vivier wrote:
> This patch allows to connect Qemu using NBD protocol to an nbd-server
> using named exports.
>    

Does using a named export make the NBD protocol incompatible with a 
server that doesn't support named exports?  IOW, it's essentially a new 
protocol layered underneath of NBD?

Regards,

Anthony Liguori

> For instance, if on the host "isoserver", in /etc/nbd-server/config, you have:
>
> [generic]
> [debian-500-ppc-netinst]
>          exportname = /ISO/debian-500-powerpc-netinst.iso
> [Fedora-10-ppc-netinst]
>          exportname = /ISO/Fedora-10-ppc-netinst.iso
>
> You can connect to it, using:
>
>      qemu -cdrom nbd:isoserver:exportname=debian-500-ppc-netinst
>      qemu -cdrom nbd:isoserver:exportname=Fedora-10-ppc-netinst
>
> NOTE: you need at least nbd-server 2.9.18
>
> Signed-off-by: Laurent Vivier<laurent@vivier.eu>
> ---
> v3: change sizeof() by strlen()
> v2: address Kevin comments (CODING STYLE)
>
>   block/nbd.c   |   68 +++++++++++++++++++++++---------
>   nbd.c         |  118 +++++++++++++++++++++++++++++++++++++++++++++++++++------
>   nbd.h         |    5 ++-
>   qemu-doc.texi |    7 +++
>   qemu-nbd.c    |    4 +-
>   5 files changed, 169 insertions(+), 33 deletions(-)
>
> diff --git a/block/nbd.c b/block/nbd.c
> index a1ec123..5e9d6cb 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -33,6 +33,8 @@
>   #include<sys/types.h>
>   #include<unistd.h>
>
> +#define EN_OPTSTR ":exportname="
> +
>   typedef struct BDRVNBDState {
>       int sock;
>       off_t size;
> @@ -42,55 +44,83 @@ typedef struct BDRVNBDState {
>   static int nbd_open(BlockDriverState *bs, const char* filename, int flags)
>   {
>       BDRVNBDState *s = bs->opaque;
> +    uint32_t nbdflags;
> +
> +    char *file;
> +    char *name;
>       const char *host;
>       const char *unixpath;
>       int sock;
>       off_t size;
>       size_t blocksize;
>       int ret;
> +    int err = -EINVAL;
> +
> +    file = qemu_strdup(filename);
>
> -    if (!strstart(filename, "nbd:",&host))
> -        return -EINVAL;
> +    name = strstr(file, EN_OPTSTR);
> +    if (name) {
> +        if (name[strlen(EN_OPTSTR)] == 0) {
> +            goto out;
> +        }
> +        name[0] = 0;
> +        name += strlen(EN_OPTSTR);
> +    }
> +
> +    if (!strstart(file, "nbd:",&host)) {
> +        goto out;
> +    }
>
>       if (strstart(host, "unix:",&unixpath)) {
>
> -        if (unixpath[0] != '/')
> -            return -EINVAL;
> +        if (unixpath[0] != '/') {
> +            goto out;
> +        }
>
>           sock = unix_socket_outgoing(unixpath);
>
>       } else {
> -        uint16_t port;
> +        uint16_t port = NBD_DEFAULT_PORT;
>           char *p, *r;
>           char hostname[128];
>
>           pstrcpy(hostname, 128, host);
>
>           p = strchr(hostname, ':');
> -        if (p == NULL)
> -            return -EINVAL;
> +        if (p != NULL) {
> +            *p = '\0';
> +            p++;
> +
> +            port = strtol(p,&r, 0);
> +            if (r == p) {
> +                goto out;
> +            }
> +        } else if (name == NULL) {
> +            goto out;
> +        }
>
> -        *p = '\0';
> -        p++;
> -
> -        port = strtol(p,&r, 0);
> -        if (r == p)
> -            return -EINVAL;
>           sock = tcp_socket_outgoing(hostname, port);
>       }
>
> -    if (sock == -1)
> -        return -errno;
> +    if (sock == -1) {
> +        err = -errno;
> +        goto out;
> +    }
>
> -    ret = nbd_receive_negotiate(sock,&size,&blocksize);
> -    if (ret == -1)
> -        return -errno;
> +    ret = nbd_receive_negotiate(sock, name,&nbdflags,&size,&blocksize);
> +    if (ret == -1) {
> +        err = -errno;
> +        goto out;
> +    }
>
>       s->sock = sock;
>       s->size = size;
>       s->blocksize = blocksize;
> +    err = 0;
>
> -    return 0;
> +out:
> +    qemu_free(file);
> +    return err;
>   }
>
>   static int nbd_read(BlockDriverState *bs, int64_t sector_num,
> diff --git a/nbd.c b/nbd.c
> index a9f295f..30dec8d 100644
> --- a/nbd.c
> +++ b/nbd.c
> @@ -62,6 +62,8 @@
>   #define NBD_SET_SIZE_BLOCKS	_IO(0xab, 7)
>   #define NBD_DISCONNECT          _IO(0xab, 8)
>
> +#define NBD_OPT_EXPORT_NAME	(1<<  0)
> +
>   /* That's all folks */
>
>   #define read_sync(fd, buffer, size) nbd_wr_sync(fd, buffer, size, true)
> @@ -296,22 +298,27 @@ int nbd_negotiate(int csock, off_t size)
>   	return 0;
>   }
>
> -int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize)
> +int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags,
> +                          off_t *size, size_t *blocksize)
>   {
> -	char buf[8 + 8 + 8 + 128];
> -	uint64_t magic;
> +	char buf[256];
> +	uint64_t magic, s;
> +	uint16_t tmp;
>
>   	TRACE("Receiving negotation.");
>
> -	if (read_sync(csock, buf, sizeof(buf)) != sizeof(buf)) {
> +	if (read_sync(csock, buf, 8) != 8) {
>   		LOG("read failed");
>   		errno = EINVAL;
>   		return -1;
>   	}
>
> -	magic = be64_to_cpup((uint64_t*)(buf + 8));
> -	*size = be64_to_cpup((uint64_t*)(buf + 16));
> -	*blocksize = 1024;
> +	buf[8] = '\0';
> +	if (strlen(buf) == 0) {
> +		LOG("server connection closed");
> +		errno = EINVAL;
> +		return -1;
> +	}
>
>   	TRACE("Magic is %c%c%c%c%c%c%c%c",
>   	      qemu_isprint(buf[0]) ? buf[0] : '.',
> @@ -322,8 +329,6 @@ int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize)
>   	      qemu_isprint(buf[5]) ? buf[5] : '.',
>   	      qemu_isprint(buf[6]) ? buf[6] : '.',
>   	      qemu_isprint(buf[7]) ? buf[7] : '.');
> -	TRACE("Magic is 0x%" PRIx64, magic);
> -	TRACE("Size is %" PRIu64, *size);
>
>   	if (memcmp(buf, "NBDMAGIC", 8) != 0) {
>   		LOG("Invalid magic received");
> @@ -331,10 +336,99 @@ int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize)
>   		return -1;
>   	}
>
> -	TRACE("Checking magic");
> +	if (read_sync(csock,&magic, sizeof(magic)) != sizeof(magic)) {
> +		LOG("read failed");
> +		errno = EINVAL;
> +		return -1;
> +	}
> +	magic = be64_to_cpu(magic);
> +	TRACE("Magic is 0x%" PRIx64, magic);
> +
> +	if (name) {
> +		uint32_t reserved = 0;
> +		uint32_t opt;
> +		uint32_t namesize;
> +
> +		TRACE("Checking magic (opts_magic)");
> +		if (magic != 0x49484156454F5054LL) {
> +			LOG("Bad magic received");
> +			errno = EINVAL;
> +			return -1;
> +		}
> +		if (read_sync(csock,&tmp, sizeof(tmp)) != sizeof(tmp)) {
> +			LOG("flags read failed");
> +			errno = EINVAL;
> +			return -1;
> +		}
> +		*flags = be16_to_cpu(tmp)<<  16;
> +		/* reserved for future use */
> +		if (write_sync(csock,&reserved, sizeof(reserved)) !=
> +		    sizeof(reserved)) {
> +			LOG("write failed (reserved)");
> +			errno = EINVAL;
> +			return -1;
> +		}
> +		/* write the export name */
> +		magic = cpu_to_be64(magic);
> +		if (write_sync(csock,&magic, sizeof(magic)) != sizeof(magic)) {
> +			LOG("write failed (magic)");
> +			errno = EINVAL;
> +			return -1;
> +		}
> +		opt = cpu_to_be32(NBD_OPT_EXPORT_NAME);
> +		if (write_sync(csock,&opt, sizeof(opt)) != sizeof(opt)) {
> +			LOG("write failed (opt)");
> +			errno = EINVAL;
> +			return -1;
> +		}
> +		namesize = cpu_to_be32(strlen(name));
> +		if (write_sync(csock,&namesize, sizeof(namesize)) !=
> +		    sizeof(namesize)) {
> +			LOG("write failed (namesize)");
> +			errno = EINVAL;
> +			return -1;
> +		}
> +		if (write_sync(csock, (char*)name, strlen(name)) != strlen(name)) {
> +			LOG("write failed (name)");
> +			errno = EINVAL;
> +			return -1;
> +		}
> +	} else {
> +		TRACE("Checking magic (cli_magic)");
> +
> +		if (magic != 0x00420281861253LL) {
> +			LOG("Bad magic received");
> +			errno = EINVAL;
> +			return -1;
> +		}
> +	}
> +
> +	if (read_sync(csock,&s, sizeof(s)) != sizeof(s)) {
> +		LOG("read failed");
> +		errno = EINVAL;
> +		return -1;
> +	}
> +	*size = be64_to_cpu(s);
> +	*blocksize = 1024;
> +	TRACE("Size is %" PRIu64, *size);
>
> -	if (magic != 0x00420281861253LL) {
> -		LOG("Bad magic received");
> +	if (!name) {
> +		if (read_sync(csock, flags, sizeof(*flags)) != sizeof(*flags)) {
> +			LOG("read failed (flags)");
> +			errno = EINVAL;
> +			return -1;
> +		}
> +		*flags = be32_to_cpup(flags);
> +	} else {
> +		if (read_sync(csock,&tmp, sizeof(tmp)) != sizeof(tmp)) {
> +			LOG("read failed (tmp)");
> +			errno = EINVAL;
> +			return -1;
> +		}
> +		*flags |= be32_to_cpu(tmp);
> +	}
> +	if (read_sync(csock,&buf, 124) != 124) {
> +		LOG("read failed (buf)");
>   		errno = EINVAL;
>   		return -1;
>   	}
> diff --git a/nbd.h b/nbd.h
> index 5a1fbdf..8ff65a1 100644
> --- a/nbd.h
> +++ b/nbd.h
> @@ -42,6 +42,8 @@ enum {
>       NBD_CMD_DISC = 2
>   };
>
> +#define NBD_DEFAULT_PORT	10809
> +
>   size_t nbd_wr_sync(int fd, void *buffer, size_t size, bool do_read);
>   int tcp_socket_outgoing(const char *address, uint16_t port);
>   int tcp_socket_incoming(const char *address, uint16_t port);
> @@ -49,7 +51,8 @@ int unix_socket_outgoing(const char *path);
>   int unix_socket_incoming(const char *path);
>
>   int nbd_negotiate(int csock, off_t size);
> -int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize);
> +int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags,
> +                          off_t *size, size_t *blocksize);
>   int nbd_init(int fd, int csock, off_t size, size_t blocksize);
>   int nbd_send_request(int csock, struct nbd_request *request);
>   int nbd_receive_reply(int csock, struct nbd_reply *reply);
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index 55a966f..d7d760f 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -620,6 +620,13 @@ qemu linux1.img -hdb nbd:unix:/tmp/my_socket
>   qemu linux2.img -hdb nbd:unix:/tmp/my_socket
>   @end example
>
> +If the nbd-server uses named exports (since NBD 2.9.18), you must use the
> +"exportname" option:
> +@example
> +qemu -cdrom nbd:localhost:exportname=debian-500-ppc-netinst
> +qemu -cdrom nbd:localhost:exportname=openSUSE-11.1-ppc-netinst
> +@end example
> +
>   @node pcsys_network
>   @section Network emulation
>
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 4e607cf..41e5c5d 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -230,6 +230,7 @@ int main(int argc, char **argv)
>       int nb_fds = 0;
>       int max_fd;
>       int persistent = 0;
> +    uint32_t nbdflags;
>
>       while ((ch = getopt_long(argc, argv, sopt, lopt,&opt_ind)) != -1) {
>           switch (ch) {
> @@ -398,7 +399,8 @@ int main(int argc, char **argv)
>                   goto out;
>               }
>
> -            ret = nbd_receive_negotiate(sock,&size,&blocksize);
> +            ret = nbd_receive_negotiate(sock, NULL,&nbdflags,
> +					&size,&blocksize);
>               if (ret == -1) {
>                   ret = 1;
>                   goto out;
>    

Re: [Qemu-devel] [PATCH][v3] Introduce NBD named exports.

[Kevin Wolf]

Am 25.08.2010 22:48, schrieb Laurent Vivier:
> This patch allows to connect Qemu using NBD protocol to an nbd-server
> using named exports.
> 
> For instance, if on the host "isoserver", in /etc/nbd-server/config, you have:
> 
> [generic]
> [debian-500-ppc-netinst]
>         exportname = /ISO/debian-500-powerpc-netinst.iso
> [Fedora-10-ppc-netinst]
>         exportname = /ISO/Fedora-10-ppc-netinst.iso
> 
> You can connect to it, using:
> 
>     qemu -cdrom nbd:isoserver:exportname=debian-500-ppc-netinst
>     qemu -cdrom nbd:isoserver:exportname=Fedora-10-ppc-netinst
> 
> NOTE: you need at least nbd-server 2.9.18
> 
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>

Thanks, applied to the block branch.

Kevin

Re: [Qemu-devel] [PATCH][v3] Introduce NBD named exports.

[Laurent Vivier]

>On 08/25/2010 03:48 PM, Laurent Vivier wrote:
>> This patch allows to connect Qemu using NBD protocol to an nbd-server
>> using named exports.
>>    
>
>Does using a named export make the NBD protocol incompatible with a 
>server that doesn't support named exports?  IOW, it's essentially a new 
>protocol layered underneath of NBD?

Named exports is fully compatible with old nbd-server protocol.

All is done in the "negotiate" part: there is a new magic number which is used when nbd-server uses named exports, this magic number allows the client to send a name to select the exported disk. Then client receives disk size and data as usual.

I have tested this patch with current "qemu-nbd" (witch tcp and unix socket) and it is working fine.

Regards,
Laurent

>Regards,
>
>Anthony Liguori
>
>> For instance, if on the host "isoserver", in /etc/nbd-server/config, you 
>have:
>>
>> [generic]
>> [debian-500-ppc-netinst]
>>          exportname = /ISO/debian-500-powerpc-netinst.iso
>> [Fedora-10-ppc-netinst]
>>          exportname = /ISO/Fedora-10-ppc-netinst.iso
>>
>> You can connect to it, using:
>>
>>      qemu -cdrom nbd:isoserver:exportname=debian-500-ppc-netinst
>>      qemu -cdrom nbd:isoserver:exportname=Fedora-10-ppc-netinst
>>
>> NOTE: you need at least nbd-server 2.9.18
>>
>> Signed-off-by: Laurent Vivier<laurent@vivier.eu>
>> ---
>> v3: change sizeof() by strlen()
>> v2: address Kevin comments (CODING STYLE)
>>
>>   block/nbd.c   |   68 +++++++++++++++++++++++---------
>>   nbd.c         |  118 
>+++++++++++++++++++++++++++++++++++++++++++++++++++------
>>   nbd.h         |    5 ++-
>>   qemu-doc.texi |    7 +++
>>   qemu-nbd.c    |    4 +-
>>   5 files changed, 169 insertions(+), 33 deletions(-)
>>
>> diff --git a/block/nbd.c b/block/nbd.c
>> index a1ec123..5e9d6cb 100644
>> --- a/block/nbd.c
>> +++ b/block/nbd.c
>> @@ -33,6 +33,8 @@
>>   #include<sys/types.h>
>>   #include<unistd.h>
>>
>> +#define EN_OPTSTR ":exportname="
>> +
>>   typedef struct BDRVNBDState {
>>       int sock;
>>       off_t size;
>> @@ -42,55 +44,83 @@ typedef struct BDRVNBDState {
>>   static int nbd_open(BlockDriverState *bs, const char* filename, int 
>flags)
>>   {
>>       BDRVNBDState *s = bs->opaque;
>> +    uint32_t nbdflags;
>> +
>> +    char *file;
>> +    char *name;
>>       const char *host;
>>       const char *unixpath;
>>       int sock;
>>       off_t size;
>>       size_t blocksize;
>>       int ret;
>> +    int err = -EINVAL;
>> +
>> +    file = qemu_strdup(filename);
>>
>> -    if (!strstart(filename, "nbd:",&host))
>> -        return -EINVAL;
>> +    name = strstr(file, EN_OPTSTR);
>> +    if (name) {
>> +        if (name[strlen(EN_OPTSTR)] == 0) {
>> +            goto out;
>> +        }
>> +        name[0] = 0;
>> +        name += strlen(EN_OPTSTR);
>> +    }
>> +
>> +    if (!strstart(file, "nbd:",&host)) {
>> +        goto out;
>> +    }
>>
>>       if (strstart(host, "unix:",&unixpath)) {
>>
>> -        if (unixpath[0] != '/')
>> -            return -EINVAL;
>> +        if (unixpath[0] != '/') {
>> +            goto out;
>> +        }
>>
>>           sock = unix_socket_outgoing(unixpath);
>>
>>       } else {
>> -        uint16_t port;
>> +        uint16_t port = NBD_DEFAULT_PORT;
>>           char *p, *r;
>>           char hostname[128];
>>
>>           pstrcpy(hostname, 128, host);
>>
>>           p = strchr(hostname, ':');
>> -        if (p == NULL)
>> -            return -EINVAL;
>> +        if (p != NULL) {
>> +            *p = '\0';
>> +            p++;
>> +
>> +            port = strtol(p,&r, 0);
>> +            if (r == p) {
>> +                goto out;
>> +            }
>> +        } else if (name == NULL) {
>> +            goto out;
>> +        }
>>
>> -        *p = '\0';
>> -        p++;
>> -
>> -        port = strtol(p,&r, 0);
>> -        if (r == p)
>> -            return -EINVAL;
>>           sock = tcp_socket_outgoing(hostname, port);
>>       }
>>
>> -    if (sock == -1)
>> -        return -errno;
>> +    if (sock == -1) {
>> +        err = -errno;
>> +        goto out;
>> +    }
>>
>> -    ret = nbd_receive_negotiate(sock,&size,&blocksize);
>> -    if (ret == -1)
>> -        return -errno;
>> +    ret = nbd_receive_negotiate(sock, name,&nbdflags,&size,&blocksize);
>> +    if (ret == -1) {
>> +        err = -errno;
>> +        goto out;
>> +    }
>>
>>       s->sock = sock;
>>       s->size = size;
>>       s->blocksize = blocksize;
>> +    err = 0;
>>
>> -    return 0;
>> +out:
>> +    qemu_free(file);
>> +    return err;
>>   }
>>
>>   static int nbd_read(BlockDriverState *bs, int64_t sector_num,
>> diff --git a/nbd.c b/nbd.c
>> index a9f295f..30dec8d 100644
>> --- a/nbd.c
>> +++ b/nbd.c
>> @@ -62,6 +62,8 @@
>>   #define NBD_SET_SIZE_BLOCKS	_IO(0xab, 7)
>>   #define NBD_DISCONNECT          _IO(0xab, 8)
>>
>> +#define NBD_OPT_EXPORT_NAME	(1<<  0)
>> +
>>   /* That's all folks */
>>
>>   #define read_sync(fd, buffer, size) nbd_wr_sync(fd, buffer, size, true)
>> @@ -296,22 +298,27 @@ int nbd_negotiate(int csock, off_t size)
>>   	return 0;
>>   }
>>
>> -int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize)
>> +int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags,
>> +                          off_t *size, size_t *blocksize)
>>   {
>> -	char buf[8 + 8 + 8 + 128];
>> -	uint64_t magic;
>> +	char buf[256];
>> +	uint64_t magic, s;
>> +	uint16_t tmp;
>>
>>   	TRACE("Receiving negotation.");
>>
>> -	if (read_sync(csock, buf, sizeof(buf)) != sizeof(buf)) {
>> +	if (read_sync(csock, buf, 8) != 8) {
>>   		LOG("read failed");
>>   		errno = EINVAL;
>>   		return -1;
>>   	}
>>
>> -	magic = be64_to_cpup((uint64_t*)(buf + 8));
>> -	*size = be64_to_cpup((uint64_t*)(buf + 16));
>> -	*blocksize = 1024;
>> +	buf[8] = '\0';
>> +	if (strlen(buf) == 0) {
>> +		LOG("server connection closed");
>> +		errno = EINVAL;
>> +		return -1;
>> +	}
>>
>>   	TRACE("Magic is %c%c%c%c%c%c%c%c",
>>   	      qemu_isprint(buf[0]) ? buf[0] : '.',
>> @@ -322,8 +329,6 @@ int nbd_receive_negotiate(int csock, off_t *size, size_t 
>*blocksize)
>>   	      qemu_isprint(buf[5]) ? buf[5] : '.',
>>   	      qemu_isprint(buf[6]) ? buf[6] : '.',
>>   	      qemu_isprint(buf[7]) ? buf[7] : '.');
>> -	TRACE("Magic is 0x%" PRIx64, magic);
>> -	TRACE("Size is %" PRIu64, *size);
>>
>>   	if (memcmp(buf, "NBDMAGIC", 8) != 0) {
>>   		LOG("Invalid magic received");
>> @@ -331,10 +336,99 @@ int nbd_receive_negotiate(int csock, off_t *size, 
>size_t *blocksize)
>>   		return -1;
>>   	}
>>
>> -	TRACE("Checking magic");
>> +	if (read_sync(csock,&magic, sizeof(magic)) != sizeof(magic)) {
>> +		LOG("read failed");
>> +		errno = EINVAL;
>> +		return -1;
>> +	}
>> +	magic = be64_to_cpu(magic);
>> +	TRACE("Magic is 0x%" PRIx64, magic);
>> +
>> +	if (name) {
>> +		uint32_t reserved = 0;
>> +		uint32_t opt;
>> +		uint32_t namesize;
>> +
>> +		TRACE("Checking magic (opts_magic)");
>> +		if (magic != 0x49484156454F5054LL) {
>> +			LOG("Bad magic received");
>> +			errno = EINVAL;
>> +			return -1;
>> +		}
>> +		if (read_sync(csock,&tmp, sizeof(tmp)) != sizeof(tmp)) {
>> +			LOG("flags read failed");
>> +			errno = EINVAL;
>> +			return -1;
>> +		}
>> +		*flags = be16_to_cpu(tmp)<<  16;
>> +		/* reserved for future use */
>> +		if (write_sync(csock,&reserved, sizeof(reserved)) !=
>> +		    sizeof(reserved)) {
>> +			LOG("write failed (reserved)");
>> +			errno = EINVAL;
>> +			return -1;
>> +		}
>> +		/* write the export name */
>> +		magic = cpu_to_be64(magic);
>> +		if (write_sync(csock,&magic, sizeof(magic)) != sizeof(magic)) {
>> +			LOG("write failed (magic)");
>> +			errno = EINVAL;
>> +			return -1;
>> +		}
>> +		opt = cpu_to_be32(NBD_OPT_EXPORT_NAME);
>> +		if (write_sync(csock,&opt, sizeof(opt)) != sizeof(opt)) {
>> +			LOG("write failed (opt)");
>> +			errno = EINVAL;
>> +			return -1;
>> +		}
>> +		namesize = cpu_to_be32(strlen(name));
>> +		if (write_sync(csock,&namesize, sizeof(namesize)) !=
>> +		    sizeof(namesize)) {
>> +			LOG("write failed (namesize)");
>> +			errno = EINVAL;
>> +			return -1;
>> +		}
>> +		if (write_sync(csock, (char*)name, strlen(name)) != strlen(name)) {
>> +			LOG("write failed (name)");
>> +			errno = EINVAL;
>> +			return -1;
>> +		}
>> +	} else {
>> +		TRACE("Checking magic (cli_magic)");
>> +
>> +		if (magic != 0x00420281861253LL) {
>> +			LOG("Bad magic received");
>> +			errno = EINVAL;
>> +			return -1;
>> +		}
>> +	}
>> +
>> +	if (read_sync(csock,&s, sizeof(s)) != sizeof(s)) {
>> +		LOG("read failed");
>> +		errno = EINVAL;
>> +		return -1;
>> +	}
>> +	*size = be64_to_cpu(s);
>> +	*blocksize = 1024;
>> +	TRACE("Size is %" PRIu64, *size);
>>
>> -	if (magic != 0x00420281861253LL) {
>> -		LOG("Bad magic received");
>> +	if (!name) {
>> +		if (read_sync(csock, flags, sizeof(*flags)) != sizeof(*flags)) {
>> +			LOG("read failed (flags)");
>> +			errno = EINVAL;
>> +			return -1;
>> +		}
>> +		*flags = be32_to_cpup(flags);
>> +	} else {
>> +		if (read_sync(csock,&tmp, sizeof(tmp)) != sizeof(tmp)) {
>> +			LOG("read failed (tmp)");
>> +			errno = EINVAL;
>> +			return -1;
>> +		}
>> +		*flags |= be32_to_cpu(tmp);
>> +	}
>> +	if (read_sync(csock,&buf, 124) != 124) {
>> +		LOG("read failed (buf)");
>>   		errno = EINVAL;
>>   		return -1;
>>   	}
>> diff --git a/nbd.h b/nbd.h
>> index 5a1fbdf..8ff65a1 100644
>> --- a/nbd.h
>> +++ b/nbd.h
>> @@ -42,6 +42,8 @@ enum {
>>       NBD_CMD_DISC = 2
>>   };
>>
>> +#define NBD_DEFAULT_PORT	10809
>> +
>>   size_t nbd_wr_sync(int fd, void *buffer, size_t size, bool do_read);
>>   int tcp_socket_outgoing(const char *address, uint16_t port);
>>   int tcp_socket_incoming(const char *address, uint16_t port);
>> @@ -49,7 +51,8 @@ int unix_socket_outgoing(const char *path);
>>   int unix_socket_incoming(const char *path);
>>
>>   int nbd_negotiate(int csock, off_t size);
>> -int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize);
>> +int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags,
>> +                          off_t *size, size_t *blocksize);
>>   int nbd_init(int fd, int csock, off_t size, size_t blocksize);
>>   int nbd_send_request(int csock, struct nbd_request *request);
>>   int nbd_receive_reply(int csock, struct nbd_reply *reply);
>> diff --git a/qemu-doc.texi b/qemu-doc.texi
>> index 55a966f..d7d760f 100644
>> --- a/qemu-doc.texi
>> +++ b/qemu-doc.texi
>> @@ -620,6 +620,13 @@ qemu linux1.img -hdb nbd:unix:/tmp/my_socket
>>   qemu linux2.img -hdb nbd:unix:/tmp/my_socket
>>   @end example
>>
>> +If the nbd-server uses named exports (since NBD 2.9.18), you must use the
>> +"exportname" option:
>> +@example
>> +qemu -cdrom nbd:localhost:exportname=debian-500-ppc-netinst
>> +qemu -cdrom nbd:localhost:exportname=openSUSE-11.1-ppc-netinst
>> +@end example
>> +
>>   @node pcsys_network
>>   @section Network emulation
>>
>> diff --git a/qemu-nbd.c b/qemu-nbd.c
>> index 4e607cf..41e5c5d 100644
>> --- a/qemu-nbd.c
>> +++ b/qemu-nbd.c
>> @@ -230,6 +230,7 @@ int main(int argc, char **argv)
>>       int nb_fds = 0;
>>       int max_fd;
>>       int persistent = 0;
>> +    uint32_t nbdflags;
>>
>>       while ((ch = getopt_long(argc, argv, sopt, lopt,&opt_ind)) != -1) {
>>           switch (ch) {
>> @@ -398,7 +399,8 @@ int main(int argc, char **argv)
>>                   goto out;
>>               }
>>
>> -            ret = nbd_receive_negotiate(sock,&size,&blocksize);
>> +            ret = nbd_receive_negotiate(sock, NULL,&nbdflags,
>> +					&size,&blocksize);
>>               if (ret == -1) {
>>                   ret = 1;
>>                   goto out;
>>    
>
>



-- 
--------------------- Laurent@vivier.eu  ---------------------
"Tout ce qui est impossible reste à accomplir"    Jules Verne
"Things are only impossible until they're not" Jean-Luc Picard