Re: [Qemu-devel] [PATCH RFC 1/2] block: Limit opening of encrypted images to qemu-img

[Kevin Wolf <kwolf@redhat.com>]

Am 11.03.2015 um 12:59 hat Markus Armbruster geschrieben:
> Kevin Wolf <kwolf@redhat.com> writes:
> 
> > Am 10.03.2015 um 18:26 hat Markus Armbruster geschrieben:
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> ---
> >>  block/qcow.c          | 5 +++++
> >>  block/qcow2.c         | 5 +++++
> >>  include/block/block.h | 3 +--
> >>  qemu-img.c            | 1 +
> >>  4 files changed, 12 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/block/qcow.c b/block/qcow.c
> >> index 0558969..f54fc86 100644
> >> --- a/block/qcow.c
> >> +++ b/block/qcow.c
> >> @@ -155,6 +155,11 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
> >>      }
> >>      s->crypt_method_header = header.crypt_method;
> >>      if (s->crypt_method_header) {
> >> +        if (!(flags & BDRV_O_CRYPT_OK)) {
> >> +            error_setg(errp, "image is encrypted, use qemu-img to decrypt it");
> >> +            ret = -EINVAL;
> >> +            goto fail;
> >> +        }
> >>          bs->encrypted = 1;
> >>      }
> >>      s->cluster_bits = header.cluster_bits;
> >> diff --git a/block/qcow2.c b/block/qcow2.c
> >> index 50e0a94..b12c67a 100644
> >> --- a/block/qcow2.c
> >> +++ b/block/qcow2.c
> >> @@ -693,6 +693,11 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
> >>      }
> >>      s->crypt_method_header = header.crypt_method;
> >>      if (s->crypt_method_header) {
> >> +        if (!(flags & BDRV_O_CRYPT_OK)) {
> >> +            error_setg(errp, "image is encrypted, use qemu-img to decrypt it");
> >> +            ret = -EINVAL;
> >> +            goto fail;
> >> +        }
> >>          bs->encrypted = 1;
> >>      }
> >
> > Can we do this in bdrv_open_common() instead of duplicating the code in
> > two drivers?
> 
> I looked for a better place to do this, but couldn't find one that
> *obviously* dominates the bdrv_open() methods, so I put it into the
> block drivers.
> 
> Can you explain why bdrv_open_common() cannot be bypassed?

Because it's the only place in qemu that calls drv->bdrv_open() and
drv->bdrv_file_open(). (Except for bdrv_snapshot_goto(), which does a
close/open on an already opened image.)

Everything else is just a - potentially indirect - caller of
bdrv_open_common().

Kevin