[Qemu-devel] A Patch to enable qemu-nbd run as an inetd service

[Qemu-devel] A Patch to enable qemu-nbd run as an inetd service

[Jun Sheng]

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

run qemu-nbd as an inetd service has some benefits
* more scriptable, such as serve multiple images to different clients
on one ip/port
* access control using tcpd

simple usage:
#!/bin/sh
# qemu-nbd wrapper, select image file according to client ip address
IMG_FILE=`sed -n "s/$REMOTE_HOST //p" /path/to/image_list.txt`
qemu-nbd -i 10 $IMG_FILE 10<&0- 1>/tmp/log 2>/tmp/log2

#end

#xinetd.conf
service nbd
{
        flags                   = REUSE
        socket_type             = stream
        wait                    = no
        user                    = some_user
        server                  = /path/to/qemu-nbd-wrapper.sh
        log_on_failure          += USERID
        disable                 = no
}

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

diff --git a/qemu-nbd.c b/qemu-nbd.c
index b524b34..13695e9 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -49,6 +49,7 @@ static NBDExport *exp;
 static int verbose;
 static char *srcpath;
 static char *sockpath;
+static int use_inetd = 0;
 static int persistent = 0;
 static enum { RUNNING, TERMINATE, TERMINATING, TERMINATED } state;
 static int shared = 1;
@@ -66,6 +67,7 @@ static void usage(const char *name)
 "Connection properties:\n"
 "  -p, --port=PORT           port to listen on (default `%d')\n"
 "  -b, --bind=IFACE          interface to bind to (default `0.0.0.0')\n"
+"  -i, --inetd=FD            run as an inetd services on FD\n"
 "  -k, --socket=PATH         path to the unix socket\n"
 "                            (default '"SOCKET_PATH"')\n"
 "  -e, --shared=NUM          device can be shared by NUM clients (default '1')\n"
@@ -363,7 +365,13 @@ static void nbd_accept(void *opaque)
     struct sockaddr_in addr;
     socklen_t addr_len = sizeof(addr);
 
