Re: [PATCH 1/1] qemu_tpm: Get swtpm pid without binary validation

[Vasily Ulyanov]

Hi Michal,

On 11/01/2022 11:00, Michal Prívozník wrote:
> 
> Firstly, this fixes the problem only for swtpm case, but there is one
> other place where the same problem can happen (qemuVhostUserGPUGetPid()).
> 

Sure, I can also try to fix the vhost gpu case. Was mostly focused on tpm hence
did not look at other places.

> From conceptual POV, what may now happen is that when PID wrapped we may
> wrongly detect another process as swtpm.
> 

Right, valid point...

> I'm wondering whether we can ditch the @binPath argument of
> virPidFileReadPathIfAlive() completely. A pid file should be locked by
> the process owning it [1], therefore what we could do is read given file
> and try to lock it. If we succeeded it means that the process is dead
> and we read a stale PID. If locking fails then the process is still
> running and the PID we read is valid.
> 
> 1: This is how libvirt treats pidfiles (see virPidFileAcquirePath()).
> Question is whether other tools which use their own pidfiles do the
> same. For instance, swtpm is given '--pid file=/some/path' argument but
> I haven't checked whether it behaves as expected. Also, to make things
> worse there are at least two types of file locks on Linux and both are
> independent of each other, so question then is what type of lock does
> given tool use.
> 

Unfortunately swtpm does not lock the pid file at all :(

> But this could all be mitigated by using virCommandSetPidFile() when
> spawning the command instead of passing arbitrary --pid arguments.
> 

So I can drop --pid and --daemon args from swtpm cmd and add

virCommandSetPidFile(cmd, pidfile);
virCommandDaemonize(cmd);

Then in virPidFileReadPathIfAlive() I can add a lock check. Hm... Nice, that
seems like a good way to go.


-- 
Vasily Ulyanov 
Software Engineer, SUSE Labs Core

[libvirt PATCH v4 6/7] ch_process: Setup emulator and iothread settings

[Praveen K Paladugu]

using virCHProcessSetupPid

Signed-off-by: Praveen K Paladugu 
---
 src/ch/ch_monitor.c | 60 +++
 src/ch/ch_monitor.h |  2 ++
 src/ch/ch_process.c | 77 -
 3 files changed, 138 insertions(+), 1 deletion(-)

diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index 6c1d889a82..2a7cffb696 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -930,3 +930,63 @@ virCHMonitorGetInfo(virCHMonitor *mon, virJSONValue **info)
 {
 return virCHMonitorGet(mon, URL_VM_INFO, info);
 }
+
+/**
+ * virCHMonitorGetIOThreads:
+ * @mon: Pointer to the monitor
+ * @iothreads: Location to return array of IOThreadInfo data
+ *
+ * Retrieve the list of iothreads defined/running for the machine
+ *
+ * Returns count of IOThreadInfo structures on success
+ *-1 on error.
+ */
+int virCHMonitorGetIOThreads(virCHMonitor *mon,
+ virDomainIOThreadInfo ***iothreads)
+{
+size_t nthreads = 0, niothreads = 0;
+int thd_index;
+virDomainIOThreadInfo **iothreadinfolist = NULL, *iothreadinfo = NULL;
+
+*iothreads = NULL;
+nthreads = virCHMonitorRefreshThreadInfo(mon);
+
+iothreadinfolist = g_new0(virDomainIOThreadInfo*, nthreads);
+
+for (thd_index = 0; thd_index < nthreads; thd_index++) {
+virBitmap *map = NULL;
+if (mon->threads[thd_index].type == virCHThreadTypeIO) {
+iothreadinfo = g_new0(virDomainIOThreadInfo, 1);
+
+iothreadinfo->iothread_id = mon->threads[thd_index].ioInfo.tid;
+
+if (!(map = virProcessGetAffinity(iothreadinfo->iothread_id)))
+goto cleanup;
+
+if (virBitmapToData(map, &(iothreadinfo->cpumap),
+&(iothreadinfo->cpumaplen)) < 0) {
+virBitmapFree(map);
+goto cleanup;
+}
+virBitmapFree(map);
+/* Append to iothreadinfolist */
+iothreadinfolist[niothreads] = iothreadinfo;
+niothreads++;
+}
+}
+VIR_DELETE_ELEMENT_INPLACE(iothreadinfolist,
+   niothreads, nthreads);
+*iothreads = iothreadinfolist;
+VIR_DEBUG("niothreads = %ld", niothreads);
+return niothreads;
+
+cleanup:
+if (iothreadinfolist) {
+for (thd_index = 0; thd_index < niothreads; thd_index++)
+VIR_FREE(iothreadinfolist[thd_index]);
+VIR_FREE(iothreadinfolist);
+}
+if (iothreadinfo)
+VIR_FREE(iothreadinfo);
+return -1;
+}
diff --git a/src/ch/ch_monitor.h b/src/ch/ch_monitor.h
index 6646316454..ffc80e8910 100644
--- a/src/ch/ch_monitor.h
+++ b/src/ch/ch_monitor.h
@@ -119,3 +119,5 @@ int virCHMonitorGetCPUInfo(virCHMonitor *mon,
size_t maxvcpus);
 size_t virCHMonitorGetThreadInfo(virCHMonitor *mon, bool refresh,
  virCHMonitorThreadInfo **threads);
+int virCHMonitorGetIOThreads(virCHMonitor *mon,
+ virDomainIOThreadInfo ***iothreads);
diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c
index 1a0730a4d1..2b81c9f757 100644
--- a/src/ch/ch_process.c
+++ b/src/ch/ch_process.c
@@ -41,7 +41,6 @@ VIR_LOG_INIT("ch.ch_process");
 #define START_VM_POSTFIX ": starting up vm\n"
 
 
-
 static virCHMonitor *
 virCHProcessConnectMonitor(virCHDriver *driver,
virDomainObj *vm)
@@ -310,6 +309,74 @@ virCHProcessSetupPid(virDomainObj *vm,
 return ret;
 }
 
+static int
+virCHProcessSetupIOThread(virDomainObj *vm,
+  virDomainIOThreadInfo *iothread)
+{
+virCHDomainObjPrivate *priv = vm->privateData;
+return virCHProcessSetupPid(vm, iothread->iothread_id,
+VIR_CGROUP_THREAD_IOTHREAD,
+iothread->iothread_id,
+priv->autoCpuset, /* This should be updated 
when CLH supports accepting
+  iothread settings from input domain 
definition */
+vm->def->cputune.iothread_period,
+vm->def->cputune.iothread_quota,
+NULL); /* CLH doesn't allow choosing a 
scheduler for iothreads.*/
+}
+
+static int
+virCHProcessSetupIOThreads(virDomainObj *vm)
+{
+virCHDomainObjPrivate *priv = vm->privateData;
+virDomainIOThreadInfo **iothreads = NULL;
+size_t i;
+size_t  niothreads;
+
+niothreads = virCHMonitorGetIOThreads(priv->monitor, );
+for (i = 0; i < niothreads; i++) {
+VIR_DEBUG("IOThread index = %ld , tid = %d", i, 
iothreads[i]->iothread_id);
+if (virCHProcessSetupIOThread(vm, iothreads[i]) < 0)
+return -1;
+}
+return 0;
+}
+
+
+static int
+virCHProcessSetupEmulatorThread(virDomainObj *vm,
+ virCHMonitorEmuThreadInfo emuthread)
+{
+return 

[libvirt PATCH v4 4/7] ch_driver: enable typed param string for numatune

[Praveen K Paladugu]

From: Vineeth Pillai 

Enable support of VIR_DRV_FEATURE_TYPED_PARAM_STRING to enable numatune

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen K Paladugu 
---
 src/ch/ch_driver.c | 31 +++
 1 file changed, 31 insertions(+)

diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 110c86168a..408aae2cd6 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -943,6 +943,36 @@ static int chStateInitialize(bool privileged,
 return ret;
 }
 
+/* Which features are supported by this driver? */
+static int
+chConnectSupportsFeature(virConnectPtr conn, int feature)
+{
+if (virConnectSupportsFeatureEnsureACL(conn) < 0)
+return -1;
+
+switch ((virDrvFeature) feature) {
+case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
+return 1;
+case VIR_DRV_FEATURE_MIGRATION_V2:
+case VIR_DRV_FEATURE_MIGRATION_V3:
+case VIR_DRV_FEATURE_MIGRATION_P2P:
+case VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION:
+case VIR_DRV_FEATURE_FD_PASSING:
+case VIR_DRV_FEATURE_XML_MIGRATABLE:
+case VIR_DRV_FEATURE_MIGRATION_OFFLINE:
+case VIR_DRV_FEATURE_MIGRATION_PARAMS:
+case VIR_DRV_FEATURE_MIGRATION_DIRECT:
+case VIR_DRV_FEATURE_MIGRATION_V1:
+case VIR_DRV_FEATURE_PROGRAM_KEEPALIVE:
+case VIR_DRV_FEATURE_REMOTE:
+case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK:
+case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK:
+case VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER:
+default:
+return 0;
+}
+}
+
 static int
 chDomainGetVcpusFlags(virDomainPtr dom,
   unsigned int flags)
@@ -1285,6 +1315,7 @@ static virHypervisorDriver chHypervisorDriver = {
 .connectListAllDomains = chConnectListAllDomains,   /* 7.5.0 */
 .connectListDomains = chConnectListDomains, /* 7.5.0 */
 .connectGetCapabilities = chConnectGetCapabilities, /* 7.5.0 */
+.connectSupportsFeature = chConnectSupportsFeature, /* 8.0.0 */
 .domainCreateXML = chDomainCreateXML,   /* 7.5.0 */
 .domainCreate = chDomainCreate, /* 7.5.0 */
 .domainCreateWithFlags = chDomainCreateWithFlags,   /* 7.5.0 */
-- 
2.27.0

[libvirt PATCH v4 1/7] qemu, hypervisor: refactor some cgroup mgmt methods

[Praveen K Paladugu]

Refactor some cgroup management methods from qemu into hypervisor.
These methods will be shared with ch driver for cgroup management.

Signed-off-by: Praveen K Paladugu 
---
 src/hypervisor/domain_cgroup.c | 457 -
 src/hypervisor/domain_cgroup.h |  72 ++
 src/libvirt_private.syms   |  14 +-
 src/qemu/qemu_cgroup.c | 413 +
 src/qemu/qemu_cgroup.h |  11 -
 src/qemu/qemu_driver.c |  14 +-
 src/qemu/qemu_hotplug.c|   7 +-
 src/qemu/qemu_process.c|  24 +-
 8 files changed, 580 insertions(+), 432 deletions(-)

diff --git a/src/hypervisor/domain_cgroup.c b/src/hypervisor/domain_cgroup.c
index 61b54f071c..05c53ff7d1 100644
--- a/src/hypervisor/domain_cgroup.c
+++ b/src/hypervisor/domain_cgroup.c
@@ -22,11 +22,12 @@
 
 #include "domain_cgroup.h"
 #include "domain_driver.h"
-
+#include "util/virnuma.h"
+#include "virlog.h"
 #include "virutil.h"
 
 #define VIR_FROM_THIS VIR_FROM_DOMAIN
-
+VIR_LOG_INIT("domain.cgroup");
 
 int
 virDomainCgroupSetupBlkio(virCgroup *cgroup, virDomainBlkiotune blkio)
@@ -269,3 +270,455 @@ virDomainCgroupSetMemoryLimitParameters(virCgroup *cgroup,
 
 return 0;
 }
+
+
+int
+virDomainCgroupSetupBlkioCgroup(virDomainObj *vm,
+virCgroup *cgroup)
+{
+
+if (!virCgroupHasController(cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) {
+if (vm->def->blkio.weight || vm->def->blkio.ndevices) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("Block I/O tuning is not available on this 
host"));
+return -1;
+} else {
+return 0;
+}
+}
+
+return virDomainCgroupSetupBlkio(cgroup, vm->def->blkio);
+}
+
+
+int
+virDomainCgroupSetupMemoryCgroup(virDomainObj *vm,
+ virCgroup *cgroup)
+{
+
+if (!virCgroupHasController(cgroup, VIR_CGROUP_CONTROLLER_MEMORY)) {
+if (virMemoryLimitIsSet(vm->def->mem.hard_limit) ||
+virMemoryLimitIsSet(vm->def->mem.soft_limit) ||
+virMemoryLimitIsSet(vm->def->mem.swap_hard_limit)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("Memory cgroup is not available on this host"));
+return -1;
+} else {
+return 0;
+}
+}
+
+return virDomainCgroupSetupMemtune(cgroup, vm->def->mem);
+}
+
+
+int
+virDomainCgroupSetupCpusetCgroup(virCgroup *cgroup)
+{
+
+if (!virCgroupHasController(cgroup, VIR_CGROUP_CONTROLLER_CPUSET))
+return 0;
+
+if (virCgroupSetCpusetMemoryMigrate(cgroup, true) < 0)
+return -1;
+
+return 0;
+}
+
+
+int
+virDomainCgroupSetupCpuCgroup(virDomainObj *vm,
+  virCgroup *cgroup)
+{
+
+if (!virCgroupHasController(cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
+if (vm->def->cputune.sharesSpecified) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("CPU tuning is not available on this host"));
+return -1;
+} else {
+return 0;
+}
+}
+
+if (vm->def->cputune.sharesSpecified) {
+
+if (virCgroupSetCpuShares(cgroup, vm->def->cputune.shares) < 0)
+return -1;
+
+}
+
+return 0;
+}
+
+
+int
+virDomainCgroupInitCgroup(const char *prefix,
+  virDomainObj * vm,
+  size_t nnicindexes,
+  int *nicindexes,
+  virCgroup * cgroup,
+  int cgroupControllers,
+  unsigned int maxThreadsPerProc,
+  bool privileged,
+  char *machineName)
+{
+if (!privileged)
+return 0;
+
+if (!virCgroupAvailable())
+return 0;
+
+virCgroupFree(cgroup);
+cgroup = NULL;
+
+if (!vm->def->resource)
+vm->def->resource = g_new0(virDomainResourceDef, 1);
+
+if (!vm->def->resource->partition)
+vm->def->resource->partition = g_strdup("/machine");
+
+if (!g_path_is_absolute(vm->def->resource->partition)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Resource partition '%s' must start with '/'"),
+   vm->def->resource->partition);
+return -1;
+}
+
+if (virCgroupNewMachine(machineName,
+prefix,
+vm->def->uuid,
+NULL,
+vm->pid,
+false,
+nnicindexes, nicindexes,
+vm->def->resource->partition,
+cgroupControllers,
+maxThreadsPerProc, ) < 0) {
+if (virCgroupNewIgnoreError())
+return 0;
+
+return -1;
+}
+
+return 0;
+}
+
+
+void

[libvirt PATCH v4 5/7] ch_driver: add numatune callbacks for CH driver

[Praveen K Paladugu]

From: Vineeth Pillai 

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen K Paladugu 
---
 src/ch/ch_driver.c | 260 +
 1 file changed, 260 insertions(+)

diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 408aae2cd6..e3c0ae2952 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -43,6 +43,7 @@
 #include "viruri.h"
 #include "virutil.h"
 #include "viruuid.h"
+#include "virnuma.h"
 
 #define VIR_FROM_THIS VIR_FROM_CH
 
@@ -1302,6 +1303,263 @@ chDomainPinVcpu(virDomainPtr dom,
   VIR_DOMAIN_AFFECT_LIVE);
 }
 
+#define CH_NB_NUMA_PARAM 2
+
+static int
+chDomainGetNumaParameters(virDomainPtr dom,
+  virTypedParameterPtr params,
+  int *nparams,
+  unsigned int flags)
+{
+size_t i;
+virDomainObj *vm = NULL;
+virDomainNumatuneMemMode tmpmode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
+virCHDomainObjPrivate *priv;
+g_autofree char *nodeset = NULL;
+int ret = -1;
+virDomainDef *def = NULL;
+bool live = false;
+virBitmap *autoNodeset = NULL;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG |
+  VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+if (!(vm = virCHDomainObjFromDomain(dom)))
+return -1;
+priv = vm->privateData;
+
+if (virDomainGetNumaParametersEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (!(def = virDomainObjGetOneDefState(vm, flags, )))
+goto cleanup;
+
+if (live)
+autoNodeset = priv->autoNodeset;
+
+if ((*nparams) == 0) {
+*nparams = CH_NB_NUMA_PARAM;
+ret = 0;
+goto cleanup;
+}
+
+for (i = 0; i < CH_NB_NUMA_PARAM && i < *nparams; i++) {
+virMemoryParameterPtr param = [i];
+
+switch (i) {
+case 0: /* fill numa mode here */
+ignore_value(virDomainNumatuneGetMode(def->numa, -1, ));
+
+if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_MODE,
+VIR_TYPED_PARAM_INT, tmpmode) < 0)
+goto cleanup;
+
+break;
+
+case 1: /* fill numa nodeset here */
+nodeset = virDomainNumatuneFormatNodeset(def->numa, autoNodeset, 
-1);
+
+if (!nodeset ||
+virTypedParameterAssign(param, VIR_DOMAIN_NUMA_NODESET,
+VIR_TYPED_PARAM_STRING, nodeset) < 0)
+goto cleanup;
+
+nodeset = NULL;
+break;
+
+/* coverity[dead_error_begin] */
+default:
+break;
+/* should not hit here */
+}
+}
+
+if (*nparams > CH_NB_NUMA_PARAM)
+*nparams = CH_NB_NUMA_PARAM;
+ret = 0;
+
+ cleanup:
+virDomainObjEndAPI();
+return ret;
+}
+
+static int
+chDomainSetNumaParamsLive(virDomainObj *vm,
+  virBitmap *nodeset)
+{
+g_autoptr(virCgroup) cgroup_temp = NULL;
+virCHDomainObjPrivate *priv = vm->privateData;
+g_autofree char *nodeset_str = NULL;
+virDomainNumatuneMemMode mode;
+size_t i = 0;
+
+
+if (virDomainNumatuneGetMode(vm->def->numa, -1, ) == 0 &&
+mode != VIR_DOMAIN_NUMATUNE_MEM_STRICT) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("change of nodeset for running domain requires strict 
numa mode"));
+return -1;
+}
+
+if (!virNumaNodesetIsAvailable(nodeset))
+return -1;
+
+/* Ensure the cpuset string is formatted before passing to cgroup */
+if (!(nodeset_str = virBitmapFormat(nodeset)))
+return -1;
+
+if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0,
+   false, _temp) < 0 ||
+virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0)
+return -1;
+
+
+for (i = 0; i < virDomainDefGetVcpusMax(vm->def); i++) {
+virDomainVcpuDef *vcpu = virDomainDefGetVcpu(vm->def, i);
+
+if (!vcpu->online)
+continue;
+
+if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, i,
+   false, _temp) < 0 ||
+virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0)
+return -1;
+
+return 0;
+}
+
+for (i = 0; i < vm->def->niothreadids; i++) {
+if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD,
+   vm->def->iothreadids[i]->iothread_id,
+   false, _temp) < 0 ||
+virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0)
+return -1;
+}
+
+/* set nodeset for root cgroup */
+if (virCgroupSetCpusetMems(priv->cgroup, nodeset_str) < 0)
+return -1;
+
+return 0;
+}
+
+static int
+chDomainSetNumaParameters(virDomainPtr dom,
+  virTypedParameterPtr params,
+  int nparams,
+ 

[libvirt PATCH v4 7/7] ch_driver: emulator threadinfo & pinning callbacks

[Praveen K Paladugu]

Signed-off-by: Praveen K Paladugu 
---
 src/ch/ch_driver.c | 154 +
 1 file changed, 154 insertions(+)

diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index e3c0ae2952..bf5bb3696c 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -1303,6 +1303,158 @@ chDomainPinVcpu(virDomainPtr dom,
   VIR_DOMAIN_AFFECT_LIVE);
 }
 
+
+
+static int
+chDomainGetEmulatorPinInfo(virDomainPtr dom,
+   unsigned char *cpumaps,
+   int maplen,
+   unsigned int flags)
+{
+virDomainObj *vm = NULL;
+virDomainDef *def;
+virCHDomainObjPrivate *priv;
+bool live;
+int ret = -1;
+virBitmap *cpumask = NULL;
+g_autoptr(virBitmap) bitmap = NULL;
+virBitmap *autoCpuset = NULL;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+if (!(vm = chDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainGetEmulatorPinInfoEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (!(def = virDomainObjGetOneDefState(vm, flags, )))
+goto cleanup;
+
+if (live) {
+priv = vm->privateData;
+autoCpuset = priv->autoCpuset;
+}
+if (def->cputune.emulatorpin) {
+cpumask = def->cputune.emulatorpin;
+} else if (def->cpumask) {
+cpumask = def->cpumask;
+} else if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO &&
+   autoCpuset) {
+cpumask = autoCpuset;
+} else {
+if (!(bitmap = virHostCPUGetAvailableCPUsBitmap()))
+goto cleanup;
+cpumask = bitmap;
+}
+
+virBitmapToDataBuf(cpumask, cpumaps, maplen);
+
+ret = 1;
+
+ cleanup:
+virDomainObjEndAPI();
+return ret;
+}
+
+static int
+chDomainPinEmulator(virDomainPtr dom,
+unsigned char *cpumap,
+int maplen,
+unsigned int flags)
+{
+virCHDriver *driver = dom->conn->privateData;
+virDomainObj *vm;
+virCgroup *cgroup_emulator = NULL;
+virDomainDef *def;
+virDomainDef *persistentDef;
+int ret = -1;
+virCHDomainObjPrivate *priv;
+virBitmap *pcpumap = NULL;
+g_autoptr(virCHDriverConfig) cfg = NULL;
+g_autofree char *str = NULL;
+virTypedParameterPtr eventParams = NULL;
+int eventNparams = 0;
+int eventMaxparams = 0;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+cfg = virCHDriverGetConfig(driver);
+
+if (!(vm = chDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainPinEmulatorEnsureACL(dom->conn, vm->def, flags) < 0)
+goto cleanup;
+
+if (virCHDomainObjBeginJob(vm, CH_JOB_MODIFY) < 0)
+goto cleanup;
+
+if (virDomainObjGetDefs(vm, flags, , ) < 0)
+goto endjob;
+
+priv = vm->privateData;
+
+if (!(pcpumap = virBitmapNewData(cpumap, maplen)))
+goto endjob;
+
+if (virBitmapIsAllClear(pcpumap)) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("Empty cpu list for pinning"));
+goto endjob;
+}
+
+if (def) {
+if (virCgroupHasController(priv->cgroup, 
VIR_CGROUP_CONTROLLER_CPUSET)) {
+if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR,
+   0, false, _emulator) < 0)
+goto endjob;
+
+if (virDomainCgroupSetupCpusetCpus(cgroup_emulator, pcpumap) < 0) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("failed to set cpuset.cpus in cgroup"
+ " for emulator threads"));
+goto endjob;
+}
+}
+
+if (virProcessSetAffinity(vm->pid, pcpumap, false) < 0)
+goto endjob;
+
+virBitmapFree(def->cputune.emulatorpin);
+def->cputune.emulatorpin = NULL;
+
+if (!(def->cputune.emulatorpin = virBitmapNewCopy(pcpumap)))
+goto endjob;
+
+if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0)
+goto endjob;
+
+str = virBitmapFormat(pcpumap);
+if (virTypedParamsAddString(, ,
+,
+VIR_DOMAIN_TUNABLE_CPU_EMULATORPIN,
+str) < 0)
+goto endjob;
+
+}
+
+
+ret = 0;
+
+endjob:
+virCHDomainObjEndJob(vm);
+
+cleanup:
+if (cgroup_emulator)
+virCgroupFree(cgroup_emulator);
+virBitmapFree(pcpumap);
+virDomainObjEndAPI();
+return ret;
+}
+
 #define CH_NB_NUMA_PARAM 2
 
 static int
