Re: [libvirt] [PATCH 4/4] Revert "docs: hvsupport: Add support for deprecating hypervisor implementations"

[Peter Krempa]

On Wed, Jun 26, 2019 at 19:24:14 +0200, Ján Tomko wrote:
> On Wed, Jun 26, 2019 at 04:54:43PM +0100, Daniel P. Berrangé wrote:
> > This reverts commit fd14dfec88957599ac5cffa28ddb7b513f404d9d.
> > 
> > Signed-off-by: Daniel P. Berrangé 
> > ---
> > docs/hvsupport.pl | 44 ++--
> > docs/libvirt.css  |  4 
> > 2 files changed, 14 insertions(+), 34 deletions(-)
> > 
> 
> Nice diffstat ;)

If we want to achieve a "Nice diffstat" we should rather remove the
whole 'hvsupport' matrix. It's cumbersome, misplaced and hard to
navigate.

It'd be way better to include the information along with the API
documentation so it does not need to be cross-checked across various
documents.


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

Re: [libvirt] [PATCH 1/4] qemu: delete methods which are no longer supported

[Peter Krempa]

On Wed, Jun 26, 2019 at 16:54:40 +0100, Daniel Berrange wrote:
> The public API entry points will report VIR_ERR_NO_SUPPORT to the
> caller when a driver does not provide an implementation of a particular
> method.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/qemu/qemu_driver.c | 31 ---
>  1 file changed, 31 deletions(-)

[...]

> @@ -22271,7 +22242,6 @@ static virHypervisorDriver qemuHypervisorDriver = {
>  .domainGetSecurityLabelList = qemuDomainGetSecurityLabelList, /* 0.10.0 
> */
>  .nodeGetSecurityModel = qemuNodeGetSecurityModel, /* 0.6.1 */
>  .domainGetXMLDesc = qemuDomainGetXMLDesc, /* 0.2.0 */
> -.connectDomainXMLFromNative = qemuConnectDomainXMLFromNative, /* 0.6.4 
> (deprecated: 5.5.0) */

This makes documentation strictly worse, where users may be lead into
thinking that this never existed.

I don't think we should just delete it without acknowledging we've
deleted it.


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

Re: [libvirt] [PATCH 3/4] Revert "docs: css: Add style for ..."

[Peter Krempa]

On Wed, Jun 26, 2019 at 19:47:07 +0200, Ján Tomko wrote:
> On Wed, Jun 26, 2019 at 04:54:42PM +0100, Daniel P. Berrangé wrote:
> > This reverts commit 71626402f481ed47ff67dafa8521b01a42625707.
> > 
> > Signed-off-by: Daniel P. Berrangé 
> > ---
> > docs/libvirt.css | 6 --
> > 1 file changed, 6 deletions(-)
> > 
> > diff --git a/docs/libvirt.css b/docs/libvirt.css
> > index 6639b1df64..54d015db98 100644
> > --- a/docs/libvirt.css
> > +++ b/docs/libvirt.css
> > @@ -274,12 +274,6 @@ span.since {
> > font-weight: bold;
> > }
> > 
> > -span.deprecated {
> > -color: darkred;
> > -font-style: italic;
> > -font-weight: bold;
> > -}
> > -
> > img.diagram {
> > background: rgb(230,230,230);
> > border: 2px dotted rgb(178,178,178);
> 
> Also, this is used by docs/drvqemu.html.in, since:
> commit d127bc3ce6eaf4b48dd09305368ac6a727d5aecf
>docs: drvqemu: Add note about deprecation of domxml-from-native
> 
> where we can either:
> a) drop the reference to the removed class
> b) drop the reference to the removed command

I don't think we should cover up that we deleted old functionality.

I don't care that much about the error code being reverted, but I think
covering up that we've deleted this is a wrong idea.



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

Re: [libvirt] [PATCH] libvirt: support block device storage type invirshParseSnapshotDiskspec

[liu.dayu]

> virsh has to be able to work remotely so this code will definitely not
> be acceptable here.
>
> I think we can only go with adding a "type" field for the diskspec
> string which will be either "file" or "block" and do this in that case.
> e.g.
> --diskspec vda,snapshot=external,driver=qcow2,type=block,file=/dev/whatever

thanks, i will reconsider adding a "type" field for --diskspec to amend this 
patch.--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

[Alex Williamson]

On Wed, 26 Jun 2019 08:37:20 -0600
Alex Williamson  wrote:

> On Wed, 26 Jun 2019 11:58:06 +0200
> Cornelia Huck  wrote:
> 
> > On Tue, 25 Jun 2019 16:52:51 -0600
> > Alex Williamson  wrote:
> >   
> > > Hi,
> > > 
> > > Based on the discussions we've had, I've rewritten the bulk of
> > > mdevctl.  I think it largely does everything we want now, modulo
> > > devices that will need some sort of 1:N values per key for
> > > configuration in the config file versus the 1:1 key:value setup we
> > > currently have (so don't consider the format final just yet).
> > 
> > We might want to factor out that config format handling while we're
> > trying to finalize it.
> > 
> > cc:ing Matt for his awareness. I'm currently not quite sure how to
> > handle those vfio-ap "write several values to an attribute one at a
> > time" requirements. Maybe 1:N key:value is the way to go; maybe we
> > need/want JSON or something like that.  
> 
> Maybe we should just do JSON for future flexibility.  I assume there
> are lots of helpers that should make it easy even from a bash script.
> I'll look at that next.

Done.  Throw away any old mdev config files, we use JSON now.  The per
mdev config now looks like this:

{
  "mdev_type": "i915-GVTg_V4_8",
  "start": "auto"
}

My expectation, and what I've already pre-enabled support in set_key
and get_key functions, is that we'd use arrays for values, so we might
have:

  "new_key": ["value1", "value2"]

set_key will automatically convert a comma separated list of values
into such an array, so I'm thinking this would be specified by the user
as:

# mdevctl modify -u UUID --key=new_key --value=value1,value2

We should think about whether ordering is important and maybe
incorporate that into key naming conventions or come up with some
syntax for specifying startup blocks.  Thanks,

Alex

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

[libvirt] Entering freeze for libvirt-5.5.0

[Daniel Veillard]

  As planned I tagged RC1 in git and pushed signed tarball and source rpm
to the usual place:

https://libvirt.org/sources/

 I'm on the road with even less testing capacity than usual, but
https://ci.centos.org/view/libvirt/ looks rather green except for some go
bindings so I assume we are in pretty good shape, i.e. it will hopefully
compile and run for most of you :-)

  I hope to be back on Fraiday and make an evening release again, then
if all goes fine we can push on Mon or Tuesday,

   please give it some testing,


 thanks,

Daniel

-- 
Daniel Veillard  | Red Hat Developers Tools http://developer.redhat.com/
veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/

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

Re: [libvirt] [PATCH v3 1/8] qemu_monitor: helper functions for CPU models

[Daniel Henrique Barboza]

On 5/30/19 11:23 AM, Collin Walling wrote:

Refactor some code in qemuMonitorJSONGetCPUModelExpansion to be
later used for the comparison and baseline functions.

This does not alter any functionality.

Signed-off-by: Collin Walling 
Reviewed-by: Bjoern Walk 
---


Reviewed-by: Daniel Henrique Barboza 


  src/qemu/qemu_monitor_json.c | 173 ++-
  1 file changed, 121 insertions(+), 52 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 53a7de8..08c734c 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5503,6 +5503,120 @@ qemuMonitorJSONParseCPUModelProperty(const char *key,
  return 0;
  }
  
+

