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.edmond...@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 <ebl...@redhat.com>

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


Reply via email to