[libvirt PATCH v8 33/37] qemu: implement keyfile auth for ssh disks with nbdkit

[Jonathon Jongsma]

For ssh disks that are served by nbdkit, we can support logging in with
an ssh key file. Pass the path to the configured key file and the
username to the nbdkit process.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 src/conf/domain_conf.c| 30 +
 src/conf/storage_source_conf.c|  2 ++
 src/conf/storage_source_conf.h|  5 ++-
 src/qemu/qemu_nbdkit.c| 15 +++--
 .../disk-network-ssh-key.args.disk0   |  9 +
 .../disk-network-ssh.args.disk2   |  9 +
 tests/qemunbdkittest.c|  1 +
 .../qemuxml2argvdata/disk-network-ssh-key.xml | 33 +++
 8 files changed, 93 insertions(+), 11 deletions(-)
 create mode 100644 tests/qemunbdkitdata/disk-network-ssh-key.args.disk0
 create mode 100644 tests/qemunbdkitdata/disk-network-ssh.args.disk2
 create mode 100644 tests/qemuxml2argvdata/disk-network-ssh-key.xml

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 842b6404b5..929e115bce 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7268,10 +7268,18 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
 return -1;
 }
 }
-if (src->protocol == VIR_STORAGE_NET_PROTOCOL_SSH &&
-(tmpnode = virXPathNode("./knownHosts", ctxt))) {
-if (!(src->ssh_known_hosts_file = virXMLPropStringRequired(tmpnode, 
"path")))
-return -1;
+if (src->protocol == VIR_STORAGE_NET_PROTOCOL_SSH) {
+if ((tmpnode = virXPathNode("./knownHosts", ctxt))) {
+if (!(src->ssh_known_hosts_file = 
virXMLPropStringRequired(tmpnode, "path")))
+return -1;
+}
+if ((tmpnode = virXPathNode("./identity", ctxt))) {
+if (!(src->ssh_user = virXMLPropStringRequired(tmpnode, 
"username")))
+return -1;
+
+if (!(src->ssh_keyfile = virXMLPropStringRequired(tmpnode, 
"keyfile")))
+return -1;
+}
 }
 
 return 0;
@@ -22280,8 +22288,18 @@ virDomainDiskSourceFormatNetwork(virBuffer *attrBuf,
 if (src->timeout)
 virBufferAsprintf(childBuf, "\n", 
src->timeout);
 
-if (src->protocol == VIR_STORAGE_NET_PROTOCOL_SSH && 
src->ssh_known_hosts_file)
-virBufferEscapeString(childBuf, "\n", 
src->ssh_known_hosts_file);
+if (src->protocol == VIR_STORAGE_NET_PROTOCOL_SSH) {
+if (src->ssh_known_hosts_file)
+virBufferEscapeString(childBuf, "\n", 
src->ssh_known_hosts_file);
+if (src->ssh_keyfile) {
+virBufferAddLit(childBuf, "ssh_user);
+virBufferEscapeString(childBuf, " keyfile='%s'", src->ssh_keyfile);
+
+virBufferAddLit(childBuf, "/>\n");
+}
+}
 }
 
 
diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c
index 906bc36a9b..ce9c1f66c2 100644
--- a/src/conf/storage_source_conf.c
+++ b/src/conf/storage_source_conf.c
@@ -896,6 +896,7 @@ virStorageSourceCopy(const virStorageSource *src,
 def->ssh_host_key_check_disabled = src->ssh_host_key_check_disabled;
 def->ssh_user = g_strdup(src->ssh_user);
 def->ssh_known_hosts_file = g_strdup(src->ssh_known_hosts_file);
+def->ssh_keyfile = g_strdup(src->ssh_keyfile);
 
 def->nfs_user = g_strdup(src->nfs_user);
 def->nfs_group = g_strdup(src->nfs_group);
@@ -1172,6 +1173,7 @@ virStorageSourceClear(virStorageSource *def)
 
 VIR_FREE(def->ssh_user);
 VIR_FREE(def->ssh_known_hosts_file);
+VIR_FREE(def->ssh_keyfile);
 
 VIR_FREE(def->nfs_user);
 VIR_FREE(def->nfs_group);
diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h
index 8a9c7d07e2..8c805664af 100644
--- a/src/conf/storage_source_conf.h
+++ b/src/conf/storage_source_conf.h
@@ -406,12 +406,11 @@ struct _virStorageSource {
 
 bool hostcdrom; /* backing device is a cdrom */
 
-/* passthrough variables for the ssh driver which we don't handle properly 
*/
-/* these must not be used apart from formatting the output JSON in the 
qemu driver */
+/* ssh variables */
 char *ssh_user;
 bool ssh_host_key_check_disabled;
-/* additional ssh variables */
 char *ssh_known_hosts_file;
+char *ssh_keyfile;
 
 /* nfs_user and nfs_group store the strings passed in by the user for NFS 
params.
  * nfs_uid and nfs_gid represent the converted/looked up ID numbers which 
are used
diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index afac71e21a..0a6c7962b0 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -1049,8 +1049,12 @@ qemuNbdkitProcessBuildCommandSSH(qemuNbdkitProcess *proc,
 if (proc->source->auth) {
 if (qemuNbdkitProcessBuildCommandAuth(proc->source->auth, cmd) < 0)
 return -1;
-} else if (proc->source->ssh_user) {
-virCommandAddArgPair(cmd, "user", proc->source->ssh_user);
+} else {
+if (proc->source->ssh_keyfile)
+

[libvirt PATCH v8 15/37] tests: add ability to test various nbdkit capabilities

[Jonathon Jongsma]

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

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 src/qemu/qemu_nbdkit.c   | 20 +---
 tests/qemuxml2argvtest.c | 11 +++
 tests/testutilsqemu.c| 26 ++
 tests/testutilsqemu.h|  4 
 4 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index 2d70e72c42..81861bae4a 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -290,10 +290,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;
@@ -334,9 +340,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 1b76b32812..d64c21ae17 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -611,6 +611,14 @@ testCompareXMLToArgv(const void *data)
 if (qemuTestCapsCacheInsert(driver.qemuCapsCache, info->qemuCaps) < 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)))
@@ -831,6 +839,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 fdbad16abe..9a607ab5a3 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -50,6 +50,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
@@ -288,6 +292,7 @@ void qemuTestDriverFree(virQEMUDriver *driver)
 virObjectUnref(driver->caps);
 virObjectUnref(driver->config);
 virObjectUnref(driver->securityManager);
+g_clear_object(>nbdkitCapsCache);
 
 virCPUDefFree(cpuDefault);
 virCPUDefFree(cpuHaswell);
@@ -487,6 +492,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;
@@ -780,6 +791,14 @@ testQemuInfoSetArgs(struct testQemuInfo *info,
 ignore_value(virBitmapSetBit(info->args.fakeCapsDel, flag));
 break;
 
+case ARG_NBDKIT_CAPS:
+if (!(info->args.fakeNbdkitCaps))
+info->args.fakeNbdkitCaps = 
virBitmapNew(QEMU_NBDKIT_CAPS_LAST);
+
+while ((flag = va_arg(argptr, int)) < QEMU_NBDKIT_CAPS_LAST)
+ignore_value(virBitmapSetBit(info->args.fakeNbdkitCaps, flag));
+break;
+
 case ARG_GIC:
 info->args.gic = va_arg(argptr, int);
 break;
@@ -1054,6 +1073,11 @@ testQemuInfoInitArgs(struct testQemuInfo *info)
 for (cap = -1; (cap = 

[libvirt PATCH v8 21/37] util: make virCommandSetSendBuffer testable

[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 
Reviewed-by: Peter Krempa 
---
 src/libvirt_private.syms  |  1 +
 src/util/vircommand.c | 17 +
 src/util/vircommand.h |  8 
 src/util/vircommandpriv.h |  4 
 4 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1e3e407097..e4da5388e7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2117,6 +2117,7 @@ virCommandNewArgs;
 virCommandNewVAList;
 virCommandNonblockingFDs;
 virCommandPassFD;
+virCommandPeekSendBuffers;
 virCommandRawStatus;
 virCommandRequireHandshake;
 virCommandRun;
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 899d413dd2..7d7ce4297e 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -78,14 +78,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  */
 
@@ -3515,3 +3507,12 @@ 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 d51449ac90..9bcdce35b9 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.41.0

[libvirt PATCH v8 12/37] qemu: Extract qemuDomainLogContext into a new file

[Jonathon Jongsma]

This will allow us to use it for nbdkit logging in upcoming commits.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 po/POTFILES|   1 +
 src/qemu/meson.build   |   1 +
 src/qemu/qemu_domain.c | 247 ++
 src/qemu/qemu_domain.h |  27 +---
 src/qemu/qemu_logcontext.c | 264 +
 src/qemu/qemu_logcontext.h |  38 ++
 src/qemu/qemu_process.c|  44 +++
 7 files changed, 346 insertions(+), 276 deletions(-)
 create mode 100644 src/qemu/qemu_logcontext.c
 create mode 100644 src/qemu/qemu_logcontext.h

diff --git a/po/POTFILES b/po/POTFILES
index 6167f98ac5..3a51aea5cb 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -174,6 +174,7 @@ src/qemu/qemu_hostdev.c
 src/qemu/qemu_hotplug.c
 src/qemu/qemu_interface.c
 src/qemu/qemu_interop_config.c
+src/qemu/qemu_logcontext.c
 src/qemu/qemu_migration.c
 src/qemu/qemu_migration_cookie.c
 src/qemu/qemu_migration_params.c
diff --git a/src/qemu/meson.build b/src/qemu/meson.build
index 9be6996195..6d7a1bfbb0 100644
--- a/src/qemu/meson.build
+++ b/src/qemu/meson.build
@@ -21,6 +21,7 @@ qemu_driver_sources = [
   'qemu_hotplug.c',
   'qemu_interface.c',
   'qemu_interop_config.c',
+  'qemu_logcontext.c',
   'qemu_migration.c',
   'qemu_migration_cookie.c',
   'qemu_migration_params.c',
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d79f9879df..23608f95bd 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -455,21 +455,8 @@ qemuDomainObjFromDomain(virDomainPtr domain)
 }
 
 
-struct _qemuDomainLogContext {
-GObject parent;
-
-int writefd;
-int readfd; /* Only used if manager == NULL */
-off_t pos;
-ino_t inode; /* Only used if manager != NULL */
-char *path;
-virLogManager *manager;
-};
-
-G_DEFINE_TYPE(qemuDomainLogContext, qemu_domain_log_context, G_TYPE_OBJECT);
 static virClass *qemuDomainSaveCookieClass;
 
-static void qemuDomainLogContextFinalize(GObject *obj);
 static void qemuDomainSaveCookieDispose(void *obj);
 
 
@@ -482,32 +469,8 @@ qemuDomainOnceInit(void)
 return 0;
 }
 
-static void qemu_domain_log_context_init(qemuDomainLogContext *logctxt 
G_GNUC_UNUSED)
-{
-}
-
-static void qemu_domain_log_context_class_init(qemuDomainLogContextClass 
*klass)
-{
-GObjectClass *obj = G_OBJECT_CLASS(klass);
-
-obj->finalize = qemuDomainLogContextFinalize;
-}
-
 VIR_ONCE_GLOBAL_INIT(qemuDomain);
 
-static void
-qemuDomainLogContextFinalize(GObject *object)
-{
-qemuDomainLogContext *ctxt = QEMU_DOMAIN_LOG_CONTEXT(object);
-VIR_DEBUG("ctxt=%p", ctxt);
-
-virLogManagerFree(ctxt->manager);
-VIR_FREE(ctxt->path);
-VIR_FORCE_CLOSE(ctxt->writefd);
-VIR_FORCE_CLOSE(ctxt->readfd);
-G_OBJECT_CLASS(qemu_domain_log_context_parent_class)->finalize(object);
-}
-
 /* qemuDomainGetMasterKeyFilePath:
  * @libDir: Directory path to domain lib files
  *
@@ -6882,7 +6845,7 @@ static void G_GNUC_PRINTF(5, 6)
 qemuDomainObjTaintMsg(virQEMUDriver *driver,
   virDomainObj *obj,
   virDomainTaintFlags taint,
-  qemuDomainLogContext *logCtxt,
+  qemuLogContext *logCtxt,
   const char *fmt, ...)
 {
 virErrorPtr orig_err = NULL;
@@ -6935,12 +6898,12 @@ qemuDomainObjTaintMsg(virQEMUDriver *driver,
 goto cleanup;
 
 if (logCtxt) {
-rc = qemuDomainLogContextWrite(logCtxt,
-   "%s: Domain id=%d is tainted: 
%s%s%s%s\n",
-   timestamp,
-   obj->def->id,
-   virDomainTaintTypeToString(taint),
-   extraprefix, extramsg, extrasuffix);
+rc = qemuLogContextWrite(logCtxt,
+ "%s: Domain id=%d is tainted: %s%s%s%s\n",
+ timestamp,
+ obj->def->id,
+ virDomainTaintTypeToString(taint),
+ extraprefix, extramsg, extrasuffix);
 } else {
 rc = qemuDomainLogAppendMessage(driver, obj,
 "%s: Domain id=%d is tainted: 
%s%s%s%s\n",
@@ -6961,7 +6924,7 @@ qemuDomainObjTaintMsg(virQEMUDriver *driver,
 void qemuDomainObjTaint(virQEMUDriver *driver,
 virDomainObj *obj,
 virDomainTaintFlags taint,
-qemuDomainLogContext *logCtxt)
+qemuLogContext *logCtxt)
 {
 qemuDomainObjTaintMsg(driver, obj, taint, logCtxt, NULL);
 qemuDomainSaveStatus(obj);
@@ -6970,7 +6933,7 @@ void qemuDomainObjTaint(virQEMUDriver *driver,
 static void
 qemuDomainObjCheckMachineTaint(virQEMUDriver *driver,
virDomainObj *obj,
-   qemuDomainLogContext *logCtxt)
+

[libvirt PATCH v8 35/37] qemu: implement ssh-agent auth for ssh disks with nbdkit

[Jonathon Jongsma]

It's not possible to use password-protected ssh keys directly with
libvirt because libvirt doesn't have any way to prompt a user for the
password. To accomodate password-protected key files, an administrator
can add these keys to an ssh agent and then configure the domain with
the path to the ssh-agent socket.

Note that this requires an administrator or management app to
configure the ssh-agent with an appropriate socket path and add the
necessary keys to it. In addition, it does not currently work with
selinux enabled. The ssh-agent socket would need a label that libvirt
would be allowed to access rather than unconfined_t.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 src/conf/domain_conf.c  | 14 --
 src/conf/storage_source_conf.c  |  2 ++
 src/conf/storage_source_conf.h  |  1 +
 src/qemu/qemu_nbdkit.c  | 10 ++
 .../disk-network-ssh-key.args.disk0 |  6 +++---
 .../disk-network-ssh-key.args.disk1 |  9 +
 tests/qemuxml2argvdata/disk-network-ssh-key.xml | 17 ++---
 7 files changed, 51 insertions(+), 8 deletions(-)
 create mode 100644 tests/qemunbdkitdata/disk-network-ssh-key.args.disk1

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 929e115bce..398f40d2be 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7277,8 +7277,17 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
 if (!(src->ssh_user = virXMLPropStringRequired(tmpnode, 
"username")))
 return -1;
 
-if (!(src->ssh_keyfile = virXMLPropStringRequired(tmpnode, 
"keyfile")))
+/* optional path to an ssh key file */
+src->ssh_keyfile = virXMLPropString(tmpnode, "keyfile");
+
+/* optional ssh-agent socket location */
+src->ssh_agent = virXMLPropString(tmpnode, "agentsock");
+if (!src->ssh_keyfile && !src->ssh_agent) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("element '%1$s' requires either 'keyfile' or 
'agentsock' attribute"),
+   tmpnode->name);
 return -1;
+}
 }
 }
 
