[pve-devel] [PATCH v2 qemu-server 1/2] resize_vm: request new size from storage after resizing

[Fabian Ebner]

On 1/13/20 11:47 AM, Fabian Ebner wrote:
> Because of alignment and rounding in the storage backend, the effective
> size might not match the 'newsize' parameter we passed along.
> 
> Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
> ---
> 
> Turns out that this happens in basically every storage backend that has
> 'volume_resize': LVM and RBD round down, ZFS and DIR for '.raw' align
> 
>   PVE/API2/Qemu.pm | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 5bae513..b83ce07 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -3607,7 +3607,8 @@ __PACKAGE__->register_method({
>   
>   	    PVE::QemuServer::qemu_block_resize($vmid, "drive-$disk", $storecfg, $volid, $newsize);
>   
> -	    $drive->{size} = $newsize;
> +	    my $effective_size = eval { PVE::Storage::volume_size_info($storecfg, $volid, 3); };
> +	    $drive->{size} = $effective_size // $newsize;
>   	    $conf->{$disk} = PVE::QemuServer::print_drive($drive);
>   
>   	    PVE::QemuConfig->write_config($vmid, $conf);
> 

Please do not apply the second and third patch in this series yet. I 
thought about this some more and think it makes more sense to always 
align to a multiple of 1 KiB.
Reasons for this are:

1. For volume export/import, in 'write_common_header' we write the size 
of the file in bytes, in 'read_common_header' there is a check whether 
the size is a multiple of 1KiB.

So not all volumes exported with 'pvesm' can be imported with 'pvesm':

root at rob0 ~ # qm resize 128 scsi2 +512
Image resized.
root at rob0 ~ # pvesm export myfs:128/vm-128-disk-0.qcow2 qcow2+size 
myoddexport --with-snapshots
52816+0 records in
52816+0 records out
216334336 bytes (216 MB, 206 MiB) copied, 0.291073 s, 743 MB/s
root at rob0 ~ # pvesm import myfs:128/vm-128-disk-1.qcow2 qcow2+size 
myoddexport --with-snapshots
import: got a bad size (not a multiple of 1K), aborting.

2. Allocating volumes happens with a size in KiB, so maybe resizing 
should too? Internally we could switch the interface for 
'PVE::Storage::volume_resize' to use KiB instead of bytes and we could 
warn users which pass a non-KiB-multiple to 'qm resize', that we rounded 
up and didn't use the requested size exactly.

3. Many plugins already round the size anyways.


The same change as here in the first patch is also needed for callers of 
'vdisk_alloc', since the size the disk has after allocating might not be 
the requested size. For example in ZFSPoolPlugin size is aligned to 1M, 
LVM uses logical extents so the effective size is also often different 
from the parameter.

Related: I ran into a weird bug [0] yesterday, where a workaround would 
require the size being a multiple of 4KiB. But that could be done for 
the 'vdisk_alloc' call in 'vm_start'.

[0]: https://bugzilla.proxmox.com/show_bug.cgi?id=2562