* [Qemu-devel] [RFC v2 1/8] block: add bdrv_measure() API
2017-03-15 9:29 [Qemu-devel] [RFC v2 0/8] qemu-img: add measure sub-command Stefan Hajnoczi
@ 2017-03-15 9:29 ` Stefan Hajnoczi
2017-03-16 1:01 ` Max Reitz
` (2 more replies)
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 2/8] raw-format: add bdrv_measure() support Stefan Hajnoczi
` (7 subsequent siblings)
8 siblings, 3 replies; 30+ messages in thread
From: Stefan Hajnoczi @ 2017-03-15 9:29 UTC (permalink / raw)
To: qemu-devel
Cc: Nir Soffer, Kevin Wolf, Maor Lipchuk, Daniel P. Berrange,
Eric Blake, Alberto Garcia, John Snow, Stefan Hajnoczi
bdrv_measure() provides a conservative maximum for the size of a new
image. This information is handy if storage needs to be allocated (e.g.
a SAN or an LVM volume) ahead of time.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
qapi/block-core.json | 19 +++++++++++++++++++
include/block/block.h | 4 ++++
include/block/block_int.h | 2 ++
block.c | 33 +++++++++++++++++++++++++++++++++
4 files changed, 58 insertions(+)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 786b39e..673569d 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -463,6 +463,25 @@
'*dirty-bitmaps': ['BlockDirtyInfo'] } }
##
+# @BlockMeasureInfo:
+#
+# Image size calculation information. This structure describes the size
+# requirements for creating a new image.
+#
+# @required-bytes: Amount of space required for image creation. This value is
+# the host file size including sparse file regions. A new 5
+# GB raw file therefore has a required size of 5 GB, not 0
+# bytes.
+#
+# @fully-allocated-bytes: Space required once data has been written to all
+# sectors
+#
+# Since: 2.10
+##
+{ 'struct': 'BlockMeasureInfo',
+ 'data': {'required-bytes': 'int', 'fully-allocated-bytes': 'int'} }
+
+##
# @query-block:
#
# Get a list of BlockInfo for all virtual block devices.
diff --git a/include/block/block.h b/include/block/block.h
index 5149260..43c789f 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -298,6 +298,10 @@ int bdrv_truncate(BdrvChild *child, int64_t offset);
int64_t bdrv_nb_sectors(BlockDriverState *bs);
int64_t bdrv_getlength(BlockDriverState *bs);
int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
+void bdrv_measure(BlockDriver *drv, QemuOpts *opts,
+ BlockDriverState *in_bs,
+ BlockMeasureInfo *info,
+ Error **errp);
void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
void bdrv_refresh_limits(BlockDriverState *bs, Error **errp);
int bdrv_commit(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 6c699ac..45a7fbe 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -201,6 +201,8 @@ struct BlockDriver {
int64_t (*bdrv_getlength)(BlockDriverState *bs);
bool has_variable_length;
int64_t (*bdrv_get_allocated_file_size)(BlockDriverState *bs);
+ void (*bdrv_measure)(QemuOpts *opts, BlockDriverState *in_bs,
+ BlockMeasureInfo *info, Error **errp);
int coroutine_fn (*bdrv_co_pwritev_compressed)(BlockDriverState *bs,
uint64_t offset, uint64_t bytes, QEMUIOVector *qiov);
diff --git a/block.c b/block.c
index cb57370..532a4d1 100644
--- a/block.c
+++ b/block.c
@@ -3260,6 +3260,39 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)
return -ENOTSUP;
}
+/*
+ * bdrv_measure:
+ * @drv: Format driver
+ * @opts: Creation options
+ * @in_bs: Existing image containing data for new image (may be NULL)
+ * @info: Result object
+ * @errp: Error object
+ *
+ * Calculate file size required to create a new image.
+ *
+ * If @in_bs is given then space for allocated clusters and zero clusters
+ * from that image are included in the calculation. If @opts contains a
+ * backing file that is shared by @in_bs then backing clusters are omitted
+ * from the calculation.
+ *
+ * If @in_bs is NULL then the calculation includes no allocated clusters
+ * unless a preallocation option is given in @opts.
+ *
+ * Note that @in_bs may use a different BlockDriver from @drv.
+ */
+void bdrv_measure(BlockDriver *drv, QemuOpts *opts,
+ BlockDriverState *in_bs, BlockMeasureInfo *info,
+ Error **errp)
+{
+ if (!drv->bdrv_measure) {
+ error_setg(errp, "Block driver '%s' does not support size measurement",
+ drv->format_name);
+ return;
+ }
+
+ drv->bdrv_measure(opts, in_bs, info, errp);
+}
+
/**
* Return number of sectors on success, -errno on error.
*/
--
2.9.3
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [RFC v2 1/8] block: add bdrv_measure() API
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 1/8] block: add bdrv_measure() API Stefan Hajnoczi
@ 2017-03-16 1:01 ` Max Reitz
2017-03-16 3:38 ` Stefan Hajnoczi
2017-03-18 0:51 ` Nir Soffer
2017-03-18 1:28 ` Nir Soffer
2 siblings, 1 reply; 30+ messages in thread
From: Max Reitz @ 2017-03-16 1:01 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Kevin Wolf, John Snow, Nir Soffer, Maor Lipchuk, Alberto Garcia
[-- Attachment #1: Type: text/plain, Size: 5589 bytes --]
On 15.03.2017 10:29, Stefan Hajnoczi wrote:
> bdrv_measure() provides a conservative maximum for the size of a new
> image. This information is handy if storage needs to be allocated (e.g.
> a SAN or an LVM volume) ahead of time.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> qapi/block-core.json | 19 +++++++++++++++++++
> include/block/block.h | 4 ++++
> include/block/block_int.h | 2 ++
> block.c | 33 +++++++++++++++++++++++++++++++++
> 4 files changed, 58 insertions(+)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 786b39e..673569d 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -463,6 +463,25 @@
> '*dirty-bitmaps': ['BlockDirtyInfo'] } }
>
> ##
> +# @BlockMeasureInfo:
> +#
> +# Image size calculation information. This structure describes the size
> +# requirements for creating a new image.
> +#
> +# @required-bytes: Amount of space required for image creation. This value is
> +# the host file size including sparse file regions. A new 5
> +# GB raw file therefore has a required size of 5 GB, not 0
> +# bytes.
This should probably note that it's a conservative estimation (and I
agree that it should be). It's nice to have it in the commit message but
few people are going to run git blame on the QAPI documentation to find
out the rest of its story. :-)
> +#
> +# @fully-allocated-bytes: Space required once data has been written to all
> +# sectors
> +#
> +# Since: 2.10
> +##
> +{ 'struct': 'BlockMeasureInfo',
> + 'data': {'required-bytes': 'int', 'fully-allocated-bytes': 'int'} }
> +
> +##
> # @query-block:
> #
> # Get a list of BlockInfo for all virtual block devices.
> diff --git a/include/block/block.h b/include/block/block.h
> index 5149260..43c789f 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -298,6 +298,10 @@ int bdrv_truncate(BdrvChild *child, int64_t offset);
> int64_t bdrv_nb_sectors(BlockDriverState *bs);
> int64_t bdrv_getlength(BlockDriverState *bs);
> int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
> +void bdrv_measure(BlockDriver *drv, QemuOpts *opts,
> + BlockDriverState *in_bs,
> + BlockMeasureInfo *info,
> + Error **errp);
> void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
> void bdrv_refresh_limits(BlockDriverState *bs, Error **errp);
> int bdrv_commit(BlockDriverState *bs);
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 6c699ac..45a7fbe 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -201,6 +201,8 @@ struct BlockDriver {
> int64_t (*bdrv_getlength)(BlockDriverState *bs);
> bool has_variable_length;
> int64_t (*bdrv_get_allocated_file_size)(BlockDriverState *bs);
> + void (*bdrv_measure)(QemuOpts *opts, BlockDriverState *in_bs,
> + BlockMeasureInfo *info, Error **errp);
>
> int coroutine_fn (*bdrv_co_pwritev_compressed)(BlockDriverState *bs,
> uint64_t offset, uint64_t bytes, QEMUIOVector *qiov);
> diff --git a/block.c b/block.c
> index cb57370..532a4d1 100644
> --- a/block.c
> +++ b/block.c
> @@ -3260,6 +3260,39 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)
> return -ENOTSUP;
> }
>
> +/*
> + * bdrv_measure:
> + * @drv: Format driver
> + * @opts: Creation options
> + * @in_bs: Existing image containing data for new image (may be NULL)
> + * @info: Result object
> + * @errp: Error object
> + *
> + * Calculate file size required to create a new image.
> + *
> + * If @in_bs is given then space for allocated clusters and zero clusters
> + * from that image are included in the calculation. If @opts contains a
> + * backing file that is shared by @in_bs then backing clusters are omitted
> + * from the calculation.
This seems to run a bit contrary to the documentation of
BlockMeasureInfo.required-bytes, and I don't fully understand it either.
What does "space for zero clusters" mean? Do zero clusters take space?
Does it depend on the image format? (i.e. would they take space for raw
but not for qcow2?)
And is space for unallocated clusters included or not? Do unallocated
clusters without a backing image count as zero clusters?
If that space is not included, then it would run contrary to the QAPI
documentation which states that it should be included.
Finally, how are you supposed to check whether the backing file in @opts
is shared by @in_bs?
> + *
> + * If @in_bs is NULL then the calculation includes no allocated clusters
> + * unless a preallocation option is given in @opts.
But the BlockMeasureInfo.required-bytes documentation states that a new
5 GB raw image should still report 5 GB of required space.
Max
> + *
> + * Note that @in_bs may use a different BlockDriver from @drv.
> + */
> +void bdrv_measure(BlockDriver *drv, QemuOpts *opts,
> + BlockDriverState *in_bs, BlockMeasureInfo *info,
> + Error **errp)
> +{
> + if (!drv->bdrv_measure) {
> + error_setg(errp, "Block driver '%s' does not support size measurement",
> + drv->format_name);
> + return;
> + }
> +
> + drv->bdrv_measure(opts, in_bs, info, errp);
> +}
> +
> /**
> * Return number of sectors on success, -errno on error.
> */
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [RFC v2 1/8] block: add bdrv_measure() API
2017-03-16 1:01 ` Max Reitz
@ 2017-03-16 3:38 ` Stefan Hajnoczi
2017-03-16 4:02 ` Max Reitz
0 siblings, 1 reply; 30+ messages in thread
From: Stefan Hajnoczi @ 2017-03-16 3:38 UTC (permalink / raw)
To: Max Reitz
Cc: qemu-devel, Kevin Wolf, John Snow, Nir Soffer, Maor Lipchuk,
Alberto Garcia
[-- Attachment #1: Type: text/plain, Size: 7148 bytes --]
On Thu, Mar 16, 2017 at 02:01:03AM +0100, Max Reitz wrote:
> On 15.03.2017 10:29, Stefan Hajnoczi wrote:
> > bdrv_measure() provides a conservative maximum for the size of a new
> > image. This information is handy if storage needs to be allocated (e.g.
> > a SAN or an LVM volume) ahead of time.
> >
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> > qapi/block-core.json | 19 +++++++++++++++++++
> > include/block/block.h | 4 ++++
> > include/block/block_int.h | 2 ++
> > block.c | 33 +++++++++++++++++++++++++++++++++
> > 4 files changed, 58 insertions(+)
> >
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index 786b39e..673569d 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -463,6 +463,25 @@
> > '*dirty-bitmaps': ['BlockDirtyInfo'] } }
> >
> > ##
> > +# @BlockMeasureInfo:
> > +#
> > +# Image size calculation information. This structure describes the size
> > +# requirements for creating a new image.
> > +#
> > +# @required-bytes: Amount of space required for image creation. This value is
> > +# the host file size including sparse file regions. A new 5
> > +# GB raw file therefore has a required size of 5 GB, not 0
> > +# bytes.
>
> This should probably note that it's a conservative estimation (and I
> agree that it should be). It's nice to have it in the commit message but
> few people are going to run git blame on the QAPI documentation to find
> out the rest of its story. :-)
Will fix.
> > +#
> > +# @fully-allocated-bytes: Space required once data has been written to all
> > +# sectors
> > +#
> > +# Since: 2.10
> > +##
> > +{ 'struct': 'BlockMeasureInfo',
> > + 'data': {'required-bytes': 'int', 'fully-allocated-bytes': 'int'} }
> > +
> > +##
> > # @query-block:
> > #
> > # Get a list of BlockInfo for all virtual block devices.
> > diff --git a/include/block/block.h b/include/block/block.h
> > index 5149260..43c789f 100644
> > --- a/include/block/block.h
> > +++ b/include/block/block.h
> > @@ -298,6 +298,10 @@ int bdrv_truncate(BdrvChild *child, int64_t offset);
> > int64_t bdrv_nb_sectors(BlockDriverState *bs);
> > int64_t bdrv_getlength(BlockDriverState *bs);
> > int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
> > +void bdrv_measure(BlockDriver *drv, QemuOpts *opts,
> > + BlockDriverState *in_bs,
> > + BlockMeasureInfo *info,
> > + Error **errp);
> > void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
> > void bdrv_refresh_limits(BlockDriverState *bs, Error **errp);
> > int bdrv_commit(BlockDriverState *bs);
> > diff --git a/include/block/block_int.h b/include/block/block_int.h
> > index 6c699ac..45a7fbe 100644
> > --- a/include/block/block_int.h
> > +++ b/include/block/block_int.h
> > @@ -201,6 +201,8 @@ struct BlockDriver {
> > int64_t (*bdrv_getlength)(BlockDriverState *bs);
> > bool has_variable_length;
> > int64_t (*bdrv_get_allocated_file_size)(BlockDriverState *bs);
> > + void (*bdrv_measure)(QemuOpts *opts, BlockDriverState *in_bs,
> > + BlockMeasureInfo *info, Error **errp);
> >
> > int coroutine_fn (*bdrv_co_pwritev_compressed)(BlockDriverState *bs,
> > uint64_t offset, uint64_t bytes, QEMUIOVector *qiov);
> > diff --git a/block.c b/block.c
> > index cb57370..532a4d1 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -3260,6 +3260,39 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)
> > return -ENOTSUP;
> > }
> >
> > +/*
> > + * bdrv_measure:
> > + * @drv: Format driver
> > + * @opts: Creation options
> > + * @in_bs: Existing image containing data for new image (may be NULL)
> > + * @info: Result object
> > + * @errp: Error object
> > + *
> > + * Calculate file size required to create a new image.
> > + *
> > + * If @in_bs is given then space for allocated clusters and zero clusters
> > + * from that image are included in the calculation. If @opts contains a
> > + * backing file that is shared by @in_bs then backing clusters are omitted
> > + * from the calculation.
>
> This seems to run a bit contrary to the documentation of
> BlockMeasureInfo.required-bytes, and I don't fully understand it either.
>
> What does "space for zero clusters" mean? Do zero clusters take space?
> Does it depend on the image format? (i.e. would they take space for raw
> but not for qcow2?)
Yes, zero clusters are an image format-specific feature. A contrived
example:
in_bs: qcow2 version=3 with zero clusters
Output format: qcow2 version=2 (zero clusters not supported!)
Image creation options: backing file given
We must take care to allocate clusters in the new image for zero
clusters in in_bs. We cannot simply skip allocating those zero clusters
since there is a backing file and the contents of the backing file
must not be visible where there is a zero cluster.
This is a scenario where zero clusters must be included in the
size calculation.
Perhaps this is an internal detail and it shouldn't be mentioned in the
doc comment?
> And is space for unallocated clusters included or not? Do unallocated
> clusters without a backing image count as zero clusters?
This depends on the output image format. The raw format requires space
even for unallocated regions. The qcow2 format is compact and only
requires space for allocated clusters.
> If that space is not included, then it would run contrary to the QAPI
> documentation which states that it should be included.
Sorry, the raw format example in the QAPI doc is misleading without a
qcow2 example. The point of the raw format example was not to state
that unallocated regions are included for *all* image formats. It was
just to show how the raw format behaves.
I'll reword things in the next revision.
> Finally, how are you supposed to check whether the backing file in @opts
> is shared by @in_bs?
qemu-img convert -B <base> and -o backing_file=<base> simply do not
check. They rely on good old-fashioned^W^W^Wthe bad practice of
trusting the user to provide valid input.
qemu-img measure will work like this:
1. If the new image has a backing file, use has_zero_init=false
semantics.
2. Do *not* rely on bdrv_get_block_status_above() because it's hard to
check how the backing chains of in_bs and the new image compare.
This means the result will be conservative - perhaps clusters could have
been shared with the backing file.
> > + *
> > + * If @in_bs is NULL then the calculation includes no allocated clusters
> > + * unless a preallocation option is given in @opts.
>
> But the BlockMeasureInfo.required-bytes documentation states that a new
> 5 GB raw image should still report 5 GB of required space.
Even with 0 allocated clusters, the raw format always reports the
virtual disk size (5 GB). There is no contradiction here.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [RFC v2 1/8] block: add bdrv_measure() API
2017-03-16 3:38 ` Stefan Hajnoczi
@ 2017-03-16 4:02 ` Max Reitz
0 siblings, 0 replies; 30+ messages in thread
From: Max Reitz @ 2017-03-16 4:02 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: qemu-devel, Kevin Wolf, John Snow, Nir Soffer, Maor Lipchuk,
Alberto Garcia
[-- Attachment #1: Type: text/plain, Size: 8223 bytes --]
On 16.03.2017 04:38, Stefan Hajnoczi wrote:
> On Thu, Mar 16, 2017 at 02:01:03AM +0100, Max Reitz wrote:
>> On 15.03.2017 10:29, Stefan Hajnoczi wrote:
>>> bdrv_measure() provides a conservative maximum for the size of a new
>>> image. This information is handy if storage needs to be allocated (e.g.
>>> a SAN or an LVM volume) ahead of time.
>>>
>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>>> ---
>>> qapi/block-core.json | 19 +++++++++++++++++++
>>> include/block/block.h | 4 ++++
>>> include/block/block_int.h | 2 ++
>>> block.c | 33 +++++++++++++++++++++++++++++++++
>>> 4 files changed, 58 insertions(+)
>>>
>>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>>> index 786b39e..673569d 100644
>>> --- a/qapi/block-core.json
>>> +++ b/qapi/block-core.json
>>> @@ -463,6 +463,25 @@
>>> '*dirty-bitmaps': ['BlockDirtyInfo'] } }
>>>
>>> ##
>>> +# @BlockMeasureInfo:
>>> +#
>>> +# Image size calculation information. This structure describes the size
>>> +# requirements for creating a new image.
>>> +#
>>> +# @required-bytes: Amount of space required for image creation. This value is
>>> +# the host file size including sparse file regions. A new 5
>>> +# GB raw file therefore has a required size of 5 GB, not 0
>>> +# bytes.
>>
>> This should probably note that it's a conservative estimation (and I
>> agree that it should be). It's nice to have it in the commit message but
>> few people are going to run git blame on the QAPI documentation to find
>> out the rest of its story. :-)
>
> Will fix.
>
>>> +#
>>> +# @fully-allocated-bytes: Space required once data has been written to all
>>> +# sectors
>>> +#
>>> +# Since: 2.10
>>> +##
>>> +{ 'struct': 'BlockMeasureInfo',
>>> + 'data': {'required-bytes': 'int', 'fully-allocated-bytes': 'int'} }
>>> +
>>> +##
>>> # @query-block:
>>> #
>>> # Get a list of BlockInfo for all virtual block devices.
>>> diff --git a/include/block/block.h b/include/block/block.h
>>> index 5149260..43c789f 100644
>>> --- a/include/block/block.h
>>> +++ b/include/block/block.h
>>> @@ -298,6 +298,10 @@ int bdrv_truncate(BdrvChild *child, int64_t offset);
>>> int64_t bdrv_nb_sectors(BlockDriverState *bs);
>>> int64_t bdrv_getlength(BlockDriverState *bs);
>>> int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
>>> +void bdrv_measure(BlockDriver *drv, QemuOpts *opts,
>>> + BlockDriverState *in_bs,
>>> + BlockMeasureInfo *info,
>>> + Error **errp);
>>> void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
>>> void bdrv_refresh_limits(BlockDriverState *bs, Error **errp);
>>> int bdrv_commit(BlockDriverState *bs);
>>> diff --git a/include/block/block_int.h b/include/block/block_int.h
>>> index 6c699ac..45a7fbe 100644
>>> --- a/include/block/block_int.h
>>> +++ b/include/block/block_int.h
>>> @@ -201,6 +201,8 @@ struct BlockDriver {
>>> int64_t (*bdrv_getlength)(BlockDriverState *bs);
>>> bool has_variable_length;
>>> int64_t (*bdrv_get_allocated_file_size)(BlockDriverState *bs);
>>> + void (*bdrv_measure)(QemuOpts *opts, BlockDriverState *in_bs,
>>> + BlockMeasureInfo *info, Error **errp);
>>>
>>> int coroutine_fn (*bdrv_co_pwritev_compressed)(BlockDriverState *bs,
>>> uint64_t offset, uint64_t bytes, QEMUIOVector *qiov);
>>> diff --git a/block.c b/block.c
>>> index cb57370..532a4d1 100644
>>> --- a/block.c
>>> +++ b/block.c
>>> @@ -3260,6 +3260,39 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)
>>> return -ENOTSUP;
>>> }
>>>
>>> +/*
>>> + * bdrv_measure:
>>> + * @drv: Format driver
>>> + * @opts: Creation options
>>> + * @in_bs: Existing image containing data for new image (may be NULL)
>>> + * @info: Result object
>>> + * @errp: Error object
>>> + *
>>> + * Calculate file size required to create a new image.
>>> + *
>>> + * If @in_bs is given then space for allocated clusters and zero clusters
>>> + * from that image are included in the calculation. If @opts contains a
>>> + * backing file that is shared by @in_bs then backing clusters are omitted
>>> + * from the calculation.
>>
>> This seems to run a bit contrary to the documentation of
>> BlockMeasureInfo.required-bytes, and I don't fully understand it either.
>>
>> What does "space for zero clusters" mean? Do zero clusters take space?
>> Does it depend on the image format? (i.e. would they take space for raw
>> but not for qcow2?)
>
> Yes, zero clusters are an image format-specific feature. A contrived
> example:
>
> in_bs: qcow2 version=3 with zero clusters
> Output format: qcow2 version=2 (zero clusters not supported!)
> Image creation options: backing file given
>
> We must take care to allocate clusters in the new image for zero
> clusters in in_bs. We cannot simply skip allocating those zero clusters
> since there is a backing file and the contents of the backing file
> must not be visible where there is a zero cluster.
>
> This is a scenario where zero clusters must be included in the
> size calculation.
>
> Perhaps this is an internal detail and it shouldn't be mentioned in the
> doc comment?
Probably, yes, or it should be phrased differently so it's clear that
zero clusters do not contribute to the required space if it is possible
to represent them efficiently.
(Or explicitly state that the reason zero clusters may contribute to the
allocation size is that they may have to be converted to allocated
clusters.)
>> And is space for unallocated clusters included or not? Do unallocated
>> clusters without a backing image count as zero clusters?
>
> This depends on the output image format. The raw format requires space
> even for unallocated regions. The qcow2 format is compact and only
> requires space for allocated clusters.
Right, that's what I would have thought.
>> If that space is not included, then it would run contrary to the QAPI
>> documentation which states that it should be included.
>
> Sorry, the raw format example in the QAPI doc is misleading without a
> qcow2 example. The point of the raw format example was not to state
> that unallocated regions are included for *all* image formats. It was
> just to show how the raw format behaves.
Oh, now I see. The "sparse file regions" bit was confusing for me. I
thought about format-specific sparse regions but it's actually about the
protocol level. Maybe that should be made more clear.
> I'll reword things in the next revision.
>
>> Finally, how are you supposed to check whether the backing file in @opts
>> is shared by @in_bs?
>
> qemu-img convert -B <base> and -o backing_file=<base> simply do not
> check. They rely on good old-fashioned^W^W^Wthe bad practice of
> trusting the user to provide valid input.
>
> qemu-img measure will work like this:
> 1. If the new image has a backing file, use has_zero_init=false
> semantics.
> 2. Do *not* rely on bdrv_get_block_status_above() because it's hard to
> check how the backing chains of in_bs and the new image compare.
>
> This means the result will be conservative - perhaps clusters could have
> been shared with the backing file.
OK, that's what I had hoped. I was asking because this comment seems to
imply that somebody actually checks whether the backing file is shared.
>>> + *
>>> + * If @in_bs is NULL then the calculation includes no allocated clusters
>>> + * unless a preallocation option is given in @opts.
>>
>> But the BlockMeasureInfo.required-bytes documentation states that a new
>> 5 GB raw image should still report 5 GB of required space.
>
> Even with 0 allocated clusters, the raw format always reports the
> virtual disk size (5 GB). There is no contradiction here.
Yes, right. For some reason I thought about "allocated clusters" in the
new image rather than assumed allocated clusters in the input. For raw,
every cluster is always fully allocated.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC v2 1/8] block: add bdrv_measure() API
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 1/8] block: add bdrv_measure() API Stefan Hajnoczi
2017-03-16 1:01 ` Max Reitz
@ 2017-03-18 0:51 ` Nir Soffer
2017-03-20 15:37 ` Stefan Hajnoczi
2017-03-18 1:28 ` Nir Soffer
2 siblings, 1 reply; 30+ messages in thread
From: Nir Soffer @ 2017-03-18 0:51 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Kevin Wolf, Maor Lipchuk, Daniel P. Berrange, Eric Blake,
Alberto Garcia, John Snow
On Wed, Mar 15, 2017 at 11:29 AM Stefan Hajnoczi <stefanha@redhat.com>
wrote:
> bdrv_measure() provides a conservative maximum for the size of a new
> image. This information is handy if storage needs to be allocated (e.g.
> a SAN or an LVM volume) ahead of time.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> qapi/block-core.json | 19 +++++++++++++++++++
> include/block/block.h | 4 ++++
> include/block/block_int.h | 2 ++
> block.c | 33 +++++++++++++++++++++++++++++++++
> 4 files changed, 58 insertions(+)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 786b39e..673569d 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -463,6 +463,25 @@
> '*dirty-bitmaps': ['BlockDirtyInfo'] } }
>
> ##
> +# @BlockMeasureInfo:
> +#
> +# Image size calculation information. This structure describes the size
> +# requirements for creating a new image.
+#
> +# @required-bytes: Amount of space required for image creation. This
> value is
> +# the host file size including sparse file regions. A
> new 5
> +# GB raw file therefore has a required size of 5 GB, not
> 0
> +# bytes.
> +#
> +# @fully-allocated-bytes: Space required once data has been written to all
> +# sectors
+#
> +# Since: 2.10
> +##
> +{ 'struct': 'BlockMeasureInfo',
> + 'data': {'required-bytes': 'int', 'fully-allocated-bytes': 'int'} }
> +
> +##
> # @query-block:
> #
> # Get a list of BlockInfo for all virtual block devices.
> diff --git a/include/block/block.h b/include/block/block.h
> index 5149260..43c789f 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -298,6 +298,10 @@ int bdrv_truncate(BdrvChild *child, int64_t offset);
> int64_t bdrv_nb_sectors(BlockDriverState *bs);
> int64_t bdrv_getlength(BlockDriverState *bs);
> int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
> +void bdrv_measure(BlockDriver *drv, QemuOpts *opts,
> + BlockDriverState *in_bs,
> + BlockMeasureInfo *info,
> + Error **errp);
> void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
> void bdrv_refresh_limits(BlockDriverState *bs, Error **errp);
> int bdrv_commit(BlockDriverState *bs);
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 6c699ac..45a7fbe 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -201,6 +201,8 @@ struct BlockDriver {
> int64_t (*bdrv_getlength)(BlockDriverState *bs);
> bool has_variable_length;
> int64_t (*bdrv_get_allocated_file_size)(BlockDriverState *bs);
> + void (*bdrv_measure)(QemuOpts *opts, BlockDriverState *in_bs,
> + BlockMeasureInfo *info, Error **errp);
>
> int coroutine_fn (*bdrv_co_pwritev_compressed)(BlockDriverState *bs,
> uint64_t offset, uint64_t bytes, QEMUIOVector *qiov);
> diff --git a/block.c b/block.c
> index cb57370..532a4d1 100644
> --- a/block.c
> +++ b/block.c
> @@ -3260,6 +3260,39 @@ int64_t
> bdrv_get_allocated_file_size(BlockDriverState *bs)
> return -ENOTSUP;
> }
>
> +/*
> + * bdrv_measure:
> + * @drv: Format driver
> + * @opts: Creation options
>
Isn't this Measure options?
> + * @in_bs: Existing image containing data for new image (may be NULL)
> + * @info: Result object
> + * @errp: Error object
> + *
> + * Calculate file size required to create a new image.
> + *
> + * If @in_bs is given then space for allocated clusters and zero clusters
> + * from that image are included in the calculation. If @opts contains a
> + * backing file that is shared by @in_bs then backing clusters are omitted
> + * from the calculation.
> + *
> + * If @in_bs is NULL then the calculation includes no allocated clusters
> + * unless a preallocation option is given in @opts.
> + *
> + * Note that @in_bs may use a different BlockDriver from @drv.
>
Maybe a note about error handling? the only way is to check for non-null
errp, right?
> + */
> +void bdrv_measure(BlockDriver *drv, QemuOpts *opts,
> + BlockDriverState *in_bs, BlockMeasureInfo *info,
> + Error **errp)
+{
> + if (!drv->bdrv_measure) {
> + error_setg(errp, "Block driver '%s' does not support size
> measurement",
> + drv->format_name);
> + return;
> + }
> +
> + drv->bdrv_measure(opts, in_bs, info, errp);
> +}
> +
> /**
> * Return number of sectors on success, -errno on error.
> */
> --
> 2.9.3
>
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [RFC v2 1/8] block: add bdrv_measure() API
2017-03-18 0:51 ` Nir Soffer
@ 2017-03-20 15:37 ` Stefan Hajnoczi
0 siblings, 0 replies; 30+ messages in thread
From: Stefan Hajnoczi @ 2017-03-20 15:37 UTC (permalink / raw)
To: Nir Soffer
Cc: qemu-devel, Kevin Wolf, Maor Lipchuk, Daniel P. Berrange,
Eric Blake, Alberto Garcia, John Snow
[-- Attachment #1: Type: text/plain, Size: 951 bytes --]
On Sat, Mar 18, 2017 at 12:51:29AM +0000, Nir Soffer wrote:
> On Wed, Mar 15, 2017 at 11:29 AM Stefan Hajnoczi <stefanha@redhat.com>
> wrote:
> > diff --git a/block.c b/block.c
> > index cb57370..532a4d1 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -3260,6 +3260,39 @@ int64_t
> > bdrv_get_allocated_file_size(BlockDriverState *bs)
> > return -ENOTSUP;
> > }
> >
> > +/*
> > + * bdrv_measure:
> > + * @drv: Format driver
> > + * @opts: Creation options
> >
>
> Isn't this Measure options?
No, these are the block driver .bdrv_create() options. They are exactly
the same as qemu-img create -o.
qemu-img measure must know the creation options of the new image file
(such as preallocation or the qcow2 image format version) because they
affect the file size.
> Maybe a note about error handling? the only way is to check for non-null
> errp, right?
Yes, errp must be checked. I'll update the doc comment.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC v2 1/8] block: add bdrv_measure() API
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 1/8] block: add bdrv_measure() API Stefan Hajnoczi
2017-03-16 1:01 ` Max Reitz
2017-03-18 0:51 ` Nir Soffer
@ 2017-03-18 1:28 ` Nir Soffer
2017-03-20 15:34 ` Stefan Hajnoczi
2 siblings, 1 reply; 30+ messages in thread
From: Nir Soffer @ 2017-03-18 1:28 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Kevin Wolf, Maor Lipchuk, Daniel P. Berrange, Eric Blake,
Alberto Garcia, John Snow
On Wed, Mar 15, 2017 at 11:29 AM Stefan Hajnoczi <stefanha@redhat.com>
wrote:
> bdrv_measure() provides a conservative maximum for the size of a new
> image. This information is handy if storage needs to be allocated (e.g.
> a SAN or an LVM volume) ahead of time.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> qapi/block-core.json | 19 +++++++++++++++++++
> include/block/block.h | 4 ++++
> include/block/block_int.h | 2 ++
> block.c | 33 +++++++++++++++++++++++++++++++++
> 4 files changed, 58 insertions(+)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 786b39e..673569d 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -463,6 +463,25 @@
> '*dirty-bitmaps': ['BlockDirtyInfo'] } }
>
> ##
> +# @BlockMeasureInfo:
> +#
> +# Image size calculation information. This structure describes the size
> +# requirements for creating a new image.
> +#
> +# @required-bytes: Amount of space required for image creation. This
> value is
> +# the host file size including sparse file regions. A
> new 5
> +# GB raw file therefore has a required size of 5 GB, not
> 0
> +# bytes.
> +#
> +# @fully-allocated-bytes: Space required once data has been written to all
> +# sectors
> +#
> +# Since: 2.10
> +##
> +{ 'struct': 'BlockMeasureInfo',
> + 'data': {'required-bytes': 'int', 'fully-allocated-bytes': 'int'} }
> +
> +##
> # @query-block:
> #
> # Get a list of BlockInfo for all virtual block devices.
> diff --git a/include/block/block.h b/include/block/block.h
> index 5149260..43c789f 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -298,6 +298,10 @@ int bdrv_truncate(BdrvChild *child, int64_t offset);
> int64_t bdrv_nb_sectors(BlockDriverState *bs);
> int64_t bdrv_getlength(BlockDriverState *bs);
> int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
> +void bdrv_measure(BlockDriver *drv, QemuOpts *opts,
> + BlockDriverState *in_bs,
> + BlockMeasureInfo *info,
>
The struct declaration is missing in this patch, right?
> + Error **errp);
> void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
> void bdrv_refresh_limits(BlockDriverState *bs, Error **errp);
> int bdrv_commit(BlockDriverState *bs);
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 6c699ac..45a7fbe 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -201,6 +201,8 @@ struct BlockDriver {
> int64_t (*bdrv_getlength)(BlockDriverState *bs);
> bool has_variable_length;
> int64_t (*bdrv_get_allocated_file_size)(BlockDriverState *bs);
> + void (*bdrv_measure)(QemuOpts *opts, BlockDriverState *in_bs,
> + BlockMeasureInfo *info, Error **errp);
>
> int coroutine_fn (*bdrv_co_pwritev_compressed)(BlockDriverState *bs,
> uint64_t offset, uint64_t bytes, QEMUIOVector *qiov);
> diff --git a/block.c b/block.c
> index cb57370..532a4d1 100644
> --- a/block.c
> +++ b/block.c
> @@ -3260,6 +3260,39 @@ int64_t
> bdrv_get_allocated_file_size(BlockDriverState *bs)
> return -ENOTSUP;
> }
>
> +/*
> + * bdrv_measure:
> + * @drv: Format driver
> + * @opts: Creation options
> + * @in_bs: Existing image containing data for new image (may be NULL)
> + * @info: Result object
> + * @errp: Error object
> + *
> + * Calculate file size required to create a new image.
> + *
> + * If @in_bs is given then space for allocated clusters and zero clusters
> + * from that image are included in the calculation. If @opts contains a
> + * backing file that is shared by @in_bs then backing clusters are omitted
> + * from the calculation.
> + *
> + * If @in_bs is NULL then the calculation includes no allocated clusters
> + * unless a preallocation option is given in @opts.
> + *
> + * Note that @in_bs may use a different BlockDriver from @drv.
> + */
> +void bdrv_measure(BlockDriver *drv, QemuOpts *opts,
> + BlockDriverState *in_bs, BlockMeasureInfo *info,
> + Error **errp)
> +{
> + if (!drv->bdrv_measure) {
> + error_setg(errp, "Block driver '%s' does not support size
> measurement",
> + drv->format_name);
> + return;
> + }
> +
> + drv->bdrv_measure(opts, in_bs, info, errp);
> +}
> +
> /**
> * Return number of sectors on success, -errno on error.
> */
> --
> 2.9.3
>
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [RFC v2 1/8] block: add bdrv_measure() API
2017-03-18 1:28 ` Nir Soffer
@ 2017-03-20 15:34 ` Stefan Hajnoczi
0 siblings, 0 replies; 30+ messages in thread
From: Stefan Hajnoczi @ 2017-03-20 15:34 UTC (permalink / raw)
To: Nir Soffer
Cc: qemu-devel, Kevin Wolf, Maor Lipchuk, Daniel P. Berrange,
Eric Blake, Alberto Garcia, John Snow
[-- Attachment #1: Type: text/plain, Size: 856 bytes --]
On Sat, Mar 18, 2017 at 01:28:04AM +0000, Nir Soffer wrote:
> On Wed, Mar 15, 2017 at 11:29 AM Stefan Hajnoczi <stefanha@redhat.com>
> wrote:
> > diff --git a/include/block/block.h b/include/block/block.h
> > index 5149260..43c789f 100644
> > --- a/include/block/block.h
> > +++ b/include/block/block.h
> > @@ -298,6 +298,10 @@ int bdrv_truncate(BdrvChild *child, int64_t offset);
> > int64_t bdrv_nb_sectors(BlockDriverState *bs);
> > int64_t bdrv_getlength(BlockDriverState *bs);
> > int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
> > +void bdrv_measure(BlockDriver *drv, QemuOpts *opts,
> > + BlockDriverState *in_bs,
> > + BlockMeasureInfo *info,
> >
>
> The struct declaration is missing in this patch, right?
BlockMeasureInfo is generated from qapi-schema.json during the build.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [RFC v2 2/8] raw-format: add bdrv_measure() support
2017-03-15 9:29 [Qemu-devel] [RFC v2 0/8] qemu-img: add measure sub-command Stefan Hajnoczi
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 1/8] block: add bdrv_measure() API Stefan Hajnoczi
@ 2017-03-15 9:29 ` Stefan Hajnoczi
2017-03-18 1:11 ` Nir Soffer
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 3/8] qcow2: extract preallocation calculation function Stefan Hajnoczi
` (6 subsequent siblings)
8 siblings, 1 reply; 30+ messages in thread
From: Stefan Hajnoczi @ 2017-03-15 9:29 UTC (permalink / raw)
To: qemu-devel
Cc: Nir Soffer, Kevin Wolf, Maor Lipchuk, Daniel P. Berrange,
Eric Blake, Alberto Garcia, John Snow, Stefan Hajnoczi
Maximum size calculation is trivial for the raw format: it's just the
requested image size (because there is no metadata).
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/raw-format.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/block/raw-format.c b/block/raw-format.c
index 86fbc65..cc88540 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -312,6 +312,27 @@ static int64_t raw_getlength(BlockDriverState *bs)
return s->size;
}
+static void raw_measure(QemuOpts *opts, BlockDriverState *in_bs,
+ BlockMeasureInfo *info,
+ Error **errp)
+{
+ if (in_bs) {
+ int64_t ssize = bdrv_getlength(in_bs);
+ if (ssize < 0) {
+ error_setg_errno(errp, -ssize, "Unable to get image size");
+ return;
+ }
+ info->required_bytes = ssize;
+ } else {
+ info->required_bytes =
+ ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+ BDRV_SECTOR_SIZE);
+ }
+
+ /* Unallocated sectors count towards the file size in raw images */
+ info->fully_allocated_bytes = info->required_bytes;
+}
+
static int raw_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
{
return bdrv_get_info(bs->file->bs, bdi);
@@ -477,6 +498,7 @@ BlockDriver bdrv_raw = {
.bdrv_truncate = &raw_truncate,
.bdrv_getlength = &raw_getlength,
.has_variable_length = true,
+ .bdrv_measure = &raw_measure,
.bdrv_get_info = &raw_get_info,
.bdrv_refresh_limits = &raw_refresh_limits,
.bdrv_probe_blocksizes = &raw_probe_blocksizes,
--
2.9.3
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [RFC v2 2/8] raw-format: add bdrv_measure() support
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 2/8] raw-format: add bdrv_measure() support Stefan Hajnoczi
@ 2017-03-18 1:11 ` Nir Soffer
0 siblings, 0 replies; 30+ messages in thread
From: Nir Soffer @ 2017-03-18 1:11 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Kevin Wolf, Maor Lipchuk, Daniel P. Berrange, Eric Blake,
Alberto Garcia, John Snow
On Wed, Mar 15, 2017 at 11:29 AM Stefan Hajnoczi <stefanha@redhat.com>
wrote:
> Maximum size calculation is trivial for the raw format: it's just the
> requested image size (because there is no metadata).
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> block/raw-format.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/block/raw-format.c b/block/raw-format.c
> index 86fbc65..cc88540 100644
> --- a/block/raw-format.c
> +++ b/block/raw-format.c
> @@ -312,6 +312,27 @@ static int64_t raw_getlength(BlockDriverState *bs)
> return s->size;
> }
>
> +static void raw_measure(QemuOpts *opts, BlockDriverState *in_bs,
> + BlockMeasureInfo *info,
> + Error **errp)
> +{
> + if (in_bs) {
> + int64_t ssize = bdrv_getlength(in_bs);
> + if (ssize < 0) {
> + error_setg_errno(errp, -ssize, "Unable to get image size");
> + return;
> + }
> + info->required_bytes = ssize;
> + } else {
> + info->required_bytes =
> + ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> + BDRV_SECTOR_SIZE);
> + }
> +
> + /* Unallocated sectors count towards the file size in raw images */
> + info->fully_allocated_bytes = info->required_bytes;
> +}
> +
> static int raw_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
> {
> return bdrv_get_info(bs->file->bs, bdi);
> @@ -477,6 +498,7 @@ BlockDriver bdrv_raw = {
> .bdrv_truncate = &raw_truncate,
> .bdrv_getlength = &raw_getlength,
> .has_variable_length = true,
> + .bdrv_measure = &raw_measure,
> .bdrv_get_info = &raw_get_info,
> .bdrv_refresh_limits = &raw_refresh_limits,
> .bdrv_probe_blocksizes = &raw_probe_blocksizes,
> --
> 2.9.3
>
>
Looks good.
Nir
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [RFC v2 3/8] qcow2: extract preallocation calculation function
2017-03-15 9:29 [Qemu-devel] [RFC v2 0/8] qemu-img: add measure sub-command Stefan Hajnoczi
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 1/8] block: add bdrv_measure() API Stefan Hajnoczi
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 2/8] raw-format: add bdrv_measure() support Stefan Hajnoczi
@ 2017-03-15 9:29 ` Stefan Hajnoczi
2017-03-16 1:18 ` Max Reitz
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 4/8] qcow2: extract image creation option parsing Stefan Hajnoczi
` (5 subsequent siblings)
8 siblings, 1 reply; 30+ messages in thread
From: Stefan Hajnoczi @ 2017-03-15 9:29 UTC (permalink / raw)
To: qemu-devel
Cc: Nir Soffer, Kevin Wolf, Maor Lipchuk, Daniel P. Berrange,
Eric Blake, Alberto Garcia, John Snow, Stefan Hajnoczi
Calculating the preallocated image size will be needed to implement
.bdrv_measure(). Extract the code out into a separate function.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/qcow2.c | 134 +++++++++++++++++++++++++++++++++-------------------------
1 file changed, 76 insertions(+), 58 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 6a92d2e..7c702f4 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2095,6 +2095,79 @@ static int preallocate(BlockDriverState *bs)
return 0;
}
+/**
+ * qcow2_calc_prealloc_size:
+ * @total_size: virtual disk size in bytes
+ * @cluster_size: cluster size in bytes
+ * @refcount_order: refcount bits power-of-2 exponent
+ *
+ * Returns: Total number of bytes required for the fully allocated image
+ * (including metadata).
+ */
+static int64_t qcow2_calc_prealloc_size(int64_t total_size,
+ size_t cluster_size,
+ int refcount_order)
+{
+ /* Note: The following calculation does not need to be exact; if it is a
+ * bit off, either some bytes will be "leaked" (which is fine) or we
+ * will need to increase the file size by some bytes (which is fine,
+ * too, as long as the bulk is allocated here). Therefore, using
+ * floating point arithmetic is fine. */
+ int64_t meta_size = 0;
+ uint64_t nreftablee, nrefblocke, nl1e, nl2e;
+ int64_t aligned_total_size = align_offset(total_size, cluster_size);
+ int cluster_bits = ctz32(cluster_size);
+ int refblock_bits, refblock_size;
+ /* refcount entry size in bytes */
+ double rces = (1 << refcount_order) / 8.;
+
+ /* see qcow2_open() */
+ refblock_bits = cluster_bits - (refcount_order - 3);
+ refblock_size = 1 << refblock_bits;
+
+ /* header: 1 cluster */
+ meta_size += cluster_size;
+
+ /* total size of L2 tables */
+ nl2e = aligned_total_size / cluster_size;
+ nl2e = align_offset(nl2e, cluster_size / sizeof(uint64_t));
+ meta_size += nl2e * sizeof(uint64_t);
+
+ /* total size of L1 tables */
+ nl1e = nl2e * sizeof(uint64_t) / cluster_size;
+ nl1e = align_offset(nl1e, cluster_size / sizeof(uint64_t));
+ meta_size += nl1e * sizeof(uint64_t);
+
+ /* total size of refcount blocks
+ *
+ * note: every host cluster is reference-counted, including metadata
+ * (even refcount blocks are recursively included).
+ * Let:
+ * a = total_size (this is the guest disk size)
+ * m = meta size not including refcount blocks and refcount tables
+ * c = cluster size
+ * y1 = number of refcount blocks entries
+ * y2 = meta size including everything
+ * rces = refcount entry size in bytes
+ * then,
+ * y1 = (y2 + a)/c
+ * y2 = y1 * rces + y1 * rces * sizeof(u64) / c + m
+ * we can get y1:
+ * y1 = (a + m) / (c - rces - rces * sizeof(u64) / c)
+ */
+ nrefblocke = (aligned_total_size + meta_size + cluster_size)
+ / (cluster_size - rces - rces * sizeof(uint64_t)
+ / cluster_size);
+ meta_size += DIV_ROUND_UP(nrefblocke, refblock_size) * cluster_size;
+
+ /* total size of refcount tables */
+ nreftablee = nrefblocke / refblock_size;
+ nreftablee = align_offset(nreftablee, cluster_size / sizeof(uint64_t));
+ meta_size += nreftablee * sizeof(uint64_t);
+
+ return meta_size + aligned_total_size;
+}
+
static int qcow2_create2(const char *filename, int64_t total_size,
const char *backing_file, const char *backing_format,
int flags, size_t cluster_size, PreallocMode prealloc,
@@ -2133,64 +2206,9 @@ static int qcow2_create2(const char *filename, int64_t total_size,
int ret;
if (prealloc == PREALLOC_MODE_FULL || prealloc == PREALLOC_MODE_FALLOC) {
- /* Note: The following calculation does not need to be exact; if it is a
- * bit off, either some bytes will be "leaked" (which is fine) or we
- * will need to increase the file size by some bytes (which is fine,
- * too, as long as the bulk is allocated here). Therefore, using
- * floating point arithmetic is fine. */
- int64_t meta_size = 0;
- uint64_t nreftablee, nrefblocke, nl1e, nl2e;
- int64_t aligned_total_size = align_offset(total_size, cluster_size);
- int refblock_bits, refblock_size;
- /* refcount entry size in bytes */
- double rces = (1 << refcount_order) / 8.;
-
- /* see qcow2_open() */
- refblock_bits = cluster_bits - (refcount_order - 3);
- refblock_size = 1 << refblock_bits;
-
- /* header: 1 cluster */
- meta_size += cluster_size;
-
- /* total size of L2 tables */
- nl2e = aligned_total_size / cluster_size;
- nl2e = align_offset(nl2e, cluster_size / sizeof(uint64_t));
- meta_size += nl2e * sizeof(uint64_t);
-
- /* total size of L1 tables */
- nl1e = nl2e * sizeof(uint64_t) / cluster_size;
- nl1e = align_offset(nl1e, cluster_size / sizeof(uint64_t));
- meta_size += nl1e * sizeof(uint64_t);
-
- /* total size of refcount blocks
- *
- * note: every host cluster is reference-counted, including metadata
- * (even refcount blocks are recursively included).
- * Let:
- * a = total_size (this is the guest disk size)
- * m = meta size not including refcount blocks and refcount tables
- * c = cluster size
- * y1 = number of refcount blocks entries
- * y2 = meta size including everything
- * rces = refcount entry size in bytes
- * then,
- * y1 = (y2 + a)/c
- * y2 = y1 * rces + y1 * rces * sizeof(u64) / c + m
- * we can get y1:
- * y1 = (a + m) / (c - rces - rces * sizeof(u64) / c)
- */
- nrefblocke = (aligned_total_size + meta_size + cluster_size)
- / (cluster_size - rces - rces * sizeof(uint64_t)
- / cluster_size);
- meta_size += DIV_ROUND_UP(nrefblocke, refblock_size) * cluster_size;
-
- /* total size of refcount tables */
- nreftablee = nrefblocke / refblock_size;
- nreftablee = align_offset(nreftablee, cluster_size / sizeof(uint64_t));
- meta_size += nreftablee * sizeof(uint64_t);
-
- qemu_opt_set_number(opts, BLOCK_OPT_SIZE,
- aligned_total_size + meta_size, &error_abort);
+ int64_t prealloc_size =
+ qcow2_calc_prealloc_size(total_size, cluster_size, refcount_order);
+ qemu_opt_set_number(opts, BLOCK_OPT_SIZE, prealloc_size, &error_abort);
qemu_opt_set(opts, BLOCK_OPT_PREALLOC, PreallocMode_lookup[prealloc],
&error_abort);
}
--
2.9.3
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [RFC v2 3/8] qcow2: extract preallocation calculation function
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 3/8] qcow2: extract preallocation calculation function Stefan Hajnoczi
@ 2017-03-16 1:18 ` Max Reitz
2017-03-16 3:40 ` Stefan Hajnoczi
0 siblings, 1 reply; 30+ messages in thread
From: Max Reitz @ 2017-03-16 1:18 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Kevin Wolf, John Snow, Nir Soffer, Maor Lipchuk, Alberto Garcia
[-- Attachment #1: Type: text/plain, Size: 534 bytes --]
On 15.03.2017 10:29, Stefan Hajnoczi wrote:
> Calculating the preallocated image size will be needed to implement
> .bdrv_measure(). Extract the code out into a separate function.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> block/qcow2.c | 134 +++++++++++++++++++++++++++++++++-------------------------
> 1 file changed, 76 insertions(+), 58 deletions(-)
So let's see whether it will be this or
http://lists.nongnu.org/archive/html/qemu-block/2017-03/msg00342.html
that lands first! ;-)
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC v2 3/8] qcow2: extract preallocation calculation function
2017-03-16 1:18 ` Max Reitz
@ 2017-03-16 3:40 ` Stefan Hajnoczi
0 siblings, 0 replies; 30+ messages in thread
From: Stefan Hajnoczi @ 2017-03-16 3:40 UTC (permalink / raw)
To: Max Reitz
Cc: qemu-devel, Kevin Wolf, John Snow, Nir Soffer, Maor Lipchuk,
Alberto Garcia
[-- Attachment #1: Type: text/plain, Size: 659 bytes --]
On Thu, Mar 16, 2017 at 02:18:36AM +0100, Max Reitz wrote:
> On 15.03.2017 10:29, Stefan Hajnoczi wrote:
> > Calculating the preallocated image size will be needed to implement
> > .bdrv_measure(). Extract the code out into a separate function.
> >
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> > block/qcow2.c | 134 +++++++++++++++++++++++++++++++++-------------------------
> > 1 file changed, 76 insertions(+), 58 deletions(-)
>
> So let's see whether it will be this or
> http://lists.nongnu.org/archive/html/qemu-block/2017-03/msg00342.html
> that lands first! ;-)
Ha, thanks for pointing that out :).
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [RFC v2 4/8] qcow2: extract image creation option parsing
2017-03-15 9:29 [Qemu-devel] [RFC v2 0/8] qemu-img: add measure sub-command Stefan Hajnoczi
` (2 preceding siblings ...)
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 3/8] qcow2: extract preallocation calculation function Stefan Hajnoczi
@ 2017-03-15 9:29 ` Stefan Hajnoczi
2017-03-18 20:14 ` Nir Soffer
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 5/8] qcow2: add bdrv_measure() support Stefan Hajnoczi
` (4 subsequent siblings)
8 siblings, 1 reply; 30+ messages in thread
From: Stefan Hajnoczi @ 2017-03-15 9:29 UTC (permalink / raw)
To: qemu-devel
Cc: Nir Soffer, Kevin Wolf, Maor Lipchuk, Daniel P. Berrange,
Eric Blake, Alberto Garcia, John Snow, Stefan Hajnoczi
The image creation options parsed by qcow2_create() are also needed to
implement .bdrv_measure(). Extract the parsing code, including input
validation.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/qcow2.c | 109 +++++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 73 insertions(+), 36 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 7c702f4..19be468 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2168,24 +2168,73 @@ static int64_t qcow2_calc_prealloc_size(int64_t total_size,
return meta_size + aligned_total_size;
}
-static int qcow2_create2(const char *filename, int64_t total_size,
- const char *backing_file, const char *backing_format,
- int flags, size_t cluster_size, PreallocMode prealloc,
- QemuOpts *opts, int version, int refcount_order,
- Error **errp)
+static size_t qcow2_opt_get_cluster_size_del(QemuOpts *opts, Error **errp)
{
+ size_t cluster_size;
int cluster_bits;
- QDict *options;
- /* Calculate cluster_bits */
+ cluster_size = qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE,
+ DEFAULT_CLUSTER_SIZE);
cluster_bits = ctz32(cluster_size);
if (cluster_bits < MIN_CLUSTER_BITS || cluster_bits > MAX_CLUSTER_BITS ||
(1 << cluster_bits) != cluster_size)
{
error_setg(errp, "Cluster size must be a power of two between %d and "
"%dk", 1 << MIN_CLUSTER_BITS, 1 << (MAX_CLUSTER_BITS - 10));
- return -EINVAL;
+ return 0;
}
+ return cluster_size;
+}
+
+static int qcow2_opt_get_version_del(QemuOpts *opts, Error **errp)
+{
+ char *buf;
+ int ret;
+
+ buf = qemu_opt_get_del(opts, BLOCK_OPT_COMPAT_LEVEL);
+ if (!buf) {
+ ret = 3; /* default */
+ } else if (!strcmp(buf, "0.10")) {
+ ret = 2;
+ } else if (!strcmp(buf, "1.1")) {
+ ret = 3;
+ } else {
+ error_setg(errp, "Invalid compatibility level: '%s'", buf);
+ ret = -EINVAL;
+ }
+ g_free(buf);
+ return ret;
+}
+
+static uint64_t qcow2_opt_get_refcount_bits_del(QemuOpts *opts, int version,
+ Error **errp)
+{
+ uint64_t refcount_bits;
+
+ refcount_bits = qemu_opt_get_number_del(opts, BLOCK_OPT_REFCOUNT_BITS, 16);
+ if (refcount_bits > 64 || !is_power_of_2(refcount_bits)) {
+ error_setg(errp, "Refcount width must be a power of two and may not "
+ "exceed 64 bits");
+ return 0;
+ }
+
+ if (version < 3 && refcount_bits != 16) {
+ error_setg(errp, "Different refcount widths than 16 bits require "
+ "compatibility level 1.1 or above (use compat=1.1 or "
+ "greater)");
+ return 0;
+ }
+
+ return refcount_bits;
+}
+
+static int qcow2_create2(const char *filename, int64_t total_size,
+ const char *backing_file, const char *backing_format,
+ int flags, size_t cluster_size, PreallocMode prealloc,
+ QemuOpts *opts, int version, int refcount_order,
+ Error **errp)
+{
+ QDict *options;
/*
* Open the image file and write a minimal qcow2 header.
@@ -2235,7 +2284,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
*header = (QCowHeader) {
.magic = cpu_to_be32(QCOW_MAGIC),
.version = cpu_to_be32(version),
- .cluster_bits = cpu_to_be32(cluster_bits),
+ .cluster_bits = cpu_to_be32(ctz32(cluster_size)),
.size = cpu_to_be64(0),
.l1_table_offset = cpu_to_be64(0),
.l1_size = cpu_to_be32(0),
@@ -2371,8 +2420,8 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
int flags = 0;
size_t cluster_size = DEFAULT_CLUSTER_SIZE;
PreallocMode prealloc;
- int version = 3;
- uint64_t refcount_bits = 16;
+ int version;
+ uint64_t refcount_bits;
int refcount_order;
Error *local_err = NULL;
int ret;
@@ -2385,8 +2434,12 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
flags |= BLOCK_FLAG_ENCRYPT;
}
- cluster_size = qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE,
- DEFAULT_CLUSTER_SIZE);
+ cluster_size = qcow2_opt_get_cluster_size_del(opts, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ ret = -EINVAL;
+ goto finish;
+ }
buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
prealloc = qapi_enum_parse(PreallocMode_lookup, buf,
PREALLOC_MODE__MAX, PREALLOC_MODE_OFF,
@@ -2396,16 +2449,10 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
ret = -EINVAL;
goto finish;
}
- g_free(buf);
- buf = qemu_opt_get_del(opts, BLOCK_OPT_COMPAT_LEVEL);
- if (!buf) {
- /* keep the default */
- } else if (!strcmp(buf, "0.10")) {
- version = 2;
- } else if (!strcmp(buf, "1.1")) {
- version = 3;
- } else {
- error_setg(errp, "Invalid compatibility level: '%s'", buf);
+
+ version = qcow2_opt_get_version_del(opts, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
ret = -EINVAL;
goto finish;
}
@@ -2428,19 +2475,9 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
goto finish;
}
- refcount_bits = qemu_opt_get_number_del(opts, BLOCK_OPT_REFCOUNT_BITS,
- refcount_bits);
- if (refcount_bits > 64 || !is_power_of_2(refcount_bits)) {
- error_setg(errp, "Refcount width must be a power of two and may not "
- "exceed 64 bits");
- ret = -EINVAL;
- goto finish;
- }
-
- if (version < 3 && refcount_bits != 16) {
- error_setg(errp, "Different refcount widths than 16 bits require "
- "compatibility level 1.1 or above (use compat=1.1 or "
- "greater)");
+ refcount_bits = qcow2_opt_get_refcount_bits_del(opts, version, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
ret = -EINVAL;
goto finish;
}
--
2.9.3
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [RFC v2 4/8] qcow2: extract image creation option parsing
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 4/8] qcow2: extract image creation option parsing Stefan Hajnoczi
@ 2017-03-18 20:14 ` Nir Soffer
2017-03-20 15:40 ` Stefan Hajnoczi
0 siblings, 1 reply; 30+ messages in thread
From: Nir Soffer @ 2017-03-18 20:14 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Kevin Wolf, Maor Lipchuk, Daniel P. Berrange, Eric Blake,
Alberto Garcia, John Snow
On Wed, Mar 15, 2017 at 11:30 AM Stefan Hajnoczi <stefanha@redhat.com>
wrote:
> The image creation options parsed by qcow2_create() are also needed to
> implement .bdrv_measure(). Extract the parsing code, including input
> validation.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> block/qcow2.c | 109
> +++++++++++++++++++++++++++++++++++++++-------------------
> 1 file changed, 73 insertions(+), 36 deletions(-)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 7c702f4..19be468 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -2168,24 +2168,73 @@ static int64_t qcow2_calc_prealloc_size(int64_t
> total_size,
> return meta_size + aligned_total_size;
> }
>
> -static int qcow2_create2(const char *filename, int64_t total_size,
> - const char *backing_file, const char
> *backing_format,
> - int flags, size_t cluster_size, PreallocMode
> prealloc,
> - QemuOpts *opts, int version, int refcount_order,
> - Error **errp)
> +static size_t qcow2_opt_get_cluster_size_del(QemuOpts *opts, Error **errp)
> {
> + size_t cluster_size;
> int cluster_bits;
> - QDict *options;
>
> - /* Calculate cluster_bits */
> + cluster_size = qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE,
> + DEFAULT_CLUSTER_SIZE);
> cluster_bits = ctz32(cluster_size);
> if (cluster_bits < MIN_CLUSTER_BITS || cluster_bits >
> MAX_CLUSTER_BITS ||
> (1 << cluster_bits) != cluster_size)
> {
> error_setg(errp, "Cluster size must be a power of two between %d
> and "
> "%dk", 1 << MIN_CLUSTER_BITS, 1 << (MAX_CLUSTER_BITS -
> 10));
> - return -EINVAL;
> + return 0;
> }
> + return cluster_size;
> +}
> +
> +static int qcow2_opt_get_version_del(QemuOpts *opts, Error **errp)
> +{
> + char *buf;
> + int ret;
> +
> + buf = qemu_opt_get_del(opts, BLOCK_OPT_COMPAT_LEVEL);
> + if (!buf) {
> + ret = 3; /* default */
> + } else if (!strcmp(buf, "0.10")) {
> + ret = 2;
> + } else if (!strcmp(buf, "1.1")) {
> + ret = 3;
> + } else {
> + error_setg(errp, "Invalid compatibility level: '%s'", buf);
> + ret = -EINVAL;
> + }
> + g_free(buf);
> + return ret;
> +}
> +
> +static uint64_t qcow2_opt_get_refcount_bits_del(QemuOpts *opts, int
> version,
> + Error **errp)
> +{
> + uint64_t refcount_bits;
> +
> + refcount_bits = qemu_opt_get_number_del(opts,
> BLOCK_OPT_REFCOUNT_BITS, 16);
> + if (refcount_bits > 64 || !is_power_of_2(refcount_bits)) {
> + error_setg(errp, "Refcount width must be a power of two and may
> not "
> + "exceed 64 bits");
> + return 0;
> + }
> +
> + if (version < 3 && refcount_bits != 16) {
> + error_setg(errp, "Different refcount widths than 16 bits require "
> + "compatibility level 1.1 or above (use compat=1.1 or "
> + "greater)");
> + return 0;
> + }
> +
> + return refcount_bits;
> +}
> +
> +static int qcow2_create2(const char *filename, int64_t total_size,
> + const char *backing_file, const char
> *backing_format,
> + int flags, size_t cluster_size, PreallocMode
> prealloc,
> + QemuOpts *opts, int version, int refcount_order,
> + Error **errp)
> +{
> + QDict *options;
>
> /*
> * Open the image file and write a minimal qcow2 header.
> @@ -2235,7 +2284,7 @@ static int qcow2_create2(const char *filename,
> int64_t total_size,
> *header = (QCowHeader) {
> .magic = cpu_to_be32(QCOW_MAGIC),
> .version = cpu_to_be32(version),
> - .cluster_bits = cpu_to_be32(cluster_bits),
> + .cluster_bits = cpu_to_be32(ctz32(cluster_size)),
>
Is this related?
> .size = cpu_to_be64(0),
> .l1_table_offset = cpu_to_be64(0),
> .l1_size = cpu_to_be32(0),
> @@ -2371,8 +2420,8 @@ static int qcow2_create(const char *filename,
> QemuOpts *opts, Error **errp)
> int flags = 0;
> size_t cluster_size = DEFAULT_CLUSTER_SIZE;
> PreallocMode prealloc;
> - int version = 3;
> - uint64_t refcount_bits = 16;
> + int version;
> + uint64_t refcount_bits;
> int refcount_order;
> Error *local_err = NULL;
> int ret;
> @@ -2385,8 +2434,12 @@ static int qcow2_create(const char *filename,
> QemuOpts *opts, Error **errp)
> if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
> flags |= BLOCK_FLAG_ENCRYPT;
> }
> - cluster_size = qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE,
> - DEFAULT_CLUSTER_SIZE);
> + cluster_size = qcow2_opt_get_cluster_size_del(opts, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + ret = -EINVAL;
> + goto finish;
> + }
> buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
> prealloc = qapi_enum_parse(PreallocMode_lookup, buf,
> PREALLOC_MODE__MAX, PREALLOC_MODE_OFF,
> @@ -2396,16 +2449,10 @@ static int qcow2_create(const char *filename,
> QemuOpts *opts, Error **errp)
> ret = -EINVAL;
> goto finish;
> }
> - g_free(buf);
> - buf = qemu_opt_get_del(opts, BLOCK_OPT_COMPAT_LEVEL);
> - if (!buf) {
> - /* keep the default */
> - } else if (!strcmp(buf, "0.10")) {
> - version = 2;
> - } else if (!strcmp(buf, "1.1")) {
> - version = 3;
> - } else {
> - error_setg(errp, "Invalid compatibility level: '%s'", buf);
> +
> + version = qcow2_opt_get_version_del(opts, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> ret = -EINVAL;
> goto finish;
> }
> @@ -2428,19 +2475,9 @@ static int qcow2_create(const char *filename,
> QemuOpts *opts, Error **errp)
> goto finish;
> }
>
> - refcount_bits = qemu_opt_get_number_del(opts, BLOCK_OPT_REFCOUNT_BITS,
> - refcount_bits);
> - if (refcount_bits > 64 || !is_power_of_2(refcount_bits)) {
> - error_setg(errp, "Refcount width must be a power of two and may
> not "
> - "exceed 64 bits");
> - ret = -EINVAL;
> - goto finish;
> - }
> -
> - if (version < 3 && refcount_bits != 16) {
> - error_setg(errp, "Different refcount widths than 16 bits require "
> - "compatibility level 1.1 or above (use compat=1.1 or "
> - "greater)");
> + refcount_bits = qcow2_opt_get_refcount_bits_del(opts, version,
> &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> ret = -EINVAL;
> goto finish;
> }
> --
> 2.9.3
>
Looks good
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [RFC v2 4/8] qcow2: extract image creation option parsing
2017-03-18 20:14 ` Nir Soffer
@ 2017-03-20 15:40 ` Stefan Hajnoczi
0 siblings, 0 replies; 30+ messages in thread
From: Stefan Hajnoczi @ 2017-03-20 15:40 UTC (permalink / raw)
To: Nir Soffer
Cc: qemu-devel, Kevin Wolf, Maor Lipchuk, Daniel P. Berrange,
Eric Blake, Alberto Garcia, John Snow
[-- Attachment #1: Type: text/plain, Size: 728 bytes --]
On Sat, Mar 18, 2017 at 08:14:45PM +0000, Nir Soffer wrote:
> > @@ -2235,7 +2284,7 @@ static int qcow2_create2(const char *filename,
> > int64_t total_size,
> > *header = (QCowHeader) {
> > .magic = cpu_to_be32(QCOW_MAGIC),
> > .version = cpu_to_be32(version),
> > - .cluster_bits = cpu_to_be32(cluster_bits),
> > + .cluster_bits = cpu_to_be32(ctz32(cluster_size)),
> >
>
> Is this related?
Yes, the main cluster_bits code was extracted into
qcow2_opt_get_cluster_size_del() above.
This is now the only place in qcow2_create2() the needs this value so I
inlined the expression and dropped the variable.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [RFC v2 5/8] qcow2: add bdrv_measure() support
2017-03-15 9:29 [Qemu-devel] [RFC v2 0/8] qemu-img: add measure sub-command Stefan Hajnoczi
` (3 preceding siblings ...)
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 4/8] qcow2: extract image creation option parsing Stefan Hajnoczi
@ 2017-03-15 9:29 ` Stefan Hajnoczi
2017-03-16 1:57 ` Max Reitz
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 6/8] qemu-img: add measure subcommand Stefan Hajnoczi
` (3 subsequent siblings)
8 siblings, 1 reply; 30+ messages in thread
From: Stefan Hajnoczi @ 2017-03-15 9:29 UTC (permalink / raw)
To: qemu-devel
Cc: Nir Soffer, Kevin Wolf, Maor Lipchuk, Daniel P. Berrange,
Eric Blake, Alberto Garcia, John Snow, Stefan Hajnoczi
Use qcow2_calc_prealloc_size() to get the required file size.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
TODO:
* Query block status and only count allocated clusters if in_bs != NULL
* Exclude backing file clusters if in_bs != NULL and -o backing_file=
is given
---
block/qcow2.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/block/qcow2.c b/block/qcow2.c
index 19be468..a4caf97 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2940,6 +2940,58 @@ static coroutine_fn int qcow2_co_flush_to_os(BlockDriverState *bs)
return 0;
}
+static void qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
+ BlockMeasureInfo *info, Error **errp)
+{
+ Error *local_err = NULL;
+ uint64_t allocated_bytes = 0;
+ uint64_t prealloc_size;
+ uint64_t size;
+ uint64_t refcount_bits;
+ size_t cluster_size;
+ int version;
+
+ size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+ BDRV_SECTOR_SIZE);
+
+ cluster_size = qcow2_opt_get_cluster_size_del(opts, &local_err);
+ if (local_err) {
+ goto err;
+ }
+
+ version = qcow2_opt_get_version_del(opts, &local_err);
+ if (local_err) {
+ goto err;
+ }
+
+ refcount_bits = qcow2_opt_get_refcount_bits_del(opts, version, &local_err);
+ if (local_err) {
+ goto err;
+ }
+
+ if (in_bs) {
+ int64_t ssize = bdrv_getlength(in_bs);
+ if (ssize < 0) {
+ error_setg_errno(errp, -ssize, "Unable to get image size");
+ return;
+ }
+
+ size = ssize;
+
+ /* TODO How many clusters are allocated modulo backing file in opts? */
+ }
+
+ prealloc_size = qcow2_calc_prealloc_size(size, cluster_size,
+ ctz32(refcount_bits));
+ info->required_bytes = prealloc_size - (align_offset(size, cluster_size) -
+ allocated_bytes);
+ info->fully_allocated_bytes = prealloc_size;
+ return;
+
+err:
+ error_propagate(errp, local_err);
+}
+
static int qcow2_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
{
BDRVQcow2State *s = bs->opaque;
@@ -3487,6 +3539,7 @@ BlockDriver bdrv_qcow2 = {
.bdrv_snapshot_delete = qcow2_snapshot_delete,
.bdrv_snapshot_list = qcow2_snapshot_list,
.bdrv_snapshot_load_tmp = qcow2_snapshot_load_tmp,
+ .bdrv_measure = qcow2_measure,
.bdrv_get_info = qcow2_get_info,
.bdrv_get_specific_info = qcow2_get_specific_info,
--
2.9.3
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [RFC v2 5/8] qcow2: add bdrv_measure() support
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 5/8] qcow2: add bdrv_measure() support Stefan Hajnoczi
@ 2017-03-16 1:57 ` Max Reitz
2017-03-16 3:41 ` Stefan Hajnoczi
0 siblings, 1 reply; 30+ messages in thread
From: Max Reitz @ 2017-03-16 1:57 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Kevin Wolf, John Snow, Nir Soffer, Maor Lipchuk, Alberto Garcia
[-- Attachment #1: Type: text/plain, Size: 2960 bytes --]
On 15.03.2017 10:29, Stefan Hajnoczi wrote:
> Use qcow2_calc_prealloc_size() to get the required file size.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> TODO:
> * Query block status and only count allocated clusters if in_bs != NULL
> * Exclude backing file clusters if in_bs != NULL and -o backing_file=
> is given
> ---
> block/qcow2.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 19be468..a4caf97 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -2940,6 +2940,58 @@ static coroutine_fn int qcow2_co_flush_to_os(BlockDriverState *bs)
> return 0;
> }
>
> +static void qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
> + BlockMeasureInfo *info, Error **errp)
> +{
> + Error *local_err = NULL;
> + uint64_t allocated_bytes = 0;
> + uint64_t prealloc_size;
> + uint64_t size;
> + uint64_t refcount_bits;
> + size_t cluster_size;
> + int version;
> +
> + size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> + BDRV_SECTOR_SIZE);
> +
> + cluster_size = qcow2_opt_get_cluster_size_del(opts, &local_err);
> + if (local_err) {
> + goto err;
> + }
> +
> + version = qcow2_opt_get_version_del(opts, &local_err);
> + if (local_err) {
> + goto err;
> + }
> +
> + refcount_bits = qcow2_opt_get_refcount_bits_del(opts, version, &local_err);
> + if (local_err) {
> + goto err;
> + }
> +
> + if (in_bs) {
> + int64_t ssize = bdrv_getlength(in_bs);
> + if (ssize < 0) {
> + error_setg_errno(errp, -ssize, "Unable to get image size");
> + return;
> + }
> +
> + size = ssize;
> +
> + /* TODO How many clusters are allocated modulo backing file in opts? */
> + }
> +
> + prealloc_size = qcow2_calc_prealloc_size(size, cluster_size,
> + ctz32(refcount_bits));
> + info->required_bytes = prealloc_size - (align_offset(size, cluster_size) -
> + allocated_bytes);
But what if @opts contains a preallocation option?
Max
> + info->fully_allocated_bytes = prealloc_size;
> + return;
> +
> +err:
> + error_propagate(errp, local_err);
> +}
> +
> static int qcow2_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
> {
> BDRVQcow2State *s = bs->opaque;
> @@ -3487,6 +3539,7 @@ BlockDriver bdrv_qcow2 = {
> .bdrv_snapshot_delete = qcow2_snapshot_delete,
> .bdrv_snapshot_list = qcow2_snapshot_list,
> .bdrv_snapshot_load_tmp = qcow2_snapshot_load_tmp,
> + .bdrv_measure = qcow2_measure,
> .bdrv_get_info = qcow2_get_info,
> .bdrv_get_specific_info = qcow2_get_specific_info,
>
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [RFC v2 5/8] qcow2: add bdrv_measure() support
2017-03-16 1:57 ` Max Reitz
@ 2017-03-16 3:41 ` Stefan Hajnoczi
0 siblings, 0 replies; 30+ messages in thread
From: Stefan Hajnoczi @ 2017-03-16 3:41 UTC (permalink / raw)
To: Max Reitz
Cc: qemu-devel, Kevin Wolf, John Snow, Nir Soffer, Maor Lipchuk,
Alberto Garcia
[-- Attachment #1: Type: text/plain, Size: 2556 bytes --]
On Thu, Mar 16, 2017 at 02:57:13AM +0100, Max Reitz wrote:
> On 15.03.2017 10:29, Stefan Hajnoczi wrote:
> > Use qcow2_calc_prealloc_size() to get the required file size.
> >
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> > TODO:
> > * Query block status and only count allocated clusters if in_bs != NULL
> > * Exclude backing file clusters if in_bs != NULL and -o backing_file=
> > is given
> > ---
> > block/qcow2.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 53 insertions(+)
> >
> > diff --git a/block/qcow2.c b/block/qcow2.c
> > index 19be468..a4caf97 100644
> > --- a/block/qcow2.c
> > +++ b/block/qcow2.c
> > @@ -2940,6 +2940,58 @@ static coroutine_fn int qcow2_co_flush_to_os(BlockDriverState *bs)
> > return 0;
> > }
> >
> > +static void qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
> > + BlockMeasureInfo *info, Error **errp)
> > +{
> > + Error *local_err = NULL;
> > + uint64_t allocated_bytes = 0;
> > + uint64_t prealloc_size;
> > + uint64_t size;
> > + uint64_t refcount_bits;
> > + size_t cluster_size;
> > + int version;
> > +
> > + size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> > + BDRV_SECTOR_SIZE);
> > +
> > + cluster_size = qcow2_opt_get_cluster_size_del(opts, &local_err);
> > + if (local_err) {
> > + goto err;
> > + }
> > +
> > + version = qcow2_opt_get_version_del(opts, &local_err);
> > + if (local_err) {
> > + goto err;
> > + }
> > +
> > + refcount_bits = qcow2_opt_get_refcount_bits_del(opts, version, &local_err);
> > + if (local_err) {
> > + goto err;
> > + }
> > +
> > + if (in_bs) {
> > + int64_t ssize = bdrv_getlength(in_bs);
> > + if (ssize < 0) {
> > + error_setg_errno(errp, -ssize, "Unable to get image size");
> > + return;
> > + }
> > +
> > + size = ssize;
> > +
> > + /* TODO How many clusters are allocated modulo backing file in opts? */
> > + }
> > +
> > + prealloc_size = qcow2_calc_prealloc_size(size, cluster_size,
> > + ctz32(refcount_bits));
> > + info->required_bytes = prealloc_size - (align_offset(size, cluster_size) -
> > + allocated_bytes);
>
> But what if @opts contains a preallocation option?
Will implement in the next revision.
Thanks,
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [RFC v2 6/8] qemu-img: add measure subcommand
2017-03-15 9:29 [Qemu-devel] [RFC v2 0/8] qemu-img: add measure sub-command Stefan Hajnoczi
` (4 preceding siblings ...)
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 5/8] qcow2: add bdrv_measure() support Stefan Hajnoczi
@ 2017-03-15 9:29 ` Stefan Hajnoczi
2017-03-16 1:46 ` Max Reitz
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 7/8] qemu-iotests: support per-format golden output files Stefan Hajnoczi
` (2 subsequent siblings)
8 siblings, 1 reply; 30+ messages in thread
From: Stefan Hajnoczi @ 2017-03-15 9:29 UTC (permalink / raw)
To: qemu-devel
Cc: Nir Soffer, Kevin Wolf, Maor Lipchuk, Daniel P. Berrange,
Eric Blake, Alberto Garcia, John Snow, Stefan Hajnoczi
The measure subcommand calculates the size required by a new image file.
This can be used by users or management tools that need to allocate
space on an LVM volume, SAN LUN, etc before creating or converting an
image file.
Suggested-by: Maor Lipchuk <mlipchuk@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
qemu-img.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
qemu-img-cmds.hx | 9 +++
2 files changed, 222 insertions(+)
diff --git a/qemu-img.c b/qemu-img.c
index 98b836b..e8c6dcc 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -59,6 +59,7 @@ enum {
OPTION_PATTERN = 260,
OPTION_FLUSH_INTERVAL = 261,
OPTION_NO_DRAIN = 262,
+ OPTION_SIZE = 263,
};
typedef enum OutputFormat {
@@ -4287,6 +4288,218 @@ out:
return 0;
}
+static void dump_json_block_measure_info(BlockMeasureInfo *info)
+{
+ QString *str;
+ QObject *obj;
+ Visitor *v = qobject_output_visitor_new(&obj);
+
+ visit_type_BlockMeasureInfo(v, NULL, &info, &error_abort);
+ visit_complete(v, &obj);
+ str = qobject_to_json_pretty(obj);
+ assert(str != NULL);
+ printf("%s\n", qstring_get_str(str));
+ qobject_decref(obj);
+ visit_free(v);
+ QDECREF(str);
+}
+
+static int img_measure(int argc, char **argv)
+{
+ static const struct option long_options[] = {
+ {"help", no_argument, 0, 'h'},
+ {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+ {"object", required_argument, 0, OPTION_OBJECT},
+ {"output", required_argument, 0, OPTION_OUTPUT},
+ {"size", required_argument, 0, OPTION_SIZE},
+ {0, 0, 0, 0}
+ };
+ OutputFormat output_format = OFORMAT_HUMAN;
+ BlockBackend *in_blk = NULL;
+ BlockDriver *drv;
+ const char *filename = NULL;
+ const char *fmt = NULL;
+ const char *out_fmt = "raw";
+ char *options = NULL;
+ char *snapshot_name = NULL;
+ QemuOpts *opts = NULL;
+ QemuOpts *object_opts = NULL;
+ QemuOpts *sn_opts = NULL;
+ QemuOptsList *create_opts = NULL;
+ bool image_opts = false;
+ uint64_t img_size = ~0ULL;
+ BlockMeasureInfo info;
+ Error *local_err = NULL;
+ int ret = 1;
+ int c;
+
+ while ((c = getopt_long(argc, argv, "hf:O:o:l:",
+ long_options, NULL)) != -1) {
+ switch (c) {
+ case '?':
+ case 'h':
+ help();
+ break;
+ case 'f':
+ fmt = optarg;
+ break;
+ case 'O':
+ out_fmt = optarg;
+ break;
+ case 'o':
+ if (!is_valid_option_list(optarg)) {
+ error_report("Invalid option list: %s", optarg);
+ goto out;
+ }
+ if (!options) {
+ options = g_strdup(optarg);
+ } else {
+ char *old_options = options;
+ options = g_strdup_printf("%s,%s", options, optarg);
+ g_free(old_options);
+ }
+ break;
+ case 'l':
+ if (strstart(optarg, SNAPSHOT_OPT_BASE, NULL)) {
+ sn_opts = qemu_opts_parse_noisily(&internal_snapshot_opts,
+ optarg, false);
+ if (!sn_opts) {
+ error_report("Failed in parsing snapshot param '%s'",
+ optarg);
+ goto out;
+ }
+ } else {
+ snapshot_name = optarg;
+ }
+ break;
+ case OPTION_OBJECT:
+ object_opts = qemu_opts_parse_noisily(&qemu_object_opts,
+ optarg, true);
+ if (!object_opts) {
+ goto out;
+ }
+ break;
+ case OPTION_IMAGE_OPTS:
+ image_opts = true;
+ break;
+ case OPTION_OUTPUT:
+ if (!strcmp(optarg, "json")) {
+ output_format = OFORMAT_JSON;
+ } else if (!strcmp(optarg, "human")) {
+ output_format = OFORMAT_HUMAN;
+ } else {
+ error_report("--output must be used with human or json "
+ "as argument.");
+ goto out;
+ }
+ break;
+ case OPTION_SIZE:
+ {
+ int64_t sval;
+
+ sval = cvtnum(optarg);
+ if (sval < 0) {
+ if (sval == -ERANGE) {
+ error_report("Image size must be less than 8 EiB!");
+ } else {
+ error_report("Invalid image size specified! You may use "
+ "k, M, G, T, P or E suffixes for ");
+ error_report("kilobytes, megabytes, gigabytes, terabytes, "
+ "petabytes and exabytes.");
+ }
+ goto out;
+ }
+ img_size = (uint64_t)sval;
+ }
+ break;
+ }
+ }
+
+ if (qemu_opts_foreach(&qemu_object_opts,
+ user_creatable_add_opts_foreach,
+ NULL, NULL)) {
+ goto out;
+ }
+
+ if (argc - optind > 1) {
+ error_report("At most one filename argument is allowed.");
+ goto out;
+ } else if (argc - optind == 1) {
+ filename = argv[optind];
+ }
+
+ if (!filename &&
+ (object_opts || image_opts || fmt || snapshot_name || sn_opts)) {
+ error_report("--object, --image-opts, -f, and -l "
+ "require a filename argument.");
+ goto out;
+ }
+ if (filename && img_size != ~0ULL) {
+ error_report("--size N cannot be used together with a filename.");
+ goto out;
+ }
+ if (!filename && img_size == ~0ULL) {
+ error_report("Either --size N or one filename must be specified.");
+ goto out;
+ }
+
+ if (filename) {
+ in_blk = img_open(image_opts, filename, fmt, 0, false, false);
+ if (!in_blk) {
+ goto out;
+ }
+ }
+
+ drv = bdrv_find_format(out_fmt);
+ if (!drv) {
+ error_report("Unknown file format '%s'", out_fmt);
+ goto out;
+ }
+ if (!drv->create_opts) {
+ error_report("Format driver '%s' does not support image creation",
+ drv->format_name);
+ goto out;
+ }
+
+ create_opts = qemu_opts_append(create_opts, drv->create_opts);
+ opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
+ if (options) {
+ qemu_opts_do_parse(opts, options, NULL, &local_err);
+ if (local_err) {
+ error_report_err(local_err);
+ error_report("Invalid options for file format '%s'", out_fmt);
+ goto out;
+ }
+ }
+ if (img_size != ~0ULL) {
+ qemu_opt_set_number(opts, BLOCK_OPT_SIZE, img_size, &error_abort);
+ }
+
+ bdrv_measure(drv, opts, in_blk ? blk_bs(in_blk) : NULL, &info, &local_err);
+ if (local_err) {
+ error_report_err(local_err);
+ goto out;
+ }
+
+ if (output_format == OFORMAT_HUMAN) {
+ printf("required bytes: %" PRIu64 "\n", info.required_bytes);
+ printf("fully allocated bytes: %" PRIu64 "\n",
+ info.fully_allocated_bytes);
+ } else {
+ dump_json_block_measure_info(&info);
+ }
+
+ ret = 0;
+
+out:
+ qemu_opts_del(object_opts);
+ qemu_opts_del(opts);
+ qemu_opts_del(sn_opts);
+ qemu_opts_free(create_opts);
+ g_free(options);
+ blk_unref(in_blk);
+ return ret;
+}
static const img_cmd_t img_cmds[] = {
#define DEF(option, callback, arg_string) \
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 9c9702c..57ef70f 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -85,5 +85,14 @@ DEF("amend", img_amend,
"amend [--object objectdef] [--image-opts] [-p] [-q] [-f fmt] [-t cache] -o options filename")
STEXI
@item amend [--object @var{objectdef}] [--image-opts] [-p] [-q] [-f @var{fmt}] [-t @var{cache}] -o @var{options} @var{filename}
+ETEXI
+
+DEF("measure", img_measure,
+"measure [--output=ofmt] [-O output_fmt] [-o options] [--size N | [--object objectdef] [--image-opts] [-f fmt] [-l snapshot_param] filename]")
+STEXI
+@item measure [--output=@var{ofmt}] [-O @var{output_fmt}] [-o @var{options}] [--size @var{N} | [--object @var{objectdef}] [--image-opts] [-f @var{fmt}] [-l @var{snapshot_param}] @var{filename}]
+ETEXI
+
+STEXI
@end table
ETEXI
--
2.9.3
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [RFC v2 6/8] qemu-img: add measure subcommand
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 6/8] qemu-img: add measure subcommand Stefan Hajnoczi
@ 2017-03-16 1:46 ` Max Reitz
2017-03-16 3:45 ` Stefan Hajnoczi
0 siblings, 1 reply; 30+ messages in thread
From: Max Reitz @ 2017-03-16 1:46 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Kevin Wolf, John Snow, Nir Soffer, Maor Lipchuk, Alberto Garcia
[-- Attachment #1: Type: text/plain, Size: 10373 bytes --]
On 15.03.2017 10:29, Stefan Hajnoczi wrote:
> The measure subcommand calculates the size required by a new image file.
> This can be used by users or management tools that need to allocate
> space on an LVM volume, SAN LUN, etc before creating or converting an
> image file.
>
> Suggested-by: Maor Lipchuk <mlipchuk@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> qemu-img.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> qemu-img-cmds.hx | 9 +++
Looking forward to the documentation in the non-RFC version. O:-)
> 2 files changed, 222 insertions(+)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index 98b836b..e8c6dcc 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -59,6 +59,7 @@ enum {
> OPTION_PATTERN = 260,
> OPTION_FLUSH_INTERVAL = 261,
> OPTION_NO_DRAIN = 262,
> + OPTION_SIZE = 263,
> };
>
> typedef enum OutputFormat {
> @@ -4287,6 +4288,218 @@ out:
> return 0;
> }
>
> +static void dump_json_block_measure_info(BlockMeasureInfo *info)
> +{
> + QString *str;
> + QObject *obj;
> + Visitor *v = qobject_output_visitor_new(&obj);
> +
> + visit_type_BlockMeasureInfo(v, NULL, &info, &error_abort);
> + visit_complete(v, &obj);
> + str = qobject_to_json_pretty(obj);
> + assert(str != NULL);
> + printf("%s\n", qstring_get_str(str));
> + qobject_decref(obj);
> + visit_free(v);
> + QDECREF(str);
> +}
> +
> +static int img_measure(int argc, char **argv)
> +{
> + static const struct option long_options[] = {
> + {"help", no_argument, 0, 'h'},
> + {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
> + {"object", required_argument, 0, OPTION_OBJECT},
> + {"output", required_argument, 0, OPTION_OUTPUT},
> + {"size", required_argument, 0, OPTION_SIZE},
> + {0, 0, 0, 0}
> + };
> + OutputFormat output_format = OFORMAT_HUMAN;
> + BlockBackend *in_blk = NULL;
> + BlockDriver *drv;
> + const char *filename = NULL;
> + const char *fmt = NULL;
> + const char *out_fmt = "raw";
> + char *options = NULL;
> + char *snapshot_name = NULL;
> + QemuOpts *opts = NULL;
> + QemuOpts *object_opts = NULL;
> + QemuOpts *sn_opts = NULL;
> + QemuOptsList *create_opts = NULL;
> + bool image_opts = false;
> + uint64_t img_size = ~0ULL;
> + BlockMeasureInfo info;
> + Error *local_err = NULL;
> + int ret = 1;
> + int c;
> +
> + while ((c = getopt_long(argc, argv, "hf:O:o:l:",
> + long_options, NULL)) != -1) {
> + switch (c) {
> + case '?':
> + case 'h':
> + help();
> + break;
> + case 'f':
> + fmt = optarg;
> + break;
> + case 'O':
> + out_fmt = optarg;
> + break;
> + case 'o':
> + if (!is_valid_option_list(optarg)) {
> + error_report("Invalid option list: %s", optarg);
> + goto out;
> + }
> + if (!options) {
> + options = g_strdup(optarg);
> + } else {
> + char *old_options = options;
> + options = g_strdup_printf("%s,%s", options, optarg);
> + g_free(old_options);
> + }
> + break;
> + case 'l':
> + if (strstart(optarg, SNAPSHOT_OPT_BASE, NULL)) {
> + sn_opts = qemu_opts_parse_noisily(&internal_snapshot_opts,
> + optarg, false);
> + if (!sn_opts) {
> + error_report("Failed in parsing snapshot param '%s'",
> + optarg);
> + goto out;
> + }
> + } else {
> + snapshot_name = optarg;
> + }
> + break;
> + case OPTION_OBJECT:
> + object_opts = qemu_opts_parse_noisily(&qemu_object_opts,
> + optarg, true);
> + if (!object_opts) {
> + goto out;
> + }
> + break;
> + case OPTION_IMAGE_OPTS:
> + image_opts = true;
> + break;
> + case OPTION_OUTPUT:
> + if (!strcmp(optarg, "json")) {
> + output_format = OFORMAT_JSON;
> + } else if (!strcmp(optarg, "human")) {
> + output_format = OFORMAT_HUMAN;
> + } else {
> + error_report("--output must be used with human or json "
> + "as argument.");
> + goto out;
> + }
> + break;
> + case OPTION_SIZE:
> + {
> + int64_t sval;
> +
> + sval = cvtnum(optarg);
> + if (sval < 0) {
> + if (sval == -ERANGE) {
> + error_report("Image size must be less than 8 EiB!");
> + } else {
> + error_report("Invalid image size specified! You may use "
> + "k, M, G, T, P or E suffixes for ");
> + error_report("kilobytes, megabytes, gigabytes, terabytes, "
> + "petabytes and exabytes.");
BTW, I love how we say "EiB" in these error messages but the non-i
suffixes the user can specify are still power-of-two units.
(Also, this error message explicitly says "kilobytes" and so on instead
of "kibibytes". I'm fine with it not least because it's pre-existing,
though. (Furthermore, I'd rather have it be "EB" than the latter message
contain "kibibytes, mebibytes" and so on.))
> + }
> + goto out;
> + }
> + img_size = (uint64_t)sval;
> + }
> + break;
> + }
> + }
> +
> + if (qemu_opts_foreach(&qemu_object_opts,
> + user_creatable_add_opts_foreach,
> + NULL, NULL)) {
> + goto out;
> + }
> +
> + if (argc - optind > 1) {
> + error_report("At most one filename argument is allowed.");
Not exactly like convert, then. :-(
But, yeah, it wouldn't really work for the current interface and we can
always extend it later.
(But maybe that means the interface could be better. Maybe this central
function here could collect a statistics of allocated/zero/free/...
clusters of the input image (with a cluster size somehow specified by
the output image driver, however that's supposed to work...) and pass it
to the output driver. That would also save us from replicating the input
block query code for all of the block drivers that implement
bdrv_measure().)
Max
> + goto out;
> + } else if (argc - optind == 1) {
> + filename = argv[optind];
> + }
> +
> + if (!filename &&
> + (object_opts || image_opts || fmt || snapshot_name || sn_opts)) {
> + error_report("--object, --image-opts, -f, and -l "
> + "require a filename argument.");
> + goto out;
> + }
> + if (filename && img_size != ~0ULL) {
> + error_report("--size N cannot be used together with a filename.");
> + goto out;
> + }
> + if (!filename && img_size == ~0ULL) {
> + error_report("Either --size N or one filename must be specified.");
> + goto out;
> + }
> +
> + if (filename) {
> + in_blk = img_open(image_opts, filename, fmt, 0, false, false);
> + if (!in_blk) {
> + goto out;
> + }
> + }
> +
> + drv = bdrv_find_format(out_fmt);
> + if (!drv) {
> + error_report("Unknown file format '%s'", out_fmt);
> + goto out;
> + }
> + if (!drv->create_opts) {
> + error_report("Format driver '%s' does not support image creation",
> + drv->format_name);
> + goto out;
> + }
> +
> + create_opts = qemu_opts_append(create_opts, drv->create_opts);
> + opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
> + if (options) {
> + qemu_opts_do_parse(opts, options, NULL, &local_err);
> + if (local_err) {
> + error_report_err(local_err);
> + error_report("Invalid options for file format '%s'", out_fmt);
> + goto out;
> + }
> + }
> + if (img_size != ~0ULL) {
> + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, img_size, &error_abort);
> + }
> +
> + bdrv_measure(drv, opts, in_blk ? blk_bs(in_blk) : NULL, &info, &local_err);
> + if (local_err) {
> + error_report_err(local_err);
> + goto out;
> + }
> +
> + if (output_format == OFORMAT_HUMAN) {
> + printf("required bytes: %" PRIu64 "\n", info.required_bytes);
> + printf("fully allocated bytes: %" PRIu64 "\n",
> + info.fully_allocated_bytes);
> + } else {
> + dump_json_block_measure_info(&info);
> + }
> +
> + ret = 0;
> +
> +out:
> + qemu_opts_del(object_opts);
> + qemu_opts_del(opts);
> + qemu_opts_del(sn_opts);
> + qemu_opts_free(create_opts);
> + g_free(options);
> + blk_unref(in_blk);
> + return ret;
> +}
>
> static const img_cmd_t img_cmds[] = {
> #define DEF(option, callback, arg_string) \
> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
> index 9c9702c..57ef70f 100644
> --- a/qemu-img-cmds.hx
> +++ b/qemu-img-cmds.hx
> @@ -85,5 +85,14 @@ DEF("amend", img_amend,
> "amend [--object objectdef] [--image-opts] [-p] [-q] [-f fmt] [-t cache] -o options filename")
> STEXI
> @item amend [--object @var{objectdef}] [--image-opts] [-p] [-q] [-f @var{fmt}] [-t @var{cache}] -o @var{options} @var{filename}
> +ETEXI
> +
> +DEF("measure", img_measure,
> +"measure [--output=ofmt] [-O output_fmt] [-o options] [--size N | [--object objectdef] [--image-opts] [-f fmt] [-l snapshot_param] filename]")
> +STEXI
> +@item measure [--output=@var{ofmt}] [-O @var{output_fmt}] [-o @var{options}] [--size @var{N} | [--object @var{objectdef}] [--image-opts] [-f @var{fmt}] [-l @var{snapshot_param}] @var{filename}]
> +ETEXI
> +
> +STEXI
> @end table
> ETEXI
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [RFC v2 6/8] qemu-img: add measure subcommand
2017-03-16 1:46 ` Max Reitz
@ 2017-03-16 3:45 ` Stefan Hajnoczi
0 siblings, 0 replies; 30+ messages in thread
From: Stefan Hajnoczi @ 2017-03-16 3:45 UTC (permalink / raw)
To: Max Reitz
Cc: qemu-devel, Kevin Wolf, John Snow, Nir Soffer, Maor Lipchuk,
Alberto Garcia
[-- Attachment #1: Type: text/plain, Size: 7464 bytes --]
On Thu, Mar 16, 2017 at 02:46:12AM +0100, Max Reitz wrote:
> On 15.03.2017 10:29, Stefan Hajnoczi wrote:
> > The measure subcommand calculates the size required by a new image file.
> > This can be used by users or management tools that need to allocate
> > space on an LVM volume, SAN LUN, etc before creating or converting an
> > image file.
> >
> > Suggested-by: Maor Lipchuk <mlipchuk@redhat.com>
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> > qemu-img.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > qemu-img-cmds.hx | 9 +++
>
> Looking forward to the documentation in the non-RFC version. O:-)
Thanks for reminding me that the qemu-img man page needs documentation
for this new sub-command :).
> > 2 files changed, 222 insertions(+)
> >
> > diff --git a/qemu-img.c b/qemu-img.c
> > index 98b836b..e8c6dcc 100644
> > --- a/qemu-img.c
> > +++ b/qemu-img.c
> > @@ -59,6 +59,7 @@ enum {
> > OPTION_PATTERN = 260,
> > OPTION_FLUSH_INTERVAL = 261,
> > OPTION_NO_DRAIN = 262,
> > + OPTION_SIZE = 263,
> > };
> >
> > typedef enum OutputFormat {
> > @@ -4287,6 +4288,218 @@ out:
> > return 0;
> > }
> >
> > +static void dump_json_block_measure_info(BlockMeasureInfo *info)
> > +{
> > + QString *str;
> > + QObject *obj;
> > + Visitor *v = qobject_output_visitor_new(&obj);
> > +
> > + visit_type_BlockMeasureInfo(v, NULL, &info, &error_abort);
> > + visit_complete(v, &obj);
> > + str = qobject_to_json_pretty(obj);
> > + assert(str != NULL);
> > + printf("%s\n", qstring_get_str(str));
> > + qobject_decref(obj);
> > + visit_free(v);
> > + QDECREF(str);
> > +}
> > +
> > +static int img_measure(int argc, char **argv)
> > +{
> > + static const struct option long_options[] = {
> > + {"help", no_argument, 0, 'h'},
> > + {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
> > + {"object", required_argument, 0, OPTION_OBJECT},
> > + {"output", required_argument, 0, OPTION_OUTPUT},
> > + {"size", required_argument, 0, OPTION_SIZE},
> > + {0, 0, 0, 0}
> > + };
> > + OutputFormat output_format = OFORMAT_HUMAN;
> > + BlockBackend *in_blk = NULL;
> > + BlockDriver *drv;
> > + const char *filename = NULL;
> > + const char *fmt = NULL;
> > + const char *out_fmt = "raw";
> > + char *options = NULL;
> > + char *snapshot_name = NULL;
> > + QemuOpts *opts = NULL;
> > + QemuOpts *object_opts = NULL;
> > + QemuOpts *sn_opts = NULL;
> > + QemuOptsList *create_opts = NULL;
> > + bool image_opts = false;
> > + uint64_t img_size = ~0ULL;
> > + BlockMeasureInfo info;
> > + Error *local_err = NULL;
> > + int ret = 1;
> > + int c;
> > +
> > + while ((c = getopt_long(argc, argv, "hf:O:o:l:",
> > + long_options, NULL)) != -1) {
> > + switch (c) {
> > + case '?':
> > + case 'h':
> > + help();
> > + break;
> > + case 'f':
> > + fmt = optarg;
> > + break;
> > + case 'O':
> > + out_fmt = optarg;
> > + break;
> > + case 'o':
> > + if (!is_valid_option_list(optarg)) {
> > + error_report("Invalid option list: %s", optarg);
> > + goto out;
> > + }
> > + if (!options) {
> > + options = g_strdup(optarg);
> > + } else {
> > + char *old_options = options;
> > + options = g_strdup_printf("%s,%s", options, optarg);
> > + g_free(old_options);
> > + }
> > + break;
> > + case 'l':
> > + if (strstart(optarg, SNAPSHOT_OPT_BASE, NULL)) {
> > + sn_opts = qemu_opts_parse_noisily(&internal_snapshot_opts,
> > + optarg, false);
> > + if (!sn_opts) {
> > + error_report("Failed in parsing snapshot param '%s'",
> > + optarg);
> > + goto out;
> > + }
> > + } else {
> > + snapshot_name = optarg;
> > + }
> > + break;
> > + case OPTION_OBJECT:
> > + object_opts = qemu_opts_parse_noisily(&qemu_object_opts,
> > + optarg, true);
> > + if (!object_opts) {
> > + goto out;
> > + }
> > + break;
> > + case OPTION_IMAGE_OPTS:
> > + image_opts = true;
> > + break;
> > + case OPTION_OUTPUT:
> > + if (!strcmp(optarg, "json")) {
> > + output_format = OFORMAT_JSON;
> > + } else if (!strcmp(optarg, "human")) {
> > + output_format = OFORMAT_HUMAN;
> > + } else {
> > + error_report("--output must be used with human or json "
> > + "as argument.");
> > + goto out;
> > + }
> > + break;
> > + case OPTION_SIZE:
> > + {
> > + int64_t sval;
> > +
> > + sval = cvtnum(optarg);
> > + if (sval < 0) {
> > + if (sval == -ERANGE) {
> > + error_report("Image size must be less than 8 EiB!");
> > + } else {
> > + error_report("Invalid image size specified! You may use "
> > + "k, M, G, T, P or E suffixes for ");
> > + error_report("kilobytes, megabytes, gigabytes, terabytes, "
> > + "petabytes and exabytes.");
>
> BTW, I love how we say "EiB" in these error messages but the non-i
> suffixes the user can specify are still power-of-two units.
>
> (Also, this error message explicitly says "kilobytes" and so on instead
> of "kibibytes". I'm fine with it not least because it's pre-existing,
> though. (Furthermore, I'd rather have it be "EB" than the latter message
> contain "kibibytes, mebibytes" and so on.))
>
> > + }
> > + goto out;
> > + }
> > + img_size = (uint64_t)sval;
> > + }
> > + break;
> > + }
> > + }
> > +
> > + if (qemu_opts_foreach(&qemu_object_opts,
> > + user_creatable_add_opts_foreach,
> > + NULL, NULL)) {
> > + goto out;
> > + }
> > +
> > + if (argc - optind > 1) {
> > + error_report("At most one filename argument is allowed.");
>
> Not exactly like convert, then. :-(
>
> But, yeah, it wouldn't really work for the current interface and we can
> always extend it later.
>
> (But maybe that means the interface could be better. Maybe this central
> function here could collect a statistics of allocated/zero/free/...
> clusters of the input image (with a cluster size somehow specified by
> the output image driver, however that's supposed to work...) and pass it
> to the output driver. That would also save us from replicating the input
> block query code for all of the block drivers that implement
> bdrv_measure().)
I will look into this for the next revision, thanks.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [RFC v2 7/8] qemu-iotests: support per-format golden output files
2017-03-15 9:29 [Qemu-devel] [RFC v2 0/8] qemu-img: add measure sub-command Stefan Hajnoczi
` (5 preceding siblings ...)
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 6/8] qemu-img: add measure subcommand Stefan Hajnoczi
@ 2017-03-15 9:29 ` Stefan Hajnoczi
2017-03-16 1:51 ` Max Reitz
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 8/8] iotests: add test 178 for qemu-img measure Stefan Hajnoczi
2017-03-17 23:45 ` [Qemu-devel] [RFC v2 0/8] qemu-img: add measure sub-command Nir Soffer
8 siblings, 1 reply; 30+ messages in thread
From: Stefan Hajnoczi @ 2017-03-15 9:29 UTC (permalink / raw)
To: qemu-devel
Cc: Nir Soffer, Kevin Wolf, Maor Lipchuk, Daniel P. Berrange,
Eric Blake, Alberto Garcia, John Snow, Stefan Hajnoczi
Some tests produce format-dependent output. Either the difference is
filtered out and ignored, or the test case is format-specific so we
don't need to worry about per-format output differences.
There is a third case: the test script is the same for all image formats
and the format-dependent output is relevant. An ugly workaround is to
copy-paste the test into multiple per-format test cases. This
duplicates code and is not maintainable.
This patch allows test cases to add per-format golden output files so a
single test case can work correctly when format-dependent output must be
checked:
123.out.qcow2
123.out.raw
123.out.vmdk
...
This naming scheme is not composable with 123.out.nocache or 123.pc.out,
two other scenarios where output files are split. I don't think it
matters since few test cases need these features.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/qemu-iotests/check | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 4b1c674..29553cf 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -338,6 +338,11 @@ do
reference="$reference_machine"
fi
+ reference_format="$source_iotests/$seq.out.$IMGFMT"
+ if [ -f "$reference_format" ]; then
+ reference="$reference_format"
+ fi
+
if [ "$CACHEMODE" = "none" ]; then
[ -f "$source_iotests/$seq.out.nocache" ] && reference="$source_iotests/$seq.out.nocache"
fi
--
2.9.3
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [RFC v2 7/8] qemu-iotests: support per-format golden output files
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 7/8] qemu-iotests: support per-format golden output files Stefan Hajnoczi
@ 2017-03-16 1:51 ` Max Reitz
0 siblings, 0 replies; 30+ messages in thread
From: Max Reitz @ 2017-03-16 1:51 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Kevin Wolf, John Snow, Nir Soffer, Maor Lipchuk, Alberto Garcia
[-- Attachment #1: Type: text/plain, Size: 2074 bytes --]
On 15.03.2017 10:29, Stefan Hajnoczi wrote:
> Some tests produce format-dependent output. Either the difference is
> filtered out and ignored, or the test case is format-specific so we
> don't need to worry about per-format output differences.
>
> There is a third case: the test script is the same for all image formats
> and the format-dependent output is relevant. An ugly workaround is to
> copy-paste the test into multiple per-format test cases. This
> duplicates code and is not maintainable.
>
> This patch allows test cases to add per-format golden output files so a
> single test case can work correctly when format-dependent output must be
> checked:
>
> 123.out.qcow2
> 123.out.raw
> 123.out.vmdk
> ...
>
> This naming scheme is not composable with 123.out.nocache or 123.pc.out,
> two other scenarios where output files are split. I don't think it
> matters since few test cases need these features.
Well, it would be pretty simple to implement by just always using
reference_x="$reference.$NEW_EXTENSION" instead of rebuilding the path
from screatch, but the number of combinations would blow up so much it
may be better to leave that out anyway.
(No R-b because I like to save my precious R-bs for non-RFCs)
Max
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> tests/qemu-iotests/check | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index 4b1c674..29553cf 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -338,6 +338,11 @@ do
> reference="$reference_machine"
> fi
>
> + reference_format="$source_iotests/$seq.out.$IMGFMT"
> + if [ -f "$reference_format" ]; then
> + reference="$reference_format"
> + fi
> +
> if [ "$CACHEMODE" = "none" ]; then
> [ -f "$source_iotests/$seq.out.nocache" ] && reference="$source_iotests/$seq.out.nocache"
> fi
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [RFC v2 8/8] iotests: add test 178 for qemu-img measure
2017-03-15 9:29 [Qemu-devel] [RFC v2 0/8] qemu-img: add measure sub-command Stefan Hajnoczi
` (6 preceding siblings ...)
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 7/8] qemu-iotests: support per-format golden output files Stefan Hajnoczi
@ 2017-03-15 9:29 ` Stefan Hajnoczi
2017-03-18 21:04 ` Nir Soffer
2017-03-17 23:45 ` [Qemu-devel] [RFC v2 0/8] qemu-img: add measure sub-command Nir Soffer
8 siblings, 1 reply; 30+ messages in thread
From: Stefan Hajnoczi @ 2017-03-15 9:29 UTC (permalink / raw)
To: qemu-devel
Cc: Nir Soffer, Kevin Wolf, Maor Lipchuk, Daniel P. Berrange,
Eric Blake, Alberto Garcia, John Snow, Stefan Hajnoczi
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/qemu-iotests/178 | 75 ++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/178.out.qcow2 | 33 ++++++++++++++++++
tests/qemu-iotests/178.out.raw | 33 ++++++++++++++++++
tests/qemu-iotests/group | 1 +
4 files changed, 142 insertions(+)
create mode 100755 tests/qemu-iotests/178
create mode 100644 tests/qemu-iotests/178.out.qcow2
create mode 100644 tests/qemu-iotests/178.out.raw
diff --git a/tests/qemu-iotests/178 b/tests/qemu-iotests/178
new file mode 100755
index 0000000..8db1241
--- /dev/null
+++ b/tests/qemu-iotests/178
@@ -0,0 +1,75 @@
+#!/bin/bash
+#
+# qemu-img measure sub-command tests
+#
+# Copyright (C) 2017 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=stefanha@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.pattern
+
+_supported_fmt raw qcow2
+_supported_proto file
+_supported_os Linux
+
+echo "Input validation"
+echo
+
+_make_test_img 1G
+
+$QEMU_IMG measure # missing arguments
+$QEMU_IMG measure --size 2G "$TEST_IMG" # only one allowed
+$QEMU_IMG measure "$TEST_IMG" a # only one filename allowed
+$QEMU_IMG measure --object secret,id=sec0,data=MTIzNDU2,format=base64 # missing filename
+$QEMU_IMG measure --image-opts # missing filename
+$QEMU_IMG measure -f qcow2 # missing filename
+$QEMU_IMG measure -l snap1 # missing filename
+$QEMU_IMG measure -o , # invalid option list
+$QEMU_IMG measure -l snapshot.foo # invalid snapshot option
+$QEMU_IMG measure --output foo # invalid output format
+$QEMU_IMG measure --size -1 # invalid image size
+$QEMU_IMG measure -O foo "$TEST_IMG" # unknown image file format
+
+echo
+echo "Size calculation"
+echo
+
+for ofmt in human json; do
+ $QEMU_IMG measure --output=$ofmt -O "$IMGFMT" --size 2G
+ $QEMU_IMG measure --output=$ofmt -f "$IMGFMT" -O "$IMGFMT" "$TEST_IMG"
+done
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/178.out.qcow2 b/tests/qemu-iotests/178.out.qcow2
new file mode 100644
index 0000000..3fe3f5f
--- /dev/null
+++ b/tests/qemu-iotests/178.out.qcow2
@@ -0,0 +1,33 @@
+QA output created by 178
+Input validation
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
+qemu-img: Either --size N or one filename must be specified.
+qemu-img: --size N cannot be used together with a filename.
+qemu-img: At most one filename argument is allowed.
+qemu-img: --object, --image-opts, -f, and -l require a filename argument.
+qemu-img: --object, --image-opts, -f, and -l require a filename argument.
+qemu-img: --object, --image-opts, -f, and -l require a filename argument.
+qemu-img: --object, --image-opts, -f, and -l require a filename argument.
+qemu-img: Invalid option list: ,
+qemu-img: Invalid parameter 'snapshot.foo'
+qemu-img: Failed in parsing snapshot param 'snapshot.foo'
+qemu-img: --output must be used with human or json as argument.
+qemu-img: Image size must be less than 8 EiB!
+qemu-img: Unknown file format 'foo'
+
+Size calculation
+
+required bytes: 589824
+fully allocated bytes: 2148073472
+required bytes: 327680
+fully allocated bytes: 1074069504
+{
+ "required-bytes": 589824,
+ "fully-allocated-bytes": 2148073472
+}
+{
+ "required-bytes": 327680,
+ "fully-allocated-bytes": 1074069504
+}
+*** done
diff --git a/tests/qemu-iotests/178.out.raw b/tests/qemu-iotests/178.out.raw
new file mode 100644
index 0000000..4b89dd0
--- /dev/null
+++ b/tests/qemu-iotests/178.out.raw
@@ -0,0 +1,33 @@
+QA output created by 178
+Input validation
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
+qemu-img: Either --size N or one filename must be specified.
+qemu-img: --size N cannot be used together with a filename.
+qemu-img: At most one filename argument is allowed.
+qemu-img: --object, --image-opts, -f, and -l require a filename argument.
+qemu-img: --object, --image-opts, -f, and -l require a filename argument.
+qemu-img: --object, --image-opts, -f, and -l require a filename argument.
+qemu-img: --object, --image-opts, -f, and -l require a filename argument.
+qemu-img: Invalid option list: ,
+qemu-img: Invalid parameter 'snapshot.foo'
+qemu-img: Failed in parsing snapshot param 'snapshot.foo'
+qemu-img: --output must be used with human or json as argument.
+qemu-img: Image size must be less than 8 EiB!
+qemu-img: Unknown file format 'foo'
+
+Size calculation
+
+required bytes: 2147483648
+fully allocated bytes: 2147483648
+required bytes: 1073741824
+fully allocated bytes: 1073741824
+{
+ "required-bytes": 2147483648,
+ "fully-allocated-bytes": 2147483648
+}
+{
+ "required-bytes": 1073741824,
+ "fully-allocated-bytes": 1073741824
+}
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 1f4bf03..846f962 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -168,3 +168,4 @@
173 rw auto
174 auto
175 auto quick
+178 auto quick
--
2.9.3
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [RFC v2 8/8] iotests: add test 178 for qemu-img measure
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 8/8] iotests: add test 178 for qemu-img measure Stefan Hajnoczi
@ 2017-03-18 21:04 ` Nir Soffer
2017-03-20 15:43 ` Stefan Hajnoczi
0 siblings, 1 reply; 30+ messages in thread
From: Nir Soffer @ 2017-03-18 21:04 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Kevin Wolf, Maor Lipchuk, Daniel P. Berrange, Eric Blake,
Alberto Garcia, John Snow
On Wed, Mar 15, 2017 at 11:30 AM Stefan Hajnoczi <stefanha@redhat.com>
wrote:
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> tests/qemu-iotests/178 | 75
> ++++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/178.out.qcow2 | 33 ++++++++++++++++++
> tests/qemu-iotests/178.out.raw | 33 ++++++++++++++++++
> tests/qemu-iotests/group | 1 +
> 4 files changed, 142 insertions(+)
> create mode 100755 tests/qemu-iotests/178
> create mode 100644 tests/qemu-iotests/178.out.qcow2
> create mode 100644 tests/qemu-iotests/178.out.raw
>
> diff --git a/tests/qemu-iotests/178 b/tests/qemu-iotests/178
> new file mode 100755
> index 0000000..8db1241
> --- /dev/null
> +++ b/tests/qemu-iotests/178
> @@ -0,0 +1,75 @@
> +#!/bin/bash
> +#
> +# qemu-img measure sub-command tests
> +#
> +# Copyright (C) 2017 Red Hat, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +# creator
> +owner=stefanha@redhat.com
> +
> +seq=`basename $0`
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +status=1 # failure is the default!
> +
> +_cleanup()
> +{
> + _cleanup_test_img
> +}
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +. ./common.pattern
> +
> +_supported_fmt raw qcow2
> +_supported_proto file
> +_supported_os Linux
> +
> +echo "Input validation"
>
Maybe:
echo "== Input validation =="
for nicer output?
> +echo
> +
> +_make_test_img 1G
> +
> +$QEMU_IMG measure # missing arguments
> +$QEMU_IMG measure --size 2G "$TEST_IMG" # only one allowed
> +$QEMU_IMG measure "$TEST_IMG" a # only one filename allowed
> +$QEMU_IMG measure --object secret,id=sec0,data=MTIzNDU2,format=base64 #
> missing filename
> +$QEMU_IMG measure --image-opts # missing filename
> +$QEMU_IMG measure -f qcow2 # missing filename
> +$QEMU_IMG measure -l snap1 # missing filename
> +$QEMU_IMG measure -o , # invalid option list
> +$QEMU_IMG measure -l snapshot.foo # invalid snapshot option
> +$QEMU_IMG measure --output foo # invalid output format
> +$QEMU_IMG measure --size -1 # invalid image size
> +$QEMU_IMG measure -O foo "$TEST_IMG" # unknown image file format
> +
> +echo
> +echo "Size calculation"
> +echo
> +
> +for ofmt in human json; do
> + $QEMU_IMG measure --output=$ofmt -O "$IMGFMT" --size 2G
>
--size 0 may be interesting test
> + $QEMU_IMG measure --output=$ofmt -f "$IMGFMT" -O "$IMGFMT" "$TEST_IMG"
>
This measure converting raw to raw or qcow2 to qcow2, not sure it is very
interesting, since in raw we don't need to measure, the size is always
the virtual size, and for converting qcow2 to qcow2 can use use the
source size.
The most interesting case is converting raw to qcow2, and we want to test:
- empty file
- file with some clusters allocated
- file with all clusters allocated
> +done
> +
> +# success, all done
> +echo "*** done"
> +rm -f $seq.full
> +status=0
> diff --git a/tests/qemu-iotests/178.out.qcow2
> b/tests/qemu-iotests/178.out.qcow2
> new file mode 100644
> index 0000000..3fe3f5f
> --- /dev/null
> +++ b/tests/qemu-iotests/178.out.qcow2
> @@ -0,0 +1,33 @@
> +QA output created by 178
> +Input validation
> +
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
> +qemu-img: Either --size N or one filename must be specified.
> +qemu-img: --size N cannot be used together with a filename.
> +qemu-img: At most one filename argument is allowed.
> +qemu-img: --object, --image-opts, -f, and -l require a filename argument.
> +qemu-img: --object, --image-opts, -f, and -l require a filename argument.
> +qemu-img: --object, --image-opts, -f, and -l require a filename argument.
> +qemu-img: --object, --image-opts, -f, and -l require a filename argument.
> +qemu-img: Invalid option list: ,
> +qemu-img: Invalid parameter 'snapshot.foo'
> +qemu-img: Failed in parsing snapshot param 'snapshot.foo'
> +qemu-img: --output must be used with human or json as argument.
> +qemu-img: Image size must be less than 8 EiB!
> +qemu-img: Unknown file format 'foo'
> +
> +Size calculation
> +
> +required bytes: 589824
> +fully allocated bytes: 2148073472 <(214)%20807-3472>
> +required bytes: 327680
> +fully allocated bytes: 1074069504
> +{
> + "required-bytes": 589824,
> + "fully-allocated-bytes": 2148073472 <(214)%20807-3472>
> +}
> +{
> + "required-bytes": 327680,
> + "fully-allocated-bytes": 1074069504
> +}
> +*** done
> diff --git a/tests/qemu-iotests/178.out.raw
> b/tests/qemu-iotests/178.out.raw
> new file mode 100644
> index 0000000..4b89dd0
> --- /dev/null
> +++ b/tests/qemu-iotests/178.out.raw
> @@ -0,0 +1,33 @@
> +QA output created by 178
> +Input validation
> +
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
> +qemu-img: Either --size N or one filename must be specified.
> +qemu-img: --size N cannot be used together with a filename.
> +qemu-img: At most one filename argument is allowed.
> +qemu-img: --object, --image-opts, -f, and -l require a filename argument.
> +qemu-img: --object, --image-opts, -f, and -l require a filename argument.
> +qemu-img: --object, --image-opts, -f, and -l require a filename argument.
> +qemu-img: --object, --image-opts, -f, and -l require a filename argument.
> +qemu-img: Invalid option list: ,
> +qemu-img: Invalid parameter 'snapshot.foo'
> +qemu-img: Failed in parsing snapshot param 'snapshot.foo'
> +qemu-img: --output must be used with human or json as argument.
> +qemu-img: Image size must be less than 8 EiB!
> +qemu-img: Unknown file format 'foo'
> +
> +Size calculation
> +
> +required bytes: 2147483648 <(214)%20748-3648>
> +fully allocated bytes: 2147483648 <(214)%20748-3648>
> +required bytes: 1073741824
> +fully allocated bytes: 1073741824
> +{
> + "required-bytes": 2147483648 <(214)%20748-3648>,
> + "fully-allocated-bytes": 2147483648 <(214)%20748-3648>
> +}
> +{
> + "required-bytes": 1073741824,
> + "fully-allocated-bytes": 1073741824
> +}
> +*** done
> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> index 1f4bf03..846f962 100644
> --- a/tests/qemu-iotests/group
> +++ b/tests/qemu-iotests/group
> @@ -168,3 +168,4 @@
> 173 rw auto
> 174 auto
> 175 auto quick
> +178 auto quick
> --
> 2.9.3
>
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [RFC v2 8/8] iotests: add test 178 for qemu-img measure
2017-03-18 21:04 ` Nir Soffer
@ 2017-03-20 15:43 ` Stefan Hajnoczi
0 siblings, 0 replies; 30+ messages in thread
From: Stefan Hajnoczi @ 2017-03-20 15:43 UTC (permalink / raw)
To: Nir Soffer
Cc: qemu-devel, Kevin Wolf, Maor Lipchuk, Daniel P. Berrange,
Eric Blake, Alberto Garcia, John Snow
[-- Attachment #1: Type: text/plain, Size: 4063 bytes --]
On Sat, Mar 18, 2017 at 09:04:59PM +0000, Nir Soffer wrote:
> On Wed, Mar 15, 2017 at 11:30 AM Stefan Hajnoczi <stefanha@redhat.com>
> wrote:
>
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> > tests/qemu-iotests/178 | 75
> > ++++++++++++++++++++++++++++++++++++++++
> > tests/qemu-iotests/178.out.qcow2 | 33 ++++++++++++++++++
> > tests/qemu-iotests/178.out.raw | 33 ++++++++++++++++++
> > tests/qemu-iotests/group | 1 +
> > 4 files changed, 142 insertions(+)
> > create mode 100755 tests/qemu-iotests/178
> > create mode 100644 tests/qemu-iotests/178.out.qcow2
> > create mode 100644 tests/qemu-iotests/178.out.raw
> >
> > diff --git a/tests/qemu-iotests/178 b/tests/qemu-iotests/178
> > new file mode 100755
> > index 0000000..8db1241
> > --- /dev/null
> > +++ b/tests/qemu-iotests/178
> > @@ -0,0 +1,75 @@
> > +#!/bin/bash
> > +#
> > +# qemu-img measure sub-command tests
> > +#
> > +# Copyright (C) 2017 Red Hat, Inc.
> > +#
> > +# This program is free software; you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation; either version 2 of the License, or
> > +# (at your option) any later version.
> > +#
> > +# This program is distributed in the hope that it will be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> > +#
> > +
> > +# creator
> > +owner=stefanha@redhat.com
> > +
> > +seq=`basename $0`
> > +echo "QA output created by $seq"
> > +
> > +here=`pwd`
> > +status=1 # failure is the default!
> > +
> > +_cleanup()
> > +{
> > + _cleanup_test_img
> > +}
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +# get standard environment, filters and checks
> > +. ./common.rc
> > +. ./common.filter
> > +. ./common.pattern
> > +
> > +_supported_fmt raw qcow2
> > +_supported_proto file
> > +_supported_os Linux
> > +
> > +echo "Input validation"
> >
>
> Maybe:
>
> echo "== Input validation =="
>
> for nicer output?
Will fix. Other test cases also use "== Title ==" so it should be
changed for consistency.
> > +echo
> > +
> > +_make_test_img 1G
> > +
> > +$QEMU_IMG measure # missing arguments
> > +$QEMU_IMG measure --size 2G "$TEST_IMG" # only one allowed
> > +$QEMU_IMG measure "$TEST_IMG" a # only one filename allowed
> > +$QEMU_IMG measure --object secret,id=sec0,data=MTIzNDU2,format=base64 #
> > missing filename
> > +$QEMU_IMG measure --image-opts # missing filename
> > +$QEMU_IMG measure -f qcow2 # missing filename
> > +$QEMU_IMG measure -l snap1 # missing filename
> > +$QEMU_IMG measure -o , # invalid option list
> > +$QEMU_IMG measure -l snapshot.foo # invalid snapshot option
> > +$QEMU_IMG measure --output foo # invalid output format
> > +$QEMU_IMG measure --size -1 # invalid image size
> > +$QEMU_IMG measure -O foo "$TEST_IMG" # unknown image file format
> > +
> > +echo
> > +echo "Size calculation"
> > +echo
> > +
> > +for ofmt in human json; do
> > + $QEMU_IMG measure --output=$ofmt -O "$IMGFMT" --size 2G
> >
>
> --size 0 may be interesting test
Will fix.
> > + $QEMU_IMG measure --output=$ofmt -f "$IMGFMT" -O "$IMGFMT" "$TEST_IMG"
> >
>
> This measure converting raw to raw or qcow2 to qcow2, not sure it is very
> interesting, since in raw we don't need to measure, the size is always
> the virtual size, and for converting qcow2 to qcow2 can use use the
> source size.
>
> The most interesting case is converting raw to qcow2, and we want to test:
> - empty file
> - file with some clusters allocated
> - file with all clusters allocated
I'll extend the test to include both raw -> $IMGFMT and $IMGFMT ->
$IMGFMT.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC v2 0/8] qemu-img: add measure sub-command
2017-03-15 9:29 [Qemu-devel] [RFC v2 0/8] qemu-img: add measure sub-command Stefan Hajnoczi
` (7 preceding siblings ...)
2017-03-15 9:29 ` [Qemu-devel] [RFC v2 8/8] iotests: add test 178 for qemu-img measure Stefan Hajnoczi
@ 2017-03-17 23:45 ` Nir Soffer
2017-03-20 15:51 ` Stefan Hajnoczi
8 siblings, 1 reply; 30+ messages in thread
From: Nir Soffer @ 2017-03-17 23:45 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Kevin Wolf, Maor Lipchuk, Daniel P. Berrange, Eric Blake,
Alberto Garcia, John Snow
On Wed, Mar 15, 2017 at 11:29 AM Stefan Hajnoczi <stefanha@redhat.com>
wrote:
> RFCv2:
> * Publishing RFC again to discuss the new user-visible interfaces. Code
> has
> changed quite a bit, I have not kept any Reviewed-by tags.
> * Rename qemu-img sub-command "measure" and API bdrv_measure() [Nir]
> * Report both "required bytes" and "fully allocated bytes" to handle the
> empty
> image file and prealloc use cases [Nir and Dan]
> * Use bdrv_getlength() instead of bdrv_nb_sectors() [Berto]
> * Rename "err" label "out" in qemu-img-cmds.c [Nir]
> * Add basic qcow2 support, doesn't support qemu-img convert from existing
> files yet
>
> RFCv1:
> * Publishing patch series with just raw support, no qcow2 yet. Please
> review
> the command-line interface and let me know if you are happy with this
> approach.
>
> Users and management tools sometimes need to know the size required for a
> new
> disk image so that an LVM volume, SAN LUN, etc can be allocated ahead of
> time.
> Image formats like qcow2 have non-trivial metadata that makes it hard to
> estimate the exact size without knowledge of file format internals.
>
> This patch series introduces a new qemu-img sub-command that calculates the
> required size for both image creation and conversion scenarios.
> The conversion scenario is:
>
> $ qemu-img measure -f raw -O qcow2 input.img
> required bytes: 1327680
> fully allocated bytes: 1074069504
Other commands like qemu-img info are using "size" instead of "bytes", I
guess
we like to keep the term "size" for consistency.
Here an existing image file is taken and the output includes the space
> required
> for data from the input image file.
>
> The creation scenario is:
>
> $ qemu-img measure -O qcow2 --size 5G
> required bytes: 327680
> fully allocated bytes: 1074069504
>
There is a third scenario that you may want to describe
here, extending a block device when it becomes full. In this case
we want to limit the block device to the fully allocated size, including
additional size needed for bitmaps. Currently ovirt is using virtual_size *
1.1
for this limit, which seems to work for images without additional bitmaps.
> Stefan Hajnoczi (8):
> block: add bdrv_measure() API
> raw-format: add bdrv_measure() support
> qcow2: extract preallocation calculation function
> qcow2: extract image creation option parsing
> qcow2: add bdrv_measure() support
> qemu-img: add measure subcommand
> qemu-iotests: support per-format golden output files
> iotests: add test 178 for qemu-img measure
>
> qapi/block-core.json | 19 +++
> include/block/block.h | 4 +
> include/block/block_int.h | 2 +
> block.c | 33 +++++
> block/qcow2.c | 296
> ++++++++++++++++++++++++++-------------
> block/raw-format.c | 22 +++
> qemu-img.c | 213 ++++++++++++++++++++++++++++
> qemu-img-cmds.hx | 9 ++
> tests/qemu-iotests/178 | 75 ++++++++++
> tests/qemu-iotests/178.out.qcow2 | 33 +++++
> tests/qemu-iotests/178.out.raw | 33 +++++
> tests/qemu-iotests/check | 5 +
> tests/qemu-iotests/group | 1 +
> 13 files changed, 651 insertions(+), 94 deletions(-)
> create mode 100755 tests/qemu-iotests/178
> create mode 100644 tests/qemu-iotests/178.out.qcow2
> create mode 100644 tests/qemu-iotests/178.out.raw
>
> --
> 2.9.3
>
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [RFC v2 0/8] qemu-img: add measure sub-command
2017-03-17 23:45 ` [Qemu-devel] [RFC v2 0/8] qemu-img: add measure sub-command Nir Soffer
@ 2017-03-20 15:51 ` Stefan Hajnoczi
0 siblings, 0 replies; 30+ messages in thread
From: Stefan Hajnoczi @ 2017-03-20 15:51 UTC (permalink / raw)
To: Nir Soffer
Cc: qemu-devel, Kevin Wolf, Maor Lipchuk, Daniel P. Berrange,
Eric Blake, Alberto Garcia, John Snow
[-- Attachment #1: Type: text/plain, Size: 3159 bytes --]
On Fri, Mar 17, 2017 at 11:45:20PM +0000, Nir Soffer wrote:
> On Wed, Mar 15, 2017 at 11:29 AM Stefan Hajnoczi <stefanha@redhat.com>
> wrote:
>
> > RFCv2:
> > * Publishing RFC again to discuss the new user-visible interfaces. Code
> > has
> > changed quite a bit, I have not kept any Reviewed-by tags.
> > * Rename qemu-img sub-command "measure" and API bdrv_measure() [Nir]
> > * Report both "required bytes" and "fully allocated bytes" to handle the
> > empty
> > image file and prealloc use cases [Nir and Dan]
> > * Use bdrv_getlength() instead of bdrv_nb_sectors() [Berto]
> > * Rename "err" label "out" in qemu-img-cmds.c [Nir]
> > * Add basic qcow2 support, doesn't support qemu-img convert from existing
> > files yet
> >
> > RFCv1:
> > * Publishing patch series with just raw support, no qcow2 yet. Please
> > review
> > the command-line interface and let me know if you are happy with this
> > approach.
> >
> > Users and management tools sometimes need to know the size required for a
> > new
> > disk image so that an LVM volume, SAN LUN, etc can be allocated ahead of
> > time.
> > Image formats like qcow2 have non-trivial metadata that makes it hard to
> > estimate the exact size without knowledge of file format internals.
> >
> > This patch series introduces a new qemu-img sub-command that calculates the
> > required size for both image creation and conversion scenarios.
>
>
> > The conversion scenario is:
> >
> > $ qemu-img measure -f raw -O qcow2 input.img
> > required bytes: 1327680
> > fully allocated bytes: 1074069504
>
>
> Other commands like qemu-img info are using "size" instead of "bytes", I
> guess
> we like to keep the term "size" for consistency.
Will fix.
> Here an existing image file is taken and the output includes the space
> > required
> > for data from the input image file.
> >
> > The creation scenario is:
> >
> > $ qemu-img measure -O qcow2 --size 5G
> > required bytes: 327680
> > fully allocated bytes: 1074069504
> >
>
> There is a third scenario that you may want to describe
> here, extending a block device when it becomes full. In this case
> we want to limit the block device to the fully allocated size, including
> additional size needed for bitmaps. Currently ovirt is using virtual_size *
> 1.1
> for this limit, which seems to work for images without additional bitmaps.
Please elaborate, I'm not sure which use case you mean:
1. The user wishes to grow the virtual disk size because the guest has
filled up the disk.
2. The block device was thinly provisioned and needs to grow so that the
guest can write more data. In this case the virtual disk size stays
constant.
Case #1 is the qemu-img convert case. We have an existing file with
some data and the new image will have a larger virtual disk size.
(Nevermind that no actual new image needs to be created.)
Regarding case #2, the management tool doesn't need qemu-img measure.
It just increments the block device size by an arbitrary amount (e.g. 1
GB) whenever the write threshold is reached.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread