* [pve-devel] [PATCH v2 qemu-server 1/2] fix VM clone from snapshot with cloudinit disk @ 2020-09-28 8:36 Mira Limbeck 2020-09-28 8:36 ` [pve-devel] [PATCH v2 qemu-server 2/2] fix clone_disk failing for nonexistent " Mira Limbeck 2020-10-16 13:40 ` [pve-devel] applied: [PATCH v2 qemu-server 1/2] fix VM clone from snapshot with " Thomas Lamprecht 0 siblings, 2 replies; 8+ messages in thread From: Mira Limbeck @ 2020-09-28 8:36 UTC (permalink / raw) To: pve-devel All volumes contained in $vollist are activated. In this case a snapshot of the volume. For cloudinit disks no snapshots are created so don't add it to the list of volumes to activate as it otherwise fails with no logical volume found. Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com> --- v2: unchanged PVE/API2/Qemu.pm | 1 + 1 file changed, 1 insertion(+) diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm index 8da616a..8e1bbc2 100644 --- a/PVE/API2/Qemu.pm +++ b/PVE/API2/Qemu.pm @@ -3000,6 +3000,7 @@ __PACKAGE__->register_method({ if !PVE::Storage::volume_has_feature($storecfg, 'clone', $drive->{file}, $snapname, $running); } $drives->{$opt} = $drive; + next if PVE::QemuServer::drive_is_cloudinit($drive); push @$vollist, $drive->{file}; } } else { -- 2.20.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH v2 qemu-server 2/2] fix clone_disk failing for nonexistent cloudinit disk 2020-09-28 8:36 [pve-devel] [PATCH v2 qemu-server 1/2] fix VM clone from snapshot with cloudinit disk Mira Limbeck @ 2020-09-28 8:36 ` Mira Limbeck 2020-10-05 15:35 ` Thomas Lamprecht 2020-10-16 13:40 ` [pve-devel] applied: " Thomas Lamprecht 2020-10-16 13:40 ` [pve-devel] applied: [PATCH v2 qemu-server 1/2] fix VM clone from snapshot with " Thomas Lamprecht 1 sibling, 2 replies; 8+ messages in thread From: Mira Limbeck @ 2020-09-28 8:36 UTC (permalink / raw) To: pve-devel After migration or a rollback the cloudinit disk might not be allocated, so volume_size_info() fails. As we override the value anyway for cloudinit and efi disks simply move the volume_size_info() call into the 'else' branch. Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com> --- v2: changed subject PVE/QemuServer.pm | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index 2747c66..49765b7 100644 --- a/PVE/QemuServer.pm +++ b/PVE/QemuServer.pm @@ -6895,10 +6895,10 @@ sub clone_disk { $storeid = $storage if $storage; my $dst_format = resolve_dst_disk_format($storecfg, $storeid, $volname, $format); - my ($size) = PVE::Storage::volume_size_info($storecfg, $drive->{file}, 3); print "create full clone of drive $drivename ($drive->{file})\n"; my $name = undef; + my $size = undef; if (drive_is_cloudinit($drive)) { $name = "vm-$newvmid-cloudinit"; $name .= ".$dst_format" if $dst_format ne 'raw'; @@ -6906,6 +6906,8 @@ sub clone_disk { $size = PVE::QemuServer::Cloudinit::CLOUDINIT_DISK_SIZE; } elsif ($drivename eq 'efidisk0') { $size = get_efivars_size($conf); + } else { + ($size) = PVE::Storage::volume_size_info($storecfg, $drive->{file}, 3); } $size /= 1024; $newvolid = PVE::Storage::vdisk_alloc($storecfg, $storeid, $newvmid, $dst_format, $name, $size); -- 2.20.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [pve-devel] [PATCH v2 qemu-server 2/2] fix clone_disk failing for nonexistent cloudinit disk 2020-09-28 8:36 ` [pve-devel] [PATCH v2 qemu-server 2/2] fix clone_disk failing for nonexistent " Mira Limbeck @ 2020-10-05 15:35 ` Thomas Lamprecht 2020-10-06 8:56 ` Mira Limbeck 2020-10-16 13:40 ` [pve-devel] applied: " Thomas Lamprecht 1 sibling, 1 reply; 8+ messages in thread From: Thomas Lamprecht @ 2020-10-05 15:35 UTC (permalink / raw) To: Proxmox VE development discussion, Mira Limbeck On 28.09.20 10:36, Mira Limbeck wrote: > After migration or a rollback the cloudinit disk might not be allocated, so > volume_size_info() fails. As we override the value anyway for cloudinit > and efi disks simply move the volume_size_info() call into the 'else' > branch. > > Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com> > --- > v2: changed subject > > PVE/QemuServer.pm | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm > index 2747c66..49765b7 100644 > --- a/PVE/QemuServer.pm > +++ b/PVE/QemuServer.pm > @@ -6895,10 +6895,10 @@ sub clone_disk { > $storeid = $storage if $storage; > > my $dst_format = resolve_dst_disk_format($storecfg, $storeid, $volname, $format); > - my ($size) = PVE::Storage::volume_size_info($storecfg, $drive->{file}, 3); > > print "create full clone of drive $drivename ($drive->{file})\n"; > my $name = undef; > + my $size = undef; > if (drive_is_cloudinit($drive)) { > $name = "vm-$newvmid-cloudinit"; > $name .= ".$dst_format" if $dst_format ne 'raw'; > @@ -6906,6 +6906,8 @@ sub clone_disk { > $size = PVE::QemuServer::Cloudinit::CLOUDINIT_DISK_SIZE; > } elsif ($drivename eq 'efidisk0') { > $size = get_efivars_size($conf); > + } else { > + ($size) = PVE::Storage::volume_size_info($storecfg, $drive->{file}, 3); > } > $size /= 1024; doesn't this logs a "use of undefined value in division" or something like that somewhere in the non-else case? > $newvolid = PVE::Storage::vdisk_alloc($storecfg, $storeid, $newvmid, $dst_format, $name, $size); > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [pve-devel] [PATCH v2 qemu-server 2/2] fix clone_disk failing for nonexistent cloudinit disk 2020-10-05 15:35 ` Thomas Lamprecht @ 2020-10-06 8:56 ` Mira Limbeck 2020-10-06 11:10 ` Mira Limbeck 0 siblings, 1 reply; 8+ messages in thread From: Mira Limbeck @ 2020-10-06 8:56 UTC (permalink / raw) To: Thomas Lamprecht, Proxmox VE development discussion On 10/5/20 5:35 PM, Thomas Lamprecht wrote: > On 28.09.20 10:36, Mira Limbeck wrote: >> After migration or a rollback the cloudinit disk might not be allocated, so >> volume_size_info() fails. As we override the value anyway for cloudinit >> and efi disks simply move the volume_size_info() call into the 'else' >> branch. >> >> Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com> >> --- >> v2: changed subject >> >> PVE/QemuServer.pm | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm >> index 2747c66..49765b7 100644 >> --- a/PVE/QemuServer.pm >> +++ b/PVE/QemuServer.pm >> @@ -6895,10 +6895,10 @@ sub clone_disk { >> $storeid = $storage if $storage; >> >> my $dst_format = resolve_dst_disk_format($storecfg, $storeid, $volname, $format); >> - my ($size) = PVE::Storage::volume_size_info($storecfg, $drive->{file}, 3); >> >> print "create full clone of drive $drivename ($drive->{file})\n"; >> my $name = undef; >> + my $size = undef; >> if (drive_is_cloudinit($drive)) { >> $name = "vm-$newvmid-cloudinit"; >> $name .= ".$dst_format" if $dst_format ne 'raw'; >> @@ -6906,6 +6906,8 @@ sub clone_disk { >> $size = PVE::QemuServer::Cloudinit::CLOUDINIT_DISK_SIZE; >> } elsif ($drivename eq 'efidisk0') { >> $size = get_efivars_size($conf); >> + } else { >> + ($size) = PVE::Storage::volume_size_info($storecfg, $drive->{file}, 3); >> } >> $size /= 1024; > doesn't this logs a "use of undefined value in division" or something like that > somewhere in the non-else case? No, in the cloudinit case we set it to a constant. In the efidisk case we call get_efivars_size() which dies if efivars is not a file, otherwise we get a size (-s). And in the else case we also die if we can't get the size. So size is set in every case before the first use. > >> $newvolid = PVE::Storage::vdisk_alloc($storecfg, $storeid, $newvmid, $dst_format, $name, $size); >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [pve-devel] [PATCH v2 qemu-server 2/2] fix clone_disk failing for nonexistent cloudinit disk 2020-10-06 8:56 ` Mira Limbeck @ 2020-10-06 11:10 ` Mira Limbeck 2020-10-16 13:45 ` Thomas Lamprecht 0 siblings, 1 reply; 8+ messages in thread From: Mira Limbeck @ 2020-10-06 11:10 UTC (permalink / raw) To: pve-devel On 10/6/20 10:56 AM, Mira Limbeck wrote: > > On 10/5/20 5:35 PM, Thomas Lamprecht wrote: >> On 28.09.20 10:36, Mira Limbeck wrote: >>> After migration or a rollback the cloudinit disk might not be >>> allocated, so >>> volume_size_info() fails. As we override the value anyway for cloudinit >>> and efi disks simply move the volume_size_info() call into the 'else' >>> branch. >>> >>> Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com> >>> --- >>> v2: changed subject >>> >>> PVE/QemuServer.pm | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm >>> index 2747c66..49765b7 100644 >>> --- a/PVE/QemuServer.pm >>> +++ b/PVE/QemuServer.pm >>> @@ -6895,10 +6895,10 @@ sub clone_disk { >>> $storeid = $storage if $storage; >>> my $dst_format = resolve_dst_disk_format($storecfg, >>> $storeid, $volname, $format); >>> - my ($size) = PVE::Storage::volume_size_info($storecfg, >>> $drive->{file}, 3); >>> print "create full clone of drive $drivename >>> ($drive->{file})\n"; >>> my $name = undef; >>> + my $size = undef; >>> if (drive_is_cloudinit($drive)) { >>> $name = "vm-$newvmid-cloudinit"; >>> $name .= ".$dst_format" if $dst_format ne 'raw'; >>> @@ -6906,6 +6906,8 @@ sub clone_disk { >>> $size = PVE::QemuServer::Cloudinit::CLOUDINIT_DISK_SIZE; >>> } elsif ($drivename eq 'efidisk0') { >>> $size = get_efivars_size($conf); >>> + } else { >>> + ($size) = PVE::Storage::volume_size_info($storecfg, >>> $drive->{file}, 3); >>> } >>> $size /= 1024; >> doesn't this logs a "use of undefined value in division" or something >> like that >> somewhere in the non-else case? > No, in the cloudinit case we set it to a constant. In the efidisk case > we call get_efivars_size() which dies if efivars is not a file, > otherwise we get a size (-s). And in the else case we also die if we > can't get the size. So size is set in every case before the first use. To clarify, the returned size in volume_size_info can be undefined, but not the other 2 cases. Would a die be a good idea in case the volume_size_info call returns 'undef'? >> >>> $newvolid = PVE::Storage::vdisk_alloc($storecfg, $storeid, >>> $newvmid, $dst_format, $name, $size); >>> > > > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [pve-devel] [PATCH v2 qemu-server 2/2] fix clone_disk failing for nonexistent cloudinit disk 2020-10-06 11:10 ` Mira Limbeck @ 2020-10-16 13:45 ` Thomas Lamprecht 0 siblings, 0 replies; 8+ messages in thread From: Thomas Lamprecht @ 2020-10-16 13:45 UTC (permalink / raw) To: Proxmox VE development discussion, Mira Limbeck On 06.10.20 13:10, Mira Limbeck wrote: > On 10/6/20 10:56 AM, Mira Limbeck wrote: >> On 10/5/20 5:35 PM, Thomas Lamprecht wrote: >>> On 28.09.20 10:36, Mira Limbeck wrote: >>>> @@ -6906,6 +6906,8 @@ sub clone_disk { >>>> $size = PVE::QemuServer::Cloudinit::CLOUDINIT_DISK_SIZE; >>>> } elsif ($drivename eq 'efidisk0') { >>>> $size = get_efivars_size($conf); >>>> + } else { >>>> + ($size) = PVE::Storage::volume_size_info($storecfg, $drive->{file}, 3); >>>> } >>>> $size /= 1024; >>> doesn't this logs a "use of undefined value in division" or something like that >>> somewhere in the non-else case? >> No, in the cloudinit case we set it to a constant. In the efidisk case we call get_efivars_size() which dies if efivars is not a file, otherwise we get a size (-s). And in the else case we also die if we can't get the size. So size is set in every case before the first use. > To clarify, the returned size in volume_size_info can be undefined, but not the other 2 cases. Would a die be a good idea in case the volume_size_info call returns 'undef'? could be, could be possible to assert that in vdisk_alloc, as there it rather seems that it's assumed to never be undef; but I just checked the base and ZFS Plugin, so.. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] applied: [PATCH v2 qemu-server 2/2] fix clone_disk failing for nonexistent cloudinit disk 2020-09-28 8:36 ` [pve-devel] [PATCH v2 qemu-server 2/2] fix clone_disk failing for nonexistent " Mira Limbeck 2020-10-05 15:35 ` Thomas Lamprecht @ 2020-10-16 13:40 ` Thomas Lamprecht 1 sibling, 0 replies; 8+ messages in thread From: Thomas Lamprecht @ 2020-10-16 13:40 UTC (permalink / raw) To: Proxmox VE development discussion, Mira Limbeck On 28.09.20 10:36, Mira Limbeck wrote: > After migration or a rollback the cloudinit disk might not be allocated, so > volume_size_info() fails. As we override the value anyway for cloudinit > and efi disks simply move the volume_size_info() call into the 'else' > branch. > > Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com> > --- > v2: changed subject > > PVE/QemuServer.pm | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > applied, thanks! ^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] applied: [PATCH v2 qemu-server 1/2] fix VM clone from snapshot with cloudinit disk 2020-09-28 8:36 [pve-devel] [PATCH v2 qemu-server 1/2] fix VM clone from snapshot with cloudinit disk Mira Limbeck 2020-09-28 8:36 ` [pve-devel] [PATCH v2 qemu-server 2/2] fix clone_disk failing for nonexistent " Mira Limbeck @ 2020-10-16 13:40 ` Thomas Lamprecht 1 sibling, 0 replies; 8+ messages in thread From: Thomas Lamprecht @ 2020-10-16 13:40 UTC (permalink / raw) To: Proxmox VE development discussion, Mira Limbeck On 28.09.20 10:36, Mira Limbeck wrote: > All volumes contained in $vollist are activated. In this case a snapshot > of the volume. For cloudinit disks no snapshots are created so don't add > it to the list of volumes to activate as it otherwise fails with no > logical volume found. > > Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com> > --- > v2: unchanged > > PVE/API2/Qemu.pm | 1 + > 1 file changed, 1 insertion(+) > > applied, thanks! ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-10-16 13:45 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-28 8:36 [pve-devel] [PATCH v2 qemu-server 1/2] fix VM clone from snapshot with cloudinit disk Mira Limbeck 2020-09-28 8:36 ` [pve-devel] [PATCH v2 qemu-server 2/2] fix clone_disk failing for nonexistent " Mira Limbeck 2020-10-05 15:35 ` Thomas Lamprecht 2020-10-06 8:56 ` Mira Limbeck 2020-10-06 11:10 ` Mira Limbeck 2020-10-16 13:45 ` Thomas Lamprecht 2020-10-16 13:40 ` [pve-devel] applied: " Thomas Lamprecht 2020-10-16 13:40 ` [pve-devel] applied: [PATCH v2 qemu-server 1/2] fix VM clone from snapshot with " Thomas Lamprecht
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox