From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1O8h4r-0005J0-GC for qemu-devel@nongnu.org; Sun, 02 May 2010 17:51:53 -0400 Received: from [140.186.70.92] (port=55760 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1O8h4q-0005Iq-Co for qemu-devel@nongnu.org; Sun, 02 May 2010 17:51:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1O8h4n-0004Sk-Tn for qemu-devel@nongnu.org; Sun, 02 May 2010 17:51:52 -0400 Received: from mail-ww0-f45.google.com ([74.125.82.45]:37996) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O8h4n-0004Sf-OH for qemu-devel@nongnu.org; Sun, 02 May 2010 17:51:49 -0400 Received: by wwb17 with SMTP id 17so1300359wwb.4 for ; Sun, 02 May 2010 14:51:48 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <4BB08700.90806@redhat.com> References: <1269796032-9166-1-git-send-email-ozaki.ryota@gmail.com> <1269796032-9166-2-git-send-email-ozaki.ryota@gmail.com> <4BB08700.90806@redhat.com> From: Ryota Ozaki Date: Mon, 3 May 2010 06:51:28 +0900 Message-ID: Subject: Re: [Qemu-devel] [PATCH 2/3] qemu-nbd: Extend read-only option to nbd device file 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: Kevin Wolf Cc: qemu-devel@nongnu.org On Mon, Mar 29, 2010 at 7:54 PM, Kevin Wolf wrote: > Am 28.03.2010 19:07, schrieb Ryota Ozaki: >> This patch allows to operate on nbd device file >> without write permission for the file if read-only >> option is specified. >> >> Signed-off-by: Ryota Ozaki > > The help for -r should be changed, too. Currently it says: > > -r, --read-only =A0 =A0 =A0export read-only Indeed. > >> --- >> =A0qemu-nbd.c | =A0 10 +++++----- >> =A01 files changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/qemu-nbd.c b/qemu-nbd.c >> index 00b8896..7ef409f 100644 >> --- a/qemu-nbd.c >> +++ b/qemu-nbd.c >> @@ -162,7 +162,7 @@ static int find_partition(BlockDriverState *bs, int = partition, >> =A0 =A0 =A0return -1; >> =A0} >> >> -static void show_parts(const char *device) >> +static void show_parts(const char *device, bool readonly) >> =A0{ >> =A0 =A0 =A0if (fork() =3D=3D 0) { >> =A0 =A0 =A0 =A0 =A0int nbd; >> @@ -172,7 +172,7 @@ static void show_parts(const char *device) >> =A0 =A0 =A0 =A0 =A0 * but remember to load the module with max_part !=3D= 0 : >> =A0 =A0 =A0 =A0 =A0 * =A0 =A0 modprobe nbd max_part=3D63 >> =A0 =A0 =A0 =A0 =A0 */ >> - =A0 =A0 =A0 =A0nbd =3D open(device, O_RDWR); >> + =A0 =A0 =A0 =A0nbd =3D open(device, readonly ? O_RDONLY : O_RDWR); >> =A0 =A0 =A0 =A0 =A0if (nbd !=3D -1) { >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0close(nbd); >> =A0 =A0 =A0 =A0 =A0} > > Can't we always use O_RDONLY here? Assuming that this is enough to > trigger a partition table update, I haven't tested it. But if it's not > enough, wouldn't be enough for readonly either. You're right. It works always with O_RDONLY. So we can remove the condition expression. However, I will drop this patch because I found I misunderstood about nbd. The patch intended to be for non-root users, but it does not make sense because nbd requires CAP_SYS_ADMIN. So I hold it until I understand well and focus on the last patch at first. I'm sorry for messing up. Thanks, ozaki-r > > Kevin >