Re: [libvirt] [PATCH] vmx: allow an odd number of vCPUs

[Pino Toscano]

On Thursday, 14 June 2018 15:34:25 CEST Pino Toscano wrote:
> Most probably this was a limitation in older ESX versions, and it seems
> it does not exist anymore in more recent versions; see the following
> thread:
> https://www.redhat.com/archives/libvir-list/2018-May/msg02159.html
> https://www.redhat.com/archives/libvir-list/2018-June/msg00043.html
> 
> Hence, allow an odd number (greater than 1) of vCPUs, since most
> probably older versions of ESXi will error out anyway.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1584091
> 
> Signed-off-by: Pino Toscano 
> ---

Polite ping.

Thanks,
-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] docs: schema: Add missing to devices

[Han Han]

For input,hub,redirdev devices, their sub-elements should be interleaved.

input device: interleave for , , 
hub device: interleave for , 
redirdev device: interleave for , , , 

Signed-off-by: Han Han 
---
 docs/schemas/domaincommon.rng | 60 +++
 1 file changed, 33 insertions(+), 27 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4a454dddb4..d262eb2b1b 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4270,11 +4270,19 @@
 
   
 
-  
-
-  
-
-  
+  
+
+  
+
+  
+
+
+  
+
+
+  
+
+  
   
 
   
@@ -4309,12 +4317,6 @@
   
 
   
-  
-
-  
-  
-
-  
 
   
   
@@ -4322,12 +4324,14 @@
   
 usb
   
-  
-
-  
-  
-
-  
+  
+
+  
+
+
+  
+
+  
 
   
   
@@ -4338,16 +4342,18 @@
   
 
   
-  
-  
-
-  
-  
-
-  
-  
-
-  
+  
+
+
+  
+
+
+  
+
+
+  
+
+  
 
   
   
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCHv1 08/12] qemu_capabilities: QMPCommandPtr without shadow qmperr

[Chris Venteicher]

Previously QMPCommandPtr (handle for issuing QMP commands) required an
external char * qmperr to persist over the lifespan of QMPCommand to
expose a QMP error string outside of QMPCommand.

Specifically the external char *qmperr was required between calls to
virQEMUCapsInitQMPCommandNew and virQEMUCapsInitQMPCommandAbort.

This change allows the qmperr pointer to be maintained within the
QMPCommand it's self avoiding the need to track the qmperr variable
across a multi-function call lifespan of a QMPCommand.

Q) Should we eliminate external qmperr completely as there seems to be
no current use of qmperr outside the lifespan of QMPCommand in which an
internal qmperr char pointer can persist and be used by anywhere we have
access to the QMPCommandPtr?

conversion functions removed
'57d6df39bd'.
---
 src/qemu/qemu_capabilities.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 89a313b55d..f2202b652d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4242,6 +4242,7 @@ struct _virQEMUCapsInitQMPCommand {
 uid_t runUid;
 gid_t runGid;
 char **qmperr;
+char *qmperr_internal;
 char *monarg;
 char *monpath;
 char *pidfile;
@@ -4319,7 +4320,11 @@ virQEMUCapsInitQMPCommandNew(char *binary,
 
 cmd->runUid = runUid;
 cmd->runGid = runGid;
-cmd->qmperr = qmperr;
+
+if (qmperr)
+cmd->qmperr = qmperr; /* external storage */
+else
+cmd->qmperr = >qmperr_internal; /* cmd internal storage */
 
 /* the ".sock" sufix is important to avoid a possible clash with a qemu
  * domain called "capabilities"
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCHv1 11/12] qemu_capabilities: Introduce virQEMUCapsQMPBaselineCPUModel (baseline using QEMU)

[Chris Venteicher]

Baseline cpu model using QEMU/QMP query-cpu-model-baseline

query-cpu-model-baseline only compares two CPUModels so multiple
exchanges are needed to evaluate more than two CPUModels.
---
 src/qemu/qemu_capabilities.c | 89 
 src/qemu/qemu_capabilities.h |  4 ++
 2 files changed, 93 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 658f88b327..1646284588 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5394,3 +5394,92 @@ virQEMUCapsStripMachineAliases(virQEMUCapsPtr qemuCaps)
 for (i = 0; i < qemuCaps->nmachineTypes; i++)
 VIR_FREE(qemuCaps->machineTypes[i].alias);
 }
+
+
+/* in:
+ *  cpus[0]->model = "z13-base";
+ *  cpus[0]->features[0].name = "xxx";
+ *  cpus[0]->features[1].name = "yyy";
+ *  ***
+ *  cpus[n]->model = "s390x";
+ *  cpus[n]->features[0].name = "xxx";
+ *  cpus[n]->features[1].name = "yyy";
+ *
+ * out:
+ *  *baseline->model = "s390x";
+ *  *baseline->features[0].name = "yyy";
+ *
+ * (ret==0) && (*baseline == NULL) if QMP baseline not supported by QEMU
+ */
+int
+virQEMUCapsQMPBaselineCPUModel(virQEMUCapsInitQMPCommandPtr cmd,
+   virCPUDefPtr *cpus,
+   virCPUDefPtr *baseline)
+{
+qemuMonitorCPUModelInfoPtr  model_baseline = NULL;
+qemuMonitorCPUModelInfoPtr  new_model_baseline = NULL;
+qemuMonitorCPUModelInfoPtr  model_b = NULL;
+bool migratable_only = true;
+int ret = -1;
+size_t i;
+
+int ncpus = virCPUDefListLength(cpus);
+
+VIR_DEBUG("ncpus = %i", ncpus);
+
+*baseline = NULL;
+
+if (!cpus || !cpus[0] || !cpus[1]) {
+virReportError(VIR_ERR_INVALID_ARG, "%s", _("less than 2 cpus"));
+goto cleanup;
+}
+
+VIR_DEBUG("cpu[0]->model = %s", NULLSTR(cpus[0]->model));
+
+if (!(model_baseline = virQEMUCapsCPUModelInfoFromCPUDef(cpus[0])))
+goto cleanup;
+
+for (i = 1; i < ncpus; i++) {
+virCPUDefPtr cpu = cpus[i];
+
+VIR_DEBUG("cpu[%lu]->model = %s", i, NULLSTR(cpu->model));
+
+if (!(model_b = virQEMUCapsCPUModelInfoFromCPUDef(cpu)))
+goto cleanup;
+
+if (!model_baseline || !model_b) {
+virReportError(VIR_ERR_INVALID_ARG, "%s", _("cpu without 
content"));
+goto cleanup;
+}
+
+if (qemuMonitorGetCPUModelBaseline(cmd->mon, model_baseline,
+   model_b, _model_baseline) < 0)
+goto cleanup;
+
+if (!new_model_baseline) {
+VIR_DEBUG("QEMU didn't support baseline");
+ret = 0;
+goto cleanup;
+}
+
+qemuMonitorCPUModelInfoFree(model_baseline);
+qemuMonitorCPUModelInfoFree(model_b);
+
+model_b = NULL;
+
+model_baseline = new_model_baseline;
+}
+
+if (!(*baseline = virQEMUCapsCPUModelInfoToCPUDef(migratable_only, 
model_baseline)))
+goto cleanup;
+
+VIR_DEBUG("baseline->model = %s", NULLSTR((*baseline)->model));
+
+ret = 0;
+
+ cleanup:
+qemuMonitorCPUModelInfoFree(model_baseline);
+qemuMonitorCPUModelInfoFree(model_b);
+
+return ret;
+}
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 9a62b014ac..6098378f6c 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -591,6 +591,10 @@ void virQEMUCapsFilterByMachineType(virQEMUCapsPtr 
qemuCaps,
 qemuMonitorCPUModelInfoPtr virQEMUCapsCPUModelInfoFromCPUDef(const virCPUDef 
*cpuDef);
 virCPUDefPtr virQEMUCapsCPUModelInfoToCPUDef(bool migratable_only, 
qemuMonitorCPUModelInfoPtr model);
 
+int virQEMUCapsQMPBaselineCPUModel(virQEMUCapsInitQMPCommandPtr cmd,
+   virCPUDefPtr *cpus,
+   virCPUDefPtr *baseline);
+
 virFileCachePtr virQEMUCapsCacheNew(const char *libDir,
 const char *cacheDir,
 uid_t uid,
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCHv1 12/12] qemu_driver: BaselineHypervisorCPU supports S390 using QEMU/QMP

[Chris Venteicher]

Transient S390 configurations require using QEMU to compute CPU Model
Baseline and to do CPU Feature Expansion.

Start and use a single QEMU instance to do both the baseline and
expansion transactions required by BaselineHypervisorCPU.
---
 src/qemu/qemu_driver.c | 66 --
 1 file changed, 57 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 921aafcd79..868d6087a9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13419,10 +13419,13 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
 virArch arch;
 virDomainVirtType virttype;
 virDomainCapsCPUModelsPtr cpuModels;
-bool migratable;
+bool migratable_only;
 virCPUDefPtr cpu = NULL;
 char *cpustr = NULL;
 char **features = NULL;
+virQEMUCapsInitQMPCommandPtr cmd = NULL;
+bool forceTCG = false;
+qemuMonitorCPUModelInfoPtr modelInfo = NULL;
 
 virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES |
   VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL);
@@ -13430,8 +13433,6 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
 if (virConnectBaselineHypervisorCPUEnsureACL(conn) < 0)
 goto cleanup;
 
-migratable = !!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE);
-
 if (!(cpus = virCPUDefListParse(xmlCPUs, ncpus, VIR_CPU_TYPE_AUTO)))
 goto cleanup;
 
@@ -13444,6 +13445,19 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
 if (!qemuCaps)
 goto cleanup;
 
+/* QEMU can enumerate non-migratable cpu model features for some archs 
like x86
+ * migratable_only == true:  ask for and include only migratable features
+ * migratable_only == false: ask for and include all features
+ */
+migratable_only = !!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE);
+
+if (ARCH_IS_S390(arch)) {
+   /* QEMU for S390 arch only enumerates migratable features
+* No reason to explicitly ask QEMU for or include non-migratable 
features
+*/
+   migratable_only = true;
+}
+
 if (!(cpuModels = virQEMUCapsGetCPUDefinitions(qemuCaps, virttype)) ||
 cpuModels->nmodels == 0) {
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
@@ -13456,18 +13470,31 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
 
 if (ARCH_IS_X86(arch)) {
 int rc = virQEMUCapsGetCPUFeatures(qemuCaps, virttype,
-   migratable, );
+   migratable_only, );
 if (rc < 0)
 goto cleanup;
 if (features && rc == 0) {
 /* We got only migratable features from QEMU if we asked for them,
  * no further filtering in virCPUBaseline is desired. */
-migratable = false;
+migratable_only = false;
 }
 
 if (!(cpu = virCPUBaseline(arch, cpus, ncpus, cpuModels,
-   (const char **)features, migratable)))
+   (const char **)features, migratable_only)))
 goto cleanup;
+} else if (ARCH_IS_S390(arch)) {
+
+   const char *binary = virQEMUCapsGetBinary(qemuCaps);
+   virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+
+   if (!(cmd = virQEMUCapsNewQMPCommandConnection(binary, cfg->libDir,
+  cfg->user, cfg->group,
+  forceTCG)))
+  goto cleanup;
+
+   if (virQEMUCapsQMPBaselineCPUModel(cmd, cpus, ) < 0)
+  goto cleanup; /* Content Error */
+
 } else {
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_("computing baseline hypervisor CPU is not supported "
@@ -13477,9 +13504,28 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
 
 cpu->fallback = VIR_CPU_FALLBACK_FORBID;
 
-if ((flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) &&
-virCPUExpandFeatures(arch, cpu) < 0)
-goto cleanup;
+if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) {
+   if (ARCH_IS_X86(arch)) {
+  if (virCPUExpandFeatures(arch, cpu) < 0)
+ goto cleanup;
+   } else if (ARCH_IS_S390(arch)) {
+  if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION))
+ goto cleanup;
+
+  if (!(modelInfo = virQEMUCapsCPUModelInfoFromCPUDef(cpu)))
+ goto cleanup;
+
+  if (qemuMonitorGetCPUModelExpansion(cmd->mon,
+  
QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL,
+  migratable_only, modelInfo) < 0)
+ goto cleanup;
+
+  virCPUDefFree(cpu);
+
+  if (!(cpu = virQEMUCapsCPUModelInfoToCPUDef(migratable_only, 
modelInfo)))
+ goto cleanup;
+   }
+}
 
 cpustr = virCPUDefFormat(cpu, NULL);
 
@@ -13488,6 +13534,8 @@ 

[libvirt] [PATCHv1 07/12] qemu_capabilities: virCPUDef / qemuMonitorCPUModelInfo conversions

[Chris Venteicher]

