[libvirt PATCH] qemu: don't reject interface update when switching to/from bridged network

[Laine Stump]

If virDomainUpdateDeviceFlags() was used to update an , and
the interface type changed from type='network' where the network was
an unmanaged bridge (so actualType == bridge) to type='bridge'
(i.e. actualType *also* == bridge), the update would fail due to the
perceived change in type.

In practice it is okay to switch between any interface types that end
up using a tap device, since libvirt just needs to attach the device
to a new bridge. But in this case we were erroneously rejecting it due
to a conditional that was too restrictive. This is what the code was doing:

  if (old->type != new->type)
 [allow update]
  else
 if ((oldActual == bridge and newActual == network)
 || (oldActual == network and newActual == bridge)) {
 [allow update]
 else
 [error]

In the case described above though, old->type and new->type don't match,
but oldActual and newActual are both 'bridge', so we get an error.

This patch changes the inner conditional so that any combination of
'network' and 'bridge' for oldActual and newActual, since they both
use a tap device connected to a bridge.

Signed-off-by: Laine Stump 
---
 src/qemu/qemu_hotplug.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index a5d57702eb..dc3bd8245f 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3770,15 +3770,15 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
  * where this can only require a minor (or even no) change,
  * but in most cases we need to do a full reconnection.
  *
- * If we switch (in either direction) between type='bridge'
- * and type='network' (for a traditional managed virtual
- * network that uses a host bridge, i.e. forward
- * mode='route|nat'), we just need to change the bridge.
+ * As long as both the new and old types use a tap device
+ * connected to a host bridge (ie VIR_DOMAIN_NET_TYPE_NETWORK
+ * or VIR_DOMAIN_NET_TYPE_BRIDGE), we just need to connect to
+ * the new bridge.
  */
-if ((oldType == VIR_DOMAIN_NET_TYPE_NETWORK &&
- newType == VIR_DOMAIN_NET_TYPE_BRIDGE) ||
-(oldType == VIR_DOMAIN_NET_TYPE_BRIDGE &&
- newType == VIR_DOMAIN_NET_TYPE_NETWORK)) {
+if ((oldType == VIR_DOMAIN_NET_TYPE_NETWORK ||
+ oldType == VIR_DOMAIN_NET_TYPE_BRIDGE) &&
+(newType == VIR_DOMAIN_NET_TYPE_NETWORK ||
+ newType == VIR_DOMAIN_NET_TYPE_BRIDGE)) {
 
 needBridgeChange = true;
 
-- 
2.25.4

Re: [PATCH 0/6] Expose QEMU's -fw_cfg

[Daniel P . Berrangé]

On Wed, Jun 03, 2020 at 07:01:32PM +0200, Michal Privoznik wrote:
> There was a discussion whether to do this or not:
> 
> https://www.redhat.com/archives/libvir-list/2020-May/msg00954.html
> 
> Before you start reviewing, naming is hard and I was unable to come up
> with a good element names and place, so suggestions are more than
> welcome!

Conceptually there's big overlap between SMBIOS and fwcfg, as they are
both data tables exposed by firmware at well defined location/format.

Currently we have use SMBIOS via 

  
...SMBIOS info...
  

It feels sensible to use that "type" attribute

  
 ...fwcfg info...
  


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

[PATCH 5/6] secdrivers: Relabel firmware config files

[Michal Privoznik]

For the case where -fw_cfg uses a file, we need to set the
seclabels on it to allow QEMU the access. While QEMU allows
writing into the file (if specified on the command line), so far
we are enabling reading only and thus we can use read only label
(in case of SELinux).

Signed-off-by: Michal Privoznik 
---
 src/security/security_dac.c | 14 ++
 src/security/security_selinux.c | 13 +
 src/security/virt-aa-helper.c   |  6 ++
 3 files changed, 33 insertions(+)

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 7b95a6f86d..a1340c242c 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -1991,6 +1991,12 @@ virSecurityDACRestoreAllLabel(virSecurityManagerPtr mgr,
 rc = -1;
 }
 
+for (i = 0; i < def->nfw_cfgs; i++) {
+if (def->fw_cfgs[i].file &&
+virSecurityDACRestoreFileLabel(mgr, def->fw_cfgs[i].file) < 0)
+rc = -1;
+}
+
 if (def->os.loader && def->os.loader->nvram &&
 virSecurityDACRestoreFileLabel(mgr, def->os.loader->nvram) < 0)
 rc = -1;
@@ -2173,6 +2179,14 @@ virSecurityDACSetAllLabel(virSecurityManagerPtr mgr,
 if (virSecurityDACGetImageIds(secdef, priv, , ))
 return -1;
 
+for (i = 0; i < def->nfw_cfgs; i++) {
+if (def->fw_cfgs[i].file &&
+virSecurityDACSetOwnership(mgr, NULL,
+   def->fw_cfgs[i].file,
+   user, group, true) < 0)
+return -1;
+}
+
 if (def->os.loader && def->os.loader->nvram &&
 virSecurityDACSetOwnership(mgr, NULL,
def->os.loader->nvram,
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 7bb7c2b7b1..c5a8e33bd7 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -2786,6 +2786,12 @@ virSecuritySELinuxRestoreAllLabel(virSecurityManagerPtr 
mgr,
  mgr) < 0)
 rc = -1;
 
+for (i = 0; i < def->nfw_cfgs; i++) {
+if (def->fw_cfgs[i].file &&
+virSecuritySELinuxRestoreFileLabel(mgr, def->fw_cfgs[i].file, 
true) < 0)
+rc = -1;
+}
+
 if (def->os.loader && def->os.loader->nvram &&
 virSecuritySELinuxRestoreFileLabel(mgr, def->os.loader->nvram, true) < 
0)
 rc = -1;
@@ -3194,6 +3200,13 @@ virSecuritySELinuxSetAllLabel(virSecurityManagerPtr mgr,
  mgr) < 0)
 return -1;
 
+for (i = 0; i < def->nfw_cfgs; i++) {
+if (def->fw_cfgs[i].file &&
+virSecuritySELinuxSetFilecon(mgr, def->fw_cfgs[i].file,
+ data->content_context, true) < 0)
+return -1;
+}
+
 /* This is different than kernel or initrd. The nvram store
  * is really a disk, qemu can read and write to it. */
 if (def->os.loader && def->os.loader->nvram &&
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 6e6dd1b1db..12beef6442 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1175,6 +1175,12 @@ get_files(vahControl * ctl)
 }
 }
 
+for (i = 0; i < ctl->def->nfw_cfgs; i++) {
+if (ctl->def->fw_cfgs[i].file &&
+vah_add_file(, ctl->def->fw_cfgs[i].file, "r") != 0)
+goto cleanup;
+}
+
 for (i = 0; i < ctl->def->nshmems; i++) {
 virDomainShmemDef *shmem = ctl->def->shmems[i];
 /* explicit server paths can be on any model to overwrites defaults.
-- 
2.26.2

[PATCH 4/6] qemu: Introduce fw_cfg capability

[Michal Privoznik]

This capability tracks whether QEMU supports -fw_cfg command line
option, more specifically whether it allows specifying filename.

There are some releases of QEMU which support -fw_cfg but not
filename. If this is ever a problem we can refine the capability
later on.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_capabilities.c   | 4 
 src/qemu/qemu_capabilities.h   | 3 +++
 tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml | 1 +
 tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml   | 1 +
 tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml   | 1 +
 tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml   | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_2.6.0.aarch64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml| 1 +
 tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml| 1 +
 tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml| 1 +
 tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml| 1 +
 tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml| 1 +
 tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml| 1 +
 tests/qemucapabilitiesdata/caps_3.0.0.riscv32.xml  | 1 +
 tests/qemucapabilitiesdata/caps_3.0.0.riscv64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml| 1 +
 tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml| 1 +
 tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml| 1 +
 tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml  | 1 +
 tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml| 1 +
 tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml| 1 +
 tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml| 1 +
 tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml| 1 +
 tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml  | 1 +
 tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml   | 1 +
 47 files changed, 52 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index f12769635a..dbf8d6d45c 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -582,6 +582,9 @@ VIR_ENUM_IMPL(virQEMUCaps,
   "tcg",
   "virtio-blk-pci.scsi.default.disabled",
   "pvscsi",
+
+  /* 370 */
+  "fw_cfg",
 );
 
 
@@ -3279,6 +3282,7 @@ static struct virQEMUCapsCommandLineProps 
virQEMUCapsCommandLine[] = {
 { "overcommit", NULL, QEMU_CAPS_OVERCOMMIT },
 { "smp-opts", "dies", QEMU_CAPS_SMP_DIES },
 { "fsdev", "multidevs", QEMU_CAPS_FSDEV_MULTIDEVS },
+{ "fw_cfg", "file", QEMU_CAPS_FW_CFG },
 };
 
 static int
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 076ecad0f7..7e5f007771 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -564,6 +564,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_VIRTIO_BLK_SCSI_DEFAULT_DISABLED, /* virtio-blk-pci.scsi 
disabled by default */
 QEMU_CAPS_SCSI_PVSCSI, /* -device pvscsi */
 
+/* 370 */
+QEMU_CAPS_FW_CFG, /* -fw_cfg command line option */
+
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
 
diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml 
b/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml
index db8a298873..0848b1e18b 100644
--- a/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml
@@ -142,6 +142,7 @@
   
   
   
+  
   201
   0
   61700287
diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml
index 15e8933300..82a37edd9c 100644
--- a/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml
@@ -142,6 +142,7 @@
   
   
   
+  
   201
   

[PATCH 1/6] domain_conf: Format NS always last

[Michal Privoznik]

I think that since  is kind of a hack, it
doesn't deserve place in the front row.

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

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1406cf079e..ff0e7e9539 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -29880,16 +29880,16 @@ virDomainDefFormatInternalSetRootName(virDomainDefPtr 
def,
 for (n = 0; n < def->nseclabels; n++)
 virSecurityLabelDefFormat(buf, def->seclabels[n], flags);
 
-if (def->namespaceData && def->ns.format) {
-if ((def->ns.format)(buf, def->namespaceData) < 0)
-goto error;
-}
-
 if (def->keywrap)
 virDomainKeyWrapDefFormat(buf, def->keywrap);
 
 virDomainSEVDefFormat(buf, def->sev);
 
+if (def->namespaceData && def->ns.format) {
+if ((def->ns.format)(buf, def->namespaceData) < 0)
+goto error;
+}
+
 virBufferAdjustIndent(buf, -2);
 virBufferAsprintf(buf, "\n", rootname);
 
-- 
2.26.2

[PATCH 3/6] qemu: Validate firmware blob configuration

[Michal Privoznik]

There are recommendations and limitations to the name of the
config blobs we need to follow [1].

Firstly, we don't want users to change any value only add new
blobs. This means, that the name must have "opt/" prefix and at
the same time must not begin with "opt/ovmf" nor "opt/org.qemu"
as these are reserved for OVMF or QEMU respectively.

Secondly, there is a limit (FW_CFG_MAX_FILE_PATH in qemu.git) of
56 characters for filename.

1: docs/specs/fw_cfg.txt from qemu.git

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

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 584d1375b8..1274159b39 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -762,6 +762,41 @@ qemuValidateDefGetVcpuHotplugGranularity(const 
virDomainDef *def)
 }
 
 
+#define QEMU_FW_CFG_MAX_FILE_PATH 55
+static int
+qemuValidateDomainDefFWCfg(const virDomainDef *def,
+   virQEMUCapsPtr qemuCaps G_GNUC_UNUSED)
+{
+size_t i;
+
+for (i = 0; i < def->nfw_cfgs; i++) {
+const virDomainFWCfgDef *f = >fw_cfgs[i];
+
+if (!STRPREFIX(f->name, "opt/")) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("Invalid firmware name"));
+return -1;
+}
+
+if (STRPREFIX(f->name, "opt/ovmf/") ||
+STRPREFIX(f->name, "opt/org.qemu/")) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("That firmware name is reserved"));
+return -1;
+}
+
+if (f->file &&
+strlen(f->file) > QEMU_FW_CFG_MAX_FILE_PATH) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("firmware file too long"));
+return -1;
+}
+}
+
+return 0;
+}
+
+
 int
 qemuValidateDomainDef(const virDomainDef *def,
   void *opaque)
@@ -978,6 +1013,9 @@ qemuValidateDomainDef(const virDomainDef *def,
 }
 }
 
+if (qemuValidateDomainDefFWCfg(def, qemuCaps) < 0)
+return -1;
+
 return 0;
 }
 
-- 
2.26.2

[PATCH 6/6] qemu: Generate command line for -fw_cfg

[Michal Privoznik]

This is pretty straightforward and self explanatory.

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

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_command.c| 38 ++
 tests/qemuxml2argvdata/fw_cfg.args | 32 +
 tests/qemuxml2argvtest.c   |  1 +
 3 files changed, 71 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/fw_cfg.args

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 419eca5675..b1e047690b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5794,6 +5794,41 @@ qemuBuildSmbiosCommandLine(virCommandPtr cmd,
 }
 
 
+static int
+qemuBuildFWCfgCommandLine(virCommandPtr cmd,
+  virQEMUCapsPtr qemuCaps,
+  const virDomainDef *def)
+{
+size_t i;
+
+if (def->nfw_cfgs == 0)
+return 0;
+
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_FW_CFG)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("fw_cfg is not supported with this QEMU"));
+return -1;
+}
+
+for (i = 0; i < def->nfw_cfgs; i++) {
+const virDomainFWCfgDef *f = >fw_cfgs[i];
+g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+
+virBufferAsprintf(, "name=%s", f->name);
+
+if (f->value)
+virBufferEscapeString(, ",string=%s", f->value);
+else
+virBufferEscapeString(, ",file=%s", f->file);
+
+virCommandAddArg(cmd, "-fw_cfg");
+virCommandAddArgBuffer(cmd, );
+}
+
+return 0;
+}
+
+
 static int
 qemuBuildVMGenIDCommandLine(virCommandPtr cmd,
 const virDomainDef *def)
@@ -9634,6 +9669,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
 if (qemuBuildSmbiosCommandLine(cmd, driver, def) < 0)
 return NULL;
 
+if (qemuBuildFWCfgCommandLine(cmd, qemuCaps, def) < 0)
+return NULL;
+
 if (qemuBuildVMGenIDCommandLine(cmd, def) < 0)
 return NULL;
 
diff --git a/tests/qemuxml2argvdata/fw_cfg.args 
b/tests/qemuxml2argvdata/fw_cfg.args
new file mode 100644
index 00..95cbe55435
--- /dev/null
+++ b/tests/qemuxml2argvdata/fw_cfg.args
@@ -0,0 +1,32 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i386 \
+-name QEMUGuest1 \
+-S \
+-machine pc,accel=tcg,usb=off,dump-guest-core=off \
+-m 214 \
+-realtime mlock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-fw_cfg 'name=opt/com.example/name,string=example value' \
+-fw_cfg name=opt/com.coreos/config,file=/tmp/provision.ign \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev 
socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
+-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 3103cac884..dc10b30fe5 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1684,6 +1684,7 @@ mymain(void)
 DO_TEST("smbios", NONE);
 DO_TEST_PARSE_ERROR("smbios-date", NONE);
 DO_TEST_PARSE_ERROR("smbios-uuid-match", NONE);
+DO_TEST("fw_cfg", QEMU_CAPS_FW_CFG);
 
 DO_TEST("watchdog", NONE);
 DO_TEST("watchdog-device", NONE);
-- 
2.26.2

[PATCH 2/6] conf: Add firmware blob configuration

[Michal Privoznik]

QEMU has -fw_cfg which allows users to tweak how firmware
configures itself and/or provide new configuration blobs.
Introduce new  element as a direct child of 
that will hold these new blobs.

It's possible to either specify new value as a string or provide
a filename which contents then serve as the value.

Signed-off-by: Michal Privoznik 
---
 docs/formatdomain.html.in   |  30 
 docs/schemas/domaincommon.rng   |  24 +++
 src/conf/domain_conf.c  | 104 
 src/conf/domain_conf.h  |  11 +++
 src/conf/virconftypes.h |   3 +
 tests/qemuxml2argvdata/fw_cfg.xml   |  40 +++
 tests/qemuxml2xmloutdata/fw_cfg.xml |   1 +
 tests/qemuxml2xmltest.c |   1 +
 8 files changed, 214 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/fw_cfg.xml
 create mode 12 tests/qemuxml2xmloutdata/fw_cfg.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 33cec1e6dd..bd67b44af8 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -595,6 +595,36 @@
   
 
 
+Firmware configuration
+
+
+  Some hypervisors provide unified way to tweak how firmware configures
+  itself, or may contain tables to be installed for the guest OS, for
+  instance boot order, ACPI, SMBIOS, etc. It even allows users to define
+  their own config blobs. In case of QEMU, these then appear under domain's
+  sysfs, under /sys/firmware/qemu_fw_cfg.
+  Since 6.5.0
+
+
+
+  firmware
+entry name="opt/com.example/name" value="example value"/
+entry name="opt/com.coreos/config" file="/tmp/provision.ign"/
+  /firmware
+
+
+
+  The firmware element can have multiple entry
+  child element. Each element then has mandatory name
+  attribute, which defines the name of the blob and must begin with
+  "opt/" and to avoid clashing with other names is advised to
+  be in form "opt/$RFQDN/$name" where $RFQDN is a
+  reverse fully qualified domain name you control.
+  Then, the element can have either value attribute (to set
+  the blob value directly), or file attribute (to set the blob
+  value from the file).
+
+
 CPU Allocation
 
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 6727cd743b..84c455d378 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -49,6 +49,9 @@
 
   
 
+
+  
+
 
 
 
@@ -5617,6 +5620,27 @@
 
   
 
+  
+
+  
+
+  
+
+  
+  
+
+  
+
+
+  
+
+  
+  
+
+  
+
+  
+
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ff0e7e9539..edbd00801a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3385,6 +3385,18 @@ virDomainSEVDefFree(virDomainSEVDefPtr def)
 }
 
 
+static void
+virDomainFWCfgDefFree(virDomainFWCfgDefPtr def)
+{
+if (!def)
+return;
+
+VIR_FREE(def->name);
+VIR_FREE(def->value);
+VIR_FREE(def->file);
+}
+
+
 void virDomainDefFree(virDomainDefPtr def)
 {
 size_t i;
@@ -3553,6 +3565,10 @@ void virDomainDefFree(virDomainDefPtr def)
 
 virSysinfoDefFree(def->sysinfo);
 
+for (i = 0; i < def->nfw_cfgs; i++)
+virDomainFWCfgDefFree(>fw_cfgs[i]);
+VIR_FREE(def->fw_cfgs);
+
 virDomainRedirFilterDefFree(def->redirfilter);
 
 for (i = 0; i < def->nshmems; i++)
@@ -20921,6 +20937,89 @@ virDomainMemorytuneDefParse(virDomainDefPtr def,
 }
 
 
+static int
+virDomainFWCfgDefParse(virDomainDefPtr def,
+   xmlXPathContextPtr ctxt)
+{
+g_autofree xmlNodePtr *nodes = NULL;
+int n;
+size_t i;
+
+if ((n = virXPathNodeSet("./firmware/entry", ctxt, )) < 0)
+return -1;
+
+if (n == 0)
+return 0;
+
+def->fw_cfgs = g_new0(virDomainFWCfgDef, n);
+
+for (i = 0; i < n; i++) {
+g_autofree char *name = NULL;
+g_autofree char *value = NULL;
+g_autofree char *file = NULL;
+
+if (!(name = virXMLPropString(nodes[i], "name"))) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("Firmware entry is missing 'name' attribute"));
+goto error;
+}
+
+value = virXMLPropString(nodes[i], "value");
+file = virXMLPropString(nodes[i], "file");
+
+if (!value && !file) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("Firmware entry must have either 'value' or "
+ "'file' attribute"));
+goto error;
+}
+
+def->fw_cfgs[i].name = g_steal_pointer();
+def->fw_cfgs[i].value = g_steal_pointer();
+def->fw_cfgs[i].file = g_steal_pointer();
+def->nfw_cfgs++;
+}
+
+return 0;

[PATCH 0/6] Expose QEMU's -fw_cfg

[Michal Privoznik]

There was a discussion whether to do this or not:

https://www.redhat.com/archives/libvir-list/2020-May/msg00954.html

Before you start reviewing, naming is hard and I was unable to come up
with a good element names and place, so suggestions are more than
welcome!

Michal Prívozník (6):
  domain_conf: Format NS always last
  conf: Add firmware blob configuration
  qemu: Validate firmware blob configuration
  qemu: Introduce fw_cfg capability
  secdrivers: Relabel firmware config files
  qemu: Generate command line for -fw_cfg

 docs/formatdomain.html.in |  30 +
 docs/schemas/domaincommon.rng |  24 
 src/conf/domain_conf.c| 114 +-
 src/conf/domain_conf.h|  11 ++
 src/conf/virconftypes.h   |   3 +
 src/qemu/qemu_capabilities.c  |   4 +
 src/qemu/qemu_capabilities.h  |   3 +
 src/qemu/qemu_command.c   |  38 ++
 src/qemu/qemu_validate.c  |  38 ++
 src/security/security_dac.c   |  14 +++
 src/security/security_selinux.c   |  13 ++
 src/security/virt-aa-helper.c |   6 +
 .../caps_2.10.0.aarch64.xml   |   1 +
 .../caps_2.10.0.ppc64.xml |   1 +
 .../caps_2.10.0.s390x.xml |   1 +
 .../caps_2.10.0.x86_64.xml|   1 +
 .../caps_2.11.0.s390x.xml |   1 +
 .../caps_2.11.0.x86_64.xml|   1 +
 .../caps_2.12.0.aarch64.xml   |   1 +
 .../caps_2.12.0.ppc64.xml |   1 +
 .../caps_2.12.0.s390x.xml |   1 +
 .../caps_2.12.0.x86_64.xml|   1 +
 .../caps_2.4.0.x86_64.xml |   1 +
 .../caps_2.5.0.x86_64.xml |   1 +
 .../caps_2.6.0.aarch64.xml|   1 +
 .../qemucapabilitiesdata/caps_2.6.0.ppc64.xml |   1 +
 .../caps_2.6.0.x86_64.xml |   1 +
 .../qemucapabilitiesdata/caps_2.7.0.s390x.xml |   1 +
 .../caps_2.7.0.x86_64.xml |   1 +
 .../qemucapabilitiesdata/caps_2.8.0.s390x.xml |   1 +
 .../caps_2.8.0.x86_64.xml |   1 +
 .../qemucapabilitiesdata/caps_2.9.0.ppc64.xml |   1 +
 .../qemucapabilitiesdata/caps_2.9.0.s390x.xml |   1 +
 .../caps_2.9.0.x86_64.xml |   1 +
 .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml |   1 +
 .../caps_3.0.0.riscv32.xml|   1 +
 .../caps_3.0.0.riscv64.xml|   1 +
 .../qemucapabilitiesdata/caps_3.0.0.s390x.xml |   1 +
 .../caps_3.0.0.x86_64.xml |   1 +
 .../qemucapabilitiesdata/caps_3.1.0.ppc64.xml |   1 +
 .../caps_3.1.0.x86_64.xml |   1 +
 .../caps_4.0.0.aarch64.xml|   1 +
 .../qemucapabilitiesdata/caps_4.0.0.ppc64.xml |   1 +
 .../caps_4.0.0.riscv32.xml|   1 +
 .../caps_4.0.0.riscv64.xml|   1 +
 .../qemucapabilitiesdata/caps_4.0.0.s390x.xml |   1 +
 .../caps_4.0.0.x86_64.xml |   1 +
 .../caps_4.1.0.x86_64.xml |   1 +
 .../caps_4.2.0.aarch64.xml|   1 +
 .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml |   1 +
 .../qemucapabilitiesdata/caps_4.2.0.s390x.xml |   1 +
 .../caps_4.2.0.x86_64.xml |   1 +
 .../caps_5.0.0.aarch64.xml|   1 +
 .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml |   1 +
 .../caps_5.0.0.riscv64.xml|   1 +
 .../caps_5.0.0.x86_64.xml |   1 +
 .../caps_5.1.0.x86_64.xml |   1 +
 tests/qemuxml2argvdata/fw_cfg.args|  32 +
 tests/qemuxml2argvdata/fw_cfg.xml |  40 ++
 tests/qemuxml2argvtest.c  |   1 +
 tests/qemuxml2xmloutdata/fw_cfg.xml   |   1 +
 tests/qemuxml2xmltest.c   |   1 +
 62 files changed, 413 insertions(+), 5 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/fw_cfg.args
 create mode 100644 tests/qemuxml2argvdata/fw_cfg.xml
 create mode 12 tests/qemuxml2xmloutdata/fw_cfg.xml

-- 
2.26.2

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

[Alex Williamson]

On Wed, 3 Jun 2020 01:24:43 -0400
Yan Zhao  wrote:

> On Tue, Jun 02, 2020 at 09:55:28PM -0600, Alex Williamson wrote:
> > On Tue, 2 Jun 2020 23:19:48 -0400
> > Yan Zhao  wrote:
> >   
> > > On Tue, Jun 02, 2020 at 04:55:27PM -0600, Alex Williamson wrote:  
> > > > On Wed, 29 Apr 2020 20:39:50 -0400
> > > > Yan Zhao  wrote:
> > > > 
> > > > > On Wed, Apr 29, 2020 at 05:48:44PM +0800, Dr. David Alan Gilbert 
> > > > > wrote:
> > > > > 
> > > > > > > > > > > > > > > > > > An mdev type is meant to define a software 
> > > > > > > > > > > > > > > > > > compatible interface, so in
> > > > > > > > > > > > > > > > > > the case of mdev->mdev migration, doesn't 
> > > > > > > > > > > > > > > > > > migrating to a different type
> > > > > > > > > > > > > > > > > > fail the most basic of compatibility tests 
> > > > > > > > > > > > > > > > > > that we expect userspace to
> > > > > > > > > > > > > > > > > > perform?  IOW, if two mdev types are 
> > > > > > > > > > > > > > > > > > migration compatible, it seems a
> > > > > > > > > > > > > > > > > > prerequisite to that is that they provide 
> > > > > > > > > > > > > > > > > > the same software interface,
> > > > > > > > > > > > > > > > > > which means they should be the same mdev 
> > > > > > > > > > > > > > > > > > type.
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > In the hybrid cases of mdev->phys or 
> > > > > > > > > > > > > > > > > > phys->mdev, how does a  
> > > > > > > > > > > > > > > > > management  
> > > > > > > > > > > > > > > > > > tool begin to even guess what might be 
> > > > > > > > > > > > > > > > > > compatible?  Are we expecting
> > > > > > > > > > > > > > > > > > libvirt to probe ever device with this 
> > > > > > > > > > > > > > > > > > attribute in the system?  Is
> > > > > > > > > > > > > > > > > > there going to be a new class hierarchy 
> > > > > > > > > > > > > > > > > > created to enumerate all
> > > > > > > > > > > > > > > > > > possible migrate-able devices?
> > > > > > > > > > > > > > > > > >  
> > > > > > > > > > > > > > > > > yes, management tool needs to guess and test 
> > > > > > > > > > > > > > > > > migration compatible
> > > > > > > > > > > > > > > > > between two devices. But I think it's not the 
> > > > > > > > > > > > > > > > > problem only for
> > > > > > > > > > > > > > > > > mdev->phys or phys->mdev. even for 
> > > > > > > > > > > > > > > > > mdev->mdev, management tool needs
> > > > > > > > > > > > > > > > > to
> > > > > > > > > > > > > > > > > first assume that the two mdevs have the same 
> > > > > > > > > > > > > > > > > type of parent devices
> > > > > > > > > > > > > > > > > (e.g.their pciids are equal). otherwise, it's 
> > > > > > > > > > > > > > > > > still enumerating
> > > > > > > > > > > > > > > > > possibilities.
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > on the other hand, for two mdevs,
> > > > > > > > > > > > > > > > > mdev1 from pdev1, its mdev_type is 1/2 of 
> > > > > > > > > > > > > > > > > pdev1;
> > > > > > > > > > > > > > > > > mdev2 from pdev2, its mdev_type is 1/4 of 
> > > > > > > > > > > > > > > > > pdev2;
> > > > > > > > > > > > > > > > > if pdev2 is exactly 2 times of pdev1, why not 
> > > > > > > > > > > > > > > > > allow migration between
> > > > > > > > > > > > > > > > > mdev1 <-> mdev2.  
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > How could the manage tool figure out that 1/2 
> > > > > > > > > > > > > > > > of pdev1 is equivalent 
> > > > > > > > > > > > > > > > to 1/4 of pdev2? If we really want to allow 
> > > > > > > > > > > > > > > > such thing happen, the best
> > > > > > > > > > > > > > > > choice is to report the same mdev type on both 
> > > > > > > > > > > > > > > > pdev1 and pdev2.  
> > > > > > > > > > > > > > > I think that's exactly the value of this 
> > > > > > > > > > > > > > > migration_version interface.
> > > > > > > > > > > > > > > the management tool can take advantage of this 
> > > > > > > > > > > > > > > interface to know if two
> > > > > > > > > > > > > > > devices are migration compatible, no matter they 
> > > > > > > > > > > > > > > are mdevs, non-mdevs,
> > > > > > > > > > > > > > > or mix.
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > as I know, (please correct me if not right), 
> > > > > > > > > > > > > > > current libvirt still
> > > > > > > > > > > > > > > requires manually generating mdev devices, and it 
> > > > > > > > > > > > > > > just duplicates src vm
> > > > > > > > > > > > > > > configuration to the target vm.
> > > > > > > > > > > > > > > for libvirt, currently it's always phys->phys and 
> > > > > > > > > > > > > > > mdev->mdev (and of the
> > > > > > > > > > > > > > > same mdev type).
> > > > > > > > > > > > > > > But it does not justify that hybrid cases should 
> > > > > > > > > > > > > > > not be allowed. otherwise,
> > > > > > > > > > > > > > > why do we need to introduce this 
> > > > > > > > > > > > > > > migration_version 

[libvirt PATCH v2 2/3] news: Convert to reStructuredText

[Andrea Bolognani]

Instead of storing release notes as XML and then converting them
to HTML and ASCII at build time using XSLT and a custom script,
we can use reStructuredText as both the source and ASCII
representation and generate HTML from it using the same tooling
we already use for the rest of the documentation.

Signed-off-by: Andrea Bolognani 
---
 Makefile.am  |   22 +-
 NEWS.rst | 3362 +++
 docs/Makefile.am |   27 +-
 docs/libvirt.css |   15 -
 docs/news-ascii.xsl  |   95 -
 docs/news-html.xsl   |  106 -
 docs/news.rng|   72 -
 docs/news.xml| 5473 --
 scripts/reformat-news.py |  102 -
 tests/virschematest.c|2 -
 10 files changed, 3374 insertions(+), 5902 deletions(-)
 create mode 100644 NEWS.rst
 delete mode 100644 docs/news-ascii.xsl
 delete mode 100644 docs/news-html.xsl
 delete mode 100644 docs/news.rng
 delete mode 100644 docs/news.xml
 delete mode 100755 scripts/reformat-news.py

diff --git a/Makefile.am b/Makefile.am
index d56deeb080..3b93170bc6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -46,6 +46,7 @@ EXTRA_DIST = \
   README.rst \
   AUTHORS.in \
   CONTRIBUTING.rst \
+  NEWS.rst \
   scripts/apibuild.py \
   scripts/augeas-gentest.py \
   build-aux/check-spacing.pl \
@@ -69,7 +70,6 @@ EXTRA_DIST = \
   scripts/minimize-po.py \
   scripts/mock-noinline.py \
   scripts/prohibit-duplicate-header.py \
-  scripts/reformat-news.py \
   scripts/test-wrap-argv.py \
   build-aux/syntax-check.mk \
   build-aux/useless-if-before-free \
@@ -83,26 +83,6 @@ EXTRA_DIST = \
 pkgconfigdir = $(libdir)/pkgconfig
 pkgconfig_DATA = libvirt.pc libvirt-qemu.pc libvirt-lxc.pc libvirt-admin.pc
 
-NEWS: \
- $(srcdir)/docs/news.xml \
- $(srcdir)/docs/news-ascii.xsl \
- $(top_srcdir)/scripts/reformat-news.py
-   $(AM_V_GEN) \
-   if [ -x $(XSLTPROC) ]; then \
- $(XSLTPROC) --nonet \
-   $(srcdir)/docs/news-ascii.xsl \
-   $(srcdir)/docs/news.xml \
- >$@-tmp \
-   || { rm -f $@-tmp; exit 1; }; \
- $(RUNUTF8) $(PYTHON) $(top_srcdir)/scripts/reformat-news.py $@-tmp 
>$@ \
-   || { rm -f $@-tmp; exit 1; }; \
- rm -f $@-tmp; \
-   fi
-EXTRA_DIST += \
-   $(srcdir)/docs/news.xml \
-   $(srcdir)/docs/news-ascii.xsl \
-   $(NULL)
-
 rpm: clean
@(unset CDPATH ; $(MAKE) dist && rpmbuild -ta $(distdir).tar.xz)
 
diff --git a/NEWS.rst b/NEWS.rst
new file mode 100644
index 00..e4d52a96e9
--- /dev/null
+++ b/NEWS.rst
@@ -0,0 +1,3362 @@
+
+libvirt releases
+
+
+This is the list of official releases for libvirt, along with an overview of
+the changes introduced by each of them.
+
+For a more fine-grained view, use the `git log`_.
+
+
+v6.5.0 (unreleased)
+===
+
+* **New features**
+
+* **Improvements**
+
+* **Bug fixes**
+
+
+v6.4.0 (2020-06-02)
+===
+
+* **New features**
+
+  * qemu: Add support for pvscsi controllers
+
+pvscsi is the VMware paravirtualized SCSI controller, which has been
+supported in QEMU for a number of years.
+
+  * cpu: Report model information for ARM CPUs
+
+``virsh capabilities`` will now include information about the host CPU when
+run on ARM machines.
+
+* **Improvements**
+
+  * qemu: stricter validation for disk type='lun'
+
+The 'lun' type is meant for SCSI command passthrough, which can't be
+achieved if qemu's block layer features are used. Disk type='lun' is now
+allowed only when the format is 'raw' and no other block layer features are
+requested.
+
+* **Bug fixes**
+
+  * qemu: fixed regression in network device hotplug with new qemu versions
+
+Starting from QEMU-5.0 it's required to conform to strict schema when
+hotplugging network devices. Libvirt didn't conform to the schema so in
+versions prior to 6.4.0 network device hotplug fails in certain cases. This
+version fixes it and adds stricter testing to prevent further issues.
+
+  * remote: Look up libxl driver correctly
+
+This makes ``xen://`` connection URIs usable in split daemon mode.
+
+  * systemd: Start libvirtd after firewalld/iptables services
+
+This solves an issue where iptables rules and chains created by libvirtd
+would get removed by a service started after it.
+
+  * network: Re-create iptables chains on firewalld restart
+
+firewalld resets all iptables rules and chains on restart, and this
+includes deleting those created by libvirt.
[...]
diff --git a/docs/Makefile.am b/docs/Makefile.am
index ba538e55a0..d8109bb65c 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -316,16 +316,18 @@ manpages/virkeyname-%.rst: 
$(top_srcdir)/src/keycodemapdb/data/keymaps.csv \
 manpagesdir = $(HTML_DIR)/manpages
 manpages_DATA = $(manpages_html)
 
-# Generate hvsupport.html and news.html first, since they take one extra step.
+# Generate hvsupport.html 

[libvirt PATCH v2 1/3] news: Output reStructuredText for the ASCII version

[Andrea Bolognani]

The ASCII output our scripts produce is already very close to
reStructuredText, and with just a few extra tweaks we can get
almost all of the way there.

Signed-off-by: Andrea Bolognani 
---
 docs/news-ascii.xsl  | 40 +---
 scripts/reformat-news.py | 20 ++--
 2 files changed, 43 insertions(+), 17 deletions(-)

diff --git a/docs/news-ascii.xsl b/docs/news-ascii.xsl
index 8dacae934c..9f6c61a3c6 100644
--- a/docs/news-ascii.xsl
+++ b/docs/news-ascii.xsl
@@ -8,36 +8,48 @@
 
   
   
-libvirt releases
+
+libvirt releases
 
+
+This is the list of official releases for libvirt, along with an overview of
+the changes introduced by each of them.
+
+For a more fine-grained view, use the `git log`_.
 
 
 
-==
+
 Older libvirt releases didn't have proper release notes: if you are interested
-in changes between them, you should check out docs/news-*.html or the full git
+in changes between them, you should check out docs/news-\*.html or the full git
 log (see instructions in ChangeLog).
+
+
+.. _git log: https://gitlab.com/libvirt/libvirt/-/commits/master
 
   
 
   
   
 
-# 
+
+
 
  (
 
 )
+===
 
+
 
   
 
   
   
 
-* 
+* **
 
-
+**
 
 
   
@@ -60,10 +72,24 @@ log (see instructions in ChangeLog).
 
   
   
+
+
 | 
-
+
 
 
   
 
+  
+  
+
+  
+
+  
+  
+ ``
+
+`` 
+  
+
 
diff --git a/scripts/reformat-news.py b/scripts/reformat-news.py
index 7bc752d821..d1c3906bd8 100755
--- a/scripts/reformat-news.py
+++ b/scripts/reformat-news.py
@@ -61,22 +61,22 @@ def reformat(line):
 # on the first character
 marker = line[0]
 
-# Release
-if marker == '#':
+# Section
+if marker == '*':
 initial_indent = 0
 indent = 2
-# Section
-elif marker == '*':
-initial_indent = 2
-indent = 4
 # Change summary
 elif marker == '-':
-initial_indent = 4
-indent = 6
+initial_indent = 2
+indent = 4
+# We use different markers to be able to tell apart the various
+# possible indentation levels, but we want to always output the
+# same marker in the generated file
+line = '*' + line[1:]
 # Change description
 elif marker == '|':
-initial_indent = 8
-indent = 8
+initial_indent = 4
+indent = 4
 # In this one case, the marker should not ultimately show
 # up in the output file, so we strip it before moving on
 line = line[1:]
-- 
2.25.4

[libvirt PATCH v2 3/3] news: Add information about old releases

[Andrea Bolognani]

Until libvirt 2.5.0 we didn't have a real process for release
notes in place, and we just published the list of commits that
had made it into each release, dividing them into categories that
mostly matched the sections we use today. Those documents haven't
been relevant for years, but they're still in the git repository
and collectively take up almost 2 MiB of disk space.

Let's import the only valuable piece of information they contain,
the release date for each libvirt versions, into the current
document and then drop them for good.

Signed-off-by: Andrea Bolognani 
---
 NEWS.rst   |  721 +++-
 docs/news-2005.html.in |   28 -
 docs/news-2006.html.in |  354 
 docs/news-2007.html.in |  534 --
 docs/news-2008.html.in |  580 ---
 docs/news-2009.html.in | 1603 -
 docs/news-2010.html.in | 2218 
 docs/news-2011.html.in | 3314 ---
 docs/news-2012.html.in | 3012 
 docs/news-2013.html.in | 3675 ---
 docs/news-2014.html.in | 3418 
 docs/news-2015.html.in | 2864 --
 docs/news-2016.html.in | 3740 
 13 files changed, 718 insertions(+), 25343 deletions(-)
 delete mode 100644 docs/news-2005.html.in
 delete mode 100644 docs/news-2006.html.in
 delete mode 100644 docs/news-2007.html.in
 delete mode 100644 docs/news-2008.html.in
 delete mode 100644 docs/news-2009.html.in
 delete mode 100644 docs/news-2010.html.in
 delete mode 100644 docs/news-2011.html.in
 delete mode 100644 docs/news-2012.html.in
 delete mode 100644 docs/news-2013.html.in
 delete mode 100644 docs/news-2014.html.in
 delete mode 100644 docs/news-2015.html.in
 delete mode 100644 docs/news-2016.html.in

diff --git a/NEWS.rst b/NEWS.rst
index e4d52a96e9..d7c4473dcb 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -3354,9 +3354,724 @@ v2.5.0 (2016-12-04)
   * Fix compilation on macOS
 
 
-Older libvirt releases didn't have proper release notes: if you are interested
-in changes between them, you should check out docs/news-\*.html or the full git
-log (see instructions in ChangeLog).
+v2.4.0 (2016-11-01)
+===
+
+No release notes.
[...]
diff --git a/docs/news-2005.html.in b/docs/news-2005.html.in
deleted file mode 100644
index 8d6dc150d3..00
--- a/docs/news-2005.html.in
+++ /dev/null
@@ -1,28 +0,0 @@
-
-
-http://www.w3.org/1999/xhtml;>
-  
-
-  
-  
-Releases (2005)
-Here is the list of official releases made during the year 2005.
-
-It is also possible to just use
-the GIT version or snapshot,
-contact the mailing list and check
-the https://libvirt.org/git/?p=libvirt.git;a=log;>GIT log
-to gauge progress.
-
-
-0.0.1: Dec 19 2005
-
-  Features:
-  First release,
-  Basic management of existing Xen domains,
-  Minimal autogenerated Python bindings
-  
-
[...]

-- 
2.25.4

[libvirt PATCH v2 0/3] news: Convert to reStructuredText

[Andrea Bolognani]

browse: https://gitlab.com/abologna/libvirt/-/tree/news-convert-cleanup
git fetch: https://gitlab.com/abologna/libvirt.git news-convert-cleanup


Changes since [v1]

  * use '*' for all bullet points, regardless of how deep they're
nested;

  * patches 1/5 and 2/5 have been pushed.


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

