Re: [libvirt] [PATCH] tests: qemucaps: Update 4.1.0 capabilities to a more recent version

[Erik Skultety]

On Thu, Jun 20, 2019 at 08:04:45AM +0200, Peter Krempa wrote:
> Update the capabilities from a non-upstream version (9c70209b63 is not
> in qemu.git) to qemu upstream commit 33d6099906 (2019/06/18) so that we
> get the QMP schema 'features' field support and are able to detect that
> the 'file' block backend supports dynamic auto-read-only.
> 
> Note that I've rebuilt this on a machine with a more modern kernel and
> microcode which exposes e.g. the recent CPU bug mitigations, thus I
> opted to keep the CPU changes rather than trying to do a franken-caps
> by updating only the output of query-qmp-schema.
> 
> Signed-off-by: Peter Krempa 

Reviewed-by: Erik Skultety 

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

[libvirt] [PATCH v2 4/7] cpu_x86: Read CPU features from IA32_ARCH_CAPABILITIES MSR

[Jiri Denemark]

This is used by the host capabilities code to construct host CPU
definition.

Signed-off-by: Jiri Denemark 
Reviewed-by: Ján Tomko 
---
 src/cpu/cpu_x86.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index a7ec0f7095..5f051950de 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -2773,6 +2773,28 @@ virCPUx86GetHost(virCPUDefPtr cpu,
 cpuidSet(CPUX86_EXTENDED, cpuData) < 0)
 goto cleanup;
 
+/* Read the IA32_ARCH_CAPABILITIES MSR (0x10a) if supported.
+ * This is best effort since there might be no way to read the MSR
+ * when we are not running as root. */
+if (virCPUx86DataCheckFeature(cpuData, "arch-capabilities") == 1) {
+uint64_t msr;
+unsigned long index = 0x10a;
+
+if (virHostCPUGetMSR(index, &msr) == 0) {
+virCPUx86DataItem item = {
+.type = VIR_CPU_X86_DATA_MSR,
+.data.msr = {
+.index = index,
+.eax = msr & 0x,
+.edx = msr >> 32,
+},
+};
+
+if (virCPUx86DataAdd(cpuData, &item) < 0)
+return -1;
+}
+}
+
 ret = x86DecodeCPUData(cpu, cpuData, models);
 cpu->microcodeVersion = virHostCPUGetMicrocodeVersion();
 
-- 
2.22.0

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

[libvirt] [PATCH v2 0/7] Finish support for IA32_ARCH_CAPABILITIES MSR features

[Jiri Denemark]

As promised in v1 this series contains a few additional patches which
limit usage of MSR features to QEMU that supports "unavailable-features"
CPU property.

This series intentionally enables MSR features for all QEMU versions
before restricting the usage to make this restriction more visible in
the test results.

Version 2:
- all but 2 patches from version 1 were pushed
- 5 new patches

Jiri Denemark (7):
  conf: Introduce virCPUDefCheckFeatures
  cpu_x86: Turn virCPUx86DataIteratorInit into a function
  cpu_x86: Introduce virCPUx86FeatureIsMSR
  cpu_x86: Read CPU features from IA32_ARCH_CAPABILITIES MSR
  cpu_map: Introduce IA32_ARCH_CAPABILITIES MSR features
  qemu: Forbid MSR features with old QEMU
  qemu: Drop MSR features from host-model with old QEMU

 src/conf/cpu_conf.c   | 33 +++
 src/conf/cpu_conf.h   |  6 ++
 src/cpu/cpu_x86.c | 96 +--
 src/cpu/cpu_x86.h |  3 +
 src/cpu_map/x86_features.xml  | 20 
 src/libvirt_private.syms  |  2 +
 src/qemu/qemu_capabilities.c  | 16 
 src/qemu/qemu_process.c   | 29 +-
 .../x86_64-cpuid-Core-i7-7600U-enabled.xml|  1 +
 .../x86_64-cpuid-Core-i7-7600U-json.xml   |  1 +
 ...86_64-cpuid-Xeon-Platinum-8268-enabled.xml |  1 +
 .../x86_64-cpuid-Xeon-Platinum-8268-guest.xml |  4 +
 .../x86_64-cpuid-Xeon-Platinum-8268-host.xml  |  4 +
 .../x86_64-cpuid-Xeon-Platinum-8268-json.xml  |  3 +
 .../qemu_4.1.0.x86_64.xml |  1 +
 15 files changed, 207 insertions(+), 13 deletions(-)

-- 
2.22.0

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

[libvirt] [PATCH v2 3/7] cpu_x86: Introduce virCPUx86FeatureIsMSR

[Jiri Denemark]

This function may be used as a virCPUDefFeatureFilter callback for
virCPUDefCheckFeatures, virCPUDefFilerFeatures, and similar functions to
filter or pick out features reported via MSR.

Signed-off-by: Jiri Denemark 
---
 src/cpu/cpu_x86.c| 40 
 src/cpu/cpu_x86.h|  3 +++
 src/libvirt_private.syms |  1 +
 3 files changed, 44 insertions(+)

diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 6eef5cef00..a7ec0f7095 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -3362,6 +3362,46 @@ virCPUx86DataAddFeature(virCPUDataPtr cpuData,
 }
 
 
+/**
+ * virCPUx86FeatureIsMSR:
+ * @name CPU feature name
+ * @opaque NULL or a pointer to bool
+ *
+ * This is a callback for functions filtering features in virCPUDef.
+ *
+ * Checks whether a given CPU feature is reported via MSR. When @opaque is NULL
+ * or a pointer to true, the function will pick out (return true for) MSR
+ * features. If @opaque is a pointer to false, the logic will be inverted and
+ * the function will filter out (return false for) MSR features.
+ */
+bool
+virCPUx86FeatureIsMSR(const char *name,
+  void *opaque)
+{
+virCPUx86FeaturePtr feature;
+virCPUx86DataIterator iter;
+virCPUx86DataItemPtr item;
+virCPUx86MapPtr map;
+bool inverted = false;
+
+if (opaque)
+inverted = !*(bool *)opaque;
+
+if ((!(map = virCPUx86GetMap()) ||
+ !(feature = x86FeatureFind(map, name))) &&
+!(feature = x86FeatureFindInternal(name)))
+return inverted ? true : false;
+
+virCPUx86DataIteratorInit(&iter, &feature->data);
+while ((item = virCPUx86DataNext(&iter))) {
+if (item->type == VIR_CPU_X86_DATA_MSR)
+return inverted ? false : true;
+}
+
+return inverted ? true : false;
+}
+
+
 struct cpuArchDriver cpuDriverX86 = {
 .name = "x86",
 .arch = archs,
diff --git a/src/cpu/cpu_x86.h b/src/cpu/cpu_x86.h
index 28ae46647a..8ae78d5e34 100644
--- a/src/cpu/cpu_x86.h
+++ b/src/cpu/cpu_x86.h
@@ -40,3 +40,6 @@ uint32_t virCPUx86DataGetSignature(virCPUDataPtr cpuData,
 
 int virCPUx86DataSetVendor(virCPUDataPtr cpuData,
const char *vendor);
+
+bool virCPUx86FeatureIsMSR(const char *name,
+   void *opaque);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 90a6d10666..aa64580d63 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1268,6 +1268,7 @@ virCPUx86DataAdd;
 virCPUx86DataGetSignature;
 virCPUx86DataSetSignature;
 virCPUx86DataSetVendor;
+virCPUx86FeatureIsMSR;
 
 
 # datatypes.h
-- 
2.22.0

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

[libvirt] [PATCH v2 6/7] qemu: Forbid MSR features with old QEMU

[Jiri Denemark]

Without "unavailable-features" CPU property we cannot properly detect
whether a specific MSR feature we asked for (either explicitly or
implicitly via a CPU model) was disabled by QEMU for some reason.
Because this could break migration, snapshots, and save/restore
operaions, it's better to just forbid any use of MSR features with QEMU
which lacks "unavailable-features" CPU property.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_process.c | 29 ++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index fa82adbc1e..45997dad49 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -59,6 +59,7 @@
 #include "qemu_firmware.h"
 
 #include "cpu/cpu.h"
+#include "cpu/cpu_x86.h"
 #include "datatypes.h"
 #include "virlog.h"
 #include "virerror.h"
@@ -5407,9 +5408,31 @@ qemuProcessStartValidate(virQEMUDriverPtr driver,
 if (qemuProcessStartValidateShmem(vm) < 0)
 return -1;
 
-if (vm->def->cpu &&
-virCPUValidateFeatures(vm->def->os.arch, vm->def->cpu) < 0)
-return -1;
+if (vm->def->cpu) {
+if (virCPUValidateFeatures(vm->def->os.arch, vm->def->cpu) < 0)
+return -1;
+
+if (ARCH_IS_X86(vm->def->os.arch) &&
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_UNAVAILABLE_FEATURES)) {
+VIR_AUTOSTRINGLIST features = NULL;
+int n;
+
+if ((n = virCPUDefCheckFeatures(vm->def->cpu,
+virCPUx86FeatureIsMSR, NULL,
+&features)) < 0)
+return -1;
+
+if (n > 0) {
+VIR_AUTOFREE(char *) str = NULL;
+
+str = virStringListJoin((const char **)features, ", ");
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Some features cannot be reliably used "
+ "with this QEMU: %s"), str);
+return -1;
+}
+}
+}
 
 if (qemuProcessStartValidateDisks(vm, qemuCaps) < 0)
 return -1;
-- 
2.22.0

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

[libvirt] [PATCH v2 5/7] cpu_map: Introduce IA32_ARCH_CAPABILITIES MSR features

[Jiri Denemark]

Signed-off-by: Jiri Denemark 
Reviewed-by: Ján Tomko 
---
 src/cpu_map/x86_features.xml  | 20 +++
 .../x86_64-cpuid-Core-i7-7600U-enabled.xml|  1 +
 .../x86_64-cpuid-Core-i7-7600U-json.xml   |  1 +
 ...86_64-cpuid-Xeon-Platinum-8268-enabled.xml |  1 +
 .../x86_64-cpuid-Xeon-Platinum-8268-guest.xml |  4 
 .../x86_64-cpuid-Xeon-Platinum-8268-host.xml  |  4 
 .../x86_64-cpuid-Xeon-Platinum-8268-json.xml  |  3 +++
 .../qemu_3.1.0.x86_64.xml |  1 +
 .../qemu_4.0.0.x86_64.xml |  1 +
 .../qemu_4.1.0.x86_64.xml |  1 +
 10 files changed, 37 insertions(+)

diff --git a/src/cpu_map/x86_features.xml b/src/cpu_map/x86_features.xml
index 370807f88e..2bed1e0372 100644
--- a/src/cpu_map/x86_features.xml
+++ b/src/cpu_map/x86_features.xml
@@ -482,4 +482,24 @@
   
 
   
+
+  
+  
+
+  
+  
+
+  
+  
+
+  
+  
+
+  
+  
+
+  
+  
+
+  
 
diff --git a/tests/cputestdata/x86_64-cpuid-Core-i7-7600U-enabled.xml 
b/tests/cputestdata/x86_64-cpuid-Core-i7-7600U-enabled.xml
index b1cdaa802a..58bc84577c 100644
--- a/tests/cputestdata/x86_64-cpuid-Core-i7-7600U-enabled.xml
+++ b/tests/cputestdata/x86_64-cpuid-Core-i7-7600U-enabled.xml
@@ -5,4 +5,5 @@
   
   
   
+  
 
diff --git a/tests/cputestdata/x86_64-cpuid-Core-i7-7600U-json.xml 
b/tests/cputestdata/x86_64-cpuid-Core-i7-7600U-json.xml
index 48089c6003..690081493b 100644
--- a/tests/cputestdata/x86_64-cpuid-Core-i7-7600U-json.xml
+++ b/tests/cputestdata/x86_64-cpuid-Core-i7-7600U-json.xml
@@ -10,4 +10,5 @@
   
   
   
+  
 
diff --git a/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-enabled.xml 
b/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-enabled.xml
index 434ac1956a..313009b156 100644
--- a/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-enabled.xml
+++ b/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-enabled.xml
@@ -5,4 +5,5 @@
   
   
   
+  
 
diff --git a/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-guest.xml 
b/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-guest.xml
index c7e8a1fccf..988fb1dbdc 100644
--- a/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-guest.xml
+++ b/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-guest.xml
@@ -30,4 +30,8 @@
   
   
   
+  
+  
+  
+  
 
diff --git a/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-host.xml 
b/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-host.xml
index d7482751b4..fdeafc4870 100644
--- a/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-host.xml
+++ b/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-host.xml
@@ -31,4 +31,8 @@
   
   
   
+  
+  
+  
+  
 
diff --git a/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-json.xml 
b/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-json.xml
index b7d12dced7..78863c61d1 100644
--- a/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-json.xml
+++ b/tests/cputestdata/x86_64-cpuid-Xeon-Platinum-8268-json.xml
@@ -7,4 +7,7 @@
   
   
   
+  
+  
+  
 
diff --git a/tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml 
b/tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml
index ca3baab88c..dfd186afba 100644
--- a/tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml
@@ -42,6 +42,7 @@
   
   
   
+  
 
 
   qemu64
diff --git a/tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml 
b/tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml
index cba841d844..36f6f1e94d 100644
--- a/tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml
@@ -42,6 +42,7 @@
   
   
   
+  
 
 
   qemu64
diff --git a/tests/domaincapsschemadata/qemu_4.1.0.x86_64.xml 
b/tests/domaincapsschemadata/qemu_4.1.0.x86_64.xml
index 389e641bbb..d3fc1ce9e7 100644
--- a/tests/domaincapsschemadata/qemu_4.1.0.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_4.1.0.x86_64.xml
@@ -41,6 +41,7 @@
   
   
   
+  
 
 
   qemu64
-- 
2.22.0

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

[libvirt] [PATCH v2 1/7] conf: Introduce virCPUDefCheckFeatures

[Jiri Denemark]

This API can be used to check whether a CPU definition contains features
matching a given filter.

Signed-off-by: Jiri Denemark 
---
 src/conf/cpu_conf.c  | 33 +
 src/conf/cpu_conf.h  |  6 ++
 src/libvirt_private.syms |  1 +
 3 files changed, 40 insertions(+)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 675d214c50..7d16a05283 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -930,6 +930,39 @@ virCPUDefFilterFeatures(virCPUDefPtr cpu,
 }
 
 
+/**
+ * virCPUDefCheckFeatures:
+ *
+ * Check CPU features for which @filter reports true and store them in a NULL
+ * terminated list returned via @features.
+ *
+ * Returns the number of features matching @filter or -1 on error.
+ */
+int
+virCPUDefCheckFeatures(virCPUDefPtr cpu,
+   virCPUDefFeatureFilter filter,
+   void *opaque,
+   char ***features)
+{
+VIR_AUTOSTRINGLIST list = NULL;
+size_t n = 0;
+size_t i;
+
+*features = NULL;
+
+for (i = 0; i < cpu->nfeatures; i++) {
+if (filter(cpu->features[i].name, opaque)) {
+if (virStringListAdd(&list, cpu->features[i].name) < 0)
+return -1;
+n++;
+}
+}
+
+VIR_STEAL_PTR(*features, list);
+return n;
+}
+
+
 bool
 virCPUDefIsEqual(virCPUDefPtr src,
  virCPUDefPtr dst,
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index 906ef5368e..19ce816ec2 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -224,6 +224,12 @@ virCPUDefFilterFeatures(virCPUDefPtr cpu,
 virCPUDefFeatureFilter filter,
 void *opaque);
 
+int
+virCPUDefCheckFeatures(virCPUDefPtr cpu,
+   virCPUDefFeatureFilter filter,
+   void *opaque,
+   char ***features);
+
 virCPUDefPtr *
 virCPUDefListParse(const char **xmlCPUs,
unsigned int ncpus,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ab2e4bc6fe..90a6d10666 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -73,6 +73,7 @@ virCapabilitiesSetNetPrefix;
 virCPUCacheModeTypeFromString;
 virCPUCacheModeTypeToString;
 virCPUDefAddFeature;
+virCPUDefCheckFeatures;
 virCPUDefCopy;
 virCPUDefCopyModel;
 virCPUDefCopyModelFilter;
-- 
2.22.0

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

[libvirt] [PATCH v2 7/7] qemu: Drop MSR features from host-model with old QEMU

[Jiri Denemark]

With QEMU versions which lack "unavailable-features" we use CPUID based
detection of features which were enabled or disabled once QEMU starts.
Thus using MSR features with host-model would result in all of them
being marked as disabled in the active domain definition even though
QEMU did not actually disable them.

Let's make sure we add MSR features to host-model only when
"unavailable-features" property is supported by QEMU.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_capabilities.c | 16 
 tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml |  1 -
 tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml |  1 -
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 4134f319ac..b7c20f3e3e 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3193,6 +3193,22 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
 goto error;
 }
 
+if (ARCH_IS_X86(qemuCaps->arch) &&
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_UNAVAILABLE_FEATURES)) {
+bool selecting = false;
+if (cpu &&
+virCPUDefFilterFeatures(cpu, virCPUx86FeatureIsMSR, &selecting) < 
0)
+goto error;
+
+if (migCPU &&
+virCPUDefFilterFeatures(migCPU, virCPUx86FeatureIsMSR, &selecting) 
< 0)
+goto error;
+
+if (fullCPU &&
+virCPUDefFilterFeatures(fullCPU, virCPUx86FeatureIsMSR, 
&selecting) < 0)
+goto error;
+}
+
 virQEMUCapsSetHostModel(qemuCaps, type, cpu, migCPU, fullCPU);
 
  cleanup:
diff --git a/tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml 
b/tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml
index dfd186afba..ca3baab88c 100644
--- a/tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml
@@ -42,7 +42,6 @@
   
   
   
-  
 
 
   qemu64
diff --git a/tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml 
b/tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml
index 36f6f1e94d..cba841d844 100644
--- a/tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml
+++ b/tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml
@@ -42,7 +42,6 @@
   
   
   
-  
 
 
   qemu64
-- 
2.22.0

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

[libvirt] [PATCH v2 2/7] cpu_x86: Turn virCPUx86DataIteratorInit into a function

[Jiri Denemark]

Until now, this was a macro usable for direct initialization when a
variable is defined. Turning the macro into a function makes it more
general.

Signed-off-by: Jiri Denemark 
---
 src/cpu/cpu_x86.c | 34 --
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index b6a94d483a..6eef5cef00 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -189,8 +189,13 @@ struct _virCPUx86DataIterator {
 };
 
 
-#define virCPUx86DataIteratorInit(data) \
-{ data, -1 }
+static void
+virCPUx86DataIteratorInit(virCPUx86DataIteratorPtr iterator,
+  const virCPUx86Data *data)
+{
+virCPUx86DataIterator iter = { data, -1 };
+*iterator = iter;
+}
 
 
 static bool
@@ -540,9 +545,10 @@ static int
 x86DataAdd(virCPUx86Data *data1,
const virCPUx86Data *data2)
 {
-virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data2);
+virCPUx86DataIterator iter;
 virCPUx86DataItemPtr item;
 