+static virJSONValuePtr
+qemuMonitorJSONMakeCPUModel(const char *model_name,
+size_t nprops,
+virCPUFeatureDefPtr props,
+bool migratable)
+{
+virJSONValuePtr value;
+virJSONValuePtr feats = NULL;
+size_t i;
+
+if (!(value = virJSONValueNewObject()))
+goto cleanup;
+
+if (virJSONValueObjectAppendString(value, "name", model_name) < 0)
+goto cleanup;
+
+if (nprops || !migratable) {
+if (!(feats = virJSONValueNewObject()))
+goto cleanup;
+
+for (i = 0; i < nprops; i++) {
+char *name = props[i].name;
+bool enabled = false;
+
+/* policy may be reported as -1 if the CPU def is a host model */
+if (props[i].policy == VIR_CPU_FEATURE_REQUIRE ||
+props[i].policy == VIR_CPU_FEATURE_FORCE ||
+props[i].policy == -1)
+enabled = true;
+
+if (virJSONValueObjectAppendBoolean(feats, name, enabled) < 0)
+goto cleanup;
+}
+
+if (!migratable &&
+virJSONValueObjectAppendBoolean(feats, "migratable", false) < 0) {
+goto cleanup;
+}
+
+if (virJSONValueObjectAppend(value, "props", feats) < 0)
+goto cleanup;
+}
+
+return value;
+
+ cleanup:
+virJSONValueFree(value);
+virJSONValueFree(feats);
+return NULL;
+}
+
+
+static int
+qemuMonitorJSONParseCPUModelData(virJSONValuePtr data,
+ virJSONValuePtr *cpu_model,
+ virJSONValuePtr *cpu_props,
+ const char **cpu_name,
+ const char *cmd_name)
+{
+if (!(*cpu_model = virJSONValueObjectGetObject(data, "model"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("%s reply data was missing 'model'"), cmd_name);
+return -1;
+}
+
+if (!(*cpu_name = virJSONValueObjectGetString(*cpu_model, "name"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("%s reply data was missing 'name'"), cmd_name);
+return -1;
+}
+
+/* Some older CPU models do not report properties */
+*cpu_props = virJSONValueObjectGetObject(*cpu_model, "props");
+
+return 0;
+}
+
+
+static int
+qemuMonitorJSONParseCPUModel(const char *cpu_name,
+ virJSONValuePtr cpu_props,
+ qemuMonitorCPUModelInfoPtr *model_info)
+{
+qemuMonitorCPUModelInfoPtr machine_model = NULL;
+int ret = -1;
+
+if (VIR_ALLOC(machine_model) < 0)
+goto cleanup;
+
+if (VIR_STRDUP(machine_model->name, cpu_name) < 0)
+goto cleanup;
+
+if (cpu_props) {
+size_t nprops = virJSONValueObjectKeysNumber(cpu_props);
+
+if (VIR_ALLOC_N(machine_model->props, nprops) < 0)
+goto cleanup;
+
+if (virJSONValueObjectForeachKeyValue(cpu_props,
+  
qemuMonitorJSONParseCPUModelProperty,
+  machine_model) < 0)
+goto cleanup;
+}
+
+VIR_STEAL_PTR(*model_info, machine_model);
+ret = 0;
+
+ cleanup:
+qemuMonitorCPUModelInfoFree(machine_model);
+return ret;
+}
+
+
  int
  qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
  qemuMonitorCPUModelExpansionType type,
@@ -5511,33 +5625,20 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
  qemuMonitorCPUModelInfoPtr *model_info)
  {
  int ret = -1;
-virJSONValuePtr model = NULL;
-virJSONValuePtr props = NULL;
+virJSONValuePtr model;
  virJSONValuePtr cmd = NULL;
  virJSONValuePtr reply = NULL;
  virJSONValuePtr data;
  virJSONValuePtr cpu_model;
-virJSONValuePtr cpu_props;
-qemuMonitorCPUModelInfoPtr machine_model = NULL;
-char const *cpu_name;
+virJSONValuePtr cpu_props = NULL;
+const char *cpu_name = "";
  const char *typeStr = "";
  
  *model_info = NULL;
  
-if (!(model = virJSONValueNewObject()))

+if (!(model = qemuMonitorJSONMakeCPUModel(model_name, 0, NULL, 

[libvirt] [PATCH v1 08/10] virpcimock: Mock the SRIOV Virtual functions

[Daniel Henrique Barboza]

From: Shivaprasad G Bhat 

The softlink to physfn is the way to know if the device is
VF or not. So, the patch softlinks 'physfn' to the parent function.
The multifunction PCI devices dont have 'physfn' softlinks.

The patch adds few Virtual functions to the mock environment and
changes the existing VFIO test xmls using the VFs to use the newly
added VFs for their use case.

Signed-off-by: Shivaprasad G Bhat 
Signed-off-by: Daniel Henrique Barboza 
---
 .../hostdev-vfio-multidomain.args   |   2 +-
 .../hostdev-vfio-multidomain.xml|   2 +-
 tests/qemuxml2argvdata/hostdev-vfio.args|   2 +-
 tests/qemuxml2argvdata/hostdev-vfio.xml |   2 +-
 tests/qemuxml2argvdata/net-hostdev-fail.xml |   2 +-
 tests/qemuxml2argvdata/net-hostdev-vfio.args|   2 +-
 tests/qemuxml2argvdata/net-hostdev-vfio.xml |   2 +-
 tests/qemuxml2xmloutdata/hostdev-vfio.xml   |   2 +-
 tests/qemuxml2xmloutdata/net-hostdev-vfio.xml   |   2 +-
 tests/virpcimock.c  |  16 
 tests/virpcitestdata/-06-12.0.config| Bin 0 -> 256 bytes
 tests/virpcitestdata/-06-12.1.config| Bin 0 -> 256 bytes
 tests/virpcitestdata/-06-12.2.config| Bin 0 -> 256 bytes
 13 files changed, 25 insertions(+), 9 deletions(-)
 create mode 100644 tests/virpcitestdata/-06-12.0.config
 create mode 100644 tests/virpcitestdata/-06-12.1.config
 create mode 100644 tests/virpcitestdata/-06-12.2.config

diff --git a/tests/qemuxml2argvdata/hostdev-vfio-multidomain.args 
b/tests/qemuxml2argvdata/hostdev-vfio-multidomain.args
index d098bff356..959d2a 100644
--- a/tests/qemuxml2argvdata/hostdev-vfio-multidomain.args
+++ b/tests/qemuxml2argvdata/hostdev-vfio-multidomain.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 vfio-pci,host=55aa:20:0f.3,id=hostdev0,bus=pci.0,addr=0x3 \
+-device vfio-pci,host=0021:de:1f.1,id=hostdev0,bus=pci.0,addr=0x3 \
 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
diff --git a/tests/qemuxml2argvdata/hostdev-vfio-multidomain.xml 
b/tests/qemuxml2argvdata/hostdev-vfio-multidomain.xml
index 832458125b..7c34b65c7f 100644
--- a/tests/qemuxml2argvdata/hostdev-vfio-multidomain.xml
+++ b/tests/qemuxml2argvdata/hostdev-vfio-multidomain.xml
@@ -25,7 +25,7 @@
 
   
   
-
+
   
 
 
diff --git a/tests/qemuxml2argvdata/hostdev-vfio.args 
b/tests/qemuxml2argvdata/hostdev-vfio.args
index b86181e267..e7456d0e49 100644
--- a/tests/qemuxml2argvdata/hostdev-vfio.args
+++ b/tests/qemuxml2argvdata/hostdev-vfio.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 vfio-pci,host=06:12.5,id=hostdev0,bus=pci.0,addr=0x3 \
+-device vfio-pci,host=06:12.1,id=hostdev0,bus=pci.0,addr=0x3 \
 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
diff --git a/tests/qemuxml2argvdata/hostdev-vfio.xml 
b/tests/qemuxml2argvdata/hostdev-vfio.xml
index 4d96b9f732..b2e6c62e7e 100644
--- a/tests/qemuxml2argvdata/hostdev-vfio.xml
+++ b/tests/qemuxml2argvdata/hostdev-vfio.xml
@@ -27,7 +27,7 @@
 
   
   
-
+
   
 
 
diff --git a/tests/qemuxml2argvdata/net-hostdev-fail.xml 
b/tests/qemuxml2argvdata/net-hostdev-fail.xml
index c815d68bd9..50b102c658 100644
--- a/tests/qemuxml2argvdata/net-hostdev-fail.xml
+++ b/tests/qemuxml2argvdata/net-hostdev-fail.xml
@@ -25,7 +25,7 @@
 
   
   
-
+
   
   
   
diff --git a/tests/qemuxml2argvdata/net-hostdev-vfio.args 
b/tests/qemuxml2argvdata/net-hostdev-vfio.args
index fcc8716fdf..a7fb5819ac 100644
--- a/tests/qemuxml2argvdata/net-hostdev-vfio.args
+++ b/tests/qemuxml2argvdata/net-hostdev-vfio.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 vfio-pci,host=03:07.1,id=hostdev0,bus=pci.0,addr=0x3 \
+-device vfio-pci,host=06:12.1,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-vfio.xml 
b/tests/qemuxml2argvdata/net-hostdev-vfio.xml
index 24034cad8b..aff681c0fb 100644
--- a/tests/qemuxml2argvdata/net-hostdev-vfio.xml
+++ b/tests/qemuxml2argvdata/net-hostdev-vfio.xml
@@ -26,7 +26,7 @@
   
   
   
-
+
   
   
 
diff --git a/tests/qemuxml2xmloutdata/hostdev-vfio.xml 
b/tests/qemuxml2xmloutdata/hostdev-vfio.xml
index 77bd62a129..15a845f4d1 100644
--- a/tests/qemuxml2xmloutdata/hostdev-vfio.xml
+++ b/tests/qemuxml2xmloutdata/hostdev-vfio.xml
@@ -32,7 +32,7 @@
 
   
   
-
+ 

[libvirt] [PATCH v1 09/10] tests: qemu: Add test case for pci-hostdev hotplug

[Daniel Henrique Barboza]

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 d7d191ec75..fddd216c93 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 115afa1c1a..05a20483e4 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -211,6 +211,7 @@ test_libraries = libshunload.la \
virpcimock.la \
virnetdevmock.la \
virrandommock.la \
+  virprocessmock.la \
virhostcpumock.la \
domaincapsmock.la \
virfilecachemock.la \
@@ -1193,6 +1194,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(&driver, vm, dev->data.watchdog);
 break;
+case VIR_DOMAIN_DEVICE_HOSTDEV:
+ret = qemuDomainAttachHostDevice(&driver, 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, &driver, 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");
+
 
 #if !WITH_YAJL
 fputs("libvirt not compiled with JSON support, skipping this test\n", 
stderr);
@@ -621,6 +644,8 @@ mymain(void)
 if (!driver.lockManager)
 r

[libvirt] [PATCH v1 07/10] virpcitest: Change the stub driver to vfio from pci-stub

[Daniel Henrique Barboza]

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 | 8 
 3 files changed, 8 insertions(+), 8 deletions(-)

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 fc8cb92ebd..f9863b2b82 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -1023,8 +1023,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);
 
 # define MAKE_PCI_DEVICE(Id, Vendor, Device, ...) \
 do { \
diff --git a/tests/virpcitest.c b/tests/virpcitest.c
index 9ecd1b7d27..3319ab3d6a 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);
@@ -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);
-- 
2.20.1

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

[libvirt] [PATCH v1 01/10] virpcimock: Move actions checking one level up

[Daniel Henrique Barboza]

From: Michal Privoznik 

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;
 }
-- 
2.20.1

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

[libvirt] [PATCH v1 10/10] tests: Add a baseline test for multifunction pci device use case

[Daniel Henrique Barboza]

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

diff --git a/tests/qemuxml2argvdata/hostdev-pci-multifunction.args 
b/tests/qemuxml2argvdata/hostdev-pci-multifunction.args
new file mode 100644
index 00..7747cca442
--- /dev/null
+++ b/tests/qemuxml2argvdata/hostdev-pci-multifunction.args
@@ -0,0 +1,35 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-delete \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-delete/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-delete/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-delete/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name delete \
+-S \
+-machine pc,accel=kvm,usb=off,dump-guest-core=off \
+-m 256 \
+-realtime mlock=off \
+-smp 4,sockets=4,cores=1,threads=1 \
+-uuid 583a8e8e-f0ce-4f53-89ab-092862148b25 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-delete/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-usb \
+-device vfio-pci,host=0005:90:01.0,id=hostdev0,bus=pci.0,addr=0x3 \
+-device vfio-pci,host=0001:01:00.1,id=hostdev1,bus=pci.0,addr=0x4 \
+-device vfio-pci,host=0001:01:00.0,id=hostdev2,bus=pci.0,addr=0x5 \
+-device vfio-pci,host=0005:90:01.2,id=hostdev3,bus=pci.0,addr=0x6 \
+-device vfio-pci,host=0005:90:01.3,id=hostdev4,bus=pci.0,addr=0x7 \
+-device vfio-pci,host=06:12.1,id=hostdev5,bus=pci.0,addr=0x8 \
+-device vfio-pci,host=06:12.2,id=hostdev6,bus=pci.0,addr=0x9 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0xa
diff --git a/tests/qemuxml2argvdata/hostdev-pci-multifunction.xml 
b/tests/qemuxml2argvdata/hostdev-pci-multifunction.xml
new file mode 100644
index 00..06c889c64d
--- /dev/null
+++ b/tests/qemuxml2argvdata/hostdev-pci-multifunction.xml
@@ -0,0 +1,59 @@
+
+  delete
+  583a8e8e-f0ce-4f53-89ab-092862148b25
+  262144
+  4
+  
+hvm
+  
+  
+/usr/bin/qemu-system-x86_64
+
+
+
+
+
+
+  
+  
+
+  
+
+
+  
+  
+
+  
+
+
+  
+  
+
+  
+
+
+  
+  
+
+  
+
+
+  
+  
+
+  
+
+
+  
+  
+
+  
+
+
+  
+  
+
+  
+
+  
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 91ca35d469..51d7392730 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1291,6 +1291,9 @@ mymain(void)
 DO_TEST_FAILURE("net-hostdev-fail",
 QEMU_CAPS_DEVICE_VFIO_PCI);
 
+DO_TEST("hostdev-pci-multifunction",
+QEMU_CAPS_KVM,
+QEMU_CAPS_DEVICE_VFIO_PCI);
 
 DO_TEST("serial-file-log",
 QEMU_CAPS_CHARDEV_FILE_APPEND,
diff --git a/tests/qemuxml2xmloutdata/hostdev-pci-multifunction.xml 
b/tests/qemuxml2xmloutdata/hostdev-pci-multifunction.xml
new file mode 100644
index 00..52ed86e305
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/hostdev-pci-multifunction.xml
@@ -0,0 +1,79 @@
+
+  delete
+  583a8e8e-f0ce-4f53-89ab-092862148b25
+  262144
+  262144
+  4
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-x86_64
+
+  
+
+
+  
+
+
+
+
+
+  
+  
+
+  
+  
+
+
+  
+  
+
+  
+  
+
+
+  
+  
+
+  
+  
+
+
+  
+  
+
+  
+  
+
+
+  
+  
+
+  
+  
+
+
+  
+  
+
+  
+  
+
+
+  
+  
+
+  
+  
+
+
+  
+
+  
+
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index a64b17ac28..5f57bdac13 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c

[libvirt] [PATCH v1 06/10] util/virhostdev: enhance VFIO device is in use detection

[Daniel Henrique Barboza]

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

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index a3647a6cf4..35be8fede1 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -55,6 +55,46 @@ struct virHostdevIsPCINodeDeviceUsedData {
 const bool usesVFIO;
 };
 
+
+static virPCIDevicePtr
+virHostdevFindActivePCIDevWithAddr(virPCIDeviceAddressPtr devAddr,
+   virHostdevManagerPtr mgr)
+{
+virPCIDevicePtr ret = NULL;
+
+ret = virPCIDeviceListFindByIDs(mgr->activePCIHostdevs,
+devAddr->domain, devAddr->bus,
+devAddr->slot, devAddr->function);
+
+return ret;
+}
+
+/* Callback to be used inside virHostdevIsPCINodeDeviceUsed to check
+ * for IOMMU ownership of a domain given by helperData->domainName. */
+static int
+virHostdevDomainHasIOMMUOwnershipCb(virPCIDeviceAddressPtr devAddr, void 
*opaque)
+{
+struct virHostdevIsPCINodeDeviceUsedData *helperData = opaque;
+virPCIDevicePtr actual;
+int ret = 0;
+
+actual = virHostdevFindActivePCIDevWithAddr(devAddr, helperData->mgr);
+if (!actual)
+return ret;
+
+if (helperData->usesVFIO) {
+const char *actual_drvname = NULL;
+const char *actual_domname = NULL;
+virPCIDeviceGetUsedBy(actual, &actual_drvname, &actual_domname);
+
+if ((actual_domname && helperData->domainName) &&
+(STRNEQ(actual_domname, helperData->domainName)))
+ret = -1;
+}
+
+return ret;
+}
+
 /* This module makes heavy use of bookkeeping lists contained inside a
  * virHostdevManager instance to keep track of the devices' status. To make
  * it easy to spot potential ownership errors when moving devices from one
@@ -82,19 +122,13 @@ static int 
virHostdevIsPCINodeDeviceUsed(virPCIDeviceAddressPtr devAddr, void *o
 int ret = -1;
 struct virHostdevIsPCINodeDeviceUsedData *helperData = opaque;
 
-actual = virPCIDeviceListFindByIDs(helperData->mgr->activePCIHostdevs,
-   devAddr->domain, devAddr->bus,
-   devAddr->slot, devAddr->function);
+actual = virHostdevFindActivePCIDevWithAddr(devAddr, helperData->mgr);
+
 if (actual) {
 const char *actual_drvname = NULL;
 const char *actual_domname = NULL;
 virPCIDeviceGetUsedBy(actual, &actual_drvname, &actual_domname);
 
-if (helperData->usesVFIO &&
-(actual_domname && helperData->domainName) &&
-(STREQ(actual_domname, helperData->domainName)))
-goto iommu_owner;
-
 if (actual_drvname && actual_domname)
 virReportError(VIR_ERR_OPERATION_INVALID,
_("PCI device %s is in use by "
@@ -105,9 +139,20 @@ static int 
virHostdevIsPCINodeDeviceUsed(virPCIDeviceAddressPtr devAddr, void *o
 virReportError(VIR_ERR_OPERATION_INVALID,
_("PCI device %s is in use"),
virPCIDeviceGetName(actual));
+
 goto cleanup;
 }
- iommu_owner:
+
+/* For VFIO devices, the domain helperData->domainName must ha

[libvirt] [PATCH v1 05/10] tests: pci: Mock the iommu groups and vfio

[Daniel Henrique Barboza]

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

diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index acd7560f8b..fc8cb92ebd 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__); \
@@ -128,6 +130,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;
@@ -139,6 +146,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;
 
@@ -255,6 +265,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;
@@ -473,6 +490,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(&kerneldir, "%s%s",
+ fakerootdir, SYSFS_KERNEL_PREFIX) < 0)
+ABORT_OOM();
+
+if (virAsprintfQuiet(&iommupath, "%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(&vfiopath, "%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(&vfiopath, "%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
@@ -626,6 +738,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 = 0;
  cleanup:
@@ -660,6 +776,10 @@ pci_driver_unbind(struct pciDriver *driver,
 unlink(driverpath) < 0)
 goto cleanup;
 
+  

[libvirt] [PATCH v1 00/10] PCI Multifunction hotplug/unplug, part 1

[Daniel Henrique Barboza]

This is the first part of the feature discussed at [1]. With the
exception of patch 06, these patches are aimed into preparing
the existing unit test infrastructure for the more sofisticated
multifunction tests to come.

First 3 Michal's patches helped me with unit test issues in
the first 3-4 patches (see [2] for more info), thus I am
relying on them being upstreamed as well. 

The whole feature can be checked out at [3]. All patches survives
unit testing. The feature was stress tested with hundreds
of consecutive hotplug/unplugs of a Broadcom BCM5719 multifunction
network card in a guest running in a Power 8 server. Hopefully
I'll find a suitable x86 env to stress test the feature there
too.

[1] https://www.redhat.com/archives/libvir-list/2019-June/msg00703.html
[2] https://www.redhat.com/archives/libvir-list/2019-June/msg00726.html
[3] https://github.com/danielhb/libvirt/tree/multifunction_latest


Daniel Henrique Barboza (1):
  util/virhostdev: enhance VFIO device is in use detection

Michal Prívozník (3):
  virpcimock: Move actions checking one level up
  Revert "virpcitest: Test virPCIDeviceDetach failure"
  virpcimock: Create driver_override file in device dirs

Shivaprasad G Bhat (6):
  tests: Fix the iommu group path in mock pci
  tests: pci: Mock the iommu groups and vfio
  virpcitest: Change the stub driver to vfio from pci-stub
  virpcimock: Mock the SRIOV Virtual functions
  tests: qemu: Add test case for pci-hostdev hotplug
  tests: Add a baseline test for multifunction pci device use case

 src/util/virhostdev.c |  74 +++--
 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 +++
 .../hostdev-pci-multifunction.args|  35 +++
 .../hostdev-pci-multifunction.xml |  59 
 .../hostdev-vfio-multidomain.args |   2 +-
 .../hostdev-vfio-multidomain.xml  |   2 +-
 tests/qemuxml2argvdata/hostdev-vfio.args  |   2 +-
 tests/qemuxml2argvdata/hostdev-vfio.xml   |   2 +-
 tests/qemuxml2argvdata/net-hostdev-fail.xml   |   2 +-
 tests/qemuxml2argvdata/net-hostdev-vfio.args  |   2 +-
 tests/qemuxml2argvdata/net-hostdev-vfio.xml   |   2 +-
 tests/qemuxml2argvtest.c  |   3 +
 .../hostdev-pci-multifunction.xml |  79 +
 tests/qemuxml2xmloutdata/hostdev-vfio.xml |   2 +-
 tests/qemuxml2xmloutdata/net-hostdev-vfio.xml |   2 +-
 tests/qemuxml2xmltest.c   |   1 +
 tests/virhostdevtest.c|   4 +-
 tests/virpcimock.c| 276 +++---
 tests/virpcitest.c|  40 +--
 tests/virpcitestdata/-06-12.0.config  | Bin 0 -> 256 bytes
 tests/virpcitestdata/-06-12.1.config  | Bin 0 -> 256 bytes
 tests/virpcitestdata/-06-12.2.config  | Bin 0 -> 256 bytes
 tests/virprocessmock.c|  28 ++
 29 files changed, 726 insertions(+), 100 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/qemuxml2argvdata/hostdev-pci-multifunction.args
 create mode 100644 tests/qemuxml2argvdata/hostdev-pci-multifunction.xml
 create mode 100644 tests/qemuxml2xmloutdata/hostdev-pci-multifunction.xml
 create mode 100644 tests/virpcitestdata/-06-12.0.config
 create mode 100644 tests/virpcitestdata/-06-12.1.config
 create mode 100644 tests/virpcitestdata/-06-12.2.config
 create mode 100644 tests/virprocessmock.c

-- 
2.20.1

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

[libvirt] [PATCH v1 04/10] tests: Fix the iommu group path in mock pci

[Daniel Henrique Barboza]

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 5d55038a26..acd7560f8b 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -409,7 +409,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",
+ "%s/../../../../kernel/iommu_groups/%d",
  devpath, dev->iommuGroup) < 0) {
 ABORT("@tmp overflow");
 }
@@ -417,7 +417,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);
-- 
2.20.1

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

[libvirt] [PATCH v1 02/10] Revert "virpcitest: Test virPCIDeviceDetach failure"

[Daniel Henrique Barboza]

From: Michal Privoznik 

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);
-- 
2.20.1

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

[libvirt] [PATCH v1 03/10] virpcimock: Create driver_override file in device dirs

[Daniel Henrique Barboza]

From: Michal Privoznik 

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 | 59 --
 1 file changed, 52 insertions(+), 7 deletions(-)

diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index 6865f992dc..5d55038a26 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,36 @@ pci_driver_find_by_path(const char *path)
 return NULL;
 }
 
+static struct pciDriver *
+pci_driver_find_by_driver_override(struct pciDevice *dev)
+{
+struct pciDriver *ret = NULL;
+char *path = NULL;
+char tmp[32];
+size_t i;
+
+if (virAsprintfQuiet(&path,
+ SYSFS_PCI_PREFIX "devices/%s/driver_override",
+ dev->id) < 0)
+return NULL;
+
+if (pci_read_file(path, tmp, sizeof(tmp), false) < 0)
+goto cleanup;
+
+for (i = 0; i < nPCIDrivers; i++) {
+struct pciDriver *driver = pciDrivers[i];
+
+if (STREQ(tmp, driver->name)) {
+ret = driver;
+break;
+}
+}
+
+ cleanup:
+VIR_FREE(path);
+return ret;
+}
+
 static int
 pci_driver_bind(struct pciDriver *driver,
 struct pciDevice *dev)
@@ -644,7 +687,7 @@ pci_driver_handle_drivers_probe(const char *path)
 static int
 pci_driver_handle_change(int fd ATTRIBUTE_UNUSED, const char *path)
 {
-int ret;
+int ret = 0;
 const char *file = last_component(path);
 
 if (STREQ(file, "bind"))
@@ -657,6 +700,8 @@ pci_driver_handle_change(int fd ATTRIBUTE_UNUSED, const 
char *path)
 ret = pci_driver_handle_remove_id(path);
 else if

Re: [libvirt] [PATCH 3/4] Revert "docs: css: Add style for ..."

[Ján Tomko]

On Wed, Jun 26, 2019 at 04:54:42PM +0100, Daniel P. Berrangé wrote:

This reverts commit 71626402f481ed47ff67dafa8521b01a42625707.

Signed-off-by: Daniel P. Berrangé 
---
docs/libvirt.css | 6 --
1 file changed, 6 deletions(-)

diff --git a/docs/libvirt.css b/docs/libvirt.css
index 6639b1df64..54d015db98 100644
--- a/docs/libvirt.css
+++ b/docs/libvirt.css
@@ -274,12 +274,6 @@ span.since {
font-weight: bold;
}

-span.deprecated {
-color: darkred;
-font-style: italic;
-font-weight: bold;
-}
-
img.diagram {
background: rgb(230,230,230);
border: 2px dotted rgb(178,178,178);


Also, this is used by docs/drvqemu.html.in, since:
commit d127bc3ce6eaf4b48dd09305368ac6a727d5aecf
   docs: drvqemu: Add note about deprecation of domxml-from-native

where we can either:
a) drop the reference to the removed class
b) drop the reference to the removed command

b) somehow smells of rewriting history - it would be nice to keep the
deprecation notice there at leastfor some time.  OTOH, we have a lot of
 elements with pre-historic libvirt versions I'm
considering removing.

Jano


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

Re: [libvirt] [PATCH 0/4] Remove VIR_ERR_DEPRECATED & associated pieces

[Ján Tomko]

On Wed, Jun 26, 2019 at 04:54:39PM +0100, Daniel P. Berrangé wrote:

A deprecation is inherantly a warning, not an error, so adding an error
with this name makes no conceptual sense.

Features in drivers may come & go at any time and we've always use the
VIR_ERR_NO_SUPPORT to indicate when a hypervisor driver does not support
a particular API.

Daniel P. Berrangé (4):
 qemu: delete methods which are no longer supported
 Revert "error: Add VIR_ERR_DEPRECATED error code"
 Revert "docs: css: Add style for  ..."
 Revert "docs: hvsupport: Add support for deprecating hypervisor
   implementations"

docs/hvsupport.pl   | 44 -
docs/libvirt.css| 10 -
include/libvirt/virterror.h |  1 -
src/qemu/qemu_driver.c  | 31 --
src/util/virerror.c |  4 
5 files changed, 14 insertions(+), 76 deletions(-)



Missing a revert of:
commit 3026f6d9d986ad63c4b4a1c57589e6d05b71bd70
   news: Mention VIR_ERR_DEPRECATED in improvements

I wish it would have been the first news revert in this release cycle.

Jano


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

Re: [libvirt] [PATCH 4/4] Revert "docs: hvsupport: Add support for deprecating hypervisor implementations"

[Ján Tomko]

On Wed, Jun 26, 2019 at 04:54:43PM +0100, Daniel P. Berrangé wrote:

This reverts commit fd14dfec88957599ac5cffa28ddb7b513f404d9d.

Signed-off-by: Daniel P. Berrangé 
---
docs/hvsupport.pl | 44 ++--
docs/libvirt.css  |  4 
2 files changed, 14 insertions(+), 34 deletions(-)



Nice diffstat ;)

Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 2/4] Revert "error: Add VIR_ERR_DEPRECATED error code"

[Ján Tomko]

On Wed, Jun 26, 2019 at 04:54:41PM +0100, Daniel P. Berrangé wrote:

This reverts commit 226094fbc483128cf4171c353aed738b8346.

A deprecation is a warning to something that use of a feature is
being discouraged. By definition it is not an error condition to
continue to use a deprecated feature.



Makes sense that from an app's PoV it's the same


A VIR_ERR_DEPRECATED constant thus makes no conceptual sense. For
features which are entirely absent we already document that the
VIR_ERR_NO_SUPPORT code will be used. There is no need to distinguish
between a feature which never existed and a feature which previously
existed and was since removed.

Signed-off-by: Daniel P. Berrangé 
---
include/libvirt/virterror.h | 1 -
src/util/virerror.c | 4 
2 files changed, 5 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 1/4] qemu: delete methods which are no longer supported

[Ján Tomko]

On Wed, Jun 26, 2019 at 04:54:40PM +0100, Daniel P. Berrangé wrote:

The public API entry points will report VIR_ERR_NO_SUPPORT to the
caller when a driver does not provide an implementation of a particular
method.

Signed-off-by: Daniel P. Berrangé 
---
src/qemu/qemu_driver.c | 31 ---
1 file changed, 31 deletions(-)



This is a followup to
commit 215d9393bb60615f957f456fec646cf187fec26e
   qemu: driver: Drop support for qemu-attach
which also introduced an exception for domainQemuAttach in 
src/check-aclrules.pl:

diff --git a/src/check-aclrules.pl b/src/check-aclrules.pl
index be618f6c9f..23872cda98 100755
--- a/src/check-aclrules.pl
+++ b/src/check-aclrules.pl
@@ -61,7 +61,6 @@ my %whitelist = (
"interfaceClose" => 1,
"connectURIProbe" => 1,
"localOnly" => 1,
-"domainQemuAttach" => 1,
);

# XXX this vzDomainMigrateConfirm3Params looks

Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 3/4] Revert "docs: css: Add style for ..."

[Ján Tomko]

On Wed, Jun 26, 2019 at 04:54:42PM +0100, Daniel P. Berrangé wrote:

This reverts commit 71626402f481ed47ff67dafa8521b01a42625707.

Signed-off-by: Daniel P. Berrangé 
---
docs/libvirt.css | 6 --
1 file changed, 6 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

[libvirt] [RFC] test_driver: Thoughts on the implementation of some FS-related APIs

[Ilias Stamatis]

Hello,

I was thinking about how to implement the following APIs in the test driver:
-virDomainFSFreeze
-virDomainFSThaw
-virDomainFSTrim

The first two are conceptually paired. They both get a mountpoints
argument. The QEMU driver (which is the only driver implementing this
currently) ignores this completely.

However, I thought that we can allow it and pretend that the only
available mountpoints are "/" and "/boot" (like in a default Fedora
installation). The following questions arise though:

- Should we keep some kind of temporary or permanent state on the test
driver about this? Because it makes sense for virDomainFSThaw to
succeed only on mountpoints for which previously virDomainFSFreeze has
been called. If so, *where* exactly should we keep this state?

- In case a non-existing mountpoint is passed to either of the first 2
APIs. Should we fail? If so, a new error code should be introduced for
this. Also for the cases above where e.g. Thaw is called on a fs that
isn't frozen probably yet another different error code must be used.
Should we add all these new error codes?

Or maybe all the above is more complicated than necessary and it's
fine if we ignore mountpoints completely like the QEMU driver does and
make all 3 APIs above to always succeed no matter what in the test
driver?

Thanks,
Ilias

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

[libvirt] [PATCH 2/4] Revert "error: Add VIR_ERR_DEPRECATED error code"

[Daniel P . Berrangé]

This reverts commit 226094fbc483128cf4171c353aed738b8346.

A deprecation is a warning to something that use of a feature is
being discouraged. By definition it is not an error condition to
continue to use a deprecated feature.

A VIR_ERR_DEPRECATED constant thus makes no conceptual sense. For
features which are entirely absent we already document that the
VIR_ERR_NO_SUPPORT code will be used. There is no need to distinguish
between a feature which never existed and a feature which previously
existed and was since removed.

Signed-off-by: Daniel P. Berrangé 
---
 include/libvirt/virterror.h | 1 -
 src/util/virerror.c | 4 
 2 files changed, 5 deletions(-)

diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 22bc3c2d27..102a2573bf 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -329,7 +329,6 @@ typedef enum {
 VIR_ERR_INVALID_NETWORK_PORT = 105, /* invalid network port object */
 VIR_ERR_NETWORK_PORT_EXIST = 106,   /* the network port already exist */
 VIR_ERR_NO_NETWORK_PORT = 107,  /* network port not found */
-VIR_ERR_DEPRECATED = 108,   /* configuration or operation is no 
longer supported */
 
 # ifdef VIR_ENUM_SENTINELS
 VIR_ERR_NUMBER_LAST
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 26f14ddd29..dfba8c5712 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -1235,10 +1235,6 @@ const virErrorMsgTuple 
virErrorMsgStrings[VIR_ERR_NUMBER_LAST] = {
 [VIR_ERR_NO_NETWORK_PORT] = {
 N_("network port not found"),
 N_("network port not found: %s") },
-[VIR_ERR_DEPRECATED] = {
-N_("operation or configuration no longer supported"),
-"%s",
-},
 };
 
 
-- 
2.21.0

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

[libvirt] [PATCH 1/4] qemu: delete methods which are no longer supported

[Daniel P . Berrangé]

The public API entry points will report VIR_ERR_NO_SUPPORT to the
caller when a driver does not provide an implementation of a particular
method.

Signed-off-by: Daniel P. Berrangé 
---
 src/qemu/qemu_driver.c | 31 ---
 1 file changed, 31 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d6ab134196..595a381734 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7316,22 +7316,6 @@ static char
 }
 
 
-static char *
-qemuConnectDomainXMLFromNative(virConnectPtr conn,
-   const char *format ATTRIBUTE_UNUSED,
-   const char *config ATTRIBUTE_UNUSED,
-   unsigned int flags)
-{
-virCheckFlags(0, NULL);
-
-if (virConnectDomainXMLFromNativeEnsureACL(conn) < 0)
-return NULL;
-
-virReportError(VIR_ERR_DEPRECATED, "%s",
-   _("converting arbitrary QEMU command lines to libvirt 
domain XML is no longer supported"));
-return NULL;
-}
-
 static char *qemuConnectDomainXMLToNative(virConnectPtr conn,
   const char *format,
   const char *xmlData,
@@ -16772,19 +16756,6 @@ static int qemuDomainQemuMonitorCommand(virDomainPtr 
domain, const char *cmd,
 }
 
 
-static virDomainPtr
-qemuDomainQemuAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
- unsigned int pid_value ATTRIBUTE_UNUSED,
- unsigned int flags)
-{
-virCheckFlags(0, NULL);
-
-virReportError(VIR_ERR_DEPRECATED, "%s",
-   _("attaching to a QEMU process started outside of libvirt 
is no longer supported"));
-return NULL;
-}
-
-
 static int
 qemuDomainOpenConsole(virDomainPtr dom,
   const char *dev_name,
@@ -22271,7 +22242,6 @@ static virHypervisorDriver qemuHypervisorDriver = {
 .domainGetSecurityLabelList = qemuDomainGetSecurityLabelList, /* 0.10.0 */
 .nodeGetSecurityModel = qemuNodeGetSecurityModel, /* 0.6.1 */
 .domainGetXMLDesc = qemuDomainGetXMLDesc, /* 0.2.0 */
-.connectDomainXMLFromNative = qemuConnectDomainXMLFromNative, /* 0.6.4 
(deprecated: 5.5.0) */
 .connectDomainXMLToNative = qemuConnectDomainXMLToNative, /* 0.6.4 */
 .connectListDefinedDomains = qemuConnectListDefinedDomains, /* 0.2.0 */
 .connectNumOfDefinedDomains = qemuConnectNumOfDefinedDomains, /* 0.2.0 */
@@ -22356,7 +22326,6 @@ static virHypervisorDriver qemuHypervisorDriver = {
 .domainRevertToSnapshot = qemuDomainRevertToSnapshot, /* 0.8.0 */
 .domainSnapshotDelete = qemuDomainSnapshotDelete, /* 0.8.0 */
 .domainQemuMonitorCommand = qemuDomainQemuMonitorCommand, /* 0.8.3 */
-.domainQemuAttach = qemuDomainQemuAttach, /* 0.9.4 (deprecated: 5.5.0) */
 .domainQemuAgentCommand = qemuDomainQemuAgentCommand, /* 0.10.0 */
 .connectDomainQemuMonitorEventRegister = 
qemuConnectDomainQemuMonitorEventRegister, /* 1.2.3 */
 .connectDomainQemuMonitorEventDeregister = 
qemuConnectDomainQemuMonitorEventDeregister, /* 1.2.3 */
-- 
2.21.0

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

[libvirt] [PATCH 4/4] Revert "docs: hvsupport: Add support for deprecating hypervisor implementations"

[Daniel P . Berrangé]

This reverts commit fd14dfec88957599ac5cffa28ddb7b513f404d9d.

Signed-off-by: Daniel P. Berrangé 
---
 docs/hvsupport.pl | 44 ++--
 docs/libvirt.css  |  4 
 2 files changed, 14 insertions(+), 34 deletions(-)

diff --git a/docs/hvsupport.pl b/docs/hvsupport.pl
index 2ea245e83a..a2b980c502 100755
--- a/docs/hvsupport.pl
+++ b/docs/hvsupport.pl
@@ -234,11 +234,10 @@ foreach my $src (@srcs) {
 }
 
 } else {
-if ($line =~ 
m!\s*\.(\w+)\s*=\s*(\w+)\s*,?\s*(?:/\*\s*(\d+\.\d+\.\d+)\s*(?:\(deprecated:\s*(\d+\.\d+\.\d+)\))?\s*\*/\s*)?$!)
 {
+if ($line =~ 
m!\s*\.(\w+)\s*=\s*(\w+)\s*,?\s*(?:/\*\s*(\d+\.\d+\.\d+)\s*\*/\s*)?$!) {
 my $api = $1;
 my $meth = $2;
 my $vers = $3;
-my $depre = $4;
 
 next if $api eq "no" || $api eq "name";
 
@@ -252,16 +251,12 @@ foreach my $src (@srcs) {
 die "Found unexpected method $api in $ingrp\n";
 }
 
-$groups{$ingrp}->{drivers}->{$impl}->{$api} = {};
-$groups{$ingrp}->{drivers}->{$impl}->{$api}->{vers} = $vers;
-$groups{$ingrp}->{drivers}->{$impl}->{$api}->{depre}  = $depre;
+$groups{$ingrp}->{drivers}->{$impl}->{$api} = $vers;
 if ($api eq "domainMigratePrepare" ||
 $api eq "domainMigratePrepare2" ||
 $api eq "domainMigratePrepare3") {
-if 
(!$groups{$ingrp}->{drivers}->{$impl}->{"domainMigrate"}) {
-$groups{$ingrp}->{drivers}->{$impl}->{"domainMigrate"} 
= {};
-
$groups{$ingrp}->{drivers}->{$impl}->{"domainMigrate"}->{vers} = $vers;
-}
+$groups{$ingrp}->{drivers}->{$impl}->{"domainMigrate"} = 
$vers
+unless 
$groups{$ingrp}->{drivers}->{$impl}->{"domainMigrate"};
 }
 
 } elsif ($line =~ /}/) {
@@ -285,7 +280,7 @@ $groups{virHypervisorDriver}->{apis}->{"domainMigrate"} = 
"virDomainMigrate";
 my $openAuthVers = (0 * 1000 * 1000) + (4 * 1000) + 0;
 
 foreach my $drv (keys %{$groups{"virHypervisorDriver"}->{drivers}}) {
-my $openVersStr = 
$groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"connectOpen"}->{vers};
+my $openVersStr = 
$groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"connectOpen"};
 my $openVers;
 if ($openVersStr =~ /(\d+)\.(\d+)\.(\d+)/) {
 $openVers = ($1 * 1000 * 1000) + ($2 * 1000) + $3;
@@ -295,16 +290,14 @@ foreach my $drv (keys 
%{$groups{"virHypervisorDriver"}->{drivers}}) {
 $groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"connectOpenReadOnly"} 
=
 $groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"connectOpen"};
 
-$groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"connectOpenAuth"} = 
{};
-
 # virConnectOpenAuth is always 0.4.0 if the driver existed
 # before this time, otherwise it matches the version of
 # the driver's virConnectOpen entry
 if ($openVersStr eq "Y" ||
 $openVers >= $openAuthVers) {
-
$groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"connectOpenAuth"}->{vers} 
= $openVersStr;
+$groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"connectOpenAuth"} 
= $openVersStr;
 } else {
-
$groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"connectOpenAuth"}->{vers} 
= "0.4.0";
+$groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"connectOpenAuth"} 
= "0.4.0";
 }
 }
 
@@ -316,23 +309,21 @@ 
$groups{virHypervisorDriver}->{apis}->{"domainCreateLinux"} = "virDomainCreateLi
 my $createAPIVers = (0 * 1000 * 1000) + (0 * 1000) + 3;
 
 foreach my $drv (keys %{$groups{"virHypervisorDriver"}->{drivers}}) {
-my $createVersStr = 
$groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"domainCreateXML"}->{vers};
+my $createVersStr = 
$groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"domainCreateXML"};
 next unless defined $createVersStr;
 my $createVers;
 if ($createVersStr =~ /(\d+)\.(\d+)\.(\d+)/) {
 $createVers = ($1 * 1000 * 1000) + ($2 * 1000) + $3;
 }
 
-$groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"domainCreateLinux"} = 
{};
-
 # virCreateLinux is always 0.0.3 if the driver existed
 # before this time, otherwise it matches the version of
 # the driver's virCreateXML entry
 if ($createVersStr eq "Y" ||
 $createVers >= $createAPIVers) {
-
$groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"domainCreateLinux"}->{vers}
 = $createVersStr;
+
$groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"domainCreateLinux"} = 
$createVersStr;
 } else {
-
$groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"domainCreateLinux"}->{vers}
 = "0.0.3";
