[pve-devel] [PATCH qemu-server v2 06/16] qmp client: better abstract peer in preparation for qemu-storage-daemon

[Fiona Ebner <f.ebner@proxmox.com>]

In preparation to add 'qsd' as a peer type for qemu-storage-daemon.

There already are two different peer types, namely 'qga' for the QEMU
guest agent and 'qmp' for the QEMU instance itself.

Future QMP peers (like the qemu-storage-daemon) are likely to use a
QMP monitor with capability negotiation like QEMU itself, so the
special handling done for the guest agent stays limited to the 'qga'
peer type.

Replace the association with a VM ID and allow specifying an arbitrary
ID.

Precise two error messages that used a hard-coded 'qmp' by specifing
the actual QMP peer type instead. The $peer structure also has a name
that is used for error messages to avoid hard-coding 'VM' there.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Changes in v2:
* Improve commit message.
* Further abstract QMP peer in QMP client/monitor modules:
  Replace 'vmid' by 'id' and allow specifying a peer name for error
  messages. This is preparation for use cases of the storage daemon
  where there might not be a single associated guest. For example,
  restoring from a backup provider via exports of a storage daemon,
  and a second storage daemon for the TPM of the VM itself.

 src/PVE/QMPClient.pm          | 56 +++++++++++++++++------------------
 src/PVE/QemuServer.pm         | 21 ++++++++-----
 src/PVE/QemuServer/Helpers.pm |  6 ++--
 src/PVE/QemuServer/Monitor.pm | 49 +++++++++++++++++++++---------
 src/PVE/VZDump/QemuServer.pm  |  9 ++++--
 src/test/snapshot-test.pm     |  2 +-
 6 files changed, 86 insertions(+), 57 deletions(-)