Bi-directional conversion functions.
Converts model / name and features / properties.
---
 src/qemu/qemu_capabilities.c | 137 +--
 src/qemu/qemu_capabilities.h |   3 +
 2 files changed, 118 insertions(+), 22 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index da6fb1f614..89a313b55d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2737,7 +2737,7 @@ virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps,
 virCPUDefPtr cpu,
 bool migratable)
 {
-size_t i;
+virCPUDefPtr tmp = NULL;
 
 if (!modelInfo) {
 if (type == VIR_DOMAIN_VIRT_KVM) {
@@ -2750,30 +2750,12 @@ virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps,
 return 2;
 }
 
-if (VIR_STRDUP(cpu->model, modelInfo->name) < 0 ||
-VIR_ALLOC_N(cpu->features, modelInfo->nprops) < 0)
+if (!(tmp = virQEMUCapsCPUModelInfoToCPUDef(migratable, modelInfo)))
 return -1;
 
-cpu->nfeatures_max = modelInfo->nprops;
-cpu->nfeatures = 0;
-
-for (i = 0; i < modelInfo->nprops; i++) {
-virCPUFeatureDefPtr feature = cpu->features + cpu->nfeatures;
-qemuMonitorCPUPropertyPtr prop = modelInfo->props + i;
+*cpu = *tmp;
 
-if (prop->type != QEMU_MONITOR_CPU_PROPERTY_BOOLEAN)
-continue;
-
-if (VIR_STRDUP(feature->name, prop->name) < 0)
-return -1;
-
-if (!prop->value.boolean ||
-(migratable && prop->migratable == VIR_TRISTATE_BOOL_NO))
-feature->policy = VIR_CPU_FEATURE_DISABLE;
-else
-feature->policy = VIR_CPU_FEATURE_REQUIRE;
-cpu->nfeatures++;
-}
+VIR_FREE(tmp);
 
 return 0;
 }
@@ -3526,6 +3508,117 @@ virQEMUCapsLoadCache(virArch hostArch,
 return ret;
 }
 
+/* virCPUDef model=> qemuMonitorCPUModelInfo name
+ * virCPUDef features => qemuMonitorCPUModelInfo boolean properties */
+qemuMonitorCPUModelInfoPtr
+virQEMUCapsCPUModelInfoFromCPUDef(const virCPUDef *cpuDef)
+{
+size_t i;
+qemuMonitorCPUModelInfoPtr cpuModel = NULL;
+qemuMonitorCPUModelInfoPtr ret = NULL;
+
+if (!cpuDef || (VIR_ALLOC(cpuModel) < 0))
+goto cleanup;
+
+VIR_DEBUG("cpuDef->model = %s", NULLSTR(cpuDef->model));
+
+if (VIR_STRDUP(cpuModel->name, cpuDef->model) < 0 ||
+VIR_ALLOC_N(cpuModel->props, cpuDef->nfeatures) < 0)
+goto cleanup;
+
+/* no visibility on migratability of properties / features */
+cpuModel->props_migratable_valid = false;
+
+cpuModel->nprops = 0;
+
+for (i = 0; i < cpuDef->nfeatures; i++) {
+qemuMonitorCPUPropertyPtr prop = &(cpuModel->props[cpuModel->nprops]);
+virCPUFeatureDefPtr feature = &(cpuDef->features[i]);
+
+if (!feature || !(feature->name))
+continue;
+
+if (VIR_STRDUP(prop->name, feature->name) < 0)
+goto cleanup;
+
+prop->migratable = VIR_TRISTATE_BOOL_ABSENT;
+prop->type = QEMU_MONITOR_CPU_PROPERTY_BOOLEAN;
+
+switch (feature->policy) {
+case VIR_CPU_FEATURE_FORCE:
+case VIR_CPU_FEATURE_REQUIRE:
+prop->value.boolean = true;
+break;
+
+case VIR_CPU_FEATURE_FORBID:
+case VIR_CPU_FEATURE_DISABLE:
+case VIR_CPU_FEATURE_OPTIONAL:
+case VIR_CPU_FEATURE_LAST:
+prop->value.boolean = false;
+break;
+}
+
+cpuModel->nprops += 1;
+}
+
+VIR_STEAL_PTR(ret, cpuModel);
+
+ cleanup:
+qemuMonitorCPUModelInfoFree(cpuModel);
+return ret;
+}
+
+
+/* qemuMonitorCPUModelInfo name   => virCPUDef model
+ * qemuMonitorCPUModelInfo boolean properties => virCPUDef features
+ *
+ * migratable_only true: mark non-migratable features as disabled
+ * false: allow all features as required
+ */
+virCPUDefPtr
+virQEMUCapsCPUModelInfoToCPUDef(bool migratable_only, 
qemuMonitorCPUModelInfoPtr model)
+{
+virCPUDefPtr cpu = NULL;
+virCPUDefPtr ret = NULL;
+size_t i;
+
+if (!model || (VIR_ALLOC(cpu) < 0))
+goto cleanup;
+
+VIR_DEBUG("model->name= %s", NULLSTR(model->name));
+
+if (VIR_STRDUP(cpu->model, model->name) < 0 ||
+VIR_ALLOC_N(cpu->features, model->nprops) < 0)
+goto cleanup;
+
+cpu->nfeatures_max = model->nprops;
+cpu->nfeatures = 0;
+
+for (i = 0; i < model->nprops; i++) {
+virCPUFeatureDefPtr feature = cpu->features + cpu->nfeatures;
+qemuMonitorCPUPropertyPtr prop = model->props + i;
+
+if (prop->type != QEMU_MONITOR_CPU_PROPERTY_BOOLEAN)
+continue;
+
+if (VIR_STRDUP(feature->name, prop->name) < 0)
+goto cleanup;
+
+if (!prop->value.boolean ||
+(migratable_only && prop->migratable == VIR_TRISTATE_BOOL_NO))
+feature->policy = VIR_CPU_FEATURE_DISABLE;
+else
+feature->policy = 

[libvirt] [PATCHv1 10/12] qemu_capabilities: Introduce virQEMUCapsNewQMPCommandConnection

[Chris Venteicher]

Start and connect to QEMU so QMP commands can be performed.

Isolates code for starting QEMU and establishing Monitor connections
from code for obtaining capabilities so that arbitrary QMP commands can
be exchanged with QEMU.
---
 src/qemu/qemu_capabilities.c | 40 +++-
 src/qemu/qemu_capabilities.h |  5 +
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 2bbbfc83f3..658f88b327 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4285,7 +4285,7 @@ 
virQEMUCapsInitQMPCommandFree(virQEMUCapsInitQMPCommandPtr cmd)
 
 
 static virQEMUCapsInitQMPCommandPtr
-virQEMUCapsInitQMPCommandNew(char *binary,
+virQEMUCapsInitQMPCommandNew(const char *binary,
  const char *libDir,
  uid_t runUid,
  gid_t runGid,
@@ -4426,6 +4426,44 @@ 
virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd,
 }
 
 
+/* Start and connect to QEMU so QMP commands can be performed.
+ */
+virQEMUCapsInitQMPCommandPtr
+virQEMUCapsNewQMPCommandConnection(const char *exec,
+  const char *libDir, uid_t runUid, gid_t runGid,
+  bool forceTCG)
+{
+virQEMUCapsInitQMPCommandPtr cmd = NULL;
+virQEMUCapsInitQMPCommandPtr rtn_cmd = NULL;
+
+VIR_DEBUG("exec =%s", NULLSTR(exec));
+
+/* Allocate and initialize QMPCommand structure */
+if (!(cmd = virQEMUCapsInitQMPCommandNew(exec, libDir,
+ runUid, runGid, NULL)))
+goto cleanup;
+
+/* Spawn QEMU and establish connection for QMP commands */
+if (virQEMUCapsInitQMPCommandRun(cmd, forceTCG) != 0)
+goto cleanup;
+
+/* Exit capabilities negotiation mode and enter QEMU command mode
+ * by issuing qmp_capabilities command to QEMU */
+if (qemuMonitorSetCapabilities(cmd->mon) < 0) {
+VIR_DEBUG("Failed to set monitor capabilities %s",
+  virGetLastErrorMessage());
+goto cleanup;
+}
+
+VIR_STEAL_PTR(rtn_cmd, cmd);
+
+ cleanup:
+virQEMUCapsInitQMPCommandFree(cmd);
+
+return rtn_cmd;
+}
+
+
 static int
 virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
const char *libDir,
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index c9618bb754..9a62b014ac 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -507,6 +507,11 @@ struct _virQEMUCapsInitQMPCommand {
 virDomainObjPtr vm;
 };
 
+virQEMUCapsInitQMPCommandPtr
+virQEMUCapsNewQMPCommandConnection(const char *exec,
+   const char *libDir, uid_t runUid, gid_t 
runGid,
+   bool forceTCG);
+
 void virQEMUCapsInitQMPCommandFree(virQEMUCapsInitQMPCommandPtr cmd);
 
 typedef struct _virQEMUCaps virQEMUCaps;
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCHv1 05/12] qemu_monitor: CPUModelExpansion on CPUModel with both name and properties

[Chris Venteicher]

Send both model name and a set of features/properties to QEMU for
expansion rather than just the model name.
---
 src/qemu/qemu_capabilities.c | 33 +--
 src/qemu/qemu_monitor.c  | 38 ++
 src/qemu/qemu_monitor.h  |  5 ++-
 src/qemu/qemu_monitor_json.c | 61 +++-
 src/qemu/qemu_monitor_json.h |  7 ++---
 tests/cputest.c  |  7 -
 6 files changed, 105 insertions(+), 46 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 0c5e4bb766..da6fb1f614 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2333,7 +2333,7 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps,
 qemuMonitorCPUModelInfoPtr modelInfo = NULL;
 qemuMonitorCPUModelInfoPtr nonMigratable = NULL;
 virHashTablePtr hash = NULL;
-const char *model;
+const char *model_name;
 qemuMonitorCPUModelExpansionType type;
 virDomainVirtType virtType;
 virQEMUCapsHostCPUDataPtr cpuData;
@@ -2344,12 +2344,20 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps,
 
 if (tcg || !virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
 virtType = VIR_DOMAIN_VIRT_QEMU;
-model = "max";
+model_name = "max";
 } else {
 virtType = VIR_DOMAIN_VIRT_KVM;
-model = "host";
+model_name = "host";
 }
 
+if ((VIR_ALLOC(modelInfo) < 0) ||
+(VIR_ALLOC(nonMigratable) < 0))
+goto cleanup;
+
+if ((qemuMonitorCPUModelInfoInit(model_name, modelInfo) < 0) ||
+(qemuMonitorCPUModelInfoInit(model_name, nonMigratable) < 0))
+goto cleanup;
+
 cpuData = virQEMUCapsGetHostCPUData(qemuCaps, virtType);
 
 /* Some x86_64 features defined in cpu_map.xml use spelling which differ
@@ -2362,15 +2370,20 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps,
 else
 type = QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC;
 
-if (qemuMonitorGetCPUModelExpansion(mon, type, model, true, ) < 
0)
-goto cleanup;
-
-/* Try to check migratability of each feature. */
-if (modelInfo &&
-qemuMonitorGetCPUModelExpansion(mon, type, model, false,
-) < 0)
+if ((qemuMonitorGetCPUModelExpansion(mon, type, true, modelInfo) < 0) ||
+(qemuMonitorGetCPUModelExpansion(mon, type, false, nonMigratable) < 0))
 goto cleanup;
 
+/* Try to check migratability of each feature */
+/* Expansion 1 sets migratable features true
+ * Expansion 2 sets migratable and non-migratable features true
+ * (non-migratable set true only in some archs like X86)
+ *
+ * If delta between Expansion 1 and 2 exists...
+ * - both migratable and non-migratable features set prop->value = true
+ * - migratable features set prop->migatable = VIR_TRISTATE_BOOL_YES
+ * - non-migratable features set prop->migatable = VIR_TRISTATE_BOOL_NO
+ */
 if (nonMigratable) {
 qemuMonitorCPUPropertyPtr prop;
 qemuMonitorCPUPropertyPtr nmProp;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 3fd73e2cd1..558bc37aff 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3615,20 +3615,46 @@ qemuMonitorCPUDefInfoFree(qemuMonitorCPUDefInfoPtr cpu)
 }
 
 
+/*
+ * type static:
+ *   Expand to static base model + delta property changes
+ *   Returned model is invariant and migration safe
+ *
+ *   model_info->name = base model name
+ *   model_info->props = features to +/- to base model to achive model_name
+ *
+ * type full:
+ *   Expand model to enumerate all properties
+ *   Returned model isn't guaranteed to be invariant or migration safe.
+ *
+ *   model_info->name = base model name
+ *   model_info->props = features to +/- to empty set to achive model_name
+ *
+ * type static_full:
+ *   Expand to static base model + delta property changes (pass 0)
+ *   Expand model to enumerate all properties (pass 1)
+ *   Returned model is invariant and migration safe
+ *
+ *   model_info->name = base model name
+ *   model_info->props = features to +/- to empty set to achive model_name
+ *
+ * migratable_only:
+ *   true: QEMU excludes non-migratable features
+ *   false: QEMU includes non-migratable features for some archs like X86
+ */
 int
 qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,
 qemuMonitorCPUModelExpansionType type,
-const char *model_name,
-bool migratable,
-qemuMonitorCPUModelInfoPtr *model_info)
+bool migratable_only,
+qemuMonitorCPUModelInfoPtr model_info)
 {
 VIR_DEBUG("type=%d model_name=%s migratable=%d",
-  type, model_name, migratable);
+  type, (model_info ? NULLSTR(model_info->name):"NULL"),
+  migratable_only);
 
 QEMU_CHECK_MONITOR(mon);
 
-

[libvirt] [PATCHv1 01/12] qemu_monitor_json: qemuMonitorCPUModelInfo / JSON conversion

[Chris Venteicher]

Bidirectional conversion functions between data structure and JSON.

JSON of form:
{"model": {"name": "IvyBridge", "props": {}}}
---
 src/qemu/qemu_monitor_json.c | 126 ++-
 1 file changed, 96 insertions(+), 30 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index aa89ea7056..afbf000fa2 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5358,6 +5358,101 @@ qemuMonitorJSONParseCPUModelProperty(const char *key,
 return 0;
 }
 
+
+/* model_json: {"name": "z13-base", "props": {}}
+ */
+static virJSONValuePtr
+qemuMonitorJSONBuildCPUModelInfoToJSON(qemuMonitorCPUModelInfoPtr model)
+{
+virJSONValuePtr cpu_props = NULL;
+virJSONValuePtr model_json = NULL;
+size_t i;
+
+if (!model)
+goto cleanup;
+
+if (!(cpu_props = virJSONValueNewObject()))
+goto cleanup;
+
+for (i = 0; i < model->nprops; i++) {
+qemuMonitorCPUPropertyPtr prop = &(model->props[i]);
+
+switch (prop->type) {
+case QEMU_MONITOR_CPU_PROPERTY_BOOLEAN:
+if (virJSONValueObjectAppendBoolean(cpu_props, prop->name,
+prop->value.boolean) < 0)
+goto cleanup;
+break;
+
+case QEMU_MONITOR_CPU_PROPERTY_STRING:
+if (virJSONValueObjectAppendString(cpu_props, prop->name,
+   prop->value.string) < 0)
+goto cleanup;
+break;
+
+case QEMU_MONITOR_CPU_PROPERTY_NUMBER:
+if (virJSONValueObjectAppendNumberLong(cpu_props, prop->name,
+   prop->value.number) < 0)
+goto cleanup;
+break;
+
+case QEMU_MONITOR_CPU_PROPERTY_LAST:
+default:
+virReportEnumRangeError(qemuMonitorCPUPropertyPtr, prop->type);
+goto cleanup;
+}
+}
+
+ignore_value(virJSONValueObjectCreate(_json, "s:name", model->name,
+  "a:props", _props, NULL));
+
+ cleanup:
+virJSONValueFree(cpu_props);
+return model_json;
+}
+
+
+/* model_json: {"name": "IvyBridge", "props": {}}
+ */
+static qemuMonitorCPUModelInfoPtr
+qemuMonitorJSONBuildCPUModelInfoFromJSON(virJSONValuePtr cpu_model)
+{
+virJSONValuePtr cpu_props;
+qemuMonitorCPUModelInfoPtr machine_model = NULL;
+qemuMonitorCPUModelInfoPtr model = NULL;
+char const *cpu_name;
+
+if (!(cpu_name = virJSONValueObjectGetString(cpu_model, "name"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Parsed JSON reply missing 'name'"));
+goto cleanup;
+}
+
+if (VIR_ALLOC(machine_model) < 0)
+goto cleanup;
+
+if (VIR_STRDUP(machine_model->name, cpu_name) < 0)
+goto cleanup;
+
+if ((cpu_props = virJSONValueObjectGetObject(cpu_model, "props"))) {
+if (VIR_ALLOC_N(machine_model->props,
+virJSONValueObjectKeysNumber(cpu_props)) < 0)
+goto cleanup;
+
+if (virJSONValueObjectForeachKeyValue(cpu_props,
+  
qemuMonitorJSONParseCPUModelProperty,
+  machine_model) < 0)
+goto cleanup;
+}
+
+VIR_STEAL_PTR(model, machine_model);
+
+ cleanup:
+qemuMonitorCPUModelInfoFree(machine_model);
+
+return model;
+}
+
 int
 qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
 qemuMonitorCPUModelExpansionType type,
@@ -5372,9 +5467,6 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
 virJSONValuePtr reply = NULL;
 virJSONValuePtr data;
 virJSONValuePtr cpu_model;
-virJSONValuePtr cpu_props;
-qemuMonitorCPUModelInfoPtr machine_model = NULL;
-char const *cpu_name;
 const char *typeStr = "";
 
 *model_info = NULL;
@@ -5447,38 +5539,12 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
 goto retry;
 }
 
-if (!(cpu_name = virJSONValueObjectGetString(cpu_model, "name"))) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("query-cpu-model-expansion reply data was missing 
'name'"));
-goto cleanup;
-}
-
-if (!(cpu_props = virJSONValueObjectGetObject(cpu_model, "props"))) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("query-cpu-model-expansion reply data was missing 
'props'"));
-goto cleanup;
-}
-
-if (VIR_ALLOC(machine_model) < 0)
-goto cleanup;
-
-if (VIR_STRDUP(machine_model->name, cpu_name) < 0)
-goto cleanup;
-
-if (VIR_ALLOC_N(machine_model->props, 
virJSONValueObjectKeysNumber(cpu_props)) < 0)
-goto cleanup;
-
-if (virJSONValueObjectForeachKeyValue(cpu_props,
-  qemuMonitorJSONParseCPUModelProperty,
-   

[libvirt] [PATCHv1 00/12] BaselineHypervisorCPU using QEMU QMP exchanges

[Chris Venteicher]

Some architectures (S390) depend on QEMU to compute baseline CPU model and 
expand a models feature set.

Interacting with QEMU requires starting the QEMU process and completing one or
more query-cpu-model-baseline QMP exchanges with QEMU in addition to a 
query-cpu-model-expansion QMP exchange to expand all features in the model.

See "s390x CPU models: exposing features" patch set on Qemu-devel for discussion
of QEMU aspects.

This is part of resolution of: 
https://bugzilla.redhat.com/show_bug.cgi?id=1511999

Signed-off-by: Chris Venteicher 

Chris Venteicher (12):
  qemu_monitor_json: qemuMonitorCPUModelInfo / JSON conversion
  qemu_monitor: Introduce qemuMonitorCPUModelInfoInit and
qemuMonitorCPUModelInfoFreeContents
  qemu_monitor: Introduce qemuMonitorGetCPUModelBaseline
(query-cpu-model-baseline)
  qemu_monitor: Indicate when CPUModelInfo props report migratablity
  qemu_monitor: CPUModelExpansion on CPUModel with both name and
properties
  cpu_conf: Calculate virCPUDef List Length function
  qemu_capabilities: virCPUDef / qemuMonitorCPUModelInfo conversions
  qemu_capabilities: QMPCommandPtr without shadow qmperr
  qemu_capabilities: Persist QEMU instance over multiple QMP Cmds
  qemu_capabilities: Introduce virQEMUCapsNewQMPCommandConnection
  qemu_capabilities: Introduce virQEMUCapsQMPBaselineCPUModel (baseline
using QEMU)
  qemu_driver: BaselineHypervisorCPU supports S390 using QEMU/QMP

 src/conf/cpu_conf.c  |  15 ++
 src/conf/cpu_conf.h  |   3 +
 src/libvirt_private.syms |   1 +
 src/qemu/qemu_capabilities.c | 335 +--
 src/qemu/qemu_capabilities.h |  33 
 src/qemu/qemu_driver.c   |  66 ++-
 src/qemu/qemu_monitor.c  |  90 +-
 src/qemu/qemu_monitor.h  |  16 +-
 src/qemu/qemu_monitor_json.c | 226 +++
 src/qemu/qemu_monitor_json.h |  14 +-
 tests/cputest.c  |   7 +-
 11 files changed, 683 insertions(+), 123 deletions(-)

-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCHv1 06/12] cpu_conf: Calculate virCPUDef List Length function

[Chris Venteicher]

Function returns number of virCPUDefPtrs in list
---
 src/conf/cpu_conf.c  | 15 +++
 src/conf/cpu_conf.h  |  3 +++
 src/libvirt_private.syms |  1 +
 3 files changed, 19 insertions(+)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 43a3ab5dcd..ff978ec083 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -1039,3 +1039,18 @@ virCPUDefListFree(virCPUDefPtr *cpus)
 
 VIR_FREE(cpus);
 }
+
+
+/*
+ * Return number of virCPUDefPtrs in list
+ */
+size_t
+virCPUDefListLength(virCPUDefPtr *cpus)
+{
+size_t i = 0;
+
+while (cpus && cpus[i])
+i++;
+
+return i;
+}
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index 9f2e7ee264..a0743e5d9b 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -227,4 +227,7 @@ virCPUDefListParse(const char **xmlCPUs,
 void
 virCPUDefListFree(virCPUDefPtr *cpus);
 
+size_t
+virCPUDefListLength(virCPUDefPtr *cpus);
+
 #endif /* __VIR_CPU_CONF_H__ */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 98913a577a..66e74e3fb7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -85,6 +85,7 @@ virCPUDefFreeFeatures;
 virCPUDefFreeModel;
 virCPUDefIsEqual;
 virCPUDefListFree;
+virCPUDefListLength;
 virCPUDefListParse;
 virCPUDefParseXML;
 virCPUDefStealModel;
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCHv1 09/12] qemu_capabilities: Persist QEMU instance over multiple QMP Cmds

[Chris Venteicher]

Makes possible to start a single QEMU instance and use that one instance
for multiple independent QMP commands without starting and stopping QEMU
for each QMP command type.

This allows functions outside qemu_capabilities
(ex.  qemuConnectBaselineHypervisorCPU in qemu_driver)
requiring multiple different QMP messages to be sent to QEMU to issue
those requests over a single QMP Command Channel without Starting and
Stopping QEMU for each independent QMP Command usage.

Now in Global Scope:
struct virQEMUCapsInitQMPCommand
virQEMUCapsInitQMPCommandFree
---
 src/qemu/qemu_capabilities.c | 21 +
 src/qemu/qemu_capabilities.h | 21 +
 2 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index f2202b652d..2bbbfc83f3 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4235,25 +4235,6 @@ virQEMUCapsInitQMPMonitorTCG(virQEMUCapsPtr qemuCaps 
ATTRIBUTE_UNUSED,
 }
 
 
-typedef struct _virQEMUCapsInitQMPCommand virQEMUCapsInitQMPCommand;
-typedef virQEMUCapsInitQMPCommand *virQEMUCapsInitQMPCommandPtr;
-struct _virQEMUCapsInitQMPCommand {
-char *binary;
-uid_t runUid;
-gid_t runGid;
-char **qmperr;
-char *qmperr_internal;
-char *monarg;
-char *monpath;
-char *pidfile;
-virCommandPtr cmd;
-qemuMonitorPtr mon;
-virDomainChrSourceDef config;
-pid_t pid;
-virDomainObjPtr vm;
-};
-
-
 static void
 virQEMUCapsInitQMPCommandAbort(virQEMUCapsInitQMPCommandPtr cmd)
 {
@@ -4288,7 +4269,7 @@ 
virQEMUCapsInitQMPCommandAbort(virQEMUCapsInitQMPCommandPtr cmd)
 }
 
 
-static void
+void
 virQEMUCapsInitQMPCommandFree(virQEMUCapsInitQMPCommandPtr cmd)
 {
 if (!cmd)
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index af263c9780..c9618bb754 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -488,6 +488,27 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
 
+typedef struct _virQEMUCapsInitQMPCommand virQEMUCapsInitQMPCommand;
+typedef virQEMUCapsInitQMPCommand *virQEMUCapsInitQMPCommandPtr;
+
+struct _virQEMUCapsInitQMPCommand {
+char *binary;
+uid_t runUid;
+gid_t runGid;
+char **qmperr;
+char *qmperr_internal;
+char *monarg;
+char *monpath;
+char *pidfile;
+virCommandPtr cmd;
+qemuMonitorPtr mon;
+virDomainChrSourceDef config;
+pid_t pid;
+virDomainObjPtr vm;
+};
+
+void virQEMUCapsInitQMPCommandFree(virQEMUCapsInitQMPCommandPtr cmd);
+
 typedef struct _virQEMUCaps virQEMUCaps;
 typedef virQEMUCaps *virQEMUCapsPtr;
 
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCHv1 04/12] qemu_monitor: Indicate when CPUModelInfo props report migratablity

[Chris Venteicher]

Renamed variable in CPUModelInfo such that
props_migratable_valid is true when properties in CPUModelInfo
indicate if property is / isn't migratable.

Property migratability is not part of QMP messages but rather is
sometimes calculated within Libvirt and stored in CPUModelInfo
properties.
---
 src/qemu/qemu_capabilities.c | 10 +-
 src/qemu/qemu_monitor.c  |  2 +-
 src/qemu/qemu_monitor.h  |  2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 419208ad5c..0c5e4bb766 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2400,7 +2400,7 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps,
 }
 }
 
-modelInfo->migratability = true;
+modelInfo->props_migratable_valid = true;
 }
 
 VIR_STEAL_PTR(cpuData->info, modelInfo);
@@ -2455,7 +2455,7 @@ virQEMUCapsGetCPUFeatures(virQEMUCapsPtr qemuCaps,
 }
 
 VIR_STEAL_PTR(*features, list);
-if (migratable && !data->info->migratability)
+if (migratable && !data->info->props_migratable_valid)
 ret = 1;
 else
 ret = 0;
@@ -2854,7 +2854,7 @@ virQEMUCapsInitCPUModel(virQEMUCapsPtr qemuCaps,
 virQEMUCapsHostCPUDataPtr cpuData = virQEMUCapsGetHostCPUData(qemuCaps, 
type);
 int ret = 1;
 
-if (migratable && cpuData->info && !cpuData->info->migratability)
+if (migratable && cpuData->info && !cpuData->info->props_migratable_valid)
 return 1;
 
 if (ARCH_IS_S390(qemuCaps->arch)) {
@@ -3037,7 +3037,7 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
_("invalid migratability value for host CPU model"));
 goto cleanup;
 }
-hostCPU->migratability = val == VIR_TRISTATE_BOOL_YES;
+hostCPU->props_migratable_valid = val == VIR_TRISTATE_BOOL_YES;
 VIR_FREE(str);
 
 ctxt->node = hostCPUNode;
@@ -3530,7 +3530,7 @@ virQEMUCapsFormatHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
 virBufferAsprintf(buf,
   "\n",
   typeStr, model->name,
-  model->migratability ? "yes" : "no");
+  model->props_migratable_valid ? "yes" : "no");
 virBufferAdjustIndent(buf, 2);
 
 for (i = 0; i < model->nprops; i++) {
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index c5c640b0d8..3fd73e2cd1 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3702,7 +3702,7 @@ qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo 
*orig)
 if (VIR_STRDUP(copy->name, orig->name) < 0)
 goto error;
 
-copy->migratability = orig->migratability;
+copy->props_migratable_valid = orig->props_migratable_valid;
 copy->nprops = orig->nprops;
 
 for (i = 0; i < orig->nprops; i++) {
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 15bebb2aaf..08b787e28c 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1005,7 +1005,7 @@ struct _qemuMonitorCPUModelInfo {
 char *name;
 size_t nprops;
 qemuMonitorCPUPropertyPtr props;
-bool migratability;
+bool props_migratable_valid;
 };
 
 typedef enum {
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCHv1 02/12] qemu_monitor: Introduce qemuMonitorCPUModelInfoInit and qemuMonitorCPUModelInfoFreeContents

[Chris Venteicher]

Init - Initial with model name and empty properties
FreeContents - Free model name and properties without freeing pointer
---
 src/qemu/qemu_monitor.c | 37 -
 src/qemu/qemu_monitor.h |  4 
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index d6771c1d52..16de54dac7 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3632,8 +3632,31 @@ qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,
 }
 
 
+int
+qemuMonitorCPUModelInfoInit(const char *name, qemuMonitorCPUModelInfoPtr model)
+{
+int ret = -1;
+
+if (!model)
+goto cleanup;
+
+model->name = NULL;
+model->nprops = 0;
+model->props = NULL;
+model->props_migratable_valid = false;
+
+if (VIR_STRDUP(model->name, name) < 0)
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+return ret;
+}
+
+
 void
-qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info)
+qemuMonitorCPUModelInfoFreeContents(qemuMonitorCPUModelInfoPtr model_info)
 {
 size_t i;
 
@@ -3648,6 +3671,18 @@ qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr 
model_info)
 
 VIR_FREE(model_info->props);
 VIR_FREE(model_info->name);
+
+model_info->name = NULL;
+model_info->nprops = 0;
+model_info->props = NULL;
+model_info->props_migratable_valid = false;
+}
+
+
+void
+qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info)
+{
+qemuMonitorCPUModelInfoFreeContents(model_info);
 VIR_FREE(model_info);
 }
 
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index b3d62324b4..7dbc993f1b 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1021,6 +1021,10 @@ int qemuMonitorGetCPUModelExpansion(qemuMonitorPtr mon,
 qemuMonitorCPUModelInfoPtr *model_info);
 
 void qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info);
+void qemuMonitorCPUModelInfoFreeContents(qemuMonitorCPUModelInfoPtr 
model_info);
+
+int qemuMonitorCPUModelInfoInit(const char *name, qemuMonitorCPUModelInfoPtr 
model)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
 qemuMonitorCPUModelInfoPtr
 qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig);
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCHv1 03/12] qemu_monitor: Introduce qemuMonitorGetCPUModelBaseline (query-cpu-model-baseline)

[Chris Venteicher]

Wrap QMP query-cpu-model-baseline command transaction with QEMU.
---
 src/qemu/qemu_monitor.c  | 13 
 src/qemu/qemu_monitor.h  |  5 +++
 src/qemu/qemu_monitor_json.c | 65 
 src/qemu/qemu_monitor_json.h |  7 
 4 files changed, 90 insertions(+)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 16de54dac7..c5c640b0d8 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3738,6 +3738,19 @@ qemuMonitorCPUModelInfoCopy(const 
qemuMonitorCPUModelInfo *orig)
 return NULL;
 }
 
