qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH][resend] Add -f option to qemu-nbd


From: Paolo Bonzini
Subject: Re: [Qemu-devel] [PATCH][resend] Add -f option to qemu-nbd
Date: Wed, 18 Jan 2012 11:51:31 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:9.0) Gecko/20111222 Thunderbird/9.0

On 01/18/2012 09:48 AM, Chunyan Liu wrote:
Stefan, could you help commit it if it's OK? Thanks.
Same as in thread:
http://lists.gnu.org/archive/html/qemu-devel/2011-12/msg01083.html
but rebase it to latest code.

Add -f option to qemu-nbd to find a free nbd device for user and connect
disk image to that device.

Hi Chunyan,

I'm now the NBD maintainer so I can take care of the patch. There are still a couple of nits.

The main problem is that the device name is never printed, which requires some imagination on part of the user. :)

This pretty much requires that you set "verbose = 1" together with -f. This disables daemonization, unfortunately, but there isn't really any better way to do so. I'm inclined towards removing daemonization altogether from 1.1; other opinions are welcome.

Anyhow, please set "verbose = 1" and change the help text for -c/-v from

"  -c, --connect=DEV    connect FILE to the local NBD device DEV\n"

"  -v, --verbose        display extra debugging information\n"

to

"  -c, --connect=DEV    connect FILE to the local NBD device DEV\n"
"                       and run in the background\n"

"  -v, --verbose        with -c, display extra information and\n"
"                       do not run in the background\n"

Since a respin is required, there are a couple of other comments inline.

syntax: qemu-nbd -f disk.img
---
  qemu-nbd.c |   76 ++++++++++++++++++++++++++++++++++++++++++-----------------
  1 files changed, 54 insertions(+), 22 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index eb61c33..f4c1437 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"

  static NBDExport *exp;
  static int verbose;
@@ -55,12 +55,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"
@@ -69,7 +70,7 @@ static void usage(const char *name)
  "  -V, --version        output version information and exit\n"
  "\n"
  "Report bugs to<address@hidden>\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;
@@ -213,9 +215,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;

Just use errx(EXIT_FAILURE, "Could not find a free NBD device.") here.

+        }
      }

      /* update partition table */
@@ -275,7 +310,7 @@ int main(int argc, char **argv)
      const char *bindto = "0.0.0.0";
      int port = NBD_DEFAULT_PORT;
      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' },
@@ -292,6 +327,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;
@@ -303,6 +339,7 @@ int main(int argc, char **argv)
      int ret;
      int fd;
      int persistent = 0;
+    int find = 0;
      pthread_t client_thread;

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

Please give an error here if "device && find" ("The -f option is incompatible with -c").

-    if (device&&  !verbose) {
+    if ((device || find)&&  !verbose) {
          int stderr_fd[2];
          pid_t pid;
          int ret;
@@ -466,18 +506,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());
          }
      }

@@ -510,10 +542,10 @@ int main(int argc, char **argv)
          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);

Just pass NULL here and test "device != NULL" in the nbd_client_thread.

          if (ret != 0) {
              errx(EXIT_FAILURE, "Failed to create client thread: %s",
                   strerror(ret));
@@ -536,7 +568,7 @@ int main(int argc, char **argv)
          unlink(sockpath);
      }

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

Thanks,

Paolo



reply via email to

[Prev in Thread] Current Thread [Next in Thread]