On October 28, 2019 12:59 pm, Stefan Reiter wrote:
> Also merge the 'mkdir's from QemuServer and QemuConfig to reduce
> duplication (both modules depend on QemuSchema anyway).
>
> nodename() is still called in multiple modules, but since it's cached by
> the INotify module it doesn't really matter.
>
> Signed-off-by: Stefan Reiter <s.rei...@proxmox.com>
> ---
>
> QemuSchema is pretty small right now, but it could hold much more of the
> static
> setup code from QemuServer.pm (JSONSchema formats and the like). This patch
> only
> moves the necessary stuff for the rest of the series to not need cyclic
> depends.
>
> I want to refactor more into this in the future, but for now I'd like to wait
> for my CPU series, since that also touches some schema stuff.
I get where you are coming from here (and splitting the schema
definition from all the stuff implemented in QemuServer.pm would be a
huge win!), but IMHO the three methods you move here are not really
schema-related - they are just VMID to local path mappings? is there
some other broader category that we could put them into (except some
awful generic 'helper' module ;)) ?
>
> PVE/CLI/qm.pm | 3 ++-
> PVE/Makefile | 3 ++-
> PVE/QMPClient.pm | 5 +++--
> PVE/QemuConfig.pm | 10 ++--------
> PVE/QemuSchema.pm | 35 +++++++++++++++++++++++++++++++++++
> PVE/QemuServer.pm | 41 ++++++++---------------------------------
> 6 files changed, 52 insertions(+), 45 deletions(-)
> create mode 100644 PVE/QemuSchema.pm
>
> diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
> index ea74ad5..44beac9 100755
> --- a/PVE/CLI/qm.pm
> +++ b/PVE/CLI/qm.pm
> @@ -21,6 +21,7 @@ use PVE::RPCEnvironment;
> use PVE::Exception qw(raise_param_exc);
> use PVE::Network;
> use PVE::GuestHelpers;
> +use PVE::QemuSchema;
> use PVE::QemuServer;
> 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::QemuSchema::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/Makefile b/PVE/Makefile
> index dc17368..5ec715e 100644
> --- a/PVE/Makefile
> +++ b/PVE/Makefile
> @@ -2,7 +2,8 @@ PERLSOURCE = \
> QemuServer.pm \
> QemuMigrate.pm \
> QMPClient.pm \
> - QemuConfig.pm
> + QemuConfig.pm \
> + QemuSchema.pm \
>
> .PHONY: install
> install:
> diff --git a/PVE/QMPClient.pm b/PVE/QMPClient.pm
> index 570dba2..188c6d7 100644
> --- a/PVE/QMPClient.pm
> +++ b/PVE/QMPClient.pm
> @@ -2,6 +2,7 @@ package PVE::QMPClient;
>
> use strict;
> use warnings;
> +use PVE::QemuSchema;
> use PVE::QemuServer;
> use IO::Multiplex;
> use POSIX qw(EINTR EAGAIN);
> @@ -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::QemuSchema::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::QemuSchema::qmp_socket($vmid, $qga);
>
> $timeout = 1 if !$timeout;
>
> diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
> index e9796a3..b63e57c 100644
> --- a/PVE/QemuConfig.pm
> +++ b/PVE/QemuConfig.pm
> @@ -5,6 +5,7 @@ use warnings;
>
> use PVE::AbstractConfig;
> use PVE::INotify;
> +use PVE::QemuSchema;
> 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;
> -
> 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::QemuSchema::lock_dir/lock-$vmid.conf";
> }
>
> sub cfs_config_path {
> diff --git a/PVE/QemuSchema.pm b/PVE/QemuSchema.pm
> new file mode 100644
> index 0000000..446177d
> --- /dev/null
> +++ b/PVE/QemuSchema.pm
> @@ -0,0 +1,35 @@
> +package PVE::QemuSchema;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::INotify;
> +
> +my $nodename = PVE::INotify::nodename();
> +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.pm b/PVE/QemuServer.pm
> index 9af690a..817394e 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::QemuSchema;
> 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;
> -
> 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::QemuSchema::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::QemuSchema::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::QemuSchema::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::QemuSchema::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::QemuSchema::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::QemuSchema::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::QemuSchema::qmp_socket($vmid);
> if (-e $sname) { # test if VM is reasonambe new and supports qmp/qga
> my $qmpclient = PVE::QMPClient->new();
>
> --
> 2.20.1
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
