[Qemu-devel] [PATCH] Don't require encryption password for 'qemu-img info' command

[Qemu-devel] [PATCH] Don't require encryption password for 'qemu-img info' command

[Daniel P. Berrange]

From: "Daniel P. Berrange" <berrange@redhat.com>

The encryption password is only required if I/O is going to be
performed on a disk image. The 'qemu-img info' command merely
reports metadata, so it should not ask for a decryption password

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 qemu-img.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index b41e670..3dd4ec7 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -221,7 +221,8 @@ static int print_block_option_help(const char *filename, const char *fmt)
 
 static BlockDriverState *bdrv_new_open(const char *filename,
                                        const char *fmt,
-                                       int flags)
+                                       int flags,
+                                       int requireIO)
 {
     BlockDriverState *bs;
     BlockDriver *drv;
@@ -246,7 +247,7 @@ static BlockDriverState *bdrv_new_open(const char *filename,
         goto fail;
     }
 
-    if (bdrv_is_encrypted(bs)) {
+    if (bdrv_is_encrypted(bs) && requireIO) {
         printf("Disk image '%s' is encrypted.\n", filename);
         if (read_password(password, sizeof(password)) < 0) {
             error_report("No password given");
@@ -413,7 +414,7 @@ static int img_check(int argc, char **argv)
     }
     filename = argv[optind++];
 
-    bs = bdrv_new_open(filename, fmt, flags);
+    bs = bdrv_new_open(filename, fmt, flags, 1);
     if (!bs) {
         return 1;
     }
@@ -520,7 +521,7 @@ static int img_commit(int argc, char **argv)
         return -1;
     }
 
-    bs = bdrv_new_open(filename, fmt, flags);
+    bs = bdrv_new_open(filename, fmt, flags, 1);
     if (!bs) {
         return 1;
     }
@@ -762,7 +763,7 @@ static int img_convert(int argc, char **argv)
 
     total_sectors = 0;
     for (bs_i = 0; bs_i < bs_n; bs_i++) {
-        bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt, BDRV_O_FLAGS);
+        bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt, BDRV_O_FLAGS, 1);
         if (!bs[bs_i]) {
             error_report("Could not open '%s'", argv[optind + bs_i]);
             ret = -1;
@@ -881,7 +882,7 @@ static int img_convert(int argc, char **argv)
         return -1;
     }
 
-    out_bs = bdrv_new_open(out_filename, out_fmt, flags);
+    out_bs = bdrv_new_open(out_filename, out_fmt, flags, 1);
     if (!out_bs) {
         ret = -1;
         goto out;
@@ -1135,7 +1136,7 @@ static int img_info(int argc, char **argv)
     }
     filename = argv[optind++];
 
-    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING);
+    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING, 0);
     if (!bs) {
         return 1;
     }
@@ -1248,7 +1249,7 @@ static int img_snapshot(int argc, char **argv)
     filename = argv[optind++];
 
     /* Open the image */
-    bs = bdrv_new_open(filename, NULL, bdrv_oflags);
+    bs = bdrv_new_open(filename, NULL, bdrv_oflags, 1);
     if (!bs) {
         return 1;
     }
@@ -1366,7 +1367,7 @@ static int img_rebase(int argc, char **argv)
      * Ignore the old backing file for unsafe rebase in case we want to correct
      * the reference to a renamed or moved backing file.
      */
-    bs = bdrv_new_open(filename, fmt, flags);
+    bs = bdrv_new_open(filename, fmt, flags, 1);
     if (!bs) {
         return 1;
     }
@@ -1639,7 +1640,7 @@ static int img_resize(int argc, char **argv)
     n = qemu_opt_get_size(param, BLOCK_OPT_SIZE, 0);
     qemu_opts_del(param);
 
-    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR);
+    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR, 1);
     if (!bs) {
         ret = -1;
         goto out;
-- 
1.7.11.2

Re: [Qemu-devel] [PATCH] Don't require encryption password for 'qemu-img info' command

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1365 bytes --]

On 08/31/2012 10:26 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@redhat.com>
> 
> The encryption password is only required if I/O is going to be
> performed on a disk image. The 'qemu-img info' command merely
> reports metadata, so it should not ask for a decryption password
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  qemu-img.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index b41e670..3dd4ec7 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -221,7 +221,8 @@ static int print_block_option_help(const char *filename, const char *fmt)
>  
>  static BlockDriverState *bdrv_new_open(const char *filename,
>                                         const char *fmt,
> -                                       int flags)
> +                                       int flags,
> +                                       int requireIO)

Since you only pass 0 or 1 for requireIO, why not make it bool?  For
that matter, why not make it part of 'flags', where the default is to
require decryption, and a new flag BDV_O_NO_IO can be added when opening
an image for no I/O and used to avoid the decryption?

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]

Re: [Qemu-devel] [PATCH] Don't require encryption password for 'qemu-img info' command

[Kevin Wolf]