+
$groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"domainCreateLinux"} = 
"0.0.3";
 }
 }
 
@@ -351,9 +342,7 @@ print <
 This page d

[libvirt] [PATCH 0/4] Remove VIR_ERR_DEPRECATED & associated pieces

[Daniel P . Berrangé]

A deprecation is inherantly a warning, not an error, so adding an error
with this name makes no conceptual sense.

Features in drivers may come & go at any time and we've always use the
VIR_ERR_NO_SUPPORT to indicate when a hypervisor driver does not support
a particular API.

Daniel P. Berrangé (4):
  qemu: delete methods which are no longer supported
  Revert "error: Add VIR_ERR_DEPRECATED error code"
  Revert "docs: css: Add style for  ..."
  Revert "docs: hvsupport: Add support for deprecating hypervisor
implementations"

 docs/hvsupport.pl   | 44 -
 docs/libvirt.css| 10 -
 include/libvirt/virterror.h |  1 -
 src/qemu/qemu_driver.c  | 31 --
 src/util/virerror.c |  4 
 5 files changed, 14 insertions(+), 76 deletions(-)

-- 
2.21.0

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

[libvirt] [PATCH 3/4] Revert "docs: css: Add style for ..."

[Daniel P . Berrangé]

This reverts commit 71626402f481ed47ff67dafa8521b01a42625707.

