Libvirt provides QMP passthrough APIs for the QEMU driver and these are
exposed in virsh. It is not especially pleasant, however, using the raw
QMP JSON syntax. QEMU has a tool 'qmp-shell' which can speak QMP and
exposes a human friendly interactive shell. It is not possible to use
this with libvirt managed guest, however, since only one client can
attach to the QMP socket at any point in time. While it would be
possible to configure a second QMP socket for a VM, it may not be
an known requirement at the time the guest is provisioned.

The virt-qmp-proxy tool aims to solve this problem. It opens a UNIX
socket and listens for incoming client connections, speaking QMP on
the connected socket. It will forward any QMP commands received onto
the running libvirt QEMU guest, and forward any replies back to the
QMP client. It will also forward back events.

  $ virsh start demo
  $ virt-qmp-proxy demo demo.qmp &
  $ qmp-shell demo.qmp
  Welcome to the QMP low-level shell!
  Connected to QEMU 6.2.0

  (QEMU) query-kvm
  {
      "return": {
          "enabled": true,
          "present": true
      }
  }

Note this tool of course has the same risks as the raw libvirt
QMP passthrough. It is safe to run query commands to fetch information
but commands which change the QEMU state risk disrupting libvirt's
management of QEMU, potentially resulting in data loss/corruption in
the worst case. Any use of this tool will cause the guest to be marked
as tainted as an warning that it could be in an unexpected state.

Since this tool introduces a python dependency it is not desirable
to include it in any of the existing RPMs in libvirt. This tool is
also QEMU specific, so isn't appropriate to bundle with the generic
tools. Thus a new RPM is introduced 'libvirt-clients-qemu', to
contain additional QEMU specific tools, with extra external deps.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---

In v3:

  - Added to libvirt-clients-qemu RPM
  - Renamed to virt-qemu-qmp-proxy

 docs/manpages/meson.build             |   1 +
 docs/manpages/virt-qemu-qmp-proxy.rst | 120 +++++++++
 libvirt.spec.in                       |  15 ++
 tools/meson.build                     |   5 +
 tools/virt-qemu-qmp-proxy             | 360 ++++++++++++++++++++++++++
 5 files changed, 501 insertions(+)
 create mode 100644 docs/manpages/virt-qemu-qmp-proxy.rst
 create mode 100755 tools/virt-qemu-qmp-proxy

