* [PATCH v2 0/2] qemu-img: Add --target-is-zero to indicate that a target is blank
@ 2020-01-24 10:34 David Edmondson
2020-01-24 10:34 ` [PATCH v2 1/2] qemu-img: Add --target-is-zero to convert David Edmondson
2020-01-24 10:34 ` [PATCH v2 2/2] doc: Use @code rather than @var for options David Edmondson
0 siblings, 2 replies; 11+ messages in thread
From: David Edmondson @ 2020-01-24 10:34 UTC (permalink / raw)
To: qemu-devel, qemu-block; +Cc: David Edmondson
qemu-img: Add --target-is-zero to indicate that a target is blank
v2:
- Remove target_is_zero, preferring to set has_zero_init
directly (Mark Kanda).
- Disallow --target-is-zero in the presence of a backing file (Max
Reitz).
- Add relevant documentation (Max Reitz).
- @var -> @code for options in qemu-img.texi.
David Edmondson (2):
qemu-img: Add --target-is-zero to convert
doc: Use @code rather than @var for options
qemu-img-cmds.hx | 4 ++--
qemu-img.c | 25 ++++++++++++++++++++++---
qemu-img.texi | 20 ++++++++++++--------
3 files changed, 36 insertions(+), 13 deletions(-)
--
2.24.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/2] qemu-img: Add --target-is-zero to convert
2020-01-24 10:34 [PATCH v2 0/2] qemu-img: Add --target-is-zero to indicate that a target is blank David Edmondson
@ 2020-01-24 10:34 ` David Edmondson
2020-01-27 22:39 ` Eric Blake
2020-02-03 18:20 ` Vladimir Sementsov-Ogievskiy
2020-01-24 10:34 ` [PATCH v2 2/2] doc: Use @code rather than @var for options David Edmondson
1 sibling, 2 replies; 11+ messages in thread
From: David Edmondson @ 2020-01-24 10:34 UTC (permalink / raw)
To: qemu-devel, qemu-block; +Cc: David Edmondson
In many cases the target of a convert operation is a newly provisioned
target that the user knows is blank (filled with zeroes). In this
situation there is no requirement for qemu-img to wastefully zero out
the entire device.
Add a new option, --target-is-zero, allowing the user to indicate that
an existing target device is already zero filled.
Signed-off-by: David Edmondson <david.edmondson@oracle.com>
---
qemu-img-cmds.hx | 4 ++--
qemu-img.c | 25 ++++++++++++++++++++++---
qemu-img.texi | 4 ++++
3 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 1c93e6d185..6f958a0a48 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -44,9 +44,9 @@ STEXI
ETEXI
DEF("convert", img_convert,
- "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] [--salvage] filename [filename2 [...]] output_filename")
+ "convert [--object objectdef] [--image-opts] [--target-image-opts] [--target-is-zero] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] [--salvage] filename [filename2 [...]] output_filename")
STEXI
-@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] [--salvage] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [--target-is-zero] [-U] [-C] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] [--salvage] @var{filename} [@var{filename2} [...]] @var{output_filename}
ETEXI
DEF("create", img_create,
diff --git a/qemu-img.c b/qemu-img.c
index 6233b8ca56..46db72dbe0 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -70,6 +70,7 @@ enum {
OPTION_PREALLOCATION = 265,
OPTION_SHRINK = 266,
OPTION_SALVAGE = 267,
+ OPTION_TARGET_IS_ZERO = 268,
};
typedef enum OutputFormat {
@@ -1984,10 +1985,9 @@ static int convert_do_copy(ImgConvertState *s)
int64_t sector_num = 0;
/* Check whether we have zero initialisation or can get it efficiently */
- if (s->target_is_new && s->min_sparse && !s->target_has_backing) {
+ if (!s->has_zero_init && s->target_is_new && s->min_sparse &&
+ !s->target_has_backing) {
s->has_zero_init = bdrv_has_zero_init(blk_bs(s->target));
- } else {
- s->has_zero_init = false;
}
if (!s->has_zero_init && !s->target_has_backing &&
@@ -2086,6 +2086,7 @@ static int img_convert(int argc, char **argv)
{"force-share", no_argument, 0, 'U'},
{"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS},
{"salvage", no_argument, 0, OPTION_SALVAGE},
+ {"target-is-zero", no_argument, 0, OPTION_TARGET_IS_ZERO},
{0, 0, 0, 0}
};
c = getopt_long(argc, argv, ":hf:O:B:Cco:l:S:pt:T:qnm:WU",
@@ -2209,6 +2210,14 @@ static int img_convert(int argc, char **argv)
case OPTION_TARGET_IMAGE_OPTS:
tgt_image_opts = true;
break;
+ case OPTION_TARGET_IS_ZERO:
+ /*
+ * The user asserting that the target is blank has the
+ * same effect as the target driver supporting zero
+ * initialisation.
+ */
+ s.has_zero_init = true;
+ break;
}
}
@@ -2247,6 +2256,11 @@ static int img_convert(int argc, char **argv)
warn_report("This will become an error in future QEMU versions.");
}
+ if (s.has_zero_init && !skip_create) {
+ error_report("--target-is-zero requires use of -n flag");
+ goto fail_getopt;
+ }
+
s.src_num = argc - optind - 1;
out_filename = s.src_num >= 1 ? argv[argc - 1] : NULL;
@@ -2380,6 +2394,11 @@ static int img_convert(int argc, char **argv)
}
s.target_has_backing = (bool) out_baseimg;
+ if (s.has_zero_init && s.target_has_backing) {
+ error_report("Cannot use --target-is-zero with a backing file");
+ goto out;
+ }
+
if (s.src_num > 1 && out_baseimg) {
error_report("Having a backing file for the target makes no sense when "
"concatenating multiple input images");
diff --git a/qemu-img.texi b/qemu-img.texi
index b5156d6316..3b6dfd8682 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -179,6 +179,10 @@ information.
Try to ignore I/O errors when reading. Unless in quiet mode (@code{-q}), errors
will still be printed. Areas that cannot be read from the source will be
treated as containing only zeroes.
+@item --target-is-zero
+Assume that the destination is filled with zeros. This parameter is
+mutually exclusive with the use of a backing file. It is required to
+also use the @code{-n} parameter to skip image creation.
@end table
Parameters to dd subcommand:
--
2.24.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/2] doc: Use @code rather than @var for options
2020-01-24 10:34 [PATCH v2 0/2] qemu-img: Add --target-is-zero to indicate that a target is blank David Edmondson
2020-01-24 10:34 ` [PATCH v2 1/2] qemu-img: Add --target-is-zero to convert David Edmondson
@ 2020-01-24 10:34 ` David Edmondson
2020-01-27 19:31 ` Eric Blake
1 sibling, 1 reply; 11+ messages in thread
From: David Edmondson @ 2020-01-24 10:34 UTC (permalink / raw)
To: qemu-devel, qemu-block; +Cc: David Edmondson
Texinfo defines @var for metasyntactic variables and such terms are
shown in upper-case or italics in the output of makeinfo. When
considering an option to a command, such as "-n", upper-casing is
undesirable as it may confuse the reader or be in conflict with the
equivalent upper-case option.
Replace the use of @var for options with @code to avoid this.
Signed-off-by: David Edmondson <david.edmondson@oracle.com>
---
qemu-img.texi | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/qemu-img.texi b/qemu-img.texi
index 3b6dfd8682..6b4a1ac961 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -74,13 +74,13 @@ keys.
@item --image-opts
Indicates that the source @var{filename} parameter is to be interpreted as a
full option string, not a plain filename. This parameter is mutually
-exclusive with the @var{-f} parameter.
+exclusive with the @code{-f} parameter.
@item --target-image-opts
Indicates that the @var{output_filename} parameter(s) are to be interpreted as
a full option string, not a plain filename. This parameter is mutually
-exclusive with the @var{-O} parameters. It is currently required to also use
-the @var{-n} parameter to skip image creation. This restriction may be relaxed
+exclusive with the @code{-O} parameters. It is currently required to also use
+the @code{-n} parameter to skip image creation. This restriction may be relaxed
in a future release.
@item --force-share (-U)
@@ -103,13 +103,13 @@ with or without a command shows help and lists the supported formats
@item -p
display progress bar (compare, convert and rebase commands only).
-If the @var{-p} option is not used for a command that supports it, the
+If the @code{-p} option is not used for a command that supports it, the
progress is reported when the process receives a @code{SIGUSR1} or
@code{SIGINFO} signal.
@item -q
Quiet mode - do not print any output (except errors). There's no progress bar
-in case both @var{-q} and @var{-p} options are used.
+in case both @code{-q} and @code{-p} options are used.
@item -S @var{size}
indicates the consecutive number of bytes that must contain only zeros
@@ -298,14 +298,14 @@ the top image stays valid).
Check if two images have the same content. You can compare images with
different format or settings.
-The format is probed unless you specify it by @var{-f} (used for
-@var{filename1}) and/or @var{-F} (used for @var{filename2}) option.
+The format is probed unless you specify it by @code{-f} (used for
+@var{filename1}) and/or @code{-F} (used for @var{filename2}) option.
By default, images with different size are considered identical if the larger
image contains only unallocated and/or zeroed sectors in the area after the end
of the other image. In addition, if any sector is not allocated in one image
and contains only zero bytes in the second one, it is evaluated as equal. You
-can use Strict mode by specifying the @var{-s} option. When compare runs in
+can use Strict mode by specifying the @code{-s} option. When compare runs in
Strict mode, it fails in case image size differs or a sector is allocated in
one image and is not allocated in the second one.
--
2.24.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] doc: Use @code rather than @var for options
2020-01-24 10:34 ` [PATCH v2 2/2] doc: Use @code rather than @var for options David Edmondson
@ 2020-01-27 19:31 ` Eric Blake
2020-01-28 7:39 ` David Edmondson
0 siblings, 1 reply; 11+ messages in thread
From: Eric Blake @ 2020-01-27 19:31 UTC (permalink / raw)
To: David Edmondson, qemu-devel, qemu-block, Peter Maydell
On 1/24/20 4:34 AM, David Edmondson wrote:
> Texinfo defines @var for metasyntactic variables and such terms are
> shown in upper-case or italics in the output of makeinfo. When
> considering an option to a command, such as "-n", upper-casing is
> undesirable as it may confuse the reader or be in conflict with the
> equivalent upper-case option.
>
> Replace the use of @var for options with @code to avoid this.
>
> Signed-off-by: David Edmondson <david.edmondson@oracle.com>
> ---
> qemu-img.texi | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
Is this patch still needed given Peter's recent push to move to rST
documentation?
>
> diff --git a/qemu-img.texi b/qemu-img.texi
> index 3b6dfd8682..6b4a1ac961 100644
> --- a/qemu-img.texi
> +++ b/qemu-img.texi
> @@ -74,13 +74,13 @@ keys.
> @item --image-opts
> Indicates that the source @var{filename} parameter is to be interpreted as a
> full option string, not a plain filename. This parameter is mutually
> -exclusive with the @var{-f} parameter.
> +exclusive with the @code{-f} parameter.
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] qemu-img: Add --target-is-zero to convert
2020-01-24 10:34 ` [PATCH v2 1/2] qemu-img: Add --target-is-zero to convert David Edmondson
@ 2020-01-27 22:39 ` Eric Blake
2020-01-28 7:46 ` David Edmondson
2020-02-03 18:20 ` Vladimir Sementsov-Ogievskiy
1 sibling, 1 reply; 11+ messages in thread
From: Eric Blake @ 2020-01-27 22:39 UTC (permalink / raw)
To: David Edmondson, qemu-devel, qemu-block
On 1/24/20 4:34 AM, David Edmondson wrote:
> In many cases the target of a convert operation is a newly provisioned
> target that the user knows is blank (filled with zeroes). In this
> situation there is no requirement for qemu-img to wastefully zero out
> the entire device.
>
> Add a new option, --target-is-zero, allowing the user to indicate that
> an existing target device is already zero filled.
>
> Signed-off-by: David Edmondson <david.edmondson@oracle.com>
> ---
> qemu-img-cmds.hx | 4 ++--
> qemu-img.c | 25 ++++++++++++++++++++++---
> qemu-img.texi | 4 ++++
> 3 files changed, 28 insertions(+), 5 deletions(-)
I'm working up a patch series that tries to auto-set this flag without
user interaction where possible (for example, if lseek(fd, 0, SEEK_DATA)
returns EOF, or if fstat() reports 0 blocks allocated, or if qcow2 sees
no L2 tables allocated, or a proposed extension to NBD passes on the
same...). I may rebase my series on top of your patch and tweak things
in yours accordingly.
But as it stands, the idea makes sense to me; even if we add ways for
some images to efficiently report initial state (and our existing
bdrv_has_zero_init() is NOT such a method), there are enough other
scenarios where the knob will be the only way to let qemu-img know the
intent.
> + case OPTION_TARGET_IS_ZERO:
> + /*
> + * The user asserting that the target is blank has the
> + * same effect as the target driver supporting zero
> + * initialisation.
Hmm. A git grep shows that 'initialization' has 200 hits,
'initialisation' has only 29. But I think it's a US vs. UK thing, so I
don't care which spelling you use.
> @@ -2247,6 +2256,11 @@ static int img_convert(int argc, char **argv)
> warn_report("This will become an error in future QEMU versions.");
> }
>
> + if (s.has_zero_init && !skip_create) {
> + error_report("--target-is-zero requires use of -n flag");
> + goto fail_getopt;
> + }
> +
Makes sense, although we could perhaps relax it to also work even when
the -n flag is supplied IF the destination image supports my proposal
for a new status bit set when an image is known to be opened with all
zero content.
> s.src_num = argc - optind - 1;
> out_filename = s.src_num >= 1 ? argv[argc - 1] : NULL;
>
> @@ -2380,6 +2394,11 @@ static int img_convert(int argc, char **argv)
> }
> s.target_has_backing = (bool) out_baseimg;
>
> + if (s.has_zero_init && s.target_has_backing) {
> + error_report("Cannot use --target-is-zero with a backing file");
> + goto out;
> + }
> +
Makes sense, although we could perhaps relax it to also work even when
there is a backing file IF the backing file supports my proposal for a
new status bit set when an image is known to be opened with all zero
content.
As my patch proposal is still not submitted, I'm fine if yours lands as-is:
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] doc: Use @code rather than @var for options
2020-01-27 19:31 ` Eric Blake
@ 2020-01-28 7:39 ` David Edmondson
2020-01-28 9:10 ` Peter Maydell
0 siblings, 1 reply; 11+ messages in thread
From: David Edmondson @ 2020-01-28 7:39 UTC (permalink / raw)
To: Eric Blake, qemu-devel, qemu-block, Peter Maydell
Eric Blake <eblake@redhat.com> writes:
> On 1/24/20 4:34 AM, David Edmondson wrote:
>> Texinfo defines @var for metasyntactic variables and such terms are
>> shown in upper-case or italics in the output of makeinfo. When
>> considering an option to a command, such as "-n", upper-casing is
>> undesirable as it may confuse the reader or be in conflict with the
>> equivalent upper-case option.
>>
>> Replace the use of @var for options with @code to avoid this.
>>
>> Signed-off-by: David Edmondson <david.edmondson@oracle.com>
>> ---
>> qemu-img.texi | 16 ++++++++--------
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> Is this patch still needed given Peter's recent push to move to rST
> documentation?
No, it would be obviated by those changes.
>>
>> diff --git a/qemu-img.texi b/qemu-img.texi
>> index 3b6dfd8682..6b4a1ac961 100644
>> --- a/qemu-img.texi
>> +++ b/qemu-img.texi
>> @@ -74,13 +74,13 @@ keys.
>> @item --image-opts
>> Indicates that the source @var{filename} parameter is to be interpreted as a
>> full option string, not a plain filename. This parameter is mutually
>> -exclusive with the @var{-f} parameter.
>> +exclusive with the @code{-f} parameter.
>>
>
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3226
> Virtualization: qemu.org | libvirt.org
dme.
--
Don't you know you're never going to get to France.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] qemu-img: Add --target-is-zero to convert
2020-01-27 22:39 ` Eric Blake
@ 2020-01-28 7:46 ` David Edmondson
0 siblings, 0 replies; 11+ messages in thread
From: David Edmondson @ 2020-01-28 7:46 UTC (permalink / raw)
To: Eric Blake, qemu-devel, qemu-block
Eric Blake <eblake@redhat.com> writes:
> On 1/24/20 4:34 AM, David Edmondson wrote:
>> In many cases the target of a convert operation is a newly provisioned
>> target that the user knows is blank (filled with zeroes). In this
>> situation there is no requirement for qemu-img to wastefully zero out
>> the entire device.
>>
>> Add a new option, --target-is-zero, allowing the user to indicate that
>> an existing target device is already zero filled.
>>
>> Signed-off-by: David Edmondson <david.edmondson@oracle.com>
>> ---
>> qemu-img-cmds.hx | 4 ++--
>> qemu-img.c | 25 ++++++++++++++++++++++---
>> qemu-img.texi | 4 ++++
>> 3 files changed, 28 insertions(+), 5 deletions(-)
>
> I'm working up a patch series that tries to auto-set this flag without
> user interaction where possible (for example, if lseek(fd, 0, SEEK_DATA)
> returns EOF, or if fstat() reports 0 blocks allocated, or if qcow2 sees
> no L2 tables allocated, or a proposed extension to NBD passes on the
> same...). I may rebase my series on top of your patch and tweak things
> in yours accordingly.
>
> But as it stands, the idea makes sense to me; even if we add ways for
> some images to efficiently report initial state (and our existing
> bdrv_has_zero_init() is NOT such a method), there are enough other
> scenarios where the knob will be the only way to let qemu-img know the
> intent.
Having qemu-img figure things out on its own is obviously desirable, but
I agree that there are enough cases where this won't be possible and,
given the resulting performance improvement, it will still be useful to
allow the caller to force things.
>> + case OPTION_TARGET_IS_ZERO:
>> + /*
>> + * The user asserting that the target is blank has the
>> + * same effect as the target driver supporting zero
>> + * initialisation.
>
> Hmm. A git grep shows that 'initialization' has 200 hits,
> 'initialisation' has only 29. But I think it's a US vs. UK thing, so I
> don't care which spelling you use.
Yes, it's British English spelling. It was unconscious - I'll switch if
there is an existing policy.
> Reviewed-by: Eric Blake <eblake@redhat.com>
Thanks.
If the conversion of the documentation to rST is imminent then I'll wait
for that before submitting a followup with corresponding changes applied
to the new docs.
dme.
--
I'd come on over but I haven't got a raincoat.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] doc: Use @code rather than @var for options
2020-01-28 7:39 ` David Edmondson
@ 2020-01-28 9:10 ` Peter Maydell
0 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2020-01-28 9:10 UTC (permalink / raw)
To: David Edmondson; +Cc: QEMU Developers, Qemu-block
On Tue, 28 Jan 2020 at 07:39, David Edmondson <dme@dme.org> wrote:
>
> Eric Blake <eblake@redhat.com> writes:
>
> > On 1/24/20 4:34 AM, David Edmondson wrote:
> >> Texinfo defines @var for metasyntactic variables and such terms are
> >> shown in upper-case or italics in the output of makeinfo. When
> >> considering an option to a command, such as "-n", upper-casing is
> >> undesirable as it may confuse the reader or be in conflict with the
> >> equivalent upper-case option.
> >>
> >> Replace the use of @var for options with @code to avoid this.
> >>
> >> Signed-off-by: David Edmondson <david.edmondson@oracle.com>
> >> ---
> >> qemu-img.texi | 16 ++++++++--------
> >> 1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > Is this patch still needed given Peter's recent push to move to rST
> > documentation?
>
> No, it would be obviated by those changes.
Yeah; in particular the rST formatting corrects this minor
inconsistency. (I noticed it because my emacs find-and-replace
operations which turn @var{foo} into *FOO* were incorrectly
turning the @var{-s} into *-S*, which they wouldn't have done
if it were @code{-s}...)
thanks
-- PMM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] qemu-img: Add --target-is-zero to convert
2020-01-24 10:34 ` [PATCH v2 1/2] qemu-img: Add --target-is-zero to convert David Edmondson
2020-01-27 22:39 ` Eric Blake
@ 2020-02-03 18:20 ` Vladimir Sementsov-Ogievskiy
2020-02-03 19:00 ` Eric Blake
2020-02-04 9:54 ` David Edmondson
1 sibling, 2 replies; 11+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-02-03 18:20 UTC (permalink / raw)
To: David Edmondson, qemu-devel, qemu-block
24.01.2020 13:34, David Edmondson wrote:
> In many cases the target of a convert operation is a newly provisioned
> target that the user knows is blank (filled with zeroes). In this
> situation there is no requirement for qemu-img to wastefully zero out
> the entire device.
>
> Add a new option, --target-is-zero, allowing the user to indicate that
> an existing target device is already zero filled.
Hi! qemu-img.c part looks OK for me, but other doesn't apply on master now.
I like this thing, and I'd like to make similar option for backup job.
>
> Signed-off-by: David Edmondson <david.edmondson@oracle.com>
> ---
> qemu-img-cmds.hx | 4 ++--
> qemu-img.c | 25 ++++++++++++++++++++++---
> qemu-img.texi | 4 ++++
> 3 files changed, 28 insertions(+), 5 deletions(-)
>
> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
> index 1c93e6d185..6f958a0a48 100644
> --- a/qemu-img-cmds.hx
> +++ b/qemu-img-cmds.hx
> @@ -44,9 +44,9 @@ STEXI
> ETEXI
>
> DEF("convert", img_convert,
> - "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] [--salvage] filename [filename2 [...]] output_filename")
> + "convert [--object objectdef] [--image-opts] [--target-image-opts] [--target-is-zero] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] [--salvage] filename [filename2 [...]] output_filename")
> STEXI
> -@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] [--salvage] @var{filename} [@var{filename2} [...]] @var{output_filename}
> +@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [--target-is-zero] [-U] [-C] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] [--salvage] @var{filename} [@var{filename2} [...]] @var{output_filename}
> ETEXI
>
> DEF("create", img_create,
> diff --git a/qemu-img.c b/qemu-img.c
> index 6233b8ca56..46db72dbe0 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -70,6 +70,7 @@ enum {
> OPTION_PREALLOCATION = 265,
> OPTION_SHRINK = 266,
> OPTION_SALVAGE = 267,
> + OPTION_TARGET_IS_ZERO = 268,
> };
>
> typedef enum OutputFormat {
> @@ -1984,10 +1985,9 @@ static int convert_do_copy(ImgConvertState *s)
> int64_t sector_num = 0;
>
> /* Check whether we have zero initialisation or can get it efficiently */
> - if (s->target_is_new && s->min_sparse && !s->target_has_backing) {
> + if (!s->has_zero_init && s->target_is_new && s->min_sparse &&
> + !s->target_has_backing) {
> s->has_zero_init = bdrv_has_zero_init(blk_bs(s->target));
> - } else {
> - s->has_zero_init = false;
> }
>
> if (!s->has_zero_init && !s->target_has_backing &&
> @@ -2086,6 +2086,7 @@ static int img_convert(int argc, char **argv)
> {"force-share", no_argument, 0, 'U'},
> {"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS},
> {"salvage", no_argument, 0, OPTION_SALVAGE},
> + {"target-is-zero", no_argument, 0, OPTION_TARGET_IS_ZERO},
> {0, 0, 0, 0}
> };
> c = getopt_long(argc, argv, ":hf:O:B:Cco:l:S:pt:T:qnm:WU",
> @@ -2209,6 +2210,14 @@ static int img_convert(int argc, char **argv)
> case OPTION_TARGET_IMAGE_OPTS:
> tgt_image_opts = true;
> break;
> + case OPTION_TARGET_IS_ZERO:
> + /*
> + * The user asserting that the target is blank has the
> + * same effect as the target driver supporting zero
> + * initialisation.
> + */
> + s.has_zero_init = true;
> + break;
> }
> }
>
> @@ -2247,6 +2256,11 @@ static int img_convert(int argc, char **argv)
> warn_report("This will become an error in future QEMU versions.");
> }
>
> + if (s.has_zero_init && !skip_create) {
> + error_report("--target-is-zero requires use of -n flag");
> + goto fail_getopt;
> + }
> +
> s.src_num = argc - optind - 1;
> out_filename = s.src_num >= 1 ? argv[argc - 1] : NULL;
>
> @@ -2380,6 +2394,11 @@ static int img_convert(int argc, char **argv)
> }
> s.target_has_backing = (bool) out_baseimg;
>
> + if (s.has_zero_init && s.target_has_backing) {
> + error_report("Cannot use --target-is-zero with a backing file");
> + goto out;
> + }
> +
> if (s.src_num > 1 && out_baseimg) {
> error_report("Having a backing file for the target makes no sense when "
> "concatenating multiple input images");
> diff --git a/qemu-img.texi b/qemu-img.texi
> index b5156d6316..3b6dfd8682 100644
> --- a/qemu-img.texi
> +++ b/qemu-img.texi
> @@ -179,6 +179,10 @@ information.
> Try to ignore I/O errors when reading. Unless in quiet mode (@code{-q}), errors
> will still be printed. Areas that cannot be read from the source will be
> treated as containing only zeroes.
> +@item --target-is-zero
> +Assume that the destination is filled with zeros. This parameter is
> +mutually exclusive with the use of a backing file. It is required to
> +also use the @code{-n} parameter to skip image creation.
> @end table
>
> Parameters to dd subcommand:
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] qemu-img: Add --target-is-zero to convert
2020-02-03 18:20 ` Vladimir Sementsov-Ogievskiy
@ 2020-02-03 19:00 ` Eric Blake
2020-02-04 9:54 ` David Edmondson
1 sibling, 0 replies; 11+ messages in thread
From: Eric Blake @ 2020-02-03 19:00 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, David Edmondson, qemu-devel,
qemu-block
On 2/3/20 12:20 PM, Vladimir Sementsov-Ogievskiy wrote:
> 24.01.2020 13:34, David Edmondson wrote:
>> In many cases the target of a convert operation is a newly provisioned
>> target that the user knows is blank (filled with zeroes). In this
>> situation there is no requirement for qemu-img to wastefully zero out
>> the entire device.
>>
>> Add a new option, --target-is-zero, allowing the user to indicate that
>> an existing target device is already zero filled.
>
> Hi! qemu-img.c part looks OK for me, but other doesn't apply on master now.
Correct. Patch 2/2 is now obsolete and no longer necessary, and patch 1
needs some tweaks now that we don't have qemu-img.texi.
>
> I like this thing, and I'd like to make similar option for backup job.
My followup patches to add an all-zero bit to qcow2 are also useful in
this regard.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] qemu-img: Add --target-is-zero to convert
2020-02-03 18:20 ` Vladimir Sementsov-Ogievskiy
2020-02-03 19:00 ` Eric Blake
@ 2020-02-04 9:54 ` David Edmondson
1 sibling, 0 replies; 11+ messages in thread
From: David Edmondson @ 2020-02-04 9:54 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel, qemu-block
On Monday, 2020-02-03 at 21:20:16 +03, Vladimir Sementsov-Ogievskiy wrote:
> 24.01.2020 13:34, David Edmondson wrote:
>> In many cases the target of a convert operation is a newly provisioned
>> target that the user knows is blank (filled with zeroes). In this
>> situation there is no requirement for qemu-img to wastefully zero out
>> the entire device.
>>
>> Add a new option, --target-is-zero, allowing the user to indicate that
>> an existing target device is already zero filled.
>
> Hi! qemu-img.c part looks OK for me, but other doesn't apply on master now.
Updated v3 just sent.
dme.
--
But he said, leave me alone, I'm a family man.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-02-04 10:05 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-01-24 10:34 [PATCH v2 0/2] qemu-img: Add --target-is-zero to indicate that a target is blank David Edmondson
2020-01-24 10:34 ` [PATCH v2 1/2] qemu-img: Add --target-is-zero to convert David Edmondson
2020-01-27 22:39 ` Eric Blake
2020-01-28 7:46 ` David Edmondson
2020-02-03 18:20 ` Vladimir Sementsov-Ogievskiy
2020-02-03 19:00 ` Eric Blake
2020-02-04 9:54 ` David Edmondson
2020-01-24 10:34 ` [PATCH v2 2/2] doc: Use @code rather than @var for options David Edmondson
2020-01-27 19:31 ` Eric Blake
2020-01-28 7:39 ` David Edmondson
2020-01-28 9:10 ` Peter Maydell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).