Re: [libvirt] Question about parallel migration connections

2020-01-09 Thread Peter Krempa
On Thu, Jan 09, 2020 at 23:04:19 +, Jim Fehlig wrote:
> Are they supported with tunneled migration? The feature seems limited to 
> native 
> migration, in which case I can send a patch prohibiting parallel migration 
> connections with the tunnel.

That is true. Libvirt's stream which is used for tunelling the
connection can carry only one connection and the APIs driving tunelled
migration allow only one stream in arguments.

It's the same reason why tunelled migration doesn't support migration of
non-shared storage using NBD.

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



[libvirt] Question about parallel migration connections

2020-01-09 Thread Jim Fehlig
Are they supported with tunneled migration? The feature seems limited to native 
migration, in which case I can send a patch prohibiting parallel migration 
connections with the tunnel.

Regards,
Jim

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



Re: [libvirt] [PATCH 3/3] qemu: backup: Implement support for backup disk bitmap name configuration

2020-01-09 Thread Daniel Henrique Barboza




On 1/9/20 3:31 PM, Peter Krempa wrote:

Use the user-configured name of the bitmap when merging the appropriate
bitmaps for an incremental backup so that the user can see it as
configured. Additionally expose the default bitmap name if nothing is
configured.

Signed-off-by: Peter Krempa 
---
  src/qemu/qemu_backup.c | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)



Reviewed-by: Daniel Henrique Barboza 

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



Re: [libvirt] [PATCH 2/3] qemu: backup: Implement support for backup disk export name configuration

2020-01-09 Thread Daniel Henrique Barboza




On 1/9/20 3:31 PM, Peter Krempa wrote:

Pass the exportname as configured when exporting the image via NBD and
fill it with the default if it's not configured.

Signed-off-by: Peter Krempa 
---
  src/qemu/qemu_backup.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)




Reviewed-by: Daniel Henrique Barboza 

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



Re: [libvirt] [PATCH 1/3] conf: backup: Allow configuration of names exported via NBD

2020-01-09 Thread Daniel Henrique Barboza




On 1/9/20 3:31 PM, Peter Krempa wrote:

If users wish to use different name for exported disks or bitmaps
the new fields allow to do so. Additionally they also document the
current settings.

Signed-off-by: Peter Krempa 
---
  docs/formatbackup.html.in |  9 +
  docs/schemas/domainbackup.rng |  8 
  src/conf/backup_conf.c| 11 +++
  src/conf/backup_conf.h|  2 ++
  tests/domainbackupxml2xmlin/backup-pull-seclabel.xml  |  2 +-
  tests/domainbackupxml2xmlout/backup-pull-seclabel.xml |  2 +-
  6 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/docs/formatbackup.html.in b/docs/formatbackup.html.in
index 1c690901c7..7d2c6f1599 100644
--- a/docs/formatbackup.html.in
+++ b/docs/formatbackup.html.in
@@ -85,6 +85,15 @@
Setting this attribute to yes(default) 
specifies
  that the disk should take part in the backup and using
  no excludes the disk from the backup.
+  exportname
+  Allows to modify the NBD export name for the given disk.
+By default equal to disk target.
+Valid only for pull mode backups. 
+  exportbitmap
+  Allows to modify the name of the bitmap describing dirty
+blocks for an incremental backup exported via NBD export name
+for the given disk.
+Valid only for pull mode backups. 
type
A mandatory attribute to describe the type of the
  disk, except when backup='no' is
diff --git a/docs/schemas/domainbackup.rng b/docs/schemas/domainbackup.rng
index c1e4d80302..395ea841f9 100644
--- a/docs/schemas/domainbackup.rng
+++ b/docs/schemas/domainbackup.rng
@@ -165,6 +165,14 @@
  

  
+
+  
+
+  
+  
+
+  
+
  

  
diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c
index aa11967d2a..a4b87baa55 100644
--- a/src/conf/backup_conf.c
+++ b/src/conf/backup_conf.c
@@ -71,6 +71,8 @@ virDomainBackupDefFree(virDomainBackupDefPtr def)
  virDomainBackupDiskDefPtr disk = def->disks + i;

  g_free(disk->name);
+g_free(disk->exportname);
+g_free(disk->exportbitmap);
  virObjectUnref(disk->store);
  }

@@ -124,6 +126,11 @@ virDomainBackupDiskDefParseXML(xmlNodePtr node,
  if (def->backup == VIR_TRISTATE_BOOL_NO)
  return 0;

+if (!push) {
+def->exportname = virXMLPropString(node, "exportname");
+def->exportbitmap = virXMLPropString(node, "exportbitmap");
+}
+
  if (internal) {
  if (!(state = virXMLPropString(node, "state")) ||
  (tmp = virDomainBackupDiskStateTypeFromString(state)) < 0) {
@@ -165,6 +172,7 @@ virDomainBackupDiskDefParseXML(xmlNodePtr node,
  storageSourceParseFlags, xmlopt) < 0)
  return -1;

+


Is this an unintended newline?


Reviewed-by: Daniel Henrique Barboza 




  if ((driver = virXPathString("string(./driver/@type)", ctxt))) {
  def->store->format = virStorageFileFormatTypeFromString(driver);
  if (def->store->format <= 0) {
@@ -333,6 +341,9 @@ virDomainBackupDiskDefFormat(virBufferPtr buf,
  if (disk->backup == VIR_TRISTATE_BOOL_YES) {
  virBufferAsprintf(, " type='%s'", 
virStorageTypeToString(disk->store->type));

+virBufferEscapeString(, " exportname='%s'", disk->exportname);
+virBufferEscapeString(, " exportbitmap='%s'", 
disk->exportbitmap);
+
  if (disk->store->format > 0)
  virBufferEscapeString(, "\n",

virStorageFileFormatTypeToString(disk->store->format));


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



[libvirt] [PATCH v1 6/8] qemu_domain.c: removing unneeded cleanup labels

2020-01-09 Thread Daniel Henrique Barboza
Previous patches deprecated some 'cleanup' labels. Let's remove
them.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_domain.c | 267 +++--
 1 file changed, 98 insertions(+), 169 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 6bf704a6f5..931554eeb6 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2034,42 +2034,41 @@ qemuDomainSecretPrepare(virQEMUDriverPtr driver,
 qemuDomainObjPrivatePtr priv = vm->privateData;
 g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 size_t i;
-int ret = -1;
 
 /* disk secrets are prepared when preparing disks */
 
 for (i = 0; i < vm->def->nhostdevs; i++) {
 if (qemuDomainSecretHostdevPrepare(priv,
vm->def->hostdevs[i]) < 0)
-goto cleanup;
+return -1;
 }
 
 for (i = 0; i < vm->def->nserials; i++) {
 if (qemuDomainSecretChardevPrepare(cfg, priv,
vm->def->serials[i]->info.alias,
vm->def->serials[i]->source) < 0)
-goto cleanup;
+return -1;
 }
 
 for (i = 0; i < vm->def->nparallels; i++) {
 if (qemuDomainSecretChardevPrepare(cfg, priv,
vm->def->parallels[i]->info.alias,
vm->def->parallels[i]->source) < 0)
-goto cleanup;
+return -1;
 }
 
 for (i = 0; i < vm->def->nchannels; i++) {
 if (qemuDomainSecretChardevPrepare(cfg, priv,
vm->def->channels[i]->info.alias,
vm->def->channels[i]->source) < 0)
-goto cleanup;
+return -1;
 }
 
 for (i = 0; i < vm->def->nconsoles; i++) {
 if (qemuDomainSecretChardevPrepare(cfg, priv,
vm->def->consoles[i]->info.alias,
vm->def->consoles[i]->source) < 0)
-goto cleanup;
+return -1;
 }
 
 for (i = 0; i < vm->def->nsmartcards; i++)
@@ -2078,32 +2077,29 @@ qemuDomainSecretPrepare(virQEMUDriverPtr driver,
 qemuDomainSecretChardevPrepare(cfg, priv,
vm->def->smartcards[i]->info.alias,

vm->def->smartcards[i]->data.passthru) < 0)
-goto cleanup;
+return -1;
 
 for (i = 0; i < vm->def->nrngs; i++) {
 if (vm->def->rngs[i]->backend == VIR_DOMAIN_RNG_BACKEND_EGD &&
 qemuDomainSecretChardevPrepare(cfg, priv,
vm->def->rngs[i]->info.alias,
vm->def->rngs[i]->source.chardev) < 
0)
-goto cleanup;
+return -1;
 }
 
 for (i = 0; i < vm->def->nredirdevs; i++) {
 if (qemuDomainSecretChardevPrepare(cfg, priv,
vm->def->redirdevs[i]->info.alias,
vm->def->redirdevs[i]->source) < 0)
-goto cleanup;
+return -1;
 }
 
 for (i = 0; i < vm->def->ngraphics; i++) {
 if (qemuDomainSecretGraphicsPrepare(cfg, priv, vm->def->graphics[i]) < 
0)
-goto cleanup;
+return -1;
 }
 
-ret = 0;
-
- cleanup:
-return ret;
+return 0;
 }
 
 
@@ -2131,10 +2127,9 @@ qemuDomainSetPrivatePaths(virQEMUDriverPtr driver,
 g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 qemuDomainObjPrivatePtr priv = vm->privateData;
 g_autofree char *domname = virDomainDefGetShortName(vm->def);
-int ret = -1;
 
 if (!domname)
-goto cleanup;
+return -1;
 
 if (!priv->libDir)
 priv->libDir = g_strdup_printf("%s/domain-%s", cfg->libDir, domname);
@@ -2143,9 +2138,7 @@ qemuDomainSetPrivatePaths(virQEMUDriverPtr driver,
 priv->channelTargetDir = g_strdup_printf("%s/domain-%s",
  cfg->channelTargetDir, 
domname);
 
-ret = 0;
- cleanup:
-return ret;
+return 0;
 }
 
 
@@ -2327,7 +2320,6 @@ qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt,
 qemuDomainStorageSourcePrivatePtr priv;
 g_autofree char *authalias = NULL;
 g_autofree char *encalias = NULL;
-int ret = -1;
 
 src->nodestorage = 
virXPathString("string(./nodenames/nodename[@type='storage']/@name)", ctxt);
 src->nodeformat = 
virXPathString("string(./nodenames/nodename[@type='format']/@name)", ctxt);
@@ -2342,25 +2334,21 @@ qemuStorageSourcePrivateDataParse(xmlXPathContextPtr 
ctxt,
 if (authalias || encalias) {
 if (!src->privateData &&
 !(src->privateData = qemuDomainStorageSourcePrivateNew()))
-goto cleanup;
+  

[libvirt] [PATCH v1 7/8] qemu_domain.c: turn qemuDomainChrDefDropDefaultPath to void

2020-01-09 Thread Daniel Henrique Barboza
qemuDomainChrDefDropDefaultPath() returns an int, but it's
always returning 0. Callers are checking for result < 0 to
run their cleanup code needlessly.

Turn the function to 'void' and adjust the callers.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_domain.c | 17 +
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 931554eeb6..153f5897bc 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8468,20 +8468,19 @@ qemuDomainDefaultNetModel(const virDomainDef *def,
  * This function clears the path for migration as well, so we need to clear
  * the path even if we are not storing it in the XML.
  */
-static int
+static void
 qemuDomainChrDefDropDefaultPath(virDomainChrDefPtr chr,
 virQEMUDriverPtr driver)
 {
 g_autoptr(virQEMUDriverConfig) cfg = NULL;
 virBuffer buf = VIR_BUFFER_INITIALIZER;
 g_autofree char *regexp = NULL;
-int ret = -1;
 
 if (chr->deviceType != VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL ||
 chr->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO ||
 chr->source->type != VIR_DOMAIN_CHR_TYPE_UNIX ||
 !chr->source->data.nix.path) {
-return 0;
+return;
 }
 
 cfg = virQEMUDriverGetConfig(driver);
@@ -8494,9 +8493,6 @@ qemuDomainChrDefDropDefaultPath(virDomainChrDefPtr chr,
 
 if (virStringMatch(chr->source->data.nix.path, regexp))
 VIR_FREE(chr->source->data.nix.path);
-
-ret = 0;
-return ret;
 }
 
 
@@ -8758,8 +8754,7 @@ qemuDomainChrDefPostParse(virDomainChrDefPtr chr,
 
 /* clear auto generated unix socket path for inactive definitions */
 if (parseFlags & VIR_DOMAIN_DEF_PARSE_INACTIVE) {
-if (qemuDomainChrDefDropDefaultPath(chr, driver) < 0)
-return -1;
+qemuDomainChrDefDropDefaultPath(chr, driver);
 
 /* For UNIX chardev if no path is provided we generate one.
  * This also implies that the mode is 'bind'. */
@@ -10118,10 +10113,8 @@ qemuDomainDefFormatBufInternal(virQEMUDriverPtr driver,
 }
 }
 
-for (i = 0; i < def->nchannels; i++) {
-if (qemuDomainChrDefDropDefaultPath(def->channels[i], driver) < 0)
-goto cleanup;
-}
+for (i = 0; i < def->nchannels; i++)
+qemuDomainChrDefDropDefaultPath(def->channels[i], driver);
 
 for (i = 0; i < def->nserials; i++) {
 virDomainChrDefPtr serial = def->serials[i];
-- 
2.24.1


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



[libvirt] [PATCH v1 2/8] conf/domain_addr.c: remove unneeded 'cleanup' labels

2020-01-09 Thread Daniel Henrique Barboza
Previous patch used 'g_autofree' to eliminate instances of
VIR_FREE(), making some cleanup labels obsolete. This
patch removes them.

Signed-off-by: Daniel Henrique Barboza 
---
 src/conf/domain_addr.c | 98 +++---
 1 file changed, 34 insertions(+), 64 deletions(-)

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index ee4dcf1d3b..0dd4b15225 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -829,30 +829,29 @@ 
virDomainPCIAddressReserveAddrInternal(virDomainPCIAddressSetPtr addrs,
unsigned int isolationGroup,
bool fromConfig)
 {
-int ret = -1;
 g_autofree char *addrStr = NULL;
 virDomainPCIAddressBusPtr bus;
 virErrorNumber errType = (fromConfig
   ? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR);
 
 if (!(addrStr = virPCIDeviceAddressAsString(addr)))
-goto cleanup;
+return -1;
 
 /* Add an extra bus if necessary */
 if (addrs->dryRun && virDomainPCIAddressSetGrow(addrs, addr, flags) < 0)
-goto cleanup;
+return -1;
 /* Check that the requested bus exists, is the correct type, and we
  * are asking for a valid slot
  */
 if (!virDomainPCIAddressValidate(addrs, addr, addrStr, flags, fromConfig))
-goto cleanup;
+return -1;
 
 bus = >buses[addr->bus];
 
 if (bus->slot[addr->slot].functions & (1 << addr->function)) {
 virReportError(errType,
_("Attempted double use of PCI Address %s"), addrStr);
-goto cleanup;
+return -1;
 }
 
 /* if this is the first function to be reserved on this slot, and
@@ -889,9 +888,7 @@ 
virDomainPCIAddressReserveAddrInternal(virDomainPCIAddressSetPtr addrs,
 VIR_DEBUG("Reserving PCI address %s (aggregate='%s')", addrStr,
   bus->slot[addr->slot].aggregate ? "true" : "false");
 
-ret = 0;
- cleanup:
-return ret;
+return 0;
 }
 
 
@@ -910,7 +907,6 @@ virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr 
addrs,
   virDomainDeviceInfoPtr dev,
   virDomainPCIConnectFlags flags)
 {
-int ret = -1;
 g_autofree char *addrStr = NULL;
 
 /* if flags is 0, the particular model of this device on this
@@ -920,7 +916,7 @@ virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr 
addrs,
return 0;
 
 if (!(addrStr = virPCIDeviceAddressAsString(>addr.pci)))
-goto cleanup;
+return -1;
 
 if (virDeviceInfoPCIAddressIsPresent(dev)) {
 /* We do not support hotplug multi-function PCI device now, so we 
should
@@ -930,31 +926,28 @@ virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr 
addrs,
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Only PCI device addresses with function=0"
  " are supported"));
-goto cleanup;
+return -1;
 }
 
 if (!virDomainPCIAddressValidate(addrs, >addr.pci,
  addrStr, flags, true))
-goto cleanup;
+return -1;
 
 if (virDomainPCIAddressReserveAddrInternal(addrs, >addr.pci,
flags, dev->isolationGroup,
true) < 0) {
-goto cleanup;
+return -1;
 }
 } else {
 if (virDomainPCIAddressReserveNextAddr(addrs, dev, flags, -1) < 0)
-goto cleanup;
+return -1;
 }
 
 dev->addr.pci.extFlags = dev->pciAddrExtFlags;
 if (virDomainPCIAddressExtensionEnsureAddr(addrs, >addr.pci) < 0)
-goto cleanup;
-
-ret = 0;
+return -1;
 
- cleanup:
-return ret;
+return 0;
 }
 
 
@@ -1102,13 +1095,12 @@ 
virDomainPCIAddressFindUnusedFunctionOnBus(virDomainPCIAddressBusPtr bus,
virDomainPCIConnectFlags flags,
bool *found)
 {
-int ret = -1;
 g_autofree char *addrStr = NULL;
 
 *found = false;
 
 if (!(addrStr = virPCIDeviceAddressAsString(searchAddr)))
-goto cleanup;
+return -1;
 
 if (!virDomainPCIAddressFlagsCompatible(searchAddr, addrStr, bus->flags,
 flags, false, false)) {
@@ -1154,10 +1146,7 @@ 
virDomainPCIAddressFindUnusedFunctionOnBus(virDomainPCIAddressBusPtr bus,
 }
 }
 
-ret = 0;
-
- cleanup:
-return ret;
+return 0;
 }
 
 
@@ -1410,7 +1399,6 @@ virDomainCCWAddressAssign(virDomainDeviceInfoPtr dev,
   virDomainCCWAddressSetPtr addrs,
   bool autoassign)
 {
-int ret = -1;
 g_autofree char *addr = NULL;
 
 if (dev->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)
@@ -1418,27 +1406,27 @@ 

[libvirt] [PATCH v1 8/8] qemu_domain_address.c: turn qemuDomainFillDeviceIsolationGroup to void

2020-01-09 Thread Daniel Henrique Barboza
Starting on commit 1f43393283ff, qemuDomainFillDeviceIsolationGroup()
returns 0 in all circunstances. Let's turn it to 'void' make it
clearer that the function will not fail. This also spares a
check for < 0 return in qemu_hotplug.c. The
qemuDomainFillDeviceIsolationGroupIter() callback now returns
0 at all times - which is already happening anyway.

Refer to 1f43393283ff commit message for more details on why
the function was changed to never return an error.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_domain_address.c | 20 ++--
 src/qemu/qemu_domain_address.h |  2 +-
 src/qemu/qemu_hotplug.c|  6 ++
 3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 9e3bcc434d..4fb9db1e14 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -1252,7 +1252,7 @@ qemuDomainFindUnusedIsolationGroup(virDomainDefPtr def)
  *
  * Return: 0 on success, <0 on failure
  * */
-int
+void
 qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
virDomainDeviceDefPtr dev)
 {
@@ -1270,7 +1270,7 @@ qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
 /* Only PCI host devices are subject to isolation */
 if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
 hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) 
{
-return 0;
+return;
 }
 
 hostAddr = >source.subsys.u.pci.addr;
@@ -1278,7 +1278,7 @@ qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
 /* If a non-default isolation has already been assigned to the
  * device, we can avoid looking up the information again */
 if (info->isolationGroup > 0)
-return 0;
+return;
 
 /* The isolation group depends on the IOMMU group assigned by the host 
*/
 tmp = virPCIDeviceAddressGetIOMMUGroupNum(hostAddr);
@@ -1288,7 +1288,7 @@ qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
  "%04x:%02x:%02x.%x, device won't be isolated",
  hostAddr->domain, hostAddr->bus,
  hostAddr->slot, hostAddr->function);
-return 0;
+return;
 }
 
 /* The isolation group for a host device is its IOMMU group,
@@ -1314,13 +1314,13 @@ qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
  * require us to isolate the guest device, so we can skip them */
 if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK ||
 virDomainNetResolveActualType(iface) != 
VIR_DOMAIN_NET_TYPE_HOSTDEV) {
-return 0;
+return;
 }
 
 /* If a non-default isolation has already been assigned to the
  * device, we can avoid looking up the information again */
 if (info->isolationGroup > 0)
-return 0;
+return;
 
 /* Obtain a synthetic isolation group for the device, since at this
  * point in time we don't have access to the IOMMU group of the host
@@ -1332,7 +1332,7 @@ qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
  "configured to use hostdev-backed network '%s', "
  "device won't be isolated",
  iface->data.network.name);
-return 0;
+return;
 }
 
 info->isolationGroup = tmp;
@@ -1341,8 +1341,6 @@ qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
   "hostdev-backed network '%s' is %u",
   iface->data.network.name, info->isolationGroup);
 }
-
-return 0;
 }
 
 
@@ -1364,7 +1362,9 @@ qemuDomainFillDeviceIsolationGroupIter(virDomainDefPtr 
def,
virDomainDeviceInfoPtr info 
G_GNUC_UNUSED,
void *opaque G_GNUC_UNUSED)
 {
-return qemuDomainFillDeviceIsolationGroup(def, dev);
+qemuDomainFillDeviceIsolationGroup(def, dev);
+
+return 0;
 }
 
 
diff --git a/src/qemu/qemu_domain_address.h b/src/qemu/qemu_domain_address.h
index bf04e6bfdb..7ef3308246 100644
--- a/src/qemu/qemu_domain_address.h
+++ b/src/qemu/qemu_domain_address.h
@@ -49,7 +49,7 @@ int qemuDomainEnsurePCIAddress(virDomainObjPtr obj,
virQEMUDriverPtr driver)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
 
-int qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
+void qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def,
virDomainDeviceDefPtr dev)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 31d455505b..cb5d587940 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1598,11 +1598,9 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
 if (qemuAssignDeviceHostdevAlias(vm->def, >alias, -1) < 0)
 

[libvirt] [PATCH v1 0/8] domain_addr and qemu_domain cleanups

2020-01-09 Thread Daniel Henrique Barboza
Hi,

These are a few cleanups in some files that I'll end up
messing with in a future series (which isn't Glibfied ATM).
Figured it's best to clean them up now instead of adding
more non-Glibfied code on top.

Patches 7 and 8 are stuff that I noted some time ago and
figured it might be worth sending as well.


Daniel Henrique Barboza (8):
  conf/domain_addr.c: use g_autofree in strings
  conf/domain_addr.c: remove unneeded 'cleanup' labels
  qemu_domain.c: remove redundant virObjectUnref()
  qemu_domain.c: use g_autofree when possible
  qemu_domain.c: use g_autoptr when possible
  qemu_domain.c: removing unneeded cleanup labels
  qemu_domain.c: turn qemuDomainChrDefDropDefaultPath to void
  qemu_domain_address.c: turn qemuDomainFillDeviceIsolationGroup to void

 src/conf/domain_addr.c | 124 +++-
 src/qemu/qemu_domain.c | 511 +++--
 src/qemu/qemu_domain_address.c |  20 +-
 src/qemu/qemu_domain_address.h |   2 +-
 src/qemu/qemu_hotplug.c|   6 +-
 5 files changed, 227 insertions(+), 436 deletions(-)

-- 
2.24.1


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



[libvirt] [PATCH v1 3/8] qemu_domain.c: remove redundant virObjectUnref()

2020-01-09 Thread Daniel Henrique Barboza
The 'caps' variable in qemuDomainObjPrivateXMLParseAutomaticPlacement()
is set to auto clean via g_autoptr(), but a 'virObjectUnref(caps)' is
being executed in the 'cleanup' label.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_domain.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 24e84a5966..999e0db49e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3106,7 +3106,6 @@ 
qemuDomainObjPrivateXMLParseAutomaticPlacement(xmlXPathContextPtr ctxt,
 ret = 0;
 
  cleanup:
-virObjectUnref(caps);
 VIR_FREE(nodeset);
 VIR_FREE(cpuset);
 
-- 
2.24.1


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



[libvirt] [PATCH v1 1/8] conf/domain_addr.c: use g_autofree in strings

2020-01-09 Thread Daniel Henrique Barboza
Use g_autofree in strings when possible to spare a VIR_FREE()
call. Unneeded 'cleanup' labels will be taken care of in the
next patch.

The 'str' string in virDomainVirtioSerialAddrReserve() was
never used by the logic, only being used in cleanup by
VIR_FREE(). Let's remove it.

Signed-off-by: Daniel Henrique Barboza 
---
 src/conf/domain_addr.c | 26 --
 1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index ef7ee80e6a..ee4dcf1d3b 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -830,7 +830,7 @@ 
virDomainPCIAddressReserveAddrInternal(virDomainPCIAddressSetPtr addrs,
bool fromConfig)
 {
 int ret = -1;
-char *addrStr = NULL;
+g_autofree char *addrStr = NULL;
 virDomainPCIAddressBusPtr bus;
 virErrorNumber errType = (fromConfig
   ? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR);
@@ -891,7 +891,6 @@ 
virDomainPCIAddressReserveAddrInternal(virDomainPCIAddressSetPtr addrs,
 
 ret = 0;
  cleanup:
-VIR_FREE(addrStr);
 return ret;
 }
 
@@ -912,7 +911,7 @@ virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr 
addrs,
   virDomainPCIConnectFlags flags)
 {
 int ret = -1;
-char *addrStr = NULL;
+g_autofree char *addrStr = NULL;
 
 /* if flags is 0, the particular model of this device on this
  * machinetype doesn't need a PCI address, so we're done.
@@ -955,7 +954,6 @@ virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr 
addrs,
 ret = 0;
 
  cleanup:
-VIR_FREE(addrStr);
 return ret;
 }
 
@@ -1105,7 +1103,7 @@ 
virDomainPCIAddressFindUnusedFunctionOnBus(virDomainPCIAddressBusPtr bus,
bool *found)
 {
 int ret = -1;
-char *addrStr = NULL;
+g_autofree char *addrStr = NULL;
 
 *found = false;
 
@@ -1159,7 +1157,6 @@ 
virDomainPCIAddressFindUnusedFunctionOnBus(virDomainPCIAddressBusPtr bus,
 ret = 0;
 
  cleanup:
-VIR_FREE(addrStr);
 return ret;
 }
 
@@ -1414,7 +1411,7 @@ virDomainCCWAddressAssign(virDomainDeviceInfoPtr dev,
   bool autoassign)
 {
 int ret = -1;
-char *addr = NULL;
+g_autofree char *addr = NULL;
 
 if (dev->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)
 return 0;
@@ -1457,7 +1454,6 @@ virDomainCCWAddressAssign(virDomainDeviceInfoPtr dev,
 ret = 0;
 
  cleanup:
-VIR_FREE(addr);
 return ret;
 }
 
@@ -1687,7 +1683,6 @@ virDomainVirtioSerialAddrReserve(virDomainDefPtr def 
G_GNUC_UNUSED,
  void *data)
 {
 virDomainVirtioSerialAddrSetPtr addrs = data;
-char *str = NULL;
 int ret = -1;
 virBitmapPtr map = NULL;
 bool b;
@@ -1729,7 +1724,6 @@ virDomainVirtioSerialAddrReserve(virDomainDefPtr def 
G_GNUC_UNUSED,
 ret = 0;
 
  cleanup:
-VIR_FREE(str);
 return ret;
 }
 
@@ -2208,7 +2202,7 @@ virDomainUSBAddressSetAddHub(virDomainUSBAddressSetPtr 
addrs,
 virDomainUSBAddressHubPtr targetHub = NULL, newHub = NULL;
 int ret = -1;
 int targetPort;
-char *portStr = NULL;
+g_autofree char *portStr = NULL;
 
 if (hub->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -2242,7 +2236,6 @@ virDomainUSBAddressSetAddHub(virDomainUSBAddressSetPtr 
addrs,
 ret = 0;
  cleanup:
 virDomainUSBAddressHubFree(newHub);
-VIR_FREE(portStr);
 return ret;
 }
 
@@ -2349,7 +2342,7 @@ 
virDomainUSBAddressAssignFromBus(virDomainUSBAddressSetPtr addrs,
 {
 unsigned int portpath[VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH] = { 0 };
 virDomainUSBAddressHubPtr hub = addrs->buses[bus];
-char *portStr = NULL;
+g_autofree char *portStr = NULL;
 int ret = -1;
 
 if (!hub)
@@ -2372,7 +2365,6 @@ 
virDomainUSBAddressAssignFromBus(virDomainUSBAddressSetPtr addrs,
 
 ret = 0;
  cleanup:
-VIR_FREE(portStr);
 return ret;
 }
 
@@ -2428,7 +2420,7 @@ virDomainUSBAddressReserve(virDomainDeviceInfoPtr info,
 {
 virDomainUSBAddressSetPtr addrs = data;
 virDomainUSBAddressHubPtr targetHub = NULL;
-char *portStr = NULL;
+g_autofree char *portStr = NULL;
 int ret = -1;
 int targetPort;
 
@@ -2459,7 +2451,6 @@ virDomainUSBAddressReserve(virDomainDeviceInfoPtr info,
 ret = 0;
 
  cleanup:
-VIR_FREE(portStr);
 return ret;
 }
 
@@ -2490,7 +2481,7 @@ virDomainUSBAddressRelease(virDomainUSBAddressSetPtr 
addrs,
virDomainDeviceInfoPtr info)
 {
 virDomainUSBAddressHubPtr targetHub = NULL;
-char *portStr = NULL;
+g_autofree char *portStr = NULL;
 int targetPort;
 int ret = -1;
 
@@ -2510,6 +2501,5 @@ virDomainUSBAddressRelease(virDomainUSBAddressSetPtr 
addrs,
 ret = 0;
 
  cleanup:
-VIR_FREE(portStr);
 return ret;
 }
-- 
2.24.1


--
libvir-list mailing list
libvir-list@redhat.com

[libvirt] [PATCH v1 5/8] qemu_domain.c: use g_autoptr when possible

2020-01-09 Thread Daniel Henrique Barboza
Avoid some of the virObjectUnref() calls by using g_autoptr.
Aside from the 'cleanup' label in qemuDomainSetFakeReboot(),
all other now deprecated cleanup labels will be removed in
the next patch.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_domain.c | 87 --
 1 file changed, 25 insertions(+), 62 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d76c6e0c0e..6bf704a6f5 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1497,10 +1497,9 @@ qemuDomainSecretPlainSetup(qemuDomainSecretInfoPtr 
secinfo,
const char *username,
virSecretLookupTypeDefPtr seclookupdef)
 {
-virConnectPtr conn;
+g_autoptr(virConnect) conn = virGetConnectSecret();
 int ret = -1;
 
-conn = virGetConnectSecret();
 if (!conn)
 return -1;
 
@@ -1511,7 +1510,6 @@ qemuDomainSecretPlainSetup(qemuDomainSecretInfoPtr 
secinfo,
>s.plain.secret,
>s.plain.secretlen);
 
-virObjectUnref(conn);
 return ret;
 }
 
@@ -1538,7 +1536,7 @@ qemuDomainSecretAESSetup(qemuDomainObjPrivatePtr priv,
  virSecretLookupTypeDefPtr seclookupdef,
  bool isLuks)
 {
-virConnectPtr conn;
+g_autoptr(virConnect) conn = virGetConnectSecret();
 int ret = -1;
 uint8_t *raw_iv = NULL;
 size_t ivlen = QEMU_DOMAIN_AES_IV_LEN;
@@ -1547,7 +1545,6 @@ qemuDomainSecretAESSetup(qemuDomainObjPrivatePtr priv,
 uint8_t *ciphertext = NULL;
 size_t ciphertextlen = 0;
 
-conn = virGetConnectSecret();
 if (!conn)
 return -1;
 
@@ -1591,7 +1588,6 @@ qemuDomainSecretAESSetup(qemuDomainObjPrivatePtr priv,
 VIR_DISPOSE_N(raw_iv, ivlen);
 VIR_DISPOSE_N(secret, secretlen);
 VIR_DISPOSE_N(ciphertext, ciphertextlen);
-virObjectUnref(conn);
 return ret;
 }
 
@@ -2036,7 +2032,7 @@ qemuDomainSecretPrepare(virQEMUDriverPtr driver,
 virDomainObjPtr vm)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 size_t i;
 int ret = -1;
 
@@ -2107,7 +2103,6 @@ qemuDomainSecretPrepare(virQEMUDriverPtr driver,
 ret = 0;
 
  cleanup:
-virObjectUnref(cfg);
 return ret;
 }
 
@@ -2118,7 +2113,7 @@ qemuDomainSetPrivatePathsOld(virQEMUDriverPtr driver,
  virDomainObjPtr vm)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 
 if (!priv->libDir)
 priv->libDir = g_strdup_printf("%s/domain-%s", cfg->libDir, 
vm->def->name);
@@ -2126,8 +2121,6 @@ qemuDomainSetPrivatePathsOld(virQEMUDriverPtr driver,
 if (!priv->channelTargetDir)
 priv->channelTargetDir = g_strdup_printf("%s/domain-%s",
  cfg->channelTargetDir, 
vm->def->name);
-
-virObjectUnref(cfg);
 }
 
 
@@ -2135,7 +2128,7 @@ int
 qemuDomainSetPrivatePaths(virQEMUDriverPtr driver,
   virDomainObjPtr vm)
 {
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 qemuDomainObjPrivatePtr priv = vm->privateData;
 g_autofree char *domname = virDomainDefGetShortName(vm->def);
 int ret = -1;
@@ -2152,7 +2145,6 @@ qemuDomainSetPrivatePaths(virQEMUDriverPtr driver,
 
 ret = 0;
  cleanup:
-virObjectUnref(cfg);
 return ret;
 }
 
@@ -3646,7 +3638,7 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
 size_t i;
 g_autofree xmlNodePtr *nodes = NULL;
 xmlNodePtr node = NULL;
-virQEMUCapsPtr qemuCaps = NULL;
+g_autoptr(virQEMUCaps) qemuCaps = NULL;
 
 if (!(priv->monConfig = virDomainChrSourceDefNew(NULL)))
 goto error;
@@ -3850,7 +3842,6 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
 priv->monConfig = NULL;
 virStringListFree(priv->qemuDevices);
 priv->qemuDevices = NULL;
-virObjectUnref(qemuCaps);
 return -1;
 }
 
@@ -5671,8 +5662,8 @@ qemuDomainDefValidate(const virDomainDef *def,
   void *opaque)
 {
 virQEMUDriverPtr driver = opaque;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
-virQEMUCapsPtr qemuCaps = NULL;
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virQEMUCaps) qemuCaps = NULL;
 int ret = -1;
 size_t i;
 
@@ -5878,8 +5869,6 @@ qemuDomainDefValidate(const virDomainDef *def,
 ret = 0;
 
  cleanup:
-virObjectUnref(qemuCaps);
-virObjectUnref(cfg);
 return ret;
 }
 
@@ -8526,7 +8515,7 @@ static int
 qemuDomainChrDefDropDefaultPath(virDomainChrDefPtr chr,

[libvirt] [PATCH v1 4/8] qemu_domain.c: use g_autofree when possible

2020-01-09 Thread Daniel Henrique Barboza
Use g_autofree to remove VIR_FREE() calls used for cleanups.
Labels that became deprecated will be removed in a later
patch.

In qemuDomainSetupDisk(), the 'dst' variable is not used at
all and could be removed.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_domain.c | 139 +
 1 file changed, 44 insertions(+), 95 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 999e0db49e..d76c6e0c0e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -877,7 +877,7 @@ int
 qemuDomainWriteMasterKeyFile(virQEMUDriverPtr driver,
  virDomainObjPtr vm)
 {
-char *path;
+g_autofree char *path = NULL;
 int fd = -1;
 int ret = -1;
 qemuDomainObjPrivatePtr priv = vm->privateData;
@@ -908,7 +908,6 @@ qemuDomainWriteMasterKeyFile(virQEMUDriverPtr driver,
 
  cleanup:
 VIR_FORCE_CLOSE(fd);
-VIR_FREE(path);
 
 return ret;
 }
@@ -944,7 +943,7 @@ qemuDomainMasterKeyFree(qemuDomainObjPrivatePtr priv)
 int
 qemuDomainMasterKeyReadFile(qemuDomainObjPrivatePtr priv)
 {
-char *path;
+g_autofree char *path = NULL;
 int fd = -1;
 uint8_t *masterKey = NULL;
 ssize_t masterKeyLen = 0;
@@ -990,7 +989,6 @@ qemuDomainMasterKeyReadFile(qemuDomainObjPrivatePtr priv)
 priv->masterKeyLen = masterKeyLen;
 
 VIR_FORCE_CLOSE(fd);
-VIR_FREE(path);
 
 return 0;
 
@@ -1000,7 +998,6 @@ qemuDomainMasterKeyReadFile(qemuDomainObjPrivatePtr priv)
 VIR_FREE(masterKey);
 
 VIR_FORCE_CLOSE(fd);
-VIR_FREE(path);
 
 return -1;
 }
@@ -1015,7 +1012,7 @@ qemuDomainMasterKeyReadFile(qemuDomainObjPrivatePtr priv)
 void
 qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv)
 {
-char *path = NULL;
+g_autofree char *path = NULL;
 
 if (!priv->masterKey)
 return;
@@ -1026,8 +1023,6 @@ qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv)
 /* Delete the master key file */
 path = qemuDomainGetMasterKeyFilePath(priv->libDir);
 unlink(path);
-
-VIR_FREE(path);
 }
 
 
@@ -1909,7 +1904,7 @@ qemuDomainSecretChardevPrepare(virQEMUDriverConfigPtr cfg,
const char *chrAlias,
virDomainChrSourceDefPtr dev)
 {
-char *charAlias = NULL;
+g_autofree char *charAlias = NULL;
 
 if (dev->type != VIR_DOMAIN_CHR_TYPE_TCP)
 return 0;
@@ -1925,7 +1920,6 @@ qemuDomainSecretChardevPrepare(virQEMUDriverConfigPtr cfg,
 chrSourcePriv->secinfo =
 qemuDomainSecretInfoTLSNew(priv, charAlias,
cfg->chardevTLSx509secretUUID);
-VIR_FREE(charAlias);
 
 if (!chrSourcePriv->secinfo)
 return -1;
@@ -2143,7 +2137,7 @@ qemuDomainSetPrivatePaths(virQEMUDriverPtr driver,
 {
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 qemuDomainObjPrivatePtr priv = vm->privateData;
-char *domname = virDomainDefGetShortName(vm->def);
+g_autofree char *domname = virDomainDefGetShortName(vm->def);
 int ret = -1;
 
 if (!domname)
@@ -2159,7 +2153,6 @@ qemuDomainSetPrivatePaths(virQEMUDriverPtr driver,
 ret = 0;
  cleanup:
 virObjectUnref(cfg);
-VIR_FREE(domname);
 return ret;
 }
 
@@ -2340,8 +2333,8 @@ qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt,
   virStorageSourcePtr src)
 {
 qemuDomainStorageSourcePrivatePtr priv;
-char *authalias = NULL;
-char *encalias = NULL;
+g_autofree char *authalias = NULL;
+g_autofree char *encalias = NULL;
 int ret = -1;
 
 src->nodestorage = 
virXPathString("string(./nodenames/nodename[@type='storage']/@name)", ctxt);
@@ -2374,8 +2367,6 @@ qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt,
 ret = 0;
 
  cleanup:
-VIR_FREE(authalias);
-VIR_FREE(encalias);
 
 return ret;
 }
@@ -2497,8 +2488,8 @@ static int
 qemuDomainObjPrivateXMLFormatAutomaticPlacement(virBufferPtr buf,
 qemuDomainObjPrivatePtr priv)
 {
-char *nodeset = NULL;
-char *cpuset = NULL;
+g_autofree char *nodeset = NULL;
+g_autofree char *cpuset = NULL;
 int ret = -1;
 
 if (!priv->autoNodeset && !priv->autoCpuset)
@@ -2520,8 +2511,6 @@ 
qemuDomainObjPrivateXMLFormatAutomaticPlacement(virBufferPtr buf,
 ret = 0;
 
  cleanup:
-VIR_FREE(nodeset);
-VIR_FREE(cpuset);
 return ret;
 }
 
@@ -3022,8 +3011,8 @@ qemuDomainObjPrivateXMLParseVcpu(xmlNodePtr node,
  virDomainDefPtr def)
 {
 virDomainVcpuDefPtr vcpu;
-char *idstr;
-char *pidstr;
+g_autofree char *idstr = NULL;
+g_autofree char *pidstr = NULL;
 unsigned int tmp;
 int ret = -1;
 
@@ -3052,8 +3041,6 @@ qemuDomainObjPrivateXMLParseVcpu(xmlNodePtr node,
 ret = 0;
 
  cleanup:
-VIR_FREE(idstr);
-VIR_FREE(pidstr);
 return ret;
 }
 
@@ -3064,8 +3051,8 @@ 

[libvirt] [PATCH 0/3] qemu: backup: Allow configuring and reading back of export an backup bitmap name

2020-01-09 Thread Peter Krempa
These were already posted before.

oVirt folks would like to be able to read the export and bitmap names.
Given that I already wrote support for configuring it and it's rather
simple I'm re-posting these as they were before.

Peter Krempa (3):
  conf: backup: Allow configuration of names exported via NBD
  qemu: backup: Implement support for backup disk export name
configuration
  qemu: backup: Implement support for backup disk bitmap name
configuration

 docs/formatbackup.html.in  |  9 +
 docs/schemas/domainbackup.rng  |  8 
 src/conf/backup_conf.c | 11 +++
 src/conf/backup_conf.h |  2 ++
 src/qemu/qemu_backup.c | 14 --
 .../domainbackupxml2xmlin/backup-pull-seclabel.xml |  2 +-
 .../backup-pull-seclabel.xml   |  2 +-
 7 files changed, 44 insertions(+), 4 deletions(-)

-- 
2.24.1

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



[libvirt] [PATCH 1/3] conf: backup: Allow configuration of names exported via NBD

2020-01-09 Thread Peter Krempa
If users wish to use different name for exported disks or bitmaps
the new fields allow to do so. Additionally they also document the
current settings.

Signed-off-by: Peter Krempa 
---
 docs/formatbackup.html.in |  9 +
 docs/schemas/domainbackup.rng |  8 
 src/conf/backup_conf.c| 11 +++
 src/conf/backup_conf.h|  2 ++
 tests/domainbackupxml2xmlin/backup-pull-seclabel.xml  |  2 +-
 tests/domainbackupxml2xmlout/backup-pull-seclabel.xml |  2 +-
 6 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/docs/formatbackup.html.in b/docs/formatbackup.html.in
index 1c690901c7..7d2c6f1599 100644
--- a/docs/formatbackup.html.in
+++ b/docs/formatbackup.html.in
@@ -85,6 +85,15 @@
   Setting this attribute to yes(default) specifies
 that the disk should take part in the backup and using
 no excludes the disk from the backup.
+  exportname
+  Allows to modify the NBD export name for the given disk.
+By default equal to disk target.
+Valid only for pull mode backups. 
+  exportbitmap
+  Allows to modify the name of the bitmap describing dirty
+blocks for an incremental backup exported via NBD export name
+for the given disk.
+Valid only for pull mode backups. 
   type
   A mandatory attribute to describe the type of the
 disk, except when backup='no' is
diff --git a/docs/schemas/domainbackup.rng b/docs/schemas/domainbackup.rng
index c1e4d80302..395ea841f9 100644
--- a/docs/schemas/domainbackup.rng
+++ b/docs/schemas/domainbackup.rng
@@ -165,6 +165,14 @@
 
   
 
+
+  
+
+  
+  
+
+  
+
 
   
 
diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c
index aa11967d2a..a4b87baa55 100644
--- a/src/conf/backup_conf.c
+++ b/src/conf/backup_conf.c
@@ -71,6 +71,8 @@ virDomainBackupDefFree(virDomainBackupDefPtr def)
 virDomainBackupDiskDefPtr disk = def->disks + i;

 g_free(disk->name);
+g_free(disk->exportname);
+g_free(disk->exportbitmap);
 virObjectUnref(disk->store);
 }

@@ -124,6 +126,11 @@ virDomainBackupDiskDefParseXML(xmlNodePtr node,
 if (def->backup == VIR_TRISTATE_BOOL_NO)
 return 0;

+if (!push) {
+def->exportname = virXMLPropString(node, "exportname");
+def->exportbitmap = virXMLPropString(node, "exportbitmap");
+}
+
 if (internal) {
 if (!(state = virXMLPropString(node, "state")) ||
 (tmp = virDomainBackupDiskStateTypeFromString(state)) < 0) {
@@ -165,6 +172,7 @@ virDomainBackupDiskDefParseXML(xmlNodePtr node,
 storageSourceParseFlags, xmlopt) < 0)
 return -1;

+
 if ((driver = virXPathString("string(./driver/@type)", ctxt))) {
 def->store->format = virStorageFileFormatTypeFromString(driver);
 if (def->store->format <= 0) {
@@ -333,6 +341,9 @@ virDomainBackupDiskDefFormat(virBufferPtr buf,
 if (disk->backup == VIR_TRISTATE_BOOL_YES) {
 virBufferAsprintf(, " type='%s'", 
virStorageTypeToString(disk->store->type));

+virBufferEscapeString(, " exportname='%s'", disk->exportname);
+virBufferEscapeString(, " exportbitmap='%s'", 
disk->exportbitmap);
+
 if (disk->store->format > 0)
 virBufferEscapeString(, "\n",
   
virStorageFileFormatTypeToString(disk->store->format));
diff --git a/src/conf/backup_conf.h b/src/conf/backup_conf.h
index 7cf44245d4..672fd52ee7 100644
--- a/src/conf/backup_conf.h
+++ b/src/conf/backup_conf.h
@@ -51,6 +51,8 @@ typedef virDomainBackupDiskDef *virDomainBackupDiskDefPtr;
 struct _virDomainBackupDiskDef {
 char *name; /* name matching the 1525889631
   
   
-
+
   
   
 
diff --git a/tests/domainbackupxml2xmlout/backup-pull-seclabel.xml 
b/tests/domainbackupxml2xmlout/backup-pull-seclabel.xml
index c631c9b979..450f007d3a 100644
--- a/tests/domainbackupxml2xmlout/backup-pull-seclabel.xml
+++ b/tests/domainbackupxml2xmlout/backup-pull-seclabel.xml
@@ -2,7 +2,7 @@
   1525889631
   
   
-
+
   
   
 
-- 
2.24.1

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



[libvirt] [PATCH 3/3] qemu: backup: Implement support for backup disk bitmap name configuration

2020-01-09 Thread Peter Krempa
Use the user-configured name of the bitmap when merging the appropriate
bitmaps for an incremental backup so that the user can see it as
configured. Additionally expose the default bitmap name if nothing is
configured.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_backup.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index 2cc0e6ab07..23518a5d40 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -322,7 +322,10 @@ qemuBackupDiskPrepareDataOne(virDomainObjPtr vm,
 return -1;

 if (incremental) {
-dd->incrementalBitmap = g_strdup_printf("backup-%s", dd->domdisk->dst);
+if (dd->backupdisk->exportbitmap)
+dd->incrementalBitmap = g_strdup(dd->backupdisk->exportbitmap);
+else
+dd->incrementalBitmap = g_strdup_printf("backup-%s", 
dd->domdisk->dst);

 if (qemuBackupDiskPrepareOneBitmaps(dd, actions, incremental,
 blockNamedNodeData) < 0)
@@ -368,6 +371,10 @@ static int
 qemuBackupDiskPrepareDataOnePull(virJSONValuePtr actions,
  struct qemuBackupDiskData *dd)
 {
+if (!dd->backupdisk->exportbitmap &&
+dd->incrementalBitmap)
+dd->backupdisk->exportbitmap = g_strdup(dd->incrementalBitmap);
+
 if (qemuMonitorTransactionBackup(actions,
  dd->domdisk->src->nodeformat,
  dd->blockjob->name,
-- 
2.24.1

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



[libvirt] [PATCH 2/3] qemu: backup: Implement support for backup disk export name configuration

2020-01-09 Thread Peter Krempa
Pass the exportname as configured when exporting the image via NBD and
fill it with the default if it's not configured.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_backup.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index c47de2f4a8..2cc0e6ab07 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -548,9 +548,12 @@ qemuBackupBeginPullExportDisks(virDomainObjPtr vm,
 for (i = 0; i < ndisks; i++) {
 struct qemuBackupDiskData *dd = disks + i;

+if (!dd->backupdisk->exportname)
+dd->backupdisk->exportname = g_strdup(dd->domdisk->dst);
+
 if (qemuMonitorNBDServerAdd(priv->mon,
 dd->store->nodeformat,
-dd->domdisk->dst,
+dd->backupdisk->exportname,
 false,
 dd->incrementalBitmap) < 0)
 return -1;
-- 
2.24.1

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



[libvirt] [RFC PATCH 15/16] tests: qemublock: Add checkpoint deletion tests for some special cases

2020-01-09 Thread Peter Krempa
Use the synthetic test data to verify that the algorithm correctly picks
bitmaps to merge when the bitmap is changed along with the image itself.

Signed-off-by: Peter Krempa 
---
 tests/qemublocktest.c |  7 +++
 .../snapshots-intermediate2-out.json  |  7 +++
 .../snapshots-intermediate3-out.json  |  7 +++
 ...hots-synthetic-checkpoint-current-out.json | 29 +
 ...ynthetic-checkpoint-intermediate1-out.json | 29 +
 ...ynthetic-checkpoint-intermediate2-out.json | 32 ++
 ...ynthetic-checkpoint-intermediate3-out.json | 59 +++
 ...ots-synthetic-checkpoint-noparent-out.json | 23 
 8 files changed, 193 insertions(+)
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-current-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate1-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate2-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate3-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-noparent-out.json

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index b8a052b19b..c2c7507e20 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -1021,6 +1021,13 @@ mymain(void)
 TEST_CHECKPOINT_DELETE_MERGE("snapshots-intermediate3", "d", "c", 
"snapshots");
 TEST_CHECKPOINT_DELETE_MERGE("snapshots-current", "current", "d", 
"snapshots");

+TEST_CHECKPOINT_DELETE_MERGE("snapshots-synthetic-checkpoint-noparent", 
"a", NULL, "snapshots-synthetic-checkpoint");
+
TEST_CHECKPOINT_DELETE_MERGE("snapshots-synthetic-checkpoint-intermediate1", 
"b", "a", "snapshots-synthetic-checkpoint");
+
TEST_CHECKPOINT_DELETE_MERGE("snapshots-synthetic-checkpoint-intermediate2", 
"c", "b", "snapshots-synthetic-checkpoint");
+
TEST_CHECKPOINT_DELETE_MERGE("snapshots-synthetic-checkpoint-intermediate3", 
"d", "c", "snapshots-synthetic-checkpoint");
+TEST_CHECKPOINT_DELETE_MERGE("snapshots-synthetic-checkpoint-current", 
"current", "d", "snapshots-synthetic-checkpoint");
+
+
  cleanup:
 virHashFree(diskxmljsondata.schema);
 qemuTestDriverFree();
diff --git 
a/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json 
b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json
index ca5627269c..4da21a9df7 100644
--- a/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json
+++ b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json
@@ -29,6 +29,13 @@
   "name": "c"
 }
   },
+  {
+"type": "block-dirty-bitmap-enable",
+"data": {
+  "node": "libvirt-3-format",
+  "name": "b"
+}
+  },
   {
 "type": "block-dirty-bitmap-merge",
 "data": {
diff --git 
a/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json 
b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json
index ee7ae97fdb..dc87dd60b8 100644
--- a/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json
+++ b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json
@@ -29,6 +29,13 @@
   "name": "d"
 }
   },
+  {
+"type": "block-dirty-bitmap-enable",
+"data": {
+  "node": "libvirt-2-format",
+  "name": "c"
+}
+  },
   {
 "type": "block-dirty-bitmap-merge",
 "data": {
diff --git 
a/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-current-out.json
 
b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-current-out.json
new file mode 100644
index 00..1b607567e8
--- /dev/null
+++ 
b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-current-out.json
@@ -0,0 +1,29 @@
+[
+  {
+"type": "block-dirty-bitmap-enable",
+"data": {
+  "node": "libvirt-1-format",
+  "name": "d"
+}
+  },
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-1-format",
+  "target": "d",
+  "bitmaps": [
+{
+  "node": "libvirt-1-format",
+  "name": "current"
+}
+  ]
+}
+  },
+  {
+"type": "block-dirty-bitmap-remove",
+"data": {
+  "node": "libvirt-1-format",
+  "name": "current"
+}
+  }
+]
diff --git 
a/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate1-out.json
 
b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate1-out.json
new file mode 100644
index 00..e979691e6f
--- /dev/null
+++ 
b/tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate1-out.json
@@ -0,0 +1,29 @@
+[
+  {
+"type": "block-dirty-bitmap-enable",
+"data": {
+  "node": "libvirt-3-format",
+  "name": "a"
+}
+  },
+  {
+"type": 

[libvirt] [RFC PATCH 16/16] qemu: checkpoint: Track and relabel images for bitmap merging

2020-01-09 Thread Peter Krempa
Allow qemu access to modify backing files in case when we want to delete
a checkpoint.

This patch adds tracking of which images need to be relabelled when
calculating the transaction, the code to relabel them and rollback.

To verify that stuff works we also output the list of images to relabel
into the test case output files in qemublocktest.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_checkpoint.c| 33 ---
 src/qemu/qemu_checkpoint.h|  3 +-
 tests/qemublocktest.c | 19 +--
 .../snapshots-intermediate1-out.json  |  2 ++
 .../snapshots-intermediate2-out.json  |  3 ++
 .../snapshots-intermediate3-out.json  |  2 ++
 .../snapshots-noparent-out.json   |  4 +++
 ...ynthetic-checkpoint-intermediate1-out.json |  2 ++
 ...ynthetic-checkpoint-intermediate2-out.json |  2 ++
 ...ynthetic-checkpoint-intermediate3-out.json |  2 ++
 ...ots-synthetic-checkpoint-noparent-out.json |  4 +++
 11 files changed, 69 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index 087a740cf8..13bd6653b7 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -155,7 +155,8 @@ qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src,
  const char *delbitmap,
  const char *parentbitmap,
  virJSONValuePtr actions,
- const char *diskdst)
+ const char *diskdst,
+ GSList **reopenimages)
 {
 virStorageSourcePtr n = src;

@@ -235,6 +236,9 @@ qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src,
srcbitmap->name) < 0)
 return -1;

+if (n != src)
+*reopenimages = g_slist_prepend(*reopenimages, n);
+
 n = n->backingStore;
 }

@@ -253,6 +257,9 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
 int rc;
 g_autoptr(virJSONValue) actions = NULL;
 size_t i;
+g_autoptr(GSList) reopenimages = NULL;
+g_autoptr(GSList) relabelimages = NULL;
+GSList *next;

 if (!(actions = virJSONValueNewArray()))
 return -1;
@@ -284,16 +291,34 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,

 if (qemuCheckpointDiscardDiskBitmaps(domdisk->src, blockNamedNodeData,
  chkdisk->bitmap, parentbitmap,
- actions, domdisk->dst) < 0)
+ actions, domdisk->dst,
+ ) < 0)
 return -1;
 }

+/* label any non-top images for read-write access */
+for (next = reopenimages; next; next = next->next) {
+virStorageSourcePtr src = next->data;
+
+if (qemuDomainStorageSourceAccessAllow(driver, vm, src, false, false) 
< 0)
+goto relabel;
+
+relabelimages = g_slist_prepend(relabelimages, src);
+}
+
 qemuDomainObjEnterMonitor(driver, vm);
 rc = qemuMonitorTransaction(priv->mon, );
-if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
 return -1;

-return 0;
+ relabel:
+for (next = relabelimages; next; next = next->next) {
+virStorageSourcePtr src = next->data;
+
+ignore_value(qemuDomainStorageSourceAccessAllow(driver, vm, src, true, 
false));
+}
+
+return rc;
 }


diff --git a/src/qemu/qemu_checkpoint.h b/src/qemu/qemu_checkpoint.h
index 976b1eed0f..cf1e9e46cb 100644
--- a/src/qemu/qemu_checkpoint.h
+++ b/src/qemu/qemu_checkpoint.h
@@ -78,4 +78,5 @@ qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src,
  const char *delbitmap,
  const char *parentbitmap,
  virJSONValuePtr actions,
- const char *diskdst);
+ const char *diskdst,
+ GSList **reopenimages);
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index c2c7507e20..a368e6a74f 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -717,6 +717,9 @@ testQemuCheckpointDeleteMerge(const void *opaque)
 g_autoptr(virJSONValue) actions = NULL;
 g_autoptr(virJSONValue) nodedatajson = NULL;
 g_autoptr(virHashTable) nodedata = NULL;
+g_autoptr(GSList) reopenimages = NULL;
+g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+GSList *tmp;

 expectpath = g_strdup_printf("%s/%s%s-out.json", abs_srcdir,
  checkpointDeletePrefix, data->name);
@@ -738,14 +741,26 @@ testQemuCheckpointDeleteMerge(const void *opaque)
  data->deletebitmap,
  

[libvirt] [RFC PATCH 10/16] tests: qemublock: Add test for checkpoint deletion bitmap merge

2020-01-09 Thread Peter Krempa
Add test infrastructure and a basic test for bitmap deletion.

Signed-off-by: Peter Krempa 
---
 tests/qemublocktest.c | 59 +++
 .../checkpointdelete/basic-noparent-out.json  |  9 +++
 2 files changed, 68 insertions(+)
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/basic-noparent-out.json

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 3e9edb85f0..3ed2486ad2 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -27,6 +27,7 @@
 #include "qemu/qemu_qapi.h"
 #include "qemu/qemu_monitor_json.h"
 #include "qemu/qemu_backup.h"
+#include "qemu/qemu_checkpoint.h"

 #include "qemu/qemu_command.h"

@@ -696,6 +697,50 @@ testQemuBackupIncrementalBitmapCalculate(const void 
*opaque)
 }


+static const char *checkpointDeletePrefix = 
"qemublocktestdata/checkpointdelete/";
+
+struct testQemuCheckpointDeleteMergeData {
+const char *name;
+virStorageSourcePtr chain;
+const char *deletebitmap;
+const char *parentbitmap;
+};
+
+
+static int
+testQemuCheckpointDeleteMerge(const void *opaque)
+{
+const struct testQemuCheckpointDeleteMergeData *data = opaque;
+g_autofree char *actual = NULL;
+g_autofree char *expectpath = NULL;
+g_autoptr(virJSONValue) actions = NULL;
+bool currentcheckpoint;
+
+expectpath = g_strdup_printf("%s/%s%s-out.json", abs_srcdir,
+ checkpointDeletePrefix, data->name);
+
+if (!(actions = virJSONValueNewArray()))
+return -1;
+
+/* hack to get the 'current' state until the function stops accepting it */
+currentcheckpoint = STREQ("current", data->deletebitmap);
+
+if (qemuCheckpointDiscardDiskBitmaps(data->chain,
+ data->deletebitmap,
+ data->parentbitmap,
+ currentcheckpoint,
+ actions) < 0) {
+VIR_TEST_VERBOSE("failed to generate checkpoint delete transaction\n");
+return -1;
+}
+
+if (!(actual = virJSONValueToString(actions, true)))
+return -1;
+
+return virTestCompareToFile(actual, expectpath);
+}
+
+
 static int
 mymain(void)
 {
@@ -705,6 +750,7 @@ mymain(void)
 struct testQemuDiskXMLToJSONData diskxmljsondata;
 struct testQemuImageCreateData imagecreatedata;
 struct testQemuBackupIncrementalBitmapCalculateData backupbitmapcalcdata;
+struct testQemuCheckpointDeleteMergeData checkpointdeletedata;
 char *capslatest_x86_64 = NULL;
 virQEMUCapsPtr caps_x86_64 = NULL;
 g_autoptr(virStorageSource) bitmapSourceChain = NULL;
@@ -941,6 +987,19 @@ mymain(void)
 TEST_BACKUP_BITMAP_CALCULATE("snapshot-intermediate", bitmapSourceChain, 
"d", "snapshots");
 TEST_BACKUP_BITMAP_CALCULATE("snapshot-deep", bitmapSourceChain, "a", 
"snapshots");

+#define TEST_CHECKPOINT_DELETE_MERGE(testname, delbmp, parbmp) \
+do { \
+checkpointdeletedata.name = testname; \
+checkpointdeletedata.chain = bitmapSourceChain; \
+checkpointdeletedata.deletebitmap = delbmp; \
+checkpointdeletedata.parentbitmap = parbmp; \
+if (virTestRun("checkpoint delete " testname, \
+   testQemuCheckpointDeleteMerge, ) < 
0) \
+ret = -1; \
+} while (0)
+
+TEST_CHECKPOINT_DELETE_MERGE("basic-noparent", "a", NULL);
+
  cleanup:
 virHashFree(diskxmljsondata.schema);
 qemuTestDriverFree();
diff --git a/tests/qemublocktestdata/checkpointdelete/basic-noparent-out.json 
b/tests/qemublocktestdata/checkpointdelete/basic-noparent-out.json
new file mode 100644
index 00..e87382fdb4
--- /dev/null
+++ b/tests/qemublocktestdata/checkpointdelete/basic-noparent-out.json
@@ -0,0 +1,9 @@
+[
+  {
+"type": "block-dirty-bitmap-remove",
+"data": {
+  "node": "libvirt-1-format",
+  "name": "a"
+}
+  }
+]
-- 
2.24.1

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



[libvirt] [RFC PATCH 13/16] qemu: checkpoint: Introduce support for deleting checkpoints accross snapshots

2020-01-09 Thread Peter Krempa
Allow deleting of checkpoints when snapshots were created along. The
code tracks and modifies the checkpoint list so that backups can still
be taken with such a backing chain. This unfortunately requires to
rename few bitmaps (by copying and deleting them) in some cases.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_checkpoint.c | 112 -
 src/qemu/qemu_checkpoint.h |   5 +-
 tests/qemublocktest.c  |  34 +++
 3 files changed, 111 insertions(+), 40 deletions(-)

diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index e75cdd0458..087a740cf8 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -24,6 +24,7 @@
 #include "qemu_capabilities.h"
 #include "qemu_monitor.h"
 #include "qemu_domain.h"
+#include "qemu_block.h"

 #include "virerror.h"
 #include "virlog.h"
@@ -150,39 +151,92 @@ qemuCheckpointFindActiveDiskInParent(virDomainObjPtr vm,

 int
 qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src,
+ virHashTablePtr blockNamedNodeData,
  const char *delbitmap,
  const char *parentbitmap,
- bool chkcurrent,
- virJSONValuePtr actions)
+ virJSONValuePtr actions,
+ const char *diskdst)
 {
-if (parentbitmap) {
-g_autoptr(virJSONValue) arr = NULL;
+virStorageSourcePtr n = src;

-if (!(arr = virJSONValueNewArray()))
-return -1;
+/* find the backing chain entry with bitmap named '@bitmap' */
+while (n) {
+qemuBlockNamedNodeDataBitmapPtr tmp;

-if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr,
- src->nodeformat,
- delbitmap) < 0)
-return -1;
+if ((tmp = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,
+ n, delbitmap))) {
+break;
+}
+
+n = n->backingStore;
+}
+
+if (!n) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("bitmap '%s' not found in backing chain of '%s'"),
+   delbitmap, diskdst);
+return -1;
+}

-if (chkcurrent) {
-if (qemuMonitorTransactionBitmapEnable(actions,
-   src->nodeformat,
-   parentbitmap) < 0)
+while (n) {
+qemuBlockNamedNodeDataBitmapPtr srcbitmap;
+
+if (!(srcbitmap = 
qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,
+n, delbitmap)))
+break;
+
+/* For the actual checkpoint deletion we will merge any bitmap into the
+ * bitmap of the parent checkpoint (@mergebitmap) or for any image
+ * where the parent checkpoint bitmap is not present we must rename
+ * the bitmap of the deleted checkpoint into the bitmap of the parent
+ * checkpoint as qemu can't currently take the allocation map and turn
+ * it into a bitmap and thus we wouldn't be able to do a backup. */
+if (parentbitmap) {
+qemuBlockNamedNodeDataBitmapPtr dstbitmap;
+g_autoptr(virJSONValue) arr = NULL;
+
+dstbitmap = 
qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,
+  n, parentbitmap);
+
+if (dstbitmap) {
+if (srcbitmap->recording && !dstbitmap->recording) {
+if (qemuMonitorTransactionBitmapEnable(actions,
+   n->nodeformat,
+   dstbitmap->name) < 
0)
+return -1;
+}
+
+} else {
+if (qemuMonitorTransactionBitmapAdd(actions,
+n->nodeformat,
+parentbitmap,
+true,
+!srcbitmap->recording,
+srcbitmap->granularity) < 
0)
+return -1;
+}
+
+if (!(arr = virJSONValueNewArray()))
+return -1;
+
+if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr,
+ n->nodeformat,
+ 
srcbitmap->name) < 0)
+return -1;
+
+if (qemuMonitorTransactionBitmapMerge(actions,
+ 

[libvirt] [RFC PATCH 14/16] tests: qemublock: Add checkpoint deletion test for deep backing chain

2020-01-09 Thread Peter Krempa
Add test cases for merging various pairs of bitmaps when snapshots were
created together with checkpoints.

Signed-off-by: Peter Krempa 
---
 tests/qemublocktest.c |  6 +++
 .../snapshots-current-out.json| 29 +++
 .../snapshots-intermediate1-out.json  | 22 
 .../snapshots-intermediate2-out.json  | 52 +++
 .../snapshots-intermediate3-out.json  | 52 +++
 .../snapshots-noparent-out.json   | 23 
 6 files changed, 184 insertions(+)
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-current-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-intermediate1-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-noparent-out.json

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 293757b8d4..b8a052b19b 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -1015,6 +1015,12 @@ mymain(void)
 TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate3", "d", "c", "basic");
 TEST_CHECKPOINT_DELETE_MERGE("basic-current", "current", "d", "basic");

+TEST_CHECKPOINT_DELETE_MERGE("snapshots-noparent", "a", NULL, "snapshots");
+TEST_CHECKPOINT_DELETE_MERGE("snapshots-intermediate1", "b", "a", 
"snapshots");
+TEST_CHECKPOINT_DELETE_MERGE("snapshots-intermediate2", "c", "b", 
"snapshots");
+TEST_CHECKPOINT_DELETE_MERGE("snapshots-intermediate3", "d", "c", 
"snapshots");
+TEST_CHECKPOINT_DELETE_MERGE("snapshots-current", "current", "d", 
"snapshots");
+
  cleanup:
 virHashFree(diskxmljsondata.schema);
 qemuTestDriverFree();
diff --git 
a/tests/qemublocktestdata/checkpointdelete/snapshots-current-out.json 
b/tests/qemublocktestdata/checkpointdelete/snapshots-current-out.json
new file mode 100644
index 00..1b607567e8
--- /dev/null
+++ b/tests/qemublocktestdata/checkpointdelete/snapshots-current-out.json
@@ -0,0 +1,29 @@
+[
+  {
+"type": "block-dirty-bitmap-enable",
+"data": {
+  "node": "libvirt-1-format",
+  "name": "d"
+}
+  },
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-1-format",
+  "target": "d",
+  "bitmaps": [
+{
+  "node": "libvirt-1-format",
+  "name": "current"
+}
+  ]
+}
+  },
+  {
+"type": "block-dirty-bitmap-remove",
+"data": {
+  "node": "libvirt-1-format",
+  "name": "current"
+}
+  }
+]
diff --git 
a/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate1-out.json 
b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate1-out.json
new file mode 100644
index 00..29fefeea63
--- /dev/null
+++ b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate1-out.json
@@ -0,0 +1,22 @@
+[
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-3-format",
+  "target": "a",
+  "bitmaps": [
+{
+  "node": "libvirt-3-format",
+  "name": "b"
+}
+  ]
+}
+  },
+  {
+"type": "block-dirty-bitmap-remove",
+"data": {
+  "node": "libvirt-3-format",
+  "name": "b"
+}
+  }
+]
diff --git 
a/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json 
b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json
new file mode 100644
index 00..ca5627269c
--- /dev/null
+++ b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json
@@ -0,0 +1,52 @@
+[
+  {
+"type": "block-dirty-bitmap-add",
+"data": {
+  "node": "libvirt-2-format",
+  "name": "b",
+  "persistent": true,
+  "disabled": true,
+  "granularity": 65536
+}
+  },
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-2-format",
+  "target": "b",
+  "bitmaps": [
+{
+  "node": "libvirt-2-format",
+  "name": "c"
+}
+  ]
+}
+  },
+  {
+"type": "block-dirty-bitmap-remove",
+"data": {
+  "node": "libvirt-2-format",
+  "name": "c"
+}
+  },
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-3-format",
+  "target": "b",
+  "bitmaps": [
+{
+  "node": "libvirt-3-format",
+  "name": "c"
+}
+  ]
+}
+  },
+  {
+"type": "block-dirty-bitmap-remove",
+"data": {
+  "node": "libvirt-3-format",
+  "name": "c"
+}
+  }
+]
diff --git 
a/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json 
b/tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json
new file mode 100644
index 00..ee7ae97fdb
--- /dev/null
+++ 

[libvirt] [RFC PATCH 11/16] tests: qemublock: Add few more test cases for checkpoint deletion

2020-01-09 Thread Peter Krempa
Add all intermediate steps and deletion of the current checkpoint on a
flat (single-image) disk image.

Signed-off-by: Peter Krempa 
---
 tests/qemublocktest.c |  4 +++
 .../checkpointdelete/basic-current-out.json   | 29 +++
 .../basic-intermediate1-out.json  | 22 ++
 .../basic-intermediate2-out.json  | 22 ++
 .../basic-intermediate3-out.json  | 22 ++
 5 files changed, 99 insertions(+)
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/basic-current-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/basic-intermediate1-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/basic-intermediate2-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/basic-intermediate3-out.json

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 3ed2486ad2..203e0f70b1 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -999,6 +999,10 @@ mymain(void)
 } while (0)

 TEST_CHECKPOINT_DELETE_MERGE("basic-noparent", "a", NULL);
+TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate1", "b", "a");
+TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate2", "c", "b");
+TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate3", "d", "c");
+TEST_CHECKPOINT_DELETE_MERGE("basic-current", "current", "d");

  cleanup:
 virHashFree(diskxmljsondata.schema);
diff --git a/tests/qemublocktestdata/checkpointdelete/basic-current-out.json 
b/tests/qemublocktestdata/checkpointdelete/basic-current-out.json
new file mode 100644
index 00..1b607567e8
--- /dev/null
+++ b/tests/qemublocktestdata/checkpointdelete/basic-current-out.json
@@ -0,0 +1,29 @@
+[
+  {
+"type": "block-dirty-bitmap-enable",
+"data": {
+  "node": "libvirt-1-format",
+  "name": "d"
+}
+  },
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-1-format",
+  "target": "d",
+  "bitmaps": [
+{
+  "node": "libvirt-1-format",
+  "name": "current"
+}
+  ]
+}
+  },
+  {
+"type": "block-dirty-bitmap-remove",
+"data": {
+  "node": "libvirt-1-format",
+  "name": "current"
+}
+  }
+]
diff --git 
a/tests/qemublocktestdata/checkpointdelete/basic-intermediate1-out.json 
b/tests/qemublocktestdata/checkpointdelete/basic-intermediate1-out.json
new file mode 100644
index 00..eccb7ed15f
--- /dev/null
+++ b/tests/qemublocktestdata/checkpointdelete/basic-intermediate1-out.json
@@ -0,0 +1,22 @@
+[
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-1-format",
+  "target": "a",
+  "bitmaps": [
+{
+  "node": "libvirt-1-format",
+  "name": "b"
+}
+  ]
+}
+  },
+  {
+"type": "block-dirty-bitmap-remove",
+"data": {
+  "node": "libvirt-1-format",
+  "name": "b"
+}
+  }
+]
diff --git 
a/tests/qemublocktestdata/checkpointdelete/basic-intermediate2-out.json 
b/tests/qemublocktestdata/checkpointdelete/basic-intermediate2-out.json
new file mode 100644
index 00..de40e4b5b0
--- /dev/null
+++ b/tests/qemublocktestdata/checkpointdelete/basic-intermediate2-out.json
@@ -0,0 +1,22 @@
+[
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-1-format",
+  "target": "b",
+  "bitmaps": [
+{
+  "node": "libvirt-1-format",
+  "name": "c"
+}
+  ]
+}
+  },
+  {
+"type": "block-dirty-bitmap-remove",
+"data": {
+  "node": "libvirt-1-format",
+  "name": "c"
+}
+  }
+]
diff --git 
a/tests/qemublocktestdata/checkpointdelete/basic-intermediate3-out.json 
b/tests/qemublocktestdata/checkpointdelete/basic-intermediate3-out.json
new file mode 100644
index 00..b5d85f43f0
--- /dev/null
+++ b/tests/qemublocktestdata/checkpointdelete/basic-intermediate3-out.json
@@ -0,0 +1,22 @@
+[
+  {
+"type": "block-dirty-bitmap-merge",
+"data": {
+  "node": "libvirt-1-format",
+  "target": "c",
+  "bitmaps": [
+{
+  "node": "libvirt-1-format",
+  "name": "d"
+}
+  ]
+}
+  },
+  {
+"type": "block-dirty-bitmap-remove",
+"data": {
+  "node": "libvirt-1-format",
+  "name": "d"
+}
+  }
+]
-- 
2.24.1

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