diff --git a/docs/manpages/meson.build b/docs/manpages/meson.build
index ba673cf472..83933227d9 100644
--- a/docs/manpages/meson.build
+++ b/docs/manpages/meson.build
@@ -18,6 +18,7 @@ docs_man_files = [
   { 'name': 'virt-pki-query-dn', 'section': '1', 'install': true },
   { 'name': 'virt-pki-validate', 'section': '1', 'install': true },
   { 'name': 'virt-qemu-run', 'section': '1', 'install': conf.has('WITH_QEMU') },
+  { 'name': 'virt-qemu-qmp-proxy', 'section': '1', 'install': conf.has('WITH_QEMU') },
   { 'name': 'virt-xml-validate', 'section': '1', 'install': true },
 
   { 'name': 'libvirt-guests', 'section': '8', 'install': conf.has('WITH_LIBVIRTD') },
diff --git a/docs/manpages/virt-qemu-qmp-proxy.rst b/docs/manpages/virt-qemu-qmp-proxy.rst
new file mode 100644
index 0000000000..b387474b19
--- /dev/null
+++ b/docs/manpages/virt-qemu-qmp-proxy.rst
@@ -0,0 +1,120 @@
+===================
+virt-qemu-qmp-proxy
+===================
+
+--------------------------------------------------
+Expose a QMP proxy server for a libvirt QEMU guest
+--------------------------------------------------
+
+:Manual section: 1
+:Manual group: Virtualization Support
+
+.. contents::
+
+
+SYNOPSIS
+========
+
+``virt-qemu-qmp-proxy`` [*OPTION*]... *DOMAIN* *QMP-SOCKET-PATH*
+
+
+DESCRIPTION
+===========
+
+This tool provides a way to expose a QMP proxy server that communicates
+with a QEMU guest managed by libvirt. This enables standard QMP client
+tools to interact with libvirt managed guests.
+
+**NOTE: use of this tool will result in the running QEMU guest being
+marked as tainted.** It is strongly recommended that this tool *only be
+used to send commands which query information* about the running guest.
+If this tool is used to make changes to the state of the guest, this
+may have negative interactions with the QEMU driver, resulting in an
+inability to manage the guest operation thereafter, and in the worst
+case **potentially lead to data loss or corruption**.
+
+The ``virt-qemu-qmp-proxy`` program will listen on a UNIX socket for incoming
+client connections, and run the QMP protocol over the connection. Any
+commands received will be sent to the running libvirt guest, and replies
+sent back.
+
+The ``virt-qemu-qmp-proxy`` program may be interrupted (eg Ctrl-C) when it
+is no longer required. The libvirt QEMU guest will continue running.
+
+
+OPTIONS
+=======
+
+*DOMAIN*
+
+The ID or UUID or Name of the libvirt QEMU guest.
+
+*QMP-SOCKET-PATH*
+
+The filesystem path at which to run the QMP server, listening for
+incoming connections.
+
+``-c`` *CONNECTION-URI*
+``--connect``\ =\ *CONNECTION-URI*
+
+The URI for the connection to the libvirt QEMU driver. If omitted,
+a URI will be auto-detected.
+
+``-v``, ``--verbose``
+
+Run in verbose mode, printing all QMP commands and replies that
+are handled.
+
+``-h``, ``--help``
+
+Display the command line help.
+
+
+EXIT STATUS
+===========
+
+Upon successful shutdown, an exit status of 0 will be set. Upon
+failure a non-zero status will be set.
+
+
+AUTHOR
+======
+
+Daniel P. Berrangé
+
+
+BUGS
+====
+
+Please report all bugs you discover.  This should be done via either:
+
+#. the mailing list
+
+   `https://libvirt.org/contact.html <https://libvirt.org/contact.html>`_
+
+#. the bug tracker
+
+   `https://libvirt.org/bugs.html <https://libvirt.org/bugs.html>`_
+
+Alternatively, you may report bugs to your software distributor / vendor.
+
+COPYRIGHT
+=========
+
+Copyright (C) 2022 by Red Hat, Inc.
+
+
+LICENSE
+=======
+
+``virt-qemu-qmp-proxy`` is distributed under the terms of the GNU LGPL v2+.
+This is free software; see the source for copying conditions. There
+is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
+PURPOSE
+
+
+SEE ALSO
+========
+
+virsh(1), `https://libvirt.org/ <https://libvirt.org/>`_,
+`QMP reference <https://www.qemu.org/docs/master/interop/qemu-qmp-ref.html>`_
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 950c5a3b6d..3082285dc0 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -899,6 +899,15 @@ Obsoletes: libvirt-bash-completion < 7.3.0
 The client binaries needed to access the virtualization
 capabilities of recent versions of Linux (and other OSes).
 
+%package client-qemu
+Summary: Additional client side utilities for QEMU
+Requires: %{name}-libs = %{version}-%{release}
+Requires: python3-libvirt >= %{version}-%{release}
+
+%description client-qemu
+The additional client binaries are used to interact
+with some QEMU specific features of libvirt.
+
 %package libs
 Summary: Client side libraries
 # So remote clients can access libvirt over SSH tunnel
@@ -2159,6 +2168,12 @@ exit 0
 
 %{_datadir}/bash-completion/completions/virsh
 
+%if %{with_qemu}
+%files client-qemu
+%{_mandir}/man1/virt-qemu-qmp-proxy.1*
+%{_bindir}/virt-qemu-qmp-proxy
+%endif
+
 %files libs -f %{name}.lang
 %license COPYING COPYING.LESSER
 %dir %attr(0700, root, root) %{_sysconfdir}/libvirt/
diff --git a/tools/meson.build b/tools/meson.build
index bb28a904dc..20509906af 100644
--- a/tools/meson.build
+++ b/tools/meson.build
@@ -320,6 +320,11 @@ if conf.has('WITH_LIBVIRTD')
   endif
 endif
 
+if conf.has('WITH_QEMU')
+    install_data('virt-qemu-qmp-proxy',
+                 install_dir: bindir)
+endif
+
 if bash_completion_dep.found()
   subdir('bash-completion')
 endif
diff --git a/tools/virt-qemu-qmp-proxy b/tools/virt-qemu-qmp-proxy
new file mode 100755
index 0000000000..d85342bd2b
--- /dev/null
+++ b/tools/virt-qemu-qmp-proxy
@@ -0,0 +1,360 @@
+#!/usr/bin/env python3
+
+import argparse
+import array
+import libvirt
+import libvirt_qemu
+import os
+import re
+import socket
+import sys
+import traceback
+import json
+import fcntl
+
+
+debug = False
+
+def get_domain(uri, domstr):
+    conn = libvirt.open(uri)
+
+    dom = None
+    saveex = None
+
+    def try_lookup(cb):
+        try:
+            return cb()
+        except libvirt.libvirtError as ex:
+            nonlocal saveex
+            if saveex is None:
+                saveex = ex
+
+    if re.match(r'^\d+$', domstr):
+        dom = try_lookup(lambda: conn.lookupByID(int(domstr)))
+
+    if dom is None and re.match(r'^[-a-f0-9]{36}|[a-f0-9]{32}$', domstr):
+        dom = try_lookup(lambda: conn.lookupByUUIDString(domstr))
+
+    if dom is None:
+        dom = try_lookup(lambda: conn.lookupByName(domstr))
+
+    if dom is None:
+        raise saveex
+
+    if not dom.isActive():
+        raise Exception("Domain must be running to use QMP")
+
+    return conn, dom
+
+
+class QMPProxy(object):
+
+    def __init__(self, conn, dom, serversock, verbose):
+        self.conn = conn
+        self.dom = dom
+        self.verbose = verbose
+
+        self.serversock = serversock
+        self.serverwatch = 0
+
+        self.clientsock = None
+        self.clientwatch = 0
+
+        self.api2sock = bytes([])
+        self.api2sockfds = []
+
+        self.sock2api = bytes([])
+        self.sock2apifds = []
+
+        self.serverwatch = libvirt.virEventAddHandle(
+            self.serversock.fileno(), libvirt.VIR_EVENT_HANDLE_READABLE,
+            self.handle_server_io, self)
+
+        libvirt_qemu.qemuMonitorEventRegister(self.conn, self.dom,
+                                              None,
+                                              self.handle_qmp_event,
+                                              self)
+
+
+    @staticmethod
+    def handle_qmp_event(conn, dom, event, secs, usecs, details, self):
+        evdoc = {
+            "event": event,
+            "timestamp": {"seconds": secs, "microseconds": usecs }
+        }
+        if details is not None:
+            evdoc["data"] = details
+
+        ev = json.dumps(evdoc)
+        if self.verbose:
+            print(ev)
+        ev += "\r\n"
+        self.api2sock += ev.encode("utf-8")
+        self.update_client_events()
+
+
+    def recv_with_fds(self):
+        # Match VIR_NET_MESSAGE_NUM_FDS_MAX in virnetprotocol.x
+        maxfds = 32
+        fds = array.array('i')
+        cmsgdatalen = socket.CMSG_LEN(maxfds * fds.itemsize)
+
+        data, cmsgdata, flags, addr = self.clientsock.recvmsg(1024,
+                                                              cmsgdatalen)
+        for cmsg_level, cmsg_type, cmsg_data in cmsgdata:
+            if (cmsg_level == socket.SOL_SOCKET and
+                cmsg_type == socket.SCM_RIGHTS):
+                fds.frombytes(cmsg_data[:len(cmsg_data) -
+                                         (len(cmsg_data) % fds.itemsize)])
+            else:
+                raise Exception("Unexpected CMSGDATA level %d type %d" % (
+                    cmsg_level, cmsg_type))
+
+        return data, [self.make_file(fd) for fd in fds]
+
+
+    def send_with_fds(self, data, fds):
+        cfds = [fd.fileno() for fd in fds]
+
+        cmsgdata = [(socket.SOL_SOCKET, socket.SCM_RIGHTS,
+                     array.array("i", cfds))]
+
+        return self.clientsock.sendmsg([data], cmsgdata)
+
+
+    @staticmethod
+    def make_file(fd):
+        flags = fcntl.fcntl(fd, fcntl.F_GETFL)
+
+        mask = os.O_RDONLY | os.O_WRONLY | os.O_RDWR | os.O_APPEND
+        flags = flags & mask
+        mode = ""
+        if flags == os.O_RDONLY:
+            mode = "rb"
+        elif flags == os.O_WRONLY:
+            mode = "wb"
+        elif flags == os.O_RDWR:
+            mode = "r+b"
+        elif flags == (os.O_WRONLY | os.O_APPEND):
+            mode = "ab"
+        elif flags == (os.O_RDWR | os.O_APPEND):
+            mode = "a+b"
+
+        return os.fdopen(fd, mode)
+
+
+    def add_client(self, sock):
+        ver = self.conn.getVersion()
+        major = int(ver / 1000000) % 1000
+        minor = int(ver / 1000) % 1000
+        micro = ver % 1000
+
+        greetingobj = {
+            "QMP": {
+                "version": {
+                    "qemu": {
+                        "major": major,
+                        "minor": minor,
+                        "micro": micro,
+                    },
+                    "package": f"qemu-{major}.{minor}.{micro}",
+                },
+                "capabilities": [
+                    "oob"
+                ],
+            }
+        }
+        greeting = json.dumps(greetingobj)
+        if self.verbose:
+            print(greeting)
+        greeting += "\r\n"
+
+        self.clientsock = sock
+        self.clientwatch = libvirt.virEventAddHandle(
+            self.clientsock.fileno(), libvirt.VIR_EVENT_HANDLE_WRITABLE,
+            self.handle_client_io, self)
+        self.api2sock += greeting.encode("utf-8")
+        self.update_server_events()
+
+
+    def remove_client(self):
+        libvirt.virEventRemoveHandle(self.clientwatch)
+        self.clientsock.close()
+        self.clientsock = None
+        self.clientwatch = 0
+        self.update_server_events()
+
+        self.api2sock = bytes([])
+        self.api2sockfds = []
+
+        self.sock2api = bytes([])
+        self.sock2apifds = []
+
+
+    def update_client_events(self):
+        # For simplicity of tracking distinct QMP cmds and their passed FDs
+        # we don't try to support "pipelining", only a single cmd may be
+        # inflight
+        if len(self.api2sock) > 0:
+            events = libvirt.VIR_EVENT_HANDLE_WRITABLE
+        else:
+            events = libvirt.VIR_EVENT_HANDLE_READABLE
+
+        libvirt.virEventUpdateHandle(self.clientwatch, events)
+
+
+    def update_server_events(self):
+        if self.clientsock is not None:
+            libvirt.virEventUpdateHandle(self.serverwatch, 0)
+        else:
+            libvirt.virEventUpdateHandle(self.serverwatch,
+                                         libvirt.VIR_EVENT_HANDLE_READABLE)
+
+
+    def try_command(self):
+        try:
+            cmdstr = self.sock2api.decode("utf-8")
+            cmd = json.loads(cmdstr)
+
+            if self.verbose:
+                cmdstr = cmdstr.strip()
+                print(cmdstr)
+        except Exception as ex:
+            if debug:
+                print("Incomplete %s: %s" % ( self.sock2api, ex))
+            return
+
+        id = None
+        if "id" in cmd:
+            id = cmd["id"]
+            del cmd["id"]
+
+        if cmd.get("execute", "") == "qmp_capabilities":
+            resobj = {
+                "return": {},
+            }
+            resfds = []
+        else:
+            if hasattr(libvirt_qemu, "qemuMonitorCommandWithFiles"):
+                res, resfds = libvirt_qemu.qemuMonitorCommandWithFiles(
+                    self.dom, json.dumps(cmd), [f.fileno() for f in self.sock2apifds])
+                resobj = json.loads(res)
+            else:
+                if len(self.sock2apifds) > 0:
+                    raise Exception("FD passing not supported")
+                res = libvirt_qemu.qemuMonitorCommand(
+                    self.dom, json.dumps(cmd))
+                resfds = []
+                resobj = json.loads(res)
+
+        if "id" in resobj:
+            del resobj["id"]
+        if id is not None:
+            resobj["id"] = id
+
+        res = json.dumps(resobj)
+        if self.verbose:
+            print(res)
+        res += "\r\n"
+
+        self.sock2api = bytes([])
+        self.sock2apifds = []
+        self.api2sock += res.encode("utf-8")
+        self.api2sockfds = resfds
+
+
+    @staticmethod
+    def handle_client_io(watch, fd, events, self):
+        error = False
+        try:
+            if events & libvirt.VIR_EVENT_HANDLE_WRITABLE:
+                done = self.send_with_fds(self.api2sock, self.api2sockfds)
+                if done > 0:
+                    self.api2sock = self.api2sock[done:]
+                    self.api2sockfds = []
+            elif events & libvirt.VIR_EVENT_HANDLE_READABLE:
+                data, fds = self.recv_with_fds()
+                if len(data) == 0:
+                    error = True
+                else:
+                    self.sock2api += data
+                    if len(fds):
+                        self.sock2apifds += fds
+
+                    self.try_command()
+            else:
+                error = True
+        except Exception as e:
+            global debug
+            if debug:
+                print("%s: %s" % (sys.argv[0], str(e)))
+                print(traceback.format_exc())
+            error = True
+
+        if error:
+            self.remove_client()
+        else:
+            self.update_client_events()
+
+
+    @staticmethod
+    def handle_server_io(watch, fd, events, self):
+        if self.clientsock is None:
+            sock, addr = self.serversock.accept()
+            self.add_client(sock)
+        else:
+            self.update_server_events()
+
+
+def parse_commandline():
+    parser = argparse.ArgumentParser(description="Libvirt QMP proxy")
+    parser.add_argument("--connect", "-c",
+                        help="Libvirt QEMU driver connection URI")
+    parser.add_argument("--debug", "-d", action='store_true',
+                        help="Display debugging information")
+    parser.add_argument("--verbose", "-v", action='store_true',
+                        help="Display QMP traffic")
+    parser.add_argument("domain", metavar="DOMAIN",
+                        help="Libvirt guest domain ID/UUID/Name")
+    parser.add_argument("sockpath", metavar="QMP-SOCK-PATH",
+                        help="UNIX socket path for QMP server")
+
+    return parser.parse_args()
+
+
+def main():
+    args = parse_commandline()
+    global debug
+    debug = args.debug
+
+    if not debug:
+        libvirt.registerErrorHandler(lambda opaque, error: None, None)
+
+    libvirt.virEventRegisterDefaultImpl()
+
+    conn, dom = get_domain(args.connect, args.domain)
+
+    if conn.getType() != "QEMU":
+        raise Exception("QMP proxy requires a QEMU driver connection not %s" %
+                        conn.getType())
+
+    sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+    if os.path.exists(args.sockpath):
+        os.unlink(args.sockpath)
+    sock.bind(args.sockpath)
+    sock.listen(1)
+
+    proxy = QMPProxy(conn, dom, sock, args.verbose)
+
+    while True:
+        libvirt.virEventRunDefaultImpl()
+
+
+try:
+    main()
+    sys.exit(0)
+except Exception as e:
+    print("%s: %s" % (sys.argv[0], str(e)))
+    if debug:
+        print(traceback.format_exc())
+    sys.exit(1)
-- 
2.37.3

On Wed, Oct 05, 2022 at 11:51:05AM +0100, Daniel P. Berrangé wrote:
> Since this tool introduces a python dependency it is not desirable
> to include it in any of the existing RPMs in libvirt. This tool is
> also QEMU specific, so isn't appropriate to bundle with the generic
> tools. Thus a new RPM is introduced 'libvirt-clients-qemu', to
> contain additional QEMU specific tools, with extra external deps.

[...]

> +%package client-qemu
> +Summary: Additional client side utilities for QEMU
> +Requires: %{name}-libs = %{version}-%{release}
> +Requires: python3-libvirt >= %{version}-%{release}
> +
> +%description client-qemu
> +The additional client binaries are used to interact
> +with some QEMU specific features of libvirt.

Jumping into this a bit late O:-) but I'm wondering if this was the
right call, naming-wise?

It appears to me that the primary reason to want this (and now
virt-qemu-sev-validate) to be in a separate package is really the
dependency on Python, which we understandably don't want to become a
requirement for the existing libvirt-client package. So wouldn't
libvirt-client-python be a more accurate and future-proof name?

Suppose later we introduce a tool that's QEMU-specific but written in
C, or a tool that's not specific to any hypervisor driver but written
in Python. Where would those go?

-- 
Andrea Bolognani / Red Hat / Virtualization

On Mon, Nov 28, 2022 at 08:33:27AM -0800, Andrea Bolognani wrote:
> On Wed, Oct 05, 2022 at 11:51:05AM +0100, Daniel P. Berrangé wrote:
> > Since this tool introduces a python dependency it is not desirable
> > to include it in any of the existing RPMs in libvirt. This tool is
> > also QEMU specific, so isn't appropriate to bundle with the generic
> > tools. Thus a new RPM is introduced 'libvirt-clients-qemu', to
> > contain additional QEMU specific tools, with extra external deps.
> 
> [...]
> 
> > +%package client-qemu
> > +Summary: Additional client side utilities for QEMU
> > +Requires: %{name}-libs = %{version}-%{release}
> > +Requires: python3-libvirt >= %{version}-%{release}
> > +
> > +%description client-qemu
> > +The additional client binaries are used to interact
> > +with some QEMU specific features of libvirt.
> 
> Jumping into this a bit late O:-) but I'm wondering if this was the
> right call, naming-wise?
> 
> It appears to me that the primary reason to want this (and now
> virt-qemu-sev-validate) to be in a separate package is really the
> dependency on Python, which we understandably don't want to become a
> requirement for the existing libvirt-client package. So wouldn't
> libvirt-client-python be a more accurate and future-proof name?

