date:20221020

[libvirt PATCH v3 12/18] qemu: include nbdkit state in private xml

2022-10-20 Thread Jonathon Jongsma

Add xml to the private data for a disk source to represent the nbdkit
process so that the state can be re-created if the libvirt daemon is
restarted. Format:

   
 /path/to/nbdkit.pid
 /path/to/nbdkit.socket
   

Signed-off-by: Jonathon Jongsma 
---
 src/qemu/qemu_domain.c | 53 ++
 src/qemu/qemu_nbdkit.c | 21 +
 src/qemu/qemu_nbdkit.h |  5 
 3 files changed, 79 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ace7ae4c61..2ae87392cb 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1849,6 +1849,34 @@ 
qemuStorageSourcePrivateDataAssignSecinfo(qemuDomainSecretInfo **secinfo,
 }
 
 
+static int
+qemuStorageSourcePrivateDataParseNbdkit(xmlNodePtr node,
+xmlXPathContextPtr ctxt,
+virStorageSource *src)
+{
+qemuDomainStorageSourcePrivate *srcpriv = 
qemuDomainStorageSourcePrivateFetch(src);
+g_autofree char *pidfile = NULL;
+g_autofree char *socketfile = NULL;
+VIR_XPATH_NODE_AUTORESTORE(ctxt);
+
+if (srcpriv->nbdkitProcess)
+return 0;
+
+ctxt->node = node;
+
+if (!(pidfile = virXPathString("string(./pidfile)", ctxt)))
+return -1;
+
+if (!(socketfile = virXPathString("string(./socketfile)", ctxt)))
+return -1;
+
+if (!(srcpriv->nbdkitProcess = qemuNbdkitProcessLoad(src, pidfile, 
socketfile)))
+return -1;
+
+return 0;
+}
+
+
 static int
 qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt,
   virStorageSource *src)
@@ -1859,6 +1887,7 @@ qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt,
 g_autofree char *httpcookiealias = NULL;
 g_autofree char *tlskeyalias = NULL;
 g_autofree char *thresholdEventWithIndex = NULL;
+xmlNodePtr nbdkitnode = NULL;
 
 src->nodestorage = 
virXPathString("string(./nodenames/nodename[@type='storage']/@name)", ctxt);
 src->nodeformat = 
virXPathString("string(./nodenames/nodename[@type='format']/@name)", ctxt);
@@ -1902,6 +1931,10 @@ qemuStorageSourcePrivateDataParse(xmlXPathContextPtr 
ctxt,
 virTristateBoolTypeFromString(thresholdEventWithIndex) == 
VIR_TRISTATE_BOOL_YES)
 src->thresholdEventWithIndex = true;
 
+if ((nbdkitnode = virXPathNode("nbdkit", ctxt))) {
+if (qemuStorageSourcePrivateDataParseNbdkit(nbdkitnode, ctxt, src) < 0)
+return -1;
+}
 return 0;
 }
 
@@ -1919,6 +1952,23 @@ qemuStorageSourcePrivateDataFormatSecinfo(virBuffer *buf,
 }
 
 
+static void
+qemuStorageSourcePrivateDataFormatNbdkit(qemuNbdkitProcess *nbdkit,
+ virBuffer *buf)
+{
+g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf);
+
+if (!nbdkit)
+return;
+
+virBufferEscapeString(, "%s\n",
+  nbdkit->pidfile);
+virBufferEscapeString(, "%s\n",
+  nbdkit->socketfile);
+virXMLFormatElement(buf, "nbdkit", NULL, );
+}
+
+
 static int
 qemuStorageSourcePrivateDataFormat(virStorageSource *src,
virBuffer *buf)
@@ -1957,6 +2007,9 @@ qemuStorageSourcePrivateDataFormat(virStorageSource *src,
 if (src->thresholdEventWithIndex)
 virBufferAddLit(buf, "\n");
 
+if (srcPriv)
+qemuStorageSourcePrivateDataFormatNbdkit(srcPriv->nbdkitProcess, buf);
+
 return 0;
 }
 
diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index 59c452a15e..5b47a15112 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -597,6 +597,27 @@ qemuNbdkitProcessNew(virStorageSource *source,
 }
 
 
+qemuNbdkitProcess *
+qemuNbdkitProcessLoad(virStorageSource *source,
+  const char *pidfile,
+  const char *socketfile)
+{
+int rc;
+g_autoptr(qemuNbdkitProcess) nbdkit = qemuNbdkitProcessNew(source, 
pidfile, socketfile);
+
+if ((rc = virPidFileReadPath(nbdkit->pidfile, >pid)) < 0)
+return NULL;
+
+if (virProcessKill(nbdkit->pid, 0) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("nbdkit process %i is not alive"), nbdkit->pid);
+return NULL;
+}
+
+return g_steal_pointer();
+}
+
+
 bool
 qemuNbdkitInitStorageSource(qemuNbdkitCaps *caps,
 virStorageSource *source,
diff --git a/src/qemu/qemu_nbdkit.h b/src/qemu/qemu_nbdkit.h
index 30cab744b0..ca7f1dcf31 100644
--- a/src/qemu/qemu_nbdkit.h
+++ b/src/qemu/qemu_nbdkit.h
@@ -89,4 +89,9 @@ qemuNbdkitProcessStop(qemuNbdkitProcess *proc);
 void
 qemuNbdkitProcessFree(qemuNbdkitProcess *proc);
 
+qemuNbdkitProcess *
+qemuNbdkitProcessLoad(virStorageSource *source,
+  const char *pidfile,
+  const char *socketfile);
+
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuNbdkitProcess, qemuNbdkitProcessFree);
-- 
2.37.3

[libvirt PATCH v3 00/18] Use nbdkit for http/ftp/ssh network drives in libvirt

2022-10-20 Thread Jonathon Jongsma

This is the third version of this patch series. See
https://bugzilla.redhat.com/show_bug.cgi?id=2016527 for more information about
the goal, but the summary is that RHEL does not want to ship the qemu storage
plugins for curl and ssh.  Handling them outside of the qemu process provides
several advantages such as reduced attack surface and stability.

A quick summary of the code:
 - at startup I query to see whether nbdkit exists on the host and if
   so, I query which plugins/filters are installed. These capabilities
   are cached and stored in the qemu driver
 - When the driver prepares the domain, we go through each disk source
   and determine whether the nbdkit capabilities allow us to support
   this disk via nbdkit, and if so, we allocate a qemuNbdkitProcess
   object and stash it in the private data of the virStorageSource.
 - The presence or absence of this qemuNbdkitProcess data then indicates
   whether this disk will be served to qemu indirectly via nbdkit or
   directly
 - When we launch the qemuProcess, as part of the "external device
   start" step, I launch a ndkit process for each disk that is supported
   by nbdkit.
 - for devices which are served by an intermediate ndkit process, I
   change the qemu commandline in the following ways:
   - I no longer pass auth/cookie secrets to qemu (those are handled by
 nbdkit)
   - I replace the actual network URL of the remote disk source with the
 path to the nbdkit unix socket
 - We create a 'monitor' for the nbdkit process that watches to see whether the
   process exits. If it does, we pause the domain, attempt to restart nbdkit,
   and then resume the domain.

Open questions
 - I think selinux will work once we add a policy for the /usr/sbin/nbdkit
   binary to allow it to be executed by libvirt, but for now it fails to
   execute nbdkit in enforcing mode. The current context for nbdkit (on fedora)
   is "system_u:object_r:bin_t:s0". When I temporarily change the context to
   something like qemu_exec_t, I am able to start nbdkit and the domain
   launches.

Known shortcomings
 - creating disks (in ssh) still isn't supported.

Changes in v3:
 - Various formatting fixes
 - Don't kill process in qemuNbdkitProcessFree() since we want the nbdkit
   daemon to continue running even if libvirt restarts.
 - Better detection and error reporting when starting the nbdkit process
 - Add monitoring for nbdkit process so that it can be restarted if if ever
   exits unexpectedly.

Changes in v2:
 - split into multiple patches
 - added a build option for nbdkit_moddir
 - don't instantiate any secret / cookie props for disks that are being served
   by nbdkit since we don't send secrets to qemu anymore
 - ensure that nbdkit processes are started/stopped for the entire backing
   chain
 - switch to virFileCache-based capabilities for nbdkit so that we don't need
   to requery every time
 - switch to using pipes for communicating sensitive data to nbdkit
 - use pidfile support built into virCommand rather than nbdkit's --pidfile
   argument
 - added significantly more tests

Jonathon Jongsma (18):
  schema: allow 'ssh' as a protocol for network disks
  qemu: Add functions for determining nbdkit availability
  qemu: expand nbdkit capabilities
  util: Allow virFileCache data to be any GObject
  qemu: implement basic virFileCache for nbdkit caps
  qemu: implement persistent file cache for nbdkit caps
  qemu: use file cache for nbdkit caps
  qemu: Add qemuNbdkitProcess
  qemu: add functions to start and stop nbdkit
  tests: add ability to test various nbdkit capabilities
  qemu: split qemuDomainSecretStorageSourcePrepare
  qemu: include nbdkit state in private xml
  qemu: use nbdkit to serve network disks if available
  tests: add tests for nbdkit invocation
  util: make virCommandSetSendBuffer testable
  qemu: pass sensitive data to nbdkit via pipe
  qemu: add test for authenticating a https network disk
  qemu: Monitor nbdkit process for exit

 build-aux/syntax-check.mk |4 +-
 meson.build   |9 +
 meson_options.txt |1 +
 po/POTFILES   |1 +
 src/conf/schemas/domaincommon.rng |1 +
 src/libvirt_private.syms  |1 +
 src/qemu/meson.build  |1 +
 src/qemu/qemu_block.c |  162 ++-
 src/qemu/qemu_conf.c  |   23 +
 src/qemu/qemu_conf.h  |7 +
 src/qemu/qemu_domain.c|  180 ++-
 src/qemu/qemu_domain.h|4 +
 src/qemu/qemu_driver.c|4 +
 src/qemu/qemu_extdevice.c |   48 +
 src/qemu/qemu_nbdkit.c| 1243 +
 src/qemu/qemu_nbdkit.h|  120 ++
 src/qemu/qemu_nbdkitpriv.h|   31 +
 src/qemu/qemu_process.c   |   13 +

[libvirt PATCH v3 07/18] qemu: use file cache for nbdkit caps

2022-10-20 Thread Jonathon Jongsma

Add the virFileCache implementation for nbdkit capabilities to the qemu
driver. This allows us to determine whether nbdkit is installed and
which plugins are supported. it also has persistent caching and the
capabilities are re-queried whenever something changes.

Signed-off-by: Jonathon Jongsma 
---
 src/qemu/qemu_conf.h   | 3 +++
 src/qemu/qemu_driver.c | 4 
 2 files changed, 7 insertions(+)

diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index a39510b0b1..95c05e6888 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -320,6 +320,9 @@ struct _virQEMUDriver {
 
 /* Immutable pointer, self-locking APIs */
 virHashAtomic *migrationErrors;
+
+/* Immutable pointer, self-locking APIs */
+virFileCache *nbdkitCapsCache;
 };
 
 virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 59a3b37b98..d42f3c2d2a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -833,6 +833,9 @@ qemuStateInitialize(bool privileged,
defsecmodel)))
 goto error;
 
+/* find whether nbdkit is available and query its capabilities */
+qemu_driver->nbdkitCapsCache = qemuNbdkitCapsCacheNew(cfg->cacheDir);
+
 /* If hugetlbfs is present, then we need to create a sub-directory within
  * it, since we can't assume the root mount point has permissions that
  * will let our spawned QEMU instances use it. */
@@ -1070,6 +1073,7 @@ qemuStateCleanup(void)
 VIR_FREE(qemu_driver->qemuImgBinary);
 virObjectUnref(qemu_driver->domains);
 virThreadPoolFree(qemu_driver->workerPool);
+virObjectUnref(qemu_driver->nbdkitCapsCache);
 
 if (qemu_driver->lockFD != -1)
 virPidFileRelease(qemu_driver->config->stateDir, "driver", 
qemu_driver->lockFD);
-- 
2.37.3

[libvirt PATCH v3 16/18] qemu: pass sensitive data to nbdkit via pipe

2022-10-20 Thread Jonathon Jongsma

Rather than passing passwords and cookies (which could contain
passwords) to nbdkit via commandline arguments, use the alternate format
that nbdkit supports where we can specify a file descriptor which nbdkit
will read to get the password or cookies.

Signed-off-by: Jonathon Jongsma 
---
 build-aux/syntax-check.mk |  4 +-
 src/qemu/qemu_nbdkit.c| 64 -
 src/util/vircommand.c |  3 +-
 src/util/virutil.h|  2 +-
 .../disk-cdrom-network.args.disk0 |  7 ++
 .../disk-cdrom-network.args.disk1 |  9 +++
 .../disk-cdrom-network.args.disk1.pipe.1778   |  1 +
 .../disk-cdrom-network.args.disk2 |  9 +++
 .../disk-cdrom-network.args.disk2.pipe.1780   |  1 +
 .../disk-network-http.args.disk0  |  7 ++
 .../disk-network-http.args.disk1  |  6 ++
 .../disk-network-http.args.disk2  |  7 ++
 .../disk-network-http.args.disk2.pipe.1778|  1 +
 .../disk-network-http.args.disk3  |  8 +++
 .../disk-network-http.args.disk3.pipe.1780|  1 +
 ...work-source-curl-nbdkit-backing.args.disk0 |  8 +++
 ...e-curl-nbdkit-backing.args.disk0.pipe.1778 |  1 +
 .../disk-network-source-curl.args.disk0   |  8 +++
 ...k-network-source-curl.args.disk0.pipe.1778 |  1 +
 .../disk-network-source-curl.args.disk1   |  8 +++
 ...k-network-source-curl.args.disk1.pipe.1780 |  1 +
 .../disk-network-source-curl.args.disk2   |  8 +++
 ...k-network-source-curl.args.disk2.pipe.1782 |  1 +
 .../disk-network-source-curl.args.disk3   |  7 ++
 .../disk-network-source-curl.args.disk4   |  7 ++
 tests/qemunbdkittest.c| 69 +--
 26 files changed, 219 insertions(+), 30 deletions(-)
 create mode 100644 tests/qemunbdkitdata/disk-cdrom-network.args.disk0
 create mode 100644 tests/qemunbdkitdata/disk-cdrom-network.args.disk1
 create mode 100644 tests/qemunbdkitdata/disk-cdrom-network.args.disk1.pipe.1778
 create mode 100644 tests/qemunbdkitdata/disk-cdrom-network.args.disk2
 create mode 100644 tests/qemunbdkitdata/disk-cdrom-network.args.disk2.pipe.1780
 create mode 100644 tests/qemunbdkitdata/disk-network-http.args.disk0
 create mode 100644 tests/qemunbdkitdata/disk-network-http.args.disk1
 create mode 100644 tests/qemunbdkitdata/disk-network-http.args.disk2
 create mode 100644 tests/qemunbdkitdata/disk-network-http.args.disk2.pipe.1778
 create mode 100644 tests/qemunbdkitdata/disk-network-http.args.disk3
 create mode 100644 tests/qemunbdkitdata/disk-network-http.args.disk3.pipe.1780
 create mode 100644 
tests/qemunbdkitdata/disk-network-source-curl-nbdkit-backing.args.disk0
 create mode 100644 
tests/qemunbdkitdata/disk-network-source-curl-nbdkit-backing.args.disk0.pipe.1778
 create mode 100644 tests/qemunbdkitdata/disk-network-source-curl.args.disk0
 create mode 100644 
tests/qemunbdkitdata/disk-network-source-curl.args.disk0.pipe.1778
 create mode 100644 tests/qemunbdkitdata/disk-network-source-curl.args.disk1
 create mode 100644 
tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.1780
 create mode 100644 tests/qemunbdkitdata/disk-network-source-curl.args.disk2
 create mode 100644 
tests/qemunbdkitdata/disk-network-source-curl.args.disk2.pipe.1782
 create mode 100644 tests/qemunbdkitdata/disk-network-source-curl.args.disk3
 create mode 100644 tests/qemunbdkitdata/disk-network-source-curl.args.disk4

diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index 68cd9dff5f..d44b1e5b17 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -1355,10 +1355,10 @@ 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)|tools/virt-qemu-qmp-proxy$$)
+  
(\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/vir(file|event)\.c|src/libvirt-stream\.c|tests/(vir.+mock\.c|commandhelper\.c|qemusecuritymock\.c|qemunbdkittest\.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$$)
+  
(^tests/(nodedevmdevctl|virhostcpu|virpcitest|virstoragetest|qemunbdkit)data/|docs/js/.*\.js|docs/fonts/.*\.woff|\.diff|tests/virconfdata/no-newline\.conf$$)
 
 exclude_file_name_regexp--sc_prohibit_fork_wrappers = \
   
(^(src/(util/(vircommand|virdaemon)|lxc/lxc_controller)|tests/testutils)\.c$$)
diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index 882a074211..0a0dc5d2a4 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@

[libvirt PATCH v3 18/18] qemu: Monitor nbdkit process for exit

2022-10-20 Thread Jonathon Jongsma

Adds the ability to monitor the nbdkit process so that we can take
action in case the child exits unexpectedly.

When the nbdkit process exits, we pause the vm, restart nbdkit, and then
resume the vm. This allows the vm to continue working in the event of a
nbdkit failure.

Eventually we may want to generalize this functionality since we may
need something similar for e.g. qemu-storage-daemon, etc.

The process is monitored with the pidfd_open() syscall if it exists
(since linux 5.3). Otherwise it resorts to checking whether the process
is alive once a second. The one-second time period was chosen somewhat
arbitrarily.

Signed-off-by: Jonathon Jongsma 
---
 meson.build |   3 +
 src/qemu/qemu_nbdkit.c  | 220 
 src/qemu/qemu_nbdkit.h  |  10 ++
 src/qemu/qemu_process.c |  13 +++
 4 files changed, 246 insertions(+)

diff --git a/meson.build b/meson.build
index e4581e74dd..b4ed170ca1 100644
--- a/meson.build
+++ b/meson.build
@@ -686,6 +686,9 @@ if host_machine.system() == 'linux'
 # Check if we have new enough kernel to support BPF devices for cgroups v2
 [ 'linux/bpf.h', 'BPF_PROG_QUERY' ],
 [ 'linux/bpf.h', 'BPF_CGROUP_DEVICE' ],
+
+# process management
+[ 'sys/syscall.h', 'SYS_pidfd_open' ],
   ]
 endif
 
diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index 0a0dc5d2a4..f17fe022ec 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -21,9 +21,11 @@
 
 #include 
 #include 
+#include 
 
 #include "vircommand.h"
 #include "virerror.h"
+#include "virevent.h"
 #include "virlog.h"
 #include "virpidfile.h"
 #include "virtime.h"
@@ -36,6 +38,7 @@
 #include "qemu_nbdkit.h"
 #define LIBVIRT_QEMU_NBDKITPRIV_H_ALLOW
 #include "qemu_nbdkitpriv.h"
+#include "qemu_process.h"
 #include "qemu_security.h"
 
 #include 
