| Message ID | 20230601192836.598602-6-andrey.drobyshev@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qemu-img: rebase: add compression support | expand |
On 6/1/23 21:28, Andrey Drobyshev wrote: > If we rebase an image whose backing file has compressed clusters, we > might end up wasting disk space since the copied clusters are now > uncompressed. In order to have better control over this, let's add > "--compress" option to the "qemu-img rebase" command. > > Note that this option affects only the clusters which are actually being > copied from the original backing file. The clusters which were > uncompressed in the target image will remain so. > > Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com> > --- > docs/tools/qemu-img.rst | 6 ++++-- > qemu-img-cmds.hx | 4 ++-- > qemu-img.c | 19 +++++++++++++++++-- > 3 files changed, 23 insertions(+), 6 deletions(-) > > diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst > index 15aeddc6d8..973a912dec 100644 > --- a/docs/tools/qemu-img.rst > +++ b/docs/tools/qemu-img.rst > @@ -663,7 +663,7 @@ Command description: > > List, apply, create or delete snapshots in image *FILENAME*. > > -.. option:: rebase [--object OBJECTDEF] [--image-opts] [-U] [-q] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-p] [-u] -b BACKING_FILE [-F BACKING_FMT] FILENAME > +.. option:: rebase [--object OBJECTDEF] [--image-opts] [-U] [-q] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-p] [-u] [-c] -b BACKING_FILE [-F BACKING_FMT] FILENAME > > Changes the backing file of an image. Only the formats ``qcow2`` and > ``qed`` support changing the backing file. > @@ -690,7 +690,9 @@ Command description: > > In order to achieve this, any clusters that differ between > *BACKING_FILE* and the old backing file of *FILENAME* are merged > - into *FILENAME* before actually changing the backing file. > + into *FILENAME* before actually changing the backing file. With ``-c`` > + option specified, the clusters which are being merged (but not the > + entire *FILENAME* image) are written in the compressed mode. > > Note that the safe mode is an expensive operation, comparable to > converting an image. It only works if the old backing file still > diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx > index 1b1dab5b17..068692d13e 100644 > --- a/qemu-img-cmds.hx > +++ b/qemu-img-cmds.hx > @@ -88,9 +88,9 @@ SRST > ERST > > DEF("rebase", img_rebase, > - "rebase [--object objectdef] [--image-opts] [-U] [-q] [-f fmt] [-t cache] [-T src_cache] [-p] [-u] -b backing_file [-F backing_fmt] filename") > + "rebase [--object objectdef] [--image-opts] [-U] [-q] [-f fmt] [-t cache] [-T src_cache] [-p] [-u] [-c] -b backing_file [-F backing_fmt] filename") > SRST > -.. option:: rebase [--object OBJECTDEF] [--image-opts] [-U] [-q] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-p] [-u] -b BACKING_FILE [-F BACKING_FMT] FILENAME > +.. option:: rebase [--object OBJECTDEF] [--image-opts] [-U] [-q] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-p] [-u] [-c] -b BACKING_FILE [-F BACKING_FMT] FILENAME > ERST > > DEF("resize", img_resize, > diff --git a/qemu-img.c b/qemu-img.c > index 9a469cd609..108da27b23 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -3517,11 +3517,13 @@ static int img_rebase(int argc, char **argv) > char *filename; > const char *fmt, *cache, *src_cache, *out_basefmt, *out_baseimg; > int c, flags, src_flags, ret; > + BdrvRequestFlags write_flags = 0; > bool writethrough, src_writethrough; > int unsafe = 0; > bool force_share = false; > int progress = 0; > bool quiet = false; > + bool compress = false; > Error *local_err = NULL; > bool image_opts = false; > > @@ -3537,9 +3539,10 @@ static int img_rebase(int argc, char **argv) > {"object", required_argument, 0, OPTION_OBJECT}, > {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, > {"force-share", no_argument, 0, 'U'}, > + {"compress", no_argument, 0, 'c'}, > {0, 0, 0, 0} > }; > - c = getopt_long(argc, argv, ":hf:F:b:upt:T:qU", > + c = getopt_long(argc, argv, ":hf:F:b:upt:T:qUc", > long_options, NULL); > if (c == -1) { > break; > @@ -3587,6 +3590,9 @@ static int img_rebase(int argc, char **argv) > case 'U': > force_share = true; > break; > + case 'c': > + compress = true; > + break; > } > } > > @@ -3639,6 +3645,14 @@ static int img_rebase(int argc, char **argv) > > unfiltered_bs = bdrv_skip_filters(bs); > > + if (compress && !block_driver_can_compress(unfiltered_bs->drv)) { > + error_report("Compression not supported for this file format"); > + ret = -1; > + goto out; > + } else if (compress) { > + write_flags |= BDRV_REQ_WRITE_COMPRESSED; > + } > + minor neat-picking. Should we get a global if (compress) { if (!block_driver_can_compress(unfiltered_bs->drv)) { report_error goto out; } write_flags |= BDRV_REQ_WRITE_COMPRESSED; } > if (out_basefmt != NULL) { > if (bdrv_find_format(out_basefmt) == NULL) { > error_report("Invalid format name: '%s'", out_basefmt); > @@ -3903,7 +3917,8 @@ static int img_rebase(int argc, char **argv) > bdi.cluster_size); > end = end > size ? size : end; > ret = blk_pwrite(blk, start, end - start, > - buf_old + (start - offset), 0); > + buf_old + (start - offset), > + write_flags); > pnum = end - (offset + written); > } > if (ret < 0) { Anyway, for both variants, Reviewed-by: Denis V. Lunev <den@openvz.org>
On 01.06.23 21:28, Andrey Drobyshev via wrote: > If we rebase an image whose backing file has compressed clusters, we > might end up wasting disk space since the copied clusters are now > uncompressed. In order to have better control over this, let's add > "--compress" option to the "qemu-img rebase" command. > > Note that this option affects only the clusters which are actually being > copied from the original backing file. The clusters which were > uncompressed in the target image will remain so. > > Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com> > --- > docs/tools/qemu-img.rst | 6 ++++-- > qemu-img-cmds.hx | 4 ++-- > qemu-img.c | 19 +++++++++++++++++-- > 3 files changed, 23 insertions(+), 6 deletions(-) Interesting. I was about to protest because we only really support writing compressed clusters to new and empty images, so the qcow2 driver does not allow overwriting existing clusters with compressed data. But by design we skip all clusters that are anything but unallocated in the top image (i.e. the one we are going to write to), so this should indeed work out well. Reviewed-by: Hanna Czenczek <hreitz@redhat.com> > diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst > index 15aeddc6d8..973a912dec 100644 > --- a/docs/tools/qemu-img.rst > +++ b/docs/tools/qemu-img.rst > @@ -663,7 +663,7 @@ Command description: > > List, apply, create or delete snapshots in image *FILENAME*. > > -.. option:: rebase [--object OBJECTDEF] [--image-opts] [-U] [-q] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-p] [-u] -b BACKING_FILE [-F BACKING_FMT] FILENAME > +.. option:: rebase [--object OBJECTDEF] [--image-opts] [-U] [-q] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-p] [-u] [-c] -b BACKING_FILE [-F BACKING_FMT] FILENAME > > Changes the backing file of an image. Only the formats ``qcow2`` and > ``qed`` support changing the backing file. > @@ -690,7 +690,9 @@ Command description: > > In order to achieve this, any clusters that differ between > *BACKING_FILE* and the old backing file of *FILENAME* are merged > - into *FILENAME* before actually changing the backing file. > + into *FILENAME* before actually changing the backing file. With ``-c`` “With the ``-c`` option specified, [...]” > + option specified, the clusters which are being merged (but not the > + entire *FILENAME* image) are written in the compressed mode. “[...] are compressed when written.” > Note that the safe mode is an expensive operation, comparable to > converting an image. It only works if the old backing file still
diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst index 15aeddc6d8..973a912dec 100644 --- a/docs/tools/qemu-img.rst +++ b/docs/tools/qemu-img.rst @@ -663,7 +663,7 @@ Command description: List, apply, create or delete snapshots in image *FILENAME*. -.. option:: rebase [--object OBJECTDEF] [--image-opts] [-U] [-q] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-p] [-u] -b BACKING_FILE [-F BACKING_FMT] FILENAME +.. option:: rebase [--object OBJECTDEF] [--image-opts] [-U] [-q] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-p] [-u] [-c] -b BACKING_FILE [-F BACKING_FMT] FILENAME Changes the backing file of an image. Only the formats ``qcow2`` and ``qed`` support changing the backing file. @@ -690,7 +690,9 @@ Command description: In order to achieve this, any clusters that differ between *BACKING_FILE* and the old backing file of *FILENAME* are merged - into *FILENAME* before actually changing the backing file. + into *FILENAME* before actually changing the backing file. With ``-c`` + option specified, the clusters which are being merged (but not the + entire *FILENAME* image) are written in the compressed mode. Note that the safe mode is an expensive operation, comparable to converting an image. It only works if the old backing file still diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx index 1b1dab5b17..068692d13e 100644 --- a/qemu-img-cmds.hx +++ b/qemu-img-cmds.hx @@ -88,9 +88,9 @@ SRST ERST DEF("rebase", img_rebase, - "rebase [--object objectdef] [--image-opts] [-U] [-q] [-f fmt] [-t cache] [-T src_cache] [-p] [-u] -b backing_file [-F backing_fmt] filename") + "rebase [--object objectdef] [--image-opts] [-U] [-q] [-f fmt] [-t cache] [-T src_cache] [-p] [-u] [-c] -b backing_file [-F backing_fmt] filename") SRST -.. option:: rebase [--object OBJECTDEF] [--image-opts] [-U] [-q] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-p] [-u] -b BACKING_FILE [-F BACKING_FMT] FILENAME +.. option:: rebase [--object OBJECTDEF] [--image-opts] [-U] [-q] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-p] [-u] [-c] -b BACKING_FILE [-F BACKING_FMT] FILENAME ERST DEF("resize", img_resize, diff --git a/qemu-img.c b/qemu-img.c index 9a469cd609..108da27b23 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -3517,11 +3517,13 @@ static int img_rebase(int argc, char **argv) char *filename; const char *fmt, *cache, *src_cache, *out_basefmt, *out_baseimg; int c, flags, src_flags, ret; + BdrvRequestFlags write_flags = 0; bool writethrough, src_writethrough; int unsafe = 0; bool force_share = false; int progress = 0; bool quiet = false; + bool compress = false; Error *local_err = NULL; bool image_opts = false; @@ -3537,9 +3539,10 @@ static int img_rebase(int argc, char **argv) {"object", required_argument, 0, OPTION_OBJECT}, {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, {"force-share", no_argument, 0, 'U'}, + {"compress", no_argument, 0, 'c'}, {0, 0, 0, 0} }; - c = getopt_long(argc, argv, ":hf:F:b:upt:T:qU", + c = getopt_long(argc, argv, ":hf:F:b:upt:T:qUc", long_options, NULL); if (c == -1) { break; @@ -3587,6 +3590,9 @@ static int img_rebase(int argc, char **argv) case 'U': force_share = true; break; + case 'c': + compress = true; + break; } } @@ -3639,6 +3645,14 @@ static int img_rebase(int argc, char **argv) unfiltered_bs = bdrv_skip_filters(bs); + if (compress && !block_driver_can_compress(unfiltered_bs->drv)) { + error_report("Compression not supported for this file format"); + ret = -1; + goto out; + } else if (compress) { + write_flags |= BDRV_REQ_WRITE_COMPRESSED; + } + if (out_basefmt != NULL) { if (bdrv_find_format(out_basefmt) == NULL) { error_report("Invalid format name: '%s'", out_basefmt); @@ -3903,7 +3917,8 @@ static int img_rebase(int argc, char **argv) bdi.cluster_size); end = end > size ? size : end; ret = blk_pwrite(blk, start, end - start, - buf_old + (start - offset), 0); + buf_old + (start - offset), + write_flags); pnum = end - (offset + written); } if (ret < 0) {
If we rebase an image whose backing file has compressed clusters, we might end up wasting disk space since the copied clusters are now uncompressed. In order to have better control over this, let's add "--compress" option to the "qemu-img rebase" command. Note that this option affects only the clusters which are actually being copied from the original backing file. The clusters which were uncompressed in the target image will remain so. Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com> --- docs/tools/qemu-img.rst | 6 ++++-- qemu-img-cmds.hx | 4 ++-- qemu-img.c | 19 +++++++++++++++++-- 3 files changed, 23 insertions(+), 6 deletions(-)