[libvirt] [RFC PATCH 06/16] qemu: checkpoint: tolerate missing disks on checkpoint deletion

2020-01-09 Thread Peter Krempa
If a disk is unplugged and then the user tries to delete a checkpoint
the code would try to use NULL node name as it was not checked.

Fix this by fetching the whole disk definition object and verifying it
was found.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_checkpoint.c | 23 +--
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index 03a8321135..326707e098 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -125,12 +125,15 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,

 for (i = 0; i < chkdef->ndisks; i++) {
 virDomainCheckpointDiskDef *chkdisk = >disks[i];
-const char *node;
+virDomainDiskDefPtr domdisk = virDomainDiskByTarget(vm->def, 
chkdisk->name);
+
+/* domdisk can be missing e.g. when it was unplugged */
+if (!domdisk)
+continue;

 if (chkdisk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
 continue;

-node = qemuDomainDiskNodeFormatLookup(vm, chkdisk->name);
 /* If any ancestor checkpoint has a bitmap for the same
  * disk, then this bitmap must be merged to the
  * ancestor. */
@@ -153,20 +156,28 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
 if (!(arr = virJSONValueNewArray()))
 return -1;

-if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr, 
node, chkdisk->bitmap) < 0)
+if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr,
+ 
domdisk->src->nodeformat,
+ 
chkdisk->bitmap) < 0)
 return -1;

 if (chkcurrent) {
-if (qemuMonitorTransactionBitmapEnable(actions, node, 
disk2->bitmap) < 0)
+if (qemuMonitorTransactionBitmapEnable(actions,
+   
domdisk->src->nodeformat,
+   disk2->bitmap) < 0)
 return -1;
 }

-if (qemuMonitorTransactionBitmapMerge(actions, node, 
disk2->bitmap, ) < 0)
+if (qemuMonitorTransactionBitmapMerge(actions,
+  domdisk->src->nodeformat,
+  disk2->bitmap, ) < 0)
 return -1;
 }
 }

-if (qemuMonitorTransactionBitmapRemove(actions, node, chkdisk->bitmap) 
< 0)
+if (qemuMonitorTransactionBitmapRemove(actions,
+   domdisk->src->nodeformat,
+   chkdisk->bitmap) < 0)
 return -1;
 }

-- 
2.24.1

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



[libvirt] [RFC PATCH 04/16] qemu: checkpoint: rename disk->chkdisk in qemuCheckpointAddActions

2020-01-09 Thread Peter Krempa
Upcomming patches will also use the domain diks definition. Rename disk
to chkdisk for clarity.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_checkpoint.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index d347b8fc6c..0aa854324b 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -319,13 +319,13 @@ qemuCheckpointAddActions(virDomainObjPtr vm,
 bool search_parents;

 for (i = 0; i < def->ndisks; i++) {
-virDomainCheckpointDiskDef *disk = >disks[i];
+virDomainCheckpointDiskDef *chkdisk = >disks[i];
 const char *node;

-if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
+if (chkdisk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
 continue;
-node = qemuDomainDiskNodeFormatLookup(vm, disk->name);
-if (qemuMonitorTransactionBitmapAdd(actions, node, disk->bitmap, true, 
false, 0) < 0)
+node = qemuDomainDiskNodeFormatLookup(vm, chkdisk->name);
+if (qemuMonitorTransactionBitmapAdd(actions, node, chkdisk->bitmap, 
true, false, 0) < 0)
 return -1;

 /* We only want one active bitmap for a disk along the
@@ -345,7 +345,7 @@ qemuCheckpointAddActions(virDomainObjPtr vm,
 virDomainCheckpointDiskDef *disk2;

 disk2 = >disks[j];
-if (STRNEQ(disk->name, disk2->name) ||
+if (STRNEQ(chkdisk->name, disk2->name) ||
 disk2->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
 continue;
 if (qemuMonitorTransactionBitmapDisable(actions, node, 
disk2->bitmap) < 0)
-- 
2.24.1

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



[libvirt] [RFC PATCH 03/16] qemu: checkpoint: rename disk->chkdisk in qemuCheckpointDiscardBitmaps

2020-01-09 Thread Peter Krempa
Upcomming patches will also use the domain diks definition. Rename disk
to chkdisk for clarity.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_checkpoint.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index 9ff3129570..d347b8fc6c 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -124,13 +124,13 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
 return -1;

 for (i = 0; i < chkdef->ndisks; i++) {
-virDomainCheckpointDiskDef *disk = >disks[i];
+virDomainCheckpointDiskDef *chkdisk = >disks[i];
 const char *node;

-if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
+if (chkdisk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
 continue;

-node = qemuDomainDiskNodeFormatLookup(vm, disk->name);
+node = qemuDomainDiskNodeFormatLookup(vm, chkdisk->name);
 /* If any ancestor checkpoint has a bitmap for the same
  * disk, then this bitmap must be merged to the
  * ancestor. */
@@ -145,7 +145,7 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
 g_autoptr(virJSONValue) arr = NULL;

 disk2 = >disks[j];
-if (STRNEQ(disk->name, disk2->name) ||
+if (STRNEQ(chkdisk->name, disk2->name) ||
 disk2->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
 continue;
 search_parents = false;
@@ -153,7 +153,7 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
 if (!(arr = virJSONValueNewArray()))
 return -1;

-if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr, 
node, disk->bitmap) < 0)
+if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr, 
node, chkdisk->bitmap) < 0)
 return -1;

 if (chkcurrent) {
@@ -166,7 +166,7 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
 }
 }

-if (qemuMonitorTransactionBitmapRemove(actions, node, disk->bitmap) < 
0)
+if (qemuMonitorTransactionBitmapRemove(actions, node, chkdisk->bitmap) 
< 0)
 return -1;
 }

-- 
2.24.1

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



[libvirt] [RFC PATCH 05/16] qemu: checkpoint: Use disk definition directly when creating checkpoint