Andrea Bolognani (3):
  news: Output reStructuredText for the ASCII version
  news: Convert to reStructuredText
  news: Add information about old releases

 Makefile.am  |   22 +-
 NEWS.rst | 4077 
 docs/Makefile.am |   27 +-
 docs/libvirt.css |   15 -
 docs/news-2005.html.in   |   28 -
 docs/news-2006.html.in   |  354 ---
 docs/news-2007.html.in   |  534 
 docs/news-2008.html.in   |  580 
 docs/news-2009.html.in   | 1603 ---
 docs/news-2010.html.in   | 2218 ---
 docs/news-2011.html.in   | 3314 ---
 docs/news-2012.html.in   | 3012 -
 docs/news-2013.html.in   | 3675 -
 docs/news-2014.html.in   | 3418 
 docs/news-2015.html.in   | 2864 
 docs/news-2016.html.in   | 3740 --
 docs/news-ascii.xsl  |   69 -
 docs/news-html.xsl   |  106 -
 docs/news.rng|   72 -
 docs/news.xml| 5473 --
 scripts/reformat-news.py |  102 -
 tests/virschematest.c|2 -
 22 files changed, 4089 insertions(+), 31216 deletions(-)
 create mode 100644 NEWS.rst
 delete mode 100644 docs/news-2005.html.in
 delete mode 100644 docs/news-2006.html.in
 delete mode 100644 docs/news-2007.html.in
 delete mode 100644 docs/news-2008.html.in
 delete mode 100644 docs/news-2009.html.in
 delete mode 100644 docs/news-2010.html.in
 delete mode 100644 docs/news-2011.html.in
 delete mode 100644 docs/news-2012.html.in
 delete mode 100644 docs/news-2013.html.in
 delete mode 100644 docs/news-2014.html.in
 delete mode 100644 docs/news-2015.html.in
 delete mode 100644 docs/news-2016.html.in
 delete mode 100644 docs/news-ascii.xsl
 delete mode 100644 docs/news-html.xsl
 delete mode 100644 docs/news.rng
 delete mode 100644 docs/news.xml
 delete mode 100755 scripts/reformat-news.py

-- 
2.25.4

Re: [PATCH] conf: force 8 byte alignment for virObjectEvent

[Rafael Fonseca]

