On October 28, 2019 12:59 pm, Stefan Reiter wrote:
> ...in addition to PVE::QMPClient for low-level.

I think I'd rather name this PVE::Qemu::Monitor , since it does both HMP 
and QMP ;) it probably makes sense to rename some of the methods, since 
we need to change all the call sites anyway

PVE::QemuServer::vm_mon_cmd
PVE::Qemu::Monitor::mon_cmd

PVE::QemuServer::vm_mon_cmd_nocheck
PVE::Qemu::Monitor::mon_cmd_nocheck

PVE::QemuServer::vm_qmp_commandd
PVE::Qemu::Monitor::qmp_cmd

PVE::QemuServer::vm_human_monitor_command
PVE::Qemu::Monitor::hmp_cmd

all of which are equal or shorter than their old variants with 
PVE::QemuServer:: prefix ;) for the first two (which are the most used) 
I'd keep the export - the latter two are not used that much?

> 
> Also move all references (most with exports, the methods are used a lot
> and have unique enough names IMO) and fix tests.
> 
> References in __snapshot_create_vol_snapshots_hook (in QemuConfig) is an
> exception, as using the exported functions breaks tests.
> 
> Signed-off-by: Stefan Reiter <s.rei...@proxmox.com>
> ---
>  PVE/API2/Qemu.pm         | 13 ++++----
>  PVE/API2/Qemu/Agent.pm   |  7 ++--
>  PVE/CLI/qm.pm            | 11 +++---
>  PVE/Makefile             |  1 +
>  PVE/QMP.pm               | 72 ++++++++++++++++++++++++++++++++++++++++
>  PVE/QemuConfig.pm        | 15 +++++----
>  PVE/QemuMigrate.pm       | 21 ++++++------
>  PVE/QemuServer.pm        | 66 ++++--------------------------------
>  PVE/QemuServer/Agent.pm  |  3 +-
>  PVE/QemuServer/Memory.pm |  9 ++---
>  PVE/VZDump/QemuServer.pm | 13 ++++----
>  test/snapshot-test.pm    | 18 +++++++---
>  12 files changed, 142 insertions(+), 107 deletions(-)
>  create mode 100644 PVE/QMP.pm
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 9912e4d..50a0592 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -21,6 +21,7 @@ use PVE::GuestHelpers;
>  use PVE::QemuConfig;
>  use PVE::QemuServer;
>  use PVE::QemuMigrate;
> +use PVE::QMP qw(vm_mon_cmd vm_qmp_command);
>  use PVE::RPCEnvironment;
>  use PVE::AccessControl;
>  use PVE::INotify;
> @@ -1835,8 +1836,8 @@ __PACKAGE__->register_method({
>       my ($ticket, undef, $remote_viewer_config) =
>           PVE::AccessControl::remote_viewer_config($authuser, $vmid, $node, 
> $proxy, $title, $port);
>  
> -     PVE::QemuServer::vm_mon_cmd($vmid, "set_password", protocol => 'spice', 
> password => $ticket);
> -     PVE::QemuServer::vm_mon_cmd($vmid, "expire_password", protocol => 
> 'spice', time => "+30");
> +     vm_mon_cmd($vmid, "set_password", protocol => 'spice', password => 
> $ticket);
> +     vm_mon_cmd($vmid, "expire_password", protocol => 'spice', time => 
> "+30");
>  
>       return $remote_viewer_config;
>      }});
> @@ -2261,7 +2262,7 @@ __PACKAGE__->register_method({
>       # checking the qmp status here to get feedback to the gui/cli/api
>       # and the status query should not take too long
>       my $qmpstatus = eval {
> -         PVE::QemuServer::vm_qmp_command($vmid, { execute => "query-status" 
> }, 0);
> +         vm_qmp_command($vmid, { execute => "query-status" }, 0);
>       };
>       my $err = $@ if $@;
>  
> @@ -2341,7 +2342,7 @@ __PACKAGE__->register_method({
>       my $vmid = extract_param($param, 'vmid');
>  
>       my $qmpstatus = eval {
> -         PVE::QemuServer::vm_qmp_command($vmid, { execute => "query-status" 
> }, 0);
> +         vm_qmp_command($vmid, { execute => "query-status" }, 0);
>       };
>       my $err = $@ if $@;
>  
> @@ -3093,7 +3094,7 @@ __PACKAGE__->register_method({
>                   PVE::QemuConfig->write_config($vmid, $conf);
>  
>                   if ($running && 
> PVE::QemuServer::parse_guest_agent($conf)->{fstrim_cloned_disks} && 
> PVE::QemuServer::qga_check_running($vmid)) {
> -                     eval { PVE::QemuServer::vm_mon_cmd($vmid, 
> "guest-fstrim"); };
> +                     eval { vm_mon_cmd($vmid, "guest-fstrim"); };
>                   }
>  
>                   eval {
> @@ -3449,7 +3450,7 @@ __PACKAGE__->register_method({
>  
>       my $res = '';
>       eval {
> -         $res = PVE::QemuServer::vm_human_monitor_command($vmid, 
> $param->{command});
> +         $res = PVE::QMP::vm_human_monitor_command($vmid, $param->{command});
>       };
>       $res = "ERROR: $@" if $@;
>  
> diff --git a/PVE/API2/Qemu/Agent.pm b/PVE/API2/Qemu/Agent.pm
> index 839146c..da7111e 100644
> --- a/PVE/API2/Qemu/Agent.pm
> +++ b/PVE/API2/Qemu/Agent.pm
> @@ -7,6 +7,7 @@ use PVE::RESTHandler;
>  use PVE::JSONSchema qw(get_standard_option);
>  use PVE::QemuServer;
>  use PVE::QemuServer::Agent qw(agent_available agent_cmd check_agent_error);
> +use PVE::QMP qw(vm_mon_cmd);
>  use MIME::Base64 qw(encode_base64 decode_base64);
>  use JSON;
>  
> @@ -190,7 +191,7 @@ sub register_command {
>           agent_available($vmid, $conf);
>  
>           my $cmd = $param->{command} // $command;
> -         my $res = PVE::QemuServer::vm_mon_cmd($vmid, "guest-$cmd");
> +         my $res = vm_mon_cmd($vmid, "guest-$cmd");
>  
>           return { result => $res };
>       }});
> @@ -415,7 +416,7 @@ __PACKAGE__->register_method({
>       my $content = "";
>  
>       while ($bytes_left > 0 && !$eof) {
> -         my $read = PVE::QemuServer::vm_mon_cmd($vmid, "guest-file-read", 
> handle => $qgafh, count => int($read_size));
> +         my $read = vm_mon_cmd($vmid, "guest-file-read", handle => $qgafh, 
> count => int($read_size));
>           check_agent_error($read, "can't read from file");
>  
>           $content .= decode_base64($read->{'buf-b64'});
> @@ -423,7 +424,7 @@ __PACKAGE__->register_method({
>           $eof = $read->{eof} // 0;
>       }
>  
> -     my $res = PVE::QemuServer::vm_mon_cmd($vmid, "guest-file-close", handle 
> => $qgafh);
> +     my $res = vm_mon_cmd($vmid, "guest-file-close", handle => $qgafh);
>       check_agent_error($res, "can't close file", 1);
>  
>       my $result = {
> diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
> index 4de0ae2..485334a 100755
> --- a/PVE/CLI/qm.pm
> +++ b/PVE/CLI/qm.pm
> @@ -27,6 +27,7 @@ use PVE::QemuServer;
>  use PVE::QemuServer::ImportDisk;
>  use PVE::QemuServer::OVF;
>  use PVE::QemuServer::Agent qw(agent_available);
> +use PVE::QMP qw(vm_mon_cmd);
>  use PVE::API2::Qemu;
>  use PVE::API2::Qemu::Agent;
>  use JSON;
> @@ -214,12 +215,12 @@ __PACKAGE__->register_method ({
>       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");
> -         PVE::QemuServer::vm_mon_cmd($vmid, "set_password", protocol => 
> 'vnc', password => $ticket);
> -         PVE::QemuServer::vm_mon_cmd($vmid, "expire_password", protocol => 
> 'vnc', time => "+30");
> +         vm_mon_cmd($vmid, "change", device => 'vnc', target => 
> "unix:$vnc_socket,password");
> +         vm_mon_cmd($vmid, "set_password", protocol => 'vnc', password => 
> $ticket);
> +         vm_mon_cmd($vmid, "expire_password", protocol => 'vnc', time => 
> "+30");
>       } else {
>           # FIXME: remove or allow to add tls-creds object, as x509 vnc param 
> is removed with qemu 4??
> -         PVE::QemuServer::vm_mon_cmd($vmid, "change", device => 'vnc', 
> target => "unix:$vnc_socket,password");
> +         vm_mon_cmd($vmid, "change", device => 'vnc', target => 
> "unix:$vnc_socket,password");
>       }
>  
>       run_vnc_proxy($vnc_socket);
> @@ -399,7 +400,7 @@ __PACKAGE__->register_method ({
>           last if $input =~ m/^\s*q(uit)?\s*$/;
>  
>           eval {
> -             print PVE::QemuServer::vm_human_monitor_command ($vmid, $input);
> +             print PVE::QMP::vm_human_monitor_command ($vmid, $input);
>           };
>           print "ERROR: $@" if $@;
>       }
> diff --git a/PVE/Makefile b/PVE/Makefile
> index 5ec715e..2ed4580 100644
> --- a/PVE/Makefile
> +++ b/PVE/Makefile
> @@ -4,6 +4,7 @@ PERLSOURCE =                  \
>       QMPClient.pm            \
>       QemuConfig.pm           \
>       QemuSchema.pm           \
> +     QMP.pm                  \
>  
>  .PHONY: install
>  install:
> diff --git a/PVE/QMP.pm b/PVE/QMP.pm
> new file mode 100644
> index 0000000..80465cb
> --- /dev/null
> +++ b/PVE/QMP.pm
> @@ -0,0 +1,72 @@
> +package PVE::QMP;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::SafeSyslog;
> +use PVE::QemuConfig;
> +use PVE::QemuSchema;
> +use PVE::QMPClient;
> +
> +use base 'Exporter';
> +our @EXPORT_OK = qw(
> +vm_qmp_command
> +vm_mon_cmd
> +vm_mon_cmd_nocheck
> +);
> +
> +sub vm_qmp_command {
> +    my ($vmid, $cmd, $nocheck) = @_;
> +
> +    my $res;
> +
> +    my $timeout;
> +    if ($cmd->{arguments}) {
> +     $timeout = delete $cmd->{arguments}->{timeout};
> +    }
> +
> +    eval {
> +     die "VM $vmid not running\n" if !PVE::QemuConfig::check_running($vmid, 
> $nocheck);

circular dependency QemuConfig <-> this file

maybe we need to split up check_running into two components?
1) check config exists on local node ( -> call to QemuConfig)
2) check if qemu process with expected pid file is running ( -> call to 
file from patch #4)

most call sites (transitively) calling vm_qmp_command already checked 1) 
before anyway, the remaining ones could probably do that check 
themselves. not sure about other calls to check_running? we could also 
leave a legacy "check both" in QemuServer.pm, and just split the two 
components and update call-sites piece by piece where appropriate.

> +     my $sname = PVE::QemuSchema::qmp_socket($vmid);
> +     if (-e $sname) { # test if VM is reasonably new and supports qmp/qga
> +         my $qmpclient = PVE::QMPClient->new();
> +
> +         $res = $qmpclient->cmd($vmid, $cmd, $timeout);
> +     } else {
> +         die "unable to open monitor socket\n";
> +     }
> +    };
> +    if (my $err = $@) {
> +     syslog("err", "VM $vmid qmp command failed - $err");
> +     die $err;
> +    }
> +
> +    return $res;
> +}
> +
> +sub vm_mon_cmd {
> +    my ($vmid, $execute, %params) = @_;
> +
> +    my $cmd = { execute => $execute, arguments => \%params };
> +    vm_qmp_command($vmid, $cmd);
> +}
> +
> +sub vm_mon_cmd_nocheck {
> +    my ($vmid, $execute, %params) = @_;
> +
> +    my $cmd = { execute => $execute, arguments => \%params };
> +    vm_qmp_command($vmid, $cmd, 1);
> +}
> +
> +sub vm_human_monitor_command {
> +    my ($vmid, $cmdline) = @_;
> +
> +    my $cmd = {
> +     execute => 'human-monitor-command',
> +     arguments => { 'command-line' => $cmdline},
> +    };
> +
> +    return vm_qmp_command($vmid, $cmd);
> +}
> +
> +1;
> diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
> index 08cac38..06ace83 100644
> --- a/PVE/QemuConfig.pm
> +++ b/PVE/QemuConfig.pm
> @@ -11,6 +11,7 @@ use PVE::INotify;
>  use PVE::ProcFSTools;
>  use PVE::QemuSchema;
>  use PVE::QemuServer;
> +use PVE::QMP qw(vm_mon_cmd vm_mon_cmd_nocheck);
>  use PVE::Storage;
>  use PVE::Tools;
>  
> @@ -183,10 +184,10 @@ sub __snapshot_freeze {
>      my ($class, $vmid, $unfreeze) = @_;
>  
>      if ($unfreeze) {
> -     eval { PVE::QemuServer::vm_mon_cmd($vmid, "guest-fsfreeze-thaw"); };
> +     eval { vm_mon_cmd($vmid, "guest-fsfreeze-thaw"); };
>       warn "guest-fsfreeze-thaw problems - $@" if $@;
>      } else {
> -     eval { PVE::QemuServer::vm_mon_cmd($vmid, "guest-fsfreeze-freeze"); };
> +     eval { vm_mon_cmd($vmid, "guest-fsfreeze-freeze"); };
>       warn "guest-fsfreeze-freeze problems - $@" if $@;
>      }
>  }
> @@ -202,9 +203,9 @@ sub __snapshot_create_vol_snapshots_hook {
>               my $path = PVE::Storage::path($storecfg, $snap->{vmstate});
>               PVE::Storage::activate_volumes($storecfg, [$snap->{vmstate}]);
>  
> -             PVE::QemuServer::vm_mon_cmd($vmid, "savevm-start", statefile => 
> $path);
> +             PVE::QMP::vm_mon_cmd($vmid, "savevm-start", statefile => $path);
>               for(;;) {
> -                 my $stat = PVE::QemuServer::vm_mon_cmd_nocheck($vmid, 
> "query-savevm");
> +                 my $stat = PVE::QMP::vm_mon_cmd_nocheck($vmid, 
> "query-savevm");
>                   if (!$stat->{status}) {
>                       die "savevm not active\n";
>                   } elsif ($stat->{status} eq 'active') {
> @@ -217,18 +218,18 @@ sub __snapshot_create_vol_snapshots_hook {
>                   }
>               }
>           } else {
> -             PVE::QemuServer::vm_mon_cmd($vmid, "savevm-start");
> +             PVE::QMP::vm_mon_cmd($vmid, "savevm-start");
>           }
>       } elsif ($hook eq "after") {
>           eval {
> -             PVE::QemuServer::vm_mon_cmd($vmid, "savevm-end");
> +             PVE::QMP::vm_mon_cmd($vmid, "savevm-end");
>               PVE::Storage::deactivate_volumes($storecfg, [$snap->{vmstate}]) 
> if $snap->{vmstate};
>           };
>           warn $@ if $@;
>       } elsif ($hook eq "after-freeze") {
>           # savevm-end is async, we need to wait
>           for (;;) {
> -             my $stat = PVE::QemuServer::vm_mon_cmd_nocheck($vmid, 
> "query-savevm");
> +             my $stat = PVE::QMP::vm_mon_cmd_nocheck($vmid, "query-savevm");
>               if (!$stat->{bytes}) {
>                   last;
>               } else {
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index e6993d4..aea7eac 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -12,6 +12,7 @@ use PVE::Cluster;
>  use PVE::Storage;
>  use PVE::QemuConfig;
>  use PVE::QemuServer;
> +use PVE::QMP qw(vm_mon_cmd vm_mon_cmd_nocheck);
>  use Time::HiRes qw( usleep );
>  use PVE::RPCEnvironment;
>  use PVE::ReplicationConfig;
> @@ -552,7 +553,7 @@ sub phase2 {
>  
>      my $spice_ticket;
>      if (PVE::QemuServer::vga_conf_has_spice($conf->{vga})) {
> -     my $res = PVE::QemuServer::vm_mon_cmd($vmid, 'query-spice');
> +     my $res = vm_mon_cmd($vmid, 'query-spice');
>       $spice_ticket = $res->{ticket};
>      }
>  
> @@ -707,7 +708,7 @@ sub phase2 {
>      $migrate_speed *= 1024;
>      $self->log('info', "migrate_set_speed: $migrate_speed");
>      eval {
> -        PVE::QemuServer::vm_mon_cmd_nocheck($vmid, "migrate_set_speed", 
> value => int($migrate_speed));
> +     vm_mon_cmd_nocheck($vmid, "migrate_set_speed", value => 
> int($migrate_speed));
>      };
>      $self->log('info', "migrate_set_speed error: $@") if $@;
>  
> @@ -716,7 +717,7 @@ sub phase2 {
>      if (defined($migrate_downtime)) {
>       $self->log('info', "migrate_set_downtime: $migrate_downtime");
>       eval {
> -         PVE::QemuServer::vm_mon_cmd_nocheck($vmid, "migrate_set_downtime", 
> value => int($migrate_downtime*100)/100);
> +         vm_mon_cmd_nocheck($vmid, "migrate_set_downtime", value => 
> int($migrate_downtime*100)/100);
>       };
>       $self->log('info', "migrate_set_downtime error: $@") if $@;
>      }
> @@ -734,7 +735,7 @@ sub phase2 {
>  
>      $self->log('info', "set cachesize: $cachesize");
>      eval {
> -     PVE::QemuServer::vm_mon_cmd_nocheck($vmid, "migrate-set-cache-size", 
> value => int($cachesize));
> +     vm_mon_cmd_nocheck($vmid, "migrate-set-cache-size", value => 
> int($cachesize));
>      };
>      $self->log('info', "migrate-set-cache-size error: $@") if $@;
>  
> @@ -750,7 +751,7 @@ sub phase2 {
>       $self->log('info', "spice client_migrate_info");
>  
>       eval {
> -         PVE::QemuServer::vm_mon_cmd_nocheck($vmid, "client_migrate_info", 
> protocol => 'spice',
> +         vm_mon_cmd_nocheck($vmid, "client_migrate_info", protocol => 
> 'spice',
>                                               hostname => $proxyticket, 
> 'port' => 0, 'tls-port' => $spice_port,
>                                               'cert-subject' => $subject);
>       };
> @@ -760,7 +761,7 @@ sub phase2 {
>  
>      $self->log('info', "start migrate command to $ruri");
>      eval {
> -        PVE::QemuServer::vm_mon_cmd_nocheck($vmid, "migrate", uri => $ruri);
> +     vm_mon_cmd_nocheck($vmid, "migrate", uri => $ruri);
>      };
>      my $merr = $@;
>      $self->log('info', "migrate uri => $ruri failed: $merr") if $merr;
> @@ -778,7 +779,7 @@ sub phase2 {
>       usleep($usleep);
>       my $stat;
>       eval {
> -         $stat = PVE::QemuServer::vm_mon_cmd_nocheck($vmid, "query-migrate");
> +         $stat = vm_mon_cmd_nocheck($vmid, "query-migrate");
>       };
>       if (my $err = $@) {
>           $err_count++;
> @@ -847,7 +848,7 @@ sub phase2 {
>                   $migrate_downtime *= 2;
>                   $self->log('info', "migrate_set_downtime: 
> $migrate_downtime");
>                   eval {
> -                     PVE::QemuServer::vm_mon_cmd_nocheck($vmid, 
> "migrate_set_downtime", value => int($migrate_downtime*100)/100);
> +                     vm_mon_cmd_nocheck($vmid, "migrate_set_downtime", value 
> => int($migrate_downtime*100)/100);
>                   };
>                   $self->log('info', "migrate_set_downtime error: $@") if $@;
>               }
> @@ -874,7 +875,7 @@ sub phase2_cleanup {
>  
>      $self->log('info', "migrate_cancel");
>      eval {
> -     PVE::QemuServer::vm_mon_cmd_nocheck($vmid, "migrate_cancel");
> +     vm_mon_cmd_nocheck($vmid, "migrate_cancel");
>      };
>      $self->log('info', "migrate_cancel error: $@") if $@;
>  
> @@ -1025,7 +1026,7 @@ sub phase3_cleanup {
>       if (PVE::QemuServer::vga_conf_has_spice($conf->{vga}) && 
> $self->{running}) {
>           $self->log('info', "Waiting for spice server migration");
>           while (1) {
> -             my $res = PVE::QemuServer::vm_mon_cmd_nocheck($vmid, 
> 'query-spice');
> +             my $res = vm_mon_cmd_nocheck($vmid, 'query-spice');
>               last if int($res->{'migrated'}) == 1;
>               last if $timer > 50;
>               $timer ++;
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index d5eba39..ed137fc 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -32,12 +32,12 @@ use PVE::INotify;
>  use PVE::JSONSchema qw(get_standard_option);
>  use PVE::ProcFSTools;
>  use PVE::RPCEnvironment;
> -use PVE::SafeSyslog;
>  use PVE::Storage;
>  use PVE::SysFSTools;
>  use PVE::Systemd;
>  use PVE::Tools qw(run_command lock_file lock_file_full file_read_firstline 
> dir_glob_foreach get_host_arch $IPV6RE);
>  
> +use PVE::QMP qw(vm_qmp_command vm_mon_cmd vm_mon_cmd_nocheck);
>  use PVE::QMPClient;
>  use PVE::QemuConfig;
>  use PVE::QemuSchema;
> @@ -4333,7 +4333,7 @@ sub qemu_driveadd {
>  
>      my $drive = print_drive_full($storecfg, $vmid, $device);
>      $drive =~ s/\\/\\\\/g;
> -    my $ret = vm_human_monitor_command($vmid, "drive_add auto \"$drive\"");
> +    my $ret = PVE::QMP::vm_human_monitor_command($vmid, "drive_add auto 
> \"$drive\"");
>  
>      # If the command succeeds qemu prints: "OK"
>      return 1 if $ret =~ m/OK/s;
> @@ -4344,7 +4344,7 @@ sub qemu_driveadd {
>  sub qemu_drivedel {
>      my($vmid, $deviceid) = @_;
>  
> -    my $ret = vm_human_monitor_command($vmid, "drive_del drive-$deviceid");
> +    my $ret = PVE::QMP::vm_human_monitor_command($vmid, "drive_del 
> drive-$deviceid");
>      $ret =~ s/^\s+//;
>  
>      return 1 if $ret eq "";
> @@ -5471,60 +5471,6 @@ sub vm_start {
>      });
>  }
>  
> -sub vm_mon_cmd {
> -    my ($vmid, $execute, %params) = @_;
> -
> -    my $cmd = { execute => $execute, arguments => \%params };
> -    vm_qmp_command($vmid, $cmd);
> -}
> -
> -sub vm_mon_cmd_nocheck {
> -    my ($vmid, $execute, %params) = @_;
> -
> -    my $cmd = { execute => $execute, arguments => \%params };
> -    vm_qmp_command($vmid, $cmd, 1);
> -}
> -
> -sub vm_qmp_command {
> -    my ($vmid, $cmd, $nocheck) = @_;
> -
> -    my $res;
> -
> -    my $timeout;
> -    if ($cmd->{arguments}) {
> -     $timeout = delete $cmd->{arguments}->{timeout};
> -    }
> -
> -    eval {
> -     die "VM $vmid not running\n" if !PVE::QemuConfig::check_running($vmid, 
> $nocheck);
> -     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();
> -
> -         $res = $qmpclient->cmd($vmid, $cmd, $timeout);
> -     } else {
> -         die "unable to open monitor socket\n";
> -     }
> -    };
> -    if (my $err = $@) {
> -     syslog("err", "VM $vmid qmp command failed - $err");
> -     die $err;
> -    }
> -
> -    return $res;
> -}
> -
> -sub vm_human_monitor_command {
> -    my ($vmid, $cmdline) = @_;
> -
> -    my $cmd = {
> -     execute => 'human-monitor-command',
> -     arguments => { 'command-line' => $cmdline},
> -    };
> -
> -    return vm_qmp_command($vmid, $cmd);
> -}
> -
>  sub vm_commandline {
>      my ($storecfg, $vmid, $snapname) = @_;
>  
> @@ -5848,7 +5794,7 @@ sub vm_sendkey {
>       my $conf = PVE::QemuConfig->load_config($vmid);
>  
>       # there is no qmp command, so we use the human monitor command
> -     my $res = vm_human_monitor_command($vmid, "sendkey $key");
> +     my $res = PVE::QMP::vm_human_monitor_command($vmid, "sendkey $key");
>       die $res if $res ne '';
>      });
>  }
> @@ -6911,7 +6857,7 @@ sub qemu_drive_mirror_monitor {
>                   my $agent_running = $qga && qga_check_running($vmid);
>                   if ($agent_running) {
>                       print "freeze filesystem\n";
> -                     eval { PVE::QemuServer::vm_mon_cmd($vmid, 
> "guest-fsfreeze-freeze"); };
> +                     eval { vm_mon_cmd($vmid, "guest-fsfreeze-freeze"); };
>                   } else {
>                       print "suspend vm\n";
>                       eval { PVE::QemuServer::vm_suspend($vmid, 1); };
> @@ -6922,7 +6868,7 @@ sub qemu_drive_mirror_monitor {
>  
>                   if ($agent_running) {
>                       print "unfreeze filesystem\n";
> -                     eval { PVE::QemuServer::vm_mon_cmd($vmid, 
> "guest-fsfreeze-thaw"); };
> +                     eval { vm_mon_cmd($vmid, "guest-fsfreeze-thaw"); };
>                   } else {
>                       print "resume vm\n";
>                       eval {  PVE::QemuServer::vm_resume($vmid, 1, 1); };
> diff --git a/PVE/QemuServer/Agent.pm b/PVE/QemuServer/Agent.pm
> index 8ffe3bc..aa51316 100644
> --- a/PVE/QemuServer/Agent.pm
> +++ b/PVE/QemuServer/Agent.pm
> @@ -5,6 +5,7 @@ use warnings;
>  
>  use PVE::QemuConfig;
>  use PVE::QemuServer;
> +use PVE::QMP;
>  use MIME::Base64 qw(decode_base64);
>  use JSON;
>  use base 'Exporter';
> @@ -60,7 +61,7 @@ sub agent_cmd {
>      my $conf = PVE::QemuConfig->load_config($vmid); # also checks if VM 
> exists
>      agent_available($vmid, $conf, $noerr);
>  
> -    my $res = PVE::QemuServer::vm_mon_cmd($vmid, "guest-$cmd", %$params);
> +    my $res = PVE::QMP::vm_mon_cmd($vmid, "guest-$cmd", %$params);
>      check_agent_error($res, $errormsg, $noerr);
>  
>      return $res;
> diff --git a/PVE/QemuServer/Memory.pm b/PVE/QemuServer/Memory.pm
> index 3f4088b..b4c9129 100644
> --- a/PVE/QemuServer/Memory.pm
> +++ b/PVE/QemuServer/Memory.pm
> @@ -4,6 +4,7 @@ use strict;
>  use warnings;
>  use PVE::QemuConfig;
>  use PVE::QemuServer;
> +use PVE::QMP qw(vm_mon_cmd vm_mon_cmd_nocheck);
>  use PVE::Tools qw(run_command lock_file lock_file_full file_read_firstline 
> dir_glob_foreach);
>  use PVE::Exception qw(raise raise_param_exc);
>  
> @@ -140,7 +141,7 @@ sub qemu_memory_hotplug {
>                       my $hugepages_host_topology = hugepages_host_topology();
>                       hugepages_allocate($hugepages_topology, 
> $hugepages_host_topology);
>  
> -                     eval { PVE::QemuServer::vm_mon_cmd($vmid, "object-add", 
> 'qom-type' => "memory-backend-file", id => "mem-$name", props => {
> +                     eval { vm_mon_cmd($vmid, "object-add", 'qom-type' => 
> "memory-backend-file", id => "mem-$name", props => {
>                                            size => int($dimm_size*1024*1024), 
> 'mem-path' => $path, share => JSON::true, prealloc => JSON::true } ); };
>                       if (my $err = $@) {
>                           hugepages_reset($hugepages_host_topology);
> @@ -152,7 +153,7 @@ sub qemu_memory_hotplug {
>                   eval { hugepages_update_locked($code); };
>  
>               } else {
> -                 eval { PVE::QemuServer::vm_mon_cmd($vmid, "object-add", 
> 'qom-type' => "memory-backend-ram", id => "mem-$name", props => { size => 
> int($dimm_size*1024*1024) } ) };
> +                 eval { vm_mon_cmd($vmid, "object-add", 'qom-type' => 
> "memory-backend-ram", id => "mem-$name", props => { size => 
> int($dimm_size*1024*1024) } ) };
>               }
>  
>               if (my $err = $@) {
> @@ -160,7 +161,7 @@ sub qemu_memory_hotplug {
>                   die $err;
>               }
>  
> -             eval { PVE::QemuServer::vm_mon_cmd($vmid, "device_add", driver 
> => "pc-dimm", id => "$name", memdev => "mem-$name", node => $numanode) };
> +             eval { vm_mon_cmd($vmid, "device_add", driver => "pc-dimm", id 
> => "$name", memdev => "mem-$name", node => $numanode) };
>               if (my $err = $@) {
>                   eval { PVE::QemuServer::qemu_objectdel($vmid, "mem-$name"); 
> };
>                   die $err;
> @@ -200,7 +201,7 @@ sub qemu_memory_hotplug {
>  sub qemu_dimm_list {
>      my ($vmid) = @_;
>  
> -    my $dimmarray = PVE::QemuServer::vm_mon_cmd_nocheck($vmid, 
> "query-memory-devices");
> +    my $dimmarray = vm_mon_cmd_nocheck($vmid, "query-memory-devices");
>      my $dimms = {};
>  
>      foreach my $dimm (@$dimmarray) {
> diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
> index 91768de..dc6b29a 100644
> --- a/PVE/VZDump/QemuServer.pm
> +++ b/PVE/VZDump/QemuServer.pm
> @@ -19,6 +19,7 @@ use PVE::VZDump;
>  
>  use PVE::QemuConfig;
>  use PVE::QemuServer;
> +use PVE::QMP qw(vm_mon_cmd);
>  
>  use base qw (PVE::VZDump::Plugin);
>  
> @@ -418,7 +419,7 @@ sub archive {
>       }
>  
>       if ($agent_running){
> -         eval { PVE::QemuServer::vm_mon_cmd($vmid, "guest-fsfreeze-freeze"); 
> };
> +         eval { vm_mon_cmd($vmid, "guest-fsfreeze-freeze"); };
>           if (my $err = $@) {
>               $self->logerr($err);
>           }
> @@ -428,7 +429,7 @@ sub archive {
>       my $qmperr = $@;
>  
>       if ($agent_running){
> -         eval { PVE::QemuServer::vm_mon_cmd($vmid, "guest-fsfreeze-thaw"); };
> +         eval { vm_mon_cmd($vmid, "guest-fsfreeze-thaw"); };
>           if (my $err = $@) {
>               $self->logerr($err);
>           }
> @@ -453,7 +454,7 @@ sub archive {
>           } else {
>               $self->loginfo("resuming VM again");
>           }
> -         PVE::QemuServer::vm_mon_cmd($vmid, 'cont');
> +         vm_mon_cmd($vmid, 'cont');
>       }
>  
>       my $status;
> @@ -466,7 +467,7 @@ sub archive {
>       my $transferred;
>  
>       while(1) {
> -         $status = PVE::QemuServer::vm_mon_cmd($vmid, 'query-backup');
> +         $status = vm_mon_cmd($vmid, 'query-backup');
>           my $total = $status->{total} || 0;
>           $transferred = $status->{transferred} || 0;
>           my $per = $total ? int(($transferred * 100)/$total) : 0;
> @@ -525,7 +526,7 @@ sub archive {
>      if ($err) {
>       $self->logerr($err);
>       $self->loginfo("aborting backup job");
> -     eval { PVE::QemuServer::vm_mon_cmd($vmid, 'backup-cancel'); };
> +     eval { vm_mon_cmd($vmid, 'backup-cancel'); };
>       if (my $err1 = $@) {
>           $self->logerr($err1);
>       }
> @@ -534,7 +535,7 @@ sub archive {
>      if ($stop_after_backup) {
>       # stop if not running
>       eval {
> -         my $resp = PVE::QemuServer::vm_mon_cmd($vmid, 'query-status');
> +         my $resp = vm_mon_cmd($vmid, 'query-status');
>           my $status = $resp && $resp->{status} ?  $resp->{status} : 
> 'unknown';
>           if ($status eq 'prelaunch') {
>               $self->loginfo("stopping kvm after backup task");
> diff --git a/test/snapshot-test.pm b/test/snapshot-test.pm
> index 53d08e6..0ed0609 100644
> --- a/test/snapshot-test.pm
> +++ b/test/snapshot-test.pm
> @@ -9,6 +9,7 @@ use PVE::Storage;
>  use PVE::Storage::Plugin;
>  use PVE::QemuServer;
>  use PVE::QemuConfig;
> +use PVE::QMP;
>  use PVE::Tools;
>  use PVE::ReplicationConfig;
>  
> @@ -304,11 +305,7 @@ sub check_running {
>  
>  # END mocked PVE::QemuConfig methods
>  
> -# BEGIN redefine PVE::QemuServer methods
> -
> -sub do_snapshots_with_qemu {
> -    return 0;
> -}
> +# BEGIN mocked PVE::QMP methods
>  
>  sub vm_qmp_command {
>      my ($vmid, $cmd, $nocheck) = @_;
> @@ -343,6 +340,14 @@ sub vm_qmp_command {
>      die "unexpected vm_qmp_command!\n";
>  }
>  
> +# END mocked PVE::QMP methods
> +
> +# BEGIN redefine PVE::QemuServer methods
> +
> +sub do_snapshots_with_qemu {
> +    return 0;
> +}
> +
>  sub vm_start {
>      my ($storecfg, $vmid, $statefile, $skiplock, $migratedfrom, $paused, 
> $forcemachine) = @_;
>  
> @@ -369,6 +374,9 @@ sub vm_stop {
>  PVE::Tools::run_command("rm -rf snapshot-working");
>  PVE::Tools::run_command("cp -a snapshot-input snapshot-working");
>  
> +my $qmp_module = new Test::MockModule('PVE::QMP');
> +$qmp_module->mock('vm_qmp_command', \&vm_qmp_command);
> +
>  my $qemu_config_module = new Test::MockModule('PVE::QemuConfig');
>  $qemu_config_module->mock('config_file_lock', \&config_file_lock);
>  $qemu_config_module->mock('cfs_config_path', \&cfs_config_path);
> -- 
> 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

Reply via email to