| Message ID | 1322043290-20349-2-git-send-email-cyliu@suse.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Nov 23, 2011 at 10:14 AM, Chunyan Liu <cyliu@suse.com> wrote: > V3: > Remove file lock in main(). > Try to find new free nbd device and connect to it if connecting to the first > first found free nbd device failed. > > Signed-off-by: Chunyan Liu <cyliu@suse.com> > --- > qemu-nbd.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 files changed, 79 insertions(+), 1 deletions(-) I not seeing the part where you adjusted the ioctl order. The /proc/partitions scanning is unnecessary since we can just loop over /dev/ndb%d and try to initialize. If a device is in use then init will fail and we need to try the next one. If a device is free we can continue with normal operation. I guess I'm saying that once you fix the ioctl order then there's no need for another mechanism to test whether or not a device is in use. Also please use QEMU coding style, you can run qemu/scripts/checkpatch.pl on your patches. Stefan
On Wed, Nov 23, 2011 at 10:56 AM, Stefan Hajnoczi <stefanha@gmail.com> wrote: > On Wed, Nov 23, 2011 at 10:14 AM, Chunyan Liu <cyliu@suse.com> wrote: >> V3: >> Remove file lock in main(). >> Try to find new free nbd device and connect to it if connecting to the first >> first found free nbd device failed. >> >> Signed-off-by: Chunyan Liu <cyliu@suse.com> >> --- >> qemu-nbd.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- >> 1 files changed, 79 insertions(+), 1 deletions(-) > > I not seeing the part where you adjusted the ioctl order. I see the other patch on the qemu-devel list now, sorry. Looked at this mail first because it was CCed to me. Stefan
2011/11/23 Stefan Hajnoczi <stefanha@gmail.com> > On Wed, Nov 23, 2011 at 10:14 AM, Chunyan Liu <cyliu@suse.com> wrote: > > V3: > > Remove file lock in main(). > > Try to find new free nbd device and connect to it if connecting to the > first > > first found free nbd device failed. > > > > Signed-off-by: Chunyan Liu <cyliu@suse.com> > > --- > > qemu-nbd.c | 80 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- > > 1 files changed, 79 insertions(+), 1 deletions(-) > > I not seeing the part where you adjusted the ioctl order. > > The /proc/partitions scanning is unnecessary since we can just loop > over /dev/ndb%d and try to initialize. If a device is in use then > init will fail and we need to try the next one. If a device is free > we can continue with normal operation. I guess I'm saying that once > you fix the ioctl order then there's no need for another mechanism to > test whether or not a device is in use. > The way of scanning /proc/partitions to find an unused nbd device first can borrow the code for "qemu-nbd -c" to do the left things. . The way of loop over /dev/nbd%d and try to initialize, from the first thought, needs do all things in the loop, including handling -v, nbd_init, nbd_client, etc. That part of code is quite similar to "qemu-nbd -c". I don't know if that is better? > Also please use QEMU coding style, you can run > qemu/scripts/checkpatch.pl on your patches. > > Stefan > >
On Thu, Nov 24, 2011 at 3:38 AM, Chunyan Liu <cyliu@suse.com> wrote: > > > 2011/11/23 Stefan Hajnoczi <stefanha@gmail.com> >> >> On Wed, Nov 23, 2011 at 10:14 AM, Chunyan Liu <cyliu@suse.com> wrote: >> > V3: >> > Remove file lock in main(). >> > Try to find new free nbd device and connect to it if connecting to the >> > first >> > first found free nbd device failed. >> > >> > Signed-off-by: Chunyan Liu <cyliu@suse.com> >> > --- >> > qemu-nbd.c | 80 >> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- >> > 1 files changed, 79 insertions(+), 1 deletions(-) >> >> I not seeing the part where you adjusted the ioctl order. >> >> The /proc/partitions scanning is unnecessary since we can just loop >> over /dev/ndb%d and try to initialize. If a device is in use then >> init will fail and we need to try the next one. If a device is free >> we can continue with normal operation. I guess I'm saying that once >> you fix the ioctl order then there's no need for another mechanism to >> test whether or not a device is in use. > > The way of scanning /proc/partitions to find an unused nbd device first can > borrow the code for "qemu-nbd -c" to do the left things. . > The way of loop over /dev/nbd%d and try to initialize, from the first > thought, needs do all things in the loop, including handling -v, nbd_init, > nbd_client, etc. That part of code is quite similar to "qemu-nbd -c". I > don't know if that is better? This might be a chance to refactor the code slightly, that would also avoid you having to introduce a goto retry. Stefan
diff --git a/qemu-nbd.c b/qemu-nbd.c index 291cba2..fcaaf50 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -32,6 +32,7 @@ #include <signal.h> #include <libgen.h> #include <pthread.h> +#include <fcntl.h> #define SOCKET_PATH "/var/lock/qemu-nbd-%s" @@ -244,6 +245,60 @@ out: return (void *) EXIT_FAILURE; } +static int is_nbd_used(int minor) +{ + FILE *proc; + int NBDMAJOR = 43; + char buf[BUFSIZ]; + int find = 0; + + proc = fopen("/proc/partitions", "r"); + if (proc != NULL) { + while (fgets(buf, sizeof(buf), proc)) { + int m, n; + unsigned long long sz; + char name[16]; + char *pname = name; + char *end; + + if (sscanf(buf, " %d %d %llu %128[^\n ]", + &m, &n, &sz, name) != 4) + continue; + if (m != NBDMAJOR) + continue; + if (strncmp(name, "nbd", 3)) + continue; + pname += 3; + n = strtol(pname, &end, 10); + if (end && end != pname && *end == '\0' && n == minor) { + find = 1; + break; + } + } + fclose(proc); + } + + return find; +} + +static char *find_free_nbd_device(void) +{ + int i; + int nbds_max = 16; + char name[16]; + char *devname = NULL; + + for (i = 0; i < nbds_max; i++) { + if (!is_nbd_used(i)) { + snprintf(name, sizeof(name), "/dev/nbd%d", i); + devname = strdup(name); + break; + } + } + + return devname; +} + int main(int argc, char **argv) { BlockDriverState *bs; @@ -256,7 +311,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 +328,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; @@ -292,6 +348,8 @@ int main(int argc, char **argv) int max_fd; int persistent = 0; pthread_t client_thread; + char *devname = NULL; + 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. @@ -374,6 +432,18 @@ int main(int argc, char **argv) case 'v': verbose = 1; break; + case 'f': + find = 1; + devname = find_free_nbd_device(); + if (devname == NULL) + exit(1); + if (argc == optind) { + printf("%s\n", devname); + free(devname); + exit(0); + } + device = devname; + break; case 'V': version(argv[0]); exit(0); @@ -464,6 +534,7 @@ int main(int argc, char **argv) /* Open before spawning new threads. In the future, we may * drop privileges after opening. */ +retry: fd = open(device, O_RDWR); if (fd == -1) { err(EXIT_FAILURE, "Failed to open %s", device); @@ -577,6 +648,13 @@ int main(int argc, char **argv) if (device) { void *ret; pthread_join(client_thread, &ret); + if (ret != EXIT_SUCCESS && find) { + free(device); + device = find_free_nbd_device(); + if (!device) + err(EXIT_FAILURE, "Could not find free nbd device"); + goto retry; + } exit(ret != NULL); } else { exit(EXIT_SUCCESS);
V3: Remove file lock in main(). Try to find new free nbd device and connect to it if connecting to the first first found free nbd device failed. Signed-off-by: Chunyan Liu <cyliu@suse.com> --- qemu-nbd.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 79 insertions(+), 1 deletions(-)