+int
+qemuMonitorGetCPUModelBaseline(qemuMonitorPtr mon,
+   qemuMonitorCPUModelInfoPtr model_a,
+   qemuMonitorCPUModelInfoPtr model_b,
+   qemuMonitorCPUModelInfoPtr *model_baseline)
+{
+VIR_DEBUG("model_a->name=%s", model_a->name);
+VIR_DEBUG("model_b->name=%s", model_b->name);
+
+QEMU_CHECK_MONITOR(mon);
+
+return qemuMonitorJSONGetCPUModelBaseline(mon, model_a, model_b, 
model_baseline);
+}
 
 int
 qemuMonitorGetCommands(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 7dbc993f1b..15bebb2aaf 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1029,6 +1029,11 @@ int qemuMonitorCPUModelInfoInit(const char *name, 
qemuMonitorCPUModelInfoPtr mod
 qemuMonitorCPUModelInfoPtr
 qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig);
 
+int qemuMonitorGetCPUModelBaseline(qemuMonitorPtr mon,
+   qemuMonitorCPUModelInfoPtr model_a,
+   qemuMonitorCPUModelInfoPtr model_b,
+   qemuMonitorCPUModelInfoPtr *model_baseline);
+
 int qemuMonitorGetCommands(qemuMonitorPtr mon,
char ***commands);
 int qemuMonitorGetEvents(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index afbf000fa2..729578414b 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5553,6 +5553,71 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
 }
 
 
+/* Note: *model_baseline == NULL && return == 0 if command not supported by 
QEMU
+ */
+int
+qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon,
+   qemuMonitorCPUModelInfoPtr model_a,
+   qemuMonitorCPUModelInfoPtr model_b,
+   qemuMonitorCPUModelInfoPtr *model_baseline)
+{
+int ret = -1;
+virJSONValuePtr cmd = NULL;
+virJSONValuePtr reply = NULL;
+virJSONValuePtr data = NULL;
+virJSONValuePtr modela = NULL;
+virJSONValuePtr modelb = NULL;
+virJSONValuePtr cpu_model = NULL;
+
+*model_baseline = NULL;
+
+if (!(modela = qemuMonitorJSONBuildCPUModelInfoToJSON(model_a)))
+goto cleanup;
+
+if (!(modelb = qemuMonitorJSONBuildCPUModelInfoToJSON(model_b)))
+goto cleanup;
+
+if (!(cmd = qemuMonitorJSONMakeCommand("query-cpu-model-baseline",
+   "a:modela", ,
+   "a:modelb", ,
+   NULL)))
+goto cleanup;
+
+if (qemuMonitorJSONCommand(mon, cmd, ) < 0)
+goto cleanup;
+
+if (qemuMonitorJSONHasError(reply, "GenericError")) {
+/* QEMU does not support query-cpu-model-baseline or cpu model */
+ret = 0;
+goto cleanup;
+}
+
+if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0)
+goto cleanup;
+
+data = virJSONValueObjectGetObject(reply, "return");
+
+if (!(cpu_model = virJSONValueObjectGetObject(data, "model"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("query-cpu-model-baseline reply data was missing 
'model'"));
+goto cleanup;
+}
+
+if (!(*model_baseline = 
qemuMonitorJSONBuildCPUModelInfoFromJSON(cpu_model)))
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+virJSONValueFree(modela);
+virJSONValueFree(modelb);
+
+return ret;
+}
+
+
 int qemuMonitorJSONGetCommands(qemuMonitorPtr mon,
char ***commands)
 {
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 6bc0dd3ad2..73e1cb6ace 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -367,6 +367,13 @@ int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
 qemuMonitorCPUModelInfoPtr *model_info)
 ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(5);
 
+int qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon,
+   qemuMonitorCPUModelInfoPtr model_a,
+   qemuMonitorCPUModelInfoPtr model_b,
+   qemuMonitorCPUModelInfoPtr 

Re: [libvirt] [GSoC] Code design for scalar and external types

[Sukrit Bhatnagar]

On Mon, 18 Jun 2018 at 17:20, Andrea Bolognani  wrote:
>
> On Mon, 2018-06-18 at 11:52 +0100, Daniel P. Berrangé wrote:
> > On Mon, Jun 18, 2018 at 12:24:39PM +0200, Pavel Hrdina wrote:
> > > # define VIR_DEFINE_AUTOCLEAR_FUNC(type, func) \
> > > static inline void VIR_AUTOCLEAR_FUNC_NAME(type)(type *_ptr) \
> > > { \
> > > (func)(_ptr); \
> > > }
> >
> > For anything where we define the impl of (func) these two macros
> > should never be used IMHO. We should just change the signature of
> > the real functions so that accept a double pointer straight away,
> > and thus not require the wrapper function. Yes, it this will
> > require updating existing code, but such a design change is
> > beneficial even without the cleanup macros, as it prevents the
> > possbility of double frees. I'd suggest we just do this kind of
> > change straightaway
>
> Agreed that we want to fix our own free functions.
>
> > > # define VIR_AUTOFREE(type) __attribute__((cleanup(virFree))) type
> >
> > I don't see the point in passing "type" in here we could simplify
> > this:
> >
> >   #define VIR_AUTOFREE __attribute__((cleanup(virFree))
> >
> >   VIR_AUTOFREE char *foo = NULL;
>
> Passing the type was suggested earlier to make usage consistent
> across all cases, eg.
>
>   VIR_AUTOFREE(char *) str = NULL;
>   VIR_AUTOPTR(virDomain) dom = NULL;
>
> and IIRC we ended up agreeing that it was a good idea overall.
>
> > > # define VIR_AUTOPTR(type) \
> > > __attribute__((cleanup(VIR_AUTOPTR_FUNC_NAME(type 
> > > VIR_AUTOPTR_TYPE_NAME(type)
> > >
> > >   The usage would not require our internal vir*Ptr types and would
> > >   be easy to use with external types as well:
> > >
> > > VIR_DEFINE_AUTOPTR_FUNC(virBitmap, virBitmapFree);
> > >
> > > VIR_AUTOPTR(virBitmap) map = NULL;
> > >
> > > VIR_DEFINE_AUTOPTR_FUNC(LIBSSH2_CHANNEL, libssh2_channel_free);
> >
> > Contrary to my general point above about VIR_DEFINE_AUTOPTR_FUNC,
> > this is a reasonable usage, because we don't control the signature
> > of the libssh2_channel_free function.
>
> This is why I suggested we might want to have two sets of
> macros, one for types we defined ourselves and one for types
> we bring in from external libraries.
>
> > > VIR_AUTOPTR(LIBSSH2_CHANNEL) channel = NULL;
> >
> > With my example above
> >
> >#define VIR_AUTOFREE_LIBSSH_CHANNEL VIR_AUTOFREE_FUNC(LIBSSH2_CHANNEL)
> >
> >VIR_AUTOFREE_LIBSSH_CHANNEL LIBSSH_CHANNEL *chan = NULL;
>
> This makes the declaration needlessly verbose, since you're
> repeating the type name twice; Pavel's approach would avoid
> that.

So, have we reached a decision about the design?

Fixing the existing vir*Free functions to take double pointers seems good to me,
as many have mentioned earlier. It will solve multiple problems at once.
I have almost 7 weeks left of GSoC, and I think it is totally doable.

On the other hand, making cleanup macros use double pointers also seems
great! But, aren't we borrowing too much design from GLib?

Anyway, both approaches take care of the double free problem.

TL;DR
I prefer cleanup macros which define functions with double pointers
over changing
existing vir*Free functions. It just looks more natural.

I apologize in advance for this long mail! Could not decide what
patches to make,
so I included a glimpse of the code in this mail.
This is just to start this discussion again and help with visualizing.

Here goes some example code. Most of it is paraphrasing what is already
discussed previously.

/**
* Cleanup macros defining functions with double pointers:
**/
#define VIR_AUTOPTR_TYPE_NAME(type) type##AutoPtr
#define VIR_AUTOPTR_FUNC_NAME(type) \
VIR_AUTOPTR_TYPE_NAME(type)##Free
#define VIR_DEFINE_AUTOPTR_FUNC(type, func) \
typedef type *VIR_AUTOPTR_TYPE_NAME(type); \
static inline void VIR_AUTOPTR_FUNC_NAME(type)(type **_ptr) \
{ \
if (*_ptr) \
(func)(*_ptr); \
*_ptr = NULL; \
} \
#define VIR_AUTOPTR(type) \
__attribute__((cleanup(VIR_AUTOPTR_FUNC_NAME(type \
VIR_AUTOPTR_TYPE_NAME(type)


/ Using this for virBitmap:

VIR_DEFINE_AUTOPTR_FUNC(virBitmap, virBitmapFree)
will generate:

typedef virBitmap *virBitmapAutoPtr;
static inline void virBitmapAutoPtrFree(virBitmap **_ptr)
{
if (*_ptr)
virBitmapFree(*_ptr);
*_ptr = NULL;
}

Then we use it like:
VIR_AUTOPTR(virBitmap) map = NULL;

which will become:
__attribute__((cleanup(virBitmapAutoPtrFree))) virBitmapAutoPtr map = NULL;


/ Using this for char ** (some of my ideas added):

typedef char *virString;

VIR_DEFINE_AUTOPTR_FUNC(virString, virStringListFree)
will generate:

typedef virString *virStringAutoPtr;
static inline void virStringAutoPtrFree(virString **_ptr)
{
if (*_ptr)
virStringListFree(*_ptr);
*_ptr = NULL;
}

Re: [libvirt] [PATCH 2/2] qemu: Escape commas for qemuBuildSCSIiSCSIHostdevDrvStr

[John Ferlan]

On 06/20/2018 09:17 AM, Anya Harter wrote:
> Add comma escaping for netsource
> 
> Signed-off-by: Anya Harter 
> ---
>  src/qemu/qemu_command.c | 4 +++-
>  tests/qemuxml2argvdata/name-escape.args | 6 +-
>  tests/qemuxml2argvdata/name-escape.xml  | 7 +++
>  tests/qemuxml2argvtest.c| 5 -
>  4 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index a99240992a..96c6c08c2d 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4677,7 +4677,9 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr 
> dev,
>  if (!(netsource = qemuBuildNetworkDriveStr(iscsisrc->src, srcPriv ?
> srcPriv->secinfo : NULL)))
>  goto cleanup;
> -virBufferAsprintf(, "file=%s,if=none,format=raw", netsource);
> +virBufferAddLit(, "file=");
> +virQEMUBuildBufferEscapeComma(, netsource);
> +virBufferAddLit(, ",if=none,format=raw");
>  }
>  
>  if (virBufferCheckError() < 0)
> diff --git a/tests/qemuxml2argvdata/name-escape.args 
> b/tests/qemuxml2argvdata/name-escape.args
> index aef7c238ca..1cbb1efd66 100644
> --- a/tests/qemuxml2argvdata/name-escape.args
> +++ b/tests/qemuxml2argvdata/name-escape.args
> @@ -22,6 +22,7 @@ bar=2/monitor.sock,server,nowait \
>  -no-shutdown \
>  -no-acpi \
>  -boot c \
> +-device lsi,id=scsi0,bus=pci.0,addr=0x3 \

[1]

>  -device usb-ccid,id=ccid0,bus=usb.0,port=1 \
>  -usb \
>  -drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-ide0-0-0,\
> @@ -41,4 +42,7 @@ 
> cert3=cert3,db=/etc/pki/nssdb,,foo,id=smartcard0,bus=ccid0.0 \
>  -spice unix,addr=/tmp/lib/domain--1-foo=1,,bar=2/spice.sock,gl=on,\
>  rendernode=/dev/dri/foo,,bar \
>  -device cirrus-vga,id=video0,bus=pci.0,addr=0x2 \
> --device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
> +-drive file=iscsi://example,,foo.org:3260/iqn.1992-01.com.example/0,if=none,\
> +format=raw,id=drive-hostdev0 \
> +-device scsi-generic,bus=scsi0.0,scsi-id=4,drive=drive-hostdev0,id=hostdev0 \
> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
> diff --git a/tests/qemuxml2argvdata/name-escape.xml 
> b/tests/qemuxml2argvdata/name-escape.xml
> index 70a1ce09d3..24dd248842 100644
> --- a/tests/qemuxml2argvdata/name-escape.xml
> +++ b/tests/qemuxml2argvdata/name-escape.xml
> @@ -27,6 +27,13 @@
>
>
>  
> +

[1] If you change this to:



Then the lsi doesn't show up...

> +
> +  
> +
> +  

Something is not quite right about seeing "example,foo.org" as a host
name. I don't see that as a valid host name regardless of comma
escaping. Do we escape commas in other "" string
fields? I don't see it being done for qemuBuildChrChardevStr (search on
TCP or "host.*name" in qemu_command.c).

Now the one thing that perhaps *could* be escaped is the  name
attribute. Currently it's set to "iqn.1992-01.com.example", but the spec
defines a structure of that name field to have a field that is a "String
defined by the naming authority" (shorter version, see
https://en.wikipedia.org/wiki/ISCSI)

So if the name is : 'iqn.1992-01.com.example:storage/2', then the
"storage/2" piece is the string defined by the naming authority. Not
that I know whether it would work, but that's where I suppose a comma
could go.

> +  
> +
>  
>
>  
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 582a9de7bb..2e891a0bea 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -2772,7 +2772,10 @@ mymain(void)
>  QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
>  QEMU_CAPS_DEVICE_ISA_SERIAL,
>  QEMU_CAPS_CHARDEV_FILE_APPEND,
> -QEMU_CAPS_CCID_EMULATED);
> +QEMU_CAPS_CCID_EMULATED,
> +QEMU_CAPS_VIRTIO_SCSI,
> +QEMU_CAPS_SCSI_LSI,

[1] and the LSI wouldn't be needed here.

John

first patch looks fine, but I'm not so convinced on this one only
because of the host name w/ a comma.

> +QEMU_CAPS_DEVICE_SCSI_GENERIC);
>  DO_TEST("debug-threads", QEMU_CAPS_NAME_DEBUG_THREADS);
>  
>  DO_TEST("master-key", QEMU_CAPS_OBJECT_SECRET);
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] qemu: Escape commas for qemuBuildDiskThrottling

[John Ferlan]

On 06/19/2018 12:20 PM, Anya Harter wrote:
> Add comma escaping for disk->blkdeviotune.group_name.
> 
> Signed-off-by: Anya Harter 
> ---
>  src/qemu/qemu_command.c |  4 ++--
>  tests/qemuxml2argvdata/name-escape.args |  5 +
>  tests/qemuxml2argvdata/name-escape.xml  | 13 +
>  tests/qemuxml2argvtest.c|  2 ++
>  4 files changed, 22 insertions(+), 2 deletions(-)
> 

Reviewed-by: John Ferlan 

(and pushed)

Tks -

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 4/4] news: add cmdDomblkinfo --all option

[John Ferlan]

On 06/19/2018 06:01 AM, Chen Hanxiao wrote:
> From: Chen Hanxiao 
> 
> Update news for cmdDomblkinfo --all option.
> 
> Signed-off-by: Chen Hanxiao 
> ---
> v3:
>   update descriptions
> 
>  docs/news.xml | 10 ++
>  1 file changed, 10 insertions(+)
> 

This needed to be in the 4.5.0 section, not the 4.4.0 section of news.

Moved and will push shortly,

Reviewed-by: John Ferlan 

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 3/4] cmdDomblkinfoPrint: support printing "-" for invalid virDomainBlockInfo

[John Ferlan]

On 06/19/2018 06:01 AM, Chen Hanxiao wrote:
> From: Chen Hanxiao 
> 
> For inactive domain, we'll set virDomainBlockInfo to 0
> if specific error code got.
> Print "-" to show the value should be ignored in this scenario.
> 
> Signed-off-by: Chen Hanxiao 
> ---
>  tools/virsh-domain-monitor.c | 73 
>  1 file changed, 49 insertions(+), 24 deletions(-)
> 

I understand why it's separate for ease of reading, but this
should have been combined with the previous patch.

> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index 43e39f79c1..3acf5450b3 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -410,6 +410,15 @@ cmdDomblkinfoPrint(vshControl *ctl,
> bool human, bool title)
>  {
>  char *cap = NULL, *alloc = NULL, *phy = NULL;
> +bool invalid = false;
> +
> +struct blockInfoText {
> +char *capacity;
> +char *allocation;
> +char *physical;
> +};
> +
> +struct blockInfoText *blkInfoText = NULL;

All this is over complicated an unnecessary - especially since
you already have cap, alloc, and phy which now went unused!
'll fix before pushing.

>  
>  if (title) {
>  vshPrintExtra(ctl, "%-10s %-15s %-15s %-15s\n", _("Target"),
> @@ -419,15 +428,23 @@ cmdDomblkinfoPrint(vshControl *ctl,
>  return;
>  }
>  
> -if (!human) {
> -if (device) {
> -vshPrint(ctl, "%-10s %-15llu %-15llu %-15llu\n", device,
> - info->capacity, info->allocation, info->physical);
> -} else {
> -vshPrint(ctl, "%-15s %llu\n", _("Capacity:"), info->capacity);
> -vshPrint(ctl, "%-15s %llu\n", _("Allocation:"), 
> info->allocation);
> -vshPrint(ctl, "%-15s %llu\n", _("Physical:"), info->physical);
> -}
> +invalid = info->capacity == 0 &&
> +  info->allocation == 0 &&
> +  info->physical == 0;
> +blkInfoText = vshCalloc(ctl, 1, sizeof(*blkInfoText));
> +
> +if (invalid) {

@invalid is unnecessary since it's used just once - I'll adjust.

> +blkInfoText->capacity = vshStrdup(ctl, "-");
> +blkInfoText->allocation = vshStrdup(ctl, "-");
> +blkInfoText->physical = vshStrdup(ctl, "-");
> +} else if (!human) {
> +if (virAsprintf(>capacity, "%llu",
> +info->capacity) < 0 ||
> +virAsprintf(>allocation, "%llu",
> +info->allocation) < 0 ||
> +virAsprintf(>physical, "%llu",
> +info->physical) < 0)
> +goto cleanup;
>  } else {
>  double val_cap, val_alloc, val_phy;
>  const char *unit_cap, *unit_alloc, *unit_phy;
> @@ -435,28 +452,36 @@ cmdDomblkinfoPrint(vshControl *ctl,
>  val_cap = vshPrettyCapacity(info->capacity, _cap);
>  val_alloc = vshPrettyCapacity(info->allocation, _alloc);
>  val_phy = vshPrettyCapacity(info->physical, _phy);
> -if (device) {
> -if (virAsprintf(, "%.3lf %s", val_cap, unit_cap) < 0 ||
> -virAsprintf(, "%.3lf %s", val_alloc, unit_alloc) < 0 ||
> -virAsprintf(, "%.3lf %s", val_phy, unit_phy) < 0)
> -goto cleanup;
>  
> -vshPrint(ctl, "%-10s %-15s %-15s %-15s\n",
> - device, cap, alloc, phy);
> -} else {
> -vshPrint(ctl, "%-15s %-.3lf %s\n", _("Capacity:"),
> - val_cap, unit_cap);
> -vshPrint(ctl, "%-15s %-.3lf %s\n", _("Allocation:"),
> - val_alloc, unit_alloc);
> -vshPrint(ctl, "%-15s %-.3lf %s\n", _("Physical:"),
> - val_phy, unit_phy);
> -}
> +if (virAsprintf(>capacity, "%.3lf %s",
> +val_cap, unit_cap) < 0 ||
> +virAsprintf(>allocation, "%.3lf %s",
> +val_alloc, unit_alloc) < 0 ||
> +virAsprintf(>physical, "%.3lf %s",
> +val_phy, unit_phy) < 0)
> +goto cleanup;
> +}
> +
> +if (device) {
> +vshPrint(ctl, "%-10s %-15s %-15s %-15s\n", device,
> + blkInfoText->capacity, blkInfoText->allocation,
> + blkInfoText->physical);
> +} else {
> +vshPrint(ctl, "%-15s %s\n", _("Capacity:"), blkInfoText->capacity);
> +vshPrint(ctl, "%-15s %s\n", _("Allocation:"), 
> blkInfoText->allocation);
> +vshPrint(ctl, "%-15s %s\n", _("Physical:"), blkInfoText->physical);
>  }
>  
>   cleanup:
>  VIR_FREE(cap);
>  VIR_FREE(alloc);
>  VIR_FREE(phy);
> +if (blkInfoText) {
> +VIR_FREE(blkInfoText->capacity);
> +VIR_FREE(blkInfoText->allocation);
> +VIR_FREE(blkInfoText->physical);
> +}
> +VIR_FREE(blkInfoText);
>  }
>  
>  static bool
> 

NB: I'm going to merge this into the previous, adding
the following to the 

Re: [libvirt] [PATCH v3 2/4] cmdDomblkinfo: add --all to show all block devices info

[John Ferlan]

On 06/19/2018 06:01 AM, Chen Hanxiao wrote:
> From: Chen Hanxiao 
> 
> This patch introduces --all to show all block devices info
> of guests like:
> 
> virsh # domblkinfo w08 --all
> Target CapacityAllocation  Physical
> ---
> hda42949672960 9878110208  9878110208
> vda10737418240 10736439296 10737418240
> 
> Target CapacityAllocation  Physical
> ---
> hda40.000 GiB  9.200 GiB   9.200 GiB
> vda10.000 GiB  9.999 GiB   10.000 GiB
> 
> Signed-off-by: Chen Hanxiao 
> ---
> v3:
>   check error code on network disk
> v2:
>   add support --human to --all
> v1.1:
>   fix self-test
> 
>  tools/virsh-domain-monitor.c | 128 +--
>  tools/virsh.pod  |   5 +-
>  2 files changed, 112 insertions(+), 21 deletions(-)
> 

Made a few adjustments as noted below... and will push shortly.

Reviewed-by: John Ferlan 

John

> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index daa86e8310..43e39f79c1 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c

[...]

>  
>  static bool
> @@ -430,25 +466,77 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
>  virDomainPtr dom;
>  bool ret = false;
>  bool human = false;

[...]

> +
> +for (i = 0; i < ndisks; i++) {
> +ctxt->node = disks[i];
> +protocol = virXPathString("string(./source/@protocol)", ctxt);
> +target = virXPathString("string(./target/@dev)", ctxt);
> +
> +rc = virDomainGetBlockInfo(dom, target, , 0);
> +
> +if (rc < 0) {

Added the following comment:

/* If protocol is present that's an indication of a networked
 * storage device which cannot provide statistics, so generate
 * 0 based data and get the next disk. */

> +if (protocol && !active &&
> +virGetLastErrorCode() == VIR_ERR_INTERNAL_ERROR &&
> +virGetLastErrorDomain() == VIR_FROM_STORAGE)
> +memset(, 0, sizeof(info));

Since we're ditching the error and continuing:

vshResetLibvirtError();

> +else
> +goto cleanup;
> +}
> +
> +cmdDomblkinfoPrint(ctl, , target, human, false);
> +
> +VIR_FREE(target);

Since we're in the loop:

VIR_FREE(protocol);

> +}
> +} else {
> +if (virDomainGetBlockInfo(dom, device, , 0) < 0)
> +goto cleanup;
> +
> +cmdDomblkinfoPrint(ctl, , NULL, human, false);
> +}
>  
>  ret = true;
[...]

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC PATCH 0/2] Add new mdev type for aggregated resources

[Alex Williamson]

On Thu, 21 Jun 2018 19:57:38 +0530
Kirti Wankhede  wrote:

> On 6/20/2018 1:10 PM, Zhenyu Wang wrote:
> > Current mdev device create interface depends on fixed mdev type, which get 
> > uuid
> > from user to create instance of mdev device. If user wants to use customized
> > number of resource for mdev device, then only can create new mdev type for 
> > that
> > which may not be flexible.
> > 
> > To allow to create user defined resources for mdev, this RFC trys
> > to extend mdev create interface by adding new "instances=xxx" parameter
> > following uuid, for target mdev type if aggregation is supported, it can
> > create new mdev device which contains resources combined by number of
> > instances, e.g
> > 
> > echo ",instances=10" > create  
> 
> This seems orthogonal to the way mdev types are meant to be used. Vendor
> driver can provide the possible types to provide flexibility to the users.

I think the goal here is to define how we create a type that supports
arbitrary combinations of resources where the total number of those
resources my be sufficiently large that the parent driver enumerating
them all is not feasible.

> Secondly, not always all resources defined for a particular mdev type
> can be multiplied, for example, a mdev type for vGPU that supports 2
> heads, that can't be multiplied to use with 20 heads.

Not all types need to define themselves this way, aiui this is an
optional extension.  Userspace can determine if this feature is
available with the new attribute and if they're not aware of the new
attribute, we operate in a backwards compatible mode where 'echo $UUID >
create' consumes one instance of that type.  Mdev, like vfio, is device
agnostic, so while a vGPU example may have scaling issues that need to
be ironed out to implement this, those don't immediately negate the
value of this proposal.  Thanks,

Alex

> > VM manager e.g libvirt can check mdev type with "aggregation" attribute
> > which can support this setting. And new sysfs attribute "instances" is
> > created for each mdev device to show allocated number. Default number
> > of 1 or no "instances" file can be used for compatibility check.
> > 
> > This RFC trys to create new KVMGT type with minimal vGPU resources which
> > can be combined with "instances=x" setting to allocate for user wanted 
> > resources.
> > 
> > Zhenyu Wang (2):
> >   vfio/mdev: Add new instances parameters for mdev create
> >   drm/i915/gvt: Add new aggregation type
> > 
> >  drivers/gpu/drm/i915/gvt/gvt.c   | 26 ---
> >  drivers/gpu/drm/i915/gvt/gvt.h   | 14 +---
> >  drivers/gpu/drm/i915/gvt/kvmgt.c |  9 +++--
> >  drivers/gpu/drm/i915/gvt/vgpu.c  | 56 
> >  drivers/s390/cio/vfio_ccw_ops.c  |  3 +-
> >  drivers/vfio/mdev/mdev_core.c| 11 ---
> >  drivers/vfio/mdev/mdev_private.h |  6 +++-
> >  drivers/vfio/mdev/mdev_sysfs.c   | 42 
> >  include/linux/mdev.h |  3 +-
> >  samples/vfio-mdev/mbochs.c   |  3 +-
> >  samples/vfio-mdev/mdpy.c |  3 +-
> >  samples/vfio-mdev/mtty.c |  3 +-
> >  12 files changed, 141 insertions(+), 38 deletions(-)
> >   

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] ANNOUNCE: libvirt-dbus 1.1.0 released

[Pavel Hrdina]

I'm happy to announce the release of libvirt-dbus 1.1.0.
  
libvirt-dbus wraps libvirt API to provide high-level object-oriented
API better suited for dbus-based applications.

You can download it here:

https://libvirt.org/sources/dbus/libvirt-dbus-1.1.0.tar.gz


  * New features

- Support for all relevant nwfilter APIs up to libvirt 3.0.0

- Support for all relevant storage volume APIs up to libvirt 3.0.0

- Support for all relevant node device APIs up to libvirt 3.0.0

  * Bug fixes

- Don't report error for GetAll on properties if some property is not 
accessible


Thanks everybody who contributed!

Pavel


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC PATCH 0/2] Add new mdev type for aggregated resources

[Kirti Wankhede]

On 6/20/2018 1:10 PM, Zhenyu Wang wrote:
> Current mdev device create interface depends on fixed mdev type, which get 
> uuid
> from user to create instance of mdev device. If user wants to use customized
> number of resource for mdev device, then only can create new mdev type for 
> that
> which may not be flexible.
> 
> To allow to create user defined resources for mdev, this RFC trys
> to extend mdev create interface by adding new "instances=xxx" parameter
> following uuid, for target mdev type if aggregation is supported, it can
> create new mdev device which contains resources combined by number of
> instances, e.g
> 
> echo ",instances=10" > create

This seems orthogonal to the way mdev types are meant to be used. Vendor
driver can provide the possible types to provide flexibility to the users.

Secondly, not always all resources defined for a particular mdev type
can be multiplied, for example, a mdev type for vGPU that supports 2
heads, that can't be multiplied to use with 20 heads.

Thanks,
Kirti

> 
> VM manager e.g libvirt can check mdev type with "aggregation" attribute
> which can support this setting. And new sysfs attribute "instances" is
> created for each mdev device to show allocated number. Default number
> of 1 or no "instances" file can be used for compatibility check.
> 
> This RFC trys to create new KVMGT type with minimal vGPU resources which
> can be combined with "instances=x" setting to allocate for user wanted 
> resources.
> 
> Zhenyu Wang (2):
>   vfio/mdev: Add new instances parameters for mdev create
>   drm/i915/gvt: Add new aggregation type
> 
>  drivers/gpu/drm/i915/gvt/gvt.c   | 26 ---
>  drivers/gpu/drm/i915/gvt/gvt.h   | 14 +---
>  drivers/gpu/drm/i915/gvt/kvmgt.c |  9 +++--
>  drivers/gpu/drm/i915/gvt/vgpu.c  | 56 
>  drivers/s390/cio/vfio_ccw_ops.c  |  3 +-
>  drivers/vfio/mdev/mdev_core.c| 11 ---
>  drivers/vfio/mdev/mdev_private.h |  6 +++-
>  drivers/vfio/mdev/mdev_sysfs.c   | 42 
>  include/linux/mdev.h |  3 +-
>  samples/vfio-mdev/mbochs.c   |  3 +-
>  samples/vfio-mdev/mdpy.c |  3 +-
>  samples/vfio-mdev/mtty.c |  3 +-
>  12 files changed, 141 insertions(+), 38 deletions(-)
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] spec: Move SASL configuration file from -libs to -daemon

[Andrea Bolognani]

SASL authentication is configured server-side, so the sample
configuration file should be shipped along with the daemon
rather than with the libraries.

Signed-off-by: Andrea Bolognani 
---
 libvirt.spec.in | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index ace05820aa..54549cc737 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1773,6 +1773,7 @@ exit 0
 %config(noreplace) %{_sysconfdir}/libvirt/libvirtd.conf
 %config(noreplace) %{_sysconfdir}/libvirt/virtlogd.conf
 %config(noreplace) %{_sysconfdir}/libvirt/virtlockd.conf
+%config(noreplace) %{_sysconfdir}/sasl2/libvirt.conf
 %config(noreplace) %{_prefix}/lib/sysctl.d/60-libvirtd.conf
 
 %config(noreplace) %{_sysconfdir}/logrotate.d/libvirtd
@@ -2060,8 +2061,6 @@ exit 0
 
 %{_datadir}/libvirt/test-screenshot.png
 
-%config(noreplace) %{_sysconfdir}/sasl2/libvirt.conf
-
 %files admin
 %{_mandir}/man1/virt-admin.1*
 %{_bindir}/virt-admin
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [jenkins-ci PATCH 2/2] jobs: Switch to github webhook instead of polling

[Pavel Hrdina]

On Thu, Jun 21, 2018 at 02:50:49PM +0100, Daniel P. Berrangé wrote:
> On Thu, Jun 21, 2018 at 03:37:49PM +0200, Andrea Bolognani wrote:
> > On Thu, 2018-06-21 at 14:21 +0200, Pavel Hrdina wrote:
> > [...]
> > > @@ -31,8 +31,7 @@
> > >  triggers:
> > >- reverse:
> > >jobs: '{obj:parent_jobs}'
> > > -  - pollscm:
> > > -  cron: "H/20 * * * *"
> > > +  - github
> > 
> > As noted in the review for the previous patch
> > 
> >   libosinfo: https://gitlab.com/libosinfo/libosinfo.git
> >   osinfo-db-tools: https://gitlab.com/libosinfo/osinfo-db-tools.git
> >   osinfo-db: https://gitlab.com/libosinfo/osinfo-db.git
> >   virt-viewer: https://pagure.io/virt-viewer.git
> > 
> > are not hosted on GitHub, so the above won't work for them.
> > 
> > There's probably a way to make something like this work for GitLab
> > too, but I'm not very confident about Pagure.
> > 
> > Perhaps we can convince the projects listed above to set up
> > mirroring to GitHub...
> 
> I think its better if we just leave the jenkins config using pollscm
> as it does today, but simply point it at an appropriate repo for
> each project, instead of using github webhooks or the local cached
> git.
> 
> This kind of need to mirror/move everything to github just to use
> their unique features, is exactly the kind of embrace & extend that
> makes github undesirable. Better to stick to core git functionality.

Yes, I agree with that.

I'll work on the changes required for the first patch.

Thanks
Pavel


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/2] virTypedParamsSerialize: minor fixes

[Marc Hartmayer]

On Thu, Jun 21, 2018 at 03:47 PM +0200, Marc Hartmayer  
wrote:
> On Wed, Jun 13, 2018 at 03:11 PM +0200, John Ferlan  
> wrote:
>> On 06/07/2018 08:17 AM, Marc Hartmayer wrote:
>>> On Wed, May 09, 2018 at 09:51 PM +0200, John Ferlan  
>>> wrote:
 On 05/07/2018 11:24 AM, Marc Hartmayer wrote:
> On Mon, Apr 30, 2018 at 03:20 PM +0200, John Ferlan  
> wrote:
>> On 04/25/2018 11:55 AM, Marc Hartmayer wrote:
>>> 1. Don't allocate if there is nothing that needs to be
>>>allocated. Especially as the result of calling calloc(0, ...) is
>>>implementation-defined.

 uh oh - another memory recollection challenge ;-)

