[libvirt] [PATCH] libxl: add tunnelled migration support

[Bob Liu]

Tunnelled migration doesn't require any extra network connections beside the
libvirt daemon.
It's capable of strong encryption and the default option of openstack-nova.

This patch adds the tunnelled migration(Tunnel3params) support to libxl.
On the src side, the data flow is:
 * libxlDoMigrateSend() -> pipe
 * libxlTunnel3MigrationFunc() polls pipe out and then write to dest stream.

While on the dest side:
Stream -> pipe -> 'recvfd of libxlDomainStartRestore'

The usage is the same as p2p migration, execpt adding one extra '--tunnelled' to
the libvirt p2p migration command.

Signed-off-by: Bob Liu 
---
 src/libxl/libxl_driver.c|  58 -
 src/libxl/libxl_migration.c | 281 +---
 src/libxl/libxl_migration.h |   9 ++
 3 files changed, 331 insertions(+), 17 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index b66cb1f..bc2633b 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -5918,6 +5918,61 @@ libxlDomainMigrateBegin3Params(virDomainPtr domain,
 }
 
 static int
+libxlDomainMigratePrepareTunnel3Params(virConnectPtr dconn,
+   virStreamPtr st,
+   virTypedParameterPtr params,
+   int nparams,
+   const char *cookiein,
+   int cookieinlen,
+   char **cookieout ATTRIBUTE_UNUSED,
+   int *cookieoutlen ATTRIBUTE_UNUSED,
+   unsigned int flags)
+{
+libxlDriverPrivatePtr driver = dconn->privateData;
+virDomainDefPtr def = NULL;
+const char *dom_xml = NULL;
+const char *dname = NULL;
+const char *uri_in = NULL;
+
+#ifdef LIBXL_HAVE_NO_SUSPEND_RESUME
+virReportUnsupportedError();
+return -1;
+#endif
+
+virCheckFlags(LIBXL_MIGRATION_FLAGS, -1);
+if (virTypedParamsValidate(params, nparams, LIBXL_MIGRATION_PARAMETERS) < 
0)
+goto error;
+
+if (virTypedParamsGetString(params, nparams,
+VIR_MIGRATE_PARAM_DEST_XML,
+_xml) < 0 ||
+virTypedParamsGetString(params, nparams,
+VIR_MIGRATE_PARAM_DEST_NAME,
+) < 0 ||
+virTypedParamsGetString(params, nparams,
+VIR_MIGRATE_PARAM_URI,
+_in) < 0)
+
+goto error;
+
+if (!(def = libxlDomainMigrationPrepareDef(driver, dom_xml, dname)))
+goto error;
+
+if (virDomainMigratePrepareTunnel3ParamsEnsureACL(dconn, def) < 0)
+goto error;
+
+if (libxlDomainMigrationPrepareTunnel3(dconn, st, , cookiein,
+   cookieinlen, flags) < 0)
+goto error;
+
+return 0;
+
+ error:
+virDomainDefFree(def);
+return -1;
+}
+
+static int
 libxlDomainMigratePrepare3Params(virConnectPtr dconn,
  virTypedParameterPtr params,
  int nparams,
@@ -6017,7 +6072,7 @@ libxlDomainMigratePerform3Params(virDomainPtr dom,
 if (virDomainMigratePerform3ParamsEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
-if (flags & VIR_MIGRATE_PEER2PEER) {
+if ((flags & (VIR_MIGRATE_TUNNELLED | VIR_MIGRATE_PEER2PEER))) {
 if (libxlDomainMigrationPerformP2P(driver, vm, dom->conn, dom_xml,
dconnuri, uri, dname, flags) < 0)
 goto cleanup;
@@ -6501,6 +6556,7 @@ static virHypervisorDriver libxlHypervisorDriver = {
 .nodeDeviceReset = libxlNodeDeviceReset, /* 1.2.3 */
 .domainMigrateBegin3Params = libxlDomainMigrateBegin3Params, /* 1.2.6 */
 .domainMigratePrepare3Params = libxlDomainMigratePrepare3Params, /* 1.2.6 
*/
+.domainMigratePrepareTunnel3Params = 
libxlDomainMigratePrepareTunnel3Params, /* 2.5.0 */
 .domainMigratePerform3Params = libxlDomainMigratePerform3Params, /* 1.2.6 
*/
 .domainMigrateFinish3Params = libxlDomainMigrateFinish3Params, /* 1.2.6 */
 .domainMigrateConfirm3Params = libxlDomainMigrateConfirm3Params, /* 1.2.6 
*/
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 534abb8..f3152dc 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -44,6 +44,7 @@
 #include "libxl_migration.h"
 #include "locking/domain_lock.h"
 #include "virtypedparam.h"
+#include "fdstream.h"
 
 #define VIR_FROM_THIS VIR_FROM_LIBXL
 
@@ -484,6 +485,90 @@ libxlDomainMigrationPrepareDef(libxlDriverPrivatePtr 
driver,
 }
 
 int
+libxlDomainMigrationPrepareTunnel3(virConnectPtr dconn,
+   virStreamPtr st,
+   virDomainDefPtr *def,
+   const char *cookiein,
+   int 

[libvirt] [PATCH 08/14] cputest: Don't use superfluous preferred model

[Jiri Denemark]

In some cases preferred model doesn't really do anything since the
result remains the same even if it is removed.

Signed-off-by: Jiri Denemark 
---
 tests/cputest.c | 13 +
 ...esult.xml => x86-host+host+host-model,models-result.xml} |  0
 ...ml => x86-host-Haswell-noTSX+Haswell,haswell-result.xml} |  0
 ...x86-host-Haswell-noTSX+Haswell-noTSX,haswell-result.xml} |  0
 xml => x86-host-Haswell-noTSX+Haswell-noTSX-result.xml} |  0
 5 files changed, 5 insertions(+), 8 deletions(-)
 rename tests/cputestdata/{x86-host+host+host-model,models,Penryn-result.xml => 
x86-host+host+host-model,models-result.xml} (100%)
 rename 
tests/cputestdata/{x86-host-Haswell-noTSX+Haswell,haswell,Haswell-result.xml => 
x86-host-Haswell-noTSX+Haswell,haswell-result.xml} (100%)
 rename 
tests/cputestdata/{x86-host-Haswell-noTSX+Haswell-noTSX,haswell,Haswell-noTSX-result.xml
 => x86-host-Haswell-noTSX+Haswell-noTSX,haswell-result.xml} (100%)
 rename 
tests/cputestdata/{x86-host-Haswell-noTSX+Haswell-noTSX,Haswell-noTSX-result.xml
 => x86-host-Haswell-noTSX+Haswell-noTSX-result.xml} (100%)

diff --git a/tests/cputest.c b/tests/cputest.c
index 7496156..6810832 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -799,18 +799,15 @@ mymain(void)
 DO_TEST_GUESTDATA("x86", "host", "guest", models, NULL, 0);
 DO_TEST_GUESTDATA("x86", "host", "guest", nomodel, NULL, -1);
 DO_TEST_GUESTDATA("x86", "host", "guest-nofallback", models, NULL, /*-1*/ 
-2);
-DO_TEST_GUESTDATA("x86", "host", "host+host-model", models, "Penryn", 0);
+DO_TEST_GUESTDATA("x86", "host", "host+host-model", models, NULL, 0);
 DO_TEST_GUESTDATA("x86", "host", "host+host-model-nofallback", models, 
NULL, /*-1*/ -2);
-DO_TEST_GUESTDATA("x86", "host-Haswell-noTSX", "Haswell",
-  haswell, "Haswell", 0);
-DO_TEST_GUESTDATA("x86", "host-Haswell-noTSX", "Haswell-noTSX",
-  haswell, "Haswell-noTSX", 0);
+DO_TEST_GUESTDATA("x86", "host-Haswell-noTSX", "Haswell", haswell, NULL, 
0);
+DO_TEST_GUESTDATA("x86", "host-Haswell-noTSX", "Haswell-noTSX", haswell, 
NULL, 0);
 DO_TEST_GUESTDATA("x86", "host-Haswell-noTSX", "Haswell-noTSX-nofallback", 
haswell, NULL, /*-1*/ -2);
-DO_TEST_GUESTDATA("x86", "host-Haswell-noTSX", "Haswell-noTSX",
-  NULL, "Haswell-noTSX", 0);
+DO_TEST_GUESTDATA("x86", "host-Haswell-noTSX", "Haswell-noTSX", NULL, 
NULL, 0);
 
 DO_TEST_GUESTDATA("ppc64", "host", "guest", ppc_models, NULL, 0);
-DO_TEST_GUESTDATA("ppc64", "host", "guest-nofallback", ppc_models, 
"POWER8", -1);
+DO_TEST_GUESTDATA("ppc64", "host", "guest-nofallback", ppc_models, NULL, 
-1);
 DO_TEST_GUESTDATA("ppc64", "host", "guest-legacy", ppc_models, NULL, 0);
 DO_TEST_GUESTDATA("ppc64", "host", "guest-legacy-incompatible", 
ppc_models, NULL, -1);
 DO_TEST_GUESTDATA("ppc64", "host", "guest-legacy-invalid", ppc_models, 
NULL, -1);
diff --git 
a/tests/cputestdata/x86-host+host+host-model,models,Penryn-result.xml 
b/tests/cputestdata/x86-host+host+host-model,models-result.xml
similarity index 100%
rename from tests/cputestdata/x86-host+host+host-model,models,Penryn-result.xml
rename to tests/cputestdata/x86-host+host+host-model,models-result.xml
diff --git 
a/tests/cputestdata/x86-host-Haswell-noTSX+Haswell,haswell,Haswell-result.xml 
b/tests/cputestdata/x86-host-Haswell-noTSX+Haswell,haswell-result.xml
similarity index 100%
rename from 
tests/cputestdata/x86-host-Haswell-noTSX+Haswell,haswell,Haswell-result.xml
rename to tests/cputestdata/x86-host-Haswell-noTSX+Haswell,haswell-result.xml
diff --git 
a/tests/cputestdata/x86-host-Haswell-noTSX+Haswell-noTSX,haswell,Haswell-noTSX-result.xml
 b/tests/cputestdata/x86-host-Haswell-noTSX+Haswell-noTSX,haswell-result.xml
similarity index 100%
rename from 
tests/cputestdata/x86-host-Haswell-noTSX+Haswell-noTSX,haswell,Haswell-noTSX-result.xml
rename to 
tests/cputestdata/x86-host-Haswell-noTSX+Haswell-noTSX,haswell-result.xml
diff --git 
a/tests/cputestdata/x86-host-Haswell-noTSX+Haswell-noTSX,Haswell-noTSX-result.xml
 b/tests/cputestdata/x86-host-Haswell-noTSX+Haswell-noTSX-result.xml
similarity index 100%
rename from 
tests/cputestdata/x86-host-Haswell-noTSX+Haswell-noTSX,Haswell-noTSX-result.xml
rename to tests/cputestdata/x86-host-Haswell-noTSX+Haswell-noTSX-result.xml
-- 
2.10.2

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

[libvirt] [PATCH 09/14] cputest: Don't use preferred CPU model at all

[Jiri Denemark]

Now that all tests pass NULL as the preferred model, we can just drop
that test parameter.

Signed-off-by: Jiri Denemark 
---
 tests/cputest.c | 69 +++--
 1 file changed, 33 insertions(+), 36 deletions(-)

diff --git a/tests/cputest.c b/tests/cputest.c
index 6810832..6aebfb0 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -62,7 +62,6 @@ struct data {
 const char **models;
 const char *modelsName;
 unsigned int nmodels;
-const char *preferred;
 unsigned int flags;
 int result;
 };
@@ -269,7 +268,7 @@ cpuTestGuestData(const void *arg)
 guest->match = VIR_CPU_MATCH_EXACT;
 guest->fallback = cpu->fallback;
 if (cpuDecode(guest, guestData, data->models,
-  data->nmodels, data->preferred) < 0) {
+  data->nmodels, NULL) < 0) {
 ret = -1;
 goto cleanup;
 }
@@ -277,8 +276,6 @@ cpuTestGuestData(const void *arg)
 virBufferAsprintf(, "%s+%s", data->host, data->name);
 if (data->nmodels)
 virBufferAsprintf(, ",%s", data->modelsName);
-if (data->preferred)
-virBufferAsprintf(, ",%s", data->preferred);
 virBufferAddLit(, "-result");
 
 if (virBufferError()) {
@@ -588,12 +585,12 @@ mymain(void)
 #endif
 
 #define DO_TEST(arch, api, name, host, cpu, \
-models, nmodels, preferred, flags, result)  \
+models, nmodels, flags, result) \
 do {\
 struct data data = {\
 arch, host, cpu, models,\
 models == NULL ? NULL : #models,\
-nmodels, preferred, flags, result   \
+nmodels, flags, result  \
 };  \
 char *testLabel;\
 char *tmp;  \
@@ -624,12 +621,12 @@ mymain(void)
 #define DO_TEST_COMPARE(arch, host, cpu, result)\
 DO_TEST(arch, cpuTestCompare,   \
 host "/" cpu " (" #result ")",  \
-host, cpu, NULL, 0, NULL, 0, result)
+host, cpu, NULL, 0, 0, result)
 
 #define DO_TEST_UPDATE_ONLY(arch, host, cpu)\
 DO_TEST(arch, cpuTestUpdate,\
 cpu " on " host,\
-host, cpu, NULL, 0, NULL, 0, 0) \
+host, cpu, NULL, 0, 0, 0)
 
 #define DO_TEST_UPDATE(arch, host, cpu, result) \
 do {\
@@ -649,7 +646,7 @@ mymain(void)
 ret = -1;   \
 } else {\
 DO_TEST(arch, cpuTestBaseline, label, NULL, \
-"baseline-" name, NULL, 0, NULL, flags, result);\
+"baseline-" name, NULL, 0, flags, result);  \
 }   \
 VIR_FREE(label);\
 } while (0)
@@ -657,21 +654,21 @@ mymain(void)
 #define DO_TEST_HASFEATURE(arch, host, feature, result) \
 DO_TEST(arch, cpuTestHasFeature,\
 host "/" feature " (" #result ")",  \
-host, feature, NULL, 0, NULL, 0, result)
+host, feature, NULL, 0, 0, result)
 