+virCPUx86DataIteratorInit(&iter, data2);
 while ((item = virCPUx86DataNext(&iter))) {
 if (virCPUx86DataAddItem(data1, item) < 0)
 return -1;
@@ -556,10 +562,11 @@ static void
 x86DataSubtract(virCPUx86Data *data1,
 const virCPUx86Data *data2)
 {
-virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data1);
+virCPUx86DataIterator iter;
 virCPUx86DataItemPtr item1;
 virCPUx86DataItemPtr item2;
 
+virCPUx86DataIteratorInit(&iter, data1);
 while ((item1 = virCPUx86DataNext(&iter))) {
 item2 = virCPUx86DataGet(data2, item1);
 virCPUx86DataItemClearBits(item1, item2);
@@ -571,10 +578,11 @@ static void
 x86DataIntersect(virCPUx86Data *data1,
  const virCPUx86Data *data2)
 {
-virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data1);
+virCPUx86DataIterator iter;
 virCPUx86DataItemPtr item1;
 virCPUx86DataItemPtr item2;
 
+virCPUx86DataIteratorInit(&iter, data1);
 while ((item1 = virCPUx86DataNext(&iter))) {
 item2 = virCPUx86DataGet(data2, item1);
 if (item2)
@@ -588,8 +596,9 @@ x86DataIntersect(virCPUx86Data *data1,
 static bool
 x86DataIsEmpty(virCPUx86Data *data)
 {
-virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data);
+virCPUx86DataIterator iter;
 
+virCPUx86DataIteratorInit(&iter, data);
 return !virCPUx86DataNext(&iter);
 }
 
@@ -598,10 +607,11 @@ static bool
 x86DataIsSubset(const virCPUx86Data *data,
 const virCPUx86Data *subset)
 {
-virCPUx86DataIterator iter = virCPUx86DataIteratorInit((virCPUx86Data 
*)subset);
+virCPUx86DataIterator iter;
 const virCPUx86DataItem *item;
 const virCPUx86DataItem *itemSubset;
 
+virCPUx86DataIteratorInit(&iter, subset);
 while ((itemSubset = virCPUx86DataNext(&iter))) {
 if (!(item = virCPUx86DataGet(data, itemSubset)) ||
 !virCPUx86DataItemMatchMasked(item, itemSubset))
@@ -1314,11 +1324,13 @@ x86ModelCompare(virCPUx86ModelPtr model1,
 virCPUx86ModelPtr model2)
 {
 virCPUx86CompareResult result = EQUAL;
-virCPUx86DataIterator iter1 = virCPUx86DataIteratorInit(&model1->data);
-virCPUx86DataIterator iter2 = virCPUx86DataIteratorInit(&model2->data);
+virCPUx86DataIterator iter1;
+virCPUx86DataIterator iter2;
 virCPUx86DataItemPtr item1;
 virCPUx86DataItemPtr item2;
 
+virCPUx86DataIteratorInit(&iter1, &model1->data);
+virCPUx86DataIteratorInit(&iter2, &model2->data);
 while ((item1 = virCPUx86DataNext(&iter1))) {
 virCPUx86CompareResult match = SUPERSET;
 
@@ -1624,10 +1636,12 @@ virCPUx86GetMap(void)
 static char *
 virCPUx86DataFormat(const virCPUData *data)
 {
-virCPUx86DataIterator iter = virCPUx86DataIteratorInit(&data->data.x86);
+virCPUx86DataIterator iter;
 virCPUx86DataItemPtr item;
 virBuffer buf = VIR_BUFFER_INITIALIZER;
 
+virCPUx86DataIteratorInit(&iter, &data->data.x86);
+
 virBufferAddLit(&buf, "\n");
 while ((item = virCPUx86DataNext(&iter))) {
 virCPUx86CPUIDPtr cpuid;
-- 
2.22.0

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

Re: [libvirt] [PATCH 24/24] cpu_map: Introduce IA32_ARCH_CAPABILITIES MSR features

[Jiri Denemark]

On Wed, Jun 19, 2019 at 14:40:27 +0200, Ján Tomko wrote:
> On Wed, Jun 19, 2019 at 11:38:21AM +0200, Jiri Denemark wrote:
> >---
> > src/cpu_map/x86_features.xml  | 20 +++
> > .../x86_64-cpuid-Core-i7-7600U-enabled.xml|  1 +
> > .../x86_64-cpuid-Core-i7-7600U-json.xml   |  1 +
> > ...86_64-cpuid-Xeon-Platinum-8268-enabled.xml |  1 +
> > .../x86_64-cpuid-Xeon-Platinum-8268-guest.xml |  4 
> > .../x86_64-cpuid-Xeon-Platinum-8268-host.xml  |  4 
> > .../x86_64-cpuid-Xeon-Platinum-8268-json.xml  |  3 +++
> > .../qemu_3.1.0.x86_64.xml |  1 +
> > .../qemu_4.0.0.x86_64.xml |  1 +
> > .../qemu_4.1.0.x86_64.xml |  1 +
> > 10 files changed, 37 insertions(+)
> >
> 
> Reviewed-by: Ján Tomko 

Thanks, I did the suggested modifications and pushed all but the last
two patches. They will come again with the few additional patches I
promised in the cover letter.

Jirka

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

Re: [libvirt] [PATCH 15/24] qemu_command: Use canonical names of CPU features

[Jiri Denemark]

On Wed, Jun 19, 2019 at 14:08:58 +0200, Ján Tomko wrote:
> On Wed, Jun 19, 2019 at 11:38:12AM +0200, Jiri Denemark wrote:
> >When building QEMU command line, we should use the preferred spelling of
> >each CPU feature without relying on compatibility aliases (which may be
> >removed at some point).
> >
> >The "unavailable-features" CPU property is used as a witness for the
> >correct names of the features in our translation table.
> >
> >Signed-off-by: Jiri Denemark 
> >---
> > src/qemu/qemu_capabilities.c  | 8 +++-
> > src/qemu/qemu_capabilities.h  | 1 +
> > src/qemu/qemu_command.c   | 2 ++
> > tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml  | 1 +
> > tests/qemuxml2argvdata/cpu-translation.x86_64-latest.args | 6 +++---
> > tests/qemuxml2argvdata/eoi-disabled.x86_64-latest.args| 2 +-
> > tests/qemuxml2argvdata/eoi-enabled.x86_64-latest.args | 2 +-
> > .../kvmclock+eoi-disabled.x86_64-latest.args  | 2 +-
> > .../pv-spinlock-disabled.x86_64-latest.args   | 2 +-
> > .../pv-spinlock-enabled.x86_64-latest.args| 2 +-
> > 10 files changed, 19 insertions(+), 9 deletions(-)
> >
> >diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> >index 3af6923e6f..c742838383 100644
> >--- a/src/qemu/qemu_capabilities.c
> >+++ b/src/qemu/qemu_capabilities.c
> >@@ -532,6 +532,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
> >   "nbd-bitmap",
> >   "x86-max-cpu",
> >   "cpu-unavailable-features",
> >+  "canonical-cpu-features",
> > );
> >
> >
> >@@ -2892,7 +2893,9 @@ virQEMUCapsCPUFeatureTranslate(virQEMUCapsPtr qemuCaps,
> > if (ARCH_IS_X86(qemuCaps->arch))
> > table = virQEMUCapsCPUFeaturesX86;
> >
> >-if (!table || !feature)
> >+if (!table ||
> >+!feature ||
> >+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CANONICAL_CPU_FEATURES))
> > return feature;
> >
> > for (entry = table; entry->libvirt; entry++) {
> >@@ -4398,6 +4401,9 @@ virQEMUCapsInitProcessCaps(virQEMUCapsPtr qemuCaps)
> >  * we are able to pass the custom 'device_id' for SCSI disks and 
> > cdroms. */
> > if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_DEVICE_ID))
> > virQEMUCapsClear(qemuCaps, QEMU_CAPS_BLOCKDEV);
> >+
> >+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_UNAVAILABLE_FEATURES))
> >+virQEMUCapsSet(qemuCaps, QEMU_CAPS_CANONICAL_CPU_FEATURES);
> > }
> 
> Do we need an alias for the QEMU_CAPS_CPU_UNAVAILABLE_FEATURES
> capability?
> 
> I think virQEMUCapsCPUFeatureTranslate can use 
> QEMU_CAPS_CPU_UNAVAILABLE_FEATURES directly

Both virQEMUCapsProbeQMPHostCPU and virQEMUCapsCPUFeatureTranslate could
use QEMU_CAPS_CPU_UNAVAILABLE_FEATURES directly, but I felt the alias
will make it clear what's going on in there since "unavailable-features"
property serves just as a witness and we'd likely need to add a comment
about this to every usage which is not doing anything with the new
property.

Jirka

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

Re: [libvirt] [PATCH 09/24] qemuxml2argvtest: Add 4.0.0 cases for kvm features tests

[Jiri Denemark]

On Wed, Jun 19, 2019 at 13:47:14 +0200, Ján Tomko wrote:
> On Wed, Jun 19, 2019 at 11:38:06AM +0200, Jiri Denemark wrote:
> 
> Again, why 4.0.0 in particular?

Forgot again :/ Newer QEMU will translate the feature names to their
canonical names so 4.0.0 is the last one which produces the results we
currently have in *-latest.args.

Jirka

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

[libvirt] [PATCH 3/3] tests: qemuMonitorTest: drop the JSON field

[Ján Tomko]

Now that we no longer support testing HMP monitor,
the json field is pointless.

Signed-off-by: Ján Tomko 
---
 tests/qemumonitortestutils.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index 9f18e511ac..d395a9b539 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -51,7 +51,6 @@ struct _qemuMonitorTest {
 virMutex lock;
 virThread thread;
 
-bool json;
 bool quit;
 bool running;
 bool started;
@@ -1189,7 +1188,6 @@ qemuMonitorTestNew(virDomainXMLOptionPtr xmlopt,
 if (!(test = qemuMonitorCommonTestNew(xmlopt, vm, &src)))
 goto error;
 
-test->json = true;
 test->qapischema = schema;
 if (!(test->mon = qemuMonitorOpen(test->vm,
   &src,
-- 
2.19.2

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

[libvirt] [PATCH 2/3] tests: qemuMonitorTestProcessCommandDefaultValidate: simplify condition

[Ján Tomko]

We return success when running this function for either non-JSON monitor
testing or guest agent testing.

However we no longer test HMP monitor and we do not try to validate
the guest agent interaction.

Drop the test->json check and report a proper error if someone tries
to run this function for the guest agent without properly wiring it up.

Signed-off-by: Ján Tomko 
---
 tests/qemumonitortestutils.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index b29c6d0f53..9f18e511ac 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -521,9 +521,16 @@ 
qemuMonitorTestProcessCommandDefaultValidate(qemuMonitorTestPtr test,
 VIR_AUTOPTR(virJSONValue) emptyargs = NULL;
 VIR_AUTOFREE(char *) schemapath = NULL;
 
-if (!test->qapischema || !test->json || test->agent)
+if (!test->qapischema)
 return 0;
 
+if (test->agent) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   "Command validation testing is not "
+   "implemented for the guest agent");
+return -1;
+}
+
 /* 'device_add' needs to be skipped as it does not have fully defined 
schema */
 if (STREQ(cmdname, "device_add"))
 return 0;
-- 
2.19.2

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

[libvirt] [PATCH 0/3] tests: drop the JSON field from qemuMonitorTest

[Ján Tomko]

Ján Tomko (3):
  tests: assume JSON in qemuMonitorTestIO
  tests: qemuMonitorTestProcessCommandDefaultValidate: simplify
condition
  tests: qemuMonitorTest: drop the JSON field

 tests/qemumonitortestutils.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

-- 
2.19.2

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

[libvirt] [PATCH 1/3] tests: assume JSON in qemuMonitorTestIO

[Ján Tomko]

The QMP monitor only uses a newline to separate lines,
while HMP and the guest agent also use a carriage return.

In preparation to dropping support for testing HMP interaction,
only skip the carriage return if we're dealing with the guest agent,
removing the need to check the 'json' field.

Signed-off-by: Ján Tomko 
---
 tests/qemumonitortestutils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index 85a2fca659..b29c6d0f53 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -296,7 +296,7 @@ qemuMonitorTestIO(virNetSocketPtr sock,
  */
 t1 = test->incoming;
 while ((t2 = strstr(t1, "\n")) ||
-(!test->json && (t2 = strstr(t1, "\r" {
+(test->agent && (t2 = strstr(t1, "\r" {
 *t2 = '\0';
 
 if (qemuMonitorTestProcessCommand(test, t1) < 0) {
-- 
2.19.2

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

Re: [libvirt] [PATCH 05/24] qemuxml2argvtest: Switch some tests to DO_TEST_CAPS_*

[Jiri Denemark]

On Wed, Jun 19, 2019 at 13:43:53 +0200, Ján Tomko wrote:
> On Wed, Jun 19, 2019 at 11:38:02AM +0200, Jiri Denemark wrote:
> >These test check all kvm CPU features that could be passed to the -cpu
> >option by libvirt.
> >
> 
> It would be nice to mention why you picked 2.7.0 here.

Yeah, I forgot to do so. With QEMU 2.7.0 and earlier, we use +|-feature
syntax for CPU features, while feature=on|off is used with newer
versions.

I added similar explanation to the commit message.

Jirka

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

Re: [libvirt] mdevctl: A shoestring mediated device management and persistence utility

[Alex Williamson]

On Wed, 19 Jun 2019 11:04:15 +0200
Sylvain Bauza  wrote:

> On Wed, Jun 19, 2019 at 12:27 AM Alex Williamson 
> wrote:
> 
> > On Tue, 18 Jun 2019 14:48:11 +0200
> > Sylvain Bauza  wrote:
> >  
> > > On Tue, Jun 18, 2019 at 1:01 PM Cornelia Huck  wrote:
> > >  
> > > > On Mon, 17 Jun 2019 11:05:17 -0600
> > > > Alex Williamson  wrote:
> > > >  
> > > > > On Mon, 17 Jun 2019 16:10:30 +0100
> > > > > Daniel P. Berrangé  wrote:
> > > > >  
> > > > > > On Mon, Jun 17, 2019 at 08:54:38AM -0600, Alex Williamson wrote:  
> > > > > > > On Mon, 17 Jun 2019 15:00:00 +0100
> > > > > > > Daniel P. Berrangé  wrote:
> > > > > > >  
> > > > > > > > On Thu, May 23, 2019 at 05:20:01PM -0600, Alex Williamson  
> > wrote:  
> > > >  
> > > > > > > > > Hi,
> > > > > > > > >
> > > > > > > > > Currently mediated device management, much like SR-IOV VF  
> > > > management,  
> > > > > > > > > is largely left as an exercise for the user.  This is an  
> > attempt  
> > > > to  
> > > > > > > > > provide something and see where it goes.  I doubt we'll solve
> > > > > > > > > everyone's needs on the first pass, but maybe we'll solve  
> > enough  
> > > > and  
> > > > > > > > > provide helpers for the rest.  Without further ado, I'll  
> > point  
> > > > to what  
> > > > > > > > > I have so far:
> > > > > > > > >
> > > > > > > > > https://github.com/awilliam/mdevctl
> > > > > > > > >
> > > > > > > > > This is inspired by driverctl, which is also a bash  
> > utility.  
> > > > mdevctl  
> > > > > > > > > uses udev and systemd to record and recreate mdev devices for
> > > > > > > > > persistence and provides a command line utility for  
> > querying,  
> > > > listing,  
> > > > > > > > > starting, stopping, adding, and removing mdev devices.  
> > > > Currently, for  
> > > > > > > > > better or worse, it considers anything created to be  
> > > > persistent.  I can  
> > > > > > > > > imagine a global configuration option that might disable  
> > this and  
> > > > > > > > > perhaps an autostart flag per mdev device, such that  
> > mdevctl  
> > > > might  
> > > > > > > > > simply "know" about some mdevs but not attempt to create them
> > > > > > > > > automatically.  Clearly command line usage help, man pages,  
> > and  
> > > > > > > > > packaging are lacking as well, release early, release  
> > often,  
> > > > plus this  
> > > > > > > > > is a discussion starter to see if perhaps this is sufficient  
> > to  
> > > > meet  
> > > > > > > > > some needs.  
> > > > > > > >
> > > > > > > > I think from libvirt's POV, we would *not* want devices to be  
> > made  
> > > > > > > > unconditionally persistent. We usually wish to expose a choice  
> > to  
> > > > > > > > applications whether to have resources be transient or  
> > persistent.  
> > > > > > > >
> > > > > > > > So from that POV, a global config option to turn off  
> > persistence  
> > > > > > > > is not workable either. We would want control per-device, with
> > > > > > > > autostart control per device too.  
> > > > > > >
> > > > > > > The code has progressed somewhat in the past 3+ weeks, we still  
> > > > persist  
> > > > > > > all devices, but the start-up mode can be selected per device  
> > or  
> > > > with a  
> > > > > > > global default mode.  Devices configured with 'auto' start-up
> > > > > > > automatically while 'manual' devices are simply known and  
> > available  
> > > > to  
> > > > > > > be started.  I imagine we could add a 'transient' mode where we  
> > purge  
> > > > > > > the information about the device when it is removed or the next  
> > time  
> > > > > > > the parent device is added.  
> > > > > >
> > > > > > Having a pesistent config written out & then purged later is still
> > > > > > problematic. If the host crashes, nothing will purge the config  
> > file,  
> > > > > > so it will become a persistent device. Also when listing devices we
> > > > > > want to be able to report whether it is persistent or transient.  
> > The  
> > > > > > obvious way todo that is to simply look if a config file exists or
> > > > > > not.  
> > > > >
> > > > > I was thinking that the config file would identify the device as
> > > > > transient, therefore if the system crashed we'd have the opportunity  
> > to  
> > > > > purge those entries on the next boot as we're processing the entries
> > > > > for that parent device.  Clearly it has yet to be implemented, but I
> > > > > expect there are some advantages to tracking devices via a transient
> > > > > config entry or else we're constantly re-discovering foreign mdevs.  
> > > >
> > > > I think we need to reach consensus about the actual scope of the
> > > > mdevctl tool.
> > > >
> > > >  
> > > Thanks Cornelia, my thoughts:
> > >
> > > - Is it supposed to be responsible for managing *all* mdev devices in  
> > > >   the system, or is it more supposed to be a convenience helper for
> > > >   users/software wanting to manage mdevs?
> > > >  
> > >
> > > The latter. If an operator (or some so

[libvirt] [PATCH v2] syntax check: update header guard check

[Jonathon Jongsma]

Internal headers should use #pragma once instead of the standard #ifndef
guard. Public headers still require the existing header guard.

Signed-off-by: Jonathon Jongsma 
---

Changes in v2:
 - fix error messages when no header guard is found (#ifndef for public 
headers, #pragma for
   internal headers)
 - fix test for identifying public headers (omit initial '/')
 - fix code spacing

 build-aux/header-ifdef.pl | 41 +--
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/build-aux/header-ifdef.pl b/build-aux/header-ifdef.pl
index 401f25fb81..dba3dbcbdc 100644
--- a/build-aux/header-ifdef.pl
+++ b/build-aux/header-ifdef.pl
@@ -6,18 +6,26 @@
 #  ...copyright header...
 #  */
 # 
-# #ifndef SYMBOL
-# # define SYMBOL
+# #pragma once
 # content
-# #endif /* SYMBOL */
 #
-# For any file ending priv.h, before the #ifndef
+#---
+#
+# For any file ending priv.h, before the #pragma once
 # We will have a further section
 #
 # #ifndef SYMBOL_ALLOW
 # # error 
 # #endif /* SYMBOL_ALLOW */
 # 
+#
+#---
+#
+# For public headers (files in include/), use the standard header guard 
instead of #pragma once:
+# #ifndef SYMBOL
+# # define SYMBOL
+# content
+# #endif /* SYMBOL */
 
 use strict;
 use warnings;
@@ -38,6 +46,7 @@ my $file = " ";
 my $ret = 0;
 my $ifdef = "";
 my $ifdefpriv = "";
+my $publicheader = 0;
 
 my $state = $STATE_EOF;
 my $mistake = 0;
@@ -64,7 +73,11 @@ while (<>) {
 } elsif ($state == $STATE_PRIV_BLANK) {
 &mistake("$file: missing blank line after priv header check");
 } elsif ($state == $STATE_GUARD_START) {
-&mistake("$file: missing '#ifndef $ifdef'");
+if ($publicheader) {
+&mistake("$file: missing '#ifndef $ifdef'");
+} else {
+&mistake("$file: missing '#pragma once' header guard");
+}
 } elsif ($state == $STATE_GUARD_DEFINE) {
 &mistake("$file: missing '# define $ifdef'");
 } elsif ($state == $STATE_GUARD_END) {
@@ -83,6 +96,7 @@ while (<>) {
 $file = $ARGV;
 $state = $STATE_COPYRIGHT_COMMENT;
 $mistake = 0;
+$publicheader = ($ARGV =~ /include\//);
 }
 
 if ($mistake ||
@@ -133,12 +147,19 @@ while (<>) {
 } elsif ($state == $STATE_GUARD_START) {
 if (/^$/) {
 &mistake("$file: too many blank lines after copyright header");
-} elsif(/#pragma once/) {
-$state = $STATE_PRAGMA;
-} elsif (/#ifndef $ifdef$/) {
-$state = $STATE_GUARD_DEFINE;
+}
+if ($publicheader) {
+if (/#ifndef $ifdef$/) {
+$state = $STATE_GUARD_DEFINE;
+} else {
+&mistake("$file: missing '#ifndef $ifdef'");
+}
 } else {
-&mistake("$file: missing '#ifndef $ifdef'");
+if (/#pragma once/) {
+$state = $STATE_PRAGMA;
+} else {
+&mistake("$file: missing '#pragma once' header guard");
+}
 }
 } elsif ($state == $STATE_GUARD_DEFINE) {
 if (/# define $ifdef$/) {
-- 
2.20.1

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

Re: [libvirt] mdevctl: A shoestring mediated device management and persistence utility

[Alex Williamson]

On Wed, 19 Jun 2019 11:46:59 +0200
Cornelia Huck  wrote:

> On Wed, 19 Jun 2019 08:28:02 +0100
> Daniel P. Berrangé  wrote:
> 
> > On Tue, Jun 18, 2019 at 04:12:10PM -0600, Alex Williamson wrote:  
> > > On Tue, 18 Jun 2019 14:48:11 +0200
> > > Sylvain Bauza  wrote:
> > > 
> > > > On Tue, Jun 18, 2019 at 1:01 PM Cornelia Huck  
> > > > wrote:  
> 
> > > > > I think we need to reach consensus about the actual scope of the
> > > > > mdevctl tool.
> > > > >
> > > > >  
> > > > Thanks Cornelia, my thoughts:
> > > > 
> > > > - Is it supposed to be responsible for managing *all* mdev devices in   
> > > >  
> > > > >   the system, or is it more supposed to be a convenience helper for
> > > > >   users/software wanting to manage mdevs?
> > > > >  
> > > > 
> > > > The latter. If an operator (or some software) wants to create mdevs by 
> > > > not
> > > > using mdevctl (and rather directly calling the sysfs), I think it's OK.
> > > > That said, mdevs created by mdevctl would be supported by systemctl, 
> > > > while
> > > > the others not but I think it's okay.
> > > 
> > > I agree (sort of), and I'm hearing that we should drop any sort of
> > > automatic persistence of mdevs created outside of mdevctl.  The problem
> > > comes when we try to draw the line between unmanaged and manged
> > > devices.  For instance, if we have a command to list mdevs it would
> > > feel incomplete if it didn't list all mdevs both those managed by
> > > mdevctl and those created elsewhere.  For managed devices, I expect
> > > we'll also have commands that allow the mode of the device to be
> > > switched between transient, saved, and persistent.  Should a user then  
> 
> Hm, what's the difference between 'saved' and 'persistent'? That
> 'saved' devices are not necessarily present?

It seems like we're coming up with the following classes:

1) transient
  a) mdevctl created
  b) foreign
2) defined
  a) automatic start-up
  b) manual start-up

I was using persistent for 2b), but that's probably not a good name
because devices can still be stopped, so they're not really
persistently available even in this class.

mdevctl today only has defined devices, transient needs to be
implemented, which should lead to a conclusion on whether 1a) and 1b)
are really the same.

> > > be allowed to promote an unmanaged device to one of these modes via the
> > > same command?  Should they be allowed to stop an unmanaged device
> > > through driverctl?  Through systemctl?  These all seem like reasonable
> > > things to do, so what then is the difference between transient and
> > > unmanaged mdev and is mdevctl therefore managing all mdevs, not just
> > > those it has created?
> > 
> > To my mind there shouldn't really need to be a difference between
> > transient mdevs created by mdevctrl and mdevs created by an user
> > directly using sysfs. Both are mdevs on the running system with
> > no config file that you have to enumerate by looking at sysfs.
> > This ties back to my belief that we shouldn't need to have any
> > config on disk for a transient mdev, just discover them all
> > dynamically when required.  
> 
> So mdevctl can potentially interact with any mdev device on the system,
> it just has to be instructed by a user or software to do so? I think we
> can work with that.

Some TBDs around systemd/init support for transient devices and how
transient devices can be promoted to defined.  For instance if a
vfio-ap device requires matrix programming after instantiation, can we
glean that programming from sysfs or is there metadata irrecoverably
lost if no config file is created for a transient device?  This would
also imply that a 1b) foreign device could not be promoted to 2x)
defined device.

> > > > - Do we want mdevctl to manage config files for individual mdevs, or
> > > > >   are they supposed to be in a common format that can also be managed
> > > > >   by e.g. libvirt?
> > > > >  
> > > > 
> > > > Unless I misunderstand, I think mdevctl just helps to create mdevs for
> > > > being used by guests created either by libvirt or QEMU or even others.
> > > > How a guest would allocate a mdev (ie. saying "I'll use this specific 
> > > > mdev
> > > > UUID") is IMHO not something for mdevctl.
> > > 
> > > Right, mdevctl isn't concerned with how a specific mdev is used, but I
> > > think what Connie is after is more the proposal from Daniel where
> > > libvirt can essentially manage mdevctl config files itself and then
> > > only invoke mdevctl for the dirty work of creating and deleting
> > > devices.  In fact, assuming systemd, libvirt could avoid direct
> > > interaction with mdevctl entirely, instead using systemctl device units
> > > to start and stop the mdevs.  Maybe where that proposal takes a turn is
> > > when we again consider non-systemd hosts, where maybe mdevctl needs to
> > > write out an init script per mdev and libvirt injecting itself into
> > > manipulation of the config files would either need to pe

Re: [libvirt] [PATCH] qemuProcessLaunch: Return earlier if spawning qemu failed

[Ján Tomko]

On Thu, May 23, 2019 at 11:03:29AM +0200, Michal Privoznik wrote:

If spawning qemu fails then we report an error and proceed to
writing status XML onto the disk. This is unnecessary as we are
sure that the domain is not running.

At the same time, if virPidFileReadPath() fails it returns
-errno. Use it in the error message. It may explain what went
wrong.

Signed-off-by: Michal Privoznik 
---
src/qemu/qemu_process.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH] remote: delete the avahi mDNS support

[Ján Tomko]

On Wed, Jun 19, 2019 at 05:09:26PM +0100, Daniel P. Berrangé wrote:

Libvirtd has long had integration with avahi for advertising libvirtd
using mDNS when TCP/TLS listening is enabled. For a long time the
virt-manager application had support for auto-detecting libvirtds
on the local network using mDNS, but this was removed last year

 commit fc8f8d5d7e3ba80a0771df19cf20e84a05ed2422
 Author: Cole Robinson 
 Date:   Sat Oct 6 20:55:31 2018 -0400

   connect: Drop avahi support

   Libvirtd can advertise itself over avahi. The feature is disabled by
   default though and in practice I hear of no one actually using it
   and frankly I don't think it's all that useful

   The 'Open Connection' wizard has a disproportionate amount of code
   devoted to this feature, but I don't think it's useful or worth
   maintaining, so let's drop it

I've never heard of any other applications having support for using
mDNS to detect libvirtd instances. Though it is theoretically possible
something exists out there, it is clearly going to be a niche use case
in the virt ecosystem as a whole.

By removing avahi integration we can cut down the dependancy chain for


dependency


the basic libvirtd install and reduce our code maint burden.

Signed-off-by: Daniel P. Berrangé 
---
cfg.mk|   3 -
configure.ac  |   3 -


[...]


.../output-data-initial-nomdns.json   |  72 --
.../virnetdaemondata/output-data-initial.json |   1 -
tests/virnetdaemontest.c  |  17 +-
34 files changed, 16 insertions(+), 1211 deletions(-)
delete mode 100644 m4/virt-avahi.m4
delete mode 100644 src/rpc/virnetservermdns.c
delete mode 100644 src/rpc/virnetservermdns.h
rename tests/virnetdaemondata/{input-data-admin-nomdns.json => 
input-data-admin.json} (100%)
delete mode 100644 tests/virnetdaemondata/input-data-initial-nomdns.json
rename tests/virnetdaemondata/{output-data-admin-nomdns.json => 
output-data-admin.json} (100%)
delete mode 100644 tests/virnetdaemondata/output-data-initial-nomdns.json



diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms
index eb7651a957..b63eac123f 100644
--- a/src/libvirt_remote.syms
+++ b/src/libvirt_remote.syms
@@ -133,7 +133,6 @@ virNetServerSetClientAuthenticated;
virNetServerSetClientLimits;
virNetServerSetThreadPoolParameters;
virNetServerSetTLSContext;
-virNetServerStart;
virNetServerUpdateServices;

Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 0/3] Ditch external JavaScript libraries

[Daniel P . Berrangé]

On Wed, Jun 19, 2019 at 05:22:56PM +0200, Martin Kletzander wrote:
> This is a response to all the discussions (mainly) other people had about all
> the JS code we're currently using, bundling, etc.
> 
> I would love some feedback on whether we can work on any of the solutions for
> getting rid of that external proxy.  We would have to:
> 
>  - either have our own proxy,

Ideally we'd not use any proxy imho

>  - send a 'Access-Control-Allow-Origin' header from the libvirt.org server to
>allow fetching the atom.xml or

Can you elaborate on this ?  This is something we'd need to set on the
libvirt.org httpd config, to allow it to access atom.xml from the
planet.virt-toos.org server ?

I can change libvirtd.org httpd as needed in general.

>  - be providing JSONP access to the RSS feed on virt-planet.

Again any more details on what this would imply ?  The planet web
server is just running centos7 httpd container in openshift:

  
https://libvirt.org/git/?p=virttools-planet.git;a=blob;f=openshift/templates/virttools-planet.json;h=28f162f43a1cf9b7d437981f7b941d0bf3da60e7;hb=HEAD#l208


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

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

Re: [libvirt] [PATCH] Revert "spec: Bump minimum supported Fedora version to 29"

[Ján Tomko]

On Tue, May 28, 2019 at 01:51:06PM +0200, Andrea Bolognani wrote:

On Tue, 2019-05-28 at 13:34 +0200, Daniel Veillard wrote:

On Tue, May 28, 2019 at 01:14:19PM +0200, Ján Tomko wrote:
> This reverts commit 8a1179831b5edc0a3590489eda693914fc0ff94f.
>
> It bumped the version prematurely, before the EOL of Fedora 28.


Yeah, that was my bad :( I'll be more careful about releases having
actually hit EOL before dropping support for them in the future.



I'd say for Fedora's case, we could happily wait even for half a year
more or so - by dropping it here we did not really gain anything - all
the libraries needed for libvirt to work on F28 are still way newer than
stuff from the long-term supported distros.

Jano


> Morover, this arbitrary bump did not let us perform any cleanups in the
> specfile.

  Right, that said I just tagged before this commit can be in.
I just won't make the binary rpms, after some discussion that's probably
not useful in general.


I agree we have no use for binary RPMs, especially when things like

 https://copr.fedorainfracloud.org/coprs/g/virtmaint-sig/virt-preview/

exist.


I would ACK but F28 will be dropped from support end of the week, so
maybe we don't want to flip/flop that at this point,


Let's just leave it as is, and just make sure we don't repeat the
same mistake in the future.



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

Re: [libvirt] [PATCH] docs: Advertise pcie-to-pci-bridge for use on q35

[Ján Tomko]

On Fri, Jun 07, 2019 at 10:53:17AM +0200, Andrea Bolognani wrote:

We support pcie-to-pci-bridge, and prefer it to
dmi-to-pci-bridge, since libvirt 4.3.0, but we didn't
update all the documentation accordingly at the time.

Signed-off-by: Andrea Bolognani 
---
docs/formatdomain.html.in | 10 +-
docs/pci-hotplug.html.in  |  5 ++---
2 files changed, 7 insertions(+), 8 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 1/3] docs: Use our own implementation for fetching the RSS data

[Daniel P . Berrangé]

On Wed, Jun 19, 2019 at 05:22:57PM +0200, Martin Kletzander wrote:
> This is just a small script I wrote.  It works the same way as all the 
> libraries
> together which we are bundling, but with just JS.  The only difference is that
> the day of the date is formatted as 2-digit, but this should be a bug in
> Firefox (at least locally for me) as the documentation states that type
> 'numeric' should actually not have leading zeros.
> 
> It will not be executed when the page is loaded locally.
> 
> It also uses the same proxy that query-rss uses.  We can get rid of that, but
> we'd either need to have our own proxy, send a sss header to allow fetching 
> the
> atom.xml or providing JSONP access to the RSS feed on virt-planet.
> 
> Signed-off-by: Martin Kletzander 
> ---
>  docs/index.html.in | 13 -
>  docs/js/main.js| 47 ++
>  2 files changed, 47 insertions(+), 13 deletions(-)
> 
> diff --git a/docs/index.html.in b/docs/index.html.in
> index f593445d061d..fa75289cad0f 100644
> --- a/docs/index.html.in
> +++ b/docs/index.html.in
> @@ -5,19 +5,6 @@
>   
>   
>   
> -
> -
> -  
> -

This code only runs on the index.html page which makes sense as that's
where the RSS feed is displayed

>
>
>  The virtualization API
> diff --git a/docs/js/main.js b/docs/js/main.js
> index e57b9f47ac11..07f79c7ff903 100644
> --- a/docs/js/main.js
> +++ b/docs/js/main.js
> @@ -29,6 +29,53 @@ function pageload() {
>  
>  simpleSearch = document.getElementById("simplesearch")
>  simplesearch.addEventListener("submit", advancedsearch)
> +
> +docLoc = document.location;
> +if (docLoc.protocol != "file:" ||
> +docLoc.origin != "null" ||
> +docLoc.host !== "" ||
> +docLoc.hostname !== "") {
> +fetchRSS()
> +}
> +}

...but unless I'm mis-reading something, this code runs on every single
page on the site. I would have expected the fetcRSS() call to have
been made from the 

Re: [libvirt] [PATCH 3/3] test_driver: consider DHCP ranges in testDomainInterfaceAddresses

[Ilias Stamatis]

On Wed, Jun 19, 2019 at 4:19 PM Michal Privoznik  wrote:
>
> On 6/19/19 4:04 PM, Ilias Stamatis wrote:
> > On Wed, Jun 19, 2019 at 3:48 PM Michal Privoznik  
> > wrote:
> >>
> >> On 6/19/19 1:18 PM, Ilias Stamatis wrote:
> >>> testDomainInterfaceAddresses always returns the same hard-coded
> >>> addresses. Change the behavior such as if there is a DHCP range defined,
> >>> addresses are returned from that pool.
> >>>
> >>> The specific address returned depends on both the domain id and the
> >>> specific guest interface in an attempt to return unique addresses *most
> >>> of the time*.
> >>>
> >>> Additionally, return IPv6 addresses too when needed.
> >>>
> >>> Signed-off-by: Ilias Stamatis 
> >>> ---
> >>>src/test/test_driver.c | 78 +++---
> >>>1 file changed, 73 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> >>> index 0c2cfdd2f7..96142b549c 100644
> >>> --- a/src/test/test_driver.c
> >>> +++ b/src/test/test_driver.c
> >>> @@ -27,6 +27,7 @@
> >>>#include 
> >>>#include 
> >>>#include 
> >>> +#include 
> >>>
> >>>
> >>>#include "virerror.h"
> >>> @@ -3380,6 +3381,11 @@ static int testDomainBlockStats(virDomainPtr 
> >>> domain,
> >>>return ret;
> >>>}
> >>>
> >>> +
> >>> +static virNetworkObjPtr testNetworkObjFindByName(testDriverPtr privconn, 
> >>> const char *name);
> >>> +static virNetworkPtr testNetworkLookupByName(virConnectPtr conn, const 
> >>> char *name);
> >>> +
> >>> +
> >>>static int
> >>>testDomainInterfaceAddresses(virDomainPtr dom,
> >>> virDomainInterfacePtr **ifaces,
> >>> @@ -3388,11 +3394,18 @@ testDomainInterfaceAddresses(virDomainPtr dom,
> >>>{
> >>>size_t i;
> >>>size_t ifaces_count = 0;
> >>> +size_t addr_offset;
> >>>int ret = -1;
> >>>char macaddr[VIR_MAC_STRING_BUFLEN];
> >>> +char ip_str[INET6_ADDRSTRLEN];
> >>> +struct sockaddr_in ipv4_addr;
> >>> +struct sockaddr_in6 ipv6_addr;
> >>>virDomainObjPtr vm = NULL;
> >>>virDomainInterfacePtr iface = NULL;
> >>>virDomainInterfacePtr *ifaces_ret = NULL;
> >>> +virNetworkPtr net = NULL;
> >>> +virNetworkObjPtr net_obj = NULL;
> >>> +virNetworkDefPtr net_obj_def = NULL;
> >>>
> >>>virCheckFlags(0, -1);
> >>>
> >>> @@ -3413,6 +3426,16 @@ testDomainInterfaceAddresses(virDomainPtr dom,
> >>>if (vm->def->nets[i]->type != VIR_DOMAIN_NET_TYPE_NETWORK)
> >>>continue;
> >>>
> >>> +virObjectUnref(net);
> >>> +if (!(net = testNetworkLookupByName(dom->conn,
> >>> +
> >>> vm->def->nets[i]->data.network.name)))
> >>> +goto cleanup;
> >>> +
> >>> +if (!(net_obj = testNetworkObjFindByName(net->conn->privateData, 
> >>> net->name)))
> >>> +goto cleanup;
> >>
> >> This is needless IMO. I mean, @net variable looks redundant to me, why
> >> not look up the network object directly? For instance:
> >>
> >> if (!(net_obj = testNetworkObjFindByName(dom->conn->privateData,
> >>
> >> vm->def->nets[i]->data.network.name)))
> >> goto cleanup;
> >
> > Aah, right. I will remove this.
> >
> >>
> >> But looking further at next hunk, I wonder if it makes sense to separate
> >> it into a separate function. Otherwise the code looks good. A bit
> >> complicated, but that was expected, since dealing with IP addresses is
> >> never a few lines of code :-(
> >
> > I managed to do it just a tiny bit simpler by using a virSocketAddr
> > instead of the 2 struct sockaddr_in ones.
>
> Awesome!
>
> >
> > But which part do you want me to separate exactly?
>
> My idea was to keep the outer line loop, and probably call functions
> from it. Something among these lines:
>
> for (i = 0; i < vm->def->nnets; i++) {
>   if (vm->def->nets[i]->type == VIR_DOMAIN_NET_TYPE_NETWORK)
> iface = testDomainInterfaceAddressFromNet(vm->def->nets[i]);
>   else
> iface = testDomainInterfaceAddressDefault(vm->def->nets[i]);
>
>   if (!iface)
> error;
>
>   VIR_APPEND_ELEMENT_INPLACE(ifaces_ret, ifaces_count, iface);
> }
>
> Alternatively, we might allocate @iface inside the loop, set its name and MAC 
> address also in the loop (so that the above functions don't duplicate code) 
> and then pass it to the functions just to fill iface->addrs.
>
> Michal

I just sent a new version in which I re-wrote most of the logic. I
think now the patch is much simpler and shorter than before so we
don't need to extract anything into separate functions. However if you
think it can be even simpler or it can somehow benefit from extracting
some code into different functions let me know.

Thanks for the review!

Ilias

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

Re: [libvirt] [PATCH 3/3] docs: Remove unused JS libraries

[Andrea Bolognani]

On Wed, 2019-06-19 at 17:22 +0200, Martin Kletzander wrote:
[...]
>  javascript = \
> -  js/main.js \
> -  js/jquery-3.1.1.min.js \
> -  js/jquery.rss.min.js \
> -  js/moment.min.js
> -
> +  js/main.js

Please keep the list as

  javascript = \
js/main.js \
$(NULL)

Smaller diff, both right now and in the future :)

[...]
>  
>  http://www.w3.org/1999/xhtml";>
>
> - 
> - 
> - 
>

You can get rid of  entirely now.


With the above addressed,

  Reviewed-by: Andrea Bolognani 

but please give Dan a chance to raise any concerns he might have
before pushing :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 2/3] docs: use case sensitive javascript

[Andrea Bolognani]

On Wed, 2019-06-19 at 17:22 +0200, Martin Kletzander wrote:
> Signed-off-by: Martin Kletzander 
> ---
>  docs/js/main.js | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

In the subject:

  s/use/Use/
  s/javascript/JavaScript/

Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 1/3] docs: Use our own implementation for fetching the RSS data

[Andrea Bolognani]

On Wed, 2019-06-19 at 17:22 +0200, Martin Kletzander wrote:
[...]
> +docLoc = document.location;
> +if (docLoc.protocol != "file:" ||
> +docLoc.origin != "null" ||
> +docLoc.host !== "" ||
> +docLoc.hostname !== "") {
> +fetchRSS()
> +}

This probably doesn't need to be this complicated, just the check
against .protocol is probably okay. If we need more flexibility we
can just add it later.

[...]
> +function fetchRSS() {
> +cb = "jsonpRSSFeedCallback"
> +window["jsonpRSSFeedCallback"] = function (data) {

You should be reusing cb here :)

[...]
> +dateOpts = { day: 'numeric', month: 'short', year: 'numeric'}

You're using both single and double quotes... Please stick with a
single style, more specifically the latter which is already used
consistently throughout the file.

[...]
> +script.src = "https://feedrapp.herokuapp.com/";
> +script.src += 
> `?q=http%3A%2F%2Fplanet.virt-tools.org%2Fatom.xml&callback=${cb}`

You didn't turn this into a clear URL :) That's alright, if it can
be done at all you might very well fix it up later.


With the above addressed,

  Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[libvirt] [PATCH v2] test_driver: properly handle DHCP ranges and IPv6 networks in testDomainInterfaceAddresses

[Ilias Stamatis]

testDomainInterfaceAddresses always returns the same hard-coded
addresses. Change the behavior such as if there is a DHCP range defined,
addresses are returned from that pool.

The specific address returned depends on both the domain id and the
specific guest interface in an attempt to return unique addresses *most
of the time*.

Additionally, properly handle IPv6 networks which were previously
ignored completely.

Signed-off-by: Ilias Stamatis 
---
 src/test/test_driver.c | 44 +-
 1 file changed, 39 insertions(+), 5 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 2a0ffbc6c5..21bd95941e 100755
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3414,6 +3414,10 @@ static int testDomainBlockStats(virDomainPtr domain,
 return ret;
 }

+
+static virNetworkObjPtr testNetworkObjFindByName(testDriverPtr privconn, const 
char *name);
+
+
 static int
 testDomainInterfaceAddresses(virDomainPtr dom,
  virDomainInterfacePtr **ifaces,
@@ -3422,11 +3426,15 @@ testDomainInterfaceAddresses(virDomainPtr dom,
 {
 size_t i;
 size_t ifaces_count = 0;
+size_t addr_offset;
 int ret = -1;
 char macaddr[VIR_MAC_STRING_BUFLEN];
 virDomainObjPtr vm = NULL;
 virDomainInterfacePtr iface = NULL;
 virDomainInterfacePtr *ifaces_ret = NULL;
+virSocketAddr addr;
+virNetworkObjPtr net = NULL;
+virNetworkDefPtr net_def = NULL;

 virCheckFlags(0, -1);

@@ -3447,6 +3455,12 @@ testDomainInterfaceAddresses(virDomainPtr dom,
 goto cleanup;

 for (i = 0; i < vm->def->nnets; i++) {
+if (!(net = testNetworkObjFindByName(dom->conn->privateData,
+ 
vm->def->nets[i]->data.network.name)))
+goto cleanup;
+
+net_def = virNetworkObjGetDef(net);
+
 if (VIR_ALLOC(iface) < 0)
 goto cleanup;

@@ -3460,14 +3474,33 @@ testDomainInterfaceAddresses(virDomainPtr dom,
 if (VIR_ALLOC(iface->addrs) < 0)
 goto cleanup;

-iface->addrs[0].type = VIR_IP_ADDR_TYPE_IPV4;
-iface->addrs[0].prefix = 24;
-if (virAsprintf(&iface->addrs[0].addr, "192.168.0.%zu", 1 + i) < 0)
-goto cleanup;
-
 iface->naddrs = 1;
+iface->addrs[0].prefix = 
virSocketAddrGetIPPrefix(&net_def->ips->address,
+  
&net_def->ips->netmask,
+  
net_def->ips->prefix);
+
+if (net_def->ips->nranges > 0)
+addr = net_def->ips->ranges[0].start;
+else
+addr = net_def->ips->address;
+
+/* try using different addresses per different inf and domain */
+addr_offset = 20 * (vm->def->id - 1) + i + 1;
+
+if (net_def->ips->family && STREQ(net_def->ips->family, "ipv6")) {
+iface->addrs[0].type = VIR_IP_ADDR_TYPE_IPV6;
+addr.data.inet6.sin6_addr.s6_addr[15] += addr_offset;
+} else {
+iface->addrs[0].type = VIR_IP_ADDR_TYPE_IPV4;
+addr.data.inet4.sin_addr.s_addr = \
+htonl(ntohl(addr.data.inet4.sin_addr.s_addr) + addr_offset);
+}
+
+if (!(iface->addrs[0].addr = virSocketAddrFormat(&addr)))
+goto cleanup;

 VIR_APPEND_ELEMENT_INPLACE(ifaces_ret, ifaces_count, iface);
+virNetworkObjEndAPI(&net);
 }

 VIR_STEAL_PTR(*ifaces, ifaces_ret);
@@ -3475,6 +3508,7 @@ testDomainInterfaceAddresses(virDomainPtr dom,

  cleanup:
 virDomainObjEndAPI(&vm);
+virNetworkObjEndAPI(&net);

 if (ifaces_ret) {
 for (i = 0; i < ifaces_count; i++)
--
2.22.0

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

Re: [libvirt] [PATCH 10/13] backup: test: Implement metadata tracking for checkpoint APIs

[Peter Krempa]

On Tue, Jun 18, 2019 at 22:47:51 -0500, Eric Blake wrote:
> A lot of this work heavily copies from the existing snapshot APIs.
> The test driver doesn't really have to do anything more than just
> expose an interface into libvirt metadata, making it possible to test
> saving and restoring XML, and tracking relations between multiple
> checkpoints.
> 
> Signed-off-by: Eric Blake 
> ---
>  src/test/test_driver.c | 427 +
>  1 file changed, 427 insertions(+)

Looks good to me at the first glance but I didn't test it or reviewed it
extra thoroughly.

I'm willing to ACK it since it's the test driver.


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

Re: [libvirt] [PATCH 13/13] backup: qemu: Implement VIR_DOMAIN_CHECKPOINT_XML_SIZE flag

[Peter Krempa]

On Tue, Jun 18, 2019 at 22:47:54 -0500, Eric Blake wrote:
> Once a checkpoint has been created, it is desirable to estimate the
> size of the disk delta that is represented between the checkpoint and
> the current operation. To do this, we have to scrape information out
> of QMP query-block on a request from the user.
> ---
>  src/qemu/qemu_monitor.h  |  4 ++
>  src/qemu/qemu_monitor_json.h |  3 ++
>  src/qemu/qemu_driver.c   | 56 +-
>  src/qemu/qemu_monitor.c  | 11 ++
>  src/qemu/qemu_monitor_json.c | 76 
>  5 files changed, 149 insertions(+), 1 deletion(-)

[...]

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 0cdb2dd1e2..585a0203eb 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c

[]

> @@ -17399,9 +17403,59 @@ 
> qemuDomainCheckpointGetXMLDesc(virDomainCheckpointPtr checkpoint,
>  goto cleanup;
>  chkdef = virDomainCheckpointObjGetDef(chk);
> 
> +if (flags & VIR_DOMAIN_CHECKPOINT_XML_SIZE) {
> +/* TODO: for non-current checkpoint, this requires a QMP sequence per
> +   disk, since the stat of one bitmap in isolation is too low,
> +   and merely adding bitmap sizes may be too high:
> + block-dirty-bitmap-create tmp
> + for each bitmap from checkpoint to current:
> +   add bitmap to src_list
> + block-dirty-bitmap-merge dst=tmp src_list
> + query-block and read tmp size
> + block-dirty-bitmap-remove tmp
> +   So for now, go with simpler query-blocks only for current.
> +*/
> +if (virDomainCheckpointGetCurrent(vm->checkpoints) != chk) {
> +virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> +   _("cannot compute size for non-current checkpoint 
> '%s'"),
> +   checkpoint->name);
> +goto cleanup;
> +}
> +
> +if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
> +goto cleanup;
> +
> +if (virDomainObjCheckActive(vm) < 0)
> +goto endjob;
> +
> +if (qemuBlockNodeNamesDetect(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
> +goto endjob;

Again, I pointed out multiple times that this should not be done, or at
least only in non-blockdev case.

> +
> +/* TODO: Shouldn't need to recompute node names. */

Well, they are not meant to be stable so if you want to match it with
the saved definition it may become fun.

Also after a snapshot the checkpoint def and VM def will differ same as
the positioning of the nodenames.

This code does not seem to take it into account in any way.

And this is getting into bigger amount of technical debt which will need
to be addressed if we want to allow checkpoints and snapshots together.

> +for (i = 0; i < chkdef->ndisks; i++) {
> +virDomainCheckpointDiskDef *disk = &chkdef->disks[i];
> +
> +if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
> +continue;
> +VIR_FREE(chk->def->dom->disks[disk->idx]->src->nodeformat);


> +if (VIR_STRDUP(chk->def->dom->disks[disk->idx]->src->nodeformat,
> +   qemuBlockNodeLookup(vm, disk->name)) < 0)
> +goto endjob;
> +}
> +
> +priv = vm->privateData;
> +qemuDomainObjEnterMonitor(driver, vm);
> +rc = qemuMonitorUpdateCheckpointSize(priv->mon, chkdef);
> +if (qemuDomainObjExitMonitor(driver, vm) < 0)

[...]

> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 2dcd65d86f..4c232105c2 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -2760,6 +2760,82 @@ int qemuMonitorJSONBlockResize(qemuMonitorPtr mon,
>  return ret;
>  }
> 
> +int qemuMonitorJSONUpdateCheckpointSize(qemuMonitorPtr mon,
> +virDomainCheckpointDefPtr chk)
> +{
> +int ret = -1;
> +size_t i, j;
> +virJSONValuePtr devices;
> +
> +if (!(devices = qemuMonitorJSONQueryBlock(mon)))
> +return -1;

Isn't this data available in 'query-named-block-nodes'?

> +
> +for (i = 0; i < virJSONValueArraySize(devices); i++) {
> +virJSONValuePtr dev = virJSONValueArrayGet(devices, i);
> +virJSONValuePtr inserted;
> +virJSONValuePtr bitmaps = NULL;
> +const char *node;
> +virDomainCheckpointDiskDefPtr disk;
> +
> +if (!(dev = qemuMonitorJSONGetBlockDev(devices, i)))
> +goto cleanup;


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

Re: [libvirt] [PATCH 12/13] backup: Wire up qemu checkpoint commands over QMP

[Peter Krempa]

On Tue, Jun 18, 2019 at 22:47:53 -0500, Eric Blake wrote:
> Time to actually issue the QMP transactions that create and
> delete persistent checkpoints.  For create, we only need one
> transaction: inside, we visit all disks affected by the
> checkpoint, and create a new enabled bitmap, as well as
> disabling the bitmap of the parent checkpoint (if any).  For
> deletion, we need multiple calls: if the checkpoint to be
> deleted is active, we must enable the parent; then we must
> merge the existing checkpoint into the parent, and finally
> we can delete the checkpoint.

At this point I'm lacking the interlocking with snapshots. It may be
posted as a separate patch, but none of this code should go in unless
that one is ACK'd.

> 
> Signed-off-by: Eric Blake 
> ---
>  src/qemu/qemu_domain.c | 105 ++---
>  src/qemu/qemu_driver.c |  92 ++--
>  2 files changed, 165 insertions(+), 32 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index e128dc169b..1364227e42 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -8809,45 +8809,93 @@ qemuDomainCheckpointDiscard(virQEMUDriverPtr driver,
>  char *chkFile = NULL;
>  int ret = -1;
>  virDomainMomentObjPtr parentchk = NULL;
> +virDomainCheckpointDefPtr parentdef = NULL;
>  virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> +size_t i, j;
> +virJSONValuePtr arr = NULL;
> 
> -if (!metadata_only) {
> -if (!virDomainObjIsActive(vm)) {
> -virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> -   _("cannot remove checkpoint from inactive 
> domain"));
> -goto cleanup;
> -} else {
> -/* TODO: Implement QMP sequence to merge bitmaps */
> -// qemuDomainObjPrivatePtr priv;
> -// priv = vm->privateData;
> -// qemuDomainObjEnterMonitor(driver, vm);
> -// /* we continue on even in the face of error */
> -// qemuMonitorDeleteCheckpoint(priv->mon, chk->def->name);
> -// ignore_value(qemuDomainObjExitMonitor(driver, vm));
> -}
> +if (!metadata_only && !virDomainObjIsActive(vm)) {
> +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> +   _("cannot remove checkpoint from inactive domain"));
> +goto cleanup;
>  }
> 
>  if (virAsprintf(&chkFile, "%s/%s/%s.xml", cfg->checkpointDir,
>  vm->def->name, chk->def->name) < 0)
>  goto cleanup;
> 
> +if (chk->def->parent_name) {
> +parentchk = virDomainCheckpointFindByName(vm->checkpoints,
> +  chk->def->parent_name);
> +if (!parentchk) {
> +VIR_WARN("missing parent checkpoint matching name '%s'",
> + chk->def->parent_name);
> +}
> +parentdef = virDomainCheckpointObjGetDef(parentchk);
> +}
> +
> +if (!metadata_only) {
> +qemuDomainObjPrivatePtr priv = vm->privateData;
> +bool success = true;
> +virDomainCheckpointDefPtr chkdef = virDomainCheckpointObjGetDef(chk);
> +
> +if (qemuBlockNodeNamesDetect(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
> +goto cleanup;

I was complaining about this in the previous version.

> +qemuDomainObjEnterMonitor(driver, vm);
> +for (i = 0; i < chkdef->ndisks; i++) {
> +virDomainCheckpointDiskDef *disk = &chkdef->disks[i];
> +const char *node;
> +
> +if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
> +continue;
> +
> +node = qemuBlockNodeLookup(vm, disk->name);
> +if (parentchk) {
> +arr = virJSONValueNewArray();
> +if (!arr ||
> +virJSONValueArrayAppendString(arr, disk->bitmap) < 0) {
> +success = false;
> +break;
> +}
> +
> +for (j = 0; j < parentdef->ndisks; j++) {
> +virDomainCheckpointDiskDef *disk2;
> +
> +disk2 = &parentdef->disks[j];
> +if (STRNEQ(disk->name, disk2->name))
> +continue;
> +if (chk == 
> virDomainCheckpointGetCurrent(vm->checkpoints) &&
> +qemuMonitorEnableBitmap(priv->mon, node,
> +disk2->bitmap) < 0) {
> +success = false;
> +break;
> +}
> +if (qemuMonitorMergeBitmaps(priv->mon, node,
> +disk2->bitmap, &arr) < 0) {

We definitely need interlocking as this will not work across the backing
chain as it seems to reference the 'node'

> +success = false;
> +break;
> + 

[libvirt] [PATCH] remote: delete the avahi mDNS support

[Daniel P . Berrangé]

Libvirtd has long had integration with avahi for advertising libvirtd
using mDNS when TCP/TLS listening is enabled. For a long time the
virt-manager application had support for auto-detecting libvirtds
on the local network using mDNS, but this was removed last year

  commit fc8f8d5d7e3ba80a0771df19cf20e84a05ed2422
  Author: Cole Robinson 
  Date:   Sat Oct 6 20:55:31 2018 -0400

connect: Drop avahi support

Libvirtd can advertise itself over avahi. The feature is disabled by
default though and in practice I hear of no one actually using it
and frankly I don't think it's all that useful

The 'Open Connection' wizard has a disproportionate amount of code
devoted to this feature, but I don't think it's useful or worth
maintaining, so let's drop it

I've never heard of any other applications having support for using
mDNS to detect libvirtd instances. Though it is theoretically possible
something exists out there, it is clearly going to be a niche use case
in the virt ecosystem as a whole.

By removing avahi integration we can cut down the dependancy chain for
the basic libvirtd install and reduce our code maint burden.

Signed-off-by: Daniel P. Berrangé 
---
 cfg.mk|   3 -
 configure.ac  |   3 -
 docs/internals/rpc.html.in|   7 -
 docs/remote.html.in   |  17 -
 docs/windows.html.in  |   1 -
 libvirt.spec.in   |   3 -
 m4/virt-avahi.m4  |  30 -
 mingw-libvirt.spec.in |   1 -
 po/POTFILES   |   1 -
 src/libvirt_remote.syms   |  13 -
 src/locking/lock_daemon.c |   6 +-
 src/logging/log_daemon.c  |   6 +-
 src/lxc/lxc_controller.c  |   3 +-
 src/remote/libvirtd.aug   |   2 -
 src/remote/libvirtd.conf  |  17 -
 src/remote/remote_daemon.c|  18 +-
 src/remote/remote_daemon_config.c |  29 -
 src/remote/remote_daemon_config.h |   3 -
 src/remote/test_libvirtd.aug.in   |   2 -
 src/rpc/Makefile.inc.am   |   4 -
 src/rpc/virnetdaemon.c|  14 -
 src/rpc/virnetserver.c|  64 +-
 src/rpc/virnetserver.h|   6 +-
 src/rpc/virnetservermdns.c| 682 --
 src/rpc/virnetservermdns.h| 107 ---
 tests/virconfdata/libvirtd.conf   |  17 -
 tests/virconfdata/libvirtd.out|  14 -
 ...dmin-nomdns.json => input-data-admin.json} |   0
 .../input-data-initial-nomdns.json|  63 --
 .../virnetdaemondata/input-data-initial.json  |   1 -
 ...min-nomdns.json => output-data-admin.json} |   0
 .../output-data-initial-nomdns.json   |  72 --
 .../virnetdaemondata/output-data-initial.json |   1 -
 tests/virnetdaemontest.c  |  17 +-
 34 files changed, 16 insertions(+), 1211 deletions(-)
 delete mode 100644 m4/virt-avahi.m4
 delete mode 100644 src/rpc/virnetservermdns.c
 delete mode 100644 src/rpc/virnetservermdns.h
 rename tests/virnetdaemondata/{input-data-admin-nomdns.json => 
input-data-admin.json} (100%)
 delete mode 100644 tests/virnetdaemondata/input-data-initial-nomdns.json
 rename tests/virnetdaemondata/{output-data-admin-nomdns.json => 
output-data-admin.json} (100%)
 delete mode 100644 tests/virnetdaemondata/output-data-initial-nomdns.json

diff --git a/cfg.mk b/cfg.mk
index c0c240b2c0..9465838175 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -146,9 +146,6 @@ useless_free_options = \
   --name=virJSONValueFree \
   --name=virLastErrFreeData \
   --name=virNetMessageFree \
-  --name=virNetServerMDNSFree \
-  --name=virNetServerMDNSEntryFree \
-  --name=virNetServerMDNSGroupFree \
   --name=virNWFilterDefFree \
   --name=virNWFilterEntryFree \
   --name=virNWFilterHashTableFree \
diff --git a/configure.ac b/configure.ac
index 1f05055d6f..3489a5725e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -245,7 +245,6 @@ LIBVIRT_LINKER_NO_UNDEFINED
 LIBVIRT_ARG_APPARMOR
 LIBVIRT_ARG_ATTR
 LIBVIRT_ARG_AUDIT
-LIBVIRT_ARG_AVAHI
 LIBVIRT_ARG_BASH_COMPLETION
 LIBVIRT_ARG_BLKID
 LIBVIRT_ARG_CAPNG
@@ -284,7 +283,6 @@ LIBVIRT_CHECK_APPARMOR
 LIBVIRT_CHECK_ATOMIC
 LIBVIRT_CHECK_ATTR
 LIBVIRT_CHECK_AUDIT
-LIBVIRT_CHECK_AVAHI
 LIBVIRT_CHECK_BASH_COMPLETION
 LIBVIRT_CHECK_BLKID
 LIBVIRT_CHECK_CAPNG
@@ -970,7 +968,6 @@ LIBVIRT_RESULT_ACL
 LIBVIRT_RESULT_APPARMOR
 LIBVIRT_RESULT_ATTR
 LIBVIRT_RESULT_AUDIT
-LIBVIRT_RESULT_AVAHI
 LIBVIRT_RESULT_BASH_COMPLETION
 LIBVIRT_RESULT_BLKID
 LIBVIRT_RESULT_CAPNG
diff --git a/docs/internals/rpc.html.in b/docs/internals/rpc.html.in
index 048192f818..40d844f31c 100644
--- a/docs/internals/rpc.html.in
+++ b/docs/internals/rpc.html.in
@@ -539,13 +539,6 @@ C <--  |32| 8 | 1 | 3 | 1 | 1 | 0 

Re: [libvirt] [PATCH 2/3] docs: use case sensitive javascript

[Christophe de Dinechin]

> On 19 Jun 2019, at 17:22, Martin Kletzander  wrote:
> 
> Signed-off-by: Martin Kletzander 
> ---
> docs/js/main.js | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/docs/js/main.js b/docs/js/main.js
> index 07f79c7ff903..668bf752b73d 100644
> --- a/docs/js/main.js
> +++ b/docs/js/main.js
> @@ -28,7 +28,7 @@ function pageload() {
> advancedSearch.className = "advancedsearch"
> 
> simpleSearch = document.getElementById("simplesearch")
> -simplesearch.addEventListener("submit", advancedsearch)
> +simpleSearch.addEventListener("submit", advancedsearch)

Reviewed-by: Christophe de Dinechin 

> 
> docLoc = document.location;
> if (docLoc.protocol != "file:" ||
> -- 
> 2.21.0
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

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

[libvirt] [PATCH 3/3] docs: Remove unused JS libraries

[Martin Kletzander]

Signed-off-by: Martin Kletzander 
---
 docs/Makefile.am|  6 +-
 docs/index.html.in  |  3 ---
 docs/js/jquery-3.1.1.min.js |  4 
 docs/js/jquery.rss.min.js   | 11 ---
 docs/js/moment.min.js   |  7 ---
 5 files changed, 1 insertion(+), 30 deletions(-)
 delete mode 100644 docs/js/jquery-3.1.1.min.js
 delete mode 100644 docs/js/jquery.rss.min.js
 delete mode 100644 docs/js/moment.min.js

diff --git a/docs/Makefile.am b/docs/Makefile.am
index 0c899f958026..3e2cc39013f3 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -66,11 +66,7 @@ css = \
   main.css
 
 javascript = \
-  js/main.js \
-  js/jquery-3.1.1.min.js \
-  js/jquery.rss.min.js \
-  js/moment.min.js
-
+  js/main.js
 
 fonts = \
   fonts/LICENSE.md \
diff --git a/docs/index.html.in b/docs/index.html.in
index fa75289cad0f..a16a2e51d4be 100644
--- a/docs/index.html.in
+++ b/docs/index.html.in
@@ -2,9 +2,6 @@
 
 http://www.w3.org/1999/xhtml";>
   
- 
- 
- 
   
   
 The virtualization API
diff --git a/docs/js/jquery-3.1.1.min.js b/docs/js/jquery-3.1.1.min.js
deleted file mode 100644
index 4c5be4c0fbe2..
--- a/docs/js/jquery-3.1.1.min.js
+++ /dev/null
@@ -1,4 +0,0 @@
-/*! jQuery v3.1.1 | (c) jQuery Foundation | jquery.org/license */
-!function(a,b){"use strict";"object"==typeof module&&"object"==typeof 
module.exports?module.exports=a.document?b(a,!0):function(a){if(!a.document)throw
 new Error("jQuery requires a window with a document");return 
b(a)}:b(a)}("undefined"!=typeof window?window:this,function(a,b){"use 
strict";var 
c=[],d=a.document,e=Object.getPrototypeOf,f=c.slice,g=c.concat,h=c.push,i=c.indexOf,j={},k=j.toString,l=j.hasOwnProperty,m=l.toString,n=m.call(Object),o={};function
 p(a,b){b=b||d;var 
c=b.createElement("script");c.text=a,b.head.appendChild(c).parentNode.removeChild(c)}var
 q="3.1.1",r=function(a,b){return new 
r.fn.init(a,b)},s=/^[\s\uFEFF\xA0]+|[\s\uFEFF\xA0]+$/g,t=/^-ms-/,u=/-([a-z])/g,v=function(a,b){return
 
b.toUpperCase()};r.fn=r.prototype={jquery:q,constructor:r,length:0,toArray:function(){return
 f.call(this)},get:function(a){return 
null==a?f.call(this):a<0?this[a+this.length]:this[a]},pushStack:function(a){var 
b=r.merge(this.constructor(),a);return 
b.prevObject=this,b},each:function(a){retu
 rn r.each(this,a)},map:function(a){return 
this.pushStack(r.map(this,function(b,c){return 
a.call(b,c,b)}))},slice:function(){return 
this.pushStack(f.apply(this,arguments))},first:function(){return 
this.eq(0)},last:function(){return this.eq(-1)},eq:function(a){var 
b=this.length,c=+a+(a<0?b:0);return 
this.pushStack(c>=0&&c0&&b-1 in a)}var x=function(a){var 
b,c,d,e,f,g,h,i,j,k,l,m,n,o,p,q,r,s,t,u="sizzle"+1*new 
Date,v=a.document,w=0,x=0,y=ha(),z=ha(),A=ha(),B=function(a,b){return 
a===b&&(l=!0),0},C={}.hasOwnProperty,D=[],E=D.pop,F=D.push,G=D.push,H=D.slice,I=function(a,b){for(var
 c=0,d=a.length;c
 +~]|"+K+")"+K+"*"),S=new RegExp("="+K+"*([^\\]'\"]*?)"+K+"*\\]","g"),T=new 
RegExp(N),U=new RegExp("^"+L+"$"),V={ID:new RegExp("^#("+L+")"),CLASS:new 
RegExp("^\\.("+L+")"),TAG:new RegExp("^("+L+"|[*])"),ATTR:new 
RegExp("^"+M),PSEUDO:new RegExp("^"+N),CHILD:new 
RegExp("^:(only|first|last|nth|nth-last)-(child|of-type)(?:\\("+K+"*(even|odd|(([+-]|)(\\d*)n|)"+K+"*(?:([+-]|)"+K+"*(\\d+)|))"+K+"*\\)|)","i"),bool:new
 RegExp("^(?:"+J+")$","i"),needsContext:new 
RegExp("^"+K+"*[>+~]|:(even|odd|eq|gt|lt|nth|first|last)(?:\\("+K+"*((?:-\\d)?\\d*)"+K+"*\\)|)(?=[^-]|$)","i")},W=/^(?:input|select|textarea|button)$/i,X=/^h\d$/i,Y=/^[^{]+\{\s*\[native
 \w/,Z=/^(?:#([\w-]+)|(\w+)|\.([\w-]+))$/,$=/[+~]/,_=new 
RegExp("([\\da-f]{1,6}"+K+"?|("+K+")|.)","ig"),aa=function(a,b,c){var 
d="0x"+b-65536;return 
d!==d||c?b:d<0?String.fromCharCode(d+65536):String.fromCharCode(d>>10|55296,1023&d|56320)},ba=/([\0-\x1f\x7f]|^-?\d)|^-$|[^\0-\x1f\x7f-\u\w-]/g,ca=function(a,b){return
 b?"\0"===a?"\ufffd":a.slice(0,-
 1)+"\\"+a.charCodeAt(a.length-1).toString(16)+" 
":"\\"+a},da=function(){m()},ea=ta(function(a){return 
a.disabled===!0&&("form"in a||"label"in 
a)},{dir:"parentNode",next:"legend"});try{G.apply(D=H.call(v.childNodes),v.childNodes),D[v.childNodes.length].nodeType}catch(fa){G={apply:D.length?function(a,b){F.apply(a,H.call(b))}:function(a,b){var
 c=a.length,d=0;while(a[c++]=b[d++]);a.length=c-1}}}function ga(a,b,d,e){var 
f,h,j,k,l,o,r,s=b&&b.ownerDocument,w=b?b.nodeType:9;if(d=d||[],"string"!=typeof 
a||!a||1!==w&&9!==w&&11!==w)return 
d;if(!e&&((b?b.ownerDocument||b:v)!==n&&m(b),b=b||n,p)){if(11!==w&&(l=Z.exec(a)))if(f=l[1]){if(9===w){if(!(j=b.getElementById(f)))return
 d;if(j.id===f)return d.push(j),d}else 
if(s&&(j=s.getElementById(f))&&t(b,j)&&j.id===f)return 
d.push(j),d}else{if(l[2])return 
G.apply(d,b.getElementsByTagName(a)),d;if((f=l[3])&&c.getElementsByClassName&&b.getElementsByClassName)return
 G.apply(d,b.getElementsByClassName(f)),d}if(c.qsa&&!A[a+" 
"]&&(!q||!q.test(a))){if(1!==w)s=
 b,r=a;else 
if("object"!==b.nodeName.toLowerCase()){(k=b.getAttribute("i

[libvirt] [PATCH 1/3] docs: Use our own implementation for fetching the RSS data

[Martin Kletzander]

This is just a small script I wrote.  It works the same way as all the libraries
together which we are bundling, but with just JS.  The only difference is that
the day of the date is formatted as 2-digit, but this should be a bug in
Firefox (at least locally for me) as the documentation states that type
'numeric' should actually not have leading zeros.

It will not be executed when the page is loaded locally.

It also uses the same proxy that query-rss uses.  We can get rid of that, but
we'd either need to have our own proxy, send a sss header to allow fetching the
atom.xml or providing JSONP access to the RSS feed on virt-planet.

Signed-off-by: Martin Kletzander 
---
 docs/index.html.in | 13 -
 docs/js/main.js| 47 ++
 2 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/docs/index.html.in b/docs/index.html.in
index f593445d061d..fa75289cad0f 100644
--- a/docs/index.html.in
+++ b/docs/index.html.in
@@ -5,19 +5,6 @@
  
  
  
-
-
-  
-
   
   
 The virtualization API
diff --git a/docs/js/main.js b/docs/js/main.js
index e57b9f47ac11..07f79c7ff903 100644
--- a/docs/js/main.js
+++ b/docs/js/main.js
@@ -29,6 +29,53 @@ function pageload() {
 
 simpleSearch = document.getElementById("simplesearch")
 simplesearch.addEventListener("submit", advancedsearch)
+
+docLoc = document.location;
+if (docLoc.protocol != "file:" ||
+docLoc.origin != "null" ||
+docLoc.host !== "" ||
+docLoc.hostname !== "") {
+fetchRSS()
+}
+}
+
+function fetchRSS() {
+cb = "jsonpRSSFeedCallback"
+window["jsonpRSSFeedCallback"] = function (data) {
+if (data.responseStatus != 200)
+return
+entries = data.responseData.feed.entries
+nEntries = Math.min(entries.length, 4)
+
+dl = document.createElement('dl')
+
+dateOpts = { day: 'numeric', month: 'short', year: 'numeric'}
+
+for (i = 0; i < nEntries; i++) {
+entry = entries[i]
+a = document.createElement('a')
+a.href = entry.link
+a.innerText = entry.title
+
+dt = document.createElement('dt')
+dt.appendChild(a)
+dl.appendChild(dt)
+
+date = new Date(entry.publishedDate)
+datestr = date.toLocaleDateString('default', dateOpts)
+
+dd = document.createElement('dd')
+dd.innerText = ` by ${entry.author} on ${datestr}`
+
+dl.appendChild(dd)
+}
+
+planet.appendChild(dl);
+};
+script = document.createElement("script")
+script.src = "https://feedrapp.herokuapp.com/";
+script.src += 
`?q=http%3A%2F%2Fplanet.virt-tools.org%2Fatom.xml&callback=${cb}`
+document.body.appendChild(script);
 }
 
 function advancedsearch(e) {
-- 
2.21.0

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

[libvirt] [PATCH 2/3] docs: use case sensitive javascript

[Martin Kletzander]

Signed-off-by: Martin Kletzander 
---
 docs/js/main.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/js/main.js b/docs/js/main.js
index 07f79c7ff903..668bf752b73d 100644
--- a/docs/js/main.js
+++ b/docs/js/main.js
@@ -28,7 +28,7 @@ function pageload() {
 advancedSearch.className = "advancedsearch"
 
 simpleSearch = document.getElementById("simplesearch")
-simplesearch.addEventListener("submit", advancedsearch)
+simpleSearch.addEventListener("submit", advancedsearch)
 
 docLoc = document.location;
 if (docLoc.protocol != "file:" ||
-- 
2.21.0

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

[libvirt] [PATCH 0/3] Ditch external JavaScript libraries

[Martin Kletzander]

This is a response to all the discussions (mainly) other people had about all
the JS code we're currently using, bundling, etc.

I would love some feedback on whether we can work on any of the solutions for
getting rid of that external proxy.  We would have to:

 - either have our own proxy,

 - send a 'Access-Control-Allow-Origin' header from the libvirt.org server to
   allow fetching the atom.xml or

 - be providing JSONP access to the RSS feed on virt-planet.

I do not have access to any of the infrastructure, so I cannot say if any of
that is possible.  It is definitely possible from the technical point of view.

Martin Kletzander (3):
  docs: Use our own implementation for fetching the RSS data
  docs: use case sensitive javascript
  docs: Remove unused JS libraries

 docs/Makefile.am|  6 +
 docs/index.html.in  | 16 
 docs/js/jquery-3.1.1.min.js |  4 ---
 docs/js/jquery.rss.min.js   | 11 -
 docs/js/main.js | 49 -
 docs/js/moment.min.js   |  7 --
 6 files changed, 49 insertions(+), 44 deletions(-)
 delete mode 100644 docs/js/jquery-3.1.1.min.js
 delete mode 100644 docs/js/jquery.rss.min.js
 delete mode 100644 docs/js/moment.min.js

-- 
2.21.0

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

Re: [libvirt] [PATCH 00/46] Finish moving header guards to #pragma once

[Ján Tomko]

The subject prefixes show up with various numbers of spaces for me.
I do not know why the list started doing that, but not including the
[libvirt] prefix in the patches I send and letting the list add it fixed
it for me.

On Tue, Jun 18, 2019 at 11:12:30AM -0500, Jonathon Jongsma wrote:

My previous series ported a bunch of internal headers to use #pragma once as 
the header guard (per
https://wiki.libvirt.org/page/BiteSizedTasks#Switch_headers_to_use_.23pragma_once).
 This series
ports the rest of the internal headers. The public headers are unchanged, and 
the syntax check
script now enforces using '#ifndef/#define/#endif' for public headers and 
'#pragma once' for
internal headers.

Jonathon Jongsma (46):
 src/locking: use #pragma once in headers
 src/lxc: use #pragma once in headers
 src/network: use #pragma once in headers
 src/nwfilter: use #pragma once in headers
 src/openvz: use #pragma once in headers
 src/phyp: use #pragma once in headers
 src/qemu: use #pragma once in headers
 src/remote: use #pragma once in headers
 src/rpc: use #pragma once in headers
 src/secret: use #pragma once in headers
 src/security: use #pragma once in headers
 src/storage: use #pragma once in headers
 src/test: use #pragma once in headers
 util: thread: use #pragma once in headers
 util: alloc: use #pragma once
 util: atomic: use #pragma once
 util: storage: use #pragma once in headers
 util: netdev: use #pragma once in headers
 util: arch.h: use #pragma once in headers
 util: file: use #pragma once in headers
 util: firewall: use #pragma once in headers
 util: host: use #pragma once in headers
 util: log: use #pragma once in headers
 util: cgroup: use #pragma once in headers
 util: error: use #pragma once in headers
 util: virprobe.h: use #pragma once in headers
 util: dbus: use #pragma once in headers
 util: command: use #pragma once in headers
 util: hash: use #pragma once in headers
 util: mac: use #pragma once in headers
 util: object: use #pragma once in headers
 util: string: use #pragma once in headers
 util: socketaddr: use #pragma once in headers
 util: sysinfo: use #pragma once in headers
 util: netlink: use #pragma once in headers
 util: xml: use #pragma once in headers
 util: scsi: use #pragma once in headers
 util: misc: use #pragma once in headers
 src/vbox: use #pragma once in headers
 src/vmware: use #pragma once in headers
 src/vmx: use #pragma once in headers
 src/vz: use #pragma once in headers
 xen: use #pragma once in headers
 tests: use #pragma once in headers
 tools: use #pragma once in headers


I have pushed the above patches.


 syntax check: update header guard check



Jano


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

Re: [libvirt] [PATCH 46/46] syntax check: update header guard check

[Ján Tomko]

On Tue, Jun 18, 2019 at 11:13:16AM -0500, Jonathon Jongsma wrote:

Internal headers should use #pragma once instead of the standard #ifndef
guard. Public headers still require the existing header guard.

Signed-off-by: Jonathon Jongsma 
---
build-aux/header-ifdef.pl | 35 ++-
1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/build-aux/header-ifdef.pl b/build-aux/header-ifdef.pl
index 401f25fb81..d63aae779b 100644
--- a/build-aux/header-ifdef.pl
+++ b/build-aux/header-ifdef.pl
@@ -6,18 +6,26 @@
#  ...copyright header...
#  */
# 
-# #ifndef SYMBOL
-# # define SYMBOL
+# #pragma once
# content
-# #endif /* SYMBOL */
#
-# For any file ending priv.h, before the #ifndef
+#---
+#
+# For any file ending priv.h, before the #pragma once
# We will have a further section
#
# #ifndef SYMBOL_ALLOW
# # error 
# #endif /* SYMBOL_ALLOW */
# 
+#
+#---
+#
+# For public headers (files in include/), use the standard header guard 
instead of #pragma once:
+# #ifndef SYMBOL
+# # define SYMBOL
+# content
+# #endif /* SYMBOL */

use strict;
use warnings;
@@ -38,6 +46,7 @@ my $file = " ";
my $ret = 0;
my $ifdef = "";
my $ifdefpriv = "";
+my $publicheader = 0;

my $state = $STATE_EOF;
my $mistake = 0;
@@ -83,6 +92,7 @@ while (<>) {
$file = $ARGV;
$state = $STATE_COPYRIGHT_COMMENT;
$mistake = 0;
+$publicheader = ($ARGV =~ /\/include\//);


The comment above says include/ but this checks for /include/, breaking
the check for me.

Also, the error messages above this code will need tuning - if I delete
everything from qemu_command.h starting with '#pragma once', I get the
wrong error:
src/qemu/qemu_command.h: missing '#ifndef LIBVIRT_QEMU_COMMAND_H' at 
./build-aux/header-ifdef.pl line 56, <> line 32534.



}

if ($mistake ||
@@ -133,12 +143,19 @@ while (<>) {
} elsif ($state == $STATE_GUARD_START) {
if (/^$/) {
&mistake("$file: too many blank lines after copyright header");
-} elsif(/#pragma once/) {
-$state = $STATE_PRAGMA;
-} elsif (/#ifndef $ifdef$/) {
-$state = $STATE_GUARD_DEFINE;
+}
+if ($publicheader) {
+if (/#ifndef $ifdef$/) {
+$state = $STATE_GUARD_DEFINE;
+} else {
+&mistake("$file: missing '#ifndef $ifdef'");
+}
} else {
-&mistake("$file: missing '#ifndef $ifdef'");
+if(/#pragma once/) {


Missing space   ^

Otherwise looks good.

Jano


+$state = $STATE_PRAGMA;
+} else {
+&mistake("$file: missing '#pragma once' header guard");
+}
}
} elsif ($state == $STATE_GUARD_DEFINE) {
if (/# define $ifdef$/) {
--
2.20.1

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


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

Re: [libvirt] [PATCH 45/46] tools: use #pragma once in headers

[Ján Tomko]

On Tue, Jun 18, 2019 at 11:13:15AM -0500, Jonathon Jongsma wrote:

Signed-off-by: Jonathon Jongsma 
---
tools/nss/libvirt_nss.h  | 27 -
tools/virsh-completer.h  |  6 +-
tools/virsh-console.h| 11 ++--
tools/virsh-domain-monitor.h |  7 +--
tools/virsh-domain.h |  7 +--
tools/virsh-host.h   |  7 +--
tools/virsh-interface.h  |  9 +--
tools/virsh-network.h|  9 +--
tools/virsh-nodedev.h|  7 +--
tools/virsh-nwfilter.h   | 11 ++--
tools/virsh-pool.h   |  9 +--
tools/virsh-secret.h |  7 +--
tools/virsh-snapshot.h   |  7 +--
tools/virsh-util.h   | 11 ++--
tools/virsh-volume.h |  9 +--
tools/virsh.h| 77 
tools/virt-admin-completer.h |  6 +-
tools/virt-admin.h   | 11 ++--
tools/virt-host-validate-bhyve.h |  5 +-
tools/virt-host-validate-common.h| 13 ++---
tools/virt-host-validate-lxc.h   |  5 +-
tools/virt-host-validate-qemu.h  |  5 +-
tools/vsh-table.h|  7 +--
tools/vsh.h  | 87 ++--
tools/wireshark/src/packet-libvirt.h |  5 +-
25 files changed, 146 insertions(+), 219 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 44/46] tests: use #pragma once in headers

[Ján Tomko]

On Tue, Jun 18, 2019 at 11:13:14AM -0500, Jonathon Jongsma wrote:

Signed-off-by: Jonathon Jongsma 
---
tests/qemumonitortestutils.h |  19 +++---
tests/qemusecuritytest.h |   7 +-
tests/testutils.h|  45 ++--
tests/testutilshostcpus.h|  11 ++-
tests/testutilslxc.h |  11 ++-
tests/testutilsqemu.h|  21 +++---
tests/testutilsqemuschema.h  |  11 ++-
tests/testutilsxen.h |  13 ++--
tests/virfilewrapper.h   |   5 +-
tests/virhashdata.h  |   5 +-
tests/virmock.h  | 129 +--
tests/virnettlshelpers.h |  17 ++---
12 files changed, 129 insertions(+), 165 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 43/46] xen: use #pragma once in headers

[Ján Tomko]

On Tue, Jun 18, 2019 at 11:13:13AM -0500, Jonathon Jongsma wrote:

Signed-off-by: Jonathon Jongsma 
---
src/xenapi/xenapi_driver.h |  5 +
src/xenapi/xenapi_driver_private.h | 17 +++--
src/xenapi/xenapi_utils.h  | 15 ++-
src/xenconfig/xen_common.h | 17 +++--
src/xenconfig/xen_sxpr.h   | 13 +
src/xenconfig/xen_xl.h | 11 ---
src/xenconfig/xen_xm.h |  9 -
src/xenconfig/xenxs_private.h  | 29 +
8 files changed, 47 insertions(+), 69 deletions(-)

diff --git a/src/xenconfig/xen_xm.h b/src/xenconfig/xen_xm.h
index b0a73fc09b..a2cc6715b4 100644
--- a/src/xenconfig/xen_xm.h
+++ b/src/xenconfig/xen_xm.h
@@ -20,12 +20,11 @@
 * .
 */

-#ifndef LIBVIRT_XEN_XM_H
-# define LIBVIRT_XEN_XM_H
+#pragma once

-# include "internal.h"
-# include "virconf.h"
-# include "domain_conf.h"
+#include "internal.h"
+#include "virconf.h"
+#include "domain_conf.h"

virConfPtr xenFormatXM(virConnectPtr conn, virDomainDefPtr def);



This also needs:
diff --git a/src/xenconfig/xen_xm.h b/src/xenconfig/xen_xm.h
index a2cc6715b4..5546b2ddf4 100644
--- a/src/xenconfig/xen_xm.h
+++ b/src/xenconfig/xen_xm.h
@@ -30,5 +30,3 @@ virConfPtr xenFormatXM(virConnectPtr conn, virDomainDefPtr 
def);

virDomainDefPtr xenParseXM(virConfPtr conf,
   virCapsPtr caps, virDomainXMLOptionPtr xmlopt);
-
-#endif /* LIBVIRT_XEN_XM_H */

With that fixed:
Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 42/46] src/vz: use #pragma once in headers

[Ján Tomko]

On Tue, Jun 18, 2019 at 11:13:12AM -0500, Jonathon Jongsma wrote:

Signed-off-by: Jonathon Jongsma 
---
src/vz/vz_driver.h |  5 +
src/vz/vz_sdk.h|  9 +++--
src/vz/vz_utils.h  | 37 +
3 files changed, 21 insertions(+), 30 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v3 0/5] docs: Update third-party JavaScript libraries

[Christophe de Dinechin]

> On 19 Jun 2019, at 14:41, Andrea Bolognani  wrote:
> 
> We're carrying around embedded copies of a few JavaScript libraries
> for use in our homepage, and we've been unforgivably bad at keeping
> them up to date. Address that.
> 
> While doing so, replace the minified version of each library with
> its non-minified counterpart so that libvirt.git and release archives
> will include the actual source code for them, rather than what is
> basically the JavaScript equivalent of binary blobs.
> 
> Note that I consider this to be merely a stop-gap measure until we
> start generating the blog roll, the only part of the libvirt website
> that uses these libraries, on the server side and finally drop them
> entirely. I don't have cycles to dedicate to that effort at the
> moment though, and this is better than leaving the situation
> unchanged for who knows how much longer.
> 
> Since the diffs are kinda big and honestly just not very interesting,
> I've snipped them liberally; the unabridged version of this series
> can be fetched with
> 
>  $ git fetch https://github.com/andreabolognani/libvirt js
> 
> 
> Changes from [v2]:
> 
>  * import non-minified libraries and minify them at build time.
> 
> Changes from [v1]:
> 
>  * drop libraries instead of updating them.
> 
> 
> [v2] https://www.redhat.com/archives/libvir-list/2019-June/msg00611.html
> [v1] https://www.redhat.com/archives/libvir-list/2019-June/msg00551.html
> 
> Andrea Bolognani (5):
>  docs: Update jQuery
>  docs: Update Moment.js
>  docs: Update jquery.rss
>  docs: Perform JavaScript minimization at build time
>  spec: Require uglifyjs for RPM build

In the spec file, why do you BuildRequire uglify-js only on Fedora?
Does that mean that if you build elsewhere, you may end up with a
build that minifies or not depending on whether uglifyjs is in path or not?

> 
> .gitignore   | 1 +
> docs/Makefile.am |37 +-
> docs/index.html.in   | 6 +-
> docs/js/jquery-3.1.1.min.js  | 4 -
> docs/js/jquery-3.4.1.js  | 10598 +
> docs/js/jquery.rss-3.3.0.js  |   333 ++
> docs/js/jquery.rss.min.js|11 -
> docs/js/moment-2.24.0.js |  4602 ++
> docs/js/moment.min.js| 7 -
> docs/page.xsl| 2 +-
> docs/site.xsl| 1 +
> docs/subsite.xsl | 1 +
> libvirt.spec.in  | 5 +
> m4/virt-external-programs.m4 | 2 +
> 14 files changed, 15578 insertions(+), 32 deletions(-)
> delete mode 100644 docs/js/jquery-3.1.1.min.js
> create mode 100644 docs/js/jquery-3.4.1.js
> create mode 100644 docs/js/jquery.rss-3.3.0.js
> delete mode 100644 docs/js/jquery.rss.min.js
> create mode 100644 docs/js/moment-2.24.0.js
> delete mode 100644 docs/js/moment.min.js
> 
> -- 
> 2.21.0
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

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

Re: [libvirt] [PATCH 41/46] src/vmx: use #pragma once in headers

[Ján Tomko]

On Tue, Jun 18, 2019 at 11:13:11AM -0500, Jonathon Jongsma wrote:

Signed-off-by: Jonathon Jongsma 
---
src/vmx/vmx.h | 21 +
1 file changed, 9 insertions(+), 12 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 40/46] src/vmware: use #pragma once in headers

[Ján Tomko]

On Tue, Jun 18, 2019 at 11:13:10AM -0500, Jonathon Jongsma wrote:

Signed-off-by: Jonathon Jongsma 
---
src/vmware/vmware_conf.h   | 19 ---
src/vmware/vmware_driver.h |  5 +
2 files changed, 9 insertions(+), 15 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 39/46] src/vbox: use #pragma once in headers

[Ján Tomko]

On Tue, Jun 18, 2019 at 11:13:09AM -0500, Jonathon Jongsma wrote:

Signed-off-by: Jonathon Jongsma 
---
src/vbox/vbox_MSCOMGlue.h |  11 +--
src/vbox/vbox_common.h| 161 +-
src/vbox/vbox_driver.h|  19 ++--
src/vbox/vbox_get_driver.h|   7 +-
src/vbox/vbox_glue.h  |  15 ++--
src/vbox/vbox_snapshot_conf.h |   9 +-
src/vbox/vbox_uniformed_api.h |  11 +--
7 files changed, 106 insertions(+), 127 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 38/46] util: misc: use #pragma once in headers

[Ján Tomko]

On Tue, Jun 18, 2019 at 11:13:08AM -0500, Jonathon Jongsma wrote:

Signed-off-by: Jonathon Jongsma 
---
src/util/virarptable.h   |  7 ++


[...]


src/util/virvsock.h  |  4 +--
src/util/virxdrdefs.h| 35 ---
64 files changed, 248 insertions(+), 434 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 36/46] util: xml: use #pragma once in headers

[Ján Tomko]

On Tue, Jun 18, 2019 at 11:13:06AM -0500, Jonathon Jongsma wrote:

Signed-off-by: Jonathon Jongsma 
---
src/util/virxml.h | 33 +++--
1 file changed, 15 insertions(+), 18 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 37/46] util: scsi: use #pragma once in headers

[Ján Tomko]

On Tue, Jun 18, 2019 at 11:13:07AM -0500, Jonathon Jongsma wrote:

Signed-off-by: Jonathon Jongsma 
---
src/util/virscsi.h  | 11 ---
src/util/virscsihost.h  |  7 ++-
src/util/virscsivhost.h | 13 +
3 files changed, 11 insertions(+), 20 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 35/46] util: netlink: use #pragma once in headers

[Ján Tomko]

On Tue, Jun 18, 2019 at 11:13:05AM -0500, Jonathon Jongsma wrote:

Signed-off-by: Jonathon Jongsma 
---
src/util/virnetlink.h | 37 +
1 file changed, 17 insertions(+), 20 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 31/46] util: object: use #pragma once in headers

[Ján Tomko]

On Tue, Jun 18, 2019 at 11:13:01AM -0500, Jonathon Jongsma wrote:

Signed-off-by: Jonathon Jongsma 
---
src/util/virobject.h | 19 ---
1 file changed, 8 insertions(+), 11 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 30/46] util: mac: use #pragma once in headers

[Ján Tomko]

On Tue, Jun 18, 2019 at 11:13:00AM -0500, Jonathon Jongsma wrote:

Signed-off-by: Jonathon Jongsma 
---
src/util/virmacaddr.h | 17 +++--
src/util/virmacmap.h  |  5 +
2 files changed, 8 insertions(+), 14 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 34/46] util: sysinfo: use #pragma once in headers

[Ján Tomko]

On Tue, Jun 18, 2019 at 11:13:04AM -0500, Jonathon Jongsma wrote:

Signed-off-by: Jonathon Jongsma 
---
src/util/virsysinfo.h | 13 +
src/util/virsysinfopriv.h |  5 +
2 files changed, 6 insertions(+), 12 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 27/46] util: dbus: use #pragma once in headers

[Ján Tomko]

On Tue, Jun 18, 2019 at 11:12:57AM -0500, Jonathon Jongsma wrote:

Signed-off-by: Jonathon Jongsma 
---
src/util/virdbus.h | 22 ++
src/util/virdbuspriv.h | 11 ---
2 files changed, 14 insertions(+), 19 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 33/46] util: socketaddr: use #pragma once in headers

[Ján Tomko]

On Tue, Jun 18, 2019 at 11:13:03AM -0500, Jonathon Jongsma wrote:

Signed-off-by: Jonathon Jongsma 
---
src/util/virsocketaddr.h | 45 +++-
1 file changed, 21 insertions(+), 24 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 32/46] util: string: use #pragma once in headers

[Ján Tomko]

On Tue, Jun 18, 2019 at 11:13:02AM -0500, Jonathon Jongsma wrote:

Signed-off-by: Jonathon Jongsma 
---
src/util/virstring.h | 37 +
1 file changed, 17 insertions(+), 20 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 28/46] util: command: use #pragma once in headers

[Ján Tomko]

On Tue, Jun 18, 2019 at 11:12:58AM -0500, Jonathon Jongsma wrote:

Signed-off-by: Jonathon Jongsma 
---
src/util/vircommand.h | 11 ---
src/util/vircommandpriv.h |  7 ++-
2 files changed, 6 insertions(+), 12 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 29/46] util: hash: use #pragma once in headers

[Ján Tomko]

On Tue, Jun 18, 2019 at 11:12:59AM -0500, Jonathon Jongsma wrote:

Signed-off-by: Jonathon Jongsma 
---
src/util/virhash.h | 8 ++--
src/util/virhashcode.h | 7 ++-
2 files changed, 4 insertions(+), 11 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 26/46] util: virprobe.h: use #pragma once in headers

[Ján Tomko]

On Tue, Jun 18, 2019 at 11:12:56AM -0500, Jonathon Jongsma wrote:

Signed-off-by: Jonathon Jongsma 
---
src/util/virprobe.h | 63 +
1 file changed, 30 insertions(+), 33 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v3 4/5] docs: Perform JavaScript minimization at build time

[Christophe de Dinechin]

> On 19 Jun 2019, at 14:41, Andrea Bolognani  wrote:
> 
> We want to store third-party JavaScript libraries in their
> non-minimized (source) form in the repository, but when users
> are browsing libvirt.org we'd rather only transmit the more
> compact minified variant.
> 
> Call uglifyjs at build time, if available, to achieve this.
> Signed-off-by: Andrea Bolognani 

Reviewed-by: Christophe de Dinechin 


> ---
> .gitignore   |  1 +
> docs/Makefile.am | 30 +++---
> docs/index.html.in   |  6 +++---
> docs/page.xsl|  2 +-
> docs/site.xsl|  1 +
> docs/subsite.xsl |  1 +
> m4/virt-external-programs.m4 |  2 ++
> 7 files changed, 36 insertions(+), 7 deletions(-)
> 
> diff --git a/.gitignore b/.gitignore
> index 727bfdb6ec..522ac762b6 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -61,6 +61,7 @@
> /docs/apibuild.py.stamp
> /docs/devhelp/libvirt.devhelp
> /docs/hvsupport.html.in
> +/docs/js/*.min.js
> /docs/libvirt-admin-*.xml
> /docs/libvirt-api.xml
> /docs/libvirt-lxc-*.xml
> diff --git a/docs/Makefile.am b/docs/Makefile.am
> index 9b79fc60c1..e787a7c47b 100644
> --- a/docs/Makefile.am
> +++ b/docs/Makefile.am
> @@ -240,6 +240,7 @@ news.html.in: \
> $(srcdir)/news.xml \
> $(srcdir)/news-html.xsl
>   $(AM_V_GEN)$(XSLTPROC) --nonet \
> + --stringparam js "$(JS)" \
>   $(srcdir)/news-html.xsl \
>   $(srcdir)/news.xml \
> >$@-tmp \
> @@ -258,7 +259,7 @@ MAINTAINERCLEANFILES += \
>   convert -rotate 90 $< $@
> 
> %.html.tmp: %.html.in site.xsl subsite.xsl page.xsl \
> - $(acl_generated)
> + $(acl_generated) $(minified_javascript)
>   $(AM_V_GEN)name=`echo $@ | sed -e 's/.tmp//'`; \
> dir=`dirname $@` ; \
> if test "$$dir" = "."; \
> @@ -270,24 +271,47 @@ MAINTAINERCLEANFILES += \
> fi; \
> $(XSLTPROC) --stringparam pagename $$name \
>   --stringparam timestamp $(timestamp) --nonet \
> + --stringparam js "$(JS)" \
>   $(top_srcdir)/docs/$$style $< > $@ \
> + || { rm $@ && exit 1; }; \
> +   sed -i 's/@JS@/$(JS)/g' "$@" \
>   || { rm $@ && exit 1; }
> 
> %.html: %.html.tmp
>   $(AM_V_GEN)$(XMLLINT) --nonet --format $< > $(srcdir)/$@ \
> || { rm $(srcdir)/$@ && exit 1; }
> 
> +
> +if HAVE_UGLIFYJS
> +JS = min.js
> +minified_javascript = $(javascript:%.js=%.min.js)
> +EXTRA_DIST += $(minified_javascript)
> +MAINTAINERCLEANFILES += $(minified_javascript)
> +
> +%.min.js: %.js
> + $(AM_V_GEN)$(UGLIFYJS) -o $@ $<
> +else ! HAVE_UGLIFYJS
> +JS = js
> +minified_javascript = $(javascript)
> +
> +dist-hook:
> + echo "uglifyjs is required to make dist." >&2

It’s silly, but the Fedora package is called uglify-js. Maybe mention that?

> + exit 1
> +endif ! HAVE_UGLIFYJS
> +
> $(apihtml_generated): html/index.html
> 
> html/index.html: libvirt-api.xml newapi.xsl page.xsl $(APIBUILD_STAMP)
>   $(AM_V_GEN)$(XSLTPROC) --nonet -o $(srcdir)/ \
> --stringparam builddir '$(abs_top_builddir)' \
> --stringparam timestamp $(timestamp) \
> +   --stringparam js "$(JS)" \
> $(srcdir)/newapi.xsl $(srcdir)/libvirt-api.xml && \
> $(XMLLINT) --nonet --noout $(srcdir)/html/*.html
> 
> $(addprefix $(srcdir)/,$(devhelphtml)): $(srcdir)/libvirt-api.xml 
> $(devhelpxsl)
>   $(AM_V_GEN)$(XSLTPROC) --stringparam timestamp $(timestamp) \
> +   --stringparam js "$(JS)" \
> --nonet -o $(srcdir)/devhelp/ \
> $(top_srcdir)/docs/devhelp/devhelp.xsl $(srcdir)/libvirt-api.xml
> 
> @@ -372,7 +396,7 @@ install-data-local:
>   for f in $(css) $(dot_html) $(gif) $(png); do \
> $(INSTALL) -m 0644 $(srcdir)/$$f $(DESTDIR)$(HTML_DIR); done
>   $(mkinstalldirs) $(DESTDIR)$(HTML_DIR)/js
> - for f in $(javascript); do \
> + for f in $(minified_javascript); do \
> $(INSTALL) -m 0644 $(srcdir)/$$f $(DESTDIR)$(HTML_DIR)/js/; done
>   $(mkinstalldirs) $(DESTDIR)$(HTML_DIR)/logos
>   for f in $(logofiles); do \
> @@ -401,7 +425,7 @@ uninstall-local:
>   for f in $(logofiles); do \
> rm -f $(DESTDIR)$(HTML_DIR)/$$f; \
>   done
> - for f in $(javascript); do \
> + for f in $(minified_javascript); do \
> rm -f $(DESTDIR)$(HTML_DIR)/$$f; \
>   done
>   for h in $(apihtml); do rm -f $(DESTDIR)$(HTML_DIR)/$$h; done
> diff --git a/docs/index.html.in b/docs/index.html.in
> index cab387ebb6..a912d7282a 100644
> --- a/docs/index.html.in
> +++ b/docs/index.html.in
> @@ -2,9 +2,9 @@
> 
> http://www.w3.org/1999/xhtml";>
>   
> - 
> - 
> - 
> + 
> + 
> + 
> 
> 
>   
-
   
   
 The virtualization API
diff --git a/docs/js/main.js b/docs/js/main.js
index e57b9f47ac11..668bf752b73d 100644
--- a/docs/js/main.js
+++ b/docs/js/main.js
@@ -28,7 +28,54 @@ function pageload() {
 advancedSearch.className = "advancedsearch"
 
 simpleSearch = document.getElementById("simplesearch")
-simplesearch.addEventListener("submit", advancedsearch)
+simpleSearch.addEventListener("submit", advancedsearch)
+
+docLoc = document.location;
+if (docLoc.protocol != "file:" ||
+docLoc.origin != "null" ||
+docLoc.host !== "" ||
+docLoc.hostname !== "") {
+fetchRSS()
+}
+}
+
+function fetchRSS() {
+cb = "jsonpRSSFeedCallback"
+window["jsonpRSSFeedCallback"] = function (data) {
+if (data.responseStatus != 200)
+return
+entries = data.responseData.feed.entries
+nEntries = Math.min(entries.length, 4)
+
+dl = document.createElement('dl')
+
+dateOpts = { day: 'numeric', month: 'short', year: 'numeric'}
+
+for (i = 0; i < nEntries; i++) {
+entry = entries[i]
+a = document.createElement('a')
+a.href = entry.link
+a.innerText = entry.title
+
+dt = document.createElement('dt')
+dt.appendChild(a)
+dl.appendChild(dt)
+
+date = new Date(entry.publishedDate)
+datestr = date.toLocaleDateString('default', dateOpts)
+
+dd = document.createElement('dd')
+dd.innerText = ` by ${entry.author} on ${datestr}`
+
+dl.appendChild(dd)
+}
+
+planet.appendChild(dl);
+};
+script = document.createElement("script")
+script.src = "https://feedrapp.herokuapp.com/";
+script.src += 
`?q=http%3A%2F%2Fplanet.virt-tools.org%2Fatom.xml&callback=${cb}`
+document.body.appendChild(script);
 }
 
 function advancedsearch(e) {
-- 
2.21.0

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

Re: [libvirt] [PATCH 3/3] test_driver: consider DHCP ranges in testDomainInterfaceAddresses

[Ilias Stamatis]

On Wed, Jun 19, 2019 at 3:48 PM Michal Privoznik  wrote:
>
> On 6/19/19 1:18 PM, Ilias Stamatis wrote:
> > testDomainInterfaceAddresses always returns the same hard-coded
> > addresses. Change the behavior such as if there is a DHCP range defined,
> > addresses are returned from that pool.
> >
> > The specific address returned depends on both the domain id and the
> > specific guest interface in an attempt to return unique addresses *most
> > of the time*.
> >
> > Additionally, return IPv6 addresses too when needed.
> >
> > Signed-off-by: Ilias Stamatis 
> > ---
> >   src/test/test_driver.c | 78 +++---
> >   1 file changed, 73 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> > index 0c2cfdd2f7..96142b549c 100644
> > --- a/src/test/test_driver.c
> > +++ b/src/test/test_driver.c
> > @@ -27,6 +27,7 @@
> >   #include 
> >   #include 
> >   #include 
> > +#include 
> >
> >
> >   #include "virerror.h"
> > @@ -3380,6 +3381,11 @@ static int testDomainBlockStats(virDomainPtr domain,
> >   return ret;
> >   }
> >
> > +
> > +static virNetworkObjPtr testNetworkObjFindByName(testDriverPtr privconn, 
> > const char *name);
> > +static virNetworkPtr testNetworkLookupByName(virConnectPtr conn, const 
> > char *name);
> > +
> > +
> >   static int
> >   testDomainInterfaceAddresses(virDomainPtr dom,
> >virDomainInterfacePtr **ifaces,
> > @@ -3388,11 +3394,18 @@ testDomainInterfaceAddresses(virDomainPtr dom,
> >   {
> >   size_t i;
> >   size_t ifaces_count = 0;
> > +size_t addr_offset;
> >   int ret = -1;
> >   char macaddr[VIR_MAC_STRING_BUFLEN];
> > +char ip_str[INET6_ADDRSTRLEN];
> > +struct sockaddr_in ipv4_addr;
> > +struct sockaddr_in6 ipv6_addr;
> >   virDomainObjPtr vm = NULL;
> >   virDomainInterfacePtr iface = NULL;
> >   virDomainInterfacePtr *ifaces_ret = NULL;
> > +virNetworkPtr net = NULL;
> > +virNetworkObjPtr net_obj = NULL;
> > +virNetworkDefPtr net_obj_def = NULL;
> >
> >   virCheckFlags(0, -1);
> >
> > @@ -3413,6 +3426,16 @@ testDomainInterfaceAddresses(virDomainPtr dom,
> >   if (vm->def->nets[i]->type != VIR_DOMAIN_NET_TYPE_NETWORK)
> >   continue;
> >
> > +virObjectUnref(net);
> > +if (!(net = testNetworkLookupByName(dom->conn,
> > +
> > vm->def->nets[i]->data.network.name)))
> > +goto cleanup;
> > +
> > +if (!(net_obj = testNetworkObjFindByName(net->conn->privateData, 
> > net->name)))
> > +goto cleanup;
>
> This is needless IMO. I mean, @net variable looks redundant to me, why
> not look up the network object directly? For instance:
>
>if (!(net_obj = testNetworkObjFindByName(dom->conn->privateData,
>
> vm->def->nets[i]->data.network.name)))
>goto cleanup;

Aah, right. I will remove this.

>
> But looking further at next hunk, I wonder if it makes sense to separate
> it into a separate function. Otherwise the code looks good. A bit
> complicated, but that was expected, since dealing with IP addresses is
> never a few lines of code :-(

I managed to do it just a tiny bit simpler by using a virSocketAddr
instead of the 2 struct sockaddr_in ones.

But which part do you want me to separate exactly?

>
> Michal

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

Re: [libvirt] [PATCH 3/3] test_driver: consider DHCP ranges in testDomainInterfaceAddresses

[Ján Tomko]

On Wed, Jun 19, 2019 at 03:47:58PM +0200, Michal Privoznik wrote:

On 6/19/19 1:18 PM, Ilias Stamatis wrote:


[...]


@@ -3413,6 +3426,16 @@ testDomainInterfaceAddresses(virDomainPtr dom,
 if (vm->def->nets[i]->type != VIR_DOMAIN_NET_TYPE_NETWORK)
 continue;

+virObjectUnref(net);
+if (!(net = testNetworkLookupByName(dom->conn,
+
vm->def->nets[i]->data.network.name)))
+goto cleanup;
+
+if (!(net_obj = testNetworkObjFindByName(net->conn->privateData, 
net->name)))
+goto cleanup;


This is needless IMO. I mean, @net variable looks redundant to me, why
not look up the network object directly? For instance:

 if (!(net_obj = testNetworkObjFindByName(dom->conn->privateData,

vm->def->nets[i]->data.network.name)))
 goto cleanup;

But looking further at next hunk, I wonder if it makes sense to 
separate it into a separate function.


Yes, please.

Jano

Otherwise the code looks good. A 
bit complicated, but that was expected, since dealing with IP 
addresses is never a few lines of code :-(


Michal

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


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

Re: [libvirt] [PATCH 1/3] test_driver: validate @source in testDomainInterfaceAddresses

[Michal Privoznik]

On 6/19/19 1:18 PM, Ilias Stamatis wrote:

Signed-off-by: Ilias Stamatis 
---
  src/test/test_driver.c | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)


ACKed and pushed.

Michal

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

Re: [libvirt] [PATCH 3/3] test_driver: consider DHCP ranges in testDomainInterfaceAddresses

[Michal Privoznik]

On 6/19/19 1:18 PM, Ilias Stamatis wrote:

testDomainInterfaceAddresses always returns the same hard-coded
addresses. Change the behavior such as if there is a DHCP range defined,
addresses are returned from that pool.

The specific address returned depends on both the domain id and the
specific guest interface in an attempt to return unique addresses *most
of the time*.

Additionally, return IPv6 addresses too when needed.

Signed-off-by: Ilias Stamatis 
---
  src/test/test_driver.c | 78 +++---
  1 file changed, 73 insertions(+), 5 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 0c2cfdd2f7..96142b549c 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -27,6 +27,7 @@
  #include 
  #include 
  #include 
+#include 


  #include "virerror.h"
@@ -3380,6 +3381,11 @@ static int testDomainBlockStats(virDomainPtr domain,
  return ret;
  }

+
+static virNetworkObjPtr testNetworkObjFindByName(testDriverPtr privconn, const 
char *name);
+static virNetworkPtr testNetworkLookupByName(virConnectPtr conn, const char 
*name);
+
+
  static int
  testDomainInterfaceAddresses(virDomainPtr dom,
   virDomainInterfacePtr **ifaces,
@@ -3388,11 +3394,18 @@ testDomainInterfaceAddresses(virDomainPtr dom,
  {
  size_t i;
  size_t ifaces_count = 0;
+size_t addr_offset;
  int ret = -1;
  char macaddr[VIR_MAC_STRING_BUFLEN];
+char ip_str[INET6_ADDRSTRLEN];
+struct sockaddr_in ipv4_addr;
+struct sockaddr_in6 ipv6_addr;
  virDomainObjPtr vm = NULL;
  virDomainInterfacePtr iface = NULL;
  virDomainInterfacePtr *ifaces_ret = NULL;
+virNetworkPtr net = NULL;
+virNetworkObjPtr net_obj = NULL;
+virNetworkDefPtr net_obj_def = NULL;

  virCheckFlags(0, -1);

@@ -3413,6 +3426,16 @@ testDomainInterfaceAddresses(virDomainPtr dom,
  if (vm->def->nets[i]->type != VIR_DOMAIN_NET_TYPE_NETWORK)
  continue;

+virObjectUnref(net);
+if (!(net = testNetworkLookupByName(dom->conn,
+
vm->def->nets[i]->data.network.name)))
+goto cleanup;
+
+if (!(net_obj = testNetworkObjFindByName(net->conn->privateData, 
net->name)))
+goto cleanup;


This is needless IMO. I mean, @net variable looks redundant to me, why
not look up the network object directly? For instance:

  if (!(net_obj = testNetworkObjFindByName(dom->conn->privateData,

vm->def->nets[i]->data.network.name)))
  goto cleanup;

But looking further at next hunk, I wonder if it makes sense to separate 
it into a separate function. Otherwise the code looks good. A bit 
complicated, but that was expected, since dealing with IP addresses is 
never a few lines of code :-(


Michal

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

Re: [libvirt] [PATCH 2/3] test_driver: return addresses only for nets of type network

[Michal Privoznik]

On 6/19/19 1:18 PM, Ilias Stamatis wrote:

Signed-off-by: Ilias Stamatis 
---
  src/test/test_driver.c | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 437df9cdf9..0c2cfdd2f7 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3409,10 +3409,10 @@ testDomainInterfaceAddresses(virDomainPtr dom,
  if (virDomainObjCheckActive(vm) < 0)
  goto cleanup;

-if (VIR_ALLOC_N(ifaces_ret, vm->def->nnets) < 0)
-goto cleanup;
-
  for (i = 0; i < vm->def->nnets; i++) {
+if (vm->def->nets[i]->type != VIR_DOMAIN_NET_TYPE_NETWORK)
+continue;
+
  if (VIR_ALLOC(iface) < 0)
  goto cleanup;

@@ -3433,7 +3433,8 @@ testDomainInterfaceAddresses(virDomainPtr dom,

  iface->naddrs = 1;

-VIR_APPEND_ELEMENT_INPLACE(ifaces_ret, ifaces_count, iface);
+if (VIR_APPEND_ELEMENT(ifaces_ret, ifaces_count, iface) < 0)
+goto cleanup;
  }

  VIR_STEAL_PTR(*ifaces, ifaces_ret);



I understand why you want this, but can't we keep the old behaviour for 
non-network interfaces?


Michal

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