Re: [PATCH 4/5] doc: rng schemas and html doc for rbd namespace attribute

[Han Han]

On Thu, Apr 16, 2020 at 9:12 PM Jason Dillaman  wrote:

> On Wed, Apr 8, 2020 at 5:06 AM Han Han  wrote:
>
>> Signed-off-by: Han Han 
>> ---
>>  docs/formatdomain.html.in | 5 +
>>  docs/schemas/domaincommon.rng | 3 +++
>>  2 files changed, 8 insertions(+)
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index d56600dc..06e1a7ee 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -3118,6 +3118,11 @@
>>(Since 6.2.0)
>>
>>
>> +  For protocol rbd, an optional attribute
>> +  namespace specifies the namespace of a rbd
>> pool.
>> +  (Since 6.3.0 and QEMU 5.0.0)
>> +  
>> +
>>For "iscsi" (since 1.0.4),
>> the
>>name attribute may include a logical unit
>> number,
>>separated from the target's name by a slash (e.g.,
>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>> index dcf2e09d..294ade34 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -1762,6 +1762,9 @@
>>  
>>
>>  
>> +
>> +  
>> +
>>
>
> My only concern about this patchset is how the "name" attribute is
> overloaded (historically) to include both the RBD pool and image name. Now
> if the RBD namespace is separated out to its own optional attribute, it
> requires constructing an invalid "/" name attribute.  Can
> the pool be separated out to its own attribute as well  (i.e.  image="image1" pool="rbd" namespace="ns1"...>) and keep a deprecated
> backwards
>
Yeah. I also noticed the historical problem of "name". It looks OK to
separate the pool and image out of the name.
Let's make namespace independent first. Then split the name and keep
backwards compatibility.

> compatible "name='pool/image'" attribute that doesn't support namespaces?
> The QAPI already has the pool and image as separate attributes.
>
>
>>  
>>
>>  
>> --
>> 2.25.0
>>
>>
>
> --
> Jason
>


-- 
Best regards,
---
Han Han
Quality Engineer
Redhat.

Email: h...@redhat.com
Phone: +861065339333

[PATCH v1 2/7] qemu: Implement the CFPC pSeries feature

[Daniel Henrique Barboza]

This patch adds the implementation of the CFPC pSeries feature,
using the QEMU_CAPS_MACHINE_PSERIES_CAP_CFPC capability added
in the previous patch.

CPFC can have the values "broken", "workaround" or "fixed". Extra
code is required to handle it since it's not a regular tristate
capability.

This is the XML format for the cap:


  


Signed-off-by: Daniel Henrique Barboza 
---
 docs/formatdomain.html.in | 11 +
 docs/schemas/domaincommon.rng | 15 +++
 src/conf/domain_conf.c| 45 +++
 src/conf/domain_conf.h| 12 +
 src/libvirt_private.syms  |  1 +
 src/qemu/qemu_command.c   |  5 +++
 src/qemu/qemu_validate.c  | 11 +
 tests/qemuxml2argvdata/pseries-features.args  |  3 +-
 tests/qemuxml2argvdata/pseries-features.xml   |  1 +
 tests/qemuxml2argvtest.c  | 17 ++-
 tests/qemuxml2xmloutdata/pseries-features.xml |  1 +
 tests/qemuxml2xmltest.c   |  3 +-
 12 files changed, 122 insertions(+), 3 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 0077666862..607e815413 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2074,6 +2074,7 @@
   htm state='on'/
   ccf-assist state='on'/
   msrs unknown='ignore'/
+  cfpc value='workaround'/
 /features
 ...
 
@@ -2379,6 +2380,16 @@
   defined, the hypervisor default will be used.
   Since 5.9.0 (QEMU/KVM only)
   
+  cfpc
+  Configure cfpc (Cache Flush on Privilege Change) availability for
+  pSeries guests.
+  Possible values for the value attribute
+  are broken (no protection), workaround
+  (software workaround available) and fixed (fixed in
+  hardware). If the attribute is not defined, the hypervisor
+  default will be used.
+  Since 6.3.0 (QEMU/KVM only)
+  
 
 
 Time keeping
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 65d6580434..40785752f7 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5430,6 +5430,9 @@
   
 
   
+  
+
+  
 
   
 
@@ -5689,6 +5692,18 @@
 
   
 
+  
+
+  
+
+  broken
+  workaround
+  fixed
+
+  
+
+  
+
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8700d56761..4bfb17b7c8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -173,6 +173,7 @@ VIR_ENUM_IMPL(virDomainFeature,
   "nested-hv",
   "msrs",
   "ccf-assist",
+  "cfpc",
 );
 
 VIR_ENUM_IMPL(virDomainCapabilitiesPolicy,
@@ -1251,6 +1252,14 @@ VIR_ENUM_IMPL(virDomainOsDefFirmware,
   "efi",
 );
 
+VIR_ENUM_IMPL(virDomainCFPC,
+  VIR_DOMAIN_CFPC_LAST,
+  "none",
+  "broken",
+  "workaround",
+  "fixed",
+);
+
 /* Internal mapping: subset of block job types that can be present in
  *  XML (remaining types are not two-phase). */
 VIR_ENUM_DECL(virDomainBlockJob);
@@ -20961,6 +20970,22 @@ virDomainDefParseXML(xmlDocPtr xml,
 }
 break;
 
+case VIR_DOMAIN_FEATURE_CFPC:
+tmp = virXMLPropString(nodes[i], "value");
+if (tmp) {
+int value = virDomainCFPCTypeFromString(tmp);
+if (value < 0 || value == VIR_DOMAIN_CFPC_NONE) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Unknown value: %s"),
+   tmp);
+goto error;
+}
+def->features[val] = value;
+VIR_FREE(tmp);
+}
+break;
+
+
 case VIR_DOMAIN_FEATURE_HTM:
 case VIR_DOMAIN_FEATURE_NESTED_HV:
 case VIR_DOMAIN_FEATURE_CCF_ASSIST:
@@ -23266,6 +23291,18 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr 
src,
 }
 break;
 
+case VIR_DOMAIN_FEATURE_CFPC:
+if (src->features[i] != dst->features[i]) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("State of feature '%s' differs: "
+ "source: '%s=%s', destination: '%s=%s'"),
+   featureName,
+   "value", 
virDomainCFPCTypeToString(src->features[i]),
+   "value", 
virDomainCFPCTypeToString(dst->features[i]));
+return false;
+}
+break;
+
 case VIR_DOMAIN_FEATURE_MSRS:
 break;
 
@@ -29024,6 +29061,14 @@ virDomainDefFormatFeatures(virBufferPtr buf,
   

[PATCH v1 7/7] news: Update for the recent added pSeries features

[Daniel Henrique Barboza]

Update news.xml to inform about the availability of CFPC, SBBC and
IBS features.

Signed-off-by: Daniel Henrique Barboza 
---
 docs/news.xml | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 4d0efd4219..2066f0eef3 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -56,6 +56,16 @@
   hotplug/unplug attempts, but this is often undesireable).
 
   
+  
+
+  qemu: Implement pSeries Spectre mitigation features
+
+
+  Users can now setup the following capabilities of pSeries guests:
+  CFPC (Cache Flush on Privilege Change), SBBC (Speculation Barrier
+  Bounds Checking) and IBS (Indirect Branch Speculation).
+
+  
 
 
 
-- 
2.25.2

[PATCH v1 6/7] qemu: Implement the IBS pSeries feature

[Daniel Henrique Barboza]

This patch adds the implementation of the IBS pSeries feature,
using the QEMU_CAPS_MACHINE_PSERIES_CAP_IBS capability added
in the previous patch.

IBS can have the following values: "broken", "workaround",
"fixed-ibs", "fixed-ccd" and "fixed-na".

This is the XML format for the cap:


  


Signed-off-by: Daniel Henrique Barboza 
---
 docs/formatdomain.html.in | 14 ++
 docs/schemas/domaincommon.rng | 17 +++
 src/conf/domain_conf.c| 46 +++
 src/conf/domain_conf.h| 14 ++
 src/libvirt_private.syms  |  1 +
 src/qemu/qemu_command.c   |  5 ++
 src/qemu/qemu_validate.c  | 11 +
 tests/qemuxml2argvdata/pseries-features.args  |  2 +-
 tests/qemuxml2argvdata/pseries-features.xml   |  1 +
 tests/qemuxml2argvtest.c  | 25 --
 tests/qemuxml2xmloutdata/pseries-features.xml |  1 +
 tests/qemuxml2xmltest.c   |  3 +-
 12 files changed, 135 insertions(+), 5 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 2146374475..01aa825fc6 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2076,6 +2076,7 @@
   msrs unknown='ignore'/
   cfpc value='workaround'/
   sbbc value='workaround'/
+  ibs value='fixed-na'/
 /features
 ...
 
@@ -2401,6 +2402,19 @@
   default will be used.
   Since 6.3.0 (QEMU/KVM only)
   
+  ibs
+  Configure ibs (Indirect Branch Speculation) availability for
+  pSeries guests.
+  Possible values for the value attribute
+  are broken (no protection), workaround
+  (count cache flush), fixed-ibs (fixed by
+  serializing indirect branches), fixed-ccd (fixed by
+  disabling the cache count) and fixed-na (fixed in
+  hardware - no longer applicable).
+  If the attribute is not defined, the hypervisor
+  default will be used.
+  Since 6.3.0 (QEMU/KVM only)
+  
 
 
 Time keeping
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 72c281aa8f..75d5cd3271 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5436,6 +5436,9 @@
   
 
   
+  
+
+  
 
   
 
@@ -5719,6 +5722,20 @@
 
   
 
+  
+
+  
+
+  broken
+  workaround
+  fixed-ibs
+  fixed-ccd
+  fixed-na
+
+  
+
+  
+
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5373b29263..dfa7421249 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -175,6 +175,7 @@ VIR_ENUM_IMPL(virDomainFeature,
   "ccf-assist",
   "cfpc",
   "sbbc",
+  "ibs",
 );
 
 VIR_ENUM_IMPL(virDomainCapabilitiesPolicy,
@@ -1269,6 +1270,16 @@ VIR_ENUM_IMPL(virDomainSBBC,
   "fixed",
 );
 
+VIR_ENUM_IMPL(virDomainIBS,
+  VIR_DOMAIN_IBS_LAST,
+  "none",
+  "broken",
+  "workaround",
+  "fixed-ibs",
+  "fixed-ccd",
+  "fixed-na",
+);
+
 /* Internal mapping: subset of block job types that can be present in
  *  XML (remaining types are not two-phase). */
 VIR_ENUM_DECL(virDomainBlockJob);
@@ -21009,6 +21020,21 @@ virDomainDefParseXML(xmlDocPtr xml,
 }
 break;
 
+case VIR_DOMAIN_FEATURE_IBS:
+tmp = virXMLPropString(nodes[i], "value");
+if (tmp) {
+int value = virDomainIBSTypeFromString(tmp);
+if (value < 0 || value == VIR_DOMAIN_IBS_NONE) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Unknown value: %s"),
+   tmp);
+goto error;
+}
+def->features[val] = value;
+VIR_FREE(tmp);
+}
+break;
+
 case VIR_DOMAIN_FEATURE_HTM:
 case VIR_DOMAIN_FEATURE_NESTED_HV:
 case VIR_DOMAIN_FEATURE_CCF_ASSIST:
@@ -23338,6 +23364,18 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr 
src,
 }
 break;
 
+case VIR_DOMAIN_FEATURE_IBS:
+if (src->features[i] != dst->features[i]) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("State of feature '%s' differs: "
+ "source: '%s=%s', destination: '%s=%s'"),
+   featureName,
+   "value", 
virDomainIBSTypeToString(src->features[i]),
+   "value", 
virDomainIBSTypeToString(dst->features[i]));
+return false;
+}
+break;
+
 case VIR_DOMAIN_FEATURE_MSRS:

[PATCH v1 1/7] qemu: Add capability for CFPC pSeries feature

[Daniel Henrique Barboza]

CFPC (Cache Flush on Privilege Change) is one of the capabilities
added to QEMU to mitigate Spectre vulnerabilities in Power chips.
It was implemented in QEMU 2.12 by commit 6898aed77f46.

This capability is still used today due to differences in how
the host setup (hardware and firmware/kernel) can handle this
mitigation. Its default value also varies with the pseries machine
version of the time. There's also certain OSes, like AIX, that
might not support the default value of the pseries machine the
guest uses.

Exposing this in the Libvirt XML as a feature will allow users to tune
CFPC values in a cleaner way, instead of hacking parameters in
 elements.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_capabilities.c | 2 ++
 src/qemu/qemu_capabilities.h | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 +
 tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml  | 1 +
 8 files changed, 9 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index fe311048d4..4a262dc71d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -573,6 +573,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
   "fsdev.multidevs",
   "virtio.packed",
   "pcie-root-port.hotplug",
+  "machine.pseries.cap-cfpc",
 );
 
 