That was one of the reasons, but the other was that these are just
written as QEMU specific tools, and I wanted to separate them from
the general purpose tools.

> Suppose later we introduce a tool that's QEMU-specific but written in
> C, or a tool that's not specific to any hypervisor driver but written
> in Python. Where would those go?

In libvirt-client-qemu and (a new) libvirt-client-python respectively

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On Tue, Dec 06, 2022 at 05:15:43PM +0000, Daniel P. Berrangé wrote:
> On Mon, Nov 28, 2022 at 08:33:27AM -0800, Andrea Bolognani wrote:
> > It appears to me that the primary reason to want this (and now
> > virt-qemu-sev-validate) to be in a separate package is really the
> > dependency on Python, which we understandably don't want to become a
> > requirement for the existing libvirt-client package. So wouldn't
> > libvirt-client-python be a more accurate and future-proof name?
>
> That was one of the reasons, but the other was that these are just
> written as QEMU specific tools, and I wanted to separate them from
> the general purpose tools.
>
> > Suppose later we introduce a tool that's QEMU-specific but written in
> > C, or a tool that's not specific to any hypervisor driver but written
> > in Python. Where would those go?
>
> In libvirt-client-qemu and (a new) libvirt-client-python respectively

Okay, I'll stick to these names in Debian then. Thanks!

