* [Qemu-devel] A Patch to enable qemu-nbd run as an inetd service
@ 2014-10-22 6:41 Jun Sheng
2014-10-22 15:29 ` Eric Blake
0 siblings, 1 reply; 5+ messages in thread
From: Jun Sheng @ 2014-10-22 6:41 UTC (permalink / raw)
To: qemu-devel
[-- 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 */
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Qemu-devel] A Patch to enable qemu-nbd run as an inetd service
2014-10-22 6:41 [Qemu-devel] A Patch to enable qemu-nbd run as an inetd service Jun Sheng
@ 2014-10-22 15:29 ` Eric Blake
0 siblings, 0 replies; 5+ messages in thread
From: Eric Blake @ 2014-10-22 15:29 UTC (permalink / raw)
To: Jun Sheng, qemu-devel
[-- 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 --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] A Patch to enable qemu-nbd run as an inetd service
@ 2014-10-22 18:54 Jun Sheng
2014-10-22 20:48 ` Eric Blake
0 siblings, 1 reply; 5+ messages in thread
From: Jun Sheng @ 2014-10-22 18:54 UTC (permalink / raw)
To: qemu-devel; +Cc: Chaos Eternal, 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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Qemu-devel] A Patch to enable qemu-nbd run as an inetd service
2014-10-22 18:54 Jun Sheng
@ 2014-10-22 20:48 ` Eric Blake
2014-10-27 12:54 ` Jun Sheng
0 siblings, 1 reply; 5+ messages in thread
From: Eric Blake @ 2014-10-22 20:48 UTC (permalink / raw)
To: Jun Sheng, qemu-devel; +Cc: Chaos Eternal
[-- 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 --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] A Patch to enable qemu-nbd run as an inetd service
2014-10-22 20:48 ` Eric Blake
@ 2014-10-27 12:54 ` Jun Sheng
0 siblings, 0 replies; 5+ messages in thread
From: Jun Sheng @ 2014-10-27 12:54 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-10-27 12:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-22 6:41 [Qemu-devel] A Patch to enable qemu-nbd run as an inetd service Jun Sheng
2014-10-22 15:29 ` Eric Blake
-- strict thread matches above, loose matches on Subject: below --
2014-10-22 18:54 Jun Sheng
2014-10-22 20:48 ` Eric Blake
2014-10-27 12:54 ` Jun Sheng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).