Signed-off-by: Daniel P. Berrangé 
---
 docs/libvirt.css | 6 --
 1 file changed, 6 deletions(-)

diff --git a/docs/libvirt.css b/docs/libvirt.css
index 6639b1df64..54d015db98 100644
--- a/docs/libvirt.css
+++ b/docs/libvirt.css
@@ -274,12 +274,6 @@ span.since {
 font-weight: bold;
 }
 
-span.deprecated {
-color: darkred;
-font-style: italic;
-font-weight: bold;
-}
-
 img.diagram {
 background: rgb(230,230,230);
 border: 2px dotted rgb(178,178,178);
-- 
2.21.0

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

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

[Alex Williamson]

On Wed, 26 Jun 2019 11:58:06 +0200
Cornelia Huck  wrote:

> On Tue, 25 Jun 2019 16:52:51 -0600
> Alex Williamson  wrote:
> 
> > Hi,
> > 
> > Based on the discussions we've had, I've rewritten the bulk of
> > mdevctl.  I think it largely does everything we want now, modulo
> > devices that will need some sort of 1:N values per key for
> > configuration in the config file versus the 1:1 key:value setup we
> > currently have (so don't consider the format final just yet).  
> 
> We might want to factor out that config format handling while we're
> trying to finalize it.
> 
> cc:ing Matt for his awareness. I'm currently not quite sure how to
> handle those vfio-ap "write several values to an attribute one at a
> time" requirements. Maybe 1:N key:value is the way to go; maybe we
> need/want JSON or something like that.

Maybe we should just do JSON for future flexibility.  I assume there
are lots of helpers that should make it easy even from a bash script.
I'll look at that next.

> > We now support "transient" devices and there's no distinction or
> > difference in handling of such devices whether they're created by
> > mdevctl or externally.  All devices will also have systemd management,
> > though systemd is no longer required, it's used if available.  The
> > instance name used for systemd device units has also changed to allow
> > us to use BindsTo= such that services are not only created, but are
> > also removed if the device is removed.  Unfortunately it's not a simple
> > UUID via the systemd route any longer.  
> 
> That's a bit unfortunate; however, making it workable without systemd
> certainly is a good thing :)

The "decoder ring" is simply that the instance value takes the systemd
device path of the mdev device itself.  The mdev device is named by the
uuid and is created under the parent device, so we just need to get the
realpath of the parent, append the uuid, and encode it with
systemd-escape.  It's not magic, but it's  not trivial on the command
line either.  We could add a command to mdevctl to print this, but it
doesn't make much sense to call into mdevctl for that and not simply
control the device via mdevctl.

> > Since the original posting, the project has moved from my personal
> > github to here:
> > 
> > https://github.com/mdevctl/mdevctl
> > 
> > Please see the README there for overview of the new commands and
> > example of their usage.  There is no attempt to maintain backwards
> > compatibility with previous versions, this tool is in its infancy.
> > Also since the original posting, RPM packaging is included, so simply
> > run 'make rpm' and install the resulting package.  
> 
> Nice.
> 
> > 
> > Highlights of this version include proper argument parsing via getopts
> > so that options can be provided in any order.  I'm still using the
> > format 'mdevctl {command} [options]' but now it's consistent that all
> > the options come after the command, in any order.  I think this is
> > relatively consistent with a variety of other tools.  
> 
> Parsing via getops is also very nice.
> 
> > 
> > Devices are no longer automatically persisted, we handle them as
> > transient, but we also can promote them to persistent through the
> > 'define' command.  The define, undefine, and modify commands all
> > operate only on the config file, so that we can define separate from
> > creating.  When promoting from a transient to defined device, we can
> > use the existing device to create the config.  Both the type and the
> > startup of a device can be modified in the config, without affecting
> > the running device.
> > 
> > Starting an mdev device no longer relies exclusively on a saved config,
> > the device can be fully specified via options to create a transient
> > device.  Specifying only a uuid is also sufficient for a defined
> > device.  Some consideration has also been given to uuid collisions.
> > The mdev interface in the kernel prevents multiple mdevs with the same
> > uuid running concurrently, but mdevctl allows mdevs to be defined with
> > the same uuid under separate parent devices.  Some options therefore
> > allow both a uuid and parent to be specified and require this if the
> > uuid alone is ambiguous.  Clearly starting two such devices at the same
> > time will fail and is left to higher level tools to manage, just like
> > the ability to define more devices than there are available instances on
> > the host system.  
> 
> I still have to look into the details of this.
> 
> > 
> > The stop and list commands are largely the same ideas as previous
> > though the semantics are completely different.  Listing running devices
> > now notes which are defined versus transient.  Perhaps it might also be
> > useful when listing defined devices to note which are running.  
> 
> Yes, I think it would be useful.
> 
> > 
> > The sbin/libexec split of mdevctl has been squashed.  There are some
> > commands in the script that are currently only intended to be used from
> > udev or systemd, t

[libvirt] [PATCH] qemu: Validate disk against domain def on coldplug

[Michal Privoznik]

https://bugzilla.redhat.com/show_bug.cgi?id=1692296#c7

This is a counterpart for ddc72f99027 and implements the same
check for coldplug.

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

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d6ab134196..ed5f832f59 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8082,6 +8082,21 @@ qemuDomainUpdateDeviceLive(virDomainObjPtr vm,
 return ret;
 }
 
+
+static int
+qemuCheckDiskConfigAgainstDomain(const virDomainDef *def,
+ const virDomainDiskDef *disk)
+{
+if (virDomainSCSIDriveAddressIsUsed(def, &disk->info.addr.drive)) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("Domain already contains a disk with that address"));
+return -1;
+}
+
+return 0;
+}
+
+
 static int
 qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
  virDomainDeviceDefPtr dev,