-- 
Andrea Bolognani / Red Hat / Virtualization

On Mon, Nov 28, 2022 at 08:33:27AM -0800, Andrea Bolognani wrote:
> On Wed, Oct 05, 2022 at 11:51:05AM +0100, Daniel P. Berrangé wrote:
> > Since this tool introduces a python dependency it is not desirable
> > to include it in any of the existing RPMs in libvirt. This tool is
> > also QEMU specific, so isn't appropriate to bundle with the generic
> > tools. Thus a new RPM is introduced 'libvirt-clients-qemu', to
> > contain additional QEMU specific tools, with extra external deps.
>
> [...]
>
> > +%package client-qemu
> > +Summary: Additional client side utilities for QEMU
> > +Requires: %{name}-libs = %{version}-%{release}
> > +Requires: python3-libvirt >= %{version}-%{release}
> > +
> > +%description client-qemu
> > +The additional client binaries are used to interact
> > +with some QEMU specific features of libvirt.
>
> Jumping into this a bit late O:-) but I'm wondering if this was the
> right call, naming-wise?
>
> It appears to me that the primary reason to want this (and now
> virt-qemu-sev-validate) to be in a separate package is really the
> dependency on Python, which we understandably don't want to become a
> requirement for the existing libvirt-client package. So wouldn't
> libvirt-client-python be a more accurate and future-proof name?
>
> Suppose later we introduce a tool that's QEMU-specific but written in
> C, or a tool that's not specific to any hypervisor driver but written
> in Python. Where would those go?

