[pve-devel] [PATCH qemu-server] Make foreach_drive order deterministic
Fabian Grünbichler
f.gruenbichler at proxmox.com
Thu Mar 3 15:45:15 CET 2016
Previously, foreach_drive iterated over all configuration
keys (in a random order) and checked whether the current key
is a valid drive name. Instead, we now iterate over a list
of valid drive names (with deterministic order) and check
whether a drive with such a name exists in the
configuration.
Also rename the two involved methods from valid_drive_name
to is_valid_drive_name (for the check) and from disknames
to valid_drive_names (for the list of valid keys), for
consistency. These two were only used in the qemu-server
code base.
---
This makes the behaviour more similar to PVE::LXC::Config->
foreach_mountpoint, and should also be more efficient.
PVE/API2/Qemu.pm | 20 ++++++++++----------
PVE/QemuServer.pm | 30 +++++++++++++++---------------
2 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index efac2c7..0be373c 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -182,7 +182,7 @@ my $check_vm_modify_config_perm = sub {
foreach my $opt (@$key_list) {
# disk checks need to be done somewhere else
- next if PVE::QemuServer::valid_drivename($opt);
+ next if PVE::QemuServer::is_valid_drivename($opt);
if ($opt eq 'sockets' || $opt eq 'cores' ||
$opt eq 'cpu' || $opt eq 'smp' || $opt eq 'vcpus' ||
@@ -371,7 +371,7 @@ __PACKAGE__->register_method({
&$check_vm_modify_config_perm($rpcenv, $authuser, $vmid, $pool, [ keys %$param]);
foreach my $opt (keys %$param) {
- if (PVE::QemuServer::valid_drivename($opt)) {
+ if (PVE::QemuServer::is_valid_drivename($opt)) {
my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt});
raise_param_exc({ $opt => "unable to parse drive options" }) if !$drive;
@@ -441,7 +441,7 @@ __PACKAGE__->register_method({
$vollist = &$create_disks($rpcenv, $authuser, $conf, $storecfg, $vmid, $pool, $param, $storage);
# try to be smart about bootdisk
- my @disks = PVE::QemuServer::disknames();
+ my @disks = PVE::QemuServer::valid_drive_names();
my $firstdisk;
foreach my $ds (reverse @disks) {
next if !$conf->{$ds};
@@ -851,7 +851,7 @@ my $update_vm_api = sub {
}
foreach my $opt (keys %$param) {
- if (PVE::QemuServer::valid_drivename($opt)) {
+ if (PVE::QemuServer::is_valid_drivename($opt)) {
# cleanup drive path
my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt});
PVE::QemuServer::cleanup_drive_path($opt, $storecfg, $drive);
@@ -915,7 +915,7 @@ my $update_vm_api = sub {
delete $conf->{$opt};
PVE::QemuServer::write_config($vmid, $conf);
}
- } elsif (PVE::QemuServer::valid_drivename($opt)) {
+ } elsif (PVE::QemuServer::is_valid_drivename($opt)) {
&$check_protection($conf, "can't remove drive '$opt'");
$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.Disk']);
PVE::QemuServer::vmconfig_register_unused_drive($storecfg, $vmid, $conf, PVE::QemuServer::parse_drive($opt, $conf->{pending}->{$opt}))
@@ -933,7 +933,7 @@ my $update_vm_api = sub {
$conf = PVE::QemuServer::load_config($vmid); # update/reload
next if defined($conf->{pending}->{$opt}) && ($param->{$opt} eq $conf->{pending}->{$opt}); # skip if nothing changed
- if (PVE::QemuServer::valid_drivename($opt)) {
+ if (PVE::QemuServer::is_valid_drivename($opt)) {
my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt});
if (PVE::QemuServer::drive_is_cdrom($drive)) { # CDROM
$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.CDROM']);
@@ -2228,7 +2228,7 @@ __PACKAGE__->register_method({
my $net = PVE::QemuServer::parse_net($value);
$net->{macaddr} = PVE::Tools::random_ether_addr();
$newconf->{$opt} = PVE::QemuServer::print_net($net);
- } elsif (PVE::QemuServer::valid_drivename($opt)) {
+ } elsif (PVE::QemuServer::is_valid_drivename($opt)) {
my $drive = PVE::QemuServer::parse_drive($opt, $value);
die "unable to parse drive options for '$opt'\n" if !$drive;
if (PVE::QemuServer::drive_is_cdrom($drive)) {
@@ -2365,7 +2365,7 @@ __PACKAGE__->register_method({
disk => {
type => 'string',
description => "The disk you want to move.",
- enum => [ PVE::QemuServer::disknames() ],
+ enum => [ PVE::QemuServer::valid_drive_names() ],
},
storage => get_standard_option('pve-storage-id', {
description => "Target storage.",
@@ -2657,7 +2657,7 @@ __PACKAGE__->register_method({
disk => {
type => 'string',
description => "The disk you want to resize.",
- enum => [PVE::QemuServer::disknames()],
+ enum => [PVE::QemuServer::valid_drive_names()],
},
size => {
type => 'string',
@@ -3115,7 +3115,7 @@ __PACKAGE__->register_method({
optional => 1,
type => 'string',
description => "If you want to convert only 1 disk to base image.",
- enum => [PVE::QemuServer::disknames()],
+ enum => [PVE::QemuServer::valid_drive_names()],
},
},
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 92b0e6a..17b43d2 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -899,7 +899,7 @@ sub kvm_user_version {
my $kernel_has_vhost_net = -c '/dev/vhost-net';
-sub disknames {
+sub valid_drive_names {
# order is important - used to autoselect boot disk
return ((map { "ide$_" } (0 .. ($MAX_IDE_DISKS - 1))),
(map { "scsi$_" } (0 .. ($MAX_SCSI_DISKS - 1))),
@@ -907,7 +907,7 @@ sub disknames {
(map { "sata$_" } (0 .. ($MAX_SATA_DISKS - 1))));
}
-sub valid_drivename {
+sub is_valid_drivename {
my $dev = shift;
return defined($drivename_hash->{$dev});
@@ -1731,7 +1731,7 @@ PVE::JSONSchema::register_format('pve-qm-bootdisk', \&verify_bootdisk);
sub verify_bootdisk {
my ($value, $noerr) = @_;
- return $value if valid_drivename($value);
+ return $value if is_valid_drivename($value);
return undef if $noerr;
@@ -2160,7 +2160,7 @@ sub write_vm_config {
$cref->{$key} = $value;
- if (!$snapname && valid_drivename($key)) {
+ if (!$snapname && is_valid_drivename($key)) {
my $drive = parse_drive($key, $value);
$used_volids->{$drive->{file}} = 1 if $drive && $drive->{file};
}
@@ -2424,7 +2424,7 @@ sub disksize {
my $bootdisk = $conf->{bootdisk};
return undef if !$bootdisk;
- return undef if !valid_drivename($bootdisk);
+ return undef if !is_valid_drivename($bootdisk);
return undef if !$conf->{$bootdisk};
@@ -2685,8 +2685,8 @@ sub foreach_reverse_dimm {
sub foreach_drive {
my ($conf, $func) = @_;
- foreach my $ds (keys %$conf) {
- next if !valid_drivename($ds);
+ foreach my $ds (valid_drive_names()) {
+ next if !defined($conf->{$ds});
my $drive = parse_drive($ds, $conf->{$ds});
next if !$drive;
@@ -3725,7 +3725,7 @@ sub qemu_deletescsihw {
my $devices_list = vm_devices_list($vmid);
foreach my $opt (keys %{$devices_list}) {
- if (PVE::QemuServer::valid_drivename($opt)) {
+ if (PVE::QemuServer::is_valid_drivename($opt)) {
my $drive = PVE::QemuServer::parse_drive($opt, $conf->{$opt});
if($drive->{interface} eq 'scsi' && $drive->{index} < (($maxdev-1)*($controller+1))) {
return 1;
@@ -4161,7 +4161,7 @@ sub vmconfig_hotplug_pending {
} elsif ($opt =~ m/^net(\d+)$/) {
die "skip\n" if !$hotplug_features->{network};
vm_deviceunplug($vmid, $conf, $opt);
- } elsif (valid_drivename($opt)) {
+ } elsif (is_valid_drivename($opt)) {
die "skip\n" if !$hotplug_features->{disk} || $opt =~ m/(ide|sata)(\d+)/;
vm_deviceunplug($vmid, $conf, $opt);
vmconfig_delete_or_detach_drive($vmid, $storecfg, $conf, $opt, $force);
@@ -4218,7 +4218,7 @@ sub vmconfig_hotplug_pending {
# some changes can be done without hotplug
vmconfig_update_net($storecfg, $conf, $hotplug_features->{network},
$vmid, $opt, $value);
- } elsif (valid_drivename($opt)) {
+ } elsif (is_valid_drivename($opt)) {
# some changes can be done without hotplug
vmconfig_update_disk($storecfg, $conf, $hotplug_features->{disk},
$vmid, $opt, $value, 1);
@@ -4297,7 +4297,7 @@ sub vmconfig_apply_pending {
if (!defined($conf->{$opt})) {
vmconfig_undelete_pending_option($conf, $opt);
write_config($vmid, $conf);
- } elsif (valid_drivename($opt)) {
+ } elsif (is_valid_drivename($opt)) {
vmconfig_delete_or_detach_drive($vmid, $storecfg, $conf, $opt, $force);
vmconfig_undelete_pending_option($conf, $opt);
delete $conf->{$opt};
@@ -4316,7 +4316,7 @@ sub vmconfig_apply_pending {
if (defined($conf->{$opt}) && ($conf->{$opt} eq $conf->{pending}->{$opt})) {
# skip if nothing changed
- } elsif (valid_drivename($opt)) {
+ } elsif (is_valid_drivename($opt)) {
vmconfig_register_unused_drive($storecfg, $vmid, $conf, parse_drive($opt, $conf->{$opt}))
if defined($conf->{$opt});
$conf->{$opt} = $conf->{pending}->{$opt};
@@ -5293,7 +5293,7 @@ sub is_volume_in_use {
foreach my $key (keys %$cref) {
my $value = $cref->{$key};
- if (valid_drivename($key)) {
+ if (is_valid_drivename($key)) {
next if $skip_drive && $key eq $skip_drive;
my $drive = parse_drive($key, $value);
next if !$drive || !$drive->{file} || drive_is_cdrom($drive);
@@ -5339,7 +5339,7 @@ sub update_disksize {
# update size info
foreach my $opt (keys %$conf) {
- if (valid_drivename($opt)) {
+ if (is_valid_drivename($opt)) {
my $drive = parse_drive($opt, $conf->{$opt});
my $volid = $drive->{file};
next if !$volid;
@@ -5849,7 +5849,7 @@ sub foreach_writable_storage {
my $sidhash = {};
foreach my $ds (keys %$conf) {
- next if !valid_drivename($ds);
+ next if !is_valid_drivename($ds);
my $drive = parse_drive($ds, $conf->{$ds});
next if !$drive;
--
2.1.4
More information about the pve-devel
mailing list