From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NvraD-00050I-E7 for qemu-devel@nongnu.org; Sun, 28 Mar 2010 08:27:13 -0400 Received: from [140.186.70.92] (port=40258 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NvraB-0004zg-Uk for qemu-devel@nongnu.org; Sun, 28 Mar 2010 08:27:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1NvraA-0003Fq-6v for qemu-devel@nongnu.org; Sun, 28 Mar 2010 08:27:11 -0400 Received: from mail-pz0-f194.google.com ([209.85.222.194]:52512) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1NvraA-0003Fg-2R for qemu-devel@nongnu.org; Sun, 28 Mar 2010 08:27:10 -0400 Received: by pzk32 with SMTP id 32so3637122pzk.4 for ; Sun, 28 Mar 2010 05:27:08 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20100327130223.GK15194@volta.aurel32.net> References: <1269066204-4376-1-git-send-email-ozaki.ryota@gmail.com> <1269066204-4376-3-git-send-email-ozaki.ryota@gmail.com> <20100327130223.GK15194@volta.aurel32.net> From: Ryota Ozaki Date: Sun, 28 Mar 2010 21:26:48 +0900 Message-ID: <5e93dcec1003280526t2d6739c3ybe669a22277c3545@mail.gmail.com> Subject: Re: [Qemu-devel] [PATCH 3/3] qemu-nbd: Improve error reporting Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aurelien Jarno Cc: qemu-devel@nongnu.org On Sat, Mar 27, 2010 at 10:02 PM, Aurelien Jarno wro= te: > On Sat, Mar 20, 2010 at 03:23:24PM +0900, Ryota Ozaki wrote: >> - use err(3) instead of errx(3) if errno is available >> =A0 to report why failed >> - let fail prior to daemon(3) if opening a nbd file >> =A0 is likely to fail after daemonizing to avoid silent >> =A0 failure exit >> >> Signed-off-by: Ryota Ozaki >> --- >> =A0qemu-nbd.c | =A0 16 +++++++++++----- >> =A01 files changed, 11 insertions(+), 5 deletions(-) >> >> diff --git a/qemu-nbd.c b/qemu-nbd.c >> index 6d854d3..8fb8cc3 100644 >> --- a/qemu-nbd.c >> +++ b/qemu-nbd.c >> @@ -316,7 +316,7 @@ int main(int argc, char **argv) >> =A0 =A0 =A0if (disconnect) { >> =A0 =A0 =A0 =A0 =A0fd =3D open(argv[optind], O_RDWR); >> =A0 =A0 =A0 =A0 =A0if (fd =3D=3D -1) >> - =A0 =A0 =A0 =A0 =A0 =A0errx(EXIT_FAILURE, "Cannot open %s", argv[optin= d]); >> + =A0 =A0 =A0 =A0 =A0 =A0err(EXIT_FAILURE, "Cannot open %s", argv[optind= ]); >> >> =A0 =A0 =A0 =A0 =A0nbd_disconnect(fd); >> >> @@ -333,23 +333,29 @@ int main(int argc, char **argv) >> =A0 =A0 =A0if (bs =3D=3D NULL) >> =A0 =A0 =A0 =A0 =A0return 1; >> >> - =A0 =A0if (bdrv_open(bs, argv[optind], flags) < 0) >> - =A0 =A0 =A0 =A0return 1; >> + =A0 =A0if ((ret =3D bdrv_open(bs, argv[optind], flags)) < 0) { >> + =A0 =A0 =A0 =A0errno =3D -ret; >> + =A0 =A0 =A0 =A0err(EXIT_FAILURE, "Failed to bdrv_open '%s'", argv[opti= nd]); >> + =A0 =A0} >> >> =A0 =A0 =A0fd_size =3D bs->total_sectors * 512; >> >> =A0 =A0 =A0if (partition !=3D -1 && >> =A0 =A0 =A0 =A0 =A0find_partition(bs, partition, &dev_offset, &fd_size)) >> - =A0 =A0 =A0 =A0errx(EXIT_FAILURE, "Could not find partition %d", parti= tion); >> + =A0 =A0 =A0 =A0err(EXIT_FAILURE, "Could not find partition %d", partit= ion); >> >> =A0 =A0 =A0if (device) { >> =A0 =A0 =A0 =A0 =A0pid_t pid; >> =A0 =A0 =A0 =A0 =A0int sock; >> >> + =A0 =A0 =A0 =A0/* want to fail before daemonizing */ >> + =A0 =A0 =A0 =A0if (access(device, R_OK|W_OK) =3D=3D -1) >> + =A0 =A0 =A0 =A0 =A0 =A0err(EXIT_FAILURE, "Could not access '%s'", devi= ce); >> + > > First of all you need to put this line between curly braces. Secondly, Oh, sorry. > qemu-nbd as a read-only option to export a block device as read-only. > The test has to be improved to also take care of that. I guess the option is intended to be only for disk image, because open(2) for nbd device file doesn't care about the option. Nonetheless, extending the option to operations to nbd device file makes sense, because the option allows to open nbd device file without write permission. So I'll correct it as well as fixing the problems you pointed out. Thanks, ozaki-r > >> =A0 =A0 =A0 =A0 =A0if (!verbose) { >> =A0 =A0 =A0 =A0 =A0 =A0 =A0/* detach client and server */ >> =A0 =A0 =A0 =A0 =A0 =A0 =A0if (daemon(0, 0) =3D=3D -1) { >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0errx(EXIT_FAILURE, "Failed to daemonize= "); >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0err(EXIT_FAILURE, "Failed to daemonize"= ); >> =A0 =A0 =A0 =A0 =A0 =A0 =A0} >> =A0 =A0 =A0 =A0 =A0} >> >> -- >> 1.6.5.2 >> >> >> >> > > -- > Aurelien Jarno =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0GPG: 10= 24D/F1BCDB73 > aurelien@aurel32.net =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 http://www.aurel32.n= et >