Any thoughts on this?

I'm going to introduce the new binary package in Debian soon, and
while I would prefer to match upstream's names as much as possible
I'm still not fully convinced by the choice made here.

-- 
Andrea Bolognani / Red Hat / Virtualization

On 10/5/22 12:51, Daniel P. Berrangé wrote:
> Libvirt provides QMP passthrough APIs for the QEMU driver and these are
> exposed in virsh. It is not especially pleasant, however, using the raw
> QMP JSON syntax. QEMU has a tool 'qmp-shell' which can speak QMP and
> exposes a human friendly interactive shell. It is not possible to use
> this with libvirt managed guest, however, since only one client can
> attach to the QMP socket at any point in time. While it would be
> possible to configure a second QMP socket for a VM, it may not be
> an known requirement at the time the guest is provisioned.
> 
> The virt-qmp-proxy tool aims to solve this problem. It opens a UNIX
> socket and listens for incoming client connections, speaking QMP on
> the connected socket. It will forward any QMP commands received onto
> the running libvirt QEMU guest, and forward any replies back to the
> QMP client. It will also forward back events.
> 
>   $ virsh start demo
>   $ virt-qmp-proxy demo demo.qmp &
>   $ qmp-shell demo.qmp
>   Welcome to the QMP low-level shell!
>   Connected to QEMU 6.2.0
> 
>   (QEMU) query-kvm
>   {
>       "return": {
>           "enabled": true,
>           "present": true
>       }
>   }
> 
> Note this tool of course has the same risks as the raw libvirt
> QMP passthrough. It is safe to run query commands to fetch information
> but commands which change the QEMU state risk disrupting libvirt's
> management of QEMU, potentially resulting in data loss/corruption in
> the worst case. Any use of this tool will cause the guest to be marked
> as tainted as an warning that it could be in an unexpected state.
> 
> Since this tool introduces a python dependency it is not desirable
> to include it in any of the existing RPMs in libvirt. This tool is
> also QEMU specific, so isn't appropriate to bundle with the generic
> tools. Thus a new RPM is introduced 'libvirt-clients-qemu', to
> contain additional QEMU specific tools, with extra external deps.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> 
> In v3:
> 
>   - Added to libvirt-clients-qemu RPM
>   - Renamed to virt-qemu-qmp-proxy
> 
>  docs/manpages/meson.build             |   1 +
>  docs/manpages/virt-qemu-qmp-proxy.rst | 120 +++++++++
>  libvirt.spec.in                       |  15 ++
>  tools/meson.build                     |   5 +
>  tools/virt-qemu-qmp-proxy             | 360 ++++++++++++++++++++++++++
>  5 files changed, 501 insertions(+)
>  create mode 100644 docs/manpages/virt-qemu-qmp-proxy.rst
>  create mode 100755 tools/virt-qemu-qmp-proxy
> 

