[Qemu-devel] [PATCH] Merge NBD client/server int qemu-nbd

[Qemu-devel] [PATCH] Merge NBD client/server int qemu-nbd

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 931 bytes --]

Hi,

this patch allows to connect directly a disk image file to an NBD
device. It introduces the use of a unix socket (instead of inet).

- To connect a file to a device:

# qemu-nbd --connect=/dev/nbd0 my_disk.qcow2

Then you can see directly your disk (no need of nbd-client):

# fdisk -l /dev/nbd0

Disk /dev/nbd0: 4294 MB, 4294967296 bytes
255 heads, 63 sectors/track, 522 cylinders
Units = cylinders of 16065 * 512 = 8225280 bytes

     Device Boot      Start         End      Blocks   Id  System
/dev/nbd0p1   *           1         492     3951958+  83  Linux
/dev/nbd0p2             493         522      240975    5  Extended
/dev/nbd0p5             493         522      240943+  82  Linux swap /
Solaris

- To disconnect the file from the device:

# qemu-nbd --disconnect /dev/nbd0

Regards,
Laurent
-- 
------------- Laurent.Vivier@bull.net ---------------
"The best way to predict the future is to invent it."
- Alan Kay

[-- Attachment #2: qemu-nbd-local.patch --]
[-- Type: text/x-patch, Size: 7873 bytes --]

Index: qemu/nbd.c
===================================================================
--- qemu.orig/nbd.c	2008-06-13 13:06:24.000000000 +0200
+++ qemu/nbd.c	2008-06-13 17:02:55.000000000 +0200
@@ -25,6 +25,7 @@
 #include <ctype.h>
 #include <inttypes.h>
 #include <sys/socket.h>
+#include <sys/un.h>
 #include <netinet/in.h>
 #include <netinet/tcp.h>
 #include <arpa/inet.h>
@@ -183,6 +184,65 @@ error:
     return -1;
 }
 
+int unix_socket_incoming(const char *path)
+{
+    int s;
+    struct sockaddr_un addr;
+    int serrno;
+
+    s = socket(PF_UNIX, SOCK_STREAM, 0);
+    if (s == -1) {
+        return -1;
+    }
+
+    memset(&addr, 0, sizeof(addr));
+    addr.sun_family = AF_UNIX;
+    pstrcpy(addr.sun_path, sizeof(addr.sun_path), path);
+
+    if (bind(s, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
+        goto error;
+    }
+
+    if (listen(s, 128) == -1) {
+        goto error;
+    }
+
+    return s;
+error:
+    serrno = errno;
+    close(s);
+    errno = serrno;
+    return -1;
+}
+
+int unix_socket_outgoing(const char *path)
+{
+    int s;
+    struct sockaddr_un addr;
+    int serrno;
+
+    s = socket(PF_UNIX, SOCK_STREAM, 0);
+    if (s == -1) {
+        return -1;
+    }
+
+    memset(&addr, 0, sizeof(addr));
+    addr.sun_family = AF_UNIX;
+    pstrcpy(addr.sun_path, sizeof(addr.sun_path), path);
+
+    if (connect(s, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
+        goto error;
+    }
+
+    return s;
+error:
+    serrno = errno;
+    close(s);
+    errno = serrno;
+    return -1;
+}
+
+
 /* Basic flow
 
    Server         Client
@@ -388,7 +448,7 @@ int nbd_trip(BlockDriverState *bs, int c
 	}
 
 	if (len > sizeof(data)) {
-		LOG("len (%u) is larger than max len (%u)",
+		LOG("len (%u) is larger than max len (%lu)",
 		    len, sizeof(data));
 		errno = EINVAL;
 		return -1;
Index: qemu/nbd.h
===================================================================
--- qemu.orig/nbd.h	2008-06-13 13:06:26.000000000 +0200
+++ qemu/nbd.h	2008-06-13 15:17:57.000000000 +0200
@@ -27,6 +27,8 @@
 #include "block_int.h"
 
 int tcp_socket_incoming(const char *address, uint16_t port);
+int unix_socket_outgoing(const char *path);
+int unix_socket_incoming(const char *path);
 
 int nbd_negotiate(BlockDriverState *bs, int csock, off_t size);
 int nbd_receive_negotiate(int fd, int csock);
Index: qemu/qemu-nbd.c
===================================================================
--- qemu.orig/qemu-nbd.c	2008-06-13 13:06:19.000000000 +0200
+++ qemu/qemu-nbd.c	2008-06-13 17:13:54.000000000 +0200
@@ -21,15 +21,18 @@
 #include "block_int.h"
 #include "nbd.h"
 
-#include <malloc.h>
 #include <stdarg.h>
 #include <stdio.h>
 #include <getopt.h>
 #include <err.h>
+#include <sys/types.h>
 #include <sys/socket.h>
 #include <netinet/in.h>
 #include <netinet/tcp.h>
 #include <arpa/inet.h>
+#include <signal.h>
+
+//#define DEBUG
 
 int verbose;
 
@@ -44,6 +47,8 @@ static void usage(const char *name)
 "  -b, --bind=IFACE     interface to bind to (default `0.0.0.0')\n"
 "  -r, --read-only      export read-only\n"
 "  -P, --partition=NUM  only expose partition NUM\n"
+"  -c, --connect=DEV    connect FILE to the local NBD device DEV\n"
+"  -d, --disconnect     disconnect the specified device\n"
 "  -v, --verbose        display extra debugging information\n"
 "  -h, --help           display this help and exit\n"
 "  -V, --version        output version information and exit\n"
@@ -145,19 +150,42 @@ static int find_partition(BlockDriverSta
     return -1;
 }
 
+static void show_parts(const char *device)
+{
+    if (fork() == 0) {
+        int nbd;
+
+	/* wait device */
+        sleep(1);
+        /* linux just needs an open() to trigger
+         * the partition table update
+         * but remember to load the module with max_part != 0 :
+         *     modprobe nbd max_part=63
+         */
+        nbd = open(device, O_RDWR);
+        if (nbd == -1)
+            return;
+        close(nbd);
+        exit(0);
+    }
+}
+
 int main(int argc, char **argv)
 {
     BlockDriverState *bs;
     off_t dev_offset = 0;
     off_t offset = 0;
     bool readonly = false;
+    bool disconnect = false;
     const char *bindto = "0.0.0.0";
     int port = 1024;
     int sock, csock;
     struct sockaddr_in addr;
     socklen_t addr_len = sizeof(addr);
     off_t fd_size;
-    const char *sopt = "hVbo:p:rsP:v";
+    char *device;
+    char sockpath[128];
+    const char *sopt = "hVbo:p:rsP:c:dv?";
     struct option lopt[] = {
         { "help", 0, 0, 'h' },
         { "version", 0, 0, 'V' },
@@ -166,8 +194,11 @@ int main(int argc, char **argv)
         { "offset", 1, 0, 'o' },
         { "read-only", 0, 0, 'r' },
         { "partition", 1, 0, 'P' },
+        { "connect", 1, 0, 'c' },
+        { "disconnect", 0, 0, 'd' },
         { "snapshot", 0, 0, 's' },
         { "verbose", 0, 0, 'v' },
+	{ 0, 0, 0, 0 }
     };
     int ch;
     int opt_ind = 0;
@@ -175,6 +206,8 @@ int main(int argc, char **argv)
     char *end;
     bool snapshot = false;
     int partition = -1;
+    int fd;
+    int ret;
 
     while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
         switch (ch) {
@@ -213,6 +246,12 @@ int main(int argc, char **argv)
             if (partition < 1 || partition > 8)
                 errx(EINVAL, "Invalid partition %d", partition);
             break;
+        case 'd':
+            disconnect = true;
+            break;
+        case 'c':
+            device = optarg;
+            break;
         case 'v':
             verbose = 1;
             break;
@@ -236,6 +275,20 @@ int main(int argc, char **argv)
              argv[0]);
     }
 
+    if (disconnect) {
+        fd = open(argv[optind], O_RDWR);
+        if (fd == -1)
+            errx(errno, "Cannot open %s", argv[optind]);
+
+        nbd_disconnect(fd);
+
+        close(fd);
+
+        printf("%s disconnected\n", argv[optind]);
+
+	return 0;
+    }
+
     bdrv_init();
 
     bs = bdrv_new("hda");
@@ -251,7 +304,56 @@ int main(int argc, char **argv)
         find_partition(bs, partition, &dev_offset, &fd_size))
         errx(errno, "Could not find partition %d", partition);
 
-    sock = tcp_socket_incoming(bindto, port);
+    if (device) {
+	pid_t pid;
+	if (!verbose)
+            daemon(0, 0);	/* detach client and server */
+
+        sprintf(sockpath, "/var/lock/qemu-img-%s", basename(device));
+
+        pid = fork();
+        if (pid < 0)
+            return 1;
+	if (pid != 0) {
+            ret = 0;
+            bdrv_close(bs);
+
+            do {
+                sock = unix_socket_outgoing(sockpath);
+                if (sock == -1) {
+                    if (errno != ENOENT && errno != ECONNREFUSED)
+                        goto exit;
+                    sleep(1);	/* wait children */
+                }
+            } while (sock == -1);
+
+            fd = open(device, O_RDWR);
+            if (fd == -1) {
+                ret = -1;
+                goto exit;
+            }
+
+            ret = nbd_receive_negotiate(fd, sock);
+            if (ret == -1)
+                goto exit;
+
+            printf("NBD device %s is now connected to file %s\n",
+                    device, argv[optind]);
+
+            nbd_client(fd, sock);
+            close(fd);
+exit:
+            kill(pid, SIGTERM);
+            unlink(sockpath);
+
+            return ret;
+        }
+        /* children */
+        sock = unix_socket_incoming(sockpath);
+    } else {
+        sock = tcp_socket_incoming(bindto, port);
+    }
+
     if (sock == -1)
         return 1;
 
@@ -265,6 +367,11 @@ int main(int argc, char **argv)
     if (nbd_negotiate(bs, csock, fd_size) == -1)
         return 1;
 
+    /* if supported, update partition table */
+
+    if (device)
+        show_parts(device);
+
     while (nbd_trip(bs, csock, fd_size, dev_offset, &offset, readonly) == 0);
 
     close(csock);

Re: [Qemu-devel] [PATCH] Merge NBD client/server int qemu-nbd

[Anthony Liguori]