On Wed, Jun 3, 2020 at 12:29 PM Daniel P. Berrangé  wrote:
>
> We need to be able to cast from virObjectEventPtr to one of
> its many subclasses. Some of these subclasses have 8 byte
> alignment on 32-bit platforms, but virObjectEventPtr only
> has 4 byte alignment.
>
> Previously the virObject base class had 8 byte alignment
> but this dropped to 4 byte when converted to inherit from
> GObject. This introduces cast alignment warnings on 32-bit:
>
> ../../src/conf/domain_event.c: In function 
> 'virDomainEventDispatchDefaultFunc':
> ../../src/conf/domain_event.c:1656:30: error: cast increases required 
> alignment of target type [-Werror=cast-align]
>  1656 | rtcChangeEvent = (virDomainEventRTCChangePtr)event;
>   |  ^
> ../../src/conf/domain_event.c:1785:34: error: cast increases required 
> alignment of target type [-Werror=cast-align]
>  1785 | balloonChangeEvent = 
> (virDomainEventBalloonChangePtr)event;
>   |  ^
> ../../src/conf/domain_event.c:1896:35: error: cast increases required 
> alignment of target type [-Werror=cast-align]
>  1896 | blockThresholdEvent = 
> (virDomainEventBlockThresholdPtr)event;
>   |   ^
> ../../src/conf/domain_event.c: In function 
> 'virDomainQemuMonitorEventDispatchFunc':
> ../../src/conf/domain_event.c:1974:24: error: cast increases required 
> alignment of target type [-Werror=cast-align]
>  1974 | qemuMonitorEvent = (virDomainQemuMonitorEventPtr)event;
>   |^
> ../../src/conf/domain_event.c: In function 'virDomainQemuMonitorEventFilter':
> ../../src/conf/domain_event.c:2179:20: error: cast increases required 
> alignment of target type [-Werror=cast-align]
>  2179 | monitorEvent = (virDomainQemuMonitorEventPtr) event;
>   |^
>
> Forcing 8-byte alignment on virObjectEventPtr removes the
> alignment increase during casts to subclasses.
>
> Signed-off-by: Daniel P. Berrangé 
> ---
>
> Technically a build-breaker, but since we don't have any existing
> usage of __attribute__((aligned)), I wanted to get a second opinion
> on this approach.
>
> One alternative approach would be to switch one of the current "int"
> fields in virObjectEvent to "long long".
>
>  src/conf/object_event_private.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/conf/object_event_private.h b/src/conf/object_event_private.h
> index b31441c53a..126464a9a5 100644
> --- a/src/conf/object_event_private.h
> +++ b/src/conf/object_event_private.h
> @@ -42,7 +42,7 @@ typedef void
>virConnectObjectEventGenericCallback cb,
>void *cbopaque);
>
> -struct _virObjectEvent {
> +struct  __attribute__((aligned(4))) _virObjectEvent {
>  virObject parent;
>  int eventID;
>  virObjectMeta meta;
> --
> 2.24.1
>

This is the same error I'm seeing with my patches in CI when
converting virStorageSource to GObject except no alignment value seems
to fix it.


-- 
Rafael Fonseca

Re: [PATCH] conf: force 8 byte alignment for virObjectEvent

[Eric Blake]

On 6/3/20 5:22 AM, Daniel P. Berrangé wrote:


Forcing 8-byte alignment on virObjectEventPtr removes the
alignment increase during casts to subclasses.

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

Technically a build-breaker, but since we don't have any existing
usage of __attribute__((aligned)), I wanted to get a second opinion
on this approach.

One alternative approach would be to switch one of the current "int"
fields in virObjectEvent to "long long".




-struct _virObjectEvent {
+struct  __attribute__((aligned(4))) _virObjectEvent {
  virObject parent;
  int eventID;
  virObjectMeta meta;


As in this, although it makes the struct larger on 32-bit platforms 
(which may in turn affect cache usage):


struct _virObjectEvent {
virObject parent;
long long eventID;
...

Another possibility: use the extension of unnamed unions (but if we're 
going to rely on gcc extensions, __attribute__ is nicer than unnamed 
unions):


struct _virObjectEvent {
union {
virObject parent;
long long alignment_;
};
int eventID;
...

We already limit ourselves to gcc and clang because of 
__attribute__((cleanup)), so I don't see any problem with your approach.


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

Re: [PATCH] autogen.sh: Restore --no-git (avoid git submodule update)

[Ian Jackson]

Pavel Hrdina writes ("Re: [PATCH] autogen.sh: Restore --no-git (avoid git 
submodule update)"):
> On Tue, Jun 02, 2020 at 04:47:45PM +0100, Ian Jackson wrote:
> > -git submodule update --init || exit 1
> > +if [ "x$1" = x--no-git ]; then
> > +   shift
> > +else
> > +   git submodule update --init || exit 1
> 
> I changed the TAB into spaces.

Ah, sorry for not following the right style.

> Reviewed-by: Pavel Hrdina 
> 
> and pushed.

Thanks :-).

Ian.

Re: [PATCH] conf: force 8 byte alignment for virObjectEvent

[Pavel Hrdina]

On Wed, Jun 03, 2020 at 11:22:47AM +0100, Daniel P. Berrangé wrote:
> We need to be able to cast from virObjectEventPtr to one of
> its many subclasses. Some of these subclasses have 8 byte
> alignment on 32-bit platforms, but virObjectEventPtr only
> has 4 byte alignment.
> 
> Previously the virObject base class had 8 byte alignment
> but this dropped to 4 byte when converted to inherit from
> GObject. This introduces cast alignment warnings on 32-bit:
> 
> ../../src/conf/domain_event.c: In function 
> 'virDomainEventDispatchDefaultFunc':
> ../../src/conf/domain_event.c:1656:30: error: cast increases required 
> alignment of target type [-Werror=cast-align]
>  1656 | rtcChangeEvent = (virDomainEventRTCChangePtr)event;
>   |  ^
> ../../src/conf/domain_event.c:1785:34: error: cast increases required 
> alignment of target type [-Werror=cast-align]
>  1785 | balloonChangeEvent = 
> (virDomainEventBalloonChangePtr)event;
>   |  ^
> ../../src/conf/domain_event.c:1896:35: error: cast increases required 
> alignment of target type [-Werror=cast-align]
>  1896 | blockThresholdEvent = 
> (virDomainEventBlockThresholdPtr)event;
>   |   ^
> ../../src/conf/domain_event.c: In function 
> 'virDomainQemuMonitorEventDispatchFunc':
> ../../src/conf/domain_event.c:1974:24: error: cast increases required 
> alignment of target type [-Werror=cast-align]
>  1974 | qemuMonitorEvent = (virDomainQemuMonitorEventPtr)event;
>   |^
> ../../src/conf/domain_event.c: In function 'virDomainQemuMonitorEventFilter':
> ../../src/conf/domain_event.c:2179:20: error: cast increases required 
> alignment of target type [-Werror=cast-align]
>  2179 | monitorEvent = (virDomainQemuMonitorEventPtr) event;
>   |^
> 
> Forcing 8-byte alignment on virObjectEventPtr removes the
> alignment increase during casts to subclasses.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
> 
> Technically a build-breaker, but since we don't have any existing
> usage of __attribute__((aligned)), I wanted to get a second opinion
> on this approach.
> 
> One alternative approach would be to switch one of the current "int"
> fields in virObjectEvent to "long long".

I personally prefer using __attribute__. We already use that GCC
extension so I don't see any reason not using another part of that
extension.

> 
>  src/conf/object_event_private.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/conf/object_event_private.h b/src/conf/object_event_private.h
> index b31441c53a..126464a9a5 100644
> --- a/src/conf/object_event_private.h
> +++ b/src/conf/object_event_private.h
> @@ -42,7 +42,7 @@ typedef void
>virConnectObjectEventGenericCallback cb,
>void *cbopaque);
>  
> -struct _virObjectEvent {
> +struct  __attribute__((aligned(4))) _virObjectEvent {
>  virObject parent;
>  int eventID;
>  virObjectMeta meta;

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature

Re: [PATCH 5/5] qemu_validate.c: revert NUMA CPU range user warning

[Daniel Henrique Barboza]

On 6/2/20 5:53 AM, Igor Mammedov wrote:

On Mon, 1 Jun 2020 17:14:20 -0300
Daniel Henrique Barboza  wrote:



 
[...]



An ideal situation would be QEMU to never accept incomplete NUMA topologies
in the first place.

At least with your series I can safely drop deprecated incomplete NUMA 
topologies
on QEMU side (which were producing warnings for a while)


Keep in mind that I'm not enabling this auto-fill feature for all guests across
the board. The code is relying on the availability of QEMU_CAPS_NUMA.

Granted, there are not many 6 year old guests running Libvirt 1.2.6/QEMU 2.2.0
around, and I'm not even sure if we care about supporting this scenario, but I
feel obliged to mention this for the record :)


Thanks,


DHB

Re: [PATCH] libxl: Normalize MAC address in device conf when hotplugging a netdev

[Jim Fehlig]

On 6/2/20 9:58 PM, Laine Stump wrote:

On 5/28/20 1:09 PM, Jim Fehlig wrote:

Similar to commit 6c17606b7cc, normalize the MAC addresses in persistent
and live device config to avoid a different MAC address for the device
once the VM is rebooted and persistent config takes affect.



Well...


This has a bigger change than commit 6c17606b7cc! :-)


For those who don't feel like digging back through the git blame history, commit 
6c17606b7cc just added a call to qemuDomainAttachDeviceLiveAndConfigHomogenize() 
in the QEMU driver version of AttachDeviceFlags() to make the MAC addresses of 
the *already-existing* "live" and "config" copies of the device object match 
each other - qemu's function had previously been changed back in commit 55ce6564 
to create separate copies of the device object for config and live (although it 
still had to save an extra pointer to the config copy, which was being consumed 
by the the qemuDomainAttachDeviceConfig(), with the pointer NULLed



The libxl driver hadn't gotten that change though, so up until now it's only had 
a single device object pointer, and although it separately parsed the XML for 
live and config, this was done sequentially into the same object, so the two 
never existed at the same time. Since both objects must exist at the same time 
to copy anything from one to the other (without "extra steps"), this new patch 
is effectively the libxl equivalent of commit 55ce6564 and commit 6c17606b7cc 
put together.


That was a long prelude to "maybe mention commit 55ce6564 in the commit log as 
well" :-)


Agreed, I'll add it. Thanks for the details.


Signed-off-by: Jim Fehlig 
---
  src/libxl/libxl_driver.c | 56 +---
  1 file changed, 46 insertions(+), 10 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 63ec0a2188..a80bc3fe3a 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4096,6 +4096,31 @@ libxlDomainUpdateDeviceConfig(virDomainDefPtr vmdef, 
virDomainDeviceDefPtr dev)

  }
+static void
+libxlDomainAttachDeviceNormalize(const virDomainDeviceDef *devConf,



What? You didn't like my verb in the QEMU version? :-)


It made me think of milk :-).

(I can't remember why I picked Homogenize instead of Normalize, but I must have 
had some reason. Maybe I was just thinking back to the dairy farm next door to 
my childhood home (true story!))


Haha, that explains it! I think I would have preferred a dairy farm over the 
steel mills and refineries of my childhood :-).



At any rate, it all looks good


Reviewed-by: Laine Stump 


Thanks for the review!

Regards,
Jim

Re: [PATCH] autogen.sh: Restore --no-git (avoid git submodule update)

[Pavel Hrdina]

On Tue, Jun 02, 2020 at 04:47:45PM +0100, Ian Jackson wrote:
> Prior to 2621d48f005a "gnulib: delete all gnulib integration",
> one could pass ./autogen.sh --no-git to prevent the libvirt build
> system from running git submodule update.
> 
> This feature is needed by systems like the Xen Project CI which want
> to explicitly control the revisions of every tree.  These will
> typically arrange to initialise the submodules check out the right
> version of everything, and then expect the build system not to mess
> with it any more.
> 
> Despite to the old documentation comments referring only to gnulib,
> the --no-git feature is required not only because of gnulib but also
> because of the other submodule, src/keycodemapdb.
> 
> (And in any case, even if it were no longer required because all the
> submodules were removed, it ought ideally to have been retained as a
> no-op for compaibility reasons.)
> 
> So restore the --no-git feature.
> 
> Because of the way the argument parsing of autogen.sh works, it is
> easiest to recognise this option only if it comes first.  This works
> for the Xen Project CI, which has always passed this option first.
> 
> If something else is using this option (and hasn't introduced a
> different workaround in the meantime), not in the first position,
> then perhaps a more sophisticated approach will be needed.  But I
> think this will do for now.
> 
> Signed-off-by: Ian Jackson 
> ---
>  autogen.sh | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/autogen.sh b/autogen.sh
> index 4e1bbceb0a..1c98273452 100755
> --- a/autogen.sh
> +++ b/autogen.sh
> @@ -1,5 +1,10 @@
>  #!/bin/sh
>  # Run this to generate all the initial makefiles, etc.
> +#
> +# THe following options must come first.  All other or subsequent
> +# arguments are passed to configure:
> +#   --no-git   skip `git submodule update --init`
> +
>  test -n "$srcdir" || srcdir=$(dirname "$0")
>  test -n "$srcdir" || srcdir=.
>  
> @@ -13,7 +18,11 @@ cd "$srcdir"
>  exit 1
>  }
>  
> -git submodule update --init || exit 1
> +if [ "x$1" = x--no-git ]; then
> + shift
> +else
> + git submodule update --init || exit 1

I changed the TAB into spaces.

> +fi
>  
>  autoreconf --verbose --force --install || exit 1

Reviewed-by: Pavel Hrdina 

and pushed.


signature.asc
Description: PGP signature

Re: [libvirt PATCH 4/5] news: Convert to reStructuredText

[Andrea Bolognani]

On Wed, 2020-06-03 at 15:33 +0200, Ján Tomko wrote:
> On a Wednesday in 2020, Andrea Bolognani wrote:
> > How about I use '-' instead of '*' everywhere? That will help
> > visually tell apart the symbols used to create the bullet point from
> > those used to make text bold.
> 
> - looks nothing like a bullet point

  - it does though

  - anyway, I understand you prefer the use of

* and I'll go for that instead

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt PATCH 4/5] news: Convert to reStructuredText

[Ján Tomko]

On a Wednesday in 2020, Andrea Bolognani wrote:

On Wed, 2020-06-03 at 14:41 +0200, Ján Tomko wrote:

On a Tuesday in 2020, Andrea Bolognani wrote:
> +v6.4.0 (2020-06-02)
> +===
> +
> +* **New features**
> +
> +  - qemu: Add support for pvscsi controllers

You can use an asterisk for the nested bullet list as well, there's no
need to mix them up.


Sure, I can use the same marker for both levels.

How about I use '-' instead of '*' everywhere? That will help
visually tell apart the symbols used to create the bullet point from
those used to make text bold.


- looks nothing like a bullet point



We don't have a formal recommendation either way, and throughout the
existing .rst files we're using both. We should probably pick one and
standardize on it, but that will have to be a separate commit.


Oh no, not another standard.

Jano



--
Andrea Bolognani / Red Hat / Virtualization



signature.asc
Description: PGP signature

Re: [PATCH 2/2] Use more of VIR_XPATH_NODE_AUTORESTORE

[Ján Tomko]

On a Wednesday in 2020, Michal Privoznik wrote:

This is convenience macro, use it more. This commit was generated
using the following spatch:

 @@
 symbol node;
 identifier old;
 identifier ctxt;
 type xmlNodePtr;
 @@
 - xmlNodePtr old;
 + VIR_XPATH_NODE_AUTORESTORE(ctxt);
   ...
 - old = ctxt->node;
   ... when != old
 - ctxt->node = old;

 @@
 symbol node;
 identifier old;
 identifier ctxt;
 type xmlNodePtr;
 @@
 - xmlNodePtr old = ctxt->node;
 + VIR_XPATH_NODE_AUTORESTORE(ctxt);
   ... when != old
 - ctxt->node = old;

Signed-off-by: Michal Privoznik 
---
src/conf/cpu_conf.c |  3 +-
src/conf/domain_conf.c  |  7 +--
src/conf/interface_conf.c   | 16 ++-
src/conf/netdev_vlan_conf.c |  3 +-
src/conf/network_conf.c | 25 +++---
src/conf/networkcommon_conf.c   |  4 +-
src/conf/node_device_conf.c | 84 +
src/conf/numa_conf.c|  3 +-
src/conf/snapshot_conf.c|  3 +-
src/conf/storage_adapter_conf.c |  3 +-
src/conf/storage_conf.c |  4 +-
src/conf/virsavecookie.c|  3 +-
src/cpu/cpu_map.c   | 12 +
src/cpu/cpu_x86.c   |  3 +-
src/lxc/lxc_domain.c|  4 +-
src/util/virstorageencryption.c |  8 +---
src/util/virstoragefile.c   |  3 +-
tools/virsh-domain.c|  6 +--
18 files changed, 52 insertions(+), 142 deletions(-)




@@ -797,12 +793,12 @@ virNodeDevCapStorageParseXML(xmlXPathContextPtr ctxt,
 xmlNodePtr node,
 virNodeDevCapStoragePtr storage)
{
-xmlNodePtr orignode, *nodes = NULL;
+VIR_XPATH_NODE_AUTORESTORE(ctxt);
+xmlNodePtr *nodes = NULL;
size_t i;
int n, ret = -1;
unsigned long long val;

-orignode = ctxt->node;
ctxt->node = node;

storage->block = virXPathString("string(./block[1])", ctxt);
@@ -835,11 +831,8 @@ virNodeDevCapStorageParseXML(xmlXPathContextPtr ctxt,
if (STREQ(type, "hotpluggable")) {
storage->flags |= VIR_NODE_DEV_CAP_STORAGE_HOTPLUGGABLE;
} else if (STREQ(type, "removable")) {
-xmlNodePtr orignode2;
-
storage->flags |= VIR_NODE_DEV_CAP_STORAGE_REMOVABLE;

-orignode2 = ctxt->node;


This set orignode2 to 'node'


ctxt->node = nodes[i];

if (virXPathBoolean("count(./media_available[. = '1']) > 0", ctxt))
@@ -851,13 +844,10 @@ virNodeDevCapStorageParseXML(xmlXPathContextPtr ctxt,
if (virNodeDevCapsDefParseULongLong("number(./media_size[1])", ctxt, 
, def,
_("no removable media size supplied 
for '%s'"),
_("invalid removable media size 
supplied for '%s'")) < 0) {
-ctxt->node = orignode2;
VIR_FREE(type);
goto out;
}
storage->removable_media_size = val;
-
-ctxt->node = orignode2;


This restored it back to 'node'.
The new code can possibly leave it at node[i],
but the code below using the context is specifically run only for
non-removable devices.

My sympathy to anyone touching this code in the future :)


} else {
virReportError(VIR_ERR_INTERNAL_ERROR,
   _("unknown storage capability type '%s' for '%s'"),
@@ -881,7 +871,6 @@ virNodeDevCapStorageParseXML(xmlXPathContextPtr ctxt,
ret = 0;
 out:
VIR_FREE(nodes);
-ctxt->node = orignode;
return ret;
}



[...]


diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 61cd72f714..069e3de064 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6818,7 +6818,7 @@ virshDomainGetVcpuBitmap(vshControl *ctl,
xmlDocPtr xml = NULL;
xmlXPathContextPtr ctxt = NULL;
xmlNodePtr *nodes = NULL;
-xmlNodePtr old;
+VIR_XPATH_NODE_AUTORESTORE(ctxt);


==1644488== Invalid read of size 8
==1644488==at 0x169BE9: virshDomainGetVcpuBitmap (virsh-domain.c:6821)
==1644488==by 0x16965A: virshVcpuinfoInactive (virsh-domain.c:6898)
==1644488==by 0x160132: cmdVcpuinfo (virsh-domain.c:6970)
==1644488==by 0x19890E: vshCommandRun (vsh.c:1291)
==1644488==by 0x1362F9: main (virsh.c:905)
==1644488==  Address 0x8 is not stack'd, malloc'd or (recently) free'd

Since 'ctxt' is freed at the end of the function, there is not much
point in restoring it.


int nnodes;
size_t i;
unsigned int curvcpus = 0;
@@ -6853,8 +6853,6 @@ virshDomainGetVcpuBitmap(vshControl *ctl,
goto cleanup;
}

-old = ctxt->node;
-
for (i = 0; i < nnodes; i++) {
ctxt->node = nodes[i];

@@ -6868,8 +6866,6 @@ virshDomainGetVcpuBitmap(vshControl *ctl,
VIR_FREE(online);
}

-ctxt->node = old;
-
if (virBitmapCountBits(ret) != curvcpus) {
vshError(ctl, "%s", _("Failed to retrieve vcpu state bitmap"));
virBitmapFree(ret);


With the crash fixed:

Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP 

Re: [PATCH] autogen.sh: Restore --no-git (avoid git submodule update)

[Ian Jackson]

Pavel Hrdina writes ("Re: [PATCH] autogen.sh: Restore --no-git (avoid git 
submodule update)"):
> To be honest I don't understand why would anyone want to keep track of
> all submodules of all projects for any CI and update it manually every
> time the upstream project changes these submodules. Sounds like a lot
> of unnecessary work but maybe I'm missing something.

Maybe I should answer this.  The short answer is that this can be done
entirely automatically.

> Well, we will break a lot more by switching to Meson build system where
> everyone building libvirt will have to change their scripts as it will
> not be compatible at all.

When that occurs we'll have to change our build rune, of course.

Our CI system (which does bisection and stable tracking and so on)
will wants to build old versions, so it'll have to have both build
runes and look for something in-tree to tell the two methods apart.

Thanks,
Ian.

Re: [libvirt PATCH 5/5] news: Add information about old releases

[Andrea Bolognani]

On Wed, 2020-06-03 at 14:44 +0200, Ján Tomko wrote:
> On a Tuesday in 2020, Andrea Bolognani wrote:
> > -Older libvirt releases didn't have proper release notes: if you are 
> > interested
> > -in changes between them, you should check out docs/news-\*.html or the 
> > full git
> > -log (see instructions in ChangeLog).
> > +v2.4.0 (2016-11-01)
> > +===
> > +
> > +No release notes.
> > +
> > [...]
> > +
> > +v0.0.1 (2005-12-19)
> > +===
> > +
> > +No release notes.
> 
> This looks excessive - I'd rather drop them for good. We still have the
> info in the tagged relase commits.

I think it's valuable to retain that information and have it all
stored in one place, and considering how many lines we're removing
with this patch we come out massively on top either way.

That said, this is not a hill I'm particularly interested in dying
on, so if you insist on dropping the information altogether I will
begrudgingly comply :)

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt PATCH 4/5] news: Convert to reStructuredText

[Andrea Bolognani]

On Wed, 2020-06-03 at 14:41 +0200, Ján Tomko wrote:
> On a Tuesday in 2020, Andrea Bolognani wrote:
> > +v6.4.0 (2020-06-02)
> > +===
> > +
> > +* **New features**
> > +
> > +  - qemu: Add support for pvscsi controllers
> 
> You can use an asterisk for the nested bullet list as well, there's no
> need to mix them up.

Sure, I can use the same marker for both levels.

How about I use '-' instead of '*' everywhere? That will help
visually tell apart the symbols used to create the bullet point from
those used to make text bold.

We don't have a formal recommendation either way, and throughout the
existing .rst files we're using both. We should probably pick one and
standardize on it, but that will have to be a separate commit.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH] autogen.sh: Restore --no-git (avoid git submodule update)

[Ian Jackson]

Pavel Hrdina writes ("Re: [PATCH] autogen.sh: Restore --no-git (avoid git 
submodule update)"):
> There should not be any need to disable this explicitly unless you want
> to build libvirt with different revisions of submodules.

The Xen Project CI has a cross-tree bisector.  It is capable of
automatically selecting revisions, including revisions of submodules,
for testing.

It therefore needs to be able to build with different revisions of
submodules, indeed.

Thanks,
Ian.

Re: [PATCH 1/2] virxml: Don't overwrite ctxt->node

[Ján Tomko]

On a Wednesday in 2020, Michal Privoznik wrote:

This reverts b897973f2e0.


Please disconnect the period from the commit ID.
(Popular choices include:
  * putting a space in there
  * removing it completely
  * wrapping the commit ID in angle brackets
)



Even though it may have been the case in the past, relative
XPaths don't overwrite the ctxt->node. Thus, there's no need to
save it.

Signed-off-by: Michal Privoznik 
---
src/util/virxml.c | 27 ---
1 file changed, 27 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

Re: [PATCH libvirt v1 6/6] tests: add test with PCI and CCW device

[Andrea Bolognani]

On Thu, 2020-04-09 at 12:31 +0200, Shalini Chellathurai Saroja wrote:
> +++ b/tests/qemuxml2argvtest.c
> @@ -1676,6 +1676,10 @@ mymain(void)
>  DO_TEST_PARSE_ERROR("hostdev-vfio-zpci-set-zero",
>  QEMU_CAPS_DEVICE_VFIO_PCI,
>  QEMU_CAPS_DEVICE_ZPCI);
> +DO_TEST("hostdev-vfio-zpci-ccw-memballoon",
> +QEMU_CAPS_CCW,
> +QEMU_CAPS_DEVICE_VFIO_PCI,
> +QEMU_CAPS_DEVICE_ZPCI);

Same comment as the previous patch - you want to add this to
qemuxml2xmltest.c as well.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH libvirt v1 5/6] tests: qemu: add more tests for ZPCI on S390

[Andrea Bolognani]

On Thu, 2020-04-09 at 12:31 +0200, Shalini Chellathurai Saroja wrote:
> +++ b/tests/qemuxml2argvtest.c
> @@ -1650,6 +1655,12 @@ mymain(void)
>  DO_TEST("hostdev-vfio-zpci-autogenerate",
>  QEMU_CAPS_DEVICE_VFIO_PCI,
>  QEMU_CAPS_DEVICE_ZPCI);
> +DO_TEST("hostdev-vfio-zpci-autogenerate-uids",
> +QEMU_CAPS_DEVICE_VFIO_PCI,
> +QEMU_CAPS_DEVICE_ZPCI);
> +DO_TEST("hostdev-vfio-zpci-autogenerate-fids",
> +QEMU_CAPS_DEVICE_VFIO_PCI,
> +QEMU_CAPS_DEVICE_ZPCI);

You want to add the successfult tests to qemuxml2xmltest.c as well.

Everything else looks good.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt PATCH 5/5] news: Add information about old releases

[Ján Tomko]

On a Tuesday in 2020, Andrea Bolognani wrote:

Until libvirt 2.5.0 we didn't have a real process for release
notes in place, and we just published the list of commits that
had made it into each release, dividing them into categories that
mostly matched the sections we use today. Those documents haven't
been relevant for years, but they're still in the git repository
and collectively take up almost 2 MiB of disk space.

Let's import the only valuable piece of information they contain,
the release date for each libvirt versions, into the current
document and then drop them for good.

Signed-off-by: Andrea Bolognani 
---
NEWS.rst   |  721 +++-
docs/news-2005.html.in |   28 -
docs/news-2006.html.in |  354 
docs/news-2007.html.in |  534 --
docs/news-2008.html.in |  580 ---
docs/news-2009.html.in | 1603 -
docs/news-2010.html.in | 2218 
docs/news-2011.html.in | 3314 ---
docs/news-2012.html.in | 3012 
docs/news-2013.html.in | 3675 ---
docs/news-2014.html.in | 3418 
docs/news-2015.html.in | 2864 --
docs/news-2016.html.in | 3740 
13 files changed, 718 insertions(+), 25343 deletions(-)
delete mode 100644 docs/news-2005.html.in
delete mode 100644 docs/news-2006.html.in
delete mode 100644 docs/news-2007.html.in
delete mode 100644 docs/news-2008.html.in
delete mode 100644 docs/news-2009.html.in
delete mode 100644 docs/news-2010.html.in
delete mode 100644 docs/news-2011.html.in
delete mode 100644 docs/news-2012.html.in
delete mode 100644 docs/news-2013.html.in
delete mode 100644 docs/news-2014.html.in
delete mode 100644 docs/news-2015.html.in
delete mode 100644 docs/news-2016.html.in

diff --git a/NEWS.rst b/NEWS.rst
index 128b899b88..82b93d5a44 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -3354,9 +3354,724 @@ v2.5.0 (2016-12-04)
  - Fix compilation on macOS


-Older libvirt releases didn't have proper release notes: if you are interested
-in changes between them, you should check out docs/news-\*.html or the full git
-log (see instructions in ChangeLog).
+v2.4.0 (2016-11-01)
+===
+
+No release notes.
+
[...]
+
+v0.0.1 (2005-12-19)
+===
+
+No release notes.



This looks excessive - I'd rather drop them for good. We still have the
info in the tagged relase commits.

With the docs/news.*html reference dropped above:

Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

Re: [libvirt PATCH 4/5] news: Convert to reStructuredText

[Ján Tomko]

On a Tuesday in 2020, Andrea Bolognani wrote:

Instead of storing release notes as XML and then converting them
to HTML and ASCII at build time using XSLT and a custom script,
we can use reStructuredText as both the source and ASCII
representation and generate HTML from it using the same tooling
we already use for the rest of the documentation.

Signed-off-by: Andrea Bolognani 
---
Makefile.am  |   22 +-
NEWS.rst | 3362 +++
docs/Makefile.am |   27 +-
docs/libvirt.css |   15 -
docs/news-ascii.xsl  |   95 -
docs/news-html.xsl   |  106 -
docs/news.rng|   72 -
docs/news.xml| 5473 --
scripts/reformat-news.py |  102 -
tests/virschematest.c|2 -
10 files changed, 3374 insertions(+), 5902 deletions(-)
create mode 100644 NEWS.rst
delete mode 100644 docs/news-ascii.xsl
delete mode 100644 docs/news-html.xsl
delete mode 100644 docs/news.rng
delete mode 100644 docs/news.xml
delete mode 100755 scripts/reformat-news.py

diff --git a/Makefile.am b/Makefile.am
index d56deeb080..3b93170bc6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -46,6 +46,7 @@ EXTRA_DIST = \
  README.rst \
  AUTHORS.in \
  CONTRIBUTING.rst \
+  NEWS.rst \
  scripts/apibuild.py \
  scripts/augeas-gentest.py \
  build-aux/check-spacing.pl \
@@ -69,7 +70,6 @@ EXTRA_DIST = \
  scripts/minimize-po.py \
  scripts/mock-noinline.py \
  scripts/prohibit-duplicate-header.py \
-  scripts/reformat-news.py \
  scripts/test-wrap-argv.py \
  build-aux/syntax-check.mk \
  build-aux/useless-if-before-free \
@@ -83,26 +83,6 @@ EXTRA_DIST = \
pkgconfigdir = $(libdir)/pkgconfig
pkgconfig_DATA = libvirt.pc libvirt-qemu.pc libvirt-lxc.pc libvirt-admin.pc

-NEWS: \
- $(srcdir)/docs/news.xml \
- $(srcdir)/docs/news-ascii.xsl \
- $(top_srcdir)/scripts/reformat-news.py
-   $(AM_V_GEN) \
-   if [ -x $(XSLTPROC) ]; then \
- $(XSLTPROC) --nonet \
-   $(srcdir)/docs/news-ascii.xsl \
-   $(srcdir)/docs/news.xml \
- >$@-tmp \
-   || { rm -f $@-tmp; exit 1; }; \
- $(RUNUTF8) $(PYTHON) $(top_srcdir)/scripts/reformat-news.py $@-tmp 
>$@ \
-   || { rm -f $@-tmp; exit 1; }; \
- rm -f $@-tmp; \
-   fi
-EXTRA_DIST += \
-   $(srcdir)/docs/news.xml \
-   $(srcdir)/docs/news-ascii.xsl \
-   $(NULL)
-
rpm: clean
@(unset CDPATH ; $(MAKE) dist && rpmbuild -ta $(distdir).tar.xz)

diff --git a/NEWS.rst b/NEWS.rst
new file mode 100644
index 00..128b899b88
--- /dev/null
+++ b/NEWS.rst
@@ -0,0 +1,3362 @@
+
+libvirt releases
+
+
+This is the list of official releases for libvirt, along with an overview of
+the changes introduced by each of them.
+
+For a more fine-grained view, use the `git log`_.
+
+
+v6.5.0 (unreleased)
+===
+
+* **New features**
+
+* **Improvements**
+
+* **Bug fixes**
+
+
+v6.4.0 (2020-06-02)
+===
+
+* **New features**
+
+  - qemu: Add support for pvscsi controllers


You can use an asterisk for the nested bullet list as well, there's no
need to mix them up.

Jano


signature.asc
Description: PGP signature

Re: [libvirt PATCH 1/5] docs: Fix dot_rst_html_in definition

[Ján Tomko]

On a Tuesday in 2020, Andrea Bolognani wrote:

As the name clearly implies, it's supposed to list the .html.in
files that are generated from .rst files, but it mistakenly lists
the corresponding .html files instead.

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



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

Re: [libvirt PATCH 2/5] news: Point to GitLab for full git log

[Ján Tomko]

On a Tuesday in 2020, Andrea Bolognani wrote:

The primary git repository is the one on GitLab these days.

Signed-off-by: Andrea Bolognani 
---
docs/news-html.xsl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

Re: [libvirt PATCH 0/7] po: updates to switch over to use of Weblate

[Pavel Hrdina]

On Tue, May 19, 2020 at 10:55:02AM +0100, Daniel P. Berrangé wrote:
> The Fedora translation team has stopped using Zanata because the
> software itself is dead upstream. In its place is the Weblate
> platform. In theory we should have been able to work with Weblate in the
> same way as we did for Zanata, pushing a pot file periodically, and
> pulling .po files periodically. In practice this fails for libvirt.git
> because Weblates RPC API doesn't scale sufficiently well. It will
> frequently throw errors with the large libvirt.pot file and it gets
> slower at an exponential rate as you add more languages.
> 
> Weblate has another mode of operating though which is way more common,
> whereby it directly pulls a .pot from your git repo, and then directly
> pushes .po files back, either using a trusted SSH key, or by opening a
> merge request for GitLab/GitHub/etc.  This is the mode we're going to
> have to use in libvirt projects.
> 
> Compared to what we're currently doing with Zanata the downsides are:
> 
>  - We have to store libvirt.pot in git and refresh it periodically
> 
>  - The .po files are only partially minimized, as while they have
>locations stripped, they still contain non-translated msgids
> 
> The plussides are
> 
>  - We don't have to interact with Weblate at all, only the libvirt
>git repo
> 
>  - We'll be able to use the normal meson i18n integration, merely
>by calling
> 
>  i18n.gettext(meson.project_name(),
>   args: ['--sort-output'],
>   preset: 'glib')
> 
> I'm intending to open discussion with weblate maintainers to see if
> either of those two downsides can be eliminated via feature enhancements
> to Weblate. In the meanwhile we just have to accept them, as otherwise
> we're not going to get any translations since Zanata is dead.
> 
> Daniel P. Berrangé (7):
>   po: switch to using LINGUAS file for list of languages
>   po: delete empty translations
>   po: refresh to drop unused translations
>   po: rename the .mini.po files to have just a .po suffix
>   po: generate .pot file with strings in alphabetical order
>   po: stop stripping non-translated strings from po files
>   po: go back to storing the .pot file in git

Today I was investigating the i18n Meson module and recalled this
series.

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature

Re: [PATCH libvirt v1 4/6] qemu: move ZPCI uid validation into device validation

[Andrea Bolognani]

On Thu, 2020-04-09 at 12:31 +0200, Shalini Chellathurai Saroja wrote:
> +static bool
> +qemuDomainDeviceDefValidateZPCIUid(virZPCIDeviceAddressPtr zpci)
> +{
> +if (zpci->uid_set &&
> +(zpci->uid > VIR_DOMAIN_DEVICE_ZPCI_MAX_UID ||
> + zpci->uid == 0)) {
> +virReportError(VIR_ERR_XML_ERROR,
> +   _("Invalid PCI address uid='0x%.4x', "
> + "must be > 0x and <= 0x%.4x"),
> +   zpci->uid,
> +   VIR_DOMAIN_DEVICE_ZPCI_MAX_UID);
> +return false;
> +}
> +
> +return true;
> +}
> +
> +
>  static int
>  qemuValidateDomainDeviceDefZPCIAddress(virDomainDeviceInfoPtr info,
> virQEMUCapsPtr qemuCaps)
> @@ -960,6 +978,12 @@ 
> qemuValidateDomainDeviceDefZPCIAddress(virDomainDeviceInfoPtr info,
>  return -1;
>  }
>  
> +/* We don't need to check fid because fid covers
> + * all range of uint32 type.
> + */
> +if (!qemuDomainDeviceDefValidateZPCIUid(>addr.pci.zpci))
> +return -1;

No need to create a separate function, just perform the check inline
here.

-- 
Andrea Bolognani / Red Hat / Virtualization

[libvirt PATCH] tools: libvirt-guests: correctly check shutdown value

[Ján Tomko]

The variable cleanup introduced a typo.

Signed-off-by: Ján Tomko 
Fixes: 08071ec0f113bb1fe8dcc263cb6bf87529e8b76b
Reported-by: Bronek Kozicki
Closes: https://gitlab.com/libvirt/libvirt/-/issues/27
---
Pushed as trivial.

 tools/libvirt-guests.sh.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libvirt-guests.sh.in b/tools/libvirt-guests.sh.in
index 7af24dab3b..534c4d5e0f 100644
--- a/tools/libvirt-guests.sh.in
+++ b/tools/libvirt-guests.sh.in
@@ -444,7 +444,7 @@ stop() {
 # last stop was not followed by start
 [ -f "$LISTFILE" ] && return 0
 
-if [ "/x$ON_SHUTDOWN" = xshutdown ]; then
+if [ "x$ON_SHUTDOWN" = xshutdown ]; then
 suspending=false
 if [ $SHUTDOWN_TIMEOUT -lt 0 ]; then
 gettext "SHUTDOWN_TIMEOUT must be equal or greater than 0"
-- 
2.25.4

Re: [PATCH 00/40] convert virObjects to GObject

[Rafael Fonseca]

On Wed, Jun 3, 2020 at 11:38 AM Daniel P. Berrangé  wrote:
>
> On Fri, May 15, 2020 at 04:13:21PM +0100, Daniel P. Berrangé wrote:
> > On Fri, May 15, 2020 at 02:34:01PM +0200, Michal Privoznik wrote:
> > > On 5/13/20 1:56 PM, Rafael Fonseca wrote:
> > > > This patch series convert various simple instances of virObject to a
> > > > GObject equivalent.
> > >
> > > I'm sorry upfront for misusing your patchset and I'm also sorry for 
> > > bringing
> > > this up again.
> > >
> > > I think we need to step back and re-evaluate whether this is worth it.
> > > GObject is horrible and frightening part of GLib. Not only one has to 
> > > define
> > > empty functions, but we can't mix virObject and GObject. For instance:
> > > virObjectRef() called over GObject will corrupt memory.
> > >
> > > Worse, there is no way to check whether your patches converted everything
> > > (and clearly they did not because I see couple of tests crashing; or maybe
> > > they did at the time of send but now the code has changed - anyway, point
> > > proven).
> > >
> > > I started reviewing and stopped at the first patch realizing, I have no 
> > > idea
> > > whether you converted every virObjectRef()/virObjectUnref() with
> > > corresponding glib call.
> > >
> > > I also wanted to write a cocci spatch that might at least identify places
> > > where that is happening, but apparently my spatch skills are poor.
> > >
> > > Does anybody have an idea how to verify these patches?
> >
> > My preferred option was to make virObject be a subclass of GObject.
> >
> > I tried this initially but hit a key problem - g_object_unref is void
> > and virObjectUnref returns bool - true if any refs still exist. We
> > rely on that for the virConnectPtr and virFDStream objects.
> >
> > I couldn't come up with a way to solve that at the time, but I've
> > just had another think and believe we can solve it using a thread
> > local set by the finalize. So I'll have another go at doing this
> > inheritance.
>
> My conversion  to make virObject a subclass of GObject has now merged,
> which eliminates the danger that Michael was concerned about.
>
> So we should be able to move forward with Rafael's patches now.

Cool. I'll rebase on master, run the CI and send a new version when
it's all green.


Att.
-- 
Rafael Fonseca

Re: [PATCH] autogen.sh: Restore --no-git (avoid git submodule update)

[Pavel Hrdina]

On Wed, Jun 03, 2020 at 11:37:08AM +0100, Daniel P. Berrangé wrote:
> On Wed, Jun 03, 2020 at 12:31:09PM +0200, Pavel Hrdina wrote:
> > On Tue, Jun 02, 2020 at 04:47:45PM +0100, Ian Jackson wrote:
> > > Prior to 2621d48f005a "gnulib: delete all gnulib integration",
> > > one could pass ./autogen.sh --no-git to prevent the libvirt build
> > > system from running git submodule update.
> > > 
> > > This feature is needed by systems like the Xen Project CI which want
> > > to explicitly control the revisions of every tree.  These will
> > > typically arrange to initialise the submodules check out the right
> > > version of everything, and then expect the build system not to mess
> > > with it any more.
> > 
> > To be honest I don't understand why would anyone want to keep track of
> > all submodules of all projects for any CI and update it manually every
> > time the upstream project changes these submodules. Sounds like a lot
> > of unnecessary work but maybe I'm missing something.
> 
> Two possible scenarios that I think are valid
> 
>  - The CI job script does not have network access
>  - The CI job script sees the source dir as read-only
> 
> In both cases the CI harness would have to initialize the submodule
> before runing the CI job.
> 
> I don't know if this is what Xen CI is hitting, but I think the
> request is reasonable none the less.
> 
> Both Jenkins and GitLab CI have option to make the harness init
> submodules prior to the job running.

My point was that running 'git submodule update --init' will not change
anything if all submodules are updated to the correct revision and if
the repository was pre-created with submodules and is read-only when the
test is executed the git command will not fail as well and everything
will be fine.

If the submodules are not updated to the correct revision then it will
fail and notify the CI users that something is broken.

There should not be any need to disable this explicitly unless you want
to build libvirt with different revisions of submodules.

> > > Despite to the old documentation comments referring only to gnulib,
> > > the --no-git feature is required not only because of gnulib but also
> > > because of the other submodule, src/keycodemapdb.
> > > 
> > > (And in any case, even if it were no longer required because all the
> > > submodules were removed, it ought ideally to have been retained as a
> > > no-op for compaibility reasons.)
> > 
> > Well, we will break a lot more by switching to Meson build system where
> > everyone building libvirt will have to change their scripts as it will
> > not be compatible at all.
> 
> Yes, but I think that's tangential, as the two above reasons will
> still apply, and Meson will cope with having submodules pre-initialized.

Yes, these are unrelated and I just wanted to point out that
compaibility reasons are in most cases not valid to changes to build
system or moving files around in git repository and so on.

Pavel


signature.asc
Description: PGP signature

Re: [PATCH] virsysinfo: Parse OEM strings

[Michal Privoznik]

On 6/3/20 1:22 PM, Daniel P. Berrangé wrote:

With the normal format, we can reliably extract each string, but some
chars might have been mangled. How about we keep your current code, but
if we see a "." in a string, then we run "dmidecode -u --oem-string N"
just for that particular string, in order to get the unmangled value.

That will mean we only run dmidecode once majority of cases.


Makes sense. Will post v2.

Michal

Re: [PATCH] virsysinfo: Parse OEM strings

[Daniel P . Berrangé]

On Tue, Jun 02, 2020 at 03:11:25PM +0200, Michal Privoznik wrote:
> On 6/2/20 12:56 PM, Daniel P. Berrangé wrote:
> > On Tue, Jun 02, 2020 at 12:53:18PM +0200, Michal Privoznik wrote:
> > > Setting OEM strings for a domain was introduced in
> > > v4.1.0-rc1~315. However, any application that wanted to use them
> > > (e.g. to point to an URL where a config file is stored) had to
> > > 'dmidecode -u --oem-string N' (where N is index of the string).
> > > Well, we can expose them under our  XML and if the
> > > domain is running Libvirt inside it can be obtained using
> > > virConnectGetSysinfo() API.
> > > 
> > > Signed-off-by: Michal Privoznik 
> > > ---
> > >   src/util/virsysinfo.c   | 60 -
> > >   tests/sysinfodata/x86sysinfo.data   | 10 +
> > >   tests/sysinfodata/x86sysinfo.expect | 10 +
> > >   3 files changed, 79 insertions(+), 1 deletion(-)
> > 
> > > diff --git a/tests/sysinfodata/x86sysinfo.data 
> > > b/tests/sysinfodata/x86sysinfo.data
> > > index 426261041d..5615e144fb 100644
> > > --- a/tests/sysinfodata/x86sysinfo.data
> > > +++ b/tests/sysinfodata/x86sysinfo.data
> > > @@ -81,3 +81,13 @@ Memory Device
> > >   Serial Number: 29057112
> > >   Asset Tag: 0839
> > >   Part Number: IMSH2GS13A1F1C-10F
> > > +
> > > +OEM Strings
> > > +String 1: Default string
> > > +String 2: Default string
> > > +String 3: MIAMI
> > > +String 4: Default string
> > > +String 5: F
> > > +String 6: F
> > > +String 7: F
> > > +String 8: Default string
> > 
> > What does dmidecode do for escaping (if anything) if I set a string
> > value of
> > 
> >  "Ha ha ha try parsing\nString 3: this correctly\n
> > String 4:then"
> 
> This is what I put into domain XML:
> 
>   
> 
>   Hello
>   World
>   Ha ha ha try parsing\n
>   String 3: this correctly\n
>   String 4:then
>   This is, more tricky value=escaped
> 
>   
> 
> This is how it appeared on the cmd line:
> 
> -smbios 'type=11,value=Hello,value=World,value=Ha ha ha try parsing\n
>   String 3: this correctly\n
>   String 4:then,value=This is,, more tricky value=escaped' \
> 
> 
> Now, inside the guest, plain dmidecode replaces \n with a dot:
> 
> # dmidecode -t 11
> Handle 0x0E00, DMI type 11, 5 bytes
> OEM Strings
> String 1: Hello
> String 2: World
> String 3: Ha ha ha try parsing\n.  String 3: this correctly\n.
> String 4:then
> String 4: This is, more tricky value=escaped
> 
> 
> 
> But if I tell it to not decode entries, then it displays the string
> correctly:
> 
> # dmidecode -u --oem-string 3
> Ha ha ha try parsing\n
>   String 3: this correctly\n
>   String 4:then
> 
> 
> So maybe we should use the latter instead? We could use 'dmidecode -u
> --oem-string count' to get the number of strings and then iterate through
> each one of them.

It is a bit tedious having to run dmidecode multiple times, especially
since there won't be any strings in most cases.

With the normal format, we can reliably extract each string, but some
chars might have been mangled. How about we keep your current code, but
if we see a "." in a string, then we run "dmidecode -u --oem-string N"
just for that particular string, in order to get the unmangled value.

That will mean we only run dmidecode once majority of cases.

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

Re: [PATCH libvirt v1 3/6] conf: fix ZPCI address auto-generation on s390

[Andrea Bolognani]

On Thu, 2020-04-09 at 12:31 +0200, Shalini Chellathurai Saroja wrote:
> +++ b/src/conf/domain_addr.c
> @@ -145,12 +145,18 @@ static void
>  virDomainZPCIAddressReleaseIds(virDomainZPCIAddressIdsPtr zpciIds,
> virZPCIDeviceAddressPtr addr)
>  {
> -if (!zpciIds || virZPCIDeviceAddressIsEmpty(addr))
> +if (!zpciIds)
>  return;
>  
> -virDomainZPCIAddressReleaseUid(zpciIds->uids, addr);
> +if (addr->uid_set) {
> +virDomainZPCIAddressReleaseUid(zpciIds->uids, addr);
> +addr->uid_set = false;
> +}

I think it would be cleaner to handle the boolean flags in the same
spot the values are taken care of, that is, in the Release{U,F}id()
functions.

> @@ -186,12 +192,16 @@ static int
>  virDomainZPCIAddressReserveAddr(virDomainZPCIAddressIdsPtr zpciIds,
>  virZPCIDeviceAddressPtr addr)
>  {
> -if (virDomainZPCIAddressReserveUid(zpciIds->uids, addr) < 0)
> +if (addr->uid_set &&
> +virDomainZPCIAddressReserveUid(zpciIds->uids, addr) < 0)
>  return -1;

Same here...

>  
> -if (virDomainZPCIAddressReserveFid(zpciIds->fids, addr) < 0) {
> -virDomainZPCIAddressReleaseUid(zpciIds->uids, addr);
> -return -1;
> +if (addr->fid_set) {
> +if (virDomainZPCIAddressReserveFid(zpciIds->fids, addr) < 0) {
> +if (addr->uid_set)
> +virDomainZPCIAddressReleaseUid(zpciIds->uids, addr);
> +return -1;
> +}
>  }
>  
>  return 0;
> @@ -202,14 +212,28 @@ static int
>  virDomainZPCIAddressReserveNextAddr(virDomainZPCIAddressIdsPtr zpciIds,
>  virZPCIDeviceAddressPtr addr)
>  {
> -if (virDomainZPCIAddressReserveNextUid(zpciIds->uids, addr) < 0)
> -return -1;
> +bool uid_set, fid_set = false;
>  
> -if (virDomainZPCIAddressReserveNextFid(zpciIds->fids, addr) < 0) {
> -virDomainZPCIAddressReleaseUid(zpciIds->uids, addr);
> -return -1;
> +if (!addr->uid_set) {
> +if (virDomainZPCIAddressReserveNextUid(zpciIds->uids, addr) < 0)
> +return -1;
> +uid_set = true;
> +}

... and here. Basicall, push all handling of boolean flags one layer
down, where the actual values are taken care of.

> @@ -234,7 +258,7 @@ 
> virDomainPCIAddressExtensionReserveNextAddr(virDomainPCIAddressSetPtr addrs,
>  virPCIDeviceAddressPtr addr)
>  {
>  if (addr->extFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) {
> -virZPCIDeviceAddress zpci = { 0 };
> +virZPCIDeviceAddress zpci = addr->zpci;
>  
>  if (virDomainZPCIAddressReserveNextAddr(addrs->zpciIds, ) < 0)
>  return -1;
> @@ -246,6 +270,7 @@ 
> virDomainPCIAddressExtensionReserveNextAddr(virDomainPCIAddressSetPtr addrs,
>  return 0;
>  }
>  
> +
>  static int
>  virDomainPCIAddressExtensionEnsureAddr(virDomainPCIAddressSetPtr addrs,
> virPCIDeviceAddressPtr addr)
> @@ -253,10 +278,10 @@ 
> virDomainPCIAddressExtensionEnsureAddr(virDomainPCIAddressSetPtr addrs,
>  if (addr->extFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) {
>  virZPCIDeviceAddressPtr zpci = >zpci;
>  
> -if (virZPCIDeviceAddressIsEmpty(zpci))
> -return virDomainZPCIAddressReserveNextAddr(addrs->zpciIds, zpci);
> -else
> -return virDomainZPCIAddressReserveAddr(addrs->zpciIds, zpci);
> +if (virDomainZPCIAddressReserveAddr(addrs->zpciIds, zpci) < 0)
> +return -1;
> +
> +return virDomainZPCIAddressReserveNextAddr(addrs->zpciIds, zpci);
>  }

I think the semantics for these functions need to be reconsidered.

The way they currently work, as evidenced by EnsureAddr(), is that
you either have a fully-specified address provided by the user, in
which case you want to reserve that address, or you have an empty
address because the user didn't provide ZPCI information, in which
case you allocate a new full address based on what uids and fids are
still available and use that one.

With your changes, we can now find ourselves in a hybrid situation,
where half of the ZPCI address has been provided by the user but the
remaining half is up to us... Ultimately, it might make sense to have
ReserveAddr(), ReserveNextAddr() and EnsureAddr() all call to the
same function which does something along the lines of

  if (!zpci->uid_set)
  AssignUid(zpci);
  if (!zpci->fid_set)
  AssignFid(zpci);

  ReserveUid(zpci);
  ReserveFid(zpci);

because that's ultimately what we're achieving anyway, only with
more obfuscation.

> +++ b/src/conf/domain_conf.c
> @@ -7471,7 +7471,7 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
>
> virTristateSwitchTypeToString(info->addr.pci.multi));
>  }
>  
> -if (!virZPCIDeviceAddressIsEmpty(>addr.pci.zpci)) {
> +if (!virZPCIDeviceAddressIsIncomplete(>addr.pci.zpci)) {
>

Re: [libvirt PATCH 0/3] Point to our gitlab.com repos more

[Daniel P . Berrangé]

On Wed, Jun 03, 2020 at 12:47:09PM +0200, Ján Tomko wrote:
> Ján Tomko (3):
>   docs: csharp: remove outdated comment
>   docs: virshcmdref: change repo URL to GitLab
>   docs: point to GitLab as the primary git hosting

Reviewed-by: Daniel P. Berrangé 


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

Re: [libvirt PATCH 3/3] docs: point to GitLab as the primary git hosting

[Daniel P . Berrangé]

On Wed, Jun 03, 2020 at 12:47:12PM +0200, Ján Tomko wrote:
> We still point to git repositories hosted on libvirt.org in various
> places. Replace the links to their gitlab.com equivalents.
> 
> Note that GitLab is trying to be smart here and
>   https://gitlab.com/libvirt/libvirt
> redirects to
>   https://gitlab.com/libvirt/libvirt.git
> when doing a 'git clone' and vice-versa when visiting from the
> browser, so I only kept the .git suffix in places that explicitly
> mentioned 'git clone'.

That's good, because in RHEL-7 version of Git you must have the
.git suffix in order to clone.

> 
> Signed-off-by: Ján Tomko 
> ---
>  ChangeLog|  4 ++--
>  docs/aclpolkit.html.in   |  2 +-
>  docs/bindings.html.in|  2 +-
>  docs/csharp.html.in  | 12 ++--
>  docs/dbus.html.in| 12 ++--
>  docs/hacking.rst |  4 ++--
>  docs/java.html.in| 12 ++--
>  docs/php.html.in | 10 ++
>  docs/securityprocess.html.in |  2 +-
>  docs/testapi.html.in |  2 +-
>  docs/testsuites.html.in  |  4 ++--
>  docs/testtck.html.in |  2 +-
>  12 files changed, 19 insertions(+), 49 deletions(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index 55eb05c5ce..897f38e56d 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -7,9 +7,9 @@ archives.
>  If you're interested in the full list of changes made to libvirt since
>  the project was started, you can clone the git repository from
>  
> -  https://libvirt.org/git/libvirt.git
> +  https://gitlab.com/libvirt/libvirt/
>  
>  and browse them locally using your favorite git history viewer or,
>  alternatively, browse them online at
>  
> -  https://libvirt.org/git/?p=libvirt.git;a=log
> +  https://gitlab.com/libvirt/libvirt/-/commits/master
> diff --git a/docs/aclpolkit.html.in b/docs/aclpolkit.html.in
> index 3e5d30a5dd..8bd8b58e02 100644
> --- a/docs/aclpolkit.html.in
> +++ b/docs/aclpolkit.html.in
> @@ -459,7 +459,7 @@ polkit.addRule(function(action, subject) {
>  
>  
>  See
> - href="https://libvirt.org/git/?p=libvirt.git;a=tree;f=examples/polkit;hb=HEAD;>source
>  code
> + href="https://gitlab.com/libvirt/libvirt/-/tree/master/examples/polkit;>source
>  code
>  for a more complex example.
>  
>  
> diff --git a/docs/bindings.html.in b/docs/bindings.html.in
> index 46930cd6f6..8a482015b9 100644
> --- a/docs/bindings.html.in
> +++ b/docs/bindings.html.in
> @@ -49,7 +49,7 @@
>
>  
>Python: Libvirt's python bindings are split to a
> -  separate  href="https://libvirt.org/git/?p=libvirt-python.git;>package
> +  separate  href="https://gitlab.com/libvirt/libvirt-python;>package
>since version 1.2.0, older versions came with direct support for 
> the
>Python language.
>  
> diff --git a/docs/csharp.html.in b/docs/csharp.html.in
> index df1db49bec..89b3957904 100644
> --- a/docs/csharp.html.in
> +++ b/docs/csharp.html.in
> @@ -33,19 +33,11 @@
>  
>The C# bindings source code is maintained in a href="http://git-scm.com/;>git repository available on
> -  https://libvirt.org/git/;>libvirt.org:
> +  https://gitlab.com/libvirt/libvirt-csharp;>gitlab.com:
>  
>  
>  
> -git clone https://libvirt.org/git/libvirt-csharp.git
> -
> -
> -
> -  They can also be browsed online:
> -
> -
> -
> - href="https://libvirt.org/git/?p=libvirt-csharp.git;a=summary;>https://libvirt.org/git/?p=libvirt-csharp.git;a=summary
> +git clone https://gitlab.com/libvirt/libvirt-csharp.git
>  
>  
>  Usage
> diff --git a/docs/dbus.html.in b/docs/dbus.html.in
> index 4dfe4ca427..99f191a685 100644
> --- a/docs/dbus.html.in
> +++ b/docs/dbus.html.in
> @@ -17,19 +17,11 @@
>  
>The D-Bus bindings source code is maintained in a
>https://git-scm.com/;>git repository available on
> -  https://libvirt.org/git/;>libvirt.org:
> +  https://gitlab.com/libvirt/libvirt-dbus;>gitlab.com:
>  
>  
>  
> -git clone https://libvirt.org/git/libvirt-dbus.git
> -
> -
> -
> -  They can also be browsed online:
> -
> -
> -
> - href="https://libvirt.org/git/?p=libvirt-dbus.git;>https://libvirt.org/git/?p=libvirt-dbus.git
> +git clone https://gitlab.com/libvirt/libvirt-dbus.git
>  
>  
>  Usage
> diff --git a/docs/hacking.rst b/docs/hacking.rst
> index 61ae6452e1..51ff862e8f 100644
> --- a/docs/hacking.rst
> +++ b/docs/hacking.rst
> @@ -9,9 +9,9 @@ Repositories and external resources
>  ===
>  
>  The official upstream repository is kept in git
> -(``https://libvirt.org/git/libvirt.git``) and is browsable
> +(``https://gitlab.com/libvirt/libvirt``) and is browsable
>  along with other libvirt-related repositories (e.g.
> -libvirt-python) `online `__.
> +libvirt-python) `online `__.
>  
>  Patches to translations are maintained via the `zanata
>  project 

Re: [libvirt PATCH 1/2] lxc: replace VIR_FREE with g_autofree / g_free

[Peter Krempa]

On Wed, Jun 03, 2020 at 06:42:10 -0400, John Ferlan wrote:
> First time in a while - Coverity complained this morning
> 
> [...]
> 
> > diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c
> > index e73b4d0690..c4223f4e06 100644
> > --- a/src/lxc/lxc_fuse.c
> > +++ b/src/lxc/lxc_fuse.c
> > @@ -326,10 +326,10 @@ int lxcSetupFuse(virLXCFusePtr *f, virDomainDefPtr 
> > def)
> >  *f = fuse;
> 
> ^^
> Event use_after_free: Using freed pointer "fuse".
> Also see events:  [alias][freed_arg]
> 
> >  return ret;
> >   cleanup1:
> > -VIR_FREE(fuse->mountpoint);
> > +g_free(fuse->mountpoint);
> >  virMutexDestroy(>lock);
> >   cleanup2:
> > -VIR_FREE(fuse);
> > +g_free(fuse);
> 
> ^^
> Event freed_arg:  "g_free" frees "fuse".
> 
> A fuse = NULL; here will make coverity happy, but not sure if that's
> standard any more... The VIR_FREE would have done thta for us IIRC.

The equivalent replacement for 'VIR_FREE' is
'g_clear_pointer(, g_free)' as actually done by VIR_FREE nowadays
and not just g_free. The side effect of VIR_FREE, non-equivalence to
g_free combined with the fact that g_clear_pointer is longer makes this
a source of possible nasty bugs.

[libvirt PATCH 2/3] docs: virshcmdref: change repo URL to GitLab

[Ján Tomko]

Also note that it's archived, because it's definitely
not maintained anymore.

Signed-off-by: Ján Tomko 
---
 docs/virshcmdref.html.in | 16 ++--
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/docs/virshcmdref.html.in b/docs/virshcmdref.html.in
index c5d8d39210..3534b6de4e 100644
--- a/docs/virshcmdref.html.in
+++ b/docs/virshcmdref.html.in
@@ -66,22 +66,10 @@
 
 DocBook source GIT repository
 
-  The DocBook source is maintained in a http://git-scm.com/;>git repository available on
-  https://libvirt.org/git/;>libvirt.org:
+  https://gitlab.com/libvirt/libvirt-virshcmdref;>gitlab.com.
 
 
-
-git clone https://libvirt.org/git/libvirt-virshcmdref.git
-
-
-
-  It can also be browsed online:
-
-
-
-https://libvirt.org/git/?p=libvirt-virshcmdref.git;>https://libvirt.org/git/?p=libvirt-virshcmdref.git
-
-
   
 
-- 
2.25.4

[libvirt PATCH 1/3] docs: csharp: remove outdated comment

[Ján Tomko]

We've had no tarballs for almost 10 years.

Give up and delete the commented out links to them.

Signed-off-by: Ján Tomko 
---
 docs/csharp.html.in | 13 -
 1 file changed, 13 deletions(-)

diff --git a/docs/csharp.html.in b/docs/csharp.html.in
index aee0b75eb8..df1db49bec 100644
--- a/docs/csharp.html.in
+++ b/docs/csharp.html.in
@@ -29,19 +29,6 @@
   compiling libvirt for windows.
 
 
-
-
 GIT source repository
 
   The C# bindings source code is maintained in a 

[libvirt PATCH 3/3] docs: point to GitLab as the primary git hosting

[Ján Tomko]

We still point to git repositories hosted on libvirt.org in various
places. Replace the links to their gitlab.com equivalents.

Note that GitLab is trying to be smart here and
  https://gitlab.com/libvirt/libvirt
redirects to
  https://gitlab.com/libvirt/libvirt.git
when doing a 'git clone' and vice-versa when visiting from the
browser, so I only kept the .git suffix in places that explicitly
mentioned 'git clone'.

Signed-off-by: Ján Tomko 
---
 ChangeLog|  4 ++--
 docs/aclpolkit.html.in   |  2 +-
 docs/bindings.html.in|  2 +-
 docs/csharp.html.in  | 12 ++--
 docs/dbus.html.in| 12 ++--
 docs/hacking.rst |  4 ++--
 docs/java.html.in| 12 ++--
 docs/php.html.in | 10 ++
 docs/securityprocess.html.in |  2 +-
 docs/testapi.html.in |  2 +-
 docs/testsuites.html.in  |  4 ++--
 docs/testtck.html.in |  2 +-
 12 files changed, 19 insertions(+), 49 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 55eb05c5ce..897f38e56d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -7,9 +7,9 @@ archives.
 If you're interested in the full list of changes made to libvirt since
 the project was started, you can clone the git repository from
 
-  https://libvirt.org/git/libvirt.git
+  https://gitlab.com/libvirt/libvirt/
 
 and browse them locally using your favorite git history viewer or,
 alternatively, browse them online at
 
-  https://libvirt.org/git/?p=libvirt.git;a=log
+  https://gitlab.com/libvirt/libvirt/-/commits/master
diff --git a/docs/aclpolkit.html.in b/docs/aclpolkit.html.in
index 3e5d30a5dd..8bd8b58e02 100644
--- a/docs/aclpolkit.html.in
+++ b/docs/aclpolkit.html.in
@@ -459,7 +459,7 @@ polkit.addRule(function(action, subject) {
 
 
 See
-https://libvirt.org/git/?p=libvirt.git;a=tree;f=examples/polkit;hb=HEAD;>source
 code
+https://gitlab.com/libvirt/libvirt/-/tree/master/examples/polkit;>source 
code
 for a more complex example.
 
 
diff --git a/docs/bindings.html.in b/docs/bindings.html.in
index 46930cd6f6..8a482015b9 100644
--- a/docs/bindings.html.in
+++ b/docs/bindings.html.in
@@ -49,7 +49,7 @@
   
 
   Python: Libvirt's python bindings are split to a
-  separate https://libvirt.org/git/?p=libvirt-python.git;>package
+  separate https://gitlab.com/libvirt/libvirt-python;>package
   since version 1.2.0, older versions came with direct support for the
   Python language.
 
diff --git a/docs/csharp.html.in b/docs/csharp.html.in
index df1db49bec..89b3957904 100644
--- a/docs/csharp.html.in
+++ b/docs/csharp.html.in
@@ -33,19 +33,11 @@
 
   The C# bindings source code is maintained in a http://git-scm.com/;>git repository available on
-  https://libvirt.org/git/;>libvirt.org:
+  https://gitlab.com/libvirt/libvirt-csharp;>gitlab.com:
 
 
 
-git clone https://libvirt.org/git/libvirt-csharp.git
-
-
-
-  They can also be browsed online:
-
-
-
-https://libvirt.org/git/?p=libvirt-csharp.git;a=summary;>https://libvirt.org/git/?p=libvirt-csharp.git;a=summary
+git clone https://gitlab.com/libvirt/libvirt-csharp.git
 
 
 Usage
diff --git a/docs/dbus.html.in b/docs/dbus.html.in
index 4dfe4ca427..99f191a685 100644
--- a/docs/dbus.html.in
+++ b/docs/dbus.html.in
@@ -17,19 +17,11 @@
 
   The D-Bus bindings source code is maintained in a
   https://git-scm.com/;>git repository available on
-  https://libvirt.org/git/;>libvirt.org:
+  https://gitlab.com/libvirt/libvirt-dbus;>gitlab.com:
 
 
 
-git clone https://libvirt.org/git/libvirt-dbus.git
-
-
-
-  They can also be browsed online:
-
-
-
-https://libvirt.org/git/?p=libvirt-dbus.git;>https://libvirt.org/git/?p=libvirt-dbus.git
+git clone https://gitlab.com/libvirt/libvirt-dbus.git
 
 
 Usage
diff --git a/docs/hacking.rst b/docs/hacking.rst
index 61ae6452e1..51ff862e8f 100644
--- a/docs/hacking.rst
+++ b/docs/hacking.rst
@@ -9,9 +9,9 @@ Repositories and external resources
 ===
 
 The official upstream repository is kept in git
-(``https://libvirt.org/git/libvirt.git``) and is browsable
+(``https://gitlab.com/libvirt/libvirt``) and is browsable
 along with other libvirt-related repositories (e.g.
-libvirt-python) `online `__.
+libvirt-python) `online `__.
 
 Patches to translations are maintained via the `zanata
 project `__. If you want to fix a
diff --git a/docs/java.html.in b/docs/java.html.in
index 147b11c166..fa3fe8f900 100644
--- a/docs/java.html.in
+++ b/docs/java.html.in
@@ -26,19 +26,11 @@ which you can use to include this in your maven 
projects.
 GIT source repository
  The Java bindings code source is now maintained in a http://git-scm.com/;>git repository available on
-https://libvirt.org/git/;>libvirt.org:
+https://gitlab.com/libvirt/libvirt-java/;>gitlab.com:
 
 

[libvirt PATCH 0/3] Point to our gitlab.com repos more

[Ján Tomko]

Ján Tomko (3):
  docs: csharp: remove outdated comment
  docs: virshcmdref: change repo URL to GitLab
  docs: point to GitLab as the primary git hosting

 ChangeLog|  4 ++--
 docs/aclpolkit.html.in   |  2 +-
 docs/bindings.html.in|  2 +-
 docs/csharp.html.in  | 25 ++---
 docs/dbus.html.in| 12 ++--
 docs/hacking.rst |  4 ++--
 docs/java.html.in| 12 ++--
 docs/php.html.in | 10 ++
 docs/securityprocess.html.in |  2 +-
 docs/testapi.html.in |  2 +-
 docs/testsuites.html.in  |  4 ++--
 docs/testtck.html.in |  2 +-
 docs/virshcmdref.html.in | 16 ++--
 13 files changed, 21 insertions(+), 76 deletions(-)

-- 
2.25.4

Re: [libvirt PATCH 1/2] lxc: replace VIR_FREE with g_autofree / g_free

[John Ferlan]

First time in a while - Coverity complained this morning

[...]

> diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c
> index e73b4d0690..c4223f4e06 100644
> --- a/src/lxc/lxc_fuse.c
> +++ b/src/lxc/lxc_fuse.c
> @@ -326,10 +326,10 @@ int lxcSetupFuse(virLXCFusePtr *f, virDomainDefPtr def)
>  *f = fuse;

^^
Event use_after_free:   Using freed pointer "fuse".
Also see events:[alias][freed_arg]

>  return ret;
>   cleanup1:
> -VIR_FREE(fuse->mountpoint);
> +g_free(fuse->mountpoint);
>  virMutexDestroy(>lock);
>   cleanup2:
> -VIR_FREE(fuse);
> +g_free(fuse);

^^
Event freed_arg:"g_free" frees "fuse".

A fuse = NULL; here will make coverity happy, but not sure if that's
standard any more... The VIR_FREE would have done thta for us IIRC.

John

[...]

Re: [PATCH] qemucapabilitiestest: Bump qemu-5.1 capabilities for x86_64

[Peter Krempa]

On Wed, Jun 03, 2020 at 11:45:49 +0200, Peter Krempa wrote:
> On Wed, Jun 03, 2020 at 11:37:21 +0200, Erik Skultety wrote:
> > On Wed, Jun 03, 2020 at 11:29:36AM +0200, Peter Krempa wrote:
> > > On Wed, Jun 03, 2020 at 11:11:08 +0200, Michal Privoznik wrote:
> > > > On 6/3/20 10:40 AM, Peter Krempa wrote:
> > > > > On Wed, Jun 03, 2020 at 10:27:57 +0200, Michal Privoznik wrote:
> > > > > > On 6/3/20 9:31 AM, Peter Krempa wrote:
> 
> [...]
> 
> > > Well. They are built on fedora, but certainly not taken from fedora.
> > > It's built from git.
> > >
> > > > Maybe we can document configure arguments for QEMU so that it is
> > > > reproducible.
> > >
> > > While I agree that we should do that to take one of the moving parts out
> > > of the equation we still can't do anything about the machine dependance
> > > of the output. Specifically it contains all cpu flags so it really can't
> > > be re-generated reproducibly on a box with even a slightly different
> > > cpu.
> > >
> > > Ideally we'd build it with the fedora spec-file, but I got lazy and I'm
> > > usually just re-running configure from my history in this case.
> > >
> > > My only idea how to provide stable output is to create a job on a box
> > > which will periodically re-build qemu and fetch the capabilities and
> > > publish them somewhere so that others could grab those and refresh the
> > > caps themselves, but I can't really think of a way how to enable others
> > > do it on their machine whithout messing up the CPU.
> > 
> > You beat me in the response time wrt reply :). Hmm, how about we add this 
> > as a
> > job to lcitool which controls how virt-install is spawned, that way + an 
> > Ansible
> > playbook you always get a reproducible environment?
> 
> I don't think that's a particularly worthwhile idea. It would have to
> run fully emulated to shield from cpu differences which would make it
> super slow. In such case I'll rather periodically or on request update
> it myself rather than deal with something which takes ages. 

Also the problem of having a stable way to generate capabilities is
orthogonal to the original problem of whether to review the capabilities
or not.

I still think manual review is necessary regardless of how stable the
way to generate the capabilities is and in some cases there are even
differences which break tests (recent dropping of the 0.13 machine type
from upstream qemu, deprecation of some commands etc.) so I don't see
a way how this part can be avoided or any reasonably simple set of rules
for avoiding review to be established.

Obviously having a stable way to create capabilities would be certainly
desirable for other scenarios, but not at the cost of running it in TCG.

If we want to achieve that we probably will need a way to strip cpu
related commands from the capabilities and leave cpu probing for a
different test.

This in the end may be desirable due to other reasons as well, but I'm
certainly not sure how to achieve that. As of such I'll gladly continue
updating the capabilities on request by others for now as it doesn't
really cause any burden for me and I need to update it anyways for stuff
I'm developing.

Re: [PATCH] autogen.sh: Restore --no-git (avoid git submodule update)

[Daniel P . Berrangé]

On Wed, Jun 03, 2020 at 12:31:09PM +0200, Pavel Hrdina wrote:
> On Tue, Jun 02, 2020 at 04:47:45PM +0100, Ian Jackson wrote:
> > Prior to 2621d48f005a "gnulib: delete all gnulib integration",
> > one could pass ./autogen.sh --no-git to prevent the libvirt build
> > system from running git submodule update.
> > 
> > This feature is needed by systems like the Xen Project CI which want
> > to explicitly control the revisions of every tree.  These will
> > typically arrange to initialise the submodules check out the right
> > version of everything, and then expect the build system not to mess
> > with it any more.
> 
> To be honest I don't understand why would anyone want to keep track of
> all submodules of all projects for any CI and update it manually every
> time the upstream project changes these submodules. Sounds like a lot
> of unnecessary work but maybe I'm missing something.

Two possible scenarios that I think are valid

 - The CI job script does not have network access
 - The CI job script sees the source dir as read-only

In both cases the CI harness would have to initialize the submodule
before runing the CI job.

I don't know if this is what Xen CI is hitting, but I think the
request is reasonable none the less.

Both Jenkins and GitLab CI have option to make the harness init
submodules prior to the job running.

> > Despite to the old documentation comments referring only to gnulib,
> > the --no-git feature is required not only because of gnulib but also
> > because of the other submodule, src/keycodemapdb.
> > 
> > (And in any case, even if it were no longer required because all the
> > submodules were removed, it ought ideally to have been retained as a
> > no-op for compaibility reasons.)
> 
> Well, we will break a lot more by switching to Meson build system where
> everyone building libvirt will have to change their scripts as it will
> not be compatible at all.

Yes, but I think that's tangential, as the two above reasons will
still apply, and Meson will cope with having submodules pre-initialized.


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

Re: [PATCH] autogen.sh: Restore --no-git (avoid git submodule update)

[Pavel Hrdina]

On Tue, Jun 02, 2020 at 04:47:45PM +0100, Ian Jackson wrote:
> Prior to 2621d48f005a "gnulib: delete all gnulib integration",
> one could pass ./autogen.sh --no-git to prevent the libvirt build
> system from running git submodule update.
> 
> This feature is needed by systems like the Xen Project CI which want
> to explicitly control the revisions of every tree.  These will
> typically arrange to initialise the submodules check out the right
> version of everything, and then expect the build system not to mess
> with it any more.

To be honest I don't understand why would anyone want to keep track of
all submodules of all projects for any CI and update it manually every
time the upstream project changes these submodules. Sounds like a lot
of unnecessary work but maybe I'm missing something.

> Despite to the old documentation comments referring only to gnulib,
> the --no-git feature is required not only because of gnulib but also
> because of the other submodule, src/keycodemapdb.
> 
> (And in any case, even if it were no longer required because all the
> submodules were removed, it ought ideally to have been retained as a
> no-op for compaibility reasons.)

Well, we will break a lot more by switching to Meson build system where
everyone building libvirt will have to change their scripts as it will
not be compatible at all.

Pavel

> So restore the --no-git feature.
> 
> Because of the way the argument parsing of autogen.sh works, it is
> easiest to recognise this option only if it comes first.  This works
> for the Xen Project CI, which has always passed this option first.
> 
> If something else is using this option (and hasn't introduced a
> different workaround in the meantime), not in the first position,
> then perhaps a more sophisticated approach will be needed.  But I
> think this will do for now.
> 
> Signed-off-by: Ian Jackson 
> ---
>  autogen.sh | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/autogen.sh b/autogen.sh
> index 4e1bbceb0a..1c98273452 100755
> --- a/autogen.sh
> +++ b/autogen.sh
> @@ -1,5 +1,10 @@
>  #!/bin/sh
>  # Run this to generate all the initial makefiles, etc.
> +#
> +# THe following options must come first.  All other or subsequent
> +# arguments are passed to configure:
> +#   --no-git   skip `git submodule update --init`
> +
>  test -n "$srcdir" || srcdir=$(dirname "$0")
>  test -n "$srcdir" || srcdir=.
>  
> @@ -13,7 +18,11 @@ cd "$srcdir"
>  exit 1
>  }
>  
> -git submodule update --init || exit 1
> +if [ "x$1" = x--no-git ]; then
> + shift
> +else
> + git submodule update --init || exit 1
> +fi
>  
>  autoreconf --verbose --force --install || exit 1
>  
> -- 
> 2.11.0
> 


signature.asc
Description: PGP signature

[PATCH] conf: force 8 byte alignment for virObjectEvent

[Daniel P . Berrangé]

We need to be able to cast from virObjectEventPtr to one of
its many subclasses. Some of these subclasses have 8 byte
alignment on 32-bit platforms, but virObjectEventPtr only
has 4 byte alignment.

Previously the virObject base class had 8 byte alignment
but this dropped to 4 byte when converted to inherit from
GObject. This introduces cast alignment warnings on 32-bit:

../../src/conf/domain_event.c: In function 'virDomainEventDispatchDefaultFunc':
../../src/conf/domain_event.c:1656:30: error: cast increases required alignment 
of target type [-Werror=cast-align]
 1656 | rtcChangeEvent = (virDomainEventRTCChangePtr)event;
  |  ^
../../src/conf/domain_event.c:1785:34: error: cast increases required alignment 
of target type [-Werror=cast-align]
 1785 | balloonChangeEvent = (virDomainEventBalloonChangePtr)event;
  |  ^
../../src/conf/domain_event.c:1896:35: error: cast increases required alignment 
of target type [-Werror=cast-align]
 1896 | blockThresholdEvent = 
(virDomainEventBlockThresholdPtr)event;
  |   ^
../../src/conf/domain_event.c: In function 
'virDomainQemuMonitorEventDispatchFunc':
../../src/conf/domain_event.c:1974:24: error: cast increases required alignment 
of target type [-Werror=cast-align]
 1974 | qemuMonitorEvent = (virDomainQemuMonitorEventPtr)event;
  |^
../../src/conf/domain_event.c: In function 'virDomainQemuMonitorEventFilter':
../../src/conf/domain_event.c:2179:20: error: cast increases required alignment 
of target type [-Werror=cast-align]
 2179 | monitorEvent = (virDomainQemuMonitorEventPtr) event;
  |^

Forcing 8-byte alignment on virObjectEventPtr removes the
alignment increase during casts to subclasses.

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

Technically a build-breaker, but since we don't have any existing
usage of __attribute__((aligned)), I wanted to get a second opinion
on this approach.

One alternative approach would be to switch one of the current "int"
fields in virObjectEvent to "long long".

 src/conf/object_event_private.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/conf/object_event_private.h b/src/conf/object_event_private.h
index b31441c53a..126464a9a5 100644
--- a/src/conf/object_event_private.h
+++ b/src/conf/object_event_private.h
@@ -42,7 +42,7 @@ typedef void
   virConnectObjectEventGenericCallback cb,
   void *cbopaque);
 
-struct _virObjectEvent {
+struct  __attribute__((aligned(4))) _virObjectEvent {
 virObject parent;
 int eventID;
 virObjectMeta meta;
-- 
2.24.1

Re: [PATCH libvirt v1 2/6] tests: qemu: add tests for ZPCI on s390

[Andrea Bolognani]

On Thu, 2020-04-09 at 12:31 +0200, Shalini Chellathurai Saroja wrote:
> Add test to verify ZPCI address validation with uid set to 0x0
> 
> Signed-off-by: Bjoern Walk 
> Signed-off-by: Shalini Chellathurai Saroja 
> Reviewed-by: Bjoern Walk 
> Reviewed-by: Boris Fiuczynski 
> ---
>  .../hostdev-vfio-zpci-uid-set-zero.xml| 20 +++
>  tests/qemuxml2argvtest.c  |  3 +++
>  2 files changed, 23 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-uid-set-zero.xml

The test cases for the new behavior should go into the patch that
implements it.

> +++ b/tests/qemuxml2argvtest.c
> @@ -1650,6 +1650,9 @@ mymain(void)
>  DO_TEST("hostdev-vfio-zpci-autogenerate",
>  QEMU_CAPS_DEVICE_VFIO_PCI,
>  QEMU_CAPS_DEVICE_ZPCI);
> +DO_TEST_PARSE_ERROR("hostdev-vfio-zpci-uid-set-zero",
> +QEMU_CAPS_DEVICE_VFIO_PCI,
> +QEMU_CAPS_DEVICE_ZPCI);

Indentation is off.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH libvirt v1 1/6] conf: fix ZPCI address validation on s390

[Andrea Bolognani]

On Thu, 2020-04-09 at 12:31 +0200, Shalini Chellathurai Saroja wrote:
> -if (uid &&
> -virStrToLong_uip(uid, NULL, 0, ) < 0) {
> -virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -   _("Cannot parse  'uid' attribute"));
> -goto cleanup;
> +if (uid) {
> +if (virStrToLong_uip(uid, NULL, 0, ) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("Cannot parse  'uid' attribute"));
> +return -1;
> +}
> +if (!virZPCIDeviceAddressIsValid())
> +return -1;
>  }

This doesn't seem right.

I understand that you're moving the call to
virZPCIDeviceAddressIsValid() inside the if(uid) branch so that you
won't get an error for something like

  

but this is not a good way to do it: in fact, you change it again in
patch 3/6 and adopt the much better approach of storing directly in
the struct whether uid and fid have been set.

You should go for that approach right away instead of implementing
this intermediate solution first.

> - cleanup:
> -VIR_FREE(uid);
> -VIR_FREE(fid);
> -return ret;
> +return 0;

Switching to g_autofree is a nice improvement, but it's a completely
independent one so please make that a separate patch that doesn't
touch the logic.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH] qemucapabilitiestest: Bump qemu-5.1 capabilities for x86_64

[Peter Krempa]

On Wed, Jun 03, 2020 at 11:34:07 +0200, Erik Skultety wrote:
> On Wed, Jun 03, 2020 at 11:11:08AM +0200, Michal Privoznik wrote:
> > On 6/3/20 10:40 AM, Peter Krempa wrote:
> > > On Wed, Jun 03, 2020 at 10:27:57 +0200, Michal Privoznik wrote:
> > > > On 6/3/20 9:31 AM, Peter Krempa wrote:
> > > > > QEMU added the machine types for the 5.1 release so let's update them.
> > > > >
> > > > > Other notable changes are 'cpu-throttle-tailslow' migration property,
> > > > > 'zlib' compression for qcow2 images and absrtact socket support.
> > > > >
> > > > > Signed-off-by: Peter Krempa 
> > > > > ---
> > > > > As usual, I'll be refreshing this until the release so that we always
> > > > > have fresh capabilities to prevent any surprises with deprecation and
> > > > > big updates.
> > > > >
> > > > >.../domaincapsdata/qemu_5.1.0-q35.x86_64.xml  |   2 +-
> > > > >.../domaincapsdata/qemu_5.1.0-tcg.x86_64.xml  |   2 +-
> > > > >tests/domaincapsdata/qemu_5.1.0.x86_64.xml|   2 +-
> > > > >.../caps_5.1.0.x86_64.replies | 357 
> > > > > +++---
> > > > >.../caps_5.1.0.x86_64.xml |  14 +-
> > > > >5 files changed, 237 insertions(+), 140 deletions(-)
> > > >
> > > > Reviewed-by: Michal Privoznik 
> > > >
> > > > Maybe we can have another rule that would allow you to push these 
> > > > without
> > > > review? I can argue both ways, so I'm just putting it out there.
> > >
> > > Yeah. I thought about that too.
> > >
> > > Specifically one thing I'd like to avoid is carelessness in case of the
> > > update. Specifically if there is some form of removal (flag being
> > > removed and such) we need to be careful and consider the implications.
> >
> > Well, for that we would need to compare with older capabilities XML and I
> > don't think we are doing that. Removal between the same capabilities XML of
> > an unreleased QEMU are uncommon. But I hear what you're saying and that's my
> > concern too.
> >
> > >
> > > In this very specific case there's nothing of note and I'd be okay with
> > > just pushing it, but the rules if we wanted to codify it somehow would
> > > require to be more nuanced and I don't think I can express all the
> > > caveats.
> > >
> > > That's why I didn't really argue for adding a special rule for this.
> > >
> > > Also one reason I'm doing periodic upgrades of this is so that others
> > > don't have to do it. The problem here is that the output is very much
> > > dependent on the machine where you run it and I don't want others to
> > > have to update the files when adding a new capability as the difference
> > > becomes unreviewable and may even regress depending on how qemu is
> > > built.
> > >
> >
> > This is a long known issue and perhaps it would be worth documenting
> > somewhere? I think these are QEMU binaries taken from Fedora, is that so?
> > Maybe we can document configure arguments for QEMU so that it is
> > reproducible.
> 
> Not only that, we could set up an upstream fedora VM following those steps
> (not just steps but the overall HW setup to unify the whole process) and

Are there any machines which are guaranteed to be stable? Machines which
are not in a cloud or something which can just be re-scheduled somewhere
else.

> generating the capabilities whenever new qemu is tagged in git. Of course,

We do this for pre-release versions too. IMO as the qemu dev cycle is
very long we want to periodically re-sync to allow developing features
in parallel and also be notified about deprecations which are detectable
via QMP introspection.

> sometimes you need them earlier than an RC is tagged, but still, I think it
> would be beneficial to automate this process by setting up the agent in a way
> it would send a patch/MR against libvirt.

Well. It's not worth doing too often though. I can't really quantify
when it's good enough to update. Specifically in some cases the bump
needs to happen as some new feature was added and you'd like to use it
so it definitely doesn't have a reasonable optimization function.

> If it turns out this to be desirable from upstream libvirt POV, I can set up
> such an upstream runner.

Re: [PATCH] qemucapabilitiestest: Bump qemu-5.1 capabilities for x86_64

[Peter Krempa]

On Wed, Jun 03, 2020 at 11:37:21 +0200, Erik Skultety wrote:
> On Wed, Jun 03, 2020 at 11:29:36AM +0200, Peter Krempa wrote:
> > On Wed, Jun 03, 2020 at 11:11:08 +0200, Michal Privoznik wrote:
> > > On 6/3/20 10:40 AM, Peter Krempa wrote:
> > > > On Wed, Jun 03, 2020 at 10:27:57 +0200, Michal Privoznik wrote:
> > > > > On 6/3/20 9:31 AM, Peter Krempa wrote:

[...]

> > Well. They are built on fedora, but certainly not taken from fedora.
> > It's built from git.
> >
> > > Maybe we can document configure arguments for QEMU so that it is
> > > reproducible.
> >
> > While I agree that we should do that to take one of the moving parts out
> > of the equation we still can't do anything about the machine dependance
> > of the output. Specifically it contains all cpu flags so it really can't
> > be re-generated reproducibly on a box with even a slightly different
> > cpu.
> >
> > Ideally we'd build it with the fedora spec-file, but I got lazy and I'm
> > usually just re-running configure from my history in this case.
> >
> > My only idea how to provide stable output is to create a job on a box
> > which will periodically re-build qemu and fetch the capabilities and
> > publish them somewhere so that others could grab those and refresh the
> > caps themselves, but I can't really think of a way how to enable others
> > do it on their machine whithout messing up the CPU.
> 
> You beat me in the response time wrt reply :). Hmm, how about we add this as a
> job to lcitool which controls how virt-install is spawned, that way + an 
> Ansible
> playbook you always get a reproducible environment?

I don't think that's a particularly worthwhile idea. It would have to
run fully emulated to shield from cpu differences which would make it
super slow. In such case I'll rather periodically or on request update
it myself rather than deal with something which takes ages. 

Re: [PATCH] qemucapabilitiestest: Bump qemu-5.1 capabilities for x86_64

[Erik Skultety]

On Wed, Jun 03, 2020 at 11:29:36AM +0200, Peter Krempa wrote:
> On Wed, Jun 03, 2020 at 11:11:08 +0200, Michal Privoznik wrote:
> > On 6/3/20 10:40 AM, Peter Krempa wrote:
> > > On Wed, Jun 03, 2020 at 10:27:57 +0200, Michal Privoznik wrote:
> > > > On 6/3/20 9:31 AM, Peter Krempa wrote:
> > > > > QEMU added the machine types for the 5.1 release so let's update them.
> > > > >
> > > > > Other notable changes are 'cpu-throttle-tailslow' migration property,
> > > > > 'zlib' compression for qcow2 images and absrtact socket support.
> > > > >
> > > > > Signed-off-by: Peter Krempa 
> > > > > ---
> > > > > As usual, I'll be refreshing this until the release so that we always
> > > > > have fresh capabilities to prevent any surprises with deprecation and
> > > > > big updates.
> > > > >
> > > > >.../domaincapsdata/qemu_5.1.0-q35.x86_64.xml  |   2 +-
> > > > >.../domaincapsdata/qemu_5.1.0-tcg.x86_64.xml  |   2 +-
> > > > >tests/domaincapsdata/qemu_5.1.0.x86_64.xml|   2 +-
> > > > >.../caps_5.1.0.x86_64.replies | 357 
> > > > > +++---
> > > > >.../caps_5.1.0.x86_64.xml |  14 +-
> > > > >5 files changed, 237 insertions(+), 140 deletions(-)
> > > >
> > > > Reviewed-by: Michal Privoznik 
> > > >
> > > > Maybe we can have another rule that would allow you to push these 
> > > > without
> > > > review? I can argue both ways, so I'm just putting it out there.
> > >
> > > Yeah. I thought about that too.
> > >
> > > Specifically one thing I'd like to avoid is carelessness in case of the
> > > update. Specifically if there is some form of removal (flag being
> > > removed and such) we need to be careful and consider the implications.
> >
> > Well, for that we would need to compare with older capabilities XML and I
> > don't think we are doing that. Removal between the same capabilities XML of
>
> I certainly am doing that when generating files after the release to
> ensure that we don't regress in anything. Obviously only on the level of
> detected capabilities.
>
> > an unreleased QEMU are uncommon. But I hear what you're saying and that's my
> > concern too.
> >
> > >
> > > In this very specific case there's nothing of note and I'd be okay with
> > > just pushing it, but the rules if we wanted to codify it somehow would
> > > require to be more nuanced and I don't think I can express all the
> > > caveats.
> > >
> > > That's why I didn't really argue for adding a special rule for this.
> > >
> > > Also one reason I'm doing periodic upgrades of this is so that others
> > > don't have to do it. The problem here is that the output is very much
> > > dependent on the machine where you run it and I don't want others to
> > > have to update the files when adding a new capability as the difference
> > > becomes unreviewable and may even regress depending on how qemu is
> > > built.
> > >
> >
> > This is a long known issue and perhaps it would be worth documenting
> > somewhere? I think these are QEMU binaries taken from Fedora, is that so?
>
> Well. They are built on fedora, but certainly not taken from fedora.
> It's built from git.
>
> > Maybe we can document configure arguments for QEMU so that it is
> > reproducible.
>
> While I agree that we should do that to take one of the moving parts out
> of the equation we still can't do anything about the machine dependance
> of the output. Specifically it contains all cpu flags so it really can't
> be re-generated reproducibly on a box with even a slightly different
> cpu.
>
> Ideally we'd build it with the fedora spec-file, but I got lazy and I'm
> usually just re-running configure from my history in this case.
>
> My only idea how to provide stable output is to create a job on a box
> which will periodically re-build qemu and fetch the capabilities and
> publish them somewhere so that others could grab those and refresh the
> caps themselves, but I can't really think of a way how to enable others
> do it on their machine whithout messing up the CPU.

You beat me in the response time wrt reply :). Hmm, how about we add this as a
job to lcitool which controls how virt-install is spawned, that way + an Ansible
playbook you always get a reproducible environment?

Erik

Re: [PATCH 00/40] convert virObjects to GObject

[Daniel P . Berrangé]

On Fri, May 15, 2020 at 04:13:21PM +0100, Daniel P. Berrangé wrote:
> On Fri, May 15, 2020 at 02:34:01PM +0200, Michal Privoznik wrote:
> > On 5/13/20 1:56 PM, Rafael Fonseca wrote:
> > > This patch series convert various simple instances of virObject to a
> > > GObject equivalent.
> > 
> > I'm sorry upfront for misusing your patchset and I'm also sorry for bringing
> > this up again.
> > 
> > I think we need to step back and re-evaluate whether this is worth it.
> > GObject is horrible and frightening part of GLib. Not only one has to define
> > empty functions, but we can't mix virObject and GObject. For instance:
> > virObjectRef() called over GObject will corrupt memory.
> > 
> > Worse, there is no way to check whether your patches converted everything
> > (and clearly they did not because I see couple of tests crashing; or maybe
> > they did at the time of send but now the code has changed - anyway, point
> > proven).
> > 
> > I started reviewing and stopped at the first patch realizing, I have no idea
> > whether you converted every virObjectRef()/virObjectUnref() with
> > corresponding glib call.
> > 
> > I also wanted to write a cocci spatch that might at least identify places
> > where that is happening, but apparently my spatch skills are poor.
> > 
> > Does anybody have an idea how to verify these patches?
> 
> My preferred option was to make virObject be a subclass of GObject.
> 
> I tried this initially but hit a key problem - g_object_unref is void
> and virObjectUnref returns bool - true if any refs still exist. We
> rely on that for the virConnectPtr and virFDStream objects.
> 
> I couldn't come up with a way to solve that at the time, but I've
> just had another think and believe we can solve it using a thread
> local set by the finalize. So I'll have another go at doing this
> inheritance.

My conversion  to make virObject a subclass of GObject has now merged,
which eliminates the danger that Michael was concerned about.

So we should be able to move forward with Rafael's patches now.

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

Re: [PATCH] qemucapabilitiestest: Bump qemu-5.1 capabilities for x86_64

[Erik Skultety]

On Wed, Jun 03, 2020 at 11:11:08AM +0200, Michal Privoznik wrote:
> On 6/3/20 10:40 AM, Peter Krempa wrote:
> > On Wed, Jun 03, 2020 at 10:27:57 +0200, Michal Privoznik wrote:
> > > On 6/3/20 9:31 AM, Peter Krempa wrote:
> > > > QEMU added the machine types for the 5.1 release so let's update them.
> > > >
> > > > Other notable changes are 'cpu-throttle-tailslow' migration property,
> > > > 'zlib' compression for qcow2 images and absrtact socket support.
> > > >
> > > > Signed-off-by: Peter Krempa 
> > > > ---
> > > > As usual, I'll be refreshing this until the release so that we always
> > > > have fresh capabilities to prevent any surprises with deprecation and
> > > > big updates.
> > > >
> > > >.../domaincapsdata/qemu_5.1.0-q35.x86_64.xml  |   2 +-
> > > >.../domaincapsdata/qemu_5.1.0-tcg.x86_64.xml  |   2 +-
> > > >tests/domaincapsdata/qemu_5.1.0.x86_64.xml|   2 +-
> > > >.../caps_5.1.0.x86_64.replies | 357 
> > > > +++---
> > > >.../caps_5.1.0.x86_64.xml |  14 +-
> > > >5 files changed, 237 insertions(+), 140 deletions(-)
> > >
> > > Reviewed-by: Michal Privoznik 
> > >
> > > Maybe we can have another rule that would allow you to push these without
> > > review? I can argue both ways, so I'm just putting it out there.
> >
> > Yeah. I thought about that too.
> >
> > Specifically one thing I'd like to avoid is carelessness in case of the
> > update. Specifically if there is some form of removal (flag being
> > removed and such) we need to be careful and consider the implications.
>
> Well, for that we would need to compare with older capabilities XML and I
> don't think we are doing that. Removal between the same capabilities XML of
> an unreleased QEMU are uncommon. But I hear what you're saying and that's my
> concern too.
>
> >
> > In this very specific case there's nothing of note and I'd be okay with
> > just pushing it, but the rules if we wanted to codify it somehow would
> > require to be more nuanced and I don't think I can express all the
> > caveats.
> >
> > That's why I didn't really argue for adding a special rule for this.
> >
> > Also one reason I'm doing periodic upgrades of this is so that others
> > don't have to do it. The problem here is that the output is very much
> > dependent on the machine where you run it and I don't want others to
> > have to update the files when adding a new capability as the difference
> > becomes unreviewable and may even regress depending on how qemu is
> > built.
> >
>
> This is a long known issue and perhaps it would be worth documenting
> somewhere? I think these are QEMU binaries taken from Fedora, is that so?
> Maybe we can document configure arguments for QEMU so that it is
> reproducible.

Not only that, we could set up an upstream fedora VM following those steps
(not just steps but the overall HW setup to unify the whole process) and
generating the capabilities whenever new qemu is tagged in git. Of course,
sometimes you need them earlier than an RC is tagged, but still, I think it
would be beneficial to automate this process by setting up the agent in a way
it would send a patch/MR against libvirt.
If it turns out this to be desirable from upstream libvirt POV, I can set up
such an upstream runner.

Erik

Re: [PATCH] qemucapabilitiestest: Bump qemu-5.1 capabilities for x86_64

[Peter Krempa]

On Wed, Jun 03, 2020 at 11:11:08 +0200, Michal Privoznik wrote:
> On 6/3/20 10:40 AM, Peter Krempa wrote:
> > On Wed, Jun 03, 2020 at 10:27:57 +0200, Michal Privoznik wrote:
> > > On 6/3/20 9:31 AM, Peter Krempa wrote:
> > > > QEMU added the machine types for the 5.1 release so let's update them.
> > > > 
> > > > Other notable changes are 'cpu-throttle-tailslow' migration property,
> > > > 'zlib' compression for qcow2 images and absrtact socket support.
> > > > 
> > > > Signed-off-by: Peter Krempa 
> > > > ---
> > > > As usual, I'll be refreshing this until the release so that we always
> > > > have fresh capabilities to prevent any surprises with deprecation and
> > > > big updates.
> > > > 
> > > >.../domaincapsdata/qemu_5.1.0-q35.x86_64.xml  |   2 +-
> > > >.../domaincapsdata/qemu_5.1.0-tcg.x86_64.xml  |   2 +-
> > > >tests/domaincapsdata/qemu_5.1.0.x86_64.xml|   2 +-
> > > >.../caps_5.1.0.x86_64.replies | 357 
> > > > +++---
> > > >.../caps_5.1.0.x86_64.xml |  14 +-
> > > >5 files changed, 237 insertions(+), 140 deletions(-)
> > > 
> > > Reviewed-by: Michal Privoznik 
> > > 
> > > Maybe we can have another rule that would allow you to push these without
> > > review? I can argue both ways, so I'm just putting it out there.
> > 
> > Yeah. I thought about that too.
> > 
> > Specifically one thing I'd like to avoid is carelessness in case of the
> > update. Specifically if there is some form of removal (flag being
> > removed and such) we need to be careful and consider the implications.
> 
> Well, for that we would need to compare with older capabilities XML and I
> don't think we are doing that. Removal between the same capabilities XML of

I certainly am doing that when generating files after the release to
ensure that we don't regress in anything. Obviously only on the level of
detected capabilities.

> an unreleased QEMU are uncommon. But I hear what you're saying and that's my
> concern too.
> 
> > 
> > In this very specific case there's nothing of note and I'd be okay with
> > just pushing it, but the rules if we wanted to codify it somehow would
> > require to be more nuanced and I don't think I can express all the
> > caveats.
> > 
> > That's why I didn't really argue for adding a special rule for this.
> > 
> > Also one reason I'm doing periodic upgrades of this is so that others
> > don't have to do it. The problem here is that the output is very much
> > dependent on the machine where you run it and I don't want others to
> > have to update the files when adding a new capability as the difference
> > becomes unreviewable and may even regress depending on how qemu is
> > built.
> > 
> 
> This is a long known issue and perhaps it would be worth documenting
> somewhere? I think these are QEMU binaries taken from Fedora, is that so?

Well. They are built on fedora, but certainly not taken from fedora.
It's built from git.

> Maybe we can document configure arguments for QEMU so that it is
> reproducible.

While I agree that we should do that to take one of the moving parts out
of the equation we still can't do anything about the machine dependance
of the output. Specifically it contains all cpu flags so it really can't
be re-generated reproducibly on a box with even a slightly different
cpu.

Ideally we'd build it with the fedora spec-file, but I got lazy and I'm
usually just re-running configure from my history in this case.

My only idea how to provide stable output is to create a job on a box
which will periodically re-build qemu and fetch the capabilities and
publish them somewhere so that others could grab those and refresh the
caps themselves, but I can't really think of a way how to enable others
do it on their machine whithout messing up the CPU.

Re: [PATCH] qemucapabilitiestest: Bump qemu-5.1 capabilities for x86_64

[Michal Privoznik]

On 6/3/20 10:40 AM, Peter Krempa wrote:

On Wed, Jun 03, 2020 at 10:27:57 +0200, Michal Privoznik wrote:

On 6/3/20 9:31 AM, Peter Krempa wrote:

QEMU added the machine types for the 5.1 release so let's update them.

Other notable changes are 'cpu-throttle-tailslow' migration property,
'zlib' compression for qcow2 images and absrtact socket support.

Signed-off-by: Peter Krempa 
---
As usual, I'll be refreshing this until the release so that we always
have fresh capabilities to prevent any surprises with deprecation and
big updates.

   .../domaincapsdata/qemu_5.1.0-q35.x86_64.xml  |   2 +-
   .../domaincapsdata/qemu_5.1.0-tcg.x86_64.xml  |   2 +-
   tests/domaincapsdata/qemu_5.1.0.x86_64.xml|   2 +-
   .../caps_5.1.0.x86_64.replies | 357 +++---
   .../caps_5.1.0.x86_64.xml |  14 +-
   5 files changed, 237 insertions(+), 140 deletions(-)


Reviewed-by: Michal Privoznik 

Maybe we can have another rule that would allow you to push these without
review? I can argue both ways, so I'm just putting it out there.


Yeah. I thought about that too.

Specifically one thing I'd like to avoid is carelessness in case of the
update. Specifically if there is some form of removal (flag being
removed and such) we need to be careful and consider the implications.


Well, for that we would need to compare with older capabilities XML and 
I don't think we are doing that. Removal between the same capabilities 
XML of an unreleased QEMU are uncommon. But I hear what you're saying 
and that's my concern too.




In this very specific case there's nothing of note and I'd be okay with
just pushing it, but the rules if we wanted to codify it somehow would
require to be more nuanced and I don't think I can express all the
caveats.

That's why I didn't really argue for adding a special rule for this.

Also one reason I'm doing periodic upgrades of this is so that others
don't have to do it. The problem here is that the output is very much
dependent on the machine where you run it and I don't want others to
have to update the files when adding a new capability as the difference
becomes unreviewable and may even regress depending on how qemu is
built.



This is a long known issue and perhaps it would be worth documenting 
somewhere? I think these are QEMU binaries taken from Fedora, is that 
so? Maybe we can document configure arguments for QEMU so that it is 
reproducible.


Michal

Re: [PATCH] qemucapabilitiestest: Bump qemu-5.1 capabilities for x86_64

[Peter Krempa]

On Wed, Jun 03, 2020 at 10:27:57 +0200, Michal Privoznik wrote:
> On 6/3/20 9:31 AM, Peter Krempa wrote:
> > QEMU added the machine types for the 5.1 release so let's update them.
> > 
> > Other notable changes are 'cpu-throttle-tailslow' migration property,
> > 'zlib' compression for qcow2 images and absrtact socket support.
> > 
> > Signed-off-by: Peter Krempa 
> > ---
> > As usual, I'll be refreshing this until the release so that we always
> > have fresh capabilities to prevent any surprises with deprecation and
> > big updates.
> > 
> >   .../domaincapsdata/qemu_5.1.0-q35.x86_64.xml  |   2 +-
> >   .../domaincapsdata/qemu_5.1.0-tcg.x86_64.xml  |   2 +-
> >   tests/domaincapsdata/qemu_5.1.0.x86_64.xml|   2 +-
> >   .../caps_5.1.0.x86_64.replies | 357 +++---
> >   .../caps_5.1.0.x86_64.xml |  14 +-
> >   5 files changed, 237 insertions(+), 140 deletions(-)
> 
> Reviewed-by: Michal Privoznik 
> 
> Maybe we can have another rule that would allow you to push these without
> review? I can argue both ways, so I'm just putting it out there.

Yeah. I thought about that too.

Specifically one thing I'd like to avoid is carelessness in case of the
update. Specifically if there is some form of removal (flag being
removed and such) we need to be careful and consider the implications.

In this very specific case there's nothing of note and I'd be okay with
just pushing it, but the rules if we wanted to codify it somehow would
require to be more nuanced and I don't think I can express all the
caveats.

That's why I didn't really argue for adding a special rule for this.

Also one reason I'm doing periodic upgrades of this is so that others
don't have to do it. The problem here is that the output is very much
dependent on the machine where you run it and I don't want others to
have to update the files when adding a new capability as the difference
becomes unreviewable and may even regress depending on how qemu is
built.

Re: [PATCH] qemucapabilitiestest: Bump qemu-5.1 capabilities for x86_64

[Michal Privoznik]

On 6/3/20 9:31 AM, Peter Krempa wrote:

QEMU added the machine types for the 5.1 release so let's update them.

Other notable changes are 'cpu-throttle-tailslow' migration property,
'zlib' compression for qcow2 images and absrtact socket support.

Signed-off-by: Peter Krempa 
---
As usual, I'll be refreshing this until the release so that we always
have fresh capabilities to prevent any surprises with deprecation and
big updates.

  .../domaincapsdata/qemu_5.1.0-q35.x86_64.xml  |   2 +-
  .../domaincapsdata/qemu_5.1.0-tcg.x86_64.xml  |   2 +-
  tests/domaincapsdata/qemu_5.1.0.x86_64.xml|   2 +-
  .../caps_5.1.0.x86_64.replies | 357 +++---
  .../caps_5.1.0.x86_64.xml |  14 +-
  5 files changed, 237 insertions(+), 140 deletions(-)


Reviewed-by: Michal Privoznik 

Maybe we can have another rule that would allow you to push these 
without review? I can argue both ways, so I'm just putting it out there.


Michal

[PATCH 2/2] Use more of VIR_XPATH_NODE_AUTORESTORE

[Michal Privoznik]

This is convenience macro, use it more. This commit was generated
using the following spatch:

  @@
  symbol node;
  identifier old;
  identifier ctxt;
  type xmlNodePtr;
  @@
  - xmlNodePtr old;
  + VIR_XPATH_NODE_AUTORESTORE(ctxt);
...
  - old = ctxt->node;
... when != old
  - ctxt->node = old;

  @@
  symbol node;
  identifier old;
  identifier ctxt;
  type xmlNodePtr;
  @@
  - xmlNodePtr old = ctxt->node;
  + VIR_XPATH_NODE_AUTORESTORE(ctxt);
... when != old
  - ctxt->node = old;

Signed-off-by: Michal Privoznik 
---
 src/conf/cpu_conf.c |  3 +-
 src/conf/domain_conf.c  |  7 +--
 src/conf/interface_conf.c   | 16 ++-
 src/conf/netdev_vlan_conf.c |  3 +-
 src/conf/network_conf.c | 25 +++---
 src/conf/networkcommon_conf.c   |  4 +-
 src/conf/node_device_conf.c | 84 +
 src/conf/numa_conf.c|  3 +-
 src/conf/snapshot_conf.c|  3 +-
 src/conf/storage_adapter_conf.c |  3 +-
 src/conf/storage_conf.c |  4 +-
 src/conf/virsavecookie.c|  3 +-
 src/cpu/cpu_map.c   | 12 +
 src/cpu/cpu_x86.c   |  3 +-
 src/lxc/lxc_domain.c|  4 +-
 src/util/virstorageencryption.c |  8 +---
 src/util/virstoragefile.c   |  3 +-
 tools/virsh-domain.c|  6 +--
 18 files changed, 52 insertions(+), 142 deletions(-)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index c6d36e0cb5..1d02e23175 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -326,7 +326,7 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
 {
 virCPUDefPtr def = NULL;
 xmlNodePtr *nodes = NULL;
-xmlNodePtr oldnode = ctxt->node;
+VIR_XPATH_NODE_AUTORESTORE(ctxt);
 int n;
 size_t i;
 char *cpuMode;
@@ -662,7 +662,6 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
 ret = 0;
 
  cleanup:
-ctxt->node = oldnode;
 VIR_FREE(fallback);
 VIR_FREE(vendor_id);
 VIR_FREE(nodes);
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1406cf079e..1cdc7971fc 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13265,16 +13265,14 @@ 
virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
 
 /* Check for an optional seclabel override in . */
 if (chr_def) {
-xmlNodePtr saved_node = ctxt->node;
+VIR_XPATH_NODE_AUTORESTORE(ctxt);
 ctxt->node = cur;
 if (virSecurityDeviceLabelDefParseXML(>seclabels,
   >nseclabels,
   ctxt,
   flags) < 0) {
-ctxt->node = saved_node;
 goto error;
 }
-ctxt->node = saved_node;
 }
 } else if (virXMLNodeNameEqual(cur, "log")) {
 if (logParsed) {
@@ -22181,11 +22179,10 @@ virDomainDefParseXML(xmlDocPtr xml,
 }
 
 if ((node = virXPathNode("./sysinfo[1]", ctxt)) != NULL) {
-xmlNodePtr oldnode = ctxt->node;
+VIR_XPATH_NODE_AUTORESTORE(ctxt);
 ctxt->node = node;
 def->sysinfo = virSysinfoParseXML(node, ctxt,
   def->uuid, uuid_generated);
-ctxt->node = oldnode;
 
 if (def->sysinfo == NULL)
 goto error;
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index d1732621b5..be5e1133a2 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -262,12 +262,11 @@ static int
 virInterfaceDefParseDhcp(virInterfaceProtocolDefPtr def,
  xmlNodePtr dhcp, xmlXPathContextPtr ctxt)
 {
-xmlNodePtr save;
+VIR_XPATH_NODE_AUTORESTORE(ctxt);
 char *tmp;
 int ret = 0;
 
 def->dhcp = 1;
-save = ctxt->node;
 ctxt->node = dhcp;
 def->peerdns = -1;
 /* Not much to do in the current version */
@@ -284,7 +283,6 @@ virInterfaceDefParseDhcp(virInterfaceProtocolDefPtr def,
 VIR_FREE(tmp);
 }
 
-ctxt->node = save;
 return ret;
 }
 
@@ -426,13 +424,11 @@ static int
 virInterfaceDefParseIfAdressing(virInterfaceDefPtr def,
 xmlXPathContextPtr ctxt)
 {
-xmlNodePtr save;
+VIR_XPATH_NODE_AUTORESTORE(ctxt);
 xmlNodePtr *protoNodes = NULL;
 int nProtoNodes, pp, ret = -1;
 char *tmp;
 
-save = ctxt->node;
-
 nProtoNodes = virXPathNodeSet("./protocol", ctxt, );
 if (nProtoNodes < 0)
 goto error;
@@ -487,7 +483,6 @@ virInterfaceDefParseIfAdressing(virInterfaceDefPtr def,
 
  error:
 VIR_FREE(protoNodes);
-ctxt->node = save;
 return ret;
 
 }
@@ -558,7 +553,7 @@ virInterfaceDefParseBondItfs(virInterfaceDefPtr def,
  xmlXPathContextPtr ctxt)
 {
 xmlNodePtr *interfaces = NULL;
-xmlNodePtr bond = ctxt->node;
+VIR_XPATH_NODE_AUTORESTORE(ctxt);
 virInterfaceDefPtr 

[PATCH 0/2] Use more of VIR_XPATH_NODE_AUTORESTORE

[Michal Privoznik]

I've been playing with cocci again.

Michal Prívozník (2):
  virxml: Don't overwrite ctxt->node
  Use more of VIR_XPATH_NODE_AUTORESTORE

 src/conf/cpu_conf.c |  3 +-
 src/conf/domain_conf.c  |  7 +--
 src/conf/interface_conf.c   | 16 ++-
 src/conf/netdev_vlan_conf.c |  3 +-
 src/conf/network_conf.c | 25 +++---
 src/conf/networkcommon_conf.c   |  4 +-
 src/conf/node_device_conf.c | 84 +
 src/conf/numa_conf.c|  3 +-
 src/conf/snapshot_conf.c|  3 +-
 src/conf/storage_adapter_conf.c |  3 +-
 src/conf/storage_conf.c |  4 +-
 src/conf/virsavecookie.c|  3 +-
 src/cpu/cpu_map.c   | 12 +
 src/cpu/cpu_x86.c   |  3 +-
 src/lxc/lxc_domain.c|  4 +-
 src/util/virstorageencryption.c |  8 +---
 src/util/virstoragefile.c   |  3 +-
 src/util/virxml.c   | 27 ---
 tools/virsh-domain.c|  6 +--
 19 files changed, 52 insertions(+), 169 deletions(-)

-- 
2.26.2

[PATCH 1/2] virxml: Don't overwrite ctxt->node

[Michal Privoznik]

This reverts b897973f2e0.

Even though it may have been the case in the past, relative
XPaths don't overwrite the ctxt->node. Thus, there's no need to
save it.

Signed-off-by: Michal Privoznik 
---
 src/util/virxml.c | 27 ---
 1 file changed, 27 deletions(-)

diff --git a/src/util/virxml.c b/src/util/virxml.c
index 9ea7b99dba..02b59ea2f8 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -74,7 +74,6 @@ virXPathString(const char *xpath,
xmlXPathContextPtr ctxt)
 {
 xmlXPathObjectPtr obj;
-xmlNodePtr relnode;
 char *ret;
 
 if ((ctxt == NULL) || (xpath == NULL)) {
@@ -82,9 +81,7 @@ virXPathString(const char *xpath,
"%s", _("Invalid parameter to virXPathString()"));
 return NULL;
 }
-relnode = ctxt->node;
 obj = xmlXPathEval(BAD_CAST xpath, ctxt);
-ctxt->node = relnode;
 if ((obj == NULL) || (obj->type != XPATH_STRING) ||
 (obj->stringval == NULL) || (obj->stringval[0] == 0)) {
 xmlXPathFreeObject(obj);
@@ -152,16 +149,13 @@ virXPathNumber(const char *xpath,
double *value)
 {
 xmlXPathObjectPtr obj;
-xmlNodePtr relnode;
 
 if ((ctxt == NULL) || (xpath == NULL) || (value == NULL)) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("Invalid parameter to virXPathNumber()"));
 return -1;
 }
-relnode = ctxt->node;
 obj = xmlXPathEval(BAD_CAST xpath, ctxt);
-ctxt->node = relnode;
 if ((obj == NULL) || (obj->type != XPATH_NUMBER) ||
 (isnan(obj->floatval))) {
 xmlXPathFreeObject(obj);
@@ -180,7 +174,6 @@ virXPathLongBase(const char *xpath,
  long *value)
 {
 xmlXPathObjectPtr obj;
-xmlNodePtr relnode;
 int ret = 0;
 
 if ((ctxt == NULL) || (xpath == NULL) || (value == NULL)) {
@@ -188,9 +181,7 @@ virXPathLongBase(const char *xpath,
"%s", _("Invalid parameter to virXPathLong()"));
 return -1;
 }
-relnode = ctxt->node;
 obj = xmlXPathEval(BAD_CAST xpath, ctxt);
-ctxt->node = relnode;
 if ((obj != NULL) && (obj->type == XPATH_STRING) &&
 (obj->stringval != NULL) && (obj->stringval[0] != 0)) {
 if (virStrToLong_l((char *) obj->stringval, NULL, base, value) < 0)
@@ -285,7 +276,6 @@ virXPathULongBase(const char *xpath,
   unsigned long *value)
 {
 xmlXPathObjectPtr obj;
-xmlNodePtr relnode;
 int ret = 0;
 
 if ((ctxt == NULL) || (xpath == NULL) || (value == NULL)) {
@@ -293,9 +283,7 @@ virXPathULongBase(const char *xpath,
"%s", _("Invalid parameter to virXPathULong()"));
 return -1;
 }
-relnode = ctxt->node;
 obj = xmlXPathEval(BAD_CAST xpath, ctxt);
-ctxt->node = relnode;
 if ((obj != NULL) && (obj->type == XPATH_STRING) &&
 (obj->stringval != NULL) && (obj->stringval[0] != 0)) {
 if (virStrToLong_ul((char *) obj->stringval, NULL, base, value) < 0)
@@ -401,7 +389,6 @@ virXPathULongLong(const char *xpath,
   unsigned long long *value)
 {
 xmlXPathObjectPtr obj;
-xmlNodePtr relnode;
 int ret = 0;
 
 if ((ctxt == NULL) || (xpath == NULL) || (value == NULL)) {
@@ -409,9 +396,7 @@ virXPathULongLong(const char *xpath,
"%s", _("Invalid parameter to virXPathULong()"));
 return -1;
 }
-relnode = ctxt->node;
 obj = xmlXPathEval(BAD_CAST xpath, ctxt);
-ctxt->node = relnode;
 if ((obj != NULL) && (obj->type == XPATH_STRING) &&
 (obj->stringval != NULL) && (obj->stringval[0] != 0)) {
 if (virStrToLong_ull((char *) obj->stringval, NULL, 10, value) < 0)
@@ -447,7 +432,6 @@ virXPathLongLong(const char *xpath,
  long long *value)
 {
 xmlXPathObjectPtr obj;
-xmlNodePtr relnode;
 int ret = 0;
 
 if ((ctxt == NULL) || (xpath == NULL) || (value == NULL)) {
@@ -455,9 +439,7 @@ virXPathLongLong(const char *xpath,
"%s", _("Invalid parameter to virXPathLongLong()"));
 return -1;
 }
-relnode = ctxt->node;
 obj = xmlXPathEval(BAD_CAST xpath, ctxt);
-ctxt->node = relnode;
 if ((obj != NULL) && (obj->type == XPATH_STRING) &&
 (obj->stringval != NULL) && (obj->stringval[0] != 0)) {
 if (virStrToLong_ll((char *) obj->stringval, NULL, 10, value) < 0)
@@ -573,7 +555,6 @@ virXPathBoolean(const char *xpath,
 xmlXPathContextPtr ctxt)
 {
 xmlXPathObjectPtr obj;
-xmlNodePtr relnode;
 int ret;
 
 if ((ctxt == NULL) || (xpath == NULL)) {
@@ -581,9 +562,7 @@ virXPathBoolean(const char *xpath,
"%s", _("Invalid parameter to virXPathBoolean()"));
 return -1;
 }
-relnode = ctxt->node;
 obj = xmlXPathEval(BAD_CAST xpath, ctxt);
-ctxt->node = relnode;
 if ((obj == NULL) || (obj->type != XPATH_BOOLEAN) ||
 (obj->boolval < 0) || (obj->boolval > 1)) {
  

[PATCH] qemucapabilitiestest: Bump qemu-5.1 capabilities for x86_64

[Peter Krempa]

QEMU added the machine types for the 5.1 release so let's update them.

Other notable changes are 'cpu-throttle-tailslow' migration property,
'zlib' compression for qcow2 images and absrtact socket support.

Signed-off-by: Peter Krempa 
---
As usual, I'll be refreshing this until the release so that we always
have fresh capabilities to prevent any surprises with deprecation and
big updates.

 .../domaincapsdata/qemu_5.1.0-q35.x86_64.xml  |   2 +-
 .../domaincapsdata/qemu_5.1.0-tcg.x86_64.xml  |   2 +-
 tests/domaincapsdata/qemu_5.1.0.x86_64.xml|   2 +-
 .../caps_5.1.0.x86_64.replies | 357 +++---
 .../caps_5.1.0.x86_64.xml |  14 +-
 5 files changed, 237 insertions(+), 140 deletions(-)

diff --git a/tests/domaincapsdata/qemu_5.1.0-q35.x86_64.xml 
b/tests/domaincapsdata/qemu_5.1.0-q35.x86_64.xml
index e152f7dec5..996461fb0f 100644
--- a/tests/domaincapsdata/qemu_5.1.0-q35.x86_64.xml
+++ b/tests/domaincapsdata/qemu_5.1.0-q35.x86_64.xml
@@ -1,7 +1,7 @@
 
   /usr/bin/qemu-system-x86_64
   kvm
-  pc-q35-5.0
+  pc-q35-5.1
   x86_64
   
   
diff --git a/tests/domaincapsdata/qemu_5.1.0-tcg.x86_64.xml 
b/tests/domaincapsdata/qemu_5.1.0-tcg.x86_64.xml
index a0eeed7c2d..e4801b2750 100644
--- a/tests/domaincapsdata/qemu_5.1.0-tcg.x86_64.xml
+++ b/tests/domaincapsdata/qemu_5.1.0-tcg.x86_64.xml
@@ -1,7 +1,7 @@
 
   /usr/bin/qemu-system-x86_64
   qemu
-  pc-i440fx-5.0
+  pc-i440fx-5.1
   x86_64
   
   
diff --git a/tests/domaincapsdata/qemu_5.1.0.x86_64.xml 
b/tests/domaincapsdata/qemu_5.1.0.x86_64.xml
index bc842730a0..e95fcec8bc 100644
--- a/tests/domaincapsdata/qemu_5.1.0.x86_64.xml
+++ b/tests/domaincapsdata/qemu_5.1.0.x86_64.xml
@@ -1,7 +1,7 @@
 
   /usr/bin/qemu-system-x86_64
   kvm
-  pc-i440fx-5.0
+  pc-i440fx-5.1
   x86_64
   
   
diff --git a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.replies 
b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.replies
index e26a74921f..c44cff7e50 100644
--- a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.replies
+++ b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.replies
@@ -21,7 +21,7 @@
   "minor": 0,
   "major": 5
 },
-"package": "v5.0.0-34-g648db19685"
+"package": "v5.0.0-870-g5cc7a54c2e"
   },
   "id": "libvirt-2"
 }
@@ -618,10 +618,6 @@

 {
   "return": [
-{
-  "name": "ich9-usb-uhci5",
-  "parent": "pci-uhci-usb"
-},
 {
   "name": "pcie-pci-bridge",
   "parent": "base-pci-bridge"
@@ -662,6 +658,10 @@
   "name": "Denverton-x86_64-cpu",
   "parent": "x86_64-cpu"
 },
+{
+  "name": "virtio-rng-device",
+  "parent": "virtio-device"
+},
 {
   "name": "filter-buffer",
   "parent": "netfilter"
@@ -671,8 +671,8 @@
   "parent": "usb-device"
 },
 {
-  "name": "pci-bridge",
-  "parent": "base-pci-bridge"
+  "name": "ich9-usb-uhci5",
+  "parent": "pci-uhci-usb"
 },
 {
   "name": "pci-ipmi-bt",
@@ -691,8 +691,8 @@
   "parent": "pci-vga"
 },
 {
-  "name": "virtio-rng-device",
-  "parent": "virtio-device"
+  "name": "pcm3680_pci",
+  "parent": "pci-device"
 },
 {
   "name": "Haswell-v1-x86_64-cpu",
@@ -703,8 +703,8 @@
   "parent": "pci-device"
 },
 {
-  "name": "core2duo-x86_64-cpu",
-  "parent": "x86_64-cpu"
+  "name": "pci-bridge",
+  "parent": "base-pci-bridge"
 },
 {
   "name": "kvm-pit",
@@ -723,8 +723,8 @@
   "parent": "virtio-device"
 },
 {
-  "name": "pcm3680_pci",
-  "parent": "pci-device"
+  "name": "core2duo-x86_64-cpu",
+  "parent": "x86_64-cpu"
 },
 {
   "name": "virtio-blk-pci-transitional",
@@ -1086,14 +1086,14 @@
   "name": "pxb-pcie",
   "parent": "pci-device"
 },
-{
-  "name": "Haswell-IBRS-x86_64-cpu",
-  "parent": "x86_64-cpu"
-},
 {
   "name": "virtio-scsi-device",
   "parent": "virtio-scsi-common"
 },
+{
+  "name": "Haswell-IBRS-x86_64-cpu",
+  "parent": "x86_64-cpu"
+},
 {
   "name": "input-barrier",
   "parent": "object"
@@ -1690,6 +1690,10 @@
   "name": "Dhyana-v1-x86_64-cpu",
   "parent": "x86_64-cpu"
 },
+{
+  "name": "pc-i440fx-5.1-machine",
+  "parent": "generic-pc-machine"
+},
 {
   "name": "sysbus-fdc",
   "parent": "base-sysbus-fdc"
@@ -1859,8 +1863,8 @@
   "parent": "megasas-base"
 },
 {
-  "name": "vmcoreinfo",
-  "parent": "device"
+  "name": "chardev-braille",
+  "parent": "chardev"
 },
 {
   "name": "virtio-iommu-pci",
@@ -1871,12 +1875,12 @@
   "parent": "x86_64-cpu"
 },
 {
-  "name": "tpci200",
-  "parent": "pci-device"
+  "name": "vmcoreinfo",
+  "parent": "device"
 },
 {
-  "name": "chardev-braille",
-  "parent": "chardev"
+  "name": "tpci200",
+  "parent": "pci-device"
 },
 {
   "name": "rocker",
@@ -1914,6 +1918,10 @@
   "name":