Re: [Qemu-devel] [PATCH v2] raw_bsd: add offset and size options

["Daniel P. Berrange" <berrange@redhat.com>]

On Tue, Oct 18, 2016 at 12:25:17AM +0200, Tomáš Golembiovský wrote:
> Added two new options 'offset' and 'size'. This makes it possible to use
> only part of the file as a device. This can be used e.g. to limit the
> access only to single partition in a disk image or use a disk inside a
> tar archive (like OVA).
> 
> When 'size' is specified we do our best to honour it.
> 
> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> ---
>  block/raw_bsd.c      | 169 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  qapi/block-core.json |  16 ++++-
>  2 files changed, 181 insertions(+), 4 deletions(-)
> 
> diff --git a/block/raw_bsd.c b/block/raw_bsd.c
> index 588d408..3fb3f13 100644
> --- a/block/raw_bsd.c
> +++ b/block/raw_bsd.c
> @@ -31,6 +31,30 @@
>  #include "qapi/error.h"
>  #include "qemu/option.h"
>  
> +typedef struct BDRVRawState {
> +    uint64_t offset;
> +    uint64_t size;
> +    bool has_size;
> +} BDRVRawState;
> +
> +static QemuOptsList raw_runtime_opts = {
> +    .name = "raw",
> +    .head = QTAILQ_HEAD_INITIALIZER(raw_runtime_opts.head),
> +    .desc = {
> +        {
> +            .name = "offset",
> +            .type = QEMU_OPT_SIZE,
> +            .help = "offset in the disk where the image starts",
> +        },
> +        {
> +            .name = "size",
> +            .type = QEMU_OPT_SIZE,
> +            .help = "virtual disk size",
> +        },
> +        { /* end of list */ }
> +    },
> +};
> +
>  static QemuOptsList raw_create_opts = {
>      .name = "raw-create-opts",
>      .head = QTAILQ_HEAD_INITIALIZER(raw_create_opts.head),
> @@ -44,17 +68,107 @@ static QemuOptsList raw_create_opts = {
>      }
>  };
>  
> +static int raw_read_options(QDict *options, BlockDriverState *bs,
> +    BDRVRawState *s, Error **errp)
> +{
> +    Error *local_err = NULL;
> +    QemuOpts *opts = NULL;
> +    int64_t real_size = 0;
> +    int ret;
> +
> +    real_size = bdrv_getlength(bs->file->bs);
> +    if (real_size < 0) {
> +        error_setg_errno(errp, -real_size, "Could not get image size");
> +        return real_size;
> +    }
> +
> +    opts = qemu_opts_create(&raw_runtime_opts, NULL, 0, &error_abort);
> +    qemu_opts_absorb_qdict(opts, options, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        ret = -EINVAL;
> +        goto fail;
> +    }
> +
> +    s->offset = qemu_opt_get_size(opts, "offset", 0);
> +    if (qemu_opt_find(opts, "size") != NULL) {
> +        s->size = qemu_opt_get_size(opts, "size", 0);
> +        s->has_size = true;
> +    } else {
> +        s->has_size = false;
> +        s->size = real_size;
> +    }
> +
> +    /* Check size and offset */
> +    if (real_size < s->offset || (real_size - s->offset) < s->size) {
> +        error_setg(errp, "The sum of offset (%"PRIu64") and size "
> +            "(%"PRIu64") has to be smaller or equal to the "
> +            " actual size of the containing file (%"PRId64").",
> +            s->offset, s->size, real_size);
> +        ret = -EINVAL;
> +        goto fail;
> +    }
> +
> +    /* Make sure size is multiple of BDRV_SECTOR_SIZE to prevent rounding
> +     * up and leaking out of the specified area. */
> +    if (s->size != QEMU_ALIGN_DOWN(s->size, BDRV_SECTOR_SIZE)) {
> +        s->size = QEMU_ALIGN_DOWN(s->size, BDRV_SECTOR_SIZE);
> +        fprintf(stderr,
> +            "WARNING: Specified size is not multiple of %llu! "
> +            "Rounding down to %"PRIu64 ". (End of the image will be "
> +            "ignored.)\n",
> +            BDRV_SECTOR_SIZE, s->size);

IMHO you should be using error_setg here - I don't see a compelling
reason to let execution continue with incorrect values set.


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