On 3/22/20 4:11 AM, Eyal Moscovici wrote:
The mapping operation of large disks especially ones stored over a
long chain of QCOW2 files can take a long time to finish.
Additionally when mapping fails there was no way recover by
restarting the mapping from the failed location.
The new options, --start-offset and --max-length allows the user to
divide these type of map operations into shorter independent tasks.
Acked-by: Mark Kanda <mark.ka...@oracle.com>
Co-developed-by: Yoav Elnekave <yoav.elnek...@oracle.com>
Signed-off-by: Yoav Elnekave <yoav.elnek...@oracle.com>
Signed-off-by: Eyal Moscovici <eyal.moscov...@oracle.com>
---
docs/tools/qemu-img.rst | 2 +-
qemu-img-cmds.hx | 4 ++--
qemu-img.c | 30 +++++++++++++++++++++++++++++-
3 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
index 0080f83a76..924e89f679 100644
--- a/docs/tools/qemu-img.rst
+++ b/docs/tools/qemu-img.rst
@@ -519,7 +519,7 @@ Command description:
``ImageInfoSpecific*`` QAPI object (e.g. ``ImageInfoSpecificQCow2``
for qcow2 images).
-.. option:: map [--object OBJECTDEF] [--image-opts] [-f FMT] [--output=OFMT] [-U] FILENAME
+.. option:: map [--object OBJECTDEF] [--image-opts] [-f FMT]
[--start-offset=offset] [--max-length=len] [--output=OFMT] [-U] FILENAME
Consistency with the rest of the line says this should be
[--start-offset=OFFSET] [--max-length=LEN]
Dump the metadata of image *FILENAME* and its backing file chain.
In particular, this commands dumps the allocation state of every sector
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index c9c54de1df..35f832816f 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -63,9 +63,9 @@ SRST
ERST
DEF("map", img_map,
- "map [--object objectdef] [--image-opts] [-f fmt] [--output=ofmt] [-U]
filename")
+ "map [--object objectdef] [--image-opts] [-f fmt] [--start-offset=offset]
[--max-length=len] [--output=ofmt] [-U] filename")
this one is fine,
SRST
-.. option:: map [--object OBJECTDEF] [--image-opts] [-f FMT] [--output=OFMT]
[-U] FILENAME
+.. option:: map [--object OBJECTDEF] [--image-opts] [-f FMT]
[--start-offset=OFFSET] [--max-length=LEN] [--output=OFMT] [-U] FILENAME
this one has the same problem as the .rst.
@@ -3005,6 +3009,26 @@ static int img_map(int argc, char **argv)
case OPTION_OUTPUT:
output = optarg;
break;
+ case 's':
+ start_offset = cvtnum(optarg);
+ if (start_offset < 0) {
+ error_report("Invalid start offset specified! You may use "
+ "k, M, G, T, P or E suffixes for ");
+ error_report("kilobytes, megabytes, gigabytes, terabytes, "
+ "petabytes and exabytes.");
Pre-existing elsewhere in the file, but this seems rather verbose -
shouldn't we have cvtnum() (or another wrapper function) give this extra
information about what is valid, rather than open-coding it at every
client of cvtnum()?
+ return 1;
+ }
+ break;
+ case 'l':
+ max_length = cvtnum(optarg);
+ if (max_length < 0) {
+ error_report("Invalid max length specified! You may use "
+ "k, M, G, T, P or E suffixes for ");
+ error_report("kilobytes, megabytes, gigabytes, terabytes, "
+ "petabytes and exabytes.");
+ return 1;
+ }
+ break;
case OPTION_OBJECT: {
QemuOpts *opts;
opts = qemu_opts_parse_noisily(&qemu_object_opts,
@@ -3050,7 +3074,11 @@ static int img_map(int argc, char **argv)
printf("[");
}
+ curr.start = start_offset;
length = blk_getlength(blk);
+ if (max_length != -1) {
+ length = MIN(start_offset + max_length, length);
+ }
Pre-existing, but where does this code check for length == -1? But your
MIN() doesn't make it any worse (if we fail to get length, we merely
skip the loop).
while (curr.start + curr.length < length) {
int64_t offset = curr.start + curr.length;
int64_t n;
Overall, the idea makes sense to me. But I'm not sure which maintainer
should actually incorporate the patch.
Reviewed-by: Eric Blake <ebl...@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org