@@ -8110,6 +8125,8 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
 return -1;
 if (qemuCheckDiskConfig(disk, NULL) < 0)
 return -1;
+if (qemuCheckDiskConfigAgainstDomain(vmdef, disk) < 0)
+return -1;
 if (virDomainDiskInsert(vmdef, disk) < 0)
 return -1;
 /* vmdef has the pointer. Generic codes for vmdef will do all jobs */
-- 
2.21.0

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

[libvirt] [PATCH] network: avoid including sys/sysctl.h on Linux

[Daniel P . Berrangé]

The sys/sysctl.h header is only needed on BSD platforms to get
the sysctlbyname() function declaration. On Linux we talk to
procfs instead to change sysctls.

Unfortunately a legacy sys/sysctl.h header does exist on Linux
and including it has recently started triggering a deprecation
warning from glibc.

Protect its inclusion with a HAVE_SYSCTLBYNAME check instead
so that it only gets used on platforms where we need that
function declaration.

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

Pushed as build fix for Fedora rawhide

src/network/bridge_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 465487432e..19faf7d514 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -33,7 +33,7 @@
 #include 
 #include 
 #include 
-#if HAVE_SYS_SYSCTL_H
+#ifdef HAVE_SYSCTLBYNAME
 # include 
 #endif
 
-- 
2.21.0

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

Re: [libvirt] [PATCH] libvirt: support block device storage type in virshParseSnapshotDiskspec

[Peter Krempa]

On Wed, Jun 26, 2019 at 14:05:41 +0800, Liu Dayu wrote:
> virsh snapshot-create-as supports 'file' storage type in --diskspec by 
> default.
> It doesn't support 'block' storage type in the process of 
> virshParseSnapshotDiskspec().
> So if a snapshot on a block device (e.g. LV) was created, the type of current 
> running
> storage source in dumpxml is inconsistent with the actual backend storage 
> source.
> It will check file-system type mismatch failed and return an error message of
> 'Migration without shared storage is unsafe' when VM performs a live migration
> after this snapshot.
> 
> Signed-off-by: Liu Dayu 
> ---
>  tools/virsh-snapshot.c | 19 +--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
> index f6bb38b..f9c55e0 100644
> --- a/tools/virsh-snapshot.c
> +++ b/tools/virsh-snapshot.c
> @@ -255,6 +255,8 @@ virshParseSnapshotDiskspec(vshControl *ctl, virBufferPtr 
> buf, const char *str)
>  char **array = NULL;
>  int narray;
>  size_t i;
> +struct stat st;
> +int stor_type = VIR_STORAGE_TYPE_NONE;
>  
>  narray = vshStringToArray(str, &array);
>  if (narray <= 0)
> @@ -272,16 +274,29 @@ virshParseSnapshotDiskspec(vshControl *ctl, 
> virBufferPtr buf, const char *str)
>  goto cleanup;
>  }
>  
> +/* possibly update storage type */
> +if (file && STRPREFIX(file, "/dev/") && stat(file, &st) == 0 && 
> S_ISBLK(st.st_mode))

virsh has to be able to work remotely so this code will definitely not
be acceptable here.

I think we can only go with adding a "type" field for the diskspec
string which will be either "file" or "block" and do this in that case.

e.g.

--diskspec vda,snapshot=external,driver=qcow2,type=block,file=/dev/whatever


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

[libvirt] [PATCH] libvirt: support block device storage type in virshParseSnapshotDiskspec

[Liu Dayu]

virsh snapshot-create-as supports 'file' storage type in --diskspec by default.
It doesn't support 'block' storage type in the process of 
virshParseSnapshotDiskspec().
So if a snapshot on a block device (e.g. LV) was created, the type of current 
running
storage source in dumpxml is inconsistent with the actual backend storage 
source.
It will check file-system type mismatch failed and return an error message of
'Migration without shared storage is unsafe' when VM performs a live migration
after this snapshot.

Signed-off-by: Liu Dayu 
---
 tools/virsh-snapshot.c | 19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index f6bb38b..f9c55e0 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -255,6 +255,8 @@ virshParseSnapshotDiskspec(vshControl *ctl, virBufferPtr 
buf, const char *str)
 char **array = NULL;
 int narray;
 size_t i;
+struct stat st;
+int stor_type = VIR_STORAGE_TYPE_NONE;
 
 narray = vshStringToArray(str, &array);
 if (narray <= 0)
@@ -272,16 +274,29 @@ virshParseSnapshotDiskspec(vshControl *ctl, virBufferPtr 
buf, const char *str)
 goto cleanup;
 }
 
+/* possibly update storage type */
+if (file && STRPREFIX(file, "/dev/") && stat(file, &st) == 0 && 
S_ISBLK(st.st_mode))
+stor_type = VIR_STORAGE_TYPE_BLOCK;
+else
+stor_type = VIR_STORAGE_TYPE_FILE;
+
 virBufferEscapeString(buf, "\n");
 virBufferAdjustIndent(buf, 2);
 if (driver)
 virBufferAsprintf(buf, "\n", driver);