@@ -1615,6 +1616,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsMachinePropsPSeries[] = {
 { "cap-htm", QEMU_CAPS_MACHINE_PSERIES_CAP_HTM },
 { "cap-nested-hv", QEMU_CAPS_MACHINE_PSERIES_CAP_NESTED_HV },
 { "cap-ccf-assist", QEMU_CAPS_MACHINE_PSERIES_CAP_CCF_ASSIST },
+{ "cap-cfpc", QEMU_CAPS_MACHINE_PSERIES_CAP_CFPC },
 };
 
 static struct virQEMUCapsStringFlags virQEMUCapsMachinePropsVirt[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 1681fc79a8..81d67d2efe 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -554,6 +554,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_FSDEV_MULTIDEVS, /* fsdev.multidevs */
 QEMU_CAPS_VIRTIO_PACKED_QUEUES, /* virtio.packed */
 QEMU_CAPS_PCIE_ROOT_PORT_HOTPLUG, /* pcie-root-port.hotplug */
+QEMU_CAPS_MACHINE_PSERIES_CAP_CFPC, /* -machine pseries.cap-cfpc */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
index 38a3103c4a..cdd4f26993 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
@@ -154,6 +154,7 @@
   
   
   
+  
   2011090
   0
   42900289
diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml
index 9a0b9c05c2..84e9ad2dcc 100644
--- a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml
@@ -156,6 +156,7 @@
   
   
   
+  
   2012050
   0
   42900239
diff --git a/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml
index 6801023208..3d70a67dab 100644
--- a/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml
@@ -161,6 +161,7 @@
   
   
   
+  
   391
   0
   42900240
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
index f7e69fcc97..ce2d470cb2 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
@@ -175,6 +175,7 @@
   
   
   
+  
   400
   0
   42900240
diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml
index 99ec98e8cd..a813776660 100644
--- a/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml
@@ -180,6 +180,7 @@
   
   
   
+  
   4001050
   0
   42900242
diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml
index b08916132a..c33786b0bf 100644
--- a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml
@@ -192,6 +192,7 @@
   
   
   
+  
   4002050
   0
   42900241
-- 
2.25.2

[PATCH v1 5/7] qemu: Add capability for IBS pSeries feature

[Daniel Henrique Barboza]

IBS (Indirect Branch Speculation) is the last capability added
in QEMU 2.12 related to Spectre mitigation for Power. It was
added in commit 4be8d4e7d935.

This patch introduces it as QEMU_CAPS_MACHINE_PSERIES_CAP_IBS.
Like CFPC and SBBC, users might want to tune in IBS based on
their HW and guest OS requirements, and it's better to do it
so in a proper Libvirt feature than to put QEMU arguments
in the middle of the domain XML.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_capabilities.c | 4 
 src/qemu/qemu_capabilities.h | 3 +++
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 +
 tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml  | 1 +
 8 files changed, 13 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index dd04393c95..4ea01d4ea9 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -575,6 +575,9 @@ VIR_ENUM_IMPL(virQEMUCaps,
   "pcie-root-port.hotplug",
   "machine.pseries.cap-cfpc",
   "machine.pseries.cap-sbbc",
+
+  /* 365 */
+  "machine.pseries.cap-ibs",
 );
 
 
@@ -1619,6 +1622,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsMachinePropsPSeries[] = {
 { "cap-ccf-assist", QEMU_CAPS_MACHINE_PSERIES_CAP_CCF_ASSIST },
 { "cap-cfpc", QEMU_CAPS_MACHINE_PSERIES_CAP_CFPC },
 { "cap-sbbc", QEMU_CAPS_MACHINE_PSERIES_CAP_SBBC },
+{ "cap-ibs", QEMU_CAPS_MACHINE_PSERIES_CAP_IBS },
 };
 
 static struct virQEMUCapsStringFlags virQEMUCapsMachinePropsVirt[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 83932fb96f..21e3eb3327 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -557,6 +557,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_MACHINE_PSERIES_CAP_CFPC, /* -machine pseries.cap-cfpc */
 QEMU_CAPS_MACHINE_PSERIES_CAP_SBBC, /* -machine pseries.cap-sbbc */
 
+/* 365 */
+QEMU_CAPS_MACHINE_PSERIES_CAP_IBS, /* -machine pseries.cap-ibs */
+
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
 
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
index 2046f1097c..4c1758fbfe 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
@@ -156,6 +156,7 @@
   
   
   
+  
   2011090
   0
   42900289
diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml
index 9e71080152..a8390a12eb 100644
--- a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml
@@ -158,6 +158,7 @@
   
   
   
+  
   2012050
   0
   42900239
diff --git a/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml
index f13b384e91..d96caaa9ed 100644
--- a/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml
@@ -163,6 +163,7 @@
   
   
   
+  
   391
   0
   42900240
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
index 674e4b4944..44c1b9205e 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
@@ -177,6 +177,7 @@
   
   
   
+  
   400
   0
   42900240
diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml
index f89498171b..2eef337cc4 100644
--- a/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml
@@ -182,6 +182,7 @@
   
   
   
+  
   4001050
   0
   42900242
diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml
index ebc39130df..d972def4b3 100644
--- a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml
@@ -194,6 +194,7 @@
   
   
   
+  
   4002050
   0
   42900241
-- 
2.25.2

[PATCH v1 4/7] qemu: Implement the SBBC pSeries feature

[Daniel Henrique Barboza]

This patch adds the implementation of the SBBC pSeries feature,
using the QEMU_CAPS_MACHINE_PSERIES_CAP_SBBC capability added
in the previous patch.

Like the previously added CPFC feature, SBBC can have the values
"broken", "workaround" or "fixed". Extra code is required to handle
it since it's not a regular tristate capability.

This is the XML format for the cap:


  


Signed-off-by: Daniel Henrique Barboza 
---
 docs/formatdomain.html.in | 11 +
 docs/schemas/domaincommon.rng | 15 +++
 src/conf/domain_conf.c| 43 +++
 src/conf/domain_conf.h| 12 ++
 src/libvirt_private.syms  |  1 +
 src/qemu/qemu_command.c   |  5 +++
 src/qemu/qemu_validate.c  | 11 +
 tests/qemuxml2argvdata/pseries-features.args  |  2 +-
 tests/qemuxml2argvdata/pseries-features.xml   |  1 +
 tests/qemuxml2argvtest.c  | 21 -
 tests/qemuxml2xmloutdata/pseries-features.xml |  1 +
 tests/qemuxml2xmltest.c   |  3 +-
 12 files changed, 122 insertions(+), 4 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 607e815413..2146374475 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2075,6 +2075,7 @@
   ccf-assist state='on'/
   msrs unknown='ignore'/
   cfpc value='workaround'/
+  sbbc value='workaround'/
 /features
 ...
 
@@ -2390,6 +2391,16 @@
   default will be used.
   Since 6.3.0 (QEMU/KVM only)
   
+  sbbc
+  Configure sbbc (Speculation Barrier Bounds Checking) availability for
+  pSeries guests.
+  Possible values for the value attribute
+  are broken (no protection), workaround
+  (software workaround available) and fixed (fixed in
+  hardware). If the attribute is not defined, the hypervisor
+  default will be used.
+  Since 6.3.0 (QEMU/KVM only)
+  
 
 
 Time keeping
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 40785752f7..72c281aa8f 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5433,6 +5433,9 @@
   
 
   
+  
+
+  
 
   
 
@@ -5704,6 +5707,18 @@
 
   
 
+  
+
+  
+
+  broken
+  workaround
+  fixed
+
+  
+
+  
+
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4bfb17b7c8..5373b29263 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -174,6 +174,7 @@ VIR_ENUM_IMPL(virDomainFeature,
   "msrs",
   "ccf-assist",
   "cfpc",
+  "sbbc",
 );
 
 VIR_ENUM_IMPL(virDomainCapabilitiesPolicy,
@@ -1260,6 +1261,14 @@ VIR_ENUM_IMPL(virDomainCFPC,
   "fixed",
 );
 
+VIR_ENUM_IMPL(virDomainSBBC,
+  VIR_DOMAIN_SBBC_LAST,
+  "none",
+  "broken",
+  "workaround",
+  "fixed",
+);
+
 /* Internal mapping: subset of block job types that can be present in
  *  XML (remaining types are not two-phase). */
 VIR_ENUM_DECL(virDomainBlockJob);
@@ -20985,6 +20994,20 @@ virDomainDefParseXML(xmlDocPtr xml,
 }
 break;
 
+case VIR_DOMAIN_FEATURE_SBBC:
+tmp = virXMLPropString(nodes[i], "value");
+if (tmp) {
+int value = virDomainSBBCTypeFromString(tmp);
+if (value < 0 || value == VIR_DOMAIN_SBBC_NONE) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Unknown value: %s"),
+   tmp);
+goto error;
+}
+def->features[val] = value;
+VIR_FREE(tmp);
+}
+break;
 
 case VIR_DOMAIN_FEATURE_HTM:
 case VIR_DOMAIN_FEATURE_NESTED_HV:
@@ -23303,6 +23326,18 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr 
src,
 }
 break;
 
+case VIR_DOMAIN_FEATURE_SBBC:
+if (src->features[i] != dst->features[i]) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("State of feature '%s' differs: "
+ "source: '%s=%s', destination: '%s=%s'"),
+   featureName,
+   "value", 
virDomainSBBCTypeToString(src->features[i]),
+   "value", 
virDomainSBBCTypeToString(dst->features[i]));
+return false;
+}
+break;
+
 case VIR_DOMAIN_FEATURE_MSRS:
 break;
 
@@ -29069,6 +29104,14 @@ virDomainDefFormatFeatures(virBufferPtr buf,
   virDomainCFPCTypeToString(def->features[i]));
 break;
 
+case 

[PATCH v1 0/7] add Spectre related PowerPC features

[Daniel Henrique Barboza]

Hi,

This series implements 3 Spectre related PowerPC features that
were added back in QEMU 2.12:

- CFPC: Cache Flush on Privilege Change
- SBBC: Speculation Barrier Bounds Checking 
- IBS:  Indirect Branch Speculation

These options aren't much of a problem for users using latest
hardware and guests with recent Linux kernels. Users with outdated
hardware/firmware or trying to run AIX guests/guests with older
kernels, however, will need to fine tune these options because
QEMU defaults won't work.

Instead of making users rely on  elements to
hardcode the options in the XML, let's support them in Libvirt.


Daniel Henrique Barboza (7):
  qemu: Add capability for CFPC pSeries feature
  qemu: Implement the CFPC pSeries feature
  qemu: Add capability for SBBC pSeries feature
  qemu: Implement the SBBC pSeries feature
  qemu: Add capability for IBS pSeries feature
  qemu: Implement the IBS pSeries feature
  news: Update for the recent added pSeries features

 docs/formatdomain.html.in |  36 +
 docs/news.xml |  10 ++
 docs/schemas/domaincommon.rng |  47 ++
 src/conf/domain_conf.c| 134 ++
 src/conf/domain_conf.h|  38 +
 src/libvirt_private.syms  |   3 +
 src/qemu/qemu_capabilities.c  |   8 ++
 src/qemu/qemu_capabilities.h  |   5 +
 src/qemu/qemu_command.c   |  15 ++
 src/qemu/qemu_validate.c  |  33 +
 .../caps_2.12.0.ppc64.xml |   3 +
 .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml |   3 +
 .../qemucapabilitiesdata/caps_3.1.0.ppc64.xml |   3 +
 .../qemucapabilitiesdata/caps_4.0.0.ppc64.xml |   3 +
 .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml |   3 +
 .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml |   3 +
 tests/qemuxml2argvdata/pseries-features.args  |   3 +-
 tests/qemuxml2argvdata/pseries-features.xml   |   3 +
 tests/qemuxml2argvtest.c  |  53 ++-
 tests/qemuxml2xmloutdata/pseries-features.xml |   3 +
 tests/qemuxml2xmltest.c   |   5 +-
 21 files changed, 411 insertions(+), 3 deletions(-)

-- 
2.25.2

[PATCH v1 3/7] qemu: Add capability for SBBC pSeries feature

[Daniel Henrique Barboza]

SBBC (Speculation Barrier Bounds Checking) is another capability
related to Spectre mitigation efforts in Power processors. It
was implemented in QEMU 2.12 by commit 09114fd81799.

This patch introduces it as QEMU_CAPS_MACHINE_PSERIES_CAP_SBBC to
be implemented in the next patch. Like the case with the now
implemented CFPC, exposing this feature in the XML allows for
a cleaner way for users to tune the SBBC accordingly, given
that not all hypervisor and guest setups supports this
Spectre mitigation.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_capabilities.c | 2 ++
 src/qemu/qemu_capabilities.h | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 +
 tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml  | 1 +
 8 files changed, 9 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 4a262dc71d..dd04393c95 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -574,6 +574,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
   "virtio.packed",
   "pcie-root-port.hotplug",
   "machine.pseries.cap-cfpc",
+  "machine.pseries.cap-sbbc",
 );
 
 
@@ -1617,6 +1618,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsMachinePropsPSeries[] = {
 { "cap-nested-hv", QEMU_CAPS_MACHINE_PSERIES_CAP_NESTED_HV },
 { "cap-ccf-assist", QEMU_CAPS_MACHINE_PSERIES_CAP_CCF_ASSIST },
 { "cap-cfpc", QEMU_CAPS_MACHINE_PSERIES_CAP_CFPC },
+{ "cap-sbbc", QEMU_CAPS_MACHINE_PSERIES_CAP_SBBC },
 };
 
 static struct virQEMUCapsStringFlags virQEMUCapsMachinePropsVirt[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 81d67d2efe..83932fb96f 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -555,6 +555,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_VIRTIO_PACKED_QUEUES, /* virtio.packed */
 QEMU_CAPS_PCIE_ROOT_PORT_HOTPLUG, /* pcie-root-port.hotplug */
 QEMU_CAPS_MACHINE_PSERIES_CAP_CFPC, /* -machine pseries.cap-cfpc */
+QEMU_CAPS_MACHINE_PSERIES_CAP_SBBC, /* -machine pseries.cap-sbbc */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
index cdd4f26993..2046f1097c 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
@@ -155,6 +155,7 @@
   
   
   
+  
   2011090
   0
   42900289
diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml
index 84e9ad2dcc..9e71080152 100644
--- a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml
@@ -157,6 +157,7 @@
   
   
   
+  
   2012050
   0
   42900239
diff --git a/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml
index 3d70a67dab..f13b384e91 100644
--- a/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml
@@ -162,6 +162,7 @@
   
   
   
+  
   391
   0
   42900240
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
index ce2d470cb2..674e4b4944 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
@@ -176,6 +176,7 @@
   
   
   
+  
   400
   0
   42900240
diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml
index a813776660..f89498171b 100644
--- a/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml
@@ -181,6 +181,7 @@
   
   
   
+  
   4001050
   0
   42900242
diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml
index c33786b0bf..ebc39130df 100644
--- a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml
@@ -193,6 +193,7 @@
   
   
   
+  
   4002050
   0
   42900241
-- 
2.25.2

Re: [libvirt PATCH] docs: kbase: add a page about debugging

[Andrea Bolognani]

On Wed, 2020-04-15 at 17:10 +0200, Ján Tomko wrote:
> Migrate the following wiki articles:
> https://wiki.libvirt.org/page/DebugLogs
> https://wiki.libvirt.org/page/Debugging
> 
> With the syntax changed to rST and rewrapped.
> 
> Signed-off-by: Ján Tomko 
> ---
> I might've changed an article or two, too.
> Or rather: the article 'the'.
> 
>  docs/kbase.html.in   |   3 +
>  docs/kbase/debugging.rst | 303 +++
>  2 files changed, 306 insertions(+)
>  create mode 100644 docs/kbase/debugging.rst

This fails to apply for me:

  Applying: docs: kbase: add a page about debugging
  error: corrupt patch at line 335
  Patch failed at 0001 docs: kbase: add a page about debugging
  hint: Use 'git am --show-current-patch' to see the failed patch
  When you have resolved this problem, run "git am --continue".
  If you prefer to skip this patch, run "git am --skip" instead.
  To restore the original branch and stop patching, run "git am --abort".

[...]
> +++ b/docs/kbase/debugging.rst
> @@ -0,0 +1,303 @@
> +=
> +Debugging libvirt
> +=
> +
> +.. contents::
> +
> +
> +If you `report a bug `_ against libvirt, in 
> most

I think we want to make links to other libvirt documentation
relative, so that they work correctly even when browsing an offline
copy locally.

[...]
> +The daemon configuration files location is dependent on the `connection URI
> +`_. For ``qemu:///system``:
> +
> +* open ``/etc/libvirt/libvirtd.conf`` in your favourite editor
> +* find & replace, or set these variables::
> +
> +   # LEGACY SETTINGS PRIOR LIBVIRT 4.4.0 SEE BELOW! #
> +   log_level = 1
> +   log_filters="1:qemu 3:remote 4:event 3:util.json 3:rpc"
> +   log_outputs="1:file:/var/log/libvirt/libvirtd.log"
> +
> +
> +   # PREFERRED SETTINGS AFTER LIBVIRT 4.4.0 #
> +   log_filters="3:remote 4:event 3:util.json 3:rpc 1:*"
> +   log_outputs="1:file:/var/log/libvirt/libvirtd.log"

You should have the code block marker on a separate line according
to https://libvirt.org/styleguide.html#code-blocks.

That also makes it possible to replicate the original layout of the
page, where each set of variables lived in its own box:

  * find & replace, or set these variables:

::

   # LEGACY SETTINGS PRIOR LIBVIRT 4.4.0 SEE BELOW! #
   log_level = 1
   log_filters="1:qemu 3:remote 4:event 3:util.json 3:rpc"
   log_outputs="1:file:/var/log/libvirt/libvirtd.log"

::

   # PREFERRED SETTINGS AFTER LIBVIRT 4.4.0 #
   log_filters="3:remote 4:event 3:util.json 3:rpc 1:*"
   log_outputs="1:file:/var/log/libvirt/libvirtd.log"

> +In the config variables above, we have set logging level to 1 (debug level),
> +set some filters (to filter out noise), e.g. from rpc only warnings (=level 
> 3)
> +and above will be reported. The logs are saved into
> +``/var/log/libvirt/libvirtd.log``. Since libvirt *4.4.0* log filters

Bold text looks **like this**, not *like this*, in reStructuredText.

Unless you did that on purpose, in which case why not drop the markup
for the version number altogether? :)

[...]
> +In order to change this set, run the same command as root, this time with 
> your own set of filters::
> +
> +   ## LEGACY APPROACH ENUMERATING ALL THE DESIRED MODULES ##
> +   # virt-admin daemon-log-filters "1:util 1:libvirt 1:storage 1:network 
> 1:nodedev 1:qemu"
> +
> +   ## CURRENT APPROACH USING SHELL GLOBBING ##
> +   # virt-admin daemon-log-filters "3:remote 4:util.json 4:rpc 1:*"

These are two separate boxes in the original as well.

[...]
> +Prerequisites
> +-
> +
> +In cases where you want to see details of what is happening, you need to have
> +debugging symbols installed, at least for the package you are trying to 
> debug.
> +Although having debugging symbols for all dependent libraries is usually
> +helpful as well. Usually ``gdb`` will tell you what you need to do in order 
> to
> +get the proper data to your machine when you run it with a binary.
> +
> +=== Example: ===

You forgot to convert this into a reStructuredText section title.

More specifically, it's a subsection of "Prerequisites", so it should
look like

  Example
  ~~~

[...]
> +When the package is installed, we can break on main and run until then (gdb's
> +command ``start`` is perfect for this)::
> +
> +   # gdb $(which libvirtd)
> +   GNU gdb (GDB) Fedora 8.2-6.fc29
> +   ...
> +   Reading symbols from /usr/sbin/libvirtd...Reading symbols from 
> /usr/lib/debug/usr/sbin/libvirtd-4.7.0-1.fc29.i386.debug...done.
> +   done.
> +   (gdb) start
> +   Temporary breakpoint 1 at 0x18fc0: file remote/remote_daemon.c, line 1030.
> +   Starting program: /usr/sbin/libvirtd
> +   Missing separate debuginfos, use: dnf debuginfo-install 
> glibc-2.28-26.fc29.i686
> +   Missing separate debuginfo for /lib/libvirt-lxc.so.0
> +   Try: dnf --enablerepo='*debug*' install 
> 

Re: [libvirt PATCH] docs: Add section about code blocks to styleguide.rst

[Daniel P . Berrangé]

On Thu, Apr 16, 2020 at 06:38:25PM +0200, Andrea Bolognani wrote:
> Signed-off-by: Andrea Bolognani 
> ---
>  docs/styleguide.rst | 12 
>  1 file changed, 12 insertions(+)

Reviewed-by: Daniel P. Berrangé 


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

Re: [libvirt PATCH 1/4] docs: Use consistent style in pci-addresses.rst

[Andrea Bolognani]

On Thu, 2020-04-16 at 12:38 +0100, Daniel P. Berrangé wrote:
> On Thu, Apr 16, 2020 at 01:35:57PM +0200, Andrea Bolognani wrote:
> > On Thu, 2020-04-16 at 12:58 +0200, Ján Tomko wrote:
> > > On a Wednesday in 2020, Andrea Bolognani wrote:
> > > > Indent all code snippets by the same number of spaces, and don't
> > > > embed the :: marker in the line preceding a code block.
> > > 
> > > But why? It's so ugly.
> > 
> > I disagree, and find it much easier on the eye than the alternative.
> > 
> > That's a matter of personal preference, of course, but the fact that
> > all existing reStructuredText files in the repository follow this
> > convention could indicate that I'm not the only one holding this
> > opinion. This change was made for consistency's sake, not to appease
> > my sense of aestethic - althought in this case the two overlap :)
> 
> BTW, we could update  styleguide.rst to mention this preference.

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

-- 
Andrea Bolognani / Red Hat / Virtualization

[libvirt PATCH] docs: Add section about code blocks to styleguide.rst

[Andrea Bolognani]

Signed-off-by: Andrea Bolognani 
---
 docs/styleguide.rst | 12 
 1 file changed, 12 insertions(+)

diff --git a/docs/styleguide.rst b/docs/styleguide.rst
index 2998e90963..3162868fb7 100644
--- a/docs/styleguide.rst
+++ b/docs/styleguide.rst
@@ -24,6 +24,18 @@ Whitespace
 
 Blocks should be indented with 3 spaces, and no tabs
 
+Code blocks
+===
+
+Code blocks should be created using
+
+::
+
+   This is regular text.
+
+   ::
+
+  This is a code block.
 
 Headings
 
-- 
2.25.2

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

[Andrea Bolognani]

On Thu, 2020-04-16 at 17:56 +0200, Boris Fiuczynski wrote:
>  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.

This sentence is unnecessary, since you explain what fid does just
a few lines later.

>  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``).
> +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
> +slot in the sysfs.

I think we need to be a bit more verbose to make sure we don't
confuse the reader, or better yet avoid mentioning "PCI slot"
altogether because the terminology is ambiguous. Something like

  [...] are 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/...``).

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[Cornelia Huck]

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.

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

>  
>  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..." ?

>  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." ?

> -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.)/
*



>  
>  
>  Device assignment

Re: [PATCH] conf: Trivial comment fix

[Leonid Bloch]

On Thu, Apr 16, 2020 at 5:39 PM Andrea Bolognani 
wrote:

> On Thu, 2020-04-16 at 14:24 +0300, Leonid Bloch wrote:
> > Trivial comment fix, reflecting the changes in
> > 4ee2b31804f4d3477ee83bac28d9991afb0c3393.
> >
> > Signed-off-by: Leonid Bloch 
> > ---
> >  src/conf/domain_conf.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Patch
>
>   Reviewed-by: Andrea Bolognani 
>
> and pushed.
>
> Congratulations on your first contribution to libvirt!
>

Thank you! :)


>
> --
> Andrea Bolognani / Red Hat / Virtualization
>
>

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

[Cornelia Huck]

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

> Changing the introduction to bring the idea of this document better across.
> 
> Signed-off-by: Andrea Bolognani 
> Reviewed-by: Boris Fiuczynski 
> ---
>  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

Reviewed-by: Cornelia Huck 

...although I'm not sure about the s-o-b chain here :)

[PATCH 2/2] Avoid unnecessary error messages handling udev events

[Mark Asselstine]

The udev monitor thread "udevEventHandleThread()" will lag the
actual/real view of devices in sysfs as it serially processes udev
monitor events. So for instance if you were to run the following cmd
to create a new veth pair and rename one of the veth endpoints

you might see the following monitor events and real world that looks like

 time
  |create v0 sysfs entry
wake udevEventHandleThread|create v1 sysfs entry
udev_monitor_receive_device(v1-add)   |move v0 sysfs to v2
udevHandleOneDevice(v1)   |
udev_monitor_receive_device(v0-add)   |
udevHandleOneDevice(v0)   | <--- error msgs in 
virNetDevGetLinkInfo()
udev_monitor_receive_device(v2-move)  |  as v0 no longer exists
udevHandleOneDevice(v2)   |
 \/

As you can see the changes in sysfs can take place well before we get
to act on the events in the udevEventHandleThread(), so by the time we
get around to processing the v0 add event, the sysfs entry has been
moved to v2.

To work around this we check if the sysfs entry is valid before
attempting to read it and don't bother trying to read link info if
not. This is safe since we will never read sysfs entries earlier than
it existing, ie. if the entry is not there it has either been removed
in the time since we enumerated the device or something bigger is
busted, in either case, no sysfs entry, no link info. In the case
described above we will eventually get the link info as we work
through the queue of monitor events and get to the 'move' event.

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

Signed-off-by: Mark Asselstine 
---
 src/util/virnetdev.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index b465bdac2e..bf256cbe35 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -2438,6 +2438,17 @@ virNetDevGetLinkInfo(const char *ifname,
 if (virNetDevSysfsFile(, ifname, "operstate") < 0)
 goto cleanup;
 
+/* The device may have been removed or moved by the time we got here.
+ * Obviously attempting to get LinkInfo on a no longer existing device
+ * is useless, so stop processing. If we got here via the udev monitor
+ * a remove or move event will follow and we will be able to get valid
+ * LinkInfo at that time */
+if (!virFileExists(path)) {
+  VIR_WARN("The interface '%s' was removed before we could query it.",
+  ifname);
+goto cleanup;
+}
+
 if (virFileReadAll(path, 1024, ) < 0) {
 virReportSystemError(errno,
  _("unable to read: %s"),
-- 
2.20.1

[PATCH 1/2] node_device_udev: handle move events

[Mark Asselstine]

It is possible and common to rename some devices, this is especially
true for ethernet devices such as veth pairs.

In the udevEventHandleThread() we will be notified of this change but
currently we only process "add", "change" and "remove"
events. Renaming a device such as above results in a "move" event, not
a "remove" followed by and "add" or vise versa. This change will add
the new/destination device to our records but unfortunately there is
no usable mechanism to identify the old/source device to remove it
from the records. So this is admittedly only a partial fix.

Signed-off-by: Mark Asselstine 
---
 src/node_device/node_device_udev.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/node_device/node_device_udev.c 
b/src/node_device/node_device_udev.c
index 8451903e8a..3149de8321 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1499,6 +1499,11 @@ udevHandleOneDevice(struct udev_device *device)
 if (STREQ(action, "remove"))
 return udevRemoveOneDevice(device);
 
+if (STREQ(action, "move")) {
+/* TODO: implement a way of finding and removing the old device */
+return udevAddOneDevice(device);
+}
+
 return 0;
 }
 
-- 
2.20.1

[PATCH 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 
---
 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 0/2] docs: Further adjustments to pci-addresses.rst

[Boris Fiuczynski]

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

 docs/pci-addresses.rst | 24 ++--
 1 file changed, 14 insertions(+), 10 deletions(-)

-- 
2.25.1

[PATCH 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 | 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
 
 
 
-  
+  
 
   
   
 
 
 
-  
+  
 
   
 
@@ -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.
 
 Neither are any changes in the PCI address 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``).
+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
+slot in the sysfs.
 
 
 Device assignment
-- 
2.25.1

Re: [libvirt PATCH 0/3] docs: misc fixes to static asset handling

[Laine Stump]

On 4/16/20 7:53 AM, Daniel P. Berrangé wrote:

When looking at switching libvirt.org to use files output from gitlab CI
I discovered some missing static assets

Daniel P. Berrangé (3):
   docs: fix handling of static assets in build dir
   docs: add missing files to static asset list
   docs: remove old unused favicon file

  docs/32favicon.png | Bin 783 -> 0 bytes
  docs/Makefile.am   |  41 -
  2 files changed, 24 insertions(+), 17 deletions(-)
  delete mode 100644 docs/32favicon.png


It looks correct, and make rpm is successful on my system (after squashing in 
the bit you forgot), so


Reviewed-by: Laine Stump 

Re: [libvirt PATCH 3/3] qemu: block: external snapshot-delete implementation for straightforward cases

[Pavel Mores]

On Fri, Apr 03, 2020 at 05:56:59PM +0200, Pavel Mores wrote:
> On Tue, Mar 31, 2020 at 12:06:15PM +0200, Peter Krempa wrote:
> > On Tue, Mar 31, 2020 at 11:18:29 +0200, Pavel Mores wrote:
> > > +}
> > > +
> > > +if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN)
> > > +topPath = disk->src->path;
> > > +else
> > > +topPath = snapdisk->src->path;
> > 
> > You must not use paths for doing this lookup. Paths at best work for
> > local files only and this would make the code not future proof.
> > 
> > Also you want to verify that:
> > - images you want to merge are actually in the backing chain
> > - the backing chain looks as snapshots describe it (e.g you unplug vda
> >   and plug a different chain back)
> 
> Let me check with you how exactly to perform this verification.
> 
> To retrieve the names of the disks included in a snapshot, I can use its
> virDomainSnapshotDef which contains an array of disks
> (virDomainSnapshotDiskDef's) whose 'name' member can be used to identify 
> disks.
> 
> To get the state of the chain at moment the snapshot was created, I can use 
> the
> 'src' member and walk its 'backingStore' list to examine the whole chain.
> 
> To get the currently running configuration, I assume I can use the names of 
> the
> relevant disks (obtained from the snapshot as mentioned above) to get a
> virDomainDiskDefPtr for each of them, whose 'src' member points to a
> virStorageSource that represents the topmost image in the disk's chain.  From
> there, I can walk the 'backingStore' list to get the other images in the chain
> all the way to the base image.
> 
> The verification succeeds if the disk names and their chains in the snapshot
> and the running config correspond.  Is the above correct?
> 
> If so, I'm still not certain how to compare two virStorageSource's.  How is
> identity of two different virStorageSource instances is established?

After having a closer look into this I'm unfortunately even less clear as to
how to perform the checks mentioned in the review.

In addition to the problem of establishing virStorageSource identity, a similar
problem seems to come up with disks.  They often seem to be identified and
referred to by the target name, however what happens if a snapshot is taken for
'vda', then the disk's target is changed to 'vdb' and now the snapshot is to be
deleted?  Is there a way to find out that the disk is still there, only under a
different name?

And as far as comparing chains goes - I know can retrieve the chain for a
running disk and I can retrieve what the chain looked like at the point a
snapshot was made.  However, how do I establish they are equivalent?  How can I
tell that snapshots in both chains compare identical in the first place?  Is
the snapshot name stable and persistent enough to be useful for this?  Is it
enough for chains to be equivalent that the chain at the point of snapshot
creation be a superset of the currently running chain?  Probably yes, provided
snapshots can't be inserted in the middle of a chain - but could that ever
happen?

Thanks in advance for any help with this!

pvl

[PATCH] bhyve: add missing param to virDomainPCIAddressBusSetModel

[Daniel P . Berrangé]

Fixes build error introduced in

  commit aa15e9259f1f246e69fb9742581ced720c88695d
  Author: Laine Stump 
  Date:   Sun Apr 5 22:40:37 2020 -0400

qemu/conf: set HOTPLUGGABLE connect flag during PCI address set init

Signed-off-by: Daniel P. Berrangé 
---

Pushed as a CI fix

 src/bhyve/bhyve_device.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c
index 13261e9935..fc52280361 100644
--- a/src/bhyve/bhyve_device.c
+++ b/src/bhyve/bhyve_device.c
@@ -71,7 +71,8 @@ bhyveDomainPCIAddressSetCreate(virDomainDefPtr def, unsigned 
int nbuses)
 return NULL;
 
 if (virDomainPCIAddressBusSetModel(>buses[0],
-   VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 
0)
+   VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT,
+   true) < 0)
 goto error;
 
 if (virDomainDeviceInfoIterate(def, bhyveCollectPCIAddress, addrs) < 0)
-- 
2.24.1

Re: [PATCH] docs: add zpci information to pci-addresses.rst

[Boris Fiuczynski]

On 4/16/20 2:21 PM, Andrea Bolognani wrote:

On Thu, 2020-04-16 at 11:46 +0200, Boris Fiuczynski wrote:

On 4/15/20 6:38 PM, Andrea Bolognani wrote:

The idea behind it was to show users that they shouldn't expect the
address in the domain XML to match the one in the guest OS, with a
few real-life examples to illustrate the point. So, I don't think
the details of how exactly zPCI IDs translate to guest-visible PCI
addresses is in scope.

It would be great information to have in some other document, though!
Is there something like that in qemu.git, or in the QEMU wiki? Those
are the places where I would expect it to live, since it's not really
tied to libvirt...


