| Message ID | 1346434010-14801-1-git-send-email-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
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?
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
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
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
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;