[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
>

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

[Jun Sheng]

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-devel] [PATCH] inetd enabled qemu-nbd

[Jun Sheng]

From: Chaos Eternal <chaos@shlug.org>


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

diff --git a/qemu-nbd.c b/qemu-nbd.c
index b524b34..44bc349 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 inetd_fd = -1;
 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 (inetd_fd < 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' },
@@ -510,6 +519,16 @@ int main(int argc, char **argv)
         case 'b':
             bindto = optarg;
             break;
+        case 'i':
+            inetd_fd = strtol(optarg, &end, 10);
+            if (*end) {
+                errx(EXIT_FAILURE, "Invalid inet fd `%s'", optarg);
+            }
+            if (inetd_fd < STDERR_FILENO) {
+                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) {
@@ -730,10 +749,14 @@ int main(int argc, char **argv)
 
     exp = nbd_export_new(bs, dev_offset, fd_size, nbdflags, nbd_export_closed);
 
-    if (sockpath) {
-        fd = unix_socket_incoming(sockpath);
+    if (inetd_fd < 0) {
+        if (sockpath) {
+            fd = unix_socket_incoming(sockpath);
+        } else {
+            fd = tcp_socket_incoming(bindto, port);
+        }
     } else {
-        fd = tcp_socket_incoming(bindto, port);
+        fd = inetd_fd;
     }
 
     if (fd < 0) {
@@ -752,9 +775,12 @@ 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,
-                         (void *)(uintptr_t)fd);
+    if (inetd_fd < 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] [PATCH] inetd enabled qemu-nbd

[Eric Blake]

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

On 10/28/2014 09:36 PM, Jun Sheng wrote:
> From: Chaos Eternal <chaos@shlug.org>
> 
> 
> Signed-off-by: Jun Sheng <chaoseternal@gmail.com>

Using a pseudonym for the git authorship (the From: line) is unusual
(but not unheard of in this project).  What is weirder is that your
S-o-B uses a real name; if you don't mind exposing your real name, then
why not use it everywhere?

Your commit is missing the body that you sent in the previous message
(<1414553785-23571-1-git-send-email-chaoseternal@gmail.com>).

When sending an updated version of a patch, use 'git send-email -v2' to
include a v2 in the subject line.  Also, it is better to send your
revision as a brand new thread rather than buried in-reply-to an
existing thread.

> ---
>  qemu-nbd.c | 42 ++++++++++++++++++++++++++++++++++--------
>  1 file changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index b524b34..44bc349 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 inetd_fd = -1;
>  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"

s/services/service/

>  "  -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 ;

No space before ';'

> +    if (inetd_fd < 0) {
> +        fd = accept(server_fd, (struct sockaddr *)&addr, &addr_len);
> +    } else {
> +      fd = server_fd;

Indentation is off.

> +    }
> +
>      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:";

Here, you list 'i' before 'b'...

>      struct option lopt[] = {
>          { "help", 0, NULL, 'h' },
>          { "version", 0, NULL, 'V' },
> +        { "inetd", 1, NULL, 'i'},
>          { "bind", 1, NULL, 'b' },

...here too...

>          { "port", 1, NULL, 'p' },
>          { "socket", 1, NULL, 'k' },
> @@ -510,6 +519,16 @@ int main(int argc, char **argv)
>          case 'b':
>              bindto = optarg;
>              break;
> +        case 'i':

...but here you listed in a different order.  I'm a fan of having the
getopt string in the same order as the case statement (a truly OCD
reviewer would want the case statement and therefore the getopt string
alphabetized, maybe allowing for case insensitive sorting - but that
would be a separate cleanup and is not something I demand).

> +            inetd_fd = strtol(optarg, &end, 10);

Improper use of strtol.  You didn't check for errors via errno.  Rather
than open-coding the correct use of strtol (which is surprisingly
difficult for people to do), you should instead reuse one of our
wrappers that already does the job (for example, util/cutils.c includes
qemu_parse_fd that sounds exactly like what you want).

> @@ -752,9 +775,12 @@ 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,
> -                         (void *)(uintptr_t)fd);
> +    if (inetd_fd < 0) {
> +        qemu_set_fd_handler2(fd, nbd_can_accept, nbd_accept, NULL,
> +                             (void *)(uintptr_t)fd);
> +    } else {
> +        nbd_accept((void *) (uintptr_t) fd);

Inconsistent spacing between the two examples of double casting.  (Alas,
this is one place where the compiler balks if you don't have the double
casting, even though C does not strictly require it).

-- 
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 --]