-#define DO_TEST_GUESTDATA(arch, host, cpu, models, preferred, result)   \
+#define DO_TEST_GUESTDATA(arch, host, cpu, models, result)  \
 DO_TEST(arch, cpuTestGuestData, \
-host "/" cpu " (" #models ", pref=" #preferred ")", \
+host "/" cpu " (" #models ")",  \
 host, cpu, models,  \
 models == NULL ? 0 : sizeof(models) / sizeof(char *),   \
-preferred, 0, result)
+0, result)
 
 #if WITH_QEMU && WITH_YAJL
 # define DO_TEST_CPUID_JSON(arch, host, json)   \
 do {\
 if (json) { \
 DO_TEST(arch, cpuTestJSONCPUID, host, host, \
-NULL, NULL, 0, NULL, 0, 0); \
+  

[libvirt] [PATCH 14/14] cpu: Drop cpuGuestData

[Jiri Denemark]

The API is not used anywhere in the code.

Signed-off-by: Jiri Denemark 
---
 src/cpu/cpu.c| 41 -
 src/cpu/cpu.h| 14 --
 src/cpu/cpu_arm.c| 10 --
 src/cpu/cpu_ppc64.c  |  9 -
 src/cpu/cpu_s390.c   |  1 -
 src/cpu/cpu_x86.c| 17 -
 src/libvirt_private.syms |  1 -
 7 files changed, 93 deletions(-)

diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 4a5fbb1..64419ee 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -373,47 +373,6 @@ cpuNodeData(virArch arch)
 
 
 /**
- * cpuGuestData:
- *
- * @host: host CPU definition
- * @guest: guest CPU definition
- * @data: computed guest CPU data
- * @msg: error message describing why the @guest and @host CPUs are considered
- *   incompatible
- *
- * Computes guest CPU data for the @guest CPU definition when run on the @host
- * CPU.
- *
- * Returns VIR_CPU_COMPARE_ERROR on error, VIR_CPU_COMPARE_INCOMPATIBLE when
- * the two CPUs are incompatible (@msg will describe the incompatibility),
- * VIR_CPU_COMPARE_IDENTICAL when the two CPUs are identical,
- * VIR_CPU_COMPARE_SUPERSET when the @guest CPU is a superset of the @host CPU.
- */
-virCPUCompareResult
-cpuGuestData(virCPUDefPtr host,
- virCPUDefPtr guest,
- virCPUDataPtr *data,
- char **msg)
-{
-struct cpuArchDriver *driver;
-
-VIR_DEBUG("host=%p, guest=%p, data=%p, msg=%p", host, guest, data, msg);
-
-if ((driver = cpuGetSubDriver(host->arch)) == NULL)
-return VIR_CPU_COMPARE_ERROR;
-
-if (driver->guestData == NULL) {
-virReportError(VIR_ERR_NO_SUPPORT,
-   _("cannot compute guest CPU data for %s architecture"),
-   virArchToString(host->arch));
-return VIR_CPU_COMPARE_ERROR;
-}
-
-return driver->guestData(host, guest, data, msg);
-}
-
-
-/**
  * cpuBaselineXML:
  *
  * @xmlCPUs: list of host CPU XML descriptions
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index ff48fb5..69c17e7 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -73,12 +73,6 @@ typedef void
 typedef virCPUDataPtr
 (*cpuArchNodeData)  (virArch arch);
 
-typedef virCPUCompareResult
-(*cpuArchGuestData) (virCPUDefPtr host,
- virCPUDefPtr guest,
- virCPUDataPtr *data,
- char **message);
-
 typedef virCPUDefPtr
 (*cpuArchBaseline)  (virCPUDefPtr *cpus,
  unsigned int ncpus,
@@ -124,7 +118,6 @@ struct cpuArchDriver {
 cpuArchEncode   encode;
 cpuArchDataFree free;
 cpuArchNodeData nodeData;
-cpuArchGuestDataguestData;
 cpuArchBaseline baseline;
 virCPUArchUpdateupdate;
 virCPUArchCheckFeature checkFeature;
@@ -175,13 +168,6 @@ cpuDataFree (virCPUDataPtr data);
 virCPUDataPtr
 cpuNodeData (virArch arch);
 
-virCPUCompareResult
-cpuGuestData(virCPUDefPtr host,
- virCPUDefPtr guest,
- virCPUDataPtr *data,
- char **msg)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
-
 char *
 cpuBaselineXML(const char **xmlCPUs,
unsigned int ncpus,
diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
index b5002c3..653b06b 100644
--- a/src/cpu/cpu_arm.c
+++ b/src/cpu/cpu_arm.c
@@ -78,15 +78,6 @@ virCPUarmUpdate(virCPUDefPtr guest,
 }
 
 
-static virCPUCompareResult
-armGuestData(virCPUDefPtr host ATTRIBUTE_UNUSED,
- virCPUDefPtr guest ATTRIBUTE_UNUSED,
- virCPUDataPtr *data ATTRIBUTE_UNUSED,
- char **message ATTRIBUTE_UNUSED)
-{
-return VIR_CPU_COMPARE_IDENTICAL;
-}
-
 static virCPUDefPtr
 armBaseline(virCPUDefPtr *cpus,
 unsigned int ncpus ATTRIBUTE_UNUSED,
@@ -128,7 +119,6 @@ struct cpuArchDriver cpuDriverArm = {
 .encode = NULL,
 .free = armDataFree,
 .nodeData = NULL,
-.guestData = armGuestData,
 .baseline = armBaseline,
 .update = virCPUarmUpdate,
 };
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 8b71ef5..225fb6d 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -745,14 +745,6 @@ ppc64DriverNodeData(virArch arch)
 return NULL;
 }
 
-static virCPUCompareResult
-ppc64DriverGuestData(virCPUDefPtr host,
- virCPUDefPtr guest,
- virCPUDataPtr *data,
- char **message)
-{
-return ppc64Compute(host, guest, data, message);
-}
 
 static int
 virCPUppc64Update(virCPUDefPtr guest,
@@ -911,7 +903,6 @@ struct cpuArchDriver cpuDriverPPC64 = {
 .encode = NULL,
 .free   = ppc64DriverFree,
 .nodeData   = ppc64DriverNodeData,
-.guestData  = ppc64DriverGuestData,
 .baseline   = ppc64DriverBaseline,
 .update = virCPUppc64Update,
 .getModels  = virCPUppc64DriverGetModels,
diff --git a/src/cpu/cpu_s390.c b/src/cpu/cpu_s390.c
index fb352a0..04a6bea 100644
--- a/src/cpu/cpu_s390.c
+++ b/src/cpu/cpu_s390.c
@@ -80,7 +80,6 @@ struct 

[libvirt] [PATCH 12/14] cpu: Introduce virCPUConvertLegacy API

[Jiri Denemark]

PPC driver needs to convert POWERx_v* legacy CPU model names into POWERx
to maintain backward compatibility with existing domains. This patch
adds a new step into the guest CPU configuration work flow which CPU
drivers can use to convert legacy CPU definitions.

Signed-off-by: Jiri Denemark 
---
 src/cpu/cpu.c| 35 +++
 src/cpu/cpu.h|  8 
 src/cpu/cpu_ppc64.c  | 30 --
 src/libvirt_private.syms |  1 +
 src/qemu/qemu_process.c  |  3 +++
 tests/cputest.c  |  5 -
 6 files changed, 63 insertions(+), 19 deletions(-)

diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 9d34206..4a5fbb1 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -934,3 +934,38 @@ virCPUTranslate(virArch arch,
 VIR_DEBUG("model=%s", NULLSTR(cpu->model));
 return 0;
 }
+
+
+/**
+ * virCPUConvertLegacy:
+ *
+ * @arch: CPU architecture
+ * @cpu: CPU definition to be converted
+ *
+ * Convert legacy CPU definition into one that the corresponding cpu driver
+ * will be able to work with. Currently this is only implemented by the PPC
+ * driver, which needs to convert legacy POWERx_v* names into POWERx.
+ *
+ * Returns -1 on error, 0 on success.
+ */
+int
+virCPUConvertLegacy(virArch arch,
+virCPUDefPtr cpu)
+{
+struct cpuArchDriver *driver;
+
+VIR_DEBUG("arch=%s, cpu=%p, model=%s",
+  virArchToString(arch), cpu, NULLSTR(cpu->model));
+
+if (!(driver = cpuGetSubDriver(arch)))
+return -1;
+
+if (!driver->convertLegacy)
+return 0;
+
+if (driver->convertLegacy(cpu) < 0)
+return -1;
+
+VIR_DEBUG("model=%s", NULLSTR(cpu->model));
+return 0;
+}
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index 5ad8112..ff48fb5 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -112,6 +112,9 @@ typedef int
const char **models,
unsigned int nmodels);
 
+typedef int
+(*virCPUArchConvertLegacy)(virCPUDefPtr cpu);
+
 struct cpuArchDriver {
 const char *name;
 const virArch *arch;
@@ -130,6 +133,7 @@ struct cpuArchDriver {
 virCPUArchDataParse dataParse;
 virCPUArchGetModels getModels;
 virCPUArchTranslate translate;
+virCPUArchConvertLegacy convertLegacy;
 };
 
 
@@ -229,6 +233,10 @@ virCPUTranslate(virArch arch,
 unsigned int nmodels)
 ATTRIBUTE_NONNULL(2);
 
+int
+virCPUConvertLegacy(virArch arch,
+virCPUDefPtr cpu)
+ATTRIBUTE_NONNULL(2);
 
 /* virCPUDataFormat and virCPUDataParse are implemented for unit tests only and
  * have no real-life usage
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index bdb026b..8b71ef5 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -63,26 +63,18 @@ struct ppc64_map {
  *   POWER7_v2.3  => POWER7
  *   POWER7+_v2.1 => POWER7
  *   POWER8_v1.0  => POWER8 */
-static virCPUDefPtr
-ppc64ConvertLegacyCPUDef(const virCPUDef *legacy)
+static int
+virCPUppc64ConvertLegacy(virCPUDefPtr cpu)
 {
-virCPUDefPtr cpu;
-
-if (!(cpu = virCPUDefCopy(legacy)))
-goto out;
-
-if (!cpu->model ||
-!(STREQ(cpu->model, "POWER7_v2.1") ||
-  STREQ(cpu->model, "POWER7_v2.3") ||
-  STREQ(cpu->model, "POWER7+_v2.1") ||
-  STREQ(cpu->model, "POWER8_v1.0"))) {
-goto out;
+if (cpu->model &&
+(STREQ(cpu->model, "POWER7_v2.1") ||
+ STREQ(cpu->model, "POWER7_v2.3") ||
+ STREQ(cpu->model, "POWER7+_v2.1") ||
+ STREQ(cpu->model, "POWER8_v1.0"))) {
+cpu->model[strlen("POWERx")] = 0;
 }
 
-cpu->model[strlen("POWERx")] = 0;
-
- out:
-return cpu;
+return 0;
 }
 
 /* Some hosts can run guests in compatibility mode, but not all
@@ -519,7 +511,8 @@ ppc64Compute(virCPUDefPtr host,
 size_t i;
 
 /* Ensure existing configurations are handled correctly */
-if (!(cpu = ppc64ConvertLegacyCPUDef(other)))
+if (!(cpu = virCPUDefCopy(other)) ||
+virCPUppc64ConvertLegacy(cpu) < 0)
 goto cleanup;
 
 if (cpu->arch != VIR_ARCH_NONE) {
@@ -922,4 +915,5 @@ struct cpuArchDriver cpuDriverPPC64 = {
 .baseline   = ppc64DriverBaseline,
 .update = virCPUppc64Update,
 .getModels  = virCPUppc64DriverGetModels,
+.convertLegacy = virCPUppc64ConvertLegacy,
 };
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 41b674d..38f8ecb 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -983,6 +983,7 @@ cpuNodeData;
 virCPUCheckFeature;
 virCPUCompare;
 virCPUCompareXML;
+virCPUConvertLegacy;
 virCPUDataCheckFeature;
 virCPUDataFormat;
 virCPUDataParse;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 14c799e..3552a31 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5066,6 +5066,9 @@ qemuProcessUpdateGuestCPU(virDomainDefPtr def,
 return -1;
 }
 
+if (virCPUConvertLegacy(caps->host.arch, def->cpu) < 0)
+

[libvirt] [PATCH 06/14] cputest: Don't use unsupported preferred model

[Jiri Denemark]

Using a preferred CPU model which is not in the list of CPU models
supported by a hypervisor does not make sense.

Signed-off-by: Jiri Denemark 
---
 tests/cputest.c   |  2 --
 tests/cputestdata/x86-host+guest,models,Penryn-result.xml | 13 -
 tests/cputestdata/x86-host+guest,models,qemu64-result.xml | 13 -
 3 files changed, 28 deletions(-)
 delete mode 100644 tests/cputestdata/x86-host+guest,models,Penryn-result.xml
 delete mode 100644 tests/cputestdata/x86-host+guest,models,qemu64-result.xml

diff --git a/tests/cputest.c b/tests/cputest.c
index 7f27521..16be61b 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -797,8 +797,6 @@ mymain(void)
 DO_TEST_GUESTDATA("x86", "host", "penryn-force", NULL, NULL, 0);
 DO_TEST_GUESTDATA("x86", "host", "guest", model486, NULL, 0);
 DO_TEST_GUESTDATA("x86", "host", "guest", models, NULL, 0);
-DO_TEST_GUESTDATA("x86", "host", "guest", models, "Penryn", 0);
-DO_TEST_GUESTDATA("x86", "host", "guest", models, "qemu64", 0);
 DO_TEST_GUESTDATA("x86", "host", "guest", nomodel, NULL, -1);
 DO_TEST_GUESTDATA("x86", "host", "guest-nofallback", models, "Penryn", -1);
 DO_TEST_GUESTDATA("x86", "host", "host+host-model", models, "Penryn", 0);
diff --git a/tests/cputestdata/x86-host+guest,models,Penryn-result.xml 
b/tests/cputestdata/x86-host+guest,models,Penryn-result.xml
deleted file mode 100644
index 6cd0668..000
--- a/tests/cputestdata/x86-host+guest,models,Penryn-result.xml
+++ /dev/null
@@ -1,13 +0,0 @@
-
-  x86_64
-  Nehalem
-  
-  
-  
-  
-  
-  
-  
-  
-  
-
diff --git a/tests/cputestdata/x86-host+guest,models,qemu64-result.xml 
b/tests/cputestdata/x86-host+guest,models,qemu64-result.xml
deleted file mode 100644
index 8b170e5..000
--- a/tests/cputestdata/x86-host+guest,models,qemu64-result.xml
+++ /dev/null
@@ -1,13 +0,0 @@
-
-  x86_64
-  qemu64
-  
-  
-  
-  
-  
-  
-  
-  
-  
-
-- 
2.10.2

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

[libvirt] [PATCH 03/14] cpu: Rename cpuDataParse

[Jiri Denemark]

The new name is virCPUDataParse.

Signed-off-by: Jiri Denemark 
---
 src/cpu/cpu.c| 4 ++--
 src/cpu/cpu.h| 8 
 src/cpu/cpu_x86.c| 4 ++--
 src/libvirt_private.syms | 2 +-
 tests/cputest.c  | 2 +-
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index e8bcce8..a357237 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -764,7 +764,7 @@ cpuDataFormat(const virCPUData *data)
 
 
 /**
- * cpuDataParse:
+ * virCPUDataParse:
  *
  * @xmlStr: XML string produced by cpuDataFormat
  *
@@ -773,7 +773,7 @@ cpuDataFormat(const virCPUData *data)
  * Returns internal CPU data structure parsed from the XML or NULL on error.
  */
 virCPUDataPtr
-cpuDataParse(const char *xmlStr)
+virCPUDataParse(const char *xmlStr)
 {
 struct cpuArchDriver *driver;
 xmlDocPtr xml = NULL;
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index 86cb96b..c2d6b12 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -102,7 +102,7 @@ typedef char *
 (*cpuArchDataFormat)(const virCPUData *data);
 
 typedef virCPUDataPtr
-(*cpuArchDataParse) (xmlXPathContextPtr ctxt);
+(*virCPUArchDataParse)(xmlXPathContextPtr ctxt);
 
 typedef int
 (*virCPUArchGetModels)(char ***models);
@@ -127,7 +127,7 @@ struct cpuArchDriver {
 virCPUArchCheckFeature checkFeature;
 virCPUArchDataCheckFeature dataCheckFeature;
 cpuArchDataFormat   dataFormat;
-cpuArchDataParsedataParse;
+virCPUArchDataParse dataParse;
 virCPUArchGetModels getModels;
 virCPUArchTranslate translate;
 };
@@ -230,12 +230,12 @@ virCPUTranslate(virArch arch,
 ATTRIBUTE_NONNULL(2);
 
 
-/* cpuDataFormat and cpuDataParse are implemented for unit tests only and
+/* cpuDataFormat and virCPUDataParse are implemented for unit tests only and
  * have no real-life usage
  */
 char *cpuDataFormat(const virCPUData *data)
 ATTRIBUTE_NONNULL(1);
-virCPUDataPtr cpuDataParse(const char *xmlStr)
+virCPUDataPtr virCPUDataParse(const char *xmlStr)
 ATTRIBUTE_NONNULL(1);
 
 #endif /* __VIR_CPU_H__ */
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 5dfe549..c3a3940 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -1437,7 +1437,7 @@ x86CPUDataFormat(const virCPUData *data)
 
 
 static virCPUDataPtr
-x86CPUDataParse(xmlXPathContextPtr ctxt)
+virCPUx86DataParse(xmlXPathContextPtr ctxt)
 {
 xmlNodePtr *nodes = NULL;
 virCPUDataPtr cpuData = NULL;
@@ -2766,7 +2766,7 @@ struct cpuArchDriver cpuDriverX86 = {
 .checkFeature = virCPUx86CheckFeature,
 .dataCheckFeature = virCPUx86DataCheckFeature,
 .dataFormat = x86CPUDataFormat,
-.dataParse  = x86CPUDataParse,
+.dataParse  = virCPUx86DataParse,
 .getModels  = virCPUx86GetModels,
 .translate  = virCPUx86Translate,
 };
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8b16dc1..d09f340 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -977,7 +977,6 @@ cpuBaseline;
 cpuBaselineXML;
 cpuDataFormat;
 cpuDataFree;
-cpuDataParse;
 cpuDecode;
 cpuEncode;
 cpuGuestData;
@@ -986,6 +985,7 @@ virCPUCheckFeature;
 virCPUCompare;
 virCPUCompareXML;
 virCPUDataCheckFeature;
+virCPUDataParse;
 virCPUGetModels;
 virCPUTranslate;
 virCPUUpdate;
diff --git a/tests/cputest.c b/tests/cputest.c
index 6842d27..0d7f3ec 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -472,7 +472,7 @@ cpuTestCPUID(bool guest, const void *arg)
 goto cleanup;
 
 if (virTestLoadFile(hostFile, ) < 0 ||
-!(hostData = cpuDataParse(host)))
+!(hostData = virCPUDataParse(host)))
 goto cleanup;
 
 if (VIR_ALLOC(cpu) < 0)
-- 
2.10.2

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

[libvirt] [PATCH 07/14] cputest: Don't use preferred model with forbidden fallback

[Jiri Denemark]

Using a preferred model for guest CPUs with forbidden fallback masks a
bug in the code. It would just happily use another CPU model supported
by a hypervisor even though it is explicitly forbidden in the CPU XML.

This patch temporarily changes the expected result to -2, which is used
when the result XML file cannot be found (but it was supposed not to be
found since the tested API should have failed). The result will be
switched back to -1 few commits later when the original bug gets fixed.

Signed-off-by: Jiri Denemark 
---
 tests/cputest.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/tests/cputest.c b/tests/cputest.c
index 16be61b..7496156 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -798,16 +798,14 @@ mymain(void)
 DO_TEST_GUESTDATA("x86", "host", "guest", model486, NULL, 0);
 DO_TEST_GUESTDATA("x86", "host", "guest", models, NULL, 0);
 DO_TEST_GUESTDATA("x86", "host", "guest", nomodel, NULL, -1);
-DO_TEST_GUESTDATA("x86", "host", "guest-nofallback", models, "Penryn", -1);
+DO_TEST_GUESTDATA("x86", "host", "guest-nofallback", models, NULL, /*-1*/ 
-2);
 DO_TEST_GUESTDATA("x86", "host", "host+host-model", models, "Penryn", 0);
-DO_TEST_GUESTDATA("x86", "host", "host+host-model-nofallback",
-  models, "Penryn", -1);
+DO_TEST_GUESTDATA("x86", "host", "host+host-model-nofallback", models, 
NULL, /*-1*/ -2);
 DO_TEST_GUESTDATA("x86", "host-Haswell-noTSX", "Haswell",
   haswell, "Haswell", 0);
 DO_TEST_GUESTDATA("x86", "host-Haswell-noTSX", "Haswell-noTSX",
   haswell, "Haswell-noTSX", 0);
-DO_TEST_GUESTDATA("x86", "host-Haswell-noTSX", "Haswell-noTSX-nofallback",
-  haswell, "Haswell-noTSX", -1);
+DO_TEST_GUESTDATA("x86", "host-Haswell-noTSX", "Haswell-noTSX-nofallback", 
haswell, NULL, /*-1*/ -2);
 DO_TEST_GUESTDATA("x86", "host-Haswell-noTSX", "Haswell-noTSX",
   NULL, "Haswell-noTSX", 0);
 
-- 
2.10.2

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

[libvirt] [PATCH 11/14] cputest: Don't test cpuGuestData

[Jiri Denemark]

The API is no longer used anywhere else since it was replaced by a much
saner work flow utilizing new APIs that work on virCPUDefPtr directly:
virCPUCompare, virCPUUpdate, and virCPUTranslate.

Not testing the new work flow caused some bugs to be hidden. This patch
reveals them, but doesn't attempt to fix them. To make sure all test
still pass after this patch, all affected test results are modified to
pretend the tests succeeded. All of the bugs will be fixed in the
following commits and the artificial modifications will be reverted.

The following is the list of bugs in the new CPU model work flow:

- a guest CPU with mode='custom' and missing  gets the vendor
  copied from host's CPU (the vendor should only be copied to host-model
  CPUs):
DO_TEST_UPDATE("x86", "host", "min", VIR_CPU_COMPARE_IDENTICAL)
DO_TEST_UPDATE("x86", "host", "pentium3", VIR_CPU_COMPARE_IDENTICAL)
DO_TEST_GUESTCPU("x86", "host-better", "pentium3", NULL, 0)

- when a guest CPU with mode='custom' needs to be translated into
  another model because the original model is not supported by a
  hypervisor, the result will have its vendor set to the vendor of the
  original CPU model as specified in cpu_map.xml even if the original
  guest CPU XML didn't contain :
DO_TEST_GUESTCPU("x86", "host", "guest", model486, 0)
DO_TEST_GUESTCPU("x86", "host", "guest", models, 0)
DO_TEST_GUESTCPU("x86", "host-Haswell-noTSX", "Haswell-noTSX",
 haswell, 0)

- legacy POWERx_v* model names are not recognized:
DO_TEST_GUESTCPU("ppc64", "host", "guest-legacy", ppc_models, 0)

Signed-off-by: Jiri Denemark 
---
 tests/cputest.c| 65 +-
 .../ppc64-host+guest,ppc_models-result.xml |  2 -
 .../ppc64-host+guest-legacy,ppc_models-result.xml  |  2 -
 .../cputestdata/x86-host+guest,model486-result.xml | 17 --
 tests/cputestdata/x86-host+guest,models-result.xml | 17 --
 tests/cputestdata/x86-host+guest-result.xml| 18 --
 .../x86-host+host+host-model,models-result.xml |  3 +-
 tests/cputestdata/x86-host+penryn-force-result.xml |  5 +-
 .../x86-host+strict-force-extra-result.xml | 31 +--
 ...6-host-Haswell-noTSX+Haswell,haswell-result.xml |  4 +-
 ...-Haswell-noTSX+Haswell-noTSX,haswell-result.xml |  3 +-
 ...x86-host-Haswell-noTSX+Haswell-noTSX-result.xml |  2 +-
 .../x86-host-better+pentium3-result.xml| 28 +-
 tests/cputestdata/x86-host-worse+guest-result.xml  | 18 --
 14 files changed, 113 insertions(+), 102 deletions(-)

diff --git a/tests/cputest.c b/tests/cputest.c
index 6aebfb0..1d7f6bc 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -237,14 +237,12 @@ cpuTestCompare(const void *arg)
 
 
 static int
-cpuTestGuestData(const void *arg)
+cpuTestGuestCPU(const void *arg)
 {
 const struct data *data = arg;
 int ret = -2;
 virCPUDefPtr host = NULL;
 virCPUDefPtr cpu = NULL;
-virCPUDefPtr guest = NULL;
-virCPUDataPtr guestData = NULL;
 virCPUCompareResult cmpResult;
 virBuffer buf = VIR_BUFFER_INITIALIZER;
 char *result = NULL;
@@ -253,22 +251,15 @@ cpuTestGuestData(const void *arg)
 !(cpu = cpuTestLoadXML(data->arch, data->name)))
 goto cleanup;
 
-cmpResult = cpuGuestData(host, cpu, , NULL);
+cmpResult = virCPUCompare(host->arch, host, cpu, false);
 if (cmpResult == VIR_CPU_COMPARE_ERROR ||
 cmpResult == VIR_CPU_COMPARE_INCOMPATIBLE) {
 ret = -1;
 goto cleanup;
 }
 
-if (VIR_ALLOC(guest) < 0)
-goto cleanup;
-
-guest->arch = host->arch;
-guest->type = VIR_CPU_TYPE_GUEST;
-guest->match = VIR_CPU_MATCH_EXACT;
-guest->fallback = cpu->fallback;
-if (cpuDecode(guest, guestData, data->models,
-  data->nmodels, NULL) < 0) {
+if (virCPUUpdate(host->arch, cpu, host) < 0 ||
+virCPUTranslate(host->arch, cpu, data->models, data->nmodels) < 0) {
 ret = -1;
 goto cleanup;
 }
@@ -284,17 +275,15 @@ cpuTestGuestData(const void *arg)
 }
 result = virBufferContentAndReset();
 
-if (cpuTestCompareXML(data->arch, guest, result, false) < 0)
+if (cpuTestCompareXML(data->arch, cpu, result, false) < 0)
 goto cleanup;
 
 ret = 0;
 
  cleanup:
 VIR_FREE(result);
-cpuDataFree(guestData);
 virCPUDefFree(host);
 virCPUDefFree(cpu);
-virCPUDefFree(guest);
 
 if (ret == data->result) {
 /* We got the result we expected, whether it was
@@ -656,8 +645,8 @@ mymain(void)
 host "/" feature " (" #result ")",  \
 host, feature, NULL, 0, 0, result)
 
-#define DO_TEST_GUESTDATA(arch, host, cpu, models, result)  \
-DO_TEST(arch, cpuTestGuestData, \
+#define DO_TEST_GUESTCPU(arch, host, cpu, models, result)   \
+DO_TEST(arch, cpuTestGuestCPU,

[libvirt] [PATCH 05/14] cputest: Don't use preferred model for minimum match CPUs

[Jiri Denemark]

Guest CPUs with match='minimum' should always be updated to match host
CPU model. Trying to get different results by supplying preferred models
does not make sense.

Signed-off-by: Jiri Denemark 
---
 tests/cputest.c|  2 --
 .../x86-host-better+pentium3,core2duo-result.xml   | 21 ---
 .../x86-host-better+pentium3,pentium3-result.xml   | 30 --
 3 files changed, 53 deletions(-)
 delete mode 100644 
tests/cputestdata/x86-host-better+pentium3,core2duo-result.xml
 delete mode 100644 
tests/cputestdata/x86-host-better+pentium3,pentium3-result.xml

diff --git a/tests/cputest.c b/tests/cputest.c
index 0d7f3ec..7f27521 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -792,8 +792,6 @@ mymain(void)
 /* computing guest data and decoding the data into a guest CPU XML */
 DO_TEST_GUESTDATA("x86", "host", "guest", NULL, NULL, 0);
 DO_TEST_GUESTDATA("x86", "host-better", "pentium3", NULL, NULL, 0);
-DO_TEST_GUESTDATA("x86", "host-better", "pentium3", NULL, "pentium3", 0);
-DO_TEST_GUESTDATA("x86", "host-better", "pentium3", NULL, "core2duo", 0);
 DO_TEST_GUESTDATA("x86", "host-worse", "guest", NULL, NULL, 0);
 DO_TEST_GUESTDATA("x86", "host", "strict-force-extra", NULL, NULL, 0);
 DO_TEST_GUESTDATA("x86", "host", "penryn-force", NULL, NULL, 0);
diff --git a/tests/cputestdata/x86-host-better+pentium3,core2duo-result.xml 
b/tests/cputestdata/x86-host-better+pentium3,core2duo-result.xml
deleted file mode 100644
index 2bf691c..000
--- a/tests/cputestdata/x86-host-better+pentium3,core2duo-result.xml
+++ /dev/null
@@ -1,21 +0,0 @@
-
-  x86_64
-  core2duo
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-
diff --git a/tests/cputestdata/x86-host-better+pentium3,pentium3-result.xml 
b/tests/cputestdata/x86-host-better+pentium3,pentium3-result.xml
deleted file mode 100644
index d1d0c4b..000
--- a/tests/cputestdata/x86-host-better+pentium3,pentium3-result.xml
+++ /dev/null
@@ -1,30 +0,0 @@
-
-  x86_64
-  pentium3
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-  
-
-- 
2.10.2

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

[libvirt] [PATCH 13/14] cpu: Avoid adding to custom CPUs

[Jiri Denemark]

Guest CPU definitions with mode='custom' and missing  are
expected to run on a host CPU from any vendor as long as the required
CPU model can be used as a guest CPU on the host. But even though no CPU
vendor was explicitly requested we would sometimes force it due to a bug
in virCPUUpdate and virCPUTranslate.

The bug would effectively forbid cross vendor migrations even if they
were previously working just fine.

Signed-off-by: Jiri Denemark 
---
 src/conf/cpu_conf.c| 28 +++---
 src/conf/cpu_conf.h|  3 ++-
 src/cpu/cpu_arm.c  |  2 +-
 src/cpu/cpu_x86.c  |  5 ++--
 .../cputestdata/x86-host+guest,model486-result.xml |  1 -
 tests/cputestdata/x86-host+guest,models-result.xml |  1 -
 tests/cputestdata/x86-host+min.xml |  1 -
 tests/cputestdata/x86-host+pentium3.xml|  1 -
 ...-Haswell-noTSX+Haswell-noTSX,haswell-result.xml |  1 -
 .../x86-host-better+pentium3-result.xml|  1 -
 10 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index f174529..9eb69c9 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -132,20 +132,42 @@ virCPUDefCopyModelFilter(virCPUDefPtr dst,
 }
 
 
+/**
+ * virCPUDefStealModel:
+ *
+ * Move CPU model related parts virCPUDef from @src to @dst. If @keepVendor
+ * is true, the function keeps the original vendor/vendor_id in @dst rather
+ * than overwriting it with the values from @src.
+ */
 void
 virCPUDefStealModel(virCPUDefPtr dst,
-virCPUDefPtr src)
+virCPUDefPtr src,
+bool keepVendor)
 {
+char *vendor;
+char *vendor_id;
+
+if (keepVendor) {
+VIR_STEAL_PTR(vendor, dst->vendor);
+VIR_STEAL_PTR(vendor_id, dst->vendor_id);
+}
+
 virCPUDefFreeModel(dst);
 
 VIR_STEAL_PTR(dst->model, src->model);
-VIR_STEAL_PTR(dst->vendor, src->vendor);
-VIR_STEAL_PTR(dst->vendor_id, src->vendor_id);
 VIR_STEAL_PTR(dst->features, src->features);
 dst->nfeatures_max = src->nfeatures_max;
 src->nfeatures_max = 0;
 dst->nfeatures = src->nfeatures;
 src->nfeatures = 0;
+
+if (keepVendor) {
+dst->vendor = vendor;
+dst->vendor_id = vendor_id;
+} else {
+VIR_STEAL_PTR(dst->vendor, src->vendor);
+VIR_STEAL_PTR(dst->vendor_id, src->vendor_id);
+}
 }
 
 
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index e084392..cc3fbf0 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -139,7 +139,8 @@ virCPUDefCopyModelFilter(virCPUDefPtr dst,
 
 void
 virCPUDefStealModel(virCPUDefPtr dst,
-virCPUDefPtr src);
+virCPUDefPtr src,
+bool keepVendor);
 
 virCPUDefPtr
 virCPUDefCopy(const virCPUDef *cpu);
diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
index db603a6..b5002c3 100644
--- a/src/cpu/cpu_arm.c
+++ b/src/cpu/cpu_arm.c
@@ -67,7 +67,7 @@ virCPUarmUpdate(virCPUDefPtr guest,
 if (virCPUDefCopyModel(updated, host, true) < 0)
 goto cleanup;
 
-virCPUDefStealModel(guest, updated);
+virCPUDefStealModel(guest, updated, false);
 guest->mode = VIR_CPU_MODE_CUSTOM;
 guest->match = VIR_CPU_MATCH_EXACT;
 ret = 0;
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 4f2a111..851ec5d 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -2576,7 +2576,8 @@ x86UpdateHostModel(virCPUDefPtr guest,
 goto cleanup;
 }
 
-virCPUDefStealModel(guest, updated);
+virCPUDefStealModel(guest, updated,
+guest->mode == VIR_CPU_MODE_CUSTOM);
 guest->mode = VIR_CPU_MODE_CUSTOM;
 guest->match = VIR_CPU_MATCH_EXACT;
 ret = 0;
@@ -2737,7 +2738,7 @@ virCPUx86Translate(virCPUDefPtr cpu,
 goto cleanup;
 }
 
-virCPUDefStealModel(cpu, translated);
+virCPUDefStealModel(cpu, translated, true);
 ret = 0;
 
  cleanup:
diff --git a/tests/cputestdata/x86-host+guest,model486-result.xml 
b/tests/cputestdata/x86-host+guest,model486-result.xml
index 88df467..85564ff 100644
--- a/tests/cputestdata/x86-host+guest,model486-result.xml
+++ b/tests/cputestdata/x86-host+guest,model486-result.xml
@@ -1,6 +1,5 @@
 
   486
-  Intel
   
   
   
diff --git a/tests/cputestdata/x86-host+guest,models-result.xml 
b/tests/cputestdata/x86-host+guest,models-result.xml
index e7a77c2..f79ed32 100644
--- a/tests/cputestdata/x86-host+guest,models-result.xml
+++ b/tests/cputestdata/x86-host+guest,models-result.xml
@@ -1,6 +1,5 @@
 
   Nehalem
-  Intel
   
   
   
diff --git a/tests/cputestdata/x86-host+min.xml 
b/tests/cputestdata/x86-host+min.xml
index a284767..8101151 100644
--- a/tests/cputestdata/x86-host+min.xml
+++ b/tests/cputestdata/x86-host+min.xml
@@ -1,6 +1,5 @@
 
   Penryn
-  Intel
   
   
   
diff --git a/tests/cputestdata/x86-host+pentium3.xml 

[libvirt] [PATCH 10/14] cpu: Make models array in virCPUTranslate constant

[Jiri Denemark]

The API doesn't change the array so let's make it constant.

Signed-off-by: Jiri Denemark 
---
 src/cpu/cpu.c   | 6 +++---
 src/cpu/cpu.h   | 2 +-
 src/qemu/qemu_process.c | 3 ++-
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 5040669..9d34206 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -896,7 +896,7 @@ virCPUGetModels(virArch arch, char ***models)
 int
 virCPUTranslate(virArch arch,
 virCPUDefPtr cpu,
-char **models,
+const char **models,
 unsigned int nmodels)
 {
 struct cpuArchDriver *driver;
@@ -911,7 +911,7 @@ virCPUTranslate(virArch arch,
 cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH)
 return 0;
 
-if (virCPUModelIsAllowed(cpu->model, (const char **) models, nmodels))
+if (virCPUModelIsAllowed(cpu->model, models, nmodels))
 return 0;
 
 if (cpu->fallback != VIR_CPU_FALLBACK_ALLOW) {
@@ -928,7 +928,7 @@ virCPUTranslate(virArch arch,
 return -1;
 }
 
-if (driver->translate(cpu, (const char **) models, nmodels) < 0)
+if (driver->translate(cpu, models, nmodels) < 0)
 return -1;
 
 VIR_DEBUG("model=%s", NULLSTR(cpu->model));
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index 917d847..5ad8112 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -225,7 +225,7 @@ virCPUGetModels(virArch arch, char ***models);
 int
 virCPUTranslate(virArch arch,
 virCPUDefPtr cpu,
-char **models,
+const char **models,
 unsigned int nmodels)
 ATTRIBUTE_NONNULL(2);
 
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 09b2a72..14c799e 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5083,7 +5083,8 @@ qemuProcessUpdateGuestCPU(virDomainDefPtr def,
 goto cleanup;
 
 if (virQEMUCapsGetCPUDefinitions(qemuCaps, , ) < 0 ||
-virCPUTranslate(def->os.arch, def->cpu, models, nmodels) < 0)
+virCPUTranslate(def->os.arch, def->cpu,
+(const char **) models, nmodels) < 0)
 goto cleanup;
 
 def->cpu->fallback = VIR_CPU_FALLBACK_FORBID;
-- 
2.10.2

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

[libvirt] [PATCH 00/14] Another round of CPU driver changes

[Jiri Denemark]

This is another step in revisiting all APIs provided by the cpu driver
and making them a bit more sane. As with the previous round(s), the
updated APIs will gain virCPU prefix so that they can be easily
distinguished from the ones that still need some work.

Jiri Denemark (14):
  cpu: Rename cpuGetModels
  cpu: Rename and document cpuModelIsAllowed
  cpu: Rename cpuDataParse
  cpu: Rename cpuDataFormat
  cputest: Don't use preferred model for minimum match CPUs
  cputest: Don't use unsupported preferred model
  cputest: Don't use preferred model with forbidden fallback
  cputest: Don't use superfluous preferred model
  cputest: Don't use preferred CPU model at all
  cpu: Make models array in virCPUTranslate constant
  cputest: Don't test cpuGuestData
  cpu: Introduce virCPUConvertLegacy API
  cpu: Avoid adding  to custom CPUs
  cpu: Drop cpuGuestData

 src/conf/cpu_conf.c|  28 -
 src/conf/cpu_conf.h|   3 +-
 src/cpu/cpu.c  | 115 +++--
 src/cpu/cpu.h  |  50 -
 src/cpu/cpu_arm.c  |  12 +--
 src/cpu/cpu_ppc64.c|  45 +++-
 src/cpu/cpu_s390.c |   1 -
 src/cpu/cpu_x86.c  |  36 ++-
 src/libvirt_private.syms   |   8 +-
 src/qemu/qemu_capabilities.c   |   2 +-
 src/qemu/qemu_driver.c |   2 +-
 src/qemu/qemu_process.c|   6 +-
 src/test/test_driver.c |   2 +-
 tests/cputest.c| 104 ---
 .../ppc64-host+guest,ppc_models-result.xml |   2 -
 .../ppc64-host+guest-legacy,ppc_models-result.xml  |   2 -
 .../cputestdata/x86-host+guest,model486-result.xml |  16 ++-
 .../x86-host+guest,models,Penryn-result.xml|  13 ---
 .../x86-host+guest,models,qemu64-result.xml|  13 ---
 tests/cputestdata/x86-host+guest,models-result.xml |  16 +--
 tests/cputestdata/x86-host+guest-result.xml|  18 ++--
 ... => x86-host+host+host-model,models-result.xml} |   3 +-
 tests/cputestdata/x86-host+min.xml |   1 -
 tests/cputestdata/x86-host+penryn-force-result.xml |   5 +-
 tests/cputestdata/x86-host+pentium3.xml|   1 -
 .../x86-host+strict-force-extra-result.xml |  31 +++---
 ...-host-Haswell-noTSX+Haswell,haswell-result.xml} |   4 +-
 ...Haswell-noTSX+Haswell-noTSX,haswell-result.xml} |   2 +-
 ...86-host-Haswell-noTSX+Haswell-noTSX-result.xml} |   2 +-
 .../x86-host-better+pentium3,core2duo-result.xml   |  21 
 .../x86-host-better+pentium3,pentium3-result.xml   |  30 --
 .../x86-host-better+pentium3-result.xml|  27 +++--
 tests/cputestdata/x86-host-worse+guest-result.xml  |  18 +++-
 tests/qemumonitorjsontest.c|   2 +-
 34 files changed, 273 insertions(+), 368 deletions(-)
 delete mode 100644 tests/cputestdata/x86-host+guest,models,Penryn-result.xml
 delete mode 100644 tests/cputestdata/x86-host+guest,models,qemu64-result.xml
 rename tests/cputestdata/{x86-host+host+host-model,models,Penryn-result.xml => 
x86-host+host+host-model,models-result.xml} (89%)
 rename 
tests/cputestdata/{x86-host-Haswell-noTSX+Haswell-noTSX,haswell,Haswell-noTSX-result.xml
 => x86-host-Haswell-noTSX+Haswell,haswell-result.xml} (77%)
 rename 
tests/cputestdata/{x86-host-Haswell-noTSX+Haswell,haswell,Haswell-result.xml => 
x86-host-Haswell-noTSX+Haswell-noTSX,haswell-result.xml} (77%)
 rename 
tests/cputestdata/{x86-host-Haswell-noTSX+Haswell-noTSX,Haswell-noTSX-result.xml
 => x86-host-Haswell-noTSX+Haswell-noTSX-result.xml} (64%)
 delete mode 100644 
tests/cputestdata/x86-host-better+pentium3,core2duo-result.xml
 delete mode 100644 
tests/cputestdata/x86-host-better+pentium3,pentium3-result.xml

-- 
2.10.2

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

[libvirt] [PATCH 01/14] cpu: Rename cpuGetModels

[Jiri Denemark]

The new name is virCPUGetModels.

Signed-off-by: Jiri Denemark 
---
 src/cpu/cpu.c| 4 ++--
 src/cpu/cpu.h| 6 +++---
 src/cpu/cpu_ppc64.c  | 4 ++--
 src/cpu/cpu_x86.c| 4 ++--
 src/libvirt_private.syms | 2 +-
 src/qemu/qemu_capabilities.c | 2 +-
 src/qemu/qemu_driver.c   | 2 +-
 src/test/test_driver.c   | 2 +-
 8 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 995d925..0332d50 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -831,7 +831,7 @@ cpuModelIsAllowed(const char *model,
 }
 
 /**
- * cpuGetModels:
+ * virCPUGetModels:
  *
  * @arch: CPU architecture
  * @models: where to store the NULL-terminated list of supported models
@@ -845,7 +845,7 @@ cpuModelIsAllowed(const char *model,
  * or -1 on error.
  */
 int
-cpuGetModels(virArch arch, char ***models)
+virCPUGetModels(virArch arch, char ***models)
 {
 struct cpuArchDriver *driver;
 
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index 51bacb7..82912a4 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -105,7 +105,7 @@ typedef virCPUDataPtr
 (*cpuArchDataParse) (xmlXPathContextPtr ctxt);
 
 typedef int
-(*cpuArchGetModels) (char ***models);
+(*virCPUArchGetModels)(char ***models);
 
 typedef int
 (*virCPUArchTranslate)(virCPUDefPtr cpu,
@@ -128,7 +128,7 @@ struct cpuArchDriver {
 virCPUArchDataCheckFeature dataCheckFeature;
 cpuArchDataFormat   dataFormat;
 cpuArchDataParsedataParse;
-cpuArchGetModelsgetModels;
+virCPUArchGetModels getModels;
 virCPUArchTranslate translate;
 };
 
@@ -220,7 +220,7 @@ cpuModelIsAllowed(const char *model,
 ATTRIBUTE_NONNULL(1);
 
 int
-cpuGetModels(virArch arch, char ***models);
+virCPUGetModels(virArch arch, char ***models);
 
 int
 virCPUTranslate(virArch arch,
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 21ea0e1..bfe50a3 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -876,7 +876,7 @@ ppc64DriverBaseline(virCPUDefPtr *cpus,
 }
 
 static int
-ppc64DriverGetModels(char ***models)
+virCPUppc64DriverGetModels(char ***models)
 {
 struct ppc64_map *map;
 size_t i;
@@ -921,5 +921,5 @@ struct cpuArchDriver cpuDriverPPC64 = {
 .guestData  = ppc64DriverGuestData,
 .baseline   = ppc64DriverBaseline,
 .update = virCPUppc64Update,
-.getModels  = ppc64DriverGetModels,
+.getModels  = virCPUppc64DriverGetModels,
 };
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 64f1c47..34c3e5b 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -2668,7 +2668,7 @@ virCPUx86DataCheckFeature(const virCPUData *data,
 }
 
 static int
-x86GetModels(char ***models)
+virCPUx86GetModels(char ***models)
 {
 virCPUx86MapPtr map;
 size_t i;
@@ -2767,6 +2767,6 @@ struct cpuArchDriver cpuDriverX86 = {
 .dataCheckFeature = virCPUx86DataCheckFeature,
 .dataFormat = x86CPUDataFormat,
 .dataParse  = x86CPUDataParse,
-.getModels  = x86GetModels,
+.getModels  = virCPUx86GetModels,
 .translate  = virCPUx86Translate,
 };
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ac6a1e1..8b16dc1 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -980,13 +980,13 @@ cpuDataFree;
 cpuDataParse;
 cpuDecode;
 cpuEncode;
-cpuGetModels;
 cpuGuestData;
 cpuNodeData;
 virCPUCheckFeature;
 virCPUCompare;
 virCPUCompareXML;
 virCPUDataCheckFeature;
+virCPUGetModels;
 virCPUTranslate;
 virCPUUpdate;
 
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index ad98d68..cfd090c 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4639,7 +4639,7 @@ virQEMUCapsFillDomainCPUCaps(virCapsPtr caps,
 virDomainCapsCPUModelsPtr filtered = NULL;
 char **models = NULL;
 
-if (cpuGetModels(domCaps->arch, ) >= 0) {
+if (virCPUGetModels(domCaps->arch, ) >= 0) {
 filtered = virDomainCapsCPUModelsFilter(qemuCaps->cpuDefinitions,
 (const char **) models);
 virStringFreeList(models);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a82e58b..608b67b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18476,7 +18476,7 @@ qemuConnectGetCPUModelNames(virConnectPtr conn,
 return -1;
 }
 
-return cpuGetModels(arch, models);
+return virCPUGetModels(arch, models);
 }
 
 static int
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 236874f..cc300f0 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -5943,7 +5943,7 @@ testConnectGetCPUModelNames(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 return -1;
 }
 
-return cpuGetModels(arch, models);
+return virCPUGetModels(arch, models);
 }
 
 static int
-- 
2.10.2

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

[libvirt] [PATCH 04/14] cpu: Rename cpuDataFormat

[Jiri Denemark]

The new name is virCPUDataFormat.

Signed-off-by: Jiri Denemark 
---
 src/cpu/cpu.c   | 6 +++---
 src/cpu/cpu.h   | 8 
 src/cpu/cpu_x86.c   | 4 ++--
 src/libvirt_private.syms| 2 +-
 tests/qemumonitorjsontest.c | 2 +-
 5 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index a357237..5040669 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -734,7 +734,7 @@ virCPUDataCheckFeature(const virCPUData *data,
 
 
 /**
- * cpuDataFormat:
+ * virCPUDataFormat:
  *
  * @data: internal CPU representation
  *
@@ -743,7 +743,7 @@ virCPUDataCheckFeature(const virCPUData *data,
  * Returns string representation of the XML describing @data or NULL on error.
  */
 char *
-cpuDataFormat(const virCPUData *data)
+virCPUDataFormat(const virCPUData *data)
 {
 struct cpuArchDriver *driver;
 
@@ -766,7 +766,7 @@ cpuDataFormat(const virCPUData *data)
 /**
  * virCPUDataParse:
  *
- * @xmlStr: XML string produced by cpuDataFormat
+ * @xmlStr: XML string produced by virCPUDataFormat
  *
  * Parses XML representation of virCPUData structure for test purposes.
  *
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index c2d6b12..917d847 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -99,7 +99,7 @@ typedef int
   const char *feature);
 
 typedef char *
-(*cpuArchDataFormat)(const virCPUData *data);
+(*virCPUArchDataFormat)(const virCPUData *data);
 
 typedef virCPUDataPtr
 (*virCPUArchDataParse)(xmlXPathContextPtr ctxt);
@@ -126,7 +126,7 @@ struct cpuArchDriver {
 virCPUArchUpdateupdate;
 virCPUArchCheckFeature checkFeature;
 virCPUArchDataCheckFeature dataCheckFeature;
-cpuArchDataFormat   dataFormat;
+virCPUArchDataFormat dataFormat;
 virCPUArchDataParse dataParse;
 virCPUArchGetModels getModels;
 virCPUArchTranslate translate;
@@ -230,10 +230,10 @@ virCPUTranslate(virArch arch,
 ATTRIBUTE_NONNULL(2);
 
 
-/* cpuDataFormat and virCPUDataParse are implemented for unit tests only and
+/* virCPUDataFormat and virCPUDataParse are implemented for unit tests only and
  * have no real-life usage
  */
-char *cpuDataFormat(const virCPUData *data)
+char *virCPUDataFormat(const virCPUData *data)
 ATTRIBUTE_NONNULL(1);
 virCPUDataPtr virCPUDataParse(const char *xmlStr)
 ATTRIBUTE_NONNULL(1);
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index c3a3940..4f2a111 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -1412,7 +1412,7 @@ virCPUx86GetMap(void)
 
 
 static char *
-x86CPUDataFormat(const virCPUData *data)
+virCPUx86DataFormat(const virCPUData *data)
 {
 virCPUx86DataIterator iter = virCPUx86DataIteratorInit(>data.x86);
 virCPUx86CPUID *cpuid;
@@ -2765,7 +2765,7 @@ struct cpuArchDriver cpuDriverX86 = {
 .update = virCPUx86Update,
 .checkFeature = virCPUx86CheckFeature,
 .dataCheckFeature = virCPUx86DataCheckFeature,
-.dataFormat = x86CPUDataFormat,
+.dataFormat = virCPUx86DataFormat,
 .dataParse  = virCPUx86DataParse,
 .getModels  = virCPUx86GetModels,
 .translate  = virCPUx86Translate,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d09f340..41b674d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -975,7 +975,6 @@ virSecretObjSetValueSize;
 # cpu/cpu.h
 cpuBaseline;
 cpuBaselineXML;
-cpuDataFormat;
 cpuDataFree;
 cpuDecode;
 cpuEncode;
@@ -985,6 +984,7 @@ virCPUCheckFeature;
 virCPUCompare;
 virCPUCompareXML;
 virCPUDataCheckFeature;
+virCPUDataFormat;
 virCPUDataParse;
 virCPUGetModels;
 virCPUTranslate;
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index d8fd958..9f889a9 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -2329,7 +2329,7 @@ testQemuMonitorJSONGetCPUData(const void *opaque)
) < 0)
 goto cleanup;
 
-if (!(actual = cpuDataFormat(cpuData)))
+if (!(actual = virCPUDataFormat(cpuData)))
 goto cleanup;
 
 if (virTestCompareToFile(actual, dataFile) < 0)
-- 
2.10.2

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

[libvirt] [PATCH 02/14] cpu: Rename and document cpuModelIsAllowed

[Jiri Denemark]

The new name is virCPUModelIsAllowed.

Signed-off-by: Jiri Denemark 
---
 src/cpu/cpu.c   | 21 +
 src/cpu/cpu.h   |  6 +++---
 src/cpu/cpu_ppc64.c |  2 +-
 src/cpu/cpu_x86.c   |  2 +-
 4 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 0332d50..e8bcce8 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -813,10 +813,22 @@ cpuDataParse(const char *xmlStr)
 return data;
 }
 
+
+/** virCPUModelIsAllowed:
+ *
+ * @model: CPU model to be checked
+ * @models: list of supported CPU models
+ * @nmodels: number of models in @models
+ *
+ * Checks whether @model can be found in the list of supported @models.
+ * If @models is empty, all models are supported.
+ *
+ * Returns true if @model is supported, false otherwise.
+ */
 bool
-cpuModelIsAllowed(const char *model,
-  const char **models,
-  unsigned int nmodels)
+virCPUModelIsAllowed(const char *model,
+ const char **models,
+ unsigned int nmodels)
 {
 size_t i;
 
@@ -830,6 +842,7 @@ cpuModelIsAllowed(const char *model,
 return false;
 }
 
+
 /**
  * virCPUGetModels:
  *
@@ -898,7 +911,7 @@ virCPUTranslate(virArch arch,
 cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH)
 return 0;
 
-if (cpuModelIsAllowed(cpu->model, (const char **) models, nmodels))
+if (virCPUModelIsAllowed(cpu->model, (const char **) models, nmodels))
 return 0;
 
 if (cpu->fallback != VIR_CPU_FALLBACK_ALLOW) {
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index 82912a4..86cb96b 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -214,9 +214,9 @@ virCPUDataCheckFeature(const virCPUData *data,
 
 
 bool
-cpuModelIsAllowed(const char *model,
-  const char **models,
-  unsigned int nmodels)
+virCPUModelIsAllowed(const char *model,
+ const char **models,
+ unsigned int nmodels)
 ATTRIBUTE_NONNULL(1);
 
 int
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index bfe50a3..bdb026b 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -691,7 +691,7 @@ ppc64DriverDecode(virCPUDefPtr cpu,
 goto cleanup;
 }
 
-if (!cpuModelIsAllowed(model->name, models, nmodels)) {
+if (!virCPUModelIsAllowed(model->name, models, nmodels)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("CPU model %s is not supported by hypervisor"),
model->name);
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 34c3e5b..5dfe549 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -1833,7 +1833,7 @@ x86Decode(virCPUDefPtr cpu,
  */
 for (i = map->nmodels - 1; i >= 0; i--) {
 candidate = map->models[i];
-if (!cpuModelIsAllowed(candidate->name, models, nmodels)) {
+if (!virCPUModelIsAllowed(candidate->name, models, nmodels)) {
 if (preferred && STREQ(candidate->name, preferred)) {
 if (cpu->fallback != VIR_CPU_FALLBACK_ALLOW) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-- 
2.10.2

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

Re: [libvirt] [PATCH] qemu: fix one reboot scenario

[Maxim Nestratov]

07-Nov-16 20:15, Michal Privoznik пишет:


On 07.11.2016 17:19, Michal Privoznik wrote:

On 03.11.2016 19:11, Maxim Nestratov wrote:

Both qemuDomainReboot and qemuDomainShutdownFlags do the following if
they were called to reboot:
1. use agent and call qemuAgentShutdown
2. then if the above function doesn't succeed, try qemuMonitorSystemPowerdown

When the first step is called, it resets fakeReboot flag, while the second one,
opposite to that, sets it. Thus, in case we tried to use agent to reboot a
guest and failed for some reason, we end up with fakeReboot flag set. After 
that,
as qemuMonitorSystemPowerdown function was called, libvirt is notified with 
POWERDOWN.
The problem is that there is no callback routine set for it. The lack of 
monitor event
reaction leads to incorrect logic and guest doesn't restart or reboot correctly.
The patch simply sets domainPowerdown monitor callback to 
qemuProcessHandleShutdown as
powerdown event processing is actually equal to shutdown.

Signed-off-by: Maxim Nestratov 
---
  src/qemu/qemu_process.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 04b25fe..0de9fa5 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1630,6 +1630,7 @@ static qemuMonitorCallbacks monitorCallbacks = {
  .diskSecretLookup = qemuProcessFindVolumeQcowPassphrase,
  .domainEvent = qemuProcessHandleEvent,
  .domainShutdown = qemuProcessHandleShutdown,
+.domainPowerdown = qemuProcessHandleShutdown,
  .domainStop = qemuProcessHandleStop,
  .domainResume = qemuProcessHandleResume,
  .domainReset = qemuProcessHandleReset,


Huh, this event was never ever handled. How did we even manage that?

Oh, now looking into the qemu code, maybe we don't want this to be
handled after all. POWERDOWN event is emitted on ACPI powerdown event,
which means that guest is still running and hence we don't want to
mangle our internal state (i.e. set it to SHUTDOWN).


ACK

I'm sorry, but I have to discard my own ACK. We need a different fix for
the scenario you're seeing.

Michal


Yeah, you are absolutely right. Please disregard the patch. I'll send another 
one.

Maxim

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

Re: [libvirt] [PATCH 4/4] qemu: Generate memory device aliases according to slot number

[Peter Krempa]

On Thu, Nov 10, 2016 at 08:58:10 -0500, John Ferlan wrote:
> 
> 
> On 11/03/2016 02:12 AM, Peter Krempa wrote:
> > The memory device alias needs to be treated as machine ABI as qemu is
> > using it in the migration stream for section labels. To simplify this
> > generate the alias from the slot number unless an existing broken
> > configuration is detected.
> > 
> > With this patch the aliases are predictable and even certain
> > configurations which would not be migratable previously are fixed.
> > 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1359135
> > ---
> >  src/qemu/qemu_alias.c  | 27 
> > ++
> >  src/qemu/qemu_alias.h  |  3 ++-
> >  src/qemu/qemu_hotplug.c|  4 +++-
> >  .../qemuxml2argv-memory-hotplug-dimm-addr.args |  4 ++--
> >  4 files changed, 29 insertions(+), 9 deletions(-)
> > 
> 
> Revisiting after my face-palm for patch2.
> 
> > diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
> > index 9737158..8521a44 100644
> > --- a/src/qemu/qemu_alias.c
> > +++ b/src/qemu/qemu_alias.c
> > @@ -332,17 +332,34 @@ qemuAssignDeviceRNGAlias(virDomainDefPtr def,
> >  }
> > 
> > 
> > +/**
> > + * qemuAssignDeviceMemoryAlias:
> > + * @def: domain definition. Necessary only if @oldAlias is true.
> 
> s/old/use  (multiple places)

I kept using oldAlias, as I think the variable should emphasize that the
old approach is used. I can change it if you think otherwise. I will be
following up with a patch adding docs for the address anyways.

Pushed; Thanks.

Peter


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

Re: [libvirt] [PATCH] qemu: Remove erroneously placed comments for numerical ordering

[Peter Krempa]

On Thu, Nov 10, 2016 at 10:55:17 -0500, John Ferlan wrote:
> Commit id '74bbb8c2ec' seems to have mismerged a bit - adding 240 comments
> out of place.  Just clean that up.
> 
> Signed-off-by: John Ferlan 
> ---
> 
>  Pushed as trivial

Oops. I had a merge conflict in 5 consecutive patches so I messed it
up:/

Thanks for fixing it.

Peter


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

Re: [libvirt] [PATCH 0/2] Allow hotplug of vhost-mq

[Michal Privoznik]

On 10.11.2016 14:04, Martin Kletzander wrote:
> On Fri, Nov 04, 2016 at 01:28:57PM +0100, Michal Privoznik wrote:
>> Basically this is trivial. Everything is prepared and the only
>> thing that prevented us from doing this was missing exception in
>> one check. Trivial.
>>
>> Michal Privoznik (2):
>>  qemuDomainAttachNetDevice: Don't overwrite error on rollback
>>  qemuDomainAttachNetDevice: Enable multiqueue for vhost-user
>>
>> src/qemu/qemu_hotplug.c | 7 ++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
> 
> ACK series

Thank you. Pushed now.

Michal

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

[libvirt] [PATCH] qemu: Remove erroneously placed comments for numerical ordering

[John Ferlan]

Commit id '74bbb8c2ec' seems to have mismerged a bit - adding 240 comments
out of place.  Just clean that up.

Signed-off-by: John Ferlan 
---

 Pushed as trivial

 src/qemu/qemu_capabilities.c | 3 +--
 src/qemu/qemu_capabilities.h | 2 --
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 5a3004d..ad98d68 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -351,8 +351,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
 
   "ivshmem-doorbell", /* 240 */
   "query-qmp-schema",
-
-  "gluster.debug_level", /* 240 */
+  "gluster.debug_level",
 );
 
 
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 02014c0..9163572 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -386,8 +386,6 @@ typedef enum {
 /* 240 */
 QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL, /* -device ivshmem-doorbell */
 QEMU_CAPS_QUERY_QMP_SCHEMA, /* query-qmp-schema command */
-
-/* 240 */
 QEMU_CAPS_GLUSTER_DEBUG_LEVEL, /* -drive gluster.debug_level={0..9} */
 
 QEMU_CAPS_LAST /* this must always be the last item */
-- 
2.7.4

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

Re: [libvirt] [PATCH v6 15/17] qemu: initially reserve one open pcie-root-port for hotplug

[Andrea Bolognani]

On Mon, 2016-11-07 at 14:50 -0500, Laine Stump wrote:
[...]
> Instead this patch just reserves one slot for a "future potential"
> PCIe device after doing the assignment for actual devices, but only
> if the only PCI controller defined prior to starting address
> assignment was pcie-root, and only if we auto-added at least one PCI
> controller during address assignment.  This assures two things:

Double space here. I wouldn't normally care about picking
one style over the other, but you've used a single space
everywhere else in the commit message :)

[...]
> This is set to reserve a single free port for now, but could be
> increased in the future if public sentiment goes in that direction
> (it's easy to increase later, but essential impossible to decrease)

s/essential/essentially/

[...]
> diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
> index fbbcfb2..15d7c1a 100644
> --- a/src/qemu/qemu_domain_address.c
> +++ b/src/qemu/qemu_domain_address.c
> @@ -1929,6 +1929,36 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
>  if (qemuDomainAssignDevicePCISlots(def, qemuCaps, addrs) < 0)
>  goto cleanup;
>  
> +/* Only for *new* domains with pcie-root (and no other
> + * manually specified PCI controllers in the definition): If,
> + * after assigning addresses/reserving slots for all devices,
> + * we see that any extra buses have been auto-added, we
> + * understand that the application has left management of PCI
> + * addresses and controllers up to libvirt. In order to allow
> + * such applications to easily support hotplug, we will do a
> + * "one time" reservation of one extra PCIE|HOTPLUGGABLE
> + * slots, which should cause su to auto-add 1 extra

s/cause su/cause us/

> + * pcie-root-ports The single slot in this root-port will be

s/pcie-root-ports/pcie-root-port./


ACK with the typos fixed.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH] fix parsing security labels from virt-aa-helper

[Christian Ehrhardt]

Sorry, I seem to become a pest more than I'd like to, but my timer on this
thread expired again :-)

Was the feedback I gave to the questions last week ok to understand the
case and maybe reproduce to achieve a ack or do we need to discuss more?
​
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/6] Add deadline scheduler

[Martin Kletzander]

On Mon, Nov 07, 2016 at 10:01:11AM +0100, Martin Polednik wrote:

The policy SCHED_DEADLINE is available since kernel 3.14 (and most likely
backported to older RT_PREEMPT kernels). It is safer to use than fifo or round
robin policies due to only limiting part of cpu time for the RT process,
leading to lack of lockups of the host.

The series adds new vcpusched/iothreadsched called 'deadline' that activates
SCHED_DEADLINE for given process. As the scheduler is linux specific, extra
implementation was required - it is not possible to use sched_setscheduler,
sched_setattr syscall must be used.

Martin Polednik (6):
 conf: add entries for deadline scheduler
 util: allow virProcessSetScheduler to set SCHED_DEADLINE
 virDomainFormatSchedDef: factor out subset code





 conf: add deadline scheduler
 schema: add deadline scheduler
 docs: mention deadline scheduler in vcpusched



These three should be merged together.  Also you should error out with
unsupported values (e.g. deadline < runtime) in virDomainDefPostParse()
so that users know it right away.  That way we can change it later, but
not when everything is acceptable now.

Otherwise looks fine from a quick look.

Have a nice day,
Martin


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

Re: [libvirt] [PATCH 2/6] util: allow virProcessSetScheduler to set SCHED_DEADLINE

[Martin Kletzander]

On Mon, Nov 07, 2016 at 10:01:13AM +0100, Martin Polednik wrote:

As sched_deadline is linux specific, it is not set through
sched_setscheduler but rather the sched_setattr syscall. Additionally,
the scheduler has new set of parameters: runtime, deadline and period.

In this part of the series, we extend virProcessSetScheduler to
accommodate the additional parameters and use sched_setattr syscall to
set SCHED_DEADLINE. Another small addition is sched_attr struct, which
is required for sched_setattr and not exposed from the kernel or
libraries.
---
src/qemu/qemu_process.c |  3 ++-
src/util/virprocess.c   | 70 +++--
src/util/virprocess.h   | 28 +++-
3 files changed, 97 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 1b67aee..91a635c 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2395,7 +2395,8 @@ qemuProcessSetupPid(virDomainObjPtr vm,

/* Set scheduler type and priority. */
if (sched &&
-virProcessSetScheduler(pid, sched->policy, sched->priority) < 0)
+virProcessSetScheduler(pid, sched->policy, sched->priority,
+   sched->runtime, sched->deadline, sched->period) 
< 0)
goto cleanup;

ret = 0;
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index d68800b..cd1cc9b 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -70,6 +70,7 @@
VIR_LOG_INIT("util.process");

#ifdef __linux__
+# include 
/*
 * Workaround older glibc. While kernel may support the setns
 * syscall, the glibc wrapper might not exist. If that's the
@@ -91,9 +92,14 @@ VIR_LOG_INIT("util.process");
#  endif
# endif

+# ifndef __NR_sched_setattr
+#  if defined(__x86_64__)
+#   define __NR_sched_setattr 314
+#  endif
+# endif
+


At least ARMs, POWERs and i386 should be defined from the start so that
tests in CI don't fail right away.  And other platforms should error out
right away.  But honestly, I'd drop this compatibility code.  What
you're adding here is not a bleeding-edge feature.


# ifndef HAVE_SETNS
#  if defined(__NR_setns)
-#   include 

static inline int setns(int fd, int nstype)
{
@@ -103,6 +109,13 @@ static inline int setns(int fd, int nstype)
#   error Please determine the syscall number for setns on your architecture
#  endif
# endif
+
+static inline int sched_setattr(pid_t pid,
+const struct sched_attr *attr,
+unsigned int flags)
+{
+return syscall(__NR_sched_setattr, pid, attr, flags);
+}
#else /* !__linux__ */
static inline int setns(int fd ATTRIBUTE_UNUSED, int nstype ATTRIBUTE_UNUSED)
{
@@ -110,6 +123,15 @@ static inline int setns(int fd ATTRIBUTE_UNUSED, int 
nstype ATTRIBUTE_UNUSED)
 _("Namespaces are not supported on this platform."));
return -1;
}
+
+static inline int sched_setattr(pid_t pid,
+const struct sched_attr *attr,
+unsigned int flags)
+{
+virReportSystemError(ENOSYS, "%s",
+ _("Deadline scheduler is not supported on this 
platform."));
+return -1;
+}
#endif

VIR_ENUM_IMPL(virProcessSchedPolicy, VIR_PROC_POLICY_LAST,
@@ -1225,7 +1247,10 @@ virProcessSchedTranslatePolicy(virProcessSchedPolicy 
policy)
int
virProcessSetScheduler(pid_t pid,
   virProcessSchedPolicy policy,
-   int priority)
+   int priority,
+   unsigned long long runtime,
+   unsigned long long deadline,
+   unsigned long long period)
{
struct sched_param param = {0};
int pol = virProcessSchedTranslatePolicy(policy);
@@ -1235,6 +1260,47 @@ virProcessSetScheduler(pid_t pid,
if (!policy)
return 0;

+if (pol == SCHED_DEADLINE) {
+int ret = 0;
+
+if (runtime < 1024 || deadline < 1024 || period < 24) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_("Scheduler runtime, deadline and period must be "
+  "higher or equal to 1024 (1 us) (runtime(%llu), "
+  "deadline(%llu), period(%llu))"), runtime, deadline, 
period);
+return -1;
+}
+
+if (!(runtime <= deadline && deadline <= period)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Scheduler configuration does not satisfy "
+ "(runtime(%llu) <= deadline(%llu) <= 
period(%llu))"),
+   runtime, deadline, period);
+return -1;
+}
+
+struct sched_attr attrs = {
+   .size = sizeof(attrs),
+   .sched_policy = SCHED_DEADLINE,
+   .sched_flags = 0,
+   .sched_nice = 0,
+   .sched_priority = 0,
+   .sched_runtime = runtime,
+   .sched_deadline = deadline,
+   

Re: [libvirt] [PATCH 3/6] virDomainFormatSchedDef: factor out subset code

[Martin Kletzander]

On Mon, Nov 07, 2016 at 10:01:14AM +0100, Martin Polednik wrote:

The code within the function is too specific for priority attribute of
RT schedulers. To allow addition of schedulers that group by different
properties, we factor out the logic to calculate cpu subset. Instead
of comparing by priority, the new code accepts comparator for the 2
sched structs.
---
src/conf/domain_conf.c | 77 +++---
1 file changed, 55 insertions(+), 22 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ca0fdfd..0ed755c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23058,6 +23058,47 @@ virDomainDefHasCapabilitiesFeatures(virDomainDefPtr 
def)
return false;
}

+static bool
+virDomainSchedPriorityComparator(virDomainThreadSchedParamPtr baseSched,
+ virDomainThreadSchedParamPtr sched)
+{
+bool ret = false;
+ret = (baseSched->priority == sched->priority);
+
+return ret;


Just return (baseSched->priority == sched->priority);  all else is not
needed here.


+}
+
+static virDomainThreadSchedParamPtr
+virDomainSchedSubsetCharacteristic(virDomainDefPtr def,
+   virBitmapPtr schedMap,
+   virBitmapPtr subsetMap,
+   virDomainThreadSchedParamPtr 
(*func)(virDomainDefPtr, unsigned int),
+   bool 
(*comparator)(virDomainThreadSchedParamPtr,
+  
virDomainThreadSchedParamPtr))
+{
+ssize_t nextidx;
+virDomainThreadSchedParamPtr sched;
+virDomainThreadSchedParamPtr baseSched = NULL;
+
+virBitmapClearAll(subsetMap);
+
+/* we need to find a subset of vCPUs with the given scheduler
+* that share the priority */
+nextidx = virBitmapNextSetBit(schedMap, -1);
+if (!(sched = func(def, nextidx)))
+return NULL;
+
+baseSched = sched;
+ignore_value(virBitmapSetBit(subsetMap, nextidx));
+
+while ((nextidx = virBitmapNextSetBit(schedMap, nextidx)) > -1) {
+sched = func(def, nextidx);
+if (sched && comparator(baseSched, sched))
+ignore_value(virBitmapSetBit(subsetMap, nextidx));
+}
+
+return baseSched;
+}

/**
 * virDomainFormatSchedDef:
@@ -23082,8 +23123,9 @@ virDomainFormatSchedDef(virDomainDefPtr def,
virBitmapPtr resourceMap)
{
virBitmapPtr schedMap = NULL;
-virBitmapPtr prioMap = NULL;
+virBitmapPtr subsetMap = NULL;
virDomainThreadSchedParamPtr sched;
+virDomainThreadSchedParamPtr baseSched;
char *tmp = NULL;
ssize_t next;
size_t i;
@@ -23098,7 +23140,7 @@ virDomainFormatSchedDef(virDomainDefPtr def,
 */

if (!(schedMap = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN)) ||
-!(prioMap = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN)))
+!(subsetMap = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN)))
goto cleanup;

for (i = VIR_PROC_POLICY_NONE + 1; i < VIR_PROC_POLICY_LAST; i++) {
@@ -23117,9 +23159,8 @@ virDomainFormatSchedDef(virDomainDefPtr def,
 * have them */
while (!virBitmapIsAllClear(schedMap)) {
virBitmapPtr currentMap = NULL;
-ssize_t nextprio;
bool hasPriority = false;
-int priority = 0;
+baseSched = NULL;

switch ((virProcessSchedPolicy) i) {
case VIR_PROC_POLICY_NONE:
@@ -23132,25 +23173,17 @@ virDomainFormatSchedDef(virDomainDefPtr def,

case VIR_PROC_POLICY_FIFO:
case VIR_PROC_POLICY_RR:
-virBitmapClearAll(prioMap);
hasPriority = true;

-/* we need to find a subset of vCPUs with the given scheduler
- * that share the priority */
-nextprio = virBitmapNextSetBit(schedMap, -1);
-if (!(sched = func(def, nextprio)))
+baseSched = virDomainSchedSubsetCharacteristic(def,
+   schedMap,
+   subsetMap,
+   func,
+   
virDomainSchedPriorityComparator);
+if (baseSched == NULL)
goto cleanup;

-priority = sched->priority;
-ignore_value(virBitmapSetBit(prioMap, nextprio));
-
-while ((nextprio = virBitmapNextSetBit(schedMap, nextprio)) > 
-1) {
-sched = func(def, nextprio);
-if (sched && sched->priority == priority)
-ignore_value(virBitmapSetBit(prioMap, nextprio));
-}
-
-currentMap = prioMap;
+currentMap = subsetMap;
break;


This looks like it was factored out but with the combination of renaming
the 

Re: [libvirt] [PATCH 2/4] qemu: process: detect if dimm aliases are broken on reconnect

[Peter Krempa]

On Thu, Nov 10, 2016 at 08:56:48 -0500, John Ferlan wrote:
> On 11/10/2016 04:11 AM, Peter Krempa wrote:
> > On Wed, Nov 09, 2016 at 18:40:28 -0500, John Ferlan wrote:
> >> On 11/03/2016 02:12 AM, Peter Krempa wrote:
> >>> Detect on reconnect to a running qemu VM whether the alias of a
> >>> hotpluggable memory device (dimm) does not match the dimm slot number
> >>> where it's connected to. This is necessary as qemu is actually
> >>> considering the alias as machine ABI used to connect the backend object
> >>> to the dimm device.
> >>>
> >>> This will require us to keep them consistent so that we can reliably
> >>> restore them on migration. In some situations it was currently possible
> >>> to create a mismatched configuration and qemu would refuse to restore
> >>> the migration stream.
> >>>
> >>> To avoid breaking existing VMs we'll need to keep the old algorithm
> >>> though.
> >>> ---
> >>>  src/qemu/qemu_domain.h  |  3 +++
> >>>  src/qemu/qemu_process.c | 25 +
> >>>  2 files changed, 28 insertions(+)

[...]

> >>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> >>> index 1b67aee..15b8ec8 100644
> >>> --- a/src/qemu/qemu_process.c
> >>> +++ b/src/qemu/qemu_process.c
> >>> @@ -3205,6 +3205,29 @@ qemuDomainPerfRestart(virDomainObjPtr vm)
> >>>  return 0;
> >>>  }
> >>>
> >>> +
> >>> +static void
> >>> +qemuProcessReconnectCheckMemoryHotplugMismatch(virDomainObjPtr vm)
> >>> +{
> >>> +size_t i;
> >>> +int aliasidx;
> >>> +virDomainDefPtr def = vm->def;
> >>> +qemuDomainObjPrivatePtr priv = vm->privateData;
> >>> +
> >>> +if (!virDomainDefHasMemoryHotplug(def) || def->nmems == 0)
> >>> +return;
> >>> +
> >>> +for (i = 0; i < def->nmems; i++) {
> >>> +aliasidx = qemuDomainDeviceAliasIndex(>mems[i]->info, 
> >>> "dimm");
> >>
> >> When/how does the info.alias get filled in during restart processing?
> > 
> > The live definition along with aliases is saved to the status XML and
> > then reloaded from the disk.
> > 
> > Otherwise if we'd not remember the aliases the whole hotunplug code
> > would not work.
> > 
> 
> face-palm - I knew I was missing something obvious, but the brain just
> wasn't able to recognize it, .
> 
> Still consider the changed XML example from patch1 (without any
> hotplug), we have:
> 
>=> alias == "dimm0"
>=> alias == "dimm1"
> 
> so the bool could be "memAliasOrderMismatch" or "memAliasUnordered".

Okay, I'll go with the former, since that describes exactly the details.

> 
> Since it's not necessarily "Hotplug" related, I think changing the
> variable and function name should be done, but it's not a requirement
> for the ACK since it's understandable why Hotplug is used.
> 
> John
> 
> FWIW:
> One other oddball path that "might" disrupt things is the
> ignore_value(qemuDomainUpdateMemoryDeviceInfo(...) hotplug code where
> we're not "guaranteed" that the dimm.slot is filled in...

It should not ever happen, but I thought about this option originally.
The code tolerates this for local usage since it's not really fatal, but
rejects migration if the slot or base address is missing.


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

Re: [libvirt] [PATCH v6 10/17] qemu: auto-add pcie-root-port/dmi-to-pci-bridge controllers as needed

[Andrea Bolognani]

On Wed, 2016-11-09 at 15:23 -0500, Laine Stump wrote:
> > > +} else if (flags & VIR_PCI_CONNECT_TYPE_PCI_BRIDGE &&
> > > +   addrs->buses[0].model == 
> > > VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) {
> > > +model = VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE;
> > 
> > Mh, what about the case where we want to add a pci-bridge
> > but the machine has a pci-root instead of a pcie-root?
> > Shouldn't that case be handled as well?
> 
> It is handled - we'll want to add a pci-bridge if flags & 
> VIR_PCI_TYPE_PCI_DEVICE is true (i.e. the first clause). In that case, 
> since pci-root has the flag VIR_PCI_CONNECT_TYPE_PCI_BRIDGE set, the for 
> loop will set neetDMIToPCIBridge = false, so we will end up adding just 
> the one pci-bridge that we need.

Sorry, I was not clear enough.

What if the function is called like

  virDomainPCIAddressSetGrow(addrs, addr,
 VIR_PCI_CONNECT_TYPE_PCI_BRIDGE)

because *the caller* is going through a virDomainDef and,
after finding a pci-bridge in it, wants to make sure the
address set can actually fit it? The case when

  flags & VIR_PCI_CONNECT_TYPE_PCI_BRIDGE

can clearly happen, as we're handling it above, but we error
out unless the guest has a pcie-root? That's the bit that I
can't quite wrap my head around.

> > > +} else if (flags & (VIR_PCI_CONNECT_TYPE_PCIE_DEVICE |
> > > +VIR_PCI_CONNECT_TYPE_PCIE_SWITCH_UPSTREAM_PORT)) 
> > > {
> > > +model = VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT;
> > > +} else {
> > > +int existingContModel = 
> > > virDomainPCIControllerConnectTypeToModel(flags);
> > > +
> > > +if (existingContModel >= 0) {
> > > +virReportError(VIR_ERR_INTERNAL_ERROR,
> > > +   _("a PCI slot is needed to connect a PCI 
> > > controller "
> > > + "model='%s', but none is available, and it "
> > > + "cannot be automatically added"),
> > > +   
> > > virDomainControllerModelPCITypeToString(existingContModel));
> > > +} else {
> > > +virReportError(VIR_ERR_INTERNAL_ERROR,
> > > +   _("Cannot automatically add a new PCI bus for 
> > > a "
> > > + "device with connect flags %.2x"), flags);
> > > +}
> > 
> > So we error out for dmi-to-pci-bridge and
> > pci{,e}-expander-bus... Shouldn't we be able to plug either
> > into into pcie-root or pci-root?
> 
> Exactly. And since those buses already exist from the start, and can't 
> be added later, there wouldn't ever be a situation where we needed to 
> automatically grow the bus structure due to one of those devices and 
> growing could actually do anything (i.e. you can't add a pcie-root or 
> pci-root).

I'm clearly missing something here :(

Don't we (potentially) grow the address set using this
function every time we reserve an address for an endpoint
device or a controller? Including when we're going through
a virDomainDef, which might contain a dmi-to-pci-bridge
or a pci{,e}-expander-bus?

I'm not expecting the pci{,e}-root case to be handled, of
course, but I can't figure out why we seem to be skipping
over a bunch of perfectly valid cases.

> > > -virDomainPCIAddressBusSetModel(>buses[i],
> > > -   
> > > VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE);
> > > +virDomainPCIAddressBusSetModel(>buses[i++],
> > > +   
> > > VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE);
> > 
> > virDomainPCIAddressBusSetModel() can fail, yet you're not
> > checking the return value neither here...
> 
> Yes, but the only way it can fail is if an unknown controller model is 
> sent to it, and we can see by simple physical inspection that that isn't 
> the case. If this was calling off to a function in some other file where 
> we didn't know just how simple it is and couldn't easily see that it 
> would never fail, then I would agree that we should check, but in this 
> case it's superfluous - if this function had an ATTRIBUTE_RETURN_CHECK 
> on it, I would have put these calls inside ignore_value().

[...]
> > >}
> > > +
> > > +for (; i < addrs->nbuses; i++)
> > > +virDomainPCIAddressBusSetModel(>buses[i], model);
> > ... nor here.
> 
> Same for this - the controller model is one of just a couple hardcoded 
> values set within the same function, not sent in from somewhere else, 
> and the function we're calling is a very simple function in the same file.

[...]
> > > @@ -947,7 +934,43 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def,
> > >
> > >if (virDomainPCIAddressBusSetModel(>buses[idx], 
> > >cont->model) < 0)
> > >goto error;
> > > -}
> 
> Since I'm sure you'll bring up the fact that I *am* checking the return 
> value of virDomainPCIAddressBusSetModel here (and everywhere else in 
> 

Re: [libvirt] [PATCH 4/4] qemu: Generate memory device aliases according to slot number

[John Ferlan]

On 11/03/2016 02:12 AM, Peter Krempa wrote:
> The memory device alias needs to be treated as machine ABI as qemu is
> using it in the migration stream for section labels. To simplify this
> generate the alias from the slot number unless an existing broken
> configuration is detected.
> 
> With this patch the aliases are predictable and even certain
> configurations which would not be migratable previously are fixed.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1359135
> ---
>  src/qemu/qemu_alias.c  | 27 
> ++
>  src/qemu/qemu_alias.h  |  3 ++-
>  src/qemu/qemu_hotplug.c|  4 +++-
>  .../qemuxml2argv-memory-hotplug-dimm-addr.args |  4 ++--
>  4 files changed, 29 insertions(+), 9 deletions(-)
> 

Revisiting after my face-palm for patch2.

> diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
> index 9737158..8521a44 100644
> --- a/src/qemu/qemu_alias.c
> +++ b/src/qemu/qemu_alias.c
> @@ -332,17 +332,34 @@ qemuAssignDeviceRNGAlias(virDomainDefPtr def,
>  }
> 
> 
> +/**
> + * qemuAssignDeviceMemoryAlias:
> + * @def: domain definition. Necessary only if @oldAlias is true.

s/old/use  (multiple places)

> + * @mem: memory device definition
> + * @oldAlias: Generate the alias according to the order of the device in @def
> + *rather than according to the slot number for legacy reasons.
> + *
> + * Generates alias for a memory device according to slot number if @oldAlias 
> is
> + * false or according to order in @def->mems otherwise.
> + *
> + * Returns 0 on success, -1 on error.
> + */
>  int
>  qemuAssignDeviceMemoryAlias(virDomainDefPtr def,
> -virDomainMemoryDefPtr mem)
> +virDomainMemoryDefPtr mem,
> +bool oldAlias)
>  {
>  size_t i;
>  int maxidx = 0;
>  int idx;
> 
> -for (i = 0; i < def->nmems; i++) {
> -if ((idx = qemuDomainDeviceAliasIndex(>mems[i]->info, "dimm")) 
> >= maxidx)
> -maxidx = idx + 1;
> +if (oldAlias) {
> +for (i = 0; i < def->nmems; i++) {
> +if ((idx = qemuDomainDeviceAliasIndex(>mems[i]->info, 
> "dimm")) >= maxidx)
> +maxidx = idx + 1;
> +}
> +} else {
> +maxidx = mem->info.addr.dimm.slot;
>  }
> 
>  if (virAsprintf(>info.alias, "dimm%d", maxidx) < 0)
> @@ -475,7 +492,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, 
> virQEMUCapsPtr qemuCaps)
>  return -1;
>  }
>  for (i = 0; i < def->nmems; i++) {
> -if (virAsprintf(>mems[i]->info.alias, "dimm%zu", i) < 0)
> +if (qemuAssignDeviceMemoryAlias(NULL, def->mems[i], false) < 0)

Since this is primarily for the startup path, then false is OK...
Although I can only wonder what would happen in the QemuAttach path...

>  return -1;
>  }
> 
> diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h
> index d298a4d..1d93912 100644
> --- a/src/qemu/qemu_alias.h
> +++ b/src/qemu/qemu_alias.h
> @@ -58,7 +58,8 @@ int qemuAssignDeviceRNGAlias(virDomainDefPtr def,
>   virDomainRNGDefPtr rng);
> 
>  int qemuAssignDeviceMemoryAlias(virDomainDefPtr def,
> -virDomainMemoryDefPtr mems);
> +virDomainMemoryDefPtr mems,
> +bool ble);

s/ble/useAlias

> 
>  int qemuAssignDeviceShmemAlias(virDomainDefPtr def,
> virDomainShmemDefPtr shmem,
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 75477cd..77176fb 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -2130,7 +2130,9 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
>  if (qemuDomainAssignMemoryDeviceSlot(vm->def, mem) < 0)
>  goto cleanup;
> 
> -if (qemuAssignDeviceMemoryAlias(vm->def, mem) < 0)
> +/* in cases where we are using a VM with aliases generated according to 
> the
> + * index of the memory device we need to keep continue using that scheme 
> */
> +if (qemuAssignDeviceMemoryAlias(vm->def, mem, 
> priv->memHotplugAliasMismatch) < 0)

This ends up being a long line > 80 cols...

ACK w/ variable name cleanup

John

>  goto cleanup;
> 
>  if (virAsprintf(, "mem%s", mem->info.alias) < 0)
> diff --git 
> a/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm-addr.args 
> b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm-addr.args
> index 23403df..fdbb4c3 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm-addr.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm-addr.args
> @@ -15,8 +15,8 @@ QEMU_AUDIO_DRV=none \
>  mem-path=/dev/hugepages2M/libvirt/qemu,size=536870912,host-nodes=1-3,\
>  policy=bind \
>  -device pc-dimm,node=0,memdev=memdimm0,id=dimm0,slot=0,addr=4294967296 \
> --object memory-backend-ram,id=memdimm1,size=536870912 \
> 

Re: [libvirt] [PATCH 2/4] qemu: process: detect if dimm aliases are broken on reconnect

[John Ferlan]

On 11/10/2016 04:11 AM, Peter Krempa wrote:
> On Wed, Nov 09, 2016 at 18:40:28 -0500, John Ferlan wrote:
>>
>>
>> On 11/03/2016 02:12 AM, Peter Krempa wrote:
>>> Detect on reconnect to a running qemu VM whether the alias of a
>>> hotpluggable memory device (dimm) does not match the dimm slot number
>>> where it's connected to. This is necessary as qemu is actually
>>> considering the alias as machine ABI used to connect the backend object
>>> to the dimm device.
>>>
>>> This will require us to keep them consistent so that we can reliably
>>> restore them on migration. In some situations it was currently possible
>>> to create a mismatched configuration and qemu would refuse to restore
>>> the migration stream.
>>>
>>> To avoid breaking existing VMs we'll need to keep the old algorithm
>>> though.
>>> ---
>>>  src/qemu/qemu_domain.h  |  3 +++
>>>  src/qemu/qemu_process.c | 25 +
>>>  2 files changed, 28 insertions(+)
>>>
>>> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
>>> index 2ee1829..1b7b375 100644
>>> --- a/src/qemu/qemu_domain.h
>>> +++ b/src/qemu/qemu_domain.h
>>> @@ -232,6 +232,9 @@ struct _qemuDomainObjPrivate {
>>>  /* private XML) - need to restore at process reconnect */
>>>  uint8_t *masterKey;
>>>  size_t masterKeyLen;
>>> +
>>> +/* note whether memory device alias does not correspond to slot number 
>>> */
>>> +bool memHotplugAliasMismatch;
>>>  };
>>>
>>>  # define QEMU_DOMAIN_PRIVATE(vm)   \
>>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>>> index 1b67aee..15b8ec8 100644
>>> --- a/src/qemu/qemu_process.c
>>> +++ b/src/qemu/qemu_process.c
>>> @@ -3205,6 +3205,29 @@ qemuDomainPerfRestart(virDomainObjPtr vm)
>>>  return 0;
>>>  }
>>>
>>> +
>>> +static void
>>> +qemuProcessReconnectCheckMemoryHotplugMismatch(virDomainObjPtr vm)
>>> +{
>>> +size_t i;
>>> +int aliasidx;
>>> +virDomainDefPtr def = vm->def;
>>> +qemuDomainObjPrivatePtr priv = vm->privateData;
>>> +
>>> +if (!virDomainDefHasMemoryHotplug(def) || def->nmems == 0)
>>> +return;
>>> +
>>> +for (i = 0; i < def->nmems; i++) {
>>> +aliasidx = qemuDomainDeviceAliasIndex(>mems[i]->info, "dimm");
>>
>> When/how does the info.alias get filled in during restart processing?
> 
> The live definition along with aliases is saved to the status XML and
> then reloaded from the disk.
> 
> Otherwise if we'd not remember the aliases the whole hotunplug code
> would not work.
> 

face-palm - I knew I was missing something obvious, but the brain just
wasn't able to recognize it, .

Still consider the changed XML example from patch1 (without any
hotplug), we have:

   => alias == "dimm0"
   => alias == "dimm1"

so the bool could be "memAliasOrderMismatch" or "memAliasUnordered".

Since it's not necessarily "Hotplug" related, I think changing the
variable and function name should be done, but it's not a requirement
for the ACK since it's understandable why Hotplug is used.

John

FWIW:
One other oddball path that "might" disrupt things is the
ignore_value(qemuDomainUpdateMemoryDeviceInfo(...) hotplug code where
we're not "guaranteed" that the dimm.slot is filled in...


>> I see it being defined during qemuAssignDeviceMemoryAlias and
>> qemuAssignDeviceAliases. The former is only called in hotplug processing
>> and the latter during qemuProcessPrepareDomain (domain startup). So I
>> think the answer is we return -1 always here, but could be proved wrong.
> 
> The aliases are reloaded so they are valid. Re-assigning them would
> break all hotunplug if you generate a different alias. It works this way
> in other parts for quite a long time now.
> 
>> Thus, I think this is doomed from the start. I also wonder how existing
>> code is affected since it's based on getting alias index - which
> 
> As most other code that needs aliases after restart ...
> 
>> wouldn't be defined, thus wouldn't a hotplug after libvirtd restart
>> result in "dimm0"?
> 
> Nope. Both paragraphs don't make sense due to the fact above.
> 
>> The good news is we do have a way to fetch/return a 'meminfo' from
>> qemuMonitorGetMemoryDeviceInfo which would be a hash table indexed by
>> the ID's provided at startup. Thus we'd just need a mechanism to match
>> 'mems' with each element of the returned meminfo hash table and
>> "generate"/assign/steal the alias from that to mems.
> 
> No need. It's saved by libvirt.
> 
>> At the very least - whatever we set will be whatever we created or was
>> hotplugged. It won't be stored in the config XML (yet), but it would
>> seemingly be bug for bug compatible.
> 
> No it's explicitly stored into the live XML. The address and slot number
> are necessary to ensure migration compatibility.
> 
>> This way - we shouldn't need all of patch4, I think... Or at least we
> 
> Patch 4 is necessary as QEMU actually makes the alias part of the qemu
> migration stream. This effectively makes the alias part of the machine
> ABI. If the aliases 

Re: [libvirt] [PATCH 0/2] Allow hotplug of vhost-mq

[Martin Kletzander]

On Fri, Nov 04, 2016 at 01:28:57PM +0100, Michal Privoznik wrote:

Basically this is trivial. Everything is prepared and the only
thing that prevented us from doing this was missing exception in
one check. Trivial.

Michal Privoznik (2):
 qemuDomainAttachNetDevice: Don't overwrite error on rollback
 qemuDomainAttachNetDevice: Enable multiqueue for vhost-user

src/qemu/qemu_hotplug.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)



ACK series


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

Re: [libvirt] [PATCH 2/2] qemu_cgroup: allow access to /dev/dri/render*

[Daniel P. Berrange]

On Thu, Nov 10, 2016 at 10:35:46AM +, Marc-André Lureau wrote:
> Hi
> 
> What's the status with this patch? If I understand the discussion, it is
> needed, but not enough. Now that SELinux has been fixed (both in f24/f25
> now), I can see only the ACL left: setfacl -m u:qemu:rw /dev/dri/renderD128
> + this patch allows me to setup a system VM with virgl. (though tbh, I
> would be fine restricting virgl to qemu:///session only)

This ties in with the discussion we've just been having around udev
and DAC/MAC labelling of device nodes. With my proposed solution of
using a new mount namespace + dedicated /dev per VM, then granting
DAC access to the DRI nodes is easy.


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

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

Re: [libvirt] [PATCH 2/2] qemu_cgroup: allow access to /dev/dri/render*

[Marc-André Lureau]

Hi

What's the status with this patch? If I understand the discussion, it is
needed, but not enough. Now that SELinux has been fixed (both in f24/f25
now), I can see only the ACL left: setfacl -m u:qemu:rw /dev/dri/renderD128
+ this patch allows me to setup a system VM with virgl. (though tbh, I
would be fine restricting virgl to qemu:///session only)

thanks

On Sat, May 21, 2016 at 1:10 AM Cole Robinson  wrote:

> On 05/20/2016 03:54 AM, Ján Tomko wrote:
> > On Thu, May 19, 2016 at 01:52:00PM +0100, Daniel P. Berrange wrote:
> >> On Thu, May 19, 2016 at 08:36:35AM -0400, Cole Robinson wrote:
> >>> On 05/19/2016 08:21 AM, Daniel P. Berrange wrote:
>  On Thu, May 19, 2016 at 01:29:07PM +0200, Ján Tomko wrote:
> > Allow access to /dev/dri/render* devices for domains
> > using  with 
> >
> > https://bugzilla.redhat.com/show_bug.cgi?id=1337290
> 
>  Ignoring cgroups for a minute, how exactly does QEMU get access to
>  the /dev/dri/render* devices in general ?  ie when QEMU is running
>  as the 'qemu:qemu' user/group account, with selinux enforcing I
>  don't see how it can possibly open these files, as we're not granting
>  access to them in any of the security drivers. Given this, allowing
>  them in cgroups seems like the least of our problems.
> 
> >
> > I saw this more as "not denying access" instead of allowing access.
> > For dac/SELinux, if the user adds qemu to the video group/adds ACLs
> > or creates a SELinux rule for it (or the more realistic solution
> > mentioned by Cole), libvirt will not interfere. But it would deny "*:*"
> > devices, giving a "Permission denied" (which is also harder to debug
> > than the other two security measures)
> >
>
> I agree with this, and the patch in general. Dave and Gerd confirmed that
> this
> is expected to be a shared resource, so I think extending the cgroups
> whitelist is going to happen eventually anyways.
>
> The svirt/dac issues are real and we need to figure that out, but it's
> kind of
> irrelevant at this stage; the word is out on this stuff and people are
> going
> to find a way to enable it one way or the other. I've already had
> discussions
> with 5 people this week about when support will be available in
> virt-manager.
> Heck I know people were already using the qemu command line passthrough
> magic
> to test GL with the qemu gtk frontend with older qemu. This is just one of
> those features that people are going to want to play with, integration
> issues
> be damned, so IMO better that we get out in front of it.
>
> What I'm afraid of specifically with this cgroups issue is people will work
> around it with a custom cgroup_device_acl, then some time later when we
> bump
> the default list to make some new qemu feature work, suddenly that old
> cgroup
> list doesn't cut it and the user get's weird errors with new qemu, which we
> have to debug. svirt and dac workarounds are less scary in that regard
>
> - Cole
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
-- 
Marc-André Lureau
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/4] qemu: process: detect if dimm aliases are broken on reconnect

[Peter Krempa]

On Wed, Nov 09, 2016 at 18:40:28 -0500, John Ferlan wrote:
> 
> 
> On 11/03/2016 02:12 AM, Peter Krempa wrote:
> > Detect on reconnect to a running qemu VM whether the alias of a
> > hotpluggable memory device (dimm) does not match the dimm slot number
> > where it's connected to. This is necessary as qemu is actually
> > considering the alias as machine ABI used to connect the backend object
> > to the dimm device.
> > 
> > This will require us to keep them consistent so that we can reliably
> > restore them on migration. In some situations it was currently possible
> > to create a mismatched configuration and qemu would refuse to restore
> > the migration stream.
> > 
> > To avoid breaking existing VMs we'll need to keep the old algorithm
> > though.
> > ---
> >  src/qemu/qemu_domain.h  |  3 +++
> >  src/qemu/qemu_process.c | 25 +
> >  2 files changed, 28 insertions(+)
> > 
> > diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> > index 2ee1829..1b7b375 100644
> > --- a/src/qemu/qemu_domain.h
> > +++ b/src/qemu/qemu_domain.h
> > @@ -232,6 +232,9 @@ struct _qemuDomainObjPrivate {
> >  /* private XML) - need to restore at process reconnect */
> >  uint8_t *masterKey;
> >  size_t masterKeyLen;
> > +
> > +/* note whether memory device alias does not correspond to slot number 
> > */
> > +bool memHotplugAliasMismatch;
> >  };
> > 
> >  # define QEMU_DOMAIN_PRIVATE(vm)   \
> > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> > index 1b67aee..15b8ec8 100644
> > --- a/src/qemu/qemu_process.c
> > +++ b/src/qemu/qemu_process.c
> > @@ -3205,6 +3205,29 @@ qemuDomainPerfRestart(virDomainObjPtr vm)
> >  return 0;
> >  }
> > 
> > +
> > +static void
> > +qemuProcessReconnectCheckMemoryHotplugMismatch(virDomainObjPtr vm)
> > +{
> > +size_t i;
> > +int aliasidx;
> > +virDomainDefPtr def = vm->def;
> > +qemuDomainObjPrivatePtr priv = vm->privateData;
> > +
> > +if (!virDomainDefHasMemoryHotplug(def) || def->nmems == 0)
> > +return;
> > +
> > +for (i = 0; i < def->nmems; i++) {
> > +aliasidx = qemuDomainDeviceAliasIndex(>mems[i]->info, "dimm");
> 
> When/how does the info.alias get filled in during restart processing?

The live definition along with aliases is saved to the status XML and
then reloaded from the disk.

Otherwise if we'd not remember the aliases the whole hotunplug code
would not work.

> I see it being defined during qemuAssignDeviceMemoryAlias and
> qemuAssignDeviceAliases. The former is only called in hotplug processing
> and the latter during qemuProcessPrepareDomain (domain startup). So I
> think the answer is we return -1 always here, but could be proved wrong.

The aliases are reloaded so they are valid. Re-assigning them would
break all hotunplug if you generate a different alias. It works this way
in other parts for quite a long time now.

> Thus, I think this is doomed from the start. I also wonder how existing
> code is affected since it's based on getting alias index - which

As most other code that needs aliases after restart ...

> wouldn't be defined, thus wouldn't a hotplug after libvirtd restart
> result in "dimm0"?

Nope. Both paragraphs don't make sense due to the fact above.

> The good news is we do have a way to fetch/return a 'meminfo' from
> qemuMonitorGetMemoryDeviceInfo which would be a hash table indexed by
> the ID's provided at startup. Thus we'd just need a mechanism to match
> 'mems' with each element of the returned meminfo hash table and
> "generate"/assign/steal the alias from that to mems.

No need. It's saved by libvirt.

> At the very least - whatever we set will be whatever we created or was
> hotplugged. It won't be stored in the config XML (yet), but it would
> seemingly be bug for bug compatible.

No it's explicitly stored into the live XML. The address and slot number
are necessary to ensure migration compatibility.

> This way - we shouldn't need all of patch4, I think... Or at least we

Patch 4 is necessary as QEMU actually makes the alias part of the qemu
migration stream. This effectively makes the alias part of the machine
ABI. If the aliases don't match on migration qemu rejects it.

> wouldn't need the memHotplugAliasMismatch logic. Forcing the alias ID to
> match the slot would probably still be good goal - still doesn't mean

That is a necessary goal. The whole purpose of this series!

> the mems list is ordered 0..def->mem.memory_slots. You could have
> 0,4,2,1,3...

Exactly. Due do the fact above the alias needs to be tied to the slot
number rather than the order.

This patch is to make sure that if we employed the wrong alias alocation
scheme the code will not refuse to hotplug memory into a existing VM
that has mismatched slots and aliases.


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

Re: [libvirt] [PATCH] qemu: Make sure shmem memory is shared

[lhuang]

I have test this patch, and it works well.

After this patch, Libvirt can generate share=yes in ivshmem-plain memory 
backend command line:


# ps aux|grep r7
...
-object 
memory-backend-file,id=shmmem-shmem0,mem-path=/dev/shm/my_shmem1,size=4194304,share=yes 
-device ivshmem-plain,id=shmem0,memdev=shmmem-shmem0,bus=pci.0,addr=0x7

...

BR,
Luyao

On 11/10/2016 03:32 PM, Martin Kletzander wrote:

Even though using /dev/shm/asdf as the backend, we still need to make
the mapping shared.  The original patch forgot to add that parameter.

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

Signed-off-by: Martin Kletzander 
---
  src/qemu/qemu_command.c   | 1 +
  tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args | 6 +++---
  2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index caa80e74c26a..d3f99d34c67f 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8565,6 +8565,7 @@ qemuBuildShmemBackendMemProps(virDomainShmemDefPtr shmem)
  virJSONValueObjectCreate(,
   "s:mem-path", mem_path,
   "U:size", shmem->size,
+ "b:share", true,
   NULL);

  VIR_FREE(mem_path);
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args 
b/tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args
index 7abc7f8c4be5..688b7c7f63e2 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args
@@ -18,13 +18,13 @@ QEMU_AUDIO_DRV=none \
  -boot c \
  -usb \
  -object memory-backend-file,id=shmmem-shmem0,mem-path=/dev/shm/shmem0,\
-size=4194304 \
+size=4194304,share=yes \
  -device ivshmem-plain,id=shmem0,memdev=shmmem-shmem0,bus=pci.0,addr=0x3 \
  -object memory-backend-file,id=shmmem-shmem1,mem-path=/dev/shm/shmem1,\
-size=134217728 \
+size=134217728,share=yes \
  -device ivshmem-plain,id=shmem1,memdev=shmmem-shmem1,bus=pci.0,addr=0x5 \
  -object memory-backend-file,id=shmmem-shmem2,mem-path=/dev/shm/shmem2,\
-size=268435456 \
+size=268435456,share=yes \
  -device ivshmem-plain,id=shmem2,memdev=shmmem-shmem2,bus=pci.0,addr=0x4 \
  -device ivshmem-doorbell,id=shmem3,chardev=charshmem3,ioeventfd=on,bus=pci.0,\
  addr=0x6 \


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

Re: [libvirt] [PATCH] qemu: Make sure shmem memory is shared

[Peter Krempa]

On Thu, Nov 10, 2016 at 08:32:33 +0100, Martin Kletzander wrote:
> Even though using /dev/shm/asdf as the backend, we still need to make
> the mapping shared.  The original patch forgot to add that parameter.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1392031
> 
> Signed-off-by: Martin Kletzander 
> ---
>  src/qemu/qemu_command.c   | 1 +
>  tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args | 6 +++---
>  2 files changed, 4 insertions(+), 3 deletions(-)

ACK


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

Re: [libvirt] [PATCH 1/2 v2] conf: List only online cpus for virsh vcpupin

[Peter Krempa]

On Thu, Nov 10, 2016 at 12:30:00 +0530, Nitesh Konkar wrote:
> Currently when the vcpu placement is static
> and cpuset is not specified, CPU Affinity
> under virsh vcpupin shows 0..CPUMAX. This
> patchset will result in display of only online
> CPU's under CPU Affinity on linux.
> 
> Signed-off-by: Nitesh Konkar 
> ---
>  src/conf/domain_conf.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a233c0c..78efbc6 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -55,6 +55,7 @@
>  #include "virtpm.h"
>  #include "virstring.h"
>  #include "virnetdev.h"
> +#include "virhostcpu.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_DOMAIN
>  
> @@ -1543,6 +1544,7 @@ virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def,
>  {
>  int maxvcpus = virDomainDefGetVcpusMax(def);
>  virBitmapPtr allcpumap = NULL;
> +virBitmapPtr bitmap = NULL;
>  size_t i;
>  
>  if (hostcpus < 0)
> @@ -1555,13 +1557,16 @@ virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def,
>  
>  for (i = 0; i < maxvcpus && i < ncpumaps; i++) {
>  virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(def, i);
> -virBitmapPtr bitmap = NULL;
> +bitmap = NULL;
>  
>  if (vcpu && vcpu->cpumask)
>  bitmap = vcpu->cpumask;

By assigning vcpu->cpumask to bitmap here, you free it at the end and
break the domain definition by freeing it's content.

>  else if (def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO &&
>   autoCpuset)
>  bitmap = autoCpuset;
> +else if (def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC 
> &&
> + virHostCPUGetOnlineBitmap())

This first call returns a different instance of the bitmap which is
leaked afterwards since it's not put into a variable.

Additionally on non-linux platforms the function call would report an
error which would not be used later.

> +bitmap = virHostCPUGetOnlineBitmap();

And this may still return NULL since it's a separate call.

>  else if (def->cpumask)
>  bitmap = def->cpumask;
>  else

Peter


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