@@ -1603,6 +1755,8 @@ static virHypervisorDriver chHypervisorDriver = {
 .domainGetVcpuPinInfo = chDomainGetVcpuPinInfo, /* 8.0.0 */
 .domainPinVcpu = chDomainPinVcpu,   /* 8.0.0 */
 

[libvirt PATCH v4 3/7] ch_driver, ch_domain: vcpupin callback in ch driver

[Praveen K Paladugu]

From: Vineeth Pillai 

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen K Paladugu 
---
 src/ch/ch_domain.c  |  30 +
 src/ch/ch_domain.h  |   7 ++-
 src/ch/ch_driver.c  | 145 
 src/ch/ch_monitor.c |   2 +-
 4 files changed, 181 insertions(+), 3 deletions(-)

diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
index 6f0cec8c6e..6fde7122f7 100644
--- a/src/ch/ch_domain.c
+++ b/src/ch/ch_domain.c
@@ -20,6 +20,7 @@
 
 #include 
 
+#include "datatypes.h"
 #include "ch_domain.h"
 #include "domain_driver.h"
 #include "viralloc.h"
@@ -416,3 +417,32 @@ virCHDomainGetMachineName(virDomainObj *vm)
 
 return ret;
 }
+
+/**
+ * virCHDomainObjFromDomain:
+ * @domain: Domain pointer that has to be looked up
+ *
+ * This function looks up @domain and returns the appropriate virDomainObjPtr
+ * that has to be released by calling virDomainObjEndAPI().
+ *
+ * Returns the domain object with incremented reference counter which is locked
+ * on success, NULL otherwise.
+ */
+virDomainObj *
+virCHDomainObjFromDomain(virDomainPtr domain)
+{
+virDomainObj *vm;
+virCHDriver *driver = domain->conn->privateData;
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+vm = virDomainObjListFindByUUID(driver->domains, domain->uuid);
+if (!vm) {
+virUUIDFormat(domain->uuid, uuidstr);
+virReportError(VIR_ERR_NO_DOMAIN,
+   _("no domain with matching uuid '%s' (%s)"),
+   uuidstr, domain->name);
+return NULL;
+}
+
+return vm;
+}
diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h
index cb94905b94..60a761ac84 100644
--- a/src/ch/ch_domain.h
+++ b/src/ch/ch_domain.h
@@ -97,5 +97,8 @@ virCHDomainGetVcpuPid(virDomainObj *vm,
 bool
 virCHDomainHasVcpuPids(virDomainObj *vm);
 
-char *
-virCHDomainGetMachineName(virDomainObj *vm);
+char
+*virCHDomainGetMachineName(virDomainObj *vm);
+
+virDomainObj
+*virCHDomainObjFromDomain(virDomain *domain);
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 53e0872207..110c86168a 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -25,6 +25,7 @@
 #include "ch_driver.h"
 #include "ch_monitor.h"
 #include "ch_process.h"
+#include "domain_cgroup.h"
 #include "datatypes.h"
 #include "driver.h"
 #include "viraccessapicheck.h"
@@ -1129,6 +1130,148 @@ chDomainGetVcpus(virDomainPtr dom,
 return ret;
 }
 
+static int
+chDomainPinVcpuLive(virDomainObj *vm,
+virDomainDef *def,
+int vcpu,
+virCHDriver *driver,
+virCHDriverConfig *cfg,
+virBitmap *cpumap)
+{
+g_autoptr(virBitmap) tmpmap = NULL;
+g_autoptr(virCgroup) cgroup_vcpu = NULL;
+virDomainVcpuDef *vcpuinfo;
+virCHDomainObjPrivate *priv = vm->privateData;
+
+g_autofree char *str = NULL;
+
+if (!virCHDomainHasVcpuPids(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   "%s", _("cpu affinity is not supported"));
+return -1;
+}
+
+if (!(vcpuinfo = virDomainDefGetVcpu(def, vcpu))) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _("vcpu %d is out of range of live cpu count %d"),
+   vcpu, virDomainDefGetVcpusMax(def));
+return -1;
+}
+
+if (!(tmpmap = virBitmapNewCopy(cpumap)))
+return -1;
+
+if (!(str = virBitmapFormat(cpumap)))
+return -1;
+
+if (vcpuinfo->online) {
+/* Configure the corresponding cpuset cgroup before set affinity. */
+if (virCgroupHasController(priv->cgroup, 
VIR_CGROUP_CONTROLLER_CPUSET)) {
+if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, vcpu,
+   false, _vcpu) < 0)
+return -1;
+if (virDomainCgroupSetupCpusetCpus(cgroup_vcpu, cpumap) < 0)
+return -1;
+}
+
+if (virProcessSetAffinity(virCHDomainGetVcpuPid(vm, vcpu), cpumap, 
false) < 0)
+return -1;
+}
+
+virBitmapFree(vcpuinfo->cpumask);
+vcpuinfo->cpumask = tmpmap;
+tmpmap = NULL;
+
+if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0)
+return -1;
+
+return 0;
+}
+
+
+static int
+chDomainPinVcpuFlags(virDomainPtr dom,
+ unsigned int vcpu,
+ unsigned char *cpumap,
+ int maplen,
+ unsigned int flags)
+{
+virCHDriver *driver = dom->conn->privateData;
+virDomainObj *vm;
+virDomainDef *def;
+virDomainDef *persistentDef;
+int ret = -1;
+virBitmap *pcpumap = NULL;
+virDomainVcpuDef *vcpuinfo = NULL;
+g_autoptr(virCHDriverConfig) cfg = NULL;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+cfg = virCHDriverGetConfig(driver);
+
+if (!(vm = virCHDomainObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainPinVcpuFlagsEnsureACL(dom->conn, 

[libvirt PATCH v4 2/7] ch: methods for cgroup mgmt in ch driver

[Praveen K Paladugu]

From: Vineeth Pillai 

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen K Paladugu 
---
 src/ch/ch_conf.c|   2 +
 src/ch/ch_conf.h|   4 +-
 src/ch/ch_domain.c  |  34 +
 src/ch/ch_domain.h  |  11 +-
 src/ch/ch_monitor.c |  96 ++
 src/ch/ch_monitor.h |  54 +++-
 src/ch/ch_process.c | 308 ++--
 src/ch/ch_process.h |   3 +
 8 files changed, 492 insertions(+), 20 deletions(-)

diff --git a/src/ch/ch_conf.c b/src/ch/ch_conf.c
index 98f1e89003..be12934dcd 100644
--- a/src/ch/ch_conf.c
+++ b/src/ch/ch_conf.c
@@ -125,6 +125,8 @@ virCHDriverConfigNew(bool privileged)
 if (!(cfg = virObjectNew(virCHDriverConfigClass)))
 return NULL;
 
+cfg->cgroupControllers = -1; /* Auto detect */
+
 if (privileged) {
 if (virGetUserID(CH_USER, >user) < 0)
 return NULL;
diff --git a/src/ch/ch_conf.h b/src/ch/ch_conf.h
index 8fe69c8545..1790295ede 100644
--- a/src/ch/ch_conf.h
+++ b/src/ch/ch_conf.h
@@ -35,11 +35,13 @@ struct _virCHDriverConfig {
 
 char *stateDir;
 char *logDir;
-
+int cgroupControllers;
 uid_t user;
 gid_t group;
 };
 
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCHDriverConfig, virObjectUnref);
+
 struct _virCHDriver
 {
 virMutex lock;
diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
index a746d0f5fd..6f0cec8c6e 100644
--- a/src/ch/ch_domain.c
+++ b/src/ch/ch_domain.c
@@ -319,6 +319,40 @@ chValidateDomainDeviceDef(const virDomainDeviceDef *dev,
_("Serial can only be enabled for a PTY"));
 return -1;
 }
+return 0;
+}
+
+int
+virCHDomainRefreshThreadInfo(virDomainObj *vm)
+{
+size_t maxvcpus = virDomainDefGetVcpusMax(vm->def);
+virCHMonitorThreadInfo *info = NULL;
+size_t nthreads, ncpus = 0;
+size_t i;
+
+nthreads = virCHMonitorGetThreadInfo(virCHDomainGetMonitor(vm),
+ true, );
+
+for (i = 0; i < nthreads; i++) {
+virCHDomainVcpuPrivate *vcpupriv;
+virDomainVcpuDef *vcpu;
+virCHMonitorCPUInfo *vcpuInfo;
+
+if (info[i].type != virCHThreadTypeVcpu)
+continue;
+
+/* TODO: hotplug support */
+vcpuInfo = [i].vcpuInfo;
+vcpu = virDomainDefGetVcpu(vm->def, vcpuInfo->cpuid);
+vcpupriv = CH_DOMAIN_VCPU_PRIVATE(vcpu);
+vcpupriv->tid = vcpuInfo->tid;
+ncpus++;
+}
+
+/* TODO: Remove the warning when hotplug is implemented.*/
+if (ncpus != maxvcpus)
+VIR_WARN("Mismatch in the number of cpus, expected: %ld, actual: %ld",
+ maxvcpus, ncpus);
 
 return 0;
 }
diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h
index 4d0b5479b8..cb94905b94 100644
--- a/src/ch/ch_domain.h
+++ b/src/ch/ch_domain.h
@@ -53,11 +53,13 @@ typedef struct _virCHDomainObjPrivate virCHDomainObjPrivate;
 struct _virCHDomainObjPrivate {
 struct virCHDomainJobObj job;
 
-virChrdevs *chrdevs;
-virCHDriver *driver;
-virCHMonitor *monitor;
 char *machineName;
 virBitmap *autoCpuset;
+virBitmap *autoNodeset;
+virCHDriver *driver;
+virCHMonitor *monitor;
+virCgroup *cgroup;
+virChrdevs *chrdevs;
 };
 
 #define CH_DOMAIN_PRIVATE(vm) \
@@ -87,7 +89,8 @@ void
 virCHDomainObjEndJob(virDomainObj *obj);
 
 int
-virCHDomainRefreshVcpuInfo(virDomainObj *vm);
+virCHDomainRefreshThreadInfo(virDomainObj *vm);
+
 pid_t
 virCHDomainGetVcpuPid(virDomainObj *vm,
   unsigned int vcpuid);
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index a19f0c7e33..d984bf9822 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -41,6 +41,7 @@ VIR_LOG_INIT("ch.ch_monitor");
 
 static virClass *virCHMonitorClass;
 static void virCHMonitorDispose(void *obj);
+static void virCHMonitorThreadInfoFree(virCHMonitor * mon);
 
 static int virCHMonitorOnceInit(void)
 {
@@ -578,6 +579,7 @@ static void virCHMonitorDispose(void *opaque)
 virCHMonitor *mon = opaque;
 
 VIR_DEBUG("mon=%p", mon);
+virCHMonitorThreadInfoFree(mon);
 virObjectUnref(mon->vm);
 }
 
@@ -743,6 +745,100 @@ virCHMonitorGet(virCHMonitor *mon, const char *endpoint, 
virJSONValue **response
 return ret;
 }
 
+static void
+virCHMonitorThreadInfoFree(virCHMonitor *mon)
+{
+mon->nthreads = 0;
+if (mon->threads)
+VIR_FREE(mon->threads);
+}
+
+static size_t
+virCHMonitorRefreshThreadInfo(virCHMonitor *mon)
+{
+virCHMonitorThreadInfo *info = NULL;
+g_autofree pid_t *tids = NULL;
+virDomainObj *vm = mon->vm;
+size_t ntids = 0;
+size_t i;
+
+
+virCHMonitorThreadInfoFree(mon);
+if (virProcessGetPids(vm->pid, , ) < 0) {
+mon->threads = NULL;
+return 0;
+}
+
+info = g_new0(virCHMonitorThreadInfo, ntids);
+for (i = 0; i < ntids; i++) {
+g_autofree char *proc = NULL;
+g_autofree char *data = NULL;
+
+proc = g_strdup_printf("/proc/%d/task/%d/comm",
+   (int)vm->pid, 

[libvirt PATCH v4 0/7] cgroup and thread management in ch driver.

[Praveen K Paladugu]

This patchset adds support for cgroup management of ch threads. This version
correctly manages cgroups for vcpu and emulator threads created by ch. cgroup
management for iothreads is not yet supported.

Along with cgroup management, this patchset also enables support for pinning
vcpu and emulator threads to selected host cpus.

v4:
* addressed all open comments in v3
* dropped all the merged commits

v3:
* addrressed all the formatting comments in v2 patch set
* dropped indentation patches are they do not adhere to libvirt coding style
* fixed build issue in qemu driver that was introduced in v2

Praveen K Paladugu (3):
  qemu,hypervisor: refactor some cgroup mgmt methods
  ch_process: Setup emulator and iothread settings
  ch_driver: emulator threadinfo & pinning callbacks

Vineeth Pillai (4):
  ch: methods for cgroup mgmt in ch driver
  ch_driver,ch_domain: vcpupin callback in ch driver
  ch_driver: enable typed param string for numatune
  ch_driver: add numatune callbacks for CH driver

 src/ch/ch_conf.c   |   2 +
 src/ch/ch_conf.h   |   4 +-
 src/ch/ch_domain.c |  64 
 src/ch/ch_domain.h |  18 +-
 src/ch/ch_driver.c | 590 +
 src/ch/ch_monitor.c| 156 +
 src/ch/ch_monitor.h|  56 +++-
 src/ch/ch_process.c| 385 -
 src/ch/ch_process.h|   3 +
 src/hypervisor/domain_cgroup.c | 457 -
 src/hypervisor/domain_cgroup.h |  72 
 src/libvirt_private.syms   |  14 +-
 src/qemu/qemu_cgroup.c | 413 +--
 src/qemu/qemu_cgroup.h |  11 -
 src/qemu/qemu_driver.c |  14 +-
 src/qemu/qemu_hotplug.c|   7 +-
 src/qemu/qemu_process.c|  24 +-
 17 files changed, 1835 insertions(+), 455 deletions(-)

-- 
2.27.0

Re: [PATCH v4] report error when virProcessGetStatInfo() is unable to parse data

[Ani Sinha]

I just realized after sending this patch that I made the change in the
Linux version of the function. It may not be applicable for FreeBSD .
In any case I simply don’t understand if errors on the logs is a problem.
If it is maybe we need a FreeBSD version of this function as well not just
a larger bucket on non Linux flavours that report error .

On Tue, Jan 11, 2022 at 23:43 Ani Sinha  wrote:

> Currently virProcessGetStatInfo() always returns success and only logs
> error
> when it is unable to parse the data. Make this function actually report the
> error and return a negative value in this error scenario.
>
> Fix the callers so that they do not override the error generated.
> Also fix non-linux implementation of this function so as to report error.
>
> Signed-off-by: Ani Sinha 
> ---
>  src/ch/ch_driver.c |  2 --
>  src/qemu/qemu_driver.c |  7 +--
>  src/util/virprocess.c  | 17 +
>  3 files changed, 14 insertions(+), 12 deletions(-)
>
> changelog:
> v4: on freebsd error on the logs is a problem appaently. try to fix
> that.
> v3: fix non linux implementation
> v2: fix callers
>
> diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
> index 53e0872207..3cbc668489 100644
> --- a/src/ch/ch_driver.c
> +++ b/src/ch/ch_driver.c
> @@ -1073,8 +1073,6 @@ chDomainHelperGetVcpus(virDomainObj *vm,
>  if (virProcessGetStatInfo(>cpuTime,
>>cpu, NULL,
>vm->pid, vcpupid) < 0) {
> -virReportSystemError(errno, "%s",
> -  _("cannot get vCPU placement & pCPU
> time"));
>  return -1;
>  }
>  }
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index d3d76c003f..65ac5ef367 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -1359,8 +1359,6 @@ qemuDomainHelperGetVcpus(virDomainObj *vm,
>  if (virProcessGetStatInfo(>cpuTime,
>>cpu, NULL,
>vm->pid, vcpupid) < 0) {
> -virReportSystemError(errno, "%s",
> - _("cannot get vCPU placement & pCPU
> time"));
>  return -1;
>  }
>  }
> @@ -2521,8 +2519,6 @@ qemuDomainGetInfo(virDomainPtr dom,
>  if (virDomainObjIsActive(vm)) {
>  if (virProcessGetStatInfo(&(info->cpuTime), NULL, NULL,
>vm->pid, 0) < 0) {
> -virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> -   _("cannot read cputime for domain"));
>  goto cleanup;
>  }
>  }
> @@ -10530,8 +10526,7 @@ qemuDomainMemoryStatsInternal(virQEMUDriver
> *driver,
>  }
>
>  if (virProcessGetStatInfo(NULL, NULL, , vm->pid, 0) < 0) {
> -virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> -   _("cannot get RSS for domain"));
> +virResetLastError();
>  } else {
>  stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_RSS;
>  stats[ret].val = rss;
> diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> index b559a4257e..cbb31441cc 100644
> --- a/src/util/virprocess.c
> +++ b/src/util/virprocess.c
> @@ -1779,12 +1779,20 @@ virProcessGetStatInfo(unsigned long long *cpuTime,
>  long rss = 0;
>  int cpu = 0;
>
> -if (!proc_stat ||
> -virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_UTIME], NULL, 10,
> ) < 0 ||
> +if (!proc_stat) {
> +VIR_WARN("Unable to get proc stats from the kernel");
> +return 0;
> +}
> +
> +if (virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_UTIME], NULL, 10,
> ) < 0 ||
>  virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_STIME], NULL, 10,
> ) < 0 ||
>  virStrToLong_l(proc_stat[VIR_PROCESS_STAT_RSS], NULL, 10, ) <
> 0 ||
>  virStrToLong_i(proc_stat[VIR_PROCESS_STAT_PROCESSOR], NULL, 10,
> ) < 0) {
> -VIR_WARN("cannot parse process status data");
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("cannot parse process status data for pid
> '%d/%d'"),
> +   (int) pid, (int) tid);
> +
> +return -1;
>  }
>
>  /* We got jiffies
> @@ -1881,7 +1889,8 @@ virProcessGetStatInfo(unsigned long long *cpuTime
> G_GNUC_UNUSED,
>pid_t pid G_GNUC_UNUSED,
>pid_t tid G_GNUC_UNUSED)
>  {
> -errno = ENOSYS;
> +virReportSystemError(ENOSYS, "%s",
> + _("Process statistics data is not supported on
> this platform"));
>  return -1;
>  }
>
> --
> 2.25.1
>
>

Re: [PATCH v3] report error when virProcessGetStatInfo() is unable to parse data

[Ani Sinha]

On Tue, 11 Jan 2022, Michal Prívozník wrote:

> On 1/11/22 11:20, Ani Sinha wrote:
> > Currently virProcessGetStatInfo() always returns success and only logs error
> > when it is unable to parse the data. Make this function actually report the
> > error and return a negative value in this error scenario.
> >
> > Fix the callers so that they do not override the error generated.
> > Also fix non-linux implementation of this function so as to report error.
> >
> > Signed-off-by: Ani Sinha 
> > ---
> >  src/ch/ch_driver.c | 2 --
> >  src/qemu/qemu_driver.c | 7 +--
> >  src/util/virprocess.c  | 9 +++--
> >  3 files changed, 8 insertions(+), 10 deletions(-)
> >
> > changelog:
> > v3: fix non-linux implementation
> > v2: fix callers
> >
>
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index d3d76c003f..9a17c93b08 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
>
> > @@ -10530,8 +10526,7 @@ qemuDomainMemoryStatsInternal(virQEMUDriver *driver,
> >  }
> >
> >  if (virProcessGetStatInfo(NULL, NULL, , vm->pid, 0) < 0) {
> > -virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> > -   _("cannot get RSS for domain"));
> > +return -1;
>
> Returning -1 changes semantics of this function. Previously it was
> tolerant to errors and now it isn't. For instance, if this function was
> called on a FreeBSD machine (yes, QEMU driver can be enabled there) then
> despite fetching mem stats earlier through monitor (not visible in the
> context) an error is now returned which in turn makes whole
> virDomainMemoryStats() API fail.
>
> The 'return -1' should be replaced with virResetLastError().
>
> Having said that, now there will be an error reported in the logs every
> time the API is called. I wonder what we can do about it. Previously it
> was "just" a warning.

Does v4 help?

[PATCH v4] report error when virProcessGetStatInfo() is unable to parse data

[Ani Sinha]

Currently virProcessGetStatInfo() always returns success and only logs error
when it is unable to parse the data. Make this function actually report the
error and return a negative value in this error scenario.

Fix the callers so that they do not override the error generated.
Also fix non-linux implementation of this function so as to report error.

Signed-off-by: Ani Sinha 
---
 src/ch/ch_driver.c |  2 --
 src/qemu/qemu_driver.c |  7 +--
 src/util/virprocess.c  | 17 +
 3 files changed, 14 insertions(+), 12 deletions(-)

changelog:
v4: on freebsd error on the logs is a problem appaently. try to fix
that.
v3: fix non linux implementation
v2: fix callers

diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 53e0872207..3cbc668489 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -1073,8 +1073,6 @@ chDomainHelperGetVcpus(virDomainObj *vm,
 if (virProcessGetStatInfo(>cpuTime,
   >cpu, NULL,
   vm->pid, vcpupid) < 0) {
-virReportSystemError(errno, "%s",
-  _("cannot get vCPU placement & pCPU 
time"));
 return -1;
 }
 }
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d3d76c003f..65ac5ef367 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1359,8 +1359,6 @@ qemuDomainHelperGetVcpus(virDomainObj *vm,
 if (virProcessGetStatInfo(>cpuTime,
   >cpu, NULL,
   vm->pid, vcpupid) < 0) {
-virReportSystemError(errno, "%s",
- _("cannot get vCPU placement & pCPU 
time"));
 return -1;
 }
 }
@@ -2521,8 +2519,6 @@ qemuDomainGetInfo(virDomainPtr dom,
 if (virDomainObjIsActive(vm)) {
 if (virProcessGetStatInfo(&(info->cpuTime), NULL, NULL,
   vm->pid, 0) < 0) {
-virReportError(VIR_ERR_OPERATION_FAILED, "%s",
-   _("cannot read cputime for domain"));
 goto cleanup;
 }
 }
@@ -10530,8 +10526,7 @@ qemuDomainMemoryStatsInternal(virQEMUDriver *driver,
 }
 
 if (virProcessGetStatInfo(NULL, NULL, , vm->pid, 0) < 0) {
-virReportError(VIR_ERR_OPERATION_FAILED, "%s",
-   _("cannot get RSS for domain"));
+virResetLastError();
 } else {
 stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_RSS;
 stats[ret].val = rss;
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index b559a4257e..cbb31441cc 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -1779,12 +1779,20 @@ virProcessGetStatInfo(unsigned long long *cpuTime,
 long rss = 0;
 int cpu = 0;
 
-if (!proc_stat ||
-virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_UTIME], NULL, 10, 
) < 0 ||
+if (!proc_stat) {
+VIR_WARN("Unable to get proc stats from the kernel");
+return 0;
+}
+
+if (virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_UTIME], NULL, 10, 
) < 0 ||
 virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_STIME], NULL, 10, 
) < 0 ||
 virStrToLong_l(proc_stat[VIR_PROCESS_STAT_RSS], NULL, 10, ) < 0 ||
 virStrToLong_i(proc_stat[VIR_PROCESS_STAT_PROCESSOR], NULL, 10, ) 
< 0) {
-VIR_WARN("cannot parse process status data");
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot parse process status data for pid '%d/%d'"),
+   (int) pid, (int) tid);
+
+return -1;
 }
 
 /* We got jiffies
@@ -1881,7 +1889,8 @@ virProcessGetStatInfo(unsigned long long *cpuTime 
G_GNUC_UNUSED,
   pid_t pid G_GNUC_UNUSED,
   pid_t tid G_GNUC_UNUSED)
 {
-errno = ENOSYS;
+virReportSystemError(ENOSYS, "%s",
+ _("Process statistics data is not supported on this 
platform"));
 return -1;
 }
 
-- 
2.25.1

[libvirt PATCH 2/3] docs: use virYesNo definition in more schemas

[Daniel P . Berrangé]

A few places are still using an expend yes/no choice instead of the
common virYesNo definition.

Signed-off-by: Daniel P. Berrangé 
---
 docs/schemas/capability.rng   |  5 +
 docs/schemas/domainbackup.rng |  5 +
 docs/schemas/domaincaps.rng   |  5 +
 docs/schemas/domaincommon.rng | 17 -
 4 files changed, 7 insertions(+), 25 deletions(-)

diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
index 7747a01558..c7e72c6f39 100644
--- a/docs/schemas/capability.rng
+++ b/docs/schemas/capability.rng
@@ -409,10 +409,7 @@
   
   
 
-  
-yes
-no
-  
+  
 
   
   
diff --git a/docs/schemas/domainbackup.rng b/docs/schemas/domainbackup.rng
index 05cc28ab00..1ac9da62c1 100644
--- a/docs/schemas/domainbackup.rng
+++ b/docs/schemas/domainbackup.rng
@@ -60,10 +60,7 @@
   
 
   
-
-  yes
-  no
-
+
   
 
 
diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
index b40ee0f35a..9cbc2467ab 100644
--- a/docs/schemas/domaincaps.rng
+++ b/docs/schemas/domaincaps.rng
@@ -156,10 +156,7 @@
   
   
 
-  
-yes
-no
-  
+  
 
   
   
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 7fa5c2b8b5..169b8d8dee 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -300,18 +300,12 @@
   
 
   
-
-  yes
-  no
-
+
   
 
 
   
-
-  yes
-  no
-
+
   
 
 
@@ -5331,11 +5325,8 @@
   
   
 
-  
-yes
-no
-  
-   
+  
+
   
 
   
-- 
2.33.1

[libvirt PATCH 1/3] docs: split example for schema

[Daniel P . Berrangé]

The docs illustration for the  schema contains a mixture of
incompatible configuration options. This is rather confusing and
misleading to users. Splitting the illustration into four separate
examples clarifies the situation.

Signed-off-by: Daniel P. Berrangé 
---
 docs/formatdomain.rst | 32 +---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index d4f30bb8af..3dee28d52a 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -110,12 +110,19 @@ harddisk, cdrom, network) determining where to 
obtain/find the boot image.
 
 ::
 
+   
...
-   
+   
  hvm
- /usr/lib/xen/boot/hvmloader
- /var/lib/libvirt/nvram/guest_VARS.fd
+ /usr/lib/xen/boot/hvmloader
  
+   
+   ...
+
+   
+   ...
+   
+ hvm
  
  
  
@@ -123,6 +130,25 @@ harddisk, cdrom, network) determining where to obtain/find 
the boot image.

...
 
+   
+   ...
+   
+ hvm
+ /usr/share/OVMF/OVMF_CODE.fd
+ /var/lib/libvirt/nvram/guest_VARS.fd
+ 
+   
+   ...
+
+   
+   ...
+   
+ hvm
+ 
+ 
+   
+   ...
+
 ``firmware``
The ``firmware`` attribute allows management applications to automatically
fill  and  elements and possibly enable some
-- 
2.33.1

[libvirt PATCH 0/3] Misc fixes / improvements

[Daniel P . Berrangé]

Just a few things I dealt with as side effect of other work
I'm doing wrt AMD SEV.

Daniel P. Berrangé (3):
  docs: split example for  schema
  docs: use virYesNo definition in more schemas
  qemu: split handling of distinct firmware enum conversions

 docs/formatdomain.rst | 32 +---
 docs/schemas/capability.rng   |  5 +
 docs/schemas/domainbackup.rng |  5 +
 docs/schemas/domaincaps.rng   |  5 +
 docs/schemas/domaincommon.rng | 17 -
 src/conf/domain_conf.h|  4 ++--
 src/qemu/qemu_firmware.c  | 21 +++--
 7 files changed, 57 insertions(+), 32 deletions(-)

-- 
2.33.1

[libvirt PATCH 3/3] qemu: split handling of distinct firmware enum conversions

[Daniel P . Berrangé]

The qemuFirmwareOSInterfaceTypeFromOsDefFirmware method
was added to convert from virDomainOsDefFirmware to the
qemuFirmwareOSInterface enum.

It was later also used to convert from virDomainLoader
to qemuFirmwareOSInterface in:

  commit 8e1804f9f66f13ca1412d22bf1a957b6d55a2365
  Author: Michal Prívozník 
  Date:   Tue Dec 17 17:45:50 2019 +0100

qemu_firmware: Try to autofill for old style UEFI specification

This caused compile errors with clang due to passing a
mis-matched enum type. These were later silenced by
stripping the enum types:

  commit 8fcee47807d29008632a7ad918cbe93ac0a20597
  Author: Michal Prívozník 
  Date:   Wed Jan 8 09:42:47 2020 +0100

qemu_firmware: Accept int in qemuFirmwareOSInterfaceTypeFromOsDefFirmware()

This is still rather confusing to humans reading the
code. It is clearer to just define a separate helper
method for the virDomainLoader type conversion.

Signed-off-by: Daniel P. Berrangé 
---
 src/conf/domain_conf.h   |  4 ++--
 src/qemu/qemu_firmware.c | 21 +++--
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 144ba4dd12..8a7e9a1668 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2292,8 +2292,8 @@ struct _virDomainOSEnv {
 
 typedef enum {
 VIR_DOMAIN_OS_DEF_FIRMWARE_NONE = 0,
-VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS = VIR_DOMAIN_LOADER_TYPE_ROM,
-VIR_DOMAIN_OS_DEF_FIRMWARE_EFI = VIR_DOMAIN_LOADER_TYPE_PFLASH,
+VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS,
+VIR_DOMAIN_OS_DEF_FIRMWARE_EFI,
 
 VIR_DOMAIN_OS_DEF_FIRMWARE_LAST
 } virDomainOsDefFirmware;
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 529ab8d68e..84c80eaacb 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -899,7 +899,7 @@ qemuFirmwareMatchesMachineArch(const qemuFirmware *fw,
 
 
 static qemuFirmwareOSInterface
-qemuFirmwareOSInterfaceTypeFromOsDefFirmware(int fw)
+qemuFirmwareOSInterfaceTypeFromOsDefFirmware(virDomainOsDefFirmware fw)
 {
 switch (fw) {
 case VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS:
@@ -915,6 +915,23 @@ qemuFirmwareOSInterfaceTypeFromOsDefFirmware(int fw)
 }
 
 
+static qemuFirmwareOSInterface
+qemuFirmwareOSInterfaceTypeFromOsDefLoaderType(virDomainLoader type)
+{
+switch (type) {
+case VIR_DOMAIN_LOADER_TYPE_ROM:
+return QEMU_FIRMWARE_OS_INTERFACE_BIOS;
+case VIR_DOMAIN_LOADER_TYPE_PFLASH:
+return QEMU_FIRMWARE_OS_INTERFACE_UEFI;
+case VIR_DOMAIN_LOADER_TYPE_NONE:
+case VIR_DOMAIN_LOADER_TYPE_LAST:
+break;
+}
+
+return QEMU_FIRMWARE_OS_INTERFACE_NONE;
+}
+
+
 #define VIR_QEMU_FIRMWARE_AMD_SEV_ES_POLICY (1 << 2)
 
 
@@ -939,7 +956,7 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
 
 if (want == QEMU_FIRMWARE_OS_INTERFACE_NONE &&
 def->os.loader) {
-want = 
qemuFirmwareOSInterfaceTypeFromOsDefFirmware(def->os.loader->type);
+want = 
qemuFirmwareOSInterfaceTypeFromOsDefLoaderType(def->os.loader->type);
 
 if (fw->mapping.device != QEMU_FIRMWARE_DEVICE_FLASH ||
 STRNEQ(def->os.loader->path, 
fw->mapping.data.flash.executable.filename)) {
-- 
2.33.1

Re: [PATCH V2 0/2] docs: Add man page for libvirt-guests

[Andrea Bolognani]

On Mon, Jan 10, 2022 at 02:23:59PM -0700, Jim Fehlig wrote:
> V2 of 
> https://listman.redhat.com/archives/libvir-list/2022-January/msg00336.html
>
> Changes since V1:
> - New patch to only install libvirt-guests when building libvirtd
> - Only install libvirt-guests man page when building libvirtd
> - Default config overridden in sysconfig file, not environment
>
> Jim Fehlig (2):
>   build: Only install libvirt-guests when building libvirtd
>   docs: Add man page for libvirt-guests
>
>  docs/manpages/index.rst  |   1 +
>  docs/manpages/libvirt-guests.rst | 151 +++
>  docs/manpages/meson.build|   1 +
>  libvirt.spec.in  |   1 +
>  tools/libvirt-guests.service.in  |   2 +-
>  tools/meson.build|  38 
>  6 files changed, 175 insertions(+), 19 deletions(-)
>  create mode 100644 docs/manpages/libvirt-guests.rst

Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH 2/3] virdnsmasq: Lookup DNSMASQ in PATH

[Andrea Bolognani]

On Tue, Jan 11, 2022 at 10:24:00AM +, Daniel P. Berrangé wrote:
> On Tue, Jan 11, 2022 at 10:18:23AM +, Andrea Bolognani wrote:
> > On Tue, Jan 11, 2022 at 09:59:06AM +, Daniel P. Berrangé wrote:
> > > Why do we need a fallback ?  If someone has put 'dnsmasq' in $PATH
> > > without the execute bit set, surely that's just a broken deployment
> > > and returning NULL is correct.
> >
> > Agreed in general, but we want the test suite to still pass even if
> > dnsmasq is not installed on the build machine. The approach I
> > suggested in my other message would achieve that.
>
> Shouldn't we just mock the virFindFileInPath call in the test suite
> to return whatever fixed binary path we need, so we don't have to
> modify the driver code with special logic for tests.

We'd have to also mock *running* the binary, as we get some
information out of that. Probably a better long-term approach, but I
don't think it should necessarily be a blocker for this series, which
already represents an improvement over the status quo.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH v6 3/4] remove sysconfig files

[Andrea Bolognani]

On Tue, Jan 11, 2022 at 10:10:30AM +, Daniel P. Berrangé wrote:
> On Tue, Jan 11, 2022 at 10:05:41AM +, Andrea Bolognani wrote:
> > But, what you're saying is that there are still valid use cases
> > for setting those in the service file, and we can't quite get rid of
> > them yet - or possibly ever?
>
> Never say never, but think "5+ years away" before they'd be something
> to consider getting rid of.

Apologies for going on and on about this, but does the "5+ years"
figure imply that further development work is necessary, or is the
code needed to make setting these in the service file obsolete
already in libvirt? Because if the latter I think we can simply drop
the comments today: anyone who was setting them in the past can still
set them even if we don't provide examples, and anyone who was not
already using them is probably going to be better off if we don't
advertise the old way of doing things.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH for 8.0 0/3] Fix creation of snapshots without libvirt metadata

[Michal Prívozník]

On 1/11/22 10:53, Peter Krempa wrote:
> After a recent unreleased refactor we don't remove the temporary
> snapshot object from the list when finishing a snapshot without metadata
> thus we should fix it before releasing a buggy release.
> 
> This patchset fixes it by not adding it into the list in the first
> place.
> 
> Peter Krempa (3):
>   virdomainmomentobjlist.h: Convert to modern header style
>   conf: moment: Export helpers to create the virDomainMoment wrapper
>   qemuSnapshotCreate: Don't insert snapshot into list with
> VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA
> 
>  src/conf/virdomainmomentobjlist.c |   4 +-
>  src/conf/virdomainmomentobjlist.h | 121 +++---
>  src/libvirt_private.syms  |   2 +
>  src/qemu/qemu_snapshot.c  |  25 +++---
>  4 files changed, 97 insertions(+), 55 deletions(-)
> 

Reviewed-by: Michal Privoznik 

Michal

Re: [PATCH 3/3] qemuSnapshotCreate: Don't insert snapshot into list with VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA

[Michal Prívozník]

On 1/11/22 10:53, Peter Krempa wrote:
> Our approach to snapshots without metadata was to insert them to the
> snapshot list and then later remove them from the list when the flag is
> present.
> 
> This quirky logic was broken in a recent refactor of the snapshot code
> causing that the snapshot stayed inserted in the snapshot list.
> 
> Recent refactor of the snapshot code didn't faithfully relocate this
> logic to the new function.
> 
> Rather than attempting to restore the quirky logic of adding and then
> removing the object, don't add the snapshot into the list at all when
> the user doesn't want metadata.
> 
> We achieve this by creating a temporary 'virDomainMomentObj' wrapper
> which is not inserted into the list and using that instead of calling
> virDomainSnapshotAssignDef.
> 
> Fixes: 9bad0fb809b
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2039131
> Signed-off-by: Peter Krempa 
> ---
>  src/qemu/qemu_snapshot.c | 25 +++--
>  1 file changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
> index 9a5d3e60aa..e9fc9051c1 100644
> --- a/src/qemu/qemu_snapshot.c
> +++ b/src/qemu/qemu_snapshot.c
> @@ -1756,7 +1756,7 @@ qemuSnapshotCreate(virDomainObj *vm,
> virQEMUDriverConfig *cfg,
> unsigned int flags)
>  {
> -
> +g_autoptr(virDomainMomentObj) noMetadataSnap = NULL;

Nitpick, this variable is used as a bool later, which creates double
negative conditions like !noMetadataSnap. But I'm failing to suggest
anything better.

Michal

Re: [PATCH v3] report error when virProcessGetStatInfo() is unable to parse data

[Michal Prívozník]

On 1/11/22 11:20, Ani Sinha wrote:
> Currently virProcessGetStatInfo() always returns success and only logs error
> when it is unable to parse the data. Make this function actually report the
> error and return a negative value in this error scenario.
> 
> Fix the callers so that they do not override the error generated.
> Also fix non-linux implementation of this function so as to report error.
> 
> Signed-off-by: Ani Sinha 
> ---
>  src/ch/ch_driver.c | 2 --
>  src/qemu/qemu_driver.c | 7 +--
>  src/util/virprocess.c  | 9 +++--
>  3 files changed, 8 insertions(+), 10 deletions(-)
> 
> changelog:
> v3: fix non-linux implementation
> v2: fix callers
> 

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index d3d76c003f..9a17c93b08 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c

> @@ -10530,8 +10526,7 @@ qemuDomainMemoryStatsInternal(virQEMUDriver *driver,
>  }
>  
>  if (virProcessGetStatInfo(NULL, NULL, , vm->pid, 0) < 0) {
> -virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> -   _("cannot get RSS for domain"));
> +return -1;

Returning -1 changes semantics of this function. Previously it was
tolerant to errors and now it isn't. For instance, if this function was
called on a FreeBSD machine (yes, QEMU driver can be enabled there) then
despite fetching mem stats earlier through monitor (not visible in the
context) an error is now returned which in turn makes whole
virDomainMemoryStats() API fail.

The 'return -1' should be replaced with virResetLastError().

Having said that, now there will be an error reported in the logs every
time the API is called. I wonder what we can do about it. Previously it
was "just" a warning.

Michal

Re: [RFC v3 1/2] Add the port allocation logic for isa-serial devices.

[Ani Sinha]

On Tue, 11 Jan 2022, Divya Garg wrote:

>
> On 11/01/22 4:09 pm, Ani Sinha wrote:
> > Please add SOB.
> Yes Thankyou so much. Should I have a new version for it or on the same thread
> send
> the updated patches ?

I am fine with your bugfix patch patch 2.
DanPB and others, could you guys provide some feedback on both patches
before she spins out another version with SOB added?

Re: [RFC v3 1/2] Add the port allocation logic for isa-serial devices.

[Divya Garg]

On 11/01/22 4:09 pm, Ani Sinha wrote:

Please add SOB.
Yes Thankyou so much. Should I have a new version for it or on the same 
thread send

the updated patches ?


On Mon, 10 Jan 2022, Divya Garg wrote:


This commit takes care of following cases:
-> Check availability of requested ports.
   ->The total number of requested ports should not be more than
 VIR_MAX_ISA_SERIAL_PORTS.
   ->The ports requested should be less than VIR_MAX_ISA_SERIAL_PORTS.
   ->VIR_MAX_ISA_SERIAL_PORTS should correspond to MAX_ISA_SERIAL_PORTS
 specified in qemu code commit def337ffda34d331404bd7f1a42726b71500df22.
-> Prevent duplicate device assignments to the same port.
-> In case no ports are provided in the XML, this patch scans the list of unused
isa-serial indices to automatically assign available ports for this VM.
---
  src/conf/domain_conf.c| 61 ---
  src/conf/domain_conf.h|  6 ++
  ...g-console-compat-2-live+console-virtio.xml |  4 +-
  .../qemuhotplug-console-compat-2-live.xml |  4 +-
  .../serial-tcp-tlsx509-chardev-notls.xml  |  2 +-
  .../serial-tcp-tlsx509-chardev-verify.xml |  2 +-
  .../serial-tcp-tlsx509-chardev.xml|  2 +-
  .../serial-tcp-tlsx509-secret-chardev.xml |  2 +-
  .../serial-tcp-tlsx509-chardev.xml|  2 +-
  9 files changed, 68 insertions(+), 17 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5691b8d2d5..e468e98045 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5330,6 +5330,56 @@ virDomainHostdevDefPostParse(virDomainHostdevDef *dev,
  }


+static int
+virDomainChrIsaSerialDefPostParse(virDomainDef *def)
+{
+size_t i, j;
+size_t isa_serial_count = 0;
+bool used_serial_port[VIR_MAX_ISA_SERIAL_PORTS] = {false};
+
+/* Perform all the required checks. */
+for (i = 0; i < def->nserials; i++) {
+
+if (def->serials[i]->targetType !=
+VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_ISA_SERIAL)
+continue;
+
+if (isa_serial_count++ >= VIR_MAX_ISA_SERIAL_PORTS ||
+def->serials[i]->target.port >= VIR_MAX_ISA_SERIAL_PORTS) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Maximum supported number of ISA serial ports is 
'%d'."), VIR_MAX_ISA_SERIAL_PORTS);
+return -1;
+}
+
+if (def->serials[i]->target.port != -1) {
+if (used_serial_port[def->serials[i]->target.port]) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("target port '%d' already allocated."), 
def->serials[i]->target.port);
+return -1;
+}
+used_serial_port[def->serials[i]->target.port] = true;
+}
+}
+
+/* Assign the ports to the devices. */
+for (i = 0; i < def->nserials; i++) {
+
+if (def->serials[i]->targetType !=
+VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_ISA_SERIAL ||
+def->serials[i]->target.port != -1)
+continue;
+
+for (j = 0; j < VIR_MAX_ISA_SERIAL_PORTS; j++) {
+if (!used_serial_port[j]) {
+def->serials[i]->target.port = j;
+used_serial_port[j] = true;
+break;
+}
+}
+}
+return 0;
+}
+
  static void
  virDomainChrDefPostParse(virDomainChrDef *chr,
   const virDomainDef *def)
@@ -6197,6 +6247,9 @@ virDomainDefPostParse(virDomainDef *def,
  goto cleanup;
  }

+if (virDomainChrIsaSerialDefPostParse(def) < 0)
+return -1;
+
  /* iterate the devices */
  ret = virDomainDeviceInfoIterateFlags(def,
virDomainDefPostParseDeviceIterator,
@@ -19929,14 +19982,6 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt,
  if (!chr)
  return NULL;

-if (chr->target.port == -1) {
-int maxport = -1;
-for (j = 0; j < i; j++) {
-if (def->serials[j]->target.port > maxport)
-maxport = def->serials[j]->target.port;
-}
-chr->target.port = maxport + 1;
-}
  def->serials[def->nserials++] = chr;
  }
  VIR_FREE(nodes);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 144ba4dd12..a8f41dc8c2 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1187,6 +1187,12 @@ typedef enum {
  VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST
  } virDomainChrConsoleTargetType;

+/*
+ * The value of VIR_MAX_ISA_SERIAL_PORTS corresponds to MAX_ISA_SERIAL_PORTS
+ * set in qemu code base.
+ */
+#define VIR_MAX_ISA_SERIAL_PORTS 4
+
  typedef enum {
  VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_NONE = 0,
  VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_ISA_SERIAL,
diff --git 
a/tests/qemuhotplugtestdomains/qemuhotplug-console-compat-2-live+console-virtio.xml
 

Re: [RFC v3 2/2] qemu: add index for isa-serial device using target.port

[Divya Garg]

On 11/01/22 4:08 pm, Ani Sinha wrote:

SOB line is missing. Patches are not accepted without SOB.


I am extremely sorry I forgot to add it. Thankyou so much.



On Mon, 10 Jan 2022, Divya Garg wrote:


VM XML accepts target.port but this does not get passed while building the qemu
command line for this VM.
---
  src/qemu/qemu_command.c   | 22 ++-
  tests/qemuxml2argvdata/bios.args  |  2 +-
  .../qemuxml2argvdata/console-compat-auto.args |  2 +-
  .../console-compat-auto.x86_64-latest.args|  2 +-
  .../console-compat-chardev.args   |  2 +-
  .../console-compat-chardev.x86_64-latest.args |  2 +-
  tests/qemuxml2argvdata/console-compat.args|  2 +-
  .../console-compat.x86_64-latest.args |  2 +-
  .../qemuxml2argvdata/console-virtio-many.args |  2 +-
  tests/qemuxml2argvdata/controller-order.args  |  2 +-
  .../name-escape.x86_64-2.11.0.args|  4 ++--
  .../name-escape.x86_64-latest.args|  4 ++--
  .../q35-virt-manager-basic.args   |  2 +-
  .../serial-dev-chardev-iobase.args|  2 +-
  ...rial-dev-chardev-iobase.x86_64-latest.args |  2 +-
  .../qemuxml2argvdata/serial-dev-chardev.args  |  2 +-
  .../serial-dev-chardev.x86_64-latest.args |  2 +-
  .../qemuxml2argvdata/serial-file-chardev.args |  2 +-
  .../serial-file-chardev.x86_64-latest.args|  2 +-
  tests/qemuxml2argvdata/serial-file-log.args   |  2 +-
  .../serial-file-log.x86_64-latest.args|  2 +-
  .../qemuxml2argvdata/serial-many-chardev.args |  4 ++--
  .../serial-many-chardev.x86_64-latest.args|  4 ++--
  .../qemuxml2argvdata/serial-pty-chardev.args  |  2 +-
  .../serial-pty-chardev.x86_64-latest.args |  2 +-
  tests/qemuxml2argvdata/serial-spiceport.args  |  2 +-
  .../serial-spiceport.x86_64-latest.args   |  2 +-
  .../qemuxml2argvdata/serial-tcp-chardev.args  |  2 +-
  .../serial-tcp-chardev.x86_64-latest.args |  2 +-
  .../serial-tcp-telnet-chardev.args|  2 +-
  ...rial-tcp-telnet-chardev.x86_64-latest.args |  2 +-
  .../serial-tcp-tlsx509-chardev-notls.args |  4 ++--
  ...p-tlsx509-chardev-notls.x86_64-latest.args |  4 ++--
  .../serial-tcp-tlsx509-chardev-verify.args|  4 ++--
  ...-tlsx509-chardev-verify.x86_64-latest.args |  4 ++--
  .../serial-tcp-tlsx509-chardev.args   |  4 ++--
  ...ial-tcp-tlsx509-chardev.x86_64-latest.args |  4 ++--
  .../serial-tcp-tlsx509-secret-chardev.args|  4 ++--
  ...-tlsx509-secret-chardev.x86_64-latest.args |  4 ++--
  .../qemuxml2argvdata/serial-udp-chardev.args  |  4 ++--
  .../serial-udp-chardev.x86_64-latest.args |  4 ++--
  .../qemuxml2argvdata/serial-unix-chardev.args |  4 ++--
  .../serial-unix-chardev.x86_64-latest.args|  4 ++--
  tests/qemuxml2argvdata/serial-vc-chardev.args |  2 +-
  .../serial-vc-chardev.x86_64-latest.args  |  2 +-
  tests/qemuxml2argvdata/user-aliases.args  |  4 ++--
  .../virtio-9p-createmode.x86_64-latest.args   |  2 +-
  .../virtio-9p-multidevs.x86_64-latest.args|  2 +-
  .../x86_64-pc-graphics.x86_64-latest.args |  2 +-
  .../x86_64-pc-headless.x86_64-latest.args |  2 +-
  .../x86_64-q35-graphics.x86_64-latest.args|  2 +-
  .../x86_64-q35-headless.x86_64-latest.args|  2 +-
  52 files changed, 85 insertions(+), 73 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d822533ccb..a1fd5443be 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -10717,6 +10717,7 @@ qemuBuildSerialChrDeviceProps(const virDomainDef *def,
  {
  g_autoptr(virJSONValue) props = NULL;
  g_autofree char *chardev = g_strdup_printf("char%s", serial->info.alias);
+const char *typestr;
  virQEMUCapsFlags caps;

  switch ((virDomainChrSerialTargetModel) serial->targetModel) {
@@ -10750,11 +10751,22 @@ qemuBuildSerialChrDeviceProps(const virDomainDef *def,
  return NULL;
  }

-if (virJSONValueObjectAdd(,
-  "s:driver", 
virDomainChrSerialTargetModelTypeToString(serial->targetModel),
-  "s:chardev", chardev,
-  "s:id", serial->info.alias,
-  NULL) < 0)
+typestr = virDomainChrSerialTargetModelTypeToString(serial->targetModel);
+
+if (serial->targetModel == VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_ISA_SERIAL) {
+if (virJSONValueObjectAdd(,
+  "s:driver", typestr,
+  "s:chardev", chardev,
+  "s:id", serial->info.alias,
+  "k:index", serial->target.port,
+  NULL) < 0)
+return NULL;
+}
+else if (virJSONValueObjectAdd(,
+   "s:driver", typestr,
+   "s:chardev", chardev,
+   "s:id", 

Re: [PATCH] docs: minor fix in launchSecurity

[Pavel Hrdina]

On Tue, Jan 11, 2022 at 10:03:32AM +0100, Boris Fiuczynski wrote:
> Correcting XML element.
> 
> Signed-off-by: Boris Fiuczynski 
> ---
>  docs/formatdomain.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature

Re: [RFC v3 1/2] Add the port allocation logic for isa-serial devices.

[Ani Sinha]

Please add SOB.

On Mon, 10 Jan 2022, Divya Garg wrote:

> This commit takes care of following cases:
> -> Check availability of requested ports.
>   ->The total number of requested ports should not be more than
> VIR_MAX_ISA_SERIAL_PORTS.
>   ->The ports requested should be less than VIR_MAX_ISA_SERIAL_PORTS.
>   ->VIR_MAX_ISA_SERIAL_PORTS should correspond to MAX_ISA_SERIAL_PORTS
> specified in qemu code commit def337ffda34d331404bd7f1a42726b71500df22.
> -> Prevent duplicate device assignments to the same port.
> -> In case no ports are provided in the XML, this patch scans the list of 
> unused
>isa-serial indices to automatically assign available ports for this VM.
> ---
>  src/conf/domain_conf.c| 61 ---
>  src/conf/domain_conf.h|  6 ++
>  ...g-console-compat-2-live+console-virtio.xml |  4 +-
>  .../qemuhotplug-console-compat-2-live.xml |  4 +-
>  .../serial-tcp-tlsx509-chardev-notls.xml  |  2 +-
>  .../serial-tcp-tlsx509-chardev-verify.xml |  2 +-
>  .../serial-tcp-tlsx509-chardev.xml|  2 +-
>  .../serial-tcp-tlsx509-secret-chardev.xml |  2 +-
>  .../serial-tcp-tlsx509-chardev.xml|  2 +-
>  9 files changed, 68 insertions(+), 17 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 5691b8d2d5..e468e98045 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -5330,6 +5330,56 @@ virDomainHostdevDefPostParse(virDomainHostdevDef *dev,
>  }
>
>
> +static int
> +virDomainChrIsaSerialDefPostParse(virDomainDef *def)
> +{
> +size_t i, j;
> +size_t isa_serial_count = 0;
> +bool used_serial_port[VIR_MAX_ISA_SERIAL_PORTS] = {false};
> +
> +/* Perform all the required checks. */
> +for (i = 0; i < def->nserials; i++) {
> +
> +if (def->serials[i]->targetType !=
> +VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_ISA_SERIAL)
> +continue;
> +
> +if (isa_serial_count++ >= VIR_MAX_ISA_SERIAL_PORTS ||
> +def->serials[i]->target.port >= VIR_MAX_ISA_SERIAL_PORTS) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("Maximum supported number of ISA serial ports 
> is '%d'."), VIR_MAX_ISA_SERIAL_PORTS);
> +return -1;
> +}
> +
> +if (def->serials[i]->target.port != -1) {
> +if (used_serial_port[def->serials[i]->target.port]) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("target port '%d' already allocated."), 
> def->serials[i]->target.port);
> +return -1;
> +}
> +used_serial_port[def->serials[i]->target.port] = true;
> +}
> +}
> +
> +/* Assign the ports to the devices. */
> +for (i = 0; i < def->nserials; i++) {
> +
> +if (def->serials[i]->targetType !=
> +VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_ISA_SERIAL ||
> +def->serials[i]->target.port != -1)
> +continue;
> +
> +for (j = 0; j < VIR_MAX_ISA_SERIAL_PORTS; j++) {
> +if (!used_serial_port[j]) {
> +def->serials[i]->target.port = j;
> +used_serial_port[j] = true;
> +break;
> +}
> +}
> +}
> +return 0;
> +}
> +
>  static void
>  virDomainChrDefPostParse(virDomainChrDef *chr,
>   const virDomainDef *def)
> @@ -6197,6 +6247,9 @@ virDomainDefPostParse(virDomainDef *def,
>  goto cleanup;
>  }
>
> +if (virDomainChrIsaSerialDefPostParse(def) < 0)
> +return -1;
> +
>  /* iterate the devices */
>  ret = virDomainDeviceInfoIterateFlags(def,
>
> virDomainDefPostParseDeviceIterator,
> @@ -19929,14 +19982,6 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt,
>  if (!chr)
>  return NULL;
>
> -if (chr->target.port == -1) {
> -int maxport = -1;
> -for (j = 0; j < i; j++) {
> -if (def->serials[j]->target.port > maxport)
> -maxport = def->serials[j]->target.port;
> -}
> -chr->target.port = maxport + 1;
> -}
>  def->serials[def->nserials++] = chr;
>  }
>  VIR_FREE(nodes);
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 144ba4dd12..a8f41dc8c2 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1187,6 +1187,12 @@ typedef enum {
>  VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST
>  } virDomainChrConsoleTargetType;
>
> +/*
> + * The value of VIR_MAX_ISA_SERIAL_PORTS corresponds to MAX_ISA_SERIAL_PORTS
> + * set in qemu code base.
> + */
> +#define VIR_MAX_ISA_SERIAL_PORTS 4
> +
>  typedef enum {
>  VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_NONE = 0,
>  VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_ISA_SERIAL,
> diff --git 
> 

Re: [RFC v3 2/2] qemu: add index for isa-serial device using target.port

[Ani Sinha]

SOB line is missing. Patches are not accepted without SOB.

On Mon, 10 Jan 2022, Divya Garg wrote:

> VM XML accepts target.port but this does not get passed while building the 
> qemu
> command line for this VM.
> ---
>  src/qemu/qemu_command.c   | 22 ++-
>  tests/qemuxml2argvdata/bios.args  |  2 +-
>  .../qemuxml2argvdata/console-compat-auto.args |  2 +-
>  .../console-compat-auto.x86_64-latest.args|  2 +-
>  .../console-compat-chardev.args   |  2 +-
>  .../console-compat-chardev.x86_64-latest.args |  2 +-
>  tests/qemuxml2argvdata/console-compat.args|  2 +-
>  .../console-compat.x86_64-latest.args |  2 +-
>  .../qemuxml2argvdata/console-virtio-many.args |  2 +-
>  tests/qemuxml2argvdata/controller-order.args  |  2 +-
>  .../name-escape.x86_64-2.11.0.args|  4 ++--
>  .../name-escape.x86_64-latest.args|  4 ++--
>  .../q35-virt-manager-basic.args   |  2 +-
>  .../serial-dev-chardev-iobase.args|  2 +-
>  ...rial-dev-chardev-iobase.x86_64-latest.args |  2 +-
>  .../qemuxml2argvdata/serial-dev-chardev.args  |  2 +-
>  .../serial-dev-chardev.x86_64-latest.args |  2 +-
>  .../qemuxml2argvdata/serial-file-chardev.args |  2 +-
>  .../serial-file-chardev.x86_64-latest.args|  2 +-
>  tests/qemuxml2argvdata/serial-file-log.args   |  2 +-
>  .../serial-file-log.x86_64-latest.args|  2 +-
>  .../qemuxml2argvdata/serial-many-chardev.args |  4 ++--
>  .../serial-many-chardev.x86_64-latest.args|  4 ++--
>  .../qemuxml2argvdata/serial-pty-chardev.args  |  2 +-
>  .../serial-pty-chardev.x86_64-latest.args |  2 +-
>  tests/qemuxml2argvdata/serial-spiceport.args  |  2 +-
>  .../serial-spiceport.x86_64-latest.args   |  2 +-
>  .../qemuxml2argvdata/serial-tcp-chardev.args  |  2 +-
>  .../serial-tcp-chardev.x86_64-latest.args |  2 +-
>  .../serial-tcp-telnet-chardev.args|  2 +-
>  ...rial-tcp-telnet-chardev.x86_64-latest.args |  2 +-
>  .../serial-tcp-tlsx509-chardev-notls.args |  4 ++--
>  ...p-tlsx509-chardev-notls.x86_64-latest.args |  4 ++--
>  .../serial-tcp-tlsx509-chardev-verify.args|  4 ++--
>  ...-tlsx509-chardev-verify.x86_64-latest.args |  4 ++--
>  .../serial-tcp-tlsx509-chardev.args   |  4 ++--
>  ...ial-tcp-tlsx509-chardev.x86_64-latest.args |  4 ++--
>  .../serial-tcp-tlsx509-secret-chardev.args|  4 ++--
>  ...-tlsx509-secret-chardev.x86_64-latest.args |  4 ++--
>  .../qemuxml2argvdata/serial-udp-chardev.args  |  4 ++--
>  .../serial-udp-chardev.x86_64-latest.args |  4 ++--
>  .../qemuxml2argvdata/serial-unix-chardev.args |  4 ++--
>  .../serial-unix-chardev.x86_64-latest.args|  4 ++--
>  tests/qemuxml2argvdata/serial-vc-chardev.args |  2 +-
>  .../serial-vc-chardev.x86_64-latest.args  |  2 +-
>  tests/qemuxml2argvdata/user-aliases.args  |  4 ++--
>  .../virtio-9p-createmode.x86_64-latest.args   |  2 +-
>  .../virtio-9p-multidevs.x86_64-latest.args|  2 +-
>  .../x86_64-pc-graphics.x86_64-latest.args |  2 +-
>  .../x86_64-pc-headless.x86_64-latest.args |  2 +-
>  .../x86_64-q35-graphics.x86_64-latest.args|  2 +-
>  .../x86_64-q35-headless.x86_64-latest.args|  2 +-
>  52 files changed, 85 insertions(+), 73 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index d822533ccb..a1fd5443be 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -10717,6 +10717,7 @@ qemuBuildSerialChrDeviceProps(const virDomainDef *def,
>  {
>  g_autoptr(virJSONValue) props = NULL;
>  g_autofree char *chardev = g_strdup_printf("char%s", serial->info.alias);
> +const char *typestr;
>  virQEMUCapsFlags caps;
>
>  switch ((virDomainChrSerialTargetModel) serial->targetModel) {
> @@ -10750,11 +10751,22 @@ qemuBuildSerialChrDeviceProps(const virDomainDef 
> *def,
>  return NULL;
>  }
>
> -if (virJSONValueObjectAdd(,
> -  "s:driver", 
> virDomainChrSerialTargetModelTypeToString(serial->targetModel),
> -  "s:chardev", chardev,
> -  "s:id", serial->info.alias,
> -  NULL) < 0)
> +typestr = virDomainChrSerialTargetModelTypeToString(serial->targetModel);
> +
> +if (serial->targetModel == 
> VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_ISA_SERIAL) {
> +if (virJSONValueObjectAdd(,
> +  "s:driver", typestr,
> +  "s:chardev", chardev,
> +  "s:id", serial->info.alias,
> +  "k:index", serial->target.port,
> +  NULL) < 0)
> +return NULL;
> +}
> +else if (virJSONValueObjectAdd(,
> +   "s:driver", typestr,
> +   "s:chardev", chardev,
> +   

Re: [PATCH 2/3] virdnsmasq: Lookup DNSMASQ in PATH

[Daniel P . Berrangé]

On Tue, Jan 11, 2022 at 10:18:23AM +, Andrea Bolognani wrote:
> On Tue, Jan 11, 2022 at 09:59:06AM +, Daniel P. Berrangé wrote:
> > On Tue, Jan 11, 2022 at 09:28:45AM +0100, Michal Prívozník wrote:
> > > Thanks, but as I was looking through virFindFileInPath() I've noticed
> > > that if the file exists but is not executable then NULL is returned, so
> > > we will need a fallback case. Effectively this needs to be squashed in:
> >
> > Why do we need a fallback ?  If someone has put 'dnsmasq' in $PATH
> > without the execute bit set, surely that's just a broken deployment
> > and returning NULL is correct.
> 
> Agreed in general, but we want the test suite to still pass even if
> dnsmasq is not installed on the build machine. The approach I
> suggested in my other message would achieve that.

Shouldn't we just mock the virFindFileInPath call in the test suite
to return whatever fixed binary path we need, so we don't have to
modify the driver code with special logic for tests.

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

[PATCH v3] report error when virProcessGetStatInfo() is unable to parse data

[Ani Sinha]

Currently virProcessGetStatInfo() always returns success and only logs error
when it is unable to parse the data. Make this function actually report the
error and return a negative value in this error scenario.

Fix the callers so that they do not override the error generated.
Also fix non-linux implementation of this function so as to report error.

Signed-off-by: Ani Sinha 
---
 src/ch/ch_driver.c | 2 --
 src/qemu/qemu_driver.c | 7 +--
 src/util/virprocess.c  | 9 +++--
 3 files changed, 8 insertions(+), 10 deletions(-)

changelog:
v3: fix non-linux implementation
v2: fix callers

diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 53e0872207..3cbc668489 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -1073,8 +1073,6 @@ chDomainHelperGetVcpus(virDomainObj *vm,
 if (virProcessGetStatInfo(>cpuTime,
   >cpu, NULL,
   vm->pid, vcpupid) < 0) {
-virReportSystemError(errno, "%s",
-  _("cannot get vCPU placement & pCPU 
time"));
 return -1;
 }
 }
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d3d76c003f..9a17c93b08 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1359,8 +1359,6 @@ qemuDomainHelperGetVcpus(virDomainObj *vm,
 if (virProcessGetStatInfo(>cpuTime,
   >cpu, NULL,
   vm->pid, vcpupid) < 0) {
-virReportSystemError(errno, "%s",
- _("cannot get vCPU placement & pCPU 
time"));
 return -1;
 }
 }
@@ -2521,8 +2519,6 @@ qemuDomainGetInfo(virDomainPtr dom,
 if (virDomainObjIsActive(vm)) {
 if (virProcessGetStatInfo(&(info->cpuTime), NULL, NULL,
   vm->pid, 0) < 0) {
-virReportError(VIR_ERR_OPERATION_FAILED, "%s",
-   _("cannot read cputime for domain"));
 goto cleanup;
 }
 }
@@ -10530,8 +10526,7 @@ qemuDomainMemoryStatsInternal(virQEMUDriver *driver,
 }
 
 if (virProcessGetStatInfo(NULL, NULL, , vm->pid, 0) < 0) {
-virReportError(VIR_ERR_OPERATION_FAILED, "%s",
-   _("cannot get RSS for domain"));
+return -1;
 } else {
 stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_RSS;
 stats[ret].val = rss;
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index b559a4257e..709ec616de 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -1784,7 +1784,11 @@ virProcessGetStatInfo(unsigned long long *cpuTime,
 virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_STIME], NULL, 10, 
) < 0 ||
 virStrToLong_l(proc_stat[VIR_PROCESS_STAT_RSS], NULL, 10, ) < 0 ||
 virStrToLong_i(proc_stat[VIR_PROCESS_STAT_PROCESSOR], NULL, 10, ) 
< 0) {
-VIR_WARN("cannot parse process status data");
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot parse process status data for pid '%d/%d'"),
+   (int) pid, (int) tid);
+
+return -1;
 }
 
 /* We got jiffies
@@ -1881,7 +1885,8 @@ virProcessGetStatInfo(unsigned long long *cpuTime 
G_GNUC_UNUSED,
   pid_t pid G_GNUC_UNUSED,
   pid_t tid G_GNUC_UNUSED)
 {
-errno = ENOSYS;
+virReportSystemError(ENOSYS, "%s",
+ _("Process statistics data is not supported on this 
platform"));
 return -1;
 }
 
-- 
2.25.1

Re: [PATCH 2/3] virdnsmasq: Lookup DNSMASQ in PATH

[Andrea Bolognani]

On Tue, Jan 11, 2022 at 09:59:06AM +, Daniel P. Berrangé wrote:
> On Tue, Jan 11, 2022 at 09:28:45AM +0100, Michal Prívozník wrote:
> > Thanks, but as I was looking through virFindFileInPath() I've noticed
> > that if the file exists but is not executable then NULL is returned, so
> > we will need a fallback case. Effectively this needs to be squashed in:
>
> Why do we need a fallback ?  If someone has put 'dnsmasq' in $PATH
> without the execute bit set, surely that's just a broken deployment
> and returning NULL is correct.

Agreed in general, but we want the test suite to still pass even if
dnsmasq is not installed on the build machine. The approach I
suggested in my other message would achieve that.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH v6 3/4] remove sysconfig files

[Daniel P . Berrangé]

On Tue, Jan 11, 2022 at 10:05:41AM +, Andrea Bolognani wrote:
> On Tue, Jan 11, 2022 at 09:56:30AM +, Daniel P. Berrangé wrote:
> > On Tue, Jan 11, 2022 at 09:51:05AM +, Andrea Bolognani wrote:
> > > On Tue, Jan 11, 2022 at 10:21:32AM +0100, Olaf Hering wrote:
> > > > Mon, 3 Jan 2022 03:18:11 -0800 Andrea Bolognani :
> > > > > The fact that we still QEMU_AUDIO_DRV and SDL_AUDIODRIVER in the
> > > > > service file even after your changes goes against this principle.
> > > >
> > > > So what should be done about these?
> > > >
> > > > I think whoever added support for these environment variables a couple
> > > > of decades ago failed to provide proper documentation. Or perhaps
> > > > whoever wrote the "sound" section of formatdomain.html.in failed to
> > > > recognize the existence of these environment variables.
> > >
> > > Dan, do you think it would be okay to simply drop these from the
> > > service/sysconfig file? IIUC we have a proper way to set them on a
> > > per-domain level now.
> >
> > I don't think we need todo anything differentl - Olaf's patches
> > are fine in this respect already IMHO.  Certainly we don't need
> > to add more documentation about these than already exists as we
> > don't need to encourage more usage.
> 
> Right, I got a bit caught up in the details and sort of lost track of
> the initial discussion. I never had a problem with Olaf's changes
> with respect to those variable, as they basically maintain the status
> quo. But, what you're saying is that there are still valid use cases
> for setting those in the service file, and we can't quite get rid of
> them yet - or possibly ever?

Never say never, but think "5+ years away" before they'd be something
to consider getting rid of.

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 v6 3/4] remove sysconfig files

[Andrea Bolognani]

On Tue, Jan 11, 2022 at 09:56:30AM +, Daniel P. Berrangé wrote:
> On Tue, Jan 11, 2022 at 09:51:05AM +, Andrea Bolognani wrote:
> > On Tue, Jan 11, 2022 at 10:21:32AM +0100, Olaf Hering wrote:
> > > Mon, 3 Jan 2022 03:18:11 -0800 Andrea Bolognani :
> > > > The fact that we still QEMU_AUDIO_DRV and SDL_AUDIODRIVER in the
> > > > service file even after your changes goes against this principle.
> > >
> > > So what should be done about these?
> > >
> > > I think whoever added support for these environment variables a couple
> > > of decades ago failed to provide proper documentation. Or perhaps
> > > whoever wrote the "sound" section of formatdomain.html.in failed to
> > > recognize the existence of these environment variables.
> >
> > Dan, do you think it would be okay to simply drop these from the
> > service/sysconfig file? IIUC we have a proper way to set them on a
> > per-domain level now.
>
> I don't think we need todo anything differentl - Olaf's patches
> are fine in this respect already IMHO.  Certainly we don't need
> to add more documentation about these than already exists as we
> don't need to encourage more usage.

Right, I got a bit caught up in the details and sort of lost track of
the initial discussion. I never had a problem with Olaf's changes
with respect to those variable, as they basically maintain the status
quo. But, what you're saying is that there are still valid use cases
for setting those in the service file, and we can't quite get rid of
them yet - or possibly ever?

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH 1/1] qemu_tpm: Get swtpm pid without binary validation

[Michal Prívozník]

On 1/3/22 08:56, Vasiliy Ulyanov wrote:
> Access to /proc/[pid]/exe may be restricted in certain environments (e.g.
> in containers) and any attempt to stat(2) or readlink(2) the file will
> result in 'permission denied' error if the calling process does not have
> CAP_SYS_PTRACE capability. According to proc(5) manpage:
> 
> Permission to dereference or read (readlink(2)) this symbolic link is
> governed by a ptrace access mode PTRACE_MODE_READ_FSCREDS check; see
> ptrace(2).
> 
> If the first call to virPidFileReadPathIfAlive fails with EACCES try to
> call it one more time without specifyng swtpm binary path in order to
> avoid dereferencing the symlink.
> 
> Signed-off-by: Vasiliy Ulyanov 
> ---
>  src/qemu/qemu_tpm.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> index 7e7b01768e..9c80e15e9b 100644
> --- a/src/qemu/qemu_tpm.c
> +++ b/src/qemu/qemu_tpm.c
> @@ -261,10 +261,17 @@ qemuTPMEmulatorGetPid(const char *swtpmStateDir,
>  g_autofree char *swtpm = virTPMGetSwtpm();
>  g_autofree char *pidfile = 
> qemuTPMEmulatorCreatePidFilename(swtpmStateDir,
>  shortName);
> +int rc;
> +
>  if (!pidfile)
>  return -1;
>  
> -if (virPidFileReadPathIfAlive(pidfile, pid, swtpm) < 0)
> +rc = virPidFileReadPathIfAlive(pidfile, pid, swtpm);
> +/* If access to /proc/[pid]/exe is restricted then skip the validation of
> + * swtpm binary. */
> +if (rc < 0 && virLastErrorIsSystemErrno(EACCES))
> +rc = virPidFileReadPathIfAlive(pidfile, pid, NULL);
> +if (rc < 0)
>  return -1;
>  
>  return 0;

Firstly, this fixes the problem only for swtpm case, but there is one
other place where the same problem can happen (qemuVhostUserGPUGetPid()).

>From conceptual POV, what may now happen is that when PID wrapped we may
wrongly detect another process as swtpm.

I'm wondering whether we can ditch the @binPath argument of
virPidFileReadPathIfAlive() completely. A pid file should be locked by
the process owning it [1], therefore what we could do is read given file
and try to lock it. If we succeeded it means that the process is dead
and we read a stale PID. If locking fails then the process is still
running and the PID we read is valid.

1: This is how libvirt treats pidfiles (see virPidFileAcquirePath()).
Question is whether other tools which use their own pidfiles do the
same. For instance, swtpm is given '--pid file=/some/path' argument but
I haven't checked whether it behaves as expected. Also, to make things
worse there are at least two types of file locks on Linux and both are
independent of each other, so question then is what type of lock does
given tool use.

But this could all be mitigated by using virCommandSetPidFile() when
spawning the command instead of passing arbitrary --pid arguments.


Michal

Re: [PATCH 2/3] virdnsmasq: Lookup DNSMASQ in PATH

[Daniel P . Berrangé]

On Tue, Jan 11, 2022 at 09:28:45AM +0100, Michal Prívozník wrote:
> On 1/10/22 18:41, Andrea Bolognani wrote:
> > On Mon, Jan 10, 2022 at 04:44:55PM +0100, Michal Privoznik wrote:
> >> While it's true that our virCommand subsystem is happy with
> >> non-absolute paths, the dnsmasq capability code is not. For
> >> instance, it does call stat() over the binary to learn its mtime
> >> (and thus decide whether capabilities need to be fetched again or
> >> not).
> >>
> >> Therefore, when constructing the capabilities structure look up
> >> the binary path. If DNSMASQ already contains an absolute path
> >> then it is returned (and virFindFileInPath() is a NOP).
> > 
> > Saying that virFindFileInPath() is a NOP is not quite correct: if you
> > pass an absolute path, it will return a copy. So I'd rewrite the
> > above as
> > 
> >   If DNSMASQ already contains an absolute path then
> >   virFindFileInPath() will simply return a copy.
> > 
> 
> Yes, this makes sense.
> 
> > Reviewed-by: Andrea Bolognani 
> > 
> 
> Thanks, but as I was looking through virFindFileInPath() I've noticed
> that if the file exists but is not executable then NULL is returned, so
> we will need a fallback case. Effectively this needs to be squashed in:

Why do we need a fallback ?  If someone has put 'dnsmasq' in $PATH
without the execute bit set, surely that's just a broken deployment
and returning NULL is correct.

> diff --git c/src/util/virdnsmasq.c w/src/util/virdnsmasq.c
> index b6ccb9d0a4..7792a65bc3 100644
> --- c/src/util/virdnsmasq.c
> +++ w/src/util/virdnsmasq.c
> @@ -703,12 +703,17 @@ static dnsmasqCaps *
>  dnsmasqCapsNewEmpty(void)
>  {
>  dnsmasqCaps *caps;
> +g_autofree char *binaryPath = NULL;
> 
>  if (dnsmasqCapsInitialize() < 0)
>  return NULL;
>  if (!(caps = virObjectNew(dnsmasqCapsClass)))
>  return NULL;
> -caps->binaryPath = virFindFileInPath(DNSMASQ);
> +
> +if (!(binaryPath = virFindFileInPath(DNSMASQ)))
> +binaryPath = g_strdup(DNSMASQ);
> +
> +caps->binaryPath = g_steal_pointer();
>  return caps;
>  }
> 
> I'll post v2 of this patch shortly.
> 
> Michal
> 

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 v2 1/3] scripts: Check spelling

[Andrea Bolognani]

On Mon, Jan 10, 2022 at 03:58:55PM -0700, Jim Fehlig wrote:
> On 1/10/22 11:21, Andrea Bolognani wrote:
> > On Mon, Jan 10, 2022 at 04:41:25PM +0100, Tim Wiederhake wrote:
> > > +("/src/security/apparmor/libvirt-lxc", "devic"),
> >
> > Looking at the context where this appears:
> >
> >deny /sys/d[^e]*{,/**} wklx,
> >deny /sys/de[^v]*{,/**} wklx,
> >deny /sys/dev[^i]*{,/**} wklx,
> >deny /sys/devi[^c]*{,/**} wklx,
> >deny /sys/devic[^e]*{,/**} wklx,
> >deny /sys/device[^s]*{,/**} wklx,
> >deny /sys/devices/[^v]*{,/**} wklx,
> >deny /sys/devices/v[^i]*{,/**} wklx,
> >deny /sys/devices/vi[^r]*{,/**} wklx,
> >deny /sys/devices/vir[^t]*{,/**} wklx,
> >deny /sys/devices/virt[^u]*{,/**} wklx,
> >deny /sys/devices/virtu[^a]*{,/**} wklx,
> >deny /sys/devices/virtua[^l]*{,/**} wklx,
> >deny /sys/devices/virtual/[^n]*{,/**} wklx,
> >deny /sys/devices/virtual/n[^e]*{,/**} wklx,
> >deny /sys/devices/virtual/ne[^t]*{,/**} wklx,
> >deny /sys/devices/virtual/net?*{,/**} wklx,
> >deny /sys/devices/virtual?*{,/**} wklx,
> >deny /sys/devices?*{,/**} wklx,
> >
> > I mean, I don't speak AppArmor but this can't be right, can it? :D
>
> It's valid apparmor. At least the apparmor parser doesn't complain :-). ISTM
> the last rule should cover the others.

I was not really suggesting that it was not a valid configuration,
it's just that looking at it immediately triggered a "that can't be
the best way to do it" reaction in me ;)

> > Jim, do you think we actually need such a slippery slope of deny
> > rules, or can we simplify things a bit?
>
> I don't know why all of these deny rules are defined in this manner.
> /sys/class, /proc/sys/kernel, and others are defined similarly. They were
> added by Cedric in commit 9265f8ab67d. Cedric, do you recall the purpose of
> defining the rules in this way?

The script that generated those rules is

  
https://github.com/lxc/lxc/blob/master/config/apparmor/lxc-generate-aa-rules.py

and that's apparently its intended behavior. So there has to be a
reason why it's done this way, right? I just have no idea what it
could possibly be.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH v2] report error when virProcessGetStatInfo() is unable to parse data

[Peter Krempa]

On Tue, Jan 11, 2022 at 15:16:36 +0530, Ani Sinha wrote:
> 
> 
> On Tue, 11 Jan 2022, Michal Prívozník wrote:
> 
> > On 1/11/22 04:42, Ani Sinha wrote:
> > > Currently virProcessGetStatInfo() always returns success and only logs 
> > > error
> > > when it is unable to parse the data. Make this function actually report 
> > > the
> > > error and return a negative value in this error scenario.
> > >
> > > Fix the callers so that they do not override the error generated.
> > >
> > > Signed-off-by: Ani Sinha 
> > > ---
> > >  src/ch/ch_driver.c | 2 --
> > >  src/qemu/qemu_driver.c | 7 +--
> > >  src/util/virprocess.c  | 6 +-
> > >  3 files changed, 6 insertions(+), 9 deletions(-)
> > >
> > > changelog:
> > > v2: fixed the callers
> > >
> >
> >
> > > diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> > > index c74bd16fe6..b9f498d5d8 100644
> > > --- a/src/util/virprocess.c
> > > +++ b/src/util/virprocess.c
> > > @@ -1783,7 +1783,11 @@ virProcessGetStatInfo(unsigned long long *cpuTime,
> > >  virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_STIME], NULL, 10, 
> > > ) < 0 ||
> > >  virStrToLong_l(proc_stat[VIR_PROCESS_STAT_RSS], NULL, 10, ) 
> > > < 0 ||
> > >  virStrToLong_i(proc_stat[VIR_PROCESS_STAT_PROCESSOR], NULL, 10, 
> > > ) < 0) {
> > > -VIR_WARN("cannot parse process status data");
> > > +virReportError(VIR_ERR_INTERNAL_ERROR,
> > > +   _("cannot parse process status data for pid 
> > > '%d/%d'"),
> > > +   (int) pid, (int) tid);
> > > +
> > > +return -1;
> > >  }
> > >
> > >  /* We got jiffies
> >
> > There's an alternative implementation for non-Linux platforms, but you
> 
> but that has not been pushed yet I believe. Correct me if I am mistaken.

It was pushed already as it was fixing the build, which had higher
priority than any cosmetic issues which can be fixed later:

commit d7c64453aa0e9e0718e9292d7269ea3fe34f7c4c
Author: Michal Prívozník 
Date:   Thu Jan 6 20:13:08 2022 +0100

virprocess: Provide non-Linux stubs for virProcessGet{Stat,Sched}Info

Both virProcessGetStatInfo() and virProcessGetSchedInfo() are
Linux centric. Provide stubs for non-Linux platforms.

Fixes: d73852c49962fbfe652fc7bec595a3f242faf847
Signed-off-by: Michal Privoznik 
Reviewed-by: Peter Krempa 
Reviewed-by: Ján Tomko 

Re: [PATCH v6 3/4] remove sysconfig files

[Daniel P . Berrangé]

On Tue, Jan 11, 2022 at 09:51:05AM +, Andrea Bolognani wrote:
> On Tue, Jan 11, 2022 at 10:21:32AM +0100, Olaf Hering wrote:
> > Mon, 3 Jan 2022 03:18:11 -0800 Andrea Bolognani :
> >
> > > The fact that we still QEMU_AUDIO_DRV and SDL_AUDIODRIVER in the
> > > service file even after your changes goes against this principle.
> >
> > So what should be done about these?
> >
> > I think whoever added support for these environment variables a couple
> > of decades ago failed to provide proper documentation. Or perhaps
> > whoever wrote the "sound" section of formatdomain.html.in failed to
> > recognize the existence of these environment variables.
> 
> Dan, do you think it would be okay to simply drop these from the
> service/sysconfig file? IIUC we have a proper way to set them on a
> per-domain level now.

I don't think we need todo anything differentl - Olaf's patches
are fine in this respect already IMHO.  Certainly we don't need
to add more documentation about these than already exists as we
don't need to encourage more usage.

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

[PATCH 3/3] qemuSnapshotCreate: Don't insert snapshot into list with VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA

[Peter Krempa]

Our approach to snapshots without metadata was to insert them to the
snapshot list and then later remove them from the list when the flag is
present.

This quirky logic was broken in a recent refactor of the snapshot code
causing that the snapshot stayed inserted in the snapshot list.

Recent refactor of the snapshot code didn't faithfully relocate this
logic to the new function.

Rather than attempting to restore the quirky logic of adding and then
removing the object, don't add the snapshot into the list at all when
the user doesn't want metadata.

We achieve this by creating a temporary 'virDomainMomentObj' wrapper
which is not inserted into the list and using that instead of calling
virDomainSnapshotAssignDef.

Fixes: 9bad0fb809b
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2039131
Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_snapshot.c | 25 +++--
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 9a5d3e60aa..e9fc9051c1 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1756,7 +1756,7 @@ qemuSnapshotCreate(virDomainObj *vm,
virQEMUDriverConfig *cfg,
unsigned int flags)
 {
-
+g_autoptr(virDomainMomentObj) noMetadataSnap = NULL;
 virDomainMomentObj *snap = NULL;
 virDomainMomentObj *current = NULL;
 virDomainSnapshotPtr ret = NULL;
@@ -1767,16 +1767,20 @@ qemuSnapshotCreate(virDomainObj *vm,
 if (qemuSnapshotPrepare(vm, def, ) < 0)
 return NULL;

-if (!(snap = virDomainSnapshotAssignDef(vm->snapshots, def)))
-return NULL;
-
-virObjectRef(def);
+if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA) {
+snap = noMetadataSnap = virDomainMomentObjNew();
+snap->def = >parent;
+} else {
+if (!(snap = virDomainSnapshotAssignDef(vm->snapshots, def)))
+return NULL;

-current = virDomainSnapshotGetCurrent(vm->snapshots);
-if (current) {
-snap->def->parent_name = g_strdup(current->def->name);
+if ((current = virDomainSnapshotGetCurrent(vm->snapshots))) {
+snap->def->parent_name = g_strdup(current->def->name);
+}
 }

+virObjectRef(def);
+
 /* actually do the snapshot */
 if (virDomainObjIsActive(vm)) {
 if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY ||
@@ -1804,7 +1808,7 @@ qemuSnapshotCreate(virDomainObj *vm,
 }
 }

-if (!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA)) {
+if (!noMetadataSnap) {
 qemuSnapshotSetCurrent(vm, snap);

 if (qemuSnapshotCreateWriteMetadata(vm, snap, driver, cfg) < 0)
@@ -1818,7 +1822,8 @@ qemuSnapshotCreate(virDomainObj *vm,
 return ret;

  error:
-virDomainSnapshotObjListRemove(vm->snapshots, snap);
+if (!noMetadataSnap)
+virDomainSnapshotObjListRemove(vm->snapshots, snap);
 return NULL;
 }

-- 
2.31.1

[PATCH 2/3] conf: moment: Export helpers to create the virDomainMoment wrapper

[Peter Krempa]

Export 'virDomainMomentObjNew' and 'virDomainMomentObjFree' and define
the latter as autoptr cleanup function for 'virDomainMomentObj'.

Signed-off-by: Peter Krempa 
---
 src/conf/virdomainmomentobjlist.c | 4 ++--
 src/conf/virdomainmomentobjlist.h | 8 
 src/libvirt_private.syms  | 2 ++
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/conf/virdomainmomentobjlist.c 
b/src/conf/virdomainmomentobjlist.c
index 60f7eec106..8993c2310b 100644
--- a/src/conf/virdomainmomentobjlist.c
+++ b/src/conf/virdomainmomentobjlist.c
@@ -205,7 +205,7 @@ virDomainMomentMoveChildren(virDomainMomentObj *from,
 }


-static virDomainMomentObj *
+virDomainMomentObj *
 virDomainMomentObjNew(void)
 {
 virDomainMomentObj *moment;
@@ -218,7 +218,7 @@ virDomainMomentObjNew(void)
 }


-static void
+void
 virDomainMomentObjFree(virDomainMomentObj *moment)
 {
 if (!moment)
diff --git a/src/conf/virdomainmomentobjlist.h 
b/src/conf/virdomainmomentobjlist.h
index e42f9a7e9e..d2ab3b46b1 100644
--- a/src/conf/virdomainmomentobjlist.h
+++ b/src/conf/virdomainmomentobjlist.h
@@ -50,6 +50,14 @@ struct _virDomainMomentObj {
 virDomainMomentObj *first_child; /* NULL if no children */
 };

+virDomainMomentObj *
+virDomainMomentObjNew(void);
+
+void
+virDomainMomentObjFree(virDomainMomentObj *moment);
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainMomentObj, virDomainMomentObjFree);
+
 int
 virDomainMomentForEachChild(virDomainMomentObj *moment,
 virHashIterator iter,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ee14b99d88..5b76e66e61 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1175,6 +1175,8 @@ virDomainMomentDropParent;
 virDomainMomentForEachChild;
 virDomainMomentForEachDescendant;
 virDomainMomentMoveChildren;
+virDomainMomentObjFree;
+virDomainMomentObjNew;


 # conf/virdomainobjlist.h
-- 
2.31.1

[PATCH for 8.0 0/3] Fix creation of snapshots without libvirt metadata

[Peter Krempa]

After a recent unreleased refactor we don't remove the temporary
snapshot object from the list when finishing a snapshot without metadata
thus we should fix it before releasing a buggy release.

This patchset fixes it by not adding it into the list in the first
place.

Peter Krempa (3):
  virdomainmomentobjlist.h: Convert to modern header style
  conf: moment: Export helpers to create the virDomainMoment wrapper
  qemuSnapshotCreate: Don't insert snapshot into list with
VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA

 src/conf/virdomainmomentobjlist.c |   4 +-
 src/conf/virdomainmomentobjlist.h | 121 +++---
 src/libvirt_private.syms  |   2 +
 src/qemu/qemu_snapshot.c  |  25 +++---
 4 files changed, 97 insertions(+), 55 deletions(-)

-- 
2.31.1

[PATCH 1/3] virdomainmomentobjlist.h: Convert to modern header style

[Peter Krempa]

Format the function prototypes the same way as in the .c file.

Signed-off-by: Peter Krempa 
---
 src/conf/virdomainmomentobjlist.h | 113 ++
 1 file changed, 70 insertions(+), 43 deletions(-)

diff --git a/src/conf/virdomainmomentobjlist.h 
b/src/conf/virdomainmomentobjlist.h
index dd8f798301..e42f9a7e9e 100644
--- a/src/conf/virdomainmomentobjlist.h
+++ b/src/conf/virdomainmomentobjlist.h
@@ -50,24 +50,33 @@ struct _virDomainMomentObj {
 virDomainMomentObj *first_child; /* NULL if no children */
 };

-int virDomainMomentForEachChild(virDomainMomentObj *moment,
-virHashIterator iter,
-void *data);
-int virDomainMomentForEachDescendant(virDomainMomentObj *moment,
- virHashIterator iter,
- void *data);
-void virDomainMomentDropParent(virDomainMomentObj *moment);
-void virDomainMomentDropChildren(virDomainMomentObj *moment);
-void virDomainMomentMoveChildren(virDomainMomentObj *from,
- virDomainMomentObj *to);
-void virDomainMomentLinkParent(virDomainMomentObjList *moments,
-   virDomainMomentObj *moment);
-
-virDomainMomentObjList *virDomainMomentObjListNew(void);
-void virDomainMomentObjListFree(virDomainMomentObjList *moments);
-
-virDomainMomentObj *virDomainMomentAssignDef(virDomainMomentObjList *moments,
-   virDomainMomentDef *def);
+int
+virDomainMomentForEachChild(virDomainMomentObj *moment,
+virHashIterator iter,
+void *data);
+int
+virDomainMomentForEachDescendant(virDomainMomentObj *moment,
+ virHashIterator iter,
+ void *data);
+void
+virDomainMomentDropParent(virDomainMomentObj *moment);
+void
+virDomainMomentDropChildren(virDomainMomentObj *moment);
+void
+virDomainMomentMoveChildren(virDomainMomentObj *from,
+virDomainMomentObj *to);
+void
+virDomainMomentLinkParent(virDomainMomentObjList *moments,
+  virDomainMomentObj *moment);
+
+virDomainMomentObjList *
+virDomainMomentObjListNew(void);
+void
+virDomainMomentObjListFree(virDomainMomentObjList *moments);
+
+virDomainMomentObj *
+virDomainMomentAssignDef(virDomainMomentObjList *moments,
+ virDomainMomentDef *def);

 /* Various enum bits that map to public API filters. Note that the
  * values of the internal bits are not the same as the public ones for
@@ -97,28 +106,46 @@ typedef enum {
 VIR_DOMAIN_MOMENT_FILTERS_METADATA | \
 VIR_DOMAIN_MOMENT_FILTERS_LEAVES)

-int virDomainMomentObjListGetNames(virDomainMomentObjList *moments,
-   virDomainMomentObj *from,
-   char **const names,
-   int maxnames,
-   unsigned int moment_flags,
-   virDomainMomentObjListFilter filter,
-   unsigned int filter_flags);
-virDomainMomentObj *virDomainMomentFindByName(virDomainMomentObjList *moments,
-const char *name);
-int virDomainMomentObjListSize(virDomainMomentObjList *moments);
-virDomainMomentObj *virDomainMomentGetCurrent(virDomainMomentObjList *moments);
-const char *virDomainMomentGetCurrentName(virDomainMomentObjList *moments);
-void virDomainMomentSetCurrent(virDomainMomentObjList *moments,
-   virDomainMomentObj *moment);
-bool virDomainMomentObjListRemove(virDomainMomentObjList *moments,
-  virDomainMomentObj *moment);
-void virDomainMomentObjListRemoveAll(virDomainMomentObjList *moments);
-int virDomainMomentForEach(virDomainMomentObjList *moments,
-   virHashIterator iter,
-   void *data);
-int virDomainMomentUpdateRelations(virDomainMomentObjList *moments);
-int virDomainMomentCheckCycles(virDomainMomentObjList *list,
-   virDomainMomentDef *def,
-   const char *domname);
-virDomainMomentObj *virDomainMomentFindLeaf(virDomainMomentObjList *list);
+int
+virDomainMomentObjListGetNames(virDomainMomentObjList *moments,
+   virDomainMomentObj *from,
+   char **const names,
+   int maxnames,
+   unsigned int moment_flags,
+   virDomainMomentObjListFilter filter,
+   unsigned int filter_flags);
+virDomainMomentObj *
+virDomainMomentFindByName(virDomainMomentObjList *moments,
+  const char *name);
+int
+virDomainMomentObjListSize(virDomainMomentObjList *moments);
+

Re: [PATCH v6 3/4] remove sysconfig files

[Andrea Bolognani]

On Tue, Jan 11, 2022 at 10:21:32AM +0100, Olaf Hering wrote:
> Mon, 3 Jan 2022 03:18:11 -0800 Andrea Bolognani :
>
> > The fact that we still QEMU_AUDIO_DRV and SDL_AUDIODRIVER in the
> > service file even after your changes goes against this principle.
>
> So what should be done about these?
>
> I think whoever added support for these environment variables a couple
> of decades ago failed to provide proper documentation. Or perhaps
> whoever wrote the "sound" section of formatdomain.html.in failed to
> recognize the existence of these environment variables.

Dan, do you think it would be okay to simply drop these from the
service/sysconfig file? IIUC we have a proper way to set them on a
per-domain level now.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH v2] report error when virProcessGetStatInfo() is unable to parse data

[Ani Sinha]

On Tue, 11 Jan 2022, Michal Prívozník wrote:

> On 1/11/22 04:42, Ani Sinha wrote:
> > Currently virProcessGetStatInfo() always returns success and only logs error
> > when it is unable to parse the data. Make this function actually report the
> > error and return a negative value in this error scenario.
> >
> > Fix the callers so that they do not override the error generated.
> >
> > Signed-off-by: Ani Sinha 
> > ---
> >  src/ch/ch_driver.c | 2 --
> >  src/qemu/qemu_driver.c | 7 +--
> >  src/util/virprocess.c  | 6 +-
> >  3 files changed, 6 insertions(+), 9 deletions(-)
> >
> > changelog:
> > v2: fixed the callers
> >
>
>
> > diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> > index c74bd16fe6..b9f498d5d8 100644
> > --- a/src/util/virprocess.c
> > +++ b/src/util/virprocess.c
> > @@ -1783,7 +1783,11 @@ virProcessGetStatInfo(unsigned long long *cpuTime,
> >  virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_STIME], NULL, 10, 
> > ) < 0 ||
> >  virStrToLong_l(proc_stat[VIR_PROCESS_STAT_RSS], NULL, 10, ) < 
> > 0 ||
> >  virStrToLong_i(proc_stat[VIR_PROCESS_STAT_PROCESSOR], NULL, 10, 
> > ) < 0) {
> > -VIR_WARN("cannot parse process status data");
> > +virReportError(VIR_ERR_INTERNAL_ERROR,
> > +   _("cannot parse process status data for pid 
> > '%d/%d'"),
> > +   (int) pid, (int) tid);
> > +
> > +return -1;
> >  }
> >
> >  /* We got jiffies
>
> There's an alternative implementation for non-Linux platforms, but you

but that has not been pushed yet I believe. Correct me if I am mistaken.

> are introducing error reporting only to this implementation. Both
> versions have to behave the same, i.e. the non-Linux implementation now
> has to report error too. Can you fix that in v3?

I think we should keep the scope of this patch as is. Why don't we do
this. Lets push this patch. Then I will fix the patch for non-linux
platforms and send it over.

Re: [PATCH 2/3] virdnsmasq: Lookup DNSMASQ in PATH

[Andrea Bolognani]

On Tue, Jan 11, 2022 at 09:28:45AM +0100, Michal Prívozník wrote:
> Thanks, but as I was looking through virFindFileInPath() I've noticed
> that if the file exists but is not executable then NULL is returned, so
> we will need a fallback case.

Right. But considering that dnsmasqCapsRefreshInternal() includes a
check for whether or not the file is executable, and an error is
returned if it's not, wouldn't it make more sense to remove that
check and simply exit early if virFindFileInPath() returns NULL?

diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
index a869b398c7..e31e78224d 100644
--- a/src/util/virdnsmasq.c
+++ b/src/util/virdnsmasq.c
@@ -670,16 +670,6 @@ dnsmasqCapsRefreshInternal(dnsmasqCaps *caps, bool force)
 return 0;
 caps->mtime = sb.st_mtime;

-/* Make sure the binary we are about to try exec'ing exists.
- * Technically we could catch the exec() failure, but that's
- * in a sub-process so it's hard to feed back a useful error.
- */
-if (!virFileIsExecutable(caps->binaryPath)) {
-virReportSystemError(errno, _("dnsmasq binary %s is not executable"),
- caps->binaryPath);
-return -1;
-}
-
 vercmd = virCommandNewArgList(caps->binaryPath, "--version", NULL);
 virCommandSetOutputBuffer(vercmd, );
 virCommandAddEnvPassCommon(vercmd);
@@ -708,7 +698,6 @@ dnsmasqCapsNewEmpty(void)
 return NULL;
 if (!(caps = virObjectNew(dnsmasqCapsClass)))
 return NULL;
-caps->binaryPath = virFindFileInPath(DNSMASQ);
 return caps;
 }

@@ -720,6 +709,8 @@ dnsmasqCapsNewFromBuffer(const char *buf)
 if (!caps)
 return NULL;

+caps->binaryPath = g_strdup(DNSMASQ);
+
 if (dnsmasqCapsSetFromBuffer(caps, buf) < 0) {
 virObjectUnref(caps);
 return NULL;
@@ -735,6 +726,11 @@ dnsmasqCapsNewFromBinary(void)
 if (!caps)
 return NULL;

+if (!(caps->binaryPath = virFindFileInPath(DNSMASQ))) {
+virObjectUnref(caps);
+return NULL;
+}
+
 if (dnsmasqCapsRefreshInternal(caps, true) < 0) {
 virObjectUnref(caps);
 return NULL;
-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH v2] report error when virProcessGetStatInfo() is unable to parse data

[Michal Prívozník]

On 1/11/22 04:42, Ani Sinha wrote:
> Currently virProcessGetStatInfo() always returns success and only logs error
> when it is unable to parse the data. Make this function actually report the
> error and return a negative value in this error scenario.
> 
> Fix the callers so that they do not override the error generated.
> 
> Signed-off-by: Ani Sinha 
> ---
>  src/ch/ch_driver.c | 2 --
>  src/qemu/qemu_driver.c | 7 +--
>  src/util/virprocess.c  | 6 +-
>  3 files changed, 6 insertions(+), 9 deletions(-)
> 
> changelog:
> v2: fixed the callers
> 


> diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> index c74bd16fe6..b9f498d5d8 100644
> --- a/src/util/virprocess.c
> +++ b/src/util/virprocess.c
> @@ -1783,7 +1783,11 @@ virProcessGetStatInfo(unsigned long long *cpuTime,
>  virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_STIME], NULL, 10, 
> ) < 0 ||
>  virStrToLong_l(proc_stat[VIR_PROCESS_STAT_RSS], NULL, 10, ) < 0 
> ||
>  virStrToLong_i(proc_stat[VIR_PROCESS_STAT_PROCESSOR], NULL, 10, 
> ) < 0) {
> -VIR_WARN("cannot parse process status data");
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("cannot parse process status data for pid '%d/%d'"),
> +   (int) pid, (int) tid);
> +
> +return -1;
>  }
>  
>  /* We got jiffies

There's an alternative implementation for non-Linux platforms, but you
are introducing error reporting only to this implementation. Both
versions have to behave the same, i.e. the non-Linux implementation now
has to report error too. Can you fix that in v3?

Michal

Re: [PATCH v6 3/4] remove sysconfig files

[Olaf Hering]

Mon, 3 Jan 2022 03:18:11 -0800 Andrea Bolognani :

> The fact that we still QEMU_AUDIO_DRV and SDL_AUDIODRIVER in the
> service file even after your changes goes against this principle.

So what should be done about these?

I think whoever added support for these environment variables a couple
of decades ago failed to provide proper documentation. Or perhaps
whoever wrote the "sound" section of formatdomain.html.in failed to
recognize the existence of these environment variables.

So in the end I will now sit down and write some blurb into the sound
section of formatdomain.rst, just to get this series off the table...


Olaf


pgpfll0vtYfbf.pgp
Description: Digitale Signatur von OpenPGP

[libvirt PATCH v5 19/20] news: Mention hvf domain type

[Andrea Bolognani]

From: Roman Bolshakov 

Signed-off-by: Roman Bolshakov 
Signed-off-by: Andrea Bolognani 
Tested-by: Brad Laue 
---
 NEWS.rst | 9 +
 1 file changed, 9 insertions(+)

diff --git a/NEWS.rst b/NEWS.rst
index b5893627e3..116d7c1251 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -53,6 +53,15 @@ v8.0.0 (unreleased)
 ``domsetlaunchsecstate`` are added to support injecting a launch secret
 in a domain's memory.
 
+  * qemu: Add hvf domain type for Hypervisor.framework
+
+QEMU introduced experimental support of Hypervisor.framework
+since 2.12.
+
+It's supported on machines with Intel VT-x feature set that includes
+Extended Page Tables (EPT) and Unrestricted Mode since macOS 10.10.
+
+
 * **Improvements**
 
   * libxl: Implement the virDomainGetMessages API
-- 
2.31.1

[libvirt PATCH v5 12/20] tests: Introduce testQemuHostOS

[Andrea Bolognani]

This new enumeration provides a way to specify the host OS
that a specific test case expects. The default is Linux, which
has been the implicit host OS until now; when Linux is selected
as the host OS, KVM support is advertised in capabilies data
exposed to test cases.

This commit doesn't result in any functional change, and simply
sets the stage for introducing macOS host OS support later.

Signed-off-by: Andrea Bolognani 
Tested-by: Brad Laue 
---
 tests/testutilsqemu.c | 86 +++
 tests/testutilsqemu.h |  4 ++
 2 files changed, 58 insertions(+), 32 deletions(-)

diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index cb665e501b..a27e290c6b 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -160,7 +160,8 @@ virHostCPUX86GetCPUID(uint32_t leaf,
 
 static int
 testQemuAddGuest(virCaps *caps,
- virArch arch)
+ virArch arch,
+ testQemuHostOS hostOS)
 {
 size_t nmachines;
 virCapsGuestMachine **machines = NULL;
@@ -193,16 +194,18 @@ testQemuAddGuest(virCaps *caps,
 virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_QEMU,
   NULL, NULL, 0, NULL);
 
-if (kvm_machines[emu_arch] != NULL) {
-nmachines = g_strv_length((char **)kvm_machines[emu_arch]);
-machines = virCapabilitiesAllocMachines(kvm_machines[emu_arch],
-nmachines);
-if (machines == NULL)
-goto error;
-
-virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_KVM,
-  qemu_emulators[emu_arch],
-  NULL, nmachines, machines);
+if (hostOS == HOST_OS_LINUX) {
+if (kvm_machines[emu_arch] != NULL) {
+nmachines = g_strv_length((char **)kvm_machines[emu_arch]);
+machines = virCapabilitiesAllocMachines(kvm_machines[emu_arch],
+nmachines);
+if (machines == NULL)
+goto error;
+
+virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_KVM,
+  qemu_emulators[emu_arch],
+  NULL, nmachines, machines);
+}
 }
 
 return 0;
@@ -213,7 +216,8 @@ testQemuAddGuest(virCaps *caps,
 }
 
 
-virCaps *testQemuCapsInit(void)
+static virCaps*
+testQemuCapsInitImpl(testQemuHostOS hostOS)
 {
 virCaps *caps;
 size_t i;
@@ -233,7 +237,7 @@ virCaps *testQemuCapsInit(void)
 goto cleanup;
 
 for (i = 0; i < VIR_ARCH_LAST; i++) {
-if (testQemuAddGuest(caps, i) < 0)
+if (testQemuAddGuest(caps, i, hostOS) < 0)
 goto cleanup;
 }
 
@@ -255,6 +259,12 @@ virCaps *testQemuCapsInit(void)
 return NULL;
 }
 
+virCaps*
+testQemuCapsInit(void)
+{
+return testQemuCapsInitImpl(HOST_OS_LINUX);
+}
+
 
 void
 qemuTestSetHostArch(virQEMUDriver *driver,
@@ -338,7 +348,8 @@ void qemuTestDriverFree(virQEMUDriver *driver)
 
 static void
 qemuTestCapsPopulateFakeMachines(virQEMUCaps *caps,
- virArch arch)
+ virArch arch,
+ testQemuHostOS hostOS)
 {
 size_t i;
 const char *defaultRAMid = NULL;
@@ -366,20 +377,22 @@ qemuTestCapsPopulateFakeMachines(virQEMUCaps *caps,
 virQEMUCapsSet(caps, QEMU_CAPS_TCG);
 }
 
-if (kvm_machines[arch] != NULL) {
-for (i = 0; kvm_machines[arch][i] != NULL; i++) {
-virQEMUCapsAddMachine(caps,
-  VIR_DOMAIN_VIRT_KVM,
-  kvm_machines[arch][i],
-  NULL,
-  NULL,
-  0,
-  false,
-  false,
-  true,
-  defaultRAMid,
-  false);
-virQEMUCapsSet(caps, QEMU_CAPS_KVM);
+if (hostOS == HOST_OS_LINUX) {
+if (kvm_machines[arch] != NULL) {
+for (i = 0; kvm_machines[arch][i] != NULL; i++) {
+virQEMUCapsAddMachine(caps,
+  VIR_DOMAIN_VIRT_KVM,
+  kvm_machines[arch][i],
+  NULL,
+  NULL,
+  0,
+  false,
+  false,
+  true,
+  defaultRAMid,
+  false);
+virQEMUCapsSet(caps, QEMU_CAPS_KVM);
+}
 }
 }
 }
@@ -399,8 +412,10 @@ qemuTestCapsCacheInsertData(virFileCache *cache,
 }
 
 
-int qemuTestCapsCacheInsert(virFileCache 

[libvirt PATCH v5 18/20] docs: Add support page for libvirt on macOS

[Andrea Bolognani]

From: Roman Bolshakov 

Signed-off-by: Roman Bolshakov 
Signed-off-by: Andrea Bolognani 
Tested-by: Brad Laue 
---
 docs/docs.html.in  |  3 +++
 docs/index.html.in |  3 ++-
 docs/macos.rst | 44 
 docs/meson.build   |  1 +
 4 files changed, 50 insertions(+), 1 deletion(-)
 create mode 100644 docs/macos.rst

diff --git a/docs/docs.html.in b/docs/docs.html.in
index 8132090762..225827b693 100644
--- a/docs/docs.html.in
+++ b/docs/docs.html.in
@@ -16,6 +16,9 @@
 Windows
 Downloads for Windows
 
+macOS
+Working with libvirt on macOS
+
 Migration
 Migrating guests between machines
 
diff --git a/docs/index.html.in b/docs/index.html.in
index 2c4aa7c6d0..3c065badb7 100644
--- a/docs/index.html.in
+++ b/docs/index.html.in
@@ -28,7 +28,8 @@
   LXC,
   BHyve and
   more
-targets Linux, FreeBSD, Windows and 
macOS
+targets Linux, FreeBSD, Windows and
+  macOS
 is used by many applications
   
   Recent / forthcoming release changes
diff --git a/docs/macos.rst b/docs/macos.rst
new file mode 100644
index 00..84ff8192b5
--- /dev/null
+++ b/docs/macos.rst
@@ -0,0 +1,44 @@
+.. role:: since
+
+=
+macOS support
+=
+
+.. contents::
+
+Libvirt works both as client (for most drivers) and server (for the
+`QEMU driver `__) on macOS.
+
+:since:`Since 8.1.0`, the "hvf" domain type can be used to run
+hardware-accelerated VMs on macOS via
+`Hypervisor.framework 
`__.
+QEMU version 2.12 or newer is needed for this to work.
+
+
+Installation
+
+
+libvirt client (virsh), server (libvirtd) and development headers can be
+installed from `Homebrew `__:
+
+::
+
+   brew install libvirt
+
+
+Running libvirtd locally
+
+
+The server can be started manually:
+
+::
+
+   $ libvirtd
+
+or on system boot:
+
+::
+
+   $ brew services start libvirt
+
+Once started, you can use virsh as you would on Linux.
diff --git a/docs/meson.build b/docs/meson.build
index 3e912f21ad..717d3c025c 100644
--- a/docs/meson.build
+++ b/docs/meson.build
@@ -106,6 +106,7 @@ docs_rst_files = [
   'hacking',
   'libvirt-go',
   'libvirt-go-xml',
+  'macos',
   'migration',
   'newreposetup',
   'pci-addresses',
-- 
2.31.1

[libvirt PATCH v5 06/20] fixup! qemu: Fix / improve virQEMUCapsProbeHVF()

[Andrea Bolognani]

Fix the architecture check so that hardware acceleration is only
ever advertised when the guest and host architecture match; exit
early if possible instead of calling sysctl every single time;
add some comments.

Signed-off-by: Andrea Bolognani 
Tested-by: Brad Laue 
---
 src/qemu/qemu_capabilities.c | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 9a0525222e..79e9b19236 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3202,17 +3202,27 @@ virQEMUCapsProbeQMPKVMState(virQEMUCaps *qemuCaps,
 static int
 virQEMUCapsProbeHVF(virQEMUCaps *qemuCaps)
 {
-int hv_support;
+int hv_support = 0;
 size_t len = sizeof(hv_support);
+virArch hostArch = virArchFromHost();
 
+/* Guest and host arch need to match for hardware acceleration
+ * to be usable */
+if (qemuCaps->arch != hostArch)
+return 0;
+
+/* We don't have a nice way to probe whether the QEMU binary
+ * contains HVF support, but we know that versions older than
+ * QEMU 2.12 didn't have the feature at all */
+if (qemuCaps->version < 2012000)
+return 0;
+
+/* We need the OS to report Hypervisor.framework availability */
 if (sysctlbyname("kern.hv_support", _support, , NULL, 0) < 0)
-hv_support = 0;
+return 0;
 
-if (qemuCaps->version >= 2012000 &&
-ARCH_IS_X86(qemuCaps->arch) &&
-hv_support) {
+if (hv_support)
 virQEMUCapsSet(qemuCaps, QEMU_CAPS_HVF);
-}
 
 return 0;
 }
-- 
2.31.1

[libvirt PATCH v5 20/20] fixup! NEWS: Mention Apple Silicon support for HVF

[Andrea Bolognani]

And simplify the entry by removing mentions of QEMU 2.12 and
macOS 10.10, which are both ancient at this point, and of
specific hardware features that are going to be present in any
recent Apple machine.

Signed-off-by: Andrea Bolognani 
Tested-by: Brad Laue 
---
 NEWS.rst | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/NEWS.rst b/NEWS.rst
index 116d7c1251..efecb87a07 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -55,11 +55,8 @@ v8.0.0 (unreleased)
 
   * qemu: Add hvf domain type for Hypervisor.framework
 
-QEMU introduced experimental support of Hypervisor.framework
-since 2.12.
-
-It's supported on machines with Intel VT-x feature set that includes
-Extended Page Tables (EPT) and Unrestricted Mode since macOS 10.10.
+It works on Intel machines as well as recent machines powered by Apple
+Silicon. QEMU 6.2.0 is needed for Apple Silicon support.
 
 
 * **Improvements**
-- 
2.31.1

[libvirt PATCH v5 17/20] docs: Note hvf support for domain elements

[Andrea Bolognani]

From: Roman Bolshakov 

Many domain elements have "QEMU and KVM only" or "QEMU/KVM since x.y.z"
remarks. Most of the elements work for HVF domain, so it makes sense to
add respective notices for HVF domain.

All the elements have been manually tested.

Signed-off-by: Roman Bolshakov 
Signed-off-by: Andrea Bolognani 
Tested-by: Brad Laue 
---
 docs/formatdomain.rst | 21 -
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 3e9de05249..5042115fac 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -21,7 +21,7 @@ Element and attribute overview
 The root element required for all virtual machines is named ``domain``. It has
 two attributes, the ``type`` specifies the hypervisor used for running the
 domain. The allowed values are driver specific, but include "xen", "kvm",
-"hvf" (:since:`since 8.0.0 and QEMU 2.12`), "qemu"
+"hvf" (:since:`since 8.1.0 and QEMU 2.12`), "qemu"
 and "lxc". The second attribute is ``id`` which is a unique integer identifier
 for the running guest machine. Inactive machines have no id value.
 
@@ -1426,7 +1426,8 @@ In case no restrictions need to be put on CPU model and 
its features, a simpler
   :since:`Since 7.1.0` with the QEMU driver.
 
Both ``host-model`` and ``host-passthrough`` modes make sense when a domain
-   can run directly on the host CPUs (for example, domains with type ``kvm``).
+   can run directly on the host CPUs (for example, domains with type ``kvm``
+   or ``hvf``).
The actual host CPU is irrelevant for domains with emulated virtual CPUs
(such as domains with type ``qemu``). However, for backward compatibility
``host-model`` may be implemented even for domains running on emulated CPUs
@@ -1750,7 +1751,7 @@ Each of these states allow for the same four possible 
actions.
The domain will be terminated and then restarted with a new name. (Only
supported by the libxl hypervisor driver.)
 
-QEMU/KVM supports the ``on_poweroff`` and ``on_reboot`` events handling the
+QEMU/KVM/HVF supports the ``on_poweroff`` and ``on_reboot`` events handling the
 ``destroy`` and ``restart`` actions, but the combination of ``on_poweroff`` set
 to ``restart`` and ``on_reboot`` set to ``destroy`` is forbidden.
 
@@ -1885,8 +1886,8 @@ are:
Physical address extension mode allows 32-bit guests to address more than 4
GB of memory.
 ``acpi``
-   ACPI is useful for power management, for example, with KVM guests it is
-   required for graceful shutdown to work.
+   ACPI is useful for power management, for example, with KVM or HVF guests it
+   is required for graceful shutdown to work.
 ``apic``
APIC allows the use of programmable IRQ management. :since:`Since 0.10.2
(QEMU only)` there is an optional attribute ``eoi`` with values ``on`` and
@@ -6195,14 +6196,16 @@ A video device.
 
You can provide the amount of video memory in kibibytes (blocks of 1024
bytes) using ``vram``. This is supported only for guest type of "vz", 
"qemu",
-   "vbox", "vmx" and "xen". If no value is provided the default is used. If the
+   "kvm", "hvf", "vbox", "vmx" and "xen".
+   If no value is provided the default is used. If the
size is not a power of two it will be rounded to closest one.
 
The number of screen can be set using ``heads``. This is supported only for
-   guests type of "vz", "kvm", "vbox" and "vmx".
+   guests type of "vz", "kvm", "hvf", "vbox" and "vmx".
 
-   For guest type of "kvm" or "qemu" and model type "qxl" there are optional
-   attributes. Attribute ``ram`` ( :since:`since 1.0.2` ) specifies the size of
+   For guest type of "kvm", "hvf" or "qemu" and model type "qxl" there are
+   optional attributes.
+   Attribute ``ram`` ( :since:`since 1.0.2` ) specifies the size of
the primary bar, while the attribute ``vram`` specifies the secondary bar
size. If ``ram`` or ``vram`` are not supplied a default value is used. The
``ram`` should also be rounded to power of two as ``vram``. There is also
-- 
2.31.1

[libvirt PATCH v5 13/20] tests: Add macOS support to testutilsqemu

[Andrea Bolognani]

This exposes a couple of macOS-specific variants of existing
APIs, which can be used when implementing test programs and
result in HVF support being advertised.

Signed-off-by: Andrea Bolognani 
Tested-by: Brad Laue 
---
 tests/testutilsqemu.c | 58 +++
 tests/testutilsqemu.h |  4 +++
 2 files changed, 62 insertions(+)

diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index a27e290c6b..e48e449f02 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -97,6 +97,18 @@ static const char *const *kvm_machines[VIR_ARCH_LAST] = {
 [VIR_ARCH_S390X] = s390x_machines,
 };
 
+static const char *const *hvf_machines[VIR_ARCH_LAST] = {
+[VIR_ARCH_I686] = NULL,
+[VIR_ARCH_X86_64] = x86_64_machines,
+[VIR_ARCH_AARCH64] = aarch64_machines,
+[VIR_ARCH_ARMV7L] = NULL,
+[VIR_ARCH_PPC64] = NULL,
+[VIR_ARCH_PPC] = NULL,
+[VIR_ARCH_RISCV32] = NULL,
+[VIR_ARCH_RISCV64] = NULL,
+[VIR_ARCH_S390X] = NULL,
+};
+
 static const char *qemu_default_ram_id[VIR_ARCH_LAST] = {
 [VIR_ARCH_I686] = "pc.ram",
 [VIR_ARCH_X86_64] = "pc.ram",
@@ -208,6 +220,20 @@ testQemuAddGuest(virCaps *caps,
 }
 }
 
+if (hostOS == HOST_OS_MACOS) {
+if (hvf_machines[emu_arch] != NULL) {
+nmachines = g_strv_length((char **)hvf_machines[emu_arch]);
+machines = virCapabilitiesAllocMachines(hvf_machines[emu_arch],
+nmachines);
+if (machines == NULL)
+goto error;
+
+virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_HVF,
+  qemu_emulators[emu_arch],
+  NULL, nmachines, machines);
+}
+}
+
 return 0;
 
  error:
@@ -265,6 +291,12 @@ testQemuCapsInit(void)
 return testQemuCapsInitImpl(HOST_OS_LINUX);
 }
 
+virCaps*
+testQemuCapsInitMacOS(void)
+{
+return testQemuCapsInitImpl(HOST_OS_MACOS);
+}
+
 
 void
 qemuTestSetHostArch(virQEMUDriver *driver,
@@ -395,6 +427,25 @@ qemuTestCapsPopulateFakeMachines(virQEMUCaps *caps,
 }
 }
 }
+
+if (hostOS == HOST_OS_MACOS) {
+if (hvf_machines[arch] != NULL) {
+for (i = 0; hvf_machines[arch][i] != NULL; i++) {
+virQEMUCapsAddMachine(caps,
+VIR_DOMAIN_VIRT_HVF,
+hvf_machines[arch][i],
+NULL,
+NULL,
+0,
+false,
+false,
+true,
+defaultRAMid,
+false);
+virQEMUCapsSet(caps, QEMU_CAPS_HVF);
+}
+}
+}
 }
 
 
@@ -492,6 +543,13 @@ qemuTestCapsCacheInsert(virFileCache *cache,
 return qemuTestCapsCacheInsertImpl(cache, caps, HOST_OS_LINUX);
 }
 
+int
+qemuTestCapsCacheInsertMacOS(virFileCache *cache,
+ virQEMUCaps *caps)
+{
+return qemuTestCapsCacheInsertImpl(cache, caps, HOST_OS_MACOS);
+}
+
 
 # define STATEDIRTEMPLATE abs_builddir "/qemustatedir-XX"
 # define CONFIGDIRTEMPLATE abs_builddir "/qemuconfigdir-XX"
diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h
index a8de6eb52b..a9202d2ae6 100644
--- a/tests/testutilsqemu.h
+++ b/tests/testutilsqemu.h
@@ -35,6 +35,7 @@ enum {
 
 typedef enum {
 HOST_OS_LINUX = 0,
+HOST_OS_MACOS,
 } testQemuHostOS;
 
 typedef enum {
@@ -92,6 +93,7 @@ struct testQemuInfo {
 };
 
 virCaps *testQemuCapsInit(void);
+virCaps *testQemuCapsInitMacOS(void);
 virDomainXMLOption *testQemuXMLConfInit(void);
 
 
@@ -113,6 +115,8 @@ int qemuTestDriverInit(virQEMUDriver *driver);
 void qemuTestDriverFree(virQEMUDriver *driver);
 int qemuTestCapsCacheInsert(virFileCache *cache,
 virQEMUCaps *caps);
+int qemuTestCapsCacheInsertMacOS(virFileCache *cache,
+ virQEMUCaps *caps);
 
 int testQemuCapsSetGIC(virQEMUCaps *qemuCaps,
int gic);
-- 
2.31.1

[libvirt PATCH v5 16/20] docs: Add hvf on QEMU driver page

[Andrea Bolognani]

From: Roman Bolshakov 

It's worth to make the domain type a little bit more visible than a row
in news. An example of hvf domain is available on QEMU driver page.

While at it, mention Hypervisor.framework on index page.

Signed-off-by: Roman Bolshakov 
Signed-off-by: Andrea Bolognani 
Tested-by: Brad Laue 
---
 docs/drvqemu.rst   | 48 +++---
 docs/index.html.in |  1 +
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/docs/drvqemu.rst b/docs/drvqemu.rst
index e18075d865..9d7dd2656b 100644
--- a/docs/drvqemu.rst
+++ b/docs/drvqemu.rst
@@ -1,13 +1,18 @@
 .. role:: since
 .. role:: removed
 
-==
-KVM/QEMU hypervisor driver
-==
+==
+QEMU/KVM/HVF hypervisor driver
+==
 
 The libvirt KVM/QEMU driver can manage any QEMU emulator from version 2.11.0 or
 later.
 
+It supports multiple QEMU accelerators: software
+emulation also known as TCG, hardware-assisted virtualization on Linux
+with KVM and hardware-assisted virtualization on macOS with
+Hypervisor.framework (:since:`since 8.1.0`).
+
 .. contents::
 
 Project Links
@@ -15,6 +20,7 @@ Project Links
 
 -  The `KVM `__ Linux hypervisor
 -  The `QEMU `__ emulator
+-  
`Hypervisor.framework`__` 
reference
 
 Deployment pre-requisites
 -
@@ -27,6 +33,9 @@ Deployment pre-requisites
 -  **KVM hypervisor**: The driver will probe ``/usr/bin`` for the presence of
``qemu-kvm`` and ``/dev/kvm`` device node. If both are found, then KVM fully
virtualized, hardware accelerated guests will be available.
+-  **Hypervisor.framework (HVF)**: The driver will probe ``sysctl`` for the
+   presence of ``Hypervisor.framework``. If it is found and QEMU is newer than
+   2.12, then it will be possible to create hardware accelerated guests.
 
 Connections to QEMU driver
 --
@@ -634,3 +643,36 @@ KVM hardware accelerated guest on i686

  

+
+HVF hardware accelerated guest on x86_64
+
+
+::
+
+   
+ hvf-demo
+ 4dea24b3-1d52-d8f3-2516-782e98a23fa0
+ 131072
+ 1
+ 
+   hvm
+ 
+ 
+   
+ 
+ 
+ 
+   /usr/local/bin/qemu-system-x86_64
+   
+   
+ 
+ 
+ 
+   
+   
+ 
+ 
+   
+   
+ 
+   
diff --git a/docs/index.html.in b/docs/index.html.in
index bf164edb58..2c4aa7c6d0 100644
--- a/docs/index.html.in
+++ b/docs/index.html.in
@@ -21,6 +21,7 @@
 is accessible from C, Python, Perl, Go and more
 is licensed under open source licenses
 supports KVM,
+  Hypervisor.framework,
   QEMU, Xen,
   Virtuozzo,
   VMWare ESX,
-- 
2.31.1

[libvirt PATCH v5 14/20] tests: Add macOS support to qemuxml2*test

[Andrea Bolognani]

The new DO_TEST_MACOS() macro makes it possible to create test
cases that verify the behavior of libvirt on a macOS machine
with HVF support available.

Signed-off-by: Andrea Bolognani 
Tested-by: Brad Laue 
---
 tests/qemuxml2argvtest.c | 25 -
 tests/qemuxml2xmltest.c  | 27 ++-
 tests/testutilsqemu.c|  4 
 tests/testutilsqemu.h|  2 ++
 4 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index cc67d806e4..b10850dcce 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -39,6 +39,8 @@
 # define VIR_FROM_THIS VIR_FROM_QEMU
 
 static virQEMUDriver driver;
+static virCaps *linuxCaps;
+static virCaps *macOSCaps;
 
 static unsigned char *
 fakeSecretGetValue(virSecretPtr obj G_GNUC_UNUSED,
@@ -716,12 +718,18 @@ testCompareXMLToArgv(const void *data)
 g_autofree char *archstr = NULL;
 virArch arch = VIR_ARCH_NONE;
 g_autoptr(virIdentity) sysident = virIdentityGetSystem();
+int rc;
 
 memset(_chr, 0, sizeof(monitor_chr));
 
 if (testQemuInfoInitArgs((struct testQemuInfo *) info) < 0)
 goto cleanup;
 
+if (info->args.hostOS == HOST_OS_MACOS)
+driver.caps = macOSCaps;
+else
+driver.caps = linuxCaps;
+
 if (info->arch != VIR_ARCH_NONE && info->arch != VIR_ARCH_X86_64)
 qemuTestSetHostArch(, info->arch);
 
@@ -771,7 +779,11 @@ testCompareXMLToArgv(const void *data)
 goto cleanup;
 }
 
-if (qemuTestCapsCacheInsert(driver.qemuCapsCache, info->qemuCaps) < 0)
+if (info->args.hostOS == HOST_OS_MACOS)
+rc = qemuTestCapsCacheInsertMacOS(driver.qemuCapsCache, 
info->qemuCaps);
+else
+rc = qemuTestCapsCacheInsert(driver.qemuCapsCache, info->qemuCaps);
+if (rc < 0)
 goto cleanup;
 
 if (info->migrateFrom &&
@@ -934,6 +946,13 @@ mymain(void)
 if (qemuTestDriverInit() < 0)
 return EXIT_FAILURE;
 
+/* By default, the driver gets a virCaps instance that's suitable for
+ * tests that expect Linux as the host OS. We create another one for
+ * macOS and keep around pointers to both: this allows us to later
+ * pick the appropriate one for each test case */
+linuxCaps = driver.caps;
+macOSCaps = testQemuCapsInitMacOS();
+
 driver.privileged = true;
 
 VIR_FREE(driver.config->defaultTLSx509certdir);
@@ -1074,6 +1093,10 @@ mymain(void)
 DO_TEST_FULL(name, "", \
  ARG_GIC, gic, \
  ARG_QEMU_CAPS, __VA_ARGS__, QEMU_CAPS_LAST, ARG_END)
+# define DO_TEST_MACOS(name, ...) \
+DO_TEST_FULL(name, "", \
+ ARG_HOST_OS, HOST_OS_MACOS, \
+ ARG_QEMU_CAPS, __VA_ARGS__, QEMU_CAPS_LAST, ARG_END)
 
 # define DO_TEST_FAILURE(name, ...) \
 DO_TEST_FULL(name, "", \
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index fb438269b9..ca351a339a 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -21,6 +21,8 @@
 #define VIR_FROM_THIS VIR_FROM_NONE
 
 static virQEMUDriver driver;
+static virCaps *linuxCaps;
+static virCaps *macOSCaps;
 
 enum {
 WHEN_INACTIVE = 1,
@@ -32,13 +34,24 @@ enum {
 static int
 testXML2XMLCommon(const struct testQemuInfo *info)
 {
+int rc;
+
 if (testQemuInfoInitArgs((struct testQemuInfo *) info) < 0)
 return -1;
 
+if (info->args.hostOS == HOST_OS_MACOS)
+driver.caps = macOSCaps;
+else
+driver.caps = linuxCaps;
+
 if (!(info->flags & FLAG_REAL_CAPS))
 virQEMUCapsInitQMPBasicArch(info->qemuCaps);
 
-if (qemuTestCapsCacheInsert(driver.qemuCapsCache, info->qemuCaps) < 0)
+if (info->args.hostOS == HOST_OS_MACOS)
+rc = qemuTestCapsCacheInsertMacOS(driver.qemuCapsCache, 
info->qemuCaps);
+else
+rc = qemuTestCapsCacheInsert(driver.qemuCapsCache, info->qemuCaps);
+if (rc < 0)
 return -1;
 
 return 0;
@@ -143,6 +156,13 @@ mymain(void)
 if (qemuTestDriverInit() < 0)
 return EXIT_FAILURE;
 
+/* By default, the driver gets a virCaps instance that's suitable for
+ * tests that expect Linux as the host OS. We create another one for
+ * macOS and keep around pointers to both: this allows us to later
+ * pick the appropriate one for each test case */
+linuxCaps = driver.caps;
+macOSCaps = testQemuCapsInitMacOS();
+
 cfg = virQEMUDriverGetConfig();
 driver.privileged = true;
 
@@ -206,6 +226,11 @@ mymain(void)
 #define DO_TEST_NOCAPS(name) \
 DO_TEST_FULL(name, "", WHEN_BOTH, ARG_END)
 
+#define DO_TEST_MACOS(name, ...) \
+DO_TEST_FULL(name, "", WHEN_BOTH, \
+ ARG_HOST_OS, HOST_OS_MACOS, \
+ ARG_QEMU_CAPS, __VA_ARGS__, QEMU_CAPS_LAST, ARG_END)
+
 /* Unset or set all envvars here that are copied in qemudBuildCommandLine
  * using ADD_ENV_COPY, otherwise these tests may fail due to unexpected
  * values for these envvars */
diff --git 

[libvirt PATCH v5 15/20] tests: Add HVF test cases

[Andrea Bolognani]

We need to use a hardcoded list of capabilities because we don't
yet have proper replies files obtained from QEMU running on actual
macOS machines.

Signed-off-by: Andrea Bolognani 
Tested-by: Brad Laue 
---
 .../hvf-aarch64-virt-headless.args| 48 +
 .../hvf-aarch64-virt-headless.xml | 45 +
 .../hvf-x86_64-q35-headless.args  | 47 +
 .../hvf-x86_64-q35-headless.x86_64-latest.err |  1 +
 .../hvf-x86_64-q35-headless.xml   | 44 +
 tests/qemuxml2argvtest.c  | 18 
 .../hvf-aarch64-virt-headless.xml | 94 ++
 .../hvf-x86_64-q35-headless.xml   | 97 +++
 tests/qemuxml2xmltest.c   | 16 +++
 9 files changed, 410 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/hvf-aarch64-virt-headless.args
 create mode 100644 tests/qemuxml2argvdata/hvf-aarch64-virt-headless.xml
 create mode 100644 tests/qemuxml2argvdata/hvf-x86_64-q35-headless.args
 create mode 100644 
tests/qemuxml2argvdata/hvf-x86_64-q35-headless.x86_64-latest.err
 create mode 100644 tests/qemuxml2argvdata/hvf-x86_64-q35-headless.xml
 create mode 100644 tests/qemuxml2xmloutdata/hvf-aarch64-virt-headless.xml
 create mode 100644 tests/qemuxml2xmloutdata/hvf-x86_64-q35-headless.xml

diff --git a/tests/qemuxml2argvdata/hvf-aarch64-virt-headless.args 
b/tests/qemuxml2argvdata/hvf-aarch64-virt-headless.args
new file mode 100644
index 00..0f1eed66c2
--- /dev/null
+++ b/tests/qemuxml2argvdata/hvf-aarch64-virt-headless.args
@@ -0,0 +1,48 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-test \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-test/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-test/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-test/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-aarch64 \
+-name guest=test,debug-threads=on \
+-S \
+-object 
secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-test/master-key.aes \
+-machine virt,usb=off,dump-guest-core=off,gic-version=2 \
+-accel hvf \
+-drive 
file=/usr/share/edk2/aarch64/QEMU_EFI-silent-pflash.raw,if=pflash,format=raw,unit=0,readonly=on
 \
+-drive 
file=/var/lib/libvirt/qemu/nvram/test_VARS.fd,if=pflash,format=raw,unit=1 \
+-m 4096 \
+-realtime mlock=off \
+-smp 2,sockets=2,cores=1,threads=1 \
+-uuid 1b826c23-8767-47ad-a6b5-c83a88277f71 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev 
socket,id=charmonitor,path=/tmp/lib/domain--1-test/monitor.sock,server=on,wait=off
 \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc,driftfix=slew \
+-no-shutdown \
+-boot strict=on \
+-device 
pcie-root-port,port=8,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,addr=0x1 \
+-device pcie-root-port,port=9,chassis=2,id=pci.2,bus=pcie.0,addr=0x1.0x1 \
+-device pcie-root-port,port=10,chassis=3,id=pci.3,bus=pcie.0,addr=0x1.0x2 \
+-device pcie-root-port,port=11,chassis=4,id=pci.4,bus=pcie.0,addr=0x1.0x3 \
+-device pcie-root-port,port=12,chassis=5,id=pci.5,bus=pcie.0,addr=0x1.0x4 \
+-device pcie-root-port,port=13,chassis=6,id=pci.6,bus=pcie.0,addr=0x1.0x5 \
+-device virtio-serial-pci,id=virtio-serial0,bus=pci.2,addr=0x0 \
+-drive 
file=/var/lib/libvirt/images/test.qcow2,format=qcow2,if=none,id=drive-virtio-disk0
 \
+-device 
virtio-blk-pci,bus=pci.3,addr=0x0,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
 \
+-netdev user,id=hostnet0 \
+-device 
virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:9a:e6:c6,bus=pci.1,addr=0x0 
\
+-chardev pty,id=charserial0 \
+-serial chardev:charserial0 \
+-chardev 
socket,id=charchannel0,path=/tmp/channel/domain--1-test/org.qemu.guest_agent.0,server=on,wait=off
 \
+-device 
virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=org.qemu.guest_agent.0
 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.4,addr=0x0 \
+-object rng-random,id=objrng0,filename=/dev/urandom \
+-device virtio-rng-pci,rng=objrng0,id=rng0,bus=pci.5,addr=0x0 \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/hvf-aarch64-virt-headless.xml 
b/tests/qemuxml2argvdata/hvf-aarch64-virt-headless.xml
new file mode 100644
index 00..ef13820e17
--- /dev/null
+++ b/tests/qemuxml2argvdata/hvf-aarch64-virt-headless.xml
@@ -0,0 +1,45 @@
+
+  test
+  1b826c23-8767-47ad-a6b5-c83a88277f71
+  4194304
+  4194304
+  2
+  
+hvm
+/usr/share/edk2/aarch64/QEMU_EFI-silent-pflash.raw
+/var/lib/libvirt/qemu/nvram/test_VARS.fd
+
+  
+  
+
+  
+  
+
+
+
+  
+  destroy
+  restart
+  restart
+  
+/usr/bin/qemu-system-aarch64
+
+  
+  
+  
+
+
+
+  
+  
+
+
+
+  
+
+
+
+  /dev/urandom
+
+  
+
diff --git a/tests/qemuxml2argvdata/hvf-x86_64-q35-headless.args 
b/tests/qemuxml2argvdata/hvf-x86_64-q35-headless.args
new file mode 100644
index 00..b3358e3d59
--- /dev/null
+++ b/tests/qemuxml2argvdata/hvf-x86_64-q35-headless.args
@@ -0,0 +1,47 @@
+LC_ALL=C \

[libvirt PATCH v5 10/20] qemu: Introduce virQEMUCapsHaveAccel

[Andrea Bolognani]

From: Roman Bolshakov 

The function should be used to check if qemu capabilities include a
hardware acceleration, i.e. accel is not TCG.

Signed-off-by: Roman Bolshakov 
Signed-off-by: Andrea Bolognani 
Tested-by: Brad Laue 
Reviewed-by: Daniel P. Berrangé 
---
 src/qemu/qemu_capabilities.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index eaf2905f4b..ee4955056d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -809,6 +809,13 @@ virQEMUCapsTypeIsAccelerated(virDomainVirtType type)
 }
 
 
+static bool
+virQEMUCapsHaveAccel(virQEMUCaps *qemuCaps)
+{
+return virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM);
+}
+
+
 /* Checks whether a domain with @guest arch can run natively on @host.
  */
 bool
@@ -5029,7 +5036,7 @@ virQEMUCapsIsValid(void *data,
 return false;
 }
 
-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
+if (virQEMUCapsHaveAccel(qemuCaps)) {
 if (STRNEQ_NULLABLE(priv->hostCPUSignature, 
qemuCaps->hostCPUSignature)) {
 VIR_DEBUG("Outdated capabilities for '%s': host CPU changed "
   "('%s' vs '%s')",
@@ -5063,7 +5070,9 @@ virQEMUCapsIsValid(void *data,
   qemuCaps->binary);
 return false;
 }
+}
 
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
 kvmSupportsNesting = virQEMUCapsKVMSupportsNesting();
 if (kvmSupportsNesting != qemuCaps->kvmSupportsNesting) {
 VIR_DEBUG("Outdated capabilities for '%s': kvm kernel nested "
@@ -5528,7 +5537,7 @@ virQEMUCapsInitQMP(virQEMUCaps *qemuCaps,
  * for TCG capabilities by asking the same binary again and turning KVM
  * off.
  */
-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM) &&
+if (virQEMUCapsHaveAccel(qemuCaps) &&
 virQEMUCapsGet(qemuCaps, QEMU_CAPS_TCG) &&
 virQEMUCapsInitQMPSingle(qemuCaps, libDir, runUid, runGid, true) < 0)
 return -1;
@@ -5592,13 +5601,15 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
 virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_KVM);
 virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_QEMU);
 
-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
+if (virQEMUCapsHaveAccel(qemuCaps)) {
 qemuCaps->hostCPUSignature = g_strdup(hostCPUSignature);
 qemuCaps->microcodeVersion = microcodeVersion;
 qemuCaps->cpuData = virCPUDataNewCopy(cpuData);
 
 qemuCaps->kernelVersion = g_strdup(kernelVersion);
+}
 
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
 qemuCaps->kvmSupportsNesting = virQEMUCapsKVMSupportsNesting();
 
 qemuCaps->kvmSupportsSecureGuest = virQEMUCapsKVMSupportsSecureGuest();
-- 
2.31.1

[libvirt PATCH v5 11/20] qemu: Correct CPU capabilities probing for hvf

[Andrea Bolognani]

From: Roman Bolshakov 

With this change virsh domcapabilites shows:

  

https://gitlab.com/libvirt/libvirt/-/issues/147

Signed-off-by: Roman Bolshakov 
Signed-off-by: Andrea Bolognani 
Tested-by: Brad Laue 
---
 src/qemu/qemu_capabilities.c | 35 ---
 src/qemu/qemu_process.c  |  2 ++
 2 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index ee4955056d..aea238c342 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -744,6 +744,7 @@ struct _virQEMUCaps {
 
 /* Capabilities which may differ depending on the accelerator. */
 virQEMUCapsAccel kvm;
+virQEMUCapsAccel hvf;
 virQEMUCapsAccel tcg;
 };
 
@@ -805,14 +806,16 @@ virQEMUCapsAccelStr(virDomainVirtType type)
 static bool
 virQEMUCapsTypeIsAccelerated(virDomainVirtType type)
 {
-return type == VIR_DOMAIN_VIRT_KVM;
+return type == VIR_DOMAIN_VIRT_KVM ||
+   type == VIR_DOMAIN_VIRT_HVF;
 }
 
 
 static bool
 virQEMUCapsHaveAccel(virQEMUCaps *qemuCaps)
 {
-return virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM);
+return virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM) ||
+   virQEMUCapsGet(qemuCaps, QEMU_CAPS_HVF);
 }
 
 
@@ -868,6 +871,8 @@ virQEMUCapsGetAccel(virQEMUCaps *qemuCaps,
 {
 if (type == VIR_DOMAIN_VIRT_KVM)
 return >kvm;
+else if (type == VIR_DOMAIN_VIRT_HVF)
+return >hvf;
 
 return >tcg;
 }
@@ -998,6 +1003,8 @@ virQEMUCapsGetMachineTypesCaps(virQEMUCaps *qemuCaps,
  * take the set of machine types we probed first. */
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
 accel = >kvm;
+else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_HVF))
+accel = >hvf;
 else
 accel = >tcg;
 
@@ -2015,6 +2022,7 @@ virQEMUCaps *virQEMUCapsNewCopy(virQEMUCaps *qemuCaps)
 ret->cpuData = virCPUDataNewCopy(qemuCaps->cpuData);
 
 if (virQEMUCapsAccelCopy(>kvm, >kvm) < 0 ||
+virQEMUCapsAccelCopy(>hvf, >hvf) < 0 ||
 virQEMUCapsAccelCopy(>tcg, >tcg) < 0)
 return NULL;
 
@@ -2068,6 +2076,7 @@ void virQEMUCapsDispose(void *obj)
 virSEVCapabilitiesFree(qemuCaps->sevCapabilities);
 
 virQEMUCapsAccelClear(>kvm);
+virQEMUCapsAccelClear(>hvf);
 virQEMUCapsAccelClear(>tcg);
 }
 
@@ -2319,6 +2328,10 @@ virQEMUCapsIsVirtTypeSupported(virQEMUCaps *qemuCaps,
 virQEMUCapsGet(qemuCaps, QEMU_CAPS_TCG))
 return true;
 
+if (virtType == VIR_DOMAIN_VIRT_HVF &&
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_HVF))
+return true;
+
 if (virtType == VIR_DOMAIN_VIRT_KVM &&
 virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
 return true;
@@ -2796,7 +2809,9 @@ bool
 virQEMUCapsHasMachines(virQEMUCaps *qemuCaps)
 {
 
-return !!qemuCaps->kvm.nmachineTypes || !!qemuCaps->tcg.nmachineTypes;
+return !!qemuCaps->kvm.nmachineTypes ||
+   !!qemuCaps->hvf.nmachineTypes ||
+   !!qemuCaps->tcg.nmachineTypes;
 }
 
 
@@ -4494,6 +4509,10 @@ virQEMUCapsLoadCache(virArch hostArch,
 virQEMUCapsLoadAccel(qemuCaps, ctxt, VIR_DOMAIN_VIRT_KVM) < 0) {
 return -1;
 }
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_HVF) &&
+virQEMUCapsLoadAccel(qemuCaps, ctxt, VIR_DOMAIN_VIRT_HVF) < 0) {
+return -1;
+}
 if (virQEMUCapsLoadAccel(qemuCaps, ctxt, VIR_DOMAIN_VIRT_QEMU) < 0)
 return -1;
 
@@ -4505,6 +4524,8 @@ virQEMUCapsLoadCache(virArch hostArch,
 
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
 virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_KVM);
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_HVF))
+virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_HVF);
 virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_QEMU);
 
 if (virXPathBoolean("boolean(./kvmSupportsNesting)", ctxt) > 0)
@@ -4739,6 +4760,8 @@ virQEMUCapsFormatCache(virQEMUCaps *qemuCaps)
 
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
 virQEMUCapsFormatAccel(qemuCaps, , VIR_DOMAIN_VIRT_KVM);
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_HVF))
+virQEMUCapsFormatAccel(qemuCaps, , VIR_DOMAIN_VIRT_HVF);
 virQEMUCapsFormatAccel(qemuCaps, , VIR_DOMAIN_VIRT_QEMU);
 
 for (i = 0; i < qemuCaps->ngicCapabilities; i++) {
@@ -5361,6 +5384,9 @@ virQEMUCapsGetVirtType(virQEMUCaps *qemuCaps)
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
 return VIR_DOMAIN_VIRT_KVM;
 
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_HVF))
+return VIR_DOMAIN_VIRT_HVF;
+
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_TCG))
 return VIR_DOMAIN_VIRT_QEMU;
 
@@ -5599,6 +5625,8 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
 
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
 virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_KVM);
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_HVF))
+virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_HVF);
 

[libvirt PATCH v5 08/20] qemu: Introduce virQEMUCapsAccelStr

[Andrea Bolognani]

From: Roman Bolshakov 

This makes possible to add more accelerators by touching less code and
reduces code duplication.

Signed-off-by: Roman Bolshakov 
Signed-off-by: Andrea Bolognani 
Tested-by: Brad Laue 
---
 src/qemu/qemu_capabilities.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 3677e0570f..c296e31a7f 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -792,6 +792,16 @@ const char *virQEMUCapsArchToString(virArch arch)
 }
 
 
+static const char *
+virQEMUCapsAccelStr(virDomainVirtType type)
+{
+if (type == VIR_DOMAIN_VIRT_QEMU)
+return "tcg";
+
+return virDomainVirtTypeToString(type);
+}
+
+
 /* Checks whether a domain with @guest arch can run natively on @host.
  */
 bool
@@ -4077,7 +4087,7 @@ virQEMUCapsLoadAccel(virQEMUCaps *qemuCaps,
  virDomainVirtType type)
 {
 virQEMUCapsAccel *caps = virQEMUCapsGetAccel(qemuCaps, type);
-const char *typeStr = type == VIR_DOMAIN_VIRT_KVM ? "kvm" : "tcg";
+const char *typeStr = virQEMUCapsAccelStr(type);
 
 if (virQEMUCapsLoadHostCPUModelInfo(caps, ctxt, typeStr) < 0)
 return -1;
@@ -4630,7 +4640,7 @@ virQEMUCapsFormatAccel(virQEMUCaps *qemuCaps,
virDomainVirtType type)
 {
 virQEMUCapsAccel *caps = virQEMUCapsGetAccel(qemuCaps, type);
-const char *typeStr = type == VIR_DOMAIN_VIRT_KVM ? "kvm" : "tcg";
+const char *typeStr = virQEMUCapsAccelStr(type);
 
 virQEMUCapsFormatHostCPUModelInfo(caps, buf, typeStr);
 virQEMUCapsFormatCPUModels(caps, buf, typeStr);
-- 
2.31.1

[libvirt PATCH v5 09/20] qemu: Introduce virQEMUCapsTypeIsAccelerated

[Andrea Bolognani]

From: Roman Bolshakov 

It replaces hardcoded checks for KVM. It'll be cleaner to use
the function once multiple accelerators are supported in the
QEMU driver.

Explicit KVM domain checks should be done only when a feature is
available only for KVM.

Signed-off-by: Roman Bolshakov 
Signed-off-by: Andrea Bolognani 
Tested-by: Brad Laue 
---
 src/qemu/qemu_capabilities.c | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index c296e31a7f..eaf2905f4b 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -802,6 +802,13 @@ virQEMUCapsAccelStr(virDomainVirtType type)
 }
 
 
+static bool
+virQEMUCapsTypeIsAccelerated(virDomainVirtType type)
+{
+return type == VIR_DOMAIN_VIRT_KVM;
+}
+
+
 /* Checks whether a domain with @guest arch can run natively on @host.
  */
 bool
@@ -2341,7 +2348,7 @@ virQEMUCapsIsCPUModeSupported(virQEMUCaps *qemuCaps,
 
 switch (mode) {
 case VIR_CPU_MODE_HOST_PASSTHROUGH:
-return type == VIR_DOMAIN_VIRT_KVM &&
+return virQEMUCapsTypeIsAccelerated(type) &&
virQEMUCapsGuestIsNative(hostarch, qemuCaps->arch);
 
 case VIR_CPU_MODE_HOST_MODEL:
@@ -3003,7 +3010,7 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCaps *qemuCaps,
qemuMonitor *mon,
virDomainVirtType virtType)
 {
-const char *model = virtType == VIR_DOMAIN_VIRT_KVM ? "host" : "max";
+const char *model = virQEMUCapsTypeIsAccelerated(virtType) ? "host" : 
"max";
 g_autoptr(qemuMonitorCPUModelInfo) modelInfo = NULL;
 g_autoptr(qemuMonitorCPUModelInfo) nonMigratable = NULL;
 g_autoptr(GHashTable) hash = NULL;
@@ -3723,7 +3730,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCaps *qemuCaps,
   virArchToString(qemuCaps->arch),
   virDomainVirtTypeToString(type));
 goto error;
-} else if (type == VIR_DOMAIN_VIRT_KVM &&
+} else if (virQEMUCapsTypeIsAccelerated(type) &&
virCPUGetHostIsSupported(qemuCaps->arch)) {
 if (!(fullCPU = virQEMUCapsProbeHostCPU(qemuCaps->arch, NULL)))
 goto error;
@@ -5856,10 +5863,10 @@ virQEMUCapsCacheLookupDefault(virFileCache *cache,
 if (virttype == VIR_DOMAIN_VIRT_NONE)
 virttype = capsType;
 
-if (virttype == VIR_DOMAIN_VIRT_KVM && capsType == VIR_DOMAIN_VIRT_QEMU) {
+if (virQEMUCapsTypeIsAccelerated(virttype) && capsType == 
VIR_DOMAIN_VIRT_QEMU) {
 virReportError(VIR_ERR_INVALID_ARG,
-   _("KVM is not supported by '%s' on this host"),
-   binary);
+   _("the accel '%s' is not supported by '%s' on this 
host"),
+   virQEMUCapsAccelStr(virttype), binary);
 return NULL;
 }
 
-- 
2.31.1

[libvirt PATCH v5 01/20] qemu: Only probe KVM on Linux

[Andrea Bolognani]

We already know it's not going to be available on other
platforms.

Suggested-by: Daniel P. Berrangé 
Signed-off-by: Andrea Bolognani 
Tested-by: Brad Laue 
---
 src/qemu/qemu_process.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 5c9ca0fe4f..c2c10d282a 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -9269,6 +9269,12 @@ qemuProcessQMPInit(qemuProcessQMP *proc)
 }
 
 
+#if defined(__linux__)
+# define hwaccel "kvm:tcg"
+#else
+# define hwaccel "tcg"
+#endif
+
 static int
 qemuProcessQMPLaunch(qemuProcessQMP *proc)
 {
@@ -9279,7 +9285,7 @@ qemuProcessQMPLaunch(qemuProcessQMP *proc)
 if (proc->forceTCG)
 machine = "none,accel=tcg";
 else
-machine = "none,accel=kvm:tcg";
+machine = "none,accel=" hwaccel;
 
 VIR_DEBUG("Try to probe capabilities of '%s' via QMP, machine %s",
   proc->binary, machine);
-- 
2.31.1

[libvirt PATCH v5 05/20] qemu: Query hvf capability on macOS

[Andrea Bolognani]

From: Roman Bolshakov 

There's no QMP command for querying if hvf is supported, therefore we
use sysctl interface that tells if Hypervisor.framework works/available
on the host.

Signed-off-by: Roman Bolshakov 
Signed-off-by: Andrea Bolognani 
Tested-by: Brad Laue 
Reviewed-by: Daniel P. Berrangé 
---
 src/qemu/qemu_capabilities.c | 33 +
 1 file changed, 33 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 2aa928dd9e..9a0525222e 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -56,6 +56,10 @@
 #include 
 #include 
 #include 
+#ifdef __APPLE__
+# include 
+# include 
+#endif
 
 #define VIR_FROM_THIS VIR_FROM_QEMU
 
@@ -3194,6 +3198,32 @@ virQEMUCapsProbeQMPKVMState(virQEMUCaps *qemuCaps,
 return 0;
 }
 
+#ifdef __APPLE__
+static int
+virQEMUCapsProbeHVF(virQEMUCaps *qemuCaps)
+{
+int hv_support;
+size_t len = sizeof(hv_support);
+
+if (sysctlbyname("kern.hv_support", _support, , NULL, 0) < 0)
+hv_support = 0;
+
+if (qemuCaps->version >= 2012000 &&
+ARCH_IS_X86(qemuCaps->arch) &&
+hv_support) {
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_HVF);
+}
+
+return 0;
+}
+#else
+static int
+virQEMUCapsProbeHVF(virQEMUCaps *qemuCaps G_GNUC_UNUSED)
+{
+  return 0;
+}
+#endif
+
 struct virQEMUCapsCommandLineProps {
 const char *option;
 const char *param;
@@ -5343,6 +5373,9 @@ virQEMUCapsInitQMPMonitor(virQEMUCaps *qemuCaps,
 if (virQEMUCapsProbeQMPKVMState(qemuCaps, mon) < 0)
 return -1;
 
+if (virQEMUCapsProbeHVF(qemuCaps) < 0)
+return -1;
+
 type = virQEMUCapsGetVirtType(qemuCaps);
 accel = virQEMUCapsGetAccel(qemuCaps, type);
 
-- 
2.31.1

[libvirt PATCH v5 07/20] qemu: Expose hvf domain type if hvf is supported

[Andrea Bolognani]

From: Roman Bolshakov 

Signed-off-by: Roman Bolshakov 
Signed-off-by: Andrea Bolognani 
Tested-by: Brad Laue 
Reviewed-by: Daniel P. Berrangé 
---
 src/qemu/qemu_capabilities.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 79e9b19236..3677e0570f 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1084,6 +1084,10 @@ virQEMUCapsInitGuestFromBinary(virCaps *caps,
 virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_KVM,
   NULL, NULL, 0, NULL);
 }
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_HVF)) {
+virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_HVF,
+  NULL, NULL, 0, NULL);
+}
 
 if ((ARCH_IS_X86(guestarch) || guestarch == VIR_ARCH_AARCH64))
 virCapabilitiesAddGuestFeatureWithToggle(guest, 
VIR_CAPS_GUEST_FEATURE_TYPE_ACPI,
-- 
2.31.1

[libvirt PATCH v5 03/20] conf: Add hvf domain type

[Andrea Bolognani]

From: Roman Bolshakov 

QEMU supports Hypervisor.framework since 2.12 as hvf accel.
Hypervisor.framework provides a lightweight interface to run a virtual
cpu on macOS without the need to install third-party kernel
extensions (KEXTs).

It's supported since macOS 10.10 on machines with Intel VT-x feature
set that includes Extended Page Tables (EPT) and Unrestricted Mode.

Signed-off-by: Roman Bolshakov 
Signed-off-by: Andrea Bolognani 
Tested-by: Brad Laue 
Reviewed-by: Daniel P. Berrangé 
---
 docs/formatdomain.rst | 3 ++-
 docs/schemas/domaincommon.rng | 1 +
 src/conf/domain_conf.c| 1 +
 src/conf/domain_conf.h| 1 +
 src/qemu/qemu_command.c   | 4 
 5 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index d4f30bb8af..3e9de05249 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -20,7 +20,8 @@ Element and attribute overview
 
 The root element required for all virtual machines is named ``domain``. It has
 two attributes, the ``type`` specifies the hypervisor used for running the
-domain. The allowed values are driver specific, but include "xen", "kvm", 
"qemu"
+domain. The allowed values are driver specific, but include "xen", "kvm",
+"hvf" (:since:`since 8.0.0 and QEMU 2.12`), "qemu"
 and "lxc". The second attribute is ``id`` which is a unique integer identifier
 for the running guest machine. Inactive machines have no id value.
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 7fa5c2b8b5..bf9c12397f 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -230,6 +230,7 @@
 phyp 
 vz
 bhyve
+hvf
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5691b8d2d5..0faecf2bb4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -125,6 +125,7 @@ VIR_ENUM_IMPL(virDomainVirt,
   "parallels",
   "bhyve",
   "vz",
+  "hvf",
 );
 
 VIR_ENUM_IMPL(virDomainOS,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 144ba4dd12..d4d8aa7e23 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -139,6 +139,7 @@ typedef enum {
 VIR_DOMAIN_VIRT_PARALLELS,
 VIR_DOMAIN_VIRT_BHYVE,
 VIR_DOMAIN_VIRT_VZ,
+VIR_DOMAIN_VIRT_HVF,
 
 VIR_DOMAIN_VIRT_LAST
 } virDomainVirtType;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d822533ccb..3bacf3edf2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7229,6 +7229,10 @@ qemuBuildAccelCommandLine(virCommand *cmd,
 }
 break;
 
+case VIR_DOMAIN_VIRT_HVF:
+virBufferAddLit(, "hvf");
+break;
+
 case VIR_DOMAIN_VIRT_KQEMU:
 case VIR_DOMAIN_VIRT_XEN:
 case VIR_DOMAIN_VIRT_LXC:
-- 
2.31.1

[libvirt PATCH v5 04/20] qemu: Define hvf capability

[Andrea Bolognani]

From: Roman Bolshakov 

Signed-off-by: Roman Bolshakov 
Signed-off-by: Andrea Bolognani 
Tested-by: Brad Laue 
Reviewed-by: Daniel P. Berrangé 
---
 src/qemu/qemu_capabilities.c | 3 +++
 src/qemu/qemu_capabilities.h | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index eeac702d91..2aa928dd9e 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -654,6 +654,9 @@ VIR_ENUM_IMPL(virQEMUCaps,
   "rbd-encryption", /* QEMU_CAPS_RBD_ENCRYPTION */
   "sev-guest-kernel-hashes", /* QEMU_CAPS_SEV_GUEST_KERNEL_HASHES 
*/
   "sev-inject-launch-secret", /* 
QEMU_CAPS_SEV_INJECT_LAUNCH_SECRET */
+
+  /* 420 */
+  "hvf", /* QEMU_CAPS_HVF */
 );
 
 
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index e3a3ab4445..af541e3ab7 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -634,6 +634,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_SEV_GUEST_KERNEL_HASHES, /* sev-guest.kernel-hashes= */
 QEMU_CAPS_SEV_INJECT_LAUNCH_SECRET, /* 'sev-inject-launch-secret' qmp 
command present */
 
+/* 420 */
+QEMU_CAPS_HVF, /* Whether Hypervisor.framework is available */
+
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
 
-- 
2.31.1

[libvirt PATCH v5 00/20] qemu: Introduce hvf domain type for Hypervisor.framework

[Andrea Bolognani]

In order to hopefully address [libvirt#147] at long last, I've picked
up Roman's patches from 2018 and attempted to forward-port them.

More specifically, I've used the [roolebo/hvf-domain] branch as a
starting point, since it seems to contain a few improvements over
[v2] and was just easier to pick up.

The code is mostly his own, so I've retained the existing authorship
information, but I've dropped Reviewed-by tags for commits that have
been modified in non-trivial ways. I've applied very minimal style
tweaks along the way, but overall I've tried to modify the existing
patches as little as possible.

I've added a few changes of my own, which I've marked as "fixup!"
when I felt that they should be squashed into the previous patch
rather than existing as separate commits.

The new test cases, such as they are, pass, and no regressions to KVM
support appear to have been introduced in the process. I don't
currently have access to a machine running macOS, so I can't verify
that it's actually possible to start a hardware-accelerated VM by
myself, but a user has confirmed on the GitLab issue that the new
feature works.

Changes from [v4]:

  * fixed an issue that prevented machine types from being probed
correctly, effectively making the entire thing non functional;
  * only report HVF support as available when the guest architecture
and the host architecture match.

Changes from [v3]:

  * reintroduced the patch that was missing in the initial version
of the forward-port;
  * converted the documentation to reStructuredText and trimmed it
significantly;
  * reworked virQEMUCapsAccelStr() based on Dan's suggestions;
  * reworked macOS support in the test suite based on Dan's
suggestions;
  * fixed a few minor issues found while doing the above.

Changes from [v2]:

  * rebased on top of master;
  * added a couple of simple test cases.

Useful links:

  * GitLab: [abologna/hvf]
  * CI: [pipeline]

[libvirt#147] https://gitlab.com/libvirt/libvirt/-/issues/147
[roolebo/hvf-domain] https://github.com/roolebo/libvirt/tree/hvf-domain
[abologna/hvf] https://gitlab.com/abologna/libvirt/-/commits/hvf
[pipeline] https://gitlab.com/abologna/libvirt/-/pipelines/443320533
[v4] https://listman.redhat.com/archives/libvir-list/2022-January/msg00280.html
[v3] https://listman.redhat.com/archives/libvir-list/2022-January/msg00131.html
[v2] https://listman.redhat.com/archives/libvir-list/2018-November/msg00802.html

Andrea Bolognani (7):
  qemu: Only probe KVM on Linux
  fixup! qemu: Fix / improve virQEMUCapsProbeHVF()
  tests: Introduce testQemuHostOS
  tests: Add macOS support to testutilsqemu
  tests: Add macOS support to qemuxml2*test
  tests: Add HVF test cases
  fixup! NEWS: Mention Apple Silicon support for HVF

Roman Bolshakov (13):
  qemu: Add KVM CPUs into cache only if KVM is present
  conf: Add hvf domain type
  qemu: Define hvf capability
  qemu: Query hvf capability on macOS
  qemu: Expose hvf domain type if hvf is supported
  qemu: Introduce virQEMUCapsAccelStr
  qemu: Introduce virQEMUCapsTypeIsAccelerated
  qemu: Introduce virQEMUCapsHaveAccel
  qemu: Correct CPU capabilities probing for hvf
  docs: Add hvf on QEMU driver page
  docs: Note hvf support for domain elements
  docs: Add support page for libvirt on macOS
  news: Mention hvf domain type

 NEWS.rst  |   6 +
 docs/docs.html.in |   3 +
 docs/drvqemu.rst  |  48 +-
 docs/formatdomain.rst |  22 +--
 docs/index.html.in|   4 +-
 docs/macos.rst|  44 ++
 docs/meson.build  |   1 +
 docs/schemas/domaincommon.rng |   1 +
 src/conf/domain_conf.c|   1 +
 src/conf/domain_conf.h|   1 +
 src/qemu/qemu_capabilities.c  | 147 --
 src/qemu/qemu_capabilities.h  |   3 +
 src/qemu/qemu_command.c   |   4 +
 src/qemu/qemu_process.c   |  10 +-
 .../hvf-aarch64-virt-headless.args|  48 ++
 .../hvf-aarch64-virt-headless.xml |  45 ++
 .../hvf-x86_64-q35-headless.args  |  47 ++
 .../hvf-x86_64-q35-headless.x86_64-latest.err |   1 +
 .../hvf-x86_64-q35-headless.xml   |  44 ++
 tests/qemuxml2argvtest.c  |  43 -
 .../hvf-aarch64-virt-headless.xml |  94 +++
 .../hvf-x86_64-q35-headless.xml   |  97 
 tests/qemuxml2xmltest.c   |  43 -
 tests/testutilsqemu.c | 146 +
 tests/testutilsqemu.h |  10 ++
 25 files changed, 849 insertions(+), 64 deletions(-)
 create mode 100644 docs/macos.rst
 create mode 100644 tests/qemuxml2argvdata/hvf-aarch64-virt-headless.args
 create mode 100644 

[libvirt PATCH v5 02/20] qemu: Add KVM CPUs into cache only if KVM is present

[Andrea Bolognani]

From: Roman Bolshakov 

virQEMUCapsFormatCache/virQEMUCapsLoadCache adds/reads KVM CPUs to/from
capabilities cache regardless of QEMU_CAPS_KVM. That can cause undesired
side-effects when KVM CPUs are present in the cache on a platform that
doesn't support it, e.g. macOS or Linux without KVM support.

Signed-off-by: Roman Bolshakov 
Signed-off-by: Andrea Bolognani 
Tested-by: Brad Laue 
---
 src/qemu/qemu_capabilities.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 5f1eb5014c..eeac702d91 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4419,8 +4419,11 @@ virQEMUCapsLoadCache(virArch hostArch,
 return -1;
 }
 
-if (virQEMUCapsLoadAccel(qemuCaps, ctxt, VIR_DOMAIN_VIRT_KVM) < 0 ||
-virQEMUCapsLoadAccel(qemuCaps, ctxt, VIR_DOMAIN_VIRT_QEMU) < 0)
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM) &&
+virQEMUCapsLoadAccel(qemuCaps, ctxt, VIR_DOMAIN_VIRT_KVM) < 0) {
+return -1;
+}
+if (virQEMUCapsLoadAccel(qemuCaps, ctxt, VIR_DOMAIN_VIRT_QEMU) < 0)
 return -1;
 
 if (virQEMUCapsParseGIC(qemuCaps, ctxt) < 0)
@@ -4429,7 +4432,8 @@ virQEMUCapsLoadCache(virArch hostArch,
 if (virQEMUCapsParseSEVInfo(qemuCaps, ctxt) < 0)
 return -1;
 
-virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_KVM);
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
+virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_KVM);
 virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_QEMU);
 
 if (virXPathBoolean("boolean(./kvmSupportsNesting)", ctxt) > 0)
@@ -4662,7 +4666,8 @@ virQEMUCapsFormatCache(virQEMUCaps *qemuCaps)
 virBufferAsprintf(, "%s\n",
   virArchToString(qemuCaps->arch));
 
-virQEMUCapsFormatAccel(qemuCaps, , VIR_DOMAIN_VIRT_KVM);
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
+virQEMUCapsFormatAccel(qemuCaps, , VIR_DOMAIN_VIRT_KVM);
 virQEMUCapsFormatAccel(qemuCaps, , VIR_DOMAIN_VIRT_QEMU);
 
 for (i = 0; i < qemuCaps->ngicCapabilities; i++) {
@@ -5516,7 +5521,8 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
 qemuCaps->libvirtCtime = virGetSelfLastChanged();
 qemuCaps->libvirtVersion = LIBVIR_VERSION_NUMBER;
 
-virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_KVM);
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
+virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_KVM);
 virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_QEMU);
 
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
-- 
2.31.1

[PATCH] docs: minor fix in launchSecurity

[Boris Fiuczynski]

Correcting XML element.

Signed-off-by: Boris Fiuczynski 
---
 docs/formatdomain.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index d4f30bb8af..69e02e0135 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -8201,7 +8201,7 @@ Note: DEA/TDEA is synonymous with DES/TDES.
 Launch Security
 ---
 
-Specifying  in a s390 domain prepares
+Specifying  in a s390 domain prepares
 the guest to run in protected virtualization secure mode, also known as
 IBM Secure Execution. For more required host and guest preparation steps, see
 `Protected Virtualization on s390 `__
-- 
2.33.1

[PATCH v2 2/3] virdnsmasq: Lookup DNSMASQ in PATH

[Michal Privoznik]

While it's true that our virCommand subsystem is happy with
non-absolute paths, the dnsmasq capability code is not. For
instance, it does call stat() over the binary to learn its mtime
(and thus decide whether capabilities need to be fetched again or
not).

Therefore, when constructing the capabilities structure look up
the binary path. If DNSMASQ already contains an absolute path
then virFindFileInPath() will simply return a copy. But, if we
failed to find the binary in $PATH then do not report error.
Either the dnsmasqCapsNewEmpty() function is called from
dnsmasqCapsNewFromBinary() which will later validate the path and
return appropriate error, or the function is called from a test
suite (via dnsmasqCapsNewFromBuffer()) and the caps are parsed
from a fixed string.

Signed-off-by: Michal Privoznik 
---
Diff to v1:
- Per my experiments we need a fallback case when DNSMASQ exists but is
  not executable.

At any rate, this whole code could use cleanup, but let's save that for
after the release.

 src/util/virdnsmasq.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
index d304929d51..8ddc2d0b78 100644
--- a/src/util/virdnsmasq.c
+++ b/src/util/virdnsmasq.c
@@ -703,12 +703,21 @@ static dnsmasqCaps *
 dnsmasqCapsNewEmpty(void)
 {
 dnsmasqCaps *caps;
+g_autofree char *binaryPath = NULL;
 
 if (dnsmasqCapsInitialize() < 0)
 return NULL;
 if (!(caps = virObjectNew(dnsmasqCapsClass)))
 return NULL;
-caps->binaryPath = g_strdup(DNSMASQ);
+
+if (!(binaryPath = virFindFileInPath(DNSMASQ))) {
+/* Don't report error here because we might be running
+ * from a test suite an initializing capabilities from
+ * a buffer (dnsmasqCapsNewFromBuffer()). */
+binaryPath = g_strdup(DNSMASQ);
+}
+
+caps->binaryPath = g_steal_pointer();
 return caps;
 }
 
-- 
2.34.1

Re: [PATCH 2/3] virdnsmasq: Lookup DNSMASQ in PATH

[Michal Prívozník]

On 1/10/22 18:41, Andrea Bolognani wrote:
> On Mon, Jan 10, 2022 at 04:44:55PM +0100, Michal Privoznik wrote:
>> While it's true that our virCommand subsystem is happy with
>> non-absolute paths, the dnsmasq capability code is not. For
>> instance, it does call stat() over the binary to learn its mtime
>> (and thus decide whether capabilities need to be fetched again or
>> not).
>>
>> Therefore, when constructing the capabilities structure look up
>> the binary path. If DNSMASQ already contains an absolute path
>> then it is returned (and virFindFileInPath() is a NOP).
> 
> Saying that virFindFileInPath() is a NOP is not quite correct: if you
> pass an absolute path, it will return a copy. So I'd rewrite the
> above as
> 
>   If DNSMASQ already contains an absolute path then
>   virFindFileInPath() will simply return a copy.
> 

Yes, this makes sense.

> Reviewed-by: Andrea Bolognani 
> 

Thanks, but as I was looking through virFindFileInPath() I've noticed
that if the file exists but is not executable then NULL is returned, so
we will need a fallback case. Effectively this needs to be squashed in:

diff --git c/src/util/virdnsmasq.c w/src/util/virdnsmasq.c
index b6ccb9d0a4..7792a65bc3 100644
--- c/src/util/virdnsmasq.c
+++ w/src/util/virdnsmasq.c
@@ -703,12 +703,17 @@ static dnsmasqCaps *
 dnsmasqCapsNewEmpty(void)
 {
 dnsmasqCaps *caps;
+g_autofree char *binaryPath = NULL;

 if (dnsmasqCapsInitialize() < 0)
 return NULL;
 if (!(caps = virObjectNew(dnsmasqCapsClass)))
 return NULL;
-caps->binaryPath = virFindFileInPath(DNSMASQ);
+
+if (!(binaryPath = virFindFileInPath(DNSMASQ)))
+binaryPath = g_strdup(DNSMASQ);
+
+caps->binaryPath = g_steal_pointer();
 return caps;
 }

I'll post v2 of this patch shortly.

Michal