@@ -72,6 +75,13 @@ struct _qemuNbdkitCaps {
 G_DEFINE_TYPE(qemuNbdkitCaps, qemu_nbdkit_caps, G_TYPE_OBJECT);
 
 
+struct _qemuNbdkitProcessPrivate {
+int monitor;
+virQEMUDriver *driver;
+virDomainObj *vm;
+};
+
+
 enum {
 PIPE_FD_READ = 0,
 PIPE_FD_WRITE = 1
@@ -588,6 +598,168 @@ qemuNbdkitCapsCacheNew(const char *cachedir)
 }
 
 
+static int
+qemuNbdkitProcessStartMonitor(qemuNbdkitProcess *proc,
+  virDomainObj *vm,
+  virQEMUDriver *driver);
+
+
+static void
+qemuNbdkitProcessHandleExit(qemuNbdkitProcess *proc)
+{
+qemuNbdkitProcessPrivate *priv = proc->priv;
+bool was_running = false;
+
+VIR_DEBUG("nbdkit process %i died", proc->pid);
+
+/* clean up resources associated with process */
+qemuNbdkitProcessStop(proc);
+
+if (!(priv->vm && priv->driver)) {
+VIR_WARN("Unable to restart nbdkit -- vm and driver not set");
+return;
+}
+
+VIR_DEBUG("restarting nbdkit process");
+
+virObjectLock(priv->vm);
+if (virDomainObjBeginJob(priv->vm, VIR_JOB_SUSPEND) < 0) {
+VIR_WARN("can't begin job");
+goto cleanup;
+}
+
+/* Pause domain */
+if (virDomainObjGetState(priv->vm, NULL) == VIR_DOMAIN_RUNNING) {
+was_running = true;
+if (qemuProcessStopCPUs(priv->driver, priv->vm,
+VIR_DOMAIN_PAUSED_IOERROR,
+VIR_ASYNC_JOB_NONE) < 0)
+goto endjob;
+VIR_DEBUG("Paused vm while we restart nbdkit backend");
+}
+
+if (qemuNbdkitProcessStart(proc, priv->vm, priv->driver) < 0)
+VIR_WARN("Unable to restart nbkdit process");
+
+if (was_running && virDomainObjIsActive(priv->vm)) {
+if (qemuProcessStartCPUs(priv->driver, priv->vm,
+ VIR_DOMAIN_RUNNING_UNPAUSED,
+ VIR_ASYNC_JOB_NONE) < 0) {
+VIR_WARN("Unable to resume guest CPUs after nbdkit restart");
+goto endjob;
+}
+VIR_DEBUG("Resumed vm");
+}
+qemuNbdkitProcessStartMonitor(proc, NULL, NULL);
+
+ endjob:
+virDomainObjEndJob(priv->vm);
+ cleanup:
+virObjectUnlock(priv->vm);
+}
+
+
+#if WITH_DECL_SYS_PIDFD_OPEN
+static void
+qemuNbdkitProcessPidfdCb(int watch G_GNUC_UNUSED,
+ int fd,
+ int events G_GNUC_UNUSED,
+ void *opaque)
+{
+qemuNbdkitProcess *proc = opaque;
+
+VIR_FORCE_CLOSE(fd);
+qemuNbdkitProcessHandleExit(proc);
+}
+#else
+static void
+qemuNbdkitProcessTimeoutCb(int timer G_GNUC_UNUSED,
+   void *opaque)
+{
+qemuNbdkitProcess *proc = opaque;
+
+if (virProcessKill(proc->pid, 0) < 0)
+qemuNbdkitProcessHandleExit(proc);
+}
+#endif /* WITH_DECL_SYS_PIDFD_OPEN */
+
+
+static int
+qemuNbdkitProcessStartMonitor(qemuNbdkitProcess *proc,
+  virDomainObj *vm,
+  virQEMUDriver *driver)
+{
+qemuNbdkitProcessPrivate *priv = proc->priv;
+#if WITH_DECL_SYS_PIDFD_OPEN
+int pidfd;
+#endif
+
+if (vm) {
+

[libvirt PATCH v3 17/18] qemu: add test for authenticating a https network disk

2022-10-20 Thread Jonathon Jongsma

Signed-off-by: Jonathon Jongsma 
---
 tests/qemunbdkitdata/disk-network-source-curl.args.1.pipe.1   | 1 +
 tests/qemunbdkitdata/disk-network-source-curl.args.disk1  | 4 +++-
 .../disk-network-source-curl.args.disk1.pipe.1780 | 2 +-
 .../disk-network-source-curl.args.disk1.pipe.1782 | 1 +
 .../disk-network-source-curl.args.disk1.pipe.49   | 1 +
 tests/qemunbdkitdata/disk-network-source-curl.args.disk2  | 2 +-
 .../disk-network-source-curl.args.disk2.pipe.1784 | 1 +
 .../disk-network-source-curl.args.disk2.pipe.51   | 1 +
 .../disk-network-source-curl.x86_64-latest.args   | 3 ++-
 tests/qemuxml2argvdata/disk-network-source-curl.xml   | 3 +++
 10 files changed, 15 insertions(+), 4 deletions(-)
 create mode 100644 tests/qemunbdkitdata/disk-network-source-curl.args.1.pipe.1
 create mode 100644 
tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.1782
 create mode 100644 
tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.49
 create mode 100644 
tests/qemunbdkitdata/disk-network-source-curl.args.disk2.pipe.1784
 create mode 100644 
tests/qemunbdkitdata/disk-network-source-curl.args.disk2.pipe.51

diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.1.pipe.1 
b/tests/qemunbdkitdata/disk-network-source-curl.args.1.pipe.1
new file mode 100644
index 00..20af4ae383
--- /dev/null
+++ b/tests/qemunbdkitdata/disk-network-source-curl.args.1.pipe.1
@@ -0,0 +1 @@
+cookie1=cookievalue1; cookie2=cookievalue2
\ No newline at end of file
diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk1 
b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1
index 13f03c545e..4b6eef8a86 100644
--- a/tests/qemunbdkitdata/disk-network-source-curl.args.disk1
+++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1
@@ -4,5 +4,7 @@ nbdkit \
 --foreground curl \
 protocols=https \
 'url=https://https.example.org:8443/path/to/disk5.iso?foo=bar' \
-cookie=-1779 \
+user=myname \
+password=-1779 \
+cookie=-1781 \
 sslverify=false
diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.1780 
b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.1780
index 20af4ae383..ccdd4033fc 100644
--- a/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.1780
+++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.1780
@@ -1 +1 @@
-cookie1=cookievalue1; cookie2=cookievalue2
\ No newline at end of file
+iscsi-mycluster_myname-secret
\ No newline at end of file
diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.1782 
b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.1782
new file mode 100644
index 00..20af4ae383
--- /dev/null
+++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.1782
@@ -0,0 +1 @@
+cookie1=cookievalue1; cookie2=cookievalue2
\ No newline at end of file
diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.49 
b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.49
new file mode 100644
index 00..20af4ae383
--- /dev/null
+++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.49
@@ -0,0 +1 @@
+cookie1=cookievalue1; cookie2=cookievalue2
\ No newline at end of file
diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk2 
b/tests/qemunbdkitdata/disk-network-source-curl.args.disk2
index 490aea3393..c950eaf6ae 100644
--- a/tests/qemunbdkitdata/disk-network-source-curl.args.disk2
+++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk2
@@ -5,4 +5,4 @@ nbdkit \
 --readonly curl \
 protocols=http \
 url=http://http.example.org:8080/path/to/disk2.iso \
-cookie=-1781
+cookie=-1783
diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk2.pipe.1784 
b/tests/qemunbdkitdata/disk-network-source-curl.args.disk2.pipe.1784
new file mode 100644
index 00..5c035e84c5
--- /dev/null
+++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk2.pipe.1784
@@ -0,0 +1 @@
+cookie1=cookievalue1; cookie2=cookievalue2; cookie3=cookievalue3
\ No newline at end of file
diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk2.pipe.51 
b/tests/qemunbdkitdata/disk-network-source-curl.args.disk2.pipe.51
new file mode 100644
index 00..5c035e84c5
--- /dev/null
+++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk2.pipe.51
@@ -0,0 +1 @@
+cookie1=cookievalue1; cookie2=cookievalue2; cookie3=cookievalue3
\ No newline at end of file
diff --git a/tests/qemuxml2argvdata/disk-network-source-curl.x86_64-latest.args 
b/tests/qemuxml2argvdata/disk-network-source-curl.x86_64-latest.args
index ec6dd13f6c..7f09e84227 100644
--- a/tests/qemuxml2argvdata/disk-network-source-curl.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/disk-network-source-curl.x86_64-latest.args
@@ -33,9 +33,10 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
 -blockdev

[libvirt PATCH v3 11/18] qemu: split qemuDomainSecretStorageSourcePrepare

2022-10-20 Thread Jonathon Jongsma

This prepares encryption secrets and authentication secrets. When we add
nbdkit-backed network storage sources, we will not need to send
authentication secrets to qemu, since they will be sent to nbdkit
instead. So split this into two different functions.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 src/qemu/qemu_domain.c | 83 ++
 1 file changed, 52 insertions(+), 31 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 3fca163415..ace7ae4c61 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1299,24 +1299,19 @@ 
qemuDomainSecretStorageSourcePrepareCookies(qemuDomainObjPrivate *priv,
 
 
 /**
- * qemuDomainSecretStorageSourcePrepare:
+ * qemuDomainSecretStorageSourcePrepareEncryption:
  * @priv: domain private object
  * @src: storage source struct to setup
- * @authalias: prefix of the alias for secret holding authentication data
- * @encalias: prefix of the alias for secret holding encryption password
+ * @alias: prefix of the alias for secret holding encryption password
  *
- * Prepares data necessary for encryption and authentication of @src. The two
- * alias prefixes are provided since in the backing chain authentication 
belongs
- * to the storage protocol data whereas encryption is relevant to the format
- * driver in qemu. The two will have different node names.
+ * Prepares data necessary for encryption of @src.
  *
  * Returns 0 on success; -1 on error while reporting an libvirt error.
  */
 static int
-qemuDomainSecretStorageSourcePrepare(qemuDomainObjPrivate *priv,
- virStorageSource *src,
- const char *aliasprotocol,
- const char *aliasformat)
+qemuDomainSecretStorageSourcePrepareEncryption(qemuDomainObjPrivate *priv,
+   virStorageSource *src,
+   const char *alias)
 {
 qemuDomainStorageSourcePrivate *srcPriv;
 bool hasEnc = src->encryption && src->encryption->nsecrets > 0;
@@ -1324,13 +1319,43 @@ 
qemuDomainSecretStorageSourcePrepare(qemuDomainObjPrivate *priv,
 if (virStorageSourceIsEmpty(src))
 return 0;
 
-if (!src->auth && !hasEnc && src->ncookies == 0)
+if (!hasEnc)
 return 0;
 
-if (!(src->privateData = qemuDomainStorageSourcePrivateNew()))
+srcPriv = qemuDomainStorageSourcePrivateFetch(src);
+
+if (!(srcPriv->encinfo = qemuDomainSecretInfoSetupFromSecret(priv, alias,
+ "encryption",
+ 
VIR_SECRET_USAGE_TYPE_VOLUME,
+ NULL,
+ 
>encryption->secrets[0]->seclookupdef)))
 return -1;
 
-srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
+return 0;
+}
+
+
+/**
+ * qemuDomainSecretStorageSourcePrepareAuth:
+ * @priv: domain private object
+ * @src: storage source struct to setup
+ * @alias: prefix of the alias for secret holding authentication data
+ *
+ * Prepares data necessary for authentication of @src.
+ *
+ * Returns 0 on success; -1 on error while reporting an libvirt error.
+ */
+static int
+qemuDomainSecretStorageSourcePrepareAuth(qemuDomainObjPrivate *priv,
+ virStorageSource *src,
+ const char *alias)
+{
+qemuDomainStorageSourcePrivate *srcPriv;
+
+if (virStorageSourceIsEmpty(src))
+return 0;
+
+srcPriv = qemuDomainStorageSourcePrivateFetch(src);
 
 if (src->auth) {
 virSecretUsageType usageType = VIR_SECRET_USAGE_TYPE_ISCSI;
@@ -1338,7 +1363,7 @@ qemuDomainSecretStorageSourcePrepare(qemuDomainObjPrivate 
*priv,
 if (src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)
 usageType = VIR_SECRET_USAGE_TYPE_CEPH;
 
-if (!(srcPriv->secinfo = qemuDomainSecretInfoSetupFromSecret(priv, 
aliasprotocol,
+if (!(srcPriv->secinfo = qemuDomainSecretInfoSetupFromSecret(priv, 
alias,
  "auth",
  usageType,
  
src->auth->username,
@@ -1346,19 +1371,10 @@ 
qemuDomainSecretStorageSourcePrepare(qemuDomainObjPrivate *priv,
 return -1;
 }
 
-if (hasEnc) {
-if (!(srcPriv->encinfo = qemuDomainSecretInfoSetupFromSecret(priv, 
aliasformat,
- 
"encryption",
- 
VIR_SECRET_USAGE_TYPE_VOLUME,
- NULL,
-

[libvirt PATCH v3 10/18] tests: add ability to test various nbdkit capabilities

2022-10-20 Thread Jonathon Jongsma

Add new DO_TEST_CAPS_LATEST_NBDKIT macro to test xml2argv for various
nbdkit capability scenarios.

Signed-off-by: Jonathon Jongsma 
---
 src/qemu/qemu_nbdkit.c   | 20 +---
 tests/qemuxml2argvtest.c | 11 +++
 tests/testutilsqemu.c| 27 +++
 tests/testutilsqemu.h|  5 +
 4 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index c5b0762f8d..59c452a15e 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -263,10 +263,16 @@ virNbkditCapsCheckModdir(const char *moddir,
 
 static bool
 virNbdkitCapsIsValid(void *data,
- void *privData G_GNUC_UNUSED)
+ void *privData)
 {
 qemuNbdkitCaps *nbdkitCaps = data;
 struct stat st;
+/* when run under test, we will use privData as a signal to indicate that
+ * we shouldn't touch the filesystem */
+bool skipValidation = (privData != NULL);
+
+if (skipValidation)
+return true;
 
 if (!nbdkitCaps->path)
 return true;
@@ -309,9 +315,17 @@ virNbdkitCapsIsValid(void *data,
 
 static void*
 virNbdkitCapsNewData(const char *binary,
- void *privData G_GNUC_UNUSED)
+ void *privData)
 {
-qemuNbdkitCaps *caps = qemuNbdkitCapsNew(binary);
+/* when run under test, we will use privData as a signal to indicate that
+ * we shouldn't touch the filesystem */
+bool skipNewData = (privData != NULL);
+qemuNbdkitCaps *caps = NULL;
+
+if (skipNewData)
+return NULL;
+
+caps = qemuNbdkitCapsNew(binary);
 qemuNbdkitCapsQuery(caps);
 
 return caps;
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index ef32cae2e9..0032bafdce 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -670,6 +670,14 @@ testCompareXMLToArgv(const void *data)
 if (rc < 0)
 goto cleanup;
 
+if (info->nbdkitCaps) {
+if (virFileCacheInsertData(driver.nbdkitCapsCache, TEST_NBDKIT_PATH,
+   g_object_ref(info->nbdkitCaps)) < 0) {
+g_object_unref(info->nbdkitCaps);
+goto cleanup;
+}
+}
+
 if (info->migrateFrom &&
 !(migrateURI = qemuMigrationDstGetURI(info->migrateFrom,
   info->migrateFd)))
@@ -927,6 +935,9 @@ mymain(void)
 # define DO_TEST_CAPS_ARCH_VER(name, arch, ver) \
 DO_TEST_CAPS_ARCH_VER_FULL(name, arch, ver, ARG_END)
 
+# define DO_TEST_CAPS_LATEST_NBDKIT(name, ...) \
+DO_TEST_CAPS_ARCH_LATEST_FULL(name, "x86_64", ARG_NBDKIT_CAPS, 
__VA_ARGS__, QEMU_NBDKIT_CAPS_LAST, ARG_END)
+
 # define DO_TEST_CAPS_LATEST(name) \
 DO_TEST_CAPS_ARCH_LATEST(name, "x86_64")
 
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 6d3decdc16..386042aa79 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -131,6 +131,10 @@ virFindFileInPath(const char *file)
 return g_strdup_printf("/usr/bin/%s", file);
 }
 
+if (g_str_equal(file, "nbdkit")) {
+return g_strdup(TEST_NBDKIT_PATH);
+}
+
 /* Nothing in tests should be relying on real files
  * in host OS, so we return NULL to try to force
  * an error in such a case
@@ -422,6 +426,7 @@ void qemuTestDriverFree(virQEMUDriver *driver)
 virObjectUnref(driver->caps);
 virObjectUnref(driver->config);
 virObjectUnref(driver->securityManager);
+g_clear_object(>nbdkitCapsCache);
 
 virCPUDefFree(cpuDefault);
 virCPUDefFree(cpuHaswell);
@@ -665,6 +670,12 @@ int qemuTestDriverInit(virQEMUDriver *driver)
 if (!driver->qemuCapsCache)
 goto error;
 
+driver->nbdkitCapsCache = qemuNbdkitCapsCacheNew("/dev/null");
+/* the nbdkitCapsCache just interprets the presence of a non-null private
+ * data pointer as a signal to skip cache validation. This prevents the
+ * cache from trying to validate the plugindir mtime, etc during test */
+virFileCacheSetPriv(driver->nbdkitCapsCache, GUINT_TO_POINTER(1));
+
 driver->xmlopt = virQEMUDriverCreateXMLConf(driver, "none");
 if (!driver->xmlopt)
 goto error;
@@ -885,6 +896,7 @@ testQemuInfoSetArgs(struct testQemuInfo *info,
 
 info->conf = conf;
 info->args.newargs = true;
+info->args.fakeNbdkitCaps = qemuNbdkitCapsNew(TEST_NBDKIT_PATH);
 
 va_start(argptr, conf);
 while ((argname = va_arg(argptr, testQemuInfoArgName)) != ARG_END) {
@@ -896,6 +908,13 @@ testQemuInfoSetArgs(struct testQemuInfo *info,
 virQEMUCapsSet(info->args.fakeCaps, flag);
 break;
 
+case ARG_NBDKIT_CAPS:
+info->args.fakeNbdkitCapsUsed = true;
+
+while ((flag = va_arg(argptr, int)) < QEMU_NBDKIT_CAPS_LAST)
+qemuNbdkitCapsSet(info->args.fakeNbdkitCaps, flag);
+break;
+
 case ARG_GIC:
 info->args.gic = va_arg(argptr, int);
 break;
@@ -1020,6 +1039,12

[libvirt PATCH v3 14/18] tests: add tests for nbdkit invocation

2022-10-20 Thread Jonathon Jongsma

We were testing the arguments that were being passed to qemu when a disk
was being served by nbdkit, but the arguments used to start nbdkit
itself were not testable. This adds a test to ensure that we're invoking
nbdkit correctly for various disk source definitions.

For now, expect failure for tests that use sensitive data such as
passwords and cookies.

Signed-off-by: Jonathon Jongsma 
---
 src/qemu/qemu_nbdkit.c|   4 +-
 src/qemu/qemu_nbdkitpriv.h|  31 +++
 tests/meson.build |   1 +
 .../disk-network-ssh.args.disk0   |   7 +
 tests/qemunbdkittest.c| 239 ++
 5 files changed, 281 insertions(+), 1 deletion(-)
 create mode 100644 src/qemu/qemu_nbdkitpriv.h
 create mode 100644 tests/qemunbdkitdata/disk-network-ssh.args.disk0
 create mode 100644 tests/qemunbdkittest.c

diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index 8cb9e0e604..882a074211 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -34,6 +34,8 @@
 #include "qemu_driver.h"
 #include "qemu_extdevice.h"
 #include "qemu_nbdkit.h"
+#define LIBVIRT_QEMU_NBDKITPRIV_H_ALLOW
+#include "qemu_nbdkitpriv.h"
 #include "qemu_security.h"
 
 #include 
@@ -830,7 +832,7 @@ qemuNbdkitProcessBuildCommandSSH(qemuNbdkitProcess *proc,
 }
 
 
-static virCommand *
+virCommand *
 qemuNbdkitProcessBuildCommand(qemuNbdkitProcess *proc)
 {
 g_autoptr(virCommand) cmd = virCommandNewArgList(proc->caps->path,
diff --git a/src/qemu/qemu_nbdkitpriv.h b/src/qemu/qemu_nbdkitpriv.h
new file mode 100644
index 00..64f9bb99d8
--- /dev/null
+++ b/src/qemu/qemu_nbdkitpriv.h
@@ -0,0 +1,31 @@
+/*
+ * qemu_nbdkitpriv.h: exposing some functions for testing
+ *
+ * Copyright (C) 2021 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ *
+ */
+
+#ifndef LIBVIRT_QEMU_NBDKITPRIV_H_ALLOW
+# error "qemu_nbdkitpriv.h may only be included by qemu_nbdkit.c or test 
suites"
+#endif /* LIBVIRT_QEMU_NBDKITPRIV_H_ALLOW */
+
+#pragma once
+
+#include "qemu_nbdkit.h"
+
+virCommand *
+qemuNbdkitProcessBuildCommand(qemuNbdkitProcess *proc);
diff --git a/tests/meson.build b/tests/meson.build
index 1149211756..87b29ab5b4 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -451,6 +451,7 @@ if conf.has('WITH_QEMU')
 { 'name': 'qemuvhostusertest', 'link_with': [ test_qemu_driver_lib ], 
'link_whole': [ test_file_wrapper_lib ] },
 { 'name': 'qemuxml2argvtest', 'link_with': [ test_qemu_driver_lib, 
test_utils_qemu_monitor_lib ], 'link_whole': [ test_utils_qemu_lib, 
test_file_wrapper_lib ] },
 { 'name': 'qemuxml2xmltest', 'link_with': [ test_qemu_driver_lib ], 
'link_whole': [ test_utils_qemu_lib, test_file_wrapper_lib ] },
+{ 'name': 'qemunbdkittest', 'link_with': [ test_qemu_driver_lib ], 
'link_whole': [ test_utils_qemu_lib, test_file_wrapper_lib ] },
   ]
 endif
 
diff --git a/tests/qemunbdkitdata/disk-network-ssh.args.disk0 
b/tests/qemunbdkitdata/disk-network-ssh.args.disk0
new file mode 100644
index 00..e0020dff6a
--- /dev/null
+++ b/tests/qemunbdkitdata/disk-network-ssh.args.disk0
@@ -0,0 +1,7 @@
+nbdkit \
+--exit-with-parent \
+--unix /tmp/statedir-0/nbdkit-test-disk-0.socket \
+--foreground ssh \
+host=example.org \
+port= \
+path=test.img
diff --git a/tests/qemunbdkittest.c b/tests/qemunbdkittest.c
new file mode 100644
index 00..c7fa80b9c5
--- /dev/null
+++ b/tests/qemunbdkittest.c
@@ -0,0 +1,239 @@
+#include 
+
+#include "internal.h"
+#include "testutils.h"
+#include "testutilsqemu.h"
+#include "qemu/qemu_domain.h"
+#include "qemu/qemu_nbdkit.h"
+#define LIBVIRT_QEMU_NBDKITPRIV_H_ALLOW
+#include "qemu/qemu_nbdkitpriv.h"
+#include "vircommand.h"
+#define LIBVIRT_VIRCOMMANDPRIV_H_ALLOW
+#include "vircommandpriv.h"
+#include "virutil.h"
+#include "virsecret.h"
+#include "datatypes.h"
+
+#define VIR_FROM_THIS VIR_FROM_QEMU
+
+static virQEMUDriver driver;
+
+
+/* Some mock implementations for testing */
+int
+virSecretGetSecretString(virConnectPtr conn G_GNUC_UNUSED,
+ virSecretLookupTypeDef *seclookupdef,
+ virSecretUsageType secretUsageType,
+ uint8_t **secret,
+ size_t *secret_size)
+{
+char uuidstr[VIR_UUID_BUFLEN];
+const char *secretname = NULL;
+char

[libvirt PATCH v3 15/18] util: make virCommandSetSendBuffer testable

2022-10-20 Thread Jonathon Jongsma

Add a private function to peek at the list of send buffers in virCommand
so that it is testable

Signed-off-by: Jonathon Jongsma 
---
 src/libvirt_private.syms  |  1 +
 src/util/vircommand.c | 16 
 src/util/vircommand.h |  8 
 src/util/vircommandpriv.h |  4 
 4 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0b0ccbafe5..bf24d65b08 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2080,6 +2080,7 @@ virCommandNewArgs;
 virCommandNewVAList;
 virCommandNonblockingFDs;
 virCommandPassFD;
+virCommandPeekSendBuffers;
 virCommandRawStatus;
 virCommandRequireHandshake;
 virCommandRun;
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index bbfbe19706..014bab9196 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -77,14 +77,6 @@ struct _virCommandFD {
 unsigned int flags;
 };
 
-typedef struct _virCommandSendBuffer virCommandSendBuffer;
-struct _virCommandSendBuffer {
-int fd;
-unsigned char *buffer;
-size_t buflen;
-off_t offset;
-};
-
 struct _virCommand {
 int has_error; /* 0 on success, -1 on error  */
 
@@ -3451,3 +3443,11 @@ virCommandSetRunAmong(virCommand *cmd,
 
 cmd->schedCore = pid;
 }
+
+void virCommandPeekSendBuffers(virCommand *cmd,
+   virCommandSendBuffer **buffers,
+   int *nbuffers)
+{
+*buffers = cmd->sendBuffers;
+*nbuffers = cmd->numSendBuffers;
+}
diff --git a/src/util/vircommand.h b/src/util/vircommand.h
index 98788bcbf7..fc2ca0f3ff 100644
--- a/src/util/vircommand.h
+++ b/src/util/vircommand.h
@@ -24,6 +24,14 @@
 #include "internal.h"
 #include "virbuffer.h"
 
+typedef struct _virCommandSendBuffer virCommandSendBuffer;
+struct _virCommandSendBuffer {
+int fd;
+unsigned char *buffer;
+size_t buflen;
+off_t offset;
+};
+
 typedef struct _virCommand virCommand;
 
 /* This will execute in the context of the first child
diff --git a/src/util/vircommandpriv.h b/src/util/vircommandpriv.h
index ff17fa5ded..d579810bb5 100644
--- a/src/util/vircommandpriv.h
+++ b/src/util/vircommandpriv.h
@@ -47,3 +47,7 @@ void virCommandSetDryRun(virCommandDryRunToken *tok,
  bool bufCommandStripPath,
  virCommandDryRunCallback cb,
  void *opaque);
+
+void virCommandPeekSendBuffers(virCommand *cmd,
+   virCommandSendBuffer **buffers,
+   int *nbuffers);
-- 
2.37.3

[libvirt PATCH v3 13/18] qemu: use nbdkit to serve network disks if available

2022-10-20 Thread Jonathon Jongsma

For virStorageSource objects that contain an nbdkitProcess, start that
nbdkit process to serve that network drive and then pass the nbdkit
socket to qemu rather than sending the network url to qemu directly.

Signed-off-by: Jonathon Jongsma 
---
 src/qemu/qemu_block.c | 162 +++---
 src/qemu/qemu_domain.c|  21 ++-
 src/qemu/qemu_extdevice.c |  48 ++
 src/qemu/qemu_nbdkit.c|  63 +++
 src/qemu/qemu_nbdkit.h|  13 ++
 ...sk-cdrom-network-nbdkit.x86_64-latest.args |  42 +
 .../disk-cdrom-network-nbdkit.xml |   1 +
 ...isk-network-http-nbdkit.x86_64-latest.args |  45 +
 .../disk-network-http-nbdkit.xml  |   1 +
 ...rce-curl-nbdkit-backing.x86_64-latest.args |  38 
 ...isk-network-source-curl-nbdkit-backing.xml |  45 +
 ...work-source-curl-nbdkit.x86_64-latest.args |  50 ++
 .../disk-network-source-curl-nbdkit.xml   |   1 +
 ...isk-network-source-curl.x86_64-latest.args |  53 ++
 .../disk-network-source-curl.xml  |  71 
 ...disk-network-ssh-nbdkit.x86_64-latest.args |  36 
 .../disk-network-ssh-nbdkit.xml   |   1 +
 tests/qemuxml2argvtest.c  |   6 +
 18 files changed, 627 insertions(+), 70 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/disk-cdrom-network-nbdkit.x86_64-latest.args
 create mode 12 tests/qemuxml2argvdata/disk-cdrom-network-nbdkit.xml
 create mode 100644 
tests/qemuxml2argvdata/disk-network-http-nbdkit.x86_64-latest.args
 create mode 12 tests/qemuxml2argvdata/disk-network-http-nbdkit.xml
 create mode 100644 
tests/qemuxml2argvdata/disk-network-source-curl-nbdkit-backing.x86_64-latest.args
 create mode 100644 
tests/qemuxml2argvdata/disk-network-source-curl-nbdkit-backing.xml
 create mode 100644 
tests/qemuxml2argvdata/disk-network-source-curl-nbdkit.x86_64-latest.args
 create mode 12 tests/qemuxml2argvdata/disk-network-source-curl-nbdkit.xml
 create mode 100644 
tests/qemuxml2argvdata/disk-network-source-curl.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/disk-network-source-curl.xml
 create mode 100644 
tests/qemuxml2argvdata/disk-network-ssh-nbdkit.x86_64-latest.args
 create mode 12 tests/qemuxml2argvdata/disk-network-ssh-nbdkit.xml

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index b82e3311e1..224d468799 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -439,6 +439,32 @@ qemuBlockStorageSourceGetCURLProps(virStorageSource *src,
 }
 
 
+static virJSONValue *
+qemuBlockStorageSourceGetNbdkitProps(virStorageSource *src)
+{
+qemuDomainStorageSourcePrivate *srcPriv = 
QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
+virJSONValue *ret = NULL;
+g_autoptr(virJSONValue) serverprops = NULL;
+virStorageNetHostDef host = { .transport = VIR_STORAGE_NET_HOST_TRANS_UNIX 
};
+
+/* srcPriv->nbdkitProcess will already be initialized if we can use nbdkit
+ * to proxy this storage source */
+if (!(srcPriv  && srcPriv->nbdkitProcess))
+return NULL;
+
+host.socket = srcPriv->nbdkitProcess->socketfile;
+serverprops = qemuBlockStorageSourceBuildJSONSocketAddress();
+
+if (!serverprops)
+return NULL;
+
+if (virJSONValueObjectAdd(, "a:server", , NULL) < 0)
+return NULL;
+
+return ret;
+}
+
+
 static virJSONValue *
 qemuBlockStorageSourceGetISCSIProps(virStorageSource *src,
 bool onlytarget)
@@ -851,69 +877,75 @@ qemuBlockStorageSourceGetBackendProps(virStorageSource 
*src,
 return NULL;
 
 case VIR_STORAGE_TYPE_NETWORK:
-switch ((virStorageNetProtocol) src->protocol) {
-case VIR_STORAGE_NET_PROTOCOL_GLUSTER:
-driver = "gluster";
-if (!(fileprops = qemuBlockStorageSourceGetGlusterProps(src, 
onlytarget)))
-return NULL;
-break;
-
-case VIR_STORAGE_NET_PROTOCOL_VXHS:
-driver = "vxhs";
-if (!(fileprops = qemuBlockStorageSourceGetVxHSProps(src, 
onlytarget)))
-return NULL;
-break;
-
-case VIR_STORAGE_NET_PROTOCOL_HTTP:
-case VIR_STORAGE_NET_PROTOCOL_HTTPS:
-case VIR_STORAGE_NET_PROTOCOL_FTP:
-case VIR_STORAGE_NET_PROTOCOL_FTPS:
-case VIR_STORAGE_NET_PROTOCOL_TFTP:
-driver = virStorageNetProtocolTypeToString(src->protocol);
-if (!(fileprops = qemuBlockStorageSourceGetCURLProps(src, 
onlytarget)))
-return NULL;
-break;
-
-case VIR_STORAGE_NET_PROTOCOL_ISCSI:
-driver = "iscsi";
-if (!(fileprops = qemuBlockStorageSourceGetISCSIProps(src, 
onlytarget)))
-return NULL;
-break;
-
-case VIR_STORAGE_NET_PROTOCOL_NBD:
+/* prefer using nbdkit for sources that are supported */
+if ((fileprops =

[libvirt PATCH v3 03/18] qemu: expand nbdkit capabilities

2022-10-20 Thread Jonathon Jongsma

In order to add caching of the nbdkit capabilities, we will need to
compare against file modification times, etc. So look up this
information when creating the nbdkit caps.

Add a nbdkit_moddir build option to allow the builder to specify the
location to look for nbdkit plugins and filters.

Signed-off-by: Jonathon Jongsma 
---
 meson.build|  6 ++
 meson_options.txt  |  1 +
 src/qemu/qemu_nbdkit.c | 40 
 3 files changed, 47 insertions(+)

diff --git a/meson.build b/meson.build
index 93de355430..e4581e74dd 100644
--- a/meson.build
+++ b/meson.build
@@ -1731,6 +1731,12 @@ if not get_option('driver_qemu').disabled()
   qemu_dbus_daemon_path = '/usr/bin/dbus-daemon'
 endif
 conf.set_quoted('QEMU_DBUS_DAEMON', qemu_dbus_daemon_path)
+
+nbdkit_moddir = get_option('nbdkit_moddir')
+if nbdkit_moddir == ''
+  nbdkit_moddir = libdir / 'nbdkit'
+endif
+conf.set_quoted('NBDKIT_MODDIR', nbdkit_moddir)
   endif
 endif
 
diff --git a/meson_options.txt b/meson_options.txt
index 861c5577d2..d5ea4376e0 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -71,6 +71,7 @@ option('driver_vbox', type: 'feature', value: 'auto', 
description: 'VirtualBox X
 option('vbox_xpcomc_dir', type: 'string', value: '', description: 'Location of 
directory containing VirtualBox XPCOMC library')
 option('driver_vmware', type: 'feature', value: 'auto', description: 'VMware 
driver')
 option('driver_vz', type: 'feature', value: 'auto', description: 'Virtuozzo 
driver')
+option('nbdkit_moddir', type: 'string', value: '', description: 'set the 
directory where nbdkit modules are located')
 
 option('secdriver_apparmor', type: 'feature', value: 'auto', description: 'use 
AppArmor security driver')
 option('apparmor_profiles', type: 'feature', value: 'auto', description: 
'install apparmor profiles')
diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index 7a7248c1ef..5de1021d89 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -41,6 +41,9 @@
 
 VIR_LOG_INIT("qemu.nbdkit");
 
+#define NBDKIT_PLUGINDIR NBDKIT_MODDIR "/plugins"
+#define NBDKIT_FILTERDIR NBDKIT_MODDIR "/filters"
+
 VIR_ENUM_IMPL(qemuNbdkitCaps,
 QEMU_NBDKIT_CAPS_LAST,
 /* 0 */
@@ -54,6 +57,11 @@ struct _qemuNbdkitCaps {
 
 char *path;
 char *version;
+time_t ctime;
+time_t libvirtCtime;
+time_t pluginDirMtime;
+time_t filterDirMtime;
+unsigned int libvirtVersion;
 
 virBitmap *flags;
 };
@@ -178,9 +186,41 @@ qemuNbdkitCapsNew(const char *path)
 }
 
 
+static time_t
+qemuNbdkitGetDirMtime(const char *moddir)
+{
+struct stat st;
+
+if (stat(moddir, ) < 0) {
+VIR_DEBUG("Failed to stat nbdkit module directory '%s': %s",
+  moddir,
+  g_strerror(errno));
+return 0;
+}
+
+return st.st_mtime;
+}
+
+
 G_GNUC_UNUSED static void
 qemuNbdkitCapsQuery(qemuNbdkitCaps *caps)
 {
+struct stat st;
+
+if (stat(caps->path, ) < 0) {
+VIR_DEBUG("Failed to stat nbdkit binary '%s': %s",
+  caps->path,
+  g_strerror(errno));
+caps->ctime  = 0;
+return;
+}
+
+caps->ctime = st.st_ctime;
+caps->filterDirMtime = qemuNbdkitGetDirMtime(NBDKIT_FILTERDIR);
+caps->pluginDirMtime = qemuNbdkitGetDirMtime(NBDKIT_PLUGINDIR);
+caps->libvirtCtime = virGetSelfLastChanged();
+caps->libvirtVersion = LIBVIR_VERSION_NUMBER;
+
 qemuNbdkitCapsQueryPlugins(caps);
 qemuNbdkitCapsQueryFilters(caps);
 qemuNbdkitCapsQueryVersion(caps);
-- 
2.37.3

[libvirt PATCH v3 05/18] qemu: implement basic virFileCache for nbdkit caps

2022-10-20 Thread Jonathon Jongsma

Preparatory step for caching nbdkit capabilities. This patch implements
the newData and isValid virFileCacheHandlers callback functions.

Signed-off-by: Jonathon Jongsma 
---
 src/qemu/qemu_nbdkit.c | 93 +-
 src/qemu/qemu_nbdkit.h |  4 ++
 2 files changed, 96 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index 5de1021d89..9ab048e9e1 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -202,7 +202,7 @@ qemuNbdkitGetDirMtime(const char *moddir)
 }
 
 
-G_GNUC_UNUSED static void
+static void
 qemuNbdkitCapsQuery(qemuNbdkitCaps *caps)
 {
 struct stat st;
@@ -241,3 +241,94 @@ qemuNbdkitCapsSet(qemuNbdkitCaps *nbdkitCaps,
 {
 ignore_value(virBitmapSetBit(nbdkitCaps->flags, flag));
 }
+
+
+static bool
+virNbkditCapsCheckModdir(const char *moddir,
+ time_t expectedMtime)
+{
+time_t mtime = qemuNbdkitGetDirMtime(moddir);
+
+if (mtime != expectedMtime) {
+VIR_DEBUG("Outdated capabilities for nbdkit: module "
+  "directory '%s' changed (%lld vs %lld)",
+  moddir,
+  (long long)mtime, (long long)expectedMtime);
+return false;
+}
+return true;
+}
+
+
+static bool
+virNbdkitCapsIsValid(void *data,
+ void *privData G_GNUC_UNUSED)
+{
+qemuNbdkitCaps *nbdkitCaps = data;
+struct stat st;
+
+if (!nbdkitCaps->path)
+return true;
+
+if (!virNbkditCapsCheckModdir(NBDKIT_PLUGINDIR, 
nbdkitCaps->pluginDirMtime))
+return false;
+if (!virNbkditCapsCheckModdir(NBDKIT_FILTERDIR, 
nbdkitCaps->filterDirMtime))
+return false;
+
+if (nbdkitCaps->libvirtCtime != virGetSelfLastChanged() ||
+nbdkitCaps->libvirtVersion != LIBVIR_VERSION_NUMBER) {
+VIR_DEBUG("Outdated capabilities for '%s': libvirt changed "
+  "(%lld vs %lld, %lu vs %lu)",
+  nbdkitCaps->path,
+  (long long)nbdkitCaps->libvirtCtime,
+  (long long)virGetSelfLastChanged(),
+  (unsigned long)nbdkitCaps->libvirtVersion,
+  (unsigned long)LIBVIR_VERSION_NUMBER);
+return false;
+}
+
+if (stat(nbdkitCaps->path, ) < 0) {
+VIR_DEBUG("Failed to stat nbdkit binary '%s': %s",
+  nbdkitCaps->path,
+  g_strerror(errno));
+return false;
+}
+
+if (st.st_ctime != nbdkitCaps->ctime) {
+VIR_DEBUG("Outdated capabilities for '%s': nbdkit binary changed "
+  "(%lld vs %lld)",
+  nbdkitCaps->path,
+  (long long)st.st_ctime, (long long)nbdkitCaps->ctime);
+return false;
+}
+
+return true;
+}
+
+
+static void*
+virNbdkitCapsNewData(const char *binary,
+ void *privData G_GNUC_UNUSED)
+{
+qemuNbdkitCaps *caps = qemuNbdkitCapsNew(binary);
+qemuNbdkitCapsQuery(caps);
+
+return caps;
+}
+
+
+virFileCacheHandlers nbdkitCapsCacheHandlers = {
+.isValid = virNbdkitCapsIsValid,
+.newData = virNbdkitCapsNewData,
+.loadFile = NULL,
+.saveFile = NULL,
+.privFree = NULL,
+};
+
+
+virFileCache*
+qemuNbdkitCapsCacheNew(const char *cachedir)
+{
+g_autofree char *dir = g_build_filename(cachedir, "nbdkitcapabilities", 
NULL);
+return virFileCacheNew(dir, "xml", );
+}
diff --git a/src/qemu/qemu_nbdkit.h b/src/qemu/qemu_nbdkit.h
index 8ffb0f7044..f6cbedaa94 100644
--- a/src/qemu/qemu_nbdkit.h
+++ b/src/qemu/qemu_nbdkit.h
@@ -23,6 +23,7 @@
 
 #include "internal.h"
 #include "virenum.h"
+#include "virfilecache.h"
 
 typedef struct _qemuNbdkitCaps qemuNbdkitCaps;
 
@@ -40,6 +41,9 @@ VIR_ENUM_DECL(qemuNbdkitCaps);
 qemuNbdkitCaps *
 qemuNbdkitCapsNew(const char *path);
 
+virFileCache *
+qemuNbdkitCapsCacheNew(const char *cachedir);
+
 bool
 qemuNbdkitCapsGet(qemuNbdkitCaps *nbdkitCaps,
   qemuNbdkitCapsFlags flag);
-- 
2.37.3

[libvirt PATCH v3 09/18] qemu: add functions to start and stop nbdkit

2022-10-20 Thread Jonathon Jongsma

Add some helper functions to build a virCommand object and run the
nbdkit process for a given virStorageSource.

Signed-off-by: Jonathon Jongsma 
---
 src/qemu/qemu_nbdkit.c | 251 +
 src/qemu/qemu_nbdkit.h |  10 ++
 2 files changed, 261 insertions(+)

diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index 291f988c7a..c5b0762f8d 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -26,6 +26,7 @@
 #include "virerror.h"
 #include "virlog.h"
 #include "virpidfile.h"
+#include "virtime.h"
 #include "virutil.h"
 #include "qemu_block.h"
 #include "qemu_conf.h"
@@ -636,6 +637,163 @@ qemuNbdkitInitStorageSource(qemuNbdkitCaps *caps,
 }
 
 
+static int
+qemuNbdkitProcessBuildCommandCurl(qemuNbdkitProcess *proc,
+  virCommand *cmd)
+{
+g_autoptr(virURI) uri = qemuBlockStorageSourceGetURI(proc->source);
+g_autofree char *uristring = virURIFormat(uri);
+
+/* nbdkit plugin name */
+virCommandAddArg(cmd, "curl");
+virCommandAddArgPair(cmd, "protocols",
+ 
virStorageNetProtocolTypeToString(proc->source->protocol));
+virCommandAddArgPair(cmd, "url", uristring);
+
+if (proc->source->auth) {
+g_autoptr(virConnect) conn = virGetConnectSecret();
+g_autofree uint8_t *secret = NULL;
+size_t secretlen = 0;
+g_autofree char *password = NULL;
+int secrettype;
+
+virCommandAddArgPair(cmd, "user",
+ proc->source->auth->username);
+
+if ((secrettype = 
virSecretUsageTypeFromString(proc->source->auth->secrettype)) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("invalid secret type %s"),
+   proc->source->auth->secrettype);
+return -1;
+}
+
+if (virSecretGetSecretString(conn,
+ >source->auth->seclookupdef,
+ secrettype,
+ ,
+ ) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("failed to get auth secret for storage"));
+return -1;
+}
+
+/* ensure that the secret is a NULL-terminated string */
+password = g_strndup((char*)secret, secretlen);
+
+/* for now, just report an error rather than passing the password in
+ * cleartext on the commandline */
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   "%s",
+   "Password not yet supported for nbdkit sources");
+return -1;
+}
+
+if (proc->source->ncookies > 0) {
+/* for now, just report an error rather than passing cookies in
+ * cleartext on the commandline */
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   "%s",
+   "Cookies not yet supported for nbdkit sources");
+return -1;
+}
+
+if (proc->source->sslverify == VIR_TRISTATE_BOOL_NO) {
+virCommandAddArgPair(cmd, "sslverify", "false");
+}
+
+if (proc->source->timeout > 0) {
+g_autofree char *timeout = g_strdup_printf("%llu", 
proc->source->timeout);
+virCommandAddArgPair(cmd, "timeout", timeout);
+}
+
+return 0;
+}
+
+
+static int
+qemuNbdkitProcessBuildCommandSSH(qemuNbdkitProcess *proc,
+ virCommand *cmd)
+{
+const char *user = NULL;
+virStorageNetHostDef *host = >source->hosts[0];
+g_autofree char *portstr = g_strdup_printf("%u", host->port);
+
+/* nbdkit plugin name */
+virCommandAddArg(cmd, "ssh");
+
+virCommandAddArgPair(cmd, "host", host->name);
+virCommandAddArgPair(cmd, "port", portstr);
+virCommandAddArgPair(cmd, "path", proc->source->path);
+
+if (proc->source->auth)
+user = proc->source->auth->username;
+else if (proc->source->ssh_user)
+user = proc->source->ssh_user;
+
+if (user)
+virCommandAddArgPair(cmd, "user", user);
+
+if (proc->source->ssh_host_key_check_disabled)
+virCommandAddArgPair(cmd, "verify-remote-host", "false");
+
+return 0;
+}
+
+
+static virCommand *
+qemuNbdkitProcessBuildCommand(qemuNbdkitProcess *proc)
+{
+g_autoptr(virCommand) cmd = virCommandNewArgList(proc->caps->path,
+ "--exit-with-parent",
+ "--unix",
+ proc->socketfile,
+ "--foreground",
+ //"--selinux-label",
+ //selinux_label,
+ NULL);
+
+if (proc->source->readonly)
+virCommandAddArg(cmd, "--readonly");
+
+if

[libvirt PATCH v3 02/18] qemu: Add functions for determining nbdkit availability

2022-10-20 Thread Jonathon Jongsma

In future commits, we will optionally use nbdkit to serve some remote
disk sources. This patch queries to see whether nbdkit is installed on
the host and queries it for capabilities. The data will be used in later
commits.

Signed-off-by: Jonathon Jongsma 
---
 po/POTFILES|   1 +
 src/qemu/meson.build   |   1 +
 src/qemu/qemu_conf.h   |   1 +
 src/qemu/qemu_nbdkit.c | 203 +
 src/qemu/qemu_nbdkit.h |  52 +++
 5 files changed, 258 insertions(+)
 create mode 100644 src/qemu/qemu_nbdkit.c
 create mode 100644 src/qemu/qemu_nbdkit.h

diff --git a/po/POTFILES b/po/POTFILES
index 169e2a41dc..d96ce4415a 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -180,6 +180,7 @@ src/qemu/qemu_monitor.c
 src/qemu/qemu_monitor_json.c
 src/qemu/qemu_monitor_text.c
 src/qemu/qemu_namespace.c
+src/qemu/qemu_nbdkit.c
 src/qemu/qemu_process.c
 src/qemu/qemu_qapi.c
 src/qemu/qemu_saveimage.c
diff --git a/src/qemu/meson.build b/src/qemu/meson.build
index 96952cc52d..101cf3591f 100644
--- a/src/qemu/meson.build
+++ b/src/qemu/meson.build
@@ -28,6 +28,7 @@ qemu_driver_sources = [
   'qemu_monitor_json.c',
   'qemu_monitor_text.c',
   'qemu_namespace.c',
+  'qemu_nbdkit.c',
   'qemu_process.c',
   'qemu_qapi.c',
   'qemu_saveimage.c',
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 8cf2dd2ec5..a39510b0b1 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -36,6 +36,7 @@
 #include "virthreadpool.h"
 #include "locking/lock_manager.h"
 #include "qemu_capabilities.h"
+#include "qemu_nbdkit.h"
 #include "virclosecallbacks.h"
 #include "virhostdev.h"
 #include "virfile.h"
diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
new file mode 100644
index 00..7a7248c1ef
--- /dev/null
+++ b/src/qemu/qemu_nbdkit.c
@@ -0,0 +1,203 @@
+/*
+ * qemu_nbdkit.c: helpers for using nbdkit
+ *
+ * Copyright (C) 2021 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ *
+ */
+
+#include 
+#include 
+
+#include "vircommand.h"
+#include "virerror.h"
+#include "virlog.h"
+#include "virpidfile.h"
+#include "virutil.h"
+#include "qemu_block.h"
+#include "qemu_conf.h"
+#include "qemu_domain.h"
+#include "qemu_driver.h"
+#include "qemu_extdevice.h"
+#include "qemu_nbdkit.h"
+#include "qemu_security.h"
+
+#include 
+
+#define VIR_FROM_THIS VIR_FROM_QEMU
+
+VIR_LOG_INIT("qemu.nbdkit");
+
+VIR_ENUM_IMPL(qemuNbdkitCaps,
+QEMU_NBDKIT_CAPS_LAST,
+/* 0 */
+"plugin-curl", /* QEMU_NBDKIT_CAPS_PLUGIN_CURL */
+"plugin-ssh", /* QEMU_NBDKIT_CAPS_PLUGIN_SSH */
+"filter-readahead", /* QEMU_NBDKIT_CAPS_FILTER_READAHEAD */
+);
+
+struct _qemuNbdkitCaps {
+GObject parent;
+
+char *path;
+char *version;
+
+virBitmap *flags;
+};
+G_DEFINE_TYPE(qemuNbdkitCaps, qemu_nbdkit_caps, G_TYPE_OBJECT);
+
+
+static void
+qemuNbdkitCheckCommandCap(qemuNbdkitCaps *nbdkit,
+  virCommand *cmd,
+  qemuNbdkitCapsFlags cap)
+{
+if (virCommandRun(cmd, NULL) != 0)
+return;
+
+VIR_DEBUG("Setting nbdkit capability %i", cap);
+ignore_value(virBitmapSetBit(nbdkit->flags, cap));
+}
+
+
+static void
+qemuNbdkitQueryFilter(qemuNbdkitCaps *nbdkit,
+  const char *filter,
+  qemuNbdkitCapsFlags cap)
+{
+g_autoptr(virCommand) cmd = virCommandNewArgList(nbdkit->path,
+ "--version",
+ NULL);
+
+virCommandAddArgPair(cmd, "--filter", filter);
+
+/* use null plugin to check for filter */
+virCommandAddArg(cmd, "null");
+
+qemuNbdkitCheckCommandCap(nbdkit, cmd, cap);
+}
+
+
+static void
+qemuNbdkitQueryPlugin(qemuNbdkitCaps *nbdkit,
+  const char *plugin,
+  qemuNbdkitCapsFlags cap)
+{
+g_autoptr(virCommand) cmd = virCommandNewArgList(nbdkit->path,
+ plugin,
+ "--version",
+ NULL);
+
+qemuNbdkitCheckCommandCap(nbdkit, cmd, cap);
+}
+
+
+static void
+qemuNbdkitCapsQueryPlugins(qemuNbdkitCaps *nbdkit)
+{
+qemuNbdkitQueryPlugin(nbdkit, "curl", QEMU_NBDKIT_CAPS_PLUGIN_CURL);
+qemuNbdkitQueryPlugin(nbdkit, "ssh",

[libvirt PATCH v3 04/18] util: Allow virFileCache data to be any GObject

2022-10-20 Thread Jonathon Jongsma

Since the libvirt documentation suggests to prefer GObject over
virObject, and since virObject is a GObject, change virFileCache to
allow GObjects as data.

Signed-off-by: Jonathon Jongsma 
---
 src/util/virfilecache.c | 14 --
 src/util/virfilecache.h |  2 +-
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/util/virfilecache.c b/src/util/virfilecache.c
index bad37c9f00..eaedf6db7e 100644
--- a/src/util/virfilecache.c
+++ b/src/util/virfilecache.c
@@ -170,7 +170,7 @@ virFileCacheLoad(virFileCache *cache,
 *data = g_steal_pointer();
 
  cleanup:
-virObjectUnref(loadData);
+g_clear_pointer(, g_object_unref);
 return ret;
 }
 
@@ -207,7 +207,7 @@ virFileCacheNewData(virFileCache *cache,
 return NULL;
 
 if (virFileCacheSave(cache, name, data) < 0) {
-g_clear_pointer(, virObjectUnref);
+g_clear_object();
 }
 }
 
@@ -239,7 +239,7 @@ virFileCacheNew(const char *dir,
 if (!(cache = virObjectNew(virFileCacheClass)))
 return NULL;
 
-cache->table = virHashNew(virObjectUnref);
+cache->table = virHashNew(g_object_unref);
 
 cache->dir = g_strdup(dir);
 
@@ -270,7 +270,7 @@ virFileCacheValidate(virFileCache *cache,
 if (*data) {
 VIR_DEBUG("Caching data '%p' for '%s'", *data, name);
 if (virHashAddEntry(cache->table, name, *data) < 0) {
-g_clear_pointer(data, virObjectUnref);
+g_clear_pointer(data, g_object_unref);
 }
 }
 }
@@ -300,7 +300,8 @@ virFileCacheLookup(virFileCache *cache,
 data = virHashLookup(cache->table, name);
 virFileCacheValidate(cache, name, );
 
-virObjectRef(data);
+if (data)
+g_object_ref(data);
 virObjectUnlock(cache);
 
 return data;
@@ -331,7 +332,8 @@ virFileCacheLookupByFunc(virFileCache *cache,
 data = virHashSearch(cache->table, iter, iterData, );
 virFileCacheValidate(cache, name, );
 
-virObjectRef(data);
+if (data)
+g_object_ref(data);
 virObjectUnlock(cache);
 
 return data;
diff --git a/src/util/virfilecache.h b/src/util/virfilecache.h
index 81be8feef5..f0d220cc86 100644
--- a/src/util/virfilecache.h
+++ b/src/util/virfilecache.h
@@ -48,7 +48,7 @@ typedef bool
  * @priv: private data created together with cache
  *
  * Creates a new data based on the @name.  The returned data must be
- * an instance of virObject.
+ * an instance of GObject.
  *
  * Returns data object or NULL on error.
  */
-- 
2.37.3

[libvirt PATCH v3 08/18] qemu: Add qemuNbdkitProcess

2022-10-20 Thread Jonathon Jongsma

An object for storing information about a nbdkit process that is serving
a specific virStorageSource. At the moment, this information is just
stored in the private data of virStorageSource and not used at all.
Future commits will use this data to actually start a nbdkit process.

Signed-off-by: Jonathon Jongsma 
---
 src/qemu/qemu_conf.c   | 23 
 src/qemu/qemu_conf.h   |  3 ++
 src/qemu/qemu_domain.c | 31 
 src/qemu/qemu_domain.h |  4 +++
 src/qemu/qemu_nbdkit.c | 82 ++
 src/qemu/qemu_nbdkit.h | 26 ++
 6 files changed, 169 insertions(+)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index ae5bbcd138..0370429da0 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1641,3 +1641,26 @@ qemuHugepageMakeBasedir(virQEMUDriver *driver,
 
 return 0;
 }
+
+
+/*
+ * qemuGetNbdkitCaps:
+ * @driver: the qemu driver
+ *
+ * Gets the capabilities for Nbdkit for the specified driver. These can be used
+ * to determine whether a particular disk source can be served by nbdkit or
+ * not.
+ *
+ * Returns: a reference to qemuNbdkitCaps or NULL
+ */
+qemuNbdkitCaps*
+qemuGetNbdkitCaps(virQEMUDriver *driver)
+{
+if (!driver->nbdkitBinary)
+driver->nbdkitBinary = virFindFileInPath("nbdkit");
+
+if (driver->nbdkitBinary)
+return virFileCacheLookup(driver->nbdkitCapsCache, 
driver->nbdkitBinary);
+
+return NULL;
+}
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 95c05e6888..8b4f6ce669 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -323,6 +323,7 @@ struct _virQEMUDriver {
 
 /* Immutable pointer, self-locking APIs */
 virFileCache *nbdkitCapsCache;
+char *nbdkitBinary;
 };
 
 virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
@@ -379,3 +380,5 @@ int qemuGetMemoryBackingPath(virQEMUDriver *driver,
 
 int qemuHugepageMakeBasedir(virQEMUDriver *driver,
 virHugeTLBFS *hugepage);
+
+qemuNbdkitCaps* qemuGetNbdkitCaps(virQEMUDriver *driver);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 9ef6c8bb64..3fca163415 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -851,6 +851,7 @@ qemuDomainStorageSourcePrivateDispose(void *obj)
 g_clear_pointer(>encinfo, qemuDomainSecretInfoFree);
 g_clear_pointer(>httpcookie, qemuDomainSecretInfoFree);
 g_clear_pointer(>tlsKeySecret, qemuDomainSecretInfoFree);
+g_clear_pointer(>nbdkitProcess, qemuNbdkitProcessFree);
 }
 
 
@@ -10055,6 +10056,34 @@ qemuDomainPrepareStorageSourceNFS(virStorageSource 
*src)
 }
 
 
+/* qemuPrepareStorageSourceNbdkit:
+ * @src: source for a disk
+ *
+ * If src is an network source that is managed by nbdkit, prepare data so that
+ * nbdkit can be launched before the domain is started
+ *
+ * Returns true if nbdkit will be used for this source,
+ */
+static bool
+qemuDomainPrepareStorageSourceNbdkit(virStorageSource *src,
+ virQEMUDriverConfig *cfg,
+ const char *alias,
+ qemuDomainObjPrivate *priv)
+{
+g_autoptr(qemuNbdkitCaps) nbdkit = NULL;
+
+if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK)
+return false;
+
+nbdkit = qemuGetNbdkitCaps(priv->driver);
+if (!nbdkit)
+return false;
+
+return qemuNbdkitInitStorageSource(nbdkit, src, priv->libDir,
+   alias, cfg->user, cfg->group);
+}
+
+
 /* qemuProcessPrepareStorageSourceTLS:
  * @source: source for a disk
  * @cfg: driver configuration
@@ -10829,6 +10858,8 @@ 
qemuDomainPrepareStorageSourceBlockdevNodename(virDomainDiskDef *disk,
 if (qemuDomainPrepareStorageSourceNFS(src) < 0)
 return -1;
 
+qemuDomainPrepareStorageSourceNbdkit(src, cfg, src->nodestorage, priv);
+
 return 0;
 }
 
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 2bbd492d62..79dd9e4329 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -33,6 +33,7 @@
 #include "qemu_conf.h"
 #include "qemu_capabilities.h"
 #include "qemu_migration_params.h"
+#include "qemu_nbdkit.h"
 #include "qemu_slirp.h"
 #include "qemu_fd.h"
 #include "virchrdev.h"
@@ -298,6 +299,9 @@ struct _qemuDomainStorageSourcePrivate {
 
 /* key for decrypting TLS certificate */
 qemuDomainSecretInfo *tlsKeySecret;
+
+/* an nbdkit process for serving network storage sources */
+qemuNbdkitProcess *nbdkitProcess;
 };
 
 virObject *qemuDomainStorageSourcePrivateNew(void);
diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index a47e169a0f..291f988c7a 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -562,3 +562,85 @@ qemuNbdkitCapsCacheNew(const char *cachedir)
 g_autofree char *dir = g_build_filename(cachedir, "nbdkitcapabilities", 
NULL);
 return virFileCacheNew(dir, "xml", );
 }
+
+
+static qemuNbdkitProcess *

[libvirt PATCH v3 06/18] qemu: implement persistent file cache for nbdkit caps

2022-10-20 Thread Jonathon Jongsma

Implement the loadFile and saveFile virFileCacheHandlers callbacks so
that nbdkit capabilities are cached perstistently across daemon
restarts. The format and implementation is modeled on the qemu
capabilities, but simplified slightly.

Signed-off-by: Jonathon Jongsma 
---
 src/qemu/qemu_nbdkit.c | 234 -
 1 file changed, 232 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index 9ab048e9e1..a47e169a0f 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -317,11 +317,241 @@ virNbdkitCapsNewData(const char *binary,
 }
 
 
+static int
+qemuNbdkitCapsValidateBinary(qemuNbdkitCaps *nbdkitCaps,
+ xmlXPathContextPtr ctxt)
+{
+g_autofree char *str = NULL;
+
+if (!(str = virXPathString("string(./path)", ctxt))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("missing path in nbdkit capabilities cache"));
+return -1;
+}
+
+if (STRNEQ(str, nbdkitCaps->path)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Expected caps for '%s' but saw '%s'"),
+   nbdkitCaps->path, str);
+return -1;
+}
+
+return 0;
+}
+
+
+static int
+qemuNbdkitCapsParseFlags(qemuNbdkitCaps *nbdkitCaps,
+ xmlXPathContextPtr ctxt)
+{
+g_autofree xmlNodePtr *nodes = NULL;
+size_t i;
+int n;
+
+if ((n = virXPathNodeSet("./flag", ctxt, )) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("failed to parse qemu capabilities flags"));
+return -1;
+}
+
+VIR_DEBUG("Got flags %d", n);
+for (i = 0; i < n; i++) {
+unsigned int flag;
+
+if (virXMLPropEnum(nodes[i], "name", qemuNbdkitCapsTypeFromString,
+   VIR_XML_PROP_REQUIRED, ) < 0)
+return -1;
+
+qemuNbdkitCapsSet(nbdkitCaps, flag);
+}
+
+return 0;
+}
+
+
+/*
+ * Parsing a doc that looks like
+ *
+ * 
+ *   /some/path
+ *   234235253
+ *   234235253
+ *   234235253
+ *   234235253
+ *   1002016
+ *   
+ *   
+ *   ...
+ * 
+ *
+ * Returns 0 on success, 1 if outdated, -1 on error
+ */
+static int
+qemuNbdkitCapsLoadCache(qemuNbdkitCaps *nbdkitCaps,
+const char *filename)
+{
+g_autoptr(xmlDoc) doc = NULL;
+g_autoptr(xmlXPathContext) ctxt = NULL;
+long long int l;
+unsigned long lu;
+
+if (!(doc = virXMLParse(filename, NULL, NULL, "nbdkitCaps", , NULL, 
false)))
+return -1;
+
+if (virXPathLongLong("string(./selfctime)", ctxt, ) < 0) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("missing selfctime in nbdkit capabilities XML"));
+return -1;
+}
+nbdkitCaps->libvirtCtime = (time_t)l;
+
+nbdkitCaps->libvirtVersion = 0;
+if (virXPathULong("string(./selfvers)", ctxt, ) == 0)
+nbdkitCaps->libvirtVersion = lu;
+
+if (nbdkitCaps->libvirtCtime != virGetSelfLastChanged() ||
+nbdkitCaps->libvirtVersion != LIBVIR_VERSION_NUMBER) {
+VIR_DEBUG("Outdated capabilities in %s: libvirt changed "
+  "(%lld vs %lld, %lu vs %lu), stopping load",
+  nbdkitCaps->path,
+  (long long)nbdkitCaps->libvirtCtime,
+  (long long)virGetSelfLastChanged(),
+  (unsigned long)nbdkitCaps->libvirtVersion,
+  (unsigned long)LIBVIR_VERSION_NUMBER);
+return 1;
+}
+
+if (qemuNbdkitCapsValidateBinary(nbdkitCaps, ctxt) < 0)
+return -1;
+
+if (virXPathLongLong("string(./nbdkitctime)", ctxt, ) < 0) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("missing nbdkitctime in nbdkit capabilities XML"));
+return -1;
+}
+nbdkitCaps->ctime = (time_t)l;
+
+if (virXPathLongLong("string(./plugindirmtime)", ctxt, ) < 0) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("missing plugindirmtime in nbdkit capabilities XML"));
+return -1;
+}
+nbdkitCaps->pluginDirMtime = (time_t)l;
+
+if (virXPathLongLong("string(./filterdirmtime)", ctxt, ) < 0) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("missing filterdirmtime in nbdkit capabilities XML"));
+return -1;
+}
+nbdkitCaps->filterDirMtime = (time_t)l;
+
+if (qemuNbdkitCapsParseFlags(nbdkitCaps, ctxt) < 0)
+return -1;
+
+if ((nbdkitCaps->version = virXPathString("string(./version)", ctxt)) == 
NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("missing version in nbdkit capabilities cache"));
+return -1;
+}
+
+return 0;
+}
+
+
+static void*
+virNbdkitCapsLoadFile(const char *filename,
+  const char *binary,
+  void *privData G_GNUC_UNUSED,
+  bool *outdated)
+{
+g_autoptr(qemuNbdkitCaps)

[libvirt PATCH v3 01/18] schema: allow 'ssh' as a protocol for network disks

2022-10-20 Thread Jonathon Jongsma

There was support in the code for parsing protocol='ssh' on network disk
sources, but it was not present in the xml schema. Add this to the
schema and mention it in the documentation.

Signed-off-by: Jonathon Jongsma 
---
 src/conf/schemas/domaincommon.rng |  1 +
 .../disk-network-ssh.x86_64-latest.args   | 36 +++
 tests/qemuxml2argvdata/disk-network-ssh.xml   | 31 
 tests/qemuxml2argvtest.c  |  1 +
 4 files changed, 69 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/disk-network-ssh.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/disk-network-ssh.xml

diff --git a/src/conf/schemas/domaincommon.rng 
b/src/conf/schemas/domaincommon.rng
index d346442510..e2d5ff1379 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -2144,6 +2144,7 @@
   
 sheepdog
 tftp
+ssh
   
 
 
diff --git a/tests/qemuxml2argvdata/disk-network-ssh.x86_64-latest.args 
b/tests/qemuxml2argvdata/disk-network-ssh.x86_64-latest.args
new file mode 100644
index 00..045474724b
--- /dev/null
+++ b/tests/qemuxml2argvdata/disk-network-ssh.x86_64-latest.args
@@ -0,0 +1,36 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
+/usr/bin/qemu-system-x86_64 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object 
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}'
 \
+-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram \
+-accel kvm \
+-cpu qemu64 \
+-m 214 \
+-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-boot strict=on \
+-device 
'{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
+-blockdev 
'{"driver":"ssh","path":"test.img","server":{"host":"example.org","port":""},"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}'
 \
+-blockdev 
'{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}'
 \
+-device 
'{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x2","drive":"libvirt-1-format","id":"virtio-disk0","bootindex":1}'
 \
+-audiodev '{"id":"audio1","driver":"none"}' \
+-sandbox 
on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/disk-network-ssh.xml 
b/tests/qemuxml2argvdata/disk-network-ssh.xml
new file mode 100644
index 00..355add4fea
--- /dev/null
+++ b/tests/qemuxml2argvdata/disk-network-ssh.xml
@@ -0,0 +1,31 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+
+  
+  
+
+
+
+  
+  
+
+
+
+
+
+
+  
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 7ede68d555..ef32cae2e9 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1325,6 +1325,7 @@ mymain(void)
 DO_TEST_CAPS_LATEST("disk-network-tlsx509-nbd-hostname");
 DO_TEST_CAPS_VER("disk-network-tlsx509-vxhs", "5.0.0");
 DO_TEST_CAPS_LATEST("disk-network-http");
+DO_TEST_CAPS_LATEST("disk-network-ssh");
 driver.config->vxhsTLS = 0;
 VIR_FREE(driver.config->vxhsTLSx509certdir);
 DO_TEST_CAPS_LATEST("disk-no-boot");
-- 
2.37.3

Re: [libvirt PATCH] qemu: do not attempt to pass unopened vsock FD

2022-10-20 Thread Peter Krempa

On Thu, Oct 20, 2022 at 16:27:45 +0200, Ján Tomko wrote:
> On normal vm startup, we open a file descriptor
> for the vsock device in qemuProcessPrepareHost.
> 
> However, when doing domxml-to-native, no file descriptors are open.
> 
> Only pass the fd if it's not -1, to make domxml-to-native work.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1777212
> 
> Signed-off-by: Ján Tomko 
> ---
> Technically a v2 of:
> https://listman.redhat.com/archives/libvir-list/2021-July/msg00803.html
> 
> I did not look at other cases, but IIRC the ones converted to use qemuFD*
> wrappers should handle missing FDs gracefully.
> 
>  src/qemu/qemu_command.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 150824f2e1..bbbde57c0f 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -9717,7 +9717,8 @@ qemuBuildVsockCommandLine(virCommand *cmd,
>  if (!(devprops = qemuBuildVsockDevProps(def, vsock, qemuCaps, "")))
>  return -1;
>  
> -virCommandPassFD(cmd, priv->vhostfd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
> +if (priv->vhostfd != -1)
> +virCommandPassFD(cmd, priv->vhostfd, 
> VIR_COMMAND_PASS_FD_CLOSE_PARENT);
>  priv->vhostfd = -1;

In 'qemuBuildVsockDevProps' the 'fd' field is formated via '%u'. Please
change it to '%d' to go along with this patch.

>  
>  if (qemuCommandAddExtDevice(cmd, >info, def, qemuCaps) < 0)
> -- 
> 2.37.3

Reviewed-by: Peter Krempa

[PATCH 5/6] storage: Add VIR_STORAGE_VOL_CREATE_VALIDATE flag

2022-10-20 Thread Peter Krempa

Allow users to request validation of the storage volume XML. Add new
flag and virsh support.

Signed-off-by: Peter Krempa 
---
 docs/manpages/virsh.rst   |  9 +++--
 include/libvirt/libvirt-storage.h |  1 +
 tools/virsh-volume.c  | 14 ++
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 532d4e779c..1e85b6ae78 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -6542,7 +6542,7 @@ vol-create

 ::

-   vol-create pool-or-uuid FILE [--prealloc-metadata]
+   vol-create pool-or-uuid FILE [--prealloc-metadata] [--validate]

 Create a volume from an XML .

@@ -6557,6 +6557,9 @@ support full allocation). This option creates a sparse 
image file with metadata,
 resulting in higher performance compared to images with no preallocation and
 only slightly higher initial disk space usage.

+If *--validate* is specified, validates the format of the XML document against
+an internal RNG schema.
+
 **Example:**

 ::
@@ -6574,7 +6577,7 @@ vol-create-from
 ::

vol-create-from pool-or-uuid FILE vol-name-or-key-or-path
-  [--inputpool pool-or-uuid]  [--prealloc-metadata] [--reflink]
+  [--inputpool pool-or-uuid]  [--prealloc-metadata] [--reflink] 
[--validate]

 Create a volume, using another volume as input.

@@ -6596,6 +6599,8 @@ When *--reflink* is specified, perform a COW lightweight 
copy,
 where the data blocks are copied only when modified.
 If this is not possible, the copy fails.

+If *--validate* is specified, validates the format of the XML document against
+an internal RNG schema.

 vol-create-as
 -
diff --git a/include/libvirt/libvirt-storage.h 
b/include/libvirt/libvirt-storage.h
index af1c50b9f1..aaad4a3da1 100644
--- a/include/libvirt/libvirt-storage.h
+++ b/include/libvirt/libvirt-storage.h
@@ -437,6 +437,7 @@ const char* virStorageVolGetKey 
(virStorageVolPtr vol);
 typedef enum {
 VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA = 1 << 0, /* (Since: 1.0.1) */
 VIR_STORAGE_VOL_CREATE_REFLINK = 1 << 1, /* perform a btrfs lightweight 
copy (Since: 1.2.13) */
+VIR_STORAGE_VOL_CREATE_VALIDATE = 1 << 2, /* Validate the XML document 
against schema (Since: 8.10.0) */
 } virStorageVolCreateFlags;

 virStorageVolPtrvirStorageVolCreateXML  (virStoragePoolPtr 
pool,
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index 300a0aa8e5..4f23481180 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -380,6 +380,10 @@ static const vshCmdOptDef opts_vol_create[] = {
  .type = VSH_OT_BOOL,
  .help = N_("preallocate metadata (for qcow2 instead of full allocation)")
 },
+{.name = "validate",
+ .type = VSH_OT_BOOL,
+ .help = N_("validate the XML against the schema")
+},
 {.name = NULL}
 };

@@ -395,6 +399,9 @@ cmdVolCreate(vshControl *ctl, const vshCmd *cmd)
 if (vshCommandOptBool(cmd, "prealloc-metadata"))
 flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;

+if (vshCommandOptBool(cmd, "validate"))
+flags |= VIR_STORAGE_VOL_CREATE_VALIDATE;
+
 if (!(pool = virshCommandOptPool(ctl, cmd, "pool", NULL)))
 return false;

@@ -446,6 +453,10 @@ static const vshCmdOptDef opts_vol_create_from[] = {
  .type = VSH_OT_BOOL,
  .help = N_("use btrfs COW lightweight copy")
 },
+{.name = "validate",
+ .type = VSH_OT_BOOL,
+ .help = N_("validate the XML against the schema")
+},
 {.name = NULL}
 };

@@ -468,6 +479,9 @@ cmdVolCreateFrom(vshControl *ctl, const vshCmd *cmd)
 if (vshCommandOptBool(cmd, "reflink"))
 flags |= VIR_STORAGE_VOL_CREATE_REFLINK;

+if (vshCommandOptBool(cmd, "validate"))
+flags |= VIR_STORAGE_VOL_CREATE_VALIDATE;
+
 if (vshCommandOptStringReq(ctl, cmd, "file", ) < 0)
 return false;

-- 
2.37.3

[PATCH 4/6] conf: storage: Add support for validating storage vol XML to virStorageVolDefParse

2022-10-20 Thread Peter Krempa

Introduce the VIR_VOL_XML_PARSE_VALIDATE parser flag and wire it up into
the validator.

Signed-off-by: Peter Krempa 
---
 src/conf/storage_conf.c | 3 ++-
 src/conf/storage_conf.h | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 0f4fe1451e..72c53872cb 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1407,9 +1407,10 @@ virStorageVolDefParse(virStoragePoolDef *pool,
 {
 g_autoptr(xmlDoc) xml = NULL;
 g_autoptr(xmlXPathContext) ctxt = NULL;
+bool validate = flags & VIR_VOL_XML_PARSE_VALIDATE;

 if (!(xml = virXMLParse(filename, xmlStr, _("(storage_volume_definition)"),
-"volume", , NULL, false)))
+"volume", , "storagevol.rng", validate)))
 return NULL;

 return virStorageVolDefParseXML(pool, ctxt, flags);
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index bbfdbc2f2f..fc67957cfe 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -284,6 +284,8 @@ typedef enum {
 VIR_VOL_XML_PARSE_NO_CAPACITY  = 1 << 0,
 /* do not require volume capacity if the volume has a backing store */
 VIR_VOL_XML_PARSE_OPT_CAPACITY = 1 << 1,
+/* validate the XML against the RNG schema */
+VIR_VOL_XML_PARSE_VALIDATE = 1 << 2,
 } virStorageVolDefParseFlags;

 virStorageVolDef *
-- 
2.37.3

[PATCH 6/6] storage|test|vbox: Implement support for validating storage volume XMLs

2022-10-20 Thread Peter Krempa

Signed-off-by: Peter Krempa 
---
 src/storage/storage_driver.c | 18 ++
 src/test/test_driver.c   | 16 
 src/vbox/vbox_storage.c  |  8 ++--
 3 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index c25d9ca1ad..d90c1c9ee8 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1877,8 +1877,13 @@ storageVolCreateXML(virStoragePoolPtr pool,
 virStorageBackend *backend;
 virStorageVolPtr vol = NULL, newvol = NULL;
 g_autoptr(virStorageVolDef) voldef = NULL;
+unsigned int parseFlags = VIR_VOL_XML_PARSE_OPT_CAPACITY;

-virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL);
+virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA |
+  VIR_STORAGE_VOL_CREATE_VALIDATE, NULL);
+
+if (flags & VIR_STORAGE_VOL_CREATE_VALIDATE)
+parseFlags |= VIR_VOL_XML_PARSE_VALIDATE;

 if (!(obj = virStoragePoolObjFromStoragePool(pool)))
 return NULL;
@@ -1893,7 +1898,7 @@ storageVolCreateXML(virStoragePoolPtr pool,
 if ((backend = virStorageBackendForType(def->type)) == NULL)
 goto cleanup;

-voldef = virStorageVolDefParse(def, xmldesc, NULL, 
VIR_VOL_XML_PARSE_OPT_CAPACITY);
+voldef = virStorageVolDefParse(def, xmldesc, NULL, parseFlags);
 if (voldef == NULL)
 goto cleanup;

@@ -2012,11 +2017,16 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool,
 virStorageVolPtr vol = NULL;
 int buildret;
 g_autoptr(virStorageVolDef) voldef = NULL;
+unsigned int parseFlags = VIR_VOL_XML_PARSE_NO_CAPACITY;

 virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA |
-  VIR_STORAGE_VOL_CREATE_REFLINK,
+  VIR_STORAGE_VOL_CREATE_REFLINK |
+  VIR_STORAGE_VOL_CREATE_VALIDATE,
   NULL);

+if (flags & VIR_STORAGE_VOL_CREATE_VALIDATE)
+parseFlags |= VIR_VOL_XML_PARSE_VALIDATE;
+
 obj = virStoragePoolObjFindByUUID(driver->pools, pool->uuid);
 if (obj && STRNEQ(pool->name, volsrc->pool)) {
 virObjectUnlock(obj);
@@ -2066,7 +2076,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool,
 goto cleanup;
 }

-voldef = virStorageVolDefParse(def, xmldesc, NULL, 
VIR_VOL_XML_PARSE_NO_CAPACITY);
+voldef = virStorageVolDefParse(def, xmldesc, NULL, parseFlags);
 if (voldef == NULL)
 goto cleanup;

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 58c2a02561..87c7d8cf65 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -7182,14 +7182,18 @@ testStorageVolCreateXML(virStoragePoolPtr pool,
 virStoragePoolDef *def;
 virStorageVolPtr ret = NULL;
 g_autoptr(virStorageVolDef) privvol = NULL;
+unsigned int parseFlags = 0;

-virCheckFlags(0, NULL);
+virCheckFlags(VIR_STORAGE_VOL_CREATE_VALIDATE, NULL);
+
+if (flags & VIR_STORAGE_VOL_CREATE_VALIDATE)
+parseFlags |= VIR_VOL_XML_PARSE_VALIDATE;

 if (!(obj = testStoragePoolObjFindActiveByName(privconn, pool->name)))
 return NULL;
 def = virStoragePoolObjGetDef(obj);

-privvol = virStorageVolDefParse(def, xmldesc, NULL, 0);
+privvol = virStorageVolDefParse(def, xmldesc, NULL, parseFlags);
 if (privvol == NULL)
 goto cleanup;

@@ -7241,14 +7245,18 @@ testStorageVolCreateXMLFrom(virStoragePoolPtr pool,
 virStorageVolDef *origvol = NULL;
 virStorageVolPtr ret = NULL;
 g_autoptr(virStorageVolDef) privvol = NULL;
+unsigned int parseFlags = 0;

-virCheckFlags(0, NULL);
+virCheckFlags(VIR_STORAGE_VOL_CREATE_VALIDATE, NULL);
+
+if (flags & VIR_STORAGE_VOL_CREATE_VALIDATE)
+parseFlags |= VIR_VOL_XML_PARSE_VALIDATE;

 if (!(obj = testStoragePoolObjFindActiveByName(privconn, pool->name)))
 return NULL;
 def = virStoragePoolObjGetDef(obj);

-privvol = virStorageVolDefParse(def, xmldesc, NULL, 0);
+privvol = virStorageVolDefParse(def, xmldesc, NULL, parseFlags);
 if (privvol == NULL)
 goto cleanup;

diff --git a/src/vbox/vbox_storage.c b/src/vbox/vbox_storage.c
index 7d1cee562f..f6ede700f9 100644
--- a/src/vbox/vbox_storage.c
+++ b/src/vbox/vbox_storage.c
@@ -409,11 +409,15 @@ vboxStorageVolCreateXML(virStoragePoolPtr pool,
 virStorageVolPtr ret = NULL;
 g_autoptr(virStorageVolDef) def = NULL;
 g_autofree char *homedir = NULL;
+unsigned int parseFlags = 0;

 if (!data->vboxObj)
 return ret;

-virCheckFlags(0, NULL);
+virCheckFlags(VIR_STORAGE_VOL_CREATE_VALIDATE, NULL);
+
+if (flags & VIR_STORAGE_VOL_CREATE_VALIDATE)
+parseFlags |= VIR_VOL_XML_PARSE_VALIDATE;

 /* since there is currently one default pool now
  * and virStorageVolDefFormat() just checks it type
@@ -423,7 +427,7 @@ vboxStorageVolCreateXML(virStoragePoolPtr pool,
 memset(, 0, sizeof(poolDef));
 poolDef.type = VIR_STORAGE_POOL_DIR;

-if ((def =

[PATCH 3/6] nodedev|test: Implement support for validating node device XMLs

2022-10-20 Thread Peter Krempa

Signed-off-by: Peter Krempa 
---
 src/node_device/node_device_driver.c | 10 ++
 src/test/test_driver.c   |  6 --
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index d067234ab3..0fdfe1db96 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -876,8 +876,9 @@ nodeDeviceCreateXML(virConnectPtr conn,
 g_autofree char *wwpn = NULL;
 virNodeDevicePtr device = NULL;
 const char *virt_type = NULL;
+bool validate = flags & VIR_NODE_DEVICE_CREATE_XML_VALIDATE;

-virCheckFlags(0, NULL);
+virCheckFlags(VIR_NODE_DEVICE_CREATE_XML_VALIDATE, NULL);

 if (nodeDeviceInitWait() < 0)
 return NULL;
@@ -885,7 +886,7 @@ nodeDeviceCreateXML(virConnectPtr conn,
 virt_type  = virConnectGetType(conn);

 if (!(def = virNodeDeviceDefParse(xmlDesc, NULL, CREATE_DEVICE, virt_type,
-  >parserCallbacks, NULL, false)))
+  >parserCallbacks, NULL, 
validate)))
 return NULL;

 if (virNodeDeviceCreateXMLEnsureACL(conn, def) < 0)
@@ -1397,8 +1398,9 @@ nodeDeviceDefineXML(virConnect *conn,
 const char *virt_type = NULL;
 g_autofree char *uuid = NULL;
 g_autofree char *name = NULL;
+bool validate = flags & VIR_NODE_DEVICE_DEFINE_XML_VALIDATE;

-virCheckFlags(0, NULL);
+virCheckFlags(VIR_NODE_DEVICE_DEFINE_XML_VALIDATE, NULL);

 if (nodeDeviceInitWait() < 0)
 return NULL;
@@ -1406,7 +1408,7 @@ nodeDeviceDefineXML(virConnect *conn,
 virt_type  = virConnectGetType(conn);

 if (!(def = virNodeDeviceDefParse(xmlDesc, NULL, CREATE_DEVICE, virt_type,
-  >parserCallbacks, NULL, false)))
+  >parserCallbacks, NULL, 
validate)))
 return NULL;

 if (virNodeDeviceDefineXMLEnsureACL(conn, def) < 0)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 9b397e66b1..58c2a02561 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -7688,10 +7688,12 @@ testNodeDeviceCreateXML(virConnectPtr conn,
 virNodeDeviceDef *objdef;
 g_autofree char *wwnn = NULL;
 g_autofree char *wwpn = NULL;
+bool validate = flags & VIR_NODE_DEVICE_CREATE_XML_VALIDATE;

-virCheckFlags(0, NULL);
+virCheckFlags(VIR_NODE_DEVICE_CREATE_XML_VALIDATE, NULL);

-if (!(def = virNodeDeviceDefParse(xmlDesc, NULL, CREATE_DEVICE, NULL, 
NULL, NULL, false)))
+if (!(def = virNodeDeviceDefParse(xmlDesc, NULL, CREATE_DEVICE, NULL, NULL,
+  NULL, validate)))
 goto cleanup;

 /* We run this simply for validation - it essentially validates that
-- 
2.37.3

[PATCH 1/6] conf: node_device: Add 'validate' argument to virNodeDeviceDefParse

2022-10-20 Thread Peter Krempa

Allow callers to request XML validation against the schema. All callers
for now pass 'false'.

Signed-off-by: Peter Krempa 
---
 src/conf/node_device_conf.c  | 5 +++--
 src/conf/node_device_conf.h  | 3 ++-
 src/hypervisor/domain_driver.c   | 6 +++---
 src/node_device/node_device_driver.c | 4 ++--
 src/test/test_driver.c   | 4 ++--
 tests/nodedevmdevctltest.c   | 4 ++--
 tests/nodedevxml2xmltest.c   | 3 ++-
 7 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index bdfbbab434..f5283a77b3 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -2479,14 +2479,15 @@ virNodeDeviceDefParse(const char *str,
   int create,
   const char *virt_type,
   virNodeDeviceDefParserCallbacks *parserCallbacks,
-  void *opaque)
+  void *opaque,
+  bool validate)
 {
 g_autoptr(xmlDoc) xml = NULL;
 g_autoptr(xmlXPathContext) ctxt = NULL;
 g_autoptr(virNodeDeviceDef) def = NULL;

 if (!(xml = virXMLParse(filename, str, _("(node_device_definition)"),
-"device", , NULL, false)))
+"device", , "nodedev.rng", validate)))
 return NULL;

 if (!(def = virNodeDeviceDefParseXML(ctxt, create, virt_type)))
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index a556358632..2b2c8f797e 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -382,7 +382,8 @@ virNodeDeviceDefParse(const char *str,
   int create,
   const char *virt_type,
   virNodeDeviceDefParserCallbacks *parserCallbacks,
-  void *opaque);
+  void *opaque,
+  bool validate);

 virNodeDeviceDef *
 virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c
index c154f00eea..d020b94921 100644
--- a/src/hypervisor/domain_driver.c
+++ b/src/hypervisor/domain_driver.c
@@ -395,7 +395,7 @@ virDomainDriverNodeDeviceReset(virNodeDevicePtr dev,
 if (!xml)
 return -1;

-def = virNodeDeviceDefParse(xml, NULL, EXISTING_DEVICE, NULL, NULL, NULL);
+def = virNodeDeviceDefParse(xml, NULL, EXISTING_DEVICE, NULL, NULL, NULL, 
false);
 if (!def)
 return -1;

@@ -440,7 +440,7 @@ virDomainDriverNodeDeviceReAttach(virNodeDevicePtr dev,
 if (!xml)
 return -1;

-def = virNodeDeviceDefParse(xml, NULL, EXISTING_DEVICE, NULL, NULL, NULL);
+def = virNodeDeviceDefParse(xml, NULL, EXISTING_DEVICE, NULL, NULL, NULL, 
false);
 if (!def)
 return -1;

@@ -488,7 +488,7 @@ virDomainDriverNodeDeviceDetachFlags(virNodeDevicePtr dev,
 if (!xml)
 return -1;

-def = virNodeDeviceDefParse(xml, NULL, EXISTING_DEVICE, NULL, NULL, NULL);
+def = virNodeDeviceDefParse(xml, NULL, EXISTING_DEVICE, NULL, NULL, NULL, 
false);
 if (!def)
 return -1;

diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index 8e93b0dd6f..d067234ab3 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -885,7 +885,7 @@ nodeDeviceCreateXML(virConnectPtr conn,
 virt_type  = virConnectGetType(conn);

 if (!(def = virNodeDeviceDefParse(xmlDesc, NULL, CREATE_DEVICE, virt_type,
-  >parserCallbacks, NULL)))
+  >parserCallbacks, NULL, false)))
 return NULL;

 if (virNodeDeviceCreateXMLEnsureACL(conn, def) < 0)
@@ -1406,7 +1406,7 @@ nodeDeviceDefineXML(virConnect *conn,
 virt_type  = virConnectGetType(conn);

 if (!(def = virNodeDeviceDefParse(xmlDesc, NULL, CREATE_DEVICE, virt_type,
-  >parserCallbacks, NULL)))
+  >parserCallbacks, NULL, false)))
 return NULL;

 if (virNodeDeviceDefineXMLEnsureACL(conn, def) < 0)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 67c70de11d..9b397e66b1 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -7629,7 +7629,7 @@ testNodeDeviceMockCreateVport(testDriver *driver,
 if (!xml)
 goto cleanup;

-if (!(def = virNodeDeviceDefParse(xml, NULL, EXISTING_DEVICE, NULL, NULL, 
NULL)))
+if (!(def = virNodeDeviceDefParse(xml, NULL, EXISTING_DEVICE, NULL, NULL, 
NULL, false)))
 goto cleanup;

 VIR_FREE(def->name);
@@ -7691,7 +7691,7 @@ testNodeDeviceCreateXML(virConnectPtr conn,

 virCheckFlags(0, NULL);

-if (!(def = virNodeDeviceDefParse(xmlDesc, NULL, CREATE_DEVICE, NULL, 
NULL, NULL)))
+if (!(def = virNodeDeviceDefParse(xmlDesc, NULL, CREATE_DEVICE, NULL, 
NULL, NULL, false)))
 goto cleanup;

 /* We run this simply for

[PATCH 2/6] nodedev: Add VIR_NODE_DEVICE_(CREATE|DEFINE)_XML_VALIDATE flags

2022-10-20 Thread Peter Krempa

The node device APIs which get XML from the user don't yet support XML
validation flags. Introduce virNodeDeviceCreateXMLFlags and
virNodeDeviceDefineXMLFlags with the appropriate flags and add virsh
support for the new flags.

Signed-off-by: Peter Krempa 
---
 docs/manpages/virsh.rst   | 10 --
 include/libvirt/libvirt-nodedev.h | 19 +++
 src/libvirt-nodedev.c |  4 ++--
 tools/virsh-nodedev.c | 20 ++--
 4 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 61fcb2e9ca..532d4e779c 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -5203,7 +5203,7 @@ nodedev-create

 ::

-   nodedev-create FILE
+   nodedev-create FILE [--validate]

 Create a device on the host node that can then be assigned to virtual
 machines. Normally, libvirt is able to automatically determine which
@@ -5211,6 +5211,9 @@ host nodes are available for use, but this allows 
registration of
 host hardware that libvirt did not automatically detect.  *file*
 contains xml for a top-level  description of a node device.

+If *--validate* is specified, validates the format of the XML document against
+an internal RNG schema.
+

 nodedev-destroy
 ---
@@ -5234,11 +5237,14 @@ nodedev-define

 ::

-   nodedev-define FILE
+   nodedev-define FILE [--validate]

 Define an inactive persistent device or modify an existing persistent one from
 the XML *FILE*.

+If *--validate* is specified, validates the format of the XML document against
+an internal RNG schema.
+

 nodedev-undefine
 
diff --git a/include/libvirt/libvirt-nodedev.h 
b/include/libvirt/libvirt-nodedev.h
index 4fccd3f614..428b0d722f 100644
--- a/include/libvirt/libvirt-nodedev.h
+++ b/include/libvirt/libvirt-nodedev.h
@@ -130,12 +130,31 @@ int 
virNodeDeviceDetachFlags(virNodeDevicePtr dev,
 int virNodeDeviceReAttach   (virNodeDevicePtr dev);
 int virNodeDeviceReset  (virNodeDevicePtr dev);

+/**
+ * virNodeDeviceCreateXMLFlags:
+ *
+ * Since: 8.10.0
+ */
+typedef enum {
+VIR_NODE_DEVICE_CREATE_XML_VALIDATE = 1 << 0, /* Validate the XML document 
against schema (Since: 8.10.0) */
+} virNodeDeviceCreateXMLFlags;
+
 virNodeDevicePtrvirNodeDeviceCreateXML  (virConnectPtr conn,
  const char *xmlDesc,
  unsigned int flags);

 int virNodeDeviceDestroy(virNodeDevicePtr dev);

+
+/**
+ * virNodeDeviceDefineXMLFlags:
+ *
+ * Since: 8.10.0
+ */
+typedef enum {
+VIR_NODE_DEVICE_DEFINE_XML_VALIDATE = 1 << 0, /* Validate the XML document 
against schema (Since: 8.10.0) */
+} virNodeDeviceDefineXMLFlags;
+
 virNodeDevicePtr virNodeDeviceDefineXML(virConnectPtr conn,
 const char *xmlDesc,
 unsigned int flags);
diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c
index d5a24ea2ef..1b7dee113e 100644
--- a/src/libvirt-nodedev.c
+++ b/src/libvirt-nodedev.c
@@ -694,7 +694,7 @@ virNodeDeviceReset(virNodeDevicePtr dev)
  * virNodeDeviceCreateXML:
  * @conn: pointer to the hypervisor connection
  * @xmlDesc: string containing an XML description of the device to be created
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: bitwise-OR of supported virNodeDeviceCreateXMLFlags
  *
  * Create a new device on the VM host machine, for example, virtual
  * HBAs created using vport_create.
@@ -778,7 +778,7 @@ virNodeDeviceDestroy(virNodeDevicePtr dev)
  * virNodeDeviceDefineXML:
  * @conn: pointer to the hypervisor connection
  * @xmlDesc: string containing an XML description of the device to be defined
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: bitwise-OR of supported virNodeDeviceDefineXMLFlags
  *
  * Define a new device on the VM host machine, for example, a mediated device
  *
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index 2adcad9c10..5dbec65367 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -50,6 +50,10 @@ static const vshCmdInfo info_node_device_create[] = {
 static const vshCmdOptDef opts_node_device_create[] = {
 VIRSH_COMMON_OPT_FILE(N_("file containing an XML description "
  "of the device")),
+{.name = "validate",
+ .type = VSH_OT_BOOL,
+ .help = N_("validate the XML against the schema")
+},
 {.name = NULL}
 };

@@ -60,6 +64,7 @@ cmdNodeDeviceCreate(vshControl *ctl, const vshCmd *cmd)
 const char *from = NULL;
 g_autofree char *buffer = NULL;
 virshControl *priv = ctl->privData;
+unsigned int flags = 0;

 if (vshCommandOptStringReq(ctl, cmd, "file", ) < 0)
 return false;
@@ -67,7 +72,10 @@ cmdNodeDeviceCreate(vshControl *ctl, const vshCmd *cmd)
 if

[PATCH 0/6] Implement XML validation feature for node devices and storage volumes

2022-10-20 Thread Peter Krempa

In the last round of adding support for built-in validation the node
device APIs and storage volume creation were not covered.

Note that due to the close freeze date I've already marked the APIs for
v8.10.

Peter Krempa (6):
  conf: node_device: Add 'validate' argument to virNodeDeviceDefParse
  nodedev: Add VIR_NODE_DEVICE_(CREATE|DEFINE)_XML_VALIDATE flags
  nodedev|test: Implement support for validating node device XMLs
  conf: storage: Add support for validating storage vol XML to
virStorageVolDefParse
  storage: Add VIR_STORAGE_VOL_CREATE_VALIDATE flag
  storage|test|vbox: Implement support for validating storage volume
XMLs

 docs/manpages/virsh.rst  | 19 +++
 include/libvirt/libvirt-nodedev.h| 19 +++
 include/libvirt/libvirt-storage.h|  1 +
 src/conf/node_device_conf.c  |  5 +++--
 src/conf/node_device_conf.h  |  3 ++-
 src/conf/storage_conf.c  |  3 ++-
 src/conf/storage_conf.h  |  2 ++
 src/hypervisor/domain_driver.c   |  6 +++---
 src/libvirt-nodedev.c|  4 ++--
 src/node_device/node_device_driver.c | 10 ++
 src/storage/storage_driver.c | 18 ++
 src/test/test_driver.c   | 24 +---
 src/vbox/vbox_storage.c  |  8 ++--
 tests/nodedevmdevctltest.c   |  4 ++--
 tests/nodedevxml2xmltest.c   |  3 ++-
 tools/virsh-nodedev.c| 20 ++--
 tools/virsh-volume.c | 14 ++
 17 files changed, 128 insertions(+), 35 deletions(-)

-- 
2.37.3

[libvirt PATCH] qemu: do not attempt to pass unopened vsock FD

2022-10-20 Thread Ján Tomko

On normal vm startup, we open a file descriptor
for the vsock device in qemuProcessPrepareHost.

However, when doing domxml-to-native, no file descriptors are open.

Only pass the fd if it's not -1, to make domxml-to-native work.

https://bugzilla.redhat.com/show_bug.cgi?id=1777212

Signed-off-by: Ján Tomko 
---
Technically a v2 of:
https://listman.redhat.com/archives/libvir-list/2021-July/msg00803.html

I did not look at other cases, but IIRC the ones converted to use qemuFD*
wrappers should handle missing FDs gracefully.

 src/qemu/qemu_command.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 150824f2e1..bbbde57c0f 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9717,7 +9717,8 @@ qemuBuildVsockCommandLine(virCommand *cmd,
 if (!(devprops = qemuBuildVsockDevProps(def, vsock, qemuCaps, "")))
 return -1;
 
-virCommandPassFD(cmd, priv->vhostfd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+if (priv->vhostfd != -1)
+virCommandPassFD(cmd, priv->vhostfd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
 priv->vhostfd = -1;
 
 if (qemuCommandAddExtDevice(cmd, >info, def, qemuCaps) < 0)
-- 
2.37.3

Re: [libvirt PATCH] build: drop LINUGAS sorting rule

2022-10-20 Thread Daniel P . Berrangé

On Thu, Oct 20, 2022 at 03:06:23PM +0200, Ján Tomko wrote:
> A recent merge request from Weblate adding a new file fails syntax-check
> because it adds a new language at the end of LINGUAS, instead of sorting
> it alphabetically. Rather than trying to work around it, drop this
> pointless rule.
> 
> Signed-off-by: Ján Tomko 
> ---
>  build-aux/syntax-check.mk | 8 
>  1 file changed, 8 deletions(-)

Reverts 8d160b7979ad2f5dcac79ffe85f79e5c3ae330d6

https://listman.redhat.com/archives/libvir-list/2022-June/232487.html

guess we forgot to actually follow up that time.

Reviewed-by: Daniel P. Berrangé 


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 :|

[libvirt PATCH] build: drop LINUGAS sorting rule

2022-10-20 Thread Ján Tomko

A recent merge request from Weblate adding a new file fails syntax-check
because it adds a new language at the end of LINGUAS, instead of sorting
it alphabetically. Rather than trying to work around it, drop this
pointless rule.

Signed-off-by: Ján Tomko 
---
 build-aux/syntax-check.mk | 8 
 1 file changed, 8 deletions(-)

diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index e35c2be734..68cd9dff5f 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -1257,14 +1257,6 @@ sc_po_check:
  rm -f $@-1 $@-2; \
fi
 
-linguas_file = $(top_srcdir)/po/LINGUAS
-
-sc_linguas_sorting:
-   @sort -u $(linguas_file) > $@-1; \
-   diff -u -L $(linguas_file) -L $(linguas_file) $(linguas_file) $@-1 \
- || { echo "$(linguas_file) is not sorted correctly" 1>&2; exit 1; }; \
-   rm -f $@-1
-
 # #if WITH_... will evaluate to false for any non numeric string.
 # That would be flagged by using -Wundef, however gnulib currently
 # tests many undefined macros, and so we can't enable that option.
-- 
2.37.3

Re: [libvirt PATCH 00/12] tools: provide virt-qemu-sev-validate for SEV(-ES) launch attestation

2022-10-20 Thread Daniel P . Berrangé

On Thu, Oct 20, 2022 at 08:18:20AM -0400, Cole Robinson wrote:
> On 10/18/22 5:22 AM, Daniel P. Berrangé wrote:
> > On Sun, Oct 16, 2022 at 03:06:17PM -0400, Cole Robinson wrote:
> >> On 10/7/22 7:42 AM, Daniel P. Berrangé wrote:
> >>> The libvirt QEMU driver provides all the functionality required for
> >>> launching a guest on AMD SEV(-ES) platforms, with a configuration
> >>> that enables attestation of the launch measurement. The documentation
> >>> for how to actually perform an attestation is severely lacking and
> >>> not suitable for mere mortals to understand. IOW, someone trying to
> >>> implement attestation is in for a world of pain and suffering.
> >>>
> >>> This series doesn't fix the documentation problem, but it does
> >>> provide a reference implementation of a tool for performing
> >>> attestation of SEV(-ES) guests in the context of libvirt / KVM.
> >>>
> >>> There will be other tools and libraries that implement attestation
> >>> logic too, but this tool is likely somewhat unique in its usage of
> >>> libvirt. Now for a attestation to be trustworthy you don't want to
> >>> perform it on the hypervisor host, since the goal is to prove that
> >>> the hypervisor has not acted maliciously. None the less it is still
> >>> beneficial to have libvirt integration to some extent.
> >>>
> >>> When running this tool on a remote (trusted) host, it can connect
> >>> to the libvirt hypervisor and fetch the data provided by the
> >>> virDomainLaunchSecurityInfo API, which is safe to trust as the
> >>> key pieces are cryptographically measured.
> >>>
> >>> Attestation is a complex problem though and it is very easy to
> >>> screw up and feed the wrong information and then waste hours trying
> >>> to figure out what piece was wrong, to cause the hash digest to
> >>> change. For debugging such problems, you can thus tell the tool
> >>> to operate insecurely, by querying libvirt for almost all of the
> >>> configuration information required to determine the expected
> >>> measurement. By comparing these results,to the results obtained
> >>> in offline mode it helps narrow down where the mistake lies.
> >>>
> >>> So I view this tool as being useful in a number of ways:
> >>>
> >>>  * Quality assurance engineers needing to test libvirt/QEMU/KVM
> >>>get a simple and reliable tool for automating tests with.
> >>>
> >>>  * Users running simple libvirt deployments without any large
> >>>management stack, get a standalone tool for attestation
> >>>they can rely on.
> >>>
> >>>  * Developers writing/integrating attestation support into
> >>>management stacks above libvirt, get a reference against
> >>>which they can debug their own tools.
> >>>
> >>>  * Users wanting to demonstrate the core SEV/SEV-ES functionality
> >>>get a simple and reliable tool to illustrate the core concepts
> >>>involved.
> >>>
> >>> Since I didn't fancy writing such complex logic in C, this tool is
> >>> a python3 program. As such, we don't want to include it in the
> >>> main libvirt-client RPM, nor any other existing RPM. THus, this
> >>> series puts it in a new libvirt-client-qemu RPM which, through no
> >>> co-inicidence at all, is the same RPM I invented a few days ago to
> >>> hold the virt-qemu-qmp-proxy command.
> >>>
> >>> Note, people will have already seen an earlier version of this
> >>> tool I hacked up some months ago. This code is very significantly
> >>> changed since that earlier version, to make it more maintainable,
> >>> and simpler to use (especially for SEV-ES) but the general theme
> >>> is still the same.
> >>>
> >>> Daniel P. Berrangé (12):
> >>>   build-aux: only forbid gethostname in C files
> >>>   tools: support validating SEV firmware boot measurements
> >>>   tools: load guest config from libvirt
> >>>   tools: support validating SEV direct kernel boot measurements
> >>>   tools: load direct kernel config from libvirt
> >>>   tools: support validating SEV-ES initial vCPU state measurements
> >>>   tools: support automatically constructing SEV-ES vCPU state
> >>>   tools: load CPU count and CPU SKU from libvirt
> >>>   tools: support generating SEV secret injection tables
> >>>   docs/kbase: describe attestation for SEV guests
> >>>   scripts: add systemtap script for capturing SEV-ES VMSA
> >>>   docs/manpages: add checklist of problems for SEV attestation
> >>
> >>
> >> After the fix I mentioned on patch 7, the script worked for my sev,
> >> sev-es, sev-es + kernel setup, with vmsa passed in and vmsa generated.
> >>
> >> Attached is some pylint output, nothing really serious:
> > 
> > We've not run pylint on libvirt tree historically, but I wonder
> > if we should introduce it to check all our build scripts too.
> 
> Pylint needs a lot of hand holding. Every distro upgrade you'll hit new
> issues, want to exclude more checks etc. I never looked into how
> projects use it for CI gating, seems kinda exhausting unless you only
> enable specific checks.

I consider this similar to

Re: [libvirt PATCH 00/12] tools: provide virt-qemu-sev-validate for SEV(-ES) launch attestation

2022-10-20 Thread Cole Robinson

On 10/18/22 5:22 AM, Daniel P. Berrangé wrote:
> On Sun, Oct 16, 2022 at 03:06:17PM -0400, Cole Robinson wrote:
>> On 10/7/22 7:42 AM, Daniel P. Berrangé wrote:
>>> The libvirt QEMU driver provides all the functionality required for
>>> launching a guest on AMD SEV(-ES) platforms, with a configuration
>>> that enables attestation of the launch measurement. The documentation
>>> for how to actually perform an attestation is severely lacking and
>>> not suitable for mere mortals to understand. IOW, someone trying to
>>> implement attestation is in for a world of pain and suffering.
>>>
>>> This series doesn't fix the documentation problem, but it does
>>> provide a reference implementation of a tool for performing
>>> attestation of SEV(-ES) guests in the context of libvirt / KVM.
>>>
>>> There will be other tools and libraries that implement attestation
>>> logic too, but this tool is likely somewhat unique in its usage of
>>> libvirt. Now for a attestation to be trustworthy you don't want to
>>> perform it on the hypervisor host, since the goal is to prove that
>>> the hypervisor has not acted maliciously. None the less it is still
>>> beneficial to have libvirt integration to some extent.
>>>
>>> When running this tool on a remote (trusted) host, it can connect
>>> to the libvirt hypervisor and fetch the data provided by the
>>> virDomainLaunchSecurityInfo API, which is safe to trust as the
>>> key pieces are cryptographically measured.
>>>
>>> Attestation is a complex problem though and it is very easy to
>>> screw up and feed the wrong information and then waste hours trying
>>> to figure out what piece was wrong, to cause the hash digest to
>>> change. For debugging such problems, you can thus tell the tool
>>> to operate insecurely, by querying libvirt for almost all of the
>>> configuration information required to determine the expected
>>> measurement. By comparing these results,to the results obtained
>>> in offline mode it helps narrow down where the mistake lies.
>>>
>>> So I view this tool as being useful in a number of ways:
>>>
>>>  * Quality assurance engineers needing to test libvirt/QEMU/KVM
>>>get a simple and reliable tool for automating tests with.
>>>
>>>  * Users running simple libvirt deployments without any large
>>>management stack, get a standalone tool for attestation
>>>they can rely on.
>>>
>>>  * Developers writing/integrating attestation support into
>>>management stacks above libvirt, get a reference against
>>>which they can debug their own tools.
>>>
>>>  * Users wanting to demonstrate the core SEV/SEV-ES functionality
>>>get a simple and reliable tool to illustrate the core concepts
>>>involved.
>>>
>>> Since I didn't fancy writing such complex logic in C, this tool is
>>> a python3 program. As such, we don't want to include it in the
>>> main libvirt-client RPM, nor any other existing RPM. THus, this
>>> series puts it in a new libvirt-client-qemu RPM which, through no
>>> co-inicidence at all, is the same RPM I invented a few days ago to
>>> hold the virt-qemu-qmp-proxy command.
>>>
>>> Note, people will have already seen an earlier version of this
>>> tool I hacked up some months ago. This code is very significantly
>>> changed since that earlier version, to make it more maintainable,
>>> and simpler to use (especially for SEV-ES) but the general theme
>>> is still the same.
>>>
>>> Daniel P. Berrangé (12):
>>>   build-aux: only forbid gethostname in C files
>>>   tools: support validating SEV firmware boot measurements
>>>   tools: load guest config from libvirt
>>>   tools: support validating SEV direct kernel boot measurements
>>>   tools: load direct kernel config from libvirt
>>>   tools: support validating SEV-ES initial vCPU state measurements
>>>   tools: support automatically constructing SEV-ES vCPU state
>>>   tools: load CPU count and CPU SKU from libvirt
>>>   tools: support generating SEV secret injection tables
>>>   docs/kbase: describe attestation for SEV guests
>>>   scripts: add systemtap script for capturing SEV-ES VMSA
>>>   docs/manpages: add checklist of problems for SEV attestation
>>
>>
>> After the fix I mentioned on patch 7, the script worked for my sev,
>> sev-es, sev-es + kernel setup, with vmsa passed in and vmsa generated.
>>
>> Attached is some pylint output, nothing really serious:
> 
> We've not run pylint on libvirt tree historically, but I wonder
> if we should introduce it to check all our build scripts too.

Pylint needs a lot of hand holding. Every distro upgrade you'll hit new
issues, want to exclude more checks etc. I never looked into how
projects use it for CI gating, seems kinda exhausting unless you only
enable specific checks.

- Cole

Re: [libvirt PATCH 02/12] tools: support validating SEV firmware boot measurements

2022-10-20 Thread Cole Robinson

On 10/20/22 8:11 AM, Cole Robinson wrote:
> On 10/18/22 5:15 AM, Daniel P. Berrangé wrote:
>> On Sun, Oct 16, 2022 at 02:54:47PM -0400, Cole Robinson wrote:
>>> On 10/7/22 7:42 AM, Daniel P. Berrangé wrote:
 The virt-qemu-sev-validate program will compare a reported SEV/SEV-ES
 domain launch measurement, to a computed launch measurement. This
 determines whether the domain has been tampered with during launch.

 This initial implementation requires all inputs to be provided
 explicitly, and as such can run completely offline, without any
 connection to libvirt.

 The tool is placed in the libvirt-client-qemu sub-RPM since it is
 specific to the QEMU driver.

 Signed-off-by: Daniel P. Berrangé 
>>>
 +try:
 +check_usage(args)
 +
 +attest(args)
 +
 +sys.exit(0)
 +except AttestationFailedException as e:
 +if not args.quiet:
 +print("ERROR: %s" % e, file=sys.stderr)
 +sys.exit(1)
 +except UnsupportedUsageException as e:
 +if not args.quiet:
 +print("ERROR: %s" % e, file=sys.stderr)
 +sys.exit(2)
 +except Exception as e:
 +if args.debug:
 +traceback.print_tb(e.__traceback__)
 +if not args.quiet:
 +print("ERROR: %s" % e, file=sys.stderr)
 +sys.exit(3)
>>>
>>> This only tracebacks on --debug for an unexpected error. I think it's
>>> more useful to have --debug always print backtrace. It helped me
>>> debugging usage of the script
>>
>> Ok, I can do that.
>>
>> Do you recall what sort of problems required you to be looking at
>> the debug output ?  Wondering if there's anything we can do to make
>> it more foolproof for less knowledgable users ?
>>
> 
> I was running the script from git, but against an older running libvirtd
> which did not support the cpu  XML, and the error didn't call
> that out specifically. I thought about suggesting an explicit error for
> that case but I think it's unlikely to happen in the real world.
> 
Hmm I see now that I did actually suggest this elsewhere :P

- Cole

Re: [libvirt PATCH 02/12] tools: support validating SEV firmware boot measurements

2022-10-20 Thread Cole Robinson

On 10/18/22 5:15 AM, Daniel P. Berrangé wrote:
> On Sun, Oct 16, 2022 at 02:54:47PM -0400, Cole Robinson wrote:
>> On 10/7/22 7:42 AM, Daniel P. Berrangé wrote:
>>> The virt-qemu-sev-validate program will compare a reported SEV/SEV-ES
>>> domain launch measurement, to a computed launch measurement. This
>>> determines whether the domain has been tampered with during launch.
>>>
>>> This initial implementation requires all inputs to be provided
>>> explicitly, and as such can run completely offline, without any
>>> connection to libvirt.
>>>
>>> The tool is placed in the libvirt-client-qemu sub-RPM since it is
>>> specific to the QEMU driver.
>>>
>>> Signed-off-by: Daniel P. Berrangé 
>>
>>> +try:
>>> +check_usage(args)
>>> +
>>> +attest(args)
>>> +
>>> +sys.exit(0)
>>> +except AttestationFailedException as e:
>>> +if not args.quiet:
>>> +print("ERROR: %s" % e, file=sys.stderr)
>>> +sys.exit(1)
>>> +except UnsupportedUsageException as e:
>>> +if not args.quiet:
>>> +print("ERROR: %s" % e, file=sys.stderr)
>>> +sys.exit(2)
>>> +except Exception as e:
>>> +if args.debug:
>>> +traceback.print_tb(e.__traceback__)
>>> +if not args.quiet:
>>> +print("ERROR: %s" % e, file=sys.stderr)
>>> +sys.exit(3)
>>
>> This only tracebacks on --debug for an unexpected error. I think it's
>> more useful to have --debug always print backtrace. It helped me
>> debugging usage of the script
> 
> Ok, I can do that.
> 
> Do you recall what sort of problems required you to be looking at
> the debug output ?  Wondering if there's anything we can do to make
> it more foolproof for less knowledgable users ?
> 

I was running the script from git, but against an older running libvirtd
which did not support the cpu  XML, and the error didn't call
that out specifically. I thought about suggesting an explicit error for
that case but I think it's unlikely to happen in the real world.

- Cole

[libvirt PATCH v3 12/18] qemu: include nbdkit state in private xml

[libvirt PATCH v3 00/18] Use nbdkit for http/ftp/ssh network drives in libvirt

[libvirt PATCH v3 07/18] qemu: use file cache for nbdkit caps

[libvirt PATCH v3 16/18] qemu: pass sensitive data to nbdkit via pipe

[libvirt PATCH v3 18/18] qemu: Monitor nbdkit process for exit

[libvirt PATCH v3 17/18] qemu: add test for authenticating a https network disk

[libvirt PATCH v3 11/18] qemu: split qemuDomainSecretStorageSourcePrepare

[libvirt PATCH v3 10/18] tests: add ability to test various nbdkit capabilities

[libvirt PATCH v3 14/18] tests: add tests for nbdkit invocation

[libvirt PATCH v3 15/18] util: make virCommandSetSendBuffer testable

[libvirt PATCH v3 13/18] qemu: use nbdkit to serve network disks if available

[libvirt PATCH v3 03/18] qemu: expand nbdkit capabilities

[libvirt PATCH v3 05/18] qemu: implement basic virFileCache for nbdkit caps

[libvirt PATCH v3 09/18] qemu: add functions to start and stop nbdkit

[libvirt PATCH v3 02/18] qemu: Add functions for determining nbdkit availability

[libvirt PATCH v3 04/18] util: Allow virFileCache data to be any GObject

[libvirt PATCH v3 08/18] qemu: Add qemuNbdkitProcess

[libvirt PATCH v3 06/18] qemu: implement persistent file cache for nbdkit caps

[libvirt PATCH v3 01/18] schema: allow 'ssh' as a protocol for network disks

Re: [libvirt PATCH] qemu: do not attempt to pass unopened vsock FD

[PATCH 5/6] storage: Add VIR_STORAGE_VOL_CREATE_VALIDATE flag

[PATCH 4/6] conf: storage: Add support for validating storage vol XML to virStorageVolDefParse

[PATCH 6/6] storage|test|vbox: Implement support for validating storage volume XMLs

[PATCH 3/6] nodedev|test: Implement support for validating node device XMLs

[PATCH 1/6] conf: node_device: Add 'validate' argument to virNodeDeviceDefParse

[PATCH 2/6] nodedev: Add VIR_NODE_DEVICE_(CREATE|DEFINE)_XML_VALIDATE flags

[PATCH 0/6] Implement XML validation feature for node devices and storage volumes

[libvirt PATCH] qemu: do not attempt to pass unopened vsock FD

Re: [libvirt PATCH] build: drop LINUGAS sorting rule

[libvirt PATCH] build: drop LINUGAS sorting rule

Re: [libvirt PATCH 00/12] tools: provide virt-qemu-sev-validate for SEV(-ES) launch attestation

Re: [libvirt PATCH 00/12] tools: provide virt-qemu-sev-validate for SEV(-ES) launch attestation

Re: [libvirt PATCH 02/12] tools: support validating SEV firmware boot measurements

Re: [libvirt PATCH 02/12] tools: support validating SEV firmware boot measurements

34 matches

Site Navigation

Mail list logo

Footer information