-if (file)
-virBufferEscapeString(buf, "\n", file);
+if (file) {
+if (stor_type == VIR_STORAGE_TYPE_BLOCK)
+virBufferEscapeString(buf, "\n", file);
+else
+virBufferEscapeString(buf, "\n", file);
+}
 virBufferAdjustIndent(buf, -2);
 virBufferAddLit(buf, "\n");
 } else {
-- 
1.8.3.1

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

Re: [libvirt] [PATCH v3 6/7] util: vircgroupv2: don't error out if enabling controller fails

[Pavel Hrdina]

On Wed, Jun 26, 2019 at 10:27:31AM +0200, Peter Krempa wrote:
> On Wed, Jun 26, 2019 at 10:23:25 +0200, Pavel Hrdina wrote:
> > On Wed, Jun 26, 2019 at 09:21:59AM +0200, Peter Krempa wrote:
> > > On Tue, Jun 25, 2019 at 13:16:25 +0200, Pavel Hrdina wrote:
> > > > Currently CPU controller cannot be enabled if there is any real-time
> > > > task running and is assigned to non-root cgroup which is the case on
> > > > several distributions with graphical environment.
> > > > 
> > > > Instead of erroring out treat it as the controller is not available.
> > > > 
> > > > Signed-off-by: Pavel Hrdina 
> > > > ---
> > > >  src/util/vircgroupv2.c | 13 -
> > > >  1 file changed, 12 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
> > > > index 133a8e0e66..348c12d5c6 100644
> > > > --- a/src/util/vircgroupv2.c
> > > > +++ b/src/util/vircgroupv2.c
> > > > @@ -433,6 +433,8 @@ virCgroupV2MakeGroup(virCgroupPtr parent 
> > > > ATTRIBUTE_UNUSED,
> > > >  } else {
> > > >  size_t i;
> > > >  for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
> > > > +int rc;
> > > > +
> > > >  if (!virCgroupV2HasController(parent, i))
> > > >  continue;
> > > >  
> > > > @@ -440,8 +442,17 @@ virCgroupV2MakeGroup(virCgroupPtr parent 
> > > > ATTRIBUTE_UNUSED,
> > > >  if (i == VIR_CGROUP_CONTROLLER_CPUACCT)
> > > >  continue;
> > > >  
> > > > -if (virCgroupV2EnableController(parent, i) < 0)
> > > > +rc = virCgroupV2EnableController(parent, i);
> > > > +if (rc < 0) {
> > > > +if (rc == -2) {
> > > > +virResetLastError();
> > > 
> > > Instead of doing this you should not report the error in the first
> > > place. Given that the refactor in the previous commit adds the error
> > > report in the called function it should be trivial to do so.
> > > 
> > > Without that the logs would be spammed by an error which does not help
> > > the users much.
> > 
> > Works for me, this was based on our in person discussion but I guess
> > there was some misunderstanding.  Do you need v4?
> 
> No, that's not necessary. You can consider that ACK'd

Thanks, all the issues are fixed and pushed now.

Pavel


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

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

[Cornelia Huck]

On Tue, 25 Jun 2019 16:52:51 -0600
Alex Williamson  wrote:

> Hi,
> 
> Based on the discussions we've had, I've rewritten the bulk of
> mdevctl.  I think it largely does everything we want now, modulo
> devices that will need some sort of 1:N values per key for
> configuration in the config file versus the 1:1 key:value setup we
> currently have (so don't consider the format final just yet).

We might want to factor out that config format handling while we're
trying to finalize it.

cc:ing Matt for his awareness. I'm currently not quite sure how to
handle those vfio-ap "write several values to an attribute one at a
time" requirements. Maybe 1:N key:value is the way to go; maybe we
need/want JSON or something like that.

> 
> We now support "transient" devices and there's no distinction or
> difference in handling of such devices whether they're created by
> mdevctl or externally.  All devices will also have systemd management,
> though systemd is no longer required, it's used if available.  The
> instance name used for systemd device units has also changed to allow
> us to use BindsTo= such that services are not only created, but are
> also removed if the device is removed.  Unfortunately it's not a simple
> UUID via the systemd route any longer.

That's a bit unfortunate; however, making it workable without systemd
certainly is a good thing :)

> 
> Since the original posting, the project has moved from my personal
> github to here:
> 
> https://github.com/mdevctl/mdevctl
> 
> Please see the README there for overview of the new commands and
> example of their usage.  There is no attempt to maintain backwards
> compatibility with previous versions, this tool is in its infancy.
> Also since the original posting, RPM packaging is included, so simply
> run 'make rpm' and install the resulting package.

Nice.

> 
> Highlights of this version include proper argument parsing via getopts
> so that options can be provided in any order.  I'm still using the
> format 'mdevctl {command} [options]' but now it's consistent that all
> the options come after the command, in any order.  I think this is
> relatively consistent with a variety of other tools.

Parsing via getops is also very nice.

> 
> Devices are no longer automatically persisted, we handle them as
> transient, but we also can promote them to persistent through the
> 'define' command.  The define, undefine, and modify commands all
> operate only on the config file, so that we can define separate from
> creating.  When promoting from a transient to defined device, we can
> use the existing device to create the config.  Both the type and the
> startup of a device can be modified in the config, without affecting
> the running device.
> 
> Starting an mdev device no longer relies exclusively on a saved config,
> the device can be fully specified via options to create a transient
> device.  Specifying only a uuid is also sufficient for a defined
> device.  Some consideration has also been given to uuid collisions.
> The mdev interface in the kernel prevents multiple mdevs with the same
> uuid running concurrently, but mdevctl allows mdevs to be defined with
> the same uuid under separate parent devices.  Some options therefore
> allow both a uuid and parent to be specified and require this if the
> uuid alone is ambiguous.  Clearly starting two such devices at the same
> time will fail and is left to higher level tools to manage, just like
> the ability to define more devices than there are available instances on
> the host system.

I still have to look into the details of this.

> 
> The stop and list commands are largely the same ideas as previous
> though the semantics are completely different.  Listing running devices
> now notes which are defined versus transient.  Perhaps it might also be
> useful when listing defined devices to note which are running.

Yes, I think it would be useful.

> 
> The sbin/libexec split of mdevctl has been squashed.  There are some
> commands in the script that are currently only intended to be used from
> udev or systemd, these are simply excluded from the help.  It's
> possible we may want to promote the start-parent-mdevs command out of
> this class, but the rest are specifically systemd helpers.
> 
> I'll include the current help test message below for further semantic
> details, but please have a look-see, or better yet give it a try.

Had a quick look, sent two pull requests with some minor tweaks. Still
have to do a proper review, and actually try the thing on an s390.

> Thanks,
> 
> Alex
> 
> PS - I'm looking at adding udev change events when a device registers
> or unregisters with the mdev core, which should help us know when to
> trigger creation of persistent, auto started devices.  That support is
> included here with the MDEV_STATE="registered|unregistered" environment
> values.  Particularly, kvmgt now supports dynamic loading an unloading,
> so as long as the enable_gvt=1 option is provided to the i915 driver
>

Re: [libvirt] [PATCH v2 4/4] qemu: Supply correct default type for 'dir' based VIR_STORAGE_TYPE_VOLUME

[Ján Tomko]

On Tue, Jun 25, 2019 at 04:30:33PM +0200, Peter Krempa wrote:

Our code would skip adding the default type in this cases, but since we
know that the only reasonable option here is 'fat' we can add it while
starting the VM.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_domain.c| 12 +---
tests/qemuxml2argvdata/disk-source-pool.args  |  4 
tests/qemuxml2argvdata/disk-source-pool.xml   |  6 ++
tests/qemuxml2xmloutdata/disk-source-pool.xml |  7 +++
4 files changed, 26 insertions(+), 3 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 2/4] qemu: command: Use 'actualType' when deciding whether to use disk format

[Ján Tomko]

On Tue, Jun 25, 2019 at 04:30:31PM +0200, Peter Krempa wrote:

qemuBuildDriveSourceStr omits the disk format string when we are
emulating a 'fat' filesystem froma directory. The logic should decide


from a


based on the 'actualType' as a disk type=pool may be converted to a
directory.

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



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 3/4] qemu: domain: Allow 'VIR_STORAGE_TYPE_VOLUME' disks with 'fat' format

[Ján Tomko]

On Tue, Jun 25, 2019 at 04:30:32PM +0200, Peter Krempa wrote:

The storage volume may in fact convert into a directory when starting
the VM so that it may be actually possible to use it.

This is a regression caused by c9b27af32d5 as moving the check to
validation time without adjustment causes problems as the volumes are
not translated yet.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_domain.c|  1 +
tests/qemuxml2argvdata/disk-source-pool.args  |  4 
tests/qemuxml2argvdata/disk-source-pool.xml   | 10 +-
tests/qemuxml2xmloutdata/disk-source-pool.xml |  7 +++
4 files changed, 21 insertions(+), 1 deletion(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v2 1/4] util: storage: Fix virStorageSourceGetActualType if volume was not translated

[Ján Tomko]

On Tue, Jun 25, 2019 at 04:30:30PM +0200, Peter Krempa wrote:

virStorageSourceGetActualType would return VIR_STORAGE_TYPE_NONE in case
when a virStorageSource of (top level) type VIR_STORAGE_TYPE_VOLUME was
not prepared to use by the vm by calling
virDomainDiskTranslateSourcePool.

Fix this issue by returning VIR_STORAGE_TYPE_VOLUME in case when the
volume was not translated yet.

Additionally also add documentation for the function describing the
quirk.

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



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] Live migration fails with "Migration without shared storage is unsafe" when using netapp storage on CentOS 7 and Libvirt 4.5.0

[Marko Todorić]

Hello everyone once again.

Regarding an email bellow, i found why is  this happening. Namely I've tried 
updating libvirt and all it's components to version 4.10.0, but still the same 
issue happened. I've reviewed xml files between CentOS 6 hypervisors and new 
one on CentOS 7. And basically this is the difference when i comes to disk 
setup:

CentOS 6 (Libvirt 0.10.2)

  
  
  
  


CentOS 7 (Libvirt 4.10.0)

  
  
  
  


So basically, even if i put disk as a path to block device, libvirt will 
recognize it as a "file" instead of "block" device. When i changed the xml file 
and tried to migrate with options bellow - all worked well!

Now, is this a bug in libvirt causing it not to recognize block device or am i 
doing something wrong? We have been creating VM's in CentOS 7 the same way we 
did in CentOS 6 hypervisors, and this is the way:

virt-install --connect=qemu+ssh://kvm_aquila-01/system \
--name=proba \
--hvm \
--graphics vnc \
--disk path=/dev/mapper/proba,bus=virtio,cache=none \
--vcpus 2 \
--ram=2048 \
--location=http://192.168.54.182/mirror \
--extra-args "ksdevice=eth0 inst.ks=http://192.168.53.99/ks/proba.cfg"; \
--network bridge=br7,model=virtio \
--boot hd,cdrom,menu=on \
--os-type=linux \
--os-variant=rhel7.0 \
--metadata description="LinuxTim - Proba" \
--noreboot

How do we make libvirt automatically default to block device instead of file 
when it comes to disk type?

Srdačan pozdrav,
Marko Todorić
Inženjer za razvoj i održavanje servisa

[cid:part1.25AC1762.814452EE@oriontelekom.rs]

Orion telekom d.o.o. Beograd
Naselje Zemun Polje, Mala pruga 8
11080 Zemun, Srbija
m: +381 64 8379 337
e: marko.todo...@oriontelekom.rs
w: www.oriontelekom.rs
On 6/25/19 5:16 PM, Marko Todoric wrote:
Hello everyone,

We've been using libvirt version 0.10.2 for a long time (on CentOS 6 OS) and 
now we have started creating new KVM cluster all based on CentOS 7 OS and 
libvirt version 4.5.0 (that came with CentOS repo).

TLDR Version:
- When we try to migrate using "virsh migrate test --live 
qemu+ssh://kvm_aquila-02/system" we get an error "error: Unsafe migration: 
Migration without shared storage is unsafe".
- When we try to migrate using same method but adding "--unsafe" migration 
works.
- When we try to migrate a VPS that is using QCOW image on a shared GFS2 image, 
mapped on both servers, migration is successful.
We have two servers in a cluster so far, they are connected to NetApp storage 
and all luns that VPS server should be using are mapped to both servers.

Long version with background:

We have plenty CentOS 6 Servers which work perfectly fine with libvirt version 
up to 0.10.2. All connected also to a NetApp storage and all luns are mapped to 
all servers. LUN's (disks) are visible under /dev/mapper directory. When we 
create VPS, we point to raw disk to the same location, /dev/mapper. For 
example, VPS "test" would use "/dev/mapper/test" for disk with "cache=none" in 
xml file. If we issue "virsh migrate test --live 
qemu+ssh://kvm_server-0X/system", it will successfully migrate VPS to another 
hypervisor without errors, because disk already exists there.
We also have a LUN mapped to each hypervisor and GFS2 established in a cluster 
between hypervisors where we keep qcow images. We can also migrate those 
machines without issues.

On new CentOS 7 hypervisors, we have pretty much the exact same setup. NetApp 
storage connected to servers. All VPS LUN's mapped to all hypervisors. 
Clustered GFS2 volume.
When we define VPS with approximately same XML, it will NOT migrate it and will 
issue out an error stated above. "--unsafe" parameter seems to work fine. BUT.

We want to setup a proper libvirt/kvm cluster with pcs. I've defined a pcs 
resource for each VPS and live migration also doesn't work if i try "pcs 
resource move" command. It will not migrate it "live" as it should, and as it 
does with QCOW images, probably because of the same reason - because it's 
unable to perform live migrate via libvirt without "unsafe". If there would be 
any trick to add "--unsafe" parameter to pcs i would do it, but I'm not aware 
of it.

Still, seems like a bug that libvirt would not realize that netapp storage is 
present in this configuration. Is this maybe corrected in later versions of 
libvirt? Unfortunately, 4.5.0 is latest in CentOS repo but i would try to 
compile it if needed.

Any help would be greatly appreciated and if more info is needed i will be more 
than happy to provide.


Thanks in advance,

Marko Todoric

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

Re: [libvirt] [PATCH 5/9] qemu: monitor: Add support for 'job-cancel' command

[Ján Tomko]

On Mon, Jun 24, 2019 at 05:54:52PM +0200, Peter Krempa wrote:

This belongs to the new job management API which can manage also
non-block based jobs.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_monitor.c  | 12 
src/qemu/qemu_monitor.h  |  4 
src/qemu/qemu_monitor_json.c | 22 ++
src/qemu/qemu_monitor_json.h |  4 
tests/qemumonitorjsontest.c  |  2 ++
5 files changed, 44 insertions(+)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 8/9] qemu: monitor: Implement support for 'JOB_STATUS_CHANGE' event

[Ján Tomko]

On Mon, Jun 24, 2019 at 05:54:55PM +0200, Peter Krempa wrote:

This new event is a superset of the BLOCK_JOB* events and also covers
jobs which don't bind to a VM disk.

In this patch the monitor part is implemented.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_monitor.c  | 13 +
src/qemu/qemu_monitor.h  |  9 +
src/qemu/qemu_monitor_json.c | 26 ++
3 files changed, 48 insertions(+)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 7/9] qemu: monitor: Add infrastructure for 'query-jobs'

