Patchwork [2/3] qemu-nbd: Extend read-only option to nbd device file

login
register
mail settings
Submitter Ryota OZAKI
Date March 28, 2010, 5:07 p.m.
Message ID <1269796032-9166-2-git-send-email-ozaki.ryota@gmail.com>
Download mbox | patch
Permalink /patch/48784/
State New
Headers show

Comments

Ryota OZAKI - March 28, 2010, 5:07 p.m.
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 <ozaki.ryota@gmail.com>
---
 qemu-nbd.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)
Kevin Wolf - March 29, 2010, 10:54 a.m.
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 <ozaki.ryota@gmail.com>

The help for -r should be changed, too. Currently it says:

-r, --read-only      export read-only

> ---
>  qemu-nbd.c |   10 +++++-----
>  1 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,
>      return -1;
>  }
>  
> -static void show_parts(const char *device)
> +static void show_parts(const char *device, bool readonly)
>  {
>      if (fork() == 0) {
>          int nbd;
> @@ -172,7 +172,7 @@ static void show_parts(const char *device)
>           * but remember to load the module with max_part != 0 :
>           *     modprobe nbd max_part=63
>           */
> -        nbd = open(device, O_RDWR);
> +        nbd = open(device, readonly ? O_RDONLY : O_RDWR);
>          if (nbd != -1) {
>                close(nbd);
>          }

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.

Kevin
Ryota OZAKI - May 2, 2010, 9:51 p.m.
On Mon, Mar 29, 2010 at 7:54 PM, Kevin Wolf <kwolf@redhat.com> 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 <ozaki.ryota@gmail.com>
>
> The help for -r should be changed, too. Currently it says:
>
> -r, --read-only      export read-only

Indeed.

>
>> ---
>>  qemu-nbd.c |   10 +++++-----
>>  1 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,
>>      return -1;
>>  }
>>
>> -static void show_parts(const char *device)
>> +static void show_parts(const char *device, bool readonly)
>>  {
>>      if (fork() == 0) {
>>          int nbd;
>> @@ -172,7 +172,7 @@ static void show_parts(const char *device)
>>           * but remember to load the module with max_part != 0 :
>>           *     modprobe nbd max_part=63
>>           */
>> -        nbd = open(device, O_RDWR);
>> +        nbd = open(device, readonly ? O_RDONLY : O_RDWR);
>>          if (nbd != -1) {
>>                close(nbd);
>>          }
>
> 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
>

Patch

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,
     return -1;
 }
 
-static void show_parts(const char *device)
+static void show_parts(const char *device, bool readonly)
 {
     if (fork() == 0) {
         int nbd;
@@ -172,7 +172,7 @@  static void show_parts(const char *device)
          * but remember to load the module with max_part != 0 :
          *     modprobe nbd max_part=63
          */
-        nbd = open(device, O_RDWR);
+        nbd = open(device, readonly ? O_RDONLY : O_RDWR);
         if (nbd != -1) {
               close(nbd);
         }
@@ -322,7 +322,7 @@  int main(int argc, char **argv)
     }
 
     if (disconnect) {
-        fd = open(argv[optind], O_RDWR);
+        fd = open(argv[optind], readonly ? O_RDONLY : O_RDWR);
         if (fd == -1) {
             errx(EXIT_FAILURE, "Cannot open %s", argv[optind]);
         }
@@ -392,7 +392,7 @@  int main(int argc, char **argv)
                 }
             } while (sock == -1);
 
-            fd = open(device, O_RDWR);
+            fd = open(device, readonly ? O_RDONLY : O_RDWR);
             if (fd == -1) {
                 ret = 1;
                 goto out;
@@ -415,7 +415,7 @@  int main(int argc, char **argv)
 
 	    /* update partition table */
 
-            show_parts(device);
+            show_parts(device, readonly);
 
             nbd_client(fd, sock);
             close(fd);