* [pve-devel] [PATCH qemu-server] fix #3324: clone disk: use larger blocksize for EFI disk when possible
@ 2021-03-01 9:42 Fabian Ebner
2021-03-01 9:54 ` Stefan Reiter
2021-03-01 10:23 ` Fabian Ebner
0 siblings, 2 replies; 9+ messages in thread
From: Fabian Ebner @ 2021-03-01 9:42 UTC (permalink / raw)
To: pve-devel
Moving to Ceph is very slow when bs=1. Instead, use the biggest possible power
of two <= 1024. At the moment our EFI image sizes are multiples of 1024, so
just using 1024 wouldn't be a problem, but this feels more future-proof.
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
I did not see an way for 'qemu-img dd' to use a larger blocksize while still
specifying the exact total size if it is not a multiple of the blocksize.
PVE/QemuServer.pm | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index f401baf..e579cdf 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -6991,7 +6991,15 @@ sub clone_disk {
# that is given by the OVMF_VARS.fd
my $src_path = PVE::Storage::path($storecfg, $drive->{file});
my $dst_path = PVE::Storage::path($storecfg, $newvolid);
- run_command(['qemu-img', 'dd', '-n', '-O', $dst_format, "bs=1", "count=$size",
+
+ # Ceph doesn't like too small blocksize, see bug #3324
+ my $bs = 1;
+ while ($bs < $size && $bs < 1024 && $size % $bs == 0) {
+ $bs *= 2;
+ }
+ my $count = $size / $bs;
+
+ run_command(['qemu-img', 'dd', '-n', '-O', $dst_format, "bs=$bs", "count=$count",
"if=$src_path", "of=$dst_path"]);
} else {
qemu_img_convert($drive->{file}, $newvolid, $size, $snapname, $sparseinit);
--
2.20.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH qemu-server] fix #3324: clone disk: use larger blocksize for EFI disk when possible
2021-03-01 9:42 [pve-devel] [PATCH qemu-server] fix #3324: clone disk: use larger blocksize for EFI disk when possible Fabian Ebner
@ 2021-03-01 9:54 ` Stefan Reiter
2021-03-01 10:06 ` Fabian Ebner
2021-03-01 10:23 ` Fabian Ebner
1 sibling, 1 reply; 9+ messages in thread
From: Stefan Reiter @ 2021-03-01 9:54 UTC (permalink / raw)
To: pve-devel
On 3/1/21 10:42 AM, Fabian Ebner wrote:
> Moving to Ceph is very slow when bs=1. Instead, use the biggest possible power
> of two <= 1024. At the moment our EFI image sizes are multiples of 1024, so
> just using 1024 wouldn't be a problem, but this feels more future-proof.
>
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>
> I did not see an way for 'qemu-img dd' to use a larger blocksize while still
> specifying the exact total size if it is not a multiple of the blocksize.
>
Could it make sense to just set the block size equal to the image size
with count=1 ? Since the images will always be very small anyway...
> PVE/QemuServer.pm | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index f401baf..e579cdf 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -6991,7 +6991,15 @@ sub clone_disk {
> # that is given by the OVMF_VARS.fd
> my $src_path = PVE::Storage::path($storecfg, $drive->{file});
> my $dst_path = PVE::Storage::path($storecfg, $newvolid);
> - run_command(['qemu-img', 'dd', '-n', '-O', $dst_format, "bs=1", "count=$size",
> +
> + # Ceph doesn't like too small blocksize, see bug #3324
> + my $bs = 1;
> + while ($bs < $size && $bs < 1024 && $size % $bs == 0) {
> + $bs *= 2;
> + }
> + my $count = $size / $bs;
> +
> + run_command(['qemu-img', 'dd', '-n', '-O', $dst_format, "bs=$bs", "count=$count",
> "if=$src_path", "of=$dst_path"]);
> } else {
> qemu_img_convert($drive->{file}, $newvolid, $size, $snapname, $sparseinit);
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH qemu-server] fix #3324: clone disk: use larger blocksize for EFI disk when possible
2021-03-01 9:54 ` Stefan Reiter
@ 2021-03-01 10:06 ` Fabian Ebner
2021-03-01 10:13 ` Stefan Reiter
0 siblings, 1 reply; 9+ messages in thread
From: Fabian Ebner @ 2021-03-01 10:06 UTC (permalink / raw)
To: Stefan Reiter, pve-devel
Am 01.03.21 um 10:54 schrieb Stefan Reiter:
> On 3/1/21 10:42 AM, Fabian Ebner wrote:
>> Moving to Ceph is very slow when bs=1. Instead, use the biggest
>> possible power
>> of two <= 1024. At the moment our EFI image sizes are multiples of
>> 1024, so
>> just using 1024 wouldn't be a problem, but this feels more future-proof.
>>
>> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
>> ---
>>
>> I did not see an way for 'qemu-img dd' to use a larger blocksize while
>> still
>> specifying the exact total size if it is not a multiple of the blocksize.
>>
>
> Could it make sense to just set the block size equal to the image size
> with count=1 ? Since the images will always be very small anyway...
>
Note that AAVMF_VARS.fd is 64 MiB. Are blocksizes that big a good idea?
>> PVE/QemuServer.pm | 10 +++++++++-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>> index f401baf..e579cdf 100644
>> --- a/PVE/QemuServer.pm
>> +++ b/PVE/QemuServer.pm
>> @@ -6991,7 +6991,15 @@ sub clone_disk {
>> # that is given by the OVMF_VARS.fd
>> my $src_path = PVE::Storage::path($storecfg, $drive->{file});
>> my $dst_path = PVE::Storage::path($storecfg, $newvolid);
>> - run_command(['qemu-img', 'dd', '-n', '-O', $dst_format,
>> "bs=1", "count=$size",
>> +
>> + # Ceph doesn't like too small blocksize, see bug #3324
>> + my $bs = 1;
>> + while ($bs < $size && $bs < 1024 && $size % $bs == 0) {
>> + $bs *= 2;
>> + }
>> + my $count = $size / $bs;
>> +
>> + run_command(['qemu-img', 'dd', '-n', '-O', $dst_format,
>> "bs=$bs", "count=$count",
>> "if=$src_path", "of=$dst_path"]);
>> } else {
>> qemu_img_convert($drive->{file}, $newvolid, $size,
>> $snapname, $sparseinit);
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH qemu-server] fix #3324: clone disk: use larger blocksize for EFI disk when possible
2021-03-01 10:06 ` Fabian Ebner
@ 2021-03-01 10:13 ` Stefan Reiter
2021-03-01 10:18 ` Dietmar Maurer
2021-03-01 10:19 ` Fabian Ebner
0 siblings, 2 replies; 9+ messages in thread
From: Stefan Reiter @ 2021-03-01 10:13 UTC (permalink / raw)
To: Fabian Ebner, pve-devel
On 3/1/21 11:06 AM, Fabian Ebner wrote:
> Am 01.03.21 um 10:54 schrieb Stefan Reiter:
>> On 3/1/21 10:42 AM, Fabian Ebner wrote:
>>> Moving to Ceph is very slow when bs=1. Instead, use the biggest
>>> possible power
>>> of two <= 1024. At the moment our EFI image sizes are multiples of
>>> 1024, so
>>> just using 1024 wouldn't be a problem, but this feels more future-proof.
>>>
>>> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
>>> ---
>>>
>>> I did not see an way for 'qemu-img dd' to use a larger blocksize
>>> while still
>>> specifying the exact total size if it is not a multiple of the
>>> blocksize.
>>>
>>
>> Could it make sense to just set the block size equal to the image size
>> with count=1 ? Since the images will always be very small anyway...
>>
>
> Note that AAVMF_VARS.fd is 64 MiB. Are blocksizes that big a good idea?
>
That'd be too much, but the VARS file shouldn't be copied anyway? Only
the efidisk attached to the VM?
>>> PVE/QemuServer.pm | 10 +++++++++-
>>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>>> index f401baf..e579cdf 100644
>>> --- a/PVE/QemuServer.pm
>>> +++ b/PVE/QemuServer.pm
>>> @@ -6991,7 +6991,15 @@ sub clone_disk {
>>> # that is given by the OVMF_VARS.fd
>>> my $src_path = PVE::Storage::path($storecfg, $drive->{file});
>>> my $dst_path = PVE::Storage::path($storecfg, $newvolid);
>>> - run_command(['qemu-img', 'dd', '-n', '-O', $dst_format,
>>> "bs=1", "count=$size",
>>> +
>>> + # Ceph doesn't like too small blocksize, see bug #3324
>>> + my $bs = 1;
>>> + while ($bs < $size && $bs < 1024 && $size % $bs == 0) {
>>> + $bs *= 2;
>>> + }
>>> + my $count = $size / $bs;
>>> +
>>> + run_command(['qemu-img', 'dd', '-n', '-O', $dst_format,
>>> "bs=$bs", "count=$count",
>>> "if=$src_path", "of=$dst_path"]);
>>> } else {
>>> qemu_img_convert($drive->{file}, $newvolid, $size,
>>> $snapname, $sparseinit);
>>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH qemu-server] fix #3324: clone disk: use larger blocksize for EFI disk when possible
2021-03-01 10:13 ` Stefan Reiter
@ 2021-03-01 10:18 ` Dietmar Maurer
2021-03-01 10:22 ` Fabian Ebner
2021-03-02 7:11 ` Fabian Ebner
2021-03-01 10:19 ` Fabian Ebner
1 sibling, 2 replies; 9+ messages in thread
From: Dietmar Maurer @ 2021-03-01 10:18 UTC (permalink / raw)
To: Proxmox VE development discussion, Stefan Reiter, Fabian Ebner
> >>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> >>> index f401baf..e579cdf 100644
> >>> --- a/PVE/QemuServer.pm
> >>> +++ b/PVE/QemuServer.pm
> >>> @@ -6991,7 +6991,15 @@ sub clone_disk {
> >>> # that is given by the OVMF_VARS.fd
> >>> my $src_path = PVE::Storage::path($storecfg, $drive->{file});
> >>> my $dst_path = PVE::Storage::path($storecfg, $newvolid);
> >>> - run_command(['qemu-img', 'dd', '-n', '-O', $dst_format,
> >>> "bs=1", "count=$size",
> >>> +
> >>> + # Ceph doesn't like too small blocksize, see bug #3324
> >>> + my $bs = 1;
> >>> + while ($bs < $size && $bs < 1024 && $size % $bs == 0) {
> >>> + $bs *= 2;
> >>> + }
now, $size % $bs != 0
I guess this is wrong...
> >>> + my $count = $size / $bs;
> >>> +
> >>> + run_command(['qemu-img', 'dd', '-n', '-O', $dst_format,
> >>> "bs=$bs", "count=$count",
> >>> "if=$src_path", "of=$dst_path"]);
> >>> } else {
> >>> qemu_img_convert($drive->{file}, $newvolid, $size,
> >>> $snapname, $sparseinit);
> >>>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH qemu-server] fix #3324: clone disk: use larger blocksize for EFI disk when possible
2021-03-01 10:13 ` Stefan Reiter
2021-03-01 10:18 ` Dietmar Maurer
@ 2021-03-01 10:19 ` Fabian Ebner
1 sibling, 0 replies; 9+ messages in thread
From: Fabian Ebner @ 2021-03-01 10:19 UTC (permalink / raw)
To: Stefan Reiter, pve-devel
Am 01.03.21 um 11:13 schrieb Stefan Reiter:
> On 3/1/21 11:06 AM, Fabian Ebner wrote:
>> Am 01.03.21 um 10:54 schrieb Stefan Reiter:
>>> On 3/1/21 10:42 AM, Fabian Ebner wrote:
>>>> Moving to Ceph is very slow when bs=1. Instead, use the biggest
>>>> possible power
>>>> of two <= 1024. At the moment our EFI image sizes are multiples of
>>>> 1024, so
>>>> just using 1024 wouldn't be a problem, but this feels more
>>>> future-proof.
>>>>
>>>> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
>>>> ---
>>>>
>>>> I did not see an way for 'qemu-img dd' to use a larger blocksize
>>>> while still
>>>> specifying the exact total size if it is not a multiple of the
>>>> blocksize.
>>>>
>>>
>>> Could it make sense to just set the block size equal to the image
>>> size with count=1 ? Since the images will always be very small anyway...
>>>
>>
>> Note that AAVMF_VARS.fd is 64 MiB. Are blocksizes that big a good idea?
>>
>
> That'd be too much, but the VARS file shouldn't be copied anyway? Only
> the efidisk attached to the VM?
>
AFAICT that's the file used for the EFI disk.
>>>> PVE/QemuServer.pm | 10 +++++++++-
>>>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>>>> index f401baf..e579cdf 100644
>>>> --- a/PVE/QemuServer.pm
>>>> +++ b/PVE/QemuServer.pm
>>>> @@ -6991,7 +6991,15 @@ sub clone_disk {
>>>> # that is given by the OVMF_VARS.fd
>>>> my $src_path = PVE::Storage::path($storecfg, $drive->{file});
>>>> my $dst_path = PVE::Storage::path($storecfg, $newvolid);
>>>> - run_command(['qemu-img', 'dd', '-n', '-O', $dst_format,
>>>> "bs=1", "count=$size",
>>>> +
>>>> + # Ceph doesn't like too small blocksize, see bug #3324
>>>> + my $bs = 1;
>>>> + while ($bs < $size && $bs < 1024 && $size % $bs == 0) {
>>>> + $bs *= 2;
>>>> + }
>>>> + my $count = $size / $bs;
>>>> +
>>>> + run_command(['qemu-img', 'dd', '-n', '-O', $dst_format,
>>>> "bs=$bs", "count=$count",
>>>> "if=$src_path", "of=$dst_path"]);
>>>> } else {
>>>> qemu_img_convert($drive->{file}, $newvolid, $size,
>>>> $snapname, $sparseinit);
>>>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH qemu-server] fix #3324: clone disk: use larger blocksize for EFI disk when possible
2021-03-01 10:18 ` Dietmar Maurer
@ 2021-03-01 10:22 ` Fabian Ebner
2021-03-02 7:11 ` Fabian Ebner
1 sibling, 0 replies; 9+ messages in thread
From: Fabian Ebner @ 2021-03-01 10:22 UTC (permalink / raw)
To: Dietmar Maurer, Proxmox VE development discussion, Stefan Reiter
Am 01.03.21 um 11:18 schrieb Dietmar Maurer:
>
>>>>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>>>>> index f401baf..e579cdf 100644
>>>>> --- a/PVE/QemuServer.pm
>>>>> +++ b/PVE/QemuServer.pm
>>>>> @@ -6991,7 +6991,15 @@ sub clone_disk {
>>>>> # that is given by the OVMF_VARS.fd
>>>>> my $src_path = PVE::Storage::path($storecfg, $drive->{file});
>>>>> my $dst_path = PVE::Storage::path($storecfg, $newvolid);
>>>>> - run_command(['qemu-img', 'dd', '-n', '-O', $dst_format,
>>>>> "bs=1", "count=$size",
>>>>> +
>>>>> + # Ceph doesn't like too small blocksize, see bug #3324
>>>>> + my $bs = 1;
>>>>> + while ($bs < $size && $bs < 1024 && $size % $bs == 0) {
>>>>> + $bs *= 2;
>>>>> + }
>
> now, $size % $bs != 0
>
> I guess this is wrong...
>
No, because of the $bs < $size check first. If we enter the loop, $size
was a multiple of $bs and at least twice $bs. I wrote it like this
because it'd avoid an endless loop if for whatever reason $size is 0.
>>>>> + my $count = $size / $bs;
>>>>> +
>>>>> + run_command(['qemu-img', 'dd', '-n', '-O', $dst_format,
>>>>> "bs=$bs", "count=$count",
>>>>> "if=$src_path", "of=$dst_path"]);
>>>>> } else {
>>>>> qemu_img_convert($drive->{file}, $newvolid, $size,
>>>>> $snapname, $sparseinit);
>>>>>
>>
>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH qemu-server] fix #3324: clone disk: use larger blocksize for EFI disk when possible
2021-03-01 9:42 [pve-devel] [PATCH qemu-server] fix #3324: clone disk: use larger blocksize for EFI disk when possible Fabian Ebner
2021-03-01 9:54 ` Stefan Reiter
@ 2021-03-01 10:23 ` Fabian Ebner
1 sibling, 0 replies; 9+ messages in thread
From: Fabian Ebner @ 2021-03-01 10:23 UTC (permalink / raw)
To: pve-devel
After a short off-list discussion with Thomas, we decided to first
assert that the size is a multiple of 1024 and then simply use bs=1024.
If a new architecture with a strange-sized VARS file comes along we have
to adapt it though.
Am 01.03.21 um 10:42 schrieb Fabian Ebner:
> Moving to Ceph is very slow when bs=1. Instead, use the biggest possible power
> of two <= 1024. At the moment our EFI image sizes are multiples of 1024, so
> just using 1024 wouldn't be a problem, but this feels more future-proof.
>
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>
> I did not see an way for 'qemu-img dd' to use a larger blocksize while still
> specifying the exact total size if it is not a multiple of the blocksize.
>
> PVE/QemuServer.pm | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index f401baf..e579cdf 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -6991,7 +6991,15 @@ sub clone_disk {
> # that is given by the OVMF_VARS.fd
> my $src_path = PVE::Storage::path($storecfg, $drive->{file});
> my $dst_path = PVE::Storage::path($storecfg, $newvolid);
> - run_command(['qemu-img', 'dd', '-n', '-O', $dst_format, "bs=1", "count=$size",
> +
> + # Ceph doesn't like too small blocksize, see bug #3324
> + my $bs = 1;
> + while ($bs < $size && $bs < 1024 && $size % $bs == 0) {
> + $bs *= 2;
> + }
> + my $count = $size / $bs;
> +
> + run_command(['qemu-img', 'dd', '-n', '-O', $dst_format, "bs=$bs", "count=$count",
> "if=$src_path", "of=$dst_path"]);
> } else {
> qemu_img_convert($drive->{file}, $newvolid, $size, $snapname, $sparseinit);
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH qemu-server] fix #3324: clone disk: use larger blocksize for EFI disk when possible
2021-03-01 10:18 ` Dietmar Maurer
2021-03-01 10:22 ` Fabian Ebner
@ 2021-03-02 7:11 ` Fabian Ebner
1 sibling, 0 replies; 9+ messages in thread
From: Fabian Ebner @ 2021-03-02 7:11 UTC (permalink / raw)
To: Dietmar Maurer, Proxmox VE development discussion
Am 01.03.21 um 11:18 schrieb Dietmar Maurer:
>
>>>>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>>>>> index f401baf..e579cdf 100644
>>>>> --- a/PVE/QemuServer.pm
>>>>> +++ b/PVE/QemuServer.pm
>>>>> @@ -6991,7 +6991,15 @@ sub clone_disk {
>>>>> # that is given by the OVMF_VARS.fd
>>>>> my $src_path = PVE::Storage::path($storecfg, $drive->{file});
>>>>> my $dst_path = PVE::Storage::path($storecfg, $newvolid);
>>>>> - run_command(['qemu-img', 'dd', '-n', '-O', $dst_format,
>>>>> "bs=1", "count=$size",
>>>>> +
>>>>> + # Ceph doesn't like too small blocksize, see bug #3324
>>>>> + my $bs = 1;
>>>>> + while ($bs < $size && $bs < 1024 && $size % $bs == 0) {
>>>>> + $bs *= 2;
>>>>> + }
>
> now, $size % $bs != 0
>
> I guess this is wrong...
>
Sorry about the confusion yesterday. Of course you are right.
>>>>> + my $count = $size / $bs;
>>>>> +
>>>>> + run_command(['qemu-img', 'dd', '-n', '-O', $dst_format,
>>>>> "bs=$bs", "count=$count",
>>>>> "if=$src_path", "of=$dst_path"]);
>>>>> } else {
>>>>> qemu_img_convert($drive->{file}, $newvolid, $size,
>>>>> $snapname, $sparseinit);
>>>>>
>>
>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-03-02 7:12 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-01 9:42 [pve-devel] [PATCH qemu-server] fix #3324: clone disk: use larger blocksize for EFI disk when possible Fabian Ebner
2021-03-01 9:54 ` Stefan Reiter
2021-03-01 10:06 ` Fabian Ebner
2021-03-01 10:13 ` Stefan Reiter
2021-03-01 10:18 ` Dietmar Maurer
2021-03-01 10:22 ` Fabian Ebner
2021-03-02 7:11 ` Fabian Ebner
2021-03-01 10:19 ` Fabian Ebner
2021-03-01 10:23 ` Fabian Ebner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal