On 10/19/2016 07:27 AM, 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ý <tgole...@redhat.com>
> ---
> block/raw_bsd.c | 168
> ++++++++++++++++++++++++++++++++++++++++++++++++++-
> qapi/block-core.json | 16 ++++-
> 2 files changed, 180 insertions(+), 4 deletions(-)
>
> diff --git a/block/raw_bsd.c b/block/raw_bsd.c
> index 588d408..25b5ba8 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;
This seems like it is duplicating a struct that QAPI should be able to
give us for free, if we would just use it.
> + /* 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);
No trailing '.' in error_setg() error messages. Also, most uses of
PRIu64 and PRId64 are separated by spaces from the rest of the "",
rather than adjacent.
> + 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)) {
Better would be:
if (!QEMU_IS_ALIGNED(s->size, BDRV_SECTOR_SIZE)) {
> + s->size = QEMU_ALIGN_DOWN(s->size, BDRV_SECTOR_SIZE);
Dead assignment, since...
> + error_setg(errp, "Specified size is not multiple of %llu!",
> + BDRV_SECTOR_SIZE);
> + ret = -EINVAL;
> + goto fail;
...you are failing anyways.
> + }
> +
> + ret = 0;
> +
> +fail:
Naming a label fail: when it is intended to be reached by fallthrough on
the success path is annoying. end: might be a better name.
> +
> + qemu_opts_del(opts);
> +
> + return ret;
> +}
> +
> @@ -117,8 +239,10 @@ static int64_t coroutine_fn
> raw_co_get_block_status(BlockDriverState *bs,
> int nb_sectors, int *pnum,
> BlockDriverState **file)
> {
> + BDRVRawState *s = bs->opaque;
> *pnum = nb_sectors;
> *file = bs->file->bs;
> + sector_num += s->offset / BDRV_SECTOR_SIZE;
Not your fault (nor necessarily for you to fix), but we should really
switch block status code to be byte-based.
> return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DATA |
> (sector_num << BDRV_SECTOR_BITS);
> }
> @@ -138,7 +262,28 @@ static int coroutine_fn raw_co_pdiscard(BlockDriverState
> *bs,
>
> static int64_t raw_getlength(BlockDriverState *bs)
> {
> - return bdrv_getlength(bs->file->bs);
> + int64_t len;
> + BDRVRawState *s = bs->opaque;
> +
> + /* Update size. It should not change unles the file was externaly
s/unles/unless/
s/externaly/externally/
If the file is being externally modified, the user deserves broken behavior.
> +++ b/qapi/block-core.json
> @@ -2224,6 +2224,20 @@
> 'data': { 'filename': 'str' } }
>
> ##
> +# @BlockdevOptionsRaw
> +#
> +# Driver specific block device options for the raw driver.
> +#
> +# @offset: #optional position where the block device starts
> +# @size: #optional the assumed size of the device
> +#
> +# Since: 2.8
> +##
> +{ 'struct': 'BlockdevOptionsRaw',
> + 'base': 'BlockdevOptionsGenericFormat',
> + 'data': { 'offset': 'int', 'size': 'int' } }
Umm, if you want these to actually be optional, you have to spell them
'*offset' and '*size'.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