>>
>> Following VIR_ALLOC_N one finds :
>>
>> VIR_ALLOC_N(params_val, nparams)
>>
>> which equates to
>>
>> # define VIR_ALLOC_N(ptr, count) \
>>  virAllocN(&(ptr), sizeof(*(ptr)), (count), true, \
>>VIR_FROM_THIS, __FILE__, __FUNCTION__, __LINE__)
>>
>> or
>>
>> virAllocN(_val, sizeof(params_val), nparams, true, ...)
>>
>> and eventually/essentially
>>
>> *params_val = calloc(sizeof(params_val), nparams)
>>
>> If the returned value is NULL then we error w/ OOM (4th param=true).
>>
>> So, unless @params_val had no elements, it won't be calloc(0,...) and
>
> I'm talking about the case that nparams == 0 => calloc(sizeof(…), 0).
>

 And I was thinking is there a specific consumer/caller of
 virTypedParamsSerialize that was passing something incorrectly.

>> thus the question becomes is there a more specific path you are
>> referencing here?
>
> It’s a “generic” serializer so it should work for every (valid)
> case. What happens, for example, if params == NULL and nparams == 0? In
> that case I would expect *remote_params_val = NULL and
> *remote_params_len = 0.
>

 Going through the callers to virTypedParamsSerialize can/does anyone
 pass params == NULL and nparams == 0?  Would that be reasonable and/or
 expected?

 My look didn't find any - either some caller checks that the passed
 @params != NULL (usually via virCheckNonNullArgGoto in the external API
 call) or @params is built up on the fly and wouldn't be NULL because
 there'd be an error causing failure beforehand. Although I'll admit the
 migration ones are also crazy to look at.

 I could have made a mistake too; hence, the is there a specific instance
 that I missed? Or perhaps this is a result of some branched/private code
 that had that error which I don't have access to?
