[pve-devel] [PATCH qemu-server] qemu_block_resize: align size to 512

[Fabian Ebner]

On 1/13/20 10:49 AM, Fabian Ebner wrote:
> On 1/9/20 5:59 PM, Thomas Lamprecht wrote:
>> On 1/9/20 11:20 AM, Fabian Ebner wrote:
>>> Doing 'qm resize 111 scsi0 +0.2G' where scsi0 is a qcow2 disk
>>> produced the following errors:
>>> "VM 111 qmp command 'block_resize' failed - The new size must be a 
>>> multiple of 512"
>>> if the VM is running and
>>> "qemu-img: The new size must be a multiple of 512"
>>
>> has it to be 512, or is this actually related to the qcow2 cluster_size,
>> which by default is 512? More out of interest, as we do not alter that
>> currently.
>>
> 
> It seems to be hard-coded as 512 in block/qcow2.c:
> 
>      if (offset & 511) {
>         error_setg(errp, "The new size must be a multiple of 512");
>          return -EINVAL;
>      }
> 
> 
>>> if the VM isn't running
>>>
>>> Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
>>> ---
>>>   PVE/QemuServer.pm | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>>> index 2b68d81..2c92c3b 100644
>>> --- a/PVE/QemuServer.pm
>>> +++ b/PVE/QemuServer.pm
>>> @@ -4668,6 +4668,10 @@ sub qemu_block_resize {
>>>       my $running = check_running($vmid);
>>> +    # aligning to 512 is required for qcow2 disks
>>
>> but this is called for all backing image types, not only qcow2??
>>
> 
> I thought it wouldn't make much of a difference if we always align to 
> 512. Otherwise we need to either check the format first (but what about 
> .raw, see the next paragraph) or do the padding in two places (once in 
> 'volume_resize' in Plugin.pm and once before issuing the qmp command).
> 
> For a '.raw' image when the VM is turned off I get a warning:
> "qemu-img: warning: Image should have been resized to 214748774 bytes, 
> but was resized to 214749184 bytes"
> so it seems to align to 512 automatically. If the VM is turned on, there 
> is no such warning, but it turns out that it quietly aligns to 512 as well.
> In both cases the value 'qemu-img info' reports (the aligned value), 
> differs from the 'size=' we write to the VM config (the unaligned value) 
> since we write the '$newsize' calculated from the original parameter of 
> the API call.
> 
> 
>>> +    my $padding = (512 - $size % 512) % 512;
>>> +    $size = $size + $padding;
>>> +
>>>       $size = 0 if !PVE::Storage::volume_resize($storecfg, $volid, 
>>> $size, $running);
>>
>> why not move the padding adjustment into the storage plugins 
>> volume_resize
>> implementation using qcow2? I.e., the base Plugin's version.
>>
>> Also, is this a problem with other backing types? (RBD, ZFS?) The RBD 
>> volume_resize
>> seems a bit fishy, as it does a plain ($size/1024/1024) - could that 
>> result in
>> truncation? Why does it allows shrinkage is also a question..
>>

Forgot to address this point in my first reply:
Here[0] is the thread that introduced the '--allow-shrink'.
It shouldn't be possible to reach truncation via the API since we do 
have a check:
die "shrinking disks is not supported\n" if $newsize < $size;

[0]: https://pve.proxmox.com/pipermail/pve-devel/2016-November/024086.html

> 
> For ZFS the new size needs to be a multiple of the volblocksize, but we 
> still only write the original parameter to the VM config. So my earlier 
> patch for ZFS introduced an inconsistency, sorry! At least for RBD and 
> '.raw' it wasn't my fault.
> 
> I think either 'qemu_block_resize' and the 'volume_resize' functions 
> need to return the new size of the volume or we request the size from 
> the storage before writing it to the config.
> 
>>>       return if !$running;
>>>
>>
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>