Re: [libvirt] [PATCH 00/18] Enhance virpcimock and test VFIO instead of KVM

[Daniel Henrique Barboza]

On 8/16/19 5:59 AM, Michal Privoznik wrote:

On 8/14/19 5:14 PM, Daniel Henrique Barboza wrote:

I've run make check with each individual patch, and everything
seems fine in my environment.

For all patches:

Tested-by: Daniel Henrique Barboza 


I'll see if I can drop some code reviews later on.


That'd be great because we are lacking reviewers (just like other 
projects) and these are very specific.


Just dropped reviews in all of them. A few comments in a couple of patches
that you can amend before pushing if you think it's worth it (not worth
another spin, in my opinion).

I couldn't help but wonder: can't we just remove the KVM stub (pci-assign)
support from the code base entirely? QEMU dropped it in 2.11. The kernel
dropped it in 4.12 (mid of 2017). Not sure if there is any existing, running
guests relying on pci-assign these days that can't move to vfio-pci.

If there are still users for pci-assign, perhaps a deprecation notice is in
order (a warning when launching the VM?). Then users can't complain
that the support was removed and they were thrown under the bus.


(... I mean, they will still complain about it, but at least we warned 
about it)




Thanks,


DHB







Thanks,
Michal


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

Re: [libvirt] [PATCH 18/18] virpcitest: Use modern VFIO

[Daniel Henrique Barboza]

Reviewed-by: Daniel Henrique Barboza 

On 8/14/19 8:57 AM, Michal Privoznik wrote:

The pci-stub is so old school that no one uses it. All modern
systems have adapted VFIO. Switch our virpcitest too.

Signed-off-by: Michal Privoznik 
---
  tests/virpcitest.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/virpcitest.c b/tests/virpcitest.c
index 9ecd1b7d27..0bd37268ef 100644
--- a/tests/virpcitest.c
+++ b/tests/virpcitest.c
@@ -106,12 +106,12 @@ testVirPCIDeviceDetach(const void *opaque 
ATTRIBUTE_UNUSED)
  if (!(dev[i] = virPCIDeviceNew(0, 0, i + 1, 0)))
  goto cleanup;
  
-virPCIDeviceSetStubDriver(dev[i], VIR_PCI_STUB_DRIVER_KVM);

+virPCIDeviceSetStubDriver(dev[i], VIR_PCI_STUB_DRIVER_VFIO);
  
  if (virPCIDeviceDetach(dev[i], activeDevs, inactiveDevs) < 0)

  goto cleanup;
  
-if (testVirPCIDeviceCheckDriver(dev[i], "pci-stub") < 0)

+if (testVirPCIDeviceCheckDriver(dev[i], "vfio-pci") < 0)
  goto cleanup;
  
  CHECK_LIST_COUNT(activeDevs, 0);

@@ -147,7 +147,7 @@ testVirPCIDeviceReset(const void *opaque ATTRIBUTE_UNUSED)
  if (!(dev[i] = virPCIDeviceNew(0, 0, i + 1, 0)))
  goto cleanup;
  
-virPCIDeviceSetStubDriver(dev[i], VIR_PCI_STUB_DRIVER_KVM);

+virPCIDeviceSetStubDriver(dev[i], VIR_PCI_STUB_DRIVER_VFIO);
  
  if (virPCIDeviceReset(dev[i], activeDevs, inactiveDevs) < 0)

  goto cleanup;
@@ -187,7 +187,7 @@ testVirPCIDeviceReattach(const void *opaque 
ATTRIBUTE_UNUSED)
  CHECK_LIST_COUNT(activeDevs, 0);
  CHECK_LIST_COUNT(inactiveDevs, i + 1);
  
-virPCIDeviceSetStubDriver(dev[i], VIR_PCI_STUB_DRIVER_KVM);

+virPCIDeviceSetStubDriver(dev[i], VIR_PCI_STUB_DRIVER_VFIO);
  }
  
  CHECK_LIST_COUNT(activeDevs, 0);

@@ -245,7 +245,7 @@ testVirPCIDeviceDetachSingle(const void *opaque)
  if (!dev)
  goto cleanup;
  
-virPCIDeviceSetStubDriver(dev, VIR_PCI_STUB_DRIVER_KVM);

+virPCIDeviceSetStubDriver(dev, VIR_PCI_STUB_DRIVER_VFIO);
  
  if (virPCIDeviceDetach(dev, NULL, NULL) < 0)

  goto cleanup;
@@ -405,7 +405,7 @@ mymain(void)
  /* Detach an unbound device */
  DO_TEST_PCI_DRIVER(0, 0x0a, 2, 0, NULL);
  DO_TEST_PCI(testVirPCIDeviceDetachSingle, 0, 0x0a, 2, 0);
-DO_TEST_PCI_DRIVER(0, 0x0a, 2, 0, "pci-stub");
+DO_TEST_PCI_DRIVER(0, 0x0a, 2, 0, "vfio-pci");
  
  /* Reattach an unknown unbound device */

  DO_TEST_PCI_DRIVER(0, 0x0a, 3, 0, NULL);


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

Re: [libvirt] [PATCH 15/18] virhostdev: Unify virHostdevPreparePCIDevices behaviour for KVM and VFIO cases

[Daniel Henrique Barboza]

Reviewed-by: Daniel Henrique Barboza 

On 8/14/19 8:57 AM, Michal Privoznik wrote:

The virHostdevPreparePCIDevices() function works in several
steps. In the very first one, it checks if devices we want to
detach from the host are not taken already by some other domain.
However, this piece of code returns different results depending
on the stub driver used (which is not wrong per se, but keep on
reading). If the stub driver is KVM then
virHostdevIsPCINodeDeviceUsed() is called which basically checks
if a PCI device from the detach list is not used by any domain
(including the one we are preparing the device for). If that is
the case, an error is reported ("device in use") and -1 is
returned.

However, that is not what happens if the stub driver is VFIO. If
the stub driver is VFIO, then we iterate over all PCI devices
from the same IOMMU group and check if they are taken by some
other domain (because a PCI device, well IOMMU group, can't be
shared between two or more qemu processes). But we fail to check,
if the device we are trying to detach from the host is not
already taken by a domain. That is, calling
virHostdevPreparePCIDevices() over a hostdev device twice
succeeds the first time and fails too late in the second run
(fortunately, virHostdevResetAllPCIDevices() will throw an error,
but this is already too late because the PCI device in question
was moved to the list of inactive PCI devices and now it appears
in both lists).

Signed-off-by: Michal Privoznik 
---
  src/util/virhostdev.c | 17 +
  1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index cb69582c21..6861b8a84e 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -53,7 +53,7 @@ struct virHostdevIsPCINodeDeviceUsedData {
  virHostdevManagerPtr mgr;
  const char *driverName;
  const char *domainName;
-const bool usesVFIO;
+bool usesVFIO;
  };
  
  /* This module makes heavy use of bookkeeping lists contained inside a

@@ -707,7 +707,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
  virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i);
  bool strict_acs_check = !!(flags & VIR_HOSTDEV_STRICT_ACS_CHECK);
  bool usesVFIO = (virPCIDeviceGetStubDriver(pci) == 
VIR_PCI_STUB_DRIVER_VFIO);
-struct virHostdevIsPCINodeDeviceUsedData data = {mgr, drv_name, 
dom_name, usesVFIO};
+struct virHostdevIsPCINodeDeviceUsedData data = {mgr, drv_name, 
dom_name, false};
  int hdrType = -1;
  
  if (virPCIGetHeaderType(pci, ) < 0)

@@ -728,18 +728,19 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
  }
  
  /* The device is in use by other active domain if

- * the dev is in list activePCIHostdevs. VFIO devices
- * belonging to same iommu group can't be shared
- * across guests.
- */
+ * the dev is in list activePCIHostdevs. */
  devAddr = virPCIDeviceGetAddress(pci);
+if (virHostdevIsPCINodeDeviceUsed(devAddr, ))
+goto cleanup;
+
+/* VFIO devices belonging to same IOMMU group can't be
+ * shared across guests. Check if that's the case. */
  if (usesVFIO) {
+data.usesVFIO = true;
  if (virPCIDeviceAddressIOMMUGroupIterate(devAddr,
   
virHostdevIsPCINodeDeviceUsed,
   ) < 0)
  goto cleanup;
-} else if (virHostdevIsPCINodeDeviceUsed(devAddr, )) {
-goto cleanup;
  }
  }
  


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

Re: [libvirt] [PATCH 16/18] qemuxml2argvtest: Switch to modern vfio backend

[Daniel Henrique Barboza]

Reviewed-by: Daniel Henrique Barboza 

On 8/14/19 8:57 AM, Michal Privoznik wrote:

The pci-assign device is so old school that no one uses it. All
modern systems have adapted VFIO. Switch our xml2argv test too.

Signed-off-by: Michal Privoznik 
---
  .../hostdev-pci-address-device.args|  2 +-
  tests/qemuxml2argvdata/hostdev-pci-address.args|  2 +-
  tests/qemuxml2argvdata/net-hostdev-bootorder.args  |  3 +--
  .../qemuxml2argvdata/net-hostdev-multidomain.args  |  2 +-
  tests/qemuxml2argvdata/net-hostdev.args|  2 +-
  tests/qemuxml2argvdata/pci-rom.args|  4 ++--
  tests/qemuxml2argvtest.c   | 14 +++---
  7 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/tests/qemuxml2argvdata/hostdev-pci-address-device.args 
b/tests/qemuxml2argvdata/hostdev-pci-address-device.args
index 5159b00ef1..79654f44bb 100644
--- a/tests/qemuxml2argvdata/hostdev-pci-address-device.args
+++ b/tests/qemuxml2argvdata/hostdev-pci-address-device.args
@@ -27,5 +27,5 @@ server,nowait \
  -usb \
  -drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \
  -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
--device pci-assign,host=:06:12.5,id=hostdev0,bus=pci.0,addr=0x3 \
+-device vfio-pci,host=:06:12.5,id=hostdev0,bus=pci.0,addr=0x3 \
  -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
diff --git a/tests/qemuxml2argvdata/hostdev-pci-address.args 
b/tests/qemuxml2argvdata/hostdev-pci-address.args
index fe6acaaf62..0a57a8f29e 100644
--- a/tests/qemuxml2argvdata/hostdev-pci-address.args
+++ b/tests/qemuxml2argvdata/hostdev-pci-address.args
@@ -27,4 +27,4 @@ server,nowait \
  -usb \
  -drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \
  -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
--device pci-assign,host=:06:12.5,id=hostdev0,bus=pci.0,addr=0x3
+-device vfio-pci,host=:06:12.5,id=hostdev0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/net-hostdev-bootorder.args 
b/tests/qemuxml2argvdata/net-hostdev-bootorder.args
index eefc247eed..515b0c58d3 100644
--- a/tests/qemuxml2argvdata/net-hostdev-bootorder.args
+++ b/tests/qemuxml2argvdata/net-hostdev-bootorder.args
@@ -27,5 +27,4 @@ server,nowait \
  -usb \
  -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
  -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=2 \
--device pci-assign,host=:03:07.1,id=hostdev0,bootindex=1,bus=pci.0,\
-addr=0x3
+-device vfio-pci,host=:03:07.1,id=hostdev0,bootindex=1,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/net-hostdev-multidomain.args 
b/tests/qemuxml2argvdata/net-hostdev-multidomain.args
index bf7686f49c..18c5e98188 100644
--- a/tests/qemuxml2argvdata/net-hostdev-multidomain.args
+++ b/tests/qemuxml2argvdata/net-hostdev-multidomain.args
@@ -27,5 +27,5 @@ server,nowait \
  -usb \
  -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
  -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
--device pci-assign,host=2424:21:1c.6,id=hostdev0,bus=pci.0,addr=0x3 \
+-device vfio-pci,host=2424:21:1c.6,id=hostdev0,bus=pci.0,addr=0x3 \
  -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
diff --git a/tests/qemuxml2argvdata/net-hostdev.args 
b/tests/qemuxml2argvdata/net-hostdev.args
index 94eac176f3..aa9e91db82 100644
--- a/tests/qemuxml2argvdata/net-hostdev.args
+++ b/tests/qemuxml2argvdata/net-hostdev.args
@@ -27,5 +27,5 @@ server,nowait \
  -usb \
  -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
  -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
--device pci-assign,host=:03:07.1,id=hostdev0,bus=pci.0,addr=0x3 \
+-device vfio-pci,host=:03:07.1,id=hostdev0,bus=pci.0,addr=0x3 \
  -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
diff --git a/tests/qemuxml2argvdata/pci-rom.args 
b/tests/qemuxml2argvdata/pci-rom.args
index 7235642ee8..4a5dc4161a 100644
--- a/tests/qemuxml2argvdata/pci-rom.args
+++ b/tests/qemuxml2argvdata/pci-rom.args
@@ -33,7 +33,7 @@ addr=0x3,rombar=1 \
  -netdev user,id=hostnet1 \
  -device 
virtio-net-pci,netdev=hostnet1,id=net1,mac=52:54:00:24:a5:9e,bus=pci.0,\
  addr=0x4,romfile=/etc/fake/bootrom.bin \
--device pci-assign,host=:06:12.5,id=hostdev0,bus=pci.0,addr=0x5,rombar=0 \
--device pci-assign,host=:06:12.6,id=hostdev1,bus=pci.0,addr=0x6,rombar=1,\
+-device vfio-pci,host=:06:12.5,id=hostdev0,bus=pci.0,addr=0x5,rombar=0 \
+-device vfio-pci,host=:06:12.6,id=hostdev1,bus=pci.0,addr=0x6,rombar=1,\
  romfile=/etc/fake/bootrom.bin \
  -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x7
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index c166fd18d6..26b512d246 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -496,7 +496,7 @@ testCompareXMLToArgv(const void *data)
  if 

Re: [libvirt] [PATCH 17/18] virhostdevtest: Use modern VFIO

[Daniel Henrique Barboza]

Reviewed-by: Daniel Henrique Barboza 

On 8/14/19 8:57 AM, Michal Privoznik wrote:

The pci-stub is so old school that no one uses it. All modern
systems have adapted VFIO. Switch our virhostdevtest too.

Signed-off-by: Michal Privoznik 
---
  tests/virhostdevtest.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c
index 20984f3442..f860426678 100644
--- a/tests/virhostdevtest.c
+++ b/tests/virhostdevtest.c
@@ -96,7 +96,7 @@ myInit(void)
  subsys.u.pci.addr.bus = 0;
  subsys.u.pci.addr.slot = i + 1;
  subsys.u.pci.addr.function = 0;
-subsys.u.pci.backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM;
+subsys.u.pci.backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO;
  hostdevs[i]->source.subsys = subsys;
  }
  
@@ -104,7 +104,7 @@ myInit(void)

  if (!(dev[i] = virPCIDeviceNew(0, 0, i + 1, 0)))
  goto cleanup;
  
-virPCIDeviceSetStubDriver(dev[i], VIR_PCI_STUB_DRIVER_KVM);

+virPCIDeviceSetStubDriver(dev[i], VIR_PCI_STUB_DRIVER_VFIO);
  }
  
  if (VIR_ALLOC(mgr) < 0)


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

Re: [libvirt] [PATCH 12/18] virpcimock: Create PCI devices under /sys/devices/pci*

[Daniel Henrique Barboza]

Reviewed-by: Daniel Henrique Barboza 

On 8/14/19 8:57 AM, Michal Privoznik wrote:

So far, we are creating devices directly under
/sys/bus/pci/devices/*. There is not much problem with it, but if
we really want to model kernel behaviour we need to create them
under /sys/devices/pci:BB and then only symlink them from the
old location.

Signed-off-by: Michal Privoznik 
---
  tests/virpcimock.c | 25 +
  1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index c10764dcdd..0774bf62d9 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -383,12 +383,17 @@ pci_device_get_path(const struct pciDevice *dev,
  if (!(devid = pci_address_format(>addr)))
  return NULL;
  
+/* PCI devices really do live under /sys/devices/pci:BB

+ * and then they are just symlinked to /sys/bus/pci/devices/
+ */
  if (file) {
-ignore_value(virAsprintfQuiet(, "%s" SYSFS_PCI_PREFIX 
"devices/%s/%s",
-  prefix, devid, file));
+ignore_value(virAsprintfQuiet(, 
"%s/sys/devices/pci%04x:%02x/%s/%s",
+  prefix, dev->addr.domain, dev->addr.bus,
+  devid, file));
  } else {
-ignore_value(virAsprintfQuiet(, "%s" SYSFS_PCI_PREFIX "devices/%s",
-  prefix, devid));
+ignore_value(virAsprintfQuiet(, "%s/sys/devices/pci%04x:%02x/%s",
+  prefix, dev->addr.domain, dev->addr.bus,
+  devid));
  }
  
  return ret;

@@ -400,6 +405,7 @@ pci_device_new_from_stub(const struct pciDevice *data)
  {
  struct pciDevice *dev;
  VIR_AUTOFREE(char *) devpath = NULL;
+VIR_AUTOFREE(char *) devsympath = NULL;
  VIR_AUTOFREE(char *) id = NULL;
  VIR_AUTOFREE(char *) devid = NULL;
  char *c;
@@ -488,6 +494,17 @@ pci_device_new_from_stub(const struct pciDevice *data)
  }
  make_symlink(devpath, "iommu_group", tmp);
  
+if (snprintf(tmp, sizeof(tmp),

+ "../../../devices/pci%04x:%02x/%s",
+ dev->addr.domain, dev->addr.bus, devid) < 0) {
+ABORT("@tmp overflow");
+}
+
+if (virAsprintfQuiet(, "%s" SYSFS_PCI_PREFIX "devices", 
fakerootdir) < 0)
+ABORT_OOM();
+
+make_symlink(devsympath, devid, tmp);
+
  if (pci_device_autobind(dev) < 0)
  ABORT("Unable to bind: %s", devid);
  


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

Re: [libvirt] [PATCH 13/18] virpcimock: Create symlink in /sys/kernel/iommu_groups/N/devices dir

[Daniel Henrique Barboza]

Reviewed-by: Daniel Henrique Barboza 

On 8/14/19 8:57 AM, Michal Privoznik wrote:

So far, we don't need to create anything under
/sys/kernel/iommu_groups/N/devices directory (which is symlinked
from /sys/bus/pci/devices/:BB:DD.F/iommu_group directory)
because virhostdevtest still tests the old KVM assignment and
thus has no notion of IOMMU groups. This will change in near
future though. And in order to discover devices belonging to the
same IOMMU group we need to do what kernel does - create symlinks
to devices.

Signed-off-by: Michal Privoznik 
---
  tests/virpcimock.c | 32 +---
  1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index 0774bf62d9..6f315217e2 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -400,6 +400,30 @@ pci_device_get_path(const struct pciDevice *dev,
  }
  
  
+static void