> diff --git a/tools/virt-qemu-qmp-proxy b/tools/virt-qemu-qmp-proxy
> new file mode 100755
> index 0000000000..d85342bd2b
> --- /dev/null
> +++ b/tools/virt-qemu-qmp-proxy

> +    @staticmethod
> +    def make_file(fd):
> +        flags = fcntl.fcntl(fd, fcntl.F_GETFL)
> +
> +        mask = os.O_RDONLY | os.O_WRONLY | os.O_RDWR | os.O_APPEND
> +        flags = flags & mask
> +        mode = ""
> +        if flags == os.O_RDONLY:
> +            mode = "rb"
> +        elif flags == os.O_WRONLY:
> +            mode = "wb"
> +        elif flags == os.O_RDWR:
> +            mode = "r+b"
> +        elif flags == (os.O_WRONLY | os.O_APPEND):
> +            mode = "ab"
> +        elif flags == (os.O_RDWR | os.O_APPEND):
> +            mode = "a+b"
> +
> +        return os.fdopen(fd, mode)


This upsets syntax-check. Squash this in:

diff --git i/build-aux/syntax-check.mk w/build-aux/syntax-check.mk
index 649eb91acb..e35c2be734 100644
--- i/build-aux/syntax-check.mk
+++ w/build-aux/syntax-check.mk
@@ -1363,7 +1363,7 @@ exclude_file_name_regexp--sc_prohibit_strdup = \
   ^(docs/|examples/|tests/virnetserverclientmock.c|tests/commandhelper.c|tools/nss/libvirt_nss_(leases|macs)\.c$$)
 
 exclude_file_name_regexp--sc_prohibit_close = \
-  (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/vir(file|event)\.c|src/libvirt-stream\.c|tests/(vir.+mock\.c|commandhelper\.c|qemusecuritymock\.c)|tools/nss/libvirt_nss_(leases|macs)\.c)$$)
+  (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/vir(file|event)\.c|src/libvirt-stream\.c|tests/(vir.+mock\.c|commandhelper\.c|qemusecuritymock\.c)|tools/nss/libvirt_nss_(leases|macs)\.c)|tools/virt-qemu-qmp-proxy$$)
 
 exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF = \
   (^tests/(nodedevmdevctl|virhostcpu|virpcitest|virstoragetest)data/|docs/js/.*\.js|docs/fonts/.*\.woff|\.diff|tests/virconfdata/no-newline\.conf$$)

