[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