Re: [Qemu-devel] [PATCH 0/2] Add reduce image for qcow2

[Kevin Wolf <kwolf@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3933 bytes --]

Am 07.06.2017 um 15:37 hat Max Reitz geschrieben:
> On 2017-06-01 13:11, Denis V. Lunev wrote:
> > On 06/01/2017 12:12 PM, Kevin Wolf wrote:
> >> Am 31.05.2017 um 17:03 hat Eric Blake geschrieben:
> >>> On 05/31/2017 09:43 AM, Pavel Butsykin wrote:
> >>>> This patch adds the reduction of the image file for qcow2. As a result, this
> >>>> allows us to reduce the virtual image size and free up space on the disk without
> >>>> copying the image. Image can be fragmented and reduction is done by punching
> >>>> holes in the image file.
> >>>>
> >>>> # ./qemu-img create -f qcow2 -o size=4G image.qcow2
> >>>> Formatting 'image.qcow2', fmt=qcow2 size=4294967296 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16
> >>>>
> >>>> # ./qemu-io -c "write -P 0x22 0 1G" image.qcow2
> >>> So this is 1G of guest-visible data...
> >>>
> >>>> # ./qemu-img resize image.qcow2 128M
> >>>> Image resized.
> >>> ...and you are truncating the image by throwing away guest-visible
> >>> content, with no warning or double-checking (such as requiring a -f
> >>> force parameter or something) about the data loss.  Shrinking images is
> >>> something we should allow, but it feels like is a rare enough operation
> >>> that you don't want it to be casually easy to throw away data.
> >>>
> >>> Is it feasible to require that a shrink operation will not be performed
> >>> unless all clusters being eliminated have been previously discarded (or
> >>> maybe written to zero), as an assurance that the guest did not care
> >>> about the tail of the image?
> >> I think that ship has sailed long ago because raw images have always
> >> supported shrinking images without any special checks or options. We
> >> want consistency between raw and qcow2, and we also need to provide
> >> backwards compatibility.
> >>
> >> The only thing I can imagine we could do now is to introduce a --shrink
> >> option in qemu-img, print a deprecation warning for shrinking without
> >> this option, and then make it mandatory in a few years.
> 
> Do I hear a "3.0"?

You do.

> >> If we want to distinguish based on the block driver, so that we can
> >> require --shrink for all drivers that didn't support shrinking until
> >> now, we'd have to check the .bdrv_truncate() implementations of all
> >> drivers to see whether it already allowed shrinking.
> 
> We could have an ugly raw-only hack directly in qemu-img (and
> qmp_block_resize) until 3.0.
> 
> I'm really concerned about someone mistyping something and accidentally
> dropping a digit.

Me too. But still we can't break working command lines (at least before
3.0).

I'm okay with temporary hacks in qemu-img, but did you check whether
it's really raw-only? We know that raw can shrink currently and qcow2
can't, but there are 12 drivers implementing .bdrv_truncate, not only
two.

> > Will the solution proposed by Pavel in the answer to Max work:
> > - if there are no data blocks in the truncated space, truncate is allowed
> >   without additional options
> > - if there are data blocks in the truncated space, truncate is allowed
> >   with the flag --force or error is generated
> 
> I'd be OK with that, but I'm not sure we really need it.

Agreed. It would add a lot of complexity for little use. As I wrote in a
previous mail: I don't even know how I would discard the unpartitioned
space after shrinking my guest filesystem.

> It would be nice to have for convenience, but I don't think it solves
> the backwards-compatibility problem (as said by Kevin), and I'm not
> sure it makes things that much more convenient: Just specifying
> --force is easier than to manually trim the device in the guest (and
> then maybe having to specify --force anyway because you didn't do it
> right).

I think it should be specifically --shrink rather than an unspecific
--force that could mean anything.

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]