>>>
>>> It was the result of private code but actually it was intended and no
>>> error :)
>>>

 To answer your "What happens, for example, if params == NULL and nparams
 == 0?", well supposedly "VIR_ALLOC_N(params_val, 0)" should return NULL,
 so params_val and thus *remote_params_val == NULL.
>>>
>>> Did you try what 'VIR_ALLOC_N(params_val, 0)' actually returns? At least
>>> for me, it doesn’t return a NULL pointer.
>>>
>>
>> I think I already determined that NULL isn't returned; otherwise, we
>> would have failed w/ OOM. I do recall trying this and seeing a non NULL
>> value returned.
>>
>> IIRC: I found no way into [de]serialize w/ a 0 parameter, so having that
>> guard didn't seem necessary,
>
> I used the libvirt-python APIs for some testing. I called an (own) API
> with 'None' as argument and this resulted in 0 parameters.

Maybe it was an empty dictionary.

[…snip]

--
Beste Grüße / Kind regards
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [jenkins-ci PATCH 2/2] jobs: Switch to github webhook instead of polling

[Daniel P . Berrangé]

On Thu, Jun 21, 2018 at 03:37:49PM +0200, Andrea Bolognani wrote:
> On Thu, 2018-06-21 at 14:21 +0200, Pavel Hrdina wrote:
> [...]
> > @@ -31,8 +31,7 @@
> >  triggers:
> >- reverse:
> >jobs: '{obj:parent_jobs}'
> > -  - pollscm:
> > -  cron: "H/20 * * * *"
> > +  - github
> 
> As noted in the review for the previous patch
> 
>   libosinfo: https://gitlab.com/libosinfo/libosinfo.git
>   osinfo-db-tools: https://gitlab.com/libosinfo/osinfo-db-tools.git
>   osinfo-db: https://gitlab.com/libosinfo/osinfo-db.git
>   virt-viewer: https://pagure.io/virt-viewer.git
> 
> are not hosted on GitHub, so the above won't work for them.
> 
> There's probably a way to make something like this work for GitLab
> too, but I'm not very confident about Pagure.
> 
> Perhaps we can convince the projects listed above to set up
> mirroring to GitHub...

I think its better if we just leave the jenkins config using pollscm
as it does today, but simply point it at an appropriate repo for
each project, instead of using github webhooks or the local cached
git.

This kind of need to mirror/move everything to github just to use
their unique features, is exactly the kind of embrace & extend that
makes github undesirable. Better to stick to core git functionality.

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/2] virTypedParamsSerialize: minor fixes

[Marc Hartmayer]

On Wed, Jun 13, 2018 at 03:11 PM +0200, John Ferlan  wrote:
> On 06/07/2018 08:17 AM, Marc Hartmayer wrote:
>> On Wed, May 09, 2018 at 09:51 PM +0200, John Ferlan  
>> wrote:
>>> On 05/07/2018 11:24 AM, Marc Hartmayer wrote:
 On Mon, Apr 30, 2018 at 03:20 PM +0200, John Ferlan  
 wrote:
> On 04/25/2018 11:55 AM, Marc Hartmayer wrote:
>> 1. Don't allocate if there is nothing that needs to be
>>allocated. Especially as the result of calling calloc(0, ...) is
>>implementation-defined.
>>>
>>> uh oh - another memory recollection challenge ;-)
>>>
>
> Following VIR_ALLOC_N one finds :
>
> VIR_ALLOC_N(params_val, nparams)
>
> which equates to
>
> # define VIR_ALLOC_N(ptr, count) \
>  virAllocN(&(ptr), sizeof(*(ptr)), (count), true, \
>VIR_FROM_THIS, __FILE__, __FUNCTION__, __LINE__)
>
> or
>
> virAllocN(_val, sizeof(params_val), nparams, true, ...)
>
> and eventually/essentially
>
> *params_val = calloc(sizeof(params_val), nparams)
>
> If the returned value is NULL then we error w/ OOM (4th param=true).
>
> So, unless @params_val had no elements, it won't be calloc(0,...) and

 I'm talking about the case that nparams == 0 => calloc(sizeof(…), 0).

>>>
>>> And I was thinking is there a specific consumer/caller of
>>> virTypedParamsSerialize that was passing something incorrectly.
>>>
> thus the question becomes is there a more specific path you are
> referencing here?

 It’s a “generic” serializer so it should work for every (valid)
 case. What happens, for example, if params == NULL and nparams == 0? In
 that case I would expect *remote_params_val = NULL and
 *remote_params_len = 0.

>>>
>>> Going through the callers to virTypedParamsSerialize can/does anyone
>>> pass params == NULL and nparams == 0?  Would that be reasonable and/or
>>> expected?
>>>
>>> My look didn't find any - either some caller checks that the passed
>>> @params != NULL (usually via virCheckNonNullArgGoto in the external API
>>> call) or @params is built up on the fly and wouldn't be NULL because
>>> there'd be an error causing failure beforehand. Although I'll admit the
>>> migration ones are also crazy to look at.
>>>
>>> I could have made a mistake too; hence, the is there a specific instance
>>> that I missed? Or perhaps this is a result of some branched/private code
>>> that had that error which I don't have access to?
>>
>> It was the result of private code but actually it was intended and no
>> error :)
>>
>>>
>>> To answer your "What happens, for example, if params == NULL and nparams
>>> == 0?", well supposedly "VIR_ALLOC_N(params_val, 0)" should return NULL,
>>> so params_val and thus *remote_params_val == NULL.
>>
>> Did you try what 'VIR_ALLOC_N(params_val, 0)' actually returns? At least
>> for me, it doesn’t return a NULL pointer.
>>
>
> I think I already determined that NULL isn't returned; otherwise, we
> would have failed w/ OOM. I do recall trying this and seeing a non NULL
> value returned.
>
> IIRC: I found no way into [de]serialize w/ a 0 parameter, so having that
> guard didn't seem necessary,

I used the libvirt-python APIs for some testing. I called an (own) API
with 'None' as argument and this resulted in 0 parameters.

> but it's been 2 months since I looked at
> this and that level of detail has long been flushed out of main memory
> cache. ;-)
>
>> The man page for calloc-posix reads:
>>
>> “If either nelem or elsize is 0, then either a null pointer or a unique
>> pointer value that can be successfully passed to free() shall be
>> returned.” [1]
>>
>> [1] https://www.unix.com/man-page/POSIX/3posix/calloc/
>>
>
> perhaps then we avoid this conundrum and take Daniel's advice in his
> response:
>
>"If there is any problem with this, then we should just change
> bootstrap.conf to use calloc-gnu instead of calloc-posix, which
> basically turns calloc(0) into calloc(1) for compat with glibc
> behaviour."

This would still require this patch, no? At least if we agree that the
following example should work:

virTypedParameterPtr params;
int nparams;

virTypedParamsDeserialize(NULL, 0, , );
assert(params == NULL && assert nparams == 0);

Do we?

[…snip]

--
Beste Grüße / Kind regards
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] news: Document recent agent job change

[Michal Privoznik]

Signed-off-by: Michal Privoznik 
---
 docs/news.xml | 13 +
 1 file changed, 13 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 08e5dcbda3..a3eea4eff6 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -71,6 +71,19 @@
   Capabilities XML now provide information about host IOMMU support.
 
   
+  
+
+  qemu: Allow concurrent access to monitor and guest agent
+
+
+  Historically libvirt prevented concurrent accesses to
+  the qemu monitor and the guest agent. Therefore two
+  independent calls (one querying the monitor and the
+  other querying guest agent) would serialize which hurts
+  performance. The code was reworked to allow two
+  independent calls run at the same time.
+
+  
 
 
   
-- 
2.16.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [jenkins-ci PATCH 2/2] jobs: Switch to github webhook instead of polling

[Andrea Bolognani]

On Thu, 2018-06-21 at 14:21 +0200, Pavel Hrdina wrote:
[...]
> @@ -31,8 +31,7 @@
>  triggers:
>- reverse:
>jobs: '{obj:parent_jobs}'
> -  - pollscm:
> -  cron: "H/20 * * * *"
> +  - github

As noted in the review for the previous patch

  libosinfo: https://gitlab.com/libosinfo/libosinfo.git
  osinfo-db-tools: https://gitlab.com/libosinfo/osinfo-db-tools.git
  osinfo-db: https://gitlab.com/libosinfo/osinfo-db.git
  virt-viewer: https://pagure.io/virt-viewer.git

are not hosted on GitHub, so the above won't work for them.

There's probably a way to make something like this work for GitLab
too, but I'm not very confident about Pagure.

Perhaps we can convince the projects listed above to set up
mirroring to GitHub...

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [jenkins-ci PATCH 1/2] jobs: Change git URL to github

[Andrea Bolognani]

On Thu, 2018-06-21 at 14:21 +0200, Pavel Hrdina wrote:
> We had our own local copy of all projects synchronized by cron on
> the host where we have the CI VMs.  This was to safe the traffic from
> libvirt.org repositories and to make the cloning for our Jenkins jobs
> faster.
> 
> We might move our VMs into a cloud in future we would not be able to
> have any local copy so this changes the git URL to use github instead.
> 
> Signed-off-by: Pavel Hrdina 
> ---
>  jobs/autotools.yaml| 2 +-
>  jobs/generic.yaml  | 2 +-
>  jobs/go.yaml   | 2 +-
>  jobs/perl-modulebuild.yaml | 2 +-
>  jobs/python-distutils.yaml | 2 +-
>  5 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/jobs/autotools.yaml b/jobs/autotools.yaml
> index f526ed2..63a1e81 100644
> --- a/jobs/autotools.yaml
> +++ b/jobs/autotools.yaml
> @@ -21,7 +21,7 @@
>num-to-keep: 1000
>  scm:
>- git:
> -  url: git://n64.pufty.ci.centos.org/{name}.git
> +  url: https://github.com/libvirt/{name}.git

This will work for

  libvirt: https://github.com/libvirt/libvirt.git
  libvirt-cim: https://github.com/libvirt/libvirt-cim.git
  libvirt-dbus: https://github.com/libvirt/libvirt-dbus.git
  libvirt-glib: https://github.com/libvirt/libvirt-glib.git
  libvirt-go: https://github.com/libvirt/libvirt-go.git
  libvirt-go-xml: https://github.com/libvirt/libvirt-go-xml.git
  libvirt-perl: https://github.com/libvirt/libvirt-perl.git
  libvirt-python: https://github.com/libvirt/libvirt-python.git
  libvirt-sandbox: https://github.com/libvirt/libvirt-sandbox.git
  libvirt-tck: https://github.com/libvirt/libvirt-tck.git

but not for

  virt-manager: https://github.com/virt-manager/virt-manager.git