(or add .py suffix, but I dislike that actually).

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal

On Fri, Oct 07, 2022 at 03:22:33PM +0200, Michal Prívozník wrote:
> On 10/5/22 12:51, Daniel P. Berrangé wrote:
> > Libvirt provides QMP passthrough APIs for the QEMU driver and these are
> > exposed in virsh. It is not especially pleasant, however, using the raw
> > QMP JSON syntax. QEMU has a tool 'qmp-shell' which can speak QMP and
> > exposes a human friendly interactive shell. It is not possible to use
> > this with libvirt managed guest, however, since only one client can
> > attach to the QMP socket at any point in time. While it would be
> > possible to configure a second QMP socket for a VM, it may not be
> > an known requirement at the time the guest is provisioned.
> > 
> > The virt-qmp-proxy tool aims to solve this problem. It opens a UNIX
> > socket and listens for incoming client connections, speaking QMP on
> > the connected socket. It will forward any QMP commands received onto
> > the running libvirt QEMU guest, and forward any replies back to the
> > QMP client. It will also forward back events.
> > 
> >   $ virsh start demo
> >   $ virt-qmp-proxy demo demo.qmp &
> >   $ qmp-shell demo.qmp
> >   Welcome to the QMP low-level shell!
> >   Connected to QEMU 6.2.0
> > 
> >   (QEMU) query-kvm
> >   {
> >       "return": {
> >           "enabled": true,
> >           "present": true
> >       }
> >   }
> > 
> > Note this tool of course has the same risks as the raw libvirt
> > QMP passthrough. It is safe to run query commands to fetch information
> > but commands which change the QEMU state risk disrupting libvirt's
> > management of QEMU, potentially resulting in data loss/corruption in
> > the worst case. Any use of this tool will cause the guest to be marked
> > as tainted as an warning that it could be in an unexpected state.
> > 
> > Since this tool introduces a python dependency it is not desirable
> > to include it in any of the existing RPMs in libvirt. This tool is
> > also QEMU specific, so isn't appropriate to bundle with the generic
> > tools. Thus a new RPM is introduced 'libvirt-clients-qemu', to
> > contain additional QEMU specific tools, with extra external deps.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > 
> > In v3:
> > 
> >   - Added to libvirt-clients-qemu RPM
> >   - Renamed to virt-qemu-qmp-proxy
> > 
> >  docs/manpages/meson.build             |   1 +
> >  docs/manpages/virt-qemu-qmp-proxy.rst | 120 +++++++++
> >  libvirt.spec.in                       |  15 ++
> >  tools/meson.build                     |   5 +
> >  tools/virt-qemu-qmp-proxy             | 360 ++++++++++++++++++++++++++
> >  5 files changed, 501 insertions(+)
> >  create mode 100644 docs/manpages/virt-qemu-qmp-proxy.rst
> >  create mode 100755 tools/virt-qemu-qmp-proxy
> > 
> 
> > diff --git a/tools/virt-qemu-qmp-proxy b/tools/virt-qemu-qmp-proxy
> > new file mode 100755
> > index 0000000000..d85342bd2b
> > --- /dev/null
> > +++ b/tools/virt-qemu-qmp-proxy
> 
> > +    @staticmethod
> > +    def make_file(fd):
> > +        flags = fcntl.fcntl(fd, fcntl.F_GETFL)
> > +
> > +        mask = os.O_RDONLY | os.O_WRONLY | os.O_RDWR | os.O_APPEND
> > +        flags = flags & mask
> > +        mode = ""
> > +        if flags == os.O_RDONLY:
> > +            mode = "rb"
> > +        elif flags == os.O_WRONLY:
> > +            mode = "wb"
> > +        elif flags == os.O_RDWR:
> > +            mode = "r+b"
> > +        elif flags == (os.O_WRONLY | os.O_APPEND):
> > +            mode = "ab"
> > +        elif flags == (os.O_RDWR | os.O_APPEND):
> > +            mode = "a+b"
> > +
> > +        return os.fdopen(fd, mode)
> 
> 
> This upsets syntax-check. Squash this in:

For that matter 'flake8' hates my whitespace, so I'll fix that too

> (or add .py suffix, but I dislike that actually).

Yeah, likewise

> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|