I disagree because fid is a parameter of the pci address just like
domain, bus, slot, function and uid.
Besides the fact that one of the first sentences in the document is
"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."


You're right, the current language is not really explaining the
purpose of the document correctly - it's making it sound like it's
the complete guide to all things PCI addresses, which it's definitely
not intended to be.

What about

   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.

?

Yes, it does bring the documents intention across better.




as a document reader/user of libvirt I would not expect to search around
in other documentation for fid if all other parameters are correlated here.

So how about a short explanatory sentence for fid like:
"The slot for the PCI device in the guest OS is defined by the fid
(function id)."


The document already contains the following sentence:

   [...] the addresses there are generated from the information
   provided via the zpci element (in fact, from the uid).

I'm not sure what "slot" means in the sentence you suggest. It
doesn't seem to be the same as slot in domain:bus:slot.function,
because in the second zPCI example that was removed

   

would result in

   0007:00:00.0 Ethernet controller: Red Hat, Inc. Virtio network device
I added the sentence "Also fid does not define slot or function of the 

PCI address." following the "Note...topology." sentence.


with the fid nowhere to be seen. Can you explain what "slot" means
in this context?
I guess that you now ask to explain it makes it most likely that it 
should be explained here, correct?


The fid (function id) is not used within the PCI address which the guest 
OS generates for the PCI device. Only the uid is used in the PCI address 
to define the PCI domain. PCI bus, slot and function address elements 
are always 0. This might change in the future BUT it would/will still be 
completely unrelated to the PCI attributes bus, slot and function of the 
address element.
The fid is used in the sysfs (/sys/bus/pci/slots/...) to represent the 
PCI card such e.g. you can power on or off the PCI card.

This is what results in guest OS.
root@qemus390x:~# ls /sys/bus/pci/slots/
0003
root@qemus390x:~# cat /sys/bus/pci/devices/0007\:00\:00.0/function_id
0x0003



Anyway, we can tweak the existing sentence to read something like

   [...] the addresses there are generated from the information
   provided via the zpci element: the uid is used as PCI domain, and
   the fid is used as [your explanation here].

[my explanation]
the fid is used as the PCI devices slot in the sysfs.

Does that together with the additional sentence above make more "wacky" 
sense?




Does that sound good?


Sure.
Since I started editing in the pci-addresses file already, I would try 
to also include these changes if you agree to them.

If you want I can also include your changes in the document introduction.


--
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] conf: Trivial comment fix

[Andrea Bolognani]

On Thu, 2020-04-16 at 14:24 +0300, Leonid Bloch wrote:
> Trivial comment fix, reflecting the changes in
> 4ee2b31804f4d3477ee83bac28d9991afb0c3393.
> 
> Signed-off-by: Leonid Bloch 
> ---
>  src/conf/domain_conf.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Patch

  Reviewed-by: Andrea Bolognani 

and pushed.

Congratulations on your first contribution to libvirt!

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt PATCH] docs: fix mistaken description of the read-only socket units

[Andrea Bolognani]

On Thu, 2020-04-16 at 14:19 +0100, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé 
> ---
>  docs/daemons.rst | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [RFC] Adding docker driver to libvirt

[Dmitry Nesterenko]

What format does the API take ?  Is it a plain REST API we will
call via the existing libcurl dependancy in libvirt ?

Yes of course. It is just what I think - plain REST API by https requests and 
already existing in project libcurl.

Dmitry

From: Daniel P. Berrangé 
Sent: Thursday, April 16, 2020 4:49 PM
To: Dmitry Nesterenko 
Cc: Nikolay Shirokovskiy ; Martin Kletzander 
; libvir-list@redhat.com ; Dmitry 
Mishin 
Subject: Re: [RFC] Adding docker driver to libvirt

On Thu, Apr 16, 2020 at 01:47:39PM +, Dmitry Nesterenko wrote:
>
> > The container based drivers in libvirt have been a bit of a square-peg /
> > round-hole thing. Given that we have a couple of them already (LXC,
> > OpenVZ, VZ), I wouldn't say no to adding a docker one too. The only
> > real issue is having people willing to do the work to implement it and
> > then maintain it thereafter.
> >
> > Describing the scope of the desired work is probably useful
>
> Hi all!
>
> I am that man who will do this work. I think it can be stateless
> driver like hyperv driver from libvirt. Most of calls to the driver
> will bring remote call to docker host by native docker API. So my
> first implementation of docker driver will iclude all calls those
> have realized for hyperv driver now.

What format does the API take ?  Is it a plain REST API we will
call via the existing libcurl dependancy in libvirt ?

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: [RFC] Adding docker driver to libvirt

[Daniel P . Berrangé]

On Thu, Apr 16, 2020 at 01:47:39PM +, Dmitry Nesterenko wrote:
> 
> > The container based drivers in libvirt have been a bit of a square-peg /
> > round-hole thing. Given that we have a couple of them already (LXC,
> > OpenVZ, VZ), I wouldn't say no to adding a docker one too. The only
> > real issue is having people willing to do the work to implement it and
> > then maintain it thereafter.
> >
> > Describing the scope of the desired work is probably useful
> 
> Hi all!
> 
> I am that man who will do this work. I think it can be stateless
> driver like hyperv driver from libvirt. Most of calls to the driver
> will bring remote call to docker host by native docker API. So my
> first implementation of docker driver will iclude all calls those
> have realized for hyperv driver now.

What format does the API take ?  Is it a plain REST API we will
call via the existing libcurl dependancy in libvirt ?

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

[libvirt PATCH] util: Fix virDaemonForkIntoBackground

[Jiri Denemark]

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(-)

diff --git a/src/util/virdaemon.c b/src/util/virdaemon.c
index 99530fd146..6182ca3c21 100644
--- a/src/util/virdaemon.c
+++ b/src/util/virdaemon.c
@@ -20,7 +20,9 @@
 
 #include 
 
+#include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -30,8 +32,6 @@
 #include "virfile.h"
 #include "virlog.h"
 #include "viralloc.h"
-#include "virprocess.h"
-#include "vircommand.h"
 
 #include "configmake.h"
 
@@ -44,7 +44,7 @@ virDaemonForkIntoBackground(const char *argv0)
 if (virPipeQuiet(statuspipe) < 0)
 return -1;
 