+pci_device_create_iommu(const struct pciDevice *dev,
+const char *devid)
+{
+VIR_AUTOFREE(char *) iommuPath = NULL;
+char tmp[256];
+
+if (virAsprintfQuiet(, "%s/sys/kernel/iommu_groups/%d/devices/",
+ fakerootdir, dev->iommuGroup) < 0)
+ABORT_OOM();
+
+if (virFileMakePath(iommuPath) < 0)
+ABORT("Unable to create: %s", iommuPath);
+
+if (snprintf(tmp, sizeof(tmp),
+ "../../../../devices/pci%04x:%02x/%s",
+ dev->addr.domain, dev->addr.bus, devid) < 0) {
+ABORT("@tmp overflow");
+}
+
+make_symlink(iommuPath, devid, tmp);
+}
+
+
  static void
  pci_device_new_from_stub(const struct pciDevice *data)
  {
@@ -480,13 +504,7 @@ pci_device_new_from_stub(const struct pciDevice *data)
  
  make_file(devpath, "driver_override", NULL, -1);
  
-if (snprintf(tmp, sizeof(tmp),

- "%s/../../../kernel/iommu_groups/%d",
- devpath, dev->iommuGroup) < 0) {
-ABORT("@tmp overflow");
-}
-if (virFileMakePath(tmp) < 0)
-ABORT("Unable to create %s", tmp);
+pci_device_create_iommu(dev, devid);
  
  if (snprintf(tmp, sizeof(tmp),

   "../../../kernel/iommu_groups/%d", dev->iommuGroup) < 0) {


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

Re: [libvirt] [PATCH 14/18] virhostdev: Check driver name too in virHostdevIsPCINodeDeviceUsed()

[Daniel Henrique Barboza]

Reviewed-by: Daniel Henrique Barboza 

On 8/14/19 8:57 AM, Michal Privoznik wrote:

It may happen that there are two domains with the same name in
two separate drivers (e.g. qemu and lxc). That is why for PCI
devices we track both names of driver and domain combination
which has taken the device. However, when we check if given PCI
device is in use (or PCI devices from the same IOMMU group) we
compare only domain name. This means that we can mistakenly claim
device as free to use while in fact it isn't.

Signed-off-by: Michal Privoznik 
---
  src/util/virhostdev.c | 11 ++-
  1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 75e44b5227..cb69582c21 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -51,6 +51,7 @@ static virHostdevManagerPtr virHostdevManagerNew(void);
  
  struct virHostdevIsPCINodeDeviceUsedData {

  virHostdevManagerPtr mgr;
+const char *driverName;
  const char *domainName;
  const bool usesVFIO;
  };
@@ -91,8 +92,8 @@ static int 
virHostdevIsPCINodeDeviceUsed(virPCIDeviceAddressPtr devAddr, void *o
  virPCIDeviceGetUsedBy(actual, _drvname, _domname);
  
  if (helperData->usesVFIO &&

-(actual_domname && helperData->domainName) &&
-(STREQ(actual_domname, helperData->domainName)))
+STREQ_NULLABLE(actual_drvname, helperData->driverName) &&
+STREQ_NULLABLE(actual_domname, helperData->domainName))
  goto iommu_owner;
  
  if (actual_drvname && actual_domname)

@@ -706,7 +707,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
  virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i);
  bool strict_acs_check = !!(flags & VIR_HOSTDEV_STRICT_ACS_CHECK);
  bool usesVFIO = (virPCIDeviceGetStubDriver(pci) == 
VIR_PCI_STUB_DRIVER_VFIO);
-struct virHostdevIsPCINodeDeviceUsedData data = { mgr, dom_name, 
usesVFIO };
+struct virHostdevIsPCINodeDeviceUsedData data = {mgr, drv_name, 
dom_name, usesVFIO};
  int hdrType = -1;
  
  if (virPCIGetHeaderType(pci, ) < 0)

@@ -1995,7 +1996,7 @@ int
  virHostdevPCINodeDeviceDetach(virHostdevManagerPtr mgr,
virPCIDevicePtr pci)
  {
-struct virHostdevIsPCINodeDeviceUsedData data = { mgr, NULL, false };
+struct virHostdevIsPCINodeDeviceUsedData data = {mgr, NULL, NULL, false};
  int ret = -1;
  
  virObjectLock(mgr->activePCIHostdevs);

@@ -2021,7 +2022,7 @@ int
  virHostdevPCINodeDeviceReAttach(virHostdevManagerPtr mgr,
  virPCIDevicePtr pci)
  {
-struct virHostdevIsPCINodeDeviceUsedData data = { mgr, NULL, false };
+struct virHostdevIsPCINodeDeviceUsedData data = {mgr, NULL, NULL, false};
  int ret = -1;
  
  virObjectLock(mgr->activePCIHostdevs);


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

Re: [libvirt] [PATCH 10/18] virpcimock: Introduce and use pci_driver_get_path()

[Daniel Henrique Barboza]

Reviewed-by: Daniel Henrique Barboza 

On 8/14/19 8:57 AM, Michal Privoznik wrote:

Have just one function to generate path to a PCI driver so that
when we change it in near future there's only few of the places
we need to fix.

Signed-off-by: Michal Privoznik 
---
  tests/virpcimock.c | 34 +++---
  1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index 52a585d0cc..de365cdb78 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -503,6 +503,29 @@ pci_device_autobind(struct pciDevice *dev)
  /*
   * PCI Driver functions
   */
+static char *
+pci_driver_get_path(const struct pciDriver *driver,
+const char *file,
+bool faked)
+{
+char *ret = NULL;
+const char *prefix = "";
+
+if (faked)
+prefix = fakerootdir;
+
+if (file) {
+ignore_value(virAsprintfQuiet(, "%s" SYSFS_PCI_PREFIX 
"drivers/%s/%s",
+  prefix, driver->name, file));
+} else {
+ignore_value(virAsprintfQuiet(, "%s" SYSFS_PCI_PREFIX "drivers/%s",
+  prefix, driver->name));
+}
+
+return ret;
+}
+
+
  static void
  pci_driver_new(const char *name, int fail, ...)
  {
@@ -513,7 +536,7 @@ pci_driver_new(const char *name, int fail, ...)
  
  if (VIR_ALLOC_QUIET(driver) < 0 ||

  VIR_STRDUP_QUIET(driver->name, name) < 0 ||
-virAsprintfQuiet(, "%s/sys/bus/pci/drivers/%s", fakerootdir, 
name) < 0)
+!(driverpath = pci_driver_get_path(driver, NULL, true)))
  ABORT_OOM();
  
  driver->fail = fail;

@@ -619,8 +642,7 @@ pci_driver_bind(struct pciDriver *driver,
  
  /* Make symlink under device tree */

  if (!(devpath = pci_device_get_path(dev, "driver", true)) ||
-virAsprintfQuiet(, "%s/sys/bus/pci/drivers/%s",
- fakerootdir, driver->name) < 0) {
+!(driverpath = pci_driver_get_path(driver, NULL, true))) {
  errno = ENOMEM;
  return -1;
  }
@@ -632,8 +654,7 @@ pci_driver_bind(struct pciDriver *driver,
  VIR_FREE(devpath);
  VIR_FREE(driverpath);
  if (!(devpath = pci_device_get_path(dev, NULL, true)) ||
-virAsprintfQuiet(, "%s/sys/bus/pci/drivers/%s/%s",
- fakerootdir, driver->name, dev->id) < 0) {
+!(driverpath = pci_driver_get_path(driver, dev->id, true))) {
  errno = ENOMEM;
  return -1;
  }
@@ -660,8 +681,7 @@ pci_driver_unbind(struct pciDriver *driver,
  
  /* Make symlink under device tree */

  if (!(devpath = pci_device_get_path(dev, "driver", true)) ||
-virAsprintfQuiet(, "%s/sys/bus/pci/drivers/%s/%s",
- fakerootdir, driver->name, dev->id) < 0) {
+!(driverpath = pci_driver_get_path(driver, dev->id, true))) {
  errno = ENOMEM;
  return -1;
  }


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

Re: [libvirt] [PATCH 09/18] virpcimock: Introduce and use pci_device_get_path()

[Daniel Henrique Barboza]

Reviewed-by: Daniel Henrique Barboza 

On 8/14/19 8:57 AM, Michal Privoznik wrote:

Have just one function to generate path to a PCI device so that
when we change it in near future there's only few of the places
we need to fix.

Signed-off-by: Michal Privoznik 
---
  tests/virpcimock.c | 42 +++---
  1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index 213f906500..52a585d0cc 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -337,6 +337,29 @@ remove_fd(int fd)
  /*
   * PCI Device functions
   */
+static char *
+pci_device_get_path(const struct pciDevice *dev,
+const char *file,
+bool faked)
+{
+char *ret = NULL;
+const char *prefix = "";
+
+if (faked)
+prefix = fakerootdir;
+
+if (file) {
+ignore_value(virAsprintfQuiet(, "%s" SYSFS_PCI_PREFIX 
"devices/%s/%s",
+  prefix, dev->id, file));
+} else {
+ignore_value(virAsprintfQuiet(, "%s" SYSFS_PCI_PREFIX "devices/%s",
+  prefix, dev->id));
+}
+
+return ret;
+}
+
+
  static void
  pci_device_new_from_stub(const struct pciDevice *data)
  {
@@ -365,12 +388,14 @@ pci_device_new_from_stub(const struct pciDevice *data)
  
  if (VIR_ALLOC_QUIET(dev) < 0 ||

  virAsprintfQuiet(, "%s/virpcitestdata/%s.config",
- abs_srcdir, id) < 0 ||
-virAsprintfQuiet(, "%s/sys/bus/pci/devices/%s", fakerootdir, 
data->id) < 0)
+ abs_srcdir, id) < 0)
  ABORT_OOM();
  
  memcpy(dev, data, sizeof(*dev));
  
+if (!(devpath = pci_device_get_path(dev, NULL, true)))

+ABORT_OOM();
+
  if (virFileMakePath(devpath) < 0)
  ABORT("Unable to create: %s", devpath);
  
@@ -563,9 +588,7 @@ pci_driver_find_by_driver_override(struct pciDevice *dev)

  char tmp[32];
  size_t i;
  
-if (virAsprintfQuiet(,

- SYSFS_PCI_PREFIX "devices/%s/driver_override",
- dev->id) < 0)
+if (!(path = pci_device_get_path(dev, "driver_override", false)))
  return NULL;
  
  if (pci_read_file(path, tmp, sizeof(tmp), false) < 0)

@@ -595,8 +618,7 @@ pci_driver_bind(struct pciDriver *driver,
  }
  
  /* Make symlink under device tree */

-if (virAsprintfQuiet(, "%s/sys/bus/pci/devices/%s/driver",
- fakerootdir, dev->id) < 0 ||
+if (!(devpath = pci_device_get_path(dev, "driver", true)) ||
  virAsprintfQuiet(, "%s/sys/bus/pci/drivers/%s",
   fakerootdir, driver->name) < 0) {
  errno = ENOMEM;
@@ -609,8 +631,7 @@ pci_driver_bind(struct pciDriver *driver,
  /* Make symlink under driver tree */
  VIR_FREE(devpath);
  VIR_FREE(driverpath);
-if (virAsprintfQuiet(, "%s/sys/bus/pci/devices/%s",
- fakerootdir, dev->id) < 0 ||
+if (!(devpath = pci_device_get_path(dev, NULL, true)) ||
  virAsprintfQuiet(, "%s/sys/bus/pci/drivers/%s/%s",
   fakerootdir, driver->name, dev->id) < 0) {
  errno = ENOMEM;
@@ -638,8 +659,7 @@ pci_driver_unbind(struct pciDriver *driver,
  }
  
  /* Make symlink under device tree */

-if (virAsprintfQuiet(, "%s/sys/bus/pci/devices/%s/driver",
- fakerootdir, dev->id) < 0 ||
+if (!(devpath = pci_device_get_path(dev, "driver", true)) ||
  virAsprintfQuiet(, "%s/sys/bus/pci/drivers/%s/%s",
   fakerootdir, driver->name, dev->id) < 0) {
  errno = ENOMEM;


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

Re: [libvirt] [PATCH 07/18] virpcimock: Rename @fakesysfspcidir

[Daniel Henrique Barboza]

Reviewed-by: Daniel Henrique Barboza 

On 8/14/19 8:57 AM, Michal Privoznik wrote:

We will need to create more directories and instead of
introducing bunch of new variables to hold their actual
paths, we can have one and reuse it.

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

diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index 296da9b453..e97dbd81f8 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -831,7 +831,7 @@ init_syms(void)
  static void
  init_env(void)
  {
-VIR_AUTOFREE(char *) fakesysfspcidir = NULL;
+VIR_AUTOFREE(char *) tmp = NULL;
  
  if (fakerootdir)

  return;
@@ -839,14 +839,14 @@ init_env(void)
  if (!(fakerootdir = getenv("LIBVIRT_FAKE_ROOT_DIR")))
  ABORT("Missing LIBVIRT_FAKE_ROOT_DIR env variable\n");
  
-if (virAsprintfQuiet(, "%s%s",

+if (virAsprintfQuiet(, "%s%s",
   fakerootdir, SYSFS_PCI_PREFIX) < 0)
  ABORT_OOM();
  
-if (virFileMakePath(fakesysfspcidir) < 0)

-ABORT("Unable to create: %s", fakesysfspcidir);
+if (virFileMakePath(tmp) < 0)
+ABORT("Unable to create: %s", tmp);
  
-make_file(fakesysfspcidir, "drivers_probe", NULL, -1);

+make_file(tmp, "drivers_probe", NULL, -1);
  
  # define MAKE_PCI_DRIVER(name, ...) \

  pci_driver_new(name, 0, __VA_ARGS__, -1, -1)


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

Re: [libvirt] [PATCH 05/18] virpcimock: Use VIR_AUTOFREE()

[Daniel Henrique Barboza]

Reviewed-by: Daniel Henrique Barboza 

On 8/14/19 8:57 AM, Michal Privoznik wrote:

It saves us couple of lines.

Signed-off-by: Michal Privoznik 
---
  tests/virpcimock.c | 109 +++--
  1 file changed, 36 insertions(+), 73 deletions(-)

diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index 853ac588e9..0950f3ba00 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -172,7 +172,7 @@ make_file(const char *path,
ssize_t len)
  {
  int fd = -1;
-char *filepath = NULL;
+VIR_AUTOFREE(char *) filepath = NULL;
  if (value && len == -1)
  len = strlen(value);
  
@@ -186,7 +186,6 @@ make_file(const char *path,

  ABORT("Unable to write: %s", filepath);
  
  VIR_FORCE_CLOSE(fd);

-VIR_FREE(filepath);
  }
  
  static void

@@ -194,15 +193,13 @@ make_symlink(const char *path,
const char *name,
const char *target)
  {
-char *filepath = NULL;
+VIR_AUTOFREE(char *) filepath = NULL;
  
  if (virAsprintfQuiet(, "%s/%s", path, name) < 0)

  ABORT_OOM();
  
  if (symlink(target, filepath) < 0)

  ABORT("Unable to create symlink filepath -> target");
-
-VIR_FREE(filepath);
  }
  
  static int

@@ -213,7 +210,7 @@ pci_read_file(const char *path,
  {
  int ret = -1;
  int fd = -1;
-char *newpath;
+VIR_AUTOFREE(char *) newpath = NULL;
  
  if (virAsprintfQuiet(, "%s/%s",

   fakesysfspcidir,
@@ -237,7 +234,6 @@ pci_read_file(const char *path,
  
  ret = 0;

   cleanup:
-VIR_FREE(newpath);
  real_close(fd);
  return ret;
  }
@@ -335,10 +331,10 @@ static void
  pci_device_new_from_stub(const struct pciDevice *data)
  {
  struct pciDevice *dev;
-char *devpath;
-char *id;
+VIR_AUTOFREE(char *) devpath = NULL;
+VIR_AUTOFREE(char *) id = NULL;
  char *c;
-char *configSrc;
+VIR_AUTOFREE(char *) configSrc = NULL;
  char tmp[256];
  struct stat sb;
  bool configSrcExists = false;
@@ -363,7 +359,6 @@ pci_device_new_from_stub(const struct pciDevice *data)
  virAsprintfQuiet(, "%s/devices/%s", fakesysfspcidir, data->id) 
< 0)
  ABORT_OOM();
  
-VIR_FREE(id);

  memcpy(dev, data, sizeof(*dev));
  
  if (virFileMakePath(devpath) < 0)

@@ -380,14 +375,13 @@ pci_device_new_from_stub(const struct pciDevice *data)
   * file, and parallel VPATH builds must not stomp on the
   * original; besides, 'make distcheck' requires the original
   * to be read-only */
-char *buf;
+VIR_AUTOFREE(char *) buf = NULL;
  ssize_t len;
  
  if ((len = virFileReadAll(configSrc, 4096, )) < 0)

  ABORT("Unable to read config file '%s'", configSrc);
  
  make_file(devpath, "config", buf, len);

-VIR_FREE(buf);
  } else {
  /* If there's no config data in the virpcitestdata dir, create a dummy
   * config file */
@@ -427,9 +421,6 @@ pci_device_new_from_stub(const struct pciDevice *data)
  
  if (VIR_APPEND_ELEMENT_QUIET(pciDevices, nPCIDevices, dev) < 0)

  ABORT_OOM();
-
-VIR_FREE(devpath);
-VIR_FREE(configSrc);
  }
  
  static struct pciDevice *

@@ -483,7 +474,7 @@ pci_driver_new(const char *name, int fail, ...)
  struct pciDriver *driver;
  va_list args;
  int vendor, device;
-char *driverpath;
+VIR_AUTOFREE(char *) driverpath = NULL;
  
  if (VIR_ALLOC_QUIET(driver) < 0 ||

  VIR_STRDUP_QUIET(driver->name, name) < 0 ||
@@ -519,8 +510,6 @@ pci_driver_new(const char *name, int fail, ...)
  
  if (VIR_APPEND_ELEMENT_QUIET(pciDrivers, nPCIDrivers, driver) < 0)

  ABORT_OOM();
-
-VIR_FREE(driverpath);
  }
  
  static struct pciDriver *

@@ -586,13 +575,13 @@ static int
  pci_driver_bind(struct pciDriver *driver,
  struct pciDevice *dev)
  {
-int ret = -1;
-char *devpath = NULL, *driverpath = NULL;
+VIR_AUTOFREE(char *) devpath = NULL;
+VIR_AUTOFREE(char *) driverpath = NULL;
  
  if (dev->driver) {

  /* Device already bound */
  errno = ENODEV;
-return ret;
+return -1;
  }
  
  /* Make symlink under device tree */

@@ -601,11 +590,11 @@ pci_driver_bind(struct pciDriver *driver,
  virAsprintfQuiet(, "%s/drivers/%s",
   fakesysfspcidir, driver->name) < 0) {
  errno = ENOMEM;
-goto cleanup;
+return -1;
  }
  
  if (symlink(driverpath, devpath) < 0)

-goto cleanup;
+return -1;
  
  /* Make symlink under driver tree */

  VIR_FREE(devpath);
@@ -615,31 +604,27 @@ pci_driver_bind(struct pciDriver *driver,
  virAsprintfQuiet(, "%s/drivers/%s/%s",
   fakesysfspcidir, driver->name, dev->id) < 0) {
  errno = ENOMEM;
-goto cleanup;
+return -1;
  }
  
  if (symlink(devpath, driverpath) < 0)

-   

Re: [libvirt] [PATCH 04/18] virpcimock: Drop needless typecast

[Daniel Henrique Barboza]

Reviewed-by: Daniel Henrique Barboza 

On 8/14/19 8:57 AM, Michal Privoznik wrote:

When creating a PCI device, the pciDevice structure contains @id
member which holds device address (.BB:DD.F) and is type of
'char *'. But the structure is initialized from a const char and
in fact we never modify or free the @id.

Signed-off-by: Michal Privoznik 
---
  tests/virpcimock.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index 1c21e4e045..853ac588e9 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -120,7 +120,7 @@ struct pciDriver {
  };
  
  struct pciDevice {

-char *id;
+const char *id;
  int vendor;
  int device;
  int klass;
@@ -878,7 +878,7 @@ init_env(void)
  
  # define MAKE_PCI_DEVICE(Id, Vendor, Device, ...) \

  do { \
-struct pciDevice dev = {.id = (char *)Id, .vendor = Vendor, \
+struct pciDevice dev = {.id = Id, .vendor = Vendor, \
  .device = Device, __VA_ARGS__}; \
  pci_device_new_from_stub(); \
  } while (0)


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

Re: [libvirt] [PATCH 03/18] virpcimock: Create driver_override file in device dirs

[Daniel Henrique Barboza]

Reviewed-by: Daniel Henrique Barboza 

On 8/14/19 8:57 AM, Michal Privoznik wrote:

Newer kernels (v3.16-rc1~29^2~6^4) have 'driver_override' file
which simplifies way of binding a PCI device to desired driver.
Libvirt has support for this for some time too (v2.3.0-rc1~236),
but not our virpcimock. So far we did not care because our code
is designed to deal with this situation. Except for one.
hypothetical case: binding a device to the vfio-pci driver can be
successful only via driver_override. Any attempt to bind a PCI
device to vfio-pci driver using old method (new_id + unbind +
bind) will fail because of b803b29c1a5. While on vanilla kernel
I'm able to use the old method successfully, it's failing on RHEL
kernels (not sure why).

Signed-off-by: Michal Privoznik 
---
  tests/virpcimock.c | 52 --
  1 file changed, 46 insertions(+), 6 deletions(-)

diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index 6865f992dc..1c21e4e045 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -87,6 +87,11 @@ char *fakesysfspcidir;
   *   Probe for a driver that handles the specified device.
   *   Data in format ":BB:DD.F" (Domain:Bus:Device.Function).
   *
+ * /sys/bus/pci/devices//driver_override
+ *   Name of a driver that overrides preferred driver can be written
+ *   here. The device will be attached to it on drivers_probe event.
+ *   Writing an empty string (or "\n") clears the override.
+ *
   * As a little hack, we are not mocking write to these files, but close()
   * instead. The advantage is we don't need any self growing array to hold the
   * partial writes and construct them back. We can let all the writes finish,
@@ -147,6 +152,7 @@ static struct pciDevice *pci_device_find_by_content(const 
char *path);
  static void pci_driver_new(const char *name, int fail, ...);
  static struct pciDriver *pci_driver_find_by_dev(struct pciDevice *dev);
  static struct pciDriver *pci_driver_find_by_path(const char *path);
+static struct pciDriver *pci_driver_find_by_driver_override(struct pciDevice 
*dev);
  static int pci_driver_bind(struct pciDriver *driver, struct pciDevice *dev);
  static int pci_driver_unbind(struct pciDriver *driver, struct pciDevice *dev);
  static int pci_driver_handle_change(int fd, const char *path);
@@ -202,7 +208,8 @@ make_symlink(const char *path,
  static int
  pci_read_file(const char *path,
char *buf,
-  size_t buf_size)
+  size_t buf_size,
+  bool truncate)
  {
  int ret = -1;
  int fd = -1;
@@ -224,7 +231,8 @@ pci_read_file(const char *path,
  goto cleanup;
  }
  
-if (ftruncate(fd, 0) < 0)

+if (truncate &&
+ftruncate(fd, 0) < 0)
  goto cleanup;
  
  ret = 0;

@@ -398,6 +406,8 @@ pci_device_new_from_stub(const struct pciDevice *data)
  ABORT("@tmp overflow");
  make_file(devpath, "class", tmp, -1);
  
+make_file(devpath, "driver_override", NULL, -1);

+
  if (snprintf(tmp, sizeof(tmp),
   "%s/../../../kernel/iommu_groups/%d",
   devpath, dev->iommuGroup) < 0) {
@@ -441,7 +451,7 @@ pci_device_find_by_content(const char *path)
  {
  char tmp[32];
  
-if (pci_read_file(path, tmp, sizeof(tmp)) < 0)

+if (pci_read_file(path, tmp, sizeof(tmp), true) < 0)
  return NULL;
  
  return pci_device_find_by_id(tmp);

@@ -450,7 +460,10 @@ pci_device_find_by_content(const char *path)
  static int
  pci_device_autobind(struct pciDevice *dev)
  {
-struct pciDriver *driver = pci_driver_find_by_dev(dev);
+struct pciDriver *driver = pci_driver_find_by_driver_override(dev);
+
+if (!driver)
+driver = pci_driver_find_by_dev(dev);
  
  if (!driver) {

  /* No driver found. Nothing to do */
@@ -544,6 +557,31 @@ pci_driver_find_by_path(const char *path)
  return NULL;
  }
  
+static struct pciDriver *

+pci_driver_find_by_driver_override(struct pciDevice *dev)
+{
+VIR_AUTOFREE(char *) path = NULL;
+char tmp[32];
+size_t i;
+
+if (virAsprintfQuiet(,
+ SYSFS_PCI_PREFIX "devices/%s/driver_override",
+ dev->id) < 0)
+return NULL;
+
+if (pci_read_file(path, tmp, sizeof(tmp), false) < 0)
+return NULL;
+
+for (i = 0; i < nPCIDrivers; i++) {
+struct pciDriver *driver = pciDrivers[i];
+
+if (STREQ(tmp, driver->name))
+return driver;
+}
+
+return NULL;
+}
+
  static int
  pci_driver_bind(struct pciDriver *driver,
  struct pciDevice *dev)
@@ -657,6 +695,8 @@ pci_driver_handle_change(int fd ATTRIBUTE_UNUSED, const 
char *path)
  ret = pci_driver_handle_remove_id(path);
  else if (STREQ(file, "drivers_probe"))
  ret = pci_driver_handle_drivers_probe(path);
+else if (STREQ(file, "driver_override"))
+ret = 0; /* nada */
  else
  ABORT("Not handled write to: %s", path);
  

Re: [libvirt] [PATCH 06/18] virpcimock: Eliminate use of @fakesysfspcidir

[Daniel Henrique Barboza]

Reviewed-by: Daniel Henrique Barboza 

On 8/14/19 8:57 AM, Michal Privoznik wrote:

The @fakesysfspcidir is derived from @fakerootdir. We don't need
two global variables that contain nearly the same content,
especially when we construct the actual path anyways.

Signed-off-by: Michal Privoznik 
---
  tests/virpcimock.c | 41 -
  1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index 0950f3ba00..296da9b453 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -41,7 +41,6 @@ static char *(*real_virFileCanonicalizePath)(const char 
*path);
   * vircgroupmock.c:462:22: error: static variable 'fakesysfsdir' is used in 
an inline function with external linkage [-Werror,-Wstatic-in-inline]
   */
  char *fakerootdir;
-char *fakesysfspcidir;
  
  # define SYSFS_PCI_PREFIX "/sys/bus/pci/"
  
@@ -212,9 +211,7 @@ pci_read_file(const char *path,

  int fd = -1;
  VIR_AUTOFREE(char *) newpath = NULL;
  
-if (virAsprintfQuiet(, "%s/%s",

- fakesysfspcidir,
- path + strlen(SYSFS_PCI_PREFIX)) < 0) {
+if (virAsprintfQuiet(, "%s/%s", fakerootdir, path) < 0) {
  errno = ENOMEM;
  goto cleanup;
  }
@@ -245,8 +242,8 @@ getrealpath(char **newpath,
  init_env();
  
  if (STRPREFIX(path, SYSFS_PCI_PREFIX)) {

-if (virAsprintfQuiet(newpath, "%s/%s",
- fakesysfspcidir,
+if (virAsprintfQuiet(newpath, "%s/sys/bus/pci/%s",
+ fakerootdir,
   path + strlen(SYSFS_PCI_PREFIX)) < 0) {
  errno = ENOMEM;
  return -1;
@@ -356,7 +353,7 @@ pci_device_new_from_stub(const struct pciDevice *data)
  if (VIR_ALLOC_QUIET(dev) < 0 ||
  virAsprintfQuiet(, "%s/virpcitestdata/%s.config",
   abs_srcdir, id) < 0 ||
-virAsprintfQuiet(, "%s/devices/%s", fakesysfspcidir, data->id) 
< 0)
+virAsprintfQuiet(, "%s/sys/bus/pci/devices/%s", fakerootdir, 
data->id) < 0)
  ABORT_OOM();
  
  memcpy(dev, data, sizeof(*dev));

@@ -478,7 +475,7 @@ pci_driver_new(const char *name, int fail, ...)
  
  if (VIR_ALLOC_QUIET(driver) < 0 ||

  VIR_STRDUP_QUIET(driver->name, name) < 0 ||
-virAsprintfQuiet(, "%s/drivers/%s", fakesysfspcidir, name) 
< 0)
+virAsprintfQuiet(, "%s/sys/bus/pci/drivers/%s", fakerootdir, 
name) < 0)
  ABORT_OOM();
  
  driver->fail = fail;

@@ -585,10 +582,10 @@ pci_driver_bind(struct pciDriver *driver,
  }
  
  /* Make symlink under device tree */

-if (virAsprintfQuiet(, "%s/devices/%s/driver",
- fakesysfspcidir, dev->id) < 0 ||
-virAsprintfQuiet(, "%s/drivers/%s",
- fakesysfspcidir, driver->name) < 0) {
+if (virAsprintfQuiet(, "%s/sys/bus/pci/devices/%s/driver",
+ fakerootdir, dev->id) < 0 ||
+virAsprintfQuiet(, "%s/sys/bus/pci/drivers/%s",
+ fakerootdir, driver->name) < 0) {
  errno = ENOMEM;
  return -1;
  }
@@ -599,10 +596,10 @@ pci_driver_bind(struct pciDriver *driver,
  /* Make symlink under driver tree */
  VIR_FREE(devpath);
  VIR_FREE(driverpath);
-if (virAsprintfQuiet(, "%s/devices/%s",
- fakesysfspcidir, dev->id) < 0 ||
-virAsprintfQuiet(, "%s/drivers/%s/%s",
- fakesysfspcidir, driver->name, dev->id) < 0) {
+if (virAsprintfQuiet(, "%s/sys/bus/pci/devices/%s",
+ fakerootdir, dev->id) < 0 ||
+virAsprintfQuiet(, "%s/sys/bus/pci/drivers/%s/%s",
+ fakerootdir, driver->name, dev->id) < 0) {
  errno = ENOMEM;
  return -1;
  }
@@ -628,10 +625,10 @@ pci_driver_unbind(struct pciDriver *driver,
  }
  
  /* Make symlink under device tree */

-if (virAsprintfQuiet(, "%s/devices/%s/driver",
- fakesysfspcidir, dev->id) < 0 ||
-virAsprintfQuiet(, "%s/drivers/%s/%s",
- fakesysfspcidir, driver->name, dev->id) < 0) {
+if (virAsprintfQuiet(, "%s/sys/bus/pci/devices/%s/driver",
+ fakerootdir, dev->id) < 0 ||
+virAsprintfQuiet(, "%s/sys/bus/pci/drivers/%s/%s",
+ fakerootdir, driver->name, dev->id) < 0) {
  errno = ENOMEM;
  return -1;
  }
@@ -834,7 +831,9 @@ init_syms(void)
  static void
  init_env(void)
  {
-if (fakerootdir && fakesysfspcidir)
+VIR_AUTOFREE(char *) fakesysfspcidir = NULL;
+
+if (fakerootdir)
  return;
  
  if (!(fakerootdir = getenv("LIBVIRT_FAKE_ROOT_DIR")))


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

Re: [libvirt] [PATCH 02/18] Revert "virpcitest: Test virPCIDeviceDetach failure"

[Daniel Henrique Barboza]

Reviewed-by: Daniel Henrique Barboza 

On 8/14/19 8:57 AM, Michal Privoznik wrote:

This reverts commit b70c093ffa00cd87c8d39d3652b798f033a81faf.

In next commit the virpcimock is going to be extended and thus
binding a PCI device to vfio-pci driver will finally succeed.
Remove this test as it will no longer make sense.

Signed-off-by: Michal Privoznik 
---
  tests/virpcitest.c | 32 
  1 file changed, 32 deletions(-)

diff --git a/tests/virpcitest.c b/tests/virpcitest.c
index 961a7eff1a..9ecd1b7d27 100644
--- a/tests/virpcitest.c
+++ b/tests/virpcitest.c
@@ -256,36 +256,6 @@ testVirPCIDeviceDetachSingle(const void *opaque)
  return ret;
  }
  
-static int

-testVirPCIDeviceDetachFail(const void *opaque)
-{
-const struct testPCIDevData *data = opaque;
-int ret = -1;
-virPCIDevicePtr dev;
-
-dev = virPCIDeviceNew(data->domain, data->bus, data->slot, data->function);
-if (!dev)
-goto cleanup;
-
-virPCIDeviceSetStubDriver(dev, VIR_PCI_STUB_DRIVER_VFIO);
-
-if (virPCIDeviceDetach(dev, NULL, NULL) < 0) {
-if (virTestGetVerbose() || virTestGetDebug())
-virDispatchError(NULL);
-virResetLastError();
-ret = 0;
-} else {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "Attaching device %s to %s should have failed",
-   virPCIDeviceGetName(dev),
-   virPCIStubDriverTypeToString(VIR_PCI_STUB_DRIVER_VFIO));
-}
-
- cleanup:
-virPCIDeviceFree(dev);
-return ret;
-}
-
  static int
  testVirPCIDeviceReattachSingle(const void *opaque)
  {
@@ -421,8 +391,6 @@ mymain(void)
  DO_TEST_PCI(testVirPCIDeviceIsAssignable, 5, 0x90, 1, 0);
  DO_TEST_PCI(testVirPCIDeviceIsAssignable, 1, 1, 0, 0);
  
-DO_TEST_PCI(testVirPCIDeviceDetachFail, 0, 0x0a, 1, 0);

-
  /* Reattach a device already bound to non-stub a driver */
  DO_TEST_PCI_DRIVER(0, 0x0a, 1, 0, "i915");
  DO_TEST_PCI(testVirPCIDeviceReattachSingle, 0, 0x0a, 1, 0);


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

Re: [libvirt] [PATCH 01/18] virpcimock: Move actions checking one level up

[Daniel Henrique Barboza]

Reviewed-by: Daniel Henrique Barboza 

On 8/14/19 8:57 AM, Michal Privoznik wrote:

The pci_driver_bind() and pci_driver_unbind() functions are
"internal implementation", meaning other parts of the code should
be able to call them and get the job done. Checking for actions
(PCI_ACTION_BIND and PCI_ACTION_UNBIND) should be done in
handlers (pci_driver_handle_bind() and
pci_driver_handle_unbind()). Surprisingly, the other two actions
(PCI_ACTION_NEW_ID and PCI_ACTION_REMOVE_ID) are checked already
at this level.

Signed-off-by: Michal Privoznik 
---
  tests/virpcimock.c | 16 
  1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index beb5e1490d..6865f992dc 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -551,8 +551,8 @@ pci_driver_bind(struct pciDriver *driver,
  int ret = -1;
  char *devpath = NULL, *driverpath = NULL;
  
-if (dev->driver || PCI_ACTION_BIND & driver->fail) {

-/* Device already bound or failing driver requested */
+if (dev->driver) {
+/* Device already bound */
  errno = ENODEV;
  return ret;
  }
@@ -598,8 +598,8 @@ pci_driver_unbind(struct pciDriver *driver,
  int ret = -1;
  char *devpath = NULL, *driverpath = NULL;
  
-if (dev->driver != driver || PCI_ACTION_UNBIND & driver->fail) {

-/* Device not bound to the @driver or failing driver used */
+if (dev->driver != driver) {
+/* Device not bound to the @driver */
  errno = ENODEV;
  return ret;
  }
@@ -669,8 +669,8 @@ pci_driver_handle_bind(const char *path)
  struct pciDevice *dev = pci_device_find_by_content(path);
  struct pciDriver *driver = pci_driver_find_by_path(path);
  
-if (!driver || !dev) {

-/* This should never happen (TM) */
+if (!driver || !dev || PCI_ACTION_BIND & driver->fail) {
+/* No driver, no device or failing driver requested */
  errno = ENODEV;
  goto cleanup;
  }
@@ -686,8 +686,8 @@ pci_driver_handle_unbind(const char *path)
  int ret = -1;
  struct pciDevice *dev = pci_device_find_by_content(path);
  
-if (!dev || !dev->driver) {

-/* This should never happen (TM) */
+if (!dev || !dev->driver || PCI_ACTION_UNBIND & dev->driver->fail) {
+/* No device, device not binded or failing driver requested */
  errno = ENODEV;
  goto cleanup;
  }


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

Re: [libvirt] [PATCH 11/18] virpcimock: Store PCI address as ints not string

[Daniel Henrique Barboza]

On 8/14/19 8:57 AM, Michal Privoznik wrote:

In upcoming patches we will need only some portions of the PCI
address. To construct that easily, it's better if the PCI address
of a device is stored as four integers rather than one string.

Signed-off-by: Michal Privoznik 
---
  tests/virpcimock.c | 76 +-
  1 file changed, 61 insertions(+), 15 deletions(-)

diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index de365cdb78..c10764dcdd 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -118,8 +118,16 @@ struct pciDriver {
  unsigned int fail;  /* Bitwise-OR of driverActions that should fail */
  };
  
+struct pciDeviceAddress {

+unsigned int domain;
+unsigned int bus;
+unsigned int device;
+unsigned int function;
+};
+# define ADDR_STR_FMT "%04x:%02x:%02x.%d"
+


I was going to complain "this stuff is similar to what we already have
in utils/virpci.c, why can't we use it here", but in a second thought I
realized virpci.h is too big of a import just for a macro and a couple
of parse functions.


Reviewed-by: Daniel Henrique Barboza 




  struct pciDevice {
-const char *id;
+struct pciDeviceAddress addr;
  int vendor;
  int device;
  int klass;
@@ -145,7 +153,7 @@ static void init_env(void);
  
  static int pci_device_autobind(struct pciDevice *dev);

  static void pci_device_new_from_stub(const struct pciDevice *data);
-static struct pciDevice *pci_device_find_by_id(const char *id);
+static struct pciDevice *pci_device_find_by_id(struct pciDeviceAddress const 
*addr);
  static struct pciDevice *pci_device_find_by_content(const char *path);
  
  static void pci_driver_new(const char *name, int fail, ...);

@@ -337,6 +345,29 @@ remove_fd(int fd)
  /*
   * PCI Device functions
   */
+static char *
+pci_address_format(struct pciDeviceAddress const *addr)
+{
+char *ret;
+
+ignore_value(virAsprintfQuiet(, ADDR_STR_FMT,
+  addr->domain, addr->bus,
+  addr->device, addr->function));
+return ret;
+}
+
+static int
+pci_address_parse(struct pciDeviceAddress *addr,
+  const char *buf)
+{
+if (sscanf(buf, ADDR_STR_FMT,
+   >domain, >bus,
+   >device, >function) != 4)
+return -1;
+return 0;
+}
+
+
  static char *
  pci_device_get_path(const struct pciDevice *dev,
  const char *file,
@@ -344,16 +375,20 @@ pci_device_get_path(const struct pciDevice *dev,
  {
  char *ret = NULL;
  const char *prefix = "";
+VIR_AUTOFREE(char *) devid = NULL;
  
  if (faked)

  prefix = fakerootdir;
  
+if (!(devid = pci_address_format(>addr)))

+return NULL;
+
  if (file) {
  ignore_value(virAsprintfQuiet(, "%s" SYSFS_PCI_PREFIX 
"devices/%s/%s",
-  prefix, dev->id, file));
+  prefix, devid, file));
  } else {
  ignore_value(virAsprintfQuiet(, "%s" SYSFS_PCI_PREFIX 
"devices/%s",
-  prefix, dev->id));
+  prefix, devid));
  }
  
  return ret;

@@ -366,13 +401,15 @@ pci_device_new_from_stub(const struct pciDevice *data)
  struct pciDevice *dev;
  VIR_AUTOFREE(char *) devpath = NULL;
  VIR_AUTOFREE(char *) id = NULL;
+VIR_AUTOFREE(char *) devid = NULL;
  char *c;
  VIR_AUTOFREE(char *) configSrc = NULL;
  char tmp[256];
  struct stat sb;
  bool configSrcExists = false;
  
-if (VIR_STRDUP_QUIET(id, data->id) < 0)

+if (!(devid = pci_address_format(>addr)) ||
+VIR_STRDUP_QUIET(id, devid) < 0)
  ABORT_OOM();
  
  /* Replace ':' with '-' to create the config filename from the

@@ -452,20 +489,20 @@ pci_device_new_from_stub(const struct pciDevice *data)
  make_symlink(devpath, "iommu_group", tmp);
  
  if (pci_device_autobind(dev) < 0)

-ABORT("Unable to bind: %s", data->id);
+ABORT("Unable to bind: %s", devid);
  
  if (VIR_APPEND_ELEMENT_QUIET(pciDevices, nPCIDevices, dev) < 0)

  ABORT_OOM();
  }
  
  static struct pciDevice *

-pci_device_find_by_id(const char *id)
+pci_device_find_by_id(struct pciDeviceAddress const *addr)
  {
  size_t i;
  for (i = 0; i < nPCIDevices; i++) {
  struct pciDevice *dev = pciDevices[i];
  
-if (STREQ(dev->id, id))

+if (!memcmp(>addr, addr, sizeof(*addr)))
  return dev;
  }
  
@@ -476,11 +513,13 @@ static struct pciDevice *

  pci_device_find_by_content(const char *path)
  {
  char tmp[32];
+struct pciDeviceAddress addr;
  
-if (pci_read_file(path, tmp, sizeof(tmp), true) < 0)

+if (pci_read_file(path, tmp, sizeof(tmp), true) < 0 ||
+pci_address_parse(, tmp) < 0)
  return NULL;
  
-return pci_device_find_by_id(tmp);

+return pci_device_find_by_id();
  }
  
  static int

@@ 

Re: [libvirt] [PATCH 08/18] virpcimock: Create devices/ and drivers/ under /sys/bus/pci upfront

[Daniel Henrique Barboza]

On 8/14/19 8:57 AM, Michal Privoznik wrote:

In near future, we will be creating devices under different
location and just symlink them under devices/. Just like real
kernel does. But for that we need the directories to exists.


nit: s/exists/exist



Signed-off-by: Michal Privoznik 
---
  tests/virpcimock.c | 15 +++
  1 file changed, 15 insertions(+)

diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index e97dbd81f8..213f906500 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -187,6 +187,19 @@ make_file(const char *path,
  VIR_FORCE_CLOSE(fd);
  }
  
+static void

+make_dir(const char *path,
+ const char *name)
+{
+VIR_AUTOFREE(char *) dirpath = NULL;
+
+if (virAsprintfQuiet(, "%s/%s", path, name) < 0)
+ABORT_OOM();
+
+if (virFileMakePath(dirpath) < 0)
+ABORT("Unable to create: %s", dirpath);
+}
+
  static void
  make_symlink(const char *path,
const char *name,
@@ -846,6 +859,8 @@ init_env(void)
  if (virFileMakePath(tmp) < 0)
  ABORT("Unable to create: %s", tmp);
  
+make_dir(tmp, "devices");

+make_dir(tmp, "drivers");


make_dir could also be used to create the tmp directory as well, like
make_dir(fakerootdir, SYSFS_PCI_PREFIX), getting rid of all the
virFileMakePath() calls inside init_env(). Not worth rerolling the patch
just because of that though.


Reviewed-by: Daniel Henrique Barboza 




  make_file(tmp, "drivers_probe", NULL, -1);
  
  # define MAKE_PCI_DRIVER(name, ...) \


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

Re: [libvirt] [PATCH v2 1/6] test_driver: implement virDomainAttachDeviceFlags

[Erik Skultety]

On Fri, Aug 16, 2019 at 05:57:36PM +0300, Ilias Stamatis wrote:
> On Fri, Aug 16, 2019 at 5:39 PM Erik Skultety  wrote:
> >
> > ...
> >
> > > +
> > > +if (operation == TEST_DEVICE_DETACH)
> > > +parse_flags |= VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
> >
> > ^This should be added by patch 3/5
> >
> > > +
> > > +if (xml) {
> > > +if (!(dev = virDomainDeviceDefParse(xml, def,
> > > +driver->caps, driver->xmlopt,
> > > +NULL, parse_flags)))
> > > +goto cleanup;
> > > +} else if (alias) {
> > > +if (VIR_ALLOC(dev) < 0 || virDomainDefFindDevice(def, alias, 
> > > dev, true) < 0)
> > > +goto cleanup;
> > > +}
> > > +
> > > +switch (operation) {
> > > +case TEST_DEVICE_ATTACH:
> > > +if (testDomainAttachDeviceLiveAndConfig(def, dev) < 0)
> > > +goto cleanup;
> > > +break;
> > > +case TEST_DEVICE_DETACH:
> > > +break;
> > > +case TEST_DEVICE_UPDATE:
> > > +break;
> > > +}
> > > +
> > > +ret = 0;
> > > + cleanup:
> > > +if (xml)
> > > +virDomainDeviceDefFree(dev);
> > > +else
> > > +VIR_FREE(dev);
> >
> > virDomainDeviceDefFree() can handle both cases.
>
> It cannot! This got me as well and made me wonder!
>
> Try attaching a device with an alias and then try detaching it with
> virDomainDetachDeviceAlias and use virDomainDeviceDefFree to free the
> resource.
>
> The program crashes with:
> free(): double free detected in tcache 2

Hmm, I'll have a look at why that is, probably on Sunday, I'll go ahead and
perform the other changes on my branch, but I won't merge the series yet.

Regards,
Erik

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

[libvirt] [PATCH] qemu: alias: Generate 'qomName' of disk with useraliases

[Peter Krempa]

Commit fb64e176f4f forgot to delete the check that short-circuits the
disk alias creation if the alias is already present. The side efect
of this is that the creation qomName which is necessary to be able to
refer to disk frontends when -blockdev is used was skipped when user
aliases are used.

Fix it by deleting the check. Also prevent any potential memory leaks
from calling this function repeatedly by creating the qomName only when
it's not present.

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

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

diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index 585cc972ba..216a18d0d9 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -182,9 +182,6 @@ qemuAssignDeviceDiskAlias(virDomainDefPtr def,
 const char *prefix = virDomainDiskBusTypeToString(disk->bus);
 int controllerModel = -1;

-if (disk->info.alias)
-return 0;
-
 if (!disk->info.alias) {
 if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) {
 if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) {
@@ -220,7 +217,8 @@ qemuAssignDeviceDiskAlias(virDomainDefPtr def,
  * on the alias in qemu. While certain disk types use just the alias, some
  * need the full path into /machine/peripheral as a historical artifact.
  */
-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) {
+if (!diskPriv->qomName &&
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) {
 switch ((virDomainDiskBus) disk->bus) {
 case VIR_DOMAIN_DISK_BUS_FDC:
 case VIR_DOMAIN_DISK_BUS_IDE:
-- 
2.21.0

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

Re: [libvirt] [PATCH] virpci:fix Secondary Bus Reset bug

[Laine Stump]

The fact that you are modifying this code implies that you are using it, 
and that implies that you are still using legacy KVM device assignment 
(i.e. the pcistub driver) instead of VFIO device assignment. (I say this 
because the function that calls this function you've patched, 
virPCIDeviceReset(), has a check very early on that short-circuits the 
entire function if the device is bound to vfio-pci (and if the device is 
bound to a host driver, then you shouldn't be resetting it anyway, so 
the only reason it would be executed is if you're using legacy KVM 
device assignment or are calling virsh nodedev-reset when you shouldn't).


Legacy PCI device assignment was deprecated over 5 years ago, and was 
removed from the kernel sometime after that. We are seriously 
considering removing all vestigal support for legacy KVM device 
assignment from libvirt, since any kernel that is less than 5 years old 
have VFIO, and most don't even support legacy KVM device assignment at all.


So are you actually using legacy KVM device assignment, or did you just 
find this bug by manual code inspection? If the latter, then you need to 
seriously look into using VFIO instead. If the latter, then this patch 
can safely be pushed, but it's not going to actually do anything (and 
the code it's in will likely no longer exist within the next 6 months).


On 8/15/19 5:44 AM, hexin900...@163.com wrote:

From: hexin 

The parent bridge configuration of the current device
should be read and reset, instead of reading the current
device configuration.

Signed-off-by: He Xin 
Signed-off-by: Liu Qi 
Signed-off-by: Zhang Yu 
---
  src/util/virpci.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/virpci.c b/src/util/virpci.c
index 61a6b359e5..483de2cb16 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -821,7 +821,7 @@ virPCIDeviceTrySecondaryBusReset(virPCIDevicePtr dev,
  /* Read the control register, set the reset flag, wait 200ms,
   * unset the reset flag and wait 200ms.
   */
-ctl = virPCIDeviceRead16(dev, cfgfd, PCI_BRIDGE_CONTROL);
+ctl = virPCIDeviceRead16(dev, parentfd, PCI_BRIDGE_CONTROL);
  
  virPCIDeviceWrite16(parent, parentfd, PCI_BRIDGE_CONTROL,

  ctl | PCI_BRIDGE_CTL_RESET);



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

[libvirt] [dockerfiles PATCH 1/2] refresh: Use fedora-30 to perform MinGW builds

[Fabiano Fidêncio]

This is a workaround needed as a set of MinGW packages got removed from
Fedora Rawhide. We have to keep this workaround till the packages are
not un-retired/re-added.

Signed-off-by: Fabiano Fidêncio 
---
 refresh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/refresh b/refresh
index 73b4f9e..5f3f5e3 100755
--- a/refresh
+++ b/refresh
@@ -95,11 +95,11 @@ class Dockerfile:
 
 for project in Dockerfile.PROJECTS[project_name]:
 self.projects += [project]
-# Fedora Rawhide is special in that we use it to perform MinGW
+# Fedora 30 is special in that we use it to perform MinGW
 # builds, so we need to add the corresponding projects as well.
 # If a specific project needs to have the MinGW variant included,
 # the corresponding value in the dictionary will be True
-if (self.os == "fedora-rawhide" and
+if (self.os == "fedora-30" and
 Dockerfile.PROJECTS[project_name][project]):
 self.projects += [project + "+mingw*"]
 
-- 
2.21.0

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

[libvirt] [dockerfiles PATCH 2/2] Refresh archived Dockerfiles

[Fabiano Fidêncio]

Let's refresh the archived Dockerfiles in order to have the fedora-30
containers performing the MinGW builds (instead of the fedora-rawhide
ones).

Signed-off-by: Fabiano Fidêncio 
---
 buildenv-libosinfo-centos-7.zip | Bin 458 -> 458 bytes
 buildenv-libosinfo-debian-10-cross-aarch64.zip  | Bin 683 -> 683 bytes
 buildenv-libosinfo-debian-10-cross-armv6l.zip   | Bin 682 -> 682 bytes
 buildenv-libosinfo-debian-10-cross-armv7l.zip   | Bin 686 -> 686 bytes
 buildenv-libosinfo-debian-10-cross-i686.zip | Bin 682 -> 682 bytes
 buildenv-libosinfo-debian-10-cross-mips.zip | Bin 678 -> 678 bytes
 buildenv-libosinfo-debian-10-cross-mips64el.zip | Bin 689 -> 689 bytes
 buildenv-libosinfo-debian-10-cross-mipsel.zip   | Bin 681 -> 681 bytes
 buildenv-libosinfo-debian-10-cross-ppc64le.zip  | Bin 687 -> 687 bytes
 buildenv-libosinfo-debian-10-cross-s390x.zip| Bin 680 -> 680 bytes
 buildenv-libosinfo-debian-10.zip| Bin 555 -> 555 bytes
 buildenv-libosinfo-debian-9-cross-aarch64.zip   | Bin 682 -> 682 bytes
 buildenv-libosinfo-debian-9-cross-armv6l.zip| Bin 681 -> 681 bytes
 buildenv-libosinfo-debian-9-cross-armv7l.zip| Bin 685 -> 685 bytes
 buildenv-libosinfo-debian-9-cross-mips.zip  | Bin 678 -> 678 bytes
 buildenv-libosinfo-debian-9-cross-mips64el.zip  | Bin 689 -> 689 bytes
 buildenv-libosinfo-debian-9-cross-mipsel.zip| Bin 680 -> 680 bytes
 buildenv-libosinfo-debian-9-cross-ppc64le.zip   | Bin 687 -> 687 bytes
 buildenv-libosinfo-debian-9-cross-s390x.zip | Bin 680 -> 680 bytes
 buildenv-libosinfo-debian-9.zip | Bin 554 -> 554 bytes
 buildenv-libosinfo-debian-sid-cross-aarch64.zip | Bin 683 -> 683 bytes
 buildenv-libosinfo-debian-sid-cross-armv6l.zip  | Bin 682 -> 682 bytes
 buildenv-libosinfo-debian-sid-cross-armv7l.zip  | Bin 686 -> 686 bytes
 buildenv-libosinfo-debian-sid-cross-i686.zip| Bin 683 -> 683 bytes
 buildenv-libosinfo-debian-sid-cross-mips.zip| Bin 678 -> 678 bytes
 ...denv-libosinfo-debian-sid-cross-mips64el.zip | Bin 689 -> 689 bytes
 buildenv-libosinfo-debian-sid-cross-mipsel.zip  | Bin 681 -> 681 bytes
 buildenv-libosinfo-debian-sid-cross-ppc64le.zip | Bin 687 -> 687 bytes
 buildenv-libosinfo-debian-sid-cross-s390x.zip   | Bin 680 -> 680 bytes
 buildenv-libosinfo-debian-sid.zip   | Bin 555 -> 555 bytes
 buildenv-libosinfo-fedora-29.zip| Bin 488 -> 488 bytes
 buildenv-libosinfo-fedora-30.zip| Bin 489 -> 551 bytes
 buildenv-libosinfo-fedora-rawhide.zip   | Bin 572 -> 507 bytes
 buildenv-libosinfo-ubuntu-16.zip| Bin 558 -> 558 bytes
 buildenv-libosinfo-ubuntu-18.zip| Bin 558 -> 558 bytes
 buildenv-libvirt-centos-7.zip   | Bin 662 -> 662 bytes
 buildenv-libvirt-debian-10-cross-aarch64.zip| Bin 887 -> 887 bytes
 buildenv-libvirt-debian-10-cross-armv6l.zip | Bin 880 -> 880 bytes
 buildenv-libvirt-debian-10-cross-armv7l.zip | Bin 884 -> 884 bytes
 buildenv-libvirt-debian-10-cross-i686.zip   | Bin 883 -> 883 bytes
 buildenv-libvirt-debian-10-cross-mips.zip   | Bin 878 -> 878 bytes
 buildenv-libvirt-debian-10-cross-mips64el.zip   | Bin 892 -> 892 bytes
 buildenv-libvirt-debian-10-cross-mipsel.zip | Bin 882 -> 882 bytes
 buildenv-libvirt-debian-10-cross-ppc64le.zip| Bin 890 -> 890 bytes
 buildenv-libvirt-debian-10-cross-s390x.zip  | Bin 883 -> 883 bytes
 buildenv-libvirt-debian-10.zip  | Bin 773 -> 773 bytes
 buildenv-libvirt-debian-9-cross-aarch64.zip | Bin 896 -> 896 bytes
 buildenv-libvirt-debian-9-cross-armv6l.zip  | Bin 887 -> 887 bytes
 buildenv-libvirt-debian-9-cross-armv7l.zip  | Bin 893 -> 893 bytes
 buildenv-libvirt-debian-9-cross-mips.zip| Bin 886 -> 886 bytes
 buildenv-libvirt-debian-9-cross-mips64el.zip| Bin 898 -> 898 bytes
 buildenv-libvirt-debian-9-cross-mipsel.zip  | Bin 889 -> 889 bytes
 buildenv-libvirt-debian-9-cross-ppc64le.zip | Bin 898 -> 898 bytes
 buildenv-libvirt-debian-9-cross-s390x.zip   | Bin 890 -> 890 bytes
 buildenv-libvirt-debian-9.zip   | Bin 777 -> 777 bytes
 buildenv-libvirt-debian-sid-cross-aarch64.zip   | Bin 886 -> 886 bytes
 buildenv-libvirt-debian-sid-cross-armv6l.zip| Bin 879 -> 879 bytes
 buildenv-libvirt-debian-sid-cross-armv7l.zip| Bin 884 -> 884 bytes
 buildenv-libvirt-debian-sid-cross-i686.zip  | Bin 883 -> 883 bytes
 buildenv-libvirt-debian-sid-cross-mips.zip  | Bin 877 -> 877 bytes
 buildenv-libvirt-debian-sid-cross-mips64el.zip  | Bin 892 -> 892 bytes
 buildenv-libvirt-debian-sid-cross-mipsel.zip| Bin 882 -> 882 bytes
 buildenv-libvirt-debian-sid-cross-ppc64le.zip   | Bin 890 -> 890 bytes
 buildenv-libvirt-debian-sid-cross-s390x.zip | Bin 883 -> 883 bytes
 buildenv-libvirt-debian-sid.zip | Bin 773 -> 773 bytes
 buildenv-libvirt-fedora-29.zip  | Bin 694 -> 694 bytes
 buildenv-libvirt-fedora-30.zip  | Bin 695 -> 

[libvirt] [dockerfiles PATCH 0/2] Use Fedora 30 to perform MinGW builds

[Fabiano Fidêncio]

Due to a recent removal of mingw packages on Rawhide, let's use
Fedora 30 machines to perform MinGW builds till the packages
are un-retired/re-added in Rawhide.

Fabiano Fidêncio (2):
  refresh: Use fedora-30 to perform MinGW builds
  Refresh archived Dockerfiles

 buildenv-libosinfo-centos-7.zip | Bin 458 -> 458 bytes
 buildenv-libosinfo-debian-10-cross-aarch64.zip  | Bin 683 -> 683 bytes
 buildenv-libosinfo-debian-10-cross-armv6l.zip   | Bin 682 -> 682 bytes
 buildenv-libosinfo-debian-10-cross-armv7l.zip   | Bin 686 -> 686 bytes
 buildenv-libosinfo-debian-10-cross-i686.zip | Bin 682 -> 682 bytes
 buildenv-libosinfo-debian-10-cross-mips.zip | Bin 678 -> 678 bytes
 buildenv-libosinfo-debian-10-cross-mips64el.zip | Bin 689 -> 689 bytes
 buildenv-libosinfo-debian-10-cross-mipsel.zip   | Bin 681 -> 681 bytes
 buildenv-libosinfo-debian-10-cross-ppc64le.zip  | Bin 687 -> 687 bytes
 buildenv-libosinfo-debian-10-cross-s390x.zip| Bin 680 -> 680 bytes
 buildenv-libosinfo-debian-10.zip| Bin 555 -> 555 bytes
 buildenv-libosinfo-debian-9-cross-aarch64.zip   | Bin 682 -> 682 bytes
 buildenv-libosinfo-debian-9-cross-armv6l.zip| Bin 681 -> 681 bytes
 buildenv-libosinfo-debian-9-cross-armv7l.zip| Bin 685 -> 685 bytes
 buildenv-libosinfo-debian-9-cross-mips.zip  | Bin 678 -> 678 bytes
 buildenv-libosinfo-debian-9-cross-mips64el.zip  | Bin 689 -> 689 bytes
 buildenv-libosinfo-debian-9-cross-mipsel.zip| Bin 680 -> 680 bytes
 buildenv-libosinfo-debian-9-cross-ppc64le.zip   | Bin 687 -> 687 bytes
 buildenv-libosinfo-debian-9-cross-s390x.zip | Bin 680 -> 680 bytes
 buildenv-libosinfo-debian-9.zip | Bin 554 -> 554 bytes
 buildenv-libosinfo-debian-sid-cross-aarch64.zip | Bin 683 -> 683 bytes
 buildenv-libosinfo-debian-sid-cross-armv6l.zip  | Bin 682 -> 682 bytes
 buildenv-libosinfo-debian-sid-cross-armv7l.zip  | Bin 686 -> 686 bytes
 buildenv-libosinfo-debian-sid-cross-i686.zip| Bin 683 -> 683 bytes
 buildenv-libosinfo-debian-sid-cross-mips.zip| Bin 678 -> 678 bytes
 ...denv-libosinfo-debian-sid-cross-mips64el.zip | Bin 689 -> 689 bytes
 buildenv-libosinfo-debian-sid-cross-mipsel.zip  | Bin 681 -> 681 bytes
 buildenv-libosinfo-debian-sid-cross-ppc64le.zip | Bin 687 -> 687 bytes
 buildenv-libosinfo-debian-sid-cross-s390x.zip   | Bin 680 -> 680 bytes
 buildenv-libosinfo-debian-sid.zip   | Bin 555 -> 555 bytes
 buildenv-libosinfo-fedora-29.zip| Bin 488 -> 488 bytes
 buildenv-libosinfo-fedora-30.zip| Bin 489 -> 551 bytes
 buildenv-libosinfo-fedora-rawhide.zip   | Bin 572 -> 507 bytes
 buildenv-libosinfo-ubuntu-16.zip| Bin 558 -> 558 bytes
 buildenv-libosinfo-ubuntu-18.zip| Bin 558 -> 558 bytes
 buildenv-libvirt-centos-7.zip   | Bin 662 -> 662 bytes
 buildenv-libvirt-debian-10-cross-aarch64.zip| Bin 887 -> 887 bytes
 buildenv-libvirt-debian-10-cross-armv6l.zip | Bin 880 -> 880 bytes
 buildenv-libvirt-debian-10-cross-armv7l.zip | Bin 884 -> 884 bytes
 buildenv-libvirt-debian-10-cross-i686.zip   | Bin 883 -> 883 bytes
 buildenv-libvirt-debian-10-cross-mips.zip   | Bin 878 -> 878 bytes
 buildenv-libvirt-debian-10-cross-mips64el.zip   | Bin 892 -> 892 bytes
 buildenv-libvirt-debian-10-cross-mipsel.zip | Bin 882 -> 882 bytes
 buildenv-libvirt-debian-10-cross-ppc64le.zip| Bin 890 -> 890 bytes
 buildenv-libvirt-debian-10-cross-s390x.zip  | Bin 883 -> 883 bytes
 buildenv-libvirt-debian-10.zip  | Bin 773 -> 773 bytes
 buildenv-libvirt-debian-9-cross-aarch64.zip | Bin 896 -> 896 bytes
 buildenv-libvirt-debian-9-cross-armv6l.zip  | Bin 887 -> 887 bytes
 buildenv-libvirt-debian-9-cross-armv7l.zip  | Bin 893 -> 893 bytes
 buildenv-libvirt-debian-9-cross-mips.zip| Bin 886 -> 886 bytes
 buildenv-libvirt-debian-9-cross-mips64el.zip| Bin 898 -> 898 bytes
 buildenv-libvirt-debian-9-cross-mipsel.zip  | Bin 889 -> 889 bytes
 buildenv-libvirt-debian-9-cross-ppc64le.zip | Bin 898 -> 898 bytes
 buildenv-libvirt-debian-9-cross-s390x.zip   | Bin 890 -> 890 bytes
 buildenv-libvirt-debian-9.zip   | Bin 777 -> 777 bytes
 buildenv-libvirt-debian-sid-cross-aarch64.zip   | Bin 886 -> 886 bytes
 buildenv-libvirt-debian-sid-cross-armv6l.zip| Bin 879 -> 879 bytes
 buildenv-libvirt-debian-sid-cross-armv7l.zip| Bin 884 -> 884 bytes
 buildenv-libvirt-debian-sid-cross-i686.zip  | Bin 883 -> 883 bytes
 buildenv-libvirt-debian-sid-cross-mips.zip  | Bin 877 -> 877 bytes
 buildenv-libvirt-debian-sid-cross-mips64el.zip  | Bin 892 -> 892 bytes
 buildenv-libvirt-debian-sid-cross-mipsel.zip| Bin 882 -> 882 bytes
 buildenv-libvirt-debian-sid-cross-ppc64le.zip   | Bin 890 -> 890 bytes
 buildenv-libvirt-debian-sid-cross-s390x.zip | Bin 883 -> 883 bytes
 buildenv-libvirt-debian-sid.zip | Bin 773 -> 773 bytes
 buildenv-libvirt-fedora-29.zip  | 

[libvirt] [jenkins-ci PATCH 3/3] jenkins: Set fedora-30 as the MinGW machine.

[Fabiano Fidêncio]

As a set of MinGW packages ended up removed from Fedora Rawhide, let's
move the MinGW builds to Fedora 30 till the packages are back to
rawhide.

Signed-off-by: Fabiano Fidêncio 
---
 jenkins/jobs/defaults.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/jenkins/jobs/defaults.yaml b/jenkins/jobs/defaults.yaml
index db87da1..050c7b5 100644
--- a/jenkins/jobs/defaults.yaml
+++ b/jenkins/jobs/defaults.yaml
@@ -17,7 +17,7 @@
   - libvirt-fedora-30
   - libvirt-fedora-rawhide
 mingw_machines:
-  - libvirt-fedora-rawhide
+  - libvirt-fedora-30
 global_env: ''
 local_env: ''
 autogen_args: ''
-- 
2.21.0

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

[libvirt] [jenkins-ci PATCH 1/3] mappings: MinGW packages are not Fedora Rawhide specific

[Fabiano Fidêncio]

MinGW packages are available in all Fedora releases, not only in
Rawhide.

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

diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml
index 658951f..5012741 100644
--- a/guests/vars/mappings.yml
+++ b/guests/vars/mappings.yml
@@ -443,166 +443,166 @@ mappings:
 FreeBSD: gmake
 
   mingw32-curl:
-FedoraRawhide: mingw32-curl
+Fedora: mingw32-curl
 
   mingw32-dbus:
-FedoraRawhide: mingw32-dbus
+Fedora: mingw32-dbus
 
   mingw32-dlfcn:
-FedoraRawhide: mingw32-dlfcn
+Fedora: mingw32-dlfcn
 
   mingw32-gcc:
-FedoraRawhide: mingw32-gcc
+Fedora: mingw32-gcc
 
   mingw32-gettext:
-FedoraRawhide: mingw32-gettext
+Fedora: mingw32-gettext
 
   mingw32-glib2:
-FedoraRawhide: mingw32-glib2
+Fedora: mingw32-glib2
 
   mingw32-glib-networking:
-FedoraRawhide: mingw32-glib-networking
+Fedora: mingw32-glib-networking
 
   mingw32-gnutls:
-FedoraRawhide: mingw32-gnutls
+Fedora: mingw32-gnutls
 
   mingw32-gstreamer1-plugins-bad-free:
-FedoraRawhide: mingw32-gstreamer1-plugins-bad-free
+Fedora: mingw32-gstreamer1-plugins-bad-free
 
   mingw32-gstreamer1-plugins-good:
-FedoraRawhide: mingw32-gstreamer1-plugins-good
+Fedora: mingw32-gstreamer1-plugins-good
 
   mingw32-gtk3:
-FedoraRawhide: mingw32-gtk3
+Fedora: mingw32-gtk3
 
   mingw32-gtk-vnc2:
-FedoraRawhide: mingw32-gtk-vnc2
+Fedora: mingw32-gtk-vnc2
 
   mingw32-json-glib:
-FedoraRawhide: mingw32-json-glib
+Fedora: mingw32-json-glib
 
   mingw32-libarchive:
-FedoraRawhide: mingw32-libarchive
+Fedora: mingw32-libarchive
 
   mingw32-libgovirt:
-FedoraRawhide: mingw32-libgovirt
+Fedora: mingw32-libgovirt
 
   mingw32-libsoup:
-FedoraRawhide: mingw32-libsoup
+Fedora: mingw32-libsoup
 
   mingw32-libssh2:
-FedoraRawhide: mingw32-libssh2
+Fedora: mingw32-libssh2
 
   mingw32-libusbx:
-FedoraRawhide: mingw32-libusbx
+Fedora: mingw32-libusbx
 
   mingw32-libxml2:
-FedoraRawhide: mingw32-libxml2
+Fedora: mingw32-libxml2
 
   mingw32-libxslt:
-FedoraRawhide: mingw32-libxslt
+Fedora: mingw32-libxslt
 
   mingw32-openssl:
-FedoraRawhide: mingw32-openssl
+Fedora: mingw32-openssl
 
   mingw32-pkg-config:
-FedoraRawhide: mingw32-pkg-config
+Fedora: mingw32-pkg-config
 
   mingw32-portablexdr:
-FedoraRawhide: mingw32-portablexdr
+Fedora: mingw32-portablexdr
 
   mingw32-readline:
-FedoraRawhide: mingw32-readline
+Fedora: mingw32-readline
 
   mingw32-rest:
-FedoraRawhide: mingw32-rest
+Fedora: mingw32-rest
 
   mingw32-spice-gtk3:
-FedoraRawhide: mingw32-spice-gtk3
+Fedora: mingw32-spice-gtk3
 
   mingw32-usbredir:
-FedoraRawhide: mingw32-usbredir
+Fedora: mingw32-usbredir
 
   mingw64-curl:
-FedoraRawhide: mingw64-curl
+Fedora: mingw64-curl
 
   mingw64-dbus:
-FedoraRawhide: mingw64-dbus
+Fedora: mingw64-dbus
 
   mingw64-dlfcn:
-FedoraRawhide: mingw64-dlfcn
+Fedora: mingw64-dlfcn
 
   mingw64-gcc:
-FedoraRawhide: mingw64-gcc
+Fedora: mingw64-gcc
 
   mingw64-gettext:
-FedoraRawhide: mingw64-gettext
+Fedora: mingw64-gettext
 
   mingw64-glib2:
-FedoraRawhide: mingw64-glib2
+Fedora: mingw64-glib2
 
   mingw64-glib-networking:
-FedoraRawhide: mingw64-glib-networking
+Fedora: mingw64-glib-networking
 
   mingw64-gnutls:
-FedoraRawhide: mingw64-gnutls
+Fedora: mingw64-gnutls
 
   mingw64-gstreamer1-plugins-bad-free:
-FedoraRawhide: mingw64-gstreamer1-plugins-bad-free
+Fedora: mingw64-gstreamer1-plugins-bad-free
 
   mingw64-gstreamer1-plugins-good:
-FedoraRawhide: mingw64-gstreamer1-plugins-good
+Fedora: mingw64-gstreamer1-plugins-good
 
   mingw64-gtk3:
-FedoraRawhide: mingw64-gtk3
+Fedora: mingw64-gtk3
 
   mingw64-gtk-vnc2:
-FedoraRawhide: mingw64-gtk-vnc2
+Fedora: mingw64-gtk-vnc2
 
   mingw64-json-glib:
-FedoraRawhide: mingw64-json-glib
+Fedora: mingw64-json-glib
 
   mingw64-libarchive:
-FedoraRawhide: mingw64-libarchive
+Fedora: mingw64-libarchive
 
   mingw64-libgovirt:
-FedoraRawhide: mingw64-libgovirt
+Fedora: mingw64-libgovirt
 
   mingw64-libsoup:
-FedoraRawhide: mingw64-libsoup
+Fedora: mingw64-libsoup
 
   mingw64-libssh2:
-FedoraRawhide: mingw64-libssh2
+Fedora: mingw64-libssh2
 
   mingw64-libusbx:
-FedoraRawhide: mingw64-libusbx
+Fedora: mingw64-libusbx
 
   mingw64-libxml2:
-FedoraRawhide: mingw64-libxml2
+Fedora: mingw64-libxml2
 
   mingw64-libxslt:
-FedoraRawhide: mingw64-libxslt
+Fedora: mingw64-libxslt
 
   mingw64-openssl:
-FedoraRawhide: mingw64-openssl
+Fedora: mingw64-openssl
 
   mingw64-pkg-config:
-FedoraRawhide: mingw64-pkg-config
+Fedora: mingw64-pkg-config
 
   mingw64-portablexdr:
-FedoraRawhide: 

[libvirt] [jenkins-ci PATCH 0/3] Use Fedora 30 to perform MinGW builds

[Fabiano Fidêncio]

Due to a recent removal of mingw packages on Rawhide, let's use
Fedora 30 machines to perform MinGW builds till the packages
are un-retired/re-added in Rawhide.

Fabiano Fidêncio (3):
  mappings: MinGW packages are not Fedora Rawhide specific
  guests: Move MinGW builds to fedora-30
  jenkins: Set fedora-30 as the MinGW machine.

 guests/host_vars/libvirt-fedora-30/main.yml   |  10 ++
 .../host_vars/libvirt-fedora-rawhide/main.yml |  10 --
 guests/vars/mappings.yml  | 108 +-
 jenkins/jobs/defaults.yaml|   2 +-
 4 files changed, 65 insertions(+), 65 deletions(-)

-- 
2.21.0

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

[libvirt] [jenkins-ci PATCH 2/3] guests: Move MinGW builds to fedora-30

[Fabiano Fidêncio]

As a set of MinGW packages ended up removed from Fedora Rawhide, let's
move the MinGW builds to Fedora 30 till the packages are back to
Rawhide.

Signed-off-by: Fabiano Fidêncio 
---
 guests/host_vars/libvirt-fedora-30/main.yml  | 10 ++
 guests/host_vars/libvirt-fedora-rawhide/main.yml | 10 --
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/guests/host_vars/libvirt-fedora-30/main.yml 
b/guests/host_vars/libvirt-fedora-30/main.yml
index 491b112..e395f32 100644
--- a/guests/host_vars/libvirt-fedora-30/main.yml
+++ b/guests/host_vars/libvirt-fedora-30/main.yml
@@ -1,10 +1,16 @@
 ---
 projects:
   - libosinfo
+  - libosinfo+mingw32
+  - libosinfo+mingw64
   - libvirt
+  - libvirt+mingw32
+  - libvirt+mingw64
   - libvirt-cim
   - libvirt-dbus
   - libvirt-glib
+  - libvirt-glib+mingw32
+  - libvirt-glib+mingw64
   - libvirt-go
   - libvirt-go-xml
   - libvirt-ocaml
@@ -14,8 +20,12 @@ projects:
   - libvirt-tck
   - osinfo-db
   - osinfo-db-tools
+  - osinfo-db-tools+mingw32
+  - osinfo-db-tools+mingw64
   - virt-manager
   - virt-viewer
+  - virt-viewer+mingw32
+  - virt-viewer+mingw64
 
 package_format: 'rpm'
 package_manager: 'dnf'
diff --git a/guests/host_vars/libvirt-fedora-rawhide/main.yml 
b/guests/host_vars/libvirt-fedora-rawhide/main.yml
index db46825..fc39363 100644
--- a/guests/host_vars/libvirt-fedora-rawhide/main.yml
+++ b/guests/host_vars/libvirt-fedora-rawhide/main.yml
@@ -1,16 +1,10 @@
 ---
 projects:
   - libosinfo
-  - libosinfo+mingw32
-  - libosinfo+mingw64
   - libvirt
-  - libvirt+mingw32
-  - libvirt+mingw64
   - libvirt-cim
   - libvirt-dbus
   - libvirt-glib
-  - libvirt-glib+mingw32
-  - libvirt-glib+mingw64
   - libvirt-go
   - libvirt-go-xml
   - libvirt-ocaml
@@ -20,12 +14,8 @@ projects:
   - libvirt-tck
   - osinfo-db
   - osinfo-db-tools
-  - osinfo-db-tools+mingw32
-  - osinfo-db-tools+mingw64
   - virt-manager
   - virt-viewer
-  - virt-viewer+mingw32
-  - virt-viewer+mingw64
 
 package_format: 'rpm'
 package_manager: 'dnf'
-- 
2.21.0

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

Re: [libvirt] [PATCH v2 1/6] test_driver: implement virDomainAttachDeviceFlags

[Ilias Stamatis]

On Fri, Aug 16, 2019 at 5:39 PM Erik Skultety  wrote:
>
> ...
>
> > +
> > +if (operation == TEST_DEVICE_DETACH)
> > +parse_flags |= VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
>
> ^This should be added by patch 3/5
>
> > +
> > +if (xml) {
> > +if (!(dev = virDomainDeviceDefParse(xml, def,
> > +driver->caps, driver->xmlopt,
> > +NULL, parse_flags)))
> > +goto cleanup;
> > +} else if (alias) {
> > +if (VIR_ALLOC(dev) < 0 || virDomainDefFindDevice(def, alias, dev, 
> > true) < 0)
> > +goto cleanup;
> > +}
> > +
> > +switch (operation) {
> > +case TEST_DEVICE_ATTACH:
> > +if (testDomainAttachDeviceLiveAndConfig(def, dev) < 0)
> > +goto cleanup;
> > +break;
> > +case TEST_DEVICE_DETACH:
> > +break;
> > +case TEST_DEVICE_UPDATE:
> > +break;
> > +}
> > +
> > +ret = 0;
> > + cleanup:
> > +if (xml)
> > +virDomainDeviceDefFree(dev);
> > +else
> > +VIR_FREE(dev);
>
> virDomainDeviceDefFree() can handle both cases.

It cannot! This got me as well and made me wonder!

Try attaching a device with an alias and then try detaching it with
virDomainDetachDeviceAlias and use virDomainDeviceDefFree to free the
resource.

The program crashes with:
free(): double free detected in tcache 2

Ilias

>
> Reviewed-by: Erik Skultety 

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

Re: [libvirt] [PATCH v2 6/6] test_driver: implement virDomainUpdateDeviceFlags

[Erik Skultety]

On Wed, Aug 14, 2019 at 07:47:10PM +0300, Ilias Stamatis wrote:
> Signed-off-by: Ilias Stamatis 
> ---
>  src/test/test_driver.c | 72 ++
>  1 file changed, 72 insertions(+)
>
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index e271bc58f8..9bf3728654 100755
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -4999,6 +4999,65 @@ testDomainDetachDeviceLiveAndConfig(virDomainDefPtr 
> vmdef,
>  }
>
>
> +static int
> +testDomainUpdateDeviceLiveAndConfig(virDomainDefPtr vmdef,
> +virDomainDeviceDefPtr dev)
> +{
> +virDomainDiskDefPtr newDisk;
> +virDomainDeviceDef oldDev = { .type = dev->type };
> +virDomainNetDefPtr net;
> +int idx;
> +int ret = -1;
> +
> +switch (dev->type) {
> +case VIR_DOMAIN_DEVICE_DISK:
> +newDisk = dev->data.disk;
> +if ((idx = virDomainDiskIndexByName(vmdef, newDisk->dst, false)) 
> < 0) {
> +virReportError(VIR_ERR_INVALID_ARG,
> +   _("target %s doesn't exist."), newDisk->dst);
> +return -1;
> +}
> +
> +oldDev.data.disk = vmdef->disks[idx];
> +if (virDomainDefCompatibleDevice(vmdef, dev, ,
> + VIR_DOMAIN_DEVICE_ACTION_UPDATE,
> + false) < 0)
> +return -1;
> +
> +virDomainDiskDefFree(vmdef->disks[idx]);
> +vmdef->disks[idx] = newDisk;
> +dev->data.disk = NULL;
> +break;
> +
> +case VIR_DOMAIN_DEVICE_NET:
> +net = dev->data.net;
> +if ((idx = virDomainNetFindIdx(vmdef, net)) < 0)
> +goto cleanup;
> +
> +oldDev.data.net = vmdef->nets[idx];
> +if (virDomainDefCompatibleDevice(vmdef, dev, ,
> + VIR_DOMAIN_DEVICE_ACTION_UPDATE,
> + false) < 0)
> +return -1;
> +
> +virDomainNetDefFree(vmdef->nets[idx]);
> +vmdef->nets[idx] = net;
> +dev->data.net = NULL;
> +ret = 0;
> +break;
> +
> +default:
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("neither persistent nor live update of device 
> is supported"));
> +goto cleanup;

return -1;

> +}
> +
> +ret = 0;
> + cleanup:

drop the cleanup label

Reviewed-by: Erik Skultety 

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

Re: [libvirt] [PATCH v2 5/6] test_driver: implement virDomainDetachDeviceAlias

[Erik Skultety]

On Wed, Aug 14, 2019 at 07:47:09PM +0300, Ilias Stamatis wrote:
> Signed-off-by: Ilias Stamatis 
> ---
Reviewed-by: Erik Skultety 

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

Re: [libvirt] [PATCH v2 3/6] test_driver: implement virDomainDetachDeviceFlags

[Erik Skultety]

> +
> +virDomainShmemDefFree(virDomainShmemDefRemove(vmdef, idx));
> +break;
> +

^Extra line

Reviewed-by: Erik Skultety 

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

Re: [libvirt] [PATCH v2 1/6] test_driver: implement virDomainAttachDeviceFlags

[Erik Skultety]

...

> +
> +if (operation == TEST_DEVICE_DETACH)
> +parse_flags |= VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;

^This should be added by patch 3/5

> +
> +if (xml) {
> +if (!(dev = virDomainDeviceDefParse(xml, def,
> +driver->caps, driver->xmlopt,
> +NULL, parse_flags)))
> +goto cleanup;
> +} else if (alias) {
> +if (VIR_ALLOC(dev) < 0 || virDomainDefFindDevice(def, alias, dev, 
> true) < 0)
> +goto cleanup;
> +}
> +
> +switch (operation) {
> +case TEST_DEVICE_ATTACH:
> +if (testDomainAttachDeviceLiveAndConfig(def, dev) < 0)
> +goto cleanup;
> +break;
> +case TEST_DEVICE_DETACH:
> +break;
> +case TEST_DEVICE_UPDATE:
> +break;
> +}
> +
> +ret = 0;
> + cleanup:
> +if (xml)
> +virDomainDeviceDefFree(dev);
> +else
> +VIR_FREE(dev);

virDomainDeviceDefFree() can handle both cases.

Reviewed-by: Erik Skultety 

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

[libvirt] [RFC PATCH 2/2] qemu: enable blockdev support

[Peter Krempa]

We require that 'auto-read-only' is dynamic for posix-file backeds this
prevents problems with libvirt's usage of sVirt where we don't grant
qemu permissions to write backing chain until it's needed.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_capabilities.c  |   3 +
 .../caps_4.1.0.x86_64.xml |   1 +
 .../controller-virtio-scsi.x86_64-latest.args |  35 +++--
 .../disk-aio.x86_64-latest.args   |  19 ++-
 ...-backing-chains-noindex.x86_64-latest.args | 145 +++---
 .../disk-cache.x86_64-latest.args |  50 --
 .../disk-cdrom-network.x86_64-latest.args |  32 ++--
 .../disk-cdrom-tray.x86_64-latest.args|  24 ++-
 .../disk-cdrom.x86_64-latest.args |  21 +--
 .../disk-copy_on_read.x86_64-latest.args  |  19 ++-
 .../disk-detect-zeroes.x86_64-latest.args |  17 +-
 .../disk-error-policy.x86_64-latest.args  |  30 ++--
 .../disk-floppy-q35-2_11.x86_64-latest.args   |  14 +-
 .../disk-floppy-q35-2_9.x86_64-latest.args|  14 +-
 .../disk-floppy.x86_64-latest.args|  21 ++-
 .../disk-network-gluster.x86_64-latest.args   |  32 ++--
 .../disk-network-iscsi.x86_64-latest.args |  58 ---
 .../disk-network-nbd.x86_64-latest.args   |  41 +++--
 .../disk-network-rbd.x86_64-latest.args   |  67 +---
 .../disk-network-sheepdog.x86_64-latest.args  |  16 +-
 ...isk-network-source-auth.x86_64-latest.args |  30 ++--
 .../disk-network-tlsx509.x86_64-latest.args   |  64 +---
 .../disk-readonly-disk.x86_64-latest.args |  14 +-
 .../disk-scsi-device-auto.x86_64-latest.args  |  14 +-
 .../disk-scsi.x86_64-latest.args  |  35 +++--
 .../disk-shared.x86_64-latest.args|  36 +++--
 ...irtio-scsi-reservations.x86_64-latest.args |  20 ++-
 .../floppy-drive-fat.x86_64-latest.args   |   7 +-
 ...egl-headless-rendernode.x86_64-latest.args |   7 +-
 .../graphics-egl-headless.x86_64-latest.args  |   7 +-
 ...threads-virtio-scsi-pci.x86_64-latest.args |  25 ++-
 ...y-hotplug-nvdimm-access.x86_64-latest.args |   7 +-
 ...ry-hotplug-nvdimm-align.x86_64-latest.args |   7 +-
 ...ry-hotplug-nvdimm-label.x86_64-latest.args |   7 +-
 ...ory-hotplug-nvdimm-pmem.x86_64-latest.args |   7 +-
 ...hotplug-nvdimm-readonly.x86_64-latest.args |   7 +-
 .../memory-hotplug-nvdimm.x86_64-latest.args  |   7 +-
 ...eo-bochs-display-device.x86_64-latest.args |  10 +-
 ...virtio-non-transitional.x86_64-latest.args |   7 +-
 .../virtio-transitional.x86_64-latest.args|   7 +-
 .../x86_64-pc-graphics.x86_64-latest.args |   8 +-
 .../x86_64-pc-headless.x86_64-latest.args |   8 +-
 .../x86_64-q35-graphics.x86_64-latest.args|   8 +-
 .../x86_64-q35-headless.x86_64-latest.args|   8 +-
 44 files changed, 710 insertions(+), 306 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 83b1a12d14..0d5b8c3e4b 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4445,6 +4445,9 @@ virQEMUCapsInitProcessCaps(virQEMUCapsPtr qemuCaps)

 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_UNAVAILABLE_FEATURES))
 virQEMUCapsSet(qemuCaps, QEMU_CAPS_CANONICAL_CPU_FEATURES);
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCK_FILE_AUTO_READONLY_DYNAMIC) &&
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_DEVICE_ID))
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_BLOCKDEV);
 }


diff --git a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
index fbde4f3a5d..c4816d1f7d 100644
--- a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
@@ -191,6 +191,7 @@
   
   
   
+  
   
   
   
diff --git a/tests/qemuxml2argvdata/controller-virtio-scsi.x86_64-latest.args 
b/tests/qemuxml2argvdata/controller-virtio-scsi.x86_64-latest.args
index 32b781ced9..1647cfdd97 100644
--- a/tests/qemuxml2argvdata/controller-virtio-scsi.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/controller-virtio-scsi.x86_64-latest.args
@@ -32,21 +32,36 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
 -device virtio-scsi-pci,id=scsi2,cmd_per_lun=50,bus=pci.0,addr=0x4 \
 -device virtio-scsi-pci,id=scsi3,max_sectors=512,bus=pci.0,addr=0x5 \
 -device virtio-scsi-pci,id=scsi4,ioeventfd=on,bus=pci.0,addr=0x6 \
--drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-scsi0-0-0-0 \
+-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1",\
+"node-name":"libvirt-5-storage","auto-read-only":true,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-5-format","read-only":false,"driver":"raw",\
+"file":"libvirt-5-storage"}' \
 -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\
-device_id=drive-scsi0-0-0-0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1 
\
--drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-scsi1-0-0-0 \
+device_id=drive-scsi0-0-0-0,drive=libvirt-5-format,id=scsi0-0-0-0,bootindex=1 \
+-blockdev 

[libvirt] [RFC PATCH 1/2] qemu: caps: Add capability for dynamic 'auto-read-only' support for files

[Peter Krempa]

Initial implementation of 'auto-read-only' didn't reopen the backing
files when needed. For '-blockdev' to work we need to be able to tel
qemu to open a file read-only and change it during blockjobs as we label
backing chains with a sVirt label which does not allow writing. The
dynamic auto-read-only supports this as it reopens files when writing
is demanded.

Add a capability to detect that the posix file based backends support
the dynamic part.

Signed-off-by: Peter Krempa 
Reviewed-by: Jiri Denemark 
---
 src/qemu/qemu_capabilities.c | 2 ++
 src/qemu/qemu_capabilities.h | 1 +
 tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml | 1 +
 3 files changed, 4 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index c9677315ab..83b1a12d14 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -537,6 +537,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
   /* 335 */
   "bochs-display",
   "migration-file-drop-cache",
+  "blockdev-file-dynamic-auto-read-only",
 );


@@ -1281,6 +1282,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsQMPSchemaQueries[] = {
 { "query-display-options/ret-type/+egl-headless/rendernode", 
QEMU_CAPS_EGL_HEADLESS_RENDERNODE },
 { "nbd-server-add/arg-type/bitmap", QEMU_CAPS_NBD_BITMAP },
 { "blockdev-add/arg-type/+file/drop-cache", 
QEMU_CAPS_MIGRATION_FILE_DROP_CACHE },
+{ "blockdev-add/arg-type/+file/$dynamic-auto-read-only", 
QEMU_CAPS_BLOCK_FILE_AUTO_READONLY_DYNAMIC },
 };

 typedef struct _virQEMUCapsObjectTypeProps virQEMUCapsObjectTypeProps;
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 68ef6c49b4..2fe0e26ced 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -518,6 +518,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 /* 335 */
 QEMU_CAPS_DEVICE_BOCHS_DISPLAY, /* -device bochs-display */
 QEMU_CAPS_MIGRATION_FILE_DROP_CACHE, /* migration with disk cache on is 
safe for type='file' disks */
+QEMU_CAPS_BLOCK_FILE_AUTO_READONLY_DYNAMIC, /* the auto-read-only property 
of block backends for files is dynamic */

 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
index f4583d7fe7..fbde4f3a5d 100644
--- a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
@@ -209,6 +209,7 @@
   
   
   
+  
   450
   0
   43100759
-- 
2.21.0

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

[libvirt] [RFC PATCH 0/2] qemu: Enable -blockdev [READ ME!] (blockdev-add saga finale?)

[Peter Krempa]

With the patches I've posted to the list libvirt should be mostly
prepared to use -blockdev instead of -drive to specify disk images.

There's one gothcha though. Internal snapshots don't work due to qemu
considering every monitor-owned node for snapshot as well. With blockdev
everything became monitor-owned including the 'file' nodes which are
definitely not snapshottable. Even when qemu fixes that bug I don't
think it will be introspectable.

Our options are either to wait for the qemu fix and then try to detect
it somehow, perhaps as a version check (yuck), or go ahead and inform
users to upgrade to a newer qemu afterwards.

Any other ideas or suggestions?

Peter Krempa (2):
  qemu: caps: Add capability for dynamic 'auto-read-only' support for
files
  qemu: enable blockdev support

 src/qemu/qemu_capabilities.c  |   5 +
 src/qemu/qemu_capabilities.h  |   1 +
 .../caps_4.1.0.x86_64.xml |   2 +
 .../controller-virtio-scsi.x86_64-latest.args |  35 +++--
 .../disk-aio.x86_64-latest.args   |  19 ++-
 ...-backing-chains-noindex.x86_64-latest.args | 145 +++---
 .../disk-cache.x86_64-latest.args |  50 --
 .../disk-cdrom-network.x86_64-latest.args |  32 ++--
 .../disk-cdrom-tray.x86_64-latest.args|  24 ++-
 .../disk-cdrom.x86_64-latest.args |  21 +--
 .../disk-copy_on_read.x86_64-latest.args  |  19 ++-
 .../disk-detect-zeroes.x86_64-latest.args |  17 +-
 .../disk-error-policy.x86_64-latest.args  |  30 ++--
 .../disk-floppy-q35-2_11.x86_64-latest.args   |  14 +-
 .../disk-floppy-q35-2_9.x86_64-latest.args|  14 +-
 .../disk-floppy.x86_64-latest.args|  21 ++-
 .../disk-network-gluster.x86_64-latest.args   |  32 ++--
 .../disk-network-iscsi.x86_64-latest.args |  58 ---
 .../disk-network-nbd.x86_64-latest.args   |  41 +++--
 .../disk-network-rbd.x86_64-latest.args   |  67 +---
 .../disk-network-sheepdog.x86_64-latest.args  |  16 +-
 ...isk-network-source-auth.x86_64-latest.args |  30 ++--
 .../disk-network-tlsx509.x86_64-latest.args   |  64 +---
 .../disk-readonly-disk.x86_64-latest.args |  14 +-
 .../disk-scsi-device-auto.x86_64-latest.args  |  14 +-
 .../disk-scsi.x86_64-latest.args  |  35 +++--
 .../disk-shared.x86_64-latest.args|  36 +++--
 ...irtio-scsi-reservations.x86_64-latest.args |  20 ++-
 .../floppy-drive-fat.x86_64-latest.args   |   7 +-
 ...egl-headless-rendernode.x86_64-latest.args |   7 +-
 .../graphics-egl-headless.x86_64-latest.args  |   7 +-
 ...threads-virtio-scsi-pci.x86_64-latest.args |  25 ++-
 ...y-hotplug-nvdimm-access.x86_64-latest.args |   7 +-
 ...ry-hotplug-nvdimm-align.x86_64-latest.args |   7 +-
 ...ry-hotplug-nvdimm-label.x86_64-latest.args |   7 +-
 ...ory-hotplug-nvdimm-pmem.x86_64-latest.args |   7 +-
 ...hotplug-nvdimm-readonly.x86_64-latest.args |   7 +-
 .../memory-hotplug-nvdimm.x86_64-latest.args  |   7 +-
 ...eo-bochs-display-device.x86_64-latest.args |  10 +-
 ...virtio-non-transitional.x86_64-latest.args |   7 +-
 .../virtio-transitional.x86_64-latest.args|   7 +-
 .../x86_64-pc-graphics.x86_64-latest.args |   8 +-
 .../x86_64-pc-headless.x86_64-latest.args |   8 +-
 .../x86_64-q35-graphics.x86_64-latest.args|   8 +-
 .../x86_64-q35-headless.x86_64-latest.args|   8 +-
 45 files changed, 714 insertions(+), 306 deletions(-)

-- 
2.21.0

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

Re: [libvirt] [PATCH v2 4/6] test_driver: implement virDomainDetachDevice

[Erik Skultety]

On Wed, Aug 14, 2019 at 07:47:08PM +0300, Ilias Stamatis wrote:
> Signed-off-by: Ilias Stamatis 
> ---
>  src/test/test_driver.c | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index ff9693ccb7..1152ca5bed 100755
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -5119,6 +5119,13 @@ testDomainDetachDeviceFlags(virDomainPtr dom,
>xml, NULL, flags);
>  }
>
> +static int
> +testDomainDetachDevice(virDomainPtr dom,
> +   const char *xml)
> +{
> +return testDomainDetachDeviceFlags(dom, xml, VIR_DOMAIN_AFFECT_LIVE);
> +}
> +
>
>  static int testDomainGetAutostart(virDomainPtr domain,
>int *autostart)
> @@ -9926,6 +9933,7 @@ static virHypervisorDriver testHypervisorDriver = {
>  .domainFSTrim = testDomainFSTrim, /* 5.7.0 */
>  .domainAttachDevice = testDomainAttachDevice, /* 5.7.0 */
>  .domainAttachDeviceFlags = testDomainAttachDeviceFlags, /* 5.7.0 */
> +.domainDetachDevice = testDomainDetachDevice, /* 5.7.0 */
>  .domainDetachDeviceFlags = testDomainDetachDeviceFlags, /* 5.7.0 */
>  .domainGetAutostart = testDomainGetAutostart, /* 0.3.2 */
>  .domainSetAutostart = testDomainSetAutostart, /* 0.3.2 */

Reviewed-by: Erik Skultety 

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

Re: [libvirt] [PATCH] ci: Adapt to container name changes

[Fabiano Fidêncio]

On Fri, 2019-08-16 at 15:51 +0200, Andrea Bolognani wrote:
> On Fri, 2019-08-16 at 14:32 +0200, Fabiano Fidêncio wrote:
> > On Mon, 2019-08-12 at 16:09 +0200, Andrea Bolognani wrote:
> > > This is connected to
> > > 
> > > https://www.redhat.com/archives/libvir-list/2019-August/msg00399.html
> > > 
> > > https://www.redhat.com/archives/libvir-list/2019-August/msg00416.html
> > > 
> > > and should only be merged once the above have been merged *and*
> > > deployed, as in, images with the new names have been generated.
> > 
> > Reviewed-by: Fabiano Fidêncio 
> 
> Thanks for the reviews!
> 
> I have pushed the other two series but I can't quite push this one
> yet because, as we already know, a bunch of MinGW packages have been
> dropped from Fedora and thus the buildenv-libvirt-fedora-rawhide
> container can't currently be built successfully:
> 
>   No match for argument: mingw32-portablexdr
>   No match for argument: mingw64-portablexdr
>   Error: Unable to find a match: mingw32-portablexdr mingw64-
> portablexdr
> 
> I think the situation would be the same for the libosinfo container.
> 
> Do you have any idea how long it will take to get the packages back
> in Fedora?

We're working on it.
For mingw-portablexdr we may end up not adding it back and adding
mingw-libtirpc.

For mingw-libsoup (which is the piece missing from libosinfo side),
we've already asked to un-retire the package.

Bugs are still open though.

> Should we just wait for that to happen before rebuilding
> the containers, temporarily disable MinGW builds, temporarily switch
> MinGW builds to Fedora 30 where I assume all MinGW packages are still
> available?

I'd go for this solution, stick to Fedora 30, at least till the
situation is back to stable on rawhide.

I'll provide you some patches for this and regenerate the images.

Best Regards,
-- 
Fabiano Fidêncio

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

Re: [libvirt] [PATCH v2 2/6] test_driver: implement virDomainAttachDevice

[Erik Skultety]

On Wed, Aug 14, 2019 at 07:47:06PM +0300, Ilias Stamatis wrote:
> Signed-off-by: Ilias Stamatis 
> ---
>  src/test/test_driver.c | 9 +
>  1 file changed, 9 insertions(+)
>
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 5f28e9017f..abf80b97cf 100755
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -4935,6 +4935,14 @@ testDomainAttachDeviceFlags(virDomainPtr dom,
>  }
>
>
> +static int
> +testDomainAttachDevice(virDomainPtr dom,
> +   const char *xml)
> +{
> +return testDomainAttachDeviceFlags(dom, xml, VIR_DOMAIN_AFFECT_LIVE);
> +}
> +
> +
>  static int testDomainGetAutostart(virDomainPtr domain,
>int *autostart)
>  {
> @@ -9739,6 +9747,7 @@ static virHypervisorDriver testHypervisorDriver = {
>  .domainFSFreeze = testDomainFSFreeze, /* 5.7.0 */
>  .domainFSThaw = testDomainFSThaw, /* 5.7.0 */
>  .domainFSTrim = testDomainFSTrim, /* 5.7.0 */
> +.domainAttachDevice = testDomainAttachDevice, /* 5.7.0 */
>  .domainAttachDeviceFlags = testDomainAttachDeviceFlags, /* 5.7.0 */
>  .domainGetAutostart = testDomainGetAutostart, /* 0.3.2 */
>  .domainSetAutostart = testDomainSetAutostart, /* 0.3.2 */

Reviewed-by: Erik Skultety 

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

[libvirt] [PATCH 09/10] qemu: Add -blockdev support for external snapshots

[Peter Krempa]

Use the code for creating or attaching new storage source in the
snapshot code and switch to 'blockdev-snapshot' for creating the
snapshot itself.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_driver.c | 104 ++---
 1 file changed, 88 insertions(+), 16 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6e10b691e9..c22f74adaa 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15270,6 +15270,8 @@ struct _qemuDomainSnapshotDiskData {
 bool prepared; /* @src was prepared using 
qemuDomainStorageSourceAccessAllow */
 virDomainDiskDefPtr disk;
 char *relPath; /* relative path component to fill into original disk */
+qemuBlockStorageSourceChainDataPtr crdata;
+bool blockdevadded;

 virStorageSourcePtr persistsrc;
 virDomainDiskDefPtr persistdisk;
@@ -15283,7 +15285,8 @@ static void
 qemuDomainSnapshotDiskDataCleanup(qemuDomainSnapshotDiskDataPtr data,
   size_t ndata,
   virQEMUDriverPtr driver,
-  virDomainObjPtr vm)
+  virDomainObjPtr vm,
+  qemuDomainAsyncJob asyncJob)
 {
 size_t i;

@@ -15294,6 +15297,15 @@ 
qemuDomainSnapshotDiskDataCleanup(qemuDomainSnapshotDiskDataPtr data,
 /* on success of the snapshot the 'src' and 'persistsrc' properties 
will
  * be set to NULL by qemuDomainSnapshotUpdateDiskSources */
 if (data[i].src) {
+if (data[i].blockdevadded) {
+if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) 
{
+
+
qemuBlockStorageSourceAttachRollback(qemuDomainGetMonitor(vm),
+ 
data[i].crdata->srcdata[0]);
+ignore_value(qemuDomainObjExitMonitor(driver, vm));
+}
+}
+
 if (data[i].created &&
 virStorageFileUnlink(data[i].src) < 0) {
 VIR_WARN("Unable to remove just-created %s",
@@ -15312,6 +15324,7 @@ 
qemuDomainSnapshotDiskDataCleanup(qemuDomainSnapshotDiskDataPtr data,
 VIR_FREE(data[i].relPath);
 }

+qemuBlockStorageSourceChainDataFree(data[i].crdata);
 VIR_FREE(data);
 }

@@ -15319,15 +15332,20 @@ 
qemuDomainSnapshotDiskDataCleanup(qemuDomainSnapshotDiskDataPtr data,
 static int
 qemuDomainSnapshotDiskDataCollectOne(virQEMUDriverPtr driver,
  virDomainObjPtr vm,
+ virQEMUDriverConfigPtr cfg,
  virDomainDiskDefPtr disk,
  virDomainSnapshotDiskDefPtr snapdisk,
  qemuDomainSnapshotDiskDataPtr dd,
- bool reuse)
+ bool reuse,
+ bool blockdev,
+ qemuDomainAsyncJob asyncJob)
 {
+qemuDomainObjPrivatePtr priv = vm->privateData;
 char *backingStoreStr;
 virDomainDiskDefPtr persistdisk;
 bool supportsCreate;
 bool supportsBacking;
+int rc;

 dd->disk = disk;

@@ -15393,6 +15411,44 @@ qemuDomainSnapshotDiskDataCollectOne(virQEMUDriverPtr 
driver,

 dd->prepared = true;

+if (blockdev) {
+/* create a terminator for the snapsot disks so that qemu does not try
+ * to open them at first */
+virObjectUnref(dd->src->backingStore);
+if (!(dd->src->backingStore = virStorageSourceNew()))
+return -1;
+
+if (qemuDomainPrepareStorageSourceBlockdev(dd->disk, dd->src,
+   priv, cfg) < 0)
+return -1;
+
+if (!(dd->crdata = 
qemuBuildStorageSourceChainAttachPrepareBlockdevTop(dd->src,
+   
priv->qemuCaps)))
+return -1;
+
+if (reuse) {
+if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+return -1;
+
+rc = qemuBlockStorageSourceAttachApply(qemuDomainGetMonitor(vm),
+   dd->crdata->srcdata[0]);
+
+if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
+return -1;
+} else {
+if (qemuBlockStorageSourceCreateDetectSize(vm, dd->src, 
dd->disk->src,
+   asyncJob) < 0)
+return -1;
+
+if (qemuBlockStorageSourceCreate(vm, dd->src, dd->disk->src,
+ NULL, dd->crdata->srcdata[0],
+ asyncJob) < 0)
+return -1;
+}
+
+dd->blockdevadded = true;
+}
+
 return 0;
 }

@@ -15407,7 +15463,10 @@ static int
 

[libvirt] [PATCH 08/10] qemu: snapshot: Skip overlay file creation/interogation if unsupported

[Peter Krempa]

With blockdev we'll be able to support protocols which are not supported
by the storage backends in libvirt. This means that we have to be able
to skip the creation and relative storage path reading if it's not
supported. This will make it impossible to use relative backing for
network protocols but that would be almost insane anyways.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_driver.c | 53 --
 1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0bbde4b52d..6e10b691e9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15326,6 +15326,8 @@ qemuDomainSnapshotDiskDataCollectOne(virQEMUDriverPtr 
driver,
 {
 char *backingStoreStr;
 virDomainDiskDefPtr persistdisk;
+bool supportsCreate;
+bool supportsBacking;

 dd->disk = disk;

@@ -15350,31 +15352,38 @@ qemuDomainSnapshotDiskDataCollectOne(virQEMUDriverPtr 
driver,
 return -1;
 }

-if (qemuDomainStorageFileInit(driver, vm, dd->src, NULL) < 0)
-return -1;
-
-dd->initialized = true;
+supportsCreate = virStorageFileSupportsCreate(dd->src);
+supportsBacking = virStorageFileSupportsBackingChainTraversal(dd->src);

-/* relative backing store paths need to be updated so that relative
- * block commit still works */
-if (reuse) {
-if (virStorageFileGetBackingStoreStr(dd->src, ) < 0)
+if (supportsCreate || supportsBacking) {
+if (qemuDomainStorageFileInit(driver, vm, dd->src, NULL) < 0)
 return -1;
-if (backingStoreStr != NULL) {
-if (virStorageIsRelative(backingStoreStr))
-VIR_STEAL_PTR(dd->relPath, backingStoreStr);
-else
-VIR_FREE(backingStoreStr);
-}
-} else {
-/* pre-create the image file so that we can label it before handing it 
to qemu */
-if (dd->src->type != VIR_STORAGE_TYPE_BLOCK) {
-if (virStorageFileCreate(dd->src) < 0) {
-virReportSystemError(errno, _("failed to create image file 
'%s'"),
- NULLSTR(dd->src->path));
-return -1;
+
+dd->initialized = true;
+
+/* relative backing store paths need to be updated so that relative
+ * block commit still works */
+if (reuse) {
+if (supportsBacking) {
+if (virStorageFileGetBackingStoreStr(dd->src, 
) < 0)
+return -1;
+if (backingStoreStr != NULL) {
+if (virStorageIsRelative(backingStoreStr))
+VIR_STEAL_PTR(dd->relPath, backingStoreStr);
+else
+VIR_FREE(backingStoreStr);
+}
+}
+} else {
+/* pre-create the image file so that we can label it before 
handing it to qemu */
+if (supportsCreate && dd->src->type != VIR_STORAGE_TYPE_BLOCK) {
+if (virStorageFileCreate(dd->src) < 0) {
+virReportSystemError(errno, _("failed to create image file 
'%s'"),
+ NULLSTR(dd->src->path));
+return -1;
+}
+dd->created = true;
 }
-dd->created = true;
 }
 }

-- 
2.21.0

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

[libvirt] [PATCH 06/10] qemu: Disband qemuDomainSnapshotCreateSingleDiskActive

[Peter Krempa]

After we always assume support for the 'transaction' command
(c358adc57113b) and follow-up cleanups
qemuDomainSnapshotCreateSingleDiskActive lost it's value. Move the code
into appropriate helpers and remove the function.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_driver.c | 53 --
 1 file changed, 20 insertions(+), 33 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 57d864cdd2..9248f912d0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15368,6 +15368,22 @@ qemuDomainSnapshotDiskDataCollectOne(virQEMUDriverPtr 
driver,
 }
 }

+/* pre-create the image file so that we can label it before handing it to 
qemu */
+if (!reuse && dd->src->type != VIR_STORAGE_TYPE_BLOCK) {
+if (virStorageFileCreate(dd->src) < 0) {
+virReportSystemError(errno, _("failed to create image file '%s'"),
+ NULLSTR(dd->src->path));
+return -1;
+}
+dd->created = true;
+}
+
+/* set correct security, cgroup and locking options on the new image */
+if (qemuDomainStorageSourceAccessAllow(driver, vm, dd->src, false, true) < 
0)
+return -1;
+
+dd->prepared = true;
+
 return 0;
 }

@@ -15463,36 +15479,6 @@ qemuDomainSnapshotUpdateDiskSources(virQEMUDriverPtr 
driver,
 }


-static int
-qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- qemuDomainSnapshotDiskDataPtr dd,
- virJSONValuePtr actions,
- bool reuse)
-{
-if (qemuBlockSnapshotAddLegacy(actions, dd->disk, dd->src, reuse) < 0)
-return -1;
-
-/* pre-create the image file so that we can label it before handing it to 
qemu */
-if (!reuse && dd->src->type != VIR_STORAGE_TYPE_BLOCK) {
-if (virStorageFileCreate(dd->src) < 0) {
-virReportSystemError(errno, _("failed to create image file '%s'"),
- NULLSTR(dd->src->path));
-return -1;
-}
-dd->created = true;
-}
-
-/* set correct security, cgroup and locking options on the new image */
-if (qemuDomainStorageSourceAccessAllow(driver, vm, dd->src, false, true) < 
0)
-return -1;
-
-dd->prepared = true;
-
-return 0;
-}
-
-
 /* The domain is expected to be locked and active. */
 static int
 qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver,
@@ -15535,9 +15521,10 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr 
driver,
   * VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL with a valid file name and
   * qcow2 format.  */
 for (i = 0; i < ndiskdata; i++) {
-if (qemuDomainSnapshotCreateSingleDiskActive(driver, vm,
- [i],
- actions, reuse) < 0)
+if (qemuBlockSnapshotAddLegacy(actions,
+   diskdata[i].disk,
+   diskdata[i].src,
+   reuse) < 0)
 goto cleanup;
 }

-- 
2.21.0

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

[libvirt] [PATCH 10/10] qemu: Defer support checks for external active snapshots to blockdev code or qemu

[Peter Krempa]

Remove libvit's support check for the target of an external snapshot to
the blockdev code or qemu. This will potentially require a more complex
cleanup but removes a level of hardcoded feature checks.

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

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c22f74adaa..d9315e0df4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14931,7 +14931,8 @@ 
qemuDomainSnapshotPrepareDiskExternalInactive(virDomainSnapshotDiskDefPtr snapdi

 static int
 qemuDomainSnapshotPrepareDiskExternalActive(virDomainSnapshotDiskDefPtr 
snapdisk,
-virDomainDiskDefPtr domdisk)
+virDomainDiskDefPtr domdisk,
+bool blockdev)
 {
 int actualType = virStorageSourceGetActualType(snapdisk->src);

@@ -14948,6 +14949,10 @@ 
qemuDomainSnapshotPrepareDiskExternalActive(virDomainSnapshotDiskDefPtr snapdisk
 break;

 case VIR_STORAGE_TYPE_NETWORK:
+/* defer all of the checking to either qemu or libvirt's blockdev code 
*/
+if (blockdev)
+break;
+
 switch ((virStorageNetProtocol) snapdisk->src->protocol) {
 case VIR_STORAGE_NET_PROTOCOL_GLUSTER:
 break;
@@ -14995,7 +15000,8 @@ static int
 qemuDomainSnapshotPrepareDiskExternal(virDomainDiskDefPtr disk,
   virDomainSnapshotDiskDefPtr snapdisk,
   bool active,
-  bool reuse)
+  bool reuse,
+  bool blockdev)
 {
 struct stat st;
 int err;
@@ -15018,7 +15024,7 @@ 
qemuDomainSnapshotPrepareDiskExternal(virDomainDiskDefPtr disk,
 if (qemuDomainSnapshotPrepareDiskExternalInactive(snapdisk, disk) < 0)
 return -1;
 } else {
-if (qemuDomainSnapshotPrepareDiskExternalActive(snapdisk, disk) < 0)
+if (qemuDomainSnapshotPrepareDiskExternalActive(snapdisk, disk, 
blockdev) < 0)
 return -1;
 }

@@ -15119,6 +15125,8 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm,
   virDomainSnapshotDefPtr def,
   unsigned int *flags)
 {
+qemuDomainObjPrivatePtr priv = vm->privateData;
+bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV);
 int ret = -1;
 size_t i;
 bool active = virDomainObjIsActive(vm);
@@ -15177,7 +15185,7 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm,
 }

 if (qemuDomainSnapshotPrepareDiskExternal(dom_disk, disk,
-  active, reuse) < 0)
+  active, reuse, blockdev) 
< 0)
 goto cleanup;

 external++;
-- 
2.21.0

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

[libvirt] [PATCH 07/10] qemu: Merge conditions depending on the 'reuse' flag in qemuDomainSnapshotDiskDataCollectOne

[Peter Krempa]

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_driver.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9248f912d0..0bbde4b52d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15366,16 +15366,16 @@ qemuDomainSnapshotDiskDataCollectOne(virQEMUDriverPtr 
driver,
 else
 VIR_FREE(backingStoreStr);
 }
-}
-
-/* pre-create the image file so that we can label it before handing it to 
qemu */
-if (!reuse && dd->src->type != VIR_STORAGE_TYPE_BLOCK) {
-if (virStorageFileCreate(dd->src) < 0) {
-virReportSystemError(errno, _("failed to create image file '%s'"),
- NULLSTR(dd->src->path));
-return -1;
+} else {
+/* pre-create the image file so that we can label it before handing it 
to qemu */
+if (dd->src->type != VIR_STORAGE_TYPE_BLOCK) {
+if (virStorageFileCreate(dd->src) < 0) {
+virReportSystemError(errno, _("failed to create image file 
'%s'"),
+ NULLSTR(dd->src->path));
+return -1;
+}
+dd->created = true;
 }
-dd->created = true;
 }

 /* set correct security, cgroup and locking options on the new image */
-- 
2.21.0

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

[libvirt] [PATCH 05/10] qemu: Split out preparing of single snapshot from qemuDomainSnapshotDiskDataCollect

[Peter Krempa]

Move the internals into qemuDomainSnapshotDiskDataCollectOne to make it
obvious what's happening after moving more code here.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_driver.c | 104 +++--
 1 file changed, 59 insertions(+), 45 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 52540eb77d..57d864cdd2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15316,6 +15316,62 @@ 
qemuDomainSnapshotDiskDataCleanup(qemuDomainSnapshotDiskDataPtr data,
 }


+static int
+qemuDomainSnapshotDiskDataCollectOne(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainDiskDefPtr disk,
+ virDomainSnapshotDiskDefPtr snapdisk,
+ qemuDomainSnapshotDiskDataPtr dd,
+ bool reuse)
+{
+char *backingStoreStr;
+virDomainDiskDefPtr persistdisk;
+
+dd->disk = disk;
+
+if (!(dd->src = virStorageSourceCopy(snapdisk->src, false)))
+return -1;
+
+if (virStorageSourceInitChainElement(dd->src, dd->disk->src, false) < 0)
+return -1;
+
+/* modify disk in persistent definition only when the source is the same */
+if (vm->newDef &&
+(persistdisk = virDomainDiskByName(vm->newDef, dd->disk->dst, false)) 
&&
+virStorageSourceIsSameLocation(dd->disk->src, persistdisk->src)) {
+
+dd->persistdisk = persistdisk;
+
+if (!(dd->persistsrc = virStorageSourceCopy(dd->src, false)))
+return -1;
+
+if (virStorageSourceInitChainElement(dd->persistsrc,
+ dd->persistdisk->src, false) < 0)
+return -1;
+}
+
+if (qemuDomainStorageFileInit(driver, vm, dd->src, NULL) < 0)
+return -1;
+
+dd->initialized = true;
+
+/* relative backing store paths need to be updated so that relative
+ * block commit still works */
+if (reuse) {
+if (virStorageFileGetBackingStoreStr(dd->src, ) < 0)
+return -1;
+if (backingStoreStr != NULL) {
+if (virStorageIsRelative(backingStoreStr))
+VIR_STEAL_PTR(dd->relPath, backingStoreStr);
+else
+VIR_FREE(backingStoreStr);
+}
+}
+
+return 0;
+}
+
+
 /**
  * qemuDomainSnapshotDiskDataCollect:
  *
@@ -15333,10 +15389,7 @@ qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr 
driver,
 size_t i;
 qemuDomainSnapshotDiskDataPtr data;
 size_t ndata = 0;
-qemuDomainSnapshotDiskDataPtr dd;
-char *backingStoreStr;
 virDomainSnapshotDefPtr snapdef = virDomainSnapshotObjGetDef(snap);
-virDomainDiskDefPtr persistdisk;
 int ret = -1;

 if (VIR_ALLOC_N(data, snapdef->ndisks) < 0)
@@ -15346,49 +15399,10 @@ qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr 
driver,
 if (snapdef->disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE)
 continue;

-dd = data + ndata;
-ndata++;
-
-dd->disk = vm->def->disks[i];
-
-if (!(dd->src = virStorageSourceCopy(snapdef->disks[i].src, false)))
-goto cleanup;
-
-if (virStorageSourceInitChainElement(dd->src, dd->disk->src, false) < 
0)
+if (qemuDomainSnapshotDiskDataCollectOne(driver, vm, vm->def->disks[i],
+ snapdef->disks + i,
+ data + ndata++, reuse) < 0)
 goto cleanup;
-
-/* modify disk in persistent definition only when the source is the 
same */
-if (vm->newDef &&
-(persistdisk = virDomainDiskByName(vm->newDef, dd->disk->dst, 
false)) &&
-virStorageSourceIsSameLocation(dd->disk->src, persistdisk->src)) {
-
-dd->persistdisk = persistdisk;
-
-if (!(dd->persistsrc = virStorageSourceCopy(dd->src, false)))
-goto cleanup;
-
-if (virStorageSourceInitChainElement(dd->persistsrc,
- dd->persistdisk->src, false) 
< 0)
-goto cleanup;
-}
-
-if (qemuDomainStorageFileInit(driver, vm, dd->src, NULL) < 0)
-goto cleanup;
-
-dd->initialized = true;
-
-/* relative backing store paths need to be updated so that relative
- * block commit still works */
-if (reuse) {
-if (virStorageFileGetBackingStoreStr(dd->src, ) < 
0)
-goto cleanup;
-if (backingStoreStr != NULL) {
-if (virStorageIsRelative(backingStoreStr))
-VIR_STEAL_PTR(dd->relPath, backingStoreStr);
-else
-VIR_FREE(backingStoreStr);
-}
-}
 }

 VIR_STEAL_PTR(*rdata, data);
-- 
2.21.0

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

[libvirt] [PATCH 03/10] qemu: Remove cleanup label in qemuDomainSnapshotPrepareDiskExternal

[Peter Krempa]

Refactor the code to avoid having a cleanup label. This will simplify
the change necessary when restricting this check in an upcoming patch.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_driver.c | 28 +++-
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 913b57855c..29a47af0a2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14997,8 +14997,9 @@ 
qemuDomainSnapshotPrepareDiskExternal(virDomainDiskDefPtr disk,
   bool active,
   bool reuse)
 {
-int ret = -1;
 struct stat st;
+int err;
+int rc;

 if (disk->src->readonly) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -15024,31 +15025,32 @@ 
qemuDomainSnapshotPrepareDiskExternal(virDomainDiskDefPtr disk,
 if (virStorageFileInit(snapdisk->src) < 0)
 return -1;

-if (virStorageFileStat(snapdisk->src, ) < 0) {
-if (errno != ENOENT) {
-virReportSystemError(errno,
+rc = virStorageFileStat(snapdisk->src, );
+err = errno;
+
+virStorageFileDeinit(snapdisk->src);
+
+if (rc < 0) {
+if (err != ENOENT) {
+virReportSystemError(err,
  _("unable to stat for disk %s: %s"),
  snapdisk->name, snapdisk->src->path);
-goto cleanup;
+return -1;
 } else if (reuse) {
-virReportSystemError(errno,
+virReportSystemError(err,
  _("missing existing file for disk %s: %s"),
  snapdisk->name, snapdisk->src->path);
-goto cleanup;
+return -1;
 }
 } else if (!S_ISBLK(st.st_mode) && st.st_size && !reuse) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("external snapshot file for disk %s already "
  "exists and is not a block device: %s"),
snapdisk->name, snapdisk->src->path);
-goto cleanup;
+return -1;
 }

-ret = 0;
-
- cleanup:
-virStorageFileDeinit(snapdisk->src);
-return ret;
+return 0;
 }


-- 
2.21.0

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

[libvirt] [PATCH 02/10] qemu: driver: Remove dead code from qemuDomainSnapshotUpdateDiskSources

[Peter Krempa]

dd->src is always allocated in this function as it contains the new
source for the snapshot which is meant to replace the disk source.

The label handling code executed if that source was not present thus is
dead code. Remove it.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_driver.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b6fbb197b8..913b57855c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15421,13 +15421,6 @@ qemuDomainSnapshotUpdateDiskSources(virQEMUDriverPtr 
driver,
 virDomainObjPtr vm,
 qemuDomainSnapshotDiskDataPtr dd)
 {
-if (!dd->src) {
-/* Remove old metadata */
-if (qemuSecurityMoveImageMetadata(driver, vm, dd->disk->src, NULL) < 0)
-VIR_WARN("Unable to remove disk metadata on vm %s", vm->def->name);
-return;
-}
-
 /* storage driver access won'd be needed */
 if (dd->initialized)
 virStorageFileDeinit(dd->src);
-- 
2.21.0

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

[libvirt] [PATCH 04/10] qemu: snapshot: Restrict file existance check only for local storage

[Peter Krempa]

Soon we'll allow more protocols and storage types with snapshots where
we in some cases can't check whether the storage already exists.
Restrict the sanity checks whether the destination images exist or not
for local storage where it's easy. For any other case we will fail
later.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_driver.c | 44 ++
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 29a47af0a2..52540eb77d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15022,32 +15022,34 @@ 
qemuDomainSnapshotPrepareDiskExternal(virDomainDiskDefPtr disk,
 return -1;
 }

-if (virStorageFileInit(snapdisk->src) < 0)
-return -1;
+if (virStorageSourceIsLocalStorage(snapdisk->src)) {
+if (virStorageFileInit(snapdisk->src) < 0)
+return -1;

-rc = virStorageFileStat(snapdisk->src, );
-err = errno;
+rc = virStorageFileStat(snapdisk->src, );
+err = errno;

-virStorageFileDeinit(snapdisk->src);
+virStorageFileDeinit(snapdisk->src);

-if (rc < 0) {
-if (err != ENOENT) {
-virReportSystemError(err,
- _("unable to stat for disk %s: %s"),
- snapdisk->name, snapdisk->src->path);
-return -1;
-} else if (reuse) {
-virReportSystemError(err,
- _("missing existing file for disk %s: %s"),
- snapdisk->name, snapdisk->src->path);
+if (rc < 0) {
+if (err != ENOENT) {
+virReportSystemError(err,
+ _("unable to stat for disk %s: %s"),
+ snapdisk->name, snapdisk->src->path);
+return -1;
+} else if (reuse) {
+virReportSystemError(err,
+ _("missing existing file for disk %s: 
%s"),
+ snapdisk->name, snapdisk->src->path);
+return -1;
+}
+} else if (!S_ISBLK(st.st_mode) && st.st_size && !reuse) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("external snapshot file for disk %s already "
+ "exists and is not a block device: %s"),
+   snapdisk->name, snapdisk->src->path);
 return -1;
 }
-} else if (!S_ISBLK(st.st_mode) && st.st_size && !reuse) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("external snapshot file for disk %s already "
- "exists and is not a block device: %s"),
-   snapdisk->name, snapdisk->src->path);
-return -1;
 }

 return 0;
-- 
2.21.0

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

[libvirt] [PATCH 01/10] qemu: snapshot: Don't modify persistent XML if disk source is different

[Peter Krempa]

While the VM is running the persistent source of a disk might differ
e.g. as the 'newDef' was redefined. Our snapshot code would blindly
rewrite the source of such disk if it shared the 'target'. Fix this by
checking whether the source is the same in the first place.

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

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 11f97dbc65..b6fbb197b8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15332,6 +15332,7 @@ qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr 
driver,
 qemuDomainSnapshotDiskDataPtr dd;
 char *backingStoreStr;
 virDomainSnapshotDefPtr snapdef = virDomainSnapshotObjGetDef(snap);
+virDomainDiskDefPtr persistdisk;
 int ret = -1;

 if (VIR_ALLOC_N(data, snapdef->ndisks) < 0)
@@ -15352,13 +15353,12 @@ qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr 
driver,
 if (virStorageSourceInitChainElement(dd->src, dd->disk->src, false) < 
0)
 goto cleanup;

-/* Note that it's unsafe to assume that the disks in the persistent
- * definition match up with the disks in the live definition just by
- * checking that the target name is the same. We've done that
- * historically this way though. */
+/* modify disk in persistent definition only when the source is the 
same */
 if (vm->newDef &&
-(dd->persistdisk = virDomainDiskByName(vm->newDef, dd->disk->dst,
-   false))) {
+(persistdisk = virDomainDiskByName(vm->newDef, dd->disk->dst, 
false)) &&
+virStorageSourceIsSameLocation(dd->disk->src, persistdisk->src)) {
+
+dd->persistdisk = persistdisk;

 if (!(dd->persistsrc = virStorageSourceCopy(dd->src, false)))
 goto cleanup;
-- 
2.21.0

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

[libvirt] [PATCH 00/10] qemu: Blockdevize external snapshot creation (blokcdev-add saga)

[Peter Krempa]

Peter Krempa (10):
  qemu: snapshot: Don't modify persistent XML if disk source is
different
  qemu: driver: Remove dead code from
qemuDomainSnapshotUpdateDiskSources
  qemu: Remove cleanup label in qemuDomainSnapshotPrepareDiskExternal
  qemu: snapshot: Restrict file existance check only for local storage
  qemu: Split out preparing of single snapshot from
qemuDomainSnapshotDiskDataCollect
  qemu: Disband qemuDomainSnapshotCreateSingleDiskActive
  qemu: Merge conditions depending on the 'reuse' flag in
qemuDomainSnapshotDiskDataCollectOne
  qemu: snapshot: Skip overlay file creation/interogation if unsupported
  qemu: Add -blockdev support for external snapshots
  qemu: Defer support checks for external active snapshots to blockdev
code or qemu

 src/qemu/qemu_driver.c | 333 ++---
 1 file changed, 210 insertions(+), 123 deletions(-)

-- 
2.21.0

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

Re: [libvirt] [PATCH 00/10] ci: Several fixes and improvements

[Andrea Bolognani]

On Fri, 2019-08-16 at 11:49 +0200, Andrea Bolognani wrote:
> See the individual commits for details, but the gist of it is that
> after this series it's possible for users to hook into the build
> process and customize it according to their needs; on top of that,
> the whole thing is made more maintainable in the process.
> 
> Andrea Bolognani (10):
>   ci: Fix /etc/sub{u,g}id parsing
>   ci: Drop $(CI_SUBMODULES)
>   ci: Move everything to a separate directory
>   ci: Create user's home directory in the container
>   ci: Move source directory under $(CI_USER_HOME)
>   ci: Introduce $(CI_BUILD_SCRIPT)
>   ci: Generalize running commands inside the container
>   ci: Introduce $(CI_PREPARE_SCRIPT)
>   ci: Run $(CI_PREPARE_SCRIPT) as root
>   ci: Stop using --workdir

Test build to prove this actually works:

  https://travis-ci.org/andreabolognani/libvirt/builds/572691710

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH] ci: Adapt to container name changes

[Andrea Bolognani]

On Fri, 2019-08-16 at 14:32 +0200, Fabiano Fidêncio wrote:
> On Mon, 2019-08-12 at 16:09 +0200, Andrea Bolognani wrote:
> > This is connected to
> > 
> > https://www.redhat.com/archives/libvir-list/2019-August/msg00399.html
> > 
> > https://www.redhat.com/archives/libvir-list/2019-August/msg00416.html
> > 
> > and should only be merged once the above have been merged *and*
> > deployed, as in, images with the new names have been generated.
> 
> Reviewed-by: Fabiano Fidêncio 

Thanks for the reviews!

I have pushed the other two series but I can't quite push this one
yet because, as we already know, a bunch of MinGW packages have been
dropped from Fedora and thus the buildenv-libvirt-fedora-rawhide
container can't currently be built successfully:

  No match for argument: mingw32-portablexdr
  No match for argument: mingw64-portablexdr
  Error: Unable to find a match: mingw32-portablexdr mingw64-portablexdr

I think the situation would be the same for the libosinfo container.

Do you have any idea how long it will take to get the packages back
in Fedora? Should we just wait for that to happen before rebuilding
the containers, temporarily disable MinGW builds, temporarily switch
MinGW builds to Fedora 30 where I assume all MinGW packages are still
available?

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [Qemu-devel] [PATCH 2/2] qapi: deprecate implicit filters

[Vladimir Sementsov-Ogievskiy]

16.08.2019 15:33, Markus Armbruster wrote:
> Kevin Wolf  writes:
> 
>> Am 15.08.2019 um 21:24 hat Markus Armbruster geschrieben:
> [...]
>>> Let's assume all libvirt ever does with deprecation notices is logging
>>> them.  Would that solve the problem of reliably alerting libvirt
>>> developers to deprecation issues?  Nope.  But it could help
>>> occasionally.
>>
>> I'm not saying that deprecation notices would hurt, just that they
>> probably won't solve problem alone.
> 
> No argument.
> 
>> Crashing if --future is given and logging otherwise seems reasonable
>> enough to me. Whether we need to wire up a new deprecation mechanism in
>> QMP for the logging or if we can just keep printing to stderr is
>> debatable. stderr already ends up in a log file, a QMP extension would
>> require new libvirt code. If libvirt would log deprecation notices more
>> prominently, or use the information for tainting or any other kind of
>> processing, a dedicated QMP mechanism could be justified.
> 
> I'd like to start with two tasks:
> 
> * A CLI option to configure what to do on use of a deprecated feature.
> 
>We currently warn.  We want to be able to crash instead.  Silencing
>the warnings might be useful.  Turning them into errors might be
>useful.
> 
>The existing ad hoc warnings need to be replaced by a call of a common
>function that implements the configurable behavior.
> 
> * QAPI feature flag "deprecated", for introspectable deprecation, and
>without ad hoc code.
> 
> Then see whether our users need more.
> 

Crashing is useful for libvirt developers, it's obvious, just enable 
crash-on-deprecated
on all testing environments and most probably we will not miss such a case.

For qapi I doubt is it really needed. Implementing code in libvirt which will 
check for command
(or it's parameter, or it's parameter "optionality" is deprecated) ? It's hard 
and what libvirt
should report to final user? It becomes a kind of synthetic error in libvirt 
code, like

...
log_error("We are going to divide by zero. It's a bug, please report it to 
developers!");
x = a / 0;
...

It's simpler to fix second line than implement special mechanism including 
protocol specification
to report such a case.

I exaggerate of course with this example, but I doubt that implementing a 
special protocol
for it worth doing. And I think notifying libvirt by email (as Peter said) and 
providing option
"crash-on-deprecated" in Qemu are enough for libvirt developers to prevent and 
to fix using
deprecated things.

In other words, I don't see why reporting deprecated feature usage is better in 
libvirt than in
Qemu (by warning, error or crash), and in Qemu it's much more simple and don't 
need QAPI protocol
extension.

(I'm sorry if I'm repeating already written arguments, I've not read the whole 
thread)

-- 
Best regards,
Vladimir

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

Re: [libvirt] [PATCH v2 0/7] PCI Multifunction hotplug/unplug, part 1

[Michal Privoznik]

On 7/25/19 8:09 PM, Daniel Henrique Barboza wrote:
>

Huh, looks like we have came across same bugs recently. Well, great 
minds think alike :D
So I guess the resolution might be this: I'll merge my patches as soon 
as I get review and then you can repost your patches (it's going to be 
shorter series that way).


Michal

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

Re: [libvirt] [PATCH v2 4/7] virpcitest: Change the stub driver to vfio from pci-stub

[Michal Privoznik]

On 7/25/19 8:09 PM, Daniel Henrique Barboza wrote:

From: Shivaprasad G Bhat 

The pci-stub is obsolete for a while now. Upcoming test cases try
to test the VFIO hotplug/unplug cases.

Change the default test driver to vfio-pci instead of pci-stub,
and fail bind for pci-stub instead.

Signed-off-by: Shivaprasad G Bhat 
Signed-off-by: Daniel Henrique Barboza 
---
  tests/virhostdevtest.c |  4 ++--
  tests/virpcimock.c |  4 ++--
  tests/virpcitest.c | 12 ++--
  3 files changed, 10 insertions(+), 10 deletions(-)


Huh, again, same patches ...



diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c
index 20eaca82e0..99ee2d44ec 100644
--- a/tests/virhostdevtest.c
+++ b/tests/virhostdevtest.c
@@ -93,7 +93,7 @@ myInit(void)
  subsys.u.pci.addr.bus = 0;
  subsys.u.pci.addr.slot = i + 1;
  subsys.u.pci.addr.function = 0;
-subsys.u.pci.backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM;
+subsys.u.pci.backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO;
  hostdevs[i]->source.subsys = subsys;
  }
  
@@ -101,7 +101,7 @@ myInit(void)

  if (!(dev[i] = virPCIDeviceNew(0, 0, i + 1, 0)))
  goto cleanup;
  
-virPCIDeviceSetStubDriver(dev[i], VIR_PCI_STUB_DRIVER_KVM);

+virPCIDeviceSetStubDriver(dev[i], VIR_PCI_STUB_DRIVER_VFIO);
  }
  
  if (VIR_ALLOC(mgr) < 0)

diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index 1b44ed0a44..deb802e860 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -978,8 +978,8 @@ init_env(void)
  
  MAKE_PCI_DRIVER("iwlwifi", 0x8086, 0x0044);

  MAKE_PCI_DRIVER("i915", 0x8086, 0x0046, 0x8086, 0x0047);
-MAKE_PCI_DRIVER("pci-stub", -1, -1);
-pci_driver_new("vfio-pci", PCI_ACTION_BIND, -1, -1);
+pci_driver_new("pci-stub", PCI_ACTION_BIND, -1, -1);
+MAKE_PCI_DRIVER("vfio-pci", -1, -1);


.. except this one. This means I can drop two patches from mine series 
if I include this change :-) Let me resping that.


Michal

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

Re: [libvirt] [PATCH v2 1/7] tests: Fix the iommu group path in mock pci

[Michal Privoznik]

On 7/25/19 8:09 PM, Daniel Henrique Barboza wrote:

From: Shivaprasad G Bhat 

The mocked path falls into /sys/bus/kernel/iommu_groups instead of
/sys/kernel/iommu_groups. Needed for adding some new test cases.

Signed-off-by: Shivaprasad G Bhat 
Signed-off-by: Daniel Henrique Barboza 
---
  tests/virpcimock.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index beb5e1490d..5ec9abe05f 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -399,7 +399,7 @@ pci_device_new_from_stub(const struct pciDevice *data)
  make_file(devpath, "class", tmp, -1);
  
  if (snprintf(tmp, sizeof(tmp),

- "%s/../../../kernel/iommu_groups/%d",
+ "%s/../../../../kernel/iommu_groups/%d",
   devpath, dev->iommuGroup) < 0) {
  ABORT("@tmp overflow");
  }
@@ -407,7 +407,7 @@ pci_device_new_from_stub(const struct pciDevice *data)
  ABORT("Unable to create %s", tmp);
  
  if (snprintf(tmp, sizeof(tmp),

- "../../../kernel/iommu_groups/%d", dev->iommuGroup) < 0) {
+ "../../../../kernel/iommu_groups/%d", dev->iommuGroup) < 0) {
  ABORT("@tmp overflow");
  }
  make_symlink(devpath, "iommu_group", tmp);



Huh, this is again something that I fix in my series. Well, in fact, 
since I'm moving the place where PCI devices are created the symlink as 
we have it is correct in fact.


Michal

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

Re: [libvirt] [PATCH v2 6/7] tests: qemu: Add test case for pci-hostdev hotplug

[Michal Privoznik]

On 7/25/19 8:09 PM, Daniel Henrique Barboza wrote:

From: Shivaprasad G Bhat 

Signed-off-by: Shivaprasad G Bhat 
Signed-off-by: Daniel Henrique Barboza 
---
  src/util/virprocess.h |  2 +-
  tests/Makefile.am |  7 +++
  tests/qemuhotplugtest.c   | 42 +-
  .../qemuhotplug-hostdev-pci.xml   |  6 ++
  .../qemuhotplug-base-live+hostdev-pci.xml | 58 +++
  ...uhotplug-pseries-base-live+hostdev-pci.xml | 51 
  .../qemuhotplug-pseries-base-live.xml | 43 ++
  tests/virprocessmock.c| 28 +
  8 files changed, 235 insertions(+), 2 deletions(-)
  create mode 100644 tests/qemuhotplugtestdevices/qemuhotplug-hostdev-pci.xml
  create mode 100644 
tests/qemuhotplugtestdomains/qemuhotplug-base-live+hostdev-pci.xml
  create mode 100644 
tests/qemuhotplugtestdomains/qemuhotplug-pseries-base-live+hostdev-pci.xml
  create mode 100644 
tests/qemuhotplugtestdomains/qemuhotplug-pseries-base-live.xml
  create mode 100644 tests/virprocessmock.c

diff --git a/src/util/virprocess.h b/src/util/virprocess.h
index 003ba1edf4..4806c592da 100644
--- a/src/util/virprocess.h
+++ b/src/util/virprocess.h
@@ -75,7 +75,7 @@ int virProcessGetNamespaces(pid_t pid,
  int virProcessSetNamespaces(size_t nfdlist,
  int *fdlist);
  
-int virProcessSetMaxMemLock(pid_t pid, unsigned long long bytes);

+int virProcessSetMaxMemLock(pid_t pid, unsigned long long bytes) 
ATTRIBUTE_NOINLINE;
  int virProcessSetMaxProcesses(pid_t pid, unsigned int procs);
  int virProcessSetMaxFiles(pid_t pid, unsigned int files);
  int virProcessSetMaxCoreSize(pid_t pid, unsigned long long bytes);
diff --git a/tests/Makefile.am b/tests/Makefile.am
index f480e68e7d..66d249a0cc 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -210,6 +210,7 @@ test_libraries = libshunload.la \
virpcimock.la \
virnetdevmock.la \
virrandommock.la \
+  virprocessmock.la \
virhostcpumock.la \
domaincapsmock.la \
virfilecachemock.la \
@@ -1182,6 +1183,12 @@ virrandommock_la_SOURCES = \
  virrandommock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
  virrandommock_la_LIBADD = $(MOCKLIBS_LIBS)
  
+virprocessmock_la_SOURCES = \

+  virprocessmock.c
+virprocessmock_la_CFLAGS = $(AM_CFLAGS)
+virprocessmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
+virprocessmock_la_LIBADD = $(MOCKLIBS_LIBS)
+
  virhostcpumock_la_SOURCES = \
virhostcpumock.c
  virhostcpumock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 354068d748..bf95bfb4a7 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -28,6 +28,7 @@
  #include "testutils.h"
  #include "testutilsqemu.h"
  #include "testutilsqemuschema.h"
+#include "virhostdev.h"
  #include "virerror.h"
  #include "virstring.h"
  #include "virthread.h"
@@ -79,6 +80,8 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
  virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_DEVICE_IVSHMEM_PLAIN);
  virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL);
  virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_SCSI_DISK_WWN);
+virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI);
+virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE);
  
  if (qemuTestCapsCacheInsert(driver.qemuCapsCache, priv->qemuCaps) < 0)

  goto cleanup;
@@ -130,6 +133,9 @@ testQemuHotplugAttach(virDomainObjPtr vm,
  case VIR_DOMAIN_DEVICE_WATCHDOG:
  ret = qemuDomainAttachWatchdog(, vm, dev->data.watchdog);
  break;
+case VIR_DOMAIN_DEVICE_HOSTDEV:
+ret = qemuDomainAttachHostDevice(, vm, dev->data.hostdev);
+break;
  default:
  VIR_TEST_VERBOSE("device type '%s' cannot be attached\n",
  virDomainDeviceTypeToString(dev->type));
@@ -151,6 +157,7 @@ testQemuHotplugDetach(virDomainObjPtr vm,
  case VIR_DOMAIN_DEVICE_CHR:
  case VIR_DOMAIN_DEVICE_SHMEM:
  case VIR_DOMAIN_DEVICE_WATCHDOG:
+case VIR_DOMAIN_DEVICE_HOSTDEV:
  ret = qemuDomainDetachDeviceLive(vm, dev, , async);
  break;
  default:
@@ -578,6 +585,7 @@ testQemuHotplugCpuIndividual(const void *opaque)
  return ret;
  }
  
+#define FAKEROOTDIRTEMPLATE abs_builddir "/fakerootdir-XX"
  
  
  static int

@@ -587,6 +595,21 @@ mymain(void)
  int ret = 0;
  struct qemuHotplugTestData data = {0};
  struct testQemuHotplugCpuParams cpudata;
+char *fakerootdir;
+
+if (VIR_STRDUP_QUIET(fakerootdir, FAKEROOTDIRTEMPLATE) < 0) {
+fprintf(stderr, "Out of memory\n");
+abort();
+}
+
+if (!mkdtemp(fakerootdir)) {
+fprintf(stderr, "Cannot create fakerootdir");
+abort();
+}
+
+setenv("LIBVIRT_FAKE_ROOT_DIR", fakerootdir, 1);
+unsetenv("LD_PRELOAD");


This unsetenv is not necessary, isn't it?

Reviewed-by: Michal Privoznik 

Michal

Re: [libvirt] [PATCH v2 7/7] tests: Add a baseline test for multifunction pci device use case

[Michal Privoznik]

On 7/25/19 8:09 PM, Daniel Henrique Barboza wrote:

From: Shivaprasad G Bhat 

There are already good number of test cases with hostdevices,
few have multifunction devices but none having more than one
than one multifunction cards.

This patch adds a case where there are two multifunction cards
and two Virtual functions part of the same XML.

0001:01:00.X & 0005:09:00.X - are Multifunction PCI cards.
:06:12.[5|6] - are SRIOV Virtual functions

The next few commits improve on automatically detecting the multifunction
cards and auto-assinging the addresses appropriately.

Signed-off-by: Shivaprasad G Bhat 
Signed-off-by: Daniel Henrique Barboza 
---
  .../hostdev-pci-multifunction.args| 35 
  .../hostdev-pci-multifunction.xml | 59 ++
  tests/qemuxml2argvtest.c  |  3 +
  .../hostdev-pci-multifunction.xml | 79 +++
  tests/qemuxml2xmltest.c   |  1 +
  5 files changed, 177 insertions(+)
  create mode 100644 tests/qemuxml2argvdata/hostdev-pci-multifunction.args
  create mode 100644 tests/qemuxml2argvdata/hostdev-pci-multifunction.xml
  create mode 100644 tests/qemuxml2xmloutdata/hostdev-pci-multifunction.xml


Reviewed-by: Michal Privoznik 

Michal

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

Re: [libvirt] [PATCH v2 2/7] tests: pci: Mock the iommu groups and vfio

[Michal Privoznik]

On 7/25/19 8:09 PM, Daniel Henrique Barboza wrote:

From: Shivaprasad G Bhat 

The iommu group, /dev/vfio/ behaviors
of the host are mocked. This patch implements support for
multifunction/multiple devices per iommu groups and emulates
the /dev/vfio/ file correctly.

This code helps adding necessary testcases for pci-hotplug
code.

Signed-off-by: Shivaprasad G Bhat 
Signed-off-by: Daniel Henrique Barboza 
---
  tests/virpcimock.c | 177 +
  1 file changed, 162 insertions(+), 15 deletions(-)


Looks good. We will have some merge conflicts because we fix the same issue.



diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index 5ec9abe05f..1b44ed0a44 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -44,6 +44,8 @@ char *fakerootdir;
  char *fakesysfspcidir;
  
  # define SYSFS_PCI_PREFIX "/sys/bus/pci/"

+# define SYSFS_KERNEL_PREFIX "/sys/kernel/"
+
  
  # define STDERR(...) \

  fprintf(stderr, "%s %zu: ", __FUNCTION__, (size_t) __LINE__); \
@@ -123,6 +125,11 @@ struct pciDevice {
  struct pciDriver *driver;   /* Driver attached. NULL if attached to no 
driver */
  };
  
+struct pciIommuGroup {

+int iommu;
+size_t nDevicesBoundToVFIO;/* Indicates the devices in the group */
+};
+
  struct fdCallback {
  int fd;
  char *path;
@@ -134,6 +141,9 @@ size_t nPCIDevices = 0;
  struct pciDriver **pciDrivers = NULL;
  size_t nPCIDrivers = 0;
  
+struct pciIommuGroup **pciIommuGroups = NULL;

+size_t npciIommuGroups = 0;
+
  struct fdCallback *callbacks = NULL;
  size_t nCallbacks = 0;
  
@@ -247,6 +257,13 @@ getrealpath(char **newpath,

  errno = ENOMEM;
  return -1;
  }
+} else if (STRPREFIX(path, SYSFS_KERNEL_PREFIX)) {
+if (virAsprintfQuiet(newpath, "%s/%s",
+ fakerootdir,
+ path) < 0) {
+errno = ENOMEM;
+return -1;
+}
  } else {
  if (VIR_STRDUP_QUIET(*newpath, path) < 0)
  return -1;
@@ -460,6 +477,101 @@ pci_device_autobind(struct pciDevice *dev)
  return pci_driver_bind(driver, dev);
  }
  
+static void

+pci_iommu_new(int num)
+{
+char *iommupath, *kerneldir;
+struct pciIommuGroup *iommuGroup;
+
+if (VIR_ALLOC_QUIET(iommuGroup) < 0)
+ABORT_OOM();
+
+iommuGroup->iommu = num;
+iommuGroup->nDevicesBoundToVFIO = 0; /* No device bound to vfio by default 
*/
+
+if (virAsprintfQuiet(, "%s%s",
+ fakerootdir, SYSFS_KERNEL_PREFIX) < 0)
+ABORT_OOM();
+
+if (virAsprintfQuiet(, "%s/iommu_groups/%d/devices", kerneldir, 
num) < 0)
+ABORT_OOM();
+VIR_FREE(kerneldir);
+
+if (virFileMakePath(iommupath) < 0)
+ABORT("Unable to create: %s", iommupath);
+VIR_FREE(iommupath);
+
+if (VIR_APPEND_ELEMENT_QUIET(pciIommuGroups, npciIommuGroups, iommuGroup) 
< 0)
+ABORT_OOM();
+}
+
+static int
+pci_vfio_release_iommu(struct pciDevice *device)
+{
+char *vfiopath = NULL;
+int ret = -1;
+size_t i = 0;
+
+for (i = 0; i < npciIommuGroups; i++) {
+if (pciIommuGroups[i]->iommu == device->iommuGroup)
+break;
+}
+
+if (i != npciIommuGroups) {
+if (pciIommuGroups[i]->nDevicesBoundToVFIO == 0) {
+ret = 0;
+goto cleanup;
+}
+pciIommuGroups[i]->nDevicesBoundToVFIO--;
+if (!pciIommuGroups[i]->nDevicesBoundToVFIO) {
+if (virAsprintfQuiet(, "%s/dev/vfio/%d",
+ fakesysfspcidir, device->iommuGroup) < 0) {
+errno = ENOMEM;
+goto cleanup;
+}
+if (unlink(vfiopath) < 0)
+goto cleanup;
+}
+}
+
+ret = 0;
+ cleanup:
+VIR_FREE(vfiopath);
+return ret;
+}
+
+static int
+pci_vfio_lock_iommu(struct pciDevice *device)
+{
+char *vfiopath = NULL;
+int ret = -1;
+size_t i = 0;
+int fd = -1;
+
+for (i = 0; i < npciIommuGroups; i++) {
+if (pciIommuGroups[i]->iommu == device->iommuGroup)
+break;
+}
+
+if (i != npciIommuGroups) {
+if (!pciIommuGroups[i]->nDevicesBoundToVFIO) {
+if (virAsprintfQuiet(, "%s/dev/vfio/%d",
+ fakesysfspcidir, device->iommuGroup) < 0) {
+errno = ENOMEM;
+goto cleanup;
+}
+if ((fd = real_open(vfiopath, O_CREAT)) < 0)
+goto cleanup;
+}
+pciIommuGroups[i]->nDevicesBoundToVFIO++;
+}
+
+ret = 0;
+ cleanup:
+real_close(fd);
+VIR_FREE(vfiopath);
+return ret;
+}
  
  /*

   * PCI Driver functions
@@ -583,6 +695,10 @@ pci_driver_bind(struct pciDriver *driver,
  if (symlink(devpath, driverpath) < 0)
  goto cleanup;
  
+if (STREQ(driver->name, "vfio-pci"))

+if (pci_vfio_lock_iommu(dev) < 0)
+goto cleanup;
+
  dev->driver = driver;
  ret 

Re: [libvirt] [PATCH v2 3/7] util/virhostdev: enhance VFIO device is in use detection

[Michal Privoznik]

On 7/25/19 8:09 PM, Daniel Henrique Barboza wrote:

The current virHostdevPreparePCIDevices code fails to detect an unmanaged
VFIO device that is in the activePCIHostdevs, and active in the same
domain name as dom_name, as a device in use. Considering a call to this
function, when activePCIHostdevs has a VFIO deviceA in the list, and
deviceA is active in domain 'dom_name', this is what happens:

- At step 1, the code goes to the 'if (usesVFIO)' block. All devices in
the same IOMMU group of deviceA are used as argument of
virHostdevIsPCINodeDeviceUsed, via the IOMMUGroupIterate function;

- Inside virHostdevIsPCINodeDeviceUsed, an 'usesVFIO' verification is
made, following to an 'iommu_owner' jump that sets ret=0. This will
happen to all devices of the IOMMU group that belongs to the same domain
as dom_name, including deviceA;

- Step 2 starts, thinking that all devices inside hostdevs are available,
which is not the case.

This error was detected when changing tests/virhostdev.c to use
vfio-pci instead of pci-stub (a change that will follow this one).
The side effect observed was a test failure in
testVirHostdevPreparePCIHostdevs_unmanaged, result of the behavior
mentioned above:

Unexpected count of items in mgr->inactivePCIHostdevs: 1, expecting 0

This patch fixes virHostdevIsPCINodeDeviceUsed to consider the case of
a VFIO device that is already active in the domain. First, check if
the device is in the activePCIHostdev list and bail immediately if
true. Otherwise, in case of VFIO, check for IOMMU group ownership
of the domain. This is done by a new callback function for
IOMMUGroupIterate. If the VFIO device isn't in the active list and
its domain has ownership of the IOMMU, then it is unused.

Signed-off-by: Daniel Henrique Barboza 
---
  src/util/virhostdev.c | 74 +--
  1 file changed, 57 insertions(+), 17 deletions(-)


As you pointed out, I've sent a patch for the same issue. And if you 
don't mind, I'd like to go with mine, because it's shorter and simpler.


Michal

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

Re: [libvirt] [Qemu-devel] [PATCH 2/2] qapi: deprecate implicit filters

[Markus Armbruster]

Kevin Wolf  writes:

> Am 15.08.2019 um 21:24 hat Markus Armbruster geschrieben:
[...]
>> Let's assume all libvirt ever does with deprecation notices is logging
>> them.  Would that solve the problem of reliably alerting libvirt
>> developers to deprecation issues?  Nope.  But it could help
>> occasionally.
>
> I'm not saying that deprecation notices would hurt, just that they
> probably won't solve problem alone.

No argument.

> Crashing if --future is given and logging otherwise seems reasonable
> enough to me. Whether we need to wire up a new deprecation mechanism in
> QMP for the logging or if we can just keep printing to stderr is
> debatable. stderr already ends up in a log file, a QMP extension would
> require new libvirt code. If libvirt would log deprecation notices more
> prominently, or use the information for tainting or any other kind of
> processing, a dedicated QMP mechanism could be justified.

I'd like to start with two tasks:

* A CLI option to configure what to do on use of a deprecated feature.

  We currently warn.  We want to be able to crash instead.  Silencing
  the warnings might be useful.  Turning them into errors might be
  useful.

  The existing ad hoc warnings need to be replaced by a call of a common
  function that implements the configurable behavior.

* QAPI feature flag "deprecated", for introspectable deprecation, and
  without ad hoc code.

Then see whether our users need more.

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

Re: [libvirt] [PATCH] ci: Adapt to container name changes

[Fabiano Fidêncio]

On Mon, 2019-08-12 at 16:09 +0200, Andrea Bolognani wrote:
> Since libvirt-dockerfile's commit 7130ffe0a0e9, the containers
> used for CI builds have been renamed from buildenv-* to
> buildenv-libvirt-* in order to make it possible for projects
> other than libvirt to be supported, so we need to update our
> Makefile.ci scaffolding accordingly.
> 
> Signed-off-by: Andrea Bolognani 
> ---
> This is connected to
> 
>   
> https://www.redhat.com/archives/libvir-list/2019-August/msg00399.html
>   
> https://www.redhat.com/archives/libvir-list/2019-August/msg00416.html
> 
> and should only be merged once the above have been merged *and*
> deployed, as in, images with the new names have been generated.
> 
>  Makefile.ci | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile.ci b/Makefile.ci
> index 8857c953b2..438a98 100644
> --- a/Makefile.ci
> +++ b/Makefile.ci
> @@ -49,7 +49,7 @@ CI_SUBMODULES = $(shell git submodule | awk '{
> print $$2 }')
>  # Location of the container images we're going to pull
>  # Can be useful to overridde to use a locally built
>  # image instead
> -CI_IMAGE_PREFIX = quay.io/libvirt/buildenv-
> +CI_IMAGE_PREFIX = quay.io/libvirt/buildenv-libvirt-
>  
>  # The default tag is ':latest' but if the container
>  # repo above uses different conventions this can override it

Reviewed-by: Fabiano Fidêncio 

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

Re: [libvirt] [dockerfiles PATCH 0/3] refresh: Generate archived Dockerfiles

[Fabiano Fidêncio]

On Mon, 2019-08-12 at 14:28 +0200, Andrea Bolognani wrote:
> See commit 1/3 for more information.
> 
> Andrea Bolognani (3):
>   refresh: Generate archived Dockerfiles
>   Remove plain Dockerfiles
>   Add archived Dockerfiles
> 

Reviewed-by: Fabiano Fidêncio 

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

Re: [libvirt] [jenkins-ci PATCH 0/2] quayadmin: Add create-build command

[Fabiano Fidêncio]

On Mon, 2019-08-12 at 15:35 +0200, Andrea Bolognani wrote:
> Once [1] has been merged, we'll be able to conveniently cause
> container images to be rebuilt without having to define custom
> triggers.
> 
> [1] 
> https://www.redhat.com/archives/libvir-list/2019-August/msg00399.html
> 
> Andrea Bolognani (2):
>   quayadmin: Fix endpoints
>   quayadmin: Add create-build command
> 
>  guests/quayadmin | 46 ++
>  1 file changed, 42 insertions(+), 4 deletions(-)
> 

Reviewed-by: Fabiano Fidêncio 

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

Re: [libvirt] [PATCH 15/18] virhostdev: Unify virHostdevPreparePCIDevices behaviour for KVM and VFIO cases

[Daniel Henrique Barboza]

On 8/16/19 5:59 AM, Michal Privoznik wrote:

On 8/14/19 6:14 PM, Daniel Henrique Barboza wrote:

Michal, I believe the problem you're trying to fix in this patch
is somehow the same I'm trying to fix in this patch here:


https://www.redhat.com/archives/libvir-list/2019-June/msg01270.html

Do you mind taking a look? If that's the case, I can drop that patch
from the series that implements Multifunction PCI hotplug.


Oh, I haven't taken a look on them yet. Yes, this patch fixes the same 
problem. If you're okay with it, I'd rather go with my approach since 
it's simpler.


I don't mind. Let's go with yours. I'll remove that patch from the series as
soon as this gets pushed (I would need to resubmit that whole series
anyway).


Thanks,


DHB



Michal


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

[libvirt] [PATCH 11/14] util: storagefile: Add handling of unusable storage sources

[Peter Krempa]

Introduce new semantics to virStorageSourceNewFromBacking and some
of the helpers used by it which propagate the return value from the
callers.

The new return value introduced by this patch allows to notify the
calller that the parsed virStorageSource correctly describes the source
but contains data such as inline authentication which libvirt does not
want to support directly. This means that such file would e.g. unusable
as a storage source (e.g. when actively commiting the overlay to it) or
would not work with blockdev.

The caller will then be able to decide whether to consider this backing
file as viable or just fall back to qemu dealing with it.

Signed-off-by: Peter Krempa 
---
 src/util/virstoragefile.c | 30 ++
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 8447c014f0..fa0233376b 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -3554,6 +3554,13 @@ virStorageSourceParseBackingJSONVxHS(virStorageSourcePtr 
src,

 struct virStorageSourceJSONDriverParser {
 const char *drvname;
+/**
+ * The callback gets a pre-allocated storage source @src and the JSON
+ * object to parse. The callback shall return -1 on error and report error
+ * 0 on success and 1 in cases when the configuration itself is valid, but
+ * can't be converted to libvirt's configuration (e.g. inline 
authentication
+ * credentials are present).
+ */
 int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque);
 int opaque;
 };
@@ -3638,15 +3645,17 @@ virStorageSourceParseBackingJSON(virStorageSourcePtr 
src,
  * @path: string representing absolute location of a storage source
  * @src: filled with virStorageSource object representing @path
  *
- * Returns 0 on success and fills @src or -1 on error and reports appropriate
- * error.
+ * Returns 0 on success, 1 if we could parse all location data but @path
+ * specified other data unrepresentable by libvirt (e.g. inline 
authentication).
+ * In both cases @src is filled. On error -1 is returned @src is NULL and an
+ * error is reported.
  */
 int
 virStorageSourceNewFromBackingAbsolute(const char *path,
virStorageSourcePtr *src)
 {
 const char *json;
-int rc;
+int rc = 0;
 VIR_AUTOUNREF(virStorageSourcePtr) def = NULL;

 *src = NULL;
@@ -3687,7 +3696,7 @@ virStorageSourceNewFromBackingAbsolute(const char *path,
 }

 VIR_STEAL_PTR(*src, def);
-return 0;
+return rc;
 }


@@ -3701,7 +3710,11 @@ virStorageSourceNewFromBackingAbsolute(const char *path,
  * and other data. Note that for local storage this function interrogates the
  * actual type of the backing store.
  *
- * Returns 0 and fills @backing, or -1 on error (with appropriate error 
reported).
+ * Returns 0 on success, 1 if we could parse all location data but the backinig
+ * store specification contained other data unrepresentable by libvirt (e.g.
+ * inline authentication).
+ * In both cases @src is filled. On error -1 is returned @src is NULL and an
+ * error is reported.
  */
 int
 virStorageSourceNewFromBacking(virStorageSourcePtr parent,
@@ -3709,6 +3722,7 @@ virStorageSourceNewFromBacking(virStorageSourcePtr parent,
 {
 struct stat st;
 VIR_AUTOUNREF(virStorageSourcePtr) def = NULL;
+int rc = 0;

 *backing = NULL;

@@ -3717,8 +3731,8 @@ virStorageSourceNewFromBacking(virStorageSourcePtr parent,

parent->backingStoreRaw)))
 return -1;
 } else {
-if (virStorageSourceNewFromBackingAbsolute(parent->backingStoreRaw,
-   ) < 0)
+if ((rc = 
virStorageSourceNewFromBackingAbsolute(parent->backingStoreRaw,
+ )) < 0)
 return -1;
 }

@@ -3742,7 +3756,7 @@ virStorageSourceNewFromBacking(virStorageSourcePtr parent,
 def->detected = true;

 VIR_STEAL_PTR(*backing, def);
-return 0;
+return rc;
 }


-- 
2.21.0

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

[libvirt] [PATCH 05/14] tests: viruri: Add test for password in URI userinfo

[Peter Krempa]

While it's a bad idea to use userinfo to pass credentials via an URI add
a test that we at least do the correct thing.

Signed-off-by: Peter Krempa 
---
 tests/viruritest.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/viruritest.c b/tests/viruritest.c
index d419711135..3255e2333a 100644
--- a/tests/viruritest.c
+++ b/tests/viruritest.c
@@ -171,6 +171,7 @@ mymain(void)

 TEST_PARSE("test://example.com", "test", "example.com", 0, NULL, NULL, 
NULL, NULL, NULL);
 TEST_PARSE("test://f...@example.com", "test", "example.com", 0, NULL, 
NULL, NULL, "foo", NULL);
+TEST_PARSE("test://foo:p...@example.com", "test", "example.com", 0, NULL, 
NULL, NULL, "foo:pass", NULL);
 TEST_PARSE("test://example.com:123", "test", "example.com", 123, NULL, 
NULL, NULL, NULL, NULL);
 TEST_PARSE("test://example.com:123/system?name=value#foo", "test", 
"example.com", 123, "/system", "name=value", "foo", NULL, params);
 TEST_PARSE("test://127.0.0.1:123/system", "test", "127.0.0.1", 123, 
"/system", NULL, NULL, NULL, NULL);
-- 
2.21.0

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

[libvirt] [PATCH 04/14] util: storagefile: Simplify cleanup in virStorageSourceParseBackingJSON

[Peter Krempa]

Automatically free the 'root' temporary variable to get rid fo some
complexity.

Signed-off-by: Peter Krempa 
---
 src/util/virstoragefile.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index e93f6285b0..6fff013e3a 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -3622,16 +3622,12 @@ static int
 virStorageSourceParseBackingJSON(virStorageSourcePtr src,
  const char *json)
 {
-virJSONValuePtr root = NULL;
-int ret = -1;
+VIR_AUTOPTR(virJSONValue) root = NULL;

 if (!(root = virJSONValueFromString(json)))
 return -1;

-ret = virStorageSourceParseBackingJSONInternal(src, root);
-
-virJSONValueFree(root);
-return ret;
+return virStorageSourceParseBackingJSONInternal(src, root);
 }


-- 
2.21.0

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

[libvirt] [PATCH 03/14] util: storagefile: Simplify cleanup handling in virStorageSourceParseBackingURI

[Peter Krempa]

Automatically clean the 'uri' variable and get rid of the 'cleanup'
label.

Signed-off-by: Peter Krempa 
---
 src/util/virstoragefile.c | 31 +--
 1 file changed, 13 insertions(+), 18 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 8af45bfbd2..e93f6285b0 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2664,32 +2664,31 @@ static int
 virStorageSourceParseBackingURI(virStorageSourcePtr src,
 const char *uristr)
 {
-virURIPtr uri = NULL;
+VIR_AUTOPTR(virURI)uri = NULL;
 const char *path = NULL;
-int ret = -1;
 VIR_AUTOSTRINGLIST scheme = NULL;

 if (!(uri = virURIParse(uristr))) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("failed to parse backing file location '%s'"),
uristr);
-goto cleanup;
+return -1;
 }

 if (VIR_ALLOC(src->hosts) < 0)
-goto cleanup;
+return -1;

 src->nhosts = 1;

 if (!(scheme = virStringSplit(uri->scheme, "+", 2)))
-goto cleanup;
+return -1;

 if (!scheme[0] ||
 (src->protocol = virStorageNetProtocolTypeFromString(scheme[0])) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("invalid backing protocol '%s'"),
NULLSTR(scheme[0]));
-goto cleanup;
+return -1;
 }

 if (scheme[1] &&
@@ -2697,13 +2696,13 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src,
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("invalid protocol transport type '%s'"),
scheme[1]);
-goto cleanup;
+return -1;
 }

 /* handle socket stored as a query */
 if (uri->query) {
 if (VIR_STRDUP(src->hosts->socket, STRSKIP(uri->query, "socket=")) < 0)
-goto cleanup;
+return -1;
 }

 /* XXX We currently don't support auth, so don't bother parsing it */
@@ -2725,7 +2724,7 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src,
 path = NULL;

 if (VIR_STRDUP(src->path, path) < 0)
-goto cleanup;
+return -1;

 if (src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER) {
 char *tmp;
@@ -2733,7 +2732,7 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src,
 if (!src->path) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("missing volume name and path for gluster 
volume"));
-goto cleanup;
+return -1;
 }

 if (!(tmp = strchr(src->path, '/')) ||
@@ -2741,13 +2740,13 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src,
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("missing volume name or file name in "
  "gluster source path '%s'"), src->path);
-goto cleanup;
+return -1;
 }

 src->volume = src->path;

 if (VIR_STRDUP(src->path, tmp + 1) < 0)
-goto cleanup;
+return -1;

 tmp[0] = '\0';
 }
@@ -2755,13 +2754,9 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src,
 src->hosts->port = uri->port;

 if (VIR_STRDUP(src->hosts->name, uri->server) < 0)
-goto cleanup;
-
-ret = 0;
+return -1;

- cleanup:
-virURIFree(uri);
-return ret;
+return 0;
 }


-- 
2.21.0

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

[libvirt] [PATCH 09/14] util: storagefile: Modify arguments of virStorageSourceNewFromBackingAbsolue

[Peter Krempa]

Return the parsed storage source via an pointer in arguments and return
an integer from the function. Describe the semantics with a comment for
the function and adjust callers to the new semantics.

Signed-off-by: Peter Krempa 
---
 src/util/virstoragefile.c | 43 ---
 src/util/virstoragefile.h |  3 ++-
 tests/qemublocktest.c |  3 ++-
 tests/virstoragetest.c|  2 +-
 4 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 192a79c025..8447c014f0 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -3633,22 +3633,32 @@ virStorageSourceParseBackingJSON(virStorageSourcePtr 
src,
 }


-virStorageSourcePtr
-virStorageSourceNewFromBackingAbsolute(const char *path)
+/**
+ * virStorageSourceNewFromBackingAbsolute
+ * @path: string representing absolute location of a storage source
+ * @src: filled with virStorageSource object representing @path
+ *
+ * Returns 0 on success and fills @src or -1 on error and reports appropriate
+ * error.
+ */
+int
+virStorageSourceNewFromBackingAbsolute(const char *path,
+   virStorageSourcePtr *src)
 {
 const char *json;
-virStorageSourcePtr ret = NULL;
 int rc;
 VIR_AUTOUNREF(virStorageSourcePtr) def = NULL;

+*src = NULL;
+
 if (!(def = virStorageSourceNew()))
-return NULL;
+return -1;

 if (virStorageIsFile(path)) {
 def->type = VIR_STORAGE_TYPE_FILE;

 if (VIR_STRDUP(def->path, path) < 0)
-return NULL;
+return -1;
 } else {
 def->type = VIR_STORAGE_TYPE_NETWORK;

@@ -3663,7 +3673,7 @@ virStorageSourceNewFromBackingAbsolute(const char *path)
 rc = virStorageSourceParseBackingColon(def, path);

 if (rc < 0)
-return NULL;
+return -1;

 virStorageSourceNetworkAssignDefaultPorts(def);

@@ -3676,8 +3686,8 @@ virStorageSourceNewFromBackingAbsolute(const char *path)
 }
 }

-VIR_STEAL_PTR(ret, def);
-return ret;
+VIR_STEAL_PTR(*src, def);
+return 0;
 }


@@ -3702,14 +3712,15 @@ virStorageSourceNewFromBacking(virStorageSourcePtr 
parent,

 *backing = NULL;

-if (virStorageIsRelative(parent->backingStoreRaw))
-def = virStorageSourceNewFromBackingRelative(parent,
- parent->backingStoreRaw);
-else
-def = virStorageSourceNewFromBackingAbsolute(parent->backingStoreRaw);
-
-if (!def)
-return -1;
+if (virStorageIsRelative(parent->backingStoreRaw)) {
+if (!(def = virStorageSourceNewFromBackingRelative(parent,
+   
parent->backingStoreRaw)))
+return -1;
+} else {
+if (virStorageSourceNewFromBackingAbsolute(parent->backingStoreRaw,
+   ) < 0)
+return -1;
+}

 /* possibly update local type */
 if (def->type == VIR_STORAGE_TYPE_FILE) {
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 3a72c62ad7..2cceaf6954 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -473,7 +473,8 @@ int 
virStorageFileGetRelativeBackingPath(virStorageSourcePtr from,

 int virStorageFileCheckCompat(const char *compat);

-virStorageSourcePtr virStorageSourceNewFromBackingAbsolute(const char *path);
+int virStorageSourceNewFromBackingAbsolute(const char *path,
+   virStorageSourcePtr *src);

 bool virStorageSourceIsRelative(virStorageSourcePtr src);

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 9321531f6c..e5d77c423c 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -85,7 +85,8 @@ testBackingXMLjsonXML(const void *args)
 if (virAsprintf(, "json:%s", propsstr) < 0)
 return -1;

-if (!(jsonsrc = virStorageSourceNewFromBackingAbsolute(protocolwrapper))) {
+if (virStorageSourceNewFromBackingAbsolute(protocolwrapper,
+   ) < 0) {
 fprintf(stderr, "failed to parse disk json\n");
 return -1;
 }
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 1c7ba466f1..0495308318 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -613,7 +613,7 @@ testBackingParse(const void *args)
 VIR_AUTOFREE(char *) xml = NULL;
 VIR_AUTOUNREF(virStorageSourcePtr) src = NULL;

-if (!(src = virStorageSourceNewFromBackingAbsolute(data->backing))) {
+if (virStorageSourceNewFromBackingAbsolute(data->backing, ) < 0) {
 if (!data->expect)
 return 0;
 else
-- 
2.21.0

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

[libvirt] [PATCH 14/14] util: storagefile: Flag backing store strings with authentication

[Peter Krempa]

Using inline authentication for storage volumes will not work properly
as libvirt requires use of the secret driver for the auth data and
thus would not be able to represent the passwords stored in the backing
store string.

Make sure that the backing store parsers return 1 which is a sign for
the caller to not use the file in certain cases.

The test data include iscsi via a json pseudo-protocol string and URIs
with the userinfo part being present.

Signed-off-by: Peter Krempa 
---
 src/util/virstoragefile.c | 11 +--
 tests/virstoragetest.c| 28 
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index efc1d84048..437dcc015d 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2705,8 +2705,6 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src,
 return -1;
 }

-/* XXX We currently don't support auth, so don't bother parsing it */
-
 /* uri->path is NULL if the URI does not contain slash after host:
  * transport://host:port */
 if (uri->path)
@@ -2756,6 +2754,10 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src,
 if (VIR_STRDUP(src->hosts->name, uri->server) < 0)
 return -1;

+/* Libvirt doesn't handle inline authentication. Make the caller aware. */
+if (uri->user)
+return 1;
+
 return 0;
 }

@@ -3311,6 +3313,11 @@ 
virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr src,
 if (virAsprintf(>path, "%s/%s", target, lun) < 0)
 return -1;

+/* Libvirt doesn't handle inline authentication. Make the caller aware. */
+if (virJSONValueObjectGetString(json, "user") ||
+virJSONValueObjectGetString(json, "password"))
+return 1;
+
 return 0;
 }

diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index be5cb98262..1d06abe8b6 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -1260,6 +1260,10 @@ mymain(void)
"\n"
"  \n"
"\n");
+TEST_BACKING_PARSE_FULL("http://user:p...@example.com/file;,
+"\n"
+"  \n"
+"\n", 1);
 TEST_BACKING_PARSE("rbd:testshare:id=asdf:mon_host=example.com",
"\n"
"  \n"
@@ -1280,6 +1284,10 @@ mymain(void)
"\n"
"  \n"
"\n");
+
TEST_BACKING_PARSE_FULL("iscsi://testuser:testp...@example.org:1234/exportname",
+"\n"
+"  \n"
+"\n", 1);

 #ifdef WITH_YAJL
 TEST_BACKING_PARSE("json:", NULL);
@@ -1484,6 +1492,26 @@ mymain(void)
"\n"
"  \n"
"\n");
+TEST_BACKING_PARSE_FULL("json:{\"file\":{\"driver\":\"iscsi\","
+"\"transport\":\"tcp\","
+"\"portal\":\"test.org\","
+"\"user\":\"testuser\","
+
"\"target\":\"iqn.2016-12.com.virttest:emulated-iscsi-auth.target\""
+"}"
+"}",
+   "\n"
+   "  \n"
+   "\n", 1);
+TEST_BACKING_PARSE_FULL("json:{\"file\":{\"driver\":\"iscsi\","
+"\"transport\":\"tcp\","
+"\"portal\":\"test.org\","
+"\"password\":\"testpass\","
+
"\"target\":\"iqn.2016-12.com.virttest:emulated-iscsi-auth.target\""
+"}"
+"}",
+   "\n"
+   "  \n"
+   "\n", 1);
 TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"iscsi\","
"\"transport\":\"tcp\","
"\"portal\":\"test.org:1234\","
-- 
2.21.0

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

[libvirt] [PATCH 12/14] util: storagefile: Clarify docs for '@report_broken' of virStorageFileGetMetadata

[Peter Krempa]

virStorageFileGetMetadata does not report error if we can't interrogate
the file somehow. Clarify this in the description of the @report_broken
flag as it implies we should report an error in that case. The problem
is that we don't know whether there's a problem and unforntnately just
offload it to qemu.

Signed-off-by: Peter Krempa 
---
 src/util/virstoragefile.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index fa0233376b..32afee1ca7 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -4996,7 +4996,8 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
  * other non-raw format at will.
  *
  * If @report_broken is true, the whole function fails with a possibly sane
- * error instead of just returning a broken chain.
+ * error instead of just returning a broken chain. Note that the inability for
+ * libvirt to traverse a given source is not considered an error.
  *
  * Caller MUST free result after use via virObjectUnref.
  */
-- 
2.21.0

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

[libvirt] [PATCH 06/14] tests: storage: Refactor cleanup in testBackingParse

[Peter Krempa]

Automatically clean the temporary buffer and get rid of the cleanup
label.

Signed-off-by: Peter Krempa 
---
 tests/virstoragetest.c | 22 --
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index ef16b3c6e0..1c7ba466f1 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -609,28 +609,27 @@ static int
 testBackingParse(const void *args)
 {
 const struct testBackingParseData *data = args;
-virBuffer buf = VIR_BUFFER_INITIALIZER;
-int ret = -1;
+VIR_AUTOCLEAN(virBuffer) buf = VIR_BUFFER_INITIALIZER;
 VIR_AUTOFREE(char *) xml = NULL;
 VIR_AUTOUNREF(virStorageSourcePtr) src = NULL;

 if (!(src = virStorageSourceNewFromBackingAbsolute(data->backing))) {
 if (!data->expect)
-ret = 0;
-
-goto cleanup;
+return 0;
+else
+return -1;
 }

 if (src && !data->expect) {
 fprintf(stderr, "parsing of backing store string '%s' should "
 "have failed\n", data->backing);
-goto cleanup;
+return -1;
 }

 if (virDomainDiskSourceFormat(, src, "source", 0, false, 0, NULL) < 0 
||
 !(xml = virBufferContentAndReset())) {
 fprintf(stderr, "failed to format disk source xml\n");
-goto cleanup;
+return -1;
 }

 if (STRNEQ(xml, data->expect)) {
@@ -638,15 +637,10 @@ testBackingParse(const void *args)
 "expected storage source xml:\n%s\n"
 "actual storage source xml:\n%s\n",
 data->backing, data->expect, xml);
-goto cleanup;
+return -1;
 }

-ret = 0;
-
- cleanup:
-virBufferFreeAndReset();
-
-return ret;
+return 0;
 }


-- 
2.21.0

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

[libvirt] [PATCH 13/14] util: storagefile: Don't traverse storage sources unusable by VM

[Peter Krempa]

virStorageFileGetMetadataRecurse would include in the backing chain
files which would not really be usable by libvirt directly e.g. when
such file would be promoted to the top layer by a active block commit as
for example inline authentication data can't be represented in the VM
xml file. The idea is to use secrets for this.

With the changes to the backing store string parsers we can report and
propagate if such a thing is present in the configuration and thus start
skipping those files in the backing chain traversal code. This approach
still allows to report the appropriate backing store string in the
storage driver which doesn't directly use the backing file.

Signed-off-by: Peter Krempa 
---
 src/util/virstoragefile.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 32afee1ca7..efc1d84048 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -4941,9 +4941,15 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
 goto cleanup;

 if (src->backingStoreRaw) {
-if (virStorageSourceNewFromBacking(src, ) < 0)
+if ((rv = virStorageSourceNewFromBacking(src, )) < 0)
 goto cleanup;

+if (rv == 1) {
+/* the backing file would not be usable for VM usage */
+ret = 0;
+goto cleanup;
+}
+
 if (backingFormat == VIR_STORAGE_FILE_AUTO)
 backingStore->format = VIR_STORAGE_FILE_RAW;
 else if (backingFormat == VIR_STORAGE_FILE_AUTO_SAFE)
-- 
2.21.0

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

[libvirt] [PATCH 00/14] Ignore backing files with inline authentication (blockdev-add saga)

[Peter Krempa]

If a backing file string contains authentication data, many things can
break as libvirt is not tracking it since we use the secret driver for
this.

Stop considering such files as viable backing store entries.

Peter Krempa (14):
  util: storage: Simplify cleanup path handling in
virStorageSourceParseBackingJSONInternal
  util: storagefile: Remove cleanup label from
virStorageSourceParseBackingJSONiSCSI
  util: storagefile: Simplify cleanup handling in
virStorageSourceParseBackingURI
  util: storagefile: Simplify cleanup in
virStorageSourceParseBackingJSON
  tests: viruri: Add test for password in URI userinfo
  tests: storage: Refactor cleanup in testBackingParse
  util: storage: Modify return value of virStorageSourceNewFromBacking
  util: storagefile: Preserve return value in
virStorageSourceParseBackingJSONUriStr
  util: storagefile: Modify arguments of
virStorageSourceNewFromBackingAbsolue
  tests: virstorage: Allow testing return value of
virStorageSourceNewFromBackingAbsolute
  util: storagefile: Add handling of unusable storage sources
  util: storagefile: Clarify docs for '@report_broken' of
virStorageFileGetMetadata
  util: storagefile: Don't traverse storage sources unusable by VM
  util: storagefile: Flag backing store strings with authentication

 src/storage/storage_util.c |   2 +-
 src/util/virstoragefile.c  | 215 ++---
 src/util/virstoragefile.h  |   7 +-
 tests/qemublocktest.c  |   3 +-
 tests/virstoragetest.c |  66 +---
 tests/viruritest.c |   1 +
 6 files changed, 185 insertions(+), 109 deletions(-)

-- 
2.21.0

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

[libvirt] [PATCH 08/14] util: storagefile: Preserve return value in virStorageSourceParseBackingJSONUriStr

[Peter Krempa]

virStorageSourceParseBackingURI will report special return values in
some cases. Preserve it in virStorageSourceParseBackingJSONUriStr.

Signed-off-by: Peter Krempa 
---
 src/util/virstoragefile.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 4e57a0438e..192a79c025 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -3073,7 +3073,9 @@ 
virStorageSourceParseBackingJSONUriStr(virStorageSourcePtr src,
const char *uri,
int protocol)
 {
-if (virStorageSourceParseBackingURI(src, uri) < 0)
+int rc;
+
+if ((rc = virStorageSourceParseBackingURI(src, uri)) < 0)
 return -1;

 if (src->protocol != protocol) {
@@ -3085,7 +3087,7 @@ 
virStorageSourceParseBackingJSONUriStr(virStorageSourcePtr src,
 return -1;
 }

-return 0;
+return rc;
 }


-- 
2.21.0

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

[libvirt] [PATCH 07/14] util: storage: Modify return value of virStorageSourceNewFromBacking

[Peter Krempa]

Return the storage source definition via a pointer in the arguments and
document the returned values. This will simplify the possibility to
ignore certain backing store types which are not representable by
libvirt.

Signed-off-by: Peter Krempa 
---
 src/storage/storage_util.c |  2 +-
 src/util/virstoragefile.c  | 59 --
 src/util/virstoragefile.h  |  4 ++-
 3 files changed, 41 insertions(+), 24 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 62f857f9ea..24c5918aa3 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -3391,7 +3391,7 @@ storageBackendProbeTarget(virStorageSourcePtr target,
 return -1;

 if (meta->backingStoreRaw) {
-if (!(target->backingStore = virStorageSourceNewFromBacking(meta)))
+if (virStorageSourceNewFromBacking(meta, >backingStore) < 0)
 return -1;

 target->backingStore->format = backingStoreFormat;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 6fff013e3a..4e57a0438e 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -3679,42 +3679,57 @@ virStorageSourceNewFromBackingAbsolute(const char *path)
 }


-virStorageSourcePtr
-virStorageSourceNewFromBacking(virStorageSourcePtr parent)
+/**
+ * virStorageSourceNewFromBacking:
+ * @parent: storage source parent
+ * @backing: returned backing store definition
+ *
+ * Creates a storage source which describes the backing image of @parent and
+ * fills it into @backing depending on the 'backingStoreRaw' property of 
@parent
+ * and other data. Note that for local storage this function interrogates the
+ * actual type of the backing store.
+ *
+ * Returns 0 and fills @backing, or -1 on error (with appropriate error 
reported).
+ */
+int
+virStorageSourceNewFromBacking(virStorageSourcePtr parent,
+   virStorageSourcePtr *backing)
 {
 struct stat st;
-virStorageSourcePtr ret = NULL;
 VIR_AUTOUNREF(virStorageSourcePtr) def = NULL;

+*backing = NULL;
+
 if (virStorageIsRelative(parent->backingStoreRaw))
 def = virStorageSourceNewFromBackingRelative(parent,
  parent->backingStoreRaw);
 else
 def = virStorageSourceNewFromBackingAbsolute(parent->backingStoreRaw);

-if (def) {
-/* possibly update local type */
-if (def->type == VIR_STORAGE_TYPE_FILE) {
-if (stat(def->path, ) == 0) {
-if (S_ISDIR(st.st_mode)) {
-def->type = VIR_STORAGE_TYPE_DIR;
-def->format = VIR_STORAGE_FILE_DIR;
-} else if (S_ISBLK(st.st_mode)) {
-def->type = VIR_STORAGE_TYPE_BLOCK;
-}
+if (!def)
+return -1;
+
+/* possibly update local type */
+if (def->type == VIR_STORAGE_TYPE_FILE) {
+if (stat(def->path, ) == 0) {
+if (S_ISDIR(st.st_mode)) {
+def->type = VIR_STORAGE_TYPE_DIR;
+def->format = VIR_STORAGE_FILE_DIR;
+} else if (S_ISBLK(st.st_mode)) {
+def->type = VIR_STORAGE_TYPE_BLOCK;
 }
 }
+}

-/* copy parent's labelling and other top level stuff */
-if (virStorageSourceInitChainElement(def, parent, true) < 0)
-return NULL;
+/* copy parent's labelling and other top level stuff */
+if (virStorageSourceInitChainElement(def, parent, true) < 0)
+return -1;

-def->readonly = true;
-def->detected = true;
-}
+def->readonly = true;
+def->detected = true;

-VIR_STEAL_PTR(ret, def);
-return ret;
+VIR_STEAL_PTR(*backing, def);
+return 0;
 }


@@ -4899,7 +4914,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
 goto cleanup;

 if (src->backingStoreRaw) {
-if (!(backingStore = virStorageSourceNewFromBacking(src)))
+if (virStorageSourceNewFromBacking(src, ) < 0)
 goto cleanup;

 if (backingFormat == VIR_STORAGE_FILE_AUTO)
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 2882bacf3e..3a72c62ad7 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -444,7 +444,9 @@ int virStorageSourceUpdateCapacity(virStorageSourcePtr src,
char *buf, ssize_t len,
bool probe);

-virStorageSourcePtr virStorageSourceNewFromBacking(virStorageSourcePtr parent);
+int virStorageSourceNewFromBacking(virStorageSourcePtr parent,
+   virStorageSourcePtr *backing);
+
 virStorageSourcePtr virStorageSourceCopy(const virStorageSource *src,
  bool backingChain)
 ATTRIBUTE_NONNULL(1);
-- 
2.21.0

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

[libvirt] [PATCH 02/14] util: storagefile: Remove cleanup label from virStorageSourceParseBackingJSONiSCSI

[Peter Krempa]

There is no cleanup code.

Signed-off-by: Peter Krempa 
---
 src/util/virstoragefile.c | 20 
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 520f531088..8af45bfbd2 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -3263,7 +3263,6 @@ virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr 
src,
 const char *lun = virJSONValueObjectGetStringOrNumber(json, "lun");
 const char *uri;
 char *port;
-int ret = -1;

 /* legacy URI based syntax passed via 'filename' option */
 if ((uri = virJSONValueObjectGetString(json, "filename")))
@@ -3277,14 +3276,14 @@ 
virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr src,
 lun = "0";

 if (VIR_ALLOC(src->hosts) < 0)
-goto cleanup;
+return -1;

 src->nhosts = 1;

 if (STRNEQ_NULLABLE(transport, "tcp")) {
 virReportError(VIR_ERR_INVALID_ARG, "%s",
_("only TCP transport is supported for iSCSI volumes"));
-goto cleanup;
+return -1;
 }

 src->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
@@ -3292,33 +3291,30 @@ 
virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr src,
 if (!portal) {
 virReportError(VIR_ERR_INVALID_ARG, "%s",
_("missing 'portal' address in iSCSI backing 
definition"));
-goto cleanup;
+return -1;
 }

 if (!target) {
 virReportError(VIR_ERR_INVALID_ARG, "%s",
_("missing 'target' in iSCSI backing definition"));
-goto cleanup;
+return -1;
 }

 if (VIR_STRDUP(src->hosts->name, portal) < 0)
-goto cleanup;
+return -1;

 if ((port = strrchr(src->hosts->name, ':')) &&
 !strchr(port, ']')) {
 if (virStringParsePort(port + 1, >hosts->port) < 0)
-goto cleanup;
+return -1;

 *port = '\0';
 }

 if (virAsprintf(>path, "%s/%s", target, lun) < 0)
-goto cleanup;
-
-ret = 0;
+return -1;

- cleanup:
-return ret;
+return 0;
 }


-- 
2.21.0

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

[libvirt] [PATCH 10/14] tests: virstorage: Allow testing return value of virStorageSourceNewFromBackingAbsolute

[Peter Krempa]

Modiy testBackingParse to allow testing other return values of the
backing store string parser.

Signed-off-by: Peter Krempa 
---
 tests/virstoragetest.c | 24 ++--
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 0495308318..be5cb98262 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -603,6 +603,7 @@ testPathRelative(const void *args)
 struct testBackingParseData {
 const char *backing;
 const char *expect;
+int rv;
 };

 static int
@@ -612,14 +613,21 @@ testBackingParse(const void *args)
 VIR_AUTOCLEAN(virBuffer) buf = VIR_BUFFER_INITIALIZER;
 VIR_AUTOFREE(char *) xml = NULL;
 VIR_AUTOUNREF(virStorageSourcePtr) src = NULL;
+int rc;
+int erc = data->rv;

-if (virStorageSourceNewFromBackingAbsolute(data->backing, ) < 0) {
-if (!data->expect)
-return 0;
-else
-return -1;
+/* expect failure return code with NULL expected data */
+if (!data->expect)
+erc = -1;
+
+if ((rc = virStorageSourceNewFromBackingAbsolute(data->backing, )) != 
erc) {
+fprintf(stderr, "expected return value '%d' actual '%d'\n", erc, rc);
+return -1;
 }

+if (!src)
+return 0;
+
 if (src && !data->expect) {
 fprintf(stderr, "parsing of backing store string '%s' should "
 "have failed\n", data->backing);
@@ -1225,15 +1233,19 @@ mymain(void)

 virTestCounterReset("Backing store parse ");

-#define TEST_BACKING_PARSE(bck, xml) \
+#define TEST_BACKING_PARSE_FULL(bck, xml, rc) \
 do { \
 data5.backing = bck; \
 data5.expect = xml; \
+data5.rv = rc; \
 if (virTestRun(virTestCounterNext(), \
testBackingParse, ) < 0) \
 ret = -1; \
 } while (0)

+#define TEST_BACKING_PARSE(bck, xml) \
+TEST_BACKING_PARSE_FULL(bck, xml, 0)
+
 TEST_BACKING_PARSE("path", "\n");
 TEST_BACKING_PARSE("://", NULL);
 TEST_BACKING_PARSE("http://example.com;,
-- 
2.21.0

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

[libvirt] [PATCH 01/14] util: storage: Simplify cleanup path handling in virStorageSourceParseBackingJSONInternal

[Peter Krempa]

Automatically free the intermediate JSON data to get rid of the cleanup
section.

Signed-off-by: Peter Krempa 
---
 src/util/virstoragefile.c | 20 +++-
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index ba56f452e9..520f531088 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -3590,22 +3590,21 @@ static int
 virStorageSourceParseBackingJSONInternal(virStorageSourcePtr src,
  virJSONValuePtr json)
 {
-virJSONValuePtr deflattened = NULL;
+VIR_AUTOPTR(virJSONValue) deflattened = NULL;
 virJSONValuePtr file;
 const char *drvname;
 size_t i;
-int ret = -1;
 VIR_AUTOFREE(char *) str = NULL;

 if (!(deflattened = virJSONValueObjectDeflatten(json)))
-goto cleanup;
+return -1;

 if (!(file = virJSONValueObjectGetObject(deflattened, "file"))) {
 str = virJSONValueToString(json, false);
 virReportError(VIR_ERR_INVALID_ARG,
_("JSON backing volume definition '%s' lacks 'file' 
object"),
NULLSTR(str));
-goto cleanup;
+return -1;
 }

 if (!(drvname = virJSONValueObjectGetString(file, "driver"))) {
@@ -3613,23 +3612,18 @@ 
virStorageSourceParseBackingJSONInternal(virStorageSourcePtr src,
 virReportError(VIR_ERR_INVALID_ARG,
_("JSON backing volume definition '%s' lacks driver 
name"),
NULLSTR(str));
-goto cleanup;
+return -1;
 }

 for (i = 0; i < ARRAY_CARDINALITY(jsonParsers); i++) {
-if (STREQ(drvname, jsonParsers[i].drvname)) {
-ret = jsonParsers[i].func(src, file, jsonParsers[i].opaque);
-goto cleanup;
-}
+if (STREQ(drvname, jsonParsers[i].drvname))
+return jsonParsers[i].func(src, file, jsonParsers[i].opaque);
 }

 virReportError(VIR_ERR_INTERNAL_ERROR,
_("missing parser implementation for JSON backing volume "
  "driver '%s'"), drvname);
-
- cleanup:
-virJSONValueFree(deflattened);
-return ret;
+return -1;
 }


-- 
2.21.0

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

[libvirt] [PATCH 05/10] ci: Move source directory under $(CI_USER_HOME)

[Andrea Bolognani]

Now that we have a home directory for the user, storing the
source there rather than in a custom top-level directory is
the obvious choice.

Later on we're also going to add some more files related to
builds, and storing everything in the user's home directory
will keep things nice and tidy.

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

diff --git a/ci/Makefile b/ci/Makefile
index f52a0bf621..dc7ee6c037 100644
--- a/ci/Makefile
+++ b/ci/Makefile
@@ -18,7 +18,7 @@ CI_HOST_SRCDIR = $(CI_SCRATCHDIR)/src
 # The directory holding the source inside the
 # container, i.e. where we want to expose
 # the $(CI_HOST_SRCDIR) directory from the host
-CI_CONT_SRCDIR = /src
+CI_CONT_SRCDIR = $(CI_USER_HOME)/libvirt
 
 # Relative directory to perform the build in. This
 # defaults to using a separate build dir, but can be
-- 
2.21.0

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

[libvirt] [PATCH 09/10] ci: Run $(CI_PREPARE_SCRIPT) as root

[Andrea Bolognani]

In order for the prepare script to be really useful, it needs
to be able to perform privileged operations such as installing
additional packages or setting up custom mount points.

In order to achieve that, we now run the container as root,
run the prepare script with full privilege, and only then
switch to the unprivileged account with sudo.

Signed-off-by: Andrea Bolognani 
---
 ci/Makefile   | 19 +++
 ci/prepare.sh |  4 
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/ci/Makefile b/ci/Makefile
index 17d85d407f..86a07049ac 100644
--- a/ci/Makefile
+++ b/ci/Makefile
@@ -174,7 +174,6 @@ CI_GIT_ARGS = \
 #   --tty Ensure we have ability to Ctrl-C the build
 CI_ENGINE_ARGS = \
--rm \
-   --user $(CI_UID):$(CI_GID) \
--interactive \
--tty \
$(CI_PODMAN_ARGS) \
@@ -215,13 +214,17 @@ ci-run-command@%: ci-prepare-tree
$(CI_ENGINE) run $(CI_ENGINE_ARGS) $(CI_IMAGE_PREFIX)$*$(CI_IMAGE_TAG) \
/bin/bash -c ' \
$(CI_USER_HOME)/prepare || exit 1; \
-   export CI_CONT_SRCDIR="$(CI_CONT_SRCDIR)"; \
-   export CI_CONT_BUILDDIR="$(CI_CONT_BUILDDIR)"; \
-   export CI_SMP="$(CI_SMP)"; \
-   export CI_CONFIGURE="$(CI_CONFIGURE)"; \
-   export CI_CONFIGURE_ARGS="$(CI_CONFIGURE_ARGS)"; \
-   export CI_MAKE_ARGS="$(CI_MAKE_ARGS)"; \
-   $(CI_COMMAND) || exit 1'
+   sudo \
+ --login \
+ --user="#$(CI_UID)" \
+ --group="#$(CI_GID)" \
+ CI_CONT_SRCDIR="$(CI_CONT_SRCDIR)" \
+ CI_CONT_BUILDDIR="$(CI_CONT_BUILDDIR)" \
+ CI_SMP="$(CI_SMP)" \
+ CI_CONFIGURE="$(CI_CONFIGURE)" \
+ CI_CONFIGURE_ARGS="$(CI_CONFIGURE_ARGS)" \
+ CI_MAKE_ARGS="$(CI_MAKE_ARGS)" \
+ $(CI_COMMAND) || exit 1'
@test "$(CI_CLEAN)" = "1" && rm -rf $(CI_SCRATCHDIR) || :
 
 ci-shell@%:
diff --git a/ci/prepare.sh b/ci/prepare.sh
index f70107bd62..da6fc9a1b5 100644
--- a/ci/prepare.sh
+++ b/ci/prepare.sh
@@ -7,3 +7,7 @@
 #  CI_PREPARE_SCRIPT=/path/to/your/prepare/script
 #
 # to make.
+#
+# Note that this script will have root privileges inside the
+# container, so it can be used for things like installing additional
+# packages.
-- 
2.21.0

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

[libvirt] [PATCH 07/10] ci: Generalize running commands inside the container

[Andrea Bolognani]

Both for ci-build and ci-shell we want to execute basically
the same setup and cleanup logic, the only difference being
that for the former we then run the build script and with the
latter a shell.

Rework the targets so that they both call the generic
ci-run-command rule passing an appropriate $(CI_COMMAND).

Signed-off-by: Andrea Bolognani 
---
 ci/Makefile | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/ci/Makefile b/ci/Makefile
index f7e31b4f97..2b5be97238 100644
--- a/ci/Makefile
+++ b/ci/Makefile
@@ -206,7 +206,7 @@ ci-prepare-tree: ci-check-engine
done ; \
fi
 
-ci-build@%: ci-prepare-tree
+ci-run-command@%: ci-prepare-tree
$(CI_ENGINE) run $(CI_ENGINE_ARGS) $(CI_IMAGE_PREFIX)$*$(CI_IMAGE_TAG) \
/bin/bash -c ' \
export CI_CONT_SRCDIR="$(CI_CONT_SRCDIR)"; \
@@ -215,16 +215,18 @@ ci-build@%: ci-prepare-tree
export CI_CONFIGURE="$(CI_CONFIGURE)"; \
export CI_CONFIGURE_ARGS="$(CI_CONFIGURE_ARGS)"; \
export CI_MAKE_ARGS="$(CI_MAKE_ARGS)"; \
-   $(CI_USER_HOME)/build || exit 1'
+   $(CI_COMMAND) || exit 1'
@test "$(CI_CLEAN)" = "1" && rm -rf $(CI_SCRATCHDIR) || :
 
+ci-shell@%:
+   $(MAKE) -C $(CI_ROOTDIR) ci-run-command@$* CI_COMMAND="/bin/bash"
+
+ci-build@%:
+   $(MAKE) -C $(CI_ROOTDIR) ci-run-command@$* 
CI_COMMAND="$(CI_USER_HOME)/build"
+
 ci-check@%:
$(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_MAKE_ARGS="check"
 
-ci-shell@%: ci-prepare-tree
-   $(CI_ENGINE) run $(CI_ENGINE_ARGS) $(CI_IMAGE_PREFIX)$*$(CI_IMAGE_TAG) 
/bin/bash
-   @test "$(CI_CLEAN)" = "1" && rm -rf $(CI_SCRATCHDIR) || :
-
 ci-help:
@echo "Build libvirt inside containers used for CI"
@echo
-- 
2.21.0

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

[libvirt] [PATCH 10/10] ci: Stop using --workdir

[Andrea Bolognani]

Now that we're using sudo, the initial work directory is no
longer relevant since the user will find themselves in their
home directory when they get control anyway.

Signed-off-by: Andrea Bolognani 
---
 ci/Makefile | 2 --
 1 file changed, 2 deletions(-)

diff --git a/ci/Makefile b/ci/Makefile
index 86a07049ac..35a35d6082 100644
--- a/ci/Makefile
+++ b/ci/Makefile
@@ -168,7 +168,6 @@ CI_GIT_ARGS = \
 # as dev so that file ownership matches host
 # instead of root:root
 #   --volume  to pass in the cloned git repo & config
-#   --workdir to set cwd to vpath build location
 #   --ulimit  lower files limit for performance reasons
 #   --interactive
 #   --tty Ensure we have ability to Ctrl-C the build
@@ -181,7 +180,6 @@ CI_ENGINE_ARGS = \
$(CI_HOME_MOUNTS) \
$(CI_SCRIPT_MOUNTS) \
--volume $(CI_HOST_SRCDIR):$(CI_CONT_SRCDIR):z \
-   --workdir $(CI_CONT_SRCDIR) \
--ulimit nofile=$(CI_ULIMIT_FILES):$(CI_ULIMIT_FILES) \
--cap-add=SYS_PTRACE \
$(NULL)
-- 
2.21.0

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

[libvirt] [PATCH 03/10] ci: Move everything to a separate directory

[Andrea Bolognani]

We're going to have a few more CI-related files in a second, and
it makes sense to have a separate directory for them rather than
littering the root directory.

$(CI_SCRATCHDIR) can now also be created inside the CI directory,
and as a bonus the make rune necessary to start CI builds without
running configure first becomes shorter.

Signed-off-by: Andrea Bolognani 
---
 .gitignore |  2 +-
 .travis.yml|  8 
 Makefile.am|  4 ++--
 Makefile.ci => ci/Makefile | 17 -
 4 files changed, 15 insertions(+), 16 deletions(-)
 rename Makefile.ci => ci/Makefile (96%)

diff --git a/.gitignore b/.gitignore
index dd5d35c762..82495e8692 100644
--- a/.gitignore
+++ b/.gitignore
@@ -44,7 +44,7 @@
 /autom4te.cache
 /build-aux/*
 /build/
-/ci-tree/
+/ci/scratch/
 /confdefs.h
 /config.cache
 /config.guess
diff --git a/.travis.yml b/.travis.yml
index b510c81083..db573fd496 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -22,28 +22,28 @@ matrix:
 - IMAGE="ubuntu-18"
 - MAKE_ARGS="syntax-check distcheck"
   script:
-- make -f Makefile.ci ci-build@$IMAGE CI_MAKE_ARGS="$MAKE_ARGS"
+- make -C ci/ ci-build@$IMAGE CI_MAKE_ARGS="$MAKE_ARGS"
 - services:
 - docker
   env:
 - IMAGE="centos-7"
 - MAKE_ARGS="syntax-check distcheck"
   script:
-- make -f Makefile.ci ci-build@$IMAGE CI_MAKE_ARGS="$MAKE_ARGS"
+- make -C ci/ ci-build@$IMAGE CI_MAKE_ARGS="$MAKE_ARGS"
 - services:
 - docker
   env:
 - IMAGE="fedora-rawhide"
 - MINGW="mingw32"
   script:
-- make -f Makefile.ci ci-build@$IMAGE CI_CONFIGURE="$MINGW-configure"
+- make -C ci/ ci-build@$IMAGE CI_CONFIGURE="$MINGW-configure"
 - services:
 - docker
   env:
 - IMAGE="fedora-rawhide"
 - MINGW="mingw64"
   script:
-- make -f Makefile.ci ci-build@$IMAGE CI_CONFIGURE="$MINGW-configure"
+- make -C ci/ ci-build@$IMAGE CI_CONFIGURE="$MINGW-configure"
 - compiler: clang
   language: c
   os: osx
diff --git a/Makefile.am b/Makefile.am
index 27c49280c4..b743b4b08b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -35,7 +35,6 @@ EXTRA_DIST = \
   libvirt-qemu.pc.in \
   libvirt-lxc.pc.in \
   libvirt-admin.pc.in \
-  Makefile.ci \
   Makefile.nonreentrant \
   autogen.sh \
   cfg.mk \
@@ -51,6 +50,7 @@ EXTRA_DIST = \
   build-aux/prohibit-duplicate-header.pl \
   build-aux/useless-if-before-free \
   build-aux/vc-list-files \
+  ci/Makefile \
   $(NULL)
 
 pkgconfigdir = $(libdir)/pkgconfig
@@ -123,4 +123,4 @@ gen-AUTHORS:
fi
 
 ci-%:
-   $(MAKE) -f Makefile.ci $@
+   $(MAKE) -C ci/ $@
diff --git a/Makefile.ci b/ci/Makefile
similarity index 96%
rename from Makefile.ci
rename to ci/Makefile
index 86e936aef8..350eb636cd 100644
--- a/Makefile.ci
+++ b/ci/Makefile
@@ -1,16 +1,15 @@
 # -*- makefile -*-
 # vim: filetype=make
 
-# Figure out name and path to this file. This isn't
-# portable but we only care for modern GNU make
-CI_MAKEFILE = $(abspath $(firstword $(MAKEFILE_LIST)))
+# The root directory of the libvirt.git checkout
+CI_GIT_ROOT = $(shell git rev-parse --show-toplevel)
+
+# The root directory for all CI-related contents
+CI_ROOTDIR = $(CI_GIT_ROOT)/ci
 
 # The directory holding content on the host that we will
 # expose to the container.
-CI_SCRATCHDIR = $(shell pwd)/ci-tree
-
-# The root directory of the libvirt.git checkout
-CI_GIT_ROOT = $(shell git rev-parse --show-toplevel)
+CI_SCRATCHDIR = $(CI_ROOTDIR)/scratch
 
 # The directory holding the clone of the git repo that
 # we will expose to the container
@@ -178,7 +177,7 @@ ci-prepare-tree: ci-check-engine
cp /etc/group $(CI_SCRATCHDIR); \
echo "Cloning $(CI_GIT_ROOT) to $(CI_HOST_SRCDIR)"; \
git clone $(CI_GIT_ARGS) $(CI_GIT_ROOT) $(CI_HOST_SRCDIR) || 
exit 1; \
-   for mod in $$(git submodule | awk '{ print $$2 }') ; \
+   for mod in $$(git submodule | awk '{ print $$2 }' | sed -E 
's,^../,,g') ; \
do \
test -f $(CI_GIT_ROOT)/$$mod/.git || continue ; \
echo "Cloning $(CI_GIT_ROOT)/$$mod to 
$(CI_HOST_SRCDIR)/$$mod"; \
@@ -221,7 +220,7 @@ ci-build@%: ci-prepare-tree
@test "$(CI_CLEAN)" = "1" && rm -rf $(CI_SCRATCHDIR) || :
 
 ci-check@%:
-   $(MAKE) -f $(CI_MAKEFILE) ci-build@$* CI_MAKE_ARGS="check"
+   $(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_MAKE_ARGS="check"
 
 ci-shell@%: ci-prepare-tree
$(CI_ENGINE) run $(CI_ENGINE_ARGS) $(CI_IMAGE_PREFIX)$*$(CI_IMAGE_TAG) 
/bin/bash
-- 
2.21.0

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

[libvirt] [PATCH 01/10] ci: Fix /etc/sub{u,g}id parsing

[Andrea Bolognani]

The $ needs to be escaped when calling shell code from a
Makefile.

Signed-off-by: Andrea Bolognani 
---
 Makefile.ci | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile.ci b/Makefile.ci
index 977e0445c6..14d595a00f 100644
--- a/Makefile.ci
+++ b/Makefile.ci
@@ -114,8 +114,8 @@ ifeq ($(CI_ENGINE),podman)
# need to be higher, but that only happens when your /etc/sub{u,g}id 
allow
# users to have more IDs.  Unless --keep-uid is supported, let's do 
this in a
# way that should work for everyone.
-   CI_MAX_UID = $(shell sed -n "s/^$USER:[^:]\+://p" /etc/subuid)
-   CI_MAX_GID = $(shell sed -n "s/^$USER:[^:]\+://p" /etc/subgid)
+   CI_MAX_UID = $(shell sed -n "s/^$$USER:[^:]\+://p" /etc/subuid)
+   CI_MAX_GID = $(shell sed -n "s/^$$USER:[^:]\+://p" /etc/subgid)
ifeq ($(CI_MAX_UID),)
CI_MAX_UID = 65536
endif
-- 
2.21.0

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

[libvirt] [PATCH 06/10] ci: Introduce $(CI_BUILD_SCRIPT)

[Andrea Bolognani]

Instead of hardcoding build instructions into the Makefile,
move them to a separate script that's mounted into the
container.

This gives us a couple of advantages: we no longer have to
deal with the awkward quoting required when embedding shell
code in a Makefile, and we also provide the users with a way
to override the default build instructions with their own.

Signed-off-by: Andrea Bolognani 
---
 Makefile.am |  1 +
 ci/Makefile | 48 ++--
 ci/build.sh | 40 
 3 files changed, 59 insertions(+), 30 deletions(-)
 create mode 100644 ci/build.sh

diff --git a/Makefile.am b/Makefile.am
index b743b4b08b..96fac92186 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -51,6 +51,7 @@ EXTRA_DIST = \
   build-aux/useless-if-before-free \
   build-aux/vc-list-files \
   ci/Makefile \
+  ci/build.sh \
   $(NULL)
 
 pkgconfigdir = $(libdir)/pkgconfig
diff --git a/ci/Makefile b/ci/Makefile
index dc7ee6c037..f7e31b4f97 100644
--- a/ci/Makefile
+++ b/ci/Makefile
@@ -41,6 +41,9 @@ CI_MAKE_ARGS =
 # Any extra arguments to pass to configure
 CI_CONFIGURE_ARGS =
 
+# Script containing build instructions
+CI_BUILD_SCRIPT = $(CI_ROOTDIR)/build.sh
+
 # Location of the container images we're going to pull
 # Can be useful to overridde to use a locally built
 # image instead
@@ -96,6 +99,10 @@ CI_HOME_MOUNTS = \
--volume $(CI_SCRATCHDIR)/home:$(CI_USER_HOME):z \
$(NULL)
 
+CI_SCRIPT_MOUNTS = \
+   --volume $(CI_SCRATCHDIR)/build:$(CI_USER_HOME)/build:z \
+   $(NULL)
+
 # Docker containers can have very large ulimits
 # for nofiles - as much as 1048576. This makes
 # libvirt very slow at exec'ing programs.
@@ -169,6 +176,7 @@ CI_ENGINE_ARGS = \
$(CI_PODMAN_ARGS) \
$(CI_PWDB_MOUNTS) \
$(CI_HOME_MOUNTS) \
+   $(CI_SCRIPT_MOUNTS) \
--volume $(CI_HOST_SRCDIR):$(CI_CONT_SRCDIR):z \
--workdir $(CI_CONT_SRCDIR) \
--ulimit nofile=$(CI_ULIMIT_FILES):$(CI_ULIMIT_FILES) \
@@ -186,6 +194,8 @@ ci-prepare-tree: ci-check-engine
cp /etc/passwd $(CI_SCRATCHDIR); \
cp /etc/group $(CI_SCRATCHDIR); \
mkdir -p $(CI_SCRATCHDIR)/home; \
+   cp "$(CI_BUILD_SCRIPT)" $(CI_SCRATCHDIR)/build; \
+   chmod +x "$(CI_SCRATCHDIR)/build"; \
echo "Cloning $(CI_GIT_ROOT) to $(CI_HOST_SRCDIR)"; \
git clone $(CI_GIT_ARGS) $(CI_GIT_ROOT) $(CI_HOST_SRCDIR) || 
exit 1; \
for mod in $$(git submodule | awk '{ print $$2 }' | sed -E 
's,^../,,g') ; \
@@ -196,38 +206,16 @@ ci-prepare-tree: ci-check-engine
done ; \
fi
 
-# $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
-# be to pass --host/--target args to trigger cross-compilation
-#
-# This can be augmented by make local args in $(CI_CONFIGURE_ARGS)
-#
-# gl_public_submodule_commit= to disable gnulib's submodule check
-# which breaks due to way we clone the submodules
 ci-build@%: ci-prepare-tree
$(CI_ENGINE) run $(CI_ENGINE_ARGS) $(CI_IMAGE_PREFIX)$*$(CI_IMAGE_TAG) \
-   /bin/bash -c '\
-   mkdir -p $(CI_CONT_BUILDDIR) || exit 1 ; \
-   cd $(CI_CONT_BUILDDIR) ; \
-   NOCONFIGURE=1 $(CI_CONT_SRCDIR)/autogen.sh || exit 1 ; \
-   $(CI_CONFIGURE) $${CONFIGURE_OPTS} $(CI_CONFIGURE_ARGS) ; \
-   if test $$? != 0 ; \
-   then \
-   test -f config.log && cat config.log ; \
-   exit 1 ; \
-   fi; \
-   find -name test-suite.log -delete ; \
-   export VIR_TEST_DEBUG=1 ; \
-   make -j$(CI_SMP) gl_public_submodule_commit= $(CI_MAKE_ARGS) ; \
-   if test $$? != 0 ; then \
-   LOGS=`find -name test-suite.log` ; \
-   if test "$${LOGS}" != "" ; then \
-   echo "=== LOG FILE(S) START ===" ; \
-   cat $${LOGS} ; \
-   echo "=== LOG FILE(S) END ===" ; \
-   fi ; \
-   exit 1 ;\
-   fi'
+   /bin/bash -c ' \
+   export CI_CONT_SRCDIR="$(CI_CONT_SRCDIR)"; \
+   export CI_CONT_BUILDDIR="$(CI_CONT_BUILDDIR)"; \
+   export CI_SMP="$(CI_SMP)"; \
+   export CI_CONFIGURE="$(CI_CONFIGURE)"; \
+   export CI_CONFIGURE_ARGS="$(CI_CONFIGURE_ARGS)"; \
+   export CI_MAKE_ARGS="$(CI_MAKE_ARGS)"; \
+   $(CI_USER_HOME)/build || exit 1'
@test "$(CI_CLEAN)" = "1" && rm -rf $(CI_SCRATCHDIR) || :
 
 ci-check@%:
diff --git a/ci/build.sh b/ci/build.sh
new file mode 100644
index 00..0874c2d1d9
--- /dev/null
+++ b/ci/build.sh
@@ -0,0 +1,40 @@
+# This script is used to build libvirt inside the container.
+#
+# 

[libvirt] [PATCH 08/10] ci: Introduce $(CI_PREPARE_SCRIPT)

[Andrea Bolognani]

This script is run before $(CI_BUILD_SCRIPT) and can be used
to tweak the environment as necessary before the build starts.

Signed-off-by: Andrea Bolognani 
---
 Makefile.am   | 1 +
 ci/Makefile   | 8 +++-
 ci/prepare.sh | 9 +
 3 files changed, 17 insertions(+), 1 deletion(-)
 create mode 100644 ci/prepare.sh

diff --git a/Makefile.am b/Makefile.am
index 96fac92186..cf9ff94f4f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -52,6 +52,7 @@ EXTRA_DIST = \
   build-aux/vc-list-files \
   ci/Makefile \
   ci/build.sh \
+  ci/prepare.sh \
   $(NULL)
 
 pkgconfigdir = $(libdir)/pkgconfig
diff --git a/ci/Makefile b/ci/Makefile
index 2b5be97238..17d85d407f 100644
--- a/ci/Makefile
+++ b/ci/Makefile
@@ -41,6 +41,9 @@ CI_MAKE_ARGS =
 # Any extra arguments to pass to configure
 CI_CONFIGURE_ARGS =
 
+# Script containing environment preparation steps
+CI_PREPARE_SCRIPT = $(CI_ROOTDIR)/prepare.sh
+
 # Script containing build instructions
 CI_BUILD_SCRIPT = $(CI_ROOTDIR)/build.sh
 
@@ -100,6 +103,7 @@ CI_HOME_MOUNTS = \
$(NULL)
 
 CI_SCRIPT_MOUNTS = \
+   --volume $(CI_SCRATCHDIR)/prepare:$(CI_USER_HOME)/prepare:z \
--volume $(CI_SCRATCHDIR)/build:$(CI_USER_HOME)/build:z \
$(NULL)
 
@@ -194,8 +198,9 @@ ci-prepare-tree: ci-check-engine
cp /etc/passwd $(CI_SCRATCHDIR); \
cp /etc/group $(CI_SCRATCHDIR); \
mkdir -p $(CI_SCRATCHDIR)/home; \
+   cp "$(CI_PREPARE_SCRIPT)" $(CI_SCRATCHDIR)/prepare; \
cp "$(CI_BUILD_SCRIPT)" $(CI_SCRATCHDIR)/build; \
-   chmod +x "$(CI_SCRATCHDIR)/build"; \
+   chmod +x "$(CI_SCRATCHDIR)/prepare" "$(CI_SCRATCHDIR)/build"; \
echo "Cloning $(CI_GIT_ROOT) to $(CI_HOST_SRCDIR)"; \
git clone $(CI_GIT_ARGS) $(CI_GIT_ROOT) $(CI_HOST_SRCDIR) || 
exit 1; \
for mod in $$(git submodule | awk '{ print $$2 }' | sed -E 
's,^../,,g') ; \
@@ -209,6 +214,7 @@ ci-prepare-tree: ci-check-engine
 ci-run-command@%: ci-prepare-tree
$(CI_ENGINE) run $(CI_ENGINE_ARGS) $(CI_IMAGE_PREFIX)$*$(CI_IMAGE_TAG) \
/bin/bash -c ' \
+   $(CI_USER_HOME)/prepare || exit 1; \
export CI_CONT_SRCDIR="$(CI_CONT_SRCDIR)"; \
export CI_CONT_BUILDDIR="$(CI_CONT_BUILDDIR)"; \
export CI_SMP="$(CI_SMP)"; \
diff --git a/ci/prepare.sh b/ci/prepare.sh
new file mode 100644
index 00..f70107bd62
--- /dev/null
+++ b/ci/prepare.sh
@@ -0,0 +1,9 @@
+# This script is used to prepare the environment that will be used
+# to build libvirt inside the container.
+#
+# You can customize it to your liking, or alternatively use a
+# completely different script by passing
+#
+#  CI_PREPARE_SCRIPT=/path/to/your/prepare/script
+#
+# to make.
-- 
2.21.0

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

[libvirt] [PATCH 02/10] ci: Drop $(CI_SUBMODULES)

[Andrea Bolognani]

We only use the list of submodules once, so no need to
store it in a variable.

Signed-off-by: Andrea Bolognani 
---
 Makefile.ci | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/Makefile.ci b/Makefile.ci
index 14d595a00f..86e936aef8 100644
--- a/Makefile.ci
+++ b/Makefile.ci
@@ -42,10 +42,6 @@ CI_MAKE_ARGS =
 # Any extra arguments to pass to configure
 CI_CONFIGURE_ARGS =
 
-# Avoid pulling submodules over the network by locally
-# cloning them
-CI_SUBMODULES = $(shell git submodule | awk '{ print $$2 }')
-
 # Location of the container images we're going to pull
 # Can be useful to overridde to use a locally built
 # image instead
@@ -182,7 +178,7 @@ ci-prepare-tree: ci-check-engine
cp /etc/group $(CI_SCRATCHDIR); \
echo "Cloning $(CI_GIT_ROOT) to $(CI_HOST_SRCDIR)"; \
git clone $(CI_GIT_ARGS) $(CI_GIT_ROOT) $(CI_HOST_SRCDIR) || 
exit 1; \
-   for mod in $(CI_SUBMODULES) ; \
+   for mod in $$(git submodule | awk '{ print $$2 }') ; \
do \
test -f $(CI_GIT_ROOT)/$$mod/.git || continue ; \
echo "Cloning $(CI_GIT_ROOT)/$$mod to 
$(CI_HOST_SRCDIR)/$$mod"; \
-- 
2.21.0

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

[libvirt] [PATCH 00/10] ci: Several fixes and improvements

[Andrea Bolognani]

See the individual commits for details, but the gist of it is that
after this series it's possible for users to hook into the build
process and customize it according to their needs; on top of that,
the whole thing is made more maintainable in the process.

Andrea Bolognani (10):
  ci: Fix /etc/sub{u,g}id parsing
  ci: Drop $(CI_SUBMODULES)
  ci: Move everything to a separate directory
  ci: Create user's home directory in the container
  ci: Move source directory under $(CI_USER_HOME)
  ci: Introduce $(CI_BUILD_SCRIPT)
  ci: Generalize running commands inside the container
  ci: Introduce $(CI_PREPARE_SCRIPT)
  ci: Run $(CI_PREPARE_SCRIPT) as root
  ci: Stop using --workdir

 .gitignore |   2 +-
 .travis.yml|   8 +--
 Makefile.am|   6 +-
 Makefile.ci => ci/Makefile | 109 +++--
 ci/build.sh|  40 ++
 ci/prepare.sh  |  13 +
 6 files changed, 118 insertions(+), 60 deletions(-)
 rename Makefile.ci => ci/Makefile (79%)
 create mode 100644 ci/build.sh
 create mode 100644 ci/prepare.sh

-- 
2.21.0

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

[libvirt] [PATCH 04/10] ci: Create user's home directory in the container

[Andrea Bolognani]

Some applications expect the user's home directory to be
present on the system and require workarounds when that's not
the case. Creating the home directory along with everything
else is easy enough for us, so let's just do that.

Signed-off-by: Andrea Bolognani 
---
 ci/Makefile | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/ci/Makefile b/ci/Makefile
index 350eb636cd..f52a0bf621 100644
--- a/ci/Makefile
+++ b/ci/Makefile
@@ -65,6 +65,11 @@ CI_REUSE = 0
 CI_UID = $(shell id -u)
 CI_GID = $(shell id -g)
 
+# We also need the user's login and home directory to prepare the
+# environment the way some programs expect it
+CI_USER_LOGIN = $(shell echo "$$USER")
+CI_USER_HOME = $(shell echo "$$HOME")
+
 CI_ENGINE = auto
 # Container engine we are going to use, can be overridden per make
 # invocation, if it is not we try podman and then default to docker.
@@ -87,6 +92,10 @@ CI_PWDB_MOUNTS = \
--volume $(CI_SCRATCHDIR)/passwd:/etc/passwd:ro,z \
$(NULL)
 
+CI_HOME_MOUNTS = \
+   --volume $(CI_SCRATCHDIR)/home:$(CI_USER_HOME):z \
+   $(NULL)
+
 # Docker containers can have very large ulimits
 # for nofiles - as much as 1048576. This makes
 # libvirt very slow at exec'ing programs.
@@ -109,8 +118,8 @@ ifeq ($(CI_ENGINE),podman)
# need to be higher, but that only happens when your /etc/sub{u,g}id 
allow
# users to have more IDs.  Unless --keep-uid is supported, let's do 
this in a
# way that should work for everyone.
-   CI_MAX_UID = $(shell sed -n "s/^$$USER:[^:]\+://p" /etc/subuid)
-   CI_MAX_GID = $(shell sed -n "s/^$$USER:[^:]\+://p" /etc/subgid)
+   CI_MAX_UID = $(shell sed -n "s/^$(CI_USER_LOGIN):[^:]\+://p" 
/etc/subuid)
+   CI_MAX_GID = $(shell sed -n "s/^$(CI_USER_LOGIN):[^:]\+://p" 
/etc/subgid)
ifeq ($(CI_MAX_UID),)
CI_MAX_UID = 65536
endif
@@ -159,6 +168,7 @@ CI_ENGINE_ARGS = \
--tty \
$(CI_PODMAN_ARGS) \
$(CI_PWDB_MOUNTS) \
+   $(CI_HOME_MOUNTS) \
--volume $(CI_HOST_SRCDIR):$(CI_CONT_SRCDIR):z \
--workdir $(CI_CONT_SRCDIR) \
--ulimit nofile=$(CI_ULIMIT_FILES):$(CI_ULIMIT_FILES) \
@@ -175,6 +185,7 @@ ci-prepare-tree: ci-check-engine
mkdir -p $(CI_SCRATCHDIR); \
cp /etc/passwd $(CI_SCRATCHDIR); \
cp /etc/group $(CI_SCRATCHDIR); \
+   mkdir -p $(CI_SCRATCHDIR)/home; \
echo "Cloning $(CI_GIT_ROOT) to $(CI_HOST_SRCDIR)"; \
git clone $(CI_GIT_ARGS) $(CI_GIT_ROOT) $(CI_HOST_SRCDIR) || 
exit 1; \
for mod in $$(git submodule | awk '{ print $$2 }' | sed -E 
's,^../,,g') ; \
-- 
2.21.0

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

[libvirt] [PATCH] virDomainGetAutostart takes a pointer that it writes the output value to

[Sage Imel]

---
 src/domain.rs | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/domain.rs b/src/domain.rs
index 11ecb3c..acb9e6e 100644
--- a/src/domain.rs
+++ b/src/domain.rs
@@ -136,7 +136,7 @@ extern "C" {
 fn virDomainGetHostname(ptr: sys::virDomainPtr, flags: libc::c_uint) -> 
*mut libc::c_char;
 fn virDomainGetUUIDString(ptr: sys::virDomainPtr, uuid: *mut libc::c_char) 
-> libc::c_int;
 fn virDomainGetXMLDesc(ptr: sys::virDomainPtr, flags: libc::c_uint) -> 
*mut libc::c_char;
-fn virDomainGetAutostart(ptr: sys::virDomainPtr) -> libc::c_int;
+fn virDomainGetAutostart(ptr: sys::virDomainPtr, autostart: *mut 
libc::c_int) -> libc::c_int;
 fn virDomainSetAutostart(ptr: sys::virDomainPtr, autostart: libc::c_uint) 
-> libc::c_int;
 fn virDomainGetID(ptr: sys::virDomainPtr) -> libc::c_uint;
 fn virDomainSetMaxMemory(ptr: sys::virDomainPtr, memory: libc::c_ulong) -> 
libc::c_int;
@@ -1036,11 +1036,12 @@ impl Domain {
 
 pub fn get_autostart() -> Result {
 unsafe {
-let ret = virDomainGetAutostart(self.as_ptr());
+let mut autostart: libc::c_int = 0;
+let ret = virDomainGetAutostart(self.as_ptr(),  autostart);
 if ret == -1 {
 return Err(Error::new());
 }
-return Ok(ret == 1);
+return Ok(autostart == 1);
 }
 }
 
-- 
2.17.1

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

Re: [libvirt] [PATCH 00/18] Enhance virpcimock and test VFIO instead of KVM

[Michal Privoznik]

On 8/14/19 5:14 PM, Daniel Henrique Barboza wrote:

I've run make check with each individual patch, and everything
seems fine in my environment.

For all patches:

Tested-by: Daniel Henrique Barboza 


I'll see if I can drop some code reviews later on.


That'd be great because we are lacking reviewers (just like other 
projects) and these are very specific.


Thanks,
Michal

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

Re: [libvirt] [PATCH 15/18] virhostdev: Unify virHostdevPreparePCIDevices behaviour for KVM and VFIO cases

[Michal Privoznik]

On 8/14/19 6:14 PM, Daniel Henrique Barboza wrote:

Michal, I believe the problem you're trying to fix in this patch
is somehow the same I'm trying to fix in this patch here:


https://www.redhat.com/archives/libvir-list/2019-June/msg01270.html

Do you mind taking a look? If that's the case, I can drop that patch
from the series that implements Multifunction PCI hotplug.


Oh, I haven't taken a look on them yet. Yes, this patch fixes the same 
problem. If you're okay with it, I'd rather go with my approach since 
it's simpler.


Michal

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

Re: [libvirt] [PATCH] virpci:fix Secondary Bus Reset bug

[Michal Privoznik]

On 8/15/19 11:44 AM, hexin900...@163.com wrote:

From: hexin 

The parent bridge configuration of the current device
should be read and reset, instead of reading the current
device configuration.

Signed-off-by: He Xin 
Signed-off-by: Liu Qi 
Signed-off-by: Zhang Yu 
---
  src/util/virpci.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Michal Privoznik 

And pushed. Nice catch.

Michal

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

Re: [libvirt] [PATCH 0/2] network: fix crash during cleanup from failure to allocate port

[Michal Privoznik]

On 8/16/19 4:41 AM, Laine Stump wrote:

The first patch fixes the bug. The 2nd patch just updates some code
that I noticed while fixing the bug (because I figured someone would
whine that I was just moving around calls to outdated APIs).

Laine Stump (2):
   network: fix crash during cleanup from failure to allocate port
   network: replace virSaveLastError() with virErrorPreserveLast()

  src/network/bridge_driver.c | 23 +++
  1 file changed, 11 insertions(+), 12 deletions(-)



Reviewed-by: Michal Privoznik 

Michal

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

Re: [libvirt] [PATCH] access: fix incorrect addition to virAccessPermNetwork

[Michal Privoznik]

On 8/15/19 10:42 PM, Laine Stump wrote:

Commit e69444e17 (first appeared in libvirt-5.5.0) added the new value
"VIR_ACCESS_PERM_NETWORK_SEARCH_PORTS" to the virAccessPerNetwork
enum, and also the string "search_ports" to the VIR_ENUM_IMPL() macro
for that enum. Unfortunately, the enum value was added in the middle
of the list, while the string was added to the end of the
VIR_ENUM_IMPL().

This patch corrects that error by moving the new value to the end of
the enum definition, so that the order matches that of the string
list.

Resolves: https://bugzilla.redhat.com/1741428
Signed-off-by: Laine Stump 
---
  src/access/viraccessperm.h | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)



Reviewed-by: Michal Privoznik 

Michal

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

Re: [libvirt] [Qemu-devel] [PATCH 2/2] qapi: deprecate implicit filters

[Kevin Wolf]

Am 15.08.2019 um 21:24 hat Markus Armbruster geschrieben:
> Kevin Wolf  writes:
> 
> > Am 15.08.2019 um 18:07 hat John Snow geschrieben:
> >> 
> >> 
> >> On 8/15/19 6:49 AM, Kevin Wolf wrote:
> >> > Am 14.08.2019 um 21:27 hat John Snow geschrieben:
> >> >>
> >> >>
> >> >> On 8/14/19 6:07 AM, Vladimir Sementsov-Ogievskiy wrote:
> >> >>> To get rid of implicit filters related workarounds in future let's
> >> >>> deprecate them now.
> >> >>>
> >> >>> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> >> >>> ---
> >> >>>  qemu-deprecated.texi  |  7 +++
> >> >>>  qapi/block-core.json  |  6 --
> >> >>>  include/block/block_int.h | 10 +-
> >> >>>  blockdev.c| 10 ++
> >> >>>  4 files changed, 30 insertions(+), 3 deletions(-)
> >> >>>
> >> >>> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> >> >>> index 2753fafd0b..8222440148 100644
> >> >>> --- a/qemu-deprecated.texi
> >> >>> +++ b/qemu-deprecated.texi
> >> >>> @@ -183,6 +183,13 @@ the 'wait' field, which is only applicable to 
> >> >>> sockets in server mode
> >> >>>  
> >> >>>  Use blockdev-mirror and blockdev-backup instead.
> >> >>>  
> >> >>> +@subsection implicit filters (since 4.2)
> >> >>> +
> >> >>> +Mirror and commit jobs inserts filters, which becomes implicit if user
> >> >>> +omitted filter-node-name parameter. So omitting it is deprecated, set 
> >> >>> it
> >> >>> +always. Note, that drive-mirror don't have this parameter, so it will
> >> >>> +create implicit filter anyway, but drive-mirror is deprecated itself 
> >> >>> too.
> >> >>> +
> >> >>>  @section Human Monitor Protocol (HMP) commands
> >> >>>  
> >> >>>  @subsection The hub_id parameter of 'hostfwd_add' / 'hostfwd_remove' 
> >> >>> (since 3.1)
> >> >>> diff --git a/qapi/block-core.json b/qapi/block-core.json
> >> >>> index 4e35526634..0505ac9d8b 100644
> >> >>> --- a/qapi/block-core.json
> >> >>> +++ b/qapi/block-core.json
> >> >>> @@ -1596,7 +1596,8 @@
> >> >>>  # @filter-node-name: the node name that should be assigned to the
> >> >>>  #filter driver that the commit job inserts into 
> >> >>> the graph
> >> >>>  #above @top. If this option is not given, a node 
> >> >>> name is
> >> >>> -#autogenerated. (Since: 2.9)
> >> >>> +#autogenerated. Omitting this option is 
> >> >>> deprecated, it will
> >> >>> +#be required in future. (Since: 2.9)
> >> >>>  #
> >> >>>  # @auto-finalize: When false, this job will wait in a PENDING state 
> >> >>> after it has
> >> >>>  # finished its work, waiting for @block-job-finalize 
> >> >>> before
> >> >>> @@ -2249,7 +2250,8 @@
> >> >>>  # @filter-node-name: the node name that should be assigned to the
> >> >>>  #filter driver that the mirror job inserts into 
> >> >>> the graph
> >> >>>  #above @device. If this option is not given, a 
> >> >>> node name is
> >> >>> -#autogenerated. (Since: 2.9)
> >> >>> +#autogenerated. Omitting this option is 
> >> >>> deprecated, it will
> >> >>> +#be required in future. (Since: 2.9)
> >> >>>  #
> >> >>>  # @copy-mode: when to copy data to the destination; defaults to 
> >> >>> 'background'
> >> >>>  # (Since: 3.0)
> >> >>> diff --git a/include/block/block_int.h b/include/block/block_int.h
> >> >>> index 3aa1e832a8..624da0b4a2 100644
> >> >>> --- a/include/block/block_int.h
> >> >>> +++ b/include/block/block_int.h
> >> >>> @@ -762,7 +762,15 @@ struct BlockDriverState {
> >> >>>  bool sg;/* if true, the device is a /dev/sg* */
> >> >>>  bool probed;/* if true, format was probed rather than 
> >> >>> specified */
> >> >>>  bool force_share; /* if true, always allow all shared permissions 
> >> >>> */
> >> >>> -bool implicit;  /* if true, this filter node was automatically 
> >> >>> inserted */
> >> >>> +
> >> >>> +/*
> >> >>> + * @implicit field is deprecated, don't set it to true for new 
> >> >>> filters.
> >> >>> + * If true, this filter node was automatically inserted and user 
> >> >>> don't
> >> >>> + * know about it and unprepared for any effects of it. So, 
> >> >>> implicit
> >> >>> + * filters are workarounded and skipped in many places of the 
> >> >>> block
> >> >>> + * layer code.
> >> >>> + */
> >> >>> +bool implicit;
> >> >>>  
> >> >>>  BlockDriver *drv; /* NULL means no media */
> >> >>>  void *opaque;
> >> >>> diff --git a/blockdev.c b/blockdev.c
> >> >>> index 36e9368e01..b3cfaccce1 100644
> >> >>> --- a/blockdev.c
> >> >>> +++ b/blockdev.c
> >> >>> @@ -3292,6 +3292,11 @@ void qmp_block_commit(bool has_job_id, const 
> >> >>> char *job_id, const char *device,
> >> >>>  BlockdevOnError on_error = BLOCKDEV_ON_ERROR_REPORT;
> >> >>>  int job_flags = JOB_DEFAULT;
> >> >>>  
> >> >>> +if (!has_filter_node_name) {
> >> >>> +

Re: [libvirt] [PATCH] tests: virpcimock: Always declare __open_2

[Peter Krempa]

On Fri, Aug 16, 2019 at 09:52:02 +0200, Erik Skultety wrote:
> On Fri, Aug 16, 2019 at 09:43:27AM +0200, Peter Krempa wrote:
> > In some cases e.g. with clang on fedora 30 __open2 isn't even declared
> > which results in the following build error:
> >
> > /home/pipo/libvirt/tests/virpcimock.c:939:1: error: no previous prototype 
> > for function
> >   '__open_2' [-Werror,-Wmissing-prototypes]
> > __open_2(const char *path, int flags)
> >
> > Add a separate declaration to appease the compiler.
> >
> > Signed-off-by: Peter Krempa 
> > ---
> I guess you could have simply pushed this as a build breaker, anyway:

I find this solution kind of ugly, that's why I wasn't completely
persuaded to push it right away.


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