From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33134) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XjUyz-0000lr-O7 for qemu-devel@nongnu.org; Wed, 29 Oct 2014 11:16:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XjUyv-0006DQ-7N for qemu-devel@nongnu.org; Wed, 29 Oct 2014 11:16:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54292) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XjUyu-0006DE-Vk for qemu-devel@nongnu.org; Wed, 29 Oct 2014 11:16:17 -0400 Message-ID: <545104BD.4060209@redhat.com> Date: Wed, 29 Oct 2014 09:16:13 -0600 From: Eric Blake MIME-Version: 1.0 References: <5448180A.8060403@redhat.com> <1414553785-23571-1-git-send-email-chaoseternal@gmail.com> <1414553785-23571-2-git-send-email-chaoseternal@gmail.com> In-Reply-To: <1414553785-23571-2-git-send-email-chaoseternal@gmail.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="03jXLTaMdnBrc6K7Qo0wlE2DKqe7MTUsG" Subject: Re: [Qemu-devel] [PATCH] inetd enabled qemu-nbd List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jun Sheng , qemu-devel@nongnu.org Cc: Chaos Eternal This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --03jXLTaMdnBrc6K7Qo0wlE2DKqe7MTUsG Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/28/2014 09:36 PM, Jun Sheng wrote: > From: Chaos Eternal >=20 >=20 > Signed-off-by: Jun Sheng 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(-) >=20 > 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 =3D -1; > static int persistent =3D 0; > static enum { RUNNING, TERMINATE, TERMINATING, TERMINATED } state; > static int shared =3D 1; > @@ -66,6 +67,7 @@ static void usage(const char *name) > "Connection properties:\n" > " -p, --port=3DPORT port to listen on (default `%d')\n" > " -b, --bind=3DIFACE interface to bind to (default `0.0.0.0'= )\n" > +" -i, --inetd=3DFD run as an inetd services on FD\n" s/services/service/ > " -k, --socket=3DPATH path to the unix socket\n" > " (default '"SOCKET_PATH"')\n" > " -e, --shared=3DNUM device can be shared by NUM clients (de= fault '1')\n" > @@ -363,7 +365,13 @@ static void nbd_accept(void *opaque) > struct sockaddr_in addr; > socklen_t addr_len =3D sizeof(addr); > =20 > - int fd =3D accept(server_fd, (struct sockaddr *)&addr, &addr_len);= > + int fd ; No space before ';' > + if (inetd_fd < 0) { > + fd =3D accept(server_fd, (struct sockaddr *)&addr, &addr_len);= > + } else { > + fd =3D 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 =3D NULL; > const char *sn_id_or_name =3D NULL; > - const char *sopt =3D "hVb:o:p:rsnP:c:dvk:e:f:tl:"; > + const char *sopt =3D "hVi:b:o:p:rsnP:c:dvk:e:f:tl:"; Here, you list 'i' before 'b'... > struct option lopt[] =3D { > { "help", 0, NULL, 'h' }, > { "version", 0, NULL, 'V' }, > + { "inetd", 1, NULL, 'i'}, > { "bind", 1, NULL, 'b' }, =2E..here too... > { "port", 1, NULL, 'p' }, > { "socket", 1, NULL, 'k' }, > @@ -510,6 +519,16 @@ int main(int argc, char **argv) > case 'b': > bindto =3D optarg; > break; > + case 'i': =2E..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 =3D 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). --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --03jXLTaMdnBrc6K7Qo0wlE2DKqe7MTUsG Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg iQEcBAEBCAAGBQJUUQS9AAoJEKeha0olJ0NqTEoH/AxK6jOB/KwRmiqY97NbGhbO IDt9DtaM59HBfiGm3S4DLCdx2rtl+oAKuYMoJdPT6wNaO/ZHWvcDVcndns8OF6U6 hiY5i0LCLvvRZD3BHLg/ZtEUX3AZoG5kp4MjENC+9PK426Iybs4jIJ364z0WrC5c 2R1D13L6/wQR4W0rjwKaTM/Qx6UJccDZCcvVpK67lmstwG1+AEKfRyJqhNSdRO6r EFudSfL5yzC/oH7iW/cLkY7SpudkghaOeecvenhbeMzz4QyFtbTlAhIJiVf9YTJ2 JSV9Q5EipKIO5vS3y7I9Tfja4MHsn/Q8X4GOXKen2ObavcjY5sMbZg/GdpW+YB0= =XxoD -----END PGP SIGNATURE----- --03jXLTaMdnBrc6K7Qo0wlE2DKqe7MTUsG--