[Ján Tomko]

On Mon, Jun 24, 2019 at 05:54:54PM +0200, Peter Krempa wrote:

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_monitor.c   | 23 +
src/qemu/qemu_monitor.h   | 49 +++
src/qemu/qemu_monitor_json.c  | 79 +
src/qemu/qemu_monitor_json.h  |  6 ++
.../query-jobs-create.json| 20 +
.../query-jobs-create.result  | 11 +++
.../qemumonitorjsondata/query-jobs-empty.json |  1 +
.../query-jobs-empty.result   |  0
tests/qemumonitorjsontest.c   | 85 +++
9 files changed, 274 insertions(+)
create mode 100644 tests/qemumonitorjsondata/query-jobs-create.json
create mode 100644 tests/qemumonitorjsondata/query-jobs-create.result
create mode 100644 tests/qemumonitorjsondata/query-jobs-empty.json
create mode 100644 tests/qemumonitorjsondata/query-jobs-empty.result



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 4/4] tests: Enable *-headless and *-graphics in qemuxml2xml

[Andrea Bolognani]

On Wed, 2019-06-26 at 10:07 +0200, Ján Tomko wrote:
> And with this squashed into 4/4:
> 
> diff --git 
> a/tests/qemuxml2xmloutdata/aarch64-virt-graphics.aarch64-latest.xml 
> b/tests/qemuxml2xmloutdata/aarch64-virt-graphics.aarch64-latest.xml
> index bf55bdace5..418fa29def 100644
> --- a/tests/qemuxml2xmloutdata/aarch64-virt-graphics.aarch64-latest.xml
> +++ b/tests/qemuxml2xmloutdata/aarch64-virt-graphics.aarch64-latest.xml
> @@ -12,7 +12,7 @@
>
>  hvm
>   type='pflash'>/usr/share/AAVMF/AAVMF_CODE.fd
> -/home/abologna/.config/libvirt/qemu/nvram/guest_VARS.fd
> +/var/lib/libvirt/qemu/nvram/guest_VARS.fd
>  
>
>
> diff --git 
> a/tests/qemuxml2xmloutdata/aarch64-virt-headless.aarch64-latest.xml 
> b/tests/qemuxml2xmloutdata/aarch64-virt-headless.aarch64-latest.xml
> index 75d86878a1..9b08a03981 100644
> --- a/tests/qemuxml2xmloutdata/aarch64-virt-headless.aarch64-latest.xml
> +++ b/tests/qemuxml2xmloutdata/aarch64-virt-headless.aarch64-latest.xml
> @@ -12,7 +12,7 @@
>
>  hvm
>   type='pflash'>/usr/share/AAVMF/AAVMF_CODE.fd
> -/home/abologna/.config/libvirt/qemu/nvram/guest_VARS.fd
> +/var/lib/libvirt/qemu/nvram/guest_VARS.fd
>  
>
>

Of course! I didn't leave this out, just forgot to mention it :)

Thanks for the review, I'll push in a minute.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 9/9] qemu: monitor: Add APIs for 'blockdev-create'

[Ján Tomko]

On Mon, Jun 24, 2019 at 05:54:56PM +0200, Peter Krempa wrote:

The 'blockdev-create' starts a job which creates a storage volume using
the given protocol or formats an existing (added) volume with one of the
supported storage formats.

This patch adds the monitor interaction bits.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_monitor.c  | 30 ++
src/qemu/qemu_monitor.h  |  4 
src/qemu/qemu_monitor_json.c | 26 ++
src/qemu/qemu_monitor_json.h |  5 +
4 files changed, 65 insertions(+)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 9358d0b1e2..90cd5ea9bd 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4418,6 +4418,36 @@ qemuMonitorSetWatchdogAction(qemuMonitorPtr mon,
}


+/**
+ * qemuMonitorBlockdevCreate:
+ * @mon: monitor object
+ * @jobname: name of the job
+ * @props: JSON object describing the blockdev to add
+ *
+ * Instructs qemu to create/format a new stroage or format layer. Note that
+ * the job does not add the created/formatted image into qemu and
+ * qemuMonitorBlockdevAdd needs to be called separately with corresponding
+ * arguments. Note that the argumetns for creating and adding are different.


*arguments


+ *
+ * Note that @props is always consumed by this function and should not be
+ * accessed after calling this function.
+ */


Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 6/9] qemu: monitor: Add support for 'job-complete' command

[Ján Tomko]

On Mon, Jun 24, 2019 at 05:54:53PM +0200, Peter Krempa wrote:

This belongs to the new job management API which can manage also
non-block based jobs.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_monitor.c  | 12 
src/qemu/qemu_monitor.h  |  4 
src/qemu/qemu_monitor_json.c | 22 ++
src/qemu/qemu_monitor_json.h |  4 
tests/qemumonitorjsontest.c  |  2 ++
5 files changed, 44 insertions(+)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 4/9] qemu: monitor: Add support for 'job-dismiss' command

[Ján Tomko]

On Mon, Jun 24, 2019 at 05:54:51PM +0200, Peter Krempa wrote:

This belongs to the new job management API for generic jobs.

The dismiss command is meant to remove a concluded job after we were
able to get the final status.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_monitor.c  | 12 
src/qemu/qemu_monitor.h  |  4 
src/qemu/qemu_monitor_json.c | 22 ++
src/qemu/qemu_monitor_json.h |  4 
tests/qemumonitorjsontest.c  |  2 ++
5 files changed, 44 insertions(+)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 3/9] qemu: monitor: Add new fields for 'blockdev-mirror' command

[Ján Tomko]

On Mon, Jun 24, 2019 at 05:54:50PM +0200, Peter Krempa wrote:

Allow using the delayed dismiss of the job so that we can reap the state
even if libvirtd was not running when qemu emitted the job completion
event.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_migration.c|  2 +-
src/qemu/qemu_monitor.c  |  9 +
src/qemu/qemu_monitor.h  |  3 ++-
src/qemu/qemu_monitor_json.c | 10 ++
src/qemu/qemu_monitor_json.h |  3 ++-
tests/qemumonitorjsontest.c  |  2 +-
6 files changed, 21 insertions(+), 8 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 1/9] qemu: monitor: Add new fields for 'block-stream' command

[Ján Tomko]

On Mon, Jun 24, 2019 at 05:54:48PM +0200, Peter Krempa wrote:

Allow using the node name to specify the base of the 'stream' operation,
allow specifying explicit job name and add support for delayed dismiss
of the job so that we can reap the state even if libvirtd was not
running when qemu emitted the job completion event.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_driver.c   |  4 ++--
src/qemu/qemu_monitor.c  | 18 +++---
src/qemu/qemu_monitor.h  |  3 +++
src/qemu/qemu_monitor_json.c | 14 ++
src/qemu/qemu_monitor_json.h |  3 +++
tests/qemumonitorjsontest.c  |  2 +-
6 files changed, 38 insertions(+), 6 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 2/9] qemu: monitor: Add new fields for 'block-commit' command

[Ján Tomko]

On Mon, Jun 24, 2019 at 05:54:49PM +0200, Peter Krempa wrote:

Allow using the node name to specify the base and top of the 'commit'
operation, allow specifying explicit job name and add support for
delayed dismiss of the job so that we can reap the state even if
libvirtd was not running when qemu emitted the job completion event.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_driver.c   |  4 ++--
src/qemu/qemu_monitor.c  | 21 +++--
src/qemu/qemu_monitor.h  |  6 +-
src/qemu/qemu_monitor_json.c | 22 --
src/qemu/qemu_monitor_json.h |  4 
tests/qemumonitorjsontest.c  |  2 +-
6 files changed, 47 insertions(+), 12 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v3 6/7] util: vircgroupv2: don't error out if enabling controller fails

[Peter Krempa]