@@ -22291,11 +22300,12 @@ virDomainDiskSourceFormatNetwork(virBuffer *attrBuf,
 if (src->protocol == VIR_STORAGE_NET_PROTOCOL_SSH) {
 if (src->ssh_known_hosts_file)
 virBufferEscapeString(childBuf, "\n", 
src->ssh_known_hosts_file);
-if (src->ssh_keyfile) {
+if (src->ssh_keyfile || src->ssh_agent) {
 virBufferAddLit(childBuf, "ssh_user);
 virBufferEscapeString(childBuf, " keyfile='%s'", src->ssh_keyfile);
+virBufferEscapeString(childBuf, " agentsock='%s'", src->ssh_agent);
 
 virBufferAddLit(childBuf, "/>\n");
 }
diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c
index ce9c1f66c2..cafa031dfe 100644
--- a/src/conf/storage_source_conf.c
+++ b/src/conf/storage_source_conf.c
@@ -897,6 +897,7 @@ virStorageSourceCopy(const virStorageSource *src,
 def->ssh_user = g_strdup(src->ssh_user);
 def->ssh_known_hosts_file = g_strdup(src->ssh_known_hosts_file);
 def->ssh_keyfile = g_strdup(src->ssh_keyfile);
+def->ssh_agent = g_strdup(src->ssh_agent);
 
 def->nfs_user = g_strdup(src->nfs_user);
 def->nfs_group = g_strdup(src->nfs_group);
@@ -1174,6 +1175,7 @@ virStorageSourceClear(virStorageSource *def)
 VIR_FREE(def->ssh_user);
 VIR_FREE(def->ssh_known_hosts_file);
 VIR_FREE(def->ssh_keyfile);
+VIR_FREE(def->ssh_agent);
 
 VIR_FREE(def->nfs_user);
 VIR_FREE(def->nfs_group);
diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h
index 8c805664af..061faa66cb 100644
--- a/src/conf/storage_source_conf.h
+++ b/src/conf/storage_source_conf.h
@@ -411,6 +411,7 @@ struct _virStorageSource {
 bool ssh_host_key_check_disabled;
 char *ssh_known_hosts_file;
 char *ssh_keyfile;
+char *ssh_agent;
 
 /* nfs_user and nfs_group store the strings passed in by the user for NFS 
params.
  * nfs_uid and nfs_gid represent the converted/looked up ID numbers which 
are used
diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index 0a6c7962b0..66b09cd240 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -1057,6 +1057,9 @@ qemuNbdkitProcessBuildCommandSSH(qemuNbdkitProcess *proc,
 virCommandAddArgPair(cmd, "user", proc->source->ssh_user);
 }
 
+if (proc->source->ssh_agent)
+virCommandAddEnvPair(cmd, "SSH_AUTH_SOCK", proc->source->ssh_agent);
+
 if (proc->source->ssh_host_key_check_disabled)
 virCommandAddArgPair(cmd, "verify-remote-host", "false");
 
@@ -1179,6 +1182,10 @@ qemuNbdkitProcessStart(qemuNbdkitProcess *proc,
 qemuSecurityDomainSetPathLabel(driver, vm, proc->source->ssh_keyfile, 
false) < 0)
 goto error;
 
+if 

[libvirt PATCH v8 34/37] schema: add ssh-agent configuration for ssh disks

[Jonathon Jongsma]

Add the ability to specify a path to a ssh-agent socket in order to use
the ssh-agent to authenticate to remote ssh disks. Example
configuration:




...

...


Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 docs/formatdomain.rst | 13 -
 src/conf/schemas/domaincommon.rng | 11 ---
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index baa2fdce7d..714fee4fbf 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -3007,11 +3007,14 @@ paravirtualized driver is specified via the ``disk`` 
element.
   are intended to be default, then the entire element may be omitted.
 
   When using an ``ssh`` protocol, this element is used to enable
-  authentication via ssh keys. In this configuration, the element has two
-  attributes. The ``username`` attribute specifies the name of the user on
-  the remote server and the ``keyfile`` attribute specifies the path to the
-  keyfile. Note that this only works for ssh keys that are not
-  password-protected.
+  authentication via ssh keys. In this configuration, the element has three
+  possible attributes. The ``username`` attribute is required and specifies
+  the name of the user on the remote server. ssh keys can be specified in
+  one of two ways. The first way is by adding them to an ssh-agent and
+  providing the path to the ssh-agent socket in the ``agentsock``
+  attribute. This method works for ssh keys with or without password
+  protection. Alternatively, for ssh keys without a password, the ssh key
+  can be specified directly by setting the ``keyfile`` attribute.
``reconnect``
   For disk type ``vhostuser`` configures reconnect timeout if the 
connection
   is lost. This is set with the two mandatory attributes ``enabled`` and
diff --git a/src/conf/schemas/domaincommon.rng 
b/src/conf/schemas/domaincommon.rng
index 47c5ee2a31..d8dd1b8c69 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -2186,9 +2186,14 @@
 
   
 
-
-  
-
+
+  
+
+  
+  
+
+  
+
   
 
   
-- 
2.41.0

[libvirt PATCH v8 29/37] qemu: implement password auth for ssh disks with nbdkit

[Jonathon Jongsma]

For ssh disks that are served by nbdkit, lookup the password from the
configured secret and securely pass it to the nbdkit process using fd
passing.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 src/qemu/qemu_nbdkit.c| 84 ++-
 .../disk-network-ssh-password.args.disk0  |  8 ++
 ...k-network-ssh-password.args.disk0.pipe.778 |  1 +
 .../disk-network-ssh.args.disk1   |  8 ++
 .../disk-network-ssh.args.disk1.pipe.778  |  1 +
 tests/qemunbdkittest.c|  1 +
 ...sk-network-ssh-password.x86_64-latest.args | 35 
 .../disk-network-ssh-password.xml | 34 
 tests/qemuxml2argvtest.c  |  1 +
 9 files changed, 134 insertions(+), 39 deletions(-)
 create mode 100644 tests/qemunbdkitdata/disk-network-ssh-password.args.disk0
 create mode 100644 
tests/qemunbdkitdata/disk-network-ssh-password.args.disk0.pipe.778
 create mode 100644 tests/qemunbdkitdata/disk-network-ssh.args.disk1
 create mode 100644 tests/qemunbdkitdata/disk-network-ssh.args.disk1.pipe.778
 create mode 100644 
tests/qemuxml2argvdata/disk-network-ssh-password.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/disk-network-ssh-password.xml

diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index db86a18321..4cd91e282b 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -950,6 +950,43 @@ qemuNbdkitCommandPassDataByPipe(virCommand *cmd,
 }
 
 
+static int
+qemuNbdkitProcessBuildCommandAuth(virStorageAuthDef *authdef,
+  virCommand *cmd)
+{
+g_autoptr(virConnect) conn = NULL;
+g_autofree uint8_t *secret = NULL;
+size_t secretlen = 0;
+int secrettype;
+
+if (!authdef)
+return 0;
+
+if ((secrettype = virSecretUsageTypeFromString(authdef->secrettype)) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("invalid secret type %1$s"),
+   authdef->secrettype);
+return -1;
+}
+
+conn = virGetConnectSecret();
+if (virSecretGetSecretString(conn,
+ >seclookupdef,
+ secrettype,
+ ,
+ ) < 0)
+return -1;
+
+virCommandAddArgPair(cmd, "user", authdef->username);
+
+if (qemuNbdkitCommandPassDataByPipe(cmd, "password",
+, secretlen) < 0)
+return -1;
+
+return 0;
+}
+
+
 static int
 qemuNbdkitProcessBuildCommandCurl(qemuNbdkitProcess *proc,
   virCommand *cmd)
@@ -968,37 +1005,8 @@ qemuNbdkitProcessBuildCommandCurl(qemuNbdkitProcess *proc,
 }
 virCommandAddArgPair(cmd, "url", uristring);
 
-if (proc->source->auth) {
-g_autoptr(virConnect) conn = virGetConnectSecret();
-g_autofree uint8_t *secret = NULL;
-size_t secretlen = 0;
-int secrettype;
-virStorageAuthDef *authdef = proc->source->auth;
-
-virCommandAddArgPair(cmd, "user",
- proc->source->auth->username);
-
-if ((secrettype = 
virSecretUsageTypeFromString(proc->source->auth->secrettype)) < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("invalid secret type %1$s"),
-   proc->source->auth->secrettype);
-return -1;
-}
-
-if (virSecretGetSecretString(conn,
- >seclookupdef,
- secrettype,
- ,
- ) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("failed to get auth secret for storage"));
-return -1;
-}
-
-if (qemuNbdkitCommandPassDataByPipe(cmd, "password",
-, secretlen) < 0)
-return -1;
-}
+if (proc->source->auth && 
qemuNbdkitProcessBuildCommandAuth(proc->source->auth, cmd) < 0)
+return -1;
 
 /* Create a pipe to send the cookies to the nbdkit process. */
 if (proc->source->ncookies) {
@@ -1027,7 +1035,6 @@ 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);
 
@@ -1038,13 +1045,12 @@ qemuNbdkitProcessBuildCommandSSH(qemuNbdkitProcess 
*proc,
 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 

[libvirt PATCH v8 37/37] ci: add libnbd to build

[Jonathon Jongsma]

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Erik Skultety 
---
 ci/buildenv/almalinux-8.sh| 1 +
 ci/buildenv/centos-stream-8.sh| 1 +
 ci/buildenv/centos-stream-9.sh| 1 +
 ci/buildenv/debian-12-cross-aarch64.sh| 1 +
 ci/buildenv/debian-12-cross-armv6l.sh | 1 +
 ci/buildenv/debian-12-cross-armv7l.sh | 1 +
 ci/buildenv/debian-12-cross-i686.sh   | 1 +
 ci/buildenv/debian-12-cross-mips64el.sh   | 1 +
 ci/buildenv/debian-12-cross-mipsel.sh | 1 +
 ci/buildenv/debian-12-cross-ppc64le.sh| 1 +
 ci/buildenv/debian-12-cross-s390x.sh  | 1 +
 ci/buildenv/debian-12.sh  | 1 +
 ci/buildenv/debian-sid-cross-aarch64.sh   | 1 +
 ci/buildenv/debian-sid-cross-armv6l.sh| 1 +
 ci/buildenv/debian-sid-cross-armv7l.sh| 1 +
 ci/buildenv/debian-sid-cross-i686.sh  | 1 +
 ci/buildenv/debian-sid-cross-mips64el.sh  | 1 +
 ci/buildenv/debian-sid-cross-mipsel.sh| 1 +
 ci/buildenv/debian-sid-cross-ppc64le.sh   | 1 +
 ci/buildenv/debian-sid-cross-s390x.sh | 1 +
 ci/buildenv/debian-sid.sh | 1 +
 ci/buildenv/fedora-37.sh  | 1 +
 ci/buildenv/fedora-38-cross-mingw32.sh| 1 +
 ci/buildenv/fedora-38-cross-mingw64.sh| 1 +
 ci/buildenv/fedora-38.sh  | 1 +
 ci/buildenv/fedora-rawhide-cross-mingw32.sh   | 1 +
 ci/buildenv/fedora-rawhide-cross-mingw64.sh   | 1 +
 ci/buildenv/fedora-rawhide.sh | 1 +
 ci/buildenv/opensuse-leap-15.sh   | 1 +
 ci/buildenv/opensuse-tumbleweed.sh| 1 +
 ci/buildenv/ubuntu-2204.sh| 1 +
 ci/containers/almalinux-8.Dockerfile  | 1 +
 ci/containers/centos-stream-8.Dockerfile  | 1 +
 ci/containers/centos-stream-9.Dockerfile  | 1 +
 ci/containers/debian-12-cross-aarch64.Dockerfile  | 1 +
 ci/containers/debian-12-cross-armv6l.Dockerfile   | 1 +
 ci/containers/debian-12-cross-armv7l.Dockerfile   | 1 +
 ci/containers/debian-12-cross-i686.Dockerfile | 1 +
 ci/containers/debian-12-cross-mips64el.Dockerfile | 1 +
 ci/containers/debian-12-cross-mipsel.Dockerfile   | 1 +
 ci/containers/debian-12-cross-ppc64le.Dockerfile  | 1 +
 ci/containers/debian-12-cross-s390x.Dockerfile| 1 +
 ci/containers/debian-12.Dockerfile| 1 +
 ci/containers/debian-sid-cross-aarch64.Dockerfile | 1 +
 ci/containers/debian-sid-cross-armv6l.Dockerfile  | 1 +
 ci/containers/debian-sid-cross-armv7l.Dockerfile  | 1 +
 ci/containers/debian-sid-cross-i686.Dockerfile| 1 +
 ci/containers/debian-sid-cross-mips64el.Dockerfile| 1 +
 ci/containers/debian-sid-cross-mipsel.Dockerfile  | 1 +
 ci/containers/debian-sid-cross-ppc64le.Dockerfile | 1 +
 ci/containers/debian-sid-cross-s390x.Dockerfile   | 1 +
 ci/containers/debian-sid.Dockerfile   | 1 +
 ci/containers/fedora-37.Dockerfile| 1 +
 ci/containers/fedora-38-cross-mingw32.Dockerfile  | 1 +
 ci/containers/fedora-38-cross-mingw64.Dockerfile  | 1 +
 ci/containers/fedora-38.Dockerfile| 1 +
 ci/containers/fedora-rawhide-cross-mingw32.Dockerfile | 1 +
 ci/containers/fedora-rawhide-cross-mingw64.Dockerfile | 1 +
 ci/containers/fedora-rawhide.Dockerfile   | 1 +
 ci/containers/opensuse-leap-15.Dockerfile | 1 +
 ci/containers/opensuse-tumbleweed.Dockerfile  | 1 +
 ci/containers/ubuntu-2204.Dockerfile  | 1 +
 ci/lcitool/projects/libvirt.yml   | 1 +
 63 files changed, 63 insertions(+)

diff --git a/ci/buildenv/almalinux-8.sh b/ci/buildenv/almalinux-8.sh
index 086b4d946b..e3c0909557 100644
--- a/ci/buildenv/almalinux-8.sh
+++ b/ci/buildenv/almalinux-8.sh
@@ -45,6 +45,7 @@ function install_buildenv() {
 libcap-ng-devel \
 libcurl-devel \
 libiscsi-devel \
+libnbd-devel \
 libnl3-devel \
 libpcap-devel \
 libpciaccess-devel \
diff --git a/ci/buildenv/centos-stream-8.sh b/ci/buildenv/centos-stream-8.sh
index 6b3de502df..f6de8ee7f7 100644
--- a/ci/buildenv/centos-stream-8.sh
+++ b/ci/buildenv/centos-stream-8.sh
@@ -46,6 +46,7 @@ function install_buildenv() {
 libcap-ng-devel \
 libcurl-devel \
 libiscsi-devel \
+libnbd-devel \
 libnl3-devel \
 libpcap-devel \
 libpciaccess-devel \
diff --git a/ci/buildenv/centos-stream-9.sh b/ci/buildenv/centos-stream-9.sh
index 454e1f6322..d6657425a2 100644
--- a/ci/buildenv/centos-stream-9.sh
+++ b/ci/buildenv/centos-stream-9.sh
@@ -43,6 +43,7 @@ function install_buildenv() {
   

[libvirt PATCH v8 10/37] qemu: add functions to start and stop nbdkit

[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 
Reviewed-by: Peter Krempa 
---
 src/qemu/qemu_nbdkit.c | 250 +
 src/qemu/qemu_nbdkit.h |  10 ++
 2 files changed, 260 insertions(+)

diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index 9a2a89224d..6bf962d0f1 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -24,6 +24,8 @@
 #include "virerror.h"
 #include "virlog.h"
 #include "virpidfile.h"
+#include "virsecureerase.h"
+#include "virtime.h"
 #include "virutil.h"
 #include "qemu_block.h"
 #include "qemu_conf.h"
@@ -666,6 +668,168 @@ 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");
+if (proc->source->protocol == VIR_STORAGE_NET_PROTOCOL_HTTP) {
+/* allow http to be upgraded to https via e.g. redirect */
+virCommandAddArgPair(cmd, "protocols", "http,https");
+} else {
+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;
+virStorageAuthDef *authdef = proc->source->auth;
+
+virCommandAddArgPair(cmd, "user",
+ proc->source->auth->username);
+
+if ((secrettype = 
virSecretUsageTypeFromString(proc->source->auth->secrettype)) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("invalid secret type %1$s"),
+   proc->source->auth->secrettype);
+return -1;
+}
+
+if (virSecretGetSecretString(conn,
+ >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);
+virSecureErase(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"));
+
+virSecureEraseString(password);
+
+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,
+ "--unix",
+ proc->socketfile,
+ "--foreground",
+ 

[libvirt PATCH v8 14/37] qemu: log error output from nbdkit

[Jonathon Jongsma]

log stderr and stdout from nbdkit into its own log so that
nbdkit-related issues can be debugged more easily.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 src/qemu/qemu_nbdkit.c | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index 6bf962d0f1..2d70e72c42 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -852,12 +852,23 @@ qemuNbdkitProcessStart(qemuNbdkitProcess *proc,
 virTimeBackOffVar timebackoff;
 g_autoptr(virURI) uri = NULL;
 g_autofree char *uristring = NULL;
+g_autofree char *basename = g_strdup_printf("%s-nbdkit-%i", vm->def->name, 
proc->source->id);
+int logfd = -1;
+g_autoptr(qemuLogContext) logContext = NULL;
 
 if (!(cmd = qemuNbdkitProcessBuildCommand(proc)))
 return -1;
 
+if (!(logContext = qemuLogContextNew(driver, vm, basename))) {
+virLastErrorPrefixMessage("%s", _("can't connect to virtlogd"));
+return -1;
+}
+
+logfd = qemuLogContextGetWriteFD(logContext);
+
 VIR_DEBUG("starting nbdkit process for %s", proc->source->nodestorage);
-virCommandSetErrorBuffer(cmd, );
+virCommandSetErrorFD(cmd, );
+virCommandSetOutputFD(cmd, );
 virCommandSetPidFile(cmd, proc->pidfile);
 
 if (qemuExtDeviceLogCommand(driver, vm, cmd, "nbdkit") < 0)
@@ -899,6 +910,9 @@ qemuNbdkitProcessStart(qemuNbdkitProcess *proc,
 if ((uri = qemuBlockStorageSourceGetURI(proc->source)))
 uristring = virURIFormat(uri);
 
+if (qemuLogContextReadFiltered(logContext, , 1024) < 0)
+VIR_WARN("Unable to read from nbdkit log");
+
 virReportError(VIR_ERR_OPERATION_FAILED,
_("Failed to connect to nbdkit for '%1$s': %2$s"),
NULLSTR(uristring), NULLSTR(errbuf));
-- 
2.41.0

[libvirt PATCH v8 28/37] schema: add password configuration for ssh disk

[Jonathon Jongsma]

Right now, ssh network disks are not usable. There is some basic support
in libvirt that is meant to support disk chains that have backing disks
located at ssh urls, but there is no real way for a user to configure a
ssh-based disk.  This commit allows users to configure an ssh disk with
password authentication. Implementation will follow.


  

  



Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 docs/formatdomain.rst | 27 ++-
 src/conf/schemas/domaincommon.rng | 23 ++-
 2 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 68f54ab3ed..39d4230ec0 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -2778,7 +2778,7 @@ paravirtualized driver is specified via the ``disk`` 
element.
``network``
   The ``protocol`` attribute specifies the protocol to access to the
   requested image. Possible values are "nbd", "iscsi", "rbd", "sheepdog",
-  "gluster", "vxhs", "nfs", "http", "https", "ftp", ftps", or "tftp".
+  "gluster", "vxhs", "nfs", "http", "https", "ftp", ftps", "tftp", or 
"ssh".
 
   For any ``protocol`` other than ``nbd`` an additional attribute ``name``
   is mandatory to specify which volume/image will be used.
@@ -2930,18 +2930,19 @@ paravirtualized driver is specified via the ``disk`` 
element.
``auth``
   :since:`Since libvirt 3.9.0` , the ``auth`` element is supported for a
   disk ``type`` "network" that is using a ``source`` element with the
-  ``protocol`` attributes "rbd" or "iscsi". If present, the ``auth`` 
element
-  provides the authentication credentials needed to access the source. It
-  includes a mandatory attribute ``username``, which identifies the 
username
-  to use during authentication, as well as a sub-element ``secret`` with
-  mandatory attribute ``type``, to tie back to a `libvirt secret
-  object `__ that holds the actual password or other
-  credentials (the domain XML intentionally does not expose the password,
-  only the reference to the object that does manage the password). Known
-  secret types are "ceph" for Ceph RBD network sources and "iscsi" for CHAP
-  authentication of iSCSI targets. Both will require either a ``uuid``
-  attribute with the UUID of the secret object or a ``usage`` attribute
-  matching the key that was specified in the secret object.
+  ``protocol`` attributes "rbd", "iscsi", or "ssh". If present, the
+  ``auth`` element provides the authentication credentials needed to access
+  the source. It includes a mandatory attribute ``username``, which
+  identifies the username to use during authentication, as well as a
+  sub-element ``secret`` with mandatory attribute ``type``, to tie back to
+  a `libvirt secret object `__ that holds the actual
+  password or other credentials (the domain XML intentionally does not
+  expose the password, only the reference to the object that does manage
+  the password). Known secret types are "ceph" for Ceph RBD network sources
+  and "iscsi" for CHAP authentication of iSCSI targets. Both will require
+  either a ``uuid`` attribute with the UUID of the secret object or a
+  ``usage`` attribute matching the key that was specified in the secret
+  object.
``encryption``
   :since:`Since libvirt 3.9.0` , the ``encryption`` can be a sub-element of
   the ``source`` element for encrypted storage sources. If present,
diff --git a/src/conf/schemas/domaincommon.rng 
b/src/conf/schemas/domaincommon.rng
index 4a475f5c36..cd838a475c 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -2172,6 +2172,27 @@
 
   
 
+  
+
+  
+
+  
+ssh
+  
+
+
+
+
+
+  
+
+
+
+  
+
+  
+
+  
   
 
   
@@ -2179,7 +2200,6 @@
   
 sheepdog
 tftp
-ssh
   
 
 
@@ -2289,6 +2309,7 @@
   
   
   
+  
   
   
   
-- 
2.41.0

[libvirt PATCH v8 25/37] qemu: Monitor nbdkit process for exit

[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  |  11 +++
 src/qemu/qemu_domain.c   |   1 +
 src/qemu/qemu_domain.h   |   1 +
 src/qemu/qemu_driver.c   |  16 
 src/qemu/qemu_nbdkit.c   | 169 ---
 src/qemu/qemu_nbdkit.h   |   8 +-
 src/qemu/qemu_process.c  |  15 +++-
 src/qemu/qemu_process.h  |   3 +
 tests/meson.build|   6 +-
 tests/qemuxml2argvtest.c |   6 +-
 10 files changed, 219 insertions(+), 17 deletions(-)

diff --git a/meson.build b/meson.build
index 965ada483b..18ec312ee6 100644
--- a/meson.build
+++ b/meson.build
@@ -682,6 +682,13 @@ symbols = [
   [ 'sched.h', 'cpu_set_t' ],
 ]
 
+if host_machine.system() == 'linux'
+  symbols += [
+# process management
+[ 'sys/syscall.h', 'SYS_pidfd_open' ],
+  ]
+endif
+
 foreach symbol : symbols
   if cc.has_header_symbol(symbol[0], symbol[1], args: '-D_GNU_SOURCE', prefix: 
symbol.get(2, ''))
 conf.set('WITH_DECL_@0@'.format(symbol[1].to_upper()), 1)
@@ -2002,6 +2009,9 @@ endif
 
 conf.set_quoted('TLS_PRIORITY', get_option('tls_priority'))
 
+if conf.has('WITH_DECL_SYS_PIDFD_OPEN')
+  conf.set('WITH_NBDKIT', 1)
+endif
 
 # Various definitions
 
@@ -2259,6 +2269,7 @@ misc_summary = {
   'firewalld-zone': conf.has('WITH_FIREWALLD_ZONE'),
   'nss': conf.has('WITH_NSS'),
   'numad': conf.has('WITH_NUMAD'),
+  'nbdkit': conf.has('WITH_NBDKIT'),
   'Init script': init_script,
   'Char device locks': chrdev_lock_files,
   'Loader/NVRAM': loader_res,
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 46fe5a1cf4..2b8ece8f19 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11511,6 +11511,7 @@ qemuProcessEventFree(struct qemuProcessEvent *event)
 case QEMU_PROCESS_EVENT_PR_DISCONNECT:
 case QEMU_PROCESS_EVENT_UNATTENDED_MIGRATION:
 case QEMU_PROCESS_EVENT_RESET:
+case QEMU_PROCESS_EVENT_NBDKIT_EXITED:
 case QEMU_PROCESS_EVENT_LAST:
 break;
 }
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index ddd20e67b4..f018b45eb6 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -465,6 +465,7 @@ typedef enum {
 QEMU_PROCESS_EVENT_MEMORY_DEVICE_SIZE_CHANGE,
 QEMU_PROCESS_EVENT_UNATTENDED_MIGRATION,
 QEMU_PROCESS_EVENT_RESET,
+QEMU_PROCESS_EVENT_NBDKIT_EXITED,
 
 QEMU_PROCESS_EVENT_LAST
 } qemuProcessEventType;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ad8428948b..c4236b872f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4040,6 +4040,19 @@ processResetEvent(virQEMUDriver *driver,
 }
 
 
+static void
+processNbdkitExitedEvent(virDomainObj *vm,
+ qemuNbdkitProcess *nbdkit)
+{
+if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0)
+return;
+
+qemuNbdkitProcessRestart(nbdkit, vm);
+
+virDomainObjEndJob(vm);
+}
+
+
 static void qemuProcessEventHandler(void *data, void *opaque)
 {
 struct qemuProcessEvent *processEvent = data;
@@ -4097,6 +4110,9 @@ static void qemuProcessEventHandler(void *data, void 
*opaque)
 case QEMU_PROCESS_EVENT_RESET:
 processResetEvent(driver, vm);
 break;
+case QEMU_PROCESS_EVENT_NBDKIT_EXITED:
+processNbdkitExitedEvent(vm, processEvent->data);
+break;
 case QEMU_PROCESS_EVENT_LAST:
 break;
 }
diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index df638e99c0..b9ced1e5cf 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -19,6 +19,7 @@
 
 #include 
 #include 
+#include 
 
 #include "vircommand.h"
 #include "virerror.h"
@@ -33,6 +34,7 @@
 #include "qemu_nbdkit.h"
 #define LIBVIRT_QEMU_NBDKITPRIV_H_ALLOW
 #include "qemu_nbdkitpriv.h"
+#include "qemu_process.h"
 #include "qemu_security.h"
 
 #include 
@@ -41,6 +43,12 @@
 
 VIR_LOG_INIT("qemu.nbdkit");
 
+#if WITH_NBDKIT
+# define WITHOUT_NBDKIT_UNUSED
+#else
+# define WITHOUT_NBDKIT_UNUSED G_GNUC_UNUSED
+#endif
+
 VIR_ENUM_IMPL(qemuNbdkitCaps,
 QEMU_NBDKIT_CAPS_LAST,
 /* 0 */
@@ -611,6 +619,116 @@ qemuNbdkitCapsCacheNew(const char *cachedir)
 }
 
 
+void
+qemuNbdkitProcessRestart(qemuNbdkitProcess *proc,
+ virDomainObj *vm)
+{
+qemuDomainObjPrivate *vmpriv = vm->privateData;
+virQEMUDriver *driver = vmpriv->driver;
+
+/* clean up resources associated with process */
+

[libvirt PATCH v8 27/37] qemu: try to connect to nbdkit early to detect errors

[Jonathon Jongsma]

When using nbdkit to serve a network disk source, the nbdkit process
will start and wait for an nbd connection before actually attempting to
connect to the (remote) disk location. Because of this, nbdkit will not
report an error until after qemu is launched and tries to read from the
disk. This results in a fairly user-unfriendly error saying that qemu
was unable to start because "Requested export not available".

Ideally we'd like to be able to tell the user *why* the export is not
available, but this sort of information is only available to nbdkit, not
qemu. It could be because the url was incorrect, or because of an
authentication failure, or one of many other possibilities.

To make this friendlier for users and easier to detect
misconfigurations, try to connect to nbdkit immediately after starting
nbdkit and before we try to start qemu. This requires adding a
dependency on libnbd. If an error occurs when connecting to nbdkit, read
back from the nbdkit error log and provide that information in the error
report from qemuNbdkitProcessStart().

User-visible change demonstrated below:
Previous error:

$ virsh start nbdkit-test
2023-01-18 19:47:45.778+: 30895: error : 
virNetClientProgramDispatchError:172 : internal
error: process exited while connecting to monitor: 
2023-01-18T19:47:45.704658Z
qemu-system-x86_64: -blockdev {"driver":"nbd","server":{"type":"unix",

"path":"/var/lib/libvirt/qemu/domain-1-nbdkit-test/nbdkit-libvirt-1-storage.socket"},
"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}: 
Requested export not
available
error: Failed to start domain 'nbdkit-test'
error: internal error: process exited while connecting to monitor: 
2023-01-18T19:47:45.704658Z
qemu-system-x86_64: -blockdev {"driver":"nbd","server":{"type":"unix",

"path":"/var/lib/libvirt/qemu/domain-1-nbdkit-test/nbdkit-libvirt-1-storage.socket"},
"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}: 
Requested export not
available

After this change:

$ virsh start nbdkit-test
2023-01-18 19:44:36.242+: 30895: error : 
virNetClientProgramDispatchError:172 : internal
error: Failed to connect to nbdkit for 
'http://localhost:/nonexistent.iso': nbdkit: curl[1]:
error: problem doing HEAD request to fetch size of URL 
[http://localhost:/nonexistent.iso]:
HTTP response code said error: The requested URL returned error: 404
error: Failed to start domain 'nbdkit-test'
error: internal error: Failed to connect to nbdkit for 
'http://localhost:/nonexistent.iso]:
error: problem doing HEAD request to fetch size of URL 
[http://localhost:/nonexistent.iso]:
HTTP response code said error: The requested URL returned error: 404

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 meson.build|  7 +++
 meson_options.txt  |  1 +
 src/qemu/meson.build   |  1 +
 src/qemu/qemu_nbdkit.c | 23 +++
 4 files changed, 32 insertions(+)

diff --git a/meson.build b/meson.build
index 18ec312ee6..132269addc 100644
--- a/meson.build
+++ b/meson.build
@@ -1002,6 +1002,12 @@ endif
 libiscsi_version = '1.18.0'
 libiscsi_dep = dependency('libiscsi', version: '>=' + libiscsi_version, 
required: get_option('libiscsi'))
 
+libnbd_version = '1.0'
+libnbd_dep = dependency('libnbd', version: '>=' + libnbd_version, required: 
get_option('libnbd'))
+if libnbd_dep.found()
+  conf.set('WITH_LIBNBD', 1)
+endif
+
 libnl_version = '3.0'
 if not get_option('libnl').disabled() and host_machine.system() == 'linux'
   libnl_dep = dependency('libnl-3.0', version: '>=' + libnl_version, required: 
get_option('libnl'))
@@ -2219,6 +2225,7 @@ libs_summary = {
   'glusterfs': glusterfs_dep.found(),
   'libiscsi': libiscsi_dep.found(),
   'libkvm': libkvm_dep.found(),
+  'libnbd': libnbd_dep.found(),
   'libnl': libnl_dep.found(),
   'libparted': libparted_dep.found(),
   'libpcap': libpcap_dep.found(),
diff --git a/meson_options.txt b/meson_options.txt
index 9174c4021c..ba6e49afc5 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -25,6 +25,7 @@ option('curl', type: 'feature', value: 'auto', description: 
'curl support')
 option('fuse', type: 'feature', value: 'auto', description: 'fuse support')
 option('glusterfs', type: 'feature', value: 'auto', description: 'glusterfs 
support')
 option('libiscsi', type: 'feature', value: 'auto', description: 'libiscsi 
support')
+option('libnbd', type: 'feature', value: 'auto', description: 'libnbd support')
 option('libnl', type: 'feature', value: 'auto', description: 'libnl support')
 option('libpcap', type: 'feature', value: 'auto', description: 'libpcap 
support')
 option('libssh', type: 'feature', value: 'auto', description: 'libssh support')
diff --git a/src/qemu/meson.build b/src/qemu/meson.build
index 6d7a1bfbb0..607b597c8c 100644
--- a/src/qemu/meson.build
+++ b/src/qemu/meson.build
@@ -99,6 +99,7 @@ if conf.has('WITH_QEMU')
   

[libvirt PATCH v8 06/37] qemu: implement persistent file cache for nbdkit caps

[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 
Reviewed-by: Peter Krempa 
---
 po/POTFILES|   1 +
 src/qemu/qemu_nbdkit.c | 226 -
 2 files changed, 225 insertions(+), 2 deletions(-)

diff --git a/po/POTFILES b/po/POTFILES
index 5d6ec195b4..6167f98ac5 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -181,6 +181,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_passt.c
 src/qemu/qemu_process.c
 src/qemu/qemu_qapi.c
diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index 9828f562f7..97612b637f 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -312,11 +312,233 @@ 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 '%1$s' but saw '%2$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;
+
+if (!(doc = virXMLParse(filename, NULL, NULL, "nbdkitCaps", , NULL, 
false)))
+return -1;
+
+if (virXPathLongLong("string(./selfctime)", ctxt, ) < 0) {
+VIR_DEBUG("missing selfctime in nbdkit capabilities XML");
+return -1;
+}
+nbdkitCaps->libvirtCtime = (time_t)l;
+
+nbdkitCaps->libvirtVersion = 0;
+virXPathUInt("string(./selfvers)", ctxt, >libvirtVersion);
+
+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) {
+VIR_DEBUG("missing nbdkitctime in nbdkit capabilities XML");
+return -1;
+}
+nbdkitCaps->ctime = (time_t)l;
+
+if (virXPathLongLong("string(./plugindirmtime)", ctxt, ) < 0) {
+VIR_DEBUG("missing plugindirmtime in nbdkit capabilities XML");
+return -1;
+}
+nbdkitCaps->pluginDirMtime = (time_t)l;
+
+if (virXPathLongLong("string(./filterdirmtime)", ctxt, ) < 0) {
+VIR_DEBUG("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) {
+VIR_DEBUG("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)
+{
+

[libvirt PATCH v8 17/37] qemu: include nbdkit state in private xml

[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 
Reviewed-by: Peter Krempa 
---
 src/qemu/qemu_domain.c| 52 +
 src/qemu/qemu_nbdkit.c| 71 +++
 src/qemu/qemu_nbdkit.h|  8 +++
 src/qemu/qemu_process.c   |  6 ++
 tests/qemustatusxml2xmldata/modern-in.xml |  4 ++
 5 files changed, 141 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 951f3127d9..8429ce1028 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1958,6 +1958,33 @@ 
qemuStorageSourcePrivateDataAssignSecinfo(qemuDomainSecretInfo **secinfo,
 }
 
 
+static int
+qemuStorageSourcePrivateDataParseNbdkit(xmlNodePtr node,
+xmlXPathContextPtr ctxt,
+virStorageSource *src)
+{
+g_autofree char *pidfile = NULL;
+g_autofree char *socketfile = NULL;
+VIR_XPATH_NODE_AUTORESTORE(ctxt);
+
+ctxt->node = node;
+
+if (!(pidfile = virXPathString("string(./pidfile)", ctxt))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing nbdkit 
pidfile"));
+return -1;
+}
+
+if (!(socketfile = virXPathString("string(./socketfile)", ctxt))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing nbdkit 
socketfile"));
+return -1;
+}
+
+qemuNbdkitReconnectStorageSource(src, pidfile, socketfile);
+
+return 0;
+}
+
+
 static int
 qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt,
   virStorageSource *src)
@@ -1971,6 +1998,7 @@ qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt,
 bool fdsetPresent = false;
 unsigned int fdSetID;
 int enccount;
+xmlNodePtr nbdkitnode = NULL;
 
 src->nodestorage = 
virXPathString("string(./nodenames/nodename[@type='storage']/@name)", ctxt);
 src->nodeformat = 
virXPathString("string(./nodenames/nodename[@type='format']/@name)", ctxt);
@@ -2036,6 +2064,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;
 }
 
@@ -2053,6 +2085,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)
@@ -2102,6 +2151,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 81861bae4a..e3923ab4f2 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -627,6 +627,77 @@ qemuNbdkitProcessNew(virStorageSource *source,
 return nbdkit;
 }
 
+/**
+ * qemuNbdkitReconnectStorageSource:
+ * @source: a storage source
+ * @pidfile: a pidfile for an nbdkit process
+ * @socketfile: the socket file associated with the nbdkit process
+ *
+ * This function constructs a new qemuNbdkitProcess object with the given 
values for @pidfile and
+ * @socketfile and stores it in @source. This is intended to be called when 
the libvirt daemon is
+ * restarted and tries to reconnect to all currently-running domains. Since 
this function is called
+ * from the code that parses the current daemon state, it should not perform 
any filesystem
+ * operations, or anything else that might fail. Additional initialization 
will be done later by
+ * calling qemuNbdkitStorageSourceManageProcess().
+ */
+void
+qemuNbdkitReconnectStorageSource(virStorageSource *source,
+ const char *pidfile,
+ const char *socketfile)
+{
+qemuDomainStorageSourcePrivate *srcpriv = 
qemuDomainStorageSourcePrivateFetch(source);
+
+if (srcpriv->nbdkitProcess) {
+VIR_WARN("source already has an nbdkit process");
+return;
+}
+
+srcpriv->nbdkitProcess = 

[libvirt PATCH v8 36/37] rpm: update spec file for for nbdkit support

[Jonathon Jongsma]

Require libnbd-devel when building the qemu driver, recommend nbdkit
packages.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 libvirt.spec.in | 8 
 1 file changed, 8 insertions(+)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index b471afebb1..744fa5c88d 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -312,6 +312,7 @@ BuildRequires: util-linux
 BuildRequires: libacl-devel
 # From QEMU RPMs, used by virstoragetest
 BuildRequires: /usr/bin/qemu-img
+BuildRequires: libnbd-devel
 %endif
 # For LVM drivers
 BuildRequires: lvm2
@@ -768,6 +769,9 @@ Requires: numad
 Recommends: passt
 Recommends: passt-selinux
 %endif
+Recommends: nbdkit
+Recommends: nbdkit-curl-plugin
+Recommends: nbdkit-ssh-plugin
 
 %description daemon-driver-qemu
 The qemu driver plugin for the libvirtd daemon, providing
@@ -1074,8 +1078,10 @@ exit 1
 
 %if %{with_qemu}
 %define arg_qemu -Ddriver_qemu=enabled
+%define arg_libnbd -Dlibnbd=enabled
 %else
 %define arg_qemu -Ddriver_qemu=disabled
+%define arg_libnbd -Dlibnbd=disabled
 %endif
 
 %if %{with_openvz}
@@ -1264,6 +1270,7 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y' 
%{_specdir}/libvirt.spec)
-Dyajl=enabled \
%{?arg_sanlock} \
-Dlibpcap=enabled \
+   %{?arg_libnbd} \
-Dlibnl=enabled \
-Daudit=enabled \
-Ddtrace=enabled \
@@ -1327,6 +1334,7 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y' 
%{_specdir}/libvirt.spec)
   -Dglusterfs=disabled \
   -Dhost_validate=disabled \
   -Dlibiscsi=disabled \
+  -Dlibnbd=disabled \
   -Dlibnl=disabled \
   -Dlibpcap=disabled \
   -Dlibssh2=disabled \
-- 
2.41.0

[libvirt PATCH v8 13/37] qemu: move qemuProcessReadLog() to qemuLogContext

[Jonathon Jongsma]

This code can be used by the nbdkit implementation for reading back
filtered log data for error reporting. Move it to qemuLogContext so that
it can be shared. Renamed to qemuLogContextReadFiltered().

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 src/qemu/qemu_logcontext.c | 65 
 src/qemu/qemu_logcontext.h |  3 ++
 src/qemu/qemu_process.c| 67 +-
 3 files changed, 69 insertions(+), 66 deletions(-)

diff --git a/src/qemu/qemu_logcontext.c b/src/qemu/qemu_logcontext.c
index 0121ae5173..6e20f58bfa 100644
--- a/src/qemu/qemu_logcontext.c
+++ b/src/qemu/qemu_logcontext.c
@@ -21,6 +21,7 @@
 #include "qemu_logcontext.h"
 #include "viralloc.h"
 #include "virlog.h"
+#include "virstring.h"
 #include "virutil.h"
 
 #include 
@@ -236,6 +237,70 @@ qemuLogContextRead(qemuLogContext *ctxt,
 }
 
 
+/**
+ * qemuLogContextFilter: Read and filter log for relevant messages
+ * @ctxt: the domain log context
+ * @msg: pointer to buffer to store the read messages in
+ * @max: maximum length of the message returned in @msg after filtering
+ *
+ * Reads log output from @ctxt and filters it. Skips messages not produced by
+ * the target executable or irrelevant messages. If @max is not zero, @buf will
+ * contain at most @max characters from the end of the log and @buf will start
+ * after a new line if possible.
+ */
+int
+qemuLogContextReadFiltered(qemuLogContext *ctxt,
+   char **msg,
+   size_t max)
+{
+char *buf;
+char *eol;
+char *filter_next;
+size_t skip;
+ssize_t got;
+
+if ((got = qemuLogContextRead(ctxt, )) < 0)
+return -1;
+
+/* Filter out debug messages from intermediate libvirt process */
+filter_next = buf;
+while ((eol = strchr(filter_next, '\n'))) {
+*eol = '\0';
+if (virLogProbablyLogMessage(filter_next) ||
+strstr(filter_next, "char device redirected to")) {
+skip = (eol + 1) - filter_next;
+memmove(filter_next, eol + 1, buf + got - eol);
+got -= skip;
+} else {
+filter_next = eol + 1;
+*eol = '\n';
+}
+}
+
+if (got > 0 &&
+buf[got - 1] == '\n') {
+buf[got - 1] = '\0';
+got--;
+}
+
+if (max > 0 && got > max) {
+skip = got - max;
+
+if (buf[skip - 1] != '\n' &&
+(eol = strchr(buf + skip, '\n')) &&
+!virStringIsEmpty(eol + 1))
+skip = eol + 1 - buf;
+
+memmove(buf, buf + skip, got - skip + 1);
+got -= skip;
+}
+
+buf = g_renew(char, buf, got + 1);
+*msg = buf;
+return 0;
+}
+
+
 int
 qemuLogContextGetWriteFD(qemuLogContext *ctxt)
 {
diff --git a/src/qemu/qemu_logcontext.h b/src/qemu/qemu_logcontext.h
index 6ad60b7b4a..738e908bc3 100644
--- a/src/qemu/qemu_logcontext.h
+++ b/src/qemu/qemu_logcontext.h
@@ -32,6 +32,9 @@ int qemuLogContextWrite(qemuLogContext *ctxt,
 const char *fmt, ...) G_GNUC_PRINTF(2, 3);
 ssize_t qemuLogContextRead(qemuLogContext *ctxt,
char **msg);
+int qemuLogContextReadFiltered(qemuLogContext *ctxt,
+   char **msg,
+   size_t max);
 int qemuLogContextGetWriteFD(qemuLogContext *ctxt);
 void qemuLogContextMarkPosition(qemuLogContext *ctxt);
 
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e03d0d8b4d..a77d2ba7de 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1908,71 +1908,6 @@ qemuConnectMonitor(virQEMUDriver *driver,
 }
 
 
-/**
- * qemuProcessReadLog: Read log file of a qemu VM
- * @logCtxt: the domain log context
- * @msg: pointer to buffer to store the read messages in
- * @max: maximum length of the message returned in @msg
- *
- * Reads log of a qemu VM. Skips messages not produced by qemu or irrelevant
- * messages. If @max is not zero, @msg will contain at most @max characters
- * from the end of the log and @msg will start after a new line if possible.
- *
- * Returns 0 on success or -1 on error
- */
-static int
-qemuProcessReadLog(qemuLogContext *logCtxt,
-   char **msg,
-   size_t max)
-{
-char *buf;
-ssize_t got;
-char *eol;
-char *filter_next;
-size_t skip;
-
-if ((got = qemuLogContextRead(logCtxt, )) < 0)
-return -1;
-
-/* Filter out debug messages from intermediate libvirt process */
-filter_next = buf;
-while ((eol = strchr(filter_next, '\n'))) {
-*eol = '\0';
-if (virLogProbablyLogMessage(filter_next) ||
-strstr(filter_next, "char device redirected to")) {
-skip = (eol + 1) - filter_next;
-memmove(filter_next, eol + 1, buf + got - eol);
-got -= skip;
-} else {
-filter_next = eol + 1;
-*eol = '\n';
-}
-}
-
-if (got > 0 &&
-buf[got 

[libvirt PATCH v8 09/37] qemu: query nbdkit module dir from binary

[Jonathon Jongsma]

Rather than hard-coding the nbdkit module directory, query the nbdkit
binary for the location to these directories. nbdkit provides a
--dump-config optiont that outputs this information and can be easily
parsed. We can also get the version from this output rather than
executing `nbdkit --version` separately.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 src/qemu/qemu_nbdkit.c | 77 --
 1 file changed, 60 insertions(+), 17 deletions(-)

diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index 12c721f7f1..9a2a89224d 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -20,7 +20,6 @@
 #include 
 #include 
 
-#include "configmake.h"
 #include "vircommand.h"
 #include "virerror.h"
 #include "virlog.h"
@@ -39,10 +38,6 @@
 
 VIR_LOG_INIT("qemu.nbdkit");
 
-#define NBDKIT_MODDIR LIBDIR "/nbdkit"
-#define NBDKIT_PLUGINDIR NBDKIT_MODDIR "/plugins"
-#define NBDKIT_FILTERDIR NBDKIT_MODDIR "/filters"
-
 VIR_ENUM_IMPL(qemuNbdkitCaps,
 QEMU_NBDKIT_CAPS_LAST,
 /* 0 */
@@ -56,6 +51,9 @@ struct _qemuNbdkitCaps {
 
 char *path;
 char *version;
+char *filterDir;
+char *pluginDir;
+
 time_t ctime;
 time_t libvirtCtime;
 time_t pluginDirMtime;
@@ -129,18 +127,47 @@ qemuNbdkitCapsQueryFilters(qemuNbdkitCaps *nbdkit)
 
 
 static int
-qemuNbdkitCapsQueryVersion(qemuNbdkitCaps *nbdkit)
+qemuNbdkitCapsQueryBuildConfig(qemuNbdkitCaps *nbdkit)
 {
+size_t i;
+g_autofree char *output = NULL;
+g_auto(GStrv) lines = NULL;
+const char *line;
 g_autoptr(virCommand) cmd = virCommandNewArgList(nbdkit->path,
- "--version",
+ "--dump-config",
  NULL);
 
-virCommandSetOutputBuffer(cmd, >version);
+virCommandSetOutputBuffer(cmd, );
 
 if (virCommandRun(cmd, NULL) != 0)
 return -1;
 
-VIR_DEBUG("Got nbdkit version %s", nbdkit->version);
+lines = g_strsplit(output, "\n", 0);
+if (!lines)
+return -1;
+
+for (i = 0; (line = lines[i]); i++) {
+const char *key;
+const char *val;
+char *p;
+
+p = strchr(line, '=');
+if (!p)
+continue;
+
+*p = '\0';
+key = line;
+val = p + 1;
+
+VIR_DEBUG("Got nbdkit config value %s=%s", key, val);
+
+if (STREQ(key, "version"))
+nbdkit->version = g_strdup(val);
+else if (STREQ(key, "filterdir"))
+nbdkit->filterDir = g_strdup(val);
+else if (STREQ(key, "plugindir"))
+nbdkit->pluginDir = g_strdup(val);
+}
 return 0;
 }
 
@@ -152,6 +179,8 @@ qemuNbdkitCapsFinalize(GObject *object)
 
 g_clear_pointer(>path, g_free);
 g_clear_pointer(>version, g_free);
+g_clear_pointer(>filterDir, g_free);
+g_clear_pointer(>pluginDir, g_free);
 g_clear_pointer(>flags, virBitmapFree);
 
 G_OBJECT_CLASS(qemu_nbdkit_caps_parent_class)->finalize(object);
@@ -214,15 +243,15 @@ qemuNbdkitCapsQuery(qemuNbdkitCaps *caps)
 return;
 }
 
+qemuNbdkitCapsQueryBuildConfig(caps);
+qemuNbdkitCapsQueryPlugins(caps);
+qemuNbdkitCapsQueryFilters(caps);
+
 caps->ctime = st.st_ctime;
-caps->filterDirMtime = qemuNbdkitGetDirMtime(NBDKIT_FILTERDIR);
-caps->pluginDirMtime = qemuNbdkitGetDirMtime(NBDKIT_PLUGINDIR);
+caps->filterDirMtime = qemuNbdkitGetDirMtime(caps->filterDir);
+caps->pluginDirMtime = qemuNbdkitGetDirMtime(caps->pluginDir);
 caps->libvirtCtime = virGetSelfLastChanged();
 caps->libvirtVersion = LIBVIR_VERSION_NUMBER;
-
-qemuNbdkitCapsQueryPlugins(caps);
-qemuNbdkitCapsQueryFilters(caps);
-qemuNbdkitCapsQueryVersion(caps);
 }
 
 
@@ -267,9 +296,9 @@ virNbdkitCapsIsValid(void *data,
 if (!nbdkitCaps->path)
 return true;
 
-if (!virNbkditCapsCheckModdir(NBDKIT_PLUGINDIR, 
nbdkitCaps->pluginDirMtime))
+if (!virNbkditCapsCheckModdir(nbdkitCaps->pluginDir, 
nbdkitCaps->pluginDirMtime))
 return false;
-if (!virNbkditCapsCheckModdir(NBDKIT_FILTERDIR, 
nbdkitCaps->filterDirMtime))
+if (!virNbkditCapsCheckModdir(nbdkitCaps->filterDir, 
nbdkitCaps->filterDirMtime))
 return false;
 
 if (nbdkitCaps->libvirtCtime != virGetSelfLastChanged() ||
@@ -421,12 +450,22 @@ qemuNbdkitCapsLoadCache(qemuNbdkitCaps *nbdkitCaps,
 }
 nbdkitCaps->ctime = (time_t)l;
 
+if ((nbdkitCaps->pluginDir = virXPathString("string(./plugindir)", ctxt)) 
== NULL) {
+VIR_DEBUG("missing plugindir in nbdkit capabilities cache");
+return -1;
+}
+
 if (virXPathLongLong("string(./plugindirmtime)", ctxt, ) < 0) {
 VIR_DEBUG("missing plugindirmtime in nbdkit capabilities XML");
 return -1;
 }
 nbdkitCaps->pluginDirMtime = (time_t)l;
 
+if ((nbdkitCaps->filterDir = virXPathString("string(./filterdir)", ctxt)) 
== NULL) {

[libvirt PATCH v8 31/37] qemu: implement knownHosts for ssh disks with nbdkit

[Jonathon Jongsma]

For ssh disks that are served by nbdkit, use the configured value for
knownHosts and pass it to the nbdkit process.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 src/conf/domain_conf.c|  8 ++
 src/conf/storage_source_conf.c|  2 ++
 src/conf/storage_source_conf.h|  2 ++
 src/qemu/qemu_extdevice.c |  4 +--
 src/qemu/qemu_hotplug.c   |  4 +--
 src/qemu/qemu_nbdkit.c| 25 +++
 src/qemu/qemu_nbdkit.h|  6 +++--
 .../disk-network-ssh-password.args.disk0  |  3 ++-
 .../disk-network-ssh.args.disk0   |  3 ++-
 .../disk-network-ssh-password.xml |  1 +
 tests/qemuxml2argvdata/disk-network-ssh.xml   |  1 +
 11 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8feaf5d055..842b6404b5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7268,6 +7268,11 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
 return -1;
 }
 }
+if (src->protocol == VIR_STORAGE_NET_PROTOCOL_SSH &&
+(tmpnode = virXPathNode("./knownHosts", ctxt))) {
+if (!(src->ssh_known_hosts_file = virXMLPropStringRequired(tmpnode, 
"path")))
+return -1;
+}
 
 return 0;
 }
@@ -22274,6 +22279,9 @@ virDomainDiskSourceFormatNetwork(virBuffer *attrBuf,
 
 if (src->timeout)
 virBufferAsprintf(childBuf, "\n", 
src->timeout);
+
+if (src->protocol == VIR_STORAGE_NET_PROTOCOL_SSH && 
src->ssh_known_hosts_file)
+virBufferEscapeString(childBuf, "\n", 
src->ssh_known_hosts_file);
 }
 
 
diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c
index dcac3a8ff6..906bc36a9b 100644
--- a/src/conf/storage_source_conf.c
+++ b/src/conf/storage_source_conf.c
@@ -895,6 +895,7 @@ virStorageSourceCopy(const virStorageSource *src,
 /* ssh config passthrough for libguestfs */
 def->ssh_host_key_check_disabled = src->ssh_host_key_check_disabled;
 def->ssh_user = g_strdup(src->ssh_user);
+def->ssh_known_hosts_file = g_strdup(src->ssh_known_hosts_file);
 
 def->nfs_user = g_strdup(src->nfs_user);
 def->nfs_group = g_strdup(src->nfs_group);
@@ -1170,6 +1171,7 @@ virStorageSourceClear(virStorageSource *def)
 VIR_FREE(def->tlsHostname);
 
 VIR_FREE(def->ssh_user);
+VIR_FREE(def->ssh_known_hosts_file);
 
 VIR_FREE(def->nfs_user);
 VIR_FREE(def->nfs_group);
diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h
index f13e7c756a..8a9c7d07e2 100644
--- a/src/conf/storage_source_conf.h
+++ b/src/conf/storage_source_conf.h
@@ -410,6 +410,8 @@ struct _virStorageSource {
 /* these must not be used apart from formatting the output JSON in the 
qemu driver */
 char *ssh_user;
 bool ssh_host_key_check_disabled;
+/* additional ssh variables */
+char *ssh_known_hosts_file;
 
 /* nfs_user and nfs_group store the strings passed in by the user for NFS 
params.
  * nfs_uid and nfs_gid represent the converted/looked up ID numbers which 
are used
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
index 42ecdf13d5..3cf3867056 100644
--- a/src/qemu/qemu_extdevice.c
+++ b/src/qemu/qemu_extdevice.c
@@ -297,11 +297,11 @@ qemuExtDevicesStop(virQEMUDriver *driver,
 
 for (i = 0; i < def->ndisks; i++) {
 virDomainDiskDef *disk = def->disks[i];
-qemuNbdkitStopStorageSource(disk->src);
+qemuNbdkitStopStorageSource(disk->src, vm);
 }
 
 if (def->os.loader && def->os.loader->nvram)
-qemuNbdkitStopStorageSource(def->os.loader->nvram);
+qemuNbdkitStopStorageSource(def->os.loader->nvram, vm);
 }
 
 
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index dc06486922..bbc5177206 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1036,7 +1036,7 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriver 
*driver,
 if (virStorageSourceChainHasManagedPR(disk->src))
 ignore_value(qemuHotplugRemoveManagedPR(vm, VIR_ASYNC_JOB_NONE));
 
-qemuNbdkitStopStorageSource(disk->src);
+qemuNbdkitStopStorageSource(disk->src, vm);
 }
 qemuDomainSecretDiskDestroy(disk);
 qemuDomainCleanupStorageSourceFD(disk->src);
@@ -4496,7 +4496,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriver *driver,
 qemuHotplugRemoveManagedPR(vm, VIR_ASYNC_JOB_NONE) < 0)
 goto cleanup;
 
-qemuNbdkitStopStorageSource(disk->src);
+qemuNbdkitStopStorageSource(disk->src, vm);
 
 if (disk->transient) {
 VIR_DEBUG("Removing transient overlay '%s' of disk '%s'",
diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index 4cd91e282b..afac71e21a 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -630,7 +630,7 @@ qemuNbdkitProcessRestart(qemuNbdkitProcess *proc,
 virQEMUDriver *driver = 

[libvirt PATCH v8 24/37] qemu: Add Taint for nbdkit restart failure

[Jonathon Jongsma]

Since the restart handler will trigger at an arbitrary time (when the
nbdkit process crashes, for instance), it's difficult to provide
feedback to the user if the restart is unsuccessful. Rather than just
relying on a warning in the log, taint the domain so that there will be
a slightly more user-visible notification.

Signed-off-by: Jonathon Jongsma 
---
 src/conf/domain_conf.c | 2 ++
 src/conf/domain_conf.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index bb4f1fdb94..8feaf5d055 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -87,6 +87,7 @@ VIR_ENUM_IMPL(virDomainTaint,
   "custom-hypervisor-feature",
   "deprecated-config",
   "custom-device",
+  "nbdkit-restart",
 );
 
 VIR_ENUM_IMPL(virDomainTaintMessage,
@@ -105,6 +106,7 @@ VIR_ENUM_IMPL(virDomainTaintMessage,
   N_("hypervisor feature autodetection override"),
   N_("use of deprecated configuration settings"),
   N_("custom device configuration"),
+  N_("nbdkit restart failed"),
 );
 
 VIR_ENUM_IMPL(virDomainVirt,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ca195a52d2..c0729905a8 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3194,6 +3194,7 @@ typedef enum {
 VIR_DOMAIN_TAINT_CUSTOM_HYPERVISOR_FEATURE, /* custom hypervisor feature 
control */
 VIR_DOMAIN_TAINT_DEPRECATED_CONFIG,  /* Configuration that is marked 
deprecated */
 VIR_DOMAIN_TAINT_CUSTOM_DEVICE, /* hypervisor device config customized */
+VIR_DOMAIN_TAINT_NBDKIT_RESTART,/* nbdkit could not be restarted */
 
 VIR_DOMAIN_TAINT_LAST
 } virDomainTaintFlags;
-- 
2.41.0

[libvirt PATCH v8 32/37] schema: add keyfile configuration for ssh disks

[Jonathon Jongsma]

Authenticating via key file to an ssh server is often preferable to
logging in via password. In order to support this functionality add a
new  xml element for ssh disks that allows the user to specify
a keyfile and username. Example configuration:


  

...
  
...


Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 docs/formatdomain.rst |  7 +++
 src/conf/schemas/domaincommon.rng | 19 ++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 496a8ebfbe..baa2fdce7d 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -3005,6 +3005,13 @@ paravirtualized driver is specified via the ``disk`` 
element.
   of these attributes is omitted, then that field is assumed to be the
   default value for the current system. If both ``user`` and ``group``
   are intended to be default, then the entire element may be omitted.
+
+  When using an ``ssh`` protocol, this element is used to enable
+  authentication via ssh keys. In this configuration, the element has two
+  attributes. The ``username`` attribute specifies the name of the user on
+  the remote server and the ``keyfile`` attribute specifies the path to the
+  keyfile. Note that this only works for ssh keys that are not
+  password-protected.
``reconnect``
   For disk type ``vhostuser`` configures reconnect timeout if the 
connection
   is lost. This is set with the two mandatory attributes ``enabled`` and
diff --git a/src/conf/schemas/domaincommon.rng 
b/src/conf/schemas/domaincommon.rng
index ca43586323..47c5ee2a31 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -2180,6 +2180,19 @@
 
   
 
+  
+
+  
+
+  
+
+
+  
+
+  
+
+  
+
   
 
   
@@ -2199,11 +2212,15 @@
   
 
 
-  
+  
+
+
+  
 
   
 
   
+
   
 
   
-- 
2.41.0

[libvirt PATCH v8 18/37] util: secure erase virCommand send buffers

[Jonathon Jongsma]

All users of virCommandSetSendBuffer() are using it to send sensitive
data to a child process. So, since these buffers contain sensitive
information, clear it with virSecureErase().

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 src/util/vircommand.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 5f094c625a..899d413dd2 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -54,6 +54,7 @@
 #include "virpidfile.h"
 #include "virprocess.h"
 #include "virbuffer.h"
+#include "virsecureerase.h"
 #include "virthread.h"
 #include "virstring.h"
 
@@ -1697,6 +1698,7 @@ virCommandFreeSendBuffers(virCommand *cmd)
 
 for (i = 0; i < virCommandGetNumSendBuffers(cmd); i++) {
 VIR_FORCE_CLOSE(cmd->sendBuffers[i].fd);
+virSecureErase(cmd->sendBuffers[i].buffer, cmd->sendBuffers[i].buflen);
 VIR_FREE(cmd->sendBuffers[i].buffer);
 }
 VIR_FREE(cmd->sendBuffers);
-- 
2.41.0

[libvirt PATCH v8 30/37] schema: add configuration for host verification of ssh disks

[Jonathon Jongsma]

In order to make ssh disks usable, we need to be able to validate a
remote host. To do this, add a  xml element for ssh disks to
allow the user to specify a location for a file that contains known host
keys. Implementation to follow.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 docs/formatdomain.rst |  8 
 src/conf/schemas/domaincommon.rng | 11 +++
 2 files changed, 19 insertions(+)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 39d4230ec0..496a8ebfbe 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -3021,6 +3021,14 @@ paravirtualized driver is specified via the ``disk`` 
element.
  paused and will be rerun after a successful reconnect. After that 
time, any
  delayed requests and all future requests before a successful reconnect
  will immediately fail. If not set the default QEMU value is 0.
+   ``knownHosts``
+  For storage accessed via the ``ssh`` protocol, this element configures a
+  path to a file that will be used to verify the remote host. This file
+  must contain the expected host key for the remote host or the connection
+  will fail. The location of the file is specified via the ``path``
+  attribute.
+  :since:`Since 9.8.0`
+
 
For a "file" or "volume" disk type which represents a cdrom or floppy (the
``device`` attribute), it is possible to define policy what to do with the
diff --git a/src/conf/schemas/domaincommon.rng 
b/src/conf/schemas/domaincommon.rng
index cd838a475c..ca43586323 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -2172,6 +2172,14 @@
 
   
 
+  
+
+  
+
+  
+
+  
+
   
 
   
@@ -2187,6 +2195,9 @@
   
 
 
+
+  
+
 
   
 
-- 
2.41.0

[libvirt PATCH v8 05/37] qemu: implement basic virFileCache for nbdkit caps

[Jonathon Jongsma]

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

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 src/qemu/qemu_nbdkit.c | 89 +-
 src/qemu/qemu_nbdkit.h |  4 ++
 2 files changed, 92 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index 58828dd89a..9828f562f7 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -201,7 +201,7 @@ qemuNbdkitGetDirMtime(const char *moddir)
 }
 
 
-G_GNUC_UNUSED static void
+static void
 qemuNbdkitCapsQuery(qemuNbdkitCaps *caps)
 {
 struct stat st;
@@ -240,3 +240,90 @@ 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 e191e1fdb4..4aba7c8455 100644
--- a/src/qemu/qemu_nbdkit.h
+++ b/src/qemu/qemu_nbdkit.h
@@ -21,6 +21,7 @@
 
 #include "internal.h"
 #include "virenum.h"
+#include "virfilecache.h"
 
 typedef struct _qemuNbdkitCaps qemuNbdkitCaps;
 
@@ -38,6 +39,9 @@ VIR_ENUM_DECL(qemuNbdkitCaps);
 qemuNbdkitCaps *
 qemuNbdkitCapsNew(const char *path);
 
+virFileCache *
+qemuNbdkitCapsCacheNew(const char *cachedir);
+
 bool
 qemuNbdkitCapsGet(qemuNbdkitCaps *nbdkitCaps,
   qemuNbdkitCapsFlags flag);
-- 
2.41.0

[libvirt PATCH v8 20/37] qemu: use nbdkit to serve network disks if available

[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 
Reviewed-by: Peter Krempa 
---
 src/qemu/qemu_block.c | 162 +++---
 src/qemu/qemu_domain.c|  13 +-
 src/qemu/qemu_extdevice.c |  62 +++
 src/qemu/qemu_hotplug.c   |   7 +
 src/qemu/qemu_nbdkit.c|  42 +
 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 |  44 +
 .../disk-network-http-nbdkit.xml  |   1 +
 ...rce-curl-nbdkit-backing.x86_64-latest.args |  37 
 ...isk-network-source-curl-nbdkit-backing.xml |  45 +
 ...work-source-curl-nbdkit.x86_64-latest.args |  49 ++
 .../disk-network-source-curl-nbdkit.xml   |   1 +
 ...isk-network-source-curl.x86_64-latest.args |  52 ++
 .../disk-network-source-curl.xml  |  71 
 ...disk-network-ssh-nbdkit.x86_64-latest.args |  35 
 .../disk-network-ssh-nbdkit.xml   |   1 +
 tests/qemuxml2argvtest.c  |   6 +
 19 files changed, 618 insertions(+), 66 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 dcdf883926..1a2dc8ffb4 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -438,6 +438,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)
@@ -890,69 +916,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 

[libvirt PATCH v8 26/37] qemu: improve error handling when restarting nbdkit

[Jonathon Jongsma]

Change the return value for qemuNbdkitProcessRestart() and
qemuNbdkitStorageSourceManageProcess() to return an error status. The
main effect of this change is that when libvirt starts up and reconnects
to an already-running domain with an nbdkit-backed disk, it will return
an error if it fails to restart any nbdkit processes that are not found
to be running or if it fails to monitor any running nbdkit processes.
These failures will result in the domain being killed.

Signed-off-by: Jonathon Jongsma 
---
 src/qemu/qemu_driver.c  |  3 ++-
 src/qemu/qemu_nbdkit.c  | 34 --
 src/qemu/qemu_nbdkit.h  |  4 ++--
 src/qemu/qemu_process.c |  6 --
 4 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c4236b872f..a469b1d04a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4047,7 +4047,8 @@ processNbdkitExitedEvent(virDomainObj *vm,
 if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0)
 return;
 
-qemuNbdkitProcessRestart(nbdkit, vm);
+if (qemuNbdkitProcessRestart(nbdkit, vm) < 0)
+virDomainObjTaint(vm, VIR_DOMAIN_TAINT_NBDKIT_RESTART);
 
 virDomainObjEndJob(vm);
 }
diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index b9ced1e5cf..2ad34d6484 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -619,7 +619,7 @@ qemuNbdkitCapsCacheNew(const char *cachedir)
 }
 
 
-void
+int
 qemuNbdkitProcessRestart(qemuNbdkitProcess *proc,
  virDomainObj *vm)
 {
@@ -629,10 +629,7 @@ qemuNbdkitProcessRestart(qemuNbdkitProcess *proc,
 /* clean up resources associated with process */
 qemuNbdkitProcessStop(proc);
 
-if (qemuNbdkitProcessStart(proc, vm, driver) < 0) {
-VIR_DEBUG("Unable to restart nbkdit process");
-virDomainObjTaint(vm, VIR_DOMAIN_TAINT_NBDKIT_RESTART);
-}
+return qemuNbdkitProcessStart(proc, vm, driver);
 }
 
 
@@ -775,7 +772,7 @@ qemuNbdkitReconnectStorageSource(virStorageSource *source,
 }
 
 
-static void
+static int
 qemuNbdkitStorageSourceManageProcessOne(virStorageSource *source,
 virDomainObj *vm)
 {
@@ -784,33 +781,31 @@ qemuNbdkitStorageSourceManageProcessOne(virStorageSource 
*source,
 qemuNbdkitProcess *proc;
 
 if (!srcpriv)
-return;
+return 0;
 
 proc = srcpriv->nbdkitProcess;
 
 if (!proc)
-return;
+return 0;
 
 if (!proc->caps)
 proc->caps = qemuGetNbdkitCaps(vmpriv->driver);
 
 if (proc->pid <= 0) {
 if (virPidFileReadPath(proc->pidfile, >pid) < 0) {
-VIR_WARN("Unable to read pidfile '%s'", proc->pidfile);
-return;
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Unable to read pidfile '%1$s'"),
+   proc->pidfile);
+return -1;
 }
 }
 
 if (virProcessKill(proc->pid, 0) < 0) {
 VIR_DEBUG("nbdkit process %i is not alive", proc->pid);
-qemuNbdkitProcessRestart(proc, vm);
-return;
+return qemuNbdkitProcessRestart(proc, vm);
 }
 
-if (qemuNbdkitProcessStartMonitor(proc, vm) < 0) {
-VIR_DEBUG("unable monitor nbdkit process");
-virDomainObjTaint(vm, VIR_DOMAIN_TAINT_NBDKIT_RESTART);
-}
+return qemuNbdkitProcessStartMonitor(proc, vm);
 }
 
 /**
@@ -822,13 +817,16 @@ qemuNbdkitStorageSourceManageProcessOne(virStorageSource 
*source,
  * @source. It is intended to be called after libvirt restarts and has loaded 
its current state from
  * disk and is attempting to re-connect to active domains.
  */
-void
+int
 qemuNbdkitStorageSourceManageProcess(virStorageSource *source,
  virDomainObj *vm)
 {
 virStorageSource *backing;
 for (backing = source; backing != NULL; backing = backing->backingStore)
-qemuNbdkitStorageSourceManageProcessOne(backing, vm);
+if (qemuNbdkitStorageSourceManageProcessOne(backing, vm) < 0)
+return -1;
+
+return 0;
 }
 
 
diff --git a/src/qemu/qemu_nbdkit.h b/src/qemu/qemu_nbdkit.h
index f33b049d38..a818ff65e1 100644
--- a/src/qemu/qemu_nbdkit.h
+++ b/src/qemu/qemu_nbdkit.h
@@ -68,7 +68,7 @@ qemuNbdkitStartStorageSource(virQEMUDriver *driver,
 void
 qemuNbdkitStopStorageSource(virStorageSource *src);
 
-void
+int
 qemuNbdkitStorageSourceManageProcess(virStorageSource *src,
  virDomainObj *vm);
 
@@ -100,7 +100,7 @@ qemuNbdkitProcessStart(qemuNbdkitProcess *proc,
virDomainObj *vm,
virQEMUDriver *driver);
 
-void
+int
 qemuNbdkitProcessRestart(qemuNbdkitProcess *proc,
  virDomainObj *vm);
 
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index a1c6cfd91e..ddf95a064f 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8993,10 +8993,12 @@ qemuProcessReconnect(void *opaque)

[libvirt PATCH v8 22/37] tests: add tests for nbdkit invocation

[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.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 build-aux/syntax-check.mk |   2 +-
 src/qemu/qemu_nbdkit.c|   4 +-
 src/qemu/qemu_nbdkitpriv.h|  31 ++
 tests/meson.build |   1 +
 .../disk-cdrom-network.args.disk0 |   6 +
 .../disk-cdrom-network.args.disk1 |   8 +
 .../disk-cdrom-network.args.disk1.pipe.778|   1 +
 .../disk-cdrom-network.args.disk2 |   8 +
 .../disk-cdrom-network.args.disk2.pipe.780|   1 +
 .../disk-network-http.args.disk0  |   6 +
 .../disk-network-http.args.disk1  |   5 +
 .../disk-network-http.args.disk2  |   6 +
 .../disk-network-http.args.disk2.pipe.778 |   1 +
 .../disk-network-http.args.disk3  |   7 +
 .../disk-network-http.args.disk3.pipe.780 |   1 +
 ...work-source-curl-nbdkit-backing.args.disk0 |   7 +
 ...ce-curl-nbdkit-backing.args.disk0.pipe.778 |   1 +
 .../disk-network-source-curl.args.disk0   |   7 +
 ...sk-network-source-curl.args.disk0.pipe.778 |   1 +
 .../disk-network-source-curl.args.disk1   |   7 +
 ...sk-network-source-curl.args.disk1.pipe.780 |   1 +
 .../disk-network-source-curl.args.disk2   |   7 +
 ...sk-network-source-curl.args.disk2.pipe.782 |   1 +
 .../disk-network-source-curl.args.disk3   |   6 +
 .../disk-network-source-curl.args.disk4   |   6 +
 .../disk-network-ssh.args.disk0   |   6 +
 tests/qemunbdkittest.c| 308 ++
 27 files changed, 444 insertions(+), 2 deletions(-)
 create mode 100644 src/qemu/qemu_nbdkitpriv.h
 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.778
 create mode 100644 tests/qemunbdkitdata/disk-cdrom-network.args.disk2
 create mode 100644 tests/qemunbdkitdata/disk-cdrom-network.args.disk2.pipe.780
 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.778
 create mode 100644 tests/qemunbdkitdata/disk-network-http.args.disk3
 create mode 100644 tests/qemunbdkitdata/disk-network-http.args.disk3.pipe.780
 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.778
 create mode 100644 tests/qemunbdkitdata/disk-network-source-curl.args.disk0
 create mode 100644 
tests/qemunbdkitdata/disk-network-source-curl.args.disk0.pipe.778
 create mode 100644 tests/qemunbdkitdata/disk-network-source-curl.args.disk1
 create mode 100644 
tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.780
 create mode 100644 tests/qemunbdkitdata/disk-network-source-curl.args.disk2
 create mode 100644 
tests/qemunbdkitdata/disk-network-source-curl.args.disk2.pipe.782
 create mode 100644 tests/qemunbdkitdata/disk-network-source-curl.args.disk3
 create mode 100644 tests/qemunbdkitdata/disk-network-source-curl.args.disk4
 create mode 100644 tests/qemunbdkitdata/disk-network-ssh.args.disk0
 create mode 100644 tests/qemunbdkittest.c

diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index 64c1e2773e..ec04402133 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -1370,7 +1370,7 @@ 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$$)
 
 exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF = \
-  
(^tests/(nodedevmdevctl|viracpi|virhostcpu|virpcitest|virstoragetest)data/|docs/js/.*\.js|docs/fonts/.*\.woff|\.diff|tests/virconfdata/no-newline\.conf$$)
+  
(^tests/(nodedevmdevctl|viracpi|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 299d8824f2..df638e99c0 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -31,6 +31,8 @@
 #include "qemu_domain.h"
 #include "qemu_extdevice.h"
 #include "qemu_nbdkit.h"
+#define LIBVIRT_QEMU_NBDKITPRIV_H_ALLOW
+#include 

[libvirt PATCH v8 04/37] util: Allow virFileCache data to be any GObject

[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 
Reviewed-by: Peter Krempa 
---
 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 c730de066e..6f698016a1 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 c3bc0f529c..944741c0a7 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.41.0

[libvirt PATCH v8 23/37] qemu: add test for authenticating a https network disk

[Jonathon Jongsma]

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 tests/qemunbdkitdata/disk-network-source-curl.args.disk1  | 4 +++-
 .../disk-network-source-curl.args.disk1.pipe.780  | 2 +-
 .../disk-network-source-curl.args.disk1.pipe.782  | 1 +
 tests/qemunbdkitdata/disk-network-source-curl.args.disk2  | 2 +-
 .../disk-network-source-curl.args.disk2.pipe.784  | 1 +
 .../disk-network-source-curl.x86_64-latest.args   | 3 ++-
 tests/qemuxml2argvdata/disk-network-source-curl.xml   | 3 +++
 7 files changed, 12 insertions(+), 4 deletions(-)
 create mode 100644 
tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.782
 create mode 100644 
tests/qemunbdkitdata/disk-network-source-curl.args.disk2.pipe.784

diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk1 
b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1
index fde6a4f533..d1288dd242 100644
--- a/tests/qemunbdkitdata/disk-network-source-curl.args.disk1
+++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1
@@ -3,5 +3,7 @@ nbdkit \
 --foreground curl \
 protocols=https \
 'url=https://https.example.org:8443/path/to/disk5.iso?foo=bar' \
-cookie=-779 \
+user=myname \
+password=-779 \
+cookie=-781 \
 sslverify=false
diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.780 
b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.780
index 20af4ae383..ccdd4033fc 100644
--- a/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.780
+++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.780
@@ -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.782 
b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.782
new file mode 100644
index 00..20af4ae383
--- /dev/null
+++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.782
@@ -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 88c9fa35a1..f1d0e1929e 100644
--- a/tests/qemunbdkitdata/disk-network-source-curl.args.disk2
+++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk2
@@ -4,4 +4,4 @@ nbdkit \
 --readonly curl \
 protocols=http,https \
 url=http://http.example.org:8080/path/to/disk2.iso \
-cookie=-781
+cookie=-783
diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk2.pipe.784 
b/tests/qemunbdkitdata/disk-network-source-curl.args.disk2.pipe.784
new file mode 100644
index 00..5c035e84c5
--- /dev/null
+++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk2.pipe.784
@@ -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 cb0e5a92ea..f6ab5532cc 100644
--- a/tests/qemuxml2argvdata/disk-network-source-curl.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/disk-network-source-curl.x86_64-latest.args
@@ -32,9 +32,10 @@ 
XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
 -blockdev 
'{"driver":"https","url":"https://https.example.org:8443/path/to/disk1.iso","cookie-secret":"libvirt-5-storage-httpcookie-secret0","node-name":"libvirt-5-storage","auto-read-only":true,"discard":"unmap"}'
 \
 -blockdev 
'{"node-name":"libvirt-5-format","read-only":true,"driver":"raw","file":"libvirt-5-storage"}'
 \
 -device 
'{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x3","drive":"libvirt-5-format","id":"virtio-disk0","bootindex":1}'
 \
+-object 
'{"qom-type":"secret","id":"libvirt-4-storage-auth-secret0","data":"9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1","keyid":"masterKey0","iv":"AAECAwQFBgcICQoLDA0ODw==","format":"base64"}'
 \
 -object 
'{"qom-type":"secret","id":"libvirt-4-storage-httpcookie-secret0","data":"BUU0KmnWfonHdjzhYhwVQZ5iTI1KweTJ22q8XWUVoBCVu1z70reDuczPBIabZtC3","keyid":"masterKey0","iv":"AAECAwQFBgcICQoLDA0ODw==","format":"base64"}'
 \
 -object 
'{"qom-type":"secret","id":"libvirt-4-format-encryption-secret0","data":"9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1","keyid":"masterKey0","iv":"AAECAwQFBgcICQoLDA0ODw==","format":"base64"}'
 \
--blockdev 
'{"driver":"https","url":"https://https.example.org:8443/path/to/disk5.iso?foo=bar","sslverify":false,"cookie-secret":"libvirt-4-storage-httpcookie-secret0","node-name":"libvirt-4-storage","auto-read-only":true,"discard":"unmap"}'
 \
+-blockdev 

[libvirt PATCH v8 16/37] qemu: split qemuDomainSecretStorageSourcePrepare

[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 | 103 +
 1 file changed, 62 insertions(+), 41 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 23608f95bd..951f3127d9 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1398,38 +1398,70 @@ 
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;
+size_t nsecrets = 0;
+size_t i;
 
-if (virStorageSourceIsEmpty(src))
+if (!(src->encryption && src->encryption->nsecrets > 0))
 return 0;
 
-if (!src->auth && !hasEnc && src->ncookies == 0)
+if (virStorageSourceIsEmpty(src))
 return 0;
 
-if (!(src->privateData = qemuDomainStorageSourcePrivateNew()))
-return -1;
+nsecrets = src->encryption->nsecrets;
+
+srcPriv = qemuDomainStorageSourcePrivateFetch(src);
+
+srcPriv->enccount = nsecrets;
+srcPriv->encinfo = g_new0(qemuDomainSecretInfo *, nsecrets);
+for (i = 0; i < nsecrets; ++i) {
+if (!(srcPriv->encinfo[i] = qemuDomainSecretInfoSetupFromSecret(priv, 
alias,
+
"encryption", i,
+
VIR_SECRET_USAGE_TYPE_VOLUME,
+NULL,
+
>encryption->secrets[i]->seclookupdef)))
+return -1;
+}
+
+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 = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
+srcPriv = qemuDomainStorageSourcePrivateFetch(src);
 
 if (src->auth) {
 virSecretUsageType usageType = VIR_SECRET_USAGE_TYPE_ISCSI;
@@ -1437,7 +1469,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", 0,
  usageType,
  
src->auth->username,
@@ -1445,26 +1477,10 @@ 
qemuDomainSecretStorageSourcePrepare(qemuDomainObjPrivate *priv,
 return -1;
 }
 
-if (hasEnc) {
-size_t nsecrets = src->encryption->nsecrets;
-size_t i;
-
-

[libvirt PATCH v8 07/37] qemu: use file cache for nbdkit caps

[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 
Reviewed-by: Peter Krempa 
---
 src/qemu/qemu_conf.h   | 3 +++
 src/qemu/qemu_driver.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 4f610d86a1..a44985fb8b 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -318,6 +318,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 5db42f0753..ad8428948b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -845,6 +845,8 @@ qemuStateInitialize(bool privileged,
defsecmodel)))
 goto error;
 
+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. */
@@ -1078,6 +1080,7 @@ qemuStateCleanup(void)
 ebtablesContextFree(qemu_driver->ebtables);
 VIR_FREE(qemu_driver->qemuImgBinary);
 virObjectUnref(qemu_driver->domains);
+virObjectUnref(qemu_driver->nbdkitCapsCache);
 
 if (qemu_driver->lockFD != -1)
 virPidFileRelease(qemu_driver->config->stateDir, "driver", 
qemu_driver->lockFD);
-- 
2.41.0

[libvirt PATCH v8 03/37] qemu: expand nbdkit capabilities

[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 
Reviewed-by: Peter Krempa 
---
 src/qemu/qemu_nbdkit.c | 42 ++
 1 file changed, 42 insertions(+)

diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index e4e8fd568e..58828dd89a 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 
+#include "configmake.h"
 #include "vircommand.h"
 #include "virerror.h"
 #include "virlog.h"
@@ -38,6 +39,10 @@
 
 VIR_LOG_INIT("qemu.nbdkit");
 
+#define NBDKIT_MODDIR LIBDIR "/nbdkit"
+#define NBDKIT_PLUGINDIR NBDKIT_MODDIR "/plugins"
+#define NBDKIT_FILTERDIR NBDKIT_MODDIR "/filters"
+
 VIR_ENUM_IMPL(qemuNbdkitCaps,
 QEMU_NBDKIT_CAPS_LAST,
 /* 0 */
@@ -51,6 +56,11 @@ struct _qemuNbdkitCaps {
 
 char *path;
 char *version;
+time_t ctime;
+time_t libvirtCtime;
+time_t pluginDirMtime;
+time_t filterDirMtime;
+unsigned int libvirtVersion;
 
 virBitmap *flags;
 };
@@ -175,9 +185,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.41.0

[libvirt PATCH v8 19/37] qemu: pass sensitive data to nbdkit via pipe

[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 
Reviewed-by: Peter Krempa 
---
 src/qemu/qemu_nbdkit.c | 54 ++
 1 file changed, 34 insertions(+), 20 deletions(-)

diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index e3923ab4f2..22a67b0748 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -24,7 +24,6 @@
 #include "virerror.h"
 #include "virlog.h"
 #include "virpidfile.h"
-#include "virsecureerase.h"
 #include "virtime.h"
 #include "virutil.h"
 #include "qemu_block.h"
@@ -753,6 +752,29 @@ qemuNbdkitInitStorageSource(qemuNbdkitCaps *caps,
 }
 
 
+static int
+qemuNbdkitCommandPassDataByPipe(virCommand *cmd,
+const char *argName,
+unsigned char **buf,
+size_t buflen)
+{
+g_autofree char *fdfmt = NULL;
+int fd = virCommandSetSendBuffer(cmd, buf, buflen);
+
+if (fd < 0)
+return -1;
+
+/* some nbdkit arguments accept a variation where nbdkit will read the data
+ * from a file descriptor, e.g. password=-FD */
+fdfmt = g_strdup_printf("-%i", fd);
+virCommandAddArgPair(cmd, argName, fdfmt);
+
+virCommandDoAsyncIO(cmd);
+
+return 0;
+}
+
+
 static int
 qemuNbdkitProcessBuildCommandCurl(qemuNbdkitProcess *proc,
   virCommand *cmd)
@@ -775,7 +797,6 @@ qemuNbdkitProcessBuildCommandCurl(qemuNbdkitProcess *proc,
 g_autoptr(virConnect) conn = virGetConnectSecret();
 g_autofree uint8_t *secret = NULL;
 size_t secretlen = 0;
-g_autofree char *password = NULL;
 int secrettype;
 virStorageAuthDef *authdef = proc->source->auth;
 
@@ -799,26 +820,19 @@ qemuNbdkitProcessBuildCommandCurl(qemuNbdkitProcess *proc,
 return -1;
 }
 
-/* ensure that the secret is a NULL-terminated string */
-password = g_strndup((char*)secret, secretlen);
-virSecureErase(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"));
-
-virSecureEraseString(password);
-
-return -1;
+if (qemuNbdkitCommandPassDataByPipe(cmd, "password",
+, secretlen) < 0)
+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;
+/* Create a pipe to send the cookies to the nbdkit process. */
+if (proc->source->ncookies) {
+g_autofree char *cookies = 
qemuBlockStorageSourceGetCookieString(proc->source);
+
+if (qemuNbdkitCommandPassDataByPipe(cmd, "cookie",
+(unsigned char**),
+strlen(cookies)) < 0)
+return -1;
 }
 
 if (proc->source->sslverify == VIR_TRISTATE_BOOL_NO) {
-- 
2.41.0

[libvirt PATCH v8 11/37] Generalize qemuDomainLogContextNew()

[Jonathon Jongsma]

Allow to specify a basename for the log file so that
qemuDomainLogContextNew() can be used to create log contexts for
secondary loggers.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 src/qemu/qemu_domain.c  | 5 +++--
 src/qemu/qemu_domain.h  | 3 ++-
 src/qemu/qemu_process.c | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 5a2eb4868a..d79f9879df 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7119,7 +7119,8 @@ void qemuDomainObjCheckNetTaint(virQEMUDriver *driver,
 
 
 qemuDomainLogContext *qemuDomainLogContextNew(virQEMUDriver *driver,
-  virDomainObj *vm)
+  virDomainObj *vm,
+  const char *basename)
 {
 g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 qemuDomainLogContext *ctxt = 
QEMU_DOMAIN_LOG_CONTEXT(g_object_new(QEMU_TYPE_DOMAIN_LOG_CONTEXT, NULL));
@@ -7128,7 +7129,7 @@ qemuDomainLogContext 
*qemuDomainLogContextNew(virQEMUDriver *driver,
 ctxt->writefd = -1;
 ctxt->readfd = -1;
 
-ctxt->path = g_strdup_printf("%s/%s.log", cfg->logDir, vm->def->name);
+ctxt->path = g_strdup_printf("%s/%s.log", cfg->logDir, basename);
 
 if (cfg->stdioLogD) {
 ctxt->manager = virLogManagerNew(driver->privileged);
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 89edc75fcf..a262555c8c 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -657,7 +657,8 @@ void qemuDomainObjCheckNetTaint(virQEMUDriver *driver,
 qemuDomainLogContext *logCtxt);
 
 qemuDomainLogContext *qemuDomainLogContextNew(virQEMUDriver *driver,
-  virDomainObj *vm);
+  virDomainObj *vm,
+  const char *basename);
 int qemuDomainLogContextWrite(qemuDomainLogContext *ctxt,
   const char *fmt, ...) G_GNUC_PRINTF(2, 3);
 ssize_t qemuDomainLogContextRead(qemuDomainLogContext *ctxt,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index a6ed69cfe2..e0385d11be 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7616,7 +7616,7 @@ qemuProcessLaunch(virConnectPtr conn,
 hookData.cfg = cfg;
 
 VIR_DEBUG("Creating domain log file");
-if (!(logCtxt = qemuDomainLogContextNew(driver, vm))) {
+if (!(logCtxt = qemuDomainLogContextNew(driver, vm, vm->def->name))) {
 virLastErrorPrefixMessage("%s", _("can't connect to virtlogd"));
 goto cleanup;
 }
-- 
2.41.0

[libvirt PATCH v8 08/37] qemu: Add qemuNbdkitProcess

[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 
Reviewed-by: Peter Krempa 
---
 src/qemu/qemu_conf.c   | 22 
 src/qemu/qemu_conf.h   |  2 ++
 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, 167 insertions(+)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 3f811d064f..a0360e8d1b 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1656,3 +1656,25 @@ 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)
+{
+char *nbdkitBinary = virFindFileInPath("nbdkit");
+
+if (!nbdkitBinary)
+return NULL;
+
+return virFileCacheLookup(driver->nbdkitCapsCache, nbdkitBinary);
+}
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index a44985fb8b..1a3ba3a0fb 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -377,3 +377,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 bfeddc7746..5a2eb4868a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -882,6 +882,7 @@ qemuDomainStorageSourcePrivateDispose(void *obj)
 g_clear_pointer(>httpcookie, qemuDomainSecretInfoFree);
 g_clear_pointer(>tlsKeySecret, qemuDomainSecretInfoFree);
 g_clear_pointer(>fdpass, qemuFDPassFree);
+g_clear_pointer(>nbdkitProcess, qemuNbdkitProcessFree);
 }
 
 
@@ -10471,6 +10472,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
@@ -11300,6 +11329,8 @@ 
qemuDomainPrepareStorageSourceBlockdevNodename(virDomainDiskDef *disk,
 if (qemuDomainPrepareStorageSourceFDs(src, priv) < 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 786f239495..89edc75fcf 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"
@@ -308,6 +309,9 @@ struct _qemuDomainStorageSourcePrivate {
 
 /* file descriptors if user asks for FDs to be passed */
 qemuFDPass *fdpass;
+
+/* 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 97612b637f..12c721f7f1 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -549,3 +549,85 @@ qemuNbdkitCapsCacheNew(const char *cachedir)
 g_autofree char *dir = g_build_filename(cachedir, "nbdkitcapabilities", 
NULL);
 return virFileCacheNew(dir, "xml", );
 }
+
+
+static qemuNbdkitProcess *
+qemuNbdkitProcessNew(virStorageSource *source,
+ const char *pidfile,
+ const char *socketfile)
+{
+qemuNbdkitProcess *nbdkit = g_new0(qemuNbdkitProcess, 1);
+/* weak reference -- source owns this object, so it will always outlive us 

[libvirt PATCH v8 02/37] qemu: Add functions for determining nbdkit availability

[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 
Reviewed-by: Peter Krempa 
---
 src/qemu/meson.build   |   1 +
 src/qemu/qemu_conf.h   |   1 +
 src/qemu/qemu_nbdkit.c | 200 +
 src/qemu/qemu_nbdkit.h |  50 +++
 4 files changed, 252 insertions(+)
 create mode 100644 src/qemu/qemu_nbdkit.c
 create mode 100644 src/qemu/qemu_nbdkit.h

diff --git a/src/qemu/meson.build b/src/qemu/meson.build
index c8806bbc36..9be6996195 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_passt.c',
   'qemu_process.c',
   'qemu_qapi.c',
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 11c740d28f..4f610d86a1 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..e4e8fd568e
--- /dev/null
+++ b/src/qemu/qemu_nbdkit.c
@@ -0,0 +1,200 @@
+/*
+ * qemu_nbdkit.c: helpers for using nbdkit
+ *
+ * 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_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", QEMU_NBDKIT_CAPS_PLUGIN_SSH);
+}
+
+
+static void
+qemuNbdkitCapsQueryFilters(qemuNbdkitCaps *nbdkit)
+{
+qemuNbdkitQueryFilter(nbdkit, "readahead",
+  QEMU_NBDKIT_CAPS_FILTER_READAHEAD);
+}
+
+
+static int
+qemuNbdkitCapsQueryVersion(qemuNbdkitCaps *nbdkit)
+{
+g_autoptr(virCommand) cmd = virCommandNewArgList(nbdkit->path,
+ 

[libvirt PATCH v8 00/37] Use nbdkit for http/ftp/ssh network drives in libvirt

[Jonathon Jongsma]

This is the eighth version of this patch series. See
https://bugzilla.redhat.com/show_bug.cgi?id=2016527 for more information.

Note that testing this requires selinux policy changes which are not fully
done, but there is a new policy in development that has allowed me to run with
selinux in enforcing mode for the common cases. See
https://bugzilla.redhat.com/show_bug.cgi?id=2182505 for more information. The
following scenarios should work now with selinux enabled using the selinux
policy from that bug:
 - http/https disks
 - ssh disks with password authentication
 - ssh disks with passwordless keyfile

The one major thing that doesn't work and is difficult to get working with
selinux enabled is the ssh-agent. This is because there doesn't seem to be any
selinux policy for ssh-agent, so by default the ssh-agent socket is labeled
unconfined_t. We cannot allow access from the libvirt/qemu to unconfined_t
because that would open up access to just about anything on the host. So
additional work will likely be necessary for ssh-agent/libvirt interaction in
the future. Fortunately ssh-agent is something that never was really supported
with the old qemu block driver either, so I think we could potentially merge
this patchset either without the ssh-agent patches or with a note that
ssh-agent won't work with selinux enabled.

Changes in v8:
 - Hopefully addressed all of Peter's issues, in addition to:
 - updated documentation to say 9.8.0, since 9.7.0 is currently in freeze
 - used WITH_NBDKIT instead of WITH_DECL_SYS_PIDFD_OPEN to make the code a bit
   more concise and understandable
 - enabled ci by adding libnbd to the dependencies, which uncovered a couple
   additional minor issues with those platforms that don't support the
   pidfd_open syscall
   - don't run nbdkit tests when WITH_NBDKIT is not defined
   - avoid warnings with unused function arguments
   - note that the ubuntu containers are currently failing due to a
 LeakSanitizer error, but I haven't reproduced it locally and can't figure
 out how to get better information from the leak sanitizer. Pointers
 appreciated: https://gitlab.com/jjongsma/libvirt/-/jobs/4991631193
 - One change of note is a new patch "qemu: improve error handling when
   restarting nbdkit". In order to provide better error reporting to the
   user and avoid VIR_WARN as suggested by Peter, some functions now return an
   error and this error is propagated up to qemuProcessReconnect(). This could
   potentially result in running domains being killed upon a libvirt restart,
   but only if they were in a state where they were was not a running nbdkit
   backend or libvirt couldn't monitor the process nbdkit.

Jonathon Jongsma (37):
  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: query nbdkit module dir from binary
  qemu: add functions to start and stop nbdkit
  Generalize qemuDomainLogContextNew()
  qemu: Extract qemuDomainLogContext into a new file
  qemu: move qemuProcessReadLog() to qemuLogContext
  qemu: log error output from nbdkit
  tests: add ability to test various nbdkit capabilities
  qemu: split qemuDomainSecretStorageSourcePrepare
  qemu: include nbdkit state in private xml
  util: secure erase virCommand send buffers
  qemu: pass sensitive data to nbdkit via pipe
  qemu: use nbdkit to serve network disks if available
  util: make virCommandSetSendBuffer testable
  tests: add tests for nbdkit invocation
  qemu: add test for authenticating a https network disk
  qemu: Add Taint for nbdkit restart failure
  qemu: Monitor nbdkit process for exit
  qemu: improve error handling when restarting nbdkit
  qemu: try to connect to nbdkit early to detect errors
  schema: add password configuration for ssh disk
  qemu: implement password auth for ssh disks with nbdkit
  schema: add configuration for host verification of ssh disks
  qemu: implement knownHosts for ssh disks with nbdkit
  schema: add keyfile configuration for ssh disks
  qemu: implement keyfile auth for ssh disks with nbdkit
  schema: add ssh-agent configuration for ssh disks
  qemu: implement ssh-agent auth for ssh disks with nbdkit
  rpm: update spec file for for nbdkit support
  ci: add libnbd to build

 build-aux/syntax-check.mk |2 +-
 ci/buildenv/almalinux-8.sh|1 +
 ci/buildenv/centos-stream-8.sh|1 +
 ci/buildenv/centos-stream-9.sh|1 +
 ci/buildenv/debian-12-cross-aarch64.sh|1 +
 ci/buildenv/debian-12-cross-armv6l.sh |1 +
 ci/buildenv/debian-12-cross-armv7l.sh |1 +
 ci/buildenv/debian-12-cross-i686.sh   |1 +
 

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

[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.

Signed-off-by: Jonathon Jongsma 
Reviewed-by: Peter Krempa 
---
 src/conf/schemas/domaincommon.rng |  1 +
 tests/qemublocktest.c |  2 +-
 ...w2-invalid.json => network-ssh-qcow2.json} |  0
 ...cow2-invalid.xml => network-ssh-qcow2.xml} |  0
 .../disk-network-ssh.x86_64-latest.args   | 35 +++
 tests/qemuxml2argvdata/disk-network-ssh.xml   | 31 
 tests/qemuxml2argvtest.c  |  1 +
 7 files changed, 69 insertions(+), 1 deletion(-)
 rename tests/qemublocktestdata/imagecreate/{network-ssh-qcow2-invalid.json => 
network-ssh-qcow2.json} (100%)
 rename tests/qemublocktestdata/imagecreate/{network-ssh-qcow2-invalid.xml => 
network-ssh-qcow2.xml} (100%)
 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 de3bd1c35c..4a475f5c36 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -2179,6 +2179,7 @@
   
 sheepdog
 tftp
+ssh
   
 
 
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 9a968477d7..8bad69e7ac 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -1213,7 +1213,7 @@ mymain(void)
 
 TEST_IMAGE_CREATE("network-gluster-qcow2", NULL);
 TEST_IMAGE_CREATE("network-rbd-qcow2", NULL);
-TEST_IMAGE_CREATE("network-ssh-qcow2-invalid", NULL);
+TEST_IMAGE_CREATE("network-ssh-qcow2", NULL);
 
 #define TEST_BITMAP_DETECT(testname) \
 do { \
diff --git a/tests/qemublocktestdata/imagecreate/network-ssh-qcow2-invalid.json 
b/tests/qemublocktestdata/imagecreate/network-ssh-qcow2.json
similarity index 100%
rename from tests/qemublocktestdata/imagecreate/network-ssh-qcow2-invalid.json
rename to tests/qemublocktestdata/imagecreate/network-ssh-qcow2.json
diff --git a/tests/qemublocktestdata/imagecreate/network-ssh-qcow2-invalid.xml 
b/tests/qemublocktestdata/imagecreate/network-ssh-qcow2.xml
similarity index 100%
rename from tests/qemublocktestdata/imagecreate/network-ssh-qcow2-invalid.xml
rename to tests/qemublocktestdata/imagecreate/network-ssh-qcow2.xml
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..b7fd30032b
--- /dev/null
+++ b/tests/qemuxml2argvdata/disk-network-ssh.x86_64-latest.args
@@ -0,0 +1,35 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/var/lib/libvirt/qemu/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":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}'
 \
+-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \
+-accel kvm \
+-cpu qemu64 \
+-m size=219136k \
+-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 \
+-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 

[PATCH] virsh: Fix net-desc --config output

[K Shiva Kiran]

Fixes the following bug:
Command:  `net-desc --config [--title] my_network`
Expected Output:  Title/Description of persistent config
Output:   Title/Description of live config

This was caused due to the usage of a single `flags` variable in
`virshGetNetworkDescription()` which ended up in a wrong enum being
passed to `virNetworkGetMetadata()` (enum being that of LIVE instead of
CONFIG).

Although the domain object has the same code, this didn't cause a problem
there because the enum values of `VIR_DOMAIN_INACTIVE_XML` and
`VIR_DOMAIN_METADATA_CONFIG` turn out to be the same (1 << 1), whereas
they are not for network equivalent ones (1 << 0, 1 << 1).

Signed-off-by: K Shiva Kiran 
---
 tools/virsh-network.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index 49778d0f4f..8965d87c9c 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -366,7 +366,8 @@ static const vshCmdOptDef opts_network_desc[] = {
 /* extract description or title from network xml */
 static char *
 virshGetNetworkDescription(vshControl *ctl, virNetworkPtr net,
-   bool title, unsigned int flags)
+   bool title, unsigned int flags,
+   unsigned int queryflags)
 {
 char *desc = NULL;
 g_autoptr(xmlDoc) doc = NULL;
@@ -394,7 +395,7 @@ virshGetNetworkDescription(vshControl *ctl, virNetworkPtr 
net,
 }
 
 /* fall back to xml */
-if (virshNetworkGetXMLFromNet(ctl, net, flags, , ) < 0)
+if (virshNetworkGetXMLFromNet(ctl, net, queryflags, , ) < 0)
 return NULL;
 
 if (title)
@@ -454,7 +455,7 @@ cmdNetworkDesc(vshControl *ctl, const vshCmd *cmd)
 g_autofree char *descNet = NULL;
 g_autofree char *descNew = NULL;
 
-if (!(descNet = virshGetNetworkDescription(ctl, net, title, 
queryflags)))
+if (!(descNet = virshGetNetworkDescription(ctl, net, title, flags, 
queryflags)))
 return false;
 
 if (!descArg)
@@ -515,7 +516,7 @@ cmdNetworkDesc(vshControl *ctl, const vshCmd *cmd)
 vshPrintExtra(ctl, "%s", _("Network description updated 
successfully"));
 
 } else {
-g_autofree char *desc = virshGetNetworkDescription(ctl, net, title, 
queryflags);
+g_autofree char *desc = virshGetNetworkDescription(ctl, net, title, 
flags, queryflags);
 if (!desc)
 return false;
 
@@ -1128,7 +1129,7 @@ cmdNetworkList(vshControl *ctl, const vshCmd *cmd 
G_GNUC_UNUSED)
 if (optTitle) {
 g_autofree char *title = NULL;
 
-if (!(title = virshGetNetworkDescription(ctl, network, true, 
0)))
+if (!(title = virshGetNetworkDescription(ctl, network, true, 
0, 0)))
 goto cleanup;
 if (vshTableRowAppend(table,
 virNetworkGetName(network),
-- 
2.42.0

[PATCH] virsh: Fix net-desc --config output

[K Shiva Kiran]

Fixes the following bug:
Command:  `net-desc --config [--title] my_network`
Expected Output:  Title/Description of persistent config
Output:   Title/Description of live config

This was caused due to the usage of a single `flags` variable in
`virshGetNetworkDescription()` which ended up in a wrong enum being
passed to `virNetworkGetMetadata()` (enum being that of LIVE instead of
CONFIG).

Although the domain object has the same code, this didn't cause a problem
there because the enum values of `VIR_DOMAIN_INACTIVE_XML` and
`VIR_DOMAIN_METADATA_CONFIG` turn out to be the same (1 << 1), whereas
they are not for network equivalent ones (1 << 0, 1 << 1).

Signed-off-by: K Shiva Kiran 
---
 tools/virsh-network.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index 49778d0f4f..8965d87c9c 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -366,7 +366,8 @@ static const vshCmdOptDef opts_network_desc[] = {
 /* extract description or title from network xml */
 static char *
 virshGetNetworkDescription(vshControl *ctl, virNetworkPtr net,
-   bool title, unsigned int flags)
+   bool title, unsigned int flags,
+   unsigned int queryflags)
 {
 char *desc = NULL;
 g_autoptr(xmlDoc) doc = NULL;
@@ -394,7 +395,7 @@ virshGetNetworkDescription(vshControl *ctl, virNetworkPtr 
net,
 }
 
 /* fall back to xml */
-if (virshNetworkGetXMLFromNet(ctl, net, flags, , ) < 0)
+if (virshNetworkGetXMLFromNet(ctl, net, queryflags, , ) < 0)
 return NULL;
 
 if (title)
@@ -454,7 +455,7 @@ cmdNetworkDesc(vshControl *ctl, const vshCmd *cmd)
 g_autofree char *descNet = NULL;
 g_autofree char *descNew = NULL;
 
-if (!(descNet = virshGetNetworkDescription(ctl, net, title, 
queryflags)))
+if (!(descNet = virshGetNetworkDescription(ctl, net, title, flags, 
queryflags)))
 return false;
 
 if (!descArg)
@@ -515,7 +516,7 @@ cmdNetworkDesc(vshControl *ctl, const vshCmd *cmd)
 vshPrintExtra(ctl, "%s", _("Network description updated 
successfully"));
 
 } else {
-g_autofree char *desc = virshGetNetworkDescription(ctl, net, title, 
queryflags);
+g_autofree char *desc = virshGetNetworkDescription(ctl, net, title, 
flags, queryflags);
 if (!desc)
 return false;
 
@@ -1128,7 +1129,7 @@ cmdNetworkList(vshControl *ctl, const vshCmd *cmd 
G_GNUC_UNUSED)
 if (optTitle) {
 g_autofree char *title = NULL;
 
-if (!(title = virshGetNetworkDescription(ctl, network, true, 
0)))
+if (!(title = virshGetNetworkDescription(ctl, network, true, 
0, 0)))
 goto cleanup;
 if (vshTableRowAppend(table,
 virNetworkGetName(network),
-- 
2.42.0

Re: [libvirt PATCH 00/33] ci: Unify the GitLab CI jobs with local executions && adopt lcitool container executions

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:08PM +0200, Erik Skultety wrote:
> Technically a v2 of:
> https://listman.redhat.com/archives/libvir-list/2023-February/237552.html
> 
> However, the approach here is slightly different and what that series said
> about migration to lcitool container executions as a replacement for
> ci/Makefile is actually done here. One of the core problems of the above
> pointed out in review was that more Shell logic was introduced including CLI
> parsing, conditional executions, etc. which we fought hard to get rid of in 
> the
> past. I reworked the Shell functions quite a bit and dropped whatever extra
> Shell logic the original series added.
> Obviously we can't get rid of Shell completely because of .gitlab-ci.yml and 
> so
> I merely extracted the recipes into functions which are then sourced as
> ci/build.sh and executed. Now, that on its own would hide the actual commands
> being run in the GitLab job log, so before any command is actually executed, 
> it
> is formatted with a color sequence so we don't miss that information as that
> would be a regression to the status quo.
> 
> Lastly, this series then takes the effort inside the ci/build.sh script and
> basically mirrors whatever GitLab would do to run a job inside a local
> container which is executed by lcitool (yes, we already have that capability).
> 
> Please give this a try and I'm already looking forward to comments as I'd like
> to expand this effort to local VM executions running the TCK integration 
> tests,
> so this series is quite important in that regard.

Overall I'm fine with what's proposed here.

Two general thoughts

 * ci/Makefile appears pretty much redundant - ci/helper can provide
   the same level of functionality AFAICT, and it'd be nice to kill
   an outstanding usage of 'make' given our goal to standardize on
   meson + python

 * ci/helper looks almost entirely independent of libvirt, aside
   from the list of 'choices' for the --job arg, and the --namespace
   arg default value, it would work with any virt project we have if
   the project created its own ci/build.sh file

   Can we fold all its logic into lcitool, and just have that as
   the entrypoint ? In ci/manifest.yml we can get the project
   namespace, and we could possibly just extra the commands by
   crudely regex matching 'ci/build.sh' content against the
   pattern '^run_.*\(\)$ {'


The removal of ci/Makefil feels like it could be done in this series,
but its fine if the ci/helper suggestion is left as separate future
work.


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

Re: [libvirt PATCH 25/33] ci: helper: Add a job argparse subparser

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:33PM +0200, Erik Skultety wrote:
> The idea behind this subcommand is to follow whatever build job we have
> defined in the GitLab CI pipeline, so that we only have a single source
> of truth for the recipes. Adds 'shell' as an extra option for
> interactive container build debugging.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/helper | 15 +++
>  1 file changed, 15 insertions(+)

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

Re: [libvirt PATCH 26/33] ci: helper: Add a required_deps higher order helper/decorator

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:34PM +0200, Erik Skultety wrote:
> Since we'll depend on GitPython for repo cloning, we need to make sure
> to emit a user friendly error if the module is not installed. This
> patch introduces a helper which future patches will use as a decorator.
> Inspiration for this helper has been taken out of lcitool where we use
> an identical helper for this purpose.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/helper | 22 ++
>  1 file changed, 22 insertions(+)

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

Re: [libvirt PATCH 27/33] ci: helper: Add Python code hangling git clones

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:35PM +0200, Erik Skultety wrote:
> This helper will be utilized by a future patch which will add the
> lcitool container execution logic. The reason why the required_deps
> decorator isn't being used here is because this is a property.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/helper | 9 +
>  1 file changed, 9 insertions(+)

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

Re: [libvirt PATCH 28/33] ci: helper: Add a helper to create a local repo clone Pythonic way

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:36PM +0200, Erik Skultety wrote:
> A proper Python equivalent of 'git clone --local'.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/helper | 4 
>  1 file changed, 4 insertions(+)

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

Re: [libvirt PATCH 29/33] ci: helper: Rework _lcitool_run method logic

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:37PM +0200, Erik Skultety wrote:
> This method wasn't even utilized before this patch. This patch adds all
> the necessary logic to successfully execute a container workload via
> lcitool (which will later allow us to ditch ci/Makefile). Because
> container executions via lcitool creates the following inside the
> container:
> 
> $ ls
> script datadir
> 
> where 'datadir' is the workload directory (in this case a local git
> repo clone) and 'script' is the code that runs whatever the workload is
> over 'datadir'.
> 
> In order to satisfy the ^above, our helper generates a trivial
> temporary 'script' that will source ci/build.sh and run whatever was
> specified as --job essentially to simulate the exact steps a GitLab
> pipeline job would go through.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/helper | 50 --
>  1 file changed, 48 insertions(+), 2 deletions(-)

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

Re: [libvirt PATCH 30/33] ci: helper: Add an action to run the container workload via lcitool

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:38PM +0200, Erik Skultety wrote:
> Just like with the other CLI sub-commands, add an action to run a
> GitLab spec job in a local container via lcitool.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/helper | 4 
>  1 file changed, 4 insertions(+)

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

Re: [libvirt PATCH 33/33] ci: helper: Drop the _make_run method

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:41PM +0200, Erik Skultety wrote:
> We've successfully migrated over to lcitool to take care of the
> container workload execution, so dropping this 'make' prep code is a
> prerequisite of finally getting rid of the ci/Makefile script.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/helper | 25 -
>  1 file changed, 25 deletions(-)

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

Re: [libvirt PATCH 32/33] ci: helper: Drop the --meson-args/--ninja-args CLI options

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:40PM +0200, Erik Skultety wrote:
> These originally allowed customizing the ci/Makefile script which was
> the core of the local container executions. The problem was that
> however flexible this may have been, it never mirrored what was being
> done as part of the GitLab jobs. Motivated by the effort of mirroring
> GitLab jobs locally, these would only ever make sense to be set/used in
> interactive shell container sessions where the developer is perfectly
> capable of using the right meson/ninja CLI options directly without
> going through another shell variable indirection as it was the case
> with these ci/helper options.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/helper | 15 ---
>  1 file changed, 15 deletions(-)

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

Re: [libvirt PATCH 31/33] ci: helper: Drop original actions

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:39PM +0200, Erik Skultety wrote:
> Previous patches added a single 'run' command parametrized with GitLab
> job specs via '--job' that cover all of these original actions, adding
> some more in the process. Drop the original actions as we don't need
> them anymore.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/helper | 36 
>  1 file changed, 36 deletions(-)

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

Re: [libvirt PATCH 24/33] ci: helper: Add --lcitool-path CLI option

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:32PM +0200, Erik Skultety wrote:
> We'll soon be relying solely on lcitool so we need to be able to run it
> from a user-provided location if it's not installed in a known
> location.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/helper | 6 ++
>  1 file changed, 6 insertions(+)

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

Re: [libvirt PATCH 23/33] ci: helper: Don't make ':' literal a static part of the image tag

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:31PM +0200, Erik Skultety wrote:
> ':' is just a connecting character, we can add it to the appropriate
> place later in the Python script later, but it doesn't make sense to be
> part of the image 'tag' string.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/helper | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

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

Re: [libvirt PATCH 22/33] ci: helper: Drop _lcitool_get_targets method

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:30PM +0200, Erik Skultety wrote:
> This method unused anywhere, so drop it.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/helper | 4 
>  1 file changed, 4 deletions(-)

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

Re: [libvirt PATCH 21/33] .gitlab-ci.yml: Convert the potfile job to the build.sh usage

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:29PM +0200, Erik Skultety wrote:
> Individual shell command executions are replaced by respective
> functions in the ci/build.sh base script. This will make sure we use
> the same recipes in GitLab jobs as well as in local executions.
> 
> Signed-off-by: Erik Skultety 
> ---
>  .gitlab-ci.yml | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

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

Re: [libvirt PATCH 20/33] .gitlab-ci.yml: Convert the codestyle job to the build.sh usage

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:28PM +0200, Erik Skultety wrote:
> Individual shell command executions are replaced by respective
> functions in the ci/build.sh base script. This will make sure we use
> the same recipes in GitLab jobs as well as in local executions.
> 
> Signed-off-by: Erik Skultety 
> ---
>  .gitlab-ci.yml | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)

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

Re: [libvirt PATCH 19/33] .gitlab-ci.yml: Convert the website build job to the build.sh usage

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:27PM +0200, Erik Skultety wrote:
> Individual shell command executions are replaced by respective
> functions in the ci/build.sh base script. This will make sure we use
> the same recipes in GitLab jobs as well as in local executions.
> 
> Signed-off-by: Erik Skultety 
> ---
>  .gitlab-ci.yml | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

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

Re: [libvirt PATCH 18/33] .gitlab-ci.yml: Convert the cross build job to the build.sh usage

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:26PM +0200, Erik Skultety wrote:
> Individual shell command executions are replaced by respective
> functions in the ci/build.sh base script. This will make sure we use
> the same recipes in GitLab jobs as well as in local executions.
> 
> Signed-off-by: Erik Skultety 
> ---
>  .gitlab-ci.yml | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)

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

Re: [libvirt PATCH 17/33] .gitlab-ci.yml: Convert the native build job to the build.sh usage

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:25PM +0200, Erik Skultety wrote:
> Individual shell command executions are replaced by respective
> functions in the ci/build.sh base script. This will make sure we use
> the same recipes in GitLab jobs as well as in local executions.
> 
> Signed-off-by: Erik Skultety 
> ---
>  .gitlab-ci.yml | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)

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

Re: [libvirt PATCH 07/33] ci: build.sh: Add a helper function to create the dist tarball

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:15PM +0200, Erik Skultety wrote:
> This helper function does not correspond to a particular GitLab job, it
> just logically separates the necessary step of creating a dist tarball
> from the RPM build job that takes over.
> One notable change here is the need to update git's file index which
> causes issues in local container executions which rely on a shallow
> copy of the libvirt repo created as:
> 
> $ git clone --local
> 
> Even if all changes have been committed, git often complained
> otherwise. Updating the index in a GitLab environment is a NOP.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/build.sh | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/ci/build.sh b/ci/build.sh
> index c2f54707ce..c56e45f51f 100644
> --- a/ci/build.sh
> +++ b/ci/build.sh
> @@ -54,3 +54,15 @@ run_build() {
>  run_meson_setup
>  run_cmd "$CMD"
>  }
> +
> +run_dist() {
> +local CMD="meson dist -C build --no-tests"
> +
> +run_meson_setup

Same comment as prev patch about conditionally invoking this

> +
> +# dist is unhappy in local container environment complaining about
> +# uncommitted changes in the repo which is often not the case - 
> refreshing
> +# git's index solves the problem
> +git update-index --refresh

This is all very peculiar but I concur, this does appear to "solve" it
for our needs.

> +run_cmd "$CMD"
> +}
> -- 
> 2.41.0
> 

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

Re: [libvirt PATCH 06/33] ci: build.sh: Add a wrapper function over the 'build' job

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:14PM +0200, Erik Skultety wrote:
> This helper is a shell function transcript of its original GitLab CI
> counterpart.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/build.sh | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/ci/build.sh b/ci/build.sh
> index d4fbf0ab37..c2f54707ce 100644
> --- a/ci/build.sh
> +++ b/ci/build.sh
> @@ -47,3 +47,10 @@ run_meson_setup() {
>  
>  run_cmd "$CMD"
>  }
> +
> +run_build() {
> +local CMD="meson compile -C build $BUILD_ARGS"
> +
> +run_meson_setup

Per the previous patch, I think we should do

   test -d $GIT_ROOT/build/build.ninja || run_meson_setup

so that 'run_meson_setup' can do its thing unconditionally if
invoked explicitly

> +run_cmd "$CMD"
> +}

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

Re: [libvirt PATCH 15/33] ci: build.sh: Drop MESON_ARGS definition from global level

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:23PM +0200, Erik Skultety wrote:
> Signed-off-by: Erik Skultety 
> ---
>  ci/build.sh | 10 --
>  1 file changed, 10 deletions(-)

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

Re: [libvirt PATCH 14/33] ci: build.sh: Drop direct invocation of meson/ninja commands

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:22PM +0200, Erik Skultety wrote:
> We've moved all invocations to the respective helper function which
> we'll execute both from gitlab CI jobs and local environments so we
> don't need to have them on the global level as it would also not work
> with "sourcing" this file to populate the environment with function
> definitions.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/build.sh | 5 -
>  1 file changed, 5 deletions(-)
> 
> diff --git a/ci/build.sh b/ci/build.sh
> index 133952f706..b075c49af3 100644
> --- a/ci/build.sh
> +++ b/ci/build.sh
> @@ -18,11 +18,6 @@ GIT_ROOT="$(git rev-parse --show-toplevel)"
>  
>  MESON_ARGS="$MESON_ARGS $MESON_OPTS"
>  
> -meson setup build --werror -Dsystem=true $MESON_ARGS || \
> -(cat build/meson-logs/meson-log.txt && exit 1)
> -
> -ninja -C build $NINJA_ARGS
> -
>  run_cmd() {
>  local CMD="$(echo $CMD | tr -s ' ')" # truncate any additional spaces
>

Now we drop immediate invokation at time of execution, I wonder if the
build.sh name is a little mis-leading.

Might be better renamed to 'functions.sh' or 'commands.sh' perhaps, so
the name doesn't suggest that it actually builds stuff, merely that it
supplies some shell logic ?


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

Re: [libvirt PATCH 13/33] ci: build.sh: Drop changing working directory to CI_CONT_DIR

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:21PM +0200, Erik Skultety wrote:
> Firstly, this would mangle with "sourcing" this file in either
> execution environment later down the road. Secondly, we won't need this
> as future ci/helper patches will generate a throwaway script that will
> take care of a correct execution of a build job in a similar fashion as
> if the job ran in a GitLab environment.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/build.sh | 2 --
>  1 file changed, 2 deletions(-)

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

Re: [libvirt PATCH 12/33] ci: build.sh: Add a wrapper function over the 'website' job

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:20PM +0200, Erik Skultety wrote:
> This helper is a shell function transcript of its original GitLab CI
> counterpart.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/build.sh | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/ci/build.sh b/ci/build.sh
> index 30f4712e4b..6673ec6b18 100644
> --- a/ci/build.sh
> +++ b/ci/build.sh
> @@ -105,3 +105,11 @@ run_rpmbuild() {
>  run_meson_setup
>  run_dist
>  }
> +
> +run_website_build() {
> +export DESTDIR="$(pwd)/install"

Perhaps use ${GIT_ROOT} instead of $(pwd) to make this stable
regardless of pwd

> +BUILD_ARGS="install-web"
> +
> +run_meson_setup

Redundant call

> +run_build
> +}
> -- 

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

Re: [libvirt PATCH 11/33] ci: build.sh: Add a wrapper function over the 'rpmbuild' job

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:19PM +0200, Erik Skultety wrote:
> This helper is a shell function transcript of its original GitLab CI
> counterpart.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/build.sh | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/ci/build.sh b/ci/build.sh
> index 6990f2d171..30f4712e4b 100644
> --- a/ci/build.sh
> +++ b/ci/build.sh
> @@ -94,3 +94,14 @@ run_potfile() {
>  run_meson_setup
>  run_build
>  }
> +
> +run_rpmbuild() {
> +local CMD="rpmbuild \
> + --clean \
> + --nodeps \
> + --define "_without_mingw 1" \
> + -ta build/meson-dist/libvirt-*.tar.xz"
> +
> +run_meson_setup

Redundant here as implied by run_dist

> +run_dist
> +}

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

Re: [libvirt PATCH 10/33] ci: build.sh: Add a wrapper function over the 'potfile' job

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:18PM +0200, Erik Skultety wrote:
> This helper is a shell function transcript of its original GitLab CI
> counterpart. There's one notable difference such that we pass '-j1' to
> the meson compile command otherwise we'd have to execute the 'run_build'
> function twice, passing 'libvirt-pot-dep' and 'libvirt-pot' targets
> in a serial manner.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/build.sh | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/ci/build.sh b/ci/build.sh
> index b2891a0c0f..6990f2d171 100644
> --- a/ci/build.sh
> +++ b/ci/build.sh
> @@ -82,3 +82,15 @@ run_codestyle() {
>  run_build
>  run_test
>  }
> +
> +run_potfile() {
> +# since meson would run jobs for each of the following target in 
> parallel,
> +# we'd have dependency issues such that one target might depend on a
> +# generated file which hasn't been generated yet by the other target, 
> hence
> +# we limit potfile job to a single build job (luckily potfile build has
> +# negligible performance impact)
> +BUILD_ARGS="-j1 libvirt-pot-dep libvirt-pot"
> +
> +run_meson_setup
> +run_build

run_meson_setup is redundant here too as implied by run_build

Rather than -j1 could we instead invoke 'run_build' twice eg:

  BUILD_ARGS=libvirt-pot-dep run_build
  BUILD_ARGS=libvirt-pot run_build

> +}

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

Re: [libvirt PATCH 09/33] ci: build.sh: Add a wrapper function over the 'codestyle' job

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:17PM +0200, Erik Skultety wrote:
> This helper is a shell function transcript of its original GitLab CI
> counterpart.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/build.sh | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/ci/build.sh b/ci/build.sh
> index ab471f2741..b2891a0c0f 100644
> --- a/ci/build.sh
> +++ b/ci/build.sh
> @@ -73,3 +73,12 @@ run_test() {
>  run_meson_setup
>  run_cmd "$CMD"
>  }
> +
> +run_codestyle() {
> +BUILD_ARGS="libvirt-pot-dep"
> +TEST_ARGS="--suite syntax-check --no-rebuild --print-errorlogs"
> +
> +run_meson_setup
> +run_build

run_build already implies run_meson_setup so we can drop it in this
case.

> +run_test
> +}


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

Re: [libvirt PATCH 04/33] ci: build.sh: Add a wrapper function over meson's setup

[Daniel P . Berrangé]

On Thu, Aug 31, 2023 at 05:37:47PM +0100, Daniel P. Berrangé wrote:
> On Fri, Aug 25, 2023 at 07:55:12PM +0200, Erik Skultety wrote:
> > The reason for this wrapper is that all job functions introduced in
> > future patches will refer to this one instead of open-coding the same
> > 'meson setup' invocation N times. It also prevents 'setup' to be called
> > multiple times as some future job functions might actually do just that
> > in a transitive manner.
> > 
> > Signed-off-by: Erik Skultety 
> > ---
> >  ci/build.sh | 17 +
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/ci/build.sh b/ci/build.sh
> > index 82016ba29c..02ff1a8388 100644
> > --- a/ci/build.sh
> > +++ b/ci/build.sh
> > @@ -24,3 +24,20 @@ meson setup build --werror -Dsystem=true $MESON_ARGS || \
> >  (cat build/meson-logs/meson-log.txt && exit 1)
> >  
> >  ninja -C build $NINJA_ARGS
> > +
> > +run_meson_setup() {
> > +if [ -d "${GIT_ROOT}/build/meson-private" ]; then
> > +return
> > +fi
> 
> This isn't as accurate as meson's own check. I interrupted meson
> part way through, and this directry exists, but meson had written
> out sufficient files todo a build.  Re-running 'meson setup build'
> directly happily runs, but this run_meson_setup command becomes
> a no-op which is kinda annoying.

Perhaps we can just push the test into the caller. eg

   test -d $GIT_ROOT/build || run_meson_setup

so commands like 'run_build' "just work", but if someone explicitly
invokes 'run_meson_setup' we just let meson detect a pre-created
build dir using its native logic.

> 
> > +
> > +local DUMP_ERR_CMD="cat ${GIT_ROOT}/build/meson-logs/meson-log.txt"
> > +local SETUP_CMD="meson \
> > + setup \
> > + build \
> > + --werror \
> > + -Dsystem=true $MESON_OPTS $MESON_ARGS"
> > +
> > +local CMD="$SETUP_CMD || ($DUMP_ERR_CMD && exit 1)"
> > +
> > +run_cmd "$CMD"
> > +}
> > -- 
> > 2.41.0
> > 
> 
> 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 :|
> 

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

Re: [libvirt PATCH 03/33] ci: build.sh: Don't mention that MESON_ARGS are available via CLI

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:11PM +0200, Erik Skultety wrote:
> Previous patches have removed the code that allowed injecting arbitrary
> meson arguments, same for ninja args.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/build.sh | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

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

Re: [libvirt PATCH 02/33] ci: build.sh: Add GIT_ROOT env helper variable

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:10PM +0200, Erik Skultety wrote:
> We'll use this one in many of the job functions future patches will
> introduce, it's a neat shortcut to avoid using relative paths.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/build.sh | 2 ++
>  1 file changed, 2 insertions(+)

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

Re: [libvirt PATCH 01/33] ci: build.sh: Add variables from .gitlab-ci.yml

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:09PM +0200, Erik Skultety wrote:
> These are common variables we wish to use in containerized environments
> both in GitLab and locally. Having these defined in a single place
> rather than twice is highly preferable.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/build.sh | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> diff --git a/ci/build.sh b/ci/build.sh
> index ed9b1f4473..d5586f457c 100644
> --- a/ci/build.sh
> +++ b/ci/build.sh
> @@ -2,7 +2,12 @@
>  
>  cd "$CI_CONT_SRCDIR"
>  
> -export VIR_TEST_DEBUG=1
> +export CCACHE_BASEDIR="$(pwd)"
> +export CCACHE_DIR="$CCACHE_BASEDIR/ccache"
> +export CCACHE_MAXSIZE="500M"
> +export PATH="$CCACHE_WRAPPERSDIR:$PATH"
> +export VIR_TEST_VERBOSE="1"
> +export VIR_TEST_DEBUG="1"

These last two vars are good for CI, because in a non-interactive env we
need the verbose / debug output upfront to stand a chance of diagnosing
problems from logs after the fact

If I'm running a build locally with our ci/Makefile, I don't especially
want it to be forced into verbose mode by default, as I have an interactive
shell I can do that as & when needed myself.

Perhaps we can do

   if ! test -t 1
   then
  export VIR_TEST_VERBOSE="1"
  export VIR_TEST_DEBUG="1"
   fi

such that we only set the verbose vars when STDOUT is NOT a TTY.


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

Re: [libvirt PATCH 04/33] ci: build.sh: Add a wrapper function over meson's setup

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:12PM +0200, Erik Skultety wrote:
> The reason for this wrapper is that all job functions introduced in
> future patches will refer to this one instead of open-coding the same
> 'meson setup' invocation N times. It also prevents 'setup' to be called
> multiple times as some future job functions might actually do just that
> in a transitive manner.
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/build.sh | 17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/ci/build.sh b/ci/build.sh
> index 82016ba29c..02ff1a8388 100644
> --- a/ci/build.sh
> +++ b/ci/build.sh
> @@ -24,3 +24,20 @@ meson setup build --werror -Dsystem=true $MESON_ARGS || \
>  (cat build/meson-logs/meson-log.txt && exit 1)
>  
>  ninja -C build $NINJA_ARGS
> +
> +run_meson_setup() {
> +if [ -d "${GIT_ROOT}/build/meson-private" ]; then
> +return
> +fi

This isn't as accurate as meson's own check. I interrupted meson
part way through, and this directry exists, but meson had written
out sufficient files todo a build.  Re-running 'meson setup build'
directly happily runs, but this run_meson_setup command becomes
a no-op which is kinda annoying.

> +
> +local DUMP_ERR_CMD="cat ${GIT_ROOT}/build/meson-logs/meson-log.txt"
> +local SETUP_CMD="meson \
> + setup \
> + build \
> + --werror \
> + -Dsystem=true $MESON_OPTS $MESON_ARGS"
> +
> +local CMD="$SETUP_CMD || ($DUMP_ERR_CMD && exit 1)"
> +
> +run_cmd "$CMD"
> +}
> -- 
> 2.41.0
> 

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

Re: [libvirt PATCH 05/33] ci: build.sh: Add a wrapper function executing 'shell' commands

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:13PM +0200, Erik Skultety wrote:
> This would normally be not needed at all, but the problem here is the
> Shell-in-YAML which GitLab interprets. It outputs every command that
> appears as a line in the 'script' segment in a color-coded fashion for
> easy identification of problems. Well, that useful feature is lost when
> there's indirection and one script calls into another in which case it
> would only output the respective script name which would make failure
> investigation harder. This simple helper tackles that by echoing the
> command to be run by any script/function with a color escape sequence
> so that we don't lose track of the *actual* shell commands being run as
> part of the GitLab job pipelines. An example of what the output then
> might look like:
> [RUN COMMAND]: 'meson compile -C build install-web'
> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/build.sh | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/ci/build.sh b/ci/build.sh
> index 02ff1a8388..d4fbf0ab37 100644
> --- a/ci/build.sh
> +++ b/ci/build.sh
> @@ -25,6 +25,12 @@ meson setup build --werror -Dsystem=true $MESON_ARGS || \
>  
>  ninja -C build $NINJA_ARGS
>  
> +run_cmd() {
> +local CMD="$(echo $CMD | tr -s ' ')" # truncate any additional spaces
> +
> +printf "\e[93m[RUN COMMAND]: '%s'\e[0m\n" "$CMD"; eval "$CMD"
> +}

Bright yellow is unfortunately almost unreadable when used on a
terminal with white/light coloured background. Using green '32m'
ought to be visible in both light/dark terminals reasonabley well.

> +
>  run_meson_setup() {
>  if [ -d "${GIT_ROOT}/build/meson-private" ]; then
>  return
> -- 
> 2.41.0
> 

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

Re: [libvirt PATCH 00/33] ci: Unify the GitLab CI jobs with local executions && adopt lcitool container executions

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:08PM +0200, Erik Skultety wrote:
> Technically a v2 of:
> https://listman.redhat.com/archives/libvir-list/2023-February/237552.html
> 
> However, the approach here is slightly different and what that series said
> about migration to lcitool container executions as a replacement for
> ci/Makefile is actually done here. One of the core problems of the above
> pointed out in review was that more Shell logic was introduced including CLI
> parsing, conditional executions, etc. which we fought hard to get rid of in 
> the
> past. I reworked the Shell functions quite a bit and dropped whatever extra
> Shell logic the original series added.
> Obviously we can't get rid of Shell completely because of .gitlab-ci.yml and 
> so
> I merely extracted the recipes into functions which are then sourced as
> ci/build.sh and executed. Now, that on its own would hide the actual commands
> being run in the GitLab job log, so before any command is actually executed, 
> it
> is formatted with a color sequence so we don't miss that information as that
> would be a regression to the status quo.
> 
> Lastly, this series then takes the effort inside the ci/build.sh script and
> basically mirrors whatever GitLab would do to run a job inside a local
> container which is executed by lcitool (yes, we already have that capability).
> 
> Please give this a try and I'm already looking forward to comments as I'd like
> to expand this effort to local VM executions running the TCK integration 
> tests,
> so this series is quite important in that regard.

Do you have a gitlab branch with this contnt somewhere. When i tried to
apply the patches to current git, it was unhappy on the 3rd patch

$ git am -3 ~/cibuild
Applying: ci: build.sh: Add variables from .gitlab-ci.yml
Applying: ci: build.sh: Add GIT_ROOT env helper variable
Applying: ci: build.sh: Don't mention that MESON_ARGS are available via CLI
error: sha1 information is lacking or useless (ci/build.sh).
error: could not build fake ancestor
Patch failed at 0003 ci: build.sh: Don't mention that MESON_ARGS are available 
via CLI
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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

Re: [libvirt PATCH 4/7] qemuSaveImageStartProcess: make it possible to use without header

[Pavel Hrdina]

On Thu, Aug 31, 2023 at 05:43:35PM +0200, Peter Krempa wrote:
> On Thu, Aug 31, 2023 at 16:55:03 +0200, Pavel Hrdina wrote:
> > When used with internal snapshots there is no header to be used and no
> > memory state to be decompressed.
> 
> I didn't yet have a look at the rest, but this made me curious. What are
> you actually doing with this with internal snapshots?
> 
> There in fact isn't a save image at all with internal snapshots as the
> memory image is stored in the qcow2 image (in a different section than
> the data -> thus also the name internal) so I'm not exactly sure what
> you are refering to here in the commit message.

All of that is correct. In PATCH 6/7 this new function is called from
qemuSnapshotRevertActive unconditionally. And it will handle reverting
internal and external snapshots and do the correct thing based on what
arguments are passed to it.

In case of internal snapshots we were calling qemuProcessStart and
passing virDomainMomentObj. To avoid code duplication this parameter
was introduced in PATCH 3/7 to this new helper so it can start QEMU
process when reverting to internal snapshot as well as when reverting to
external snapshots.


signature.asc
Description: PGP signature

Re: Sunset libvirt-snmp?

[Daniel P . Berrangé]

On Mon, Aug 21, 2023 at 10:32:42AM +0200, Michal Prívozník wrote:
> It's been a while since libvirt-snmp was actively developed. Now it
> receives only libvirt-ci related commits. The code compiles with
> net-snmp-5.9.3 but the freshly released net-snmp-5.9.4 [1] breaks
> compilation [2]. Now, libvirt-snmp has this crazy architecture, where
> some sources are manually generated from src/LIBVIRT-MIB.txt, then
> edited (added code to talk to libvirt) and then added to git.

I'm also strongly inclined to archive the following:

  * libvirt-cim

  Similar situation to SNMP binding, CIM is an outdated way
  to interact with libvirt that hasn't been developed in
  years and I doubt anyone uses it.


  * libvirt-appdev-guide-python
  * libvirt-publican

  The publican toolchain was removed from Fedora, and the
  build in Debian is broken. While I can probably fixe the
  Debian build, I'm not especially inclined to invest in
  this since we're not actively adding content to this
  doc. If we ever did take it forward again I'd probably
  suggest it be ported to Sphinx / RST instead of Docbook.
 
  
  * libvirt-sandbox
  * libvirt-sandbox-image

  The Kata and libkrun projects both conceptually overlap with
  what this tried to do and are actually actively used and
  developed unlike this.


  * libvirt-designer

  This project is dormant as we never put all that much investment
  into it, and I don't see it being useful to apps in future either


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

Re: [libvirt PATCH 6/7] qemu_snapshot: correctly load the saved memory state file

[Peter Krempa]

On Thu, Aug 31, 2023 at 16:55:05 +0200, Pavel Hrdina wrote:
> Original code assumed that the memory state file is only migration
> stream but it has additional metadata stored by libvirt. To correctly
> load the memory state file we need to reuse code that is used when
> restoring domain from saved image.
> 
> Signed-off-by: Pavel Hrdina 
> ---
>  src/qemu/qemu_snapshot.c | 78 
>  1 file changed, 47 insertions(+), 31 deletions(-)
> 
> diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
> index ff85d56572..869b46a9a8 100644
> --- a/src/qemu/qemu_snapshot.c
> +++ b/src/qemu/qemu_snapshot.c
> @@ -2004,6 +2004,21 @@ qemuSnapshotRevertWriteMetadata(virDomainObj *vm,
>  }
>  
>  
> +typedef struct _qemuSnapshotRevertMemoryData {
> +int fd;
> +char *path;
> +virQEMUSaveData *data;
> +} qemuSnapshotRevertMemoryData;
> +
> +static void
> +qemuSnapshotClearRevertMemoryData(qemuSnapshotRevertMemoryData *memdata)
> +{
> +VIR_FORCE_CLOSE(memdata->fd);
> +virQEMUSaveDataFree(memdata->data);
> +}
> +G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(qemuSnapshotRevertMemoryData, 
> qemuSnapshotClearRevertMemoryData);
> +
> +
>  /**
>   * qemuSnapshotRevertExternalPrepare:
>   * @vm: domain object
> @@ -2011,15 +2026,13 @@ qemuSnapshotRevertWriteMetadata(virDomainObj *vm,
>   * @snap: snapshot object we are reverting to
>   * @config: live domain definition
>   * @inactiveConfig: offline domain definition
> - * memsnapFD: pointer to store memory state file FD or NULL
> - * memsnapPath: pointer to store memory state file path or NULL
> + * @memdata: struct with data to load memory state
>   *
>   * Prepare new temporary snapshot definition @tmpsnapdef that will
>   * be used while creating new overlay files after reverting to snapshot
>   * @snap. In case we are reverting to snapshot with memory state it will
> - * open it and pass FD via @memsnapFD and path to the file via
> - * @memsnapPath, caller is responsible for freeing both @memsnapFD and
> - * memsnapPath.
> + * open it and store necessary data in @memdata. Caller is responsible
> + * to clear the data by using qemuSnapshotClearRevertMemoryData().
>   *
>   * Returns 0 in success, -1 on error.
>   */
> @@ -2029,8 +2042,7 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm,
>virDomainMomentObj *snap,
>virDomainDef *config,
>virDomainDef *inactiveConfig,
> -  int *memsnapFD,
> -  char **memsnapPath)
> +  qemuSnapshotRevertMemoryData *memdata)
>  {
>  size_t i;
>  bool active = virDomainObjIsActive(vm);
> @@ -2065,12 +2077,21 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm,
>  return -1;
>  }
>  
> -if (memsnapFD && memsnapPath && snapdef->memorysnapshotfile) {
> +if (memdata && snapdef->memorysnapshotfile) {
>  virQEMUDriver *driver = ((qemuDomainObjPrivate *) 
> vm->privateData)->driver;
> -g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
> +g_autoptr(virDomainDef) savedef = NULL;
>  
> -*memsnapPath = snapdef->memorysnapshotfile;
> -*memsnapFD = qemuDomainOpenFile(cfg, NULL, *memsnapPath, O_RDONLY, 
> NULL);
> +memdata->path = snapdef->memorysnapshotfile;
> +memdata->fd = qemuSaveImageOpen(driver, NULL, memdata->path,
> +, >data,
> +false, NULL,
> +false, false);
> +
> +if (memdata->fd < 0)
> +return -1;
> +
> +if (!virDomainDefCheckABIStability(savedef, domdef, driver->xmlopt))
> +return -1;
>  }
>  
>  return 0;
> @@ -2254,13 +2275,13 @@ qemuSnapshotRevertActive(virDomainObj *vm,
>  virObjectEvent *event = NULL;
>  virObjectEvent *event2 = NULL;
>  virDomainMomentObj *loadSnap = NULL;
> -VIR_AUTOCLOSE memsnapFD = -1;
> -char *memsnapPath = NULL;
>  int detail;
>  bool defined = false;
>  qemuDomainSaveCookie *cookie = (qemuDomainSaveCookie *) snapdef->cookie;
>  int rc;
>  g_autoptr(virDomainSnapshotDef) tmpsnapdef = NULL;
> +g_auto(qemuSnapshotRevertMemoryData) memdata = { -1, NULL, NULL };
> +bool started = false;
>  
>  start_flags |= VIR_QEMU_PROCESS_START_PAUSED;
>  
> @@ -2284,7 +2305,7 @@ qemuSnapshotRevertActive(virDomainObj *vm,
>  
>  if (qemuSnapshotRevertExternalPrepare(vm, tmpsnapdef, snap,
>*config, *inactiveConfig,
> -  , ) < 0) 
> {
> +  ) < 0) {
>  return -1;
>  }
>  } else {
> @@ -2298,28 +2319,24 @@ qemuSnapshotRevertActive(virDomainObj *vm,
>  
>  virDomainObjAssignDef(vm, config, true, 

Re: [libvirt PATCH 0/7] external snapshot revert fixes

[Peter Krempa]

On Thu, Aug 31, 2023 at 17:06:35 +0200, Pavel Hrdina wrote:
> On Thu, Aug 31, 2023 at 04:54:59PM +0200, Pavel Hrdina wrote:
> > This fixes reverting external snapshots to not error out in cases where  
> > it should work and makes it correctly load the memory state when 
> > reverting to snapshot of running VM.
> 
> Forget to mention that we need this to be in 9.7.0 otherwise I will have
> to revert commit  introducing
> capability to indicate that reverting external snapshots is usable.

Given that the release is tomorrow and I conceptually don't like using
the 'save image' function to open iternal snapshots which do not have a
save image at all (memory state is stored _internally_ in qcow2) I think
the safest will be if you also send this variant.

Re: [libvirt PATCH 4/7] qemuSaveImageStartProcess: make it possible to use without header

[Peter Krempa]

On Thu, Aug 31, 2023 at 16:55:03 +0200, Pavel Hrdina wrote:
> When used with internal snapshots there is no header to be used and no
> memory state to be decompressed.

I didn't yet have a look at the rest, but this made me curious. What are
you actually doing with this with internal snapshots?

There in fact isn't a save image at all with internal snapshots as the
memory image is stored in the qcow2 image (in a different section than
the data -> thus also the name internal) so I'm not exactly sure what
you are refering to here in the commit message.

Re: Sunset libvirt-snmp?

[Daniel P . Berrangé]

On Thu, Aug 31, 2023 at 08:09:05AM -0700, Andrea Bolognani wrote:
> On Tue, Aug 22, 2023 at 12:44:16PM +0100, Daniel P. Berrangé wrote:
> > On Tue, Aug 22, 2023 at 01:26:47PM +0200, Erik Skultety wrote:
> > > On Mon, Aug 21, 2023 at 10:32:42AM +0200, Michal Prívozník wrote:
> > > > It's been a while since libvirt-snmp was actively developed. Now it
> > > > receives only libvirt-ci related commits. The code compiles with
> > > > net-snmp-5.9.3 but the freshly released net-snmp-5.9.4 [1] breaks
> > > > compilation [2]. Now, libvirt-snmp has this crazy architecture, where
> > > > some sources are manually generated from src/LIBVIRT-MIB.txt, then
> > > > edited (added code to talk to libvirt) and then added to git.
> > > >
> > > > This is labor extensive and since I don't think libvirt-snmp is actually
> > > > used I'd like to sunset it. According to repology [3] only Gentoo (and
> > > > its clones) has the latest version (released ~5 years ago). And I doubt
> > > > it has any real users there.
> > > >
> > > > Thoughts?
> > >
> > > Per our private discussion Michal, Peter, and I concluded that archiving 
> > > the
> > > project in GitLab is a harmless operation that can be undone at any point 
> > > in
> > > time, so I went ahead and toggled the flag.
> >
> > Yes, archiving is the right thing to do in this scenario, and is trivially
> > reversed. We've already got a bunch of other archived repos :-)
> 
> We should clean up the container registry for this and other archived
> repositories too. I'm betting they take up a non-trivial amount of
> our group's storage quota for no good reason.

Turns out you can't delete containers if the project is archived :-)

So I've temporarily unarchived it, and am deleting the containers
and all the CI pipelines too, which should eliminate all major
storage usage.


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

Re: [PATCH v2] accel: Remove HAX accelerator

[Alex Bennée]

Philippe Mathieu-Daudé  writes:

> HAX is deprecated since commits 73741fda6c ("MAINTAINERS: Abort
> HAXM maintenance") and 90c167a1da ("docs/about/deprecated: Mark
> HAXM in QEMU as deprecated"), released in v8.0.0.
>
> Per the latest HAXM release (v7.8 [*]), the latest QEMU supported
> is v7.2:
>
>   Note: Up to this release, HAXM supports QEMU from 2.9.0 to 7.2.0.
>
> The next commit (https://github.com/intel/haxm/commit/da1b8ec072)
> added:
>
>   HAXM v7.8.0 is our last release and we will not accept
>   pull requests or respond to issues after this.
>
> It became very hard to build and test HAXM. Its previous
> maintainers made it clear they won't help.  It doesn't seem to be
> a very good use of QEMU maintainers to spend their time in a dead
> project. Save our time by removing this orphan zombie code.
>
> [*] https://github.com/intel/haxm/releases/tag/v7.8.0
>
> Reviewed-by: Richard Henderson 
> Acked-by: Markus Armbruster 
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Alex Bennée 

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: Sunset libvirt-snmp?

[Andrea Bolognani]

On Tue, Aug 22, 2023 at 12:44:16PM +0100, Daniel P. Berrangé wrote:
> On Tue, Aug 22, 2023 at 01:26:47PM +0200, Erik Skultety wrote:
> > On Mon, Aug 21, 2023 at 10:32:42AM +0200, Michal Prívozník wrote:
> > > It's been a while since libvirt-snmp was actively developed. Now it
> > > receives only libvirt-ci related commits. The code compiles with
> > > net-snmp-5.9.3 but the freshly released net-snmp-5.9.4 [1] breaks
> > > compilation [2]. Now, libvirt-snmp has this crazy architecture, where
> > > some sources are manually generated from src/LIBVIRT-MIB.txt, then
> > > edited (added code to talk to libvirt) and then added to git.
> > >
> > > This is labor extensive and since I don't think libvirt-snmp is actually
> > > used I'd like to sunset it. According to repology [3] only Gentoo (and
> > > its clones) has the latest version (released ~5 years ago). And I doubt
> > > it has any real users there.
> > >
> > > Thoughts?
> >
> > Per our private discussion Michal, Peter, and I concluded that archiving the
> > project in GitLab is a harmless operation that can be undone at any point in
> > time, so I went ahead and toggled the flag.
>
> Yes, archiving is the right thing to do in this scenario, and is trivially
> reversed. We've already got a bunch of other archived repos :-)

We should clean up the container registry for this and other archived
repositories too. I'm betting they take up a non-trivial amount of
our group's storage quota for no good reason.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt PATCH 0/7] external snapshot revert fixes

[Pavel Hrdina]

On Thu, Aug 31, 2023 at 04:54:59PM +0200, Pavel Hrdina wrote:
> This fixes reverting external snapshots to not error out in cases where  
> it should work and makes it correctly load the memory state when 
> reverting to snapshot of running VM.

Forget to mention that we need this to be in 9.7.0 otherwise I will have
to revert commit  introducing
capability to indicate that reverting external snapshots is usable.

Pavel

> Pavel Hrdina (7):
>   qemu_saveimage: extract starting process to qemuSaveImageStartProcess
>   qemuSaveImageStartProcess: allow setting reason for audit log
>   qemuSaveImageStartProcess: add snapshot argument
>   qemuSaveImageStartProcess: make it possible to use without header
>   qemu_snapshot: fix reverting external snapshot when not all disks are
> included
>   qemu_snapshot: correctly load the saved memory state file
>   NEWS: document support for reverting external snapshots
> 
>  NEWS.rst  |   8 +++
>  src/qemu/qemu_saveimage.c | 111 ++
>  src/qemu/qemu_saveimage.h |  14 +
>  src/qemu/qemu_snapshot.c  |  90 ---
>  4 files changed, 158 insertions(+), 65 deletions(-)
> 
> -- 
> 2.41.0
> 


signature.asc
Description: PGP signature

[libvirt PATCH 2/7] qemuSaveImageStartProcess: allow setting reason for audit log

[Pavel Hrdina]

When called by snapshot code we will need to use different reason.

Signed-off-by: Pavel Hrdina 
---
 src/qemu/qemu_saveimage.c | 6 --
 src/qemu/qemu_saveimage.h | 1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index 86f31d1820..1eedc900b9 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -576,6 +576,7 @@ qemuSaveImageOpen(virQEMUDriver *driver,
  * @cookie: cookie from memory state file
  * @asyncJob: type of asynchronous job
  * @start_flags: flags to start QEMU process with
+ * @reason: audit log reason
  * @started: boolean to store if QEMU process was started
  *
  * Start VM with existing memory state. Make sure that the stored memory state
@@ -593,6 +594,7 @@ qemuSaveImageStartProcess(virConnectPtr conn,
   qemuDomainSaveCookie *cookie,
   virDomainAsyncJob asyncJob,
   unsigned int start_flags,
+  const char *reason,
   bool *started)
 {
 qemuDomainObjPrivate *priv = vm->privateData;
@@ -660,7 +662,7 @@ qemuSaveImageStartProcess(virConnectPtr conn,
 rc = -1;
 }
 
-virDomainAuditStart(vm, "restored", *started);
+virDomainAuditStart(vm, reason, started);
 if (!*started || rc < 0)
 return -1;
 
@@ -700,7 +702,7 @@ qemuSaveImageStartVM(virConnectPtr conn,
 goto cleanup;
 
 if (qemuSaveImageStartProcess(conn, driver, vm, fd, path, header, cookie,
-  asyncJob, start_flags, ) < 0) {
+  asyncJob, start_flags, "restored", ) 
< 0) {
 goto cleanup;
 }
 
diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h
index af30b7f2ec..c6a701dcf5 100644
--- a/src/qemu/qemu_saveimage.h
+++ b/src/qemu/qemu_saveimage.h
@@ -67,6 +67,7 @@ qemuSaveImageStartProcess(virConnectPtr conn,
   qemuDomainSaveCookie *cookie,
   virDomainAsyncJob asyncJob,
   unsigned int start_flags,
+  const char *reason,
   bool *started);
 
 int
-- 
2.41.0

[libvirt PATCH 0/7] external snapshot revert fixes

[Pavel Hrdina]

This fixes reverting external snapshots to not error out in cases where  
it should work and makes it correctly load the memory state when 
reverting to snapshot of running VM.

Pavel Hrdina (7):
  qemu_saveimage: extract starting process to qemuSaveImageStartProcess
  qemuSaveImageStartProcess: allow setting reason for audit log
  qemuSaveImageStartProcess: add snapshot argument
  qemuSaveImageStartProcess: make it possible to use without header
  qemu_snapshot: fix reverting external snapshot when not all disks are
included
  qemu_snapshot: correctly load the saved memory state file
  NEWS: document support for reverting external snapshots

 NEWS.rst  |   8 +++
 src/qemu/qemu_saveimage.c | 111 ++
 src/qemu/qemu_saveimage.h |  14 +
 src/qemu/qemu_snapshot.c  |  90 ---
 4 files changed, 158 insertions(+), 65 deletions(-)

-- 
2.41.0

[libvirt PATCH 5/7] qemu_snapshot: fix reverting external snapshot when not all disks are included

[Pavel Hrdina]

We need to skip all disks that have snapshot type other than 'external'.

Signed-off-by: Pavel Hrdina 
---
 src/qemu/qemu_snapshot.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index d943281e35..ff85d56572 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2058,6 +2058,9 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm,
 virDomainSnapshotDiskDef *snapdisk = >disks[i];
 virDomainDiskDef *domdisk = domdef->disks[i];
 
+if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+continue;
+
 if (qemuSnapshotPrepareDiskExternal(domdisk, snapdisk, active, false) 
< 0)
 return -1;
 }
@@ -2098,6 +2101,9 @@ qemuSnapshotRevertExternalActive(virDomainObj *vm,
 return -1;
 
 for (i = 0; i < tmpsnapdef->ndisks; i++) {
+if (tmpsnapdef->disks[i].snapshot != 
VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+continue;
+
 if (qemuSnapshotDiskPrepareOne(snapctxt,
vm->def->disks[i],
tmpsnapdef->disks + i,
@@ -2188,6 +2194,9 @@ qemuSnapshotRevertExternalFinish(virDomainObj *vm,
 for (i = 0; i < curdef->nrevertdisks; i++) {
 virDomainSnapshotDiskDef *snapdisk = &(curdef->revertdisks[i]);
 
+if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+continue;
+
 if (virStorageSourceInit(snapdisk->src) < 0 ||
 virStorageSourceUnlink(snapdisk->src) < 0) {
 VIR_WARN("Failed to remove snapshot image '%s'",
@@ -2203,6 +2212,9 @@ qemuSnapshotRevertExternalFinish(virDomainObj *vm,
 for (i = 0; i < curdef->ndisks; i++) {
 virDomainSnapshotDiskDef *snapdisk = &(curdef->disks[i]);
 
+if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+continue;
+
 if (virStorageSourceInit(snapdisk->src) < 0 ||
 virStorageSourceUnlink(snapdisk->src) < 0) {
 VIR_WARN("Failed to remove snapshot image '%s'",
-- 
2.41.0

[libvirt PATCH 3/7] qemuSaveImageStartProcess: add snapshot argument

[Pavel Hrdina]

When called from snapshot code we will need to pass snapshot object in
order to make internal snapshots work correctly.

Signed-off-by: Pavel Hrdina 
---
 src/qemu/qemu_saveimage.c | 6 --
 src/qemu/qemu_saveimage.h | 1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index 1eedc900b9..73115af42d 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -572,6 +572,7 @@ qemuSaveImageOpen(virQEMUDriver *driver,
  * @vm: domain object
  * @fd: FD pointer of memory state file
  * @path: path to memory state file
+ * @snapshot: snapshot to load when starting QEMU process or NULL
  * @header: header from memory state file
  * @cookie: cookie from memory state file
  * @asyncJob: type of asynchronous job
@@ -590,6 +591,7 @@ qemuSaveImageStartProcess(virConnectPtr conn,
   virDomainObj *vm,
   int *fd,
   const char *path,
+  virDomainMomentObj *snapshot,
   virQEMUSaveHeader *header,
   qemuDomainSaveCookie *cookie,
   virDomainAsyncJob asyncJob,
@@ -634,7 +636,7 @@ qemuSaveImageStartProcess(virConnectPtr conn,
 priv->disableSlirp = true;
 
 if (qemuProcessStart(conn, driver, vm, cookie ? cookie->cpu : NULL,
- asyncJob, "stdio", *fd, path, NULL,
+ asyncJob, "stdio", *fd, path, snapshot,
  VIR_NETDEV_VPORT_PROFILE_OP_RESTORE,
  start_flags) == 0)
 *started = true;
@@ -701,7 +703,7 @@ qemuSaveImageStartVM(virConnectPtr conn,
  
virDomainXMLOptionGetSaveCookie(driver->xmlopt)) < 0)
 goto cleanup;
 
-if (qemuSaveImageStartProcess(conn, driver, vm, fd, path, header, cookie,
+if (qemuSaveImageStartProcess(conn, driver, vm, fd, path, NULL, header, 
cookie,
   asyncJob, start_flags, "restored", ) 
< 0) {
 goto cleanup;
 }
diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h
index c6a701dcf5..c5ad50558e 100644
--- a/src/qemu/qemu_saveimage.h
+++ b/src/qemu/qemu_saveimage.h
@@ -63,6 +63,7 @@ qemuSaveImageStartProcess(virConnectPtr conn,
   virDomainObj *vm,
   int *fd,
   const char *path,
+  virDomainMomentObj *snapshot,
   virQEMUSaveHeader *header,
   qemuDomainSaveCookie *cookie,
   virDomainAsyncJob asyncJob,
-- 
2.41.0

[libvirt PATCH 6/7] qemu_snapshot: correctly load the saved memory state file

[Pavel Hrdina]

Original code assumed that the memory state file is only migration
stream but it has additional metadata stored by libvirt. To correctly
load the memory state file we need to reuse code that is used when
restoring domain from saved image.

Signed-off-by: Pavel Hrdina 
---
 src/qemu/qemu_snapshot.c | 78 
 1 file changed, 47 insertions(+), 31 deletions(-)

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index ff85d56572..869b46a9a8 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2004,6 +2004,21 @@ qemuSnapshotRevertWriteMetadata(virDomainObj *vm,
 }
 
 
+typedef struct _qemuSnapshotRevertMemoryData {
+int fd;
+char *path;
+virQEMUSaveData *data;
+} qemuSnapshotRevertMemoryData;
+
+static void
+qemuSnapshotClearRevertMemoryData(qemuSnapshotRevertMemoryData *memdata)
+{
+VIR_FORCE_CLOSE(memdata->fd);
+virQEMUSaveDataFree(memdata->data);
+}
+G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(qemuSnapshotRevertMemoryData, 
qemuSnapshotClearRevertMemoryData);
+
+
 /**
  * qemuSnapshotRevertExternalPrepare:
  * @vm: domain object
@@ -2011,15 +2026,13 @@ qemuSnapshotRevertWriteMetadata(virDomainObj *vm,
  * @snap: snapshot object we are reverting to
  * @config: live domain definition
  * @inactiveConfig: offline domain definition
- * memsnapFD: pointer to store memory state file FD or NULL
- * memsnapPath: pointer to store memory state file path or NULL
+ * @memdata: struct with data to load memory state
  *
  * Prepare new temporary snapshot definition @tmpsnapdef that will
  * be used while creating new overlay files after reverting to snapshot
  * @snap. In case we are reverting to snapshot with memory state it will
- * open it and pass FD via @memsnapFD and path to the file via
- * @memsnapPath, caller is responsible for freeing both @memsnapFD and
- * memsnapPath.
+ * open it and store necessary data in @memdata. Caller is responsible
+ * to clear the data by using qemuSnapshotClearRevertMemoryData().
  *
  * Returns 0 in success, -1 on error.
  */
@@ -2029,8 +2042,7 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm,
   virDomainMomentObj *snap,
   virDomainDef *config,
   virDomainDef *inactiveConfig,
-  int *memsnapFD,
-  char **memsnapPath)
+  qemuSnapshotRevertMemoryData *memdata)
 {
 size_t i;
 bool active = virDomainObjIsActive(vm);
@@ -2065,12 +2077,21 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm,
 return -1;
 }
 
-if (memsnapFD && memsnapPath && snapdef->memorysnapshotfile) {
+if (memdata && snapdef->memorysnapshotfile) {
 virQEMUDriver *driver = ((qemuDomainObjPrivate *) 
vm->privateData)->driver;
-g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virDomainDef) savedef = NULL;
 
-*memsnapPath = snapdef->memorysnapshotfile;
-*memsnapFD = qemuDomainOpenFile(cfg, NULL, *memsnapPath, O_RDONLY, 
NULL);
+memdata->path = snapdef->memorysnapshotfile;
+memdata->fd = qemuSaveImageOpen(driver, NULL, memdata->path,
+, >data,
+false, NULL,
+false, false);
+
+if (memdata->fd < 0)
+return -1;
+
+if (!virDomainDefCheckABIStability(savedef, domdef, driver->xmlopt))
+return -1;
 }
 
 return 0;
@@ -2254,13 +2275,13 @@ qemuSnapshotRevertActive(virDomainObj *vm,
 virObjectEvent *event = NULL;
 virObjectEvent *event2 = NULL;
 virDomainMomentObj *loadSnap = NULL;
-VIR_AUTOCLOSE memsnapFD = -1;
-char *memsnapPath = NULL;
 int detail;
 bool defined = false;
 qemuDomainSaveCookie *cookie = (qemuDomainSaveCookie *) snapdef->cookie;
 int rc;
 g_autoptr(virDomainSnapshotDef) tmpsnapdef = NULL;
+g_auto(qemuSnapshotRevertMemoryData) memdata = { -1, NULL, NULL };
+bool started = false;
 
 start_flags |= VIR_QEMU_PROCESS_START_PAUSED;
 
@@ -2284,7 +2305,7 @@ qemuSnapshotRevertActive(virDomainObj *vm,
 
 if (qemuSnapshotRevertExternalPrepare(vm, tmpsnapdef, snap,
   *config, *inactiveConfig,
-  , ) < 0) {
+  ) < 0) {
 return -1;
 }
 } else {
@@ -2298,28 +2319,24 @@ qemuSnapshotRevertActive(virDomainObj *vm,
 
 virDomainObjAssignDef(vm, config, true, NULL);
 
-/* No cookie means libvirt which saved the domain was too old to
- * mess up the CPU definitions.
- */
-if (cookie &&
-qemuDomainFixupCPUs(vm, >cpu) < 0)
+if (qemuSaveImageStartProcess(snapshot->domain->conn, driver, vm,
+  , memdata.path, 

[libvirt PATCH 7/7] NEWS: document support for reverting external snapshots

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 NEWS.rst | 8 
 1 file changed, 8 insertions(+)

diff --git a/NEWS.rst b/NEWS.rst
index e40c8ac259..a3be76d6cc 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -28,6 +28,14 @@ v9.7.0 (unreleased)
 2) pre-binding the variant driver using the ``--driver`` option of
``virsh nodedev-detach``.
 
+  * QEMU: implement reverting external snapshots
+
+Reverting external snapshots is now possible using the existing API
+``virDomainSnapshotRevert()``. Management application can check host
+capabilities for  element within the list of
+guest features to see if the current libvirt supports both deleting
+and reverting external snapshots.
+
 * **Improvements**
 
 * **Bug fixes**
-- 
2.41.0

[libvirt PATCH 4/7] qemuSaveImageStartProcess: make it possible to use without header

[Pavel Hrdina]

When used with internal snapshots there is no header to be used and no
memory state to be decompressed.

Signed-off-by: Pavel Hrdina 
---
 src/qemu/qemu_saveimage.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index 73115af42d..2538732901 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -573,7 +573,7 @@ qemuSaveImageOpen(virQEMUDriver *driver,
  * @fd: FD pointer of memory state file
  * @path: path to memory state file
  * @snapshot: snapshot to load when starting QEMU process or NULL
- * @header: header from memory state file
+ * @header: header from memory state file or NULL
  * @cookie: cookie from memory state file
  * @asyncJob: type of asynchronous job
  * @start_flags: flags to start QEMU process with
@@ -605,7 +605,7 @@ qemuSaveImageStartProcess(virConnectPtr conn,
 g_autofree char *errbuf = NULL;
 int rc = 0;
 
-if ((header->version == 2) &&
+if (header && (header->version == 2) &&
 (header->compressed != QEMU_SAVE_FORMAT_RAW)) {
 if (!(cmd = qemuSaveImageGetCompressionCommand(header->compressed)))
 return -1;
-- 
2.41.0

[libvirt PATCH 1/7] qemu_saveimage: extract starting process to qemuSaveImageStartProcess

[Pavel Hrdina]

Part of qemuSaveImageStartVM() function will be used when reverting
external snapshots. To avoid duplicating code and logic extract the
shared bits into separate function.

Signed-off-by: Pavel Hrdina 
---
 src/qemu/qemu_saveimage.c | 103 ++
 src/qemu/qemu_saveimage.h |  12 +
 2 files changed, 83 insertions(+), 32 deletions(-)

diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index 41310d6a9a..86f31d1820 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -565,42 +565,46 @@ qemuSaveImageOpen(virQEMUDriver *driver,
 return ret;
 }
 
+/**
+ * qemuSaveImageStartProcess:
+ * @conn: connection object
+ * @driver: qemu driver object
+ * @vm: domain object
+ * @fd: FD pointer of memory state file
+ * @path: path to memory state file
+ * @header: header from memory state file
+ * @cookie: cookie from memory state file
+ * @asyncJob: type of asynchronous job
+ * @start_flags: flags to start QEMU process with
+ * @started: boolean to store if QEMU process was started
+ *
+ * Start VM with existing memory state. Make sure that the stored memory state
+ * is correctly decompressed so it can be loaded by QEMU process.
+ *
+ * Returns 0 on success, -1 on error.
+ */
 int
-qemuSaveImageStartVM(virConnectPtr conn,
- virQEMUDriver *driver,
- virDomainObj *vm,
- int *fd,
- virQEMUSaveData *data,
- const char *path,
- bool start_paused,
- bool reset_nvram,
- virDomainAsyncJob asyncJob)
+qemuSaveImageStartProcess(virConnectPtr conn,
+  virQEMUDriver *driver,
+  virDomainObj *vm,
+  int *fd,
+  const char *path,
+  virQEMUSaveHeader *header,
+  qemuDomainSaveCookie *cookie,
+  virDomainAsyncJob asyncJob,
+  unsigned int start_flags,
+  bool *started)
 {
 qemuDomainObjPrivate *priv = vm->privateData;
-int ret = -1;
-bool started = false;
-virObjectEvent *event;
 VIR_AUTOCLOSE intermediatefd = -1;
 g_autoptr(virCommand) cmd = NULL;
 g_autofree char *errbuf = NULL;
-g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
-virQEMUSaveHeader *header = >header;
-g_autoptr(qemuDomainSaveCookie) cookie = NULL;
 int rc = 0;
-unsigned int start_flags = VIR_QEMU_PROCESS_START_PAUSED |
-VIR_QEMU_PROCESS_START_GEN_VMID;
-
-if (reset_nvram)
-start_flags |= VIR_QEMU_PROCESS_START_RESET_NVRAM;
-
-if (virSaveCookieParseString(data->cookie, (virObject **),
- 
virDomainXMLOptionGetSaveCookie(driver->xmlopt)) < 0)
-goto cleanup;
 
 if ((header->version == 2) &&
 (header->compressed != QEMU_SAVE_FORMAT_RAW)) {
 if (!(cmd = qemuSaveImageGetCompressionCommand(header->compressed)))
-goto cleanup;
+return -1;
 
 intermediatefd = *fd;
 *fd = -1;
@@ -613,7 +617,7 @@ qemuSaveImageStartVM(virConnectPtr conn,
 if (virCommandRunAsync(cmd, NULL) < 0) {
 *fd = intermediatefd;
 intermediatefd = -1;
-goto cleanup;
+return -1;
 }
 }
 
@@ -622,7 +626,7 @@ qemuSaveImageStartVM(virConnectPtr conn,
  */
 if (cookie &&
 qemuDomainFixupCPUs(vm, >cpu) < 0)
-goto cleanup;
+return -1;
 
 if (cookie && !cookie->slirpHelper)
 priv->disableSlirp = true;
@@ -631,12 +635,12 @@ qemuSaveImageStartVM(virConnectPtr conn,
  asyncJob, "stdio", *fd, path, NULL,
  VIR_NETDEV_VPORT_PROFILE_OP_RESTORE,
  start_flags) == 0)
-started = true;
+*started = true;
 
 if (intermediatefd != -1) {
 virErrorPtr orig_err = NULL;
 
-if (!started) {
+if (!*started) {
 /* if there was an error setting up qemu, the intermediate
  * process will wait forever to write to stdout, so we
  * must manually kill it and ignore any error related to
@@ -656,15 +660,50 @@ qemuSaveImageStartVM(virConnectPtr conn,
 rc = -1;
 }
 
-virDomainAuditStart(vm, "restored", started);
-if (!started || rc < 0)
-goto cleanup;
+virDomainAuditStart(vm, "restored", *started);
+if (!*started || rc < 0)
+return -1;
 
 /* qemuProcessStart doesn't unset the qemu error reporting infrastructure
  * in case of migration (which is used in this case) so we need to reset it
  * so that the handle to virtlogd is not held open unnecessarily */
 qemuMonitorSetDomainLog(qemuDomainGetMonitor(vm), NULL, NULL, NULL);
 
+return 0;
+}
+
+int
+qemuSaveImageStartVM(virConnectPtr conn,
+ 

Re: [libvirt PATCH 16/33] gitlab-ci.yml: Add 'after_script' stage to prep for artifact collection

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:24PM +0200, Erik Skultety wrote:
> This is one of the preparation steps that if not done would otherwise
> collide with local container executions where we:
> 1) don't collect artifacts
> 2) are not limited by GitLab's environment and hence moving build
>artifacts to unusual places would only cause confusion when doing
>local build inspection in an interactive container shell session
> 
> Signed-off-by: Erik Skultety 
> ---
>  .gitlab-ci.yml | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)

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

Re: [libvirt PATCH 05/33] ci: build.sh: Add a wrapper function executing 'shell' commands

[Daniel P . Berrangé]

On Fri, Aug 25, 2023 at 07:55:13PM +0200, Erik Skultety wrote:
> This would normally be not needed at all, but the problem here is the
> Shell-in-YAML which GitLab interprets. It outputs every command that
> appears as a line in the 'script' segment in a color-coded fashion for
> easy identification of problems. Well, that useful feature is lost when
> there's indirection and one script calls into another in which case it
> would only output the respective script name which would make failure
> investigation harder. This simple helper tackles that by echoing the
> command to be run by any script/function with a color escape sequence
> so that we don't lose track of the *actual* shell commands being run as
> part of the GitLab job pipelines. An example of what the output then
> might look like:
> [RUN COMMAND]: 'meson compile -C build install-web'

FWIW, I happened to come across this

  https://docs.gitlab.com/ee/ci/jobs/index.html#custom-collapsible-sections

which lets scripts output messages in a special format that get
turned into collapsible sections when browsing logs in gitlab UI.
Not sure it is something we'd use in this specific patch, but
might be conceptually useful somewhere.

> 
> Signed-off-by: Erik Skultety 
> ---
>  ci/build.sh | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/ci/build.sh b/ci/build.sh
> index 02ff1a8388..d4fbf0ab37 100644
> --- a/ci/build.sh
> +++ b/ci/build.sh
> @@ -25,6 +25,12 @@ meson setup build --werror -Dsystem=true $MESON_ARGS || \
>  
>  ninja -C build $NINJA_ARGS
>  
> +run_cmd() {
> +local CMD="$(echo $CMD | tr -s ' ')" # truncate any additional spaces
> +
> +printf "\e[93m[RUN COMMAND]: '%s'\e[0m\n" "$CMD"; eval "$CMD"
> +}
> +
>  run_meson_setup() {
>  if [ -d "${GIT_ROOT}/build/meson-private" ]; then
>  return
> -- 
> 2.41.0
> 

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

[PULL 01/41] accel: Remove HAX accelerator

[Philippe Mathieu-Daudé]

HAX is deprecated since commits 73741fda6c ("MAINTAINERS: Abort
HAXM maintenance") and 90c167a1da ("docs/about/deprecated: Mark
HAXM in QEMU as deprecated"), released in v8.0.0.

Per the latest HAXM release (v7.8 [*]), the latest QEMU supported
is v7.2:

  Note: Up to this release, HAXM supports QEMU from 2.9.0 to 7.2.0.

The next commit (https://github.com/intel/haxm/commit/da1b8ec072)
added:

  HAXM v7.8.0 is our last release and we will not accept
  pull requests or respond to issues after this.

It became very hard to build and test HAXM. Its previous
maintainers made it clear they won't help.  It doesn't seem to be
a very good use of QEMU maintainers to spend their time in a dead
project. Save our time by removing this orphan zombie code.

[*] https://github.com/intel/haxm/releases/tag/v7.8.0

Reviewed-by: Richard Henderson 
Acked-by: Markus Armbruster 
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Thomas Huth 
Message-Id: <20230831082016.60885-1-phi...@linaro.org>
---
 MAINTAINERS   |8 -
 docs/about/build-platforms.rst|2 +-
 docs/about/deprecated.rst |6 -
 docs/about/index.rst  |2 +-
 docs/about/removed-features.rst   |   11 +-
 docs/system/index.rst |2 +-
 docs/system/introduction.rst  |3 -
 meson.build   |7 -
 include/exec/poison.h |1 -
 include/hw/core/cpu.h |2 +-
 include/sysemu/hax.h  |   49 -
 include/sysemu/hw_accel.h |1 -
 target/i386/hax/hax-accel-ops.h   |   31 -
 target/i386/hax/hax-i386.h|   98 --
 target/i386/hax/hax-interface.h   |  369 --
 target/i386/hax/hax-posix.h   |   61 -
 target/i386/hax/hax-windows.h |   88 --
 accel/stubs/hax-stub.c|   24 -
 hw/intc/apic_common.c |3 +-
 softmmu/cpus.c|6 -
 softmmu/vl.c  |6 -
 target/i386/hax/hax-accel-ops.c   |  105 --
 target/i386/hax/hax-all.c | 1141 -
 target/i386/hax/hax-mem.c |  323 -
 target/i386/hax/hax-posix.c   |  305 -
 target/i386/hax/hax-windows.c |  485 ---
 accel/Kconfig |3 -
 accel/stubs/meson.build   |1 -
 meson_options.txt |2 -
 qemu-options.hx   |8 +-
 .../ci/org.centos/stream/8/x86_64/configure   |1 -
 scripts/meson-buildoptions.sh |3 -
 target/i386/hax/meson.build   |7 -
 target/i386/meson.build   |1 -
 34 files changed, 16 insertions(+), 3149 deletions(-)
 delete mode 100644 include/sysemu/hax.h
 delete mode 100644 target/i386/hax/hax-accel-ops.h
 delete mode 100644 target/i386/hax/hax-i386.h
 delete mode 100644 target/i386/hax/hax-interface.h
 delete mode 100644 target/i386/hax/hax-posix.h
 delete mode 100644 target/i386/hax/hax-windows.h
 delete mode 100644 accel/stubs/hax-stub.c
 delete mode 100644 target/i386/hax/hax-accel-ops.c
 delete mode 100644 target/i386/hax/hax-all.c
 delete mode 100644 target/i386/hax/hax-mem.c
 delete mode 100644 target/i386/hax/hax-posix.c
 delete mode 100644 target/i386/hax/hax-windows.c
 delete mode 100644 target/i386/hax/meson.build

diff --git a/MAINTAINERS b/MAINTAINERS
index 6111b6b4d9..3b29568ed4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -543,14 +543,6 @@ F: include/sysemu/xen.h
 F: include/sysemu/xen-mapcache.h
 F: stubs/xen-hw-stub.c
 
-Guest CPU Cores (HAXM)
--
-X86 HAXM CPUs
-S: Orphan
-F: accel/stubs/hax-stub.c
-F: include/sysemu/hax.h
-F: target/i386/hax/
-
 Guest CPU Cores (NVMM)
 --
 NetBSD Virtual Machine Monitor (NVMM) CPU support
diff --git a/docs/about/build-platforms.rst b/docs/about/build-platforms.rst
index 0e2cb9e770..f2a7aec56f 100644
--- a/docs/about/build-platforms.rst
+++ b/docs/about/build-platforms.rst
@@ -52,7 +52,7 @@ Those hosts are officially supported, with various 
accelerators:
* - SPARC
  - tcg
* - x86
- - hax, hvf (64 bit only), kvm, nvmm, tcg, whpx (64 bit only), xen
+ - hvf (64 bit only), kvm, nvmm, tcg, whpx (64 bit only), xen
 
 Other host architectures are not supported. It is possible to build QEMU system
 emulation on an unsupported host architecture using the configure
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 92a2bafd2b..dc4da95329 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -105,12 +105,6 @@ Use ``-machine hpet=off`` instead.
 The ``-no-acpi`` setting has been turned into a machine property.
 Use ``-machine 

Re: [libvirt PATCH] rpm: Recommend libvirt-daemon for with_modular_daemons distros

[Andrea Bolognani]

On Thu, Aug 31, 2023 at 10:29:23AM +0100, Daniel P. Berrangé wrote:
> On Thu, Aug 31, 2023 at 02:14:49AM -0700, Andrea Bolognani wrote:
> > On Thu, Aug 31, 2023 at 09:50:07AM +0100, Daniel P. Berrangé wrote:
> > > I'm not seeing a good way to deal with the upgrade problem though.
> >
> > Yeah. The two alternatives that I was able to come up with are
> >
> >   1) documenting in the release notes for Fedora and RHEL that
> >  people using the monolithic daemon need to run
> >
> >$ dnf mark install libvirt-daemon
> >
> >  before performing the upgrade;
> >
> >   2) in libvirt-daemon's %postinst, detect whether libvirtd is
> >  enabled and if so run the command above.
> >
> > I don't like 1) because it's really easy to miss something like that,
> > and if we're being honest most people don't even read the release
> > notes before performing an upgrade. The expectation is that things
> > will just work without user intervention, and I don't think it's an
> > unreasonable one.
> >
> > 2) would work but it feels so hacky that I didn't really consider
> > proposing it as an actual patch.
>
> Isnt %postinst too late in the upgrade process for this to work ?

I haven't actually tested this approach, but I think it should be
fine. The libvirt-daemon package gets updated as expected during
upgrade, it's just that *after* the upgrade it's considered an
obsolete dependency and thus candidate for autoremoval.

So if you run

  $ dnf update && dnf autoremove

the attempt to remove libvirt-daemon will only occur when the second
command is executed, by which point %postinst will have already run.

> > > Possibly the next step would be to stop building libvirtd by
> > > default in upstream releases[1], and figure out a way to attempt
> > > to auto switch installs to modular daemons during upgrade.
> >
> > We have purposefully avoided converting monolithic deployments to
> > modular ones so far, but if we want to ever be able to drop the
> > monolithic daemon I'm afraid that at some point we'll have to take
> > the plunge. The alternatives are breaking all monolithic deployments
> > or carrying it around forever.
>
> Yep, we'll have to bite the bullet eventually and make a decision,
> but for now lets keep our heads in the sand a bit longer :-)
>
> Meanwhile to be explicit about your current patch
>
>   Reviewed-by: Daniel P. Berrangé 

Thanks, I'll push it then.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt PATCH] rpm: Recommend libvirt-daemon for with_modular_daemons distros

[Daniel P . Berrangé]

On Thu, Aug 31, 2023 at 02:14:49AM -0700, Andrea Bolognani wrote:
> On Thu, Aug 31, 2023 at 09:50:07AM +0100, Daniel P. Berrangé wrote:
> > I don't much like it to be honest, as I was hoping we would get away
> > from having it exist in any new installs, such that people were not
> > mis-directed into trying to use it.
> >
> > I'm not seeing a good way to deal with the upgrade problem though.
> 
> Yeah. The two alternatives that I was able to come up with are
> 
>   1) documenting in the release notes for Fedora and RHEL that
>  people using the monolithic daemon need to run
> 
>$ dnf mark install libvirt-daemon
> 
>  before performing the upgrade;
> 
>   2) in libvirt-daemon's %postinst, detect whether libvirtd is
>  enabled and if so run the command above.
> 
> I don't like 1) because it's really easy to miss something like that,
> and if we're being honest most people don't even read the release
> notes before performing an upgrade. The expectation is that things
> will just work without user intervention, and I don't think it's an
> unreasonable one.
> 
> 2) would work but it feels so hacky that I didn't really consider
> proposing it as an actual patch.

Isnt %postinst too late in the upgrade process for this to work ?

> > Possibly the next step would be to stop building libvirtd by
> > default in upstream releases[1], and figure out a way to attempt
> > to auto switch installs to modular daemons during upgrade.
> 
> We have purposefully avoided converting monolithic deployments to
> modular ones so far, but if we want to ever be able to drop the
> monolithic daemon I'm afraid that at some point we'll have to take
> the plunge. The alternatives are breaking all monolithic deployments
> or carrying it around forever.

Yep, we'll have to bite the bullet eventually and make a decision,
but for now lets keep our heads in the sand a bit longer :-)

Meanwhile to be explicit about your current patch

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

Re: [libvirt PATCH] rpm: Recommend libvirt-daemon for with_modular_daemons distros

[Erik Skultety]

On Thu, Aug 31, 2023 at 09:50:07AM +0100, Daniel P. Berrangé wrote:
> On Thu, Aug 31, 2023 at 01:43:44AM -0700, Andrea Bolognani wrote:
> > On Wed, Aug 30, 2023 at 07:25:23PM +0200, Erik Skultety wrote:
> > > On Wed, Aug 30, 2023 at 06:22:33PM +0200, Andrea Bolognani wrote:
> > > > A default deployment on modern distros uses modular daemons but
> > > > switching back to the monolithic daemon, while not recommended,
> > > > is still considered a perfectly valid option.
> > > >
> > > > For a monolithic daemon deployment, the upgrade to libvirt 9.2.0
> > > > or newer works as expected; a subsequent call to dnf autoremove,
> > > > however, results in the libvirt-daemon package being removed and
> > > > the deployment no longer working.
> > > >
> > > > In order to avoid that situation, mark the libvirt-daemon as
> > > > recommended.
> > > >
> > > > This will unfortunately result in it being included in most
> > > > installations despite not being necessary, but considering that
> > > > the alternative is breaking existing setups on upgrade it feels
> > > > like a reasonable tradeoff.
> > > >
> > > > Moreover, since the dependency on libvirt-daemon is just a weak
> > > > one, it's still possible for people looking to minimize the
> > > > footprint of their installation to manually remove the package
> > > > after installation, mitigating the drawbacks of this approach.
> > >
> > > Not just that, experienced users can even skip installation of it with 
> > > using
> > > '--setopt=install_weak_deps=False'.
> > 
> > Yeah, but that tells dnf to skip *all* weak dependencies, so for
> > example you won't get passt either, which you might have wanted.
> > 
> > apt is better in this regard, because while you can use
> > 
> >   $ apt-get install --no-install-recommends libvirt-daemon-kvm
> > 
> > to skip all weak dependencies, same as dnf, you can also use
> > 
> >   $ apt-get install libvirt-daemon-kvm libvirt-daemon-
> > 
> > (note the trailing -) to install all other weak dependencies but skip
> > libvirt-daemon specifically.
> > 
> > > Footprint aside, I don't see a practical
> > > problem having both installed even if unused since it allows users 
> > > switching
> > > between the deployment modes seamlessly without having to go and install 
> > > an
> > > extra package.
> > 
> > Having both available means that it's easier for people to end up
> > with messed up deployments.
> > 
> > For example, some automation that was written before modular daemons
> > were introduced might unconditionally enable libvirtd. If libvirtd is
> > present on the machine, that will result in both modular and
> > monolithic daemons being enabled; if it isn't, you will get an error
> > message and an opportunity to improve the automation so that it's
> > aware of modular daemons.
> > 
> > That said, this has been the status quo for the last few years, when
> > it was simply not possible to get rid of libvirtd, so this patch at
> > least doesn't make things any worse :)
> > 
> > > FWIW:
> > > Reviewed-by: Erik Skultety 
> > 
> > Thanks.
> > 
> > Daniel, Jirka, do you have any objections or can I go ahead and
> > merge?
> 
> I don't much like it to be honest, as I was hoping we would get away
> from having it exist in any new installs, such that people were not
> mis-directed into trying to use it.

This is IMO a problem that ought to be solved by thorough documentation as
something that despite best effort can't be eliminated easily in a transparent
manner and an easy fix is in place (enable/disable the right services).

> 
> I'm not seeing a good way to deal with the upgrade problem though.
> I could possibly make the argument that Fedora has defaulted to
> modular daemons for new installs for long enough that we should
> assume all installs are running modular. Still, we have a proof
> by counter-example that such an assumption is not entirely true.
> 
> I guess your proposal here is still better than the historical
> practice though, because even if it gets installd by default,
> we can at least now erase it if desired.
> 
> Possibly the next step would be to stop building libvirtd by
> default in upstream releases[1], and figure out a way to attempt
> to auto switch installs to modular daemons during upgrade.
> 
> And then eventually we can delete it entirely upstream.

Why though? Monolithic daemon still plays an important role in development
deployments where this setup is much much easier to debug than in the modular
daemons setup.

Erik

Re: [libvirt PATCH] rpm: Recommend libvirt-daemon for with_modular_daemons distros

[Daniel P . Berrangé]

On Thu, Aug 31, 2023 at 11:00:27AM +0200, Erik Skultety wrote:
> On Thu, Aug 31, 2023 at 09:50:07AM +0100, Daniel P. Berrangé wrote:
> > On Thu, Aug 31, 2023 at 01:43:44AM -0700, Andrea Bolognani wrote:
> > > On Wed, Aug 30, 2023 at 07:25:23PM +0200, Erik Skultety wrote:
> > > > On Wed, Aug 30, 2023 at 06:22:33PM +0200, Andrea Bolognani wrote:
> > > > > A default deployment on modern distros uses modular daemons but
> > > > > switching back to the monolithic daemon, while not recommended,
> > > > > is still considered a perfectly valid option.
> > > > >
> > > > > For a monolithic daemon deployment, the upgrade to libvirt 9.2.0
> > > > > or newer works as expected; a subsequent call to dnf autoremove,
> > > > > however, results in the libvirt-daemon package being removed and
> > > > > the deployment no longer working.
> > > > >
> > > > > In order to avoid that situation, mark the libvirt-daemon as
> > > > > recommended.
> > > > >
> > > > > This will unfortunately result in it being included in most
> > > > > installations despite not being necessary, but considering that
> > > > > the alternative is breaking existing setups on upgrade it feels
> > > > > like a reasonable tradeoff.
> > > > >
> > > > > Moreover, since the dependency on libvirt-daemon is just a weak
> > > > > one, it's still possible for people looking to minimize the
> > > > > footprint of their installation to manually remove the package
> > > > > after installation, mitigating the drawbacks of this approach.
> > > >
> > > > Not just that, experienced users can even skip installation of it with 
> > > > using
> > > > '--setopt=install_weak_deps=False'.
> > > 
> > > Yeah, but that tells dnf to skip *all* weak dependencies, so for
> > > example you won't get passt either, which you might have wanted.
> > > 
> > > apt is better in this regard, because while you can use
> > > 
> > >   $ apt-get install --no-install-recommends libvirt-daemon-kvm
> > > 
> > > to skip all weak dependencies, same as dnf, you can also use
> > > 
> > >   $ apt-get install libvirt-daemon-kvm libvirt-daemon-
> > > 
> > > (note the trailing -) to install all other weak dependencies but skip
> > > libvirt-daemon specifically.
> > > 
> > > > Footprint aside, I don't see a practical
> > > > problem having both installed even if unused since it allows users 
> > > > switching
> > > > between the deployment modes seamlessly without having to go and 
> > > > install an
> > > > extra package.
> > > 
> > > Having both available means that it's easier for people to end up
> > > with messed up deployments.
> > > 
> > > For example, some automation that was written before modular daemons
> > > were introduced might unconditionally enable libvirtd. If libvirtd is
> > > present on the machine, that will result in both modular and
> > > monolithic daemons being enabled; if it isn't, you will get an error
> > > message and an opportunity to improve the automation so that it's
> > > aware of modular daemons.
> > > 
> > > That said, this has been the status quo for the last few years, when
> > > it was simply not possible to get rid of libvirtd, so this patch at
> > > least doesn't make things any worse :)
> > > 
> > > > FWIW:
> > > > Reviewed-by: Erik Skultety 
> > > 
> > > Thanks.
> > > 
> > > Daniel, Jirka, do you have any objections or can I go ahead and
> > > merge?
> > 
> > I don't much like it to be honest, as I was hoping we would get away
> > from having it exist in any new installs, such that people were not
> > mis-directed into trying to use it.
> 
> This is IMO a problem that ought to be solved by thorough documentation as
> something that despite best effort can't be eliminated easily in a transparent
> manner and an easy fix is in place (enable/disable the right services).

The volume of bugs I see which involve confusion between modular and
monolithic daemon is large enough that I don't think docs are a viable
solution. The confusion will remain as long as both exist in the
installation IMHO.

> > I'm not seeing a good way to deal with the upgrade problem though.
> > I could possibly make the argument that Fedora has defaulted to
> > modular daemons for new installs for long enough that we should
> > assume all installs are running modular. Still, we have a proof
> > by counter-example that such an assumption is not entirely true.
> > 
> > I guess your proposal here is still better than the historical
> > practice though, because even if it gets installd by default,
> > we can at least now erase it if desired.
> > 
> > Possibly the next step would be to stop building libvirtd by
> > default in upstream releases[1], and figure out a way to attempt
> > to auto switch installs to modular daemons during upgrade.
> > 
> > And then eventually we can delete it entirely upstream.
> 
> Why though? Monolithic daemon still plays an important role in development
> deployments where this setup is much much easier to debug than in the modular
> daemons setup.

Personally I find the 

Re: [libvirt PATCH] rpm: Recommend libvirt-daemon for with_modular_daemons distros

[Andrea Bolognani]

On Thu, Aug 31, 2023 at 09:50:07AM +0100, Daniel P. Berrangé wrote:
> I don't much like it to be honest, as I was hoping we would get away
> from having it exist in any new installs, such that people were not
> mis-directed into trying to use it.
>
> I'm not seeing a good way to deal with the upgrade problem though.

Yeah. The two alternatives that I was able to come up with are

  1) documenting in the release notes for Fedora and RHEL that
 people using the monolithic daemon need to run

   $ dnf mark install libvirt-daemon

 before performing the upgrade;

  2) in libvirt-daemon's %postinst, detect whether libvirtd is
 enabled and if so run the command above.

I don't like 1) because it's really easy to miss something like that,
and if we're being honest most people don't even read the release
notes before performing an upgrade. The expectation is that things
will just work without user intervention, and I don't think it's an
unreasonable one.

2) would work but it feels so hacky that I didn't really consider
proposing it as an actual patch.

> Possibly the next step would be to stop building libvirtd by
> default in upstream releases[1], and figure out a way to attempt
> to auto switch installs to modular daemons during upgrade.

We have purposefully avoided converting monolithic deployments to
modular ones so far, but if we want to ever be able to drop the
monolithic daemon I'm afraid that at some point we'll have to take
the plunge. The alternatives are breaking all monolithic deployments
or carrying it around forever.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt PATCH] rpm: Recommend libvirt-daemon for with_modular_daemons distros

[Daniel P . Berrangé]

On Thu, Aug 31, 2023 at 01:43:44AM -0700, Andrea Bolognani wrote:
> On Wed, Aug 30, 2023 at 07:25:23PM +0200, Erik Skultety wrote:
> > On Wed, Aug 30, 2023 at 06:22:33PM +0200, Andrea Bolognani wrote:
> > > A default deployment on modern distros uses modular daemons but
> > > switching back to the monolithic daemon, while not recommended,
> > > is still considered a perfectly valid option.
> > >
> > > For a monolithic daemon deployment, the upgrade to libvirt 9.2.0
> > > or newer works as expected; a subsequent call to dnf autoremove,
> > > however, results in the libvirt-daemon package being removed and
> > > the deployment no longer working.
> > >
> > > In order to avoid that situation, mark the libvirt-daemon as
> > > recommended.
> > >
> > > This will unfortunately result in it being included in most
> > > installations despite not being necessary, but considering that
> > > the alternative is breaking existing setups on upgrade it feels
> > > like a reasonable tradeoff.
> > >
> > > Moreover, since the dependency on libvirt-daemon is just a weak
> > > one, it's still possible for people looking to minimize the
> > > footprint of their installation to manually remove the package
> > > after installation, mitigating the drawbacks of this approach.
> >
> > Not just that, experienced users can even skip installation of it with using
> > '--setopt=install_weak_deps=False'.
> 
> Yeah, but that tells dnf to skip *all* weak dependencies, so for
> example you won't get passt either, which you might have wanted.
> 
> apt is better in this regard, because while you can use
> 
>   $ apt-get install --no-install-recommends libvirt-daemon-kvm
> 
> to skip all weak dependencies, same as dnf, you can also use
> 
>   $ apt-get install libvirt-daemon-kvm libvirt-daemon-
> 
> (note the trailing -) to install all other weak dependencies but skip
> libvirt-daemon specifically.
> 
> > Footprint aside, I don't see a practical
> > problem having both installed even if unused since it allows users switching
> > between the deployment modes seamlessly without having to go and install an
> > extra package.
> 
> Having both available means that it's easier for people to end up
> with messed up deployments.
> 
> For example, some automation that was written before modular daemons
> were introduced might unconditionally enable libvirtd. If libvirtd is
> present on the machine, that will result in both modular and
> monolithic daemons being enabled; if it isn't, you will get an error
> message and an opportunity to improve the automation so that it's
> aware of modular daemons.
> 
> That said, this has been the status quo for the last few years, when
> it was simply not possible to get rid of libvirtd, so this patch at
> least doesn't make things any worse :)
> 
> > FWIW:
> > Reviewed-by: Erik Skultety 
> 
> Thanks.
> 
> Daniel, Jirka, do you have any objections or can I go ahead and
> merge?

I don't much like it to be honest, as I was hoping we would get away
from having it exist in any new installs, such that people were not
mis-directed into trying to use it.

I'm not seeing a good way to deal with the upgrade problem though.
I could possibly make the argument that Fedora has defaulted to
modular daemons for new installs for long enough that we should
assume all installs are running modular. Still, we have a proof
by counter-example that such an assumption is not entirely true.

I guess your proposal here is still better than the historical
practice though, because even if it gets installd by default,
we can at least now erase it if desired.

Possibly the next step would be to stop building libvirtd by
default in upstream releases[1], and figure out a way to attempt
to auto switch installs to modular daemons during upgrade.

And then eventually we can delete it entirely upstream.

With regards,
Daniel

[1] still want a build opt-in for a while for any (probably enterprise0
distros that want to keep it around longer for compatibility reasons
-- 
|: 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 :|