* [pve-devel] [PATCH v2 qemu-server] fix cloning/restoring of cloudinit disks in raw format
@ 2020-12-02 12:50 Mira Limbeck
2020-12-15 13:57 ` Thomas Lamprecht
2020-12-15 15:18 ` [pve-devel] applied: " Thomas Lamprecht
0 siblings, 2 replies; 4+ messages in thread
From: Mira Limbeck @ 2020-12-02 12:50 UTC (permalink / raw)
To: pve-devel
We only added the format extension when it was not 'raw'. But on file level
storages we always require it. To fix this, always add the format
extension if the storage provides the 'path' property.
This is the same logic we use in create_disks for cloudinit disks.
Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
---
v2:
- added the same fix when restoring from backup
PVE/QemuServer.pm | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 4989938..412113e 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5797,7 +5797,10 @@ my $restore_allocate_devices = sub {
my $name;
if ($d->{is_cloudinit}) {
$name = "vm-$vmid-cloudinit";
- $name .= ".$d->{format}" if $d->{format} ne 'raw';
+ my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
+ if ($scfg->{path}) {
+ $name .= ".$d->{format}";
+ }
}
my $volid = PVE::Storage::vdisk_alloc(
@@ -6945,7 +6948,10 @@ sub clone_disk {
my $size = undef;
if (drive_is_cloudinit($drive)) {
$name = "vm-$newvmid-cloudinit";
- $name .= ".$dst_format" if $dst_format ne 'raw';
+ my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
+ if ($scfg->{path}) {
+ $name .= ".$dst_format";
+ }
$snapname = undef;
$size = PVE::QemuServer::Cloudinit::CLOUDINIT_DISK_SIZE;
} elsif ($drivename eq 'efidisk0') {
--
2.20.1
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [pve-devel] [PATCH v2 qemu-server] fix cloning/restoring of cloudinit disks in raw format
2020-12-02 12:50 [pve-devel] [PATCH v2 qemu-server] fix cloning/restoring of cloudinit disks in raw format Mira Limbeck
@ 2020-12-15 13:57 ` Thomas Lamprecht
2020-12-15 14:44 ` Mira Limbeck
2020-12-15 15:18 ` [pve-devel] applied: " Thomas Lamprecht
1 sibling, 1 reply; 4+ messages in thread
From: Thomas Lamprecht @ 2020-12-15 13:57 UTC (permalink / raw)
To: Proxmox VE development discussion, Mira Limbeck
On 02.12.20 13:50, Mira Limbeck wrote:
> We only added the format extension when it was not 'raw'. But on file level
> storages we always require it. To fix this, always add the format
> extension if the storage provides the 'path' property.
> This is the same logic we use in create_disks for cloudinit disks.
>
> Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
> ---
> v2:
> - added the same fix when restoring from backup
>
clone seems to be fine, even if I slowly start to feel real hatred for those
cloudinit disk edge cases, anyway, is there an actual reason why we need to
restore that disk - isn't in gonna get regenerated on first start anyway?
> PVE/QemuServer.pm | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 4989938..412113e 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -5797,7 +5797,10 @@ my $restore_allocate_devices = sub {
> my $name;
> if ($d->{is_cloudinit}) {
> $name = "vm-$vmid-cloudinit";
> - $name .= ".$d->{format}" if $d->{format} ne 'raw';
> + my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
> + if ($scfg->{path}) {
> + $name .= ".$d->{format}";
> + }
> }
>
> my $volid = PVE::Storage::vdisk_alloc(
> @@ -6945,7 +6948,10 @@ sub clone_disk {
> my $size = undef;
> if (drive_is_cloudinit($drive)) {
> $name = "vm-$newvmid-cloudinit";
> - $name .= ".$dst_format" if $dst_format ne 'raw';
> + my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
> + if ($scfg->{path}) {
> + $name .= ".$dst_format";
> + }
> $snapname = undef;
> $size = PVE::QemuServer::Cloudinit::CLOUDINIT_DISK_SIZE;
> } elsif ($drivename eq 'efidisk0') {
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [pve-devel] [PATCH v2 qemu-server] fix cloning/restoring of cloudinit disks in raw format
2020-12-15 13:57 ` Thomas Lamprecht
@ 2020-12-15 14:44 ` Mira Limbeck
0 siblings, 0 replies; 4+ messages in thread
From: Mira Limbeck @ 2020-12-15 14:44 UTC (permalink / raw)
To: Thomas Lamprecht, Proxmox VE development discussion
On 12/15/20 2:57 PM, Thomas Lamprecht wrote:
> On 02.12.20 13:50, Mira Limbeck wrote:
>> We only added the format extension when it was not 'raw'. But on file level
>> storages we always require it. To fix this, always add the format
>> extension if the storage provides the 'path' property.
>> This is the same logic we use in create_disks for cloudinit disks.
>>
>> Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
>> ---
>> v2:
>> - added the same fix when restoring from backup
>>
> clone seems to be fine, even if I slowly start to feel real hatred for those
> cloudinit disk edge cases, anyway, is there an actual reason why we need to
> restore that disk - isn't in gonna get regenerated on first start anyway?
We don't actually restore the disk, but we have to allocate it to
account for a possibly new VMID as well as a different storage (e.g.
directory storage with the VMID as part of the path).
>
>> PVE/QemuServer.pm | 10 ++++++++--
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>> index 4989938..412113e 100644
>> --- a/PVE/QemuServer.pm
>> +++ b/PVE/QemuServer.pm
>> @@ -5797,7 +5797,10 @@ my $restore_allocate_devices = sub {
>> my $name;
>> if ($d->{is_cloudinit}) {
>> $name = "vm-$vmid-cloudinit";
>> - $name .= ".$d->{format}" if $d->{format} ne 'raw';
>> + my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
>> + if ($scfg->{path}) {
>> + $name .= ".$d->{format}";
>> + }
>> }
>>
>> my $volid = PVE::Storage::vdisk_alloc(
>> @@ -6945,7 +6948,10 @@ sub clone_disk {
>> my $size = undef;
>> if (drive_is_cloudinit($drive)) {
>> $name = "vm-$newvmid-cloudinit";
>> - $name .= ".$dst_format" if $dst_format ne 'raw';
>> + my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
>> + if ($scfg->{path}) {
>> + $name .= ".$dst_format";
>> + }
>> $snapname = undef;
>> $size = PVE::QemuServer::Cloudinit::CLOUDINIT_DISK_SIZE;
>> } elsif ($drivename eq 'efidisk0') {
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [pve-devel] applied: [PATCH v2 qemu-server] fix cloning/restoring of cloudinit disks in raw format
2020-12-02 12:50 [pve-devel] [PATCH v2 qemu-server] fix cloning/restoring of cloudinit disks in raw format Mira Limbeck
2020-12-15 13:57 ` Thomas Lamprecht
@ 2020-12-15 15:18 ` Thomas Lamprecht
1 sibling, 0 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2020-12-15 15:18 UTC (permalink / raw)
To: Proxmox VE development discussion, Mira Limbeck
On 02.12.20 13:50, Mira Limbeck wrote:
> We only added the format extension when it was not 'raw'. But on file level
> storages we always require it. To fix this, always add the format
> extension if the storage provides the 'path' property.
> This is the same logic we use in create_disks for cloudinit disks.
>
> Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
> ---
> v2:
> - added the same fix when restoring from backup
>
> PVE/QemuServer.pm | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
>
applied, thanks!
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-12-15 15:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-02 12:50 [pve-devel] [PATCH v2 qemu-server] fix cloning/restoring of cloudinit disks in raw format Mira Limbeck
2020-12-15 13:57 ` Thomas Lamprecht
2020-12-15 14:44 ` Mira Limbeck
2020-12-15 15:18 ` [pve-devel] applied: " Thomas Lamprecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox