From f7d1505b0c9347c8664c86bb64c44ab8714d6cd4 Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Fri, 16 Oct 2020 17:52:51 +0200 Subject: [PATCH] tree wide cleanups Signed-off-by: Thomas Lamprecht --- PVE/CLI/qm.pm | 13 ++++--------- PVE/QMPClient.pm | 4 ++-- PVE/QemuServer.pm | 16 +++++++--------- PVE/QemuServer/CPUConfig.pm | 14 ++++++++++---- PVE/QemuServer/Cloudinit.pm | 2 +- PVE/QemuServer/Drive.pm | 4 ++-- PVE/QemuServer/Memory.pm | 30 ++++++++++++++---------------- PVE/QemuServer/PCI.pm | 3 ++- test/snapshot-test.pm | 12 ++++++------ 9 files changed, 48 insertions(+), 50 deletions(-) diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm index 16e654bc..b3b9251c 100755 --- a/PVE/CLI/qm.pm +++ b/PVE/CLI/qm.pm @@ -61,7 +61,7 @@ sub run_vnc_proxy { die "unable to connect to socket '$path' - $!" if !$s; - my $select = new IO::Select; + my $select = IO::Select->new(); $select->add(\*STDIN); $select->add($s); @@ -392,19 +392,14 @@ __PACKAGE__->register_method ({ print "Entering Qemu Monitor for VM $vmid - type 'help' for help\n"; - my $term = new Term::ReadLine ('qm'); + my $term = Term::ReadLine->new('qm'); - my $input; - while (defined ($input = $term->readline('qm> '))) { + while (defined(my $input = $term->readline('qm> '))) { chomp $input; - next if $input =~ m/^\s*$/; - last if $input =~ m/^\s*q(uit)?\s*$/; - eval { - print PVE::QemuServer::Monitor::hmp_cmd($vmid, $input); - }; + eval { print PVE::QemuServer::Monitor::hmp_cmd($vmid, $input) }; print "ERROR: $@" if $@; } diff --git a/PVE/QMPClient.pm b/PVE/QMPClient.pm index 9d41112b..ea4dc0ba 100644 --- a/PVE/QMPClient.pm +++ b/PVE/QMPClient.pm @@ -17,12 +17,12 @@ use PVE::QemuServer::Helpers; # This implementation uses IO::Multiplex (libio-multiplex-perl) and # allows you to issue qmp and qga commands to different VMs in parallel. -# Note: qemu can onyl handle 1 connection, so we close connections asap +# Note: qemu can only handle 1 connection, so we close connections asap sub new { my ($class, $eventcb) = @_; - my $mux = new IO::Multiplex; + my $mux = IO::Multiplex->new(); my $self = bless { mux => $mux, diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index a138b2e3..418a2a0f 100644 --- a/PVE/QemuServer.pm +++ b/PVE/QemuServer.pm @@ -5936,7 +5936,8 @@ sub update_disk_config { my $volid = $drive->{file}; return if !$volid; - my $path = $volid_hash->{$volid}->{path} if $volid_hash->{$volid}; + my $path; + $path = $volid_hash->{$volid}->{path} if $volid_hash->{$volid}; if ($referenced->{$volid} || ($path && $referencedpath->{$path})) { print "$prefix remove entry '$opt', its volume '$volid' is in use\n"; $changes = 1; @@ -6157,8 +6158,7 @@ sub restore_proxmox_backup_archive { $fh->seek(0, 0) || die "seek failed - $!\n"; - my $outfd = new IO::File ($tmpfn, "w") || - die "unable to write config for VM $vmid\n"; + my $outfd = IO::File->new($tmpfn, "w") || die "unable to write config for VM $vmid\n"; my $cookie = { netcount => 0 }; while (defined(my $line = <$fh>)) { @@ -6331,8 +6331,7 @@ sub restore_vma_archive { $fh->seek(0, 0) || die "seek failed - $!\n"; - my $outfd = new IO::File ($tmpfn, "w") || - die "unable to write config for VM $vmid\n"; + my $outfd = IO::File->new($tmpfn, "w") || die "unable to write config for VM $vmid\n"; my $cookie = { netcount => 0 }; while (defined(my $line = <$fh>)) { @@ -6482,11 +6481,9 @@ sub restore_tar_archive { my $confsrc = "$tmpdir/qemu-server.conf"; - my $srcfd = new IO::File($confsrc, "r") || - die "unable to open file '$confsrc'\n"; + my $srcfd = IO::File->new($confsrc, "r") || die "unable to open file '$confsrc'\n"; - my $outfd = new IO::File ($tmpfn, "w") || - die "unable to write config for VM $vmid\n"; + my $outfd = IO::File->new($tmpfn, "w") || die "unable to write config for VM $vmid\n"; my $cookie = { netcount => 0 }; while (defined (my $line = <$srcfd>)) { @@ -6541,6 +6538,7 @@ sub do_snapshots_with_qemu { my $storage_name = PVE::Storage::parse_volume_id($volid); my $scfg = $storecfg->{ids}->{$storage_name}; + die "could not find storage '$storage_name'\n" if !defined($scfg); if ($qemu_snap_storage->{$scfg->{type}} && !$scfg->{krbd}){ return 1; diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm index 6a7dbd05..32192f20 100644 --- a/PVE/QemuServer/CPUConfig.pm +++ b/PVE/QemuServer/CPUConfig.pm @@ -485,11 +485,17 @@ sub get_cpu_options { if defined($cpu->{'hv-vendor-id'}); } - my $pve_flags = get_pve_cpu_flags($conf, $kvm, $cputype, $arch, - $machine_version); + my $pve_flags = get_pve_cpu_flags($conf, $kvm, $cputype, $arch, $machine_version); - my $hv_flags = get_hyperv_enlightenments($winversion, $machine_version, - $conf->{bios}, $gpu_passthrough, $hv_vendor_id) if $kvm; + my $hv_flags = $kvm + ? get_hyperv_enlightenments( + $winversion, + $machine_version, + $conf->{bios}, + $gpu_passthrough, + $hv_vendor_id, + ) + : undef; my $custom_cputype_flags = parse_cpuflag_list($cpu_flag_any_re, "set by custom CPU model", $custom_cpu->{flags}); diff --git a/PVE/QemuServer/Cloudinit.pm b/PVE/QemuServer/Cloudinit.pm index 439de99c..8b8ffd48 100644 --- a/PVE/QemuServer/Cloudinit.pm +++ b/PVE/QemuServer/Cloudinit.pm @@ -128,7 +128,7 @@ sub cloudinit_userdata { if (defined(my $keys = $conf->{sshkeys})) { $keys = URI::Escape::uri_unescape($keys); - $keys = [map { chomp $_; $_ } split(/\n/, $keys)]; + $keys = [map { my $key = $_; chomp $key; $key } split(/\n/, $keys)]; $keys = [grep { /\S/ } @$keys]; $content .= "ssh_authorized_keys:\n"; foreach my $k (@$keys) { diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm index 53c95589..d5609375 100644 --- a/PVE/QemuServer/Drive.pm +++ b/PVE/QemuServer/Drive.pm @@ -504,8 +504,8 @@ sub print_drive { sub get_bootdisks { my ($conf) = @_; - my $bootcfg = PVE::JSONSchema::parse_property_string('pve-qm-boot', $conf->{boot}) - if $conf->{boot}; + my $bootcfg; + $bootcfg = PVE::JSONSchema::parse_property_string('pve-qm-boot', $conf->{boot}) if $conf->{boot}; if (!defined($bootcfg) || $bootcfg->{legacy}) { return [$conf->{bootdisk}] if $conf->{bootdisk}; diff --git a/PVE/QemuServer/Memory.pm b/PVE/QemuServer/Memory.pm index b7cf5d57..f3e15f1d 100644 --- a/PVE/QemuServer/Memory.pm +++ b/PVE/QemuServer/Memory.pm @@ -93,11 +93,11 @@ sub foreach_reverse_dimm { for (my $j = 0; $j < 8; $j++) { for (my $i = 0; $i < 32; $i++) { - my $name = "dimm${dimm_id}"; - $dimm_id--; + my $name = "dimm${dimm_id}"; + $dimm_id--; my $numanode = $numa_map[(31-$i) % @numa_map]; - $current_size -= $dimm_size; - &$func($conf, $vmid, $name, $dimm_size, $numanode, $current_size, $memory); + $current_size -= $dimm_size; + &$func($conf, $vmid, $name, $dimm_size, $numanode, $current_size, $memory); return $current_size if $current_size <= $memory; } $dimm_size /= 2; @@ -124,14 +124,15 @@ sub qemu_memory_hotplug { if($value > $memory) { - my $numa_hostmap = get_numa_guest_to_host_map($conf) if $conf->{hugepages}; + my $numa_hostmap; - foreach_dimm($conf, $vmid, $value, $sockets, sub { + foreach_dimm($conf, $vmid, $value, $sockets, sub { my ($conf, $vmid, $name, $dimm_size, $numanode, $current_size, $memory) = @_; return if $current_size <= $conf->{memory}; if ($conf->{hugepages}) { + $numa_hostmap = get_numa_guest_to_host_map($conf) if !$numa_hostmap; my $hugepages_size = hugepages_size($conf, $dimm_size); my $path = hugepages_mount_path($hugepages_size); @@ -174,14 +175,14 @@ sub qemu_memory_hotplug { } else { - foreach_reverse_dimm($conf, $vmid, $value, $sockets, sub { + foreach_reverse_dimm($conf, $vmid, $value, $sockets, sub { my ($conf, $vmid, $name, $dimm_size, $numanode, $current_size, $memory) = @_; return if $current_size >= $conf->{memory}; print "try to unplug memory dimm $name\n"; my $retry = 0; - while (1) { + while (1) { eval { PVE::QemuServer::qemu_devicedel($vmid, $name) }; sleep 3; my $dimm_list = qemu_dimm_list($vmid); @@ -292,21 +293,18 @@ sub config { if $numa_totalmemory && $numa_totalmemory != $static_memory; #if no custom tology, we split memory and cores across numa nodes - if(!$numa_totalmemory) { - + if (!$numa_totalmemory) { my $numa_memory = ($static_memory / $sockets); for (my $i = 0; $i < $sockets; $i++) { die "host NUMA node$i doesn't exist\n" if ! -d "/sys/devices/system/node/node$i/" && $conf->{hugepages}; - my $cpustart = ($cores * $i); - my $cpuend = ($cpustart + $cores - 1) if $cores && $cores > 1; - my $cpus = $cpustart; - $cpus .= "-$cpuend" if $cpuend; - my $mem_object = print_mem_object($conf, "ram-node$i", $numa_memory); - push @$cmd, '-object', $mem_object; + + my $cpus = ($cores * $i); + $cpus .= "-" . ($cpus + $cores - 1) if $cores > 1; + push @$cmd, '-numa', "node,nodeid=$i,cpus=$cpus,memdev=ram-node$i"; } } diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm index 789dd223..92adb90d 100644 --- a/PVE/QemuServer/PCI.pm +++ b/PVE/QemuServer/PCI.pm @@ -363,6 +363,7 @@ sub print_hostpci_devices { my $gpu_passthrough = 0; my $legacy_igd = 0; + my $pciaddr; for (my $i = 0; $i < $MAX_HOSTPCI_DEVICES; $i++) { my $id = "hostpci$i"; my $d = parse_hostpci($conf->{$id}); @@ -388,7 +389,7 @@ sub print_hostpci_devices { } my $pcidevices = $d->{pciid}; - my $multifunction = 1 if @$pcidevices > 1; + my $multifunction = @$pcidevices > 1; if ($d->{'legacy-igd'}) { die "only one device can be assigned in legacy-igd mode\n" diff --git a/test/snapshot-test.pm b/test/snapshot-test.pm index 0b073798..cd007250 100644 --- a/test/snapshot-test.pm +++ b/test/snapshot-test.pm @@ -381,13 +381,13 @@ sub vm_stop { PVE::Tools::run_command("rm -rf snapshot-working"); PVE::Tools::run_command("cp -a snapshot-input snapshot-working"); -my $qemu_helpers_module = new Test::MockModule('PVE::QemuServer::Helpers'); +my $qemu_helpers_module = Test::MockModule->new('PVE::QemuServer::Helpers'); $qemu_helpers_module->mock('vm_running_locally', \&vm_running_locally); -my $qemu_monitor_module = new Test::MockModule('PVE::QemuServer::Monitor'); +my $qemu_monitor_module = Test::MockModule->new('PVE::QemuServer::Monitor'); $qemu_monitor_module->mock('qmp_cmd', \&qmp_cmd); -my $qemu_config_module = new Test::MockModule('PVE::QemuConfig'); +my $qemu_config_module = Test::MockModule->new('PVE::QemuConfig'); $qemu_config_module->mock('config_file_lock', \&config_file_lock); $qemu_config_module->mock('cfs_config_path', \&cfs_config_path); $qemu_config_module->mock('load_config', \&load_config); @@ -397,11 +397,11 @@ $qemu_config_module->mock('__snapshot_save_vmstate', \&__snapshot_save_vmstate); $qemu_config_module->mock('assert_config_exists_on_node', \&assert_config_exists_on_node); # ignore existing replication config -my $repl_config_module = new Test::MockModule('PVE::ReplicationConfig'); +my $repl_config_module = Test::MockModule->new('PVE::ReplicationConfig'); $repl_config_module->mock('new' => sub { return bless {}, "PVE::ReplicationConfig" }); $repl_config_module->mock('check_for_existing_jobs' => sub { return }); -my $storage_module = new Test::MockModule('PVE::Storage'); +my $storage_module = Test::MockModule->new('PVE::Storage'); $storage_module->mock('config', sub { return; }); $storage_module->mock('path', sub { return "/some/store/statefile/path"; }); $storage_module->mock('activate_volumes', \&mocked_activate_volumes); @@ -506,7 +506,7 @@ $vol_snapshot_rollback_possible->{"local:snapshotable-disk-4"} = 1; #printf("\n"); #printf("Setting up Mocking for PVE::Tools\n"); -#my $tools_module = new Test::MockModule('PVE::Tools'); +#my $tools_module = Test::MockModule->new('PVE::Tools'); #$tools_module->mock('run_command' => \&mocked_run_command); #printf("\trun_command() mocked\n"); #