2020-01-09 Thread Peter Krempa
Lookup the whole disk definition rather than just the node name.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_checkpoint.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index 0aa854324b..03a8321135 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -320,12 +320,16 @@ qemuCheckpointAddActions(virDomainObjPtr vm,

 for (i = 0; i < def->ndisks; i++) {
 virDomainCheckpointDiskDef *chkdisk = >disks[i];
-const char *node;
+virDomainDiskDefPtr domdisk = virDomainDiskByTarget(vm->def, 
chkdisk->name);

-if (chkdisk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
+/* checkpoint definition validator mandates that the corresponding
+ * domdisk should exist */
+if (!domdisk ||
+chkdisk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
 continue;
-node = qemuDomainDiskNodeFormatLookup(vm, chkdisk->name);
-if (qemuMonitorTransactionBitmapAdd(actions, node, chkdisk->bitmap, 
true, false, 0) < 0)
+
+if (qemuMonitorTransactionBitmapAdd(actions, domdisk->src->nodeformat,
+chkdisk->bitmap, true, false, 0) < 
0)
 return -1;

 /* We only want one active bitmap for a disk along the
@@ -348,7 +352,9 @@ qemuCheckpointAddActions(virDomainObjPtr vm,
 if (STRNEQ(chkdisk->name, disk2->name) ||
 disk2->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
 continue;
-if (qemuMonitorTransactionBitmapDisable(actions, node, 
disk2->bitmap) < 0)
+if (qemuMonitorTransactionBitmapDisable(actions,
+
domdisk->src->nodeformat,
+disk2->bitmap) < 0)
 return -1;
 search_parents = false;
 break;
-- 
2.24.1

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



[libvirt] [RFC PATCH 12/16] tests: qemublock: Add synthetic snapshot+checkpoint test data

2020-01-09 Thread Peter Krempa
Add a faked qemu output which would simulate scenario where libvirt
would take a snapshot and checkpoint simultaneously. This is visible in
libvirt-2-format node where bitmap 'c' appears, but bitmap 'b' which is
active in the previous layer is not present.

Signed-off-by: Peter Krempa 
---
 tests/qemublocktest.c |   1 +
 .../snapshots-synthetic-checkpoint.json   | 827 ++
 .../bitmap/snapshots-synthetic-checkpoint.out |  13 +
 3 files changed, 841 insertions(+)
 create mode 100644 
tests/qemublocktestdata/bitmap/snapshots-synthetic-checkpoint.json
 create mode 100644 
tests/qemublocktestdata/bitmap/snapshots-synthetic-checkpoint.out

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 203e0f70b1..8a3f7da029 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -966,6 +966,7 @@ mymain(void)
 TEST_BITMAP_DETECT("basic");
 TEST_BITMAP_DETECT("synthetic");
 TEST_BITMAP_DETECT("snapshots");
+TEST_BITMAP_DETECT("snapshots-synthetic-checkpoint");

 #define TEST_BACKUP_BITMAP_CALCULATE(testname, source, incrbackup, named) \
 do { \
diff --git a/tests/qemublocktestdata/bitmap/snapshots-synthetic-checkpoint.json 
b/tests/qemublocktestdata/bitmap/snapshots-synthetic-checkpoint.json
new file mode 100644
index 00..25cc150d67
--- /dev/null
+++ b/tests/qemublocktestdata/bitmap/snapshots-synthetic-checkpoint.json
@@ -0,0 +1,827 @@
+[
+{
+"iops_rd": 0,
+"detect_zeroes": "off",
+"image": {
+"backing-image": {
+"backing-image": {
+"backing-image": {
+"backing-image": {
+"virtual-size": 10485760,
+"filename": "/tmp/pull4.qcow2",
+"cluster-size": 65536,
+"format": "qcow2",
+"actual-size": 208896,
+"format-specific": {
+"type": "qcow2",
+"data": {
+"compat": "1.1",
+"lazy-refcounts": false,
+"bitmaps": [
+{
+"flags": [
+"auto"
+],
+"name": "a",
+"granularity": 65536
+}
+],
+"refcount-bits": 16,
+"corrupt": false
+}
+},
+"dirty-flag": false
+},
+"backing-filename-format": "qcow2",
+"virtual-size": 10485760,
+"filename": "/tmp/pull4.1575911522",
+"cluster-size": 65536,
+"format": "qcow2",
+"actual-size": 208896,
+"format-specific": {
+"type": "qcow2",
+"data": {
+"compat": "1.1",
+"lazy-refcounts": false,
+"bitmaps": [
+{
+"flags": [
+"auto"
+],
+"name": "a",
+"granularity": 65536
+}
+],
+"refcount-bits": 16,
+"corrupt": false
+}
+},
+"full-backing-filename": "/tmp/pull4.qcow2",
+"backing-filename": "/tmp/pull4.qcow2",
+"dirty-flag": false
+},
+"backing-filename-format": "qcow2",
+"virtual-size": 10485760,
+"filename": "/tmp/pull4.1575911527",
+"cluster-size": 65536,
+"format": "qcow2",
+"actual-size": 217088,
+"format-specific": {
+"type": "qcow2",
+"data": {
+"compat": "1.1",
+"lazy-refcounts": false,
+"bitmaps": [
+{
+"flags": [
+"auto"
+],
+  

[libvirt] [RFC PATCH 09/16] qemu: checkpoint: Extract calculation of bitmap merging for checkpoint deletion

2020-01-09 Thread Peter Krempa
This will allow some testing before refactoring.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_checkpoint.c | 72 --
 src/qemu/qemu_checkpoint.h |  7 
 2 files changed, 53 insertions(+), 26 deletions(-)

diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index 1100f6e744..e75cdd0458 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -148,6 +148,46 @@ qemuCheckpointFindActiveDiskInParent(virDomainObjPtr vm,
 }


+int
+qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src,
+ const char *delbitmap,
+ const char *parentbitmap,
+ bool chkcurrent,
+ virJSONValuePtr actions)
+{
+if (parentbitmap) {
+g_autoptr(virJSONValue) arr = NULL;
+
+if (!(arr = virJSONValueNewArray()))
+return -1;
+
+if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr,
+ src->nodeformat,
+ delbitmap) < 0)
+return -1;
+
+if (chkcurrent) {
+if (qemuMonitorTransactionBitmapEnable(actions,
+   src->nodeformat,
+   parentbitmap) < 0)
+return -1;
+}
+
+if (qemuMonitorTransactionBitmapMerge(actions,
+  src->nodeformat,
+  parentbitmap, ) < 0)
+return -1;
+}
+
+if (qemuMonitorTransactionBitmapRemove(actions,
+   src->nodeformat,
+   delbitmap) < 0)
+return -1;
+
+return 0;
+}
+
+
 static int
 qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
  virDomainCheckpointDefPtr chkdef,
@@ -167,6 +207,7 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
 virDomainCheckpointDiskDef *chkdisk = >disks[i];
 virDomainDiskDefPtr domdisk = virDomainDiskByTarget(vm->def, 
chkdisk->name);
 virDomainCheckpointDiskDef *parentchkdisk = NULL;
+const char *parentbitmap = NULL;

 /* domdisk can be missing e.g. when it was unplugged */
 if (!domdisk)
@@ -178,33 +219,12 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
 /* If any ancestor checkpoint has a bitmap for the same
  * disk, then this bitmap must be merged to the
  * ancestor. */
-if ((parentchkdisk = qemuCheckpointFindActiveDiskInParent(vm, parent, 
chkdisk->name))) {
-g_autoptr(virJSONValue) arr = NULL;
-
-if (!(arr = virJSONValueNewArray()))
-return -1;
-
-if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr,
- 
domdisk->src->nodeformat,
- 
chkdisk->bitmap) < 0)
-return -1;
-
-if (chkcurrent) {
-if (qemuMonitorTransactionBitmapEnable(actions,
-   
domdisk->src->nodeformat,
-   parentchkdisk->bitmap) 
< 0)
-return -1;
-}
-
-if (qemuMonitorTransactionBitmapMerge(actions,
-  domdisk->src->nodeformat,
-  parentchkdisk->bitmap, ) 
< 0)
-return -1;
-}
+if ((parentchkdisk = qemuCheckpointFindActiveDiskInParent(vm, parent,
+  
chkdisk->name)))
+parentbitmap = parentchkdisk->name;

-if (qemuMonitorTransactionBitmapRemove(actions,
-   domdisk->src->nodeformat,
-   chkdisk->bitmap) < 0)
+if (qemuCheckpointDiscardDiskBitmaps(domdisk->src, chkdisk->bitmap,
+ parentbitmap, chkcurrent, 
actions) < 0)
 return -1;
 }

diff --git a/src/qemu/qemu_checkpoint.h b/src/qemu/qemu_checkpoint.h
index eb85611ea6..85fd453d50 100644
--- a/src/qemu/qemu_checkpoint.h
+++ b/src/qemu/qemu_checkpoint.h
@@ -71,3 +71,10 @@ qemuCheckpointCreateFinalize(virQEMUDriverPtr driver,
 void
 qemuCheckpointRollbackMetadata(virDomainObjPtr vm,
virDomainMomentObjPtr chk);
+
+int
+qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src,
+ const char *delbitmap,
+ const char *parentbitmap,
+ bool chkcurrent,
+ virJSONValuePtr actions);
-- 

[libvirt] [RFC PATCH 08/16] qemu: checkpoint: Introduce helper to find checkpoint disk definition in parents

2020-01-09 Thread Peter Krempa
The algorithm is used in two places to find the parent checkpoint object
which contains given disk and then uses data from the disk. Additionally
the code is written in a very non-obvious way. Factor out the lookup of
the disk into a function which also simplifies the callers.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_checkpoint.c | 129 -
 1 file changed, 71 insertions(+), 58 deletions(-)

diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index 326707e098..1100f6e744 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -104,6 +104,50 @@ qemuCheckpointWriteMetadata(virDomainObjPtr vm,
 }


+/**
+ * qemuCheckpointFindActiveDiskInParent:
+ * @vm: domain object
+ * @from: starting moment object
+ * @diskname: name (target) of the disk to find
+ *
+ * Find the first checkpoint starting from @from continuing through parents
+ * of the checkpoint which describes disk @diskname. Return the pointer to the
+ * definition of the disk.
+ */
+static virDomainCheckpointDiskDef *
+qemuCheckpointFindActiveDiskInParent(virDomainObjPtr vm,
+ virDomainMomentObjPtr from,
+ const char *diskname)
+{
+virDomainMomentObjPtr parent = from;
+virDomainCheckpointDefPtr parentdef = NULL;
+size_t i;
+
+while (parent) {
+parentdef = virDomainCheckpointObjGetDef(parent);
+
+for (i = 0; i < parentdef->ndisks; i++) {
+virDomainCheckpointDiskDef *chkdisk = >disks[i];
+
+if (STRNEQ(chkdisk->name, diskname))
+continue;
+
+/* currently inspected checkpoint doesn't describe the disk,
+ * continue into parent checkpoint */
+if (chkdisk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
+break;
+
+return chkdisk;
+}
+
+parent = virDomainCheckpointFindByName(vm->checkpoints,
+   parentdef->parent.parent_name);
+}
+
+return NULL;
+}
+
+
 static int
 qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
  virDomainCheckpointDefPtr chkdef,
@@ -112,13 +156,9 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
 virQEMUDriverPtr driver = priv->driver;
-virDomainMomentObjPtr moment;
-virDomainCheckpointDefPtr parentdef = NULL;
-bool search_parents;
 int rc;
 g_autoptr(virJSONValue) actions = NULL;
 size_t i;
-size_t j;

 if (!(actions = virJSONValueNewArray()))
 return -1;
@@ -126,6 +166,7 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
 for (i = 0; i < chkdef->ndisks; i++) {
 virDomainCheckpointDiskDef *chkdisk = >disks[i];
 virDomainDiskDefPtr domdisk = virDomainDiskByTarget(vm->def, 
chkdisk->name);
+virDomainCheckpointDiskDef *parentchkdisk = NULL;

 /* domdisk can be missing e.g. when it was unplugged */
 if (!domdisk)
@@ -137,42 +178,28 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
 /* If any ancestor checkpoint has a bitmap for the same
  * disk, then this bitmap must be merged to the
  * ancestor. */
-search_parents = true;
-for (moment = parent;
- search_parents && moment;
- moment = virDomainCheckpointFindByName(vm->checkpoints,
-
parentdef->parent.parent_name)) {
-parentdef = virDomainCheckpointObjGetDef(moment);
-for (j = 0; j < parentdef->ndisks; j++) {
-virDomainCheckpointDiskDef *disk2;
-g_autoptr(virJSONValue) arr = NULL;
-
-disk2 = >disks[j];
-if (STRNEQ(chkdisk->name, disk2->name) ||
-disk2->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
-continue;
-search_parents = false;
-
-if (!(arr = virJSONValueNewArray()))
-return -1;
+if ((parentchkdisk = qemuCheckpointFindActiveDiskInParent(vm, parent, 
chkdisk->name))) {
+g_autoptr(virJSONValue) arr = NULL;

-if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr,
- 
domdisk->src->nodeformat,
- 
chkdisk->bitmap) < 0)
-return -1;
+if (!(arr = virJSONValueNewArray()))
+return -1;

-if (chkcurrent) {
-if (qemuMonitorTransactionBitmapEnable(actions,
-   
domdisk->src->nodeformat,
-   disk2->bitmap) < 0)
-return -1;
-}
+if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr,

[libvirt] [RFC PATCH 07/16] qemu: domain: Remove unused qemuDomainDiskNodeFormatLookup

2020-01-09 Thread Peter Krempa
The function has no users now and there's no need for it as the common
pattern is to look up the whole disk object anyways.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_domain.c | 14 --
 src/qemu/qemu_domain.h |  3 ---
 2 files changed, 17 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 24e84a5966..4966f5ef36 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -12048,20 +12048,6 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
 }


-/* Return the format node name for a given disk of an online guest */
-const char *
-qemuDomainDiskNodeFormatLookup(virDomainObjPtr vm,
-   const char *disk)
-{
-size_t i;
-
-for (i = 0; i < vm->def->ndisks; i++) {
-if (STREQ(vm->def->disks[i]->dst, disk))
-return vm->def->disks[i]->src->nodeformat;
-}
-return NULL;
-}
-
 bool
 qemuDomainDiskBlockJobIsActive(virDomainDiskDefPtr disk)
 {
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index c6afc484f6..4e2ddf6880 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -864,9 +864,6 @@ int qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
 bool qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
virDomainDiskDefPtr orig_disk);

-const char *qemuDomainDiskNodeFormatLookup(virDomainObjPtr vm,
-   const char *disk);
-
 void qemuDomainGetImageIds(virQEMUDriverConfigPtr cfg,
virDomainObjPtr vm,
virStorageSourcePtr src,
-- 
2.24.1

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



[libvirt] [RFC PATCH 01/16] qemu: checkpoint: Store whether deleted checkpoint is current in a variable

2020-01-09 Thread Peter Krempa
Avoid two computations by using a boolean.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_checkpoint.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index 2fa5c1ae00..d13d4c2a37 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -117,6 +117,7 @@ qemuCheckpointDiscard(virQEMUDriverPtr driver,
 size_t i, j;
 g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 g_autofree char *chkFile = NULL;
+bool chkcurrent = chk == virDomainCheckpointGetCurrent(vm->checkpoints);

 if (!metadata_only && !virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
@@ -172,7 +173,7 @@ qemuCheckpointDiscard(virQEMUDriverPtr driver,
 if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr, 
node, disk->bitmap) < 0)
 return -1;

-if (chk == virDomainCheckpointGetCurrent(vm->checkpoints)) 
{
+if (chkcurrent) {
 if (qemuMonitorTransactionBitmapEnable(actions, node, 
disk2->bitmap) < 0)
 return -1;
 }
@@ -192,7 +193,7 @@ qemuCheckpointDiscard(virQEMUDriverPtr driver,
 return -1;
 }

-if (chk == virDomainCheckpointGetCurrent(vm->checkpoints)) {
+if (chkcurrent) {
 virDomainCheckpointSetCurrent(vm->checkpoints, NULL);
 if (update_parent && parent) {
 virDomainCheckpointSetCurrent(vm->checkpoints, parent);
-- 
2.24.1

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



[libvirt] [RFC PATCH 00/16] qemu: checkpoint: Add support for deleting checkpoints accross snapshots

2020-01-09 Thread Peter Krempa
Posted as RFC because qemu probably doesn't reopen backing images for
bitmap operations resulting in:

 $ virsh checkpoint-delete vm --checkpointname a
 error: Failed to delete checkpoint a
 error: internal error: unable to execute QEMU command 'transaction': Bitmap 
'a' is readonly and cannot be modified

Unfortunately this can't be done manually because 'blockdev-reopen' is
still experimental.

Peter Krempa (16):
  qemu: checkpoint: Store whether deleted checkpoint is current in a
variable
  qemu: checkpoint: split out checkpoint deletion bitmaps
  qemu: checkpoint: rename disk->chkdisk in qemuCheckpointDiscardBitmaps
  qemu: checkpoint: rename disk->chkdisk in qemuCheckpointAddActions
  qemu: checkpoint: Use disk definition directly when creating
checkpoint
  qemu: checkpoint: tolerate missing disks on checkpoint deletion
  qemu: domain: Remove unused qemuDomainDiskNodeFormatLookup
  qemu: checkpoint: Introduce helper to find checkpoint disk definition
in parents
  qemu: checkpoint: Extract calculation of bitmap merging for checkpoint
deletion
  tests: qemublock: Add test for checkpoint deletion bitmap merge
  tests: qemublock: Add few more test cases for checkpoint deletion
  tests: qemublock: Add synthetic snapshot+checkpoint test data
  qemu: checkpoint: Introduce support for deleting checkpoints accross
snapshots
  tests: qemublock: Add checkpoint deletion test for deep backing chain
  tests: qemublock: Add checkpoint deletion tests for some special cases
  qemu: checkpoint: Track and relabel images for bitmap merging

 src/qemu/qemu_checkpoint.c| 329 +--
 src/qemu/qemu_checkpoint.h|   9 +
 src/qemu/qemu_domain.c|  14 -
 src/qemu/qemu_domain.h|   3 -
 tests/qemublocktest.c | 102 +++
 .../snapshots-synthetic-checkpoint.json   | 827 ++
 .../bitmap/snapshots-synthetic-checkpoint.out |  13 +
 .../checkpointdelete/basic-current-out.json   |  29 +
 .../basic-intermediate1-out.json  |  22 +
 .../basic-intermediate2-out.json  |  22 +
 .../basic-intermediate3-out.json  |  22 +
 .../checkpointdelete/basic-noparent-out.json  |   9 +
 .../snapshots-current-out.json|  29 +
 .../snapshots-intermediate1-out.json  |  24 +
 .../snapshots-intermediate2-out.json  |  62 ++
 .../snapshots-intermediate3-out.json  |  61 ++
 .../snapshots-noparent-out.json   |  27 +
 ...hots-synthetic-checkpoint-current-out.json |  29 +
 ...ynthetic-checkpoint-intermediate1-out.json |  31 +
 ...ynthetic-checkpoint-intermediate2-out.json |  34 +
 ...ynthetic-checkpoint-intermediate3-out.json |  61 ++
 ...ots-synthetic-checkpoint-noparent-out.json |  27 +
 22 files changed, 1680 insertions(+), 106 deletions(-)
 create mode 100644 
tests/qemublocktestdata/bitmap/snapshots-synthetic-checkpoint.json
 create mode 100644 
tests/qemublocktestdata/bitmap/snapshots-synthetic-checkpoint.out
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/basic-current-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/basic-intermediate1-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/basic-intermediate2-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/basic-intermediate3-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/basic-noparent-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-current-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-intermediate1-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-intermediate2-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-intermediate3-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-noparent-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-current-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate1-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate2-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-intermediate3-out.json
 create mode 100644 
tests/qemublocktestdata/checkpointdelete/snapshots-synthetic-checkpoint-noparent-out.json

-- 
2.24.1

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



[libvirt] [RFC PATCH 02/16] qemu: checkpoint: split out checkpoint deletion bitmaps

2020-01-09 Thread Peter Krempa
qemuCheckpointDiscard is a massive function that can be separated into
smaller bits. Extract the part that actually modifies the disk from the
metadata handling.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_checkpoint.c | 137 -
 1 file changed, 76 insertions(+), 61 deletions(-)

diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index d13d4c2a37..9ff3129570 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -104,6 +104,81 @@ qemuCheckpointWriteMetadata(virDomainObjPtr vm,
 }


+static int
+qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
+ virDomainCheckpointDefPtr chkdef,
+ bool chkcurrent,
+ virDomainMomentObjPtr parent)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+virQEMUDriverPtr driver = priv->driver;
+virDomainMomentObjPtr moment;
+virDomainCheckpointDefPtr parentdef = NULL;
+bool search_parents;
+int rc;
+g_autoptr(virJSONValue) actions = NULL;
+size_t i;
+size_t j;
+
+if (!(actions = virJSONValueNewArray()))
+return -1;
+
+for (i = 0; i < chkdef->ndisks; i++) {
+virDomainCheckpointDiskDef *disk = >disks[i];
+const char *node;
+
+if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
+continue;
+
+node = qemuDomainDiskNodeFormatLookup(vm, disk->name);
+/* If any ancestor checkpoint has a bitmap for the same
+ * disk, then this bitmap must be merged to the
+ * ancestor. */
+search_parents = true;
+for (moment = parent;
+ search_parents && moment;
+ moment = virDomainCheckpointFindByName(vm->checkpoints,
+
parentdef->parent.parent_name)) {
+parentdef = virDomainCheckpointObjGetDef(moment);
+for (j = 0; j < parentdef->ndisks; j++) {
+virDomainCheckpointDiskDef *disk2;
+g_autoptr(virJSONValue) arr = NULL;
+
+disk2 = >disks[j];
+if (STRNEQ(disk->name, disk2->name) ||
+disk2->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
+continue;
+search_parents = false;
+
+if (!(arr = virJSONValueNewArray()))
+return -1;
+
+if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr, 
node, disk->bitmap) < 0)
+return -1;
+
+if (chkcurrent) {
+if (qemuMonitorTransactionBitmapEnable(actions, node, 
disk2->bitmap) < 0)
+return -1;
+}
+
+if (qemuMonitorTransactionBitmapMerge(actions, node, 
disk2->bitmap, ) < 0)
+return -1;
+}
+}
+
+if (qemuMonitorTransactionBitmapRemove(actions, node, disk->bitmap) < 
0)
+return -1;
+}
+
+qemuDomainObjEnterMonitor(driver, vm);
+rc = qemuMonitorTransaction(priv->mon, );
+if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
+return -1;
+
+return 0;
+}
+
+
 static int
 qemuCheckpointDiscard(virQEMUDriverPtr driver,
   virDomainObjPtr vm,
@@ -112,9 +187,6 @@ qemuCheckpointDiscard(virQEMUDriverPtr driver,
   bool metadata_only)
 {
 virDomainMomentObjPtr parent = NULL;
-virDomainMomentObjPtr moment;
-virDomainCheckpointDefPtr parentdef = NULL;
-size_t i, j;
 g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 g_autofree char *chkFile = NULL;
 bool chkcurrent = chk == virDomainCheckpointGetCurrent(vm->checkpoints);
@@ -129,67 +201,10 @@ qemuCheckpointDiscard(virQEMUDriverPtr driver,
   chk->def->name);

 if (!metadata_only) {
-qemuDomainObjPrivatePtr priv = vm->privateData;
-bool search_parents;
 virDomainCheckpointDefPtr chkdef = virDomainCheckpointObjGetDef(chk);
-int rc;
-g_autoptr(virJSONValue) actions = NULL;
-
-if (!(actions = virJSONValueNewArray()))
-return -1;
-
 parent = virDomainCheckpointFindByName(vm->checkpoints,
chk->def->parent_name);
-for (i = 0; i < chkdef->ndisks; i++) {
-virDomainCheckpointDiskDef *disk = >disks[i];
-const char *node;
-
-if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
-continue;
-
-node = qemuDomainDiskNodeFormatLookup(vm, disk->name);
-/* If any ancestor checkpoint has a bitmap for the same
- * disk, then this bitmap must be merged to the
- * ancestor. */
-search_parents = true;
-for (moment = parent;
- search_parents && moment;
- moment = virDomainCheckpointFindByName(vm->checkpoints,
- 

[libvirt] [PATCH] tests: avoid re-execing test once for each mock

2020-01-09 Thread Daniel P . Berrangé
When debugging tests under GDB there is a significant time
delay each time an execve is done as GDB scans shared libraries
once again. For tests which use many mock libraries, we have
been invoking execve many times which makes the GDB experience
horrible. This changes our framework to activate the full
set of mock libraries in one single execve.

Signed-off-by: Daniel P. Berrangé 
---
 tests/qemucapsprobe.c |  8 +++-
 tests/testutils.c | 29 -
 tests/testutils.h | 10 +++---
 3 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/tests/qemucapsprobe.c b/tests/qemucapsprobe.c
index 6837f2a0f6..c7e8f3309d 100644
--- a/tests/qemucapsprobe.c
+++ b/tests/qemucapsprobe.c
@@ -46,8 +46,14 @@ main(int argc, char **argv)
 {
 virThread thread;
 virQEMUCapsPtr caps;
+const char *mock = VIR_TEST_MOCK("qemucapsprobe");
 
-VIR_TEST_PRELOAD(VIR_TEST_MOCK("qemucapsprobe"));
+if (!virFileIsExecutable(mock)) {
+perror(mock);
+return EXIT_FAILURE;
+}
+
+VIR_TEST_PRELOAD(mock);
 
 if (argc != 2) {
 fprintf(stderr, "%s QEMU_binary\n", argv[0]);
diff --git a/tests/testutils.c b/tests/testutils.c
index 6f0ef2b2f1..b490609e36 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -856,15 +856,34 @@ int virTestMain(int argc,
 virLogOutputPtr *outputs = NULL;
 g_autofree char *baseprogname = NULL;
 const char *progname;
-
-if (getenv("VIR_TEST_FILE_ACCESS"))
-VIR_TEST_PRELOAD(VIR_TEST_MOCK("virtest"));
+g_autofree const char **preloads = NULL;
+size_t npreloads = 0;
+g_autofree char *mock = NULL;
+
+if (getenv("VIR_TEST_FILE_ACCESS")) {
+preloads = g_renew(const char *, preloads, npreloads + 2);
+preloads[npreloads++] = VIR_TEST_MOCK("virtest");
+preloads[npreloads] = NULL;
+}
 
 va_start(ap, func);
-while ((lib = va_arg(ap, const char *)))
-VIR_TEST_PRELOAD(lib);
+while ((lib = va_arg(ap, const char *))) {
+if (!virFileIsExecutable(lib)) {
+perror(lib);
+return EXIT_FAILURE;
+}
+
+preloads = g_renew(const char *, preloads, npreloads + 2);
+preloads[npreloads++] = lib;
+preloads[npreloads] = NULL;
+}
 va_end(ap);
 
+if (preloads) {
+mock = g_strjoinv(":", (char **)preloads);
+VIR_TEST_PRELOAD(mock);
+}
+
 progname = baseprogname = g_path_get_basename(argv[0]);
 if (STRPREFIX(progname, "lt-"))
 progname += 3;
diff --git a/tests/testutils.h b/tests/testutils.h
index 8a7ea38f43..2944c25e8b 100644
--- a/tests/testutils.h
+++ b/tests/testutils.h
@@ -127,19 +127,15 @@ int virTestMain(int argc,
 # define MOCK_EXT ".so"
 #endif
 
-#define VIR_TEST_PRELOAD(lib) \
+#define VIR_TEST_PRELOAD(libs) \
 do { \
 const char *preload = getenv(PRELOAD_VAR); \
 if (preload == NULL || strstr(preload, lib) == NULL) { \
 char *newenv; \
-if (!virFileIsExecutable(lib)) { \
-perror(lib); \
-return EXIT_FAILURE; \
-} \
 if (!preload) { \
-newenv = (char *) lib; \
+newenv = (char *) libs; \
 } else { \
-newenv = g_strdup_printf("%s:%s", lib, preload); \
+newenv = g_strdup_printf("%s:%s", libs, preload); \
 } \
 g_setenv(PRELOAD_VAR, newenv, TRUE); \
 FORCE_FLAT_NAMESPACE \
-- 
2.24.1

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

Re: [libvirt] [PATCH v2 03/12] bootstrap.conf: drop gnulib tests from libvirt

2020-01-09 Thread Daniel P . Berrangé
On Thu, Jan 09, 2020 at 03:53:13PM +0100, Pavel Hrdina wrote:
> We are in process of removing gnulib and adopting meson as our build
> system.  In order to help with the transition let's drop gnulib tests.
> 
> This will also help with the fact that before we will be able to drop
> gnulib completely we will store output of bootstrap in git.
> 
> Signed-off-by: Pavel Hrdina 
> ---
>  .gitignore   |  2 --
>  Makefile.am  |  2 +-
>  bootstrap.conf   | 13 -
>  configure.ac |  1 -
>  gnulib/tests/Makefile.am | 32 
>  5 files changed, 1 insertion(+), 49 deletions(-)
>  delete mode 100644 gnulib/tests/Makefile.am

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

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

Re: [libvirt] [PATCH v2 02/12] configure.ac: add check for getegid function

2020-01-09 Thread Daniel P . Berrangé
On Thu, Jan 09, 2020 at 03:53:12PM +0100, Pavel Hrdina wrote:
> We already use this function and so far we've been lucky that the same
> check is done by gnulib.  This will change once we will drop gnulib and
> also make it obvious that we have to do the same check in Meson as well.
> 
> Signed-off-by: Pavel Hrdina 
> ---
>  configure.ac | 1 +
>  1 file changed, 1 insertion(+)

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

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

Re: [libvirt] [PATCH v2 01/12] secret: move virSecretGetSecretString into virsecret

2020-01-09 Thread Daniel P . Berrangé
On Thu, Jan 09, 2020 at 03:53:11PM +0100, Pavel Hrdina wrote:
> The function virSecretGetSecretString calls into secret driver and is
> used from other hypervisros drivers and as such makes more sense in
> util.

s/hypervisros/hypervisors/

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

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

Re: [libvirt] [PATCH] tests: avoid probing host CPU from bhyve test

2020-01-09 Thread Michal Privoznik

On 1/8/20 7:20 PM, Daniel P. Berrangé wrote:

bhyveargv2xmlmock calls virBhyveCapsBuild which in turn
calls virCPUProbeHost, probing the real host CPU. This
causes a test failure if the host CPU happens to contain
the 'arch-capabilities' feature as it triggers a call
to virHostCPUGetMSR() which fails on FreeBSD.

Fortunately we already have convenient code for mocking
the host CPU probing.

Signed-off-by: Daniel P. Berrangé 
---
  tests/bhyveargv2xmlmock.c | 8 
  1 file changed, 8 insertions(+)


With this squashed in:

diff --git i/src/cpu/cpu.h w/src/cpu/cpu.h
index 13909bb7a4..2e8b8923ae 100644
--- i/src/cpu/cpu.h
+++ w/src/cpu/cpu.h
@@ -193,7 +193,7 @@ virCPUGetHost(virArch arch,
   virDomainCapsCPUModelsPtr models);

 virCPUDefPtr
-virCPUProbeHost(virArch arch);
+virCPUProbeHost(virArch arch) G_GNUC_NO_INLINE;

 virCPUDefPtr
 virCPUBaseline(virArch arch,



Reviewed-by: Michal Privoznik 

Michal

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

Re: [libvirt] [PATCH] qemu: Don't use NULL path from qemuDomainGetHostdevPath

2020-01-09 Thread Michal Privoznik

On 1/9/20 3:50 PM, Jiri Denemark wrote:

Commit v5.10.0-290-g3a4787a301 refactored qemuDomainGetHostdevPath to
return a single path rather than an array of paths. When the function is
called on a missing device, it will now return NULL in @path rather than
a NULL array with zero items and the callers need to be adapted
properly.

Signed-off-by: Jiri Denemark 
---
  src/qemu/qemu_cgroup.c | 32 ++--
  src/qemu/qemu_domain.c |  9 +
  2 files changed, 23 insertions(+), 18 deletions(-)


Why do I have the feeling that I've seen this patch somewhere? ;-)

Reviewed-by: Michal Privoznik 

Michal

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



Re: [libvirt] [PATCH v3 0/5] introduce support for an embedded driver mode

2020-01-09 Thread Michal Privoznik

On 1/9/20 2:15 PM, Daniel P. Berrangé wrote:

On Wed, Jan 08, 2020 at 05:30:23PM +0100, Michal Privoznik wrote:

On 12/20/19 3:16 PM, Daniel P. Berrangé wrote:




Hm.. maybe I'm doing something wrong, but the following doesn't work for me.
Note, "fedora" is a VM with two disks:

 
   
   
   
   
   
 
 
   
   
 
 
   
 
 
   
 
   
   
   
 


libvirt.git/_build # ./tools/virsh -c qemu:///embed?root=/tmp/embed/
Welcome to virsh, the virtualization interactive terminal.

Type:  'help' for help with commands
'quit' to quit

virsh # list --all
  Id   Name State
-
  -fedora   shut off

virsh # connect secret:///embed?root=/tmp/embed


Ok, you're opening the secret driver in embedded mode

If you *also* open the QEMU driver now, it will use
this embedded secret driver directly.



virsh # secret-list
  UUID   Usage
-
  4cf62bac-6a9f-4b9a-ba33-8c4d96b0e2e4   iscsi iscsi-secret-pool


I guess you created the XML file for this secrete previously ?



virsh # connect qemu:///embed?root=/tmp/embed


Note that this now *closes* the existing connection, so the
embeded secret driver is now closed, and QEMU will speak to
libvirtd (or virtsecretd) for secrets now.

Basically virsh is not a suitable tool for using the
drivers in embedded mode since it is only capable of
opening a single driver connection at a time.


virsh # start fedora
2020-01-08 15:37:57.294+: 44566: info : libvirt version: 6.0.0
2020-01-08 15:37:57.294+: 44566: info : hostname: moe
2020-01-08 15:37:57.294+: 44566: warning : qemuDomainDefValidate:5835 :
CPU topology doesn't match numa CPU count; partial NUMA mapping is obsoleted
and will be removed in future
error: Failed to start domain fedora
error: internal error: URI must be secret:///embed


Oh, that's odd - it seems to be trying to access the embedded
secret driver but failing a URI sanity check. This is probably
a result of you previously opening & then closing the embedded
secret driver. This is not really a supported use case anyway.



Okay, since your program works, you have my:

Reviewed-by: Michal Privoznik 

for the series. However, as I suggested on the list, we should tell 
users explicitly that this feature is still under development and we may 
not be able to guarantee full backwards compatibility. May be worth 
putting somewhere into news.xml ;-)






However, running the domain (with the same disks) from regular URI is
impossible either:

libvirt.git/_build # ./tools/virsh -c qemu:///system start fedora
error: Failed to start domain fedora
error: internal error: no internalFlags support


This is because if the secret is private, then we don't want to allow
clients getting its value. And if running the monolithic daemon, the
conn->secretDrive is initialized to point right to the secret driver. But
when using split daemons, then the connection points to the remote secret
driver and virtqemud is then unable to obtain the secret value.
Unfortunately, I don't see a way around this. I mean other than allow
getting the value on RPC layer.


Basically we need to establish a trust relationship between
virtqemud and virtsecretd. I think we could relax this to mean a
trust relationship between virtsecretd and any client which is
running as the same user ID by default. A stronger trust relation
could be set using the fine grained polkit ACLs, with a ACL check
based on the API flag.


Yes, this is unrelated and pre-existing, so not a show stopper. We 
should probably track it somewhere though.


Michal

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

[libvirt] Designing XML for HMAT

2020-01-09 Thread Michal Privoznik

Dear list,

QEMU gained support for configuring HMAT recently (see 
v4.2.0-415-g9b12dfa03a
and friends). HMAT stands for Heterogeneous Memory Attribute Table and 
defines

various attributes to NUMA. Guest OS/app can read these information and fine
tune optimization. See [1] for more info (esp. links in the transcript).

QEMU defines so called initiator, which is an attribute to a NUMA node 
and if

specified points to another node that has the best performance to this node.

For instance:

  -machine hmat=on \
  -m 2G,slots=2,maxmem=4G \
  -object memory-backend-ram,size=1G,id=m0 \
  -object memory-backend-ram,size=1G,id=m1 \
  -numa node,nodeid=0,memdev=m0 \
  -numa node,nodeid=1,memdev=m1,initiator=0 \
  -smp 2,sockets=2,maxcpus=2 \
  -numa cpu,node-id=0,socket-id=0 \
  -numa cpu,node-id=0,socket-id=1

creates a machine with 2 NUMA nodes, node 0 has CPUs and node 1 has 
memory only

and it's initiator is node 0 (yes, HMAT allows you to create CPU-less "NUMA"
nodes). The initiator of node 0 is not specified, but since the node has at
least one CPU it is initiator to itself (and has to be per specs).

This could be represented by an attribute to our /domain/cpu/numa/cell 
element.

For instance like this:

  
2

  


  

  


Then, QEMU allows us to control two other important memory attributes:

  1) hmat-lb for Latency and Bandwidth

  2) hmat-cache for cache attributes

For example:

  -machine hmat=on \
  -m 2G,slots=2,maxmem=4G \
  -object memory-backend-ram,size=1G,id=m0 \
  -object memory-backend-ram,size=1G,id=m1 \
  -smp 2,sockets=2,maxcpus=2 \
  -numa node,nodeid=0,memdev=m0 \
  -numa node,nodeid=1,memdev=m1,initiator=0 \
  -numa cpu,node-id=0,socket-id=0 \
  -numa cpu,node-id=0,socket-id=1 \
  -numa 
hmat-lb,initiator=0,target=0,hierarchy=memory,data-type=access-latency,latency=5 
\
  -numa 
hmat-lb,initiator=0,target=0,hierarchy=memory,data-type=access-bandwidth,bandwidth=200M 
\
  -numa 
hmat-lb,initiator=0,target=1,hierarchy=memory,data-type=access-latency,latency=10 
\
  -numa 
hmat-lb,initiator=0,target=1,hierarchy=memory,data-type=access-bandwidth,bandwidth=100M 
\
  -numa 
hmat-cache,node-id=0,size=10K,level=1,associativity=direct,policy=write-back,line=8 
\
  -numa 
hmat-cache,node-id=1,size=10K,level=1,associativity=direct,policy=write-back,line=8


This extends previous example by defining some latencies and cache 
attributes.
The node 0 has access latency of 5 ns and bandwidth of 200MB/s and node 
1 has
access latency of 10ns and bandwidth of only 100MB/s. The memory cache 
level 1
on both nodes is 10KB, cache line is 8B long with write-back policy and 
direct

associativity (whatever that means).

For better future extensibility I'd express these as separate elements, 
rather

than attributes to  element. For instance like this:

  
2

  

  


  
  

  
  

  


  


  
  

  
  

  

  

  

Thing is, the @hierarchy argument accepts: memory (referring to whole 
memory),

or first-level|second-level|third-level (referring to side caches for each
domain). I haven't figured out yet, how to express the levels in XML yet.

The @data-type argument accepts access|read|write (this is expressed by 
@type
attribute to  and  elements). Latency and 
bandwidth can

be combined with each type: access-latency, read-latency, write-latency,
access-bandwidth, read-bandwidth, write-bandwidth. And these 6 can then be
combined with aforementioned @hierarchy, producing 24 combinations (if I 
read

qemu cmd line specs correctly [2]).



What are your thoughts?

Michal


1: https://bugzilla.redhat.com/show_bug.cgi?id=1786303
2: 
https://git.qemu.org/?p=qemu.git;a=blob;f=qemu-options.hx;h=d4b73ef60c1d4589148169ac658a34eee5f54522;hb=HEAD#l174


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



Re: [libvirt] backing file chains, -blockdev, and storage file autodetection

2020-01-09 Thread Peter Krempa
On Thu, Jan 09, 2020 at 14:47:22 +, Martin Wilck wrote:
> On Thu, 2020-01-09 at 14:34 +, Daniel P. Berrangé wrote:

[...]

> > 
> >  - Run qemu-img rebase to set the backing_fmt
> > 
> > or
> > 
> >  - Update the guest XML to set the  format value
> > 
> > or
> > 
> >  - Update the /etc/libvirt/qemu.conf to set capability_filters
> >to disable "blockdev"
> 
> I wasn't aware of this option, thanks. I'd actually looked for
> a way to revert libvirt's behavior to what it did in previous versions.

Please DO NOT use this in production. This is for debugging and
workarounds. This may break stuff in the future and is not officially
supported. The comment in the file should say so.

> 
> > Always assuming the format is raw certainly avoids the security
> > danger, but is unhelpful to users who relied on scenario (2).
> > 
> > I'm inclined to say we should make scenario (2)/(3) into a hard
> > error. Only allow scenario (1) to run normally.
> > 
> > ie, we should probe the disk format for the backing file, and
> > if it is *not* raw, then report an immediate error, with the
> > error message pointing to the kbase page explaining how to
> > fix this. This will help the 99% common case identify the
> > fix more quickly, with no obvious downside that I see.
> 
> Sounds good. I'd appreciate mention of the capability_filters option on
> the kbase page. 

No. That is not a proper fix unfortunately.

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



Re: [libvirt] backing file chains, -blockdev, and storage file autodetection

2020-01-09 Thread Daniel P . Berrangé
On Thu, Jan 09, 2020 at 02:47:22PM +, Martin Wilck wrote:
> On Thu, 2020-01-09 at 14:34 +, Daniel P. Berrangé wrote:
> > 
> > IIUC there are three scenarios at play here
> > 
> >  1. qcow2 file, first level raw backing
> >  2. qcow2 file, first level qcow2 backing, no  backing
> >  3. qcow2 file, first level qcow2 backing, with second level backing
> > 
> > Historically with the SELinux driver, if no backing_fmt is set,
> > then we blindly assume that scenario (1) is in effect.
> > 
> > We cannot tell QEMU that we treated it as scenario (1) though,
> > so QEMU will still probe format and potentially open the
> > first level backing as qcow2 still.
> > 
> > IOW, despite our SELinux & QEMU driver assumption for scenario (1),
> > scenario (2) would still suceed in the past.  Scenario (3) would
> > always have failed, because SELinux won't have labelled the second
> > level backing.
> 
> But it only failed with SELinux, right? Not every distro is mandating
> SELinux usage with libvirt. SUSE distros don't, for example.

No, that's just one example. It should also fail with AppArmor,
and should also fail with system libvirtd, since libvirt DAC
driver chowns files from root -> qemu user ID.

Only case where all three scenarios would work are if using
the session libvirtd as an unprivileged user, or if using
systemd libvirtd with QEMU configured to run as root (a
really bad idea).

> 
> > Essentially scenario (2) worked by accident, not design, but
> > IIUC we have not been warning users about this.
> > 
> > With new libvirt and -blockdev we still assume the backing_fmt
> > is raw if not set in the SELinux driver, but we now have the
> > ability to tell QEMU about our assumption via -blockdev. As
> > such we've not ensured scenario (2) fails.
> > 
> > 
> > Users who were silently relying on scenario (2) are now broken
> > and have three options IIUC
> > 
> >  - Run qemu-img rebase to set the backing_fmt
> > 
> > or
> > 
> >  - Update the guest XML to set the  format value
> > 
> > or
> > 
> >  - Update the /etc/libvirt/qemu.conf to set capability_filters
> >to disable "blockdev"
> 
> I wasn't aware of this option, thanks. I'd actually looked for
> a way to revert libvirt's behavior to what it did in previous versions.

Note, that we call that a hack. There's no guarantee it will
continue working correctly long term & there's no testing of
it in general.

So at most it should be used as a short term solution until you
can use qemu-img rebase or update the XML backingStore for all
affected VM.

> 
> > Always assuming the format is raw certainly avoids the security
> > danger, but is unhelpful to users who relied on scenario (2).
> > 
> > I'm inclined to say we should make scenario (2)/(3) into a hard
> > error. Only allow scenario (1) to run normally.
> > 
> > ie, we should probe the disk format for the backing file, and
> > if it is *not* raw, then report an immediate error, with the
> > error message pointing to the kbase page explaining how to
> > fix this. This will help the 99% common case identify the
> > fix more quickly, with no obvious downside that I see.
> 
> Sounds good. I'd appreciate mention of the capability_filters option on
> the kbase page. 

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

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

Re: [libvirt] backing file chains, -blockdev, and storage file autodetection

2020-01-09 Thread Martin Wilck
On Wed, 2020-01-08 at 15:49 +, Daniel P. Berrangé wrote:
> 
> I wonder if there's any scope for improving qemu-img here so that it
> can 'do the right thing', or at least issue a warning if the suer
> doesn't set a backing format. 

Yes, someone should send a patch that would make qemu-img print a big
fat warning when a backing file was specified without "-F".

Regards,
Martin

-- 
Dr. Martin Wilck , Tel. +49 (0)911 74053 2107
SUSE  Software Solutions Germany GmbH
HRB 36809, AG Nürnberg GF: Felix
Imendörffer



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



Re: [libvirt] backing file chains, -blockdev, and storage file autodetection

2020-01-09 Thread Martin Wilck
On Thu, 2020-01-09 at 14:34 +, Daniel P. Berrangé wrote:
> 
> IIUC there are three scenarios at play here
> 
>  1. qcow2 file, first level raw backing
>  2. qcow2 file, first level qcow2 backing, no  backing
>  3. qcow2 file, first level qcow2 backing, with second level backing
> 
> Historically with the SELinux driver, if no backing_fmt is set,
> then we blindly assume that scenario (1) is in effect.
> 
> We cannot tell QEMU that we treated it as scenario (1) though,
> so QEMU will still probe format and potentially open the
> first level backing as qcow2 still.
> 
> IOW, despite our SELinux & QEMU driver assumption for scenario (1),
> scenario (2) would still suceed in the past.  Scenario (3) would
> always have failed, because SELinux won't have labelled the second
> level backing.

But it only failed with SELinux, right? Not every distro is mandating
SELinux usage with libvirt. SUSE distros don't, for example.

> Essentially scenario (2) worked by accident, not design, but
> IIUC we have not been warning users about this.
> 
> With new libvirt and -blockdev we still assume the backing_fmt
> is raw if not set in the SELinux driver, but we now have the
> ability to tell QEMU about our assumption via -blockdev. As
> such we've not ensured scenario (2) fails.
> 
> 
> Users who were silently relying on scenario (2) are now broken
> and have three options IIUC
> 
>  - Run qemu-img rebase to set the backing_fmt
> 
> or
> 
>  - Update the guest XML to set the  format value
> 
> or
> 
>  - Update the /etc/libvirt/qemu.conf to set capability_filters
>to disable "blockdev"

I wasn't aware of this option, thanks. I'd actually looked for
a way to revert libvirt's behavior to what it did in previous versions.

> Always assuming the format is raw certainly avoids the security
> danger, but is unhelpful to users who relied on scenario (2).
> 
> I'm inclined to say we should make scenario (2)/(3) into a hard
> error. Only allow scenario (1) to run normally.
> 
> ie, we should probe the disk format for the backing file, and
> if it is *not* raw, then report an immediate error, with the
> error message pointing to the kbase page explaining how to
> fix this. This will help the 99% common case identify the
> fix more quickly, with no obvious downside that I see.

Sounds good. I'd appreciate mention of the capability_filters option on
the kbase page. 

Thanks,
Martin


-- 
Dr. Martin Wilck , Tel. +49 (0)911 74053 2107
SUSE  Software Solutions Germany GmbH
HRB 36809, AG Nürnberg GF: Felix
Imendörffer



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



[libvirt] [PATCH v2 09/12] autogen.sh: fix autoreconf step

2020-01-09 Thread Pavel Hrdina
Running bootstrap and autoreconf from autogen.sh produced different
files in build-aux directory.  The reason is that gnulib usually have
newer version of these files and overwrites them after the autoreconf
step.

In order to fix it remove the --install and --force options, in addition
introduce --verbose option in order to reflect what bootstrap is doing.

Signed-off-by: Pavel Hrdina 
---
 autogen.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/autogen.sh b/autogen.sh
index a1f1943adb..47446dc785 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -134,7 +134,7 @@ if test -d .git || test -f .git; then
 # dry run, of course...
 if test -z "$dry_run"; then
 echo "Running autoreconf..."
-autoreconf -if || {
+autoreconf -v || {
 die "autoreconf failed"
 }
 fi
-- 
2.24.1

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



[libvirt] [PATCH v2 00/12] include result of bootstrap in git

2020-01-09 Thread Pavel Hrdina
This patch series is motivated by the effort to adopt Meson as our
build system but all of it improves the current build system on its own.

It moves the burden to run bootstrap/autoreconf to developers which
means it will save some time for others when there is any change to
the build system.

Since one of the patches is slightly bigger here is link to git:

 branch 

Changes in v2:
- some patches removed

Pavel Hrdina (12):
  secret: move virSecretGetSecretString into virsecret
  configure.ac: add check for getegid function
  bootstrap.conf: drop gnulib tests from libvirt
  bootstrap.conf: always copy files
  bootstrap.conf: declare bootstrap sync in configuration file
  bootstrap.conf: disable VC ignore files
  bootstrap.conf: stop creating AUTHORS file
  syntax-check: remove deleted daemon directory from space_indent_check
  autogen.sh: fix autoreconf step
  gnulib: store result of bootstrap in git
  autogen.sh: remove --dry-run option
  autogen.sh: remove --no-git and --gnulib-srcdir options

 .git-module-status | 2 +
 .gitignore |18 -
 INSTALL|   368 +
 Makefile.am| 2 +-
 Makefile.in|  2193 +
 aclocal.m4 |  1717 +
 autogen.sh |82 +-
 bootstrap.conf |27 +-
 build-aux/compile  |   348 +
 build-aux/config.guess |  1667 +
 build-aux/config.sub   |  1793 +
 build-aux/depcomp  |   791 +
 build-aux/install-sh   |   529 +
 build-aux/ltmain.sh| 11149 
 build-aux/missing  |   215 +
 build-aux/syntax-check.mk  |23 +-
 build-aux/test-driver  |   148 +
 config.h.in|  2288 +
 configure  | 55547 +++
 configure.ac   | 2 +-
 docs/Makefile.in   |  2484 +
 docs/compiling.html.in |13 -
 examples/Makefile.in   |  2364 +
 gnulib/lib/Makefile.in |  3705 ++
 gnulib/lib/_Noreturn.h |40 +
 gnulib/lib/accept.c|52 +
 gnulib/lib/alloca.c|   478 +
 gnulib/lib/alloca.in.h |71 +
 gnulib/lib/arg-nonnull.h   |26 +
 gnulib/lib/arpa_inet.in.h  |   150 +
 gnulib/lib/asnprintf.c |34 +
 gnulib/lib/assure.h|37 +
 gnulib/lib/binary-io.c |39 +
 gnulib/lib/binary-io.h |77 +
 gnulib/lib/bind.c  |49 +
 gnulib/lib/c++defs.h   |   316 +
 gnulib/lib/chown.c |   151 +
 gnulib/lib/cloexec.c   |83 +
 gnulib/lib/cloexec.h   |38 +
 gnulib/lib/close.c |71 +
 gnulib/lib/connect.c   |56 +
 gnulib/lib/dup2.c  |   235 +
 gnulib/lib/errno.in.h  |   279 +
 gnulib/lib/fchown-stub.c   |16 +
 gnulib/lib/fcntl.c |   627 +
 gnulib/lib/fcntl.in.h  |   392 +
 gnulib/lib/fd-hook.c   |   116 +
 gnulib/lib/fd-hook.h   |   119 +
 gnulib/lib/filename.h  |54 +
 gnulib/lib/float+.h|   147 +
 gnulib/lib/float.c |33 +
 gnulib/lib/float.in.h  |   188 +
 gnulib/lib/fseek.c |30 +
 gnulib/lib/fseeko.c|   164 +
 gnulib/lib/fstat.c |94 +
 gnulib/lib/gai_strerror.c  |92 +
 gnulib/lib/getaddrinfo.c   |   473 +
 gnulib/lib/getdelim.c  |   147 +
 gnulib/lib/getdtablesize.c |   124 +
 gnulib/lib/getgroups.c |   131 +
 gnulib/lib/gethostname.c   |   104 +
 gnulib/lib/getline.c   |27 +
 gnulib/lib/getpass.c   |   231 +
 gnulib/lib/getpass.h   | 4 +
 gnulib/lib/getpeername.c   |49 +
 gnulib/lib/getsockname.c   |49 +
 gnulib/lib/gettext.h   |   301 +
 gnulib/lib/getugroups.c|   128 +
 gnulib/lib/getugroups.h|19 +
 gnulib/lib/glthread/threadlib.c|73 +
 gnulib/lib/gnulib.mk   |  2629 +
 gnulib/lib/inet_ntop.c |   260 +
 gnulib/lib/intprops.h 

[libvirt] [PATCH v2 10/12] gnulib: store result of bootstrap in git

2020-01-09 Thread Pavel Hrdina
There is no need to run bootstrap every time a clean git checkout is
done as the result will be always the same.  We only need to run it
when there is some change to gnulib modules or when we update to new
gnulib or there is a change to the build system.  All of the steps
above are done as by developers and therefore it can be part of git.

The benefits are that from this point nothing will modify source dir
when building libvirt, the configure time will be way quicker and it
helps with the planned adoption of Meson as our build system.

The drawbacks are that developers will have to regenerate these files
when doing changes mentioned above and there is a significant amount
of files and code lines part of our git checkout.  However, with the
ongoing effort to eliminate gnulib in favor of GLib all of the gnulib
code will be eventually dropped.

Signed-off-by: Pavel Hrdina 
---

This patch is truncated as it's too big, you can get it from git
repository mentioned in cover latter.

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



[libvirt] [PATCH v2 06/12] bootstrap.conf: disable VC ignore files

2020-01-09 Thread Pavel Hrdina
We already ignore most of these files and the .gitignore files as well.

Signed-off-by: Pavel Hrdina 
---
 .gitignore | 5 -
 bootstrap.conf | 3 +++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index 2139d176da..949bd3bc5a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -14,9 +14,12 @@
 /INSTALL
 /aclocal.m4
 /autom4te.cache
-/build-aux/.gitignore
 /build-aux/compile
+/build-aux/config.guess
+/build-aux/config.sub
 /build-aux/depcomp
+/build-aux/install-sh
+/build-aux/ltmain.sh
 /build-aux/missing
 /build-aux/test-driver
 /config.h.in
diff --git a/bootstrap.conf b/bootstrap.conf
index 5e2e7b1310..43f762da1e 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -131,6 +131,8 @@ copy=true
 
 bootstrap_sync=true
 
+vc_ignore=
+
 
 # Tell gnulib to:
 #   require LGPLv2+
@@ -145,6 +147,7 @@ gnulib_tool_option_extras="\
  --lgpl=2\
  --makefile-name=gnulib.mk\
  --avoid=pt_chown\
+ --no-vc-files\
 "
 local_gl_dir=gnulib/local
 
-- 
2.24.1

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



[libvirt] [PATCH v2 12/12] autogen.sh: remove --no-git and --gnulib-srcdir options

2020-01-09 Thread Pavel Hrdina
Now that we have bootstrap output stored in git there is no need for
these options.

Signed-off-by: Pavel Hrdina 
---
 autogen.sh | 29 +++--
 docs/compiling.html.in | 13 -
 2 files changed, 3 insertions(+), 39 deletions(-)

diff --git a/autogen.sh b/autogen.sh
index f20a61ed29..f947c3f963 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -20,24 +20,9 @@ test -f src/libvirt.c || {
 die "$0 must live in the top-level libvirt directory"
 }
 
-no_git=
-gnulib_srcdir=
 extra_args=
 while test "$#" -gt 0; do
 case "$1" in
---no-git)
-no_git=" $1"
-shift
-;;
---gnulib-srcdir=*)
-gnulib_srcdir=" $1"
-shift
-;;
---gnulib-srcdir)
-gnulib_srcdir=" $1=$2"
-shift
-shift
-;;
 --system)
 prefix=/usr
 sysconfdir=/etc
@@ -57,17 +42,9 @@ while test "$#" -gt 0; do
 ;;
 esac
 done
-no_git="$no_git$gnulib_srcdir"
 
 gnulib_hash()
 {
-local no_git=$1
-
-if test "$no_git"; then
-echo "no-git"
-return
-fi
-
 # Compute the hash we'll use to determine whether rerunning bootstrap
 # is required. The first is just the SHA1 that selects a gnulib snapshot.
 # The second ensures that whenever we change the set of gnulib modules used
@@ -94,7 +71,7 @@ if test -d .git || test -f .git; then
 esac
 done
 fi
-if test "$CLEAN_SUBMODULE" && test -z "$no_git"; then
+if test "$CLEAN_SUBMODULE"; then
 echo "Cleaning up submodules..."
 git submodule foreach 'git clean -dfqx && git reset --hard' || {
 die "Cleaning up submodules failed"
@@ -113,7 +90,7 @@ if test -d .git || test -f .git; then
 # successful bootstrap run, is stored on disk
 state_file=.git-module-status
 expected_hash=$(cat "$state_file" 2>/dev/null)
-actual_hash=$(gnulib_hash "$no_git")
+actual_hash=$(gnulib_hash)
 
 if test "$actual_hash" = "$expected_hash"; then
 # The gnulib hash matches our expectations, and all the files
@@ -128,7 +105,7 @@ if test -d .git || test -f .git; then
 # has been changed in some way (see gnulib_hash) we need to
 # run bootstrap again.
 echo "Running bootstrap..."
-./bootstrap$no_git || {
+./bootstrap || {
 die "bootstrap failed"
 }
 gnulib_hash >"$state_file"
diff --git a/docs/compiling.html.in b/docs/compiling.html.in
index 5869ebb90f..31833decc6 100644
--- a/docs/compiling.html.in
+++ b/docs/compiling.html.in
@@ -82,19 +82,6 @@ $ sudo make install
   disk space requirements and network download time, regardless of
   which actual commit you have in that reference directory.
 
-
-  However, if you are developing on a platform where git is not
-  available, or are behind a firewall that does not allow for git
-  to easily obtain the gnulib submodule, it is possible to instead
-  use a static mode of operation where you are then responsible
-  for updating the git submodule yourself.  In this mode, you must
-  track the exact gnulib commit needed by libvirt (usually not the
-  latest gnulib.git) via alternative means, such as a shared NFS
-  drive or manual download, and run this any time libvirt.git
-  updates the commit stored in the .gnulib submodule:
-
-$ GNULIB_SRCDIR=/path/to/gnulib ./autogen.sh --no-git
-
 
 To build  install libvirt to your home
   directory the following commands can be run:
-- 
2.24.1

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



[libvirt] [PATCH v2 05/12] bootstrap.conf: declare bootstrap sync in configuration file

2020-01-09 Thread Pavel Hrdina
Signed-off-by: Pavel Hrdina 
---
 autogen.sh | 2 +-
 bootstrap.conf | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/autogen.sh b/autogen.sh
index 9afad8f9d5..a119b099cb 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -147,7 +147,7 @@ if test -d .git || test -f .git; then
 dry_run=2
 else
 echo "Running bootstrap..."
-./bootstrap$no_git --bootstrap-sync || {
+./bootstrap$no_git || {
 die "bootstrap failed"
 }
 gnulib_hash >"$state_file"
diff --git a/bootstrap.conf b/bootstrap.conf
index 3798284dad..5e2e7b1310 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -129,6 +129,8 @@ SKIP_PO=true
 
 copy=true
 
+bootstrap_sync=true
+
 
 # Tell gnulib to:
 #   require LGPLv2+
-- 
2.24.1

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



[libvirt] [PATCH v2 08/12] syntax-check: remove deleted daemon directory from space_indent_check

2020-01-09 Thread Pavel Hrdina
Signed-off-by: Pavel Hrdina 
---
 build-aux/syntax-check.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index 7e7c59c3df..b2f4bf59f8 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -572,7 +572,7 @@ sc_size_of_brackets:
 # Ensure that no C source file, docs, or rng schema uses TABs for
 # indentation.  Also match *.h.in files, to get libvirt.h.in.  Exclude
 # files in gnulib, since they're imported.
-space_indent_files=(\.(aug(\.in)?|rng|s?[ch](\.in)?|html.in|py|pl|syms)|(daemon|tools)/.*\.in)
+space_indent_files=(\.(aug(\.in)?|rng|s?[ch](\.in)?|html.in|py|pl|syms)|tools/.*\.in)
 sc_TAB_in_indentation:
@prohibit='^ *  ' \
in_vc_files='$(space_indent_files)$$' \
-- 
2.24.1

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



[libvirt] [PATCH v2 02/12] configure.ac: add check for getegid function

2020-01-09 Thread Pavel Hrdina
We already use this function and so far we've been lucky that the same
check is done by gnulib.  This will change once we will drop gnulib and
also make it obvious that we have to do the same check in Meson as well.

Signed-off-by: Pavel Hrdina 
---
 configure.ac | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configure.ac b/configure.ac
index 002a3dcdb0..cbfb3fe1aa 100644
--- a/configure.ac
+++ b/configure.ac
@@ -355,6 +355,7 @@ dnl and various less common threadsafe functions
 AC_CHECK_FUNCS_ONCE([\
   cfmakeraw \
   fallocate \
+  getegid \
   geteuid \
   getgid \
   getifaddrs \
-- 
2.24.1

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



[libvirt] [PATCH v2 11/12] autogen.sh: remove --dry-run option

2020-01-09 Thread Pavel Hrdina
Signed-off-by: Pavel Hrdina 
---
 autogen.sh | 53 +++--
 1 file changed, 15 insertions(+), 38 deletions(-)

diff --git a/autogen.sh b/autogen.sh
index 47446dc785..f20a61ed29 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -20,21 +20,11 @@ test -f src/libvirt.c || {
 die "$0 must live in the top-level libvirt directory"
 }
 
-dry_run=
 no_git=
 gnulib_srcdir=
 extra_args=
 while test "$#" -gt 0; do
 case "$1" in
---dry-run)
-# This variable will serve both as an indicator of the fact that
-# a dry run has been requested, and to store the result of the
-# dry run. It will be ultimately used as return code for the
-# script: 0 means no action is necessary, 2 means that autogen.sh
-# needs to be executed, and 1 is reserved for failures
-dry_run=0
-shift
-;;
 --no-git)
 no_git=" $1"
 shift
@@ -105,12 +95,10 @@ if test -d .git || test -f .git; then
 done
 fi
 if test "$CLEAN_SUBMODULE" && test -z "$no_git"; then
-if test -z "$dry_run"; then
-echo "Cleaning up submodules..."
-git submodule foreach 'git clean -dfqx && git reset --hard' || {
-die "Cleaning up submodules failed"
-}
-fi
+echo "Cleaning up submodules..."
+git submodule foreach 'git clean -dfqx && git reset --hard' || {
+die "Cleaning up submodules failed"
+}
 fi
 
 # Update all submodules. If any of the submodules has not been
@@ -130,34 +118,23 @@ if test -d .git || test -f .git; then
 if test "$actual_hash" = "$expected_hash"; then
 # The gnulib hash matches our expectations, and all the files
 # that can only be generated through bootstrap are present:
-# we just need to run autoreconf. Unless we're performing a
-# dry run, of course...
-if test -z "$dry_run"; then
-echo "Running autoreconf..."
-autoreconf -v || {
-die "autoreconf failed"
-}
-fi
+# we just need to run autoreconf.
+echo "Running autoreconf..."
+autoreconf -v || {
+die "autoreconf failed"
+}
 else
 # Whenever the gnulib submodule or any of the related bits
 # has been changed in some way (see gnulib_hash) we need to
-# run bootstrap again. If we're performing a dry run, we
-# change the return code instead to signal our caller
-if test "$dry_run"; then
-dry_run=2
-else
-echo "Running bootstrap..."
-./bootstrap$no_git || {
-die "bootstrap failed"
-}
-gnulib_hash >"$state_file"
-fi
+# run bootstrap again.
+echo "Running bootstrap..."
+./bootstrap$no_git || {
+die "bootstrap failed"
+}
+gnulib_hash >"$state_file"
 fi
 fi
 
-# When performing a dry run, we can stop here
-test "$dry_run" && exit "$dry_run"
-
 # If asked not to run configure, we can stop here
 test "$NOCONFIGURE" && exit 0
 
-- 
2.24.1

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



[libvirt] [PATCH v2 01/12] secret: move virSecretGetSecretString into virsecret

2020-01-09 Thread Pavel Hrdina
The function virSecretGetSecretString calls into secret driver and is
used from other hypervisros drivers and as such makes more sense in
util.

Signed-off-by: Pavel Hrdina 
---
 po/POTFILES.in |   1 -
 src/libvirt_private.syms   |   5 +-
 src/libxl/libxl_conf.c |   2 +-
 src/qemu/qemu_domain.c |   2 +-
 src/qemu/qemu_process.c|   2 +-
 src/qemu/qemu_tpm.c|   2 +-
 src/secret/Makefile.inc.am |  11 ---
 src/secret/secret_util.c   | 102 -
 src/secret/secret_util.h   |  33 ---
 src/storage/storage_backend_iscsi.c|   2 +-
 src/storage/storage_backend_iscsi_direct.c |   2 +-
 src/storage/storage_backend_rbd.c  |   2 +-
 src/storage/storage_util.c |   2 +-
 src/util/virsecret.c   |  69 ++
 src/util/virsecret.h   |   8 ++
 15 files changed, 86 insertions(+), 159 deletions(-)
 delete mode 100644 src/secret/secret_util.c
 delete mode 100644 src/secret/secret_util.h

diff --git a/po/POTFILES.in b/po/POTFILES.in
index faf173584e..e266871907 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -190,7 +190,6 @@
 @SRCDIR@/src/rpc/virnetsshsession.c
 @SRCDIR@/src/rpc/virnettlscontext.c
 @SRCDIR@/src/secret/secret_driver.c
-@SRCDIR@/src/secret/secret_util.c
 @SRCDIR@/src/security/security_apparmor.c
 @SRCDIR@/src/security/security_dac.c
 @SRCDIR@/src/security/security_driver.c
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b97906b852..b1a56f1261 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1449,10 +1449,6 @@ virLogManagerFree;
 virLogManagerNew;
 
 
-# secret/secret_util.h
-virSecretGetSecretString;
-
-
 # security/security_driver.h
 virSecurityDriverLookup;
 
@@ -3001,6 +2997,7 @@ virSecurityLabelDefNew;
 
 
 # util/virsecret.h
+virSecretGetSecretString;
 virSecretLookupDefClear;
 virSecretLookupDefCopy;
 virSecretLookupFormatSecret;
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 2488bb9d32..e41e84e3e2 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -41,7 +41,7 @@
 #include "libxl_conf.h"
 #include "libxl_utils.h"
 #include "virstoragefile.h"
-#include "secret_util.h"
+#include "virsecret.h"
 #include "cpu/cpu.h"
 #include "xen_common.h"
 #include "xen_xl.h"
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 24e84a5966..ec8207b36f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -56,7 +56,7 @@
 #include "vircrypto.h"
 #include "virrandom.h"
 #include "virsystemd.h"
-#include "secret_util.h"
+#include "virsecret.h"
 #include "logging/log_manager.h"
 #include "locking/domain_lock.h"
 #include "virdomainsnapshotobjlist.h"
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 4195042194..3c2f2458b5 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -83,7 +83,7 @@
 #include "virnuma.h"
 #include "virstring.h"
 #include "virhostdev.h"
-#include "secret_util.h"
+#include "virsecret.h"
 #include "configmake.h"
 #include "nwfilter_conf.h"
 #include "netdev_bandwidth_conf.h"
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 28800a100c..262e6c4f07 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -42,7 +42,7 @@
 #include "configmake.h"
 #include "qemu_tpm.h"
 #include "virtpm.h"
-#include "secret_util.h"
+#include "virsecret.h"
 
 #define VIR_FROM_THIS VIR_FROM_NONE
 
diff --git a/src/secret/Makefile.inc.am b/src/secret/Makefile.inc.am
index d332060e38..4f0956a7a4 100644
--- a/src/secret/Makefile.inc.am
+++ b/src/secret/Makefile.inc.am
@@ -5,11 +5,6 @@ SECRET_DRIVER_SOURCES = \
secret/secret_driver.c \
$(NULL)
 
-SECRET_UTIL_SOURCES = \
-   secret/secret_util.h \
-   secret/secret_util.c \
-   $(NULL)
-
 
 DRIVER_SOURCE_FILES += $(addprefix $(srcdir)/,$(SECRET_DRIVER_SOURCES))
 STATEFUL_DRIVER_SOURCE_FILES += \
@@ -17,14 +12,8 @@ STATEFUL_DRIVER_SOURCE_FILES += \
 
 EXTRA_DIST += \
$(SECRET_DRIVER_SOURCES) \
-   $(SECRET_UTIL_SOURCES) \
$(NULL)
 
-noinst_LTLIBRARIES += libvirt_secret.la
-libvirt_la_BUILT_LIBADD += libvirt_secret.la
-libvirt_secret_la_CFLAGS = $(AM_CFLAGS)
-libvirt_secret_la_LDFLAGS = $(AM_LDFLAGS)
-libvirt_secret_la_SOURCES = $(SECRET_UTIL_SOURCES)
 
 if WITH_SECRETS
 mod_LTLIBRARIES += libvirt_driver_secret.la
diff --git a/src/secret/secret_util.c b/src/secret/secret_util.c
deleted file mode 100644
index 27e164a425..00
--- a/src/secret/secret_util.c
+++ /dev/null
@@ -1,102 +0,0 @@
-/*
- * secret_util.c: secret related utility functions
- *
- * Copyright (C) 2016 Red Hat, Inc.
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the 

[libvirt] [PATCH v2 03/12] bootstrap.conf: drop gnulib tests from libvirt

2020-01-09 Thread Pavel Hrdina
We are in process of removing gnulib and adopting meson as our build
system.  In order to help with the transition let's drop gnulib tests.

This will also help with the fact that before we will be able to drop
gnulib completely we will store output of bootstrap in git.

Signed-off-by: Pavel Hrdina 
---
 .gitignore   |  2 --
 Makefile.am  |  2 +-
 bootstrap.conf   | 13 -
 configure.ac |  1 -
 gnulib/tests/Makefile.am | 32 
 5 files changed, 1 insertion(+), 49 deletions(-)
 delete mode 100644 gnulib/tests/Makefile.am

diff --git a/.gitignore b/.gitignore
index 2d6e3e3194..2139d176da 100644
--- a/.gitignore
+++ b/.gitignore
@@ -27,12 +27,10 @@ Makefile.in
 
 # gnulib related ignores
 !/gnulib/lib/Makefile.am
-!/gnulib/tests/Makefile.am
 *.rej
 *~
 /gnulib/lib/*
 /gnulib/m4/*
-/gnulib/tests/*
 
 # git related ignores
 *.orig
diff --git a/Makefile.am b/Makefile.am
index 0d7ccc74db..8a8eecb697 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -23,7 +23,7 @@ GENHTML = genhtml
 # so force it explicitly
 DISTCHECK_CONFIGURE_FLAGS = --enable-werror
 
-SUBDIRS = . gnulib/lib include/libvirt src tools docs gnulib/tests \
+SUBDIRS = . gnulib/lib include/libvirt src tools docs \
   tests po examples
 
 XZ_OPT ?= -v -T0
diff --git a/bootstrap.conf b/bootstrap.conf
index ae9ecb4039..13d0e77514 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -143,13 +143,10 @@ fi
 gnulib_name=libgnu
 m4_base=m4
 source_base=gnulib/lib
-tests_base=gnulib/tests
 gnulib_tool_option_extras="\
  --lgpl=2\
- --with-tests\
  --makefile-name=gnulib.mk\
  --avoid=pt_chown\
- --avoid=lock-tests\
 "
 local_gl_dir=gnulib/local
 
@@ -184,16 +181,6 @@ gnulib_extra_files="
 doc/INSTALL
 "
 
-
-bootstrap_post_import_hook()
-{
-  # Change paths in gnulib/tests/gnulib.mk from "../../.." to "../..",
-  # and make tests conditional by changing "TESTS" to "GNULIB_TESTS".
-  m=gnulib/tests/gnulib.mk
-  sed 's,\.\./\.\./\.\.,../..,g; s/^TESTS /GNULIB_TESTS /' $m > $m-t
-  mv -f $m-t $m
-}
-
 bootstrap_epilogue()
 {
 echo "$0: done.  Now you can run 'mkdir build && cd build && 
../configure'."
diff --git a/configure.ac b/configure.ac
index cbfb3fe1aa..2be909b7ce 100644
--- a/configure.ac
+++ b/configure.ac
@@ -922,7 +922,6 @@ AC_CONFIG_FILES([run],
 AC_CONFIG_FILES([\
 Makefile src/Makefile include/libvirt/Makefile docs/Makefile \
 gnulib/lib/Makefile \
-gnulib/tests/Makefile \
 .color_coded \
 .ycm_extra_conf.py \
 libvirt.pc \
diff --git a/gnulib/tests/Makefile.am b/gnulib/tests/Makefile.am
deleted file mode 100644
index 7062cbaf87..00
--- a/gnulib/tests/Makefile.am
+++ /dev/null
@@ -1,32 +0,0 @@
-## Makefile for gnulib/lib
-
-## Copyright (C) 2011, 2013 Red Hat, Inc.
-##
-## This library is free software; you can redistribute it and/or
-## modify it under the terms of the GNU Lesser General Public
-## License as published by the Free Software Foundation; either
-## version 2.1 of the License, or (at your option) any later version.
-##
-## This library is distributed in the hope that it will be useful,
-## but WITHOUT ANY WARRANTY; without even the implied warranty of
-## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-## Lesser General Public License for more details.
-##
-## You should have received a copy of the GNU Lesser General Public
-## License along with this library.  If not, see
-## .
-
-include gnulib.mk
-
-GNULIB_TESTS0 =
-GNULIB_TESTS1 = $(GNULIB_TESTS)
-if WITH_EXPENSIVE_TESTS
-## Automake requires that at least one conditional call out all tests to
-## be run, for those tests to be shipped in the tarball
-TESTS = $(GNULIB_TESTS)
-endif WITH_EXPENSIVE_TESTS
-## However, we want to change the set of tests based on the make environment,
-## where the default was set at configure time.  Use GNU make constructs to
-## hide our actions from Automake, so we don't get it too confused.
-VIR_TEST_EXPENSIVE ?= $(VIR_TEST_EXPENSIVE_DEFAULT)
-$(eval TESTS=$(GNULIB_TESTS$(VIR_TEST_EXPENSIVE)))
-- 
2.24.1

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



[libvirt] [PATCH v2 07/12] bootstrap.conf: stop creating AUTHORS file

2020-01-09 Thread Pavel Hrdina
The existence of AUTHORS file is required for GNU projects but since
commit <8bfb36db40f38e92823b657b5a342652064b5adc> we do not require
these files to exist.

Signed-off-by: Pavel Hrdina 
---
 autogen.sh | 2 +-
 bootstrap.conf | 3 ---
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/autogen.sh b/autogen.sh
index a119b099cb..a1f1943adb 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -127,7 +127,7 @@ if test -d .git || test -f .git; then
 expected_hash=$(cat "$state_file" 2>/dev/null)
 actual_hash=$(gnulib_hash "$no_git")
 
-if test "$actual_hash" = "$expected_hash" && test -f AUTHORS; then
+if test "$actual_hash" = "$expected_hash"; then
 # The gnulib hash matches our expectations, and all the files
 # that can only be generated through bootstrap are present:
 # we just need to run autoreconf. Unless we're performing a
diff --git a/bootstrap.conf b/bootstrap.conf
index 43f762da1e..b498c1e34b 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -170,9 +170,6 @@ xmllint-
 xsltproc   -
 "
 
-# Automake requires that AUTHORS exist.
-touch AUTHORS || exit 1
-
 # Override bootstrap's list - we don't use mdate-sh or texinfo.tex.
 gnulib_extra_files="
 build-aux/install-sh
-- 
2.24.1

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



[libvirt] [PATCH v2 04/12] bootstrap.conf: always copy files

2020-01-09 Thread Pavel Hrdina
Preparation for having bootstrap result in git.

Signed-off-by: Pavel Hrdina 
---
 bootstrap.conf | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index 13d0e77514..3798284dad 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -127,11 +127,7 @@ gnulib_modules="$gnulib_modules wcwidth"
 
 SKIP_PO=true
 
-# Enable copy-mode for MSYS/MinGW. MSYS' ln doesn't work well in the way
-# bootstrap uses it with relative paths.
-if test -n "$MSYSTEM"; then
-copy=true
-fi
+copy=true
 
 
 # Tell gnulib to:
-- 
2.24.1

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



[libvirt] [PATCH] qemu: Don't use NULL path from qemuDomainGetHostdevPath

2020-01-09 Thread Jiri Denemark
Commit v5.10.0-290-g3a4787a301 refactored qemuDomainGetHostdevPath to
return a single path rather than an array of paths. When the function is
called on a missing device, it will now return NULL in @path rather than
a NULL array with zero items and the callers need to be adapted
properly.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_cgroup.c | 32 ++--
 src/qemu/qemu_domain.c |  9 +
 2 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 2bcc0527f6..45701b4c6e 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -426,13 +426,15 @@ qemuSetupHostdevCgroup(virDomainObjPtr vm,
 if (qemuDomainGetHostdevPath(dev, , ) < 0)
 return -1;
 
-VIR_DEBUG("Cgroup allow %s perms=%d", path, perms);
-rv = virCgroupAllowDevicePath(priv->cgroup, path, perms, false);
-virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path,
- virCgroupGetDevicePermsString(perms),
- rv);
-if (rv < 0)
-return -1;
+if (path) {
+VIR_DEBUG("Cgroup allow %s perms=%d", path, perms);
+rv = virCgroupAllowDevicePath(priv->cgroup, path, perms, false);
+virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path,
+ virCgroupGetDevicePermsString(perms),
+ rv);
+if (rv < 0)
+return -1;
+}
 
 if (qemuHostdevNeedsVFIO(dev)) {
 VIR_DEBUG("Cgroup allow %s perms=%d", QEMU_DEV_VFIO, 
VIR_CGROUP_DEVICE_RW);
@@ -473,13 +475,15 @@ qemuTeardownHostdevCgroup(virDomainObjPtr vm,
 if (qemuDomainGetHostdevPath(dev, , NULL) < 0)
 return -1;
 
-VIR_DEBUG("Cgroup deny %s", path);
-rv = virCgroupDenyDevicePath(priv->cgroup, path,
- VIR_CGROUP_DEVICE_RWM, false);
-virDomainAuditCgroupPath(vm, priv->cgroup,
- "deny", path, "rwm", rv);
-if (rv < 0)
-return -1;
+if (path) {
+VIR_DEBUG("Cgroup deny %s", path);
+rv = virCgroupDenyDevicePath(priv->cgroup, path,
+ VIR_CGROUP_DEVICE_RWM, false);
+virDomainAuditCgroupPath(vm, priv->cgroup,
+ "deny", path, "rwm", rv);
+if (rv < 0)
+return -1;
+}
 
 if (qemuHostdevNeedsVFIO(dev) &&
 !qemuDomainNeedsVFIO(vm->def)) {
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 24e84a5966..1f358544ab 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -14001,7 +14001,8 @@ qemuDomainNeedsVFIO(const virDomainDef *def)
  *
  * For given device @dev fetch its host path and store it at
  * @path. Optionally, caller can get @perms on the path (e.g.
- * rw/ro).
+ * rw/ro). When called on a missing device, the function will return success
+ * and store NULL at @path.
  *
  * The caller is responsible for freeing the @path when no longer
  * needed.
@@ -14625,7 +14626,7 @@ qemuDomainSetupHostdev(virQEMUDriverConfigPtr cfg 
G_GNUC_UNUSED,
 if (qemuDomainGetHostdevPath(dev, , NULL) < 0)
 return -1;
 
-if (qemuDomainCreateDevice(path, data, false) < 0)
+if (path && qemuDomainCreateDevice(path, data, false) < 0)
 return -1;
 
 if (qemuHostdevNeedsVFIO(dev) &&
@@ -15688,7 +15689,7 @@ qemuDomainNamespaceSetupHostdev(virDomainObjPtr vm,
 if (qemuDomainGetHostdevPath(hostdev, , NULL) < 0)
 return -1;
 
-if (qemuDomainNamespaceMknodPath(vm, path) < 0)
+if (path && qemuDomainNamespaceMknodPath(vm, path) < 0)
 return -1;
 
 if (qemuHostdevNeedsVFIO(hostdev) &&
@@ -15720,7 +15721,7 @@ qemuDomainNamespaceTeardownHostdev(virDomainObjPtr vm,
 if (qemuDomainGetHostdevPath(hostdev, , NULL) < 0)
 return -1;
 
-if (qemuDomainNamespaceUnlinkPath(vm, path) < 0)
+if (path && qemuDomainNamespaceUnlinkPath(vm, path) < 0)
 return -1;
 
 if (qemuHostdevNeedsVFIO(hostdev) &&
-- 
2.24.1

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



Re: [libvirt] backing file chains, -blockdev, and storage file autodetection

2020-01-09 Thread Peter Krempa
On Thu, Jan 09, 2020 at 14:34:08 +, Daniel Berrange wrote:
> On Wed, Jan 08, 2020 at 03:46:59PM +0100, Peter Krempa wrote:
> > On Wed, Jan 08, 2020 at 13:34:14 +, Martin Wilck wrote:
> > > The recent change in libvirt to pass storage arguments to qemu via 
> > > "-blockdev", explicity passing backing file chain information rather
> > > than relying on qemu to figure it out, has bitten me quite painfully.
> > 
> > I'm sorry for causing this. I'm also sorry for going to sound too
> > dismissive later on.
> 
> snip
> 
> > > While I can't deny that an attack like this might be feasible, I am
> > > still wondering why this hasn't been an issue in past years (with qemu
> > > auto-detecting the backing file format).
> > 
> > Well, for distros using selinux this attack is mitigated by selinux
> > usage. That was also the reason why "assuming raw" was good enough. It
> > was also 'good enough' because there wasn't any other option.
> > 
> > It additionally created a whole lot of users which were already bitten
> > by this many years ago and now use the format correctly since we'd not
> > allow access otherwise when selinux is used.
> 
> IIUC there are three scenarios at play here
> 
>  1. qcow2 file, first level raw backing
>  2. qcow2 file, first level qcow2 backing, no  backing
>  3. qcow2 file, first level qcow2 backing, with second level backing
> 
> Historically with the SELinux driver, if no backing_fmt is set,
> then we blindly assume that scenario (1) is in effect.
> 
> We cannot tell QEMU that we treated it as scenario (1) though,
> so QEMU will still probe format and potentially open the
> first level backing as qcow2 still.
> 
> IOW, despite our SELinux & QEMU driver assumption for scenario (1),
> scenario (2) would still suceed in the past.  Scenario (3) would
> always have failed, because SELinux won't have labelled the second
> level backing.
> 
> Essentially scenario (2) worked by accident, not design, but
> IIUC we have not been warning users about this.

Exactly.

> With new libvirt and -blockdev we still assume the backing_fmt
> is raw if not set in the SELinux driver, but we now have the
> ability to tell QEMU about our assumption via -blockdev. As
> such we've not ensured scenario (2) fails.
> 
> 
> Users who were silently relying on scenario (2) are now broken
> and have three options IIUC
> 
>  - Run qemu-img rebase to set the backing_fmt
> 
> or
> 
>  - Update the guest XML to set the  format value
> 
> or
> 
>  - Update the /etc/libvirt/qemu.conf to set capability_filters
>to disable "blockdev"

This thing is only for experimenting though. I don't think we should
suggest it as a solution as it may cause problems in the future.

> Always assuming the format is raw certainly avoids the security
> danger, but is unhelpful to users who relied on scenario (2).
> 
> I'm inclined to say we should make scenario (2)/(3) into a hard

They are a hard error now:

commit 3615e8b39badf2a526996a69dc91a92b04cf262e
Author: Peter Krempa 
Date:   Tue Dec 17 17:04:04 2019 +0100

util: storage: Don't treat files with missing backing store format as 'raw'

Assuming that the backing image format is raw is wrong when doing image
detection:

1) In -drive mode qemu will still probe the image format of the backing
   image. This means it will try to open a backing file of the image
   which will fail if a more advanced security model is in use.

2) In blockdev mode the image will be opened as raw actually which is
   wrong since it might be qcow. Not opening the backing images will
   also end up in the guest seeing corrupted data.

Rather than attempt to solve various corner cases when us assuming the
storage file being raw and actually being right forbid startup when the
guest image doesn't have the format specified in the metadata.

> error. Only allow scenario (1) to run normally.

but that makes also scenario 1 an error.

> 
> ie, we should probe the disk format for the backing file, and
> if it is *not* raw, then report an immediate error, with the

My only grief here is that we'd actually have to run format detection at
all. That code was deleted from the startup path long time ago and I
don't feel particularly good about re-adding it.

> error message pointing to the kbase page explaining how to
> fix this. This will help the 99% common case identify the
> fix more quickly, with no obvious downside that I see.

This would be the first error message to contain URI. Or do you have any
other suggestion how to word it?

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



[libvirt] [dockerfiles PATCH 0/2] openSUSE updates

2020-01-09 Thread Andrea Bolognani
Pushed under the Dockerfile update rule.

Andrea Bolognani (2):
  Update openSUSE Leap 15.1 to install Meson from pip
  Add openSUSE Leap 15.1 Dockerfile for libosinfo

 buildenv-libosinfo-opensuse-151.zip | Bin 0 -> 570 bytes
 buildenv-libvirt-opensuse-151.zip   | Bin 761 -> 785 bytes
 2 files changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 buildenv-libosinfo-opensuse-151.zip

-- 
2.24.1

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



[libvirt] [dockerfiles PATCH 1/2] Update openSUSE Leap 15.1 to install Meson from pip

2020-01-09 Thread Andrea Bolognani
The corresponding libvirt-jenkins-ci commit is 732952acd532.

Signed-off-by: Andrea Bolognani 
---
 buildenv-libvirt-opensuse-151.zip | Bin 761 -> 785 bytes
 1 file changed, 0 insertions(+), 0 deletions(-)

diff --git a/buildenv-libvirt-opensuse-151.zip 
b/buildenv-libvirt-opensuse-151.zip
index 701149f..1a835c5 100644
--- a/buildenv-libvirt-opensuse-151.zip
+++ b/buildenv-libvirt-opensuse-151.zip
@@ -59,7 +59,6 @@ RUN zypper update -y && \
lsof \
lvm2 \
make \
-   meson \
ncurses-devel \
net-tools \
nfs-utils \
@@ -75,6 +74,7 @@ RUN zypper update -y && \
python3 \
python3-docutils \
python3-flake8 \
+   python3-pip \
python3-setuptools \
qemu-tools \
radvd \
@@ -93,4 +93,7 @@ RUN zypper update -y && \
xfsprogs-devel && \
 zypper clean --all
 
+RUN pip3 install \
+ meson==0.49.0
+
 ENV LANG "en_US.UTF-8"

-- 
2.24.1

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



[libvirt] [dockerfiles PATCH 2/2] Add openSUSE Leap 15.1 Dockerfile for libosinfo

2020-01-09 Thread Andrea Bolognani
The corresponding libvirt-jenkins-ci commit is c83eda1bc92d.

Signed-off-by: Andrea Bolognani 
---
 buildenv-libosinfo-opensuse-151.zip | Bin 0 -> 570 bytes
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 buildenv-libosinfo-opensuse-151.zip

diff --git a/buildenv-libosinfo-opensuse-151.zip 
b/buildenv-libosinfo-opensuse-151.zip
new file mode 100644
index 000..689344b
--- /dev/null
+++ b/buildenv-libosinfo-opensuse-151.zip
@@ -0,0 +1,57 @@
+FROM opensuse/leap:15.1
+
+RUN zypper update -y && \
+zypper install -y \
+   autoconf \
+   automake \
+   bash \
+   bash-completion \
+   ca-certificates \
+   ccache \
+   check-devel \
+   chrony \
+   cppi \
+   gcc \
+   gdb \
+   gettext \
+   gettext-devel \
+   git \
+   glib2-devel \
+   glibc-devel \
+   glibc-locale \
+   gobject-introspection-devel \
+   gtk-doc \
+   hwdata \
+   intltool \
+   json-glib-devel \
+   libarchive-devel \
+   libsoup-devel \
+   libtool \
+   libxml2 \
+   libxml2-devel \
+   libxslt-devel \
+   lsof \
+   make \
+   net-tools \
+   ninja \
+   patch \
+   perl \
+   pkgconfig \
+   python3 \
+   python3-lxml \
+   python3-pip \
+   python3-pytest \
+   python3-requests \
+   python3-setuptools \
+   rpm-build \
+   screen \
+   strace \
+   sudo \
+   vala \
+   vim && \
+zypper clean --all
+
+RUN pip3 install \
+ meson==0.49.0
+
+ENV LANG "en_US.UTF-8"

-- 
2.24.1

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



Re: [libvirt] backing file chains, -blockdev, and storage file autodetection

2020-01-09 Thread Daniel P . Berrangé
On Wed, Jan 08, 2020 at 03:46:59PM +0100, Peter Krempa wrote:
> On Wed, Jan 08, 2020 at 13:34:14 +, Martin Wilck wrote:
> > The recent change in libvirt to pass storage arguments to qemu via 
> > "-blockdev", explicity passing backing file chain information rather
> > than relying on qemu to figure it out, has bitten me quite painfully.
> 
> I'm sorry for causing this. I'm also sorry for going to sound too
> dismissive later on.

snip

> > While I can't deny that an attack like this might be feasible, I am
> > still wondering why this hasn't been an issue in past years (with qemu
> > auto-detecting the backing file format).
> 
> Well, for distros using selinux this attack is mitigated by selinux
> usage. That was also the reason why "assuming raw" was good enough. It
> was also 'good enough' because there wasn't any other option.
> 
> It additionally created a whole lot of users which were already bitten
> by this many years ago and now use the format correctly since we'd not
> allow access otherwise when selinux is used.

IIUC there are three scenarios at play here

 1. qcow2 file, first level raw backing
 2. qcow2 file, first level qcow2 backing, no  backing
 3. qcow2 file, first level qcow2 backing, with second level backing

Historically with the SELinux driver, if no backing_fmt is set,
then we blindly assume that scenario (1) is in effect.

We cannot tell QEMU that we treated it as scenario (1) though,
so QEMU will still probe format and potentially open the
first level backing as qcow2 still.

IOW, despite our SELinux & QEMU driver assumption for scenario (1),
scenario (2) would still suceed in the past.  Scenario (3) would
always have failed, because SELinux won't have labelled the second
level backing.

Essentially scenario (2) worked by accident, not design, but
IIUC we have not been warning users about this.

With new libvirt and -blockdev we still assume the backing_fmt
is raw if not set in the SELinux driver, but we now have the
ability to tell QEMU about our assumption via -blockdev. As
such we've not ensured scenario (2) fails.


Users who were silently relying on scenario (2) are now broken
and have three options IIUC

 - Run qemu-img rebase to set the backing_fmt

or

 - Update the guest XML to set the  format value

or

 - Update the /etc/libvirt/qemu.conf to set capability_filters
   to disable "blockdev"


Always assuming the format is raw certainly avoids the security
danger, but is unhelpful to users who relied on scenario (2).

I'm inclined to say we should make scenario (2)/(3) into a hard
error. Only allow scenario (1) to run normally.

ie, we should probe the disk format for the backing file, and
if it is *not* raw, then report an immediate error, with the
error message pointing to the kbase page explaining how to
fix this. This will help the 99% common case identify the
fix more quickly, with no obvious downside that I see.

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

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



Re: [libvirt] [PATCH v3 0/5] introduce support for an embedded driver mode

2020-01-09 Thread Daniel P . Berrangé
On Wed, Jan 08, 2020 at 05:30:23PM +0100, Michal Privoznik wrote:
> On 12/20/19 3:16 PM, Daniel P. Berrangé wrote:
> > 
> 
> Hm.. maybe I'm doing something wrong, but the following doesn't work for me.
> Note, "fedora" is a VM with two disks:
> 
> 
>   
>   
>   
>   
>   
> 
> 
>   
>name='iqn.2017-03.com.mprivozn:server-lun-0/0'>
> 
> 
>   
> 
> 
>   
> 
>   
>   
>function='0x0'/>
> 
> 
> 
> libvirt.git/_build # ./tools/virsh -c qemu:///embed?root=/tmp/embed/
> Welcome to virsh, the virtualization interactive terminal.
> 
> Type:  'help' for help with commands
>'quit' to quit
> 
> virsh # list --all
>  Id   Name State
> -
>  -fedora   shut off
> 
> virsh # connect secret:///embed?root=/tmp/embed

Ok, you're opening the secret driver in embedded mode

If you *also* open the QEMU driver now, it will use
this embedded secret driver directly.

> 
> virsh # secret-list
>  UUID   Usage
> -
>  4cf62bac-6a9f-4b9a-ba33-8c4d96b0e2e4   iscsi iscsi-secret-pool

I guess you created the XML file for this secrete previously ?


> virsh # connect qemu:///embed?root=/tmp/embed

Note that this now *closes* the existing connection, so the
embeded secret driver is now closed, and QEMU will speak to
libvirtd (or virtsecretd) for secrets now.

Basically virsh is not a suitable tool for using the
drivers in embedded mode since it is only capable of
opening a single driver connection at a time.

> virsh # start fedora
> 2020-01-08 15:37:57.294+: 44566: info : libvirt version: 6.0.0
> 2020-01-08 15:37:57.294+: 44566: info : hostname: moe
> 2020-01-08 15:37:57.294+: 44566: warning : qemuDomainDefValidate:5835 :
> CPU topology doesn't match numa CPU count; partial NUMA mapping is obsoleted
> and will be removed in future
> error: Failed to start domain fedora
> error: internal error: URI must be secret:///embed

Oh, that's odd - it seems to be trying to access the embedded
secret driver but failing a URI sanity check. This is probably
a result of you previously opening & then closing the embedded
secret driver. This is not really a supported use case anyway.


> However, running the domain (with the same disks) from regular URI is
> impossible either:
> 
> libvirt.git/_build # ./tools/virsh -c qemu:///system start fedora
> error: Failed to start domain fedora
> error: internal error: no internalFlags support
> 
> 
> This is because if the secret is private, then we don't want to allow
> clients getting its value. And if running the monolithic daemon, the
> conn->secretDrive is initialized to point right to the secret driver. But
> when using split daemons, then the connection points to the remote secret
> driver and virtqemud is then unable to obtain the secret value.
> Unfortunately, I don't see a way around this. I mean other than allow
> getting the value on RPC layer.

Basically we need to establish a trust relationship between
virtqemud and virtsecretd. I think we could relax this to mean a
trust relationship between virtsecretd and any client which is
running as the same user ID by default. A stronger trust relation
could be set using the fine grained polkit ACLs, with a ACL check
based on the API flag.


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

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

Re: [libvirt] [PATCH v3] driver: Include source as a flag to virDomainGetHostname

2020-01-09 Thread Michal Privoznik

On 12/27/19 9:36 PM, Julio Faracco wrote:

There is a lots of possibilities to retrieve hostname information from
domain. Libvirt could use lease information from dnsmasq to get current
hostname too. QEMU supports QEMU-agent but it can use lease source. See
'domifaddr' as an example.

This commit still adds lease options for QEMU. It will get the first
hostname available from domain networks.

This case, QEMU driver has a default section inside switch to keep
compatibility. So, if someone call 'domhostname' without specifying
source, it will get the default option. Some other drivers like LXC and
OpenVZ supports only one type, virCheckFlags will handle this case.

Signed-off-by: Julio Faracco 
---
v1-v2: Moving sources into flags.
v2-v3: Applying Michal's suggestions.
---
  docs/manpages/virsh.rst  |  7 ++-
  include/libvirt/libvirt-domain.h |  5 ++
  src/lxc/lxc_driver.c | 78 
  src/qemu/qemu_driver.c   | 76 +++
  tools/virsh-completer-domain.c   | 17 +++
  tools/virsh-completer-domain.h   |  4 ++
  tools/virsh-domain.c | 26 ++-
  7 files changed, 201 insertions(+), 12 deletions(-)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index fea0527caf..ba483d4d00 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -1797,10 +1797,15 @@ domhostname
  
  .. code-block::
  
-   domhostname domain

+   domhostname domain [--source lease|agent]
  
  Returns the hostname of a domain, if the hypervisor makes it available.
  
+The *--source* argument specifies what data source to use for the

+hostnames, currently 'lease' to read DHCP leases or 'agent' to query
+the guest OS via an agent. If unspecified, driver returns the default
+method available (some drivers support only one type of source).
+
  
  domid

  -
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index e60003978a..666c1875cc 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -4833,6 +4833,11 @@ typedef struct _virTypedParameter virMemoryParameter;
   */
  typedef virMemoryParameter *virMemoryParameterPtr;
  
+typedef enum {

+VIR_DOMAIN_GET_HOSTNAME_LEASE = (1 << 0), /* Parse DHCP lease file */
+VIR_DOMAIN_GET_HOSTNAME_AGENT = (1 << 1), /* Query qemu guest agent */
+} virDomainGetHostnameFlags;
+


This can go closer to the function declaration.


  typedef enum {
  VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE = 0, /* Parse DHCP lease file */
  VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT = 1, /* Query qemu guest agent */
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 780c6ed4a2..2dac730e70 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -5291,6 +5291,83 @@ lxcDomainGetCPUStats(virDomainPtr dom,
  }
  
  
+static char *

+lxcDomainGetHostname(virDomainPtr dom,
+ unsigned int flags)
+{
+virLXCDriverPtr driver = dom->conn->privateData;
+virDomainObjPtr vm = NULL;
+char macaddr[VIR_MAC_STRING_BUFLEN];
+g_autoptr(virConnect) conn = NULL;
+virNetworkDHCPLeasePtr *leases = NULL;
+int n_leases;
+size_t i, j;
+char *hostname = NULL;
+
+virCheckFlags(VIR_DOMAIN_GET_HOSTNAME_LEASE, NULL);
+
+if (!(vm = lxcDomObjFromDomain(dom)))
+return NULL;
+
+if (virDomainGetHostnameEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_QUERY) < 0)
+goto cleanup;
+
+if (virDomainObjCheckActive(vm) < 0)
+goto endjob;
+
+if (!(conn = virGetConnectNetwork()))
+goto endjob;
+
+for (i = 0; i < vm->def->nnets; i++) {
+g_autoptr(virNetwork) network = NULL;
+
+if (vm->def->nets[i]->type != VIR_DOMAIN_NET_TYPE_NETWORK)
+continue;
+
+virMacAddrFormat(&(vm->def->nets[i]->mac), macaddr);
+network = virNetworkLookupByName(conn,
+ vm->def->nets[i]->data.network.name);
+
+if (!network)
+goto endjob;
+
+if ((n_leases = virNetworkGetDHCPLeases(network, macaddr,
+, 0)) < 0)
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("There is no available hostname %d"),
+   flags);
+
+for (j = 0; j < n_leases; j++) {
+virNetworkDHCPLeasePtr lease = leases[j];
+if (lease->hostname) {
+hostname = g_strdup(lease->hostname);
+
+for (j = 0; j < n_leases; j++)
+virNetworkDHCPLeaseFree(leases[j]);
+
+VIR_FREE(leases);
+
+goto endjob;


Huh, you fixed qemu version of this code but not LXC one.


+}
+
+virNetworkDHCPLeaseFree(lease);
+}
+
+VIR_FREE(leases);
+}
+
+ endjob:
+virLXCDomainObjEndJob(driver, vm);
+
+ cleanup:

[libvirt] [PATCH v4 5/5] virsh: Expose virDomainGetHostnameFlags

2020-01-09 Thread Michal Privoznik
From: Julio Faracco 

Our virsh already has 'domhostname' command. Add '--source'
argument to it so that users can chose between 'lease' and
'agent' sources. Also, implement completer for the argument.

Signed-off-by: Julio Faracco 
Signed-off-by: Michal Privoznik 
---
 docs/manpages/virsh.rst|  7 ++-
 tools/virsh-completer-domain.c | 19 +
 tools/virsh-completer-domain.h |  4 
 tools/virsh-domain.c   | 37 +-
 tools/virsh-domain.h   |  8 
 5 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 4522259657..95a20aef9c 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -1797,10 +1797,15 @@ domhostname
 
 .. code-block::
 
-   domhostname domain
+   domhostname domain [--source lease|agent]
 
 Returns the hostname of a domain, if the hypervisor makes it available.
 
+The *--source* argument specifies what data source to use for the
+hostnames, currently 'lease' to read DHCP leases or 'agent' to query
+the guest OS via an agent. If unspecified, driver returns the default
+method available (some drivers support only one type of source).
+
 
 domid
 -
diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c
index 6da603048e..4472ee08f2 100644
--- a/tools/virsh-completer-domain.c
+++ b/tools/virsh-completer-domain.c
@@ -316,3 +316,22 @@ virshDomainInterfaceAddrSourceCompleter(vshControl *ctl 
G_GNUC_UNUSED,
 
 return ret;
 }
+
+
+char **
+virshDomainHostnameSourceCompleter(vshControl *ctl G_GNUC_UNUSED,
+   const vshCmd *cmd G_GNUC_UNUSED,
+   unsigned int flags)
+{
+char **ret = NULL;
+size_t i;
+
+virCheckFlags(0, NULL);
+
+ret = g_new0(typeof(*ret), VIRSH_DOMAIN_HOSTNAME_SOURCE_LAST + 1);
+
+for (i = 0; i < VIRSH_DOMAIN_HOSTNAME_SOURCE_LAST; i++)
+ret[i] = g_strdup(virshDomainHostnameSourceTypeToString(i));
+
+return ret;
+}
diff --git a/tools/virsh-completer-domain.h b/tools/virsh-completer-domain.h
index 79beec2cfe..b00b05e3bd 100644
--- a/tools/virsh-completer-domain.h
+++ b/tools/virsh-completer-domain.h
@@ -58,3 +58,7 @@ char **
 virshDomainInterfaceAddrSourceCompleter(vshControl *ctl,
 const vshCmd *cmd,
 unsigned int flags);
+
+char ** virshDomainHostnameSourceCompleter(vshControl *ctl,
+   const vshCmd *cmd,
+   unsigned int flags);
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 9315755990..0b6a9f2fbd 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11741,20 +11741,55 @@ static const vshCmdInfo info_domhostname[] = {
 
 static const vshCmdOptDef opts_domhostname[] = {
 VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
+{.name = "source",
+ .type = VSH_OT_STRING,
+ .flags = VSH_OFLAG_NONE,
+ .completer = virshDomainHostnameSourceCompleter,
+ .help = N_("address source: 'lease' or 'agent'")},
 {.name = NULL}
 };
 
+VIR_ENUM_IMPL(virshDomainHostnameSource,
+  VIRSH_DOMAIN_HOSTNAME_SOURCE_LAST,
+  "agent",
+  "lease");
+
 static bool
 cmdDomHostname(vshControl *ctl, const vshCmd *cmd)
 {
 char *hostname;
 virDomainPtr dom;
 bool ret = false;
+const char *sourcestr = NULL;
+int flags = 0; /* Use default value. Drivers can have its own default. */
 
 if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
 return false;
 
-hostname = virDomainGetHostname(dom, 0);
+if (vshCommandOptStringReq(ctl, cmd, "source", ) < 0)
+goto error;
+
+if (sourcestr) {
+int source = virshDomainHostnameSourceTypeFromString(sourcestr);
+
+if (source < 0) {
+vshError(ctl, _("Unknown data source '%s'"), sourcestr);
+goto error;
+}
+
+switch ((virshDomainHostnameSource) source) {
+case VIRSH_DOMAIN_HOSTNAME_SOURCE_AGENT:
+flags |= VIR_DOMAIN_GET_HOSTNAME_AGENT;
+break;
+case VIRSH_DOMAIN_HOSTNAME_SOURCE_LEASE:
+flags |= VIR_DOMAIN_GET_HOSTNAME_LEASE;
+break;
+case VIRSH_DOMAIN_HOSTNAME_SOURCE_LAST:
+break;
+}
+}
+
+hostname = virDomainGetHostname(dom, flags);
 if (hostname == NULL) {
 vshError(ctl, "%s", _("failed to get hostname"));
 goto error;
diff --git a/tools/virsh-domain.h b/tools/virsh-domain.h
index 02996d51b1..0d59c579d4 100644
--- a/tools/virsh-domain.h
+++ b/tools/virsh-domain.h
@@ -30,4 +30,12 @@ typedef struct virshDomainEventCallback 
virshDomainEventCallback;
 
 extern virshDomainEventCallback virshDomainEventCallbacks[];
 
+typedef enum {
+VIRSH_DOMAIN_HOSTNAME_SOURCE_AGENT,
+VIRSH_DOMAIN_HOSTNAME_SOURCE_LEASE,
+

[libvirt] [PATCH v4 2/5] Introduce source flags to virDomainGetHostname()

2020-01-09 Thread Michal Privoznik
From: Julio Faracco 

There is a lots of possibilities to retrieve hostname information
from domain. Libvirt could use lease information from dnsmasq to
get current hostname too. QEMU supports QEMU-agent but it can use
lease source.

Signed-off-by: Michal Privoznik 
Signed-off-by: Julio Faracco 
---
 include/libvirt/libvirt-domain.h | 6 ++
 include/libvirt/virterror.h  | 1 +
 src/libvirt-domain.c | 9 +
 src/remote/remote_daemon.c   | 1 +
 src/util/virerror.c  | 3 +++
 5 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index c1b9a9d1d0..44f6b62913 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -1567,6 +1567,12 @@ int virDomainSetMemoryStatsPeriod 
(virDomainPtr domain,
 int virDomainGetMaxVcpus(virDomainPtr domain);
 int virDomainGetSecurityLabel (virDomainPtr domain,
virSecurityLabelPtr 
seclabel);
+
+typedef enum {
+VIR_DOMAIN_GET_HOSTNAME_LEASE = (1 << 0), /* Parse DHCP lease file */
+VIR_DOMAIN_GET_HOSTNAME_AGENT = (1 << 1), /* Query qemu guest agent */
+} virDomainGetHostnameFlags;
+
 char *  virDomainGetHostname(virDomainPtr domain,
  unsigned int flags);
 int virDomainGetSecurityLabelList (virDomainPtr domain,
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 54f4f8190d..b7aa2a0ec3 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -332,6 +332,7 @@ typedef enum {
 VIR_ERR_INVALID_NETWORK_PORT = 105, /* invalid network port object */
 VIR_ERR_NETWORK_PORT_EXIST = 106,   /* the network port already exist */
 VIR_ERR_NO_NETWORK_PORT = 107,  /* network port not found */
+VIR_ERR_NO_HOSTNAME = 108,  /* no domain's hostname found */
 
 # ifdef VIR_ENUM_SENTINELS
 VIR_ERR_NUMBER_LAST
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index eb66999f07..d0304e174f 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11025,12 +11025,13 @@ virDomainGetDiskErrors(virDomainPtr dom,
 /**
  * virDomainGetHostname:
  * @domain: a domain object
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: bitwise-OR of virDomainGetHostnameFlags
  *
- * Get the hostname for that domain.
+ * Get the hostname for that domain. If no hostname is found,
+ * then an error is raised with VIR_ERR_NO_HOSTNAME code.
  *
- * Dependent on hypervisor used, this may require a guest agent to be
- * available.
+ * Dependent on hypervisor and @flags used, this may require a
+ * guest agent to be available.
  *
  * Returns the hostname which must be freed by the caller, or
  * NULL if there was an error.
diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index cd55b2c39e..1c224f8050 100644
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -296,6 +296,7 @@ static int daemonErrorLogFilter(virErrorPtr err, int 
priority)
 case VIR_ERR_NO_DOMAIN_METADATA:
 case VIR_ERR_NO_SERVER:
 case VIR_ERR_NO_CLIENT:
+case VIR_ERR_NO_HOSTNAME:
 return VIR_LOG_DEBUG;
 }
 
diff --git a/src/util/virerror.c b/src/util/virerror.c
index aac6ee3597..0f3ee1faaa 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -1233,6 +1233,9 @@ static const virErrorMsgTuple virErrorMsgStrings[] = {
 [VIR_ERR_NO_NETWORK_PORT] = {
 N_("network port not found"),
 N_("network port not found: %s") },
+[VIR_ERR_NO_HOSTNAME] = {
+N_("no hostname found"),
+N_("no hostname found: %s") },
 };
 
 G_STATIC_ASSERT(G_N_ELEMENTS(virErrorMsgStrings) == VIR_ERR_NUMBER_LAST);
-- 
2.24.1

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



[libvirt] [PATCH v4 3/5] qemu: Implement virDomainGetHostnameFlags

2020-01-09 Thread Michal Privoznik
From: Julio Faracco 

We have to keep the default - querying the agent if no flag is
set.

Signed-off-by: Michal Privoznik 
Signed-off-by: Julio Faracco 
---
 src/qemu/qemu_driver.c | 125 +++--
 1 file changed, 109 insertions(+), 16 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1fc662b3c8..6b33342be8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20257,25 +20257,16 @@ qemuConnectGetCPUModelNames(virConnectPtr conn,
 }
 
 
-static char *
-qemuDomainGetHostname(virDomainPtr dom,
-  unsigned int flags)
+static int
+qemuDomainGetHostnameAgent(virQEMUDriverPtr driver,
+   virDomainObjPtr vm,
+   char **hostname)
 {
-virQEMUDriverPtr driver = dom->conn->privateData;
-virDomainObjPtr vm = NULL;
 qemuAgentPtr agent;
-char *hostname = NULL;
-
-virCheckFlags(0, NULL);
-
-if (!(vm = qemuDomainObjFromDomain(dom)))
-return NULL;
-
-if (virDomainGetHostnameEnsureACL(dom->conn, vm->def) < 0)
-goto cleanup;
+int ret = -1;
 
 if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_QUERY) < 0)
-goto cleanup;
+return -1;
 
 if (virDomainObjCheckActive(vm) < 0)
 goto endjob;
@@ -20284,11 +20275,113 @@ qemuDomainGetHostname(virDomainPtr dom,
 goto endjob;
 
 agent = qemuDomainObjEnterAgent(vm);
-ignore_value(qemuAgentGetHostname(agent, ));
+ignore_value(qemuAgentGetHostname(agent, hostname));
 qemuDomainObjExitAgent(vm, agent);
 
+ret = 0;
  endjob:
 qemuDomainObjEndAgentJob(vm);
+return ret;
+}
+
+
+static int
+qemuDomainGetHostnameLease(virQEMUDriverPtr driver,
+   virDomainObjPtr vm,
+   char **hostname)
+{
+char macaddr[VIR_MAC_STRING_BUFLEN];
+g_autoptr(virConnect) conn = NULL;
+virNetworkDHCPLeasePtr *leases = NULL;
+int n_leases;
+size_t i, j;
+int ret = -1;
+
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
+return -1;
+
+if (virDomainObjCheckActive(vm) < 0)
+goto endjob;
+
+if (!(conn = virGetConnectNetwork()))
+goto endjob;
+
+for (i = 0; i < vm->def->nnets; i++) {
+g_autoptr(virNetwork) network = NULL;
+virDomainNetDefPtr net = vm->def->nets[i];
+
+if (net->type != VIR_DOMAIN_NET_TYPE_NETWORK)
+continue;
+
+virMacAddrFormat(>mac, macaddr);
+network = virNetworkLookupByName(conn, net->data.network.name);
+
+if (!network)
+goto endjob;
+
+if ((n_leases = virNetworkGetDHCPLeases(network, macaddr,
+, 0)) < 0)
+goto endjob;
+
+for (j = 0; j < n_leases; j++) {
+virNetworkDHCPLeasePtr lease = leases[j];
+if (lease->hostname && !*hostname)
+*hostname = g_strdup(lease->hostname);
+
+virNetworkDHCPLeaseFree(lease);
+}
+
+VIR_FREE(leases);
+
+if (*hostname)
+goto endjob;
+}
+
+ret = 0;
+ endjob:
+qemuDomainObjEndJob(driver, vm);
+return ret;
+}
+
+
+static char *
+qemuDomainGetHostname(virDomainPtr dom,
+  unsigned int flags)
+{
+virQEMUDriverPtr driver = dom->conn->privateData;
+virDomainObjPtr vm = NULL;
+char *hostname = NULL;
+
+virCheckFlags(VIR_DOMAIN_GET_HOSTNAME_LEASE |
+  VIR_DOMAIN_GET_HOSTNAME_AGENT, NULL);
+
+VIR_EXCLUSIVE_FLAGS_RET(VIR_DOMAIN_GET_HOSTNAME_LEASE,
+VIR_DOMAIN_GET_HOSTNAME_AGENT,
+NULL);
+
+if (!(flags & VIR_DOMAIN_GET_HOSTNAME_LEASE))
+flags |= VIR_DOMAIN_GET_HOSTNAME_AGENT;
+
+if (!(vm = qemuDomainObjFromDomain(dom)))
+return NULL;
+
+if (virDomainGetHostnameEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (flags & VIR_DOMAIN_GET_HOSTNAME_AGENT) {
+if (qemuDomainGetHostnameAgent(driver, vm, ) < 0)
+goto cleanup;
+} else if (flags & VIR_DOMAIN_GET_HOSTNAME_LEASE) {
+if (qemuDomainGetHostnameLease(driver, vm, ) < 0)
+goto cleanup;
+}
+
+if (!hostname) {
+virReportError(VIR_ERR_NO_HOSTNAME,
+   _("no hostname found for domain %s"),
+   vm->def->name);
+goto cleanup;
+}
 
  cleanup:
 virDomainObjEndAPI();
-- 
2.24.1

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



[libvirt] [PATCH v4 1/5] virerror: Make it easier to add new error number

2020-01-09 Thread Michal Privoznik
In v5.0.0-rc1~94 we switched from one huge switch() to an array
for translating error numbers into error messages. However, the
array is declared to have VIR_ERR_NUMBER_LAST items which makes
it impossible to spot this place by compile checking when adding
new error number.

Signed-off-by: Michal Privoznik 
---
 scripts/apibuild.py | 6 ++
 src/util/virerror.c | 4 +++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/scripts/apibuild.py b/scripts/apibuild.py
index 2f7314b379..595c004a4c 100755
--- a/scripts/apibuild.py
+++ b/scripts/apibuild.py
@@ -1657,6 +1657,12 @@ class CParser:
 token = ("name", "virloginit")
 return token
 
+elif token[0] == "name" and token[1] == "G_STATIC_ASSERT":
+# skip whole line
+while token is not None and token[0] != "sep" or token[1] != ";":
+token = self.token()
+return self.token()
+
 elif token[0] == "name":
 if self.type == "":
 self.type = token[1]
diff --git a/src/util/virerror.c b/src/util/virerror.c
index fd2f77329f..aac6ee3597 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -910,7 +910,7 @@ typedef struct {
 } virErrorMsgTuple;
 
 
-const virErrorMsgTuple virErrorMsgStrings[VIR_ERR_NUMBER_LAST] = {
+static const virErrorMsgTuple virErrorMsgStrings[] = {
 [VIR_ERR_OK] = { NULL, NULL },
 [VIR_ERR_INTERNAL_ERROR] = {
 N_("internal error"),
@@ -1235,6 +1235,8 @@ const virErrorMsgTuple 
virErrorMsgStrings[VIR_ERR_NUMBER_LAST] = {
 N_("network port not found: %s") },
 };
 
+G_STATIC_ASSERT(G_N_ELEMENTS(virErrorMsgStrings) == VIR_ERR_NUMBER_LAST);
+
 
 /**
  * virErrorMsg:
-- 
2.24.1

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



[libvirt] [PATCH v4 4/5] lxc: Implement virDomainGetHostnameFlags

2020-01-09 Thread Michal Privoznik
From: Julio Faracco 

Since there is no guest agent in LXC world (yet), we can
implement _LEASE flag only.

Signed-off-by: Michal Privoznik 
---
 src/lxc/lxc_driver.c | 79 
 1 file changed, 79 insertions(+)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 780c6ed4a2..bf1f8f8190 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -5291,6 +5291,84 @@ lxcDomainGetCPUStats(virDomainPtr dom,
 }
 
 
+static char *
+lxcDomainGetHostname(virDomainPtr dom,
+ unsigned int flags)
+{
+virLXCDriverPtr driver = dom->conn->privateData;
+virDomainObjPtr vm = NULL;
+char macaddr[VIR_MAC_STRING_BUFLEN];
+g_autoptr(virConnect) conn = NULL;
+virNetworkDHCPLeasePtr *leases = NULL;
+int n_leases;
+size_t i, j;
+char *hostname = NULL;
+
+virCheckFlags(VIR_DOMAIN_GET_HOSTNAME_LEASE, NULL);
+
+if (!(vm = lxcDomObjFromDomain(dom)))
+return NULL;
+
+if (virDomainGetHostnameEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_QUERY) < 0)
+goto cleanup;
+
+if (virDomainObjCheckActive(vm) < 0)
+goto endjob;
+
+if (!(conn = virGetConnectNetwork()))
+goto endjob;
+
+for (i = 0; i < vm->def->nnets; i++) {
+g_autoptr(virNetwork) network = NULL;
+virDomainNetDefPtr net = vm->def->nets[i];
+
+if (net->type != VIR_DOMAIN_NET_TYPE_NETWORK)
+continue;
+
+virMacAddrFormat(>mac, macaddr);
+network = virNetworkLookupByName(conn, net->data.network.name);
+
+if (!network)
+goto endjob;
+
+if ((n_leases = virNetworkGetDHCPLeases(network, macaddr,
+, 0)) < 0)
+goto endjob;
+
+for (j = 0; j < n_leases; j++) {
+virNetworkDHCPLeasePtr lease = leases[j];
+
+if (lease->hostname && !hostname)
+hostname = g_strdup(lease->hostname);
+
+virNetworkDHCPLeaseFree(lease);
+}
+
+VIR_FREE(leases);
+
+if (hostname)
+goto endjob;
+}
+
+if (!hostname) {
+virReportError(VIR_ERR_NO_HOSTNAME,
+   _("no hostname found for domain %s"),
+   vm->def->name);
+goto endjob;
+}
+
+ endjob:
+virLXCDomainObjEndJob(driver, vm);
+
+ cleanup:
+virDomainObjEndAPI();
+return hostname;
+}
+
+
 static int
 lxcNodeGetFreePages(virConnectPtr conn,
 unsigned int npages,
@@ -5436,6 +5514,7 @@ static virHypervisorDriver lxcHypervisorDriver = {
 .domainSetMetadata = lxcDomainSetMetadata, /* 1.1.3 */
 .domainGetMetadata = lxcDomainGetMetadata, /* 1.1.3 */
 .domainGetCPUStats = lxcDomainGetCPUStats, /* 1.2.2 */
+.domainGetHostname = lxcDomainGetHostname, /* 6.0.0 */
 .nodeGetMemoryParameters = lxcNodeGetMemoryParameters, /* 0.10.2 */
 .nodeSetMemoryParameters = lxcNodeSetMemoryParameters, /* 0.10.2 */
 .domainSendProcessSignal = lxcDomainSendProcessSignal, /* 1.0.1 */
-- 
2.24.1

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



[libvirt] [PATCH v4 0/5] Introduce flags to virDomainGetHostname()

2020-01-09 Thread Michal Privoznik
v4 of:

https://www.redhat.com/archives/libvir-list/2019-December/msg01453.html

diff to v3:
- I've split Julio's one patch into 4 smaller ones,
- I've fixed issues I've raised in v3, like new error code (patch 1/5 is
  completely new that's why I'm authoring it), fixed the completer and
  some memleaks.

Julio Faracco (4):
  Introduce source flags to virDomainGetHostname()
  qemu: Implement virDomainGetHostnameFlags
  lxc: Implement virDomainGetHostnameFlags
  virsh: Expose virDomainGetHostnameFlags

Michal Prívozník (1):
  virerror: Make it easier to add new error number

 docs/manpages/virsh.rst  |   7 +-
 include/libvirt/libvirt-domain.h |   6 ++
 include/libvirt/virterror.h  |   1 +
 scripts/apibuild.py  |   6 ++
 src/libvirt-domain.c |   9 ++-
 src/lxc/lxc_driver.c |  79 +++
 src/qemu/qemu_driver.c   | 125 +++
 src/remote/remote_daemon.c   |   1 +
 src/util/virerror.c  |   7 +-
 tools/virsh-completer-domain.c   |  19 +
 tools/virsh-completer-domain.h   |   4 +
 tools/virsh-domain.c |  37 -
 tools/virsh-domain.h |   8 ++
 13 files changed, 286 insertions(+), 23 deletions(-)

-- 
2.24.1

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

Re: [libvirt] [jenkins-ci PATCH v3 0/3] Add all possible projects to openSUSE 15.1

2020-01-09 Thread Andrea Bolognani
On Thu, 2020-01-09 at 11:57 +0100, Fabiano Fidêncio wrote:
> Now that openSUSE 15.1 is a thing on libvirt-jenkins-ci, let's add all
> possible projects to the OS, doing the needed mappings adjustments
> whenever needed.
> 
> Changes since v2:
> https://www.redhat.com/archives/libvir-list/2020-January/msg00333.html
> - Split meson change into its specific patch;
> - Re-ordered the commits based on Andrea's request;
> - (Possibly) Fixed all the issues pointed by Andrea;
> 
> Changes since v1:
> https://www.redhat.com/archives/libvir-list/2020-January/msg00321.html
> - Added libvirt-perl and updated the mappings accordingly.
> 
> Fabiano Fidêncio (3):
>   mappings: Use meson from pip on openSUSE 15.1
>   mappings: Adjust mappings for openSUSE 15.1
>   guests: Add projects to openSUSE 15.1
> 
>  guests/host_vars/libvirt-opensuse-151/main.yml   | 14 ++
>  guests/playbooks/build/projects/libvirt-dbus.yml |  1 +
>  .../playbooks/build/projects/libvirt-sandbox.yml |  1 +
>  guests/playbooks/build/projects/virt-manager.yml |  2 ++
>  guests/vars/mappings.yml | 16 
>  5 files changed, 34 insertions(+)

Looks good now!

  Reviewed-by: Andrea Bolognani 

and pushed :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 00/16] include result of bootstrap in git

2020-01-09 Thread Pavel Hrdina
On Thu, Jan 09, 2020 at 10:04:31AM +, Daniel P. Berrangé wrote:
> On Thu, Jan 09, 2020 at 09:16:30AM +0100, Pavel Hrdina wrote:
> > This patch series is motivated by the effort to adopt Meson as our
> > build system but all of it improves the current build system on its own.
> > 
> > It moves the burden to run bootstrap/autoreconf to developers which
> > means it will save some time for others when there is any change to
> > the build system.
> 
> So effectively we're committing  all of the GNULIB modules we're
> currently using into GIT. Historically this was undesirable as
> we didn't really want to version control files that we're already
> getting from the sub-module & have huge pointless diffs everytime
> we refresh GNULIB. Given our intent to eliminate GNULIB though,
> and switch to Meson, this is reasonable to do as a short term
> hack to simplify our Meson integration.
> 
> FWIW, I have more patches pending that will eliminate another
> 20-ish GNULIB modules that I hope to submit this week, though
> given freeze tomorrow, they'll have to wait till 6.1.0 open.
> 
> I think its probably wise to keep this series until 6.1.0 too,
> since changing the build system in a big way just before release
> freeze is kinda risky.

Agreed on that, I was not planning to push it before freeze.

Pavel


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

[libvirt] [jenkins-ci PATCH v3 3/3] guests: Add projects to openSUSE 15.1

2020-01-09 Thread Fabiano Fidêncio
libvirt-tck and libvirt-cim could not be added due to missing packages.

Signed-off-by: Fabiano Fidêncio 
---
 guests/host_vars/libvirt-opensuse-151/main.yml | 14 ++
 guests/playbooks/build/projects/libvirt-dbus.yml   |  1 +
 .../playbooks/build/projects/libvirt-sandbox.yml   |  1 +
 guests/playbooks/build/projects/virt-manager.yml   |  2 ++
 4 files changed, 18 insertions(+)

diff --git a/guests/host_vars/libvirt-opensuse-151/main.yml 
b/guests/host_vars/libvirt-opensuse-151/main.yml
index f422a9e..88d5dfd 100644
--- a/guests/host_vars/libvirt-opensuse-151/main.yml
+++ b/guests/host_vars/libvirt-opensuse-151/main.yml
@@ -1,6 +1,20 @@
 ---
 projects:
+  - gtk-vnc
+  - libosinfo
   - libvirt
+  - libvirt-dbus
+  - libvirt-glib
+  - libvirt-go
+  - libvirt-go-xml
+  - libvirt-ocaml
+  - libvirt-perl
+  - libvirt-python
+  - libvirt-sandbox
+  - osinfo-db
+  - osinfo-db-tools
+  - virt-manager
+  - virt-viewer
 
 package_format: 'rpm'
 package_manager: 'zypper'
diff --git a/guests/playbooks/build/projects/libvirt-dbus.yml 
b/guests/playbooks/build/projects/libvirt-dbus.yml
index 66bc1fa..e29d74b 100644
--- a/guests/playbooks/build/projects/libvirt-dbus.yml
+++ b/guests/playbooks/build/projects/libvirt-dbus.yml
@@ -20,6 +20,7 @@
   - libvirt-debian-sid
   - libvirt-fedora-30
   - libvirt-fedora-rawhide
+  - libvirt-opensuse-151
   - libvirt-ubuntu-1804
 - include: '{{ playbook_base }}/jobs/meson-rpm-job.yml'
   vars:
diff --git a/guests/playbooks/build/projects/libvirt-sandbox.yml 
b/guests/playbooks/build/projects/libvirt-sandbox.yml
index 0b4fe50..d9e00d4 100644
--- a/guests/playbooks/build/projects/libvirt-sandbox.yml
+++ b/guests/playbooks/build/projects/libvirt-sandbox.yml
@@ -11,6 +11,7 @@
   - libvirt-fedora-30
   - libvirt-fedora-31
   - libvirt-fedora-rawhide
+  - libvirt-opensuse-151
   - libvirt-ubuntu-1604
   - libvirt-ubuntu-1804
 archive_format: gz
diff --git a/guests/playbooks/build/projects/virt-manager.yml 
b/guests/playbooks/build/projects/virt-manager.yml
index 4b0e6dd..01f353e 100644
--- a/guests/playbooks/build/projects/virt-manager.yml
+++ b/guests/playbooks/build/projects/virt-manager.yml
@@ -13,6 +13,7 @@
   - libvirt-freebsd-11
   - libvirt-freebsd-12
   - libvirt-freebsd-current
+  - libvirt-opensuse-151
   - libvirt-ubuntu-1804
 archive_format: gz
 git_url: '{{ git_urls["virt-manager"][git_remote] }}'
@@ -35,6 +36,7 @@
   - libvirt-freebsd-11
   - libvirt-freebsd-12
   - libvirt-freebsd-current
+  - libvirt-opensuse-151
   - libvirt-ubuntu-1804
 - include: '{{ playbook_base }}/jobs/python-distutils-rpm-job.yml'
   vars:
-- 
2.24.1

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

Re: [libvirt] [PATCH 04/16] bootstrap.conf: add pthread-h module

2020-01-09 Thread Pavel Hrdina
On Thu, Jan 09, 2020 at 10:07:34AM +, Daniel P. Berrangé wrote:
> On Thu, Jan 09, 2020 at 09:16:34AM +0100, Pavel Hrdina wrote:
> > We use this module but we don't have explicit dependency on it.
> > 
> > Signed-off-by: Pavel Hrdina 
> > ---
> >  bootstrap.conf | 2 ++
> >  1 file changed, 2 insertions(+)
> 
> GNULIB says
> 
>   https://www.gnu.org/software/gnulib/manual/html_node/pthread_002eh.html
> 
> Portability problems fixed by Gnulib:
> 
>   This header pollutes the namespace with some broken macro
>   implementations for various functions such as strtok_r and
>   gmtime_r: mingw 3.0. 
> 
> We currently build with mingw32-winpthreads-6.0.0, and AFAICT
> that doesn't have the described problem. So I don't think we
> actually need this pthread_h module from GNULIB at all.

I'll drop this patch as I don't remember why I wrote it in the first
place.

Pavel


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

[libvirt] [jenkins-ci PATCH v3 2/3] mappings: Adjust mappings for openSUSE 15.1

2020-01-09 Thread Fabiano Fidêncio
Between all the adjustments done, it's worth to mention that there's no
equivalent of perl-Time-HiRes packages on openSUSE as its already part
of the base perl packages and that perl-generators is a Fedora/RHEL-ism
used for RPM auto-dependencies (which is not present nor needed on
openSUSE).

Signed-off-by: Fabiano Fidêncio 
---
 guests/vars/mappings.yml | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml
index 03044d0..69750ad 100644
--- a/guests/vars/mappings.yml
+++ b/guests/vars/mappings.yml
@@ -134,6 +134,7 @@ mappings:
   dbus-daemon:
 default: dbus
 Fedora: dbus-daemon
+OpenSUSE: dbus-1
 
   device-mapper:
 deb: libdevmapper-dev
@@ -181,6 +182,7 @@ mappings:
 rpm: gdk-pixbuf2-devel
 deb: libgdk-pixbuf2.0-dev
 pkg: gdk-pixbuf2
+OpenSUSE: gdk-pixbuf-devel
 cross-policy-deb: foreign
 
   gettext:
@@ -224,6 +226,7 @@ mappings:
   go:
 default: golang
 FreeBSD: go
+OpenSUSE: go
 
   gobject-introspection:
 deb: libgirepository1.0-dev
@@ -243,6 +246,7 @@ mappings:
 
   gtk-update-icon-cache:
 default: gtk-update-icon-cache
+OpenSUSE: gtk3-tools
 Ubuntu1604: libgtk2.0-bin
 
   hal:
@@ -267,6 +271,7 @@ mappings:
   isoinfo:
 default: genisoimage
 FreeBSD: cdrkit
+OpenSUSE: mkisofs
 
   java:
 deb: openjdk-11-jre-headless
@@ -800,6 +805,7 @@ mappings:
 deb: libtime-hr-perl
 pkg: p5-Time-HiRes
 rpm: perl-Time-HiRes
+OpenSUSE: perl
 
   perl-XML-Twig:
 deb: libxml-twig-perl
@@ -823,6 +829,7 @@ mappings:
 
   perl-generators:
 rpm: perl-generators
+OpenSUSE:
 CentOS7:
 
   pkg-config:
@@ -836,6 +843,7 @@ mappings:
   pulseaudio:
 deb: libpulse-dev
 rpm: pulseaudio-libs-devel
+OpenSUSE: libpulse-devel
 cross-policy-deb: foreign
 
   python3-docutils:
@@ -849,12 +857,14 @@ mappings:
   python3-dbus:
 default: python3-dbus
 FreeBSD: py37-dbus
+OpenSUSE: python3-dbus-python
 CentOS7: python36-dbus
 
   python3-devel:
 deb: python3-dev
 pkg: python3
 Fedora: python3-devel
+OpenSUSE: python3-devel
 CentOS7: python36-devel
 cross-policy-deb: foreign
 
@@ -867,6 +877,7 @@ mappings:
   python3-libxml2:
 default: python3-libxml2
 FreeBSD: py37-libxml2
+OpenSUSE: python3-libxml2-python
 CentOS7:
 Ubuntu1604:
 
@@ -950,6 +961,7 @@ mappings:
 deb: libspice-client-gtk-3.0-dev
 pkg: spice-gtk
 rpm: spice-gtk3-devel
+OpenSUSE: spice-gtk-devel
 cross-policy-deb: foreign
 
   strace:
@@ -1018,6 +1030,7 @@ mappings:
   xz-static:
 deb: liblzma-dev
 Fedora: xz-static
+OpenSUSE: xz-static-devel
 cross-policy-deb: foreign
 
   yajl:
@@ -1041,6 +1054,7 @@ mappings:
   zlib-static:
 deb: zlib1g-dev
 rpm: zlib-static
+OpenSUSE: zlib-devel-static
 cross-policy-deb: foreign
 
 
-- 
2.24.1

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

[libvirt] [jenkins-ci PATCH v3 1/3] mappings: Use meson from pip on openSUSE 15.1

2020-01-09 Thread Fabiano Fidêncio
The meson version present on openSUSE 15.1 is too old (0.46.0) to build
our projects, which require 0.49.0. Knowing that, meson from pip has to
be used.

Signed-off-by: Fabiano Fidêncio 
---
 guests/vars/mappings.yml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml
index b80a9b4..03044d0 100644
--- a/guests/vars/mappings.yml
+++ b/guests/vars/mappings.yml
@@ -481,6 +481,7 @@ mappings:
 default: meson
 CentOS7:
 Debian9:
+OpenSUSE151:
 Ubuntu1604:
 Ubuntu1804:
 
@@ -882,6 +883,7 @@ mappings:
   python3-pip:
 CentOS7: python3-pip
 Debian9: python3-pip
+OpenSUSE151: python3-pip
 Ubuntu1604: python3-pip
 Ubuntu1804: python3-pip
 
-- 
2.24.1

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

[libvirt] [jenkins-ci PATCH v3 0/3] Add all possible projects to openSUSE 15.1

2020-01-09 Thread Fabiano Fidêncio
Now that openSUSE 15.1 is a thing on libvirt-jenkins-ci, let's add all
possible projects to the OS, doing the needed mappings adjustments
whenever needed.

Changes since v2:
https://www.redhat.com/archives/libvir-list/2020-January/msg00333.html
- Split meson change into its specific patch;
- Re-ordered the commits based on Andrea's request;
- (Possibly) Fixed all the issues pointed by Andrea;

Changes since v1:
https://www.redhat.com/archives/libvir-list/2020-January/msg00321.html
- Added libvirt-perl and updated the mappings accordingly.

Fabiano Fidêncio (3):
  mappings: Use meson from pip on openSUSE 15.1
  mappings: Adjust mappings for openSUSE 15.1
  guests: Add projects to openSUSE 15.1

 guests/host_vars/libvirt-opensuse-151/main.yml   | 14 ++
 guests/playbooks/build/projects/libvirt-dbus.yml |  1 +
 .../playbooks/build/projects/libvirt-sandbox.yml |  1 +
 guests/playbooks/build/projects/virt-manager.yml |  2 ++
 guests/vars/mappings.yml | 16 
 5 files changed, 34 insertions(+)

-- 
2.24.1

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

Re: [libvirt] [PATCH 04/16] bootstrap.conf: add pthread-h module

2020-01-09 Thread Daniel P . Berrangé
On Thu, Jan 09, 2020 at 09:16:34AM +0100, Pavel Hrdina wrote:
> We use this module but we don't have explicit dependency on it.
> 
> Signed-off-by: Pavel Hrdina 
> ---
>  bootstrap.conf | 2 ++
>  1 file changed, 2 insertions(+)

GNULIB says

  https://www.gnu.org/software/gnulib/manual/html_node/pthread_002eh.html

Portability problems fixed by Gnulib:

  This header pollutes the namespace with some broken macro
  implementations for various functions such as strtok_r and
  gmtime_r: mingw 3.0. 

We currently build with mingw32-winpthreads-6.0.0, and AFAICT
that doesn't have the described problem. So I don't think we
actually need this pthread_h module from GNULIB at all.

> 
> diff --git a/bootstrap.conf b/bootstrap.conf
> index 13d0e77514..a302475198 100644
> --- a/bootstrap.conf
> +++ b/bootstrap.conf
> @@ -84,6 +84,8 @@ gnulib_modules="$gnulib_modules pipe2"
>  gnulib_modules="$gnulib_modules poll"
>  # -> Meson
>  gnulib_modules="$gnulib_modules posix-shell"
> +# -> GThread
> +gnulib_modules="$gnulib_modules pthread-h"
>  # -> open code conditional logic
>  gnulib_modules="$gnulib_modules pthread_sigmask"
>  # -> GSocket
> -- 
> 2.24.1
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

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

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



Re: [libvirt] [PATCH 00/16] include result of bootstrap in git

2020-01-09 Thread Daniel P . Berrangé
On Thu, Jan 09, 2020 at 09:16:30AM +0100, Pavel Hrdina wrote:
> This patch series is motivated by the effort to adopt Meson as our
> build system but all of it improves the current build system on its own.
> 
> It moves the burden to run bootstrap/autoreconf to developers which
> means it will save some time for others when there is any change to
> the build system.

So effectively we're committing  all of the GNULIB modules we're
currently using into GIT. Historically this was undesirable as
we didn't really want to version control files that we're already
getting from the sub-module & have huge pointless diffs everytime
we refresh GNULIB. Given our intent to eliminate GNULIB though,
and switch to Meson, this is reasonable to do as a short term
hack to simplify our Meson integration.

FWIW, I have more patches pending that will eliminate another
20-ish GNULIB modules that I hope to submit this week, though
given freeze tomorrow, they'll have to wait till 6.1.0 open.

I think its probably wise to keep this series until 6.1.0 too,
since changing the build system in a big way just before release
freeze is kinda risky.


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

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



Re: [libvirt] [PATCH 00/16] include result of bootstrap in git

2020-01-09 Thread Pavel Hrdina
On Thu, Jan 09, 2020 at 09:16:30AM +0100, Pavel Hrdina wrote:
> This patch series is motivated by the effort to adopt Meson as our
> build system but all of it improves the current build system on its own.
> 
> It moves the burden to run bootstrap/autoreconf to developers which
> means it will save some time for others when there is any change to
> the build system.
> 
> Since one of the patches is slightly bigger here is link to git:
> 
> 

So running configure without autogen.sh doesn't work from clean git
checkout as git submodules are not initialized.  I'll post v2 where
I'll drop some of the patches and keep autogen.sh mandatory.

It will be eventually replaced by Meson so it doesn't make sense to
come up with any clever solution.

Pavel


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

[libvirt] [PATCH 14/16] autogen.sh: remove configure related code

2020-01-09 Thread Pavel Hrdina
autogen.sh is no longer used in the build process, it will now serve
only as a script to regenerate files that are stored in git.

Signed-off-by: Pavel Hrdina 
---
 autogen.sh | 54 --
 1 file changed, 54 deletions(-)

diff --git a/autogen.sh b/autogen.sh
index 75e8b70d4e..49ca91bf64 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -7,8 +7,6 @@ die()
 exit 1
 }
 
-starting_point=$(pwd)
-
 srcdir=$(dirname "$0")
 test "$srcdir" || srcdir=.
 
@@ -140,55 +138,3 @@ if test -d .git || test -f .git; then
 fi
 fi
 fi
-
-# When performing a dry run, we can stop here
-test "$dry_run" && exit "$dry_run"
-
-# If asked not to run configure, we can stop here
-test "$NOCONFIGURE" && exit 0
-
-cd "$starting_point" || {
-die "Failed to cd into $starting_point"
-}
-
-if test "$OBJ_DIR"; then
-mkdir -p "$OBJ_DIR" || {
-die "Failed to create $OBJ_DIR"
-}
-cd "$OBJ_DIR" || {
-die "Failed to cd into $OBJ_DIR"
-}
-fi
-
-# Make sure we can find GNU make and tell the user
-# the right command to run
-MAKE=
-for cmd in make gmake; do
-if $cmd -v 2>&1 | grep -q "GNU Make"; then
-MAKE=$cmd
-break
-fi
-done
-test "$MAKE" || {
-die "GNU make is required to build libvirt"
-}
-
-if test -z "$*" && test -f config.status; then
-echo "Running config.status..."
-./config.status --recheck || {
-die "config.status failed"
-}
-else
-if test -z "$*"; then
-echo "I am going to run configure with no arguments - if you wish"
-echo "to pass any to it, please specify them on the $0 command line."
-else
-echo "Running configure with $@"
-fi
-"$srcdir/configure" "$@" || {
-die "configure failed"
-}
-fi
-
-echo
-echo "Now type '$MAKE' to compile libvirt."
-- 
2.24.1

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



[libvirt] [PATCH 16/16] autogen.sh: remove --no-git and --gnulib-srcdir options

2020-01-09 Thread Pavel Hrdina
Now that we have bootstrap output stored in git there is no need for
these options.

Signed-off-by: Pavel Hrdina 
---
 autogen.sh | 38 +++---
 docs/compiling.html.in | 13 -
 2 files changed, 3 insertions(+), 48 deletions(-)

diff --git a/autogen.sh b/autogen.sh
index 78c4320af5..b705b3f298 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -18,40 +18,8 @@ test -f src/libvirt.c || {
 die "$0 must live in the top-level libvirt directory"
 }
 
-no_git=
-gnulib_srcdir=
-while test "$#" -gt 0; do
-case "$1" in
---no-git)
-no_git=" $1"
-shift
-;;
---gnulib-srcdir=*)
-gnulib_srcdir=" $1"
-shift
-;;
---gnulib-srcdir)
-gnulib_srcdir=" $1=$2"
-shift
-shift
-;;
-*)
-# All remaining arguments will be passed to configure verbatim
-break
-;;
-esac
-done
-no_git="$no_git$gnulib_srcdir"
-
 gnulib_hash()
 {
-local no_git=$1
-
-if test "$no_git"; then
-echo "no-git"
-return
-fi
-
 # Compute the hash we'll use to determine whether rerunning bootstrap
 # is required. The first is just the SHA1 that selects a gnulib snapshot.
 # The second ensures that whenever we change the set of gnulib modules used
@@ -78,7 +46,7 @@ if test -d .git || test -f .git; then
 esac
 done
 fi
-if test "$CLEAN_SUBMODULE" && test -z "$no_git"; then
+if test "$CLEAN_SUBMODULE"; then
 echo "Cleaning up submodules..."
 git submodule foreach 'git clean -dfqx && git reset --hard' || {
 die "Cleaning up submodules failed"
@@ -97,7 +65,7 @@ if test -d .git || test -f .git; then
 # successful bootstrap run, is stored on disk
 state_file=.git-module-status
 expected_hash=$(cat "$state_file" 2>/dev/null)
-actual_hash=$(gnulib_hash "$no_git")
+actual_hash=$(gnulib_hash)
 
 if test "$actual_hash" = "$expected_hash"; then
 # The gnulib hash matches our expectations, and all the files
@@ -114,7 +82,7 @@ if test -d .git || test -f .git; then
 # run bootstrap again. If we're performing a dry run, we
 # change the return code instead to signal our caller
 echo "Running bootstrap..."
-./bootstrap$no_git || {
+./bootstrap || {
 die "bootstrap failed"
 }
 gnulib_hash >"$state_file"
diff --git a/docs/compiling.html.in b/docs/compiling.html.in
index 52fd5f5397..be1cc42189 100644
--- a/docs/compiling.html.in
+++ b/docs/compiling.html.in
@@ -80,19 +80,6 @@ $ sudo make install
   disk space requirements and network download time, regardless of
   which actual commit you have in that reference directory.
 
-
-  However, if you are developing on a platform where git is not
-  available, or are behind a firewall that does not allow for git
-  to easily obtain the gnulib submodule, it is possible to instead
-  use a static mode of operation where you are then responsible
-  for updating the git submodule yourself.  In this mode, you must
-  track the exact gnulib commit needed by libvirt (usually not the
-  latest gnulib.git) via alternative means, such as a shared NFS
-  drive or manual download, and run this any time libvirt.git
-  updates the commit stored in the .gnulib submodule:
-
-$ GNULIB_SRCDIR=/path/to/gnulib ./autogen.sh --no-git
-
 
 To build  install libvirt to your home
   directory the following commands can be run:
-- 
2.24.1

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



[libvirt] [PATCH 15/16] autogen.sh: remove --dry-run option

2020-01-09 Thread Pavel Hrdina
Signed-off-by: Pavel Hrdina 
---
 autogen.sh | 44 +---
 1 file changed, 13 insertions(+), 31 deletions(-)

diff --git a/autogen.sh b/autogen.sh
index 49ca91bf64..78c4320af5 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -18,20 +18,10 @@ test -f src/libvirt.c || {
 die "$0 must live in the top-level libvirt directory"
 }
 
-dry_run=
 no_git=
 gnulib_srcdir=
 while test "$#" -gt 0; do
 case "$1" in
---dry-run)
-# This variable will serve both as an indicator of the fact that
-# a dry run has been requested, and to store the result of the
-# dry run. It will be ultimately used as return code for the
-# script: 0 means no action is necessary, 2 means that autogen.sh
-# needs to be executed, and 1 is reserved for failures
-dry_run=0
-shift
-;;
 --no-git)
 no_git=" $1"
 shift
@@ -89,12 +79,10 @@ if test -d .git || test -f .git; then
 done
 fi
 if test "$CLEAN_SUBMODULE" && test -z "$no_git"; then
-if test -z "$dry_run"; then
-echo "Cleaning up submodules..."
-git submodule foreach 'git clean -dfqx && git reset --hard' || {
-die "Cleaning up submodules failed"
-}
-fi
+echo "Cleaning up submodules..."
+git submodule foreach 'git clean -dfqx && git reset --hard' || {
+die "Cleaning up submodules failed"
+}
 fi
 
 # Update all submodules. If any of the submodules has not been
@@ -116,25 +104,19 @@ if test -d .git || test -f .git; then
 # that can only be generated through bootstrap are present:
 # we just need to run autoreconf. Unless we're performing a
 # dry run, of course...
-if test -z "$dry_run"; then
-echo "Running autoreconf..."
-autoreconf -v || {
-die "autoreconf failed"
-}
-fi
+echo "Running autoreconf..."
+autoreconf -v || {
+die "autoreconf failed"
+}
 else
 # Whenever the gnulib submodule or any of the related bits
 # has been changed in some way (see gnulib_hash) we need to
 # run bootstrap again. If we're performing a dry run, we
 # change the return code instead to signal our caller
-if test "$dry_run"; then
-dry_run=2
-else
-echo "Running bootstrap..."
-./bootstrap$no_git || {
-die "bootstrap failed"
-}
-gnulib_hash >"$state_file"
-fi
+echo "Running bootstrap..."
+./bootstrap$no_git || {
+die "bootstrap failed"
+}
+gnulib_hash >"$state_file"
 fi
 fi
-- 
2.24.1

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



[libvirt] [PATCH 13/16] replace any autogen.sh reference with configure

2020-01-09 Thread Pavel Hrdina
>From now on there is no need to run autogen.sh in order to build
libvirt from clean git checkout as the result of running autogen.sh
is stored in git.

Signed-off-by: Pavel Hrdina 
---
 .gitlab-ci.yml | 2 +-
 .travis.yml| 4 ++--
 README-hacking | 5 ++---
 ci/build.sh| 1 -
 docs/compiling.html.in | 8 +++-
 5 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index ea49c6178b..99ec6b82bf 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -2,7 +2,7 @@
   script:
 - mkdir build
 - cd build
-- ../autogen.sh $CONFIGURE_OPTS || (cat config.log && exit 1)
+- ../configure $CONFIGURE_OPTS || (cat config.log && exit 1)
 - make -j $(getconf _NPROCESSORS_ONLN)
 
 # We could run every arch on every versions, but it is a little
diff --git a/.travis.yml b/.travis.yml
index b243e3d5c4..86d6ffea6a 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -81,7 +81,7 @@ matrix:
 # We can't run 'distcheck' or 'syntax-check' because they fail on
 # macOS, but doing 'install' and 'dist' gives us some useful coverage
 - mkdir build && cd build
-- ../autogen.sh --prefix=$(pwd)/install-root && make -j3 && make -j3 
install && make -j3 dist
+- ../configure --prefix=$(pwd)/install-root && make -j3 && make -j3 
install && make -j3 dist
 - compiler: clang
   language: c
   os: osx
@@ -96,7 +96,7 @@ matrix:
 # We can't run 'distcheck' or 'syntax-check' because they fail on
 # macOS, but doing 'install' and 'dist' gives us some useful coverage
 - mkdir build && cd build
-- ../autogen.sh --prefix=$(pwd)/install-root && make -j3 && make -j3 
install && make -j3 dist
+- ../configure --prefix=$(pwd)/install-root && make -j3 && make -j3 
install && make -j3 dist
 
 git:
   submodules: true
diff --git a/README-hacking b/README-hacking
index 7da940eb13..29589ce445 100644
--- a/README-hacking
+++ b/README-hacking
@@ -38,10 +38,9 @@ We require to have the build directory different than the 
source directory:
 
 $ mkdir build && cd build
 
-The next step is to get all required pieces from gnulib,
-to run autoreconf, and to invoke ../autogen.sh:
+The next step is to run configure:
 
-$ ../autogen.sh
+$ ../configure
 
 And there you are!  Just
 
diff --git a/ci/build.sh b/ci/build.sh
index 0874c2d1d9..476e45d513 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -11,7 +11,6 @@ mkdir -p "$CI_CONT_BUILDDIR" || exit 1
 cd "$CI_CONT_BUILDDIR"
 
 export VIR_TEST_DEBUG=1
-NOCONFIGURE=1 "$CI_CONT_SRCDIR/autogen.sh" || exit 1
 
 # $CONFIGURE_OPTS is a env that can optionally be set in the container,
 # populated at build time from the Dockerfile. A typical use case would
diff --git a/docs/compiling.html.in b/docs/compiling.html.in
index 3731bf0873..52fd5f5397 100644
--- a/docs/compiling.html.in
+++ b/docs/compiling.html.in
@@ -63,9 +63,7 @@ $ sudo make install
 Building from a GIT checkout
 
 
-  The libvirt build process uses GNU autotools, so after obtaining a
-  checkout it is necessary to generate the configure script and Makefile.in
-  templates using the autogen.sh command. By default when
+  The libvirt build process uses GNU autotools. By default when
   the configure script is run from within a GIT checkout, it
   will turn on -Werror for builds. This can be disabled with
   --disable-werror, but this is not recommended.
@@ -101,7 +99,7 @@ $ GNULIB_SRCDIR=/path/to/gnulib ./autogen.sh --no-git
 
 
 
-$ ./autogen.sh --prefix=$HOME/usr
+$ ./configure --prefix=$HOME/usr
 $ make
 $ sudo make install
 
@@ -113,7 +111,7 @@ $ sudo make install
 
 
 
-$ ./autogen.sh --with-system
+$ ./configure --with-system
 $ make
 
 
-- 
2.24.1

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



[libvirt] [PATCH 08/16] bootstrap.conf: stop creating AUTHORS file

2020-01-09 Thread Pavel Hrdina
The existence of AUTHORS file is required for GNU projects but since
commit <8bfb36db40f38e92823b657b5a342652064b5adc> we do not require
these files to exist.

Signed-off-by: Pavel Hrdina 
---
 autogen.sh | 2 +-
 bootstrap.conf | 3 ---
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/autogen.sh b/autogen.sh
index a119b099cb..a1f1943adb 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -127,7 +127,7 @@ if test -d .git || test -f .git; then
 expected_hash=$(cat "$state_file" 2>/dev/null)
 actual_hash=$(gnulib_hash "$no_git")
 
-if test "$actual_hash" = "$expected_hash" && test -f AUTHORS; then
+if test "$actual_hash" = "$expected_hash"; then
 # The gnulib hash matches our expectations, and all the files
 # that can only be generated through bootstrap are present:
 # we just need to run autoreconf. Unless we're performing a
diff --git a/bootstrap.conf b/bootstrap.conf
index d682afc2a7..99207b9b8a 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -172,9 +172,6 @@ xmllint-
 xsltproc   -
 "
 
-# Automake requires that AUTHORS exist.
-touch AUTHORS || exit 1
-
 # Override bootstrap's list - we don't use mdate-sh or texinfo.tex.
 gnulib_extra_files="
 build-aux/install-sh
-- 
2.24.1

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



[libvirt] [PATCH 00/16] include result of bootstrap in git

2020-01-09 Thread Pavel Hrdina
This patch series is motivated by the effort to adopt Meson as our
build system but all of it improves the current build system on its own.

It moves the burden to run bootstrap/autoreconf to developers which
means it will save some time for others when there is any change to
the build system.

Since one of the patches is slightly bigger here is link to git:



Pavel Hrdina (16):
  secret: move virSecretGetSecretString into virsecret
  configure.ac: add check for getegid function
  bootstrap.conf: drop gnulib tests from libvirt
  bootstrap.conf: add pthread-h module
  bootstrap.conf: always copy files
  bootstrap.conf: declare bootstrap sync in configuration file
  bootstrap.conf: disable VC ignore files
  bootstrap.conf: stop creating AUTHORS file
  syntax-check: remove deleted daemon directory from space_indent_check
  autogen.sh: fix autoreconf step
  autogen.sh: move --system option to configure as --with-system
  gnulib: store result of bootstrap in git
  replace any autogen.sh reference with configure
  autogen.sh: remove configure related code
  autogen.sh: remove --dry-run option
  autogen.sh: remove --no-git and --gnulib-srcdir options

 .git-module-status | 2 +
 .gitignore |18 -
 .gitlab-ci.yml | 2 +-
 .travis.yml| 4 +-
 INSTALL|   368 +
 Makefile.am| 2 +-
 Makefile.in|  2323 +
 README-hacking | 5 +-
 aclocal.m4 |  1719 +
 autogen.sh |   150 +-
 bootstrap.conf |29 +-
 build-aux/compile  |   348 +
 build-aux/config.guess |  1667 +
 build-aux/config.sub   |  1793 +
 build-aux/depcomp  |   791 +
 build-aux/install-sh   |   529 +
 build-aux/ltmain.sh| 11149 
 build-aux/missing  |   215 +
 build-aux/syntax-check.mk  |23 +-
 build-aux/test-driver  |   148 +
 ci/build.sh| 1 -
 config.h.in|  2303 +
 configure  | 56615 +++
 configure.ac   |16 +-
 docs/Makefile.in   |  2614 +
 docs/compiling.html.in |21 +-
 examples/Makefile.in   |  2494 +
 gnulib/lib/Makefile.in |  3997 ++
 gnulib/lib/_Noreturn.h |40 +
 gnulib/lib/accept.c|52 +
 gnulib/lib/alloca.c|   478 +
 gnulib/lib/alloca.in.h |71 +
 gnulib/lib/arg-nonnull.h   |26 +
 gnulib/lib/arpa_inet.in.h  |   150 +
 gnulib/lib/asnprintf.c |34 +
 gnulib/lib/assure.h|37 +
 gnulib/lib/binary-io.c |39 +
 gnulib/lib/binary-io.h |77 +
 gnulib/lib/bind.c  |49 +
 gnulib/lib/c++defs.h   |   316 +
 gnulib/lib/chown.c |   151 +
 gnulib/lib/cloexec.c   |83 +
 gnulib/lib/cloexec.h   |38 +
 gnulib/lib/close.c |71 +
 gnulib/lib/connect.c   |56 +
 gnulib/lib/dup2.c  |   235 +
 gnulib/lib/errno.in.h  |   279 +
 gnulib/lib/fchown-stub.c   |16 +
 gnulib/lib/fcntl.c |   627 +
 gnulib/lib/fcntl.in.h  |   392 +
 gnulib/lib/fd-hook.c   |   116 +
 gnulib/lib/fd-hook.h   |   119 +
 gnulib/lib/filename.h  |54 +
 gnulib/lib/float+.h|   147 +
 gnulib/lib/float.c |33 +
 gnulib/lib/float.in.h  |   188 +
 gnulib/lib/fseek.c |30 +
 gnulib/lib/fseeko.c|   164 +
 gnulib/lib/fstat.c |94 +
 gnulib/lib/gai_strerror.c  |92 +
 gnulib/lib/getaddrinfo.c   |   473 +
 gnulib/lib/getdelim.c  |   147 +
 gnulib/lib/getdtablesize.c |   124 +
 gnulib/lib/getgroups.c |   131 +
 gnulib/lib/gethostname.c   |   104 +
 gnulib/lib/getline.c   |27 +
 gnulib/lib/getpass.c   |   231 +
 gnulib/lib/getpass.h   | 4 +
 gnulib/lib/getpeername.c   |49 +
 

[libvirt] [PATCH 05/16] bootstrap.conf: always copy files

2020-01-09 Thread Pavel Hrdina
Preparation for having bootstrap result in git.

Signed-off-by: Pavel Hrdina 
---
 bootstrap.conf | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index a302475198..7009d22681 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -129,11 +129,7 @@ gnulib_modules="$gnulib_modules wcwidth"
 
 SKIP_PO=true
 
-# Enable copy-mode for MSYS/MinGW. MSYS' ln doesn't work well in the way
-# bootstrap uses it with relative paths.
-if test -n "$MSYSTEM"; then
-copy=true
-fi
+copy=true
 
 
 # Tell gnulib to:
-- 
2.24.1

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



[libvirt] [PATCH 10/16] autogen.sh: fix autoreconf step

2020-01-09 Thread Pavel Hrdina
Running bootstrap and autoreconf from autogen.sh produced different
files in build-aux directory.  The reason is that gnulib usually have
newer version of these files and overwrites them after the autoreconf
step.

In order to fix it remove the --install and --force options, in addition
introduce --verbose option in order to reflect what bootstrap is doing.

Signed-off-by: Pavel Hrdina 
---
 autogen.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/autogen.sh b/autogen.sh
index a1f1943adb..47446dc785 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -134,7 +134,7 @@ if test -d .git || test -f .git; then
 # dry run, of course...
 if test -z "$dry_run"; then
 echo "Running autoreconf..."
-autoreconf -if || {
+autoreconf -v || {
 die "autoreconf failed"
 }
 fi
-- 
2.24.1

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



[libvirt] [PATCH 11/16] autogen.sh: move --system option to configure as --with-system

2020-01-09 Thread Pavel Hrdina
As preparation to run configure directly when building libvirt.

Signed-off-by: Pavel Hrdina 
---
 autogen.sh | 22 --
 configure.ac   | 14 ++
 docs/compiling.html.in |  2 +-
 3 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/autogen.sh b/autogen.sh
index 47446dc785..75e8b70d4e 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -23,7 +23,6 @@ test -f src/libvirt.c || {
 dry_run=
 no_git=
 gnulib_srcdir=
-extra_args=
 while test "$#" -gt 0; do
 case "$1" in
 --dry-run)
@@ -48,19 +47,6 @@ while test "$#" -gt 0; do
 shift
 shift
 ;;
---system)
-prefix=/usr
-sysconfdir=/etc
-localstatedir=/var
-if test -d $prefix/lib64; then
-libdir=$prefix/lib64
-else
-libdir=$prefix/lib
-fi
-extra_args="--prefix=$prefix --localstatedir=$localstatedir"
-extra_args="$extra_args --sysconfdir=$sysconfdir --libdir=$libdir"
-shift
-;;
 *)
 # All remaining arguments will be passed to configure verbatim
 break
@@ -187,19 +173,19 @@ test "$MAKE" || {
 die "GNU make is required to build libvirt"
 }
 
-if test -z "$*" && test -z "$extra_args" && test -f config.status; then
+if test -z "$*" && test -f config.status; then
 echo "Running config.status..."
 ./config.status --recheck || {
 die "config.status failed"
 }
 else
-if test -z "$*" && test -z "$extra_args"; then
+if test -z "$*"; then
 echo "I am going to run configure with no arguments - if you wish"
 echo "to pass any to it, please specify them on the $0 command line."
 else
-echo "Running configure with $extra_args $@"
+echo "Running configure with $@"
 fi
-"$srcdir/configure" $extra_args "$@" || {
+"$srcdir/configure" "$@" || {
 die "configure failed"
 }
 fi
diff --git a/configure.ac b/configure.ac
index 2be909b7ce..d5926b9b90 100644
--- a/configure.ac
+++ b/configure.ac
@@ -151,6 +151,20 @@ then
   runstatedir=$with_runstatedir
 fi
 
+LIBVIRT_ARG_WITH([SYSTEM],
+ [configure libvirt to be compatible with normal OS vendor 
prefixes],
+ [no])
+if test "x$with_system" != "xno"; then
+prefix=/usr
+sysconfdir=/etc
+localstatedir=/var
+if test -d $prefix/lib64; then
+libdir=$prefix/lib64
+else
+libdir=$prefix/lib
+fi
+fi
+
 
 gl_EARLY
 gl_INIT
diff --git a/docs/compiling.html.in b/docs/compiling.html.in
index 5869ebb90f..3731bf0873 100644
--- a/docs/compiling.html.in
+++ b/docs/compiling.html.in
@@ -113,7 +113,7 @@ $ sudo make install
 
 
 
-$ ./autogen.sh --system
+$ ./autogen.sh --with-system
 $ make
 
 
-- 
2.24.1

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



[libvirt] [PATCH 01/16] secret: move virSecretGetSecretString into virsecret

2020-01-09 Thread Pavel Hrdina
The function virSecretGetSecretString calls into secret driver and is
used from other hypervisros drivers and as such makes more sense in
util.

Signed-off-by: Pavel Hrdina 
---
 po/POTFILES.in |   1 -
 src/libvirt_private.syms   |   5 +-
 src/libxl/libxl_conf.c |   2 +-
 src/qemu/qemu_domain.c |   2 +-
 src/qemu/qemu_process.c|   2 +-
 src/qemu/qemu_tpm.c|   2 +-
 src/secret/Makefile.inc.am |  11 ---
 src/secret/secret_util.c   | 102 -
 src/secret/secret_util.h   |  33 ---
 src/storage/storage_backend_iscsi.c|   2 +-
 src/storage/storage_backend_iscsi_direct.c |   2 +-
 src/storage/storage_backend_rbd.c  |   2 +-
 src/storage/storage_util.c |   2 +-
 src/util/virsecret.c   |  69 ++
 src/util/virsecret.h   |   8 ++
 15 files changed, 86 insertions(+), 159 deletions(-)
 delete mode 100644 src/secret/secret_util.c
 delete mode 100644 src/secret/secret_util.h

diff --git a/po/POTFILES.in b/po/POTFILES.in
index faf173584e..e266871907 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -190,7 +190,6 @@
 @SRCDIR@/src/rpc/virnetsshsession.c
 @SRCDIR@/src/rpc/virnettlscontext.c
 @SRCDIR@/src/secret/secret_driver.c
-@SRCDIR@/src/secret/secret_util.c
 @SRCDIR@/src/security/security_apparmor.c
 @SRCDIR@/src/security/security_dac.c
 @SRCDIR@/src/security/security_driver.c
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b97906b852..b1a56f1261 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1449,10 +1449,6 @@ virLogManagerFree;
 virLogManagerNew;
 
 
-# secret/secret_util.h
-virSecretGetSecretString;
-
-
 # security/security_driver.h
 virSecurityDriverLookup;
 
@@ -3001,6 +2997,7 @@ virSecurityLabelDefNew;
 
 
 # util/virsecret.h
+virSecretGetSecretString;
 virSecretLookupDefClear;
 virSecretLookupDefCopy;
 virSecretLookupFormatSecret;
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 2488bb9d32..e41e84e3e2 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -41,7 +41,7 @@
 #include "libxl_conf.h"
 #include "libxl_utils.h"
 #include "virstoragefile.h"
-#include "secret_util.h"
+#include "virsecret.h"
 #include "cpu/cpu.h"
 #include "xen_common.h"
 #include "xen_xl.h"
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 24e84a5966..ec8207b36f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -56,7 +56,7 @@
 #include "vircrypto.h"
 #include "virrandom.h"
 #include "virsystemd.h"
-#include "secret_util.h"
+#include "virsecret.h"
 #include "logging/log_manager.h"
 #include "locking/domain_lock.h"
 #include "virdomainsnapshotobjlist.h"
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 4195042194..3c2f2458b5 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -83,7 +83,7 @@
 #include "virnuma.h"
 #include "virstring.h"
 #include "virhostdev.h"
-#include "secret_util.h"
+#include "virsecret.h"
 #include "configmake.h"
 #include "nwfilter_conf.h"
 #include "netdev_bandwidth_conf.h"
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 28800a100c..262e6c4f07 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -42,7 +42,7 @@
 #include "configmake.h"
 #include "qemu_tpm.h"
 #include "virtpm.h"
-#include "secret_util.h"
+#include "virsecret.h"
 
 #define VIR_FROM_THIS VIR_FROM_NONE
 
diff --git a/src/secret/Makefile.inc.am b/src/secret/Makefile.inc.am
index d332060e38..4f0956a7a4 100644
--- a/src/secret/Makefile.inc.am
+++ b/src/secret/Makefile.inc.am
@@ -5,11 +5,6 @@ SECRET_DRIVER_SOURCES = \
secret/secret_driver.c \
$(NULL)
 
-SECRET_UTIL_SOURCES = \
-   secret/secret_util.h \
-   secret/secret_util.c \
-   $(NULL)
-
 
 DRIVER_SOURCE_FILES += $(addprefix $(srcdir)/,$(SECRET_DRIVER_SOURCES))
 STATEFUL_DRIVER_SOURCE_FILES += \
@@ -17,14 +12,8 @@ STATEFUL_DRIVER_SOURCE_FILES += \
 
 EXTRA_DIST += \
$(SECRET_DRIVER_SOURCES) \
-   $(SECRET_UTIL_SOURCES) \
$(NULL)
 
-noinst_LTLIBRARIES += libvirt_secret.la
-libvirt_la_BUILT_LIBADD += libvirt_secret.la
-libvirt_secret_la_CFLAGS = $(AM_CFLAGS)
-libvirt_secret_la_LDFLAGS = $(AM_LDFLAGS)
-libvirt_secret_la_SOURCES = $(SECRET_UTIL_SOURCES)
 
 if WITH_SECRETS
 mod_LTLIBRARIES += libvirt_driver_secret.la
diff --git a/src/secret/secret_util.c b/src/secret/secret_util.c
deleted file mode 100644
index 27e164a425..00
--- a/src/secret/secret_util.c
+++ /dev/null
@@ -1,102 +0,0 @@
-/*
- * secret_util.c: secret related utility functions
- *
- * Copyright (C) 2016 Red Hat, Inc.
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the 

[libvirt] [PATCH 09/16] syntax-check: remove deleted daemon directory from space_indent_check

2020-01-09 Thread Pavel Hrdina
Signed-off-by: Pavel Hrdina 
---
 build-aux/syntax-check.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index 7e7c59c3df..b2f4bf59f8 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -572,7 +572,7 @@ sc_size_of_brackets:
 # Ensure that no C source file, docs, or rng schema uses TABs for
 # indentation.  Also match *.h.in files, to get libvirt.h.in.  Exclude
 # files in gnulib, since they're imported.
-space_indent_files=(\.(aug(\.in)?|rng|s?[ch](\.in)?|html.in|py|pl|syms)|(daemon|tools)/.*\.in)
+space_indent_files=(\.(aug(\.in)?|rng|s?[ch](\.in)?|html.in|py|pl|syms)|tools/.*\.in)
 sc_TAB_in_indentation:
@prohibit='^ *  ' \
in_vc_files='$(space_indent_files)$$' \
-- 
2.24.1

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



[libvirt] [PATCH 07/16] bootstrap.conf: disable VC ignore files

2020-01-09 Thread Pavel Hrdina
We already ignore most of these files and the .gitignore files as well.

Signed-off-by: Pavel Hrdina 
---
 .gitignore | 5 -
 bootstrap.conf | 3 +++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index 2139d176da..949bd3bc5a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -14,9 +14,12 @@
 /INSTALL
 /aclocal.m4
 /autom4te.cache
-/build-aux/.gitignore
 /build-aux/compile
+/build-aux/config.guess
+/build-aux/config.sub
 /build-aux/depcomp
+/build-aux/install-sh
+/build-aux/ltmain.sh
 /build-aux/missing
 /build-aux/test-driver
 /config.h.in
diff --git a/bootstrap.conf b/bootstrap.conf
index e0253a6f7d..d682afc2a7 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -133,6 +133,8 @@ copy=true
 
 bootstrap_sync=true
 
+vc_ignore=
+
 
 # Tell gnulib to:
 #   require LGPLv2+
@@ -147,6 +149,7 @@ gnulib_tool_option_extras="\
  --lgpl=2\
  --makefile-name=gnulib.mk\
  --avoid=pt_chown\
+ --no-vc-files\
 "
 local_gl_dir=gnulib/local
 
-- 
2.24.1

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



[libvirt] [PATCH 06/16] bootstrap.conf: declare bootstrap sync in configuration file

2020-01-09 Thread Pavel Hrdina
Signed-off-by: Pavel Hrdina 
---
 autogen.sh | 2 +-
 bootstrap.conf | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/autogen.sh b/autogen.sh
index 9afad8f9d5..a119b099cb 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -147,7 +147,7 @@ if test -d .git || test -f .git; then
 dry_run=2
 else
 echo "Running bootstrap..."
-./bootstrap$no_git --bootstrap-sync || {
+./bootstrap$no_git || {
 die "bootstrap failed"
 }
 gnulib_hash >"$state_file"
diff --git a/bootstrap.conf b/bootstrap.conf
index 7009d22681..e0253a6f7d 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -131,6 +131,8 @@ SKIP_PO=true
 
 copy=true
 
+bootstrap_sync=true
+
 
 # Tell gnulib to:
 #   require LGPLv2+
-- 
2.24.1

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



[libvirt] [PATCH 03/16] bootstrap.conf: drop gnulib tests from libvirt

2020-01-09 Thread Pavel Hrdina
We are in process of removing gnulib and adopting meson as our build
system.  In order to help with the transition let's drop gnulib tests.

This will also help with the fact that before we will be able to drop
gnulib completely we will store output of bootstrap in git.

Signed-off-by: Pavel Hrdina 
---
 .gitignore   |  2 --
 Makefile.am  |  2 +-
 bootstrap.conf   | 13 -
 configure.ac |  1 -
 gnulib/tests/Makefile.am | 32 
 5 files changed, 1 insertion(+), 49 deletions(-)
 delete mode 100644 gnulib/tests/Makefile.am

diff --git a/.gitignore b/.gitignore
index 2d6e3e3194..2139d176da 100644
--- a/.gitignore
+++ b/.gitignore
@@ -27,12 +27,10 @@ Makefile.in
 
 # gnulib related ignores
 !/gnulib/lib/Makefile.am
-!/gnulib/tests/Makefile.am
 *.rej
 *~
 /gnulib/lib/*
 /gnulib/m4/*
-/gnulib/tests/*
 
 # git related ignores
 *.orig
diff --git a/Makefile.am b/Makefile.am
index 0d7ccc74db..8a8eecb697 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -23,7 +23,7 @@ GENHTML = genhtml
 # so force it explicitly
 DISTCHECK_CONFIGURE_FLAGS = --enable-werror
 
-SUBDIRS = . gnulib/lib include/libvirt src tools docs gnulib/tests \
+SUBDIRS = . gnulib/lib include/libvirt src tools docs \
   tests po examples
 
 XZ_OPT ?= -v -T0
diff --git a/bootstrap.conf b/bootstrap.conf
index ae9ecb4039..13d0e77514 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -143,13 +143,10 @@ fi
 gnulib_name=libgnu
 m4_base=m4
 source_base=gnulib/lib
-tests_base=gnulib/tests
 gnulib_tool_option_extras="\
  --lgpl=2\
- --with-tests\
  --makefile-name=gnulib.mk\
  --avoid=pt_chown\
- --avoid=lock-tests\
 "
 local_gl_dir=gnulib/local
 
@@ -184,16 +181,6 @@ gnulib_extra_files="
 doc/INSTALL
 "
 
-
-bootstrap_post_import_hook()
-{
-  # Change paths in gnulib/tests/gnulib.mk from "../../.." to "../..",
-  # and make tests conditional by changing "TESTS" to "GNULIB_TESTS".
-  m=gnulib/tests/gnulib.mk
-  sed 's,\.\./\.\./\.\.,../..,g; s/^TESTS /GNULIB_TESTS /' $m > $m-t
-  mv -f $m-t $m
-}
-
 bootstrap_epilogue()
 {
 echo "$0: done.  Now you can run 'mkdir build && cd build && 
../configure'."
diff --git a/configure.ac b/configure.ac
index cbfb3fe1aa..2be909b7ce 100644
--- a/configure.ac
+++ b/configure.ac
@@ -922,7 +922,6 @@ AC_CONFIG_FILES([run],
 AC_CONFIG_FILES([\
 Makefile src/Makefile include/libvirt/Makefile docs/Makefile \
 gnulib/lib/Makefile \
-gnulib/tests/Makefile \
 .color_coded \
 .ycm_extra_conf.py \
 libvirt.pc \
diff --git a/gnulib/tests/Makefile.am b/gnulib/tests/Makefile.am
deleted file mode 100644
index 7062cbaf87..00
--- a/gnulib/tests/Makefile.am
+++ /dev/null
@@ -1,32 +0,0 @@
-## Makefile for gnulib/lib
-
-## Copyright (C) 2011, 2013 Red Hat, Inc.
-##
-## This library is free software; you can redistribute it and/or
-## modify it under the terms of the GNU Lesser General Public
-## License as published by the Free Software Foundation; either
-## version 2.1 of the License, or (at your option) any later version.
-##
-## This library is distributed in the hope that it will be useful,
-## but WITHOUT ANY WARRANTY; without even the implied warranty of
-## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-## Lesser General Public License for more details.
-##
-## You should have received a copy of the GNU Lesser General Public
-## License along with this library.  If not, see
-## .
-
-include gnulib.mk
-
-GNULIB_TESTS0 =
-GNULIB_TESTS1 = $(GNULIB_TESTS)
-if WITH_EXPENSIVE_TESTS
-## Automake requires that at least one conditional call out all tests to
-## be run, for those tests to be shipped in the tarball
-TESTS = $(GNULIB_TESTS)
-endif WITH_EXPENSIVE_TESTS
-## However, we want to change the set of tests based on the make environment,
-## where the default was set at configure time.  Use GNU make constructs to
-## hide our actions from Automake, so we don't get it too confused.
-VIR_TEST_EXPENSIVE ?= $(VIR_TEST_EXPENSIVE_DEFAULT)
-$(eval TESTS=$(GNULIB_TESTS$(VIR_TEST_EXPENSIVE)))
-- 
2.24.1

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



[libvirt] [PATCH 02/16] configure.ac: add check for getegid function

2020-01-09 Thread Pavel Hrdina
We already use this function and so far we've been lucky that the same
check is done by gnulib.  This will change once we will drop gnulib and
also make it obvious that we have to do the same check in Meson as well.

Signed-off-by: Pavel Hrdina 
---
 configure.ac | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configure.ac b/configure.ac
index 002a3dcdb0..cbfb3fe1aa 100644
--- a/configure.ac
+++ b/configure.ac
@@ -355,6 +355,7 @@ dnl and various less common threadsafe functions
 AC_CHECK_FUNCS_ONCE([\
   cfmakeraw \
   fallocate \
+  getegid \
   geteuid \
   getgid \
   getifaddrs \
-- 
2.24.1

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



[libvirt] [PATCH 04/16] bootstrap.conf: add pthread-h module

2020-01-09 Thread Pavel Hrdina
We use this module but we don't have explicit dependency on it.

Signed-off-by: Pavel Hrdina 
---
 bootstrap.conf | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/bootstrap.conf b/bootstrap.conf
index 13d0e77514..a302475198 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -84,6 +84,8 @@ gnulib_modules="$gnulib_modules pipe2"
 gnulib_modules="$gnulib_modules poll"
 # -> Meson
 gnulib_modules="$gnulib_modules posix-shell"
+# -> GThread
+gnulib_modules="$gnulib_modules pthread-h"
 # -> open code conditional logic
 gnulib_modules="$gnulib_modules pthread_sigmask"
 # -> GSocket
-- 
2.24.1

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



  1   2   >