Re: [Qemu-devel] [PATCH 2/3] qemu-nbd: Extend read-only option to nbd device file

[Ryota Ozaki <ozaki.ryota@gmail.com>]

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
>