Previously a VMID conflict was possible when creating a VM on another node
between locking the config with lock_config_full and writing to it for the
first time with write_config.
Using create_and_lock_config eliminates this possibility. This means that now
the "lock" property is set in the config instead of using flock only.
$param was empty when it was assigned the three values "name", "memory" and
"cores" before being assigned to $conf later on. Assigning those values
directly to $conf avoids confusion about what the two variables contain.
Signed-off-by: Dominic Jäger <d.jae...@proxmox.com>
---
v1->v2:
- Add note about $param in commit message
- Improve commit message, especially replacing "parameter lock" with "lock
config"
- Remove unnecessary semicolon in one-liner
- Adapted error message
- Use return early pattern
PVE/CLI/qm.pm | 66 +++++++++++++++++++++++++--------------------------
1 file changed, 32 insertions(+), 34 deletions(-)
diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
index 3bf5f97..a441ac1 100755
--- a/PVE/CLI/qm.pm
+++ b/PVE/CLI/qm.pm
@@ -621,47 +621,45 @@ __PACKAGE__->register_method ({
return;
}
- $param->{name} = $parsed->{qm}->{name} if
defined($parsed->{qm}->{name});
- $param->{memory} = $parsed->{qm}->{memory} if
defined($parsed->{qm}->{memory});
- $param->{cores} = $parsed->{qm}->{cores} if
defined($parsed->{qm}->{cores});
+ eval { PVE::QemuConfig->create_and_lock_config($vmid, 0) };
+ die "Reserving empty config for OVF import failed: $@" if $@;
- my $importfn = sub {
+ my $conf = PVE::QemuConfig->load_config($vmid);
+ die "Internal error: Expected 'create' lock in config of VM $vmid!"
+ if !PVE::QemuConfig->has_lock($conf, "create");
- PVE::Cluster::check_vmid_unused($vmid);
+ $conf->{name} = $parsed->{qm}->{name} if defined($parsed->{qm}->{name});
+ $conf->{memory} = $parsed->{qm}->{memory} if
defined($parsed->{qm}->{memory});
+ $conf->{cores} = $parsed->{qm}->{cores} if
defined($parsed->{qm}->{cores});
- my $conf = $param;
-
- eval {
- # order matters, as do_import() will load_config() internally
- $conf->{vmgenid} = PVE::QemuServer::generate_uuid();
- $conf->{smbios1} = PVE::QemuServer::generate_smbios1_uuid();
- PVE::QemuConfig->write_config($vmid, $conf);
-
- foreach my $disk (@{ $parsed->{disks} }) {
- my ($file, $drive) = ($disk->{backing_file},
$disk->{disk_address});
- PVE::QemuServer::ImportDisk::do_import($file, $vmid,
$storeid,
- 0, { drive_name => $drive, format => $format });
- }
-
- # reload after disks entries have been created
- $conf = PVE::QemuConfig->load_config($vmid);
- my $firstdisk = PVE::QemuServer::resolve_first_disk($conf);
- $conf->{bootdisk} = $firstdisk if $firstdisk;
- PVE::QemuConfig->write_config($vmid, $conf);
- };
+ eval {
+ # order matters, as do_import() will load_config() internally
+ $conf->{vmgenid} = PVE::QemuServer::generate_uuid();
+ $conf->{smbios1} = PVE::QemuServer::generate_smbios1_uuid();
+ PVE::QemuConfig->write_config($vmid, $conf);
- my $err = $@;
- if ($err) {
- my $skiplock = 1;
- # eval for additional safety in error path
- eval { PVE::QemuServer::destroy_vm($storecfg, $vmid, undef,
$skiplock) };
- warn "Could not destroy VM $vmid: $@" if "$@";
- die "import failed - $err";
+ foreach my $disk (@{ $parsed->{disks} }) {
+ my ($file, $drive) = ($disk->{backing_file},
$disk->{disk_address});
+ PVE::QemuServer::ImportDisk::do_import($file, $vmid, $storeid,
+ 1, { drive_name => $drive, format => $format });
}
+
+ # reload after disks entries have been created
+ $conf = PVE::QemuConfig->load_config($vmid);
+ my $firstdisk = PVE::QemuServer::resolve_first_disk($conf);
+ $conf->{bootdisk} = $firstdisk if $firstdisk;
+ PVE::QemuConfig->write_config($vmid, $conf);
};
- my $wait_for_lock = 1;
- PVE::QemuConfig->lock_config_full($vmid, $wait_for_lock, $importfn);
+ my $err = $@;
+ if ($err) {
+ my $skiplock = 1;
+ # eval for additional safety in error path
+ eval { PVE::QemuServer::destroy_vm($storecfg, $vmid, undef,
$skiplock) };
+ warn "Could not destroy VM $vmid: $@" if "$@";
+ die "import failed - $err";
+ }
+ PVE::QemuConfig->remove_lock ($vmid, "create");
return undef;
--
2.20.1
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
