[pve-devel] [PATCH v3 qemu-server 1/8] refactor: create QemuServer::Helpers and move file/dir code
Fabian Grünbichler
f.gruenbichler at proxmox.com
Tue Nov 19 10:29:05 CET 2019
On November 4, 2019 2:57 pm, Stefan Reiter wrote:
> Also merge the 'mkdir's from QemuServer and QemuConfig to reduce
> duplication (both modules depend on Helpers anyway).
>
> nodename() is still called in multiple places, but since it's cached by
> INotify it doesn't really matter.
>
> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
> ---
> PVE/CLI/qm.pm | 3 ++-
> PVE/QMPClient.pm | 5 +++--
> PVE/QemuConfig.pm | 10 ++--------
> PVE/QemuServer.pm | 41 ++++++++-------------------------------
> PVE/QemuServer/Helpers.pm | 37 +++++++++++++++++++++++++++++++++++
> PVE/QemuServer/Makefile | 1 +
> 6 files changed, 53 insertions(+), 44 deletions(-)
> create mode 100644 PVE/QemuServer/Helpers.pm
>
> diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
> index 59a3861..1a841b7 100755
> --- a/PVE/CLI/qm.pm
> +++ b/PVE/CLI/qm.pm
> @@ -27,6 +27,7 @@ use PVE::Tools qw(extract_param);
>
> use PVE::API2::Qemu::Agent;
> use PVE::API2::Qemu;
> +use PVE::QemuServer::Helpers;
> use PVE::QemuServer::Agent qw(agent_available);
> use PVE::QemuServer::ImportDisk;
> use PVE::QemuServer::OVF;
> @@ -209,7 +210,7 @@ __PACKAGE__->register_method ({
> my ($param) = @_;
>
> my $vmid = $param->{vmid};
> - my $vnc_socket = PVE::QemuServer::vnc_socket($vmid);
> + my $vnc_socket = PVE::QemuServer::Helpers::vnc_socket($vmid);
>
> if (my $ticket = $ENV{LC_PVE_TICKET}) { # NOTE: ssh on debian only pass LC_* variables
> PVE::QemuServer::vm_mon_cmd($vmid, "change", device => 'vnc', target => "unix:$vnc_socket,password");
> diff --git a/PVE/QMPClient.pm b/PVE/QMPClient.pm
> index 96b6a24..f665f52 100644
> --- a/PVE/QMPClient.pm
> +++ b/PVE/QMPClient.pm
> @@ -11,6 +11,7 @@ use Time::HiRes qw(usleep gettimeofday tv_interval);
>
> use PVE::IPCC;
> use PVE::QemuServer;
no longer using PVE::QemuServer! one less circular use ;)
> +use PVE::QemuServer::Helpers;
>
> # Qemu Monitor Protocol (QMP) client.
> #
> @@ -58,7 +59,7 @@ my $push_cmd_to_queue = sub {
>
> my $qga = ($execute =~ /^guest\-+/) ? 1 : 0;
>
> - my $sname = PVE::QemuServer::qmp_socket($vmid, $qga);
> + my $sname = PVE::QemuServer::Helpers::qmp_socket($vmid, $qga);
>
> $self->{queue_info}->{$sname} = { qga => $qga, vmid => $vmid, sname => $sname, cmds => [] }
> if !$self->{queue_info}->{$sname};
> @@ -186,7 +187,7 @@ my $open_connection = sub {
> my $vmid = $queue_info->{vmid};
> my $qga = $queue_info->{qga};
>
> - my $sname = PVE::QemuServer::qmp_socket($vmid, $qga);
> + my $sname = PVE::QemuServer::Helpers::qmp_socket($vmid, $qga);
>
> $timeout = 1 if !$timeout;
>
> diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
> index e9796a3..499d5e3 100644
> --- a/PVE/QemuConfig.pm
> +++ b/PVE/QemuConfig.pm
> @@ -5,6 +5,7 @@ use warnings;
>
> use PVE::AbstractConfig;
> use PVE::INotify;
> +use PVE::QemuServer::Helpers;
> use PVE::QemuServer;
> use PVE::Storage;
> use PVE::Tools;
> @@ -13,13 +14,6 @@ use base qw(PVE::AbstractConfig);
>
> my $nodename = PVE::INotify::nodename();
>
> -mkdir "/etc/pve/nodes/$nodename";
> -my $confdir = "/etc/pve/nodes/$nodename/qemu-server";
> -mkdir $confdir;
> -
> -my $lock_dir = "/var/lock/qemu-server";
> -mkdir $lock_dir;
> -
I am not sure whether these could not just stay here? if you lock the
config, you do it via QemuConfig->lock_config(), same for
reading/writing/.. even the manual "check for existence" should use
cfs_config_path ;)
> my $MAX_UNUSED_DISKS = 256;
>
> # BEGIN implemented abstract methods from PVE::AbstractConfig
> @@ -37,7 +31,7 @@ sub __config_max_unused_disks {
> sub config_file_lock {
> my ($class, $vmid) = @_;
>
> - return "$lock_dir/lock-$vmid.conf";
> + return "$PVE::QemuServer::Helpers::lock_dir/lock-$vmid.conf";
> }
>
> sub cfs_config_path {
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index bfe6662..24bfd16 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -40,6 +40,7 @@ use PVE::Tools qw(run_command lock_file lock_file_full file_read_firstline dir_g
>
> use PVE::QMPClient;
> use PVE::QemuConfig;
> +use PVE::QemuServer::Helpers;
> use PVE::QemuServer::Cloudinit;
> use PVE::QemuServer::Memory;
> use PVE::QemuServer::PCI qw(print_pci_addr print_pcie_addr print_pcie_root_port);
> @@ -108,16 +109,6 @@ sub cgroups_write {
>
> my $nodename = PVE::INotify::nodename();
>
> -mkdir "/etc/pve/nodes/$nodename";
> -my $confdir = "/etc/pve/nodes/$nodename/qemu-server";
> -mkdir $confdir;
> -
> -my $var_run_tmpdir = "/var/run/qemu-server";
> -mkdir $var_run_tmpdir;
> -
> -my $lock_dir = "/var/lock/qemu-server";
> -mkdir $lock_dir;
> -
removing these makes sense for sure ;)
> my $cpu_vendor_list = {
> # Intel CPUs
> 486 => 'GenuineIntel',
> @@ -2995,7 +2986,7 @@ sub check_running {
> die "unable to find configuration file for VM $vmid - no such machine\n"
> if !$nocheck && ! -f $filename;
>
> - my $pidfile = pidfile_name($vmid);
> + my $pidfile = PVE::QemuServer::Helpers::pidfile_name($vmid);
>
> if (my $fd = IO::File->new("<$pidfile")) {
> my $st = stat($fd);
> @@ -3024,7 +3015,7 @@ sub vzlist {
>
> my $vzlist = config_list();
>
> - my $fd = IO::Dir->new($var_run_tmpdir) || return $vzlist;
> + my $fd = IO::Dir->new($PVE::QemuServer::Helpers::var_run_tmpdir) || return $vzlist;
>
> while (defined(my $de = $fd->read)) {
> next if $de !~ m/^(\d+)\.pid$/;
> @@ -3564,7 +3555,7 @@ sub config_to_command {
>
> my $use_virtio = 0;
>
> - my $qmpsocket = qmp_socket($vmid);
> + my $qmpsocket = PVE::QemuServer::Helpers::qmp_socket($vmid);
> push @$cmd, '-chardev', "socket,id=qmp,path=$qmpsocket,server,nowait";
> push @$cmd, '-mon', "chardev=qmp,mode=control";
>
> @@ -3573,7 +3564,7 @@ sub config_to_command {
> push @$cmd, '-mon', "chardev=qmp-event,mode=control";
> }
>
> - push @$cmd, '-pidfile' , pidfile_name($vmid);
> + push @$cmd, '-pidfile' , PVE::QemuServer::Helpers::pidfile_name($vmid);
>
> push @$cmd, '-daemonize';
>
> @@ -3854,7 +3845,7 @@ sub config_to_command {
>
> if ($vga->{type} && $vga->{type} !~ m/^serial\d+$/ && $vga->{type} ne 'none'){
> push @$devices, '-device', print_vga_device($conf, $vga, $arch, $machine_type, undef, $qxlnum, $bridges);
> - my $socket = vnc_socket($vmid);
> + my $socket = PVE::QemuServer::Helpers::vnc_socket($vmid);
> push @$cmd, '-vnc', "unix:$socket,password";
> } else {
> push @$cmd, '-vga', 'none' if $vga->{type} eq 'none';
> @@ -3905,7 +3896,7 @@ sub config_to_command {
> push @$cmd, '-k', $conf->{keyboard} if defined($conf->{keyboard});
>
> if (parse_guest_agent($conf)->{enabled}) {
> - my $qgasocket = qmp_socket($vmid, 1);
> + my $qgasocket = PVE::QemuServer::Helpers::qmp_socket($vmid, 1);
> my $pciaddr = print_pci_addr("qga0", $bridges, $arch, $machine_type);
> push @$devices, '-chardev', "socket,path=$qgasocket,server,nowait,id=qga0";
> push @$devices, '-device', "virtio-serial,id=qga0$pciaddr";
> @@ -4119,11 +4110,6 @@ sub config_to_command {
> return wantarray ? ($cmd, $vollist, $spice_port) : $cmd;
> }
>
> -sub vnc_socket {
> - my ($vmid) = @_;
> - return "${var_run_tmpdir}/$vmid.vnc";
> -}
> -
> sub spice_port {
> my ($vmid) = @_;
>
> @@ -4132,17 +4118,6 @@ sub spice_port {
> return $res->{'tls-port'} || $res->{'port'} || die "no spice port\n";
> }
>
> -sub qmp_socket {
> - my ($vmid, $qga) = @_;
> - my $sockettype = $qga ? 'qga' : 'qmp';
> - return "${var_run_tmpdir}/$vmid.$sockettype";
> -}
> -
> -sub pidfile_name {
> - my ($vmid) = @_;
> - return "${var_run_tmpdir}/$vmid.pid";
> -}
> -
> sub vm_devices_list {
> my ($vmid) = @_;
>
> @@ -5581,7 +5556,7 @@ sub vm_qmp_command {
>
> eval {
> die "VM $vmid not running\n" if !check_running($vmid, $nocheck);
> - my $sname = qmp_socket($vmid);
> + my $sname = PVE::QemuServer::Helpers::qmp_socket($vmid);
> if (-e $sname) { # test if VM is reasonambe new and supports qmp/qga
> my $qmpclient = PVE::QMPClient->new();
>
> diff --git a/PVE/QemuServer/Helpers.pm b/PVE/QemuServer/Helpers.pm
> new file mode 100644
> index 0000000..91e2c83
> --- /dev/null
> +++ b/PVE/QemuServer/Helpers.pm
> @@ -0,0 +1,37 @@
> +package PVE::QemuServer::Helpers;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::INotify;
> +
> +my $nodename = PVE::INotify::nodename();
> +
> +# Paths and directories
> +mkdir "/etc/pve/nodes/$nodename";
> +my $confdir = "/etc/pve/nodes/$nodename/qemu-server";
> +mkdir $confdir;
> +
> +our $var_run_tmpdir = "/var/run/qemu-server";
> +mkdir $var_run_tmpdir;
> +
> +our $lock_dir = "/var/lock/qemu-server";
> +mkdir $lock_dir;
> +
> +sub qmp_socket {
> + my ($vmid, $qga) = @_;
> + my $sockettype = $qga ? 'qga' : 'qmp';
> + return "${var_run_tmpdir}/$vmid.$sockettype";
> +}
> +
> +sub pidfile_name {
> + my ($vmid) = @_;
> + return "${var_run_tmpdir}/$vmid.pid";
> +}
> +
> +sub vnc_socket {
> + my ($vmid) = @_;
> + return "${var_run_tmpdir}/$vmid.vnc";
> +}
> +
> +1;
> diff --git a/PVE/QemuServer/Makefile b/PVE/QemuServer/Makefile
> index afc39a3..56d1493 100644
> --- a/PVE/QemuServer/Makefile
> +++ b/PVE/QemuServer/Makefile
> @@ -5,6 +5,7 @@ SOURCES=PCI.pm \
> OVF.pm \
> Cloudinit.pm \
> Agent.pm \
> + Helpers.pm \
>
> .PHONY: install
> install: ${SOURCES}
> --
> 2.20.1
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
More information about the pve-devel
mailing list