-pid_t pid = virFork();
+pid_t pid = fork();
 switch (pid) {
 case 0:
 {
@@ -73,7 +73,7 @@ virDaemonForkIntoBackground(const char *argv0)
 if (setsid() < 0)
 goto cleanup;
 
-nextpid = virFork();
+nextpid = fork();
 switch (nextpid) {
 case 0: /* grandchild */
 return statuspipe[1];
@@ -97,14 +97,15 @@ virDaemonForkIntoBackground(const char *argv0)
 default:
 {
 /* parent */
-int exitstatus = 0;
+int got, exitstatus = 0;
 int ret;
 char status;
 
 VIR_FORCE_CLOSE(statuspipe[1]);
 
 /* We wait to make sure the first child forked successfully */
-if (virProcessWait(pid, , 0) < 0 ||
+if ((got = waitpid(pid, , 0)) < 0 ||
+got != pid ||
 exitstatus != 0) {
 goto error;
 }
-- 
2.26.1

Re: [RFC] Adding docker driver to libvirt

[Dmitry Nesterenko]

> The container based drivers in libvirt have been a bit of a square-peg /
> round-hole thing. Given that we have a couple of them already (LXC,
> OpenVZ, VZ), I wouldn't say no to adding a docker one too. The only
> real issue is having people willing to do the work to implement it and
> then maintain it thereafter.
>
> Describing the scope of the desired work is probably useful

Hi all!

I am that man who will do this work. I think it can be stateless driver like 
hyperv driver from libvirt. Most of calls to the driver will bring remote call 
to docker host by native docker API. So my first implementation of docker 
driver will iclude all calls those have realized for hyperv driver now.

Dmitry


From: Nikolay Shirokovskiy 
Sent: Thursday, April 16, 2020 3:18 PM
To: Daniel P. Berrangé 
Cc: Martin Kletzander ; libvir-list@redhat.com 
; Dmitry Nesterenko ; 
Dmitry Mishin 
Subject: Re: [RFC] Adding docker driver to libvirt

Adding to cc again, now keeping mailing list

On 15.04.2020 16:09, Daniel P. Berrangé wrote:
> On Tue, Apr 14, 2020 at 09:56:24AM +0300, nshirokovskiy wrote:
>>
>>
>> On 12.04.2020 12:39, Martin Kletzander wrote:
>>> On Thu, Apr 09, 2020 at 03:30:11PM +0300, nshirokovskiy wrote:
 Hi, all.

 Does it make sense to add such a driver? I can't say I have a big picture
 of docker functionality in mind but at least container lifecycle management
 and container networking are common to both.

>>>
>>> I think we had something in virt-tools that was able to pull an image from
>>> docker hub and run it with lxc.  Or was it part of sandbox?  I don't know.
>>>
>>> Anyway, what would be the benefit of that?
>>>
>>
>> We wanted to add Windows containers to the libvirt API. They are available
>> under docker API thus the idea to add a docker driver. The docker itself
>> uses some API to manage Windows containers but this API lacks documentation
>> thus again the willingness to use just docker API to bring Windows containers
>> to libvirt.
>
> The container based drivers in libvirt have been a bit of a square-peg /
> round-hole thing. Given that we have a couple of them already (LXC,
> OpenVZ, VZ), I wouldn't say no to adding a docker one too. The only
> real issue is having people willing to do the work to implement it and
> then maintain it thereafter.
>
> Describing the scope of the desired work is probably useful. With docker,
> a big part is in the image download/listing/upload and build process.
> The container lifecycle is only a small part of the API coverage. The
> image parts have no mapping in libvirt, and I'm not sure whether we
> should to expand libvirt scope to that too.
>
> Regards,
> Daniel
>

Re: [PATCH 2/5] qemu: Add free and copy function for qemuDomainJobInfo and use it

[Eric Blake]

On 4/16/20 4:55 AM, Peter Krempa wrote:

In order to add a string to qemuDomainJobInfo we must ensure that it's
freed and copied properly. Add helpers to copy and free the structure
and adjust the code to use them properly for the new semantics.

Additionally also allocation is changed to g_new0 as it includes the
type and thus it's very easy to grep for all the allocations of a given
type.

Signed-off-by: Peter Krempa 
---


Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 1/5] remote: remoteDispatchDomainGetJobStats: Encode typed parameter strings

[Eric Blake]

On 4/16/20 4:55 AM, Peter Krempa wrote:

String typed parameter values were introduced in v0.9.7-30-g40624d32fb.
virDomainGetJobStats was introduced in v1.0.2-239-g4dd00f4238 so all
clients already support typed parameter stings at that time thus we can
enable it unconditionally.

Signed-off-by: Peter Krempa 
---
  src/remote/remote_daemon_dispatch.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH] qemuDomainDefPostParse: Fail if unable to fill machine type

[Pavel Mores]

On Thu, Apr 16, 2020 at 02:33:33PM +0200, Michal Privoznik wrote:
> Previously, we used virCapabilitiesDomainDataLookup() to fill
> machine type in post parse callback if none was provided in the
> domain XML. If machine type couldn't be filled in an error was
> reported. After 4a4132b4625 we've changed it to
> virQEMUCapsGetPreferredMachine() which returns NULL, but we no
> longer report an error and proceed with the post parse callbacks
> processing. This may lead to a crash because the code later on
> assumes def->os.machine is not NULL.
> 
> Fixes: 4a4132b4625
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_domain.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 91e234d644..98ffd23a71 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -4973,6 +4973,14 @@ qemuDomainDefPostParse(virDomainDefPtr def,
>  if (!def->os.machine) {
>  const char *machine = virQEMUCapsGetPreferredMachine(qemuCaps,
>   def->virtType);
> +if (!machine) {
> +virReportError(VIR_ERR_INVALID_ARG,
> +   _("could not get preferred machine for %s 
> type=%s"),
> +   def->emulator,
> +   virDomainVirtTypeToString(def->virtType));
> +return -1;
> +}
> +
>  def->os.machine = g_strdup(machine);
>  }
>  
> -- 
> 2.25.3
> 

Reviewed-by: Pavel Mores 

[libvirt PATCH] docs: fix mistaken description of the read-only socket units

[Daniel P . Berrangé]

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

diff --git a/docs/daemons.rst b/docs/daemons.rst
index 13c244de7b..e0b95105e4 100644
--- a/docs/daemons.rst
+++ b/docs/daemons.rst
@@ -145,7 +145,7 @@ Libvirt ships a number of unit files for controlling 
``libvirtd``:
   UNIX socket ``/var/run/libvirt/libvirt-sock``. This socket is recommended to
   be started on boot by default.
 
-* ``libvirtd-ro.socket`` - the unit file corresponding to the main read-write
+* ``libvirtd-ro.socket`` - the unit file corresponding to the main read-only
   UNIX socket ``/var/run/libvirt/libvirt-sock-ro``. This socket is recommended
   to be started on boot by default.
 
@@ -316,7 +316,7 @@ Libvirt ships a number of unit files for controlling 
``virt${DRIVER}d``:
   recommended to be started on boot by default.
 
 * ``virt${DRIVER}d-ro.socket`` - the unit file corresponding to the main
-  read-write UNIX socket ``/var/run/libvirt/virt${DRIVER}d-sock-ro``. This
+  read-only UNIX socket ``/var/run/libvirt/virt${DRIVER}d-sock-ro``. This
   socket is recommended to be started on boot by default.
 
 * ``virt${DRIVER}d-admin.socket`` - the unit file corresponding to the
@@ -471,7 +471,7 @@ Libvirt ships a number of unit files for controlling 
``virtproxyd``:
   UNIX socket ``/var/run/libvirt/libvirt-sock``. This socket is recommended to
   be started on boot by default.
 
-* ``virtproxyd-ro.socket`` - the unit file corresponding to the main read-write
+* ``virtproxyd-ro.socket`` - the unit file corresponding to the main read-only
   UNIX socket ``/var/run/libvirt/libvirt-sock-ro``. This socket is recommended
   to be started on boot by default.
 
-- 
2.25.2

Re: [PATCH 4/5] doc: rng schemas and html doc for rbd namespace attribute

[Jason Dillaman]

On Wed, Apr 8, 2020 at 5:06 AM Han Han  wrote:

> Signed-off-by: Han Han 
> ---
>  docs/formatdomain.html.in | 5 +
>  docs/schemas/domaincommon.rng | 3 +++
>  2 files changed, 8 insertions(+)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index d56600dc..06e1a7ee 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -3118,6 +3118,11 @@
>(Since 6.2.0)
>
>
> +  For protocol rbd, an optional attribute
> +  namespace specifies the namespace of a rbd
> pool.
> +  (Since 6.3.0 and QEMU 5.0.0)
> +  
> +
>For "iscsi" (since 1.0.4), the
>name attribute may include a logical unit
> number,
>separated from the target's name by a slash (e.g.,
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index dcf2e09d..294ade34 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1762,6 +1762,9 @@
>  
>
>  
> +
> +  
> +
>

My only concern about this patchset is how the "name" attribute is
overloaded (historically) to include both the RBD pool and image name. Now
if the RBD namespace is separated out to its own optional attribute, it
requires constructing an invalid "/" name attribute.  Can
the pool be separated out to its own attribute as well  (i.e. ) and keep a deprecated
backwards compatible "name='pool/image'" attribute that doesn't support
namespaces? The QAPI already has the pool and image as separate attributes.


>  
>
>  
> --
> 2.25.0
>
>

-- 
Jason

[PATCH] qemuDomainDefPostParse: Fail if unable to fill machine type

[Michal Privoznik]

Previously, we used virCapabilitiesDomainDataLookup() to fill
machine type in post parse callback if none was provided in the
domain XML. If machine type couldn't be filled in an error was
reported. After 4a4132b4625 we've changed it to
virQEMUCapsGetPreferredMachine() which returns NULL, but we no
longer report an error and proceed with the post parse callbacks
processing. This may lead to a crash because the code later on
assumes def->os.machine is not NULL.

Fixes: 4a4132b4625

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_domain.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 91e234d644..98ffd23a71 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4973,6 +4973,14 @@ qemuDomainDefPostParse(virDomainDefPtr def,
 if (!def->os.machine) {
 const char *machine = virQEMUCapsGetPreferredMachine(qemuCaps,
  def->virtType);
+if (!machine) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _("could not get preferred machine for %s type=%s"),
+   def->emulator,
+   virDomainVirtTypeToString(def->virtType));
+return -1;
+}
+
 def->os.machine = g_strdup(machine);
 }
 
-- 
2.25.3

Re: [libvirt PATCH 4/4] docs: Remove one example from pci-addresses.rst

[Andrea Bolognani]

On Thu, 2020-04-16 at 12:29 +0200, Boris Fiuczynski wrote:
> I suggest to use the zpci addressing from the removed example because it 
> outlines more clearly the differences in the parameters.
> Something like the example below:
> 
> For s390x machines, PCI addresses are handled yet differently. No 
> topology information is relayed in the PCI addresses; instead, the fid 
> and uid elements of the zpci device convey information. In the simplest 
> case, the following XML snippet
> 
> 
> 
>
>
> function='0x0'>
>  
>
> 
> 
>
>
>
> function='0x0'>
>  
>
> 
> 
> will result in the following in a Linux guest:
> 
> 0007:00:00.0 Ethernet controller: Red Hat, Inc. Virtio network device

You're right, this example is more illustrative. Care to submit a
patch changing it?

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH] docs: add zpci information to pci-addresses.rst

[Andrea Bolognani]

On Thu, 2020-04-16 at 11:46 +0200, Boris Fiuczynski wrote:
> On 4/15/20 6:38 PM, Andrea Bolognani wrote:
> > The idea behind it was to show users that they shouldn't expect the
> > address in the domain XML to match the one in the guest OS, with a
> > few real-life examples to illustrate the point. So, I don't think
> > the details of how exactly zPCI IDs translate to guest-visible PCI
> > addresses is in scope.
> > 
> > It would be great information to have in some other document, though!
> > Is there something like that in qemu.git, or in the QEMU wiki? Those
> > are the places where I would expect it to live, since it's not really
> > tied to libvirt...
> 
> I disagree because fid is a parameter of the pci address just like 
> domain, bus, slot, function and uid.
> Besides the fact that one of the first sentences in the document is
> "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."

You're right, the current language is not really explaining the
purpose of the document correctly - it's making it sound like it's
the complete guide to all things PCI addresses, which it's definitely
not intended to be.

What about

  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.

?

> as a document reader/user of libvirt I would not expect to search around 
> in other documentation for fid if all other parameters are correlated here.
> 
> So how about a short explanatory sentence for fid like:
> "The slot for the PCI device in the guest OS is defined by the fid 
> (function id)."

The document already contains the following sentence:

  [...] the addresses there are generated from the information
  provided via the zpci element (in fact, from the uid).

I'm not sure what "slot" means in the sentence you suggest. It
doesn't seem to be the same as slot in domain:bus:slot.function,
because in the second zPCI example that was removed

  

would result in

  0007:00:00.0 Ethernet controller: Red Hat, Inc. Virtio network device

with the fid nowhere to be seen. Can you explain what "slot" means
in this context?

Anyway, we can tweak the existing sentence to read something like

  [...] the addresses there are generated from the information
  provided via the zpci element: the uid is used as PCI domain, and
  the fid is used as [your explanation here].

Does that sound good?

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [RFC] Adding docker driver to libvirt

[Nikolay Shirokovskiy]

Adding to cc again, now keeping mailing list

On 15.04.2020 16:09, Daniel P. Berrangé wrote:
> On Tue, Apr 14, 2020 at 09:56:24AM +0300, nshirokovskiy wrote:
>>
>>
>> On 12.04.2020 12:39, Martin Kletzander wrote:
>>> On Thu, Apr 09, 2020 at 03:30:11PM +0300, nshirokovskiy wrote:
 Hi, all.

 Does it make sense to add such a driver? I can't say I have a big picture
 of docker functionality in mind but at least container lifecycle management
 and container networking are common to both.

>>>
>>> I think we had something in virt-tools that was able to pull an image from
>>> docker hub and run it with lxc.  Or was it part of sandbox?  I don't know.
>>>
>>> Anyway, what would be the benefit of that?
>>>
>>
>> We wanted to add Windows containers to the libvirt API. They are available
>> under docker API thus the idea to add a docker driver. The docker itself
>> uses some API to manage Windows containers but this API lacks documentation
>> thus again the willingness to use just docker API to bring Windows containers
>> to libvirt.
> 
> The container based drivers in libvirt have been a bit of a square-peg /
> round-hole thing. Given that we have a couple of them already (LXC,
> OpenVZ, VZ), I wouldn't say no to adding a docker one too. The only
> real issue is having people willing to do the work to implement it and
> then maintain it thereafter.
> 
> Describing the scope of the desired work is probably useful. With docker,
> a big part is in the image download/listing/upload and build process.
> The container lifecycle is only a small part of the API coverage. The
> image parts have no mapping in libvirt, and I'm not sure whether we
> should to expand libvirt scope to that too.
> 
> Regards,
> Daniel
> 

Re: [libvirt PATCH 3/3] docs: remove old unused favicon file

[Daniel P . Berrangé]

On Thu, Apr 16, 2020 at 12:53:45PM +0100, Daniel P. Berrangé wrote:
> The use of 32favicon.png was removed when the new favicons were
> introduced in
> 
>   commit 40cb5581c4ace6c4a5b68990aaac4dff1b656054
>   Author: Daniel P. Berrangé 
>   Date:   Wed Jul 26 18:22:11 2017 +0100
> 
> docs: add full set of "favicon" files to support modern clients
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  docs/32favicon.png | Bin 783 -> 0 bytes
>  1 file changed, 0 insertions(+), 0 deletions(-)
>  delete mode 100644 docs/32favicon.png

Sorry, didn't "git add" a chunk - consider this patch to also contain

diff --git a/docs/Makefile.am b/docs/Makefile.am
index f7b565c250..ce3d296b19 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -151,7 +151,6 @@ logofilesdir = $(HTML_DIR)/logos
 logofiles_DATA = $(logofiles)
 
 assets = \
-  32favicon.png \
   android-chrome-192x192.png \
   android-chrome-256x256.png \
   apple-touch-icon.png \


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

[libvirt PATCH 3/3] docs: remove old unused favicon file

[Daniel P . Berrangé]

The use of 32favicon.png was removed when the new favicons were
introduced in

  commit 40cb5581c4ace6c4a5b68990aaac4dff1b656054
  Author: Daniel P. Berrangé 
  Date:   Wed Jul 26 18:22:11 2017 +0100

docs: add full set of "favicon" files to support modern clients

Signed-off-by: Daniel P. Berrangé 
---
 docs/32favicon.png | Bin 783 -> 0 bytes
 1 file changed, 0 insertions(+), 0 deletions(-)
 delete mode 100644 docs/32favicon.png

diff --git a/docs/32favicon.png b/docs/32favicon.png
deleted file mode 100644
index 
1af7c70d71996d00317384d37def563b1c25..
GIT binary patch
literal 0
HcmV?d1

literal 783
zcmV+q1MvKbP)2bhPj!A!?^3SX2m>
zF50PsLQ3eZP)Q^x(Su83#Pn?=v$M{uGdlAzpzqCl|M{r~q5x%++}&$UPQi2$uu
zi`g8hRElpz%l=#q?VYFT6#~ECKb=mAGAs#T{Yq$WqMY5#1Jx#pK*@uh-lDG<`nbWHMP#kjZ2e
zrG(q<7SG+^Wo8m%e)$To%|tnn=_jEqTFg&3Ga~F$KOpMBY$=2sO^TW(p|FBOU
z+`9wtS89~dI4+XHtN*F_os`~@kDrLw>$O^~!{M;eXs8xSB9Sl}jqK21Fjy>>d_FHy
z2iw8vbdJYkyWP$XT`pHHmy1Lq;c!@;2;jTNw5Y+2+iJFpLBq(yY#m%gU_A%0(a!Tnk^VqS0tD7^Emluh%Qd0_vpXG@3vl&}=qGqtRe6=ytm{n+<#@
zoY`!qPR?bsS(C{Gi|ml5X$)yW7upD#m8wBMLMZe3+;@XK3=xVV_6y|M^J6hk2M}cM
z|a1sVNgQ!8;WR``DoIBuIhZHZC4FD1?lHFSXO#`jtxPhRJ2!MuzC>0T)?XB;N
z2t}f%)`L|mNt{*W{Lcn_PMi~7vP41h3X@g3mHEGm#cuNHrQ5f}`@u%%3l34;|MKST
zpQ9sj0-5gfIfOpJ`q%2AIvowg0hJV%$hYbt8-!MQTC*Hy1@TXS0RXFX9Y3%?2
N002ovPDHLkV1hUQZOi}w

-- 
2.25.2

[libvirt PATCH 0/3] docs: misc fixes to static asset handling

[Daniel P . Berrangé]

When looking at switching libvirt.org to use files output from gitlab CI
I discovered some missing static assets

Daniel P. Berrangé (3):
  docs: fix handling of static assets in build dir
  docs: add missing files to static asset list
  docs: remove old unused favicon file

 docs/32favicon.png | Bin 783 -> 0 bytes
 docs/Makefile.am   |  41 -
 2 files changed, 24 insertions(+), 17 deletions(-)
 delete mode 100644 docs/32favicon.png

-- 
2.25.2

[libvirt PATCH 2/3] docs: add missing files to static asset list

[Daniel P . Berrangé]

The various favicon files were missing from the favicon list, so never
installed, as was an example code diagram.

Signed-off-by: Daniel P. Berrangé 
---
 docs/Makefile.am | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/docs/Makefile.am b/docs/Makefile.am
index c6518d653e..f7b565c250 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -152,19 +152,29 @@ logofiles_DATA = $(logofiles)
 
 assets = \
   32favicon.png \
+  android-chrome-192x192.png \
+  android-chrome-256x256.png \
+  apple-touch-icon.png \
   architecture.gif \
+  browserconfig.xml \
+  favicon.ico \
+  favicon-16x16.png \
+  favicon-32x32.png \
   generic.css \
   libvirt.css \
   libvirt-daemon-arch.png \
   libvirt-driver-arch.png \
   libvirt-object-model.png \
+  libvirt-virConnect-example.png \
   main.css \
+  manifest.json \
   migration-managed-direct.png \
   migration-managed-p2p.png \
   migration-native.png \
   migration-tunnel.png \
   migration-unmanaged-direct.png \
   mobile.css \
+  mstile-150x150.png \
   node.gif \
   $(NULL)
 
-- 
2.25.2

[libvirt PATCH 1/3] docs: fix handling of static assets in build dir

[Daniel P . Berrangé]

We previously added a hack to symlink CSS files from the source dir into
the build dir, to allow the website to be browsed locally. We should
have also done this for any images.

This change merges several variables into one "$(assets)" so that we
treat all static files in the root dir the same way.

Signed-off-by: Daniel P. Berrangé 
---
 docs/Makefile.am | 31 ++-
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/docs/Makefile.am b/docs/Makefile.am
index 6860efc888..c6518d653e 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -48,7 +48,7 @@ vpathhack:
do \
  test -e $$dir || ln -s $(srcdir)/$$dir $$dir ; \
done
-   @for file in $(css); \
+   @for file in $(assets); \
do \
  test -e $$file || ln -s $(srcdir)/$$file $$file ; \
done
@@ -58,7 +58,7 @@ clean-local:
do \
  rm -f $$dir ; \
done
-   for file in $(css); \
+   for file in $(assets); \
do \
  rm -f $$file ; \
done
@@ -104,12 +104,6 @@ apipng = \
 apirefdir = $(HTML_DIR)/html
 apiref_DATA = $(apihtml) $(apiadminhtml) $(apiqemuhtml) $(apilxchtml) $(apipng)
 
-css = \
-  generic.css \
-  libvirt.css \
-  mobile.css \
-  main.css
-
 javascript = \
   js/main.js \
   $(NULL)
@@ -156,20 +150,23 @@ logofiles = \
 logofilesdir = $(HTML_DIR)/logos
 logofiles_DATA = $(logofiles)
 
-png = \
+assets = \
   32favicon.png \
+  architecture.gif \
+  generic.css \
+  libvirt.css \
   libvirt-daemon-arch.png \
   libvirt-driver-arch.png \
   libvirt-object-model.png \
+  main.css \
   migration-managed-direct.png \
   migration-managed-p2p.png \
   migration-native.png \
   migration-tunnel.png \
-  migration-unmanaged-direct.png
-
-gif = \
-  architecture.gif \
-  node.gif
+  migration-unmanaged-direct.png \
+  mobile.css \
+  node.gif \
+  $(NULL)
 
 internals_html_in = \
   $(patsubst $(srcdir)/%,%,$(wildcard $(srcdir)/internals/*.html.in))
@@ -326,7 +323,7 @@ dot_html = \
   $(dot_rst_html_in:%.html.in=%.html)
 
 htmldir = $(HTML_DIR)
-html_DATA = $(css) $(png) $(gif) $(dot_html)
+html_DATA = $(assets) $(dot_html)
 
 apidir = $(pkgdatadir)/api
 api_DATA = \
@@ -351,8 +348,8 @@ schema_DATA = $(wildcard $(srcdir)/schemas/*.rng)
 EXTRA_DIST= \
   site.xsl subsite.xsl newapi.xsl page.xsl \
   wrapstring.xsl \
-  $(dot_html_in) $(dot_rst) $(gif) $(apipng) \
-  $(fig) $(png) $(css) \
+  $(dot_html_in) $(dot_rst) $(apipng) \
+  $(fig) $(assets) \
   $(javascript) $(logofiles) \
   $(internals_html_in) $(internals_rst) $(fonts) \
   $(kbase_html_in) $(kbase_rst) \
-- 
2.25.2

Re: [libvirt PATCH 1/4] docs: Use consistent style in pci-addresses.rst

[Daniel P . Berrangé]

On Thu, Apr 16, 2020 at 01:35:57PM +0200, Andrea Bolognani wrote:
> On Thu, 2020-04-16 at 12:58 +0200, Ján Tomko wrote:
> > On a Wednesday in 2020, Andrea Bolognani wrote:
> > > Indent all code snippets by the same number of spaces, and don't
> > > embed the :: marker in the line preceding a code block.
> > 
> > But why? It's so ugly.
> 
> I disagree, and find it much easier on the eye than the alternative.
> 
> That's a matter of personal preference, of course, but the fact that
> all existing reStructuredText files in the repository follow this
> convention could indicate that I'm not the only one holding this
> opinion. This change was made for consistency's sake, not to appease
> my sense of aestethic - althought in this case the two overlap :)

BTW, we could update  styleguide.rst to mention this preference.


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

Re: [libvirt PATCH 1/4] docs: Use consistent style in pci-addresses.rst

[Andrea Bolognani]

On Thu, 2020-04-16 at 12:58 +0200, Ján Tomko wrote:
> On a Wednesday in 2020, Andrea Bolognani wrote:
> > Indent all code snippets by the same number of spaces, and don't
> > embed the :: marker in the line preceding a code block.
> 
> But why? It's so ugly.

I disagree, and find it much easier on the eye than the alternative.

That's a matter of personal preference, of course, but the fact that
all existing reStructuredText files in the repository follow this
convention could indicate that I'm not the only one holding this
opinion. This change was made for consistency's sake, not to appease
my sense of aestethic - althought in this case the two overlap :)

More objectively, the fact that the indicator is on its own line
makes it stand out more and make it obvious at a glance that what
you're looking at is a code block rather than a block quote: this is
more important when looking at diffs than when working in the editor,
but even with syntax highlighting turned on that small splash of
color at the end of a line is easier to miss than when it forms a
line of its own.

Additionally, the fact that

  Here's some code::

and

  Here's some code ::

render differently makes this form more error-prone.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt-php] libvirt_domain_interface_addresses causing segfault

[Fernando Casas Schössow]

Excellent and thanks again the rapid response.

Fernando

On jue, abr 16, 2020 at 1:22 PM, Michal Privoznik  
wrote:

On 4/16/20 12:32 PM, Fernando Casas Schössow wrote:
>

Pushed fix for both problems.

Michal

[PATCH] conf: Trivial comment fix

[Leonid Bloch]

Trivial comment fix, reflecting the changes in
4ee2b31804f4d3477ee83bac28d9991afb0c3393.

Signed-off-by: Leonid Bloch 
---
 src/conf/domain_conf.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index aad3f82db7..7d8f1aa31b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2084,7 +2084,7 @@ struct _virDomainTimerDef {
 virDomainTimerCatchupDef catchup;
 
 /* track is only valid for name='platform|rtc' */
-int track;  /* host|guest */
+int track;  /* boot|guest|wall */
 
 /* frequency & mode are only valid for name='tsc' */
 unsigned long frequency; /* in Hz, unspecified = 0 */
-- 
2.26.1

Re: [libvirt-php] libvirt_domain_interface_addresses causing segfault

[Michal Privoznik]

On 4/16/20 12:32 PM, Fernando Casas Schössow wrote:
>

Pushed fix for both problems.

Michal

Re: [libvirt PATCH 1/4] docs: Use consistent style in pci-addresses.rst

[Daniel P . Berrangé]

On Thu, Apr 16, 2020 at 12:58:43PM +0200, Ján Tomko wrote:
> On a Wednesday in 2020, Andrea Bolognani wrote:
> > Indent all code snippets by the same number of spaces, and don't
> > embed the :: marker in the line preceding a code block.
> 
> But why? It's so ugly.

That aligns with the style we've used in all other rst docs we've
done so far.

Ideally we wouldn't use a bare "::" anyway - we really ought to be
using  ".. code-block:: $LANGUAGE" to instruct the renderer how to
highlight the code snippets. Our current website generator doesn't
use this, but hopefully we will in future

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

FYI: upstream bug reporting now using https://gitlab.com/libvirt

[Daniel P . Berrangé]

In keeping with our general aim to consolidate libvirt project infrastructure
services on GitLab, we now wish to have issues for upstream libvirt projects
reported on the appropriate https://gitlab.com/libvirt/ repository issue
tracker. For further details see:

   https://libvirt.org/bugs.html

Bugs against OS distro provided binary packages should continue to be filed
against the OS distros' own bug trackers unless the problem is known to also
affect current upstream libvirt versions.

Note that as part of this change, the "Virtualization Tools" component on
bugzilla.redhat.com has been updated to remove the ability to file new
bugs against libvirt related components.  Existing bugs there still exist
in bugzilla, but new bugs can only be filed against gitlab.com.

Over time we hope to triage old bugs and close those that are either
invalid, or those not likely to be addressed given current maintainer
resources available.

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

Re: [libvirt PATCH 1/4] docs: Use consistent style in pci-addresses.rst

[Ján Tomko]

On a Wednesday in 2020, Andrea Bolognani wrote:

Indent all code snippets by the same number of spaces, and don't
embed the :: marker in the line preceding a code block.


But why? It's so ugly.

Jano



This commit is best viewed with 'git show -w'.

Signed-off-by: Andrea Bolognani 
---
docs/pci-addresses.rst | 60 ++
1 file changed, 32 insertions(+), 28 deletions(-)


signature.asc
Description: PGP signature

Re: [libvirt-php] libvirt_domain_interface_addresses() only returns only on IP address for NICs with multiple IPs

[Michal Privoznik]

On 4/16/20 9:08 AM, Fernando Casas Schössow wrote:

Forgot to add the output of the equivalent virsh command for the same VM:

Name MAC address Protocol Address
--- 


lo 00:00:00:00:00:00 ipv4 127.0.0.1/8
lo 00:00:00:00:00:00 ipv6 ::1/128
eth0 52:54:00:12:b9:22 ipv4 192.168.7.31/26
eth0 52:54:00:12:b9:22 ipv6 fe80::5054:ff:fe12:b922/64
eth1 52:54:00:8e:b0:35 ipv4 192.168.1.34/24
eth1 52:54:00:8e:b0:35 ipv6 fe80::5054:ff:fe8e:b035/64


Thanks,

Fernando

On jue, abr 16, 2020 at 12:58 AM, Fernando Casas Schössow 
 wrote:

Hi,

While using the function libvirt_domain_interface_addresses() with 
qemu guest agent as the source, for interfaces with more than one IP 
address, the function will only return one IP.


Example code:

$netdetails = libvirt_domain_interface_addresses($vmres, 2);
foreach ($netdetails as $nic) {
print_r($nic);
}


Example output:

Array
(
   [name] => eth0
   [hwaddr] => 52:54:00:12:b9:22
   [naddrs] => 2
   [addrs] => Array
   (
   [addr] => fe80::5054:ff:fe12:b922
   [prefix] => 64
   [type] => 1
   )


Ah, this is another clear bug in PHP bindings. The problem is we use 
associative array for 'addrs'. As you can see, 'naddrs' is 2, so the API 
returned 2 IP addresses (IPv4 and IPv6 ones). So we've constructed an 
associative array for IPv4 and set $nic['addrs'] to it. Then we did the 
same for IPv6. This is obviously wrong. Let me see if I can fix it.


Michal

Re: [libvirt-php] libvirt_domain_interface_addresses causing segfault

[Fernando Casas Schössow]

That was fast!

Thanks a lot Michal.

Kind regards,

Fernando

On jue, abr 16, 2020 at 12:21 PM, Michal Privoznik 
 wrote:

On 4/16/20 12:08 AM, Fernando Casas Schössow wrote:
I think I found the problem and it seems to affect only Windows 
guests. I'm not sure about the cause tough.


Possible causes:

1- Network interface names containing white spaces.
2- One of the interfaces not reporting a MAC address.


Actually, it's the latter. MAC address can be NULL even on Linux 
(e.g. various point to point interfaces don't have a MAC address). 
I'm pushing the fix as we speak.


Thanks for the report.

Michal

Re: [libvirt PATCH 4/4] docs: Remove one example from pci-addresses.rst

[Boris Fiuczynski]

On 4/15/20 7:47 PM, Cornelia Huck wrote:

On Wed, 15 Apr 2020 19:31:36 +0200
Andrea Bolognani  wrote:


The idea behind this document is to show, with actual examples,
that users should not expect PCI addresses in the domain XML and
in the guest OS to match.

The first zPCI example already serves this purpose perfectly, so
in the interest of keeping the page as brief and easy to digest
as possible the second one is removed.

Signed-off-by: Andrea Bolognani 
---
  docs/pci-addresses.rst | 19 ---
  1 file changed, 19 deletions(-)

diff --git a/docs/pci-addresses.rst b/docs/pci-addresses.rst
index 86a41df6ce..1d2dc8e5fc 100644
--- a/docs/pci-addresses.rst
+++ b/docs/pci-addresses.rst
@@ -204,25 +204,6 @@ 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``).
  
-Therefore, replacing the virtio-net device definition with the following XML

-snippet
-
-::
-
-  
-
-
-
-  
-
-  
-
-will yield the following result in a Linux guest:
-
-::
-
-  0007:00:00.0 Ethernet controller: Red Hat, Inc. Virtio network device
-
I suggest to use the zpci addressing from the removed example because it 
outlines more clearly the differences in the parameters.

Something like the example below:

For s390x machines, PCI addresses are handled yet differently. No 
topology information is relayed in the PCI addresses; instead, the fid 
and uid elements of the zpci device convey information. In the simplest 
case, the following XML snippet




  
  
  function='0x0'>


  


  
  
  
  function='0x0'>


  


will result in the following in a Linux guest:

0007:00:00.0 Ethernet controller: Red Hat, Inc. Virtio network device

The slot for the PCI device in the guest OS is defined by the fid 
(function id).


  
  Device assignment

  =


Hm, should that rather go somewhere else? What I wanted to show is "you
can have the same PCI address in the XML and still get a different PCI
address in the guest, if you change the zpci values", as that might be
another source of confusion.




--
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: [libvirt-php] libvirt_domain_interface_addresses causing segfault

[Michal Privoznik]

On 4/16/20 12:08 AM, Fernando Casas Schössow wrote:
I think I found the problem and it seems to affect only Windows guests. 
I'm not sure about the cause tough.


Possible causes:

1- Network interface names containing white spaces.
2- One of the interfaces not reporting a MAC address.


Actually, it's the latter. MAC address can be NULL even on Linux (e.g. 
various point to point interfaces don't have a MAC address). I'm pushing 
the fix as we speak.


Thanks for the report.

Michal

[PATCH 3/5] API: Add VIR_DOMAIN_JOB_ERRMSG domain job statistics field

[Peter Krempa]

In some cases it's useful to report the error which caused the domain
job to fail. Add an optional field for holding the error message so that
it can be later retrieved from statistics of a completed job.

Add the field name macro and code for extracting it in virsh.

Signed-off-by: Peter Krempa 
---
 include/libvirt/libvirt-domain.h | 9 +
 tools/virsh-domain.c | 8 
 2 files changed, 17 insertions(+)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index b440818ec2..f129e6a1af 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -3612,6 +3612,15 @@ typedef enum {
  */
 # define VIR_DOMAIN_JOB_SUCCESS "success"

+/**
+ * VIR_DOMAIN_JOB_ERRMSG:
+ *
+ * virDomainGetJobStats field: Present only in statistics for a completed job.
+ * Optional error message for a failed job.
+ */
+# define VIR_DOMAIN_JOB_ERRMSG "errmsg"
+
+
 /**
  * VIR_DOMAIN_JOB_DISK_TEMP_USED:
  * virDomainGetJobStats field: current usage of temporary disk space for the
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index d52eb7bc2f..f2dc41cb31 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6228,6 +6228,7 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd)
 unsigned long long value;
 unsigned int flags = 0;
 int ivalue;
+const char *svalue;
 int op;
 int rc;
 size_t i;
@@ -6508,6 +6509,13 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd)
 vshPrint(ctl, "%-17s %-.3lf %s\n", _("Temporary disk space total:"), 
val, unit);
 }

+if ((rc = virTypedParamsGetString(params, nparams, VIR_DOMAIN_JOB_ERRMSG,
+  )) < 0) {
+goto save_error;
+} else if (rc == 1) {
+vshPrint(ctl, "%-17s %s\n", _("Error message:"), svalue);
+}
+
 ret = true;

  cleanup:
-- 
2.26.0

[PATCH 5/5] backup: Store error message for failed backups

[Peter Krempa]

If a backup job fails midway it's hard to figure out what happened as
it's running asynchronous. Use the VIR_DOMAIN_JOB_ERRMSG job statistics
field to pass through the error from the first failed backup-blockjob
so that both the consumer of the virDomainGetJobStats and the
corresponding event can see the error.

event 'job-completed' for domain backup-test:
operation: 9
time_elapsed: 46
disk_total: 104857600
disk_processed: 10158080
disk_remaining: 94699520
success: 0
errmsg: No space left on device

virsh domjobinfo backup-test --completed --anystats
Job type: Failed
Operation:Backup
Time elapsed: 46   ms
File processed:   9.688 MiB
File remaining:   90.312 MiB
File total:   100.000 MiB
Error message:No space left on device

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

Signed-off-by: Peter Krempa 
---
 src/conf/backup_conf.c   |  1 +
 src/conf/backup_conf.h   |  2 ++
 src/qemu/qemu_backup.c   | 10 --
 src/qemu/qemu_backup.h   |  1 +
 src/qemu/qemu_blockjob.c |  2 +-
 src/qemu/qemu_domain.c   |  4 
 6 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c
index 64c8f6cc09..c5677cdb05 100644
--- a/src/conf/backup_conf.c
+++ b/src/conf/backup_conf.c
@@ -65,6 +65,7 @@ virDomainBackupDefFree(virDomainBackupDefPtr def)
 return;

 g_free(def->incremental);
+g_free(def->errmsg);
 virStorageNetHostDefFree(1, def->server);

 for (i = 0; i < def->ndisks; i++) {
diff --git a/src/conf/backup_conf.h b/src/conf/backup_conf.h
index 672fd52ee7..b5685317c5 100644
--- a/src/conf/backup_conf.h
+++ b/src/conf/backup_conf.h
@@ -79,6 +79,8 @@ struct _virDomainBackupDef {
 unsigned long long push_total;
 unsigned long long pull_tmp_used;
 unsigned long long pull_tmp_total;
+
+char *errmsg; /* error message of failed sub-blockjob */
 };

 typedef enum {
diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index 03d34c9378..80fc5d77f8 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -650,6 +650,7 @@ qemuBackupJobTerminate(virDomainObjPtr vm,
 priv->job.completed->stats.backup.tmp_total = priv->backup->pull_tmp_total;

 priv->job.completed->status = jobstatus;
+priv->job.completed->errmsg = g_strdup(priv->backup->errmsg);

 qemuDomainEventEmitJobCompleted(priv->driver, vm);

@@ -951,6 +952,7 @@ void
 qemuBackupNotifyBlockjobEnd(virDomainObjPtr vm,
 virDomainDiskDefPtr disk,
 qemuBlockjobState state,
+const char *errmsg,
 unsigned long long cur,
 unsigned long long end,
 int asyncJob)
@@ -964,8 +966,8 @@ qemuBackupNotifyBlockjobEnd(virDomainObjPtr vm,
 virDomainBackupDefPtr backup = priv->backup;
 size_t i;

-VIR_DEBUG("vm: '%s', disk:'%s', state:'%d'",
-  vm->def->name, disk->dst, state);
+VIR_DEBUG("vm: '%s', disk:'%s', state:'%d' errmsg:'%s'",
+  vm->def->name, disk->dst, state, NULLSTR(errmsg));

 if (!backup)
 return;
@@ -985,6 +987,10 @@ qemuBackupNotifyBlockjobEnd(virDomainObjPtr vm,
 backup->push_total += end;
 }

+/* record first error message */
+if (!backup->errmsg)
+backup->errmsg = g_strdup(errmsg);
+
 for (i = 0; i < backup->ndisks; i++) {
 virDomainBackupDiskDefPtr backupdisk = backup->disks + i;

diff --git a/src/qemu/qemu_backup.h b/src/qemu/qemu_backup.h
index 3321ba0b6f..b19c3bf1c9 100644
--- a/src/qemu/qemu_backup.h
+++ b/src/qemu/qemu_backup.h
@@ -38,6 +38,7 @@ void
 qemuBackupNotifyBlockjobEnd(virDomainObjPtr vm,
 virDomainDiskDefPtr disk,
 qemuBlockjobState state,
+const char *errmsg,
 unsigned long long cur,
 unsigned long long end,
 int asyncJob);
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 2032c0c1c5..b9eecd3f98 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -1435,7 +1435,7 @@ qemuBlockJobProcessEventConcludedBackup(virQEMUDriverPtr 
driver,
 g_autoptr(qemuBlockStorageSourceAttachData) backend = NULL;
 g_autoptr(virJSONValue) actions = NULL;

-qemuBackupNotifyBlockjobEnd(vm, job->disk, newstate,
+qemuBackupNotifyBlockjobEnd(vm, job->disk, newstate, job->errmsg,
 progressCurrent, progressTotal, asyncJob);

 if (job->data.backup.store &&
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index dba222dde5..22bb808177 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -836,6 +836,10 @@ qemuDomainBackupJobInfoToParams(qemuDomainJobInfoPtr 
jobInfo,
 VIR_DOMAIN_JOB_SUCCESS) < 0)
 

[PATCH 2/5] qemu: Add free and copy function for qemuDomainJobInfo and use it

[Peter Krempa]

In order to add a string to qemuDomainJobInfo we must ensure that it's
freed and copied properly. Add helpers to copy and free the structure
and adjust the code to use them properly for the new semantics.

Additionally also allocation is changed to g_new0 as it includes the
type and thus it's very easy to grep for all the allocations of a given
type.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_backup.c   |  5 ++---
 src/qemu/qemu_domain.c   | 26 +-
 src/qemu/qemu_domain.h   |  8 +++
 src/qemu/qemu_driver.c   | 37 
 src/qemu/qemu_migration.c| 16 ++
 src/qemu/qemu_migration_cookie.c | 10 -
 6 files changed, 61 insertions(+), 41 deletions(-)

diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index 5d18720f53..03d34c9378 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -641,9 +641,8 @@ qemuBackupJobTerminate(virDomainObjPtr vm,

 qemuDomainJobInfoUpdateTime(priv->job.current);

-g_free(priv->job.completed);
-priv->job.completed = g_new0(qemuDomainJobInfo, 1);
-*priv->job.completed = *priv->job.current;
+g_clear_pointer(>job.completed, qemuDomainJobInfoFree);
+priv->job.completed = qemuDomainJobInfoCopy(priv->job.current);

 priv->job.completed->stats.backup.total = priv->backup->push_total;
 priv->job.completed->stats.backup.transferred = 
priv->backup->push_transferred;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 91e234d644..13b1c4e402 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -305,6 +305,23 @@ qemuDomainDisableNamespace(virDomainObjPtr vm,
 }


+void
+qemuDomainJobInfoFree(qemuDomainJobInfoPtr info)
+{
+g_free(info);
+}
+
+
+qemuDomainJobInfoPtr
+qemuDomainJobInfoCopy(qemuDomainJobInfoPtr info)
+{
+qemuDomainJobInfoPtr ret = g_new0(qemuDomainJobInfo, 1);
+
+memcpy(ret, info, sizeof(*info));
+
+return ret;
+}
+
 void
 qemuDomainEventEmitJobCompleted(virQEMUDriverPtr driver,
 virDomainObjPtr vm)
@@ -385,7 +402,7 @@ qemuDomainObjResetAsyncJob(qemuDomainObjPrivatePtr priv)
 job->spiceMigrated = false;
 job->dumpCompleted = false;
 VIR_FREE(job->error);
-VIR_FREE(job->current);
+g_clear_pointer(>current, qemuDomainJobInfoFree);
 qemuMigrationParamsFree(job->migParams);
 job->migParams = NULL;
 job->apiFlags = 0;
@@ -415,8 +432,8 @@ qemuDomainObjFreeJob(qemuDomainObjPrivatePtr priv)
 {
 qemuDomainObjResetJob(priv);
 qemuDomainObjResetAsyncJob(priv);
-VIR_FREE(priv->job.current);
-VIR_FREE(priv->job.completed);
+g_clear_pointer(>job.current, qemuDomainJobInfoFree);
+g_clear_pointer(>job.completed, qemuDomainJobInfoFree);
 virCondDestroy(>job.cond);
 virCondDestroy(>job.asyncCond);
 }
@@ -6291,8 +6308,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
   qemuDomainAsyncJobTypeToString(asyncJob),
   obj, obj->def->name);
 qemuDomainObjResetAsyncJob(priv);
-if (VIR_ALLOC(priv->job.current) < 0)
-goto cleanup;
+priv->job.current = g_new0(qemuDomainJobInfo, 1);
 priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE;
 priv->job.asyncJob = asyncJob;
 priv->job.asyncOwner = virThreadSelfID();
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index cf19f4d101..c7f28b04c2 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -177,6 +177,14 @@ struct _qemuDomainJobInfo {
 qemuDomainMirrorStats mirrorStats;
 };

+void
+qemuDomainJobInfoFree(qemuDomainJobInfoPtr info);
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuDomainJobInfo, qemuDomainJobInfoFree);
+
+qemuDomainJobInfoPtr
+qemuDomainJobInfoCopy(qemuDomainJobInfoPtr info);
+
 typedef struct _qemuDomainJobObj qemuDomainJobObj;
 typedef qemuDomainJobObj *qemuDomainJobObjPtr;
 struct _qemuDomainJobObj {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 31f199fdef..63e92b84c6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3746,7 +3746,7 @@ qemuDumpToFd(virQEMUDriverPtr driver,
 if (detach)
 priv->job.current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP;
 else
-VIR_FREE(priv->job.current);
+g_clear_pointer(>job.current, qemuDomainJobInfoFree);

 if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
 return -1;
@@ -13598,16 +13598,16 @@ static int
 qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
   virDomainObjPtr vm,
   bool completed,
-  qemuDomainJobInfoPtr jobInfo)
+  qemuDomainJobInfoPtr *jobInfo)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
 int ret = -1;

+*jobInfo = NULL;
+
 if (completed) {
 if (priv->job.completed && 

[PATCH 4/5] qemu: domain: Add 'errmsg' field to qemuDomainJobInfo

[Peter Krempa]

The field can be used by jobs to add an optional error message to a
completed (failed) job.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_domain.c | 3 +++
 src/qemu/qemu_domain.h | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 13b1c4e402..dba222dde5 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -308,6 +308,7 @@ qemuDomainDisableNamespace(virDomainObjPtr vm,
 void
 qemuDomainJobInfoFree(qemuDomainJobInfoPtr info)
 {
+g_free(info->errmsg);
 g_free(info);
 }

@@ -319,6 +320,8 @@ qemuDomainJobInfoCopy(qemuDomainJobInfoPtr info)

 memcpy(ret, info, sizeof(*info));

+ret->errmsg = g_strdup(info->errmsg);
+
 return ret;
 }

diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index c7f28b04c2..639d27d8a5 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -175,6 +175,8 @@ struct _qemuDomainJobInfo {
 qemuDomainBackupStats backup;
 } stats;
 qemuDomainMirrorStats mirrorStats;
+
+char *errmsg; /* optional error message for failed completed jobs */
 };

 void
-- 
2.26.0

[PATCH 1/5] remote: remoteDispatchDomainGetJobStats: Encode typed parameter strings

[Peter Krempa]

String typed parameter values were introduced in v0.9.7-30-g40624d32fb.
virDomainGetJobStats was introduced in v1.0.2-239-g4dd00f4238 so all
clients already support typed parameter stings at that time thus we can
enable it unconditionally.

Signed-off-by: Peter Krempa 
---
 src/remote/remote_daemon_dispatch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/remote/remote_daemon_dispatch.c 
b/src/remote/remote_daemon_dispatch.c
index c5506c2e11..5d1c6971c0 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -5505,7 +5505,7 @@ remoteDispatchDomainGetJobStats(virNetServerPtr server 
G_GNUC_UNUSED,
 REMOTE_DOMAIN_JOB_STATS_MAX,
 (virTypedParameterRemotePtr *) 
>params.params_val,
 >params.params_len,
-0) < 0)
+VIR_TYPED_PARAM_STRING_OKAY) < 0)
 goto cleanup;

 rv = 0;
-- 
2.26.0

[PATCH 0/5] backup: Preserve error of failed backup job

[Peter Krempa]

Aid in problem identification by storing the error for users as it
wasn't accessible besides from logs.

Peter Krempa (5):
  remote: remoteDispatchDomainGetJobStats: Encode typed parameter
strings
  qemu: Add free and copy function for qemuDomainJobInfo and use it
  API: Add VIR_DOMAIN_JOB_ERRMSG domain job statistics field
  qemu: domain: Add 'errmsg' field to qemuDomainJobInfo
  backup: Store error message for failed backups

 include/libvirt/libvirt-domain.h|  9 +++
 src/conf/backup_conf.c  |  1 +
 src/conf/backup_conf.h  |  2 ++
 src/qemu/qemu_backup.c  | 15 
 src/qemu/qemu_backup.h  |  1 +
 src/qemu/qemu_blockjob.c|  2 +-
 src/qemu/qemu_domain.c  | 33 +
 src/qemu/qemu_domain.h  | 10 
 src/qemu/qemu_driver.c  | 37 +++--
 src/qemu/qemu_migration.c   | 16 ++---
 src/qemu/qemu_migration_cookie.c| 10 
 src/remote/remote_daemon_dispatch.c |  2 +-
 tools/virsh-domain.c|  8 +++
 13 files changed, 101 insertions(+), 45 deletions(-)

-- 
2.26.0

Re: [PATCH] docs: add zpci information to pci-addresses.rst

[Boris Fiuczynski]

On 4/15/20 6:38 PM, Andrea Bolognani wrote:

On Wed, 2020-04-15 at 16:45 +0200, Cornelia Huck wrote:

On Wed, 15 Apr 2020 16:23:46 +0200
Boris Fiuczynski  wrote:

Once I understand you confusion above I will provide a patch...


I'd say just go ahead.


While I appreciate the sentiment, I would rather not have an
excessive amount of detail added to this page.

The idea behind it was to show users that they shouldn't expect the
address in the domain XML to match the one in the guest OS, with a
few real-life examples to illustrate the point. So, I don't think
the details of how exactly zPCI IDs translate to guest-visible PCI
addresses is in scope.

It would be great information to have in some other document, though!
Is there something like that in qemu.git, or in the QEMU wiki? Those
are the places where I would expect it to live, since it's not really
tied to libvirt...



I disagree because fid is a parameter of the pci address just like 
domain, bus, slot, function and uid.

Besides the fact that one of the first sentences in the document is
"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."
as a document reader/user of libvirt I would not expect to search around 
in other documentation for fid if all other parameters are correlated here.


So how about a short explanatory sentence for fid like:
"The slot for the PCI device in the guest OS is defined by the fid 
(function id)."


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

[libvirt-php] libvirt_domain_interface_addresses causing segfault

[Fernando Casas Schössow]

Hi,

While executing libvirt_domain_interface_addresses() with the 
qemu-agent running in the guest as the source, the function causes PHP 
to segfault. If I change the source to either dhcp or arp, the function 
executes correctly.


Example:

$netdetails = libvirt_domain_interface_addresses($vmres, 0); <-- 
executes correctly (dhcp source)
$netdetails = libvirt_domain_interface_addresses($vmres, 1); <-- 
segfault (qemu agent source)
$netdetails = libvirt_domain_interface_addresses($vmres, 2); <-- 
executes correctly (arp source)


I tested the equivalent function using virsh against the same host and 
it executes correctly with any source.
I have a php-cgi core file that I can share if that may help on finding 
the problem.


gdb output:

GNU gdb (Debian 9.1-2) 9.1
Copyright (C) 2020 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 


This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
.
Find the GDB manual and other documentation resources online at:
   .

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /usr/bin/php-cgi...
(No debugging symbols found in /usr/bin/php-cgi)
[New LWP 2594]
[Thread debugging using libthread_db enabled]
Using host libthread_db library 
"/lib/x86_64-linux-gnu/libthread_db.so.1".

Core was generated by `php-cgi index.php'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 __strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:85
85  ../sysdeps/x86_64/multiarch/../strlen.S: No such file or directory.
(gdb) bt
#0 __strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:85
#1 0x55c6a4ceacd2 in add_assoc_string_ex ()
#2 0x7f4d36175253 in ?? () from /usr/lib/php/20180731/libvirt-php.so
#3 0x55c6a4d6b477 in execute_ex ()
#4 0x55c6a4d705c7 in zend_execute ()
#5 0x55c6a4ce8ed4 in zend_execute_scripts ()
#6 0x55c6a4c8be38 in php_execute_script ()
#7 0x55c6a4b53da8 in ?? ()
#8 0x7f4d39efce0b in __libc_start_main (main=0x55c6a4b51cf0, 
argc=2, argv=0x7fff35718588, init=, fini=,
   rtld_fini=, stack_end=0x7fff35718578) at 
../csu/libc-start.c:308

#9 0x55c6a4b544ca in _start ()


System information:

OS: Debian Bullseye
PHP version (fast cgi): 7.3.15
Libvirt PHP version: 0.5.5
Libvirt version: 6.0.0

KVM host libvirt version: 5.9.0

If you need any other information please let me know.

Thanks.

Fernando

[libvirt-php] libvirt_domain_interface_addresses() only returns only on IP address for NICs with multiple IPs

[Fernando Casas Schössow]

Hi,

While using the function libvirt_domain_interface_addresses() with qemu 
guest agent as the source, for interfaces with more than one IP 
address, the function will only return one IP.


Example code:

$netdetails = libvirt_domain_interface_addresses($vmres, 2);
foreach ($netdetails as $nic) {
print_r($nic);
}


Example output:

Array
(
   [name] => eth0
   [hwaddr] => 52:54:00:12:b9:22
   [naddrs] => 2
   [addrs] => Array
   (
   [addr] => fe80::5054:ff:fe12:b922
   [prefix] => 64
   [type] => 1
   )

)
Array
(
   [name] => eth1
   [hwaddr] => 52:54:00:8e:b0:35
   [naddrs] => 2
   [addrs] => Array
   (
   [addr] => fe80::5054:ff:fe8e:b035
   [prefix] => 64
   [type] => 1
   )

)

As you can see for this guest the function returns two interfaces (eth0 
and eth1). For both interfaces naddrs => 2 which is correct because 
both interfaces have an ipv4 and an ipv6 address but the addrs array 
only contains the ipv6 address information while the ipv4 address 
information is missing.


System information:

OS: Debian Bullseye
PHP version (fast cgi): 7.3.15
Libvirt PHP version: 0.5.5
Libvirt version: 6.0.0

KVM host libvirt version: 5.9.0

If you need any other information please let me know.

Thanks.

Fernando

Re: [libvirt-php] libvirt_domain_interface_addresses causing segfault

[Fernando Casas Schössow]

I think I found the problem and it seems to affect only Windows guests. 
I'm not sure about the cause tough.


Possible causes:

1- Network interface names containing white spaces.
2- One of the interfaces not reporting a MAC address.

As an example here is the output of the equivalent virsh command for a 
Windows guest:


Name MAC address Protocol Address
---
Ethernet 2 52:54:00:99:b5:62 ipv6 fe80::71fd:2467:8755:cb0b%13/64
Ethernet 2 52:54:00:99:b5:62 ipv4 192.168.0.77/26
Loopback Pseudo-Interface 1 ipv6 ::1/128
Loopback Pseudo-Interface 1 ipv4 127.0.0.1/8
isatap.{974B556C-9BAF-4A84-A1B1-9D1FDABEC29B} 00:00:00:00:00:00 ipv6 
fe80::5efe:192.168.7.21%12/128


Please note the name containing white spaces: "Ethernet 2", "Loopback 
Pseudo-Interface 1". And also interface "Loopback Pseudo-Interface 1" 
not reporting a MAC address.


Hope this additional information helps.

Thanks.

Fernando

On mié, abr 15, 2020 at 11:41 PM, Fernando Casas Schössow 
 wrote:

Hi,

While executing libvirt_domain_interface_addresses() with the 
qemu-agent running in the guest as the source, the function causes 
PHP to segfault. If I change the source to either dhcp or arp, the 
function executes correctly.


Example:

$netdetails = libvirt_domain_interface_addresses($vmres, 0); <-- 
executes correctly (dhcp source)
$netdetails = libvirt_domain_interface_addresses($vmres, 1); <-- 
segfault (qemu agent source)
$netdetails = libvirt_domain_interface_addresses($vmres, 2); <-- 
executes correctly (arp source)


I tested the equivalent function using virsh against the same host 
and it executes correctly with any source.
I have a php-cgi core file that I can share if that may help on 
finding the problem.


gdb output:

GNU gdb (Debian 9.1-2) 9.1
Copyright (C) 2020 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 


This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
.
Find the GDB manual and other documentation resources online at:
.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /usr/bin/php-cgi...
(No debugging symbols found in /usr/bin/php-cgi)
[New LWP 2594]
[Thread debugging using libthread_db enabled]
Using host libthread_db library 
"/lib/x86_64-linux-gnu/libthread_db.so.1".

Core was generated by `php-cgi index.php'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 __strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:85
85  ../sysdeps/x86_64/multiarch/../strlen.S: No such file or directory.
(gdb) bt
#0 __strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:85
#1 0x55c6a4ceacd2 in add_assoc_string_ex ()
#2 0x7f4d36175253 in ?? () from 
/usr/lib/php/20180731/libvirt-php.so

#3 0x55c6a4d6b477 in execute_ex ()
#4 0x55c6a4d705c7 in zend_execute ()
#5 0x55c6a4ce8ed4 in zend_execute_scripts ()
#6 0x55c6a4c8be38 in php_execute_script ()
#7 0x55c6a4b53da8 in ?? ()
#8 0x7f4d39efce0b in __libc_start_main (main=0x55c6a4b51cf0, 
argc=2, argv=0x7fff35718588, init=, fini=out>,
rtld_fini=, stack_end=0x7fff35718578) at 
../csu/libc-start.c:308

#9 0x55c6a4b544ca in _start ()


System information:

OS: Debian Bullseye
PHP version (fast cgi): 7.3.15
Libvirt PHP version: 0.5.5
Libvirt version: 6.0.0

KVM host libvirt version: 5.9.0

If you need any other information please let me know.

Thanks.

Fernando

Re: [libvirt-php] libvirt_domain_interface_addresses() only returns only on IP address for NICs with multiple IPs

[Fernando Casas Schössow]

Forgot to add the output of the equivalent virsh command for the same 
VM:


Name MAC address Protocol Address
---
lo 00:00:00:00:00:00 ipv4 127.0.0.1/8
lo 00:00:00:00:00:00 ipv6 ::1/128
eth0 52:54:00:12:b9:22 ipv4 192.168.7.31/26
eth0 52:54:00:12:b9:22 ipv6 fe80::5054:ff:fe12:b922/64
eth1 52:54:00:8e:b0:35 ipv4 192.168.1.34/24
eth1 52:54:00:8e:b0:35 ipv6 fe80::5054:ff:fe8e:b035/64


Thanks,

Fernando

On jue, abr 16, 2020 at 12:58 AM, Fernando Casas Schössow 
 wrote:

Hi,

While using the function libvirt_domain_interface_addresses() with 
qemu guest agent as the source, for interfaces with more than one IP 
address, the function will only return one IP.


Example code:

$netdetails = libvirt_domain_interface_addresses($vmres, 2);
foreach ($netdetails as $nic) {
print_r($nic);
}


Example output:

Array
(
   [name] => eth0
   [hwaddr] => 52:54:00:12:b9:22
   [naddrs] => 2
   [addrs] => Array
   (
   [addr] => fe80::5054:ff:fe12:b922
   [prefix] => 64
   [type] => 1
   )

)
Array
(
   [name] => eth1
   [hwaddr] => 52:54:00:8e:b0:35
   [naddrs] => 2
   [addrs] => Array
   (
   [addr] => fe80::5054:ff:fe8e:b035
   [prefix] => 64
   [type] => 1
   )

)

As you can see for this guest the function returns two interfaces 
(eth0 and eth1). For both interfaces naddrs => 2 which is correct 
because both interfaces have an ipv4 and an ipv6 address but the 
addrs array only contains the ipv6 address information while the ipv4 
address information is missing.


System information:

OS: Debian Bullseye
PHP version (fast cgi): 7.3.15
Libvirt PHP version: 0.5.5
Libvirt version: 6.0.0

KVM host libvirt version: 5.9.0

If you need any other information please let me know.

Thanks.

Fernando

Re: [PATCH v2] qemu: Revoke access to mirror on failed blockcopy

[Pavel Mores]

On Thu, Apr 16, 2020 at 09:42:46AM +0200, Michal Privoznik wrote:
> When preparing to do a blockcopy, the mirror image is modified so
> that QEMU can access it. For instance, the mirror has seclabels
> set, if it is a NVMe disk it is detached from the host and so on.
> And usually, the restore is done upon successful finish of the
> blockcopy operation. But, if something fails then we need to
> explicitly revoke the access to the mirror image (and thus
> reattach NVMe disk back to the host).
> 
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1822538
> 
> Signed-off-by: Michal Privoznik 
> ---
> 
> diff to v1:
> - Fix call of qemuDomainStorageSourceChainAccessRevoke() so that it is
> called even if data = crdata = NULL.
> 
>  src/qemu/qemu_driver.c | 21 +
>  1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 31f199fdef..dfe0adaad8 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -17950,6 +17950,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
>  virDomainDiskDefPtr disk = NULL;
>  int ret = -1;
>  bool need_unlink = false;
> +bool need_revoke = false;
>  g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
>  const char *format = NULL;
>  bool mirror_reuse = !!(flags & VIR_DOMAIN_BLOCK_COPY_REUSE_EXT);
> @@ -18124,6 +18125,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
>  
>  if (qemuDomainStorageSourceChainAccessAllow(driver, vm, mirror) < 0)
>  goto endjob;
> +need_revoke = true;
>  
>  if (blockdev) {
>  if (mirror_reuse) {
> @@ -18232,14 +18234,17 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
>  
>   endjob:
>  if (ret < 0 &&
> -virDomainObjIsActive(vm) &&
> -(data || crdata)) {
> -qemuDomainObjEnterMonitor(driver, vm);
> -if (data)
> -qemuBlockStorageSourceChainDetach(priv->mon, data);
> -if (crdata)
> -qemuBlockStorageSourceAttachRollback(priv->mon, 
> crdata->srcdata[0]);
> -ignore_value(qemuDomainObjExitMonitor(driver, vm));
> +virDomainObjIsActive(vm)) {
> +if (data || crdata) {
> +qemuDomainObjEnterMonitor(driver, vm);
> +if (data)
> +qemuBlockStorageSourceChainDetach(priv->mon, data);
> +if (crdata)
> +qemuBlockStorageSourceAttachRollback(priv->mon, 
> crdata->srcdata[0]);
> +ignore_value(qemuDomainObjExitMonitor(driver, vm));
> +}
> +if (need_revoke)
> +qemuDomainStorageSourceChainAccessRevoke(driver, vm, mirror);
>  }
>  if (need_unlink && virStorageFileUnlink(mirror) < 0)
>  VIR_WARN("%s", _("unable to remove just-created copy target"));
> -- 
> 2.25.3
> 

Reviewed-by: Pavel Mores 

Re: [libvirt PATCH 4/4] docs: Remove one example from pci-addresses.rst

[Cornelia Huck]

On Thu, 16 Apr 2020 09:28:58 +0200
Andrea Bolognani  wrote:

> On Wed, 2020-04-15 at 19:47 +0200, Cornelia Huck wrote:
> > On Wed, 15 Apr 2020 19:31:36 +0200
> > Andrea Bolognani  wrote:  
> > > -Therefore, replacing the virtio-net device definition with the following 
> > > XML
> > > -snippet
> > > -
> > > -::
> > > -
> > > -  
> > > -
> > > -
> > > - > > function='0x3'>
> > > -  
> > > -
> > > -  
> > > -
> > > -will yield the following result in a Linux guest:
> > > -
> > > -::
> > > -
> > > -  0007:00:00.0 Ethernet controller: Red Hat, Inc. Virtio network device  
> > 
> > Hm, should that rather go somewhere else? What I wanted to show is "you
> > can have the same PCI address in the XML and still get a different PCI
> > address in the guest, if you change the zpci values", as that might be
> > another source of confusion.  
> 
> I think the previous example, specifically the last bit where you
> explain how changing the PCI address completely in the domain XML
> would not change what the guest OS sees because the latter is derived
> from uid and fid, already drives the point home. It's really two
> sides of the same coin.
> 
> Additionally, as I explained elsewhere, this document is not meant to
> list every possible situation in which PCI addresses in the domain
> XML and in the guest OS are out of sync, but merely to show that such
> cases exist. It's valuable to mention the zPCI scenario, but we don't
> need to show more than one variation of it in my opinion.
> 

Fair enough.

Reviewed-by: Cornelia Huck 

Re: [libvirt] [PATCH] qemu: fix hang in p2p + xbzrle compression + parallel migration

[Jiri Denemark]

On Thu, Apr 16, 2020 at 12:44:51 +0800, Lin Ma wrote:
> When we do parallel migration, The multifd-channels migration parameter
> needs to be set on the destination side as well before incoming migration
> URI, unless we accept the default number of connections(2).
> 
> Usually, This can be correctly handled by libvirtd. But in this case if
> we use p2p + xbzrle compression without parameter '--comp-xbzrle-cache',
> qemuMigrationParamsDump returns too early, The corresponding migration 
> parameter will not be set on the destination side, It results QEMU hangs.
> 
> Reproducer:
> virsh migrate --live --p2p --comp-methods xbzrle \
> --parallel --parallel-connections 3 GUEST qemu+ssh://dsthost/system
> 
> or
> 
> virsh migrate --live --p2p --compressed \
> --parallel --parallel-connections 3 GUEST qemu+ssh://dsthost/system
> 
> Signed-off-by: Lin Ma 
> ---
>  src/qemu/qemu_migration_params.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_migration_params.c 
> b/src/qemu/qemu_migration_params.c
> index dd5e2ce1b6..810199370f 100644
> --- a/src/qemu/qemu_migration_params.c
> +++ b/src/qemu/qemu_migration_params.c
> @@ -630,7 +630,6 @@ qemuMigrationParamsDump(qemuMigrationParamsPtr migParams,
>  if (migParams->compMethods == 1ULL << QEMU_MIGRATION_COMPRESS_XBZRLE &&
>  !migParams->params[QEMU_MIGRATION_PARAM_XBZRLE_CACHE_SIZE].set) {
>  *flags |= VIR_MIGRATE_COMPRESSED;
> -return 0;
>  }
>  
>  for (i = 0; i < QEMU_MIGRATION_COMPRESS_LAST; ++i) {

Oops, looks like a leftover from the past when xbzrle-cache-size was
the only parameter.

Reviewed-by: Jiri Denemark 

[PATCH v2] qemu: Revoke access to mirror on failed blockcopy

[Michal Privoznik]

When preparing to do a blockcopy, the mirror image is modified so
that QEMU can access it. For instance, the mirror has seclabels
set, if it is a NVMe disk it is detached from the host and so on.
And usually, the restore is done upon successful finish of the
blockcopy operation. But, if something fails then we need to
explicitly revoke the access to the mirror image (and thus
reattach NVMe disk back to the host).

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1822538

Signed-off-by: Michal Privoznik 
---

diff to v1:
- Fix call of qemuDomainStorageSourceChainAccessRevoke() so that it is
called even if data = crdata = NULL.

 src/qemu/qemu_driver.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 31f199fdef..dfe0adaad8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17950,6 +17950,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
 virDomainDiskDefPtr disk = NULL;
 int ret = -1;
 bool need_unlink = false;
+bool need_revoke = false;
 g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 const char *format = NULL;
 bool mirror_reuse = !!(flags & VIR_DOMAIN_BLOCK_COPY_REUSE_EXT);
@@ -18124,6 +18125,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
 
 if (qemuDomainStorageSourceChainAccessAllow(driver, vm, mirror) < 0)
 goto endjob;
+need_revoke = true;
 
 if (blockdev) {
 if (mirror_reuse) {
@@ -18232,14 +18234,17 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
 
  endjob:
 if (ret < 0 &&
-virDomainObjIsActive(vm) &&
-(data || crdata)) {
-qemuDomainObjEnterMonitor(driver, vm);
-if (data)
-qemuBlockStorageSourceChainDetach(priv->mon, data);
-if (crdata)
-qemuBlockStorageSourceAttachRollback(priv->mon, 
crdata->srcdata[0]);
-ignore_value(qemuDomainObjExitMonitor(driver, vm));
+virDomainObjIsActive(vm)) {
+if (data || crdata) {
+qemuDomainObjEnterMonitor(driver, vm);
+if (data)
+qemuBlockStorageSourceChainDetach(priv->mon, data);
+if (crdata)
+qemuBlockStorageSourceAttachRollback(priv->mon, 
crdata->srcdata[0]);
+ignore_value(qemuDomainObjExitMonitor(driver, vm));
+}
+if (need_revoke)
+qemuDomainStorageSourceChainAccessRevoke(driver, vm, mirror);
 }
 if (need_unlink && virStorageFileUnlink(mirror) < 0)
 VIR_WARN("%s", _("unable to remove just-created copy target"));
-- 
2.25.3

Re: [libvirt PATCH 4/4] docs: Remove one example from pci-addresses.rst

[Andrea Bolognani]

On Wed, 2020-04-15 at 19:47 +0200, Cornelia Huck wrote:
> On Wed, 15 Apr 2020 19:31:36 +0200
> Andrea Bolognani  wrote:
> > -Therefore, replacing the virtio-net device definition with the following 
> > XML
> > -snippet
> > -
> > -::
> > -
> > -  
> > -
> > -
> > - > function='0x3'>
> > -  
> > -
> > -  
> > -
> > -will yield the following result in a Linux guest:
> > -
> > -::
> > -
> > -  0007:00:00.0 Ethernet controller: Red Hat, Inc. Virtio network device
> 
> Hm, should that rather go somewhere else? What I wanted to show is "you
> can have the same PCI address in the XML and still get a different PCI
> address in the guest, if you change the zpci values", as that might be
> another source of confusion.

I think the previous example, specifically the last bit where you
explain how changing the PCI address completely in the domain XML
would not change what the guest OS sees because the latter is derived
from uid and fid, already drives the point home. It's really two
sides of the same coin.

Additionally, as I explained elsewhere, this document is not meant to
list every possible situation in which PCI addresses in the domain
XML and in the guest OS are out of sync, but merely to show that such
cases exist. It's valuable to mention the zPCI scenario, but we don't
need to show more than one variation of it in my opinion.

-- 
Andrea Bolognani / Red Hat / Virtualization