On Wed, Jun 26, 2019 at 10:23:25 +0200, Pavel Hrdina wrote:
> On Wed, Jun 26, 2019 at 09:21:59AM +0200, Peter Krempa wrote:
> > On Tue, Jun 25, 2019 at 13:16:25 +0200, Pavel Hrdina wrote:
> > > Currently CPU controller cannot be enabled if there is any real-time
> > > task running and is assigned to non-root cgroup which is the case on
> > > several distributions with graphical environment.
> > > 
> > > Instead of erroring out treat it as the controller is not available.
> > > 
> > > Signed-off-by: Pavel Hrdina 
> > > ---
> > >  src/util/vircgroupv2.c | 13 -
> > >  1 file changed, 12 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
> > > index 133a8e0e66..348c12d5c6 100644
> > > --- a/src/util/vircgroupv2.c
> > > +++ b/src/util/vircgroupv2.c
> > > @@ -433,6 +433,8 @@ virCgroupV2MakeGroup(virCgroupPtr parent 
> > > ATTRIBUTE_UNUSED,
> > >  } else {
> > >  size_t i;
> > >  for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
> > > +int rc;
> > > +
> > >  if (!virCgroupV2HasController(parent, i))
> > >  continue;
> > >  
> > > @@ -440,8 +442,17 @@ virCgroupV2MakeGroup(virCgroupPtr parent 
> > > ATTRIBUTE_UNUSED,
> > >  if (i == VIR_CGROUP_CONTROLLER_CPUACCT)
> > >  continue;
> > >  
> > > -if (virCgroupV2EnableController(parent, i) < 0)
> > > +rc = virCgroupV2EnableController(parent, i);
> > > +if (rc < 0) {
> > > +if (rc == -2) {
> > > +virResetLastError();
> > 
> > Instead of doing this you should not report the error in the first
> > place. Given that the refactor in the previous commit adds the error
> > report in the called function it should be trivial to do so.
> > 
> > Without that the logs would be spammed by an error which does not help
> > the users much.
> 
> Works for me, this was based on our in person discussion but I guess
> there was some misunderstanding.  Do you need v4?

No, that's not necessary. You can consider that ACK'd



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

Re: [libvirt] [PATCH v3 2/7] util: vircgroup: improve controller detection

[Peter Krempa]

On Tue, Jun 25, 2019 at 13:16:21 +0200, Pavel Hrdina wrote:
> This affects only cgroups v2 where enabled controllers are not based on
> available mount points but on the list provided in cgroup.controllers
> file.  However, moving it will fill in placement as well, so it needs
> to be freed together with mount point if we don't need that controller.
> 
> Before this patch we were assuming that all controllers available in
> root cgroup where available in all other sub-cgroups which was wrong.
> 
> In order to fix it we need to move the cgroup controllers detection
> after cgroup placement was prepared in order to build correct path for
> cgroup.controllers file.
> 
> Signed-off-by: Pavel Hrdina 
> ---
> 
> Notes:
> Changes in v3:
> - Moving the detection after placement means we need to free it
>   together with mount point if that controller is not required.

ACK


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

Re: [libvirt] [PATCH v3 6/7] util: vircgroupv2: don't error out if enabling controller fails

[Pavel Hrdina]

On Wed, Jun 26, 2019 at 09:21:59AM +0200, Peter Krempa wrote:
> On Tue, Jun 25, 2019 at 13:16:25 +0200, Pavel Hrdina wrote:
> > Currently CPU controller cannot be enabled if there is any real-time
> > task running and is assigned to non-root cgroup which is the case on
> > several distributions with graphical environment.
> > 
> > Instead of erroring out treat it as the controller is not available.
> > 
> > Signed-off-by: Pavel Hrdina 
> > ---
> >  src/util/vircgroupv2.c | 13 -
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
> > index 133a8e0e66..348c12d5c6 100644
> > --- a/src/util/vircgroupv2.c
> > +++ b/src/util/vircgroupv2.c
> > @@ -433,6 +433,8 @@ virCgroupV2MakeGroup(virCgroupPtr parent 
> > ATTRIBUTE_UNUSED,
> >  } else {
> >  size_t i;
> >  for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
> > +int rc;
> > +
> >  if (!virCgroupV2HasController(parent, i))
> >  continue;
> >  
> > @@ -440,8 +442,17 @@ virCgroupV2MakeGroup(virCgroupPtr parent 
> > ATTRIBUTE_UNUSED,
> >  if (i == VIR_CGROUP_CONTROLLER_CPUACCT)
> >  continue;
> >  
> > -if (virCgroupV2EnableController(parent, i) < 0)
> > +rc = virCgroupV2EnableController(parent, i);
> > +if (rc < 0) {
> > +if (rc == -2) {
> > +virResetLastError();
> 
> Instead of doing this you should not report the error in the first
> place. Given that the refactor in the previous commit adds the error
> report in the called function it should be trivial to do so.
> 
> Without that the logs would be spammed by an error which does not help
> the users much.

Works for me, this was based on our in person discussion but I guess
there was some misunderstanding.  Do you need v4?

Pavel


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

Re: [libvirt] [PATCH v3 7/7] util: vircgroupv2: mark only requested controllers as available

[Peter Krempa]

On Tue, Jun 25, 2019 at 13:16:26 +0200, Pavel Hrdina wrote:
> When detecting available controllers on host we can be limited by list
> of controllers from qemu.conf file.
> 
> Signed-off-by: Pavel Hrdina 
> ---
> 
> Notes:
> Introduced in v3
> 
>  src/util/vircgroupv2.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
> index 348c12d5c6..fd883f3c7f 100644
> --- a/src/util/vircgroupv2.c
> +++ b/src/util/vircgroupv2.c
> @@ -302,15 +302,15 @@ virCgroupV2DetectControllers(virCgroupPtr group,
>  group->unified.controllers |= 1 << VIR_CGROUP_CONTROLLER_CPUACCT;
>  }
>  
> +if (controllers >= 0)
> +group->unified.controllers &= controllers;

The use of 'int' here for 'controllers' makes it super non-obvious and
super sketchy in what's happening here. Especially since you are then
doing bitwise operations with it afterwards.

It's pre-existing though. Still super ugly.

> +
>  for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++)
>  VIR_DEBUG("Controller '%s' present=%s",
>virCgroupV2ControllerTypeToString(i),
>(group->unified.controllers & 1 << i) ? "yes" : "no");
>  
> -if (controllers >= 0)
> -return controllers & group->unified.controllers;
> -else
> -return group->unified.controllers;
> +return group->unified.controllers;
>  }

ACK


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

Re: [libvirt] [PATCH 4/4] tests: Enable *-headless and *-graphics in qemuxml2xml

[Ján Tomko]

On Wed, Jun 26, 2019 at 09:58:51AM +0200, Andrea Bolognani wrote:

On Tue, 2019-06-25 at 20:38 +0200, Ján Tomko wrote:

On Fri, Jun 14, 2019 at 10:04:39AM +0200, Andrea Bolognani wrote:

[...]

> +/usr/share/AAVMF/AAVMF_CODE.fd
> +/home/abologna/.config/libvirt/qemu/nvram/guest_VARS.fd

Haha, nope.
928) QEMU XML-2-XML-inactive aarch64-virt-graphics ...
In 
'/home/jtomko/work/libvirt/tests/qemuxml2xmloutdata/aarch64-virt-graphics.aarch64-latest.xml':
Offset 582
Expect [abologna]
Actual [jtomko]
  ... 
Expected result code=0 but received code=4FAILED


Just rename your home directory and it will pass O:-)

Anyway, if I squash the diff below into 2/4 this problem will go
away. I'm a bit confused by the fact that it would only show up with
xml2xml and not with xml2argv, which generates the correct NVRAM
path just as I would expect... I'd have to dig a bit further to
figure out what's going on there.

In the meantime, are you okay with this going in once I've squashed
the diff below into 2/4?


diff --git a/tests/qemuxml2argvdata/aarch64-virt-graphics.xml 
b/tests/qemuxml2argvdata/aarch64-virt-graphics.xml
index 490eb45daf..e908d24ac2 100644
--- a/tests/qemuxml2argvdata/aarch64-virt-graphics.xml
+++ b/tests/qemuxml2argvdata/aarch64-virt-graphics.xml
@@ -12,6 +12,7 @@
  
hvm
/usr/share/AAVMF/AAVMF_CODE.fd
+/var/lib/libvirt/qemu/nvram/guest_VARS.fd

  
  
diff --git a/tests/qemuxml2argvdata/aarch64-virt-headless.xml 
b/tests/qemuxml2argvdata/aarch64-virt-headless.xml
index 2ed7d9a904..7e4fe64b43 100644
--- a/tests/qemuxml2argvdata/aarch64-virt-headless.xml
+++ b/tests/qemuxml2argvdata/aarch64-virt-headless.xml
@@ -12,6 +12,7 @@
  
hvm
/usr/share/AAVMF/AAVMF_CODE.fd
+/var/lib/libvirt/qemu/nvram/guest_VARS.fd

  
  


And with this squashed into 4/4:

diff --git a/tests/qemuxml2xmloutdata/aarch64-virt-graphics.aarch64-latest.xml 
b/tests/qemuxml2xmloutdata/aarch64-virt-graphics.aarch64-latest.xml
index bf55bdace5..418fa29def 100644
--- a/tests/qemuxml2xmloutdata/aarch64-virt-graphics.aarch64-latest.xml
+++ b/tests/qemuxml2xmloutdata/aarch64-virt-graphics.aarch64-latest.xml
@@ -12,7 +12,7 @@
  
hvm
/usr/share/AAVMF/AAVMF_CODE.fd
-/home/abologna/.config/libvirt/qemu/nvram/guest_VARS.fd
+/var/lib/libvirt/qemu/nvram/guest_VARS.fd

  
  
diff --git a/tests/qemuxml2xmloutdata/aarch64-virt-headless.aarch64-latest.xml 
b/tests/qemuxml2xmloutdata/aarch64-virt-headless.aarch64-latest.xml
index 75d86878a1..9b08a03981 100644
--- a/tests/qemuxml2xmloutdata/aarch64-virt-headless.aarch64-latest.xml
+++ b/tests/qemuxml2xmloutdata/aarch64-virt-headless.aarch64-latest.xml
@@ -12,7 +12,7 @@
  
hvm
/usr/share/AAVMF/AAVMF_CODE.fd
-/home/abologna/.config/libvirt/qemu/nvram/guest_VARS.fd
+/var/lib/libvirt/qemu/nvram/guest_VARS.fd

  
  

Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 4/4] tests: Enable *-headless and *-graphics in qemuxml2xml

[Andrea Bolognani]

On Tue, 2019-06-25 at 20:38 +0200, Ján Tomko wrote:
> On Fri, Jun 14, 2019 at 10:04:39AM +0200, Andrea Bolognani wrote:
[...]
> > + > type='pflash'>/usr/share/AAVMF/AAVMF_CODE.fd
> > +/home/abologna/.config/libvirt/qemu/nvram/guest_VARS.fd
> 
> Haha, nope.
> 928) QEMU XML-2-XML-inactive aarch64-virt-graphics ... 
> In 
> '/home/jtomko/work/libvirt/tests/qemuxml2xmloutdata/aarch64-virt-graphics.aarch64-latest.xml':
> Offset 582
> Expect [abologna]
> Actual [jtomko]
>   ... 
> Expected result code=0 but received code=4FAILED

Just rename your home directory and it will pass O:-)

Anyway, if I squash the diff below into 2/4 this problem will go
away. I'm a bit confused by the fact that it would only show up with
xml2xml and not with xml2argv, which generates the correct NVRAM
path just as I would expect... I'd have to dig a bit further to
figure out what's going on there.

In the meantime, are you okay with this going in once I've squashed
the diff below into 2/4?


diff --git a/tests/qemuxml2argvdata/aarch64-virt-graphics.xml 
b/tests/qemuxml2argvdata/aarch64-virt-graphics.xml
index 490eb45daf..e908d24ac2 100644
--- a/tests/qemuxml2argvdata/aarch64-virt-graphics.xml
+++ b/tests/qemuxml2argvdata/aarch64-virt-graphics.xml
@@ -12,6 +12,7 @@
   
 hvm
 /usr/share/AAVMF/AAVMF_CODE.fd
+/var/lib/libvirt/qemu/nvram/guest_VARS.fd
 
   
   
diff --git a/tests/qemuxml2argvdata/aarch64-virt-headless.xml 
b/tests/qemuxml2argvdata/aarch64-virt-headless.xml
index 2ed7d9a904..7e4fe64b43 100644
--- a/tests/qemuxml2argvdata/aarch64-virt-headless.xml
+++ b/tests/qemuxml2argvdata/aarch64-virt-headless.xml
@@ -12,6 +12,7 @@
   
 hvm
 /usr/share/AAVMF/AAVMF_CODE.fd
+/var/lib/libvirt/qemu/nvram/guest_VARS.fd
 
   
   
-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 03/19] qemu_firmware: only set nfeatures on success

[Ján Tomko]

On Wed, Jun 05, 2019 at 12:31:01PM +0200, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

The field is set just before returning on success.

Signed-off-by: Marc-André Lureau 
---
src/qemu/qemu_firmware.c | 2 --
1 file changed, 2 deletions(-)



Reviewed-by: Ján Tomko 
and I pushed the first three patches already.

Jano


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

Re: [libvirt] [PATCH v3 5/7] util: vircgroupv2: separate return values of virCgroupV2EnableController

[Peter Krempa]

On Tue, Jun 25, 2019 at 13:16:24 +0200, Pavel Hrdina wrote:
> In order to skip controllers that we are not able to activate we need
> to return different return value so the caller can decide what to do.
> 
> Signed-off-by: Pavel Hrdina 
> ---
> 
> Notes:
> Introduced in v2

ACK although based on the review of the next patch you probably also
want to add a boolean to suppres the error reporting in case of a known
ignorable error.


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

Re: [libvirt] [PATCH 02/19] tpm: minor argument comment fix

[Ján Tomko]

On Wed, Jun 05, 2019 at 12:31:00PM +0200, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
---
src/qemu/qemu_tpm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 835a9caf46..41fef1f8f5 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -745,7 +745,7 @@ qemuExtTPMCleanupHost(virDomainDefPtr def)
 * qemuExtTPMStartEmulator:
 *
 * @driver: QEMU driver
- * @def: domain definition
+ * @vm: the VM domain


s/VM domain/domain object/


 * @logCtxt: log context
 *
 * Start the external TPM Emulator:


Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 01/19] device-conf: removed unneeded virDomainDeviceInfoCopy()

[Ján Tomko]

On Wed, Jun 05, 2019 at 12:30:59PM +0200, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
---
src/conf/device_conf.c   | 21 -
src/conf/device_conf.h   |  2 --
src/libvirt_private.syms |  1 -
3 files changed, 24 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v3 6/7] util: vircgroupv2: don't error out if enabling controller fails

[Peter Krempa]

On Tue, Jun 25, 2019 at 13:16:25 +0200, Pavel Hrdina wrote:
> Currently CPU controller cannot be enabled if there is any real-time
> task running and is assigned to non-root cgroup which is the case on
> several distributions with graphical environment.
> 
> Instead of erroring out treat it as the controller is not available.
> 
> Signed-off-by: Pavel Hrdina 
> ---
>  src/util/vircgroupv2.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
> index 133a8e0e66..348c12d5c6 100644
> --- a/src/util/vircgroupv2.c
> +++ b/src/util/vircgroupv2.c
> @@ -433,6 +433,8 @@ virCgroupV2MakeGroup(virCgroupPtr parent ATTRIBUTE_UNUSED,
>  } else {
>  size_t i;
>  for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
> +int rc;
> +
>  if (!virCgroupV2HasController(parent, i))
>  continue;
>  
> @@ -440,8 +442,17 @@ virCgroupV2MakeGroup(virCgroupPtr parent 
> ATTRIBUTE_UNUSED,
>  if (i == VIR_CGROUP_CONTROLLER_CPUACCT)
>  continue;
>  
> -if (virCgroupV2EnableController(parent, i) < 0)
> +rc = virCgroupV2EnableController(parent, i);
> +if (rc < 0) {
> +if (rc == -2) {
> +virResetLastError();

Instead of doing this you should not report the error in the first
place. Given that the refactor in the previous commit adds the error
report in the called function it should be trivial to do so.

Without that the logs would be spammed by an error which does not help
the users much.


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

Re: [libvirt] [PATCH libvirt-python] Fix regression in lxcOpenNamespace

[Ján Tomko]

On Wed, Jun 26, 2019 at 11:27:45AM +1000, Sergei Turchanov wrote:

This fixes regression caused by the 1d39dbaf637db03f6e597ed56b96aa065710b4a1

fdlist[i] erroneously was replaced by fdlist[1] which caused
lxcOpenNamespace to return a list with identical elements.

Signed-off-by: Sergei Turchanov 
---
libvirt-lxc-override.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)



Reviewed-by: Ján Tomko 
and pushed now.

Jano


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