Am 31.08.2012 19:30, schrieb Eric Blake:
> On 08/31/2012 10:26 AM, Daniel P. Berrange wrote:
>> From: "Daniel P. Berrange" <berrange@redhat.com>
>>
>> The encryption password is only required if I/O is going to be
>> performed on a disk image. The 'qemu-img info' command merely
>> reports metadata, so it should not ask for a decryption password
>>
>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>> ---
>>  qemu-img.c | 21 +++++++++++----------
>>  1 file changed, 11 insertions(+), 10 deletions(-)
>>
>> diff --git a/qemu-img.c b/qemu-img.c
>> index b41e670..3dd4ec7 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -221,7 +221,8 @@ static int print_block_option_help(const char *filename, const char *fmt)
>>  
>>  static BlockDriverState *bdrv_new_open(const char *filename,
>>                                         const char *fmt,
>> -                                       int flags)
>> +                                       int flags,
>> +                                       int requireIO)
> 
> Since you only pass 0 or 1 for requireIO, why not make it bool? 

Yes, please. bool require_io, to be precise. The qemu coding style
doesn't use camelCase here.

> For
> that matter, why not make it part of 'flags', where the default is to
> require decryption, and a new flag BDV_O_NO_IO can be added when opening
> an image for no I/O and used to avoid the decryption?

Because the block layer isn't really interested in it, this should be
kept inside qemu-img.

Kevin

Re: [Qemu-devel] [PATCH] Don't require encryption password for 'qemu-img info' command

[Kevin Wolf]

Am 04.09.2012 16:23, schrieb Kevin Wolf:
> Am 31.08.2012 19:30, schrieb Eric Blake:
>> On 08/31/2012 10:26 AM, Daniel P. Berrange wrote:
>>> From: "Daniel P. Berrange" <berrange@redhat.com>
>>>
>>> The encryption password is only required if I/O is going to be
>>> performed on a disk image. The 'qemu-img info' command merely
>>> reports metadata, so it should not ask for a decryption password
>>>
>>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>>> ---
>>>  qemu-img.c | 21 +++++++++++----------
>>>  1 file changed, 11 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/qemu-img.c b/qemu-img.c
>>> index b41e670..3dd4ec7 100644
>>> --- a/qemu-img.c
>>> +++ b/qemu-img.c
>>> @@ -221,7 +221,8 @@ static int print_block_option_help(const char *filename, const char *fmt)
>>>  
>>>  static BlockDriverState *bdrv_new_open(const char *filename,
>>>                                         const char *fmt,
>>> -                                       int flags)
>>> +                                       int flags,
>>> +                                       int requireIO)
>>
>> Since you only pass 0 or 1 for requireIO, why not make it bool? 
> 
> Yes, please. bool require_io, to be precise. The qemu coding style
> doesn't use camelCase here.
> 
>> For
>> that matter, why not make it part of 'flags', where the default is to
>> require decryption, and a new flag BDV_O_NO_IO can be added when opening
>> an image for no I/O and used to avoid the decryption?
> 
> Because the block layer isn't really interested in it, this should be
> kept inside qemu-img.

Oh, Dan, seems you've been dropped from the CC list somehow. Are you
going to send a v2?

Kevin

Re: [Qemu-devel] [PATCH] Don't require encryption password for 'qemu-img info' command

[Daniel P. Berrange]

On Mon, Sep 10, 2012 at 12:44:52PM +0200, Kevin Wolf wrote:
> Am 04.09.2012 16:23, schrieb Kevin Wolf:
> > Am 31.08.2012 19:30, schrieb Eric Blake:
> >> On 08/31/2012 10:26 AM, Daniel P. Berrange wrote:
> >>> From: "Daniel P. Berrange" <berrange@redhat.com>
> >>>
> >>> The encryption password is only required if I/O is going to be
> >>> performed on a disk image. The 'qemu-img info' command merely
> >>> reports metadata, so it should not ask for a decryption password
> >>>
> >>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> >>> ---
> >>>  qemu-img.c | 21 +++++++++++----------
> >>>  1 file changed, 11 insertions(+), 10 deletions(-)
> >>>
> >>> diff --git a/qemu-img.c b/qemu-img.c
> >>> index b41e670..3dd4ec7 100644
> >>> --- a/qemu-img.c
> >>> +++ b/qemu-img.c
> >>> @@ -221,7 +221,8 @@ static int print_block_option_help(const char *filename, const char *fmt)
> >>>  
> >>>  static BlockDriverState *bdrv_new_open(const char *filename,
> >>>                                         const char *fmt,
> >>> -                                       int flags)
> >>> +                                       int flags,
> >>> +                                       int requireIO)
> >>
> >> Since you only pass 0 or 1 for requireIO, why not make it bool? 
> > 
> > Yes, please. bool require_io, to be precise. The qemu coding style
> > doesn't use camelCase here.
> > 
> >> For
> >> that matter, why not make it part of 'flags', where the default is to
> >> require decryption, and a new flag BDV_O_NO_IO can be added when opening
> >> an image for no I/O and used to avoid the decryption?
> > 
> > Because the block layer isn't really interested in it, this should be
> > kept inside qemu-img.
> 
> Oh, Dan, seems you've been dropped from the CC list somehow. Are you
> going to send a v2?

Yes, I'll send an update soon.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|