diff --git a/src/PVE/QMPClient.pm b/src/PVE/QMPClient.pm
index 1935a336..e4f7a515 100644
--- a/src/PVE/QMPClient.pm
+++ b/src/PVE/QMPClient.pm
@@ -53,15 +53,13 @@ my $qga_allow_close_cmds = {
 };
 
 my $push_cmd_to_queue = sub {
-    my ($self, $vmid, $cmd) = @_;
+    my ($self, $peer, $cmd) = @_;
 
     my $execute = $cmd->{execute} || die "no command name specified";
 
-    my $qga = ($execute =~ /^guest\-+/) ? 1 : 0;
+    my $sname = PVE::QemuServer::Helpers::qmp_socket($peer);
 
-    my $sname = PVE::QemuServer::Helpers::qmp_socket($vmid, $qga);
-
-    $self->{queue_info}->{$sname} = { qga => $qga, vmid => $vmid, sname => $sname, cmds => [] }
+    $self->{queue_info}->{$sname} = { peer => $peer, sname => $sname, cmds => [] }
         if !$self->{queue_info}->{$sname};
 
     push @{ $self->{queue_info}->{$sname}->{cmds} }, $cmd;
@@ -72,26 +70,26 @@ my $push_cmd_to_queue = sub {
 # add a single command to the queue for later execution
 # with queue_execute()
 sub queue_cmd {
-    my ($self, $vmid, $callback, $execute, %params) = @_;
+    my ($self, $peer, $callback, $execute, %params) = @_;
 
     my $cmd = {};
     $cmd->{execute} = $execute;
     $cmd->{arguments} = \%params;
     $cmd->{callback} = $callback;
 
-    &$push_cmd_to_queue($self, $vmid, $cmd);
+    &$push_cmd_to_queue($self, $peer, $cmd);
 
     return;
 }
 
 # execute a single command
 sub cmd {
-    my ($self, $vmid, $cmd, $timeout, $noerr) = @_;
+    my ($self, $peer, $cmd, $timeout, $noerr) = @_;
 
     my $result;
 
     my $callback = sub {
-        my ($vmid, $resp) = @_;
+        my ($id, $resp) = @_;
         $result = $resp->{'return'};
         $result = { error => $resp->{'error'} } if !defined($result) && $resp->{'error'};
     };
@@ -101,7 +99,7 @@ sub cmd {
     $cmd->{callback} = $callback;
     $cmd->{arguments} = {} if !defined($cmd->{arguments});
 
-    my $queue_info = &$push_cmd_to_queue($self, $vmid, $cmd);
+    my $queue_info = &$push_cmd_to_queue($self, $peer, $cmd);
 
     if (!$timeout) {
         # hack: monitor sometime blocks
@@ -158,7 +156,8 @@ sub cmd {
     $self->queue_execute($timeout, 2);
 
     if (defined($queue_info->{error})) {
-        die "VM $vmid qmp command '$cmd->{execute}' failed - $queue_info->{error}" if !$noerr;
+        die "$peer->{name} $peer->{type} command '$cmd->{execute}' failed - $queue_info->{error}"
+            if !$noerr;
         $result = { error => $queue_info->{error} };
         $result->{'error-is-timeout'} = 1 if $queue_info->{'error-is-timeout'};
     }
@@ -206,10 +205,10 @@ my $open_connection = sub {
 
     die "duplicate call to open" if defined($queue_info->{fh});
 
-    my $vmid = $queue_info->{vmid};
-    my $qga = $queue_info->{qga};
+    my $peer = $queue_info->{peer};
+    my ($peer_name, $sotype) = $peer->@{qw(name type)};
 
-    my $sname = PVE::QemuServer::Helpers::qmp_socket($vmid, $qga);
+    my $sname = PVE::QemuServer::Helpers::qmp_socket($peer);
 
     $timeout = 1 if !$timeout;
 
@@ -217,18 +216,17 @@ my $open_connection = sub {
     my $starttime = [gettimeofday];
     my $count = 0;
 
-    my $sotype = $qga ? 'qga' : 'qmp';
-
     for (;;) {
         $count++;
         $fh = IO::Socket::UNIX->new(Peer => $sname, Blocking => 0, Timeout => 1);
         last if $fh;
         if ($! != EINTR && $! != EAGAIN) {
-            die "unable to connect to VM $vmid $sotype socket - $!\n";
+            die "unable to connect to $peer_name $sotype socket - $!\n";
         }
         my $elapsed = tv_interval($starttime, [gettimeofday]);
         if ($elapsed >= $timeout) {
-            die "unable to connect to VM $vmid $sotype socket - timeout after $count retries\n";
+            die
+                "unable to connect to $peer_name $sotype socket - timeout after $count retries\n";
         }
         usleep(100000);
     }
@@ -253,7 +251,7 @@ my $check_queue = sub {
         my $fh = $queue_info->{fh};
         next if !$fh;
 
-        my $qga = $queue_info->{qga};
+        my $qga = $queue_info->{peer}->{type} eq 'qga';
 
         if ($queue_info->{error}) {
             &$close_connection($self, $queue_info);
@@ -339,7 +337,7 @@ sub queue_execute {
         eval {
             &$open_connection($self, $queue_info, $timeout);
 
-            if (!$queue_info->{qga}) {
+            if ($queue_info->{peer}->{type} ne 'qga') {
                 my $cap_cmd = { execute => 'qmp_capabilities', arguments => {} };
                 unshift @{ $queue_info->{cmds} }, $cap_cmd;
             }
@@ -397,11 +395,11 @@ sub mux_input {
     return if !$queue_info;
 
     my $sname = $queue_info->{sname};
-    my $vmid = $queue_info->{vmid};
-    my $qga = $queue_info->{qga};
+    my ($id, $peer_name) = $queue_info->{peer}->@{qw(id name)};
+    my $qga = $queue_info->{peer}->{type} eq 'qga';
 
     my $curcmd = $queue_info->{current};
-    die "unable to lookup current command for VM $vmid ($sname)\n" if !$curcmd;
+    die "unable to lookup current command for $peer_name ($sname)\n" if !$curcmd;
 
     my $raw;
 
@@ -437,7 +435,7 @@ sub mux_input {
             $obj = from_json($jsons[1]);
 
             if (my $callback = $curcmd->{callback}) {
-                &$callback($vmid, $obj);
+                &$callback($id, $obj);
             }
 
             return;
@@ -471,7 +469,7 @@ sub mux_input {
             delete $queue_info->{current};
 
             if (my $callback = $curcmd->{callback}) {
-                &$callback($vmid, $obj);
+                &$callback($id, $obj);
             }
         }
     };
@@ -501,11 +499,11 @@ sub mux_eof {
     return if !$queue_info;
 
     my $sname = $queue_info->{sname};
-    my $vmid = $queue_info->{vmid};
-    my $qga = $queue_info->{qga};
+    my ($id, $peer_name) = $queue_info->{peer}->@{qw(id name)};
+    my $qga = $queue_info->{peer}->{type} eq 'qga';
 
     my $curcmd = $queue_info->{current};
-    die "unable to lookup current command for VM $vmid ($sname)\n" if !$curcmd;
+    die "unable to lookup current command for $peer_name ($sname)\n" if !$curcmd;
 
     if ($qga && $qga_allow_close_cmds->{ $curcmd->{execute} }) {
 
@@ -522,7 +520,7 @@ sub mux_eof {
             delete $queue_info->{current};
 
             if (my $callback = $curcmd->{callback}) {
-                &$callback($vmid, undef);
+                &$callback($id, undef);
             }
         };
         if (my $err = $@) {
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index df2476aa..e8bce20f 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -2706,13 +2706,15 @@ sub vmstatus {
     my $statuscb = sub {
         my ($vmid, $resp) = @_;
 
-        $qmpclient->queue_cmd($vmid, $proxmox_support_cb, 'query-proxmox-support');
-        $qmpclient->queue_cmd($vmid, $blockstatscb, 'query-blockstats');
-        $qmpclient->queue_cmd($vmid, $machinecb, 'query-machines');
-        $qmpclient->queue_cmd($vmid, $versioncb, 'query-version');
+        my $qmp_peer = { name => "VM $vmid", id => $vmid, type => 'qmp' };
+
+        $qmpclient->queue_cmd($qmp_peer, $proxmox_support_cb, 'query-proxmox-support');
+        $qmpclient->queue_cmd($qmp_peer, $blockstatscb, 'query-blockstats');
+        $qmpclient->queue_cmd($qmp_peer, $machinecb, 'query-machines');
+        $qmpclient->queue_cmd($qmp_peer, $versioncb, 'query-version');
         # this fails if ballon driver is not loaded, so this must be
         # the last command (following command are aborted if this fails).
-        $qmpclient->queue_cmd($vmid, $ballooncb, 'query-balloon');
+        $qmpclient->queue_cmd($qmp_peer, $ballooncb, 'query-balloon');
 
         my $status = 'unknown';
         if (!defined($status = $resp->{'return'}->{status})) {
@@ -2726,7 +2728,8 @@ sub vmstatus {
     foreach my $vmid (keys %$list) {
         next if $opt_vmid && ($vmid ne $opt_vmid);
         next if !$res->{$vmid}->{pid}; # not running
-        $qmpclient->queue_cmd($vmid, $statuscb, 'query-status');
+        my $qmp_peer = { name => "VM $vmid", id => $vmid, type => 'qmp' };
+        $qmpclient->queue_cmd($qmp_peer, $statuscb, 'query-status');
     }
 
     $qmpclient->queue_execute(undef, 2);
@@ -3180,7 +3183,8 @@ sub config_to_command {
 
     my $use_virtio = 0;
 
-    my $qmpsocket = PVE::QemuServer::Helpers::qmp_socket($vmid);
+    my $qmpsocket =
+        PVE::QemuServer::Helpers::qmp_socket({ name => "VM $vmid", id => $vmid, type => 'qmp' });
     push @$cmd, '-chardev', "socket,id=qmp,path=$qmpsocket,server=on,wait=off";
     push @$cmd, '-mon', "chardev=qmp,mode=control";
 
@@ -3417,7 +3421,8 @@ sub config_to_command {
     my $guest_agent = parse_guest_agent($conf);
 
     if ($guest_agent->{enabled}) {
-        my $qgasocket = PVE::QemuServer::Helpers::qmp_socket($vmid, 1);
+        my $qgasocket = PVE::QemuServer::Helpers::qmp_socket(
+            { name => "VM $vmid", id => $vmid, type => 'qga' });
         push @$devices, '-chardev', "socket,path=$qgasocket,server=on,wait=off,id=qga0";
 
         if (!$guest_agent->{type} || $guest_agent->{type} eq 'virtio') {
diff --git a/src/PVE/QemuServer/Helpers.pm b/src/PVE/QemuServer/Helpers.pm
index 3e444839..87f4f841 100644
--- a/src/PVE/QemuServer/Helpers.pm
+++ b/src/PVE/QemuServer/Helpers.pm
@@ -79,9 +79,9 @@ our $var_run_tmpdir = "/var/run/qemu-server";
 mkdir $var_run_tmpdir;
 
 sub qmp_socket {
-    my ($vmid, $qga) = @_;
-    my $sockettype = $qga ? 'qga' : 'qmp';
-    return "${var_run_tmpdir}/$vmid.$sockettype";
+    my ($peer) = @_;
+    my ($id, $type) = $peer->@{qw(id type)};
+    return "${var_run_tmpdir}/${id}.${type}";
 }
 
 sub pidfile_name {
diff --git a/src/PVE/QemuServer/Monitor.pm b/src/PVE/QemuServer/Monitor.pm
index 0cccdfbe..293679fe 100644
--- a/src/PVE/QemuServer/Monitor.pm
+++ b/src/PVE/QemuServer/Monitor.pm
@@ -15,19 +15,32 @@ our @EXPORT_OK = qw(
 =head3 qmp_cmd
 
     my $cmd = { execute => $qmp_command_name, arguments => \%params };
-    my $result = qmp_cmd($vmid, $cmd);
+    my $peer = { name => $name, id => $id, type => $type };
+    my $result = qmp_cmd($peer, $cmd);
 
-Execute the C<$qmp_command_name> with arguments C<%params> for VM C<$vmid>. Dies if the VM is not
-running or the monitor socket cannot be reached, even if the C<noerr> argument is used. Returns the
-structured result from the QMP side converted from JSON to structured Perl data. In case the
-C<noerr> argument is used and the QMP command failed or timed out, the result is a hash reference
-with an C<error> key containing the error message.
+Execute the C<$qmp_command_name> with arguments C<%params> for the peer C<$peer>. The type C<$type>
+of the peer can be C<qmp> for the QEMU instance of the VM or C<qga> for the guest agent of the VM.
+Dies if the VM is not running or the monitor socket cannot be reached, even if the C<noerr> argument
+is used. Returns the structured result from the QMP side converted from JSON to structured Perl
+data. In case the C<noerr> argument is used and the QMP command failed or timed out, the result is a
+hash reference with an C<error> key containing the error message.
 
 Parameters:
 
 =over
 
-=item C<$vmid>: The ID of the virtual machine.
+=item C<$peer>: The peer to communicate with. A hash reference with:
+
+=over
+
+=item C<$name>: Name of the peer used in error messages.
+
+=item C<$id>: Identifier for the peer. The pair C<($id, $type)> uniquely identifies a peer.
+
+=item C<$type>: Type of the peer to communicate with. This can be C<qmp> for the VM's QEMU instance
+or C<qga> for the VM's guest agent.
+
+=back
 
 =item C<$cmd>: Hash reference containing the QMP command name for the C<execute> key and additional
 arguments for the QMP command under the C<arguments> key. The following custom arguments are not
@@ -48,7 +61,7 @@ handle the error that is returned as a structured result.
 =cut
 
 sub qmp_cmd {
-    my ($vmid, $cmd) = @_;
+    my ($peer, $cmd) = @_;
 
     my $res;
 
@@ -58,18 +71,24 @@ sub qmp_cmd {
     }
 
     eval {
-        die "VM $vmid not running\n" if !PVE::QemuServer::Helpers::vm_running_locally($vmid);
-        my $sname = PVE::QemuServer::Helpers::qmp_socket($vmid);
+        if ($peer->{type} eq 'qmp' || $peer->{type} eq 'qga') {
+            die "$peer->{name} not running\n"
+                if !PVE::QemuServer::Helpers::vm_running_locally($peer->{id});
+        } else {
+            die "qmp_cmd - unknown peer type $peer->{type}\n";
+        }
+
+        my $sname = PVE::QemuServer::Helpers::qmp_socket($peer);
         if (-e $sname) { # test if VM is reasonably new and supports qmp/qga
             my $qmpclient = PVE::QMPClient->new();
 
-            $res = $qmpclient->cmd($vmid, $cmd, $timeout, $noerr);
+            $res = $qmpclient->cmd($peer, $cmd, $timeout, $noerr);
         } else {
             die "unable to open monitor socket\n";
         }
     };
     if (my $err = $@) {
-        syslog("err", "VM $vmid qmp command failed - $err");
+        syslog("err", "$peer->{name} $peer->{type} command failed - $err");
         die $err;
     }
 
@@ -81,7 +100,9 @@ sub mon_cmd {
 
     my $cmd = { execute => $execute, arguments => \%params };
 
-    return qmp_cmd($vmid, $cmd);
+    my $type = ($execute =~ /^guest\-+/) ? 'qga' : 'qmp';
+
+    return qmp_cmd({ name => "VM $vmid", id => $vmid, type => $type }, $cmd);
 }
 
 sub hmp_cmd {
@@ -92,7 +113,7 @@ sub hmp_cmd {
         arguments => { 'command-line' => $cmdline, timeout => $timeout },
     };
 
-    return qmp_cmd($vmid, $cmd);
+    return qmp_cmd({ name => "VM $vmid", id => $vmid, type => 'qmp' }, $cmd);
 }
 
 1;
diff --git a/src/PVE/VZDump/QemuServer.pm b/src/PVE/VZDump/QemuServer.pm
index dd789652..84ebbe80 100644
--- a/src/PVE/VZDump/QemuServer.pm
+++ b/src/PVE/VZDump/QemuServer.pm
@@ -995,6 +995,7 @@ sub archive_vma {
         }
 
         my $qmpclient = PVE::QMPClient->new();
+        my $qmp_peer = { name => "VM $vmid", id => $vmid, type => 'qmp' };
         my $backup_cb = sub {
             my ($vmid, $resp) = @_;
             $backup_job_uuid = $resp->{return}->{UUID};
@@ -1012,10 +1013,14 @@ sub archive_vma {
             $params->{fleecing} = JSON::true if $task->{'use-fleecing'};
             add_backup_performance_options($params, $opts->{performance}, $qemu_support);
 
-            $qmpclient->queue_cmd($vmid, $backup_cb, 'backup', %$params);
+            $qmpclient->queue_cmd($qmp_peer, $backup_cb, 'backup', %$params);
         };
 
-        $qmpclient->queue_cmd($vmid, $add_fd_cb, 'getfd', fd => $outfileno, fdname => "backup");
+        $qmpclient->queue_cmd(
+            $qmp_peer, $add_fd_cb, 'getfd',
+            fd => $outfileno,
+            fdname => "backup",
+        );
 
         my $fs_frozen = $self->qga_fs_freeze($task, $vmid);
 
diff --git a/src/test/snapshot-test.pm b/src/test/snapshot-test.pm
index f61cd64b..5808f032 100644
--- a/src/test/snapshot-test.pm
+++ b/src/test/snapshot-test.pm
@@ -356,7 +356,7 @@ sub vm_running_locally {
 # BEGIN mocked PVE::QemuServer::Monitor methods
 
 sub qmp_cmd {
-    my ($vmid, $cmd) = @_;
+    my ($peer, $cmd) = @_;
 
     my $exec = $cmd->{execute};
     if ($exec eq "guest-ping") {
-- 
2.47.3



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel