[Qemu-devel] [PATCH v4 1/3] Update ioctl order in nbd_init() to detect EBUSY

[Qemu-devel] [PATCH v4 1/3] Update ioctl order in nbd_init() to detect EBUSY

[Chunyan Liu]

Update ioctl(s) in nbd_init() to detect device busy early.

Current nbd_init() issues NBD_CLEAR_SOCKET before NBD_SET_SOCKET, if issuing
"qemu-nbd -c /dev/nbd0 disk.img" twice, the second time won't detect EBUSY in
nbd_init(), but in nbd_client will report EBUSY and do clear socket (the 1st
time command will be affacted too because of no socket any more.)

No change to previous version.

Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 nbd.c |   27 +++++++++------------------
 1 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/nbd.c b/nbd.c
index e6c931c..4184614 100644
--- a/nbd.c
+++ b/nbd.c
@@ -348,6 +348,15 @@ int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags,
 #ifdef __linux__
 int nbd_init(int fd, int csock, uint32_t flags, off_t size, size_t blocksize)
 {
+    TRACE("Setting NBD socket");
+
+    if (ioctl(fd, NBD_SET_SOCK, csock) == -1) {
+        int serrno = errno;
+        LOG("Failed to set NBD socket");
+        errno = serrno;
+        return -1;
+    }
+
     TRACE("Setting block size to %lu", (unsigned long)blocksize);
 
     if (ioctl(fd, NBD_SET_BLKSIZE, blocksize) == -1) {
@@ -386,24 +395,6 @@ int nbd_init(int fd, int csock, uint32_t flags, off_t size, size_t blocksize)
         return -1;
     }
 
-    TRACE("Clearing NBD socket");
-
-    if (ioctl(fd, NBD_CLEAR_SOCK) == -1) {
-        int serrno = errno;
-        LOG("Failed clearing NBD socket");
-        errno = serrno;
-        return -1;
-    }
-
-    TRACE("Setting NBD socket");
-
-    if (ioctl(fd, NBD_SET_SOCK, csock) == -1) {
-        int serrno = errno;
-        LOG("Failed to set NBD socket");
-        errno = serrno;
-        return -1;
-    }
-
     TRACE("Negotiation ended");
 
     return 0;
-- 
1.7.3.4

[Qemu-devel] [PATCH v4 2/3] Extract code to nbd_setup function to be used for many purposes

[Chunyan Liu]

According to Stefan's suggestion, will loop over /dev/nbd%d to do "qemu-nbd -f
disk.img", if fails, try next device. To make "qemu-nbd -c" and "qemu-nbd -f"
share codes as more as possible, extract the shared codes to a function
nbd_setup(). Current qemu-nbd functions work well still. 

Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 qemu-nbd.c |  300 ++++++++++++++++++++++++++++++++----------------------------
 1 files changed, 159 insertions(+), 141 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index 291cba2..83ae30f 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -42,6 +42,18 @@ static int verbose;
 static char *device;
 static char *srcpath;
 static char *sockpath;
+static int is_sockpath_option;
+static int sigterm_fd[2];
+static off_t dev_offset;
+static uint32_t nbdflags;
+static bool disconnect;
+static const char *bindto = "0.0.0.0";
+static int port = NBD_DEFAULT_PORT;
+static int li;
+static int flags = BDRV_O_RDWR;
+static int partition = -1;
+static int shared = 1;
+static int persistent;
 
 static void usage(const char *name)
 {
@@ -171,9 +183,7 @@ static int find_partition(BlockDriverState *bs, int partition,
 static void termsig_handler(int signum)
 {
     static int sigterm_reported;
-    if (!sigterm_reported) {
-        sigterm_reported = (write(sigterm_wfd, "", 1) == 1);
-    }
+    sigterm_reported = (write(sigterm_wfd, "", 1) == 1);
 }
 
 static void *show_parts(void *arg)
@@ -244,18 +254,152 @@ out:
     return (void *) EXIT_FAILURE;
 }
 
-int main(int argc, char **argv)
+static int nbd_setup(void)
 {
     BlockDriverState *bs;
-    off_t dev_offset = 0;
     off_t offset = 0;
-    uint32_t nbdflags = 0;
-    bool disconnect = false;
-    const char *bindto = "0.0.0.0";
-    int port = NBD_DEFAULT_PORT;
     struct sockaddr_in addr;
     socklen_t addr_len = sizeof(addr);
     off_t fd_size;
+    uint8_t *data;
+    fd_set fds;
+    int *sharing_fds;
+    int fd;
+    int i;
+    int nb_fds = 0;
+    int max_fd;
+    pthread_t client_thread;
+    int ret;
+
+    if (device) {
+        /* Open before spawning new threads.  In the future, we may
+         * drop privileges after opening.
+         */
+        fd = open(device, O_RDWR);
+        if (fd == -1) {
+            err(EXIT_FAILURE, "Failed to open %s", device);
+        }
+
+        if (sockpath == NULL) {
+            sockpath = g_malloc(128);
+            snprintf(sockpath, 128, SOCKET_PATH, basename(device));
+        }
+    }
+
+    bs = bdrv_new("hda");
+    if ((ret = bdrv_open(bs, srcpath, flags, NULL)) < 0) {
+        errno = -ret;
+        err(EXIT_FAILURE, "Failed to bdrv_open '%s'", srcpath);
+    }
+
+    fd_size = bs->total_sectors * 512;
+
+    if (partition != -1 &&
+        find_partition(bs, partition, &dev_offset, &fd_size)) {
+        err(EXIT_FAILURE, "Could not find partition %d", partition);
+    }
+
+    sharing_fds = g_malloc((shared + 1) * sizeof(int));
+
+    if (sockpath) {
+        sharing_fds[0] = unix_socket_incoming(sockpath);
+    } else {
+        sharing_fds[0] = tcp_socket_incoming(bindto, port);
+    }
+
+    if (sharing_fds[0] == -1)
+        return 1;
+
+    if (device) {
+        int ret;
+
+        ret = pthread_create(&client_thread, NULL, nbd_client_thread, &fd);
+        if (ret != 0) {
+            errx(EXIT_FAILURE, "Failed to create client thread: %s",
+                 strerror(ret));
+        }
+    } else {
+        /* Shut up GCC warnings.  */
+        memset(&client_thread, 0, sizeof(client_thread));
+    }
+
+    max_fd = sharing_fds[0];
+    nb_fds++;
+
+    data = qemu_blockalign(bs, NBD_BUFFER_SIZE);
+    if (data == NULL) {
+        errx(EXIT_FAILURE, "Cannot allocate data buffer");
+    }
+
+    do {
+        FD_ZERO(&fds);
+        FD_SET(sigterm_fd[0], &fds);
+        for (i = 0; i < nb_fds; i++)
+            FD_SET(sharing_fds[i], &fds);
+
+        do {
+            ret = select(max_fd + 1, &fds, NULL, NULL, NULL);
+        } while (ret == -1 && errno == EINTR);
+        if (ret == -1 || FD_ISSET(sigterm_fd[0], &fds)) {
+            int sigbuf[1];
+            read(sigterm_fd[0], sigbuf, 1);
+            break;
+        }
+
+        if (FD_ISSET(sharing_fds[0], &fds))
+            ret--;
+        for (i = 1; i < nb_fds && ret; i++) {
+            if (FD_ISSET(sharing_fds[i], &fds)) {
+                if (nbd_trip(bs, sharing_fds[i], fd_size, dev_offset,
+                    &offset, nbdflags, data, NBD_BUFFER_SIZE) != 0) {
+                    close(sharing_fds[i]);
+                    nb_fds--;
+                    sharing_fds[i] = sharing_fds[nb_fds];
+                    i--;
+                }
+                ret--;
+            }
+        }
+        /* new connection ? */
+        if (FD_ISSET(sharing_fds[0], &fds)) {
+            if (nb_fds < shared + 1) {
+                sharing_fds[nb_fds] = accept(sharing_fds[0],
+                                             (struct sockaddr *)&addr,
+                                             &addr_len);
+                if (sharing_fds[nb_fds] != -1 &&
+                    nbd_negotiate(sharing_fds[nb_fds], fd_size, nbdflags) != -1) {
+                        if (sharing_fds[nb_fds] > max_fd)
+                            max_fd = sharing_fds[nb_fds];
+                        nb_fds++;
+                }
+            }
+        }
+    } while (persistent || nb_fds > 1);
+    qemu_vfree(data);
+
+    close(sharing_fds[0]);
+    g_free(sharing_fds);
+    if (sockpath) {
+        unlink(sockpath);
+    }
+    if (bs) {
+        bdrv_delete(bs);
+    }
+    if (!is_sockpath_option) {
+        sockpath = NULL;
+    }
+
+    if (device) {
+        void *ret;
+        pthread_join(client_thread, &ret);
+        return (ret != NULL) ? EXIT_FAILURE : EXIT_SUCCESS;
+    } else {
+        return EXIT_SUCCESS;
+    }
+}
+
+int main(int argc, char **argv)
+{
     const char *sopt = "hVb:o:p:rsnP:c:dvk:e:t";
     struct option lopt[] = {
         { "help", 0, NULL, 'h' },
@@ -277,27 +421,14 @@ int main(int argc, char **argv)
     };
     int ch;
     int opt_ind = 0;
-    int li;
     char *end;
-    int flags = BDRV_O_RDWR;
-    int partition = -1;
-    int ret;
-    int shared = 1;
-    uint8_t *data;
-    fd_set fds;
-    int *sharing_fds;
-    int fd;
-    int i;
-    int nb_fds = 0;
-    int max_fd;
-    int persistent = 0;
-    pthread_t client_thread;
+    int ret = 0;
 
     /* The client thread uses SIGTERM to interrupt the server.  A signal
      * handler ensures that "qemu-nbd -v -c" exits with a nice status code.
      */
     struct sigaction sa_sigterm;
-    int sigterm_fd[2];
+
     if (qemu_pipe(sigterm_fd) == -1) {
         err(EXIT_FAILURE, "Error setting up communication pipe");
     }
@@ -352,6 +483,7 @@ int main(int argc, char **argv)
             sockpath = optarg;
             if (sockpath[0] != '/')
                 errx(EXIT_FAILURE, "socket path must be absolute\n");
+            is_sockpath_option = 1;
             break;
         case 'd':
             disconnect = true;
@@ -395,6 +527,7 @@ int main(int argc, char **argv)
     }
 
     if (disconnect) {
+        int fd;
         fd = open(argv[optind], O_RDWR);
         if (fd == -1)
             err(EXIT_FAILURE, "Cannot open %s", argv[optind]);
@@ -411,7 +544,6 @@ int main(int argc, char **argv)
     if (device && !verbose) {
         int stderr_fd[2];
         pid_t pid;
-        int ret;
 
         if (qemu_pipe(stderr_fd) == -1) {
             err(EXIT_FAILURE, "Error setting up communication pipe");
@@ -460,125 +592,11 @@ int main(int argc, char **argv)
         }
     }
 
-    if (device) {
-        /* Open before spawning new threads.  In the future, we may
-         * drop privileges after opening.
-         */
-        fd = open(device, O_RDWR);
-        if (fd == -1) {
-            err(EXIT_FAILURE, "Failed to open %s", device);
-        }
-
-        if (sockpath == NULL) {
-            sockpath = g_malloc(128);
-            snprintf(sockpath, 128, SOCKET_PATH, basename(device));
-        }
-    }
-
     bdrv_init();
     atexit(bdrv_close_all);
-
-    bs = bdrv_new("hda");
     srcpath = argv[optind];
-    if ((ret = bdrv_open(bs, srcpath, flags, NULL)) < 0) {
-        errno = -ret;
-        err(EXIT_FAILURE, "Failed to bdrv_open '%s'", argv[optind]);
-    }
-
-    fd_size = bs->total_sectors * 512;
-
-    if (partition != -1 &&
-        find_partition(bs, partition, &dev_offset, &fd_size)) {
-        err(EXIT_FAILURE, "Could not find partition %d", partition);
-    }
-
-    sharing_fds = g_malloc((shared + 1) * sizeof(int));
-
-    if (sockpath) {
-        sharing_fds[0] = unix_socket_incoming(sockpath);
-    } else {
-        sharing_fds[0] = tcp_socket_incoming(bindto, port);
-    }
-
-    if (sharing_fds[0] == -1)
-        return 1;
-
-    if (device) {
-        int ret;
-
-        ret = pthread_create(&client_thread, NULL, nbd_client_thread, &fd);
-        if (ret != 0) {
-            errx(EXIT_FAILURE, "Failed to create client thread: %s",
-                 strerror(ret));
-        }
-    } else {
-        /* Shut up GCC warnings.  */
-        memset(&client_thread, 0, sizeof(client_thread));
-    }
-
-    max_fd = sharing_fds[0];
-    nb_fds++;
-
-    data = qemu_blockalign(bs, NBD_BUFFER_SIZE);
-    if (data == NULL) {
-        errx(EXIT_FAILURE, "Cannot allocate data buffer");
-    }
-
-    do {
-        FD_ZERO(&fds);
-        FD_SET(sigterm_fd[0], &fds);
-        for (i = 0; i < nb_fds; i++)
-            FD_SET(sharing_fds[i], &fds);
-
-        do {
-            ret = select(max_fd + 1, &fds, NULL, NULL, NULL);
-        } while (ret == -1 && errno == EINTR);
-        if (ret == -1 || FD_ISSET(sigterm_fd[0], &fds)) {
-            break;
-        }
-
-        if (FD_ISSET(sharing_fds[0], &fds))
-            ret--;
-        for (i = 1; i < nb_fds && ret; i++) {
-            if (FD_ISSET(sharing_fds[i], &fds)) {
-                if (nbd_trip(bs, sharing_fds[i], fd_size, dev_offset,
-                    &offset, nbdflags, data, NBD_BUFFER_SIZE) != 0) {
-                    close(sharing_fds[i]);
-                    nb_fds--;
-                    sharing_fds[i] = sharing_fds[nb_fds];
-                    i--;
-                }
-                ret--;
-            }
-        }
-        /* new connection ? */
-        if (FD_ISSET(sharing_fds[0], &fds)) {
-            if (nb_fds < shared + 1) {
-                sharing_fds[nb_fds] = accept(sharing_fds[0],
-                                             (struct sockaddr *)&addr,
-                                             &addr_len);
-                if (sharing_fds[nb_fds] != -1 &&
-                    nbd_negotiate(sharing_fds[nb_fds], fd_size, nbdflags) != -1) {
-                        if (sharing_fds[nb_fds] > max_fd)
-                            max_fd = sharing_fds[nb_fds];
-                        nb_fds++;
-                }
-            }
-        }
-    } while (persistent || nb_fds > 1);
-    qemu_vfree(data);
 
-    close(sharing_fds[0]);
-    g_free(sharing_fds);
-    if (sockpath) {
-        unlink(sockpath);
-    }
+    ret = nbd_setup();
 
-    if (device) {
-        void *ret;
-        pthread_join(client_thread, &ret);
-        exit(ret != NULL);
-    } else {
-        exit(EXIT_SUCCESS);
-    }
+    exit(ret);
 }
-- 
1.7.3.4

Re: [Qemu-devel] [PATCH v4 2/3] Extract code to nbd_setup function to be used for many purposes

[Paolo Bonzini]

On 12/02/2011 04:27 PM, Chunyan Liu wrote:
> @@ -42,6 +42,18 @@ static int verbose;
>   static char *device;
>   static char *srcpath;
>   static char *sockpath;
> +static int is_sockpath_option;
> +static int sigterm_fd[2];
> +static off_t dev_offset;
> +static uint32_t nbdflags;
> +static bool disconnect;
> +static const char *bindto = "0.0.0.0";
> +static int port = NBD_DEFAULT_PORT;
> +static int li;
> +static int flags = BDRV_O_RDWR;
> +static int partition = -1;
> +static int shared = 1;
> +static int persistent;

A lot of statics... "li" seems unused.

I took patch 1/3 in my tree (git://github.com/bonzini/qemu.git branch 
nbd-server).  I'll post it together with my patches next week.

Paolo

Re: [Qemu-devel] [PATCH v4 2/3] Extract code to nbd_setup function to be used for many purposes

[Chunyan Liu]

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

2011/12/3 Paolo Bonzini <pbonzini@redhat.com>

> On 12/02/2011 04:27 PM, Chunyan Liu wrote:
>
>> @@ -42,6 +42,18 @@ static int verbose;
>>  static char *device;
>>  static char *srcpath;
>>  static char *sockpath;
>> +static int is_sockpath_option;
>> +static int sigterm_fd[2];
>> +static off_t dev_offset;
>> +static uint32_t nbdflags;
>> +static bool disconnect;
>> +static const char *bindto = "0.0.0.0";
>> +static int port = NBD_DEFAULT_PORT;
>> +static int li;
>> +static int flags = BDRV_O_RDWR;
>> +static int partition = -1;
>> +static int shared = 1;
>> +static int persistent;
>>
>
> A lot of statics... "li" seems unused.
>

Using these statics simply because most of them are global parameters
getting from command line options, will be used later. Otherwise, the
nbd_setup() function should take many parameters.

Ahh, "li" could be defined in main(). After getting parameters from option,
later places can use "port".
       case 'p':
            li = strtol(optarg, &end, 0);
            if (*end) {
                errx(EXIT_FAILURE, "Invalid port `%s'", optarg);
            }
            if (li < 1 || li > 65535) {
                errx(EXIT_FAILURE, "Port out of range `%s'", optarg);
            }
            port = (uint16_t)li;


> I took patch 1/3 in my tree (git://github.com/bonzini/**qemu.git<http://github.com/bonzini/qemu.git>branch nbd-server).  I'll post it together with my patches next week.
>
> Paolo
>
>

[-- Attachment #2: Type: text/html, Size: 2225 bytes --]

Re: [Qemu-devel] [PATCH v4 2/3] Extract code to nbd_setup function to be used for many purposes

[Stefan Hajnoczi]

On Mon, Dec 5, 2011 at 5:46 AM, Chunyan Liu <cyliu@suse.com> wrote:
> 2011/12/3 Paolo Bonzini <pbonzini@redhat.com>
>>
>> On 12/02/2011 04:27 PM, Chunyan Liu wrote:
>>>
>>> @@ -42,6 +42,18 @@ static int verbose;
>>>  static char *device;
>>>  static char *srcpath;
>>>  static char *sockpath;
>>> +static int is_sockpath_option;
>>> +static int sigterm_fd[2];
>>> +static off_t dev_offset;
>>> +static uint32_t nbdflags;
>>> +static bool disconnect;
>>> +static const char *bindto = "0.0.0.0";
>>> +static int port = NBD_DEFAULT_PORT;
>>> +static int li;
>>> +static int flags = BDRV_O_RDWR;
>>> +static int partition = -1;
>>> +static int shared = 1;
>>> +static int persistent;
>>
>>
>> A lot of statics... "li" seems unused.
>
>
> Using these statics simply because most of them are global parameters
> getting from command line options, will be used later. Otherwise, the
> nbd_setup() function should take many parameters.
>
> Ahh, "li" could be defined in main(). After getting parameters from option,
> later places can use "port".
>        case 'p':
>             li = strtol(optarg, &end, 0);
>             if (*end) {
>                 errx(EXIT_FAILURE, "Invalid port `%s'", optarg);
>             }
>             if (li < 1 || li > 65535) {
>                 errx(EXIT_FAILURE, "Port out of range `%s'", optarg);
>             }
>             port = (uint16_t)li;

You can also move the bdrv_new()/bdrv_open()/bdrv_delete() out of the
nbd_setup() function.

The sigterm_fd[] variable could also be handled more cleanly.
Basically, I'd rather see a bigger patch that arranges things nicely
than chopping the function in half and making most variables global in
order to have a shorter patch.  We need to maintain this code so if it
needs to be rearranged in order to fit with the new requirements then
that's worth doing so it will be easier to read and maintain in the
future.

nbd_setup() is a slightly confusing name because the entire main loop
for the executes in the function.  Perhaps nbd_run() is better.

Stefan

Re: [Qemu-devel] [PATCH v4 2/3] Extract code to nbd_setup function to be used for many purposes

[Chunyan Liu]

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

2011/12/5 Stefan Hajnoczi <stefanha@gmail.com>

> On Mon, Dec 5, 2011 at 5:46 AM, Chunyan Liu <cyliu@suse.com> wrote:
> > 2011/12/3 Paolo Bonzini <pbonzini@redhat.com>
> >>
> >> On 12/02/2011 04:27 PM, Chunyan Liu wrote:
> >>>
> >>> @@ -42,6 +42,18 @@ static int verbose;
> >>>  static char *device;
> >>>  static char *srcpath;
> >>>  static char *sockpath;
> >>> +static int is_sockpath_option;
> >>> +static int sigterm_fd[2];
> >>> +static off_t dev_offset;
> >>> +static uint32_t nbdflags;
> >>> +static bool disconnect;
> >>> +static const char *bindto = "0.0.0.0";
> >>> +static int port = NBD_DEFAULT_PORT;
> >>> +static int li;
> >>> +static int flags = BDRV_O_RDWR;
> >>> +static int partition = -1;
> >>> +static int shared = 1;
> >>> +static int persistent;
> >>
> >>
> >> A lot of statics... "li" seems unused.
> >
> >
> > Using these statics simply because most of them are global parameters
> > getting from command line options, will be used later. Otherwise, the
> > nbd_setup() function should take many parameters.
> >
> > Ahh, "li" could be defined in main(). After getting parameters from
> option,
> > later places can use "port".
> >        case 'p':
> >             li = strtol(optarg, &end, 0);
> >             if (*end) {
> >                 errx(EXIT_FAILURE, "Invalid port `%s'", optarg);
> >             }
> >             if (li < 1 || li > 65535) {
> >                 errx(EXIT_FAILURE, "Port out of range `%s'", optarg);
> >             }
> >             port = (uint16_t)li;
>
> You can also move the bdrv_new()/bdrv_open()/bdrv_delete() out of the
> nbd_setup() function.
>

Sure. But different parameters needed by nbd_setup(). (bs, dev_offset,
fd_size instead of srcpath, dev_offset, partition, flags).

The sigterm_fd[] variable could also be handled more cleanly.
>

Any suggestion?


> Basically, I'd rather see a bigger patch that arranges things nicely
> than chopping the function in half and making most variables global in
> order to have a shorter patch.


Currently, the nbd_setup needs parameters: device, srcpath, flags,
partition, dev_offset, nbdflags, sockpath, bindto, port, shared,
persistent, verbose, sigterm_rfd. More than 10 parameters. I still didn't
find a better way to reduce parameters. Making variables global is a
workaround to avoid nbd_setup taking too many parameters. Actually, except
for sigterm_rfd, all others are pared from command line options.

To improve, what I can think of is to define a structure to save those
values parsed from command line options and pass that structure to
nbd_setup as a parameter. Of course, bdrv_new/open/delete can be moved out,
just passing more parameters to nbd_setup. Temporarily, I couldn't think of
others. Do you have any better ideas?

We need to maintain this code so if it
> needs to be rearranged in order to fit with the new requirements then
> that's worth doing so it will be easier to read and maintain in the
> future.

nbd_setup() is a slightly confusing name because the entire main loop
> for the executes in the function.  Perhaps nbd_run() is better.

No problem, could be changed.


> Stefan
>
>

[-- Attachment #2: Type: text/html, Size: 4840 bytes --]

Re: [Qemu-devel] [PATCH v4 2/3] Extract code to nbd_setup function to be used for many purposes

[Paolo Bonzini]

On 12/06/2011 07:56 AM, Chunyan Liu wrote:
>
> Currently, the nbd_setup needs parameters: device, srcpath, flags,
> partition, dev_offset, nbdflags, sockpath, bindto, port, shared,
> persistent, verbose, sigterm_rfd. More than 10 parameters. I still
> didn't find a better way to reduce parameters. Making variables global
> is a workaround to avoid nbd_setup taking too many parameters. Actually,
> except for sigterm_rfd, all others are pared from command line options.

Reading again this patch, I am not sure why you are doing it this way.

There is no reason why bdrv_new/open/delete has to be redone for every 
/dev/nbdX we try (or if there is a reason, _that_ is what should be 
fixed first).  Also the "tail" of nbd_setup, basically the select loop, 
should not be tried multiple times.

I do not understand why you cannot simply do it like this:

- in the server thread, do everything as it is now

- pass "device" to the client thread instead of opening it in main()

- in the client thread, either use "device" as it is or (if device == 
NULL, which implies find == 1) loop until nbd_init succeeds.

Am I just confused?

Paolo

Re: [Qemu-devel] [PATCH v4 2/3] Extract code to nbd_setup function to be used for many purposes

[Chunyan Liu]

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

2011/12/6 Paolo Bonzini <pbonzini@redhat.com>

> On 12/06/2011 07:56 AM, Chunyan Liu wrote:
>
>>
>> Currently, the nbd_setup needs parameters: device, srcpath, flags,
>> partition, dev_offset, nbdflags, sockpath, bindto, port, shared,
>> persistent, verbose, sigterm_rfd. More than 10 parameters. I still
>> didn't find a better way to reduce parameters. Making variables global
>> is a workaround to avoid nbd_setup taking too many parameters. Actually,
>> except for sigterm_rfd, all others are pared from command line options.
>>
>
> Reading again this patch, I am not sure why you are doing it this way.
>
> There is no reason why bdrv_new/open/delete has to be redone for every
> /dev/nbdX we try (or if there is a reason, _that_ is what should be fixed
> first).


Well, it's not "had to be redone", did it just to avoid passing too many
parameters to nbd_setup. (otherwise, should pass "bs, dev_offset and
fd_size to nbd_setup.) Can be moved out.


> Also the "tail" of nbd_setup, basically the select loop, should not be
> tried multiple times.
>
> I do not understand why you cannot simply do it like this:
>
> - in the server thread, do everything as it is now
>

Nope. When device changes, both client thread and server thread should be
refreshed. sockpath and sharing_fds[] is changed with different device.


> - pass "device" to the client thread instead of opening it in main()
>
> - in the client thread, either use "device" as it is or (if device ==
> NULL, which implies find == 1) loop until nbd_init succeeds.

Am I just confused?
>
> Paolo
>
>

[-- Attachment #2: Type: text/html, Size: 2579 bytes --]

Re: [Qemu-devel] [PATCH v4 2/3] Extract code to nbd_setup function to be used for many purposes

[Paolo Bonzini]

On 12/06/2011 09:42 AM, Chunyan Liu wrote:
>
>     I do not understand why you cannot simply do it like this:
>
>     - in the server thread, do everything as it is now
>
> Nope. When device changes, both client thread and server thread should
> be refreshed. sockpath and sharing_fds[] is changed with different device.

Then let's change the default sockpath to include the pid rather than 
the NBD device name.

Paolo

Re: [Qemu-devel] [PATCH v4 2/3] Extract code to nbd_setup function to be used for many purposes

[Chunyan Liu]

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

2011/12/6 Paolo Bonzini <pbonzini@redhat.com>

> On 12/06/2011 09:42 AM, Chunyan Liu wrote:
>
>>
>>    I do not understand why you cannot simply do it like this:
>>
>>    - in the server thread, do everything as it is now
>>
>> Nope. When device changes, both client thread and server thread should
>> be refreshed. sockpath and sharing_fds[] is changed with different device.
>>
>
> Then let's change the default sockpath to include the pid rather than the
> NBD device name.
>
> Sounds good. Will try. Thanks.


> Paolo
>
>

[-- Attachment #2: Type: text/html, Size: 1204 bytes --]

Re: [Qemu-devel] [PATCH v4 2/3] Extract code to nbd_setup function to be used for many purposes

[Chunyan Liu]

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

Add -f option to qemu-nbd. New implementation. Do not need nbd_setup.
Tested and worked.

Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 qemu-nbd.c |   71
+++++++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 49 insertions(+), 22 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index 291cba2..ba66da1 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -33,7 +33,7 @@
 #include <libgen.h>
 #include <pthread.h>

-#define SOCKET_PATH    "/var/lock/qemu-nbd-%s"
+#define SOCKET_PATH    "/var/lock/qemu-nbd-%d"

 #define NBD_BUFFER_SIZE (1024*1024)

@@ -53,7 +53,7 @@ static void usage(const char *name)
 "  -o, --offset=OFFSET  offset into the image\n"
 "  -b, --bind=IFACE     interface to bind to (default `0.0.0.0')\n"
 "  -k, --socket=PATH    path to the unix socket\n"
-"                       (default '"SOCKET_PATH"')\n"
+"                       (default /var/lock/qemu-nbd-%s)\n"
 "  -r, --read-only      export read-only\n"
 "  -P, --partition=NUM  only expose partition NUM\n"
 "  -s, --snapshot       use snapshot file\n"
@@ -67,7 +67,7 @@ static void usage(const char *name)
 "  -V, --version        output version information and exit\n"
 "\n"
 "Report bugs to <anthony@codemonkey.ws>\n"
-    , name, NBD_DEFAULT_PORT, "DEVICE");
+    , name, NBD_DEFAULT_PORT, "PID");
 }

 static void version(const char *name)
@@ -194,7 +194,8 @@ static void *show_parts(void *arg)

 static void *nbd_client_thread(void *arg)
 {
-    int fd = *(int *)arg;
+    int fd = -1;
+    int find = *(int *)arg;
     off_t size;
     size_t blocksize;
     uint32_t nbdflags;
@@ -215,9 +216,38 @@ static void *nbd_client_thread(void *arg)
         goto out;
     }

-    ret = nbd_init(fd, sock, nbdflags, size, blocksize);
-    if (ret == -1) {
-        goto out;
+    if (!find) {
+        fd = open(device, O_RDWR);
+        if (fd == -1) {
+            fprintf(stderr, "Failed to open %s", device);
+            goto out;
+        }
+
+        ret = nbd_init(fd, sock, nbdflags, size, blocksize);
+        if (ret == -1) {
+            goto out;
+        }
+    } else {
+        int i = 0;
+        int max_nbd = 16;
+        for (i = 0; i < max_nbd; i++) {
+            if (!asprintf(&device, "/dev/nbd%d", i)) {
+                continue;
+            }
+
+            fd = open(device, O_RDWR);
+            if (fd == -1 || nbd_init(fd, sock, nbdflags, size, blocksize)
== -1) {
+                free(device);
+                continue;
+            }
+
+            break;
+        }
+
+        if (i >= max_nbd) {
+            fprintf(stderr, "Fail to find a free nbd device\n");
+            goto out;
+        }
     }

     /* update partition table */
@@ -256,7 +286,7 @@ int main(int argc, char **argv)
     struct sockaddr_in addr;
     socklen_t addr_len = sizeof(addr);
     off_t fd_size;
-    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:t";
+    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:tf";
     struct option lopt[] = {
         { "help", 0, NULL, 'h' },
         { "version", 0, NULL, 'V' },
@@ -273,6 +303,7 @@ int main(int argc, char **argv)
         { "shared", 1, NULL, 'e' },
         { "persistent", 0, NULL, 't' },
         { "verbose", 0, NULL, 'v' },
+        { "find", 0, NULL, 'f'},
         { NULL, 0, NULL, 0 }
     };
     int ch;
@@ -291,6 +322,7 @@ int main(int argc, char **argv)
     int nb_fds = 0;
     int max_fd;
     int persistent = 0;
+    int find = 0;
     pthread_t client_thread;

     /* The client thread uses SIGTERM to interrupt the server.  A signal
@@ -374,6 +406,9 @@ int main(int argc, char **argv)
         case 'v':
             verbose = 1;
             break;
+        case 'f':
+            find = 1;
+            break;
         case 'V':
             version(argv[0]);
             exit(0);
@@ -408,7 +443,7 @@ int main(int argc, char **argv)
     return 0;
     }

-    if (device && !verbose) {
+    if ((device || find) && !verbose) {
         int stderr_fd[2];
         pid_t pid;
         int ret;
@@ -460,18 +495,10 @@ int main(int argc, char **argv)
         }
     }

-    if (device) {
-        /* Open before spawning new threads.  In the future, we may
-         * drop privileges after opening.
-         */
-        fd = open(device, O_RDWR);
-        if (fd == -1) {
-            err(EXIT_FAILURE, "Failed to open %s", device);
-        }
-
+    if (device || find) {
         if (sockpath == NULL) {
             sockpath = g_malloc(128);
-            snprintf(sockpath, 128, SOCKET_PATH, basename(device));
+            snprintf(sockpath, 128, SOCKET_PATH, getpid());
         }
     }

@@ -503,10 +530,10 @@ int main(int argc, char **argv)
     if (sharing_fds[0] == -1)
         return 1;

-    if (device) {
+    if (device || find) {
         int ret;

-        ret = pthread_create(&client_thread, NULL, nbd_client_thread, &fd);
+        ret = pthread_create(&client_thread, NULL, nbd_client_thread,
&find);
         if (ret != 0) {
             errx(EXIT_FAILURE, "Failed to create client thread: %s",
                  strerror(ret));
@@ -574,7 +601,7 @@ int main(int argc, char **argv)
         unlink(sockpath);
     }

-    if (device) {
+    if (device || find) {
         void *ret;
         pthread_join(client_thread, &ret);
         exit(ret != NULL);
-- 
1.7.3.4


2011/12/6 Chunyan Liu <cyliu@suse.com>

>
>
> 2011/12/6 Paolo Bonzini <pbonzini@redhat.com>
>
>> On 12/06/2011 09:42 AM, Chunyan Liu wrote:
>>
>>>
>>>    I do not understand why you cannot simply do it like this:
>>>
>>>    - in the server thread, do everything as it is now
>>>
>>> Nope. When device changes, both client thread and server thread should
>>> be refreshed. sockpath and sharing_fds[] is changed with different
>>> device.
>>>
>>
>> Then let's change the default sockpath to include the pid rather than the
>> NBD device name.
>>
>> Sounds good. Will try. Thanks.
>
>
>> Paolo
>>
>>
>

[-- Attachment #2: Type: text/html, Size: 7866 bytes --]

Re: [Qemu-devel] [PATCH v4 2/3] Extract code to nbd_setup function to be used for many purposes

[Paolo Bonzini]

On 12/07/2011 05:23 AM, Chunyan Liu wrote:
> Add -f option to qemu-nbd. New implementation. Do not need nbd_setup.
> Tested and worked.
>
> Signed-off-by: Chunyan Liu <cyliu@suse.com <mailto:cyliu@suse.com>>
> ---
>   qemu-nbd.c |   71 +++++++++++++++++++++++++++++++++++++++++------------------
>   1 files changed, 49 insertions(+), 22 deletions(-)

Looks much nicer, thanks.

Paolo

Re: [Qemu-devel] [PATCH v4 2/3] Extract code to nbd_setup function to be used for many purposes

[Stefan Hajnoczi]

On Wed, Dec 7, 2011 at 4:23 AM, Chunyan Liu <cyliu@suse.com> wrote:

Overall looks good, some suggestions:

> @@ -53,7 +53,7 @@ static void usage(const char *name)
>  "  -o, --offset=OFFSET  offset into the image\n"
>  "  -b, --bind=IFACE     interface to bind to (default `0.0.0.0')\n"
>  "  -k, --socket=PATH    path to the unix socket\n"
> -"                       (default '"SOCKET_PATH"')\n"
> +"                       (default /var/lock/qemu-nbd-%s)\n"
>  "  -r, --read-only      export read-only\n"
>  "  -P, --partition=NUM  only expose partition NUM\n"
>  "  -s, --snapshot       use snapshot file\n"
> @@ -67,7 +67,7 @@ static void usage(const char *name)
>  "  -V, --version        output version information and exit\n"
>  "\n"
>  "Report bugs to <anthony@codemonkey.ws>\n"
> -    , name, NBD_DEFAULT_PORT, "DEVICE");
> +    , name, NBD_DEFAULT_PORT, "PID");
>  }

Since we're not using the SOCKET_PATH format string anymore it's nicer
to drop this format string argument and just change to "(default
/var/lock/qemu-nbd-PID" above.

> +        fd = open(device, O_RDWR);
> +        if (fd == -1) {
> +            fprintf(stderr, "Failed to open %s", device);

Missing \n in string.

> +            goto out;
> +        }
> +
> +        ret = nbd_init(fd, sock, nbdflags, size, blocksize);
> +        if (ret == -1) {
> +            goto out;
> +        }
> +    } else {
> +        int i = 0;
> +        int max_nbd = 16;
> +        for (i = 0; i < max_nbd; i++) {
> +            if (!asprintf(&device, "/dev/nbd%d", i)) {

asprintf() is GNU and BSD only (not C or POSIX).  I suggest using
g_strdup_printf() instead which is guaranteed to be available because
QEMU builds against glib:

http://developer.gnome.org/glib/2.28/glib-String-Utility-Functions.html#g-strdup-printf

> +                continue;
> +            }
>
> +
> +            fd = open(device, O_RDWR);
> +            if (fd == -1 || nbd_init(fd, sock, nbdflags, size, blocksize)
> == -1) {

nbd_init() does not close(fd) on failure.  Please separate the open()
and nbd_init() error cases and close the fd.

Stefan

Re: [Qemu-devel] [PATCH v4 2/3] Extract code to nbd_setup function to be used for many purposes

[Chunyan Liu]

Thanks for your suggestions. Revision.

Add -f option to qemu-nbd.

Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 qemu-nbd.c |   76 ++++++++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 54 insertions(+), 22 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index 291cba2..2dc0742 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -33,7 +33,7 @@
 #include <libgen.h>
 #include <pthread.h>

-#define SOCKET_PATH    "/var/lock/qemu-nbd-%s"
+#define SOCKET_PATH    "/var/lock/qemu-nbd-%d"

 #define NBD_BUFFER_SIZE (1024*1024)

@@ -53,12 +53,13 @@ static void usage(const char *name)
 "  -o, --offset=OFFSET  offset into the image\n"
 "  -b, --bind=IFACE     interface to bind to (default `0.0.0.0')\n"
 "  -k, --socket=PATH    path to the unix socket\n"
-"                       (default '"SOCKET_PATH"')\n"
+"                       (default /var/lock/qemu-nbd-PID)\n"
 "  -r, --read-only      export read-only\n"
 "  -P, --partition=NUM  only expose partition NUM\n"
 "  -s, --snapshot       use snapshot file\n"
 "  -n, --nocache        disable host cache\n"
 "  -c, --connect=DEV    connect FILE to the local NBD device DEV\n"
+"  -f, --find           find a free NBD device and connect FILE to it\n"
 "  -d, --disconnect     disconnect the specified device\n"
 "  -e, --shared=NUM     device can be shared by NUM clients (default '1')\n"
 "  -t, --persistent     don't exit on the last connection\n"
@@ -67,7 +68,7 @@ static void usage(const char *name)
 "  -V, --version        output version information and exit\n"
 "\n"
 "Report bugs to <anthony@codemonkey.ws>\n"
-    , name, NBD_DEFAULT_PORT, "DEVICE");
+    , name, NBD_DEFAULT_PORT);
 }

 static void version(const char *name)
@@ -194,7 +195,8 @@ static void *show_parts(void *arg)

 static void *nbd_client_thread(void *arg)
 {
-    int fd = *(int *)arg;
+    int fd = -1;
+    int find = *(int *)arg;
     off_t size;
     size_t blocksize;
     uint32_t nbdflags;
@@ -215,9 +217,42 @@ static void *nbd_client_thread(void *arg)
         goto out;
     }

-    ret = nbd_init(fd, sock, nbdflags, size, blocksize);
-    if (ret == -1) {
-        goto out;
+    if (!find) {
+        fd = open(device, O_RDWR);
+        if (fd == -1) {
+            fprintf(stderr, "Failed to open %s\n", device);
+            goto out;
+        }
+
+        ret = nbd_init(fd, sock, nbdflags, size, blocksize);
+        if (ret == -1) {
+            goto out;
+        }
+    } else {
+        int i = 0;
+        int max_nbd = 16;
+        device = g_malloc(64);
+
+        for (i = 0; i < max_nbd; i++) {
+            snprintf(device, 64, "/dev/nbd%d", i);
+            fd = open(device, O_RDWR);
+            if (fd == -1) {
+                continue;
+            }
+
+            if (nbd_init(fd, sock, nbdflags, size, blocksize) == -1) {
+                close(fd);
+                continue;
+            }
+
+            break;
+        }
+
+        if (i >= max_nbd) {
+            fprintf(stderr, "Fail to find a free nbd device\n");
+            g_free(device);
+            goto out;
+        }
     }

     /* update partition table */
@@ -256,7 +291,7 @@ int main(int argc, char **argv)
     struct sockaddr_in addr;
     socklen_t addr_len = sizeof(addr);
     off_t fd_size;
-    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:t";
+    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:tf";
     struct option lopt[] = {
         { "help", 0, NULL, 'h' },
         { "version", 0, NULL, 'V' },
@@ -273,6 +308,7 @@ int main(int argc, char **argv)
         { "shared", 1, NULL, 'e' },
         { "persistent", 0, NULL, 't' },
         { "verbose", 0, NULL, 'v' },
+        { "find", 0, NULL, 'f'},
         { NULL, 0, NULL, 0 }
     };
     int ch;
@@ -291,6 +327,7 @@ int main(int argc, char **argv)
     int nb_fds = 0;
     int max_fd;
     int persistent = 0;
+    int find = 0;
     pthread_t client_thread;

     /* The client thread uses SIGTERM to interrupt the server.  A signal
@@ -374,6 +411,9 @@ int main(int argc, char **argv)
         case 'v':
             verbose = 1;
             break;
+        case 'f':
+            find = 1;
+            break;
         case 'V':
             version(argv[0]);
             exit(0);
@@ -408,7 +448,7 @@ int main(int argc, char **argv)
 	return 0;
     }

-    if (device && !verbose) {
+    if ((device || find) && !verbose) {
         int stderr_fd[2];
         pid_t pid;
         int ret;
@@ -460,18 +500,10 @@ int main(int argc, char **argv)
         }
     }

-    if (device) {
-        /* Open before spawning new threads.  In the future, we may
-         * drop privileges after opening.
-         */
-        fd = open(device, O_RDWR);
-        if (fd == -1) {
-            err(EXIT_FAILURE, "Failed to open %s", device);
-        }
-
+    if (device || find) {
         if (sockpath == NULL) {
             sockpath = g_malloc(128);
-            snprintf(sockpath, 128, SOCKET_PATH, basename(device));
+            snprintf(sockpath, 128, SOCKET_PATH, getpid());
         }
     }

@@ -503,10 +535,10 @@ int main(int argc, char **argv)
     if (sharing_fds[0] == -1)
         return 1;

-    if (device) {
+    if (device || find) {
         int ret;

-        ret = pthread_create(&client_thread, NULL, nbd_client_thread, &fd);
+        ret = pthread_create(&client_thread, NULL, nbd_client_thread, &find);
         if (ret != 0) {
             errx(EXIT_FAILURE, "Failed to create client thread: %s",
                  strerror(ret));
@@ -574,7 +606,7 @@ int main(int argc, char **argv)
         unlink(sockpath);
     }

-    if (device) {
+    if (device || find) {
         void *ret;
         pthread_join(client_thread, &ret);
         exit(ret != NULL);
-- 
1.7.3.4



2011/12/8 Stefan Hajnoczi <stefanha@gmail.com>:
> On Wed, Dec 7, 2011 at 4:23 AM, Chunyan Liu <cyliu@suse.com> wrote:
>
> Overall looks good, some suggestions:
>
>> @@ -53,7 +53,7 @@ static void usage(const char *name)
>>  "  -o, --offset=OFFSET  offset into the image\n"
>>  "  -b, --bind=IFACE     interface to bind to (default `0.0.0.0')\n"
>>  "  -k, --socket=PATH    path to the unix socket\n"
>> -"                       (default '"SOCKET_PATH"')\n"
>> +"                       (default /var/lock/qemu-nbd-%s)\n"
>>  "  -r, --read-only      export read-only\n"
>>  "  -P, --partition=NUM  only expose partition NUM\n"
>>  "  -s, --snapshot       use snapshot file\n"
>> @@ -67,7 +67,7 @@ static void usage(const char *name)
>>  "  -V, --version        output version information and exit\n"
>>  "\n"
>>  "Report bugs to <anthony@codemonkey.ws>\n"
>> -    , name, NBD_DEFAULT_PORT, "DEVICE");
>> +    , name, NBD_DEFAULT_PORT, "PID");
>>  }
>
> Since we're not using the SOCKET_PATH format string anymore it's nicer
> to drop this format string argument and just change to "(default
> /var/lock/qemu-nbd-PID" above.
>
>> +        fd = open(device, O_RDWR);
>> +        if (fd == -1) {
>> +            fprintf(stderr, "Failed to open %s", device);
>
> Missing \n in string.
>
>> +            goto out;
>> +        }
>> +
>> +        ret = nbd_init(fd, sock, nbdflags, size, blocksize);
>> +        if (ret == -1) {
>> +            goto out;
>> +        }
>> +    } else {
>> +        int i = 0;
>> +        int max_nbd = 16;
>> +        for (i = 0; i < max_nbd; i++) {
>> +            if (!asprintf(&device, "/dev/nbd%d", i)) {
>
> asprintf() is GNU and BSD only (not C or POSIX).  I suggest using
> g_strdup_printf() instead which is guaranteed to be available because
> QEMU builds against glib:

Using g_malloc, snprintf, g_free instead. Doesn't need extra header file.

> http://developer.gnome.org/glib/2.28/glib-String-Utility-Functions.html#g-strdup-printf
>
>> +                continue;
>> +            }
>>
>> +
>> +            fd = open(device, O_RDWR);
>> +            if (fd == -1 || nbd_init(fd, sock, nbdflags, size, blocksize)
>> == -1) {
>
> nbd_init() does not close(fd) on failure.  Please separate the open()
> and nbd_init() error cases and close the fd.
>
> Stefan
>

Re: [Qemu-devel] [PATCH v4 2/3] Extract code to nbd_setup function to be used for many purposes

[Stefan Hajnoczi]

On Fri, Dec 9, 2011 at 8:31 AM, Chunyan Liu <cyliu@suse.com> wrote:
> Thanks for your suggestions. Revision.
>
> Add -f option to qemu-nbd.
>
> Signed-off-by: Chunyan Liu <cyliu@suse.com>
> ---
>  qemu-nbd.c |   76 ++++++++++++++++++++++++++++++++++++++++++-----------------
>  1 files changed, 54 insertions(+), 22 deletions(-)

Looks good, thanks!

Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

[Qemu-devel] [PATCH v4 3/3] Add -f option to qemu-nbd

[Chunyan Liu]

Add -f option to qemu-nbd so that it can find a free nbd device and connect
disk.img to that device.

Changes to v3:
a. According to Stefan's suggestion, loop over /dev/nbd%d to do "qemu-nbd -f
disk.img", if fails, try next device.
b. syntax "qemu-nbd -f disk.img" only 

Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
 qemu-nbd.c |   25 ++++++++++++++++++++++---
 1 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index 83ae30f..431373d 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -400,7 +400,7 @@ static int nbd_setup(void)
 
 int main(int argc, char **argv)
 {
-    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:t";
+    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:tf";
     struct option lopt[] = {
         { "help", 0, NULL, 'h' },
         { "version", 0, NULL, 'V' },
@@ -417,12 +417,14 @@ int main(int argc, char **argv)
         { "shared", 1, NULL, 'e' },
         { "persistent", 0, NULL, 't' },
         { "verbose", 0, NULL, 'v' },
+        { "find", 0, NULL, 'f' },
         { NULL, 0, NULL, 0 }
     };
     int ch;
     int opt_ind = 0;
     char *end;
     int ret = 0;
+    int find = 0;
 
     /* The client thread uses SIGTERM to interrupt the server.  A signal
      * handler ensures that "qemu-nbd -v -c" exits with a nice status code.
@@ -491,6 +493,9 @@ int main(int argc, char **argv)
         case 'c':
             device = optarg;
             break;
+        case 'f':
+            find = 1;
+            break;
         case 'e':
             shared = strtol(optarg, &end, 0);
             if (*end) {
@@ -541,7 +546,7 @@ int main(int argc, char **argv)
 	return 0;
     }
 
-    if (device && !verbose) {
+    if ((device || find) && !verbose) {
         int stderr_fd[2];
         pid_t pid;
 
@@ -596,7 +601,21 @@ int main(int argc, char **argv)
     atexit(bdrv_close_all);
     srcpath = argv[optind];
 
-    ret = nbd_setup();
+    if (!find) {
+        ret = nbd_setup();
+    } else {
+        int max_nbd = 16;
+        int i;
+        for (i = 0; i < max_nbd; i++) {
+            if (!asprintf(&device, "/dev/nbd%d", i))
+                continue;
+            ret = nbd_setup();
+            if (ret != EXIT_SUCCESS) {
+                free(device);
+                continue;
+            }
+        }
+    }
 
     exit(ret);
 }
-- 
1.7.3.4