-    int fd = accept(server_fd, (struct sockaddr *)&addr, &addr_len);
+    
+    int fd ;
+    if (use_inetd == 0) 
+      fd = accept(server_fd, (struct sockaddr *)&addr, &addr_len);
+    else
+      fd = server_fd;
+
     if (fd < 0) {
         perror("accept");
         return;
@@ -395,10 +403,11 @@ int main(int argc, char **argv)
     off_t fd_size;
     QemuOpts *sn_opts = NULL;
     const char *sn_id_or_name = NULL;
-    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:";
+    const char *sopt = "hVi:b:o:p:rsnP:c:dvk:e:f:tl:";
     struct option lopt[] = {
         { "help", 0, NULL, 'h' },
         { "version", 0, NULL, 'V' },
+	{ "inetd", 1, NULL, 'i'},
         { "bind", 1, NULL, 'b' },
         { "port", 1, NULL, 'p' },
         { "socket", 1, NULL, 'k' },
@@ -430,6 +439,7 @@ int main(int argc, char **argv)
     int partition = -1;
     int ret;
     int fd;
+    int inet_fd = 10;
     bool seen_cache = false;
     bool seen_discard = false;
 #ifdef CONFIG_LINUX_AIO
@@ -510,6 +520,16 @@ int main(int argc, char **argv)
         case 'b':
             bindto = optarg;
             break;
+	case 'i':
+  	    use_inetd = 1;
+	    inet_fd = strtol(optarg, &end, 0);
+	    if (*end) {
+                errx(EXIT_FAILURE, "Invalid inet fd `%s'", optarg);
+            }
+            if (inet_fd < 3 || inet_fd > 65535) {
+                errx(EXIT_FAILURE, "Inet fd out of range `%s', should be in [3, 65535]", optarg);
+            }
+	    break;
         case 'p':
             li = strtol(optarg, &end, 0);
             if (*end) {
@@ -729,11 +749,15 @@ int main(int argc, char **argv)
     }
 
     exp = nbd_export_new(bs, dev_offset, fd_size, nbdflags, nbd_export_closed);
-
-    if (sockpath) {
+    
+    if (use_inetd == 0) {
+      if (sockpath) {
         fd = unix_socket_incoming(sockpath);
-    } else {
+      } else {
         fd = tcp_socket_incoming(bindto, port);
+      }
+    } else {
+      fd = inet_fd;
     }
 
     if (fd < 0) {
@@ -752,9 +776,11 @@ int main(int argc, char **argv)
         /* Shut up GCC warnings.  */
         memset(&client_thread, 0, sizeof(client_thread));
     }
-
-    qemu_set_fd_handler2(fd, nbd_can_accept, nbd_accept, NULL,
+    if (use_inetd == 0)
+      qemu_set_fd_handler2(fd, nbd_can_accept, nbd_accept, NULL,
                          (void *)(uintptr_t)fd);
+    else
+      nbd_accept((void *) (uintptr_t) fd);
 
     /* now when the initialization is (almost) complete, chdir("/")
      * to free any busy filesystems */

Re: [Qemu-devel] A Patch to enable qemu-nbd run as an inetd service

[Eric Blake]

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

On 10/22/2014 12:41 AM, Jun Sheng wrote:
> run qemu-nbd as an inetd service has some benefits
> * more scriptable, such as serve multiple images to different clients
> on one ip/port
> * access control using tcpd
> 
> simple usage:
> #!/bin/sh
> # qemu-nbd wrapper, select image file according to client ip address
> IMG_FILE=`sed -n "s/$REMOTE_HOST //p" /path/to/image_list.txt`
> qemu-nbd -i 10 $IMG_FILE 10<&0- 1>/tmp/log 2>/tmp/log2

Thanks for the patch.  However, it is lacking a Signed-off-by line, and
was sent in an awkward manner (we prefer the patch inline as part of the
mail, rather than an attachment; git send-email does this properly).
You may have better luck if you review
http://wiki.qemu.org/Contribute/SubmitAPatch and try again.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

[Qemu-devel] A Patch to enable qemu-nbd run as an inetd service

[Jun Sheng]

From: Chaos Eternal <chaos@shlug.org>


run qemu-nbd as an inetd service has some benefits
* more scriptable, such as serve multiple images to different clients
on one ip/port
* access control using tcpd

simple usage:
#!/bin/sh
# qemu-nbd wrapper, select image file according to client ip address
IMG_FILE=`sed -n "s/$REMOTE_HOST //p" /path/to/image_list.txt`
qemu-nbd -i 10 $IMG_FILE 10<&0- 1>/tmp/log 2>/tmp/log2

#end

#xinetd.conf
service nbd
{
        flags                   = REUSE
        socket_type             = stream
        wait                    = no
        user                    = some_user
        server                  = /path/to/qemu-nbd-wrapper.sh
        log_on_failure          += USERID
        disable                 = no
}

Signed-off-by: Jun Sheng <chaoseternal@gmail.com>
---
 qemu-nbd.c | 40 +++++++++++++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index b524b34..13695e9 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -49,6 +49,7 @@ static NBDExport *exp;
 static int verbose;
 static char *srcpath;
 static char *sockpath;
+static int use_inetd = 0;
 static int persistent = 0;
 static enum { RUNNING, TERMINATE, TERMINATING, TERMINATED } state;
 static int shared = 1;
@@ -66,6 +67,7 @@ static void usage(const char *name)
 "Connection properties:\n"
 "  -p, --port=PORT           port to listen on (default `%d')\n"
 "  -b, --bind=IFACE          interface to bind to (default `0.0.0.0')\n"
+"  -i, --inetd=FD            run as an inetd services on FD\n"
 "  -k, --socket=PATH         path to the unix socket\n"
 "                            (default '"SOCKET_PATH"')\n"
 "  -e, --shared=NUM          device can be shared by NUM clients (default '1')\n"
@@ -363,7 +365,13 @@ static void nbd_accept(void *opaque)
     struct sockaddr_in addr;
     socklen_t addr_len = sizeof(addr);
 
-    int fd = accept(server_fd, (struct sockaddr *)&addr, &addr_len);
+    
+    int fd ;
+    if (use_inetd == 0) 
+      fd = accept(server_fd, (struct sockaddr *)&addr, &addr_len);
+    else
+      fd = server_fd;
+
     if (fd < 0) {
         perror("accept");
         return;
@@ -395,10 +403,11 @@ int main(int argc, char **argv)
     off_t fd_size;
     QemuOpts *sn_opts = NULL;
     const char *sn_id_or_name = NULL;
-    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:";
+    const char *sopt = "hVi:b:o:p:rsnP:c:dvk:e:f:tl:";
     struct option lopt[] = {
         { "help", 0, NULL, 'h' },
         { "version", 0, NULL, 'V' },
+	{ "inetd", 1, NULL, 'i'},
         { "bind", 1, NULL, 'b' },
         { "port", 1, NULL, 'p' },
         { "socket", 1, NULL, 'k' },
@@ -430,6 +439,7 @@ int main(int argc, char **argv)
     int partition = -1;
     int ret;
     int fd;
+    int inet_fd = 10;
     bool seen_cache = false;
     bool seen_discard = false;
 #ifdef CONFIG_LINUX_AIO
@@ -510,6 +520,16 @@ int main(int argc, char **argv)
         case 'b':
             bindto = optarg;
             break;
+	case 'i':
+  	    use_inetd = 1;
+	    inet_fd = strtol(optarg, &end, 0);
+	    if (*end) {
+                errx(EXIT_FAILURE, "Invalid inet fd `%s'", optarg);
+            }
+            if (inet_fd < 3 || inet_fd > 65535) {
+                errx(EXIT_FAILURE, "Inet fd out of range `%s', should be in [3, 65535]", optarg);
+            }
+	    break;
         case 'p':
             li = strtol(optarg, &end, 0);
             if (*end) {
@@ -729,11 +749,15 @@ int main(int argc, char **argv)
     }
 
     exp = nbd_export_new(bs, dev_offset, fd_size, nbdflags, nbd_export_closed);
-
-    if (sockpath) {
+    
+    if (use_inetd == 0) {
+      if (sockpath) {
         fd = unix_socket_incoming(sockpath);
-    } else {
+      } else {
         fd = tcp_socket_incoming(bindto, port);
+      }
+    } else {
+      fd = inet_fd;
     }
 
     if (fd < 0) {
@@ -752,9 +776,11 @@ int main(int argc, char **argv)
         /* Shut up GCC warnings.  */
         memset(&client_thread, 0, sizeof(client_thread));
     }
-
-    qemu_set_fd_handler2(fd, nbd_can_accept, nbd_accept, NULL,
+    if (use_inetd == 0)
+      qemu_set_fd_handler2(fd, nbd_can_accept, nbd_accept, NULL,
                          (void *)(uintptr_t)fd);
+    else
+      nbd_accept((void *) (uintptr_t) fd);
 
     /* now when the initialization is (almost) complete, chdir("/")
      * to free any busy filesystems */
-- 
2.1.2

Re: [Qemu-devel] A Patch to enable qemu-nbd run as an inetd service

[Eric Blake]

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

On 10/22/2014 12:54 PM, Jun Sheng wrote:
> From: Chaos Eternal <chaos@shlug.org>
> 
> 
> run qemu-nbd as an inetd service has some benefits
> * more scriptable, such as serve multiple images to different clients
> on one ip/port
> * access control using tcpd
> 

> @@ -363,7 +365,13 @@ static void nbd_accept(void *opaque)
>      struct sockaddr_in addr;
>      socklen_t addr_len = sizeof(addr);
>  
> -    int fd = accept(server_fd, (struct sockaddr *)&addr, &addr_len);
> +    
> +    int fd ;
> +    if (use_inetd == 0) 
> +      fd = accept(server_fd, (struct sockaddr *)&addr, &addr_len);
> +    else
> +      fd = server_fd;

Please run ./scripts/checkpatch.pl on your submission.  It would have
pointed out that our coding style requires {} on both branches of
if/else statements, indentation by 4-space multiples, as well as no
space before semicolon.  This is documented on
http://wiki.qemu.org/Contribute/SubmitAPatch
:";
>      struct option lopt[] = {
>          { "help", 0, NULL, 'h' },
>          { "version", 0, NULL, 'V' },
> +	{ "inetd", 1, NULL, 'i'},

TAB damage. Also would have been caught by checkpatch.pl

>          { "bind", 1, NULL, 'b' },
>          { "port", 1, NULL, 'p' },
>          { "socket", 1, NULL, 'k' },
> @@ -430,6 +439,7 @@ int main(int argc, char **argv)
>      int partition = -1;
>      int ret;
>      int fd;
> +    int inet_fd = 10;

Magic number.  Also, why do you even need to pre-initialize it?  But
pre-initializing fds to -1 feels safer than to a random value that may
or may not be a valid fd.

>      bool seen_cache = false;
>      bool seen_discard = false;
>  #ifdef CONFIG_LINUX_AIO
> @@ -510,6 +520,16 @@ int main(int argc, char **argv)
>          case 'b':
>              bindto = optarg;
>              break;
> +	case 'i':
> +  	    use_inetd = 1;

Prefer bool (with true/false values) over int (with 0/1 values).  Or
even better, use inet_fd==-1 as the witness of no inetd parameter, and a
non-negative value as witness of a user-supplied fd, and then you don't
need 'use_inetd' at all.

> +	    inet_fd = strtol(optarg, &end, 0);
> +	    if (*end) {
> +                errx(EXIT_FAILURE, "Invalid inet fd `%s'", optarg);

Improper use of strtol.  You are correctly rejecting "0a" as garbage,
but fail to detect overflow like "99999999999999999" or the empty string
"" as garbage.

> +            }
> +            if (inet_fd < 3 || inet_fd > 65535) {

Magic numbers.  I can understand why you are requiring an fd larger than
STDERR_FILENO (but spell it 'inet_fd <= STDERR_FILENO', not 'inet_fd <
3); but arbitrarily capping at 64k is bogus.  Better to just try and use
the fd, and it either works,

> +                errx(EXIT_FAILURE, "Inet fd out of range `%s', should be in [3, 65535]", optarg);

errx() is non-standard; qemu-nbd.c is the only file that uses it, but it
would be a nice cleanup (as a separate patch) to convert over to more
idiomatic error reporting similar to the rest of the qemu code base.

> @@ -752,9 +776,11 @@ int main(int argc, char **argv)
>          /* Shut up GCC warnings.  */
>          memset(&client_thread, 0, sizeof(client_thread));
>      }
> -
> -    qemu_set_fd_handler2(fd, nbd_can_accept, nbd_accept, NULL,
> +    if (use_inetd == 0)
> +      qemu_set_fd_handler2(fd, nbd_can_accept, nbd_accept, NULL,
>                           (void *)(uintptr_t)fd);

Indentation of the second line needs to be modified when moving the
first line.  Same comments as earlier about {} and 4-space indentation.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

Re: [Qemu-devel] A Patch to enable qemu-nbd run as an inetd service

[Jun Sheng]

Hi,

Since errx is used everywhere in qemu-nbd.c, I suggest that I follow
this convention in submitting my patch.

On Thu, Oct 23, 2014 at 4:48 AM, Eric Blake <eblake@redhat.com> wrote:
> On 10/22/2014 12:54 PM, Jun Sheng wrote:
>> From: Chaos Eternal <chaos@shlug.org>
>>
>>
>> run qemu-nbd as an inetd service has some benefits
>> * more scriptable, such as serve multiple images to different clients
>> on one ip/port
>> * access control using tcpd
>>
>
>> @@ -363,7 +365,13 @@ static void nbd_accept(void *opaque)
>>      struct sockaddr_in addr;
>>      socklen_t addr_len = sizeof(addr);
>>
>> -    int fd = accept(server_fd, (struct sockaddr *)&addr, &addr_len);
>> +
>> +    int fd ;
>> +    if (use_inetd == 0)
>> +      fd = accept(server_fd, (struct sockaddr *)&addr, &addr_len);
>> +    else
>> +      fd = server_fd;
>
> Please run ./scripts/checkpatch.pl on your submission.  It would have
> pointed out that our coding style requires {} on both branches of
> if/else statements, indentation by 4-space multiples, as well as no
> space before semicolon.  This is documented on
> http://wiki.qemu.org/Contribute/SubmitAPatch
> :";
>>      struct option lopt[] = {
>>          { "help", 0, NULL, 'h' },
>>          { "version", 0, NULL, 'V' },
>> +     { "inetd", 1, NULL, 'i'},
>
> TAB damage. Also would have been caught by checkpatch.pl
>
>>          { "bind", 1, NULL, 'b' },
>>          { "port", 1, NULL, 'p' },
>>          { "socket", 1, NULL, 'k' },
>> @@ -430,6 +439,7 @@ int main(int argc, char **argv)
>>      int partition = -1;
>>      int ret;
>>      int fd;
>> +    int inet_fd = 10;
>
> Magic number.  Also, why do you even need to pre-initialize it?  But
> pre-initializing fds to -1 feels safer than to a random value that may
> or may not be a valid fd.
>
>>      bool seen_cache = false;
>>      bool seen_discard = false;
>>  #ifdef CONFIG_LINUX_AIO
>> @@ -510,6 +520,16 @@ int main(int argc, char **argv)
>>          case 'b':
>>              bindto = optarg;
>>              break;
>> +     case 'i':
>> +         use_inetd = 1;
>
> Prefer bool (with true/false values) over int (with 0/1 values).  Or
> even better, use inet_fd==-1 as the witness of no inetd parameter, and a
> non-negative value as witness of a user-supplied fd, and then you don't
> need 'use_inetd' at all.
>
>> +         inet_fd = strtol(optarg, &end, 0);
>> +         if (*end) {
>> +                errx(EXIT_FAILURE, "Invalid inet fd `%s'", optarg);
>
> Improper use of strtol.  You are correctly rejecting "0a" as garbage,
> but fail to detect overflow like "99999999999999999" or the empty string
> "" as garbage.
>
>> +            }
>> +            if (inet_fd < 3 || inet_fd > 65535) {
>
> Magic numbers.  I can understand why you are requiring an fd larger than
> STDERR_FILENO (but spell it 'inet_fd <= STDERR_FILENO', not 'inet_fd <
> 3); but arbitrarily capping at 64k is bogus.  Better to just try and use
> the fd, and it either works,
>
>> +                errx(EXIT_FAILURE, "Inet fd out of range `%s', should be in [3, 65535]", optarg);
>
> errx() is non-standard; qemu-nbd.c is the only file that uses it, but it
> would be a nice cleanup (as a separate patch) to convert over to more
> idiomatic error reporting similar to the rest of the qemu code base.
>
>> @@ -752,9 +776,11 @@ int main(int argc, char **argv)
>>          /* Shut up GCC warnings.  */
>>          memset(&client_thread, 0, sizeof(client_thread));
>>      }
>> -
>> -    qemu_set_fd_handler2(fd, nbd_can_accept, nbd_accept, NULL,
>> +    if (use_inetd == 0)
>> +      qemu_set_fd_handler2(fd, nbd_can_accept, nbd_accept, NULL,
>>                           (void *)(uintptr_t)fd);
>
> Indentation of the second line needs to be modified when moving the
> first line.  Same comments as earlier about {} and 4-space indentation.
>
> --
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>