Laurent Vivier wrote:
> Hi,
>
> this patch allows to connect directly a disk image file to an NBD
> device. It introduces the use of a unix socket (instead of inet).
>
> - To connect a file to a device:
>
> # qemu-nbd --connect=/dev/nbd0 my_disk.qcow2
>
> Then you can see directly your disk (no need of nbd-client):
>
> # fdisk -l /dev/nbd0
>
> Disk /dev/nbd0: 4294 MB, 4294967296 bytes
> 255 heads, 63 sectors/track, 522 cylinders
> Units = cylinders of 16065 * 512 = 8225280 bytes
>
>      Device Boot      Start         End      Blocks   Id  System
> /dev/nbd0p1   *           1         492     3951958+  83  Linux
> /dev/nbd0p2             493         522      240975    5  Extended
> /dev/nbd0p5             493         522      240943+  82  Linux swap /
> Solaris
>
> - To disconnect the file from the device:
>
> # qemu-nbd --disconnect /dev/nbd0
>
> Regards,
> Laurent
>   
> +static void show_parts(const char *device)
> +{
> +    if (fork() == 0) {
> +        int nbd;
> +
> +	/* wait device */
> +        sleep(1);
>   

This looks like a big red-herring.  What is this sleep waiting for any 
can it be possibly made into something less racy?

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] Merge NBD client/server int qemu-nbd

[Avi Kivity]

Anthony Liguori wrote:
>>
>> # qemu-nbd --disconnect /dev/nbd0
>>
>> Regards,
>> Laurent
>>   +static void show_parts(const char *device)
>> +{
>> +    if (fork() == 0) {
>> +        int nbd;
>> +
>> +    /* wait device */
>> +        sleep(1);
>>   
>
> This looks like a big red-herring.  What is this sleep waiting for any 
> can it be possibly made into something less racy?

It's called udevsettle, if the host is running udev.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH] Merge NBD client/server int qemu-nbd

[Laurent Vivier]

Le vendredi 13 juin 2008 à 12:49 -0500, Anthony Liguori a écrit :
> Laurent Vivier wrote:
[...]
> > +static void show_parts(const char *device)
> > +{
> > +    if (fork() == 0) {
> > +        int nbd;
> > +
> > +	/* wait device */
> > +        sleep(1);
> >   
> 
> This looks like a big red-herring.  What is this sleep waiting for any 
> can it be possibly made into something less racy?

Yes, I know, it's BAD (and it can failed sometime...)

But show_parts() must wait its parent has entered in nbd_trip() loop.

Because the open() calls /dev/nbd0 which calls through the socket its
parent and IMHO there is no easy way to know if the server is ready to
process the request.

Perhaps something like:

int timeout = 5;
while ((fd = open(device, O_RDWR)) == -1 && timeout--)
    sleep(1);

Any suggestion ???

Regards,
Laurent
-- 
------------- Laurent.Vivier@bull.net ---------------
"The best way to predict the future is to invent it."
- Alan Kay

Re: [Qemu-devel] [PATCH] Merge NBD client/server int qemu-nbd

[Anthony Liguori]

Laurent Vivier wrote:
> Le vendredi 13 juin 2008 à 12:49 -0500, Anthony Liguori a écrit :
>   
>> Laurent Vivier wrote:
>>     
> [...]
>   
>>> +static void show_parts(const char *device)
>>> +{
>>> +    if (fork() == 0) {
>>> +        int nbd;
>>> +
>>> +	/* wait device */
>>> +        sleep(1);
>>>   
>>>       
>> This looks like a big red-herring.  What is this sleep waiting for any 
>> can it be possibly made into something less racy?
>>     
>
> Yes, I know, it's BAD (and it can failed sometime...)
>
> But show_parts() must wait its parent has entered in nbd_trip() loop.
>   

You mean, listen() has to be called on the fd in the server?  You have 
two choices I think.  You could wait to fork the child until after 
you've listen()'d which is probably the most elegant solution.  If 
that's difficult to do, you could have the child inherit one end of a 
pipe() and use that in the child to wait for the server to be ready.

Regards,

Anthony Liguori

> Because the open() calls /dev/nbd0 which calls through the socket its
> parent and IMHO there is no easy way to know if the server is ready to
> process the request.
>
> Perhaps something like:
>
> int timeout = 5;
> while ((fd = open(device, O_RDWR)) == -1 && timeout--)
>     sleep(1);
>
> Any suggestion ???
>
> Regards,
> Laurent
>   

Re: [Qemu-devel] [PATCH] Merge NBD client/server int qemu-nbd

[Avi Kivity]

Laurent Vivier wrote:
> int timeout = 5;
> while ((fd = open(device, O_RDWR)) == -1 && timeout--)
>     sleep(1);
>
> Any suggestion ???
>
>   

/sbin/udevsettle

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH] Merge NBD client/server int qemu-nbd

[Laurent Vivier]

Le samedi 14 juin 2008 à 10:12 -0500, Anthony Liguori a écrit :
> Laurent Vivier wrote:
> > Le vendredi 13 juin 2008 à 12:49 -0500, Anthony Liguori a écrit :
> >   
> >> Laurent Vivier wrote:
> >>     
> > [...]
> >   
> >>> +static void show_parts(const char *device)
> >>> +{
> >>> +    if (fork() == 0) {
> >>> +        int nbd;
> >>> +
> >>> +	/* wait device */
> >>> +        sleep(1);
> >>>   
> >>>       
> >> This looks like a big red-herring.  What is this sleep waiting for any 
> >> can it be possibly made into something less racy?
> >>     
> >
> > Yes, I know, it's BAD (and it can failed sometime...)
> >
> > But show_parts() must wait its parent has entered in nbd_trip() loop.
> >   
> 
> You mean, listen() has to be called on the fd in the server?  You have 
> two choices I think.  You could wait to fork the child until after 
> you've listen()'d which is probably the most elegant solution.  If 

It is already after the listen (even after the accept...).
It is why I call show_parts() from the server and not from client.

> that's difficult to do, you could have the child inherit one end of a 
> pipe() and use that in the child to wait for the server to be ready.

perhaps I'm wrong, but it seems the server must be waiting on the read()
to allow to have a success on the open(), so I don't see how the server
can launch show_parts() whereas it is already waiting on the read().

Regards,
Laurent
-- 
------------- Laurent.Vivier@bull.net ---------------
"The best way to predict the future is to invent it."
- Alan Kay

Re: [Qemu-devel] [PATCH] Merge NBD client/server int qemu-nbd

[Laurent Vivier]

Le samedi 14 juin 2008 à 20:18 +0300, Avi Kivity a écrit :
> Laurent Vivier wrote:
> > int timeout = 5;
> > while ((fd = open(device, O_RDWR)) == -1 && timeout--)
> >     sleep(1);
> >
> > Any suggestion ???
> >
> >   
> 
> /sbin/udevsettle

Why do you think udev is involved at this moment ?

/dev/nbd0 is created when the module is loaded (thus before) and the
partitions when the partition table is read (from within the open).

Regards,
Laurent
-- 
------------- Laurent.Vivier@bull.net ---------------
"The best way to predict the future is to invent it."
- Alan Kay

Re: [Qemu-devel] [PATCH] Merge NBD client/server int qemu-nbd

[Avi Kivity]

Laurent Vivier wrote:
> Le samedi 14 juin 2008 à 20:18 +0300, Avi Kivity a écrit :
>   
>> Laurent Vivier wrote:
>>     
>>> int timeout = 5;
>>> while ((fd = open(device, O_RDWR)) == -1 && timeout--)
>>>     sleep(1);
>>>
>>> Any suggestion ???
>>>
>>>   
>>>       
>> /sbin/udevsettle
>>     
>
> Why do you think udev is involved at this moment ?
>
> /dev/nbd0 is created when the module is loaded (thus before) and the
> partitions when the partition table is read (from within the open).
>   

It is udev that creates the devices, based on events it receives from 
the kernel.  Calling udevsettle after the kernel instructs udev to 
create the device files will wait until they are actually created.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH] Merge NBD client/server int qemu-nbd

[Laurent Vivier]

Le samedi 14 juin 2008 à 12:06 -0700, Avi Kivity a écrit :
> Laurent Vivier wrote:
> > Le samedi 14 juin 2008 à 20:18 +0300, Avi Kivity a écrit :
> >   
> >> Laurent Vivier wrote:
> >>     
> >>> int timeout = 5;
> >>> while ((fd = open(device, O_RDWR)) == -1 && timeout--)
> >>>     sleep(1);
> >>>
> >>> Any suggestion ???
> >>>
> >>>   
> >>>       
> >> /sbin/udevsettle
> >>     
> >
> > Why do you think udev is involved at this moment ?
> >
> > /dev/nbd0 is created when the module is loaded (thus before) and the
> > partitions when the partition table is read (from within the open).
> >   
> 
> It is udev that creates the devices, based on events it receives from 
> the kernel.  Calling udevsettle after the kernel instructs udev to 
> create the device files will wait until they are actually created.

Yes, I agree but the kernel events are generated by the open(), so I
think I can't use this to know if I can use open(). 

I've used udevmonitor to see what happen:
- without open() -> no events
- whith open() whithout sleep() -> no events
-> open() + sleep() -> events generated by the partition creation.

So, what do I miss ?

Laurent
-- 
------------- Laurent.Vivier@bull.net ---------------
"The best way to predict the future is to invent it."
- Alan Kay

Re: [Qemu-devel] [PATCH] Merge NBD client/server int qemu-nbd

[Avi Kivity]

Laurent Vivier wrote:

  

>> It is udev that creates the devices, based on events it receives from 
>> the kernel.  Calling udevsettle after the kernel instructs udev to 
>> create the device files will wait until they are actually created.
>>     
>
> Yes, I agree but the kernel events are generated by the open(), so I
> think I can't use this to know if I can use open(). 
>
> I've used udevmonitor to see what happen:
> - without open() -> no events
> - whith open() whithout sleep() -> no events
> -> open() + sleep() -> events generated by the partition creation.
>
> So, what do I miss ?
>   

For this, it seems like sleep() generates the events, which is a little 
unlikely.  I suggest re-checking this.

There is also some ioctl which can be used to force re-reading the 
partition table, perhaps invoking that and then udevsettle will suffice.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH] Merge NBD client/server int qemu-nbd

[Laurent Vivier]

Le samedi 14 juin 2008 à 12:35 -0700, Avi Kivity a écrit :
> Laurent Vivier wrote:
> 
>   
> 
> >> It is udev that creates the devices, based on events it receives from 
> >> the kernel.  Calling udevsettle after the kernel instructs udev to 
> >> create the device files will wait until they are actually created.
> >>     
> >
> > Yes, I agree but the kernel events are generated by the open(), so I
> > think I can't use this to know if I can use open(). 
> >
> > I've used udevmonitor to see what happen:
> > - without open() -> no events
> > - whith open() whithout sleep() -> no events
> > -> open() + sleep() -> events generated by the partition creation.
> >
> > So, what do I miss ?
> >   
> 
> For this, it seems like sleep() generates the events, which is a little 
> unlikely.  I suggest re-checking this.

re-checked, no events with sleep() only.

events generated with sleep() + open() are:

UEVENT[1213473088.354639] change@/block/nbd0
ACTION=change
DEVPATH=/block/nbd0
SUBSYSTEM=block
MAJOR=43
MINOR=0
DEVTYPE=disk
SEQNUM=2921

UEVENT[1213473088.354721] add@/block/nbd0/nbd0p1
ACTION=add
DEVPATH=/block/nbd0/nbd0p1
SUBSYSTEM=block
MAJOR=43
MINOR=1
DEVTYPE=partition
SEQNUM=2922

UEVENT[1213473088.354781] add@/block/nbd0/nbd0p2
ACTION=add
DEVPATH=/block/nbd0/nbd0p2
SUBSYSTEM=block
MAJOR=43
MINOR=2
DEVTYPE=partition
SEQNUM=2923

UEVENT[1213473088.354842] add@/block/nbd0/nbd0p5
ACTION=add
DEVPATH=/block/nbd0/nbd0p5
SUBSYSTEM=block
MAJOR=43
MINOR=5
DEVTYPE=partition
SEQNUM=2924

UDEV  [1213473088.355617] change@/block/nbd0
UDEV_LOG=3
ACTION=change
DEVPATH=/block/nbd0
SUBSYSTEM=block
MAJOR=43
MINOR=0
DEVTYPE=disk
SEQNUM=2921
UDEVD_EVENT=1
DEVNAME=/dev/nbd0

UDEV  [1213473088.356818] add@/block/nbd0/nbd0p1
UDEV_LOG=3
ACTION=add
DEVPATH=/block/nbd0/nbd0p1
SUBSYSTEM=block
MAJOR=43
MINOR=1
DEVTYPE=partition
SEQNUM=2922
UDEVD_EVENT=1
DEVNAME=/dev/nbd0p1

UDEV  [1213473088.358335] add@/block/nbd0/nbd0p5
UDEV_LOG=3
ACTION=add
DEVPATH=/block/nbd0/nbd0p5
SUBSYSTEM=block
MAJOR=43
MINOR=5
DEVTYPE=partition
SEQNUM=2924
UDEVD_EVENT=1
DEVNAME=/dev/nbd0p5

UDEV  [1213473088.358390] add@/block/nbd0/nbd0p2
UDEV_LOG=3
ACTION=add
DEVPATH=/block/nbd0/nbd0p2
SUBSYSTEM=block
MAJOR=43
MINOR=2
DEVTYPE=partition
SEQNUM=2923
UDEVD_EVENT=1
DEVNAME=/dev/nbd0p2

> There is also some ioctl which can be used to force re-reading the 
> partition table, perhaps invoking that and then udevsettle will suffice.

Yes, I know that, but to make the ioctl() you need an open()... and the
open() is enough.

Regards,
Laurent
-- 
------------- Laurent.Vivier@bull.net ---------------
"The best way to predict the future is to invent it."
- Alan Kay

Re: [Qemu-devel] [PATCH] Merge NBD client/server int qemu-nbd

[Anthony Liguori]

Laurent Vivier wrote:
> Le samedi 14 juin 2008 à 12:35 -0700, Avi Kivity a écrit :
>   
>> Laurent Vivier wrote:
>>
>>   
>>
>>     
>>>> It is udev that creates the devices, based on events it receives from 
>>>> the kernel.  Calling udevsettle after the kernel instructs udev to 
>>>> create the device files will wait until they are actually created.
>>>>     
>>>>         
>>> Yes, I agree but the kernel events are generated by the open(), so I
>>> think I can't use this to know if I can use open(). 
>>>
>>> I've used udevmonitor to see what happen:
>>> - without open() -> no events
>>> - whith open() whithout sleep() -> no events
>>> -> open() + sleep() -> events generated by the partition creation.
>>>
>>> So, what do I miss ?
>>>   
>>>       
>> For this, it seems like sleep() generates the events, which is a little 
>> unlikely.  I suggest re-checking this.
>>     
>
> re-checked, no events with sleep() only.
>
> events generated with sleep() + open() are:
>   

I imagine the sleep() is causing another task to be scheduled (probably 
the server) which is what ends up generating the event.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] Merge NBD client/server int qemu-nbd

[Laurent Vivier]

Le dimanche 15 juin 2008 à 09:25 -0500, Anthony Liguori a écrit :
> Laurent Vivier wrote:
> > Le samedi 14 juin 2008 à 12:35 -0700, Avi Kivity a écrit :
> >   
> >> Laurent Vivier wrote:
> >>
> >>   
> >>
> >>     
> >>>> It is udev that creates the devices, based on events it receives from 
> >>>> the kernel.  Calling udevsettle after the kernel instructs udev to 
> >>>> create the device files will wait until they are actually created.
> >>>>     
> >>>>         
> >>> Yes, I agree but the kernel events are generated by the open(), so I
> >>> think I can't use this to know if I can use open(). 
> >>>
> >>> I've used udevmonitor to see what happen:
> >>> - without open() -> no events
> >>> - whith open() whithout sleep() -> no events
> >>> -> open() + sleep() -> events generated by the partition creation.
> >>>
> >>> So, what do I miss ?
> >>>   
> >>>       
> >> For this, it seems like sleep() generates the events, which is a little 
> >> unlikely.  I suggest re-checking this.
> >>     
> >
> > re-checked, no events with sleep() only.
> >
> > events generated with sleep() + open() are:
> >   
> 
> I imagine the sleep() is causing another task to be scheduled (probably 
> the server) which is what ends up generating the event.

No, for me, the open() generates the events:

open() -> blkdev_open() -> do_open() -> rescan_partitions() ->
add_partitions() -> device_add() -> kobject_uevent()

If you remove show_parts() from the main(), the partitions appear only
on the first open on the device, for instance you can run manually
"cmp /dev/nbd0 /dev/zero" after "qemu-nbd --connect=/dev/nbd0 ...".

Laurent
-- 
------------- Laurent.Vivier@bull.net ---------------
"The best way to predict the future is to invent it."
- Alan Kay

Re: [Qemu-devel] [PATCH] Merge NBD client/server int qemu-nbd

[Avi Kivity]

Laurent Vivier wrote:
> No, for me, the open() generates the events:
>
> open() -> blkdev_open() -> do_open() -> rescan_partitions() ->
> add_partitions() -> device_add() -> kobject_uevent()
>
> If you remove show_parts() from the main(), the partitions appear only
> on the first open on the device, for instance you can run manually
> "cmp /dev/nbd0 /dev/zero" after "qemu-nbd --connect=/dev/nbd0 ...".
>
>   

Well then, simply call udevsettle after the open of the block device, 
after which you're guaranteed to have the paritions.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH] Merge NBD client/server int qemu-nbd

[Laurent Vivier]

Le dimanche 15 juin 2008 à 09:00 -0700, Avi Kivity a écrit :
> Laurent Vivier wrote:
> > No, for me, the open() generates the events:
> >
> > open() -> blkdev_open() -> do_open() -> rescan_partitions() ->
> > add_partitions() -> device_add() -> kobject_uevent()
> >
> > If you remove show_parts() from the main(), the partitions appear only
> > on the first open on the device, for instance you can run manually
> > "cmp /dev/nbd0 /dev/zero" after "qemu-nbd --connect=/dev/nbd0 ...".
> >
> >   
> 
> Well then, simply call udevsettle after the open of the block device, 
> after which you're guaranteed to have the paritions.

Not possible: if I call the open too early the partitions are not
updated and thus udevsettle is useless (remember: this open() is here
only to update the partition table).

Laurent
-- 
------------- Laurent.Vivier@bull.net ---------------
"The best way to predict the future is to invent it."
- Alan Kay

[Qemu-devel] [PATCH][v2] Merge NBD client/server int qemu-nbd

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 1104 bytes --]

Hi,

this patch allows to connect directly a disk image file to an NBD
device. It introduces the use of a unix socket (instead of inet).

- To connect a file to a device:

# qemu-nbd --connect=/dev/nbd0 my_disk.qcow2

Then you can see directly your disk (no need of nbd-client):

# fdisk -l /dev/nbd0

Disk /dev/nbd0: 4294 MB, 4294967296 bytes
255 heads, 63 sectors/track, 522 cylinders
Units = cylinders of 16065 * 512 = 8225280 bytes

     Device Boot      Start         End      Blocks   Id  System
/dev/nbd0p1   *           1         492     3951958+  83  Linux
/dev/nbd0p2             493         522      240975    5  Extended
/dev/nbd0p5             493         522      240943+  82  Linux swap /
Solaris

- To disconnect the file from the device:

# qemu-nbd --disconnect /dev/nbd0

Changelog:
- v2: call show_parts() from client and avoid the sleep(1). Thank you to
Avi and Anthony. Include my cleanup patch and comments from Carlo
Marcelo Arenas Belon.

Regards,
Laurent
-- 
------------- Laurent.Vivier@bull.net ---------------
"The best way to predict the future is to invent it."
- Alan Kay

[-- Attachment #2: qemu-nbd-local.patch --]
[-- Type: text/x-patch, Size: 7624 bytes --]

Index: qemu/nbd.c
===================================================================
--- qemu.orig/nbd.c	2008-06-18 17:05:46.000000000 +0200
+++ qemu/nbd.c	2008-06-19 10:47:46.000000000 +0200
@@ -25,6 +25,7 @@
 #include <ctype.h>
 #include <inttypes.h>
 #include <sys/socket.h>
+#include <sys/un.h>
 #include <netinet/in.h>
 #include <netinet/tcp.h>
 #include <arpa/inet.h>
@@ -183,6 +184,65 @@ error:
     return -1;
 }
 
+int unix_socket_incoming(const char *path)
+{
+    int s;
+    struct sockaddr_un addr;
+    int serrno;
+
+    s = socket(PF_UNIX, SOCK_STREAM, 0);
+    if (s == -1) {
+        return -1;
+    }
+
+    memset(&addr, 0, sizeof(addr));
+    addr.sun_family = AF_UNIX;
+    pstrcpy(addr.sun_path, sizeof(addr.sun_path), path);
+
+    if (bind(s, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
+        goto error;
+    }
+
+    if (listen(s, 128) == -1) {
+        goto error;
+    }
+
+    return s;
+error:
+    serrno = errno;
+    close(s);
+    errno = serrno;
+    return -1;
+}
+
+int unix_socket_outgoing(const char *path)
+{
+    int s;
+    struct sockaddr_un addr;
+    int serrno;
+
+    s = socket(PF_UNIX, SOCK_STREAM, 0);
+    if (s == -1) {
+        return -1;
+    }
+
+    memset(&addr, 0, sizeof(addr));
+    addr.sun_family = AF_UNIX;
+    pstrcpy(addr.sun_path, sizeof(addr.sun_path), path);
+
+    if (connect(s, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
+        goto error;
+    }
+
+    return s;
+error:
+    serrno = errno;
+    close(s);
+    errno = serrno;
+    return -1;
+}
+
+
 /* Basic flow
 
    Server         Client
@@ -388,8 +448,8 @@ int nbd_trip(BlockDriverState *bs, int c
 	}
 
 	if (len > sizeof(data)) {
-		LOG("len (%u) is larger than max len (%u)",
-		    len, sizeof(data));
+		LOG("len (%u) is larger than max len (%lu)",
+		    len, (unsigned long)sizeof(data));
 		errno = EINVAL;
 		return -1;
 	}
Index: qemu/nbd.h
===================================================================
--- qemu.orig/nbd.h	2008-06-18 17:05:46.000000000 +0200
+++ qemu/nbd.h	2008-06-19 10:47:46.000000000 +0200
@@ -27,6 +27,8 @@
 #include "block_int.h"
 
 int tcp_socket_incoming(const char *address, uint16_t port);
+int unix_socket_outgoing(const char *path);
+int unix_socket_incoming(const char *path);
 
 int nbd_negotiate(BlockDriverState *bs, int csock, off_t size);
 int nbd_receive_negotiate(int fd, int csock);
Index: qemu/qemu-nbd.c
===================================================================
--- qemu.orig/qemu-nbd.c	2008-06-18 17:05:46.000000000 +0200
+++ qemu/qemu-nbd.c	2008-06-19 10:48:49.000000000 +0200
@@ -21,15 +21,18 @@
 #include "block_int.h"
 #include "nbd.h"
 
-#include <malloc.h>
 #include <stdarg.h>
 #include <stdio.h>
 #include <getopt.h>
 #include <err.h>
+#include <sys/types.h>
 #include <sys/socket.h>
 #include <netinet/in.h>
 #include <netinet/tcp.h>
 #include <arpa/inet.h>
+#include <signal.h>
+
+//#define DEBUG
 
 int verbose;
 
@@ -44,6 +47,8 @@ static void usage(const char *name)
 "  -b, --bind=IFACE     interface to bind to (default `0.0.0.0')\n"
 "  -r, --read-only      export read-only\n"
 "  -P, --partition=NUM  only expose partition NUM\n"
+"  -c, --connect=DEV    connect FILE to the local NBD device DEV\n"
+"  -d, --disconnect     disconnect the specified device\n"
 "  -v, --verbose        display extra debugging information\n"
 "  -h, --help           display this help and exit\n"
 "  -V, --version        output version information and exit\n"
@@ -145,19 +150,39 @@ static int find_partition(BlockDriverSta
     return -1;
 }
 
+static void show_parts(const char *device)
+{
+    if (fork() == 0) {
+        int nbd;
+
+        /* linux just needs an open() to trigger
+         * the partition table update
+         * but remember to load the module with max_part != 0 :
+         *     modprobe nbd max_part=63
+         */
+        nbd = open(device, O_RDWR);
+        if (nbd != -1)
+        	close(nbd);
+        exit(0);
+    }
+}
+
 int main(int argc, char **argv)
 {
     BlockDriverState *bs;
     off_t dev_offset = 0;
     off_t offset = 0;
     bool readonly = false;
+    bool disconnect = false;
     const char *bindto = "0.0.0.0";
     int port = 1024;
     int sock, csock;
     struct sockaddr_in addr;
     socklen_t addr_len = sizeof(addr);
     off_t fd_size;
-    const char *sopt = "hVbo:p:rsP:v";
+    char *device;
+    char sockpath[128];
+    const char *sopt = "hVbo:p:rsP:c:dv?";
     struct option lopt[] = {
         { "help", 0, 0, 'h' },
         { "version", 0, 0, 'V' },
@@ -166,8 +191,11 @@ int main(int argc, char **argv)
         { "offset", 1, 0, 'o' },
         { "read-only", 0, 0, 'r' },
         { "partition", 1, 0, 'P' },
+        { "connect", 1, 0, 'c' },
+        { "disconnect", 0, 0, 'd' },
         { "snapshot", 0, 0, 's' },
         { "verbose", 0, 0, 'v' },
+	{ 0, 0, 0, 0 }
     };
     int ch;
     int opt_ind = 0;
@@ -175,6 +203,8 @@ int main(int argc, char **argv)
     char *end;
     bool snapshot = false;
     int partition = -1;
+    int fd;
+    int ret;
 
     while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
         switch (ch) {
@@ -213,6 +243,12 @@ int main(int argc, char **argv)
             if (partition < 1 || partition > 8)
                 errx(EINVAL, "Invalid partition %d", partition);
             break;
+        case 'd':
+            disconnect = true;
+            break;
+        case 'c':
+            device = optarg;
+            break;
         case 'v':
             verbose = 1;
             break;
@@ -236,6 +272,20 @@ int main(int argc, char **argv)
              argv[0]);
     }
 
+    if (disconnect) {
+        fd = open(argv[optind], O_RDWR);
+        if (fd == -1)
+            errx(errno, "Cannot open %s", argv[optind]);
+
+        nbd_disconnect(fd);
+
+        close(fd);
+
+        printf("%s disconnected\n", argv[optind]);
+
+	return 0;
+    }
+
     bdrv_init();
 
     bs = bdrv_new("hda");
@@ -251,7 +301,60 @@ int main(int argc, char **argv)
         find_partition(bs, partition, &dev_offset, &fd_size))
         errx(errno, "Could not find partition %d", partition);
 
-    sock = tcp_socket_incoming(bindto, port);
+    if (device) {
+	pid_t pid;
+	if (!verbose)
+            daemon(0, 0);	/* detach client and server */
+
+        sprintf(sockpath, "/var/lock/qemu-img-%s", basename(device));
+
+        pid = fork();
+        if (pid < 0)
+            return 1;
+	if (pid != 0) {
+            ret = 0;
+            bdrv_close(bs);
+
+            do {
+                sock = unix_socket_outgoing(sockpath);
+                if (sock == -1) {
+                    if (errno != ENOENT && errno != ECONNREFUSED)
+                        goto exit;
+                    sleep(1);	/* wait children */
+                }
+            } while (sock == -1);
+
+            fd = open(device, O_RDWR);
+            if (fd == -1) {
+                ret = -1;
+                goto exit;
+            }
+
+            ret = nbd_receive_negotiate(fd, sock);
+            if (ret == -1)
+                goto exit;
+
+            printf("NBD device %s is now connected to file %s\n",
+                    device, argv[optind]);
+
+	    /* if supported, update partition table */
+
+            show_parts(device);
+
+            nbd_client(fd, sock);
+            close(fd);
+exit:
+            kill(pid, SIGTERM);
+            unlink(sockpath);
+
+            return ret;
+        }
+        /* children */
+        sock = unix_socket_incoming(sockpath);
+    } else {
+        sock = tcp_socket_incoming(bindto, port);
+    }
+
     if (sock == -1)
         return 1;
 

Re: [Qemu-devel] [PATCH][v2] Merge NBD client/server int qemu-nbd

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 153 bytes --]

Oops, this one is up-to-date.
-- 
------------- Laurent.Vivier@bull.net ---------------
"The best way to predict the future is to invent it."
- Alan Kay

[-- Attachment #2: qemu-nbd-local.patch --]
[-- Type: text/x-patch, Size: 7597 bytes --]

Index: qemu/nbd.c
===================================================================
--- qemu.orig/nbd.c	2008-06-18 17:05:46.000000000 +0200
+++ qemu/nbd.c	2008-06-19 11:09:39.000000000 +0200
@@ -25,6 +25,7 @@
 #include <ctype.h>
 #include <inttypes.h>
 #include <sys/socket.h>
+#include <sys/un.h>
 #include <netinet/in.h>
 #include <netinet/tcp.h>
 #include <arpa/inet.h>
@@ -183,6 +184,65 @@ error:
     return -1;
 }
 
+int unix_socket_incoming(const char *path)
+{
+    int s;
+    struct sockaddr_un addr;
+    int serrno;
+
+    s = socket(PF_UNIX, SOCK_STREAM, 0);
+    if (s == -1) {
+        return -1;
+    }
+
+    memset(&addr, 0, sizeof(addr));
+    addr.sun_family = AF_UNIX;
+    pstrcpy(addr.sun_path, sizeof(addr.sun_path), path);
+
+    if (bind(s, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
+        goto error;
+    }
+
+    if (listen(s, 128) == -1) {
+        goto error;
+    }
+
+    return s;
+error:
+    serrno = errno;
+    close(s);
+    errno = serrno;
+    return -1;
+}
+
+int unix_socket_outgoing(const char *path)
+{
+    int s;
+    struct sockaddr_un addr;
+    int serrno;
+
+    s = socket(PF_UNIX, SOCK_STREAM, 0);
+    if (s == -1) {
+        return -1;
+    }
+
+    memset(&addr, 0, sizeof(addr));
+    addr.sun_family = AF_UNIX;
+    pstrcpy(addr.sun_path, sizeof(addr.sun_path), path);
+
+    if (connect(s, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
+        goto error;
+    }
+
+    return s;
+error:
+    serrno = errno;
+    close(s);
+    errno = serrno;
+    return -1;
+}
+
+
 /* Basic flow
 
    Server         Client
@@ -388,8 +448,8 @@ int nbd_trip(BlockDriverState *bs, int c
 	}
 
 	if (len > sizeof(data)) {
-		LOG("len (%u) is larger than max len (%u)",
-		    len, sizeof(data));
+		LOG("len (%u) is larger than max len (%lu)",
+		    len, (unsigned long)sizeof(data));
 		errno = EINVAL;
 		return -1;
 	}
Index: qemu/nbd.h
===================================================================
--- qemu.orig/nbd.h	2008-06-18 17:05:46.000000000 +0200
+++ qemu/nbd.h	2008-06-19 11:09:39.000000000 +0200
@@ -27,6 +27,8 @@
 #include "block_int.h"
 
 int tcp_socket_incoming(const char *address, uint16_t port);
+int unix_socket_outgoing(const char *path);
+int unix_socket_incoming(const char *path);
 
 int nbd_negotiate(BlockDriverState *bs, int csock, off_t size);
 int nbd_receive_negotiate(int fd, int csock);
Index: qemu/qemu-nbd.c
===================================================================
--- qemu.orig/qemu-nbd.c	2008-06-18 17:05:46.000000000 +0200
+++ qemu/qemu-nbd.c	2008-06-19 11:13:41.000000000 +0200
@@ -21,15 +21,16 @@
 #include "block_int.h"
 #include "nbd.h"
 
-#include <malloc.h>
 #include <stdarg.h>
 #include <stdio.h>
 #include <getopt.h>
 #include <err.h>
+#include <sys/types.h>
 #include <sys/socket.h>
 #include <netinet/in.h>
 #include <netinet/tcp.h>
 #include <arpa/inet.h>
+#include <signal.h>
 
 int verbose;
 
@@ -44,6 +45,8 @@ static void usage(const char *name)
 "  -b, --bind=IFACE     interface to bind to (default `0.0.0.0')\n"
 "  -r, --read-only      export read-only\n"
 "  -P, --partition=NUM  only expose partition NUM\n"
+"  -c, --connect=DEV    connect FILE to the local NBD device DEV\n"
+"  -d, --disconnect     disconnect the specified device\n"
 "  -v, --verbose        display extra debugging information\n"
 "  -h, --help           display this help and exit\n"
 "  -V, --version        output version information and exit\n"
@@ -145,19 +148,39 @@ static int find_partition(BlockDriverSta
     return -1;
 }
 
+static void show_parts(const char *device)
+{
+    if (fork() == 0) {
+        int nbd;
+
+        /* linux just needs an open() to trigger
+         * the partition table update
+         * but remember to load the module with max_part != 0 :
+         *     modprobe nbd max_part=63
+         */
+        nbd = open(device, O_RDWR);
+        if (nbd != -1)
+        	close(nbd);
+        exit(0);
+    }
+}
+
 int main(int argc, char **argv)
 {
     BlockDriverState *bs;
     off_t dev_offset = 0;
     off_t offset = 0;
     bool readonly = false;
+    bool disconnect = false;
     const char *bindto = "0.0.0.0";
     int port = 1024;
     int sock, csock;
     struct sockaddr_in addr;
     socklen_t addr_len = sizeof(addr);
     off_t fd_size;
-    const char *sopt = "hVbo:p:rsP:v";
+    char *device = NULL;
+    char sockpath[128];
+    const char *sopt = "hVbo:p:rsP:c:dv";
     struct option lopt[] = {
         { "help", 0, 0, 'h' },
         { "version", 0, 0, 'V' },
@@ -166,8 +189,11 @@ int main(int argc, char **argv)
         { "offset", 1, 0, 'o' },
         { "read-only", 0, 0, 'r' },
         { "partition", 1, 0, 'P' },
+        { "connect", 1, 0, 'c' },
+        { "disconnect", 0, 0, 'd' },
         { "snapshot", 0, 0, 's' },
         { "verbose", 0, 0, 'v' },
+	{ 0, 0, 0, 0 }
     };
     int ch;
     int opt_ind = 0;
@@ -175,6 +201,8 @@ int main(int argc, char **argv)
     char *end;
     bool snapshot = false;
     int partition = -1;
+    int fd;
+    int ret;
 
     while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
         switch (ch) {
@@ -213,6 +241,12 @@ int main(int argc, char **argv)
             if (partition < 1 || partition > 8)
                 errx(EINVAL, "Invalid partition %d", partition);
             break;
+        case 'd':
+            disconnect = true;
+            break;
+        case 'c':
+            device = optarg;
+            break;
         case 'v':
             verbose = 1;
             break;
@@ -236,6 +270,20 @@ int main(int argc, char **argv)
              argv[0]);
     }
 
+    if (disconnect) {
+        fd = open(argv[optind], O_RDWR);
+        if (fd == -1)
+            errx(errno, "Cannot open %s", argv[optind]);
+
+        nbd_disconnect(fd);
+
+        close(fd);
+
+        printf("%s disconnected\n", argv[optind]);
+
+	return 0;
+    }
+
     bdrv_init();
 
     bs = bdrv_new("hda");
@@ -251,7 +299,60 @@ int main(int argc, char **argv)
         find_partition(bs, partition, &dev_offset, &fd_size))
         errx(errno, "Could not find partition %d", partition);
 
-    sock = tcp_socket_incoming(bindto, port);
+    if (device) {
+	pid_t pid;
+	if (!verbose)
+            daemon(0, 0);	/* detach client and server */
+
+        sprintf(sockpath, "/var/lock/qemu-img-%s", basename(device));
+
+        pid = fork();
+        if (pid < 0)
+            return 1;
+	if (pid != 0) {
+            ret = 0;
+            bdrv_close(bs);
+
+            do {
+                sock = unix_socket_outgoing(sockpath);
+                if (sock == -1) {
+                    if (errno != ENOENT && errno != ECONNREFUSED)
+                        goto exit;
+                    sleep(1);	/* wait children */
+                }
+            } while (sock == -1);
+
+            fd = open(device, O_RDWR);
+            if (fd == -1) {
+                ret = -1;
+                goto exit;
+            }
+
+            ret = nbd_receive_negotiate(fd, sock);
+            if (ret == -1)
+                goto exit;
+
+            printf("NBD device %s is now connected to file %s\n",
+                    device, argv[optind]);
+
+	    /* update partition table */
+
+            show_parts(device);
+
+            nbd_client(fd, sock);
+            close(fd);
+exit:
+            kill(pid, SIGTERM);
+            unlink(sockpath);
+
+            return ret;
+        }
+        /* children */
+        sock = unix_socket_incoming(sockpath);
+    } else {
+        sock = tcp_socket_incoming(bindto, port);
+    }
+
     if (sock == -1)
         return 1;