because of the different namespace, and also not for

  libosinfo: https://gitlab.com/libosinfo/libosinfo.git
  osinfo-db-tools: https://gitlab.com/libosinfo/osinfo-db-tools.git
  osinfo-db: https://gitlab.com/libosinfo/osinfo-db.git
  virt-viewer: https://pagure.io/virt-viewer.git

because of the completely different hosting platform, so
we're going to need this to be overridable on a per-project
basis.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] virDomainSnapshotDefParse: Prefer VIR_STEAL_PTR

[Michal Privoznik]

Signed-off-by: Michal Privoznik 
---
 src/conf/snapshot_conf.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 787c3d0feb..9c537ac7d1 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -331,8 +331,7 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
  "disk-only snapshot"));
 goto cleanup;
 }
-def->file = memoryFile;
-memoryFile = NULL;
+VIR_STEAL_PTR(def->file, memoryFile);
 
 /* verify that memory path is absolute */
 if (def->file && def->file[0] != '/') {
@@ -372,7 +371,7 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
 if (!offline && virSaveCookieParse(ctxt, >cookie, saveCookie) < 0)
 goto cleanup;
 
-ret = def;
+VIR_STEAL_PTR(ret, def);
 
  cleanup:
 VIR_FREE(creation);
@@ -380,8 +379,7 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
 VIR_FREE(nodes);
 VIR_FREE(memorySnapshot);
 VIR_FREE(memoryFile);
-if (ret == NULL)
-virDomainSnapshotDefFree(def);
+virDomainSnapshotDefFree(def);
 
 return ret;
 }
-- 
2.16.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [jenkins-ci PATCH 0/2] git changes to reflect new CI configuration

[Pavel Hrdina]

Pavel Hrdina (2):
  jobs: Change git URL to github
  jobs: Switch to github webhook instead of polling

 jobs/autotools.yaml| 5 ++---
 jobs/generic.yaml  | 5 ++---
 jobs/go.yaml   | 5 ++---
 jobs/perl-modulebuild.yaml | 5 ++---
 jobs/python-distutils.yaml | 5 ++---
 5 files changed, 10 insertions(+), 15 deletions(-)

-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [jenkins-ci PATCH 1/2] jobs: Change git URL to github

[Pavel Hrdina]

We had our own local copy of all projects synchronized by cron on
the host where we have the CI VMs.  This was to safe the traffic from
libvirt.org repositories and to make the cloning for our Jenkins jobs
faster.

We might move our VMs into a cloud in future we would not be able to
have any local copy so this changes the git URL to use github instead.

Signed-off-by: Pavel Hrdina 
---
 jobs/autotools.yaml| 2 +-
 jobs/generic.yaml  | 2 +-
 jobs/go.yaml   | 2 +-
 jobs/perl-modulebuild.yaml | 2 +-
 jobs/python-distutils.yaml | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/jobs/autotools.yaml b/jobs/autotools.yaml
index f526ed2..63a1e81 100644
--- a/jobs/autotools.yaml
+++ b/jobs/autotools.yaml
@@ -21,7 +21,7 @@
   num-to-keep: 1000
 scm:
   - git:
-  url: git://n64.pufty.ci.centos.org/{name}.git
+  url: https://github.com/libvirt/{name}.git
   branches:
 - origin/{branch}
   clean:
diff --git a/jobs/generic.yaml b/jobs/generic.yaml
index 3e20962..7919274 100644
--- a/jobs/generic.yaml
+++ b/jobs/generic.yaml
@@ -21,7 +21,7 @@
   num-to-keep: 1000
 scm:
   - git:
-  url: git://n64.pufty.ci.centos.org/{name}.git
+  url: https://github.com/libvirt/{name}.git
   branches:
 - origin/{branch}
   clean:
diff --git a/jobs/go.yaml b/jobs/go.yaml
index bffe56e..04ba49a 100644
--- a/jobs/go.yaml
+++ b/jobs/go.yaml
@@ -21,7 +21,7 @@
   num-to-keep: 1000
 scm:
   - git:
-  url: git://n64.pufty.ci.centos.org/{name}.git
+  url: https://github.com/libvirt/{name}.git
   branches:
 - origin/{branch}
   clean:
diff --git a/jobs/perl-modulebuild.yaml b/jobs/perl-modulebuild.yaml
index 4a79bab..ef16c6e 100644
--- a/jobs/perl-modulebuild.yaml
+++ b/jobs/perl-modulebuild.yaml
@@ -21,7 +21,7 @@
   num-to-keep: 1000
 scm:
   - git:
-  url: git://n64.pufty.ci.centos.org/{name}.git
+  url: https://github.com/libvirt/{name}.git
   branches:
 - origin/{branch}
   clean:
diff --git a/jobs/python-distutils.yaml b/jobs/python-distutils.yaml
index c075245..9e64c84 100644
--- a/jobs/python-distutils.yaml
+++ b/jobs/python-distutils.yaml
@@ -21,7 +21,7 @@
   num-to-keep: 1000
 scm:
   - git:
-  url: git://n64.pufty.ci.centos.org/{name}.git
+  url: https://github.com/libvirt/{name}.git
   branches:
 - origin/{branch}
   clean:
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [jenkins-ci PATCH 2/2] jobs: Switch to github webhook instead of polling

[Pavel Hrdina]

If we use github as the remote repository we can take advantage of
github webhooks in order to trigger our Jobs instead of blind polling.
CentOS CI provides that option [1].

[1] 

Signed-off-by: Pavel Hrdina 
---
 jobs/autotools.yaml| 3 +--
 jobs/generic.yaml  | 3 +--
 jobs/go.yaml   | 3 +--
 jobs/perl-modulebuild.yaml | 3 +--
 jobs/python-distutils.yaml | 3 +--
 5 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/jobs/autotools.yaml b/jobs/autotools.yaml
index 63a1e81..8a97a7a 100644
--- a/jobs/autotools.yaml
+++ b/jobs/autotools.yaml
@@ -31,8 +31,7 @@
 triggers:
   - reverse:
   jobs: '{obj:parent_jobs}'
-  - pollscm:
-  cron: "H/20 * * * *"
+  - github
 axes:
   - axis:
   name: systems
diff --git a/jobs/generic.yaml b/jobs/generic.yaml
index 7919274..770b781 100644
--- a/jobs/generic.yaml
+++ b/jobs/generic.yaml
@@ -31,8 +31,7 @@
 triggers:
   - reverse:
   jobs: '{obj:parent_jobs}'
-  - pollscm:
-  cron: "H/20 * * * *"
+  - github
 axes:
   - axis:
   name: systems
diff --git a/jobs/go.yaml b/jobs/go.yaml
index 04ba49a..7616185 100644
--- a/jobs/go.yaml
+++ b/jobs/go.yaml
@@ -31,8 +31,7 @@
 triggers:
   - reverse:
   jobs: '{obj:parent_jobs}'
-  - pollscm:
-  cron: "H/20 * * * *"
+  - github
 axes:
   - axis:
   name: systems
diff --git a/jobs/perl-modulebuild.yaml b/jobs/perl-modulebuild.yaml
index ef16c6e..329bfcd 100644
--- a/jobs/perl-modulebuild.yaml
+++ b/jobs/perl-modulebuild.yaml
@@ -31,8 +31,7 @@
 triggers:
   - reverse:
   jobs: '{obj:parent_jobs}'
-  - pollscm:
-  cron: "H/20 * * * *"
+  - github
 axes:
   - axis:
   name: systems
diff --git a/jobs/python-distutils.yaml b/jobs/python-distutils.yaml
index 9e64c84..dca016e 100644
--- a/jobs/python-distutils.yaml
+++ b/jobs/python-distutils.yaml
@@ -31,8 +31,7 @@
 triggers:
   - reverse:
   jobs: '{obj:parent_jobs}'
-  - pollscm:
-  cron: "H/20 * * * *"
+  - github
 axes:
   - axis:
   name: systems
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/2] qemuDomainObjBeginJobInternal: Log agent job too

[Jiri Denemark]

On Wed, Jun 20, 2018 at 14:32:03 +0200, Michal Privoznik wrote:
> If a thread is unable to start a job (e.g. because of timeout)
> an warning is printed into the logs. So far, the message does not

s/an/a/

> contain agent job info. Add it as it might help future debugging.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_domain.c | 16 +++-
>  1 file changed, 11 insertions(+), 5 deletions(-)

Reviewed-by: Jiri Denemark 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] set-lifecycle-action: add description of type and action

[Chen Hanxiao]

From: Chen Hanxiao 

In [1],  are described as "on_poweroff",
   "on_reboot", "on_crash".
   but we accept "poweroff", "reboot" and "crash".
This patch adds docs about them.

[1]: https://libvirt.org/formatdomain.html#elementsEvents

Signed-off-by: Chen Hanxiao 
---
 tools/virsh.pod | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index 7cb8c8a6e4..1b479f318d 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2414,8 +2414,12 @@ the B command.
 =item B I I I
 [[I<--config>] [I<--live>] | [I<--current>]]
 
-Set the lifecycle I for specified lifecycle I. For the list of
-lifecycle types and actions and possible combinations see the documentation at
+Set the lifecycle I for specified lifecycle I.
+The valid I are "poweroff", "reboot" and "crash", each of them
+allow valid I are "destroy", "restart", "rename-restart", "preserve".
+For I "crash", additional actions "coredump-destroy"
+and "coredump-restart" are supported. For the list of lifecycle types
+and actions and possible combinations see the documentation at
 L.
 
 =item B I I I [I<--encrypted>]
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [dbus PATCH v2 0/5] tests: Some fixes and cleanups

[Pavel Hrdina]

On Thu, Jun 21, 2018 at 01:04:54PM +0200, Andrea Bolognani wrote:
> Changes from [v1]:
> 
> * use the correct paths;
> * fix running libvirt-dbus from VPATH builds;
> * fix distcheck.
> 
> [v1] https://www.redhat.com/archives/libvir-list/2018-June/msg01453.html
> 
> Andrea Bolognani (5):
>   tests: Include xmldata.py in EXTRA_DIST
>   run: Fix VIRT_DBUS_INTERFACES_DIR
>   run: Export abs_top_builddir
>   tests: Drop fallback path for abs_top_builddir
>   tests: Use AM_TESTS_ENVIRONMENT

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [dbus PATCH v2 2/5] run: Fix VIRT_DBUS_INTERFACES_DIR

[Andrea Bolognani]

The D-Bus interface files are in the source directory,
not in the build directory.

Signed-off-by: Andrea Bolognani 
---
 run.in | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/run.in b/run.in
index e98edaf..03e7c64 100644
--- a/run.in
+++ b/run.in
@@ -14,9 +14,6 @@
 #   ./run gdb --args ./src/libvirt-dbus [args ...]
 #
 
-# Find this script
-b=@abs_top_builddir@
-
-export VIRT_DBUS_INTERFACES_DIR="$b/data"
+export VIRT_DBUS_INTERFACES_DIR="@abs_top_srcdir@/data"
 
 exec "$@"
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [dbus PATCH v2 4/5] tests: Drop fallback path for abs_top_builddir

[Andrea Bolognani]

We have made sure all callers set up the environment
correctly, so it's pointless to have a fallback path now.

Signed-off-by: Andrea Bolognani 
---
 tests/libvirttest.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/libvirttest.py b/tests/libvirttest.py
index 06e52c0..3741abd 100644
--- a/tests/libvirttest.py
+++ b/tests/libvirttest.py
@@ -10,7 +10,7 @@ import time
 import xmldata
 
 
-root = os.environ.get('abs_top_builddir', 
os.path.dirname(os.path.dirname(__file__)))
+root = os.environ.get('abs_top_builddir')
 exe = os.path.join(root, 'src', 'libvirt-dbus')
 
 DBusGMainLoop(set_as_default=True)
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [dbus PATCH v2 1/5] tests: Include xmldata.py in EXTRA_DIST

[Andrea Bolognani]

This fixes distcheck.

Signed-off-by: Andrea Bolognani 
---
 tests/Makefile.am | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index e841627..3872d7b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -1,4 +1,5 @@
 test_helpers = \
+   xmldata.py \
libvirttest.py \
conftest.py
 
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [dbus PATCH v2 5/5] tests: Use AM_TESTS_ENVIRONMENT

[Andrea Bolognani]

According to the documentation[1], TESTS_ENVIRONMENT is
reserved for the user, which means we shouldn't be using it.

AM_TESTS_ENVIRONMENT needs to be terminated with a
semicolon, which in turn requires exporting the variables
instead of merely declaring them, so do both of those
things; since we're rewriting the whole hunk anyway, we
also take the opportunity use proper quoting.

[1] 
https://www.gnu.org/software/automake/manual/html_node/Scripts_002dbased-Testsuites.html

Signed-off-by: Andrea Bolognani 
---
 tests/Makefile.am | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 3872d7b..1fc240b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -37,6 +37,6 @@ EXTRA_DIST = \
 
 TESTS = $(test_programs)
 
-TESTS_ENVIRONMENT = \
-   abs_top_builddir=$(abs_top_builddir) \
-   VIRT_DBUS_INTERFACES_DIR=$(abs_top_srcdir)/data
+AM_TESTS_ENVIRONMENT = \
+   export abs_top_builddir="$(abs_top_builddir)"; \
+   export VIRT_DBUS_INTERFACES_DIR="$(abs_top_srcdir)/data";
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [dbus PATCH v2 3/5] run: Export abs_top_builddir

[Andrea Bolognani]

The test suite uses this environment variable, if it's
present, to locate the libvirt-dbus binary.

Signed-off-by: Andrea Bolognani 
---
 run.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/run.in b/run.in
index 03e7c64..0744a80 100644
--- a/run.in
+++ b/run.in
@@ -14,6 +14,7 @@
 #   ./run gdb --args ./src/libvirt-dbus [args ...]
 #
 
+export abs_top_builddir="@abs_top_builddir@"
 export VIRT_DBUS_INTERFACES_DIR="@abs_top_srcdir@/data"
 
 exec "$@"
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [dbus PATCH v2 0/5] tests: Some fixes and cleanups

[Andrea Bolognani]

Changes from [v1]:

* use the correct paths;
* fix running libvirt-dbus from VPATH builds;
* fix distcheck.

[v1] https://www.redhat.com/archives/libvir-list/2018-June/msg01453.html

Andrea Bolognani (5):
  tests: Include xmldata.py in EXTRA_DIST
  run: Fix VIRT_DBUS_INTERFACES_DIR
  run: Export abs_top_builddir
  tests: Drop fallback path for abs_top_builddir
  tests: Use AM_TESTS_ENVIRONMENT

 run.in   | 6 ++
 tests/Makefile.am| 7 ---
 tests/libvirttest.py | 2 +-
 3 files changed, 7 insertions(+), 8 deletions(-)

-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] cpu: add 'amd-ssbd' and 'amd-no-ssb' CPU features (CVE-2018-3639)

[Kashyap Chamarthy]

