[PATCH 2/5] libxl: make use of passthrough hypervisor feature

[Jim Fehlig]

Signed-off-by: Jim Fehlig 
---
 src/libxl/libxl_conf.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 05d671bae7..458dfc2399 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -158,6 +158,27 @@ libxlMakeDomCreateInfo(libxl_ctx *ctx,
 c_info->type = LIBXL_DOMAIN_TYPE_PV;
 }
 
+#ifdef LIBXL_HAVE_CREATEINFO_PASSTHROUGH
+if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) {
+switch ((virTristateSwitch) 
def->xen_features[VIR_DOMAIN_XEN_PASSTHROUGH]) {
+case VIR_TRISTATE_SWITCH_ON:
+if (def->xen_passthrough_mode == 
VIR_DOMAIN_XEN_PASSTHROUGH_MODE_SYNC_PT)
+c_info->passthrough = LIBXL_PASSTHROUGH_SYNC_PT;
+else if (def->xen_passthrough_mode == 
VIR_DOMAIN_XEN_PASSTHROUGH_MODE_SHARE_PT)
+c_info->passthrough = LIBXL_PASSTHROUGH_SHARE_PT;
+else
+c_info->passthrough = LIBXL_PASSTHROUGH_ENABLED;
+break;
+case VIR_TRISTATE_SWITCH_OFF:
+c_info->passthrough = LIBXL_PASSTHROUGH_DISABLED;
+break;
+case VIR_TRISTATE_SWITCH_ABSENT:
+case VIR_TRISTATE_SWITCH_LAST:
+break;
+}
+}
+#endif
+
 c_info->name = g_strdup(def->name);
 
 if (def->nseclabels &&
-- 
2.26.0

[PATCH 4/5] xenconfig: Add support for 'passthrough' hypervisor feature

[Jim Fehlig]

Add support for xl.cfg(5) 'passthrough' option in the domXML-to-xenconfig
configuration converter.

Signed-off-by: Jim Fehlig 
---
 src/libvirt_private.syms |  2 ++
 src/libxl/xen_common.c   | 50 +++-
 2 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ec367653d5..54a0378f36 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -649,6 +649,8 @@ virDomainWatchdogActionTypeToString;
 virDomainWatchdogDefFree;
 virDomainWatchdogModelTypeFromString;
 virDomainWatchdogModelTypeToString;
+virDomainXenPassthroughModeTypeFromString;
+virDomainXenPassthroughModeTypeToString;
 virDomainXMLOptionGetNamespace;
 virDomainXMLOptionGetSaveCookie;
 virDomainXMLOptionNew;
diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c
index 87c66becaf..5c37e431eb 100644
--- a/src/libxl/xen_common.c
+++ b/src/libxl/xen_common.c
@@ -530,14 +530,14 @@ xenParseCPU(virConfPtr conf,
 static int
 xenParseHypervisorFeatures(virConfPtr conf, virDomainDefPtr def)
 {
-g_autofree char *tsc_mode = NULL;
+g_autofree char *strval = NULL;
 virDomainTimerDefPtr timer;
 int val = 0;
 
-if (xenConfigGetString(conf, "tsc_mode", _mode, NULL) < 0)
+if (xenConfigGetString(conf, "tsc_mode", , NULL) < 0)
 return -1;
 
-if (tsc_mode) {
+if (strval) {
 if (VIR_EXPAND_N(def->clock.timers, def->clock.ntimers, 1) < 0 ||
 VIR_ALLOC(timer) < 0)
 return -1;
@@ -547,16 +547,40 @@ xenParseHypervisorFeatures(virConfPtr conf, 
virDomainDefPtr def)
 timer->tickpolicy = -1;
 timer->mode = VIR_DOMAIN_TIMER_MODE_AUTO;
 timer->track = -1;
-if (STREQ_NULLABLE(tsc_mode, "always_emulate"))
+if (STREQ_NULLABLE(strval, "always_emulate"))
 timer->mode = VIR_DOMAIN_TIMER_MODE_EMULATE;
-else if (STREQ_NULLABLE(tsc_mode, "native"))
+else if (STREQ_NULLABLE(strval, "native"))
 timer->mode = VIR_DOMAIN_TIMER_MODE_NATIVE;
-else if (STREQ_NULLABLE(tsc_mode, "native_paravirt"))
+else if (STREQ_NULLABLE(strval, "native_paravirt"))
 timer->mode = VIR_DOMAIN_TIMER_MODE_PARAVIRT;
 
 def->clock.timers[def->clock.ntimers - 1] = timer;
 }
 
+if (xenConfigGetString(conf, "passthrough", , NULL) < 0)
+return -1;
+
+if (strval) {
+if (STREQ(strval, "disabled")) {
+def->features[VIR_DOMAIN_FEATURE_XEN] = VIR_TRISTATE_SWITCH_OFF;
+def->xen_features[VIR_DOMAIN_XEN_PASSTHROUGH] = 
VIR_TRISTATE_SWITCH_OFF;
+} else if (STREQ(strval, "enabled")) {
+def->features[VIR_DOMAIN_FEATURE_XEN] = VIR_TRISTATE_SWITCH_ON;
+def->xen_features[VIR_DOMAIN_XEN_PASSTHROUGH] = 
VIR_TRISTATE_SWITCH_ON;
+} else if (STREQ(strval, "sync_pt")) {
+def->features[VIR_DOMAIN_FEATURE_XEN] = VIR_TRISTATE_SWITCH_ON;
+def->xen_features[VIR_DOMAIN_XEN_PASSTHROUGH] = 
VIR_TRISTATE_SWITCH_ON;
+def->xen_passthrough_mode = 
VIR_DOMAIN_XEN_PASSTHROUGH_MODE_SYNC_PT;
+} else if (STREQ(strval, "share_pt")) {
+def->features[VIR_DOMAIN_FEATURE_XEN] = VIR_TRISTATE_SWITCH_ON;
+def->xen_features[VIR_DOMAIN_XEN_PASSTHROUGH] = 
VIR_TRISTATE_SWITCH_ON;
+def->xen_passthrough_mode = 
VIR_DOMAIN_XEN_PASSTHROUGH_MODE_SHARE_PT;
+} else {
+virReportError(VIR_ERR_CONF_SYNTAX,
+   _("Invalid passthrough mode %s"), strval);
+}
+}
+
 if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
 if (xenConfigGetBool(conf, "pae", , 1) < 0)
 return -1;
@@ -2163,6 +2187,20 @@ xenFormatHypervisorFeatures(virConfPtr conf, 
virDomainDefPtr def)
 }
 }
 
+if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) {
+if (def->xen_features[VIR_DOMAIN_XEN_PASSTHROUGH] == 
VIR_TRISTATE_SWITCH_ON) {
+if (def->xen_passthrough_mode == 
VIR_DOMAIN_XEN_PASSTHROUGH_MODE_SYNC_PT ||
+def->xen_passthrough_mode == 
VIR_DOMAIN_XEN_PASSTHROUGH_MODE_SHARE_PT) {
+if (xenConfigSetString(conf, "passthrough",
+   
virDomainXenPassthroughModeTypeToString(def->xen_passthrough_mode)) < 0)
+return -1;
+} else {
+if (xenConfigSetString(conf, "passthrough", "enabled") < 0)
+return -1;
+}
+}
+}
+
 for (i = 0; i < def->clock.ntimers; i++) {
 switch ((virDomainTimerNameType)def->clock.timers[i]->name) {
 case VIR_DOMAIN_TIMER_NAME_TSC:
-- 
2.26.0

[PATCH 3/5] libxl: refactor cpu and hypervisor feature parser/formatter

[Jim Fehlig]

Signed-off-by: Jim Fehlig 
---
 src/libxl/xen_common.c | 38 --
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c
index bbb9739e01..87c66becaf 100644
--- a/src/libxl/xen_common.c
+++ b/src/libxl/xen_common.c
@@ -493,15 +493,12 @@ xenParsePCIList(virConfPtr conf, virDomainDefPtr def)
 
 
 static int
-xenParseCPUFeatures(virConfPtr conf,
-virDomainDefPtr def,
-virDomainXMLOptionPtr xmlopt)
+xenParseCPU(virConfPtr conf,
+virDomainDefPtr def,
+virDomainXMLOptionPtr xmlopt)
 {
 unsigned long count = 0;
 g_autofree char *cpus = NULL;
-g_autofree char *tsc_mode = NULL;
-int val = 0;
-virDomainTimerDefPtr timer;
 
 if (xenConfigGetULong(conf, "vcpus", , 1) < 0)
 return -1;
@@ -526,6 +523,17 @@ xenParseCPUFeatures(virConfPtr conf,
 if (cpus && (virBitmapParse(cpus, >cpumask, 4096) < 0))
 return -1;
 
+return 0;
+}
+
+
+static int
+xenParseHypervisorFeatures(virConfPtr conf, virDomainDefPtr def)
+{
+g_autofree char *tsc_mode = NULL;
+virDomainTimerDefPtr timer;
+int val = 0;
+
 if (xenConfigGetString(conf, "tsc_mode", _mode, NULL) < 0)
 return -1;
 
@@ -552,27 +560,26 @@ xenParseCPUFeatures(virConfPtr conf,
 if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
 if (xenConfigGetBool(conf, "pae", , 1) < 0)
 return -1;
-
 else if (val)
 def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_TRISTATE_SWITCH_ON;
+
 if (xenConfigGetBool(conf, "acpi", , 1) < 0)
 return -1;
-
 else if (val)
 def->features[VIR_DOMAIN_FEATURE_ACPI] = VIR_TRISTATE_SWITCH_ON;
+
 if (xenConfigGetBool(conf, "apic", , 1) < 0)
 return -1;
-
 else if (val)
 def->features[VIR_DOMAIN_FEATURE_APIC] = VIR_TRISTATE_SWITCH_ON;
+
 if (xenConfigGetBool(conf, "hap", , 1) < 0)
 return -1;
-
 else if (!val)
 def->features[VIR_DOMAIN_FEATURE_HAP] = VIR_TRISTATE_SWITCH_OFF;
+
 if (xenConfigGetBool(conf, "viridian", , 0) < 0)
 return -1;
-
 else if (val)
 def->features[VIR_DOMAIN_FEATURE_VIRIDIAN] = 
VIR_TRISTATE_SWITCH_ON;
 
@@ -1483,7 +1490,10 @@ xenParseConfigCommon(virConfPtr conf,
 if (xenParseEventsActions(conf, def) < 0)
 return -1;
 
-if (xenParseCPUFeatures(conf, def, xmlopt) < 0)
+if (xenParseCPU(conf, def, xmlopt) < 0)
+return -1;
+
+if (xenParseHypervisorFeatures(conf, def) < 0)
 return -1;
 
 if (xenParseTimeOffset(conf, def) < 0)
@@ -2115,7 +2125,7 @@ xenFormatCPUAllocation(virConfPtr conf, virDomainDefPtr 
def)
 
 
 static int
-xenFormatCPUFeatures(virConfPtr conf, virDomainDefPtr def)
+xenFormatHypervisorFeatures(virConfPtr conf, virDomainDefPtr def)
 {
 size_t i;
 bool hvm = !!(def->os.type == VIR_DOMAIN_OSTYPE_HVM);
@@ -2423,7 +2433,7 @@ xenFormatConfigCommon(virConfPtr conf,
 if (xenFormatCPUAllocation(conf, def) < 0)
 return -1;
 
-if (xenFormatCPUFeatures(conf, def) < 0)
+if (xenFormatHypervisorFeatures(conf, def) < 0)
 return -1;
 
 if (xenFormatTimeOffset(conf, def) < 0)
-- 
2.26.0

[PATCH 0/5] Xen: Add support for xl.cfg passthrough setting

[Jim Fehlig]

Hi All,

Note: This series is based on Marek's patches adding support for e820_host

https://www.redhat.com/archives/libvir-list/2020-April/msg00633.html

which I've ACK'ed, but am waiting to commit until this related setting is
hashed out.

This series adds support for Xen's xl.cfg(5) 'passthrough' setting.
Starting with xen 4.13 this setting must be enabled in order to assign
PCI passthrough devices to a guest. libxl will enable it automatically
if the guest config has PCI devices at creation time, but otherwise
'passthrough' must be enabled to hotplug PCI devices.

The passthrough setting is mapped to a xen hypervisor feature of the
same name. Xen hypervisor features were recently added by the series
mentioned above.

Jim Fehlig (5):
  conf: add xen hypervisor feature 'passthrough'
  libxl: make use of passthrough hypervisor feature
  libxl: refactor cpu and hypervisor feature parser/formatter
  xenconfig: Add support for 'passthrough' hypervisor feature
  tests: check conversion of passthrough hypervisor feature

 docs/formatdomain.html.in |   7 ++
 docs/schemas/domaincommon.rng |  12 ++
 src/conf/domain_conf.c| 115 ++
 src/conf/domain_conf.h|  11 ++
 src/libvirt_private.syms  |   2 +
 src/libxl/libxl_conf.c|  21 
 src/libxl/xen_common.c|  86 ++---
 .../test-fullvirt-hypervisor-features.cfg |  26 
 .../test-fullvirt-hypervisor-features.xml |  50 
 .../xlconfigdata/test-paravirt-e820_host.cfg  |   1 +
 .../xlconfigdata/test-paravirt-e820_host.xml  |   1 +
 tests/xlconfigtest.c  |   3 +
 12 files changed, 290 insertions(+), 45 deletions(-)
 create mode 100644 tests/xlconfigdata/test-fullvirt-hypervisor-features.cfg
 create mode 100644 tests/xlconfigdata/test-fullvirt-hypervisor-features.xml

-- 
2.26.0

[PATCH 5/5] tests: check conversion of passthrough hypervisor feature

[Jim Fehlig]

Add a new test to check the 'mode' attribute of the passthrough element
and augment an existing, related test to check enablement of the
passthrough element only.

Signed-off-by: Jim Fehlig 
---
 .../test-fullvirt-hypervisor-features.cfg | 26 ++
 .../test-fullvirt-hypervisor-features.xml | 50 +++
 .../xlconfigdata/test-paravirt-e820_host.cfg  |  1 +
 .../xlconfigdata/test-paravirt-e820_host.xml  |  1 +
 tests/xlconfigtest.c  |  3 ++
 5 files changed, 81 insertions(+)

diff --git a/tests/xlconfigdata/test-fullvirt-hypervisor-features.cfg 
b/tests/xlconfigdata/test-fullvirt-hypervisor-features.cfg
new file mode 100644
index 00..88f018c823
--- /dev/null
+++ b/tests/xlconfigdata/test-fullvirt-hypervisor-features.cfg
@@ -0,0 +1,26 @@
+name = "XenGuest2"
+uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809"
+maxmem = 579
+memory = 394
+vcpus = 1
+pae = 1
+acpi = 1
+apic = 1
+viridian = 0
+passthrough = "share_pt"
+rtc_timeoffset = 0
+localtime = 0
+on_poweroff = "destroy"
+on_reboot = "restart"
+on_crash = "restart"
+device_model = "/usr/lib/xen/bin/qemu-system-i386"
+sdl = 0
+vnc = 1
+vncunused = 1
+vnclisten = "127.0.0.1"
+vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000" ]
+parallel = "none"
+serial = "none"
+builder = "hvm"
+boot = "c"
+disk = [ 
"format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ]
diff --git a/tests/xlconfigdata/test-fullvirt-hypervisor-features.xml 
b/tests/xlconfigdata/test-fullvirt-hypervisor-features.xml
new file mode 100644
index 00..c36290bb6a
--- /dev/null
+++ b/tests/xlconfigdata/test-fullvirt-hypervisor-features.xml
@@ -0,0 +1,50 @@
+
+  XenGuest2
+  c7a5fdb2-cdaf-9455-926a-d65c16db1809
+  592896
+  403456
+  1
+  
+hvm
+/usr/lib/xen/boot/hvmloader
+
+  
+  
+
+
+
+
+  
+
+  
+  
+  destroy
+  restart
+  restart
+  
+/usr/lib/xen/bin/qemu-system-i386
+
+  
+  
+  
+  
+
+
+
+
+  
+  
+  
+  
+
+
+
+
+  
+
+
+  
+
+
+  
+
diff --git a/tests/xlconfigdata/test-paravirt-e820_host.cfg 
b/tests/xlconfigdata/test-paravirt-e820_host.cfg
index b9e5a482a4..ad6cb8420b 100644
--- a/tests/xlconfigdata/test-paravirt-e820_host.cfg
+++ b/tests/xlconfigdata/test-paravirt-e820_host.cfg
@@ -4,6 +4,7 @@ maxmem = 512
 memory = 512
 vcpus = 4
 e820_host = 1
+passthrough = "enabled"
 localtime = 0
 on_poweroff = "preserve"
 on_reboot = "restart"
diff --git a/tests/xlconfigdata/test-paravirt-e820_host.xml 
b/tests/xlconfigdata/test-paravirt-e820_host.xml
index 955a780ffa..d3bfb156eb 100644
--- a/tests/xlconfigdata/test-paravirt-e820_host.xml
+++ b/tests/xlconfigdata/test-paravirt-e820_host.xml
@@ -11,6 +11,7 @@
   
 
   
+  
 
   
   
diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c
index 8ea250347b..b2e045dfa5 100644
--- a/tests/xlconfigtest.c
+++ b/tests/xlconfigtest.c
@@ -301,6 +301,9 @@ mymain(void)
 DO_TEST("usb");
 DO_TEST("usbctrl");
 DO_TEST("paravirt-e820_host");
+#ifdef LIBXL_HAVE_CREATEINFO_PASSTHROUGH
+DO_TEST("fullvirt-hypervisor-features");
+#endif
 
 testXLFreeDriver(driver);
 
-- 
2.26.0

[PATCH 1/5] conf: add xen hypervisor feature 'passthrough'

[Jim Fehlig]

'passthrough' is Xen-Specific guest configuration option new to Xen 4.13
that enables IOMMU mappings for a guest and hence whether it supports PCI
passthrough. The default is disabled. See the xl.cfg(5) man page and
xen.git commit babde47a3fe for more details.

The default state of disabled prevents hotlugging PCI devices. However,
if the guest configuration contains a PCI passthrough device at time of
creation, libxl will automatically enable 'passthrough' and subsequent
hotplugging of PCI devices will also be possible. It is not possible to
unconditionally enable 'passthrough' since it would introduce a migration
incompatibility due to guest ABI change. Instead, introduce another Xen
hypervisor feature that can be used to enable guest PCI passthrough

  

  

  

To allow finer control over how IOMMU maps to guest P2M table, the
passthrough element also supports a 'mode' attribute with values
restricted to snyc_pt and share_pt, similar to xl.cfg(5) 'passthrough'
setting .

Signed-off-by: Jim Fehlig 
---
 docs/formatdomain.html.in |   7 +++
 docs/schemas/domaincommon.rng |  12 
 src/conf/domain_conf.c| 115 ++
 src/conf/domain_conf.h|  11 
 4 files changed, 119 insertions(+), 26 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 81060d6730..9ccae54e04 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2063,6 +2063,7 @@
   /kvm
   xen
 e820_host state='on'/
+passthrough state='on' mode='share_pt'/
   /xen
   pvspinlock state='on'/
   gic version='2'/
@@ -2260,6 +2261,12 @@
   on, off
   6.3.0
 
+
+  passthrough
+  Enable IOMMU mappings allowing PCI passthrough)
+  on, off; mode - optional string sync_pt or share_pt
+  6.3.0
+
   
   
   pmu
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 2b5f844658..a6f6e8ab83 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -6349,6 +6349,18 @@
 
   
 
+
+  
+
+
+  
+
+  (sync_pt|share_pt)
+
+  
+
+  
+
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index dabbceb265..9f3362c934 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -209,7 +209,15 @@ VIR_ENUM_IMPL(virDomainKVM,
 
 VIR_ENUM_IMPL(virDomainXen,
   VIR_DOMAIN_XEN_LAST,
-  "e820_host"
+  "e820_host",
+  "passthrough",
+);
+
+VIR_ENUM_IMPL(virDomainXenPassthroughMode,
+  VIR_DOMAIN_XEN_PASSTHROUGH_MODE_LAST,
+  "default",
+  "sync_pt",
+  "share_pt",
 );
 
 VIR_ENUM_IMPL(virDomainMsrsUnknown,
@@ -21182,6 +21190,8 @@ virDomainDefParseXML(xmlDocPtr xml,
 if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) {
 int feature;
 int value;
+g_autofree char *ptval = NULL;
+
 if ((n = virXPathNodeSet("./features/xen/*", ctxt, )) < 0)
 goto error;
 
@@ -21194,27 +21204,53 @@ virDomainDefParseXML(xmlDocPtr xml,
 goto error;
 }
 
+if (!(tmp = virXMLPropString(nodes[i], "state"))) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("missing 'state' attribute for "
+ "Xen feature '%s'"),
+   nodes[i]->name);
+goto error;
+}
+
+if ((value = virTristateSwitchTypeFromString(tmp)) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("invalid value of state argument "
+ "for Xen feature '%s'"),
+   nodes[i]->name);
+goto error;
+}
+
+VIR_FREE(tmp);
+def->xen_features[feature] = value;
+
 switch ((virDomainXen) feature) {
 case VIR_DOMAIN_XEN_E820_HOST:
-if (!(tmp = virXMLPropString(nodes[i], "state"))) {
-virReportError(VIR_ERR_XML_ERROR,
-   _("missing 'state' attribute for "
- "Xen feature '%s'"),
-   nodes[i]->name);
-goto error;
-}
+break;
 
-if ((value = virTristateSwitchTypeFromString(tmp)) < 0) {
+case VIR_DOMAIN_XEN_PASSTHROUGH:
+if (value != VIR_TRISTATE_SWITCH_ON)
+break;
+
+if ((ptval = virXMLPropString(nodes[i], "mode"))) {
+int mode = 
virDomainXenPassthroughModeTypeFromString(ptval);
+
+ 

[libvirt PATCH] util: Do not include sys/wait.h on Win32

[Jiri Denemark]

This fixes build on mingw broken by my previous commit 36e125296a.

Signed-off-by: Jiri Denemark 
---

Pushed.

 src/util/virdaemon.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/util/virdaemon.c b/src/util/virdaemon.c
index 6182ca3c21..31cc24e703 100644
--- a/src/util/virdaemon.c
+++ b/src/util/virdaemon.c
@@ -22,7 +22,9 @@
 
 #include 
 #include 
-#include 
+#ifndef WIN32
+# include 
+#endif
 #include 
 #include 
 #include 
-- 
2.26.1

[PATCH] Add 'permissive' option for PCI devices

[Marek Marczykowski-Górecki]

From: Simon Gaiser 

By setting the permissive flag the guest access to the PCI config space
is not filtered. This might be a security risk, but it's required for
some devices and the IOMMU and interrupt remapping should (mostly?)
contain it.

Signed-off-by: Simon Gaiser 
Signed-off-by: Marek Marczykowski-Górecki 
---
 docs/formatdomain.html.in |  3 +++
 docs/schemas/domaincommon.rng |  5 +
 src/conf/domain_conf.c| 12 
 src/conf/domain_conf.h|  1 +
 src/libxl/libxl_conf.c|  1 +
 5 files changed, 22 insertions(+)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 6f43976815..79a5176ccd 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -5051,6 +5051,9 @@
 or hot-plugging the device and virNodeDeviceReAttach
 (or virsh nodedev-reattach) after hot-unplug or
 stopping the guest.
+When permissive is "yes" the pci config space access
+will not be filtered. This might be a security issue. The default
+is "no".
   
   scsi
   For SCSI devices, user is responsible to make sure the device
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 65d6580434..9389eec3d8 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3064,6 +3064,11 @@
   
 
   
+  
+
+  
+
+  
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8e8146374c..607cae61d4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8419,6 +8419,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
 virDomainHostdevSubsysSCSIVHostPtr scsihostsrc = 
>source.subsys.u.scsi_host;
 virDomainHostdevSubsysMediatedDevPtr mdevsrc = >source.subsys.u.mdev;
 g_autofree char *managed = NULL;
+g_autofree char *permissive = NULL;
 g_autofree char *sgio = NULL;
 g_autofree char *rawio = NULL;
 g_autofree char *backendStr = NULL;
@@ -8434,6 +8435,11 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
 if ((managed = virXMLPropString(node, "managed")) != NULL)
 ignore_value(virStringParseYesNo(managed, >managed));
 
+if ((permissive = virXMLPropString(node, "permissive")) != NULL) {
+if (STREQ(permissive, "yes"))
+def->permissive = true;
+}
+
 sgio = virXMLPropString(node, "sgio");
 rawio = virXMLPropString(node, "rawio");
 model = virXMLPropString(node, "model");
@@ -25942,6 +25948,8 @@ virDomainActualNetDefFormat(virBufferPtr buf,
 virDomainHostdevDefPtr hostdef = virDomainNetGetActualHostdev(def);
 if  (hostdef && hostdef->managed)
 virBufferAddLit(buf, " managed='yes'");
+if  (hostdef && hostdef->permissive)
+virBufferAddLit(buf, " permissive='yes'");
 }
 if (def->trustGuestRxFilters)
 virBufferAsprintf(buf, " trustGuestRxFilters='%s'",
@@ -26130,6 +26138,8 @@ virDomainNetDefFormat(virBufferPtr buf,
 virBufferAsprintf(buf, "managed)
 virBufferAddLit(buf, " managed='yes'");
+if (hostdef && hostdef->permissive)
+virBufferAddLit(buf, " permissive='yes'");
 if (def->trustGuestRxFilters)
 virBufferAsprintf(buf, " trustGuestRxFilters='%s'",
   
virTristateBoolTypeToString(def->trustGuestRxFilters));
@@ -27914,6 +27924,8 @@ virDomainHostdevDefFormat(virBufferPtr buf,
 if (def->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
 virBufferAsprintf(buf, " managed='%s'",
   def->managed ? "yes" : "no");
+if (def->permissive)
+virBufferAddLit(buf, " permissive='yes'");
 
 if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI &&
 scsisrc->sgio)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index aad3f82db7..b81a3ce901 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -345,6 +345,7 @@ struct _virDomainHostdevDef {
 bool missing;
 bool readonly;
 bool shareable;
+bool permissive;
 union {
 virDomainHostdevSubsys subsys;
 virDomainHostdevCaps caps;
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index b3f67f817a..55f2a09e3e 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -2249,6 +2249,7 @@ libxlMakePCI(virDomainHostdevDefPtr hostdev, 
libxl_device_pci *pcidev)
 pcidev->bus = pcisrc->addr.bus;
 pcidev->dev = pcisrc->addr.slot;
 pcidev->func = pcisrc->addr.function;
+pcidev->permissive = hostdev->permissive;
 
 return 0;
 }
-- 
2.21.1

Re: [PATCH 0/2] Include lease time option into DHCP settings

[Daniel P . Berrangé]

On Wed, Apr 15, 2020 at 01:25:37PM -0300, Julio Faracco wrote:
> I resubmitted this series because our team needs to hack dnsmasq
> settings to change lease time. This feature would be so important to
> us to avoid workarounds.
> 
> It is based on Alberto's patch from 2017. But personally I don't like
> this approach.
> IMHO, it would be nice to have specific attributes to configure lease time.
> For example:
> 
> 

It is generally considered bad practice to have an XML attribute
value which then has to be parsed again to extract information.

For this reason, libvirt will use two attrbutes, one of the
value and one for the units (with some sensible default
units if not specified), even though this is admittedly
more verbose.

I agree it would be useful to have lease time per-host, as well
as globally though, and IIRC one of the original versions of
this patch did support that.

We could do one of 

  

  
  

  

or

  

  
  

  

or

  

  
 
  
  
 
  

  

In all of these we can default to "minutes" if no units are given.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH 1/2] conf: Add option for settings

[Daniel P . Berrangé]

On Wed, Apr 15, 2020 at 01:18:49PM -0300, Julio Faracco wrote:
> If an user is trying to configure a dhcp neetwork settings, it is not
> possible to change the leasetime of a range or a host entry. This is
> available using dnsmasq extra options, but they are associated with
> dhcp-range or dhcp-hosts fields. This patch implements a default
> leasetime for both. If this XML entry is defined, it applies leasetime
> for each range or host defined under DHCP scope.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=913446
> 
> Signed-off-by: Julio Faracco 
> ---
>  docs/schemas/basictypes.rng |  9 ++
>  docs/schemas/network.rng| 11 +++
>  src/conf/network_conf.c | 62 -
>  src/conf/network_conf.h | 14 +
>  src/libvirt_private.syms|  2 ++
>  src/network/bridge_driver.c | 37 --
>  src/util/virdnsmasq.c   | 40 
>  src/util/virdnsmasq.h   |  1 +
>  8 files changed, 152 insertions(+), 24 deletions(-)
> 
> diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
> index 81465273c8..12f085c101 100644
> --- a/docs/schemas/basictypes.rng
> +++ b/docs/schemas/basictypes.rng
> @@ -607,4 +607,13 @@
>  
>
>  
> +  
> +
> +  seconds
> +  minutes
> +  hours
> +  infinite

"infinite" is not really a unit - it is a value, so I don't think
we should have this. At least one version of Alberto's patches
used the value "0" to indicate no expiry, which I think is
reasonable, as 0 is otherwise invalid.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH v2 0/2] docs: Further adjustments to pci-addresses.rst

[Boris Fiuczynski]

On 4/17/20 3:06 PM, Andrea Bolognani wrote:

On Fri, 2020-04-17 at 14:27 +0200, Boris Fiuczynski wrote:

Andrea Bolognani (1):
   docs: Update introduction in pci-addresses.rst
Boris Fiuczynski (1):
   docs: Improve zPCI section in pci-addresses.rst

  docs/pci-addresses.rst | 31 ++-
  1 file changed, 18 insertions(+), 13 deletions(-)


Series

   Reviewed-by: Andrea Bolognani 

and pushed.

Thank you both for your contributions and your patience :)


Thanks for pushing the changes and your patience. Have a nice weekend.

--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

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

Re: [PATCH 2/3] security: Introduce virSecurityManagerDomainSetIncomingPathLabel

[Erik Skultety]

...

> > >
> > > +/**
> > > + * virSecurityManagerDomainSetIncomingPathLabel:
> > > + * @mgr: security manager object
> > > + * @vm: domain definition object
> > > + * @path: path to label
> > > + *
> > > + * This function relabels given @path so that @vm can restore for
> >
> > maybe add "host" @path here to make it even clearer.
> >
> > > + * it.  This allows the driver backend to use different label than
> > > + * virSecurityManagerDomainSetPathLabel().
>
> This would then be:
>
> This function relabels given @path for read only access, which is in
> contrast with virSecurityManagerDomainSetPathLabel() which gives read write
> access.

Yep, I'm okay with *SetPathLabelRO, don't forget to include ^this commentary
bit :).

--
Erik Skultety

Re: [PATCH v2 0/2] docs: Further adjustments to pci-addresses.rst

[Andrea Bolognani]

On Fri, 2020-04-17 at 14:27 +0200, Boris Fiuczynski wrote:
> Andrea Bolognani (1):
>   docs: Update introduction in pci-addresses.rst
> Boris Fiuczynski (1):
>   docs: Improve zPCI section in pci-addresses.rst
> 
>  docs/pci-addresses.rst | 31 ++-
>  1 file changed, 18 insertions(+), 13 deletions(-)

Series

  Reviewed-by: Andrea Bolognani 

and pushed.

Thank you both for your contributions and your patience :)

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH 2/3] security: Introduce virSecurityManagerDomainSetIncomingPathLabel

[Michal Privoznik]

On 4/17/20 12:57 PM, Erik Skultety wrote:

On Fri, Apr 03, 2020 at 05:58:02PM +0200, Michal Privoznik wrote:

This API allows drivers to separate out handling of @stdin_path
of virSecurityManagerSetAllLabel(). The thing is, the QEMU driver
uses transactions for virSecurityManagerSetAllLabel() which
relabels devices from inside of domain's namespace. This is what
we usually want. Except when resuming domain from a file. The
file is opened before any namespace is set up and the FD is
passed to QEMU to read the migration stream from. Because of
this, the file lives outside of the namespace and if it so
happens that the file is a block device (i.e. it lives under
/dev) its copy will be created in the namespace. But the FD that
is passed to QEMU points to the original living in the host and
not in the namespace. So relabeling the file inside the namespace
helps nothing.

But if we have a separate API for relabeling the restore file
then the QEMU driver can continue calling
virSecurityManagerSetAllLabel() with transactions enabled and
call this new API without transactions.

We already have an API for relabeling a single file
(virSecurityManagerDomainSetPathLabel()) but in case of SELinux
it uses @imagelabel (which allows RW access) and we want to use
@content_context (which allows RO access).


Since this patch is only adding a generic driver API rather than a specific
security backend API, I'd move ^this last paragraph to the next patch, I
actually appreciated the info there much more than here during the review.



Signed-off-by: Michal Privoznik 
---
  src/libvirt_private.syms|  1 +
  src/security/security_driver.h  |  4 
  src/security/security_manager.c | 29 +
  src/security/security_manager.h |  4 
  src/security/security_stack.c   | 21 +
  5 files changed, 59 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e276f55bb1..2c63f37fc2 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1523,6 +1523,7 @@ virSecurityDriverLookup;
  # security/security_manager.h
  virSecurityManagerCheckAllLabel;
  virSecurityManagerClearSocketLabel;
+virSecurityManagerDomainSetIncomingPathLabel;


IncomingPath sounds a bit confusing to me, I'd go with something like HostPath
instead, in patch 3/3 you also have an explicit commentary why we're using the
respective backend API for this rather than simply using SetPathLabel.


Yeah, naming is hard. Part of my reasoning was that if the name is not 
obvious people will not use it and default to 
virSecurityDomainSetPathLabel() which should be used as it guarantees RW 
access. But on the secdriver API level we don't distinguish transactions 
and thus host/namespaces. But if my reasoning was flawed and we might 
actually encourage people to use it? I mean, how about 
virSecurityDomainSetPathLabelRO() for the name, how does that sound?


We can leave the transactions aside and at hypervisor driver discretion 
because in fact, it's only QEMU who uses transactions.





  virSecurityManagerDomainSetPathLabel;
  virSecurityManagerGenLabel;
  virSecurityManagerGetBaseLabel;
diff --git a/src/security/security_driver.h b/src/security/security_driver.h
index 3353955813..6cbe8613c9 100644
--- a/src/security/security_driver.h
+++ b/src/security/security_driver.h
@@ -140,6 +140,9 @@ typedef int (*virSecurityDomainSetPathLabel) 
(virSecurityManagerPtr mgr,
virDomainDefPtr def,
const char *path,
bool allowSubtree);
+typedef int (*virSecurityDomainSetIncomingPathLabel) (virSecurityManagerPtr 
mgr,
+  virDomainDefPtr def,
+  const char *path);
  typedef int (*virSecurityDomainSetChardevLabel) (virSecurityManagerPtr mgr,
   virDomainDefPtr def,
   virDomainChrSourceDefPtr 
dev_source,
@@ -211,6 +214,7 @@ struct _virSecurityDriver {
  virSecurityDriverGetBaseLabel getBaseLabel;

  virSecurityDomainSetPathLabel domainSetPathLabel;
+virSecurityDomainSetIncomingPathLabel domainSetIncomingPathLabel;

  virSecurityDomainSetChardevLabel domainSetSecurityChardevLabel;
  virSecurityDomainRestoreChardevLabel domainRestoreSecurityChardevLabel;
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index fe032746ff..a76b953ee4 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -1077,6 +1077,35 @@ 
virSecurityManagerDomainSetPathLabel(virSecurityManagerPtr mgr,
  }


+/**
+ * virSecurityManagerDomainSetIncomingPathLabel:
+ * @mgr: security manager object
+ * @vm: domain definition object
+ * @path: path to label
+ *
+ * This function relabels given @path so that @vm can 

Re: [PATCH 1/3] Improve blockpull man entry

[Peter Krempa]

On Fri, Apr 17, 2020 at 14:52:06 +0200, Peter Krempa wrote:
> On Wed, Apr 15, 2020 at 11:34:04 +, Sebastian Mitterle wrote:
> > 1. Mention usage of `--base` and `--bandwidth` and fix cmd syntax.
> > 2. Explain valid arguments for `base`.
> > 3. Move explanation for `--keep-relative` to end considering it
> >less frequent use case because libvirt doesn't create relative
> >backing chain names.
> > 4. Add reference to documentation for relative paths in backing chains.
> > 
> > Signed-off-by: Sebastian Mitterle 
> > ---
> >  docs/manpages/virsh.rst | 19 +--
> >  1 file changed, 13 insertions(+), 6 deletions(-)

[...]

> > +
> > +Using the *--keep-relative* flag will keep the backing chain names
> > +relative (details on `https://www.libvirt.org/kbase/backing_chains.html
> > +`__).
> 
> I don't think this document contains information on relative file paths.

It actually does. Unfortunately it doesn't specify how to take a
snapshot with relative paths thoug. But this is probably okay then.

Re: [PATCH 1/3] Improve blockpull man entry

[Peter Krempa]

On Wed, Apr 15, 2020 at 11:34:04 +, Sebastian Mitterle wrote:
> 1. Mention usage of `--base` and `--bandwidth` and fix cmd syntax.
> 2. Explain valid arguments for `base`.
> 3. Move explanation for `--keep-relative` to end considering it
>less frequent use case because libvirt doesn't create relative
>backing chain names.
> 4. Add reference to documentation for relative paths in backing chains.
> 
> Signed-off-by: Sebastian Mitterle 
> ---
>  docs/manpages/virsh.rst | 19 +--
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> index dc404ddfe8..27ecc53d56 100644
> --- a/docs/manpages/virsh.rst
> +++ b/docs/manpages/virsh.rst
> @@ -1345,7 +1345,7 @@ blockpull
>  
>  .. code-block::
>  
> -   blockpull domain path [bandwidth] [--bytes] [base]
> +   blockpull domain path { [bandwidth [--bytes] [base]] | [--bandwidth 
> [--bytes]] [--base] }

For any argument where the '--' can be skipped we use the non-dash
syntax. Additionally your version is missing the argument placeholder.

>[--wait [--verbose] [--timeout seconds] [--async]]
>[--keep-relative]
>  
> @@ -1353,7 +1353,7 @@ Populate a disk from its backing image chain. By 
> default, this command
>  flattens the entire chain; but if *base* is specified, containing the
>  name of one of the backing files in the chain, then that file becomes
>  the new backing file and only the intermediate portion of the chain is
> -pulled.  Once all requested data from the backing image chain has been
> +pulled. Once all requested data from the backing image chain has been
>  pulled, the disk no longer depends on that portion of the backing chain.

This is somewhat common in our docs. (two spaces between sentences)

>  
>  By default, this command returns as soon as possible, and data for
> @@ -1367,16 +1367,23 @@ is triggered, *--async* will return control to the 
> user as fast as
>  possible, otherwise the command may continue to block a little while
>  longer until the job is done cleaning up.
>  
> -Using the *--keep-relative* flag will keep the backing chain names
> -relative.
> -
>  *path* specifies fully-qualified path of the disk; it corresponds
>  to a unique target name () or source file (  file='name'/>) for one of the disk devices attached to *domain* (see
>  also ``domblklist`` for listing these names).
> +
>  *bandwidth* specifies copying bandwidth limit in MiB/s. For further 
> information
>  on the *bandwidth* argument see the corresponding section for the 
> ``blockjob``
> -command.
> +command. Using *--bytes* flag indicates the value in *bandwidth* is given in
> +bytes.
> +
> +*base* specifies fully-qualified path of the backing file; it corresponds
> +to a unique indexed target name 'name[i]' (..
> +) or source file 'name' ().

Well, it's either a fully qualified path, or preferrably the 'name[i]'
thing. but 'name[i]' is not a fully qualified path.

> +
> +Using the *--keep-relative* flag will keep the backing chain names
> +relative (details on `https://www.libvirt.org/kbase/backing_chains.html
> +`__).

I don't think this document contains information on relative file paths.

Re: [PATCH 0/3] doc: minor improvements

[Sebastian Mitterle]

friendly ping

On Wed, Apr 15, 2020 at 1:34 PM Sebastian Mitterle 
wrote:

> Minor man/doc improvements related to blockpull.
>
> Sebastian Mitterle (3):
>   Improve blockpull man entry
>   Improve Disk image chains documentation
>   Add version info on domaincaps doc
>
>  docs/formatdomaincaps.html.in |  3 ++-
>  docs/kbase/backing_chains.rst | 36 +--
>  docs/manpages/virsh.rst   | 19 --
>  3 files changed, 33 insertions(+), 25 deletions(-)
>
> --
> 2.25.2
>
>

-- 
Best,
Sebastian

Re: [PATCH v2 2/2] docs: Improve zPCI section in pci-addresses.rst

[Cornelia Huck]

On Fri, 17 Apr 2020 14:27:34 +0200
Boris Fiuczynski  wrote:

> Improving the zPCI example by choosing more distinct values and
> adding explanation for fid.
> 
> Signed-off-by: Boris Fiuczynski 
> ---
>  docs/pci-addresses.rst | 22 --
>  1 file changed, 12 insertions(+), 10 deletions(-)

Reviewed-by: Cornelia Huck 

[PATCH v2 1/2] docs: Update introduction in pci-addresses.rst

[Boris Fiuczynski]

Changing the introduction to bring the idea of this document better across.

Signed-off-by: Andrea Bolognani 
Reviewed-by: Boris Fiuczynski 
Reviewed-by: Cornelia Huck 
---
 docs/pci-addresses.rst | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/docs/pci-addresses.rst b/docs/pci-addresses.rst
index 1d2dc8e5fc..7c8e9edd73 100644
--- a/docs/pci-addresses.rst
+++ b/docs/pci-addresses.rst
@@ -4,9 +4,12 @@ PCI addresses in domain XML and guest OS
 
 .. contents::
 
-When discussing PCI addresses, it's important to understand the
-relationship between the addresses that can be seen in the domain XML
-and those that are visible inside the guest OS.
+Looking at the configuration for a guest, it would be reasonable
+to expect that each PCI device would show up in the guest OS with
+a PCI address that matches the one present in the corresponding
+ element of the domain XML, but that's not guaranteed
+to happen and will in fact not be the case in all but the simplest
+scenarios.
 
 
 Simple cases
-- 
2.25.1

[PATCH v2 2/2] docs: Improve zPCI section in pci-addresses.rst

[Boris Fiuczynski]

Improving the zPCI example by choosing more distinct values and
adding explanation for fid.

Signed-off-by: Boris Fiuczynski 
---
 docs/pci-addresses.rst | 22 --
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/docs/pci-addresses.rst b/docs/pci-addresses.rst
index 7c8e9edd73..885d50517a 100644
--- a/docs/pci-addresses.rst
+++ b/docs/pci-addresses.rst
@@ -176,14 +176,14 @@ In the simplest case, the following XML snippet
 
 
 
-  
+  
 
   
   
 
 
 
-  
+  
 
   
 
@@ -191,21 +191,23 @@ will result in the following in a Linux guest:
 
 ::
 
-  0001:00:00.0 Ethernet controller: Red Hat, Inc. Virtio network device
+  0007:00:00.0 Ethernet controller: Red Hat, Inc. Virtio network device
 
 Note that the PCI bridge is not visible in the guest; s390x always has a flat
-topology.
+topology. The PCI address in the guest is generated from the information
+provided via the ``zpci`` element: more specifically, ``uid`` is used as the
+PCI domain. ``fid`` doesn't appear in the PCI address itself, but it will be
+used in sysfs (``/sys/bus/pci/slots/$fid/...``).
 
-Neither are any changes in the PCI address visible in the guest; replacing
-the PCI address for the ``virtio-net`` device with
+Any changes in the PCI address are not visible in the guest; replacing the PCI
+address for the ``virtio-net`` device with
 
 ::
 
-  
+  
 
-will result in the exactly same view in the guest, as the addresses there
-are generated from the information provided via the ``zpci`` element (in
-fact, from the ``uid``).
+will result in the exactly same view in the guest, as the ``fid`` and ``uid``
+values in the ``zpci`` element remain unchanged.
 
 
 Device assignment
-- 
2.25.1

[PATCH v2 0/2] docs: Further adjustments to pci-addresses.rst

[Boris Fiuczynski]

Andrea Bolognani (1):
  docs: Update introduction in pci-addresses.rst
Boris Fiuczynski (1):
  docs: Improve zPCI section in pci-addresses.rst

 docs/pci-addresses.rst | 31 ++-
 1 file changed, 18 insertions(+), 13 deletions(-)

-- 
2.25.1

Re: [PATCH 2/2] docs: Improve zPCI section in pci-addresses.rst

[Andrea Bolognani]

On Fri, 2020-04-17 at 13:31 +0200, Cornelia Huck wrote:
> On Fri, 17 Apr 2020 13:29:43 +0200
> Boris Fiuczynski  wrote:
> > On 4/17/20 11:02 AM, Cornelia Huck wrote:
> > > Sounds good.
> > > 
> > > (Also the rest of the changes.)
> > >   
> > Before I break the r-b chain as well... Is that your r-b? :)
> 
> I'll add my R-b to a final patch :)

Same :)

-- 
Andrea Bolognani / Red Hat / Virtualization

[libvirt PATCH 07/10] vbox: add support for version 6.0 SDK

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---
 src/vbox/Makefile.inc.am  |  2 ++
 src/vbox/vbox_V6_0.c  | 13 +
 src/vbox/vbox_common.h|  2 ++
 src/vbox/vbox_storage.c   |  2 ++
 src/vbox/vbox_tmpl.c  |  9 -
 src/vbox/vbox_uniformed_api.h |  1 +
 6 files changed, 28 insertions(+), 1 deletion(-)
 create mode 100644 src/vbox/vbox_V6_0.c

diff --git a/src/vbox/Makefile.inc.am b/src/vbox/Makefile.inc.am
index 9d827eff97..fdc6537d51 100644
--- a/src/vbox/Makefile.inc.am
+++ b/src/vbox/Makefile.inc.am
@@ -7,6 +7,8 @@ VBOX_DRIVER_SOURCES = \
vbox/vbox_snapshot_conf.h \
vbox/vbox_V5_2.c \
vbox/vbox_CAPI_v5_2.h \
+   vbox/vbox_CAPI_v6_0.h \
+   vbox/vbox_V6_0.c \
vbox/vbox_common.c \
vbox/vbox_common.h \
vbox/vbox_uniformed_api.h \
diff --git a/src/vbox/vbox_V6_0.c b/src/vbox/vbox_V6_0.c
new file mode 100644
index 00..4e4db6326f
--- /dev/null
+++ b/src/vbox/vbox_V6_0.c
@@ -0,0 +1,13 @@
+/** @file vbox_V6_0.c
+ * C file to include support for multiple versions of VirtualBox
+ * at runtime.
+ */
+
+#include 
+
+/** The API Version */
+#define VBOX_API_VERSION 600
+/** Version specific prefix. */
+#define NAME(name) vbox60##name
+
+#include "vbox_tmpl.c"
diff --git a/src/vbox/vbox_common.h b/src/vbox/vbox_common.h
index 9ea984276d..7c29d92789 100644
--- a/src/vbox/vbox_common.h
+++ b/src/vbox/vbox_common.h
@@ -431,6 +431,8 @@ typedef nsISupports IKeyboard;
 result = 0; \
 if (uVersion >= 5001051 && uVersion < 5002051) { \
 vbox52InstallUniformedAPI(); \
+} else if (uVersion >= 600 && uVersion < 651) { \
+vbox60InstallUniformedAPI(); \
 } else { \
 result = -1; \
 } \
diff --git a/src/vbox/vbox_storage.c b/src/vbox/vbox_storage.c
index e0b82f4bd8..83172ee1fe 100644
--- a/src/vbox/vbox_storage.c
+++ b/src/vbox/vbox_storage.c
@@ -882,6 +882,8 @@ virStorageDriverPtr vboxGetStorageDriver(uint32_t uVersion)
  */
 if (uVersion >= 5001051 && uVersion < 5002051) {
 vbox52InstallUniformedAPI();
+} else if (uVersion >= 600 && uVersion < 651) {
+vbox60InstallUniformedAPI();
 } else {
 return NULL;
 }
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 5bf305db4d..9ac7f88b0f 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -51,8 +51,10 @@
 /* This one changes from version to version. */
 #if VBOX_API_VERSION == 5002000
 # include "vbox_CAPI_v5_2.h"
+#elif VBOX_API_VERSION == 600
+# include "vbox_CAPI_v6_0.h"
 #else
-# error "Unsupport VBOX_API_VERSION"
+# error "Unsupported VBOX_API_VERSION"
 #endif
 
 /* Include this *last* or we'll get the wrong vbox_CAPI_*.h. */
@@ -729,8 +731,13 @@ _machineCreateSharedFolder(IMachine *machine, PRUnichar 
*name,
PRUnichar *hostPath, PRBool writable,
PRBool automount G_GNUC_UNUSED)
 {
+#if VBOX_API_VERSION >= 600
+return machine->vtbl->CreateSharedFolder(machine, name, hostPath,
+ writable, automount, NULL);
+#else
 return machine->vtbl->CreateSharedFolder(machine, name, hostPath,
  writable, automount);
+#endif
 }
 
 static nsresult
diff --git a/src/vbox/vbox_uniformed_api.h b/src/vbox/vbox_uniformed_api.h
index a4468711fc..9ab3afc4d5 100644
--- a/src/vbox/vbox_uniformed_api.h
+++ b/src/vbox/vbox_uniformed_api.h
@@ -556,3 +556,4 @@ virDomainPtr vboxDomainLookupByUUID(virConnectPtr conn,
 
 /* Version specified functions for installing uniformed API */
 void vbox52InstallUniformedAPI(vboxUniformedAPI *pVBoxAPI);
+void vbox60InstallUniformedAPI(vboxUniformedAPI *pVBoxAPI);
-- 
2.25.2

[libvirt PATCH 10/10] docs: add news about virtualbox version support changes

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---
 docs/news.xml | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 4d0efd4219..99c65447b0 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -56,11 +56,33 @@
   hotplug/unplug attempts, but this is often undesireable).
 
   
+  
+
+  vbox: added support for version 6.0 and 6.1 APIs
+
+
+  libvirt can now support use of the VirtualBox 6.0 and 6.1
+  APIs. This is compile tested only, so we are looking for
+  feedback from users on how well it works in practice.
+
+  
 
 
 
 
 
+
+  
+
+  vbox: removed support for version 5.0 and 5.1 APIs
+
+
+  libvirt no longer supports use of VirtualBox 5.0 and 5.1
+  since these versions reached their end of life on 2017/05
+  and 2018/04 respectively.
+
+  
+
   
   
 
-- 
2.25.2

[libvirt PATCH 09/10] vbox: add support for version 6.1 SDK

[Daniel P . Berrangé]

Changes in the API:

 - APIs related to the graphics adapter are no longer on the
   IMachine interface, but on a IGraphicsAdapter interface
 - The LaunchVMProcess method takes a list of env variables
   instead of a single variable containing a concatenated
   list. Since we only ever pass a single env variable, we
   can simply stuff it straight into a list.
 - The DHCP server start method no longer needs the network
   name

Signed-off-by: Daniel P. Berrangé 
---
 src/vbox/Makefile.inc.am  |  2 +
 src/vbox/vbox_V6_1.c  | 13 ++
 src/vbox/vbox_common.h|  2 +
 src/vbox/vbox_storage.c   |  2 +
 src/vbox/vbox_tmpl.c  | 87 ++-
 src/vbox/vbox_uniformed_api.h |  1 +
 6 files changed, 106 insertions(+), 1 deletion(-)
 create mode 100644 src/vbox/vbox_V6_1.c

diff --git a/src/vbox/Makefile.inc.am b/src/vbox/Makefile.inc.am
index fdc6537d51..c5c6d538e7 100644
--- a/src/vbox/Makefile.inc.am
+++ b/src/vbox/Makefile.inc.am
@@ -9,6 +9,8 @@ VBOX_DRIVER_SOURCES = \
vbox/vbox_CAPI_v5_2.h \
vbox/vbox_CAPI_v6_0.h \
vbox/vbox_V6_0.c \
+   vbox/vbox_CAPI_v6_1.h \
+   vbox/vbox_V6_1.c \
vbox/vbox_common.c \
vbox/vbox_common.h \
vbox/vbox_uniformed_api.h \
diff --git a/src/vbox/vbox_V6_1.c b/src/vbox/vbox_V6_1.c
new file mode 100644
index 00..aa4017043b
--- /dev/null
+++ b/src/vbox/vbox_V6_1.c
@@ -0,0 +1,13 @@
+/** @file vbox_V6_1.c
+ * C file to include support for multiple versions of VirtualBox
+ * at runtime.
+ */
+
+#include 
+
+/** The API Version */
+#define VBOX_API_VERSION 6001000
+/** Version specific prefix. */
+#define NAME(name) vbox61##name
+
+#include "vbox_tmpl.c"
diff --git a/src/vbox/vbox_common.h b/src/vbox/vbox_common.h
index 7c29d92789..6144714477 100644
--- a/src/vbox/vbox_common.h
+++ b/src/vbox/vbox_common.h
@@ -433,6 +433,8 @@ typedef nsISupports IKeyboard;
 vbox52InstallUniformedAPI(); \
 } else if (uVersion >= 600 && uVersion < 651) { \
 vbox60InstallUniformedAPI(); \
+} else if (uVersion >= 651 && uVersion < 6001051) { \
+vbox61InstallUniformedAPI(); \
 } else { \
 result = -1; \
 } \
diff --git a/src/vbox/vbox_storage.c b/src/vbox/vbox_storage.c
index 83172ee1fe..ee491fa596 100644
--- a/src/vbox/vbox_storage.c
+++ b/src/vbox/vbox_storage.c
@@ -884,6 +884,8 @@ virStorageDriverPtr vboxGetStorageDriver(uint32_t uVersion)
 vbox52InstallUniformedAPI();
 } else if (uVersion >= 600 && uVersion < 651) {
 vbox60InstallUniformedAPI();
+} else if (uVersion >= 651 && uVersion < 6001051) {
+vbox61InstallUniformedAPI();
 } else {
 return NULL;
 }
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 9ac7f88b0f..a1a462cc74 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -53,6 +53,8 @@
 # include "vbox_CAPI_v5_2.h"
 #elif VBOX_API_VERSION == 600
 # include "vbox_CAPI_v6_0.h"
+#elif VBOX_API_VERSION == 6001000
+# include "vbox_CAPI_v6_1.h"
 #else
 # error "Unsupported VBOX_API_VERSION"
 #endif
@@ -753,8 +755,14 @@ _machineLaunchVMProcess(vboxDriverPtr data,
 PRUnichar *sessionType, PRUnichar *env,
 IProgress **progress)
 {
+#if VBOX_API_VERSION >= 6001000
+PRUnichar *envlist[] = { env };
+return machine->vtbl->LaunchVMProcess(machine, data->vboxSession,
+  sessionType, 1, envlist, progress);
+#else
 return machine->vtbl->LaunchVMProcess(machine, data->vboxSession,
   sessionType, env, progress);
+#endif
 }
 
 static nsresult
@@ -914,51 +922,123 @@ _machineSetBootOrder(IMachine *machine, PRUint32 
position, PRUint32 device)
 static nsresult
 _machineGetVRAMSize(IMachine *machine, PRUint32 *VRAMSize)
 {
+#if VBOX_API_VERSION >= 6001000
+IGraphicsAdapter *ga;
+nsresult ret;
+ret = machine->vtbl->GetGraphicsAdapter(machine, );
+if (NS_FAILED(ret))
+return ret;
+return ga->vtbl->GetVRAMSize(ga, VRAMSize);
+#else
 return machine->vtbl->GetVRAMSize(machine, VRAMSize);
+#endif
 }
 
 static nsresult
 _machineSetVRAMSize(IMachine *machine, PRUint32 VRAMSize)
 {
+#if VBOX_API_VERSION >= 6001000
+IGraphicsAdapter *ga;
+nsresult ret;
+ret = machine->vtbl->GetGraphicsAdapter(machine, );
+if (NS_FAILED(ret))
+return ret;
+return ga->vtbl->SetVRAMSize(ga, VRAMSize);
+#else
 return machine->vtbl->SetVRAMSize(machine, VRAMSize);
+#endif
 }
 
 static nsresult
 _machineGetMonitorCount(IMachine *machine, PRUint32 *monitorCount)
 {
+#if VBOX_API_VERSION >= 6001000
+IGraphicsAdapter *ga;
+nsresult ret;
+ret = machine->vtbl->GetGraphicsAdapter(machine, );
+if (NS_FAILED(ret))
+return ret;
+return ga->vtbl->GetMonitorCount(ga, monitorCount);
+#else
 return machine->vtbl->GetMonitorCount(machine, 

[libvirt PATCH 03/10] vbox: remove support for version 5.1 API

[Daniel P . Berrangé]

This is no longer supported since 2018/04

Signed-off-by: Daniel P. Berrangé 
---
 src/vbox/Makefile.inc.am  |  2 --
 src/vbox/vbox_V5_1.c  | 13 -
 src/vbox/vbox_XPCOMCGlue.h|  2 +-
 src/vbox/vbox_common.h|  4 +---
 src/vbox/vbox_storage.c   |  4 +---
 src/vbox/vbox_tmpl.c  |  4 +---
 src/vbox/vbox_uniformed_api.h |  1 -
 7 files changed, 4 insertions(+), 26 deletions(-)
 delete mode 100644 src/vbox/vbox_V5_1.c

diff --git a/src/vbox/Makefile.inc.am b/src/vbox/Makefile.inc.am
index c05fa7a7b9..3b4c6b8f98 100644
--- a/src/vbox/Makefile.inc.am
+++ b/src/vbox/Makefile.inc.am
@@ -7,8 +7,6 @@ VBOX_DRIVER_SOURCES = \
vbox/vbox_driver.h \
vbox/vbox_snapshot_conf.c \
vbox/vbox_snapshot_conf.h \
-   vbox/vbox_V5_1.c \
-   vbox/vbox_CAPI_v5_1.h \
vbox/vbox_V5_2.c \
vbox/vbox_CAPI_v5_2.h \
vbox/vbox_common.c \
diff --git a/src/vbox/vbox_V5_1.c b/src/vbox/vbox_V5_1.c
deleted file mode 100644
index 839bceddb3..00
--- a/src/vbox/vbox_V5_1.c
+++ /dev/null
@@ -1,13 +0,0 @@
-/** @file vbox_V5_1.c
- * C file to include support for multiple versions of VirtualBox
- * at runtime.
- */
-
-#include 
-
-/** The API Version */
-#define VBOX_API_VERSION 5001000
-/** Version specific prefix. */
-#define NAME(name) vbox51##name
-
-#include "vbox_tmpl.c"
diff --git a/src/vbox/vbox_XPCOMCGlue.h b/src/vbox/vbox_XPCOMCGlue.h
index 0f3fc41512..517b02393f 100644
--- a/src/vbox/vbox_XPCOMCGlue.h
+++ b/src/vbox/vbox_XPCOMCGlue.h
@@ -30,7 +30,7 @@
 # define ___VBoxXPCOMC_cglue_h
 
 /* This has to be the oldest version we support. */
-# include "vbox_CAPI_v5_1.h"
+# include "vbox_CAPI_v5_2.h"
 
 /** Pointer to VBoxGetXPCOMCFunctions for the loaded VBoxXPCOMC so/dylib/dll. 
*/
 extern PFNVBOXGETXPCOMCFUNCTIONS g_pfnGetFunctions;
diff --git a/src/vbox/vbox_common.h b/src/vbox/vbox_common.h
index 0a699e3277..9ea984276d 100644
--- a/src/vbox/vbox_common.h
+++ b/src/vbox/vbox_common.h
@@ -429,9 +429,7 @@ typedef nsISupports IKeyboard;
 #define installUniformedAPI(gVBoxAPI, result) \
 do { \
 result = 0; \
-if (uVersion >= 551 && uVersion < 5001051) { \
-vbox51InstallUniformedAPI(); \
-} else if (uVersion >= 5001051 && uVersion < 5002051) { \
+if (uVersion >= 5001051 && uVersion < 5002051) { \
 vbox52InstallUniformedAPI(); \
 } else { \
 result = -1; \
diff --git a/src/vbox/vbox_storage.c b/src/vbox/vbox_storage.c
index 730c8c5a05..e0b82f4bd8 100644
--- a/src/vbox/vbox_storage.c
+++ b/src/vbox/vbox_storage.c
@@ -880,9 +880,7 @@ virStorageDriverPtr vboxGetStorageDriver(uint32_t uVersion)
 /* Install gVBoxAPI according to the vbox API version.
  * Return -1 for unsupported version.
  */
-if (uVersion >= 551 && uVersion < 5001051) {
-vbox51InstallUniformedAPI();
-} else if (uVersion >= 5001051 && uVersion < 5002051) {
+if (uVersion >= 5001051 && uVersion < 5002051) {
 vbox52InstallUniformedAPI();
 } else {
 return NULL;
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 1ceb8b487a..94fdf211d9 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -49,9 +49,7 @@
 #include "virutil.h"
 
 /* This one changes from version to version. */
-#if VBOX_API_VERSION == 5001000
-# include "vbox_CAPI_v5_1.h"
-#elif VBOX_API_VERSION == 5002000
+#if VBOX_API_VERSION == 5002000
 # include "vbox_CAPI_v5_2.h"
 #else
 # error "Unsupport VBOX_API_VERSION"
diff --git a/src/vbox/vbox_uniformed_api.h b/src/vbox/vbox_uniformed_api.h
index 2a592e0cb4..8e2f217439 100644
--- a/src/vbox/vbox_uniformed_api.h
+++ b/src/vbox/vbox_uniformed_api.h
@@ -555,5 +555,4 @@ virDomainPtr vboxDomainLookupByUUID(virConnectPtr conn,
 const unsigned char *uuid);
 
 /* Version specified functions for installing uniformed API */
-void vbox51InstallUniformedAPI(vboxUniformedAPI *pVBoxAPI);
 void vbox52InstallUniformedAPI(vboxUniformedAPI *pVBoxAPI);
-- 
2.25.2

[libvirt PATCH 05/10] vbox: remove unused support for Windows MSCOM

[Daniel P . Berrangé]

Long ago we switched the vbox driver to run inside libvirtd to avoid
libvirt.so being polluted with GPLv2-only code. Since libvirtd is not
built on Windows, we disabled vbox on Windows builds. Thus the MSCOM
glue code is not required.

Signed-off-by: Daniel P. Berrangé 
---
 po/POTFILES.in|   1 -
 src/vbox/Makefile.inc.am  |   8 +-
 src/vbox/vbox_MSCOMGlue.c | 779 --
 src/vbox/vbox_MSCOMGlue.h |  45 --
 src/vbox/vbox_driver.c|   2 +-
 src/vbox/vbox_glue.c  |  28 --
 src/vbox/vbox_glue.h  |  28 --
 src/vbox/vbox_tmpl.c  |   2 +-
 src/vbox/vbox_uniformed_api.h |   2 +-
 9 files changed, 5 insertions(+), 890 deletions(-)
 delete mode 100644 src/vbox/vbox_MSCOMGlue.c
 delete mode 100644 src/vbox/vbox_MSCOMGlue.h
 delete mode 100644 src/vbox/vbox_glue.c
 delete mode 100644 src/vbox/vbox_glue.h

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 0a3efca6d7..6607e298f2 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -315,7 +315,6 @@
 @SRCDIR@/src/util/virvhba.c
 @SRCDIR@/src/util/virvsock.c
 @SRCDIR@/src/util/virxml.c
-@SRCDIR@/src/vbox/vbox_MSCOMGlue.c
 @SRCDIR@/src/vbox/vbox_XPCOMCGlue.c
 @SRCDIR@/src/vbox/vbox_common.c
 @SRCDIR@/src/vbox/vbox_driver.c
diff --git a/src/vbox/Makefile.inc.am b/src/vbox/Makefile.inc.am
index 3b4c6b8f98..9d827eff97 100644
--- a/src/vbox/Makefile.inc.am
+++ b/src/vbox/Makefile.inc.am
@@ -1,8 +1,6 @@
 # vim: filetype=automake
 
 VBOX_DRIVER_SOURCES = \
-   vbox/vbox_glue.c \
-   vbox/vbox_glue.h \
vbox/vbox_driver.c \
vbox/vbox_driver.h \
vbox/vbox_snapshot_conf.c \
@@ -15,15 +13,13 @@ VBOX_DRIVER_SOURCES = \
vbox/vbox_get_driver.h \
vbox/vbox_network.c \
vbox/vbox_storage.c \
+   vbox/vbox_XPCOMCGlue.c \
+   vbox/vbox_XPCOMCGlue.h \
$(NULL)
 
 VBOX_DRIVER_EXTRA_DIST = \
vbox/vbox_tmpl.c \
vbox/README \
-   vbox/vbox_MSCOMGlue.c \
-   vbox/vbox_MSCOMGlue.h \
-   vbox/vbox_XPCOMCGlue.c \
-   vbox/vbox_XPCOMCGlue.h \
$(NULL)
 
 DRIVER_SOURCE_FILES += \
diff --git a/src/vbox/vbox_MSCOMGlue.c b/src/vbox/vbox_MSCOMGlue.c
deleted file mode 100644
index 18dbb0ffe1..00
--- a/src/vbox/vbox_MSCOMGlue.c
+++ /dev/null
@@ -1,779 +0,0 @@
-/*
- * vbox_MSCOMGlue.c: glue to the MSCOM based VirtualBox API
- *
- * Copyright (C) 2013 Red Hat, Inc.
- * Copyright (C) 2010-2011 Matthias Bolte 
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library.  If not, see
- * .
- *
- */
-
-#include 
-
-#define nsCID CLSID
-
-#include "internal.h"
-#include "viralloc.h"
-#include "virlog.h"
-#include "virerror.h"
-#include "virstring.h"
-#include "virutil.h"
-#include "virsocket.h"
-#include "vbox_MSCOMGlue.h"
-
-#define VIR_FROM_THIS VIR_FROM_VBOX
-
-VIR_LOG_INIT("vbox.vbox_MSCOMGlue");
-
-#define VBOX_REGKEY_ORACLE "Software\\Oracle\\VirtualBox"
-#define VBOX_REGKEY_SUN "Software\\Sun\\xVM VirtualBox"
-
-#define IVIRTUALBOX_IID_STR_v2_2 "779264f4-65ed-48ed-be39-518ca549e296"
-#define ISESSION_IID_STR_v2_2 "12F4DCDB-12B2-4ec1-B7CD-DDD9F6C5BF4D"
-
-
-
-typedef struct _VBOXXPCOMC_v1 VBOXXPCOMC_v1;
-typedef struct _VBOXXPCOMC_v2 VBOXXPCOMC_v2;
-
-struct _VBOXXPCOMC_v1 {
-unsigned cb;
-unsigned uVersion;
-unsigned int (*pfnGetVersion)(void);
-void (*pfnComInitialize)(IVirtualBox **virtualBox, ISession **session);
-void (*pfnComUninitialize)(void);
-void (*pfnComUnallocMem)(void *pv);
-void (*pfnUtf16Free)(PRUnichar *pwszString);
-void (*pfnUtf8Free)(char *pszString);
-int (*pfnUtf16ToUtf8)(const PRUnichar *pwszString, char **ppszString);
-int (*pfnUtf8ToUtf16)(const char *pszString, PRUnichar **ppwszString);
-unsigned uEndVersion;
-};
-
-struct _VBOXXPCOMC_v2 {
-unsigned cb;
-unsigned uVersion;
-unsigned int (*pfnGetVersion)(void);
-void (*pfnComInitialize)(const char *pszVirtualBoxIID,
- IVirtualBox **ppVirtualBox,
- const char *pszSessionIID,
- ISession **ppSession);
-void (*pfnComUninitialize)(void);
-void (*pfnComUnallocMem)(void *pv);
-void (*pfnUtf16Free)(PRUnichar *pwszString);
-void (*pfnUtf8Free)(char *pszString);
-int (*pfnUtf16ToUtf8)(const PRUnichar *pwszString, char **ppszString);
-int (*pfnUtf8ToUtf16)(const char *pszString, 

[libvirt PATCH 00/10] vbox: update to support newer versions and drop eol versions

[Daniel P . Berrangé]

This is the bare minimum patches required to let libvirt compile with
the latest virtualbox SDK for 6.0 and 6.1. Since I do not use
virtualbox this is compile tested only. This will at least give users
something to try and provide feedback on. In addition currently EOL
versions of virtualbox are dropped.

Daniel P. Berrangé (10):
  vbox: remove support for version 5.0 API
  vbox: remove version 5.0 CAPI header
  vbox: remove support for version 5.1 API
  vbox: remove version 5.1 CAPI header
  vbox: remove unused support for Windows MSCOM
  vbox: add version 6.0 CAPI header
  vbox: add support for version 6.0 SDK
  vbox: add version 6.1 CAPI header
  vbox: add support for version 6.1 SDK
  docs: add news about virtualbox version support changes

 docs/news.xml |22 +
 po/POTFILES.in| 1 -
 src/vbox/Makefile.inc.am  |16 +-
 .../{vbox_CAPI_v5_1.h => vbox_CAPI_v6_0.h}| 14959 +++---
 .../{vbox_CAPI_v5_0.h => vbox_CAPI_v6_1.h}| 37642 +---
 src/vbox/vbox_MSCOMGlue.c |   779 -
 src/vbox/vbox_MSCOMGlue.h |45 -
 src/vbox/{vbox_V5_0.c => vbox_V6_0.c} | 6 +-
 src/vbox/{vbox_V5_1.c => vbox_V6_1.c} | 6 +-
 src/vbox/vbox_XPCOMCGlue.h| 2 +-
 src/vbox/vbox_common.h|10 +-
 src/vbox/vbox_driver.c| 2 +-
 src/vbox/vbox_glue.c  |28 -
 src/vbox/vbox_glue.h  |28 -
 src/vbox/vbox_storage.c   |10 +-
 src/vbox/vbox_tmpl.c  |   104 +-
 src/vbox/vbox_uniformed_api.h | 6 +-
 17 files changed, 31851 insertions(+), 21815 deletions(-)
 rename src/vbox/{vbox_CAPI_v5_1.h => vbox_CAPI_v6_0.h} (85%)
 rename src/vbox/{vbox_CAPI_v5_0.h => vbox_CAPI_v6_1.h} (73%)
 delete mode 100644 src/vbox/vbox_MSCOMGlue.c
 delete mode 100644 src/vbox/vbox_MSCOMGlue.h
 rename src/vbox/{vbox_V5_0.c => vbox_V6_0.c} (68%)
 rename src/vbox/{vbox_V5_1.c => vbox_V6_1.c} (68%)
 delete mode 100644 src/vbox/vbox_glue.c
 delete mode 100644 src/vbox/vbox_glue.h

-- 
2.25.2

[libvirt PATCH 01/10] vbox: remove support for version 5.0 API

[Daniel P . Berrangé]

This is no longer supported since 2017/05

Signed-off-by: Daniel P. Berrangé 
---
 src/vbox/Makefile.inc.am  |  2 --
 src/vbox/vbox_V5_0.c  | 13 -
 src/vbox/vbox_XPCOMCGlue.h|  2 +-
 src/vbox/vbox_common.h|  4 +---
 src/vbox/vbox_storage.c   |  4 +---
 src/vbox/vbox_tmpl.c  |  4 +---
 src/vbox/vbox_uniformed_api.h |  1 -
 7 files changed, 4 insertions(+), 26 deletions(-)
 delete mode 100644 src/vbox/vbox_V5_0.c

diff --git a/src/vbox/Makefile.inc.am b/src/vbox/Makefile.inc.am
index 8d2467f39e..c05fa7a7b9 100644
--- a/src/vbox/Makefile.inc.am
+++ b/src/vbox/Makefile.inc.am
@@ -7,8 +7,6 @@ VBOX_DRIVER_SOURCES = \
vbox/vbox_driver.h \
vbox/vbox_snapshot_conf.c \
vbox/vbox_snapshot_conf.h \
-   vbox/vbox_V5_0.c \
-   vbox/vbox_CAPI_v5_0.h \
vbox/vbox_V5_1.c \
vbox/vbox_CAPI_v5_1.h \
vbox/vbox_V5_2.c \
diff --git a/src/vbox/vbox_V5_0.c b/src/vbox/vbox_V5_0.c
deleted file mode 100644
index 4293944724..00
--- a/src/vbox/vbox_V5_0.c
+++ /dev/null
@@ -1,13 +0,0 @@
-/** @file vbox_V5_0.c
- * C file to include support for multiple versions of VirtualBox
- * at runtime.
- */
-
-#include 
-
-/** The API Version */
-#define VBOX_API_VERSION 500
-/** Version specific prefix. */
-#define NAME(name) vbox50##name
-
-#include "vbox_tmpl.c"
diff --git a/src/vbox/vbox_XPCOMCGlue.h b/src/vbox/vbox_XPCOMCGlue.h
index df4c94cbbf..0f3fc41512 100644
--- a/src/vbox/vbox_XPCOMCGlue.h
+++ b/src/vbox/vbox_XPCOMCGlue.h
@@ -30,7 +30,7 @@
 # define ___VBoxXPCOMC_cglue_h
 
 /* This has to be the oldest version we support. */
-# include "vbox_CAPI_v5_0.h"
+# include "vbox_CAPI_v5_1.h"
 
 /** Pointer to VBoxGetXPCOMCFunctions for the loaded VBoxXPCOMC so/dylib/dll. 
*/
 extern PFNVBOXGETXPCOMCFUNCTIONS g_pfnGetFunctions;
diff --git a/src/vbox/vbox_common.h b/src/vbox/vbox_common.h
index f3294334d8..0a699e3277 100644
--- a/src/vbox/vbox_common.h
+++ b/src/vbox/vbox_common.h
@@ -429,9 +429,7 @@ typedef nsISupports IKeyboard;
 #define installUniformedAPI(gVBoxAPI, result) \
 do { \
 result = 0; \
-if (uVersion >= 500 && uVersion < 551) { \
-vbox50InstallUniformedAPI(); \
-} else if (uVersion >= 551 && uVersion < 5001051) { \
+if (uVersion >= 551 && uVersion < 5001051) { \
 vbox51InstallUniformedAPI(); \
 } else if (uVersion >= 5001051 && uVersion < 5002051) { \
 vbox52InstallUniformedAPI(); \
diff --git a/src/vbox/vbox_storage.c b/src/vbox/vbox_storage.c
index b9dcacc62e..730c8c5a05 100644
--- a/src/vbox/vbox_storage.c
+++ b/src/vbox/vbox_storage.c
@@ -880,9 +880,7 @@ virStorageDriverPtr vboxGetStorageDriver(uint32_t uVersion)
 /* Install gVBoxAPI according to the vbox API version.
  * Return -1 for unsupported version.
  */
-if (uVersion >= 500 && uVersion < 551) {
-vbox50InstallUniformedAPI();
-} else if (uVersion >= 551 && uVersion < 5001051) {
+if (uVersion >= 551 && uVersion < 5001051) {
 vbox51InstallUniformedAPI();
 } else if (uVersion >= 5001051 && uVersion < 5002051) {
 vbox52InstallUniformedAPI();
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index a2d3db4307..1ceb8b487a 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -49,9 +49,7 @@
 #include "virutil.h"
 
 /* This one changes from version to version. */
-#if VBOX_API_VERSION == 500
-# include "vbox_CAPI_v5_0.h"
-#elif VBOX_API_VERSION == 5001000
+#if VBOX_API_VERSION == 5001000
 # include "vbox_CAPI_v5_1.h"
 #elif VBOX_API_VERSION == 5002000
 # include "vbox_CAPI_v5_2.h"
diff --git a/src/vbox/vbox_uniformed_api.h b/src/vbox/vbox_uniformed_api.h
index ca307e30b1..2a592e0cb4 100644
--- a/src/vbox/vbox_uniformed_api.h
+++ b/src/vbox/vbox_uniformed_api.h
@@ -555,6 +555,5 @@ virDomainPtr vboxDomainLookupByUUID(virConnectPtr conn,
 const unsigned char *uuid);
 
 /* Version specified functions for installing uniformed API */
-void vbox50InstallUniformedAPI(vboxUniformedAPI *pVBoxAPI);
 void vbox51InstallUniformedAPI(vboxUniformedAPI *pVBoxAPI);
 void vbox52InstallUniformedAPI(vboxUniformedAPI *pVBoxAPI);
-- 
2.25.2

Re: [PATCH 2/2] docs: Improve zPCI section in pci-addresses.rst

[Cornelia Huck]

On Fri, 17 Apr 2020 13:29:43 +0200
Boris Fiuczynski  wrote:

> On 4/17/20 11:02 AM, Cornelia Huck wrote:
> > On Fri, 17 Apr 2020 10:50:02 +0200
> > Boris Fiuczynski  wrote:
> >   
> >> On 4/16/20 6:14 PM, Cornelia Huck wrote:  
> >>> On Thu, 16 Apr 2020 17:56:18 +0200
> >>> Boris Fiuczynski  wrote:
> >>>  
>  Improving the zPCI example by choosing more distinct values and
>  adding explanation for fid.
> 
>  Signed-off-by: Boris Fiuczynski 
>  ---
> docs/pci-addresses.rst | 15 ---
> 1 file changed, 8 insertions(+), 7 deletions(-)
> 
>  diff --git a/docs/pci-addresses.rst b/docs/pci-addresses.rst
>  index 7c8e9edd73..4492389da5 100644
>  --- a/docs/pci-addresses.rst
>  +++ b/docs/pci-addresses.rst
>  @@ -176,14 +176,14 @@ In the simplest case, the following XML snippet
> 
> 
>   function='0x0'>
>  -  
>  +
> >>>
> >>> Why this change? The pci-bridge does not show up in the guest anyway.  
> >> My assumption was that uid and fid for this would be autogenerated.
> >> Since uid 0x0001 and fid 0x have been freed up due to the change
> >> below this would be the autogenerated set.  
> > 
> > If that makes the XML look saner, no objection.
> >   
> >>  
> >>>  
> 
>   
>   
> 
> 
>   function='0x0'>
>  -  
>  +  
> 
>   
> 
>  @@ -191,21 +191,22 @@ will result in the following in a Linux guest:
> 
> ::
> 
>  -  0001:00:00.0 Ethernet controller: Red Hat, Inc. Virtio network device
>  +  0007:00:00.0 Ethernet controller: Red Hat, Inc. Virtio network device
> 
> Note that the PCI bridge is not visible in the guest; s390x always 
>  has a flat
>  -topology.
>  +topology. Also ``fid`` does not define slot or function of the PCI 
>  address.  
> >>>
> >>> I find the sentence regarding 'fid' confusing. Maybe instead move up
> >>> the explanation from below regarding uid and fid?
> >>>
> >>> "The PCI address in the guest is generated from..."
> >>>  
> >> Lets join your proposal with Andreas and move his rewrite up to here.
> >> Like:
> >> ...topology.
> >> The PCI address in the guest is generated from the information provided
> >> via the ``zpci`` element: more specifically, ``uid`` is used as the PCI
> >> domain.``fid`` doesn't appear in the PCI address itself, but it will be
> >> used in sysfs (``/sys/bus/pci/slots/$fid/...``).  
> > 
> > Sounds good.
> > 
> > (Also the rest of the changes.)
> >   
> Before I break the r-b chain as well... Is that your r-b? :)
> 
> 

I'll add my R-b to a final patch :)

Re: [PATCH 2/2] docs: Improve zPCI section in pci-addresses.rst

[Boris Fiuczynski]

On 4/17/20 11:02 AM, Cornelia Huck wrote:

On Fri, 17 Apr 2020 10:50:02 +0200
Boris Fiuczynski  wrote:


On 4/16/20 6:14 PM, Cornelia Huck wrote:

On Thu, 16 Apr 2020 17:56:18 +0200
Boris Fiuczynski  wrote:
   

Improving the zPCI example by choosing more distinct values and
adding explanation for fid.

Signed-off-by: Boris Fiuczynski 
---
   docs/pci-addresses.rst | 15 ---
   1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/docs/pci-addresses.rst b/docs/pci-addresses.rst
index 7c8e9edd73..4492389da5 100644
--- a/docs/pci-addresses.rst
+++ b/docs/pci-addresses.rst
@@ -176,14 +176,14 @@ In the simplest case, the following XML snippet
   
   
   
-  
+  


Why this change? The pci-bridge does not show up in the guest anyway.

My assumption was that uid and fid for this would be autogenerated.
Since uid 0x0001 and fid 0x have been freed up due to the change
below this would be the autogenerated set.


If that makes the XML look saner, no objection.



   

   
 
 
   
   
   
-  
+  
   
 
   
@@ -191,21 +191,22 @@ will result in the following in a Linux guest:
   
   ::
   
-  0001:00:00.0 Ethernet controller: Red Hat, Inc. Virtio network device

+  0007:00:00.0 Ethernet controller: Red Hat, Inc. Virtio network device
   
   Note that the PCI bridge is not visible in the guest; s390x always has a flat

-topology.
+topology. Also ``fid`` does not define slot or function of the PCI address.


I find the sentence regarding 'fid' confusing. Maybe instead move up
the explanation from below regarding uid and fid?

"The PCI address in the guest is generated from..."
   

Lets join your proposal with Andreas and move his rewrite up to here.
Like:
...topology.
The PCI address in the guest is generated from the information provided
via the ``zpci`` element: more specifically, ``uid`` is used as the PCI
domain.``fid`` doesn't appear in the PCI address itself, but it will be
used in sysfs (``/sys/bus/pci/slots/$fid/...``).


Sounds good.

(Also the rest of the changes.)


Before I break the r-b chain as well... Is that your r-b? :)


--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

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

Re: [PATCH v5 0/4] introduction of migration_version attribute for VFIO live migration

[Cornelia Huck]

On Fri, 17 Apr 2020 05:52:02 -0400
Yan Zhao  wrote:

> On Fri, Apr 17, 2020 at 04:44:50PM +0800, Cornelia Huck wrote:
> > On Mon, 13 Apr 2020 01:52:01 -0400
> > Yan Zhao  wrote:
> >   
> > > This patchset introduces a migration_version attribute under sysfs of VFIO
> > > Mediated devices.
> > > 
> > > This migration_version attribute is used to check migration compatibility
> > > between two mdev devices.
> > > 
> > > Currently, it has two locations:
> > > (1) under mdev_type node,
> > > which can be used even before device creation, but only for mdev
> > > devices of the same mdev type.
> > > (2) under mdev device node,
> > > which can only be used after the mdev devices are created, but the src
> > > and target mdev devices are not necessarily be of the same mdev type
> > > (The second location is newly added in v5, in order to keep consistent
> > > with the migration_version node for migratable pass-though devices)  
> > 
> > What is the relationship between those two attributes?
> >   
> (1) is for mdev devices specifically, and (2) is provided to keep the same
> sysfs interface as with non-mdev cases. so (2) is for both mdev devices and
> non-mdev devices.
> 
> in future, if we enable vfio-pci vendor ops, (i.e. a non-mdev device
> is binding to vfio-pci, but is able to register migration region and do
> migration transactions from a vendor provided affiliate driver),
> the vendor driver would export (2) directly, under device node.
> It is not able to provide (1) as there're no mdev devices involved.

Ok, creating an alternate attribute for non-mdev devices makes sense.
However, wouldn't that rather be a case (3)? The change here only
refers to mdev devices.

> 
> > Is existence (and compatibility) of (1) a pre-req for possible
> > existence (and compatibility) of (2)?
> >  
> no. (2) does not reply on (1).

Hm. Non-existence of (1) seems to imply "this type does not support
migration". If an mdev created for such a type suddenly does support
migration, it feels a bit odd.

(It obviously cannot be a prereq for what I called (3) above.)

> 
> > Does userspace need to check (1) or can it completely rely on (2), if
> > it so chooses?
> >  
> I think it can completely reply on (2) if compatibility check before
> mdev creation is not required.
> 
> > If devices with a different mdev type are indeed compatible, it seems
> > userspace can only find out after the devices have actually been
> > created, as (1) does not apply?  
> yes, I think so. 

How useful would it be for userspace to even look at (1) in that case?
It only knows if things have a chance of working if it actually goes
ahead and creates devices.

> 
> > One of my worries is that the existence of an attribute with the same
> > name in two similar locations might lead to confusion. But maybe it
> > isn't a problem.
> >  
> Yes, I have the same feeling. but as (2) is for sysfs interface
> consistency, to make it transparent to userspace tools like libvirt,
> I guess the same name is necessary?

What do we actually need here, I wonder? (1) and (2) seem to serve
slightly different purposes, while (2) and what I called (3) have the
same purpose. Is it important to userspace that (1) and (2) have the
same name?

Re: [PATCH 3/3] qemu: Label restore path outside of secdriver transactions

[Erik Skultety]

On Fri, Apr 03, 2020 at 05:58:03PM +0200, Michal Privoznik wrote:
> As explained in the previous commit, we need to relabel the file
> we are restoring the domain from. That is the FD that is passed
> to QEMU. If the file is not under /dev then the file inside the
> namespace is the very same as the one in the host. And regardless
> of using transactions, the file will be relabeled. But, if the
> file is under /dev then when using transactions only the copy
> inside the namespace is relabeled and the one in the host is not.
> But QEMU is reading from the one in the host, actually.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1772838
>
> Signed-off-by: Michal Privoznik 
> ---
...

>
>  /*
>   * virSecuritySELinuxSetFileLabels:
> @@ -3596,6 +3606,7 @@ virSecurityDriver virSecurityDriverSELinux = {
>  .getBaseLabel   = virSecuritySELinuxGetBaseLabel,
>
>  .domainSetPathLabel = 
> virSecuritySELinuxDomainSetPathLabel,
> +.domainSetIncomingPathLabel = 
> virSecuritySELinuxDomainSetIncomingPathLabel,

"HostPath" would IMO feel better than "IncomingPath" in this patch as well.

Reviewed-by: Erik Skultety 

Re: [PATCH 2/3] security: Introduce virSecurityManagerDomainSetIncomingPathLabel

[Erik Skultety]

On Fri, Apr 03, 2020 at 05:58:02PM +0200, Michal Privoznik wrote:
> This API allows drivers to separate out handling of @stdin_path
> of virSecurityManagerSetAllLabel(). The thing is, the QEMU driver
> uses transactions for virSecurityManagerSetAllLabel() which
> relabels devices from inside of domain's namespace. This is what
> we usually want. Except when resuming domain from a file. The
> file is opened before any namespace is set up and the FD is
> passed to QEMU to read the migration stream from. Because of
> this, the file lives outside of the namespace and if it so
> happens that the file is a block device (i.e. it lives under
> /dev) its copy will be created in the namespace. But the FD that
> is passed to QEMU points to the original living in the host and
> not in the namespace. So relabeling the file inside the namespace
> helps nothing.
>
> But if we have a separate API for relabeling the restore file
> then the QEMU driver can continue calling
> virSecurityManagerSetAllLabel() with transactions enabled and
> call this new API without transactions.
>
> We already have an API for relabeling a single file
> (virSecurityManagerDomainSetPathLabel()) but in case of SELinux
> it uses @imagelabel (which allows RW access) and we want to use
> @content_context (which allows RO access).

Since this patch is only adding a generic driver API rather than a specific
security backend API, I'd move ^this last paragraph to the next patch, I
actually appreciated the info there much more than here during the review.

>
> Signed-off-by: Michal Privoznik 
> ---
>  src/libvirt_private.syms|  1 +
>  src/security/security_driver.h  |  4 
>  src/security/security_manager.c | 29 +
>  src/security/security_manager.h |  4 
>  src/security/security_stack.c   | 21 +
>  5 files changed, 59 insertions(+)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index e276f55bb1..2c63f37fc2 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1523,6 +1523,7 @@ virSecurityDriverLookup;
>  # security/security_manager.h
>  virSecurityManagerCheckAllLabel;
>  virSecurityManagerClearSocketLabel;
> +virSecurityManagerDomainSetIncomingPathLabel;

IncomingPath sounds a bit confusing to me, I'd go with something like HostPath
instead, in patch 3/3 you also have an explicit commentary why we're using the
respective backend API for this rather than simply using SetPathLabel.

>  virSecurityManagerDomainSetPathLabel;
>  virSecurityManagerGenLabel;
>  virSecurityManagerGetBaseLabel;
> diff --git a/src/security/security_driver.h b/src/security/security_driver.h
> index 3353955813..6cbe8613c9 100644
> --- a/src/security/security_driver.h
> +++ b/src/security/security_driver.h
> @@ -140,6 +140,9 @@ typedef int (*virSecurityDomainSetPathLabel) 
> (virSecurityManagerPtr mgr,
>virDomainDefPtr def,
>const char *path,
>bool allowSubtree);
> +typedef int (*virSecurityDomainSetIncomingPathLabel) (virSecurityManagerPtr 
> mgr,
> +  virDomainDefPtr def,
> +  const char *path);
>  typedef int (*virSecurityDomainSetChardevLabel) (virSecurityManagerPtr mgr,
>   virDomainDefPtr def,
>   virDomainChrSourceDefPtr 
> dev_source,
> @@ -211,6 +214,7 @@ struct _virSecurityDriver {
>  virSecurityDriverGetBaseLabel getBaseLabel;
>
>  virSecurityDomainSetPathLabel domainSetPathLabel;
> +virSecurityDomainSetIncomingPathLabel domainSetIncomingPathLabel;
>
>  virSecurityDomainSetChardevLabel domainSetSecurityChardevLabel;
>  virSecurityDomainRestoreChardevLabel domainRestoreSecurityChardevLabel;
> diff --git a/src/security/security_manager.c b/src/security/security_manager.c
> index fe032746ff..a76b953ee4 100644
> --- a/src/security/security_manager.c
> +++ b/src/security/security_manager.c
> @@ -1077,6 +1077,35 @@ 
> virSecurityManagerDomainSetPathLabel(virSecurityManagerPtr mgr,
>  }
>
>
> +/**
> + * virSecurityManagerDomainSetIncomingPathLabel:
> + * @mgr: security manager object
> + * @vm: domain definition object
> + * @path: path to label
> + *
> + * This function relabels given @path so that @vm can restore for

maybe add "host" @path here to make it even clearer.

> + * it.  This allows the driver backend to use different label than
> + * virSecurityManagerDomainSetPathLabel().
> + *
> + * Returns: 0 on success, -1 on error.
> + */
> +int
> +virSecurityManagerDomainSetIncomingPathLabel(virSecurityManagerPtr mgr,
> + virDomainDefPtr vm,
> + const char *path)
> +{
> +if (mgr->drv->domainSetIncomingPathLabel) {
> 

Re: [PATCH 1/3] selinux: Don't remember label for restore path

[Erik Skultety]

On Fri, Apr 03, 2020 at 05:58:01PM +0200, Michal Privoznik wrote:
> The seclabel for @stdin_path in virSecuritySELinuxSetAllLabel()
> is not restored, because at virSecuritySELinuxRestoreAllLabel()
> phase it's too late and the caller (QEMU driver) simply doesn't
> care. Well, don't remember the label and let the perms leak.
>
> Signed-off-by: Michal Privoznik 
> ---
>  src/security/security_selinux.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index 8aeb6e45a5..f47bfbdba9 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -3233,7 +3233,7 @@ virSecuritySELinuxSetAllLabel(virSecurityManagerPtr mgr,
>
>  if (stdin_path &&
>  virSecuritySELinuxSetFilecon(mgr, stdin_path,
> - data->content_context, true) < 0)
> + data->content_context, false) < 0)

You're removing this whole condition in patch 3/3, so I suggest, you drop this
patch completely.

--
Erik Skultety

Re: [libvirt PATCH] util: Fix virDaemonForkIntoBackground

[Michal Privoznik]

On 4/16/20 3:49 PM, Jiri Denemark wrote:

This commit partially reverts

 commit c360ea28dc267802690e129fbad08ca2f22a44e9
 Refs: v6.2.0-rc1-1-gc360ea28dc
 Author: Rafael Fonseca 
 AuthorDate: Fri Mar 27 18:40:47 2020 +0100
 Commit: Michal Prívozník 
 CommitDate: Mon Mar 30 09:48:22 2020 +0200

 util: virdaemon: fix compilation on mingw

 The daemons are not supported on Win32 and therefore were not compiled
 in that platform. However, with the daemon code sharing, all the code in
 utils *is* compiled and it failed because `waitpid`, `fork`, and
 `setsid` are not available. So, as before, let's not build them on
 Win32 and make the code more portable by using existing vir* wrappers.

Not compiling virDaemonForkIntoBackground on Win32 is good, but the
second part of the original patch incorrectly replaced waitpid and fork
with our virProcessWait and virFork APIs. These APIs are more than just
simple wrappers and we don't want any of the extra functionality.
Especially virFork would reset any setup made before
virDaemonForkIntoBackground is called, such as logging, signal handling,
etc.

As a result of the change the additional fix in v6.2.0-67-ga87e4788d2
(util: virdaemon: fix waiting for child processes) is no longer
needed and it is effectively reverted by this commit.

Signed-off-by: Jiri Denemark 
---
  src/util/virdaemon.c | 13 +++--
  1 file changed, 7 insertions(+), 6 deletions(-)


Ooops.

Reviewed-by: Michal Privoznik 

Michal

Re: [PATCH 2/2] docs: Improve zPCI section in pci-addresses.rst

[Andrea Bolognani]

On Fri, 2020-04-17 at 11:02 +0200, Cornelia Huck wrote:
> On Fri, 17 Apr 2020 10:50:02 +0200
> Boris Fiuczynski  wrote:
> > > >   
> > > >   
> > > >> > > function='0x0'>
> > > > -  
> > > > +
> > > 
> > > Why this change? The pci-bridge does not show up in the guest anyway.  
> > 
> > My assumption was that uid and fid for this would be autogenerated.
> > Since uid 0x0001 and fid 0x have been freed up due to the change 
> > below this would be the autogenerated set.
> 
> If that makes the XML look saner, no objection.

I don't think it makes a lot of difference, but it doesn't make it
any more confusing either, so I'm okay with changing it :)

> > > >   Note that the PCI bridge is not visible in the guest; s390x always 
> > > > has a flat
> > > > -topology.
> > > > +topology. Also ``fid`` does not define slot or function of the PCI 
> > > > address.  
> > > 
> > > I find the sentence regarding 'fid' confusing. Maybe instead move up
> > > the explanation from below regarding uid and fid?
> > > 
> > > "The PCI address in the guest is generated from..."
> > >   
> > Lets join your proposal with Andreas and move his rewrite up to here.
> > Like:
> > ...topology.
> > The PCI address in the guest is generated from the information provided 
> > via the ``zpci`` element: more specifically, ``uid`` is used as the PCI 
> > domain.``fid`` doesn't appear in the PCI address itself, but it will be
> > used in sysfs (``/sys/bus/pci/slots/$fid/...``).
> 
> Sounds good.
> 
> (Also the rest of the changes.)

Cool.

Boris, are you going to post a v2 squashing in all proposed changes?

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH v5 0/4] introduction of migration_version attribute for VFIO live migration

[Yan Zhao]

On Fri, Apr 17, 2020 at 04:44:50PM +0800, Cornelia Huck wrote:
> On Mon, 13 Apr 2020 01:52:01 -0400
> Yan Zhao  wrote:
> 
> > This patchset introduces a migration_version attribute under sysfs of VFIO
> > Mediated devices.
> > 
> > This migration_version attribute is used to check migration compatibility
> > between two mdev devices.
> > 
> > Currently, it has two locations:
> > (1) under mdev_type node,
> > which can be used even before device creation, but only for mdev
> > devices of the same mdev type.
> > (2) under mdev device node,
> > which can only be used after the mdev devices are created, but the src
> > and target mdev devices are not necessarily be of the same mdev type
> > (The second location is newly added in v5, in order to keep consistent
> > with the migration_version node for migratable pass-though devices)
> 
> What is the relationship between those two attributes?
> 
(1) is for mdev devices specifically, and (2) is provided to keep the same
sysfs interface as with non-mdev cases. so (2) is for both mdev devices and
non-mdev devices.

in future, if we enable vfio-pci vendor ops, (i.e. a non-mdev device
is binding to vfio-pci, but is able to register migration region and do
migration transactions from a vendor provided affiliate driver),
the vendor driver would export (2) directly, under device node.
It is not able to provide (1) as there're no mdev devices involved.

> Is existence (and compatibility) of (1) a pre-req for possible
> existence (and compatibility) of (2)?
>
no. (2) does not reply on (1).

> Does userspace need to check (1) or can it completely rely on (2), if
> it so chooses?
>
I think it can completely reply on (2) if compatibility check before
mdev creation is not required.

> If devices with a different mdev type are indeed compatible, it seems
> userspace can only find out after the devices have actually been
> created, as (1) does not apply?
yes, I think so. 

> One of my worries is that the existence of an attribute with the same
> name in two similar locations might lead to confusion. But maybe it
> isn't a problem.
>
Yes, I have the same feeling. but as (2) is for sysfs interface
consistency, to make it transparent to userspace tools like libvirt,
I guess the same name is necessary?

Thanks
Yan
> > 
> > Patch 1 defines migration_version attribute for the first location in
> > Documentation/vfio-mediated-device.txt
> > 
> > Patch 2 uses GVT as an example for patch 1 to show how to expose
> > migration_version attribute and check migration compatibility in vendor
> > driver.
> > 
> > Patch 3 defines migration_version attribute for the second location in
> > Documentation/vfio-mediated-device.txt
> > 
> > Patch 4 uses GVT as an example for patch 3 to show how to expose
> > migration_version attribute and check migration compatibility in vendor
> > driver.
> > 
> > (The previous "Reviewed-by" and "Acked-by" for patch 1 and patch 2 are
> > kept in v5, as there are only small changes to commit messages of the two
> > patches.)
> > 
> > v5:
> > added patch 2 and 4 for mdev device part of migration_version attribute.
> > 
> > v4:
> > 1. fixed indentation/spell errors, reworded several error messages
> > 2. added a missing memory free for error handling in patch 2
> > 
> > v3:
> > 1. renamed version to migration_version
> > 2. let errno to be freely defined by vendor driver
> > 3. let checking mdev_type be prerequisite of migration compatibility check
> > 4. reworded most part of patch 1
> > 5. print detailed error log in patch 2 and generate migration_version
> > string at init time
> > 
> > v2:
> > 1. renamed patched 1
> > 2. made definition of device version string completely private to vendor
> > driver
> > 3. reverted changes to sample mdev drivers
> > 4. described intent and usage of version attribute more clearly.
> > 
> > 
> > Yan Zhao (4):
> >   vfio/mdev: add migration_version attribute for mdev (under mdev_type
> > node)
> >   drm/i915/gvt: export migration_version to mdev sysfs (under mdev_type
> > node)
> >   vfio/mdev: add migration_version attribute for mdev (under mdev device
> > node)
> >   drm/i915/gvt: export migration_version to mdev sysfs (under mdev
> > device node)
> > 
> >  .../driver-api/vfio-mediated-device.rst   | 183 ++
> >  drivers/gpu/drm/i915/gvt/Makefile |   2 +-
> >  drivers/gpu/drm/i915/gvt/gvt.c|  39 
> >  drivers/gpu/drm/i915/gvt/gvt.h|   7 +
> >  drivers/gpu/drm/i915/gvt/kvmgt.c  |  55 ++
> >  drivers/gpu/drm/i915/gvt/migration_version.c  | 170 
> >  drivers/gpu/drm/i915/gvt/vgpu.c   |  13 +-
> >  7 files changed, 466 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/gpu/drm/i915/gvt/migration_version.c
> > 
> 

Re: [PATCH 2/2] docs: Improve zPCI section in pci-addresses.rst

[Cornelia Huck]

On Fri, 17 Apr 2020 10:50:02 +0200
Boris Fiuczynski  wrote:

> On 4/16/20 6:14 PM, Cornelia Huck wrote:
> > On Thu, 16 Apr 2020 17:56:18 +0200
> > Boris Fiuczynski  wrote:
> >   
> >> Improving the zPCI example by choosing more distinct values and
> >> adding explanation for fid.
> >>
> >> Signed-off-by: Boris Fiuczynski 
> >> ---
> >>   docs/pci-addresses.rst | 15 ---
> >>   1 file changed, 8 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/docs/pci-addresses.rst b/docs/pci-addresses.rst
> >> index 7c8e9edd73..4492389da5 100644
> >> --- a/docs/pci-addresses.rst
> >> +++ b/docs/pci-addresses.rst
> >> @@ -176,14 +176,14 @@ In the simplest case, the following XML snippet
> >>   
> >>   
> >>>> function='0x0'>
> >> -  
> >> +
> > 
> > Why this change? The pci-bridge does not show up in the guest anyway.  
> My assumption was that uid and fid for this would be autogenerated.
> Since uid 0x0001 and fid 0x have been freed up due to the change 
> below this would be the autogenerated set.

If that makes the XML look saner, no objection.

> 
> >   
> >>   
> >> 
> >> 
> >>   
> >>   
> >>>> function='0x0'>
> >> -  
> >> +  
> >>   
> >> 
> >>   
> >> @@ -191,21 +191,22 @@ will result in the following in a Linux guest:
> >>   
> >>   ::
> >>   
> >> -  0001:00:00.0 Ethernet controller: Red Hat, Inc. Virtio network device
> >> +  0007:00:00.0 Ethernet controller: Red Hat, Inc. Virtio network device
> >>   
> >>   Note that the PCI bridge is not visible in the guest; s390x always has a 
> >> flat
> >> -topology.
> >> +topology. Also ``fid`` does not define slot or function of the PCI 
> >> address.  
> > 
> > I find the sentence regarding 'fid' confusing. Maybe instead move up
> > the explanation from below regarding uid and fid?
> > 
> > "The PCI address in the guest is generated from..."
> >   
> Lets join your proposal with Andreas and move his rewrite up to here.
> Like:
> ...topology.
> The PCI address in the guest is generated from the information provided 
> via the ``zpci`` element: more specifically, ``uid`` is used as the PCI 
> domain.``fid`` doesn't appear in the PCI address itself, but it will be
> used in sysfs (``/sys/bus/pci/slots/$fid/...``).

Sounds good.

(Also the rest of the changes.)

Re: [PATCH V2 4/5] cpu: Add helper funtions to parse vendor and model

[Zhenyu Zheng]

ping for reviews

On Fri, Apr 10, 2020 at 3:58 PM Zhenyu Zheng 
wrote:

>
>> Add helper functions to parse vendor and model from
>> xml for ARM arch, and use them as callbacks when
>> load cpu maps.
>>
>> Signed-off-by: Zhenyu Zheng 
>>
>> ---
>>  src/cpu/cpu_arm.c | 171 +-
>>  1 file changed, 170 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
>> index d8f571cae3..24404eac2c 100644
>> --- a/src/cpu/cpu_arm.c
>> +++ b/src/cpu/cpu_arm.c
>> @@ -206,6 +206,174 @@ virCPUarmMapFeatureParse(xmlXPathContextPtr ctxt
>> G_GNUC_UNUSED,
>>  return 0;
>>  }
>>
>> +static virCPUarmVendorPtr
>> +virCPUarmVendorFindByID(virCPUarmMapPtr map,
>> +unsigned long vendor_id)
>> +{
>> +size_t i;
>> +
>> +for (i = 0; i < map->nvendors; i++) {
>> +if (map->vendors[i]->value == vendor_id)
>> +return map->vendors[i];
>> +}
>> +
>> +return NULL;
>> +}
>> +
>> +
>> +static virCPUarmVendorPtr
>> +virCPUarmVendorFindByName(virCPUarmMapPtr map,
>> +  const char *name)
>> +{
>> +size_t i;
>> +
>> +for (i = 0; i < map->nvendors; i++) {
>> +if (STREQ(map->vendors[i]->name, name))
>> +return map->vendors[i];
>> +}
>> +
>> +return NULL;
>> +}
>> +
>> +
>> +static int
>> +virCPUarmVendorParse(xmlXPathContextPtr ctxt,
>> +   const char *name,
>> +   void *data)
>> +{
>> +virCPUarmMapPtr map = data;
>> +virCPUarmVendorPtr vendor = NULL;
>> +int ret = -1;
>> +
>> +if (VIR_ALLOC(vendor) < 0)
>> +return ret;
>> +
>> +vendor->name = g_strdup(name);
>> +
>> +if (virCPUarmVendorFindByName(map, vendor->name)) {
>> +virReportError(VIR_ERR_INTERNAL_ERROR,
>> +   _("CPU vendor %s already defined"),
>> +   vendor->name);
>> +goto cleanup;
>> +}
>> +
>> +if (virXPathULongHex("string(@value)", ctxt, >value) < 0) {
>> +virReportError(VIR_ERR_INTERNAL_ERROR,
>> +   "%s", _("Missing CPU vendor value"));
>> +goto cleanup;
>> +}
>> +
>> +if (virCPUarmVendorFindByID(map, vendor->value)) {
>> +virReportError(VIR_ERR_INTERNAL_ERROR,
>> +   _("CPU vendor value 0x%2lx already defined"),
>> +   vendor->value);
>> +goto cleanup;
>> +}
>> +
>> +if (VIR_APPEND_ELEMENT(map->vendors, map->nvendors, vendor) < 0)
>> +goto cleanup;
>> +
>> +ret = 0;
>> +
>> + cleanup:
>> +virCPUarmVendorFree(vendor);
>> +return ret;
>> +
>> +}
>> +
>> +static virCPUarmModelPtr
>> +virCPUarmModelFind(virCPUarmMapPtr map,
>> +   const char *name)
>> +{
>> +size_t i;
>> +
>> +for (i = 0; i < map->nmodels; i++) {
>> +if (STREQ(map->models[i]->name, name))
>> +return map->models[i];
>> +}
>> +
>> +return NULL;
>> +}
>> +
>> +#if defined(__aarch64__)
>> +static virCPUarmModelPtr
>> +virCPUarmModelFindByPVR(virCPUarmMapPtr map,
>> +unsigned long pvr)
>> +{
>> +size_t i;
>> +
>> +for (i = 0; i < map->nmodels; i++) {
>> +if (map->models[i]->data.pvr == pvr)
>> +return map->models[i];
>> +}
>> +
>> +return NULL;
>> +}
>> +#endif
>> +
>> +static int
>> +virCPUarmModelParse(xmlXPathContextPtr ctxt,
>> +  const char *name,
>> +  void *data)
>> +{
>> +virCPUarmMapPtr map = data;
>> +virCPUarmModel *model;
>> +g_autofree xmlNodePtr *nodes = NULL;
>> +g_autofree char *vendor = NULL;
>> +
>> +if (VIR_ALLOC(model) < 0)
>> +goto error;
>> +
>> +model->name = g_strdup(name);
>> +
>> +if (virCPUarmModelFind(map, model->name)) {
>> +virReportError(VIR_ERR_INTERNAL_ERROR,
>> +   _("CPU model %s already defined"),
>> +   model->name);
>> +goto error;
>> +}
>> +
>> +if (virXPathBoolean("boolean(./vendor)", ctxt)) {
>> +vendor = virXPathString("string(./vendor/@name)", ctxt);
>> +if (!vendor) {
>> +virReportError(VIR_ERR_INTERNAL_ERROR,
>> +   _("Invalid vendor element in CPU model %s"),
>> +   model->name);
>> +goto error;
>> +}
>> +
>> +if (!(model->vendor = virCPUarmVendorFindByName(map, vendor))) {
>> +virReportError(VIR_ERR_INTERNAL_ERROR,
>> +   _("Unknown vendor %s referenced by CPU model
>> %s"),
>> +   vendor, model->name);
>> +goto error;
>> +}
>> +}
>> +
>> +if (!virXPathBoolean("boolean(./pvr)", ctxt)) {
>> +virReportError(VIR_ERR_INTERNAL_ERROR,
>> +   _("Missing PVR information for CPU model %s"),
>> +   model->name);
>> +goto error;
>> +}
>> +
>> +if 

Re: [PATCH V2 5/5] cpu: Introduce getHost support for ARM CPU driver

[Zhenyu Zheng]

Ping for reviews

On Fri, Apr 10, 2020 at 5:55 PM Andrea Bolognani 
wrote:

> On Thu, 2020-04-09 at 13:03 +0200, Jiri Denemark wrote:
> > On Thu, Apr 02, 2020 at 17:03:59 +0800, Zhenyu Zheng wrote:
> > > +static int
> > > +armCpuDataFromRegs(virCPUarmData *data)
> > > +{
> > > +/* Generate human readable flag list according to the order of */
> > > +/* AT_HWCAP bit map */
> > > +const char *flag_list[MAX_CPU_FLAGS] = {
> > > +"fp", "asimd", "evtstrm", "aes", "pmull", "sha1", "sha2",
> > > +"crc32", "atomics", "fphp", "asimdhp", "cpuid", "asimdrdm",
> > > +"jscvt", "fcma", "lrcpc", "dcpop", "sha3", "sm3", "sm4",
> > > +"asimddp", "sha512", "sve", "asimdfhm", "dit", "uscat",
> > > +"ilrcpc", "flagm", "ssbs", "sb", "paca", "pacg"};
> >
> > I would expect these to be defined in src/cpu_map/arm_features.xml,
> > although I'm not sure how well the new features would play with the
> > existing ones... What do you think, Andrea?
>
> You tell me :)
>
> As you know, the main thing that sets x86 and ARM apart in this area
> is that on x86 you can basically mix and match CPU features at your
> heart's content, but on ARM this is not (yet) possible: the only
> features that you can really control are the SVE-related ones.
>
> I assume that, at some point further down the line, it will be
> possible to control ARM CPU features with a similar level of
> granularity as it's currently possible on x86, and at that point
> something like
>
>   
> 
> 
>   
>
> will be possible, but that's certainly not the case now, and
> attempting to do so for non-SVE features will result in QEMU refusing
> to start.
>
> With that in mind, I don't think it's a problem to include these
> features in cpu_map upfront and report them in the context of the
> host CPU: any attempt to use them in guest CPU context will be
> blocked by QEMU anyway.
>
> Perhaps we could go one step further and fail validation in libvirt
> until such a time comes when QEMU is able to accept them? That would
> make for a more pleasant user experience, I think.
>
> Do you foresee any potential issues caused by this approach? As I
> said it seems to me like it would be fine, but you're the expert when
> it comes to the CPU drivers :)
>
> --
> Andrea Bolognani / Red Hat / Virtualization
>
>

Re: [PATCH 2/2] docs: Improve zPCI section in pci-addresses.rst

[Boris Fiuczynski]

On 4/16/20 6:14 PM, Cornelia Huck wrote:

On Thu, 16 Apr 2020 17:56:18 +0200
Boris Fiuczynski  wrote:


Improving the zPCI example by choosing more distinct values and
adding explanation for fid.

Signed-off-by: Boris Fiuczynski 
---
  docs/pci-addresses.rst | 15 ---
  1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/docs/pci-addresses.rst b/docs/pci-addresses.rst
index 7c8e9edd73..4492389da5 100644
--- a/docs/pci-addresses.rst
+++ b/docs/pci-addresses.rst
@@ -176,14 +176,14 @@ In the simplest case, the following XML snippet
  
  
  
-  
+  


Why this change? The pci-bridge does not show up in the guest anyway.

My assumption was that uid and fid for this would be autogenerated.
Since uid 0x0001 and fid 0x have been freed up due to the change 
below this would be the autogenerated set.





  


  
  
  
-  
+  
  

  
@@ -191,21 +191,22 @@ will result in the following in a Linux guest:
  
  ::
  
-  0001:00:00.0 Ethernet controller: Red Hat, Inc. Virtio network device

+  0007:00:00.0 Ethernet controller: Red Hat, Inc. Virtio network device
  
  Note that the PCI bridge is not visible in the guest; s390x always has a flat

-topology.
+topology. Also ``fid`` does not define slot or function of the PCI address.


I find the sentence regarding 'fid' confusing. Maybe instead move up
the explanation from below regarding uid and fid?

"The PCI address in the guest is generated from..."


Lets join your proposal with Andreas and move his rewrite up to here.
Like:
...topology.
The PCI address in the guest is generated from the information provided 
via the ``zpci`` element: more specifically, ``uid`` is used as the PCI 
domain.``fid`` doesn't appear in the PCI address itself, but it will be

used in sysfs (``/sys/bus/pci/slots/$fid/...``).


  
  Neither are any changes in the PCI address visible in the guest; replacing


The 'neither' is now a bit confusing; what about

"Any changes in the PCI address are not visible in the guest..." ?

Agreed.




  the PCI address for the ``virtio-net`` device with
  
  ::
  
-  

+  
  
  will result in the exactly same view in the guest, as the addresses there


If you move the fid/uid stuff up, make this

"as the fid and uid values in the zpci element remain unchanged." ?

Yes.




-are generated from the information provided via the ``zpci`` element (in
-fact, from the ``uid``).
+are generated from the information provided via the ``zpci`` element:
+the ``uid`` is used as PCI domain, and the ``fid`` is used as the PCI devices


s/devices/device's/


+slot in the sysfs.


s/the sysfs./sysfs (it does not influence the PCI device address.)/
*

Andreas rewrite makes it clear.





  
  
  Device assignment





--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

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

Re: [PATCH v5 0/4] introduction of migration_version attribute for VFIO live migration

[Cornelia Huck]

On Mon, 13 Apr 2020 01:52:01 -0400
Yan Zhao  wrote:

> This patchset introduces a migration_version attribute under sysfs of VFIO
> Mediated devices.
> 
> This migration_version attribute is used to check migration compatibility
> between two mdev devices.
> 
> Currently, it has two locations:
> (1) under mdev_type node,
> which can be used even before device creation, but only for mdev
> devices of the same mdev type.
> (2) under mdev device node,
> which can only be used after the mdev devices are created, but the src
> and target mdev devices are not necessarily be of the same mdev type
> (The second location is newly added in v5, in order to keep consistent
> with the migration_version node for migratable pass-though devices)

What is the relationship between those two attributes?

Is existence (and compatibility) of (1) a pre-req for possible
existence (and compatibility) of (2)?

Does userspace need to check (1) or can it completely rely on (2), if
it so chooses?

If devices with a different mdev type are indeed compatible, it seems
userspace can only find out after the devices have actually been
created, as (1) does not apply?

One of my worries is that the existence of an attribute with the same
name in two similar locations might lead to confusion. But maybe it
isn't a problem.

> 
> Patch 1 defines migration_version attribute for the first location in
> Documentation/vfio-mediated-device.txt
> 
> Patch 2 uses GVT as an example for patch 1 to show how to expose
> migration_version attribute and check migration compatibility in vendor
> driver.
> 
> Patch 3 defines migration_version attribute for the second location in
> Documentation/vfio-mediated-device.txt
> 
> Patch 4 uses GVT as an example for patch 3 to show how to expose
> migration_version attribute and check migration compatibility in vendor
> driver.
> 
> (The previous "Reviewed-by" and "Acked-by" for patch 1 and patch 2 are
> kept in v5, as there are only small changes to commit messages of the two
> patches.)
> 
> v5:
> added patch 2 and 4 for mdev device part of migration_version attribute.
> 
> v4:
> 1. fixed indentation/spell errors, reworded several error messages
> 2. added a missing memory free for error handling in patch 2
> 
> v3:
> 1. renamed version to migration_version
> 2. let errno to be freely defined by vendor driver
> 3. let checking mdev_type be prerequisite of migration compatibility check
> 4. reworded most part of patch 1
> 5. print detailed error log in patch 2 and generate migration_version
> string at init time
> 
> v2:
> 1. renamed patched 1
> 2. made definition of device version string completely private to vendor
> driver
> 3. reverted changes to sample mdev drivers
> 4. described intent and usage of version attribute more clearly.
> 
> 
> Yan Zhao (4):
>   vfio/mdev: add migration_version attribute for mdev (under mdev_type
> node)
>   drm/i915/gvt: export migration_version to mdev sysfs (under mdev_type
> node)
>   vfio/mdev: add migration_version attribute for mdev (under mdev device
> node)
>   drm/i915/gvt: export migration_version to mdev sysfs (under mdev
> device node)
> 
>  .../driver-api/vfio-mediated-device.rst   | 183 ++
>  drivers/gpu/drm/i915/gvt/Makefile |   2 +-
>  drivers/gpu/drm/i915/gvt/gvt.c|  39 
>  drivers/gpu/drm/i915/gvt/gvt.h|   7 +
>  drivers/gpu/drm/i915/gvt/kvmgt.c  |  55 ++
>  drivers/gpu/drm/i915/gvt/migration_version.c  | 170 
>  drivers/gpu/drm/i915/gvt/vgpu.c   |  13 +-
>  7 files changed, 466 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/gvt/migration_version.c
>