2011/11/24 Stefan Hajnoczi <stefa...@gmail.com>
> On Thu, Nov 24, 2011 at 3:38 AM, Chunyan Liu <cy...@suse.com> wrote:
> >
> >
> > 2011/11/23 Stefan Hajnoczi <stefa...@gmail.com>
> >>
> >> On Wed, Nov 23, 2011 at 10:14 AM, Chunyan Liu <cy...@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 <cy...@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.
>
> About detail implementation of the loop over /dev/nbd%d way, have done
some coding and testing. I'm afraid the reorganization work is not very
slight, not sure how do you think.
First, through nbd_init() success or fail to check if the device is in use,
at least nbd_init() and operations before nbd_init() but device relative
should be in the loop. Usually, there are two ways to be considered:
1. Try to divide current "qemu-nbd -c" processing code into two parts (
nbd_init() and before, and operations after nbd_init()), 1st part put into
the loop, 2nd part outside loop. This way is hard to achieve. nbd_init()
should be in client thread, but the loop is outside that thread, it's not
proper to drag out nbd_init() from client thread and put it into the loop.
2. Put all "qemu-nbd -c" processing code (device relative) in the loop,
directly check the "qemu-nbd -c /dev/nbd%d disk.img" result, if fail, try
next nbd device. In this way, will adjust current code order and extract
device related codes to a new function so that both "qemu-nbd -c" and
"qemu-nbd -f" can use.(some initialization work and cleanup work to be
cared)
A draft is in attachment. qemu-nbd -f is working, since I'm not sure such
change is acceptable or not, not extract code in the loop into a function
yet.
> Stefan
>
>
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 291cba2..38c3bc3 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -171,9 +171,9 @@ static int find_partition(BlockDriverState *bs, int partition,
static void termsig_handler(int signum)
{
static int sigterm_reported;
- if (!sigterm_reported) {
+// if (!sigterm_reported) {
sigterm_reported = (write(sigterm_wfd, "", 1) == 1);
- }
+// }
}
static void *show_parts(void *arg)
@@ -256,7 +256,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 +273,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,7 +293,8 @@ int main(int argc, char **argv)
int max_fd;
int persistent = 0;
pthread_t client_thread;
-
+ int find = 0;
+ int find_minor;
/* 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 +376,10 @@ int main(int argc, char **argv)
case 'v':
verbose = 1;
break;
+ case 'f':
+ find = 1;
+ device = "/dev/nbd0";
+ break;
case 'V':
version(argv[0]);
exit(0);
@@ -459,11 +465,34 @@ int main(int argc, char **argv)
exit(errors);
}
}
+ bdrv_init();
+ atexit(bdrv_close_all);
+
+ bs = bdrv_new("hdb");
+ 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);
+ }
+
+if (find) {
+ for (find_minor=0; find_minor<16; find_minor++){
+ asprintf(&device, "/dev/nbd%d", find_minor);
+ if (!device) {
+ continue;
+ }
+
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);
@@ -475,23 +504,6 @@ int main(int argc, char **argv)
}
}
- 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) {
@@ -534,11 +546,14 @@ int main(int argc, char **argv)
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))
+ 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,
@@ -577,8 +592,18 @@ int main(int argc, char **argv)
if (device) {
void *ret;
pthread_join(client_thread, &ret);
- exit(ret != NULL);
+ if (ret != EXIT_SUCCESS && find) {
+ sockpath = NULL;
+ nb_fds = 0;
+ free(device);
+ continue;
+ }
+ else {
+ exit(ret != NULL);
+ }
} else {
exit(EXIT_SUCCESS);
}
+ }
+}
}