On Thu, Jun 21, 2018 at 12:24:13PM +0200, Kashyap Chamarthy wrote:
> On Thu, Jun 14, 2018 at 11:48:41AM +0100, Daniel P. Berrangé wrote:
> 
> [...]
> 
> > Note that neither amd-ssbd or amd-no-ssb will be reported by the kernel
> > in /proc/cpuinfo. It knows about these CPUID bits and does the right thing,
> > but doesn't report their existance as distinct flags in /proc/cpuinfo.
> 
> Since it isn't pushed yet, minor nit-pick: s/existance/existence/
> 
> Should the commit message be amended to mention that `/proc/cpuinfo`
> will, confusingly enough, report 'ssbd' (for 'amd-ssbd')?

Maybe it's not so confusing if it will show up as 'ssbd' for all (Intel
/ AMD) processors.  Useful for tooling to look for one flag across all
the processors.

[...]

-- 
/kashyap

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] cpu: add 'amd-ssbd' and 'amd-no-ssb' CPU features (CVE-2018-3639)

[Kashyap Chamarthy]

On Thu, Jun 14, 2018 at 11:48:41AM +0100, Daniel P. Berrangé wrote:

[...]

> Note that neither amd-ssbd or amd-no-ssb will be reported by the kernel
> in /proc/cpuinfo. It knows about these CPUID bits and does the right thing,
> but doesn't report their existance as distinct flags in /proc/cpuinfo.

Since it isn't pushed yet, minor nit-pick: s/existance/existence/

Should the commit message be amended to mention that `/proc/cpuinfo`
will, confusingly enough, report 'ssbd' (for 'amd-ssbd')?

Because reading this thread on 'qemu-devel':

https://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg03660.html
[PATCH 1/2] i386: define the AMD 'amd-ssbd' CPUID feature bit

Says:

[quote]
It [kernel] will only report 'ssbd' but not 'amd-ssb-no' nor
'amd-ssbd'.

[...]

The code that finds the AMD_SSBD and sets the 'ssbd' is:

+   if (cpu_has(c, X86_FEATURE_AMD_SSBD)) {
+   set_cpu_cap(c, X86_FEATURE_SSBD);
+   set_cpu_cap(c, X86_FEATURE_MSR_SPEC_CTRL);
+   clear_cpu_cap(c, X86_FEATURE_VIRT_SSBD);
+   }

Meaning the 'ssbd' will show up in /proc/cpuinfo 
[/quote]

[...]

-- 
/kashyap

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 0/1] *** SUBJECT HERE ***

[Han Han]

*** BLURB HERE ***

Han Han (1):
  docs: schema: Add missing  to devices

 docs/schemas/domaincommon.rng | 60 +++
 1 file changed, 33 insertions(+), 27 deletions(-)

-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/1] docs: schema: Add missing to devices

[Han Han]

input device: , ,  should be interleaved.
hub device: ,  should be interleaved.
redirdev device: , , ,  should be interleaved.

Signed-off-by: Han Han 
---
 docs/schemas/domaincommon.rng | 60 +++
 1 file changed, 33 insertions(+), 27 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4a454dddb4..d262eb2b1b 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4270,11 +4270,19 @@
 
   
 
-  
-
-  
-
-  
+  
+
+  
+
+  
+
+
+  
+
+
+  
+
+  
   
 
   
@@ -4309,12 +4317,6 @@
   
 
   
-  
-
-  
-  
-
-  
 
   
   
@@ -4322,12 +4324,14 @@
   
 usb
   
-  
-
-  
-  
-
-  
+  
+
+  
+
+
+  
+
+  
 
   
   
@@ -4338,16 +4342,18 @@
   
 
   
-  
-  
-
-  
-  
-
-  
-  
-
-  
+  
+
+
+  
+
+
+  
+
+
+  
+
+  
 
   
   
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] schema: allow a element with no subelements in a nwfilter

[Michal Privoznik]

On 06/21/2018 08:12 AM, Laine Stump wrote:
> This is a regression in behavior caused by commit 37359814. It was
> intended to limit the schema to allow only a single subelement of
> , but it is also acceptable for  to have no subelement at
> all.
> 
> To prevent the same error from reoccurring in the future, the
> examples/xml/nwfilter directory was added to the list of nwfilter
> schema test directories.
> 
> Resolves: https://bugzilla.redhat.com/1593549
> Signed-off-by: Laine Stump 
> ---
>  docs/schemas/nwfilter.rng | 1 +
>  tests/virschematest.c | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)

ACK

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/2] Two small problems with hyperv Makefile

[Michal Privoznik]

On 06/21/2018 09:54 AM, Laine Stump wrote:
> I found these when make failed after running "make maintainer-clean".
> 
> Laine Stump (2):
>   hyperv: fix typo in Makefile.am.inc
>   hyperv: erase "generated files" sentinel during make maintainer-clean
> 
>  src/hyperv/Makefile.inc.am | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 

ACK

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [dbus PATCH 3/3] tests: Use AM_TESTS_ENVIRONMENT

[Pavel Hrdina]

On Wed, Jun 20, 2018 at 02:20:29PM +0200, Andrea Bolognani wrote:
> According to the documentation[1], TESTS_ENVIRONMENT
> is reserved for the user, which means we shouldn't be
> using it.
> 
> AM_TESTS_ENVIRONMENT needs to be terminated with a
> semicolon, so do that; that requires exporting the
> variables instead of merely declaring them as well.
> 
> [1] 
> https://www.gnu.org/software/automake/manual/html_node/Scripts_002dbased-Testsuites.html
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  tests/Makefile.am | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 81cb263..e27a24a 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -36,5 +36,5 @@ EXTRA_DIST = \
>  
>  TESTS = $(test_programs)
>  
> -TESTS_ENVIRONMENT = \
> - VIRT_DBUS_INTERFACES_DIR=$(abs_top_builddir)/data
> +AM_TESTS_ENVIRONMENT = \
> + export VIRT_DBUS_INTERFACES_DIR=$(abs_top_builddir)/data;

This will need to be modified to cover the original environment
variables but otherwise looks good.

Pavel


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 0/2] Two small problems with hyperv Makefile

[Laine Stump]

I found these when make failed after running "make maintainer-clean".

Laine Stump (2):
  hyperv: fix typo in Makefile.am.inc
  hyperv: erase "generated files" sentinel during make maintainer-clean

 src/hyperv/Makefile.inc.am | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.14.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/2] hyperv: erase "generated files" sentinel during make maintainer-clean

[Laine Stump]

Re-generating of generated source files in the hyperv directory
depends on src/.hyperv_wmi_generator.stamp not existing, or having a
timestamp older than src/hyperv/hyperv_wmi_generator.py. "make
maintainer-clean" erases the generated files, but not this sentinel
file, so the erased files aren't regenerated during the next
make. Once we add it to the list of MAINTAINERCLEANFILES, it gets
deleted at the same time as the generated files, so make is able to
understand they need regeneration.

Signed-off-by: Laine Stump 
---
 src/hyperv/Makefile.inc.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/hyperv/Makefile.inc.am b/src/hyperv/Makefile.inc.am
index 3833589470..a768272b72 100644
--- a/src/hyperv/Makefile.inc.am
+++ b/src/hyperv/Makefile.inc.am
@@ -43,7 +43,7 @@ $(HYPERV_GENERATED_STAMP): 
$(srcdir)/hyperv/hyperv_wmi_generator.input \
  $(srcdir)/hyperv/hyperv_wmi_generator.py \
  && touch $@
 
-MAINTAINERCLEANFILES += $(HYPERV_DRIVER_GENERATED)
+MAINTAINERCLEANFILES += $(HYPERV_DRIVER_GENERATED) $(HYPERV_GENERATED_STAMP)
 
 if WITH_HYPERV
 noinst_LTLIBRARIES += libvirt_driver_hyperv.la
-- 
2.14.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/2] hyperv: fix typo in Makefile.am.inc

[Laine Stump]

The problem has been around for quite awhile - the misspelling was
faithfully copied from src/Makefile.am to src/hyperv/Makefile.am.inc
in commit 253b528c.

Signed-off-by: Laine Stump 
---
 src/hyperv/Makefile.inc.am | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/hyperv/Makefile.inc.am b/src/hyperv/Makefile.inc.am
index 1478d77b08..3833589470 100644
--- a/src/hyperv/Makefile.inc.am
+++ b/src/hyperv/Makefile.inc.am
@@ -26,10 +26,10 @@ HYPERV_DRIVER_EXTRA_DIST = \
$(HYPERV_GENERATED_STAMP) \
$(NULL)
 
-DRIVER_SOURCE_FILES += $(HYPERV_DRIVER_SORUCES)
+DRIVER_SOURCE_FILES += $(HYPERV_DRIVER_SOURCES)
 
 EXTRA_DIST += \
-   $(HYPERV_DRIVER_SORUCES) \
+   $(HYPERV_DRIVER_SOURCES) \
$(HYPERV_DRIVER_EXTRA_DIST) \
$(NULL)
 
-- 
2.14.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [dbus PATCH 2/3] tests: Don't loop up abs_top_builddir in the environment

[Pavel Hrdina]

On Wed, Jun 20, 2018 at 02:20:28PM +0200, Andrea Bolognani wrote:
> This only works if the caller has prepared the environment
> accordingly; however, there is already a fallback path in
> place and it works just fine, so much so that when test
> cases are executed manually through the 'run' script
> that's the only one being involved.
> 
> Drop environment handling entirely and rely on path
> manipulation instead.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  tests/Makefile.am| 1 -
>  tests/libvirttest.py | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 10d2935..81cb263 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -37,5 +37,4 @@ EXTRA_DIST = \
>  TESTS = $(test_programs)
>  
>  TESTS_ENVIRONMENT = \
> - abs_top_builddir=$(abs_top_builddir) \
>   VIRT_DBUS_INTERFACES_DIR=$(abs_top_builddir)/data
> diff --git a/tests/libvirttest.py b/tests/libvirttest.py
> index 06e52c0..c1543e8 100644
> --- a/tests/libvirttest.py
> +++ b/tests/libvirttest.py
> @@ -10,7 +10,7 @@ import time
>  import xmldata
>  
>  
> -root = os.environ.get('abs_top_builddir', 
> os.path.dirname(os.path.dirname(__file__)))
> +root = os.path.dirname(os.path.dirname(__file__))
>  exe = os.path.join(root, 'src', 'libvirt-dbus')

NACK, this is essential for VPATH build.

Pavel


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [dbus PATCH 1/3] tests: Fix VIRT_DBUS_INTERFACES_DIR

[Pavel Hrdina]

On Wed, Jun 20, 2018 at 02:20:27PM +0200, Andrea Bolognani wrote:
> D-Bus interface files are generated, so we need to look
> for them inside builddir instead of srcdir for the test
> suite to pass before installation.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  tests/Makefile.am | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index e841627..10d2935 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -38,4 +38,4 @@ TESTS = $(test_programs)
>  
>  TESTS_ENVIRONMENT = \
>   abs_top_builddir=$(abs_top_builddir) \
> - VIRT_DBUS_INTERFACES_DIR=$(abs_top_srcdir)/data
> + VIRT_DBUS_INTERFACES_DIR=$(abs_top_builddir)/data

NACK, they are not generated.

Pavel


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PATCH RFC] hw/pc: set q35 as the default x86 machine

[Daniel P . Berrangé]

On Wed, Jun 20, 2018 at 02:28:24PM -0300, Eduardo Habkost wrote:
> On Mon, Jun 18, 2018 at 08:18:16PM +0300, Michael S. Tsirkin wrote:
> > On Mon, Jun 18, 2018 at 02:14:31PM -0300, Eduardo Habkost wrote:
> > > > Sure if someone does that, we'll have no choice, but as long as 'pc' is
> > > > shipped we shouldn't gratuitously break apps by changing the default.
> > > 
> > > Right.  I just want to make sure "omitting the machine-type may
> > > stop working in the future" is documented somehow.
> > 
> > I still think we should just add links to the qemu binary and
> > use ARGV to detect the machine type.
> > 
> > qemu-pc-i386
> > qemu-q35-x86_64
> 
> Why having separate QEMU binaries would help?  We still need to
> define and document what will happen when both the machine-type
> and the QEMU binary are omitted in the domain XML.

It would not help libvirt at all, and in fact it would cause extra pain
for applications, because we don't have ability to associated separate
QEMU binaries with machine types in our capabilities design. So not
only would libvirt need changing, but apps using libvirt too.

> Personally I prefer to document this as "we recommend you always
> specify the machine-type" instead of "we recommend you always
> specify the QEMU binary path".

Indeed, the former is something apps already do in many cases.

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PATCH RFC] hw/pc: set q35 as the default x86 machine

[Daniel P . Berrangé]

On Wed, Jun 20, 2018 at 06:33:51PM +0100, Peter Maydell wrote:
> On 18 June 2018 at 18:18, Michael S. Tsirkin  wrote:
> > On Mon, Jun 18, 2018 at 02:14:31PM -0300, Eduardo Habkost wrote:
> >> > Sure if someone does that, we'll have no choice, but as long as 'pc' is
> >> > shipped we shouldn't gratuitously break apps by changing the default.
> >>
> >> Right.  I just want to make sure "omitting the machine-type may
> >> stop working in the future" is documented somehow.
> >
> > I still think we should just add links to the qemu binary and
> > use ARGV to detect the machine type.
> >
> > qemu-pc-i386
> > qemu-q35-x86_64
> 
> Do you really want 60 different qemu-something-arm symlinks?

Absolutely not !

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] schema: allow a element with no subelements in a nwfilter

[Laine Stump]

This is a regression in behavior caused by commit 37359814. It was
intended to limit the schema to allow only a single subelement of
, but it is also acceptable for  to have no subelement at
all.

To prevent the same error from reoccurring in the future, the
examples/xml/nwfilter directory was added to the list of nwfilter
schema test directories.

Resolves: https://bugzilla.redhat.com/1593549
Signed-off-by: Laine Stump 
---
 docs/schemas/nwfilter.rng | 1 +
 tests/virschematest.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/docs/schemas/nwfilter.rng b/docs/schemas/nwfilter.rng
index cca6ff2954..17cda5c78d 100644
--- a/docs/schemas/nwfilter.rng
+++ b/docs/schemas/nwfilter.rng
@@ -20,6 +20,7 @@
   
 
 
+
 
   
   
diff --git a/tests/virschematest.c b/tests/virschematest.c
index 2d35833919..aa65a434ff 100644
--- a/tests/virschematest.c
+++ b/tests/virschematest.c
@@ -229,7 +229,7 @@ mymain(void)
 DO_TEST_DIR("network.rng", "../src/network", "networkxml2xmlin",
 "networkxml2xmlout", "networkxml2confdata");
 DO_TEST_DIR("nodedev.rng", "nodedevschemadata");
-DO_TEST_DIR("nwfilter.rng", "nwfilterxml2xmlout");
+DO_TEST_DIR("nwfilter.rng", "nwfilterxml2xmlout", 
"../examples/xml/nwfilter");
 DO_TEST_DIR("secret.rng", "secretxml2xmlin");
 DO_TEST_DIR("storagepool.rng", "storagepoolxml2xmlin", 
"storagepoolxml2xmlout",
 "storagepoolschemadata");
-- 
2.14.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list