[libvirt] [PATCH] events: Remove ATTRIBUTE_NONNULL for virObjectEventStateQueue[Remote]

2018-06-15 Thread John Ferlan
Commit aad3a0b5f altered virObjectEventStateQueueRemote to move
the "if (!event) return" call added in the previous commit 031eb8f6
to virObjectEventStateQueue. Neither commit altered the function
prototype which used ATTRIBUTE_NONNULL(2).

This caused Coverity build problems. Since @event is now checked,
just remove the ATTRIBUTE_NONNULL check from both prototypes.

Signed-off-by: John Ferlan 
---
 src/conf/object_event.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

 Although a Coverity build breaker, that doesn't affect everyone so
 I'll wait for R-By or ACK for push.


diff --git a/src/conf/object_event.h b/src/conf/object_event.h
index 133f7ed914..70e9579e81 100644
--- a/src/conf/object_event.h
+++ b/src/conf/object_event.h
@@ -62,13 +62,13 @@ typedef void 
(*virConnectObjectEventGenericCallback)(virConnectPtr conn,
 void
 virObjectEventStateQueue(virObjectEventStatePtr state,
  virObjectEventPtr event)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+ATTRIBUTE_NONNULL(1);
 
 void
 virObjectEventStateQueueRemote(virObjectEventStatePtr state,
virObjectEventPtr event,
int remoteID)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+ATTRIBUTE_NONNULL(1);
 
 int
 virObjectEventStateDeregisterID(virConnectPtr conn,
-- 
2.14.4

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


[libvirt] [dockerfiles PATCH] README: Provide information about the images

2018-06-15 Thread Andrea Bolognani
Signed-off-by: Andrea Bolognani 
---
Note that this document will be displayed both for the
repository containing the Dockerfiles

  https://github.com/libvirt/libvirt-dockerfiles

and for the images produced by said Dockerfiles, eg.

  https://hub.docker.com/r/libvirt/buildenv-centos-7/

so the language is intentionally vague at times. I believe
I've achieved a decent balance between the needs of the two
scenarios, but I'm of course totally open to feedback :)

 README.md | 50 ++
 1 file changed, 50 insertions(+)

diff --git a/README.md b/README.md
index a84caf3..ccd7ad1 100644
--- a/README.md
+++ b/README.md
@@ -1,2 +1,52 @@
 Docker-based build environments for libvirt
 ===
+
+These images come with all libvirt build dependencies, including
+optional ones, already installed: this makes it possible to run
+something like
+
+$ docker run \
+  -v $(pwd):/libvirt \
+  -w /libvirt \
+  -it \
+  buildenv-centos-7
+
+from a git clone and start building libvirt right away.
+
+Image availability is influenced by libvirt's
+[platform support policy](https://libvirt.org/platforms.html),
+with the obvious caveat that non-Linux operating systems can't
+be run on top of a Linux kernel and as such are not included.
+
+
+Intended use
+
+
+The images are primarily intended for use on
+[Travis CI](https://travis-ci.org/libvirt/libvirt).
+
+The primary CI environment for the libvirt project is hosted on
+[CentOS CI](https://ci.centos.org/view/libvirt/); however, since
+that environment feeds off the `master` branch of the various
+projects, it can only detect issues after the code causing them
+has already been merged.
+
+While testing on Travis CI doesn't cover as many platforms or the
+interactions between as many components, it can be very useful as
+a smoke test of sorts that allows developers to catch mistakes
+before posting patches to the mailing list.
+
+As an alternative, images can be used locally without relying on
+third-party services; in this scenario, the number of platforms
+patches are tested against is only limited by image availability
+and hardware resources.
+
+
+Information about build dependencies
+
+
+The list of build dependencies for libvirt (as well as many
+other virtualization-related projects) is taken from the
+[libvirt-jenkins-ci](https://libvirt.org/git/?p=libvirt-jenkins-ci.git)
+repository, which also contains the tooling used to generate
+Dockerfiles.
-- 
2.17.1

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


[libvirt] [dbus PATCH 09/14] Implement GetXMLDesc property for NodeDevice Interface

2018-06-15 Thread Katerina Koukiou
Signed-off-by: Katerina Koukiou 
---
 data/org.libvirt.NodeDevice.xml |  6 ++
 src/nodedev.c   | 28 
 tests/test_nodedev.py   |  6 ++
 3 files changed, 40 insertions(+)

diff --git a/data/org.libvirt.NodeDevice.xml b/data/org.libvirt.NodeDevice.xml
index c4b4075..1185bca 100644
--- a/data/org.libvirt.NodeDevice.xml
+++ b/data/org.libvirt.NodeDevice.xml
@@ -22,5 +22,11 @@
 value="See 
https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceDetachFlags"/>
   
 
+
+  https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceGetXMLDesc"/>
+  
+  
+
   
 
diff --git a/src/nodedev.c b/src/nodedev.c
index c597411..e61242d 100644
--- a/src/nodedev.c
+++ b/src/nodedev.c
@@ -110,6 +110,33 @@ virtDBusNodeDeviceDetach(GVariant *inArgs,
 virtDBusUtilSetLastVirtError(error);
 }
 
+static void
+virtDBusNodeDeviceGetXMLDesc(GVariant *inArgs,
+ GUnixFDList *inFDs G_GNUC_UNUSED,
+ const gchar *objectPath,
+ gpointer userData,
+ GVariant **outArgs,
+ GUnixFDList **outFDs G_GNUC_UNUSED,
+ GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virNodeDevice) dev = NULL;
+g_autofree gchar *xml = NULL;
+guint flags;
+
+g_variant_get(inArgs, "(u)", &flags);
+
+dev = virtDBusNodeDeviceGetVirNodeDevice(connect, objectPath, error);
+if (!dev)
+return;
+
+xml = virNodeDeviceGetXMLDesc(dev, flags);
+if (!xml)
+return virtDBusUtilSetLastVirtError(error);
+
+*outArgs = g_variant_new("(s)", xml);
+}
+
 static virtDBusGDBusPropertyTable virtDBusNodeDevicePropertyTable[] = {
 { "Name", virtDBusNodeDeviceGetName, NULL },
 { "Parent", virtDBusNodeDeviceGetParent, NULL },
@@ -119,6 +146,7 @@ static virtDBusGDBusPropertyTable 
virtDBusNodeDevicePropertyTable[] = {
 static virtDBusGDBusMethodTable virtDBusNodeDeviceMethodTable[] = {
 { "Destroy", virtDBusNodeDeviceDestroy },
 { "Detach", virtDBusNodeDeviceDetach },
+{ "GetXMLDesc", virtDBusNodeDeviceGetXMLDesc },
 { 0 }
 };
 
diff --git a/tests/test_nodedev.py b/tests/test_nodedev.py
index 39343ae..64c97b2 100755
--- a/tests/test_nodedev.py
+++ b/tests/test_nodedev.py
@@ -26,6 +26,12 @@ class TestNodeDevice(libvirttest.BaseTestClass):
 
 self.main_loop()
 
+def test_node_device_get_xml_description(self):
+test_node_device_path = self.node_device_create()
+obj = self.bus.get_object('org.libvirt', test_node_device_path)
+interface_obj = dbus.Interface(obj, 'org.libvirt.NodeDevice')
+assert isinstance(interface_obj.GetXMLDesc(0), dbus.String)
+
 def test_node_device_properties_type(self):
 """ Ensure correct return type for NodeDevice properties
 """
-- 
2.15.0

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


[libvirt] [dbus PATCH 12/14] Implement NodeDeviceLookupSCSIHostByWWN method for Connect Interface

2018-06-15 Thread Katerina Koukiou
Signed-off-by: Katerina Koukiou 
---
 data/org.libvirt.Connect.xml |  8 
 src/connect.c| 32 
 2 files changed, 40 insertions(+)

diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index a030d57..7014a09 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -221,6 +221,14 @@
   
   
 
+
+  https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceLookupSCSIHostByWWN"/>
+  
+  
+  
+  
+
 
   https://libvirt.org/html/libvirt-libvirt-nwfilter.html#virNWFilterDefineXML"/>
diff --git a/src/connect.c b/src/connect.c
index 6192f47..e1406dd 100644
--- a/src/connect.c
+++ b/src/connect.c
@@ -1126,6 +1126,37 @@ virtDBusConnectNodeDeviceLookupByName(GVariant *inArgs,
 *outArgs = g_variant_new("(o)", path);
 }
 
+static void
+virtDBusConnectNodeDeviceLookupSCSIHostByWWN(GVariant *inArgs,
+ GUnixFDList *inFDs G_GNUC_UNUSED,
+ const gchar *objectPath 
G_GNUC_UNUSED,
+ gpointer userData,
+ GVariant **outArgs,
+ GUnixFDList **outFDs 
G_GNUC_UNUSED,
+ GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virNodeDevice) dev = NULL;
+g_autofree gchar *path = NULL;
+const gchar *wwnn;
+const gchar *wwpn;
+guint flags;
+
+g_variant_get(inArgs, "(&s&su)", &wwnn, &wwpn, &flags);
+
+if (!virtDBusConnectOpen(connect, error))
+return;
+
+dev = virNodeDeviceLookupSCSIHostByWWN(connect->connection, wwnn, wwpn,
+   flags);
+if (!dev)
+return virtDBusUtilSetLastVirtError(error);
+
+path = virtDBusUtilBusPathForVirNodeDevice(dev, connect->devPath);
+
+*outArgs = g_variant_new("(o)", path);
+}
+
 static void
 virtDBusConnectNWFilterDefineXML(GVariant *inArgs,
  GUnixFDList *inFDs G_GNUC_UNUSED,
@@ -1746,6 +1777,7 @@ static virtDBusGDBusMethodTable 
virtDBusConnectMethodTable[] = {
 { "NetworkLookupByUUID", virtDBusConnectNetworkLookupByUUID },
 { "NodeDeviceCreateXML", virtDBusConnectNodeDeviceCreateXML },
 { "NodeDeviceLookupByName", virtDBusConnectNodeDeviceLookupByName },
+{ "NodeDeviceLookupSCSIHostByWWN", 
virtDBusConnectNodeDeviceLookupSCSIHostByWWN },
 { "NWFilterDefineXML", virtDBusConnectNWFilterDefineXML },
 { "NWFilterLookupByName", virtDBusConnectNWFilterLookupByName },
 { "NWFilterLookupByUUID", virtDBusConnectNWFilterLookupByUUID },
-- 
2.15.0

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


[libvirt] [dbus PATCH 11/14] Implement NodeDeviceLookupByName method for Connect Interface

2018-06-15 Thread Katerina Koukiou
Signed-off-by: Katerina Koukiou 
---
 data/org.libvirt.Connect.xml |  6 ++
 src/connect.c| 29 +
 tests/test_connect.py| 13 +
 3 files changed, 48 insertions(+)

diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index d8b0d9e..a030d57 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -215,6 +215,12 @@
   
   
 
+
+  https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceLookupByName"/>
+  
+  
+
 
   https://libvirt.org/html/libvirt-libvirt-nwfilter.html#virNWFilterDefineXML"/>
diff --git a/src/connect.c b/src/connect.c
index 5e146a6..6192f47 100644
--- a/src/connect.c
+++ b/src/connect.c
@@ -1098,6 +1098,34 @@ virtDBusConnectNodeDeviceCreateXML(GVariant *inArgs,
 *outArgs = g_variant_new("(o)", path);
 }
 
+static void
+virtDBusConnectNodeDeviceLookupByName(GVariant *inArgs,
+  GUnixFDList *inFDs G_GNUC_UNUSED,
+  const gchar *objectPath G_GNUC_UNUSED,
+  gpointer userData,
+  GVariant **outArgs,
+  GUnixFDList **outFDs G_GNUC_UNUSED,
+  GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virNodeDevice) dev = NULL;
+g_autofree gchar *path = NULL;
+const gchar *name;
+
+g_variant_get(inArgs, "(s)", &name);
+
+if (!virtDBusConnectOpen(connect, error))
+return;
+
+dev = virNodeDeviceLookupByName(connect->connection, name);
+if (!dev)
+return virtDBusUtilSetLastVirtError(error);
+
+path = virtDBusUtilBusPathForVirNodeDevice(dev, connect->devPath);
+
+*outArgs = g_variant_new("(o)", path);
+}
+
 static void
 virtDBusConnectNWFilterDefineXML(GVariant *inArgs,
  GUnixFDList *inFDs G_GNUC_UNUSED,
@@ -1717,6 +1745,7 @@ static virtDBusGDBusMethodTable 
virtDBusConnectMethodTable[] = {
 { "NetworkLookupByName", virtDBusConnectNetworkLookupByName },
 { "NetworkLookupByUUID", virtDBusConnectNetworkLookupByUUID },
 { "NodeDeviceCreateXML", virtDBusConnectNodeDeviceCreateXML },
+{ "NodeDeviceLookupByName", virtDBusConnectNodeDeviceLookupByName },
 { "NWFilterDefineXML", virtDBusConnectNWFilterDefineXML },
 { "NWFilterLookupByName", virtDBusConnectNWFilterLookupByName },
 { "NWFilterLookupByUUID", virtDBusConnectNWFilterLookupByUUID },
diff --git a/tests/test_connect.py b/tests/test_connect.py
index e532016..7084397 100755
--- a/tests/test_connect.py
+++ b/tests/test_connect.py
@@ -143,6 +143,19 @@ class TestConnect(libvirttest.BaseTestClass):
 
 self.main_loop()
 
+@pytest.mark.usefixtures("node_device_create")
+@pytest.mark.parametrize("lookup_method_name,lookup_item", [
+("NodeDeviceLookupByName", 'Name'),
+])
+def test_connect_node_device_lookup_by_property(self, lookup_method_name, 
lookup_item):
+"""Parameterized test for all NodeDeviceLookupBy* API calls of Connect 
interface
+"""
+original_path = self.node_device_create()
+obj = self.bus.get_object('org.libvirt', original_path)
+prop = obj.Get('org.libvirt.NodeDevice', lookup_item, 
dbus_interface=dbus.PROPERTIES_IFACE)
+path = getattr(self.connect, lookup_method_name)(prop)
+assert original_path == path
+
 @pytest.mark.parametrize("lookup_method_name,lookup_item", [
 ("NetworkLookupByName", 'Name'),
 ("NetworkLookupByUUID", 'UUID'),
-- 
2.15.0

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


[libvirt] [dbus PATCH 13/14] Implement ReAttach method for NodeDevice Interface

2018-06-15 Thread Katerina Koukiou
Signed-off-by: Katerina Koukiou 
---
 data/org.libvirt.NodeDevice.xml |  4 
 src/nodedev.c   | 21 +
 2 files changed, 25 insertions(+)

diff --git a/data/org.libvirt.NodeDevice.xml b/data/org.libvirt.NodeDevice.xml
index bf85958..56deee3 100644
--- a/data/org.libvirt.NodeDevice.xml
+++ b/data/org.libvirt.NodeDevice.xml
@@ -33,5 +33,9 @@
 value="See 
https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceListCaps"/>
   
 
+
+  https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceReAttach"/>
+
   
 
diff --git a/src/nodedev.c b/src/nodedev.c
index 01fcd35..1d15d79 100644
--- a/src/nodedev.c
+++ b/src/nodedev.c
@@ -173,6 +173,26 @@ virtDBusNodeDeviceListCaps(GVariant *inArgs G_GNUC_UNUSED,
 *outArgs = g_variant_new_tuple(&gret, 1);
 }
 
+static void
+virtDBusNodeDeviceReAttach(GVariant *inArgs G_GNUC_UNUSED,
+   GUnixFDList *inFDs G_GNUC_UNUSED,
+   const gchar *objectPath,
+   gpointer userData,
+   GVariant **outArgs G_GNUC_UNUSED,
+   GUnixFDList **outFDs G_GNUC_UNUSED,
+   GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virNodeDevice) dev = NULL;
+
+dev = virtDBusNodeDeviceGetVirNodeDevice(connect, objectPath, error);
+if (!dev)
+return;
+
+if (virNodeDeviceReAttach(dev) < 0)
+virtDBusUtilSetLastVirtError(error);
+}
+
 static virtDBusGDBusPropertyTable virtDBusNodeDevicePropertyTable[] = {
 { "Name", virtDBusNodeDeviceGetName, NULL },
 { "Parent", virtDBusNodeDeviceGetParent, NULL },
@@ -184,6 +204,7 @@ static virtDBusGDBusMethodTable 
virtDBusNodeDeviceMethodTable[] = {
 { "Detach", virtDBusNodeDeviceDetach },
 { "GetXMLDesc", virtDBusNodeDeviceGetXMLDesc },
 { "ListCaps", virtDBusNodeDeviceListCaps },
+{ "ReAttach", virtDBusNodeDeviceReAttach },
 { 0 }
 };
 
-- 
2.15.0

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


[libvirt] [dbus PATCH 10/14] Implement ListCaps method for NodeDevicesInterface

2018-06-15 Thread Katerina Koukiou
Signed-off-by: Katerina Koukiou 
---
 data/org.libvirt.NodeDevice.xml |  5 +
 src/nodedev.c   | 37 +
 tests/test_nodedev.py   |  6 ++
 3 files changed, 48 insertions(+)

diff --git a/data/org.libvirt.NodeDevice.xml b/data/org.libvirt.NodeDevice.xml
index 1185bca..bf85958 100644
--- a/data/org.libvirt.NodeDevice.xml
+++ b/data/org.libvirt.NodeDevice.xml
@@ -28,5 +28,10 @@
   
   
 
+
+  https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceListCaps"/>
+  
+
   
 
diff --git a/src/nodedev.c b/src/nodedev.c
index e61242d..01fcd35 100644
--- a/src/nodedev.c
+++ b/src/nodedev.c
@@ -137,6 +137,42 @@ virtDBusNodeDeviceGetXMLDesc(GVariant *inArgs,
 *outArgs = g_variant_new("(s)", xml);
 }
 
+static void
+virtDBusNodeDeviceListCaps(GVariant *inArgs G_GNUC_UNUSED,
+   GUnixFDList *inFDs G_GNUC_UNUSED,
+   const gchar *objectPath,
+   gpointer userData,
+   GVariant **outArgs,
+   GUnixFDList **outFDs G_GNUC_UNUSED,
+   GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virNodeDevice) dev = NULL;
+g_autoptr(virtDBusCharArray) caps = NULL;
+gint ncaps;
+GVariant *gret;
+GVariantBuilder builder;
+
+dev = virtDBusNodeDeviceGetVirNodeDevice(connect, objectPath, error);
+if (!dev)
+return;
+
+if ((ncaps = virNodeDeviceNumOfCaps(dev)) < 0)
+return virtDBusUtilSetLastVirtError(error);
+
+caps = g_new0(char *, ncaps + 1);
+
+if ((ncaps = virNodeDeviceListCaps(dev, caps, ncaps)) < 0)
+return virtDBusUtilSetLastVirtError(error);
+
+g_variant_builder_init(&builder, G_VARIANT_TYPE("as"));
+for (gint i = 0; i < ncaps; i++)
+g_variant_builder_add(&builder, "s", caps[i]);
+gret = g_variant_builder_end(&builder);
+
+*outArgs = g_variant_new_tuple(&gret, 1);
+}
+
 static virtDBusGDBusPropertyTable virtDBusNodeDevicePropertyTable[] = {
 { "Name", virtDBusNodeDeviceGetName, NULL },
 { "Parent", virtDBusNodeDeviceGetParent, NULL },
@@ -147,6 +183,7 @@ static virtDBusGDBusMethodTable 
virtDBusNodeDeviceMethodTable[] = {
 { "Destroy", virtDBusNodeDeviceDestroy },
 { "Detach", virtDBusNodeDeviceDetach },
 { "GetXMLDesc", virtDBusNodeDeviceGetXMLDesc },
+{ "ListCaps", virtDBusNodeDeviceListCaps },
 { 0 }
 };
 
diff --git a/tests/test_nodedev.py b/tests/test_nodedev.py
index 64c97b2..6aa6211 100755
--- a/tests/test_nodedev.py
+++ b/tests/test_nodedev.py
@@ -32,6 +32,12 @@ class TestNodeDevice(libvirttest.BaseTestClass):
 interface_obj = dbus.Interface(obj, 'org.libvirt.NodeDevice')
 assert isinstance(interface_obj.GetXMLDesc(0), dbus.String)
 
+def test_node_device_list_caps(self):
+test_node_device_path = self.node_device_create()
+obj = self.bus.get_object('org.libvirt', test_node_device_path)
+interface_obj = dbus.Interface(obj, 'org.libvirt.NodeDevice')
+assert isinstance(interface_obj.ListCaps(), dbus.Array)
+
 def test_node_device_properties_type(self):
 """ Ensure correct return type for NodeDevice properties
 """
-- 
2.15.0

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


[libvirt] [dbus PATCH 03/14] Register NodeDevice Lifecycle Events

2018-06-15 Thread Katerina Koukiou
Signed-off-by: Katerina Koukiou 
---
 data/org.libvirt.Connect.xml |  7 +++
 src/connect.c| 13 +
 src/connect.h|  1 +
 src/events.c | 42 ++
 4 files changed, 63 insertions(+)

diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index 137e67b..3828832 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -339,6 +339,13 @@
   
   
 
+
+  https://libvirt.org/html/libvirt-libvirt-nodedev.html#virConnectNodeDeviceEventLifecycleCallback"/>
+  
+  
+  
+
 
   https://libvirt.org/html/libvirt-libvirt-secret.html#virConnectSecretEventLifecycleCallback"/>
diff --git a/src/connect.c b/src/connect.c
index 919172a..1c27768 100644
--- a/src/connect.c
+++ b/src/connect.c
@@ -65,6 +65,16 @@ virtDBusConnectClose(virtDBusConnect *connect,
 }
 }
 
+for (gint i = 0; i < VIR_NODE_DEVICE_EVENT_ID_LAST; i++) {
+if (connect->devCallbackIds[i] >= 0) {
+if (deregisterEvents) {
+virConnectNetworkEventDeregisterAny(connect->connection,
+
connect->devCallbackIds[i]);
+}
+connect->devCallbackIds[i] = -1;
+}
+}
+
 for (gint i = 0; i < VIR_SECRET_EVENT_ID_LAST; i++) {
 if (connect->secretCallbackIds[i] >= 0) {
 if (deregisterEvents) {
@@ -1744,6 +1754,9 @@ virtDBusConnectNew(virtDBusConnect **connectp,
 for (gint i = 0; i < VIR_NETWORK_EVENT_ID_LAST; i++)
 connect->networkCallbackIds[i] = -1;
 
+for (gint i = 0; i < VIR_NODE_DEVICE_EVENT_ID_LAST; i++)
+connect->devCallbackIds[i] = -1;
+
 for (gint i = 0; i < VIR_SECRET_EVENT_ID_LAST; i++)
 connect->secretCallbackIds[i] = -1;
 
diff --git a/src/connect.h b/src/connect.h
index 3b62edd..a864041 100644
--- a/src/connect.h
+++ b/src/connect.h
@@ -24,6 +24,7 @@ struct virtDBusConnect {
 
 gint domainCallbackIds[VIR_DOMAIN_EVENT_ID_LAST];
 gint networkCallbackIds[VIR_NETWORK_EVENT_ID_LAST];
+gint devCallbackIds[VIR_NODE_DEVICE_EVENT_ID_LAST];
 gint secretCallbackIds[VIR_SECRET_EVENT_ID_LAST];
 gint storagePoolCallbackIds[VIR_STORAGE_POOL_EVENT_ID_LAST];
 };
diff --git a/src/events.c b/src/events.c
index b51664f..60cbecd 100644
--- a/src/events.c
+++ b/src/events.c
@@ -567,6 +567,29 @@ virtDBusEventsNetworkEvent(virConnectPtr connection 
G_GNUC_UNUSED,
 return 0;
 }
 
+static gint
+virtDBusEventsNodeDeviceEvent(virConnectPtr connection G_GNUC_UNUSED,
+  virNodeDevicePtr dev,
+  gint event,
+  gint detail,
+  gpointer opaque)
+{
+virtDBusConnect *connect = opaque;
+g_autofree gchar *path = NULL;
+
+path = virtDBusUtilBusPathForVirNodeDevice(dev, connect->devPath);
+
+g_dbus_connection_emit_signal(connect->bus,
+  NULL,
+  connect->connectPath,
+  VIRT_DBUS_CONNECT_INTERFACE,
+  "NodeDeviceEvent",
+  g_variant_new("(oii)", path, event, detail),
+  NULL);
+
+return 0;
+}
+
 static gint
 virtDBusEventsSecretEvent(virConnectPtr connection G_GNUC_UNUSED,
   virSecretPtr secret,
@@ -666,6 +689,21 @@ virtDBusEventsRegisterNetworkEvent(virtDBusConnect 
*connect,
 NULL);
 }
 
+static void
+virtDBusEventsRegisterNodeDeviceEvent(virtDBusConnect *connect,
+  gint id,
+  virConnectNodeDeviceEventGenericCallback 
callback)
+{
+g_assert(connect->devCallbackIds[id] == -1);
+
+connect->devCallbackIds[id] = 
virConnectNodeDeviceEventRegisterAny(connect->connection,
+   NULL,
+   id,
+   
VIR_NODE_DEVICE_EVENT_CALLBACK(callback),
+   connect,
+   NULL);
+}
+
 static void
 virtDBusEventsRegisterSecretEvent(virtDBusConnect *connect,
   gint id,
@@ -791,6 +829,10 @@ virtDBusEventsRegister(virtDBusConnect *connect)
VIR_NETWORK_EVENT_ID_LIFECYCLE,

VIR_NETWORK_EVENT_CALLBACK(virtDBusEventsNetworkEvent));
 
+virtDBusEventsRegisterNodeDeviceEvent(connect,
+  VIR_NODE_DEVICE_EVENT_ID_LIFECYCLE,
+

[libvirt] [dbus PATCH 07/14] Implement Name property for NodeDevice Interface

2018-06-15 Thread Katerina Koukiou
Signed-off-by: Katerina Koukiou 
---
 data/org.libvirt.NodeDevice.xml |  5 +
 src/nodedev.c   | 22 ++
 tests/test_nodedev.py   |  8 
 3 files changed, 35 insertions(+)

diff --git a/data/org.libvirt.NodeDevice.xml b/data/org.libvirt.NodeDevice.xml
index ea9bd7c..b4caac3 100644
--- a/data/org.libvirt.NodeDevice.xml
+++ b/data/org.libvirt.NodeDevice.xml
@@ -3,6 +3,11 @@
 
 
   
+
+  https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceGetName"/>
+  
+
 
   https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceDestroy"/>
diff --git a/src/nodedev.c b/src/nodedev.c
index e8551b8..9647fb6 100644
--- a/src/nodedev.c
+++ b/src/nodedev.c
@@ -24,6 +24,27 @@ virtDBusNodeDeviceGetVirNodeDevice(virtDBusConnect *connect,
 return dev;
 }
 
+static void
+virtDBusNodeDeviceGetName(const gchar *objectPath,
+  gpointer userData,
+  GVariant **value,
+  GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virNodeDevice) dev = NULL;
+const gchar *name;
+
+dev = virtDBusNodeDeviceGetVirNodeDevice(connect, objectPath, error);
+if (!dev)
+return;
+
+name = virNodeDeviceGetName(dev);
+if (!name)
+return virtDBusUtilSetLastVirtError(error);
+
+*value = g_variant_new("s", name);
+}
+
 static void
 virtDBusNodeDeviceDestroy(GVariant *inArgs G_GNUC_UNUSED,
   GUnixFDList *inFDs G_GNUC_UNUSED,
@@ -69,6 +90,7 @@ virtDBusNodeDeviceDetach(GVariant *inArgs,
 }
 
 static virtDBusGDBusPropertyTable virtDBusNodeDevicePropertyTable[] = {
+{ "Name", virtDBusNodeDeviceGetName, NULL },
 { 0 }
 };
 
diff --git a/tests/test_nodedev.py b/tests/test_nodedev.py
index 8ef8e32..cc7199a 100755
--- a/tests/test_nodedev.py
+++ b/tests/test_nodedev.py
@@ -26,6 +26,14 @@ class TestNodeDevice(libvirttest.BaseTestClass):
 
 self.main_loop()
 
+def test_node_device_properties_type(self):
+""" Ensure correct return type for NodeDevice properties
+"""
+test_node_device_path = self.node_device_create()
+obj = self.bus.get_object('org.libvirt', test_node_device_path)
+props = obj.GetAll('org.libvirt.NodeDevice', 
dbus_interface=dbus.PROPERTIES_IFACE)
+assert isinstance(props['Name'], dbus.String)
+
 
 if __name__ == '__main__':
 libvirttest.run()
-- 
2.15.0

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


[libvirt] [dbus PATCH 08/14] Implement Parent property for NodeDevice Interface

2018-06-15 Thread Katerina Koukiou
Signed-off-by: Katerina Koukiou 
---
 data/org.libvirt.NodeDevice.xml |  5 +
 src/nodedev.c   | 22 ++
 tests/test_nodedev.py   |  1 +
 3 files changed, 28 insertions(+)

diff --git a/data/org.libvirt.NodeDevice.xml b/data/org.libvirt.NodeDevice.xml
index b4caac3..c4b4075 100644
--- a/data/org.libvirt.NodeDevice.xml
+++ b/data/org.libvirt.NodeDevice.xml
@@ -8,6 +8,11 @@
 value="See 
https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceGetName"/>
   
 
+
+  https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceGetParent"/>
+  
+
 
   https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceDestroy"/>
diff --git a/src/nodedev.c b/src/nodedev.c
index 9647fb6..c597411 100644
--- a/src/nodedev.c
+++ b/src/nodedev.c
@@ -45,6 +45,27 @@ virtDBusNodeDeviceGetName(const gchar *objectPath,
 *value = g_variant_new("s", name);
 }
 
+static void
+virtDBusNodeDeviceGetParent(const gchar *objectPath,
+gpointer userData,
+GVariant **value,
+GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virNodeDevice) dev = NULL;
+const gchar *parent;
+
+dev = virtDBusNodeDeviceGetVirNodeDevice(connect, objectPath, error);
+if (!dev)
+return;
+
+parent = virNodeDeviceGetParent(dev);
+if (!parent)
+return virtDBusUtilSetLastVirtError(error);
+
+*value = g_variant_new("s", parent);
+}
+
 static void
 virtDBusNodeDeviceDestroy(GVariant *inArgs G_GNUC_UNUSED,
   GUnixFDList *inFDs G_GNUC_UNUSED,
@@ -91,6 +112,7 @@ virtDBusNodeDeviceDetach(GVariant *inArgs,
 
 static virtDBusGDBusPropertyTable virtDBusNodeDevicePropertyTable[] = {
 { "Name", virtDBusNodeDeviceGetName, NULL },
+{ "Parent", virtDBusNodeDeviceGetParent, NULL },
 { 0 }
 };
 
diff --git a/tests/test_nodedev.py b/tests/test_nodedev.py
index cc7199a..39343ae 100755
--- a/tests/test_nodedev.py
+++ b/tests/test_nodedev.py
@@ -33,6 +33,7 @@ class TestNodeDevice(libvirttest.BaseTestClass):
 obj = self.bus.get_object('org.libvirt', test_node_device_path)
 props = obj.GetAll('org.libvirt.NodeDevice', 
dbus_interface=dbus.PROPERTIES_IFACE)
 assert isinstance(props['Name'], dbus.String)
+assert isinstance(props['Parent'], dbus.String)
 
 
 if __name__ == '__main__':
-- 
2.15.0

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


[libvirt] [dbus PATCH 06/14] Implement Detach method for NodeDevice Interface

2018-06-15 Thread Katerina Koukiou
Signed-off-by: Katerina Koukiou 
---
 data/org.libvirt.NodeDevice.xml |  5 +
 src/nodedev.c   | 25 +
 2 files changed, 30 insertions(+)

diff --git a/data/org.libvirt.NodeDevice.xml b/data/org.libvirt.NodeDevice.xml
index 4ca2e11..ea9bd7c 100644
--- a/data/org.libvirt.NodeDevice.xml
+++ b/data/org.libvirt.NodeDevice.xml
@@ -7,5 +7,10 @@
   https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceDestroy"/>
 
+
+  https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceDetachFlags"/>
+  
+
   
 
diff --git a/src/nodedev.c b/src/nodedev.c
index ec87bd8..e8551b8 100644
--- a/src/nodedev.c
+++ b/src/nodedev.c
@@ -44,12 +44,37 @@ virtDBusNodeDeviceDestroy(GVariant *inArgs G_GNUC_UNUSED,
 virtDBusUtilSetLastVirtError(error);
 }
 
+static void
+virtDBusNodeDeviceDetach(GVariant *inArgs,
+ GUnixFDList *inFDs G_GNUC_UNUSED,
+ const gchar *objectPath,
+ gpointer userData,
+ GVariant **outArgs G_GNUC_UNUSED,
+ GUnixFDList **outFDs G_GNUC_UNUSED,
+ GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virNodeDevice) dev = NULL;
+const gchar *driverName;
+guint flags;
+
+g_variant_get(inArgs, "(&su)", &driverName, &flags);
+
+dev = virtDBusNodeDeviceGetVirNodeDevice(connect, objectPath, error);
+if (!dev)
+return;
+
+if (virNodeDeviceDetachFlags(dev, driverName, flags) < 0)
+virtDBusUtilSetLastVirtError(error);
+}
+
 static virtDBusGDBusPropertyTable virtDBusNodeDevicePropertyTable[] = {
 { 0 }
 };
 
 static virtDBusGDBusMethodTable virtDBusNodeDeviceMethodTable[] = {
 { "Destroy", virtDBusNodeDeviceDestroy },
+{ "Detach", virtDBusNodeDeviceDetach },
 { 0 }
 };
 
-- 
2.15.0

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


[libvirt] [dbus PATCH 14/14] Implement Reset method for NodeDevice Interface

2018-06-15 Thread Katerina Koukiou
Signed-off-by: Katerina Koukiou 
---
 data/org.libvirt.NodeDevice.xml |  4 
 src/nodedev.c   | 21 +
 2 files changed, 25 insertions(+)

diff --git a/data/org.libvirt.NodeDevice.xml b/data/org.libvirt.NodeDevice.xml
index 56deee3..5464c9a 100644
--- a/data/org.libvirt.NodeDevice.xml
+++ b/data/org.libvirt.NodeDevice.xml
@@ -37,5 +37,9 @@
   https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceReAttach"/>
 
+
+  https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceReset"/>
+
   
 
diff --git a/src/nodedev.c b/src/nodedev.c
index 1d15d79..7982fd1 100644
--- a/src/nodedev.c
+++ b/src/nodedev.c
@@ -193,6 +193,26 @@ virtDBusNodeDeviceReAttach(GVariant *inArgs G_GNUC_UNUSED,
 virtDBusUtilSetLastVirtError(error);
 }
 
+static void
+virtDBusNodeDeviceReset(GVariant *inArgs G_GNUC_UNUSED,
+GUnixFDList *inFDs G_GNUC_UNUSED,
+const gchar *objectPath,
+gpointer userData,
+GVariant **outArgs G_GNUC_UNUSED,
+GUnixFDList **outFDs G_GNUC_UNUSED,
+GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virNodeDevice) dev = NULL;
+
+dev = virtDBusNodeDeviceGetVirNodeDevice(connect, objectPath, error);
+if (!dev)
+return;
+
+if (virNodeDeviceReset(dev) < 0)
+virtDBusUtilSetLastVirtError(error);
+}
+
 static virtDBusGDBusPropertyTable virtDBusNodeDevicePropertyTable[] = {
 { "Name", virtDBusNodeDeviceGetName, NULL },
 { "Parent", virtDBusNodeDeviceGetParent, NULL },
@@ -205,6 +225,7 @@ static virtDBusGDBusMethodTable 
virtDBusNodeDeviceMethodTable[] = {
 { "GetXMLDesc", virtDBusNodeDeviceGetXMLDesc },
 { "ListCaps", virtDBusNodeDeviceListCaps },
 { "ReAttach", virtDBusNodeDeviceReAttach },
+{ "Reset", virtDBusNodeDeviceReset },
 { 0 }
 };
 
-- 
2.15.0

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


[libvirt] [dbus PATCH 05/14] Implement Destroy method for NodeDevice Interface

2018-06-15 Thread Katerina Koukiou
Signed-off-by: Katerina Koukiou 
---

Some note about the test:

1: The previous test should not use the node_device_create fixture introduced 
here 
for the following reason:
The fixture is creating the device in the setup of the test since the
device creation is not supposed to be tested in other tests apart from
the CreateXML test. Only that test should not have the creation in the
setup, but in the actual test `call` part.

2: I didn't create the get_test_node_device function as other entities have
because ListNodeDevices method is not supported by the test driver

 data/org.libvirt.NodeDevice.xml |  4 
 src/nodedev.c   | 42 +
 tests/Makefile.am   |  1 +
 tests/libvirttest.py| 10 ++
 tests/test_nodedev.py   | 31 ++
 5 files changed, 88 insertions(+)
 create mode 100755 tests/test_nodedev.py

diff --git a/data/org.libvirt.NodeDevice.xml b/data/org.libvirt.NodeDevice.xml
index 7ca26d0..4ca2e11 100644
--- a/data/org.libvirt.NodeDevice.xml
+++ b/data/org.libvirt.NodeDevice.xml
@@ -3,5 +3,9 @@
 
 
   
+
+  https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceDestroy"/>
+
   
 
diff --git a/src/nodedev.c b/src/nodedev.c
index 188df74..ec87bd8 100644
--- a/src/nodedev.c
+++ b/src/nodedev.c
@@ -3,11 +3,53 @@
 
 #include 
 
+static virNodeDevicePtr
+virtDBusNodeDeviceGetVirNodeDevice(virtDBusConnect *connect,
+   const gchar *objectPath,
+   GError **error)
+{
+virNodeDevicePtr dev;
+
+if (virtDBusConnectOpen(connect, error) < 0)
+return NULL;
+
+dev = virtDBusUtilVirNodeDeviceFromBusPath(connect->connection,
+   objectPath,
+   connect->devPath);
+if (!dev) {
+virtDBusUtilSetLastVirtError(error);
+return NULL;
+}
+
+return dev;
+}
+
+static void
+virtDBusNodeDeviceDestroy(GVariant *inArgs G_GNUC_UNUSED,
+  GUnixFDList *inFDs G_GNUC_UNUSED,
+  const gchar *objectPath,
+  gpointer userData,
+  GVariant **outArgs G_GNUC_UNUSED,
+  GUnixFDList **outFDs G_GNUC_UNUSED,
+  GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virNodeDevice) dev = NULL;
+
+dev = virtDBusNodeDeviceGetVirNodeDevice(connect, objectPath, error);
+if (!dev)
+return;
+
+if (virNodeDeviceDestroy(dev) < 0)
+virtDBusUtilSetLastVirtError(error);
+}
+
 static virtDBusGDBusPropertyTable virtDBusNodeDevicePropertyTable[] = {
 { 0 }
 };
 
 static virtDBusGDBusMethodTable virtDBusNodeDeviceMethodTable[] = {
+{ "Destroy", virtDBusNodeDeviceDestroy },
 { 0 }
 };
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 4cae303..e841627 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -7,6 +7,7 @@ test_programs = \
test_connect.py \
test_domain.py \
test_network.py \
+   test_nodedev.py \
test_storage.py
 
 check_PROGRAMS = \
diff --git a/tests/libvirttest.py b/tests/libvirttest.py
index 6b6cb4c..06e52c0 100644
--- a/tests/libvirttest.py
+++ b/tests/libvirttest.py
@@ -71,6 +71,16 @@ class BaseTestClass():
 if self.timeout:
 raise TimeoutError()
 
+@pytest.fixture
+def node_device_create(self):
+""" Fixture to create dummy node device on the test driver
+
+This fixture should be used in the setup of every test manipulating
+with node devices.
+"""
+path = 
self.connect.NodeDeviceCreateXML(xmldata.minimal_node_device_xml, 0)
+return path
+
 @pytest.fixture
 def storage_volume_create(self):
 """ Fixture to create dummy storage volume on the test driver
diff --git a/tests/test_nodedev.py b/tests/test_nodedev.py
new file mode 100755
index 000..8ef8e32
--- /dev/null
+++ b/tests/test_nodedev.py
@@ -0,0 +1,31 @@
+#!/usr/bin/env python3
+
+import dbus
+import libvirttest
+import pytest
+
+
+@pytest.mark.usefixtures("node_device_create")
+class TestNodeDevice(libvirttest.BaseTestClass):
+""" Tests for methods and properties of the NodeDevice interface
+"""
+
+def test_node_device_destroy(self):
+def node_device_deleted(path, event, _detail):
+if event != libvirttest.NodeDeviceEvent.DELETED:
+return
+assert isinstance(path, dbus.ObjectPath)
+self.loop.quit()
+
+self.connect.connect_to_signal('NodeDeviceEvent', node_device_deleted)
+
+test_node_device_path = self.node_device_create()
+obj = self.bus.get_object('org.libvirt', test_node_device_path)
+interface_obj = dbus.Interface(obj, 'org.libvirt.NodeDevice')
+interface_obj.Destroy()
+
+self.main_loop()
+
+

[libvirt] [dbus PATCH 01/14] Introduce NodeDevice Interface

2018-06-15 Thread Katerina Koukiou
Signed-off-by: Katerina Koukiou 
---
 data/Makefile.am|  1 +
 data/org.libvirt.NodeDevice.xml |  7 +
 src/Makefile.am |  1 +
 src/connect.c   |  6 
 src/connect.h   |  1 +
 src/nodedev.c   | 65 +
 src/nodedev.h   |  9 ++
 src/util.c  | 35 ++
 src/util.h  | 15 ++
 9 files changed, 140 insertions(+)
 create mode 100644 data/org.libvirt.NodeDevice.xml
 create mode 100644 src/nodedev.c
 create mode 100644 src/nodedev.h

diff --git a/data/Makefile.am b/data/Makefile.am
index 721b874..b3fa614 100644
--- a/data/Makefile.am
+++ b/data/Makefile.am
@@ -22,6 +22,7 @@ interfaces_files = \
org.libvirt.Connect.xml \
org.libvirt.Domain.xml \
org.libvirt.Network.xml \
+   org.libvirt.NodeDevice.xml \
 org.libvirt.NWFilter.xml \
org.libvirt.Secret.xml \
org.libvirt.StoragePool.xml \
diff --git a/data/org.libvirt.NodeDevice.xml b/data/org.libvirt.NodeDevice.xml
new file mode 100644
index 000..7ca26d0
--- /dev/null
+++ b/data/org.libvirt.NodeDevice.xml
@@ -0,0 +1,7 @@
+http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd";>
+
+
+  
+  
+
diff --git a/src/Makefile.am b/src/Makefile.am
index 53d1a23..3ef3472 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -10,6 +10,7 @@ DAEMON_SOURCES = \
events.c events.h \
gdbus.c gdbus.h \
network.c network.h \
+   nodedev.c nodedev.h \
nwfilter.c nwfilter.h \
secret.c secret.h \
storagepool.c storagepool.h \
diff --git a/src/connect.c b/src/connect.c
index 4f2bdb6..08898c8 100644
--- a/src/connect.c
+++ b/src/connect.c
@@ -2,6 +2,7 @@
 #include "domain.h"
 #include "events.h"
 #include "network.h"
+#include "nodedev.h"
 #include "nwfilter.h"
 #include "secret.h"
 #include "storagepool.h"
@@ -1668,6 +1669,7 @@ virtDBusConnectFree(virtDBusConnect *connect)
 if (connect->connection)
 virtDBusConnectClose(connect, TRUE);
 
+g_free(connect->devPath);
 g_free(connect->domainPath);
 g_free(connect->networkPath);
 g_free(connect->nwfilterPath);
@@ -1729,6 +1731,10 @@ virtDBusConnectNew(virtDBusConnect **connectp,
 if (error && *error)
 return;
 
+virtDBusNodeDeviceRegister(connect, error);
+if (error && *error)
+return;
+
 virtDBusNWFilterRegister(connect, error);
 if (error && *error)
 return;
diff --git a/src/connect.h b/src/connect.h
index 341dfc4..3b62edd 100644
--- a/src/connect.h
+++ b/src/connect.h
@@ -12,6 +12,7 @@ struct virtDBusConnect {
 GDBusConnection *bus;
 const gchar *uri;
 const gchar *connectPath;
+gchar *devPath;
 gchar *domainPath;
 gchar *networkPath;
 gchar *nwfilterPath;
diff --git a/src/nodedev.c b/src/nodedev.c
new file mode 100644
index 000..188df74
--- /dev/null
+++ b/src/nodedev.c
@@ -0,0 +1,65 @@
+#include "nodedev.h"
+#include "util.h"
+
+#include 
+
+static virtDBusGDBusPropertyTable virtDBusNodeDevicePropertyTable[] = {
+{ 0 }
+};
+
+static virtDBusGDBusMethodTable virtDBusNodeDeviceMethodTable[] = {
+{ 0 }
+};
+
+static gchar **
+virtDBusNodeDeviceEnumerate(gpointer userData)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virNodeDevicePtr) devs = NULL;
+gint num = 0;
+gchar **ret = NULL;
+
+if (!virtDBusConnectOpen(connect, NULL))
+return NULL;
+
+num = virConnectListAllNodeDevices(connect->connection, &devs, 0);
+if (num < 0)
+return NULL;
+
+if (num == 0)
+return NULL;
+
+ret = g_new0(gchar *, num + 1);
+
+for (gint i = 0; i < num; i++) {
+ret[i] = virtDBusUtilBusPathForVirNodeDevice(devs[i],
+ connect->devPath);
+}
+
+return ret;
+}
+
+static GDBusInterfaceInfo *interfaceInfo;
+
+void
+virtDBusNodeDeviceRegister(virtDBusConnect *connect,
+   GError **error)
+{
+connect->devPath = g_strdup_printf("%s/nodedev",
+   connect->connectPath);
+
+if (!interfaceInfo) {
+interfaceInfo = 
virtDBusGDBusLoadIntrospectData(VIRT_DBUS_NODEDEV_INTERFACE,
+error);
+if (!interfaceInfo)
+return;
+}
+
+virtDBusGDBusRegisterSubtree(connect->bus,
+ connect->devPath,
+ interfaceInfo,
+ virtDBusNodeDeviceEnumerate,
+ virtDBusNodeDeviceMethodTable,
+ virtDBusNodeDevicePropertyTable,
+ connect);
+}
diff --git a/src/nodedev.h b/src/nodedev.h
new file mode 100644
index 000..3cd2bdf
--- /dev/null
+++ b/src/nodedev.h
@@ -0,0 +1,9 @@
+#pragma once
+
+#include "connect.

[libvirt] [dbus PATCH 02/14] Implement ListNodeDevices method for Connect Interface

2018-06-15 Thread Katerina Koukiou
Signed-off-by: Katerina Koukiou 
---
 data/org.libvirt.Connect.xml |  6 ++
 src/connect.c| 38 ++
 2 files changed, 44 insertions(+)

diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index 37daed0..137e67b 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -160,6 +160,12 @@
   
   
 
+
+  https://libvirt.org/html/libvirt-libvirt-nodedev.html#virConnectListAllNodeDevices"/>
+  
+  
+
 
   https://libvirt.org/html/libvirt-libvirt-nwfilter.html#virConnectListAllNWFilters"/>
diff --git a/src/connect.c b/src/connect.c
index 08898c8..919172a 100644
--- a/src/connect.c
+++ b/src/connect.c
@@ -798,6 +798,43 @@ virtDBusConnectListNetworks(GVariant *inArgs,
 *outArgs = g_variant_new_tuple(&gnetworks, 1);
 }
 
+static void
+virtDBusConnectListNodeDevices(GVariant *inArgs,
+   GUnixFDList *inFDs G_GNUC_UNUSED,
+   const gchar *objectPath G_GNUC_UNUSED,
+   gpointer userData,
+   GVariant **outArgs,
+   GUnixFDList **outFDs G_GNUC_UNUSED,
+   GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virNodeDevicePtr) devs = NULL;
+guint flags;
+GVariantBuilder builder;
+GVariant *gdevs;
+
+g_variant_get(inArgs, "(u)", &flags);
+
+if (!virtDBusConnectOpen(connect, error))
+return;
+
+if (virConnectListAllNodeDevices(connect->connection, &devs, flags) < 0)
+return virtDBusUtilSetLastVirtError(error);
+
+g_variant_builder_init(&builder, G_VARIANT_TYPE("ao"));
+
+for (gint i = 0; devs[i]; i++) {
+g_autofree gchar *path = NULL;
+path = virtDBusUtilBusPathForVirNodeDevice(devs[i],
+   connect->devPath);
+
+g_variant_builder_add(&builder, "o", path);
+}
+
+gdevs = g_variant_builder_end(&builder);
+*outArgs = g_variant_new_tuple(&gdevs, 1);
+}
+
 static void
 virtDBusConnectListNWFilters(GVariant *inArgs,
  GUnixFDList *inFDs G_GNUC_UNUSED,
@@ -1632,6 +1669,7 @@ static virtDBusGDBusMethodTable 
virtDBusConnectMethodTable[] = {
 { "GetSysinfo", virtDBusConnectGetSysinfo },
 { "ListDomains", virtDBusConnectListDomains },
 { "ListNetworks", virtDBusConnectListNetworks },
+{ "ListNodeDevices", virtDBusConnectListNodeDevices },
 { "ListNWFilters", virtDBusConnectListNWFilters },
 { "ListSecrets", virtDBusConnectListSecrets },
 { "ListStoragePools", virtDBusConnectListStoragePools },
-- 
2.15.0

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


[libvirt] [dbus PATCH 04/14] Implement NodeDeviceCreateXML method for Connect Interface

2018-06-15 Thread Katerina Koukiou
Signed-off-by: Katerina Koukiou 
---
 data/org.libvirt.Connect.xml |  7 +++
 src/connect.c| 30 ++
 tests/libvirttest.py |  3 +++
 tests/test_connect.py| 14 ++
 tests/xmldata.py | 16 
 5 files changed, 70 insertions(+)

diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index 3828832..d8b0d9e 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -208,6 +208,13 @@
   
   
 
+
+  https://libvirt.org/html/libvirt-libvirt-nodedev.html#virNodeDeviceCreateXML"/>
+  
+  
+  
+
 
   https://libvirt.org/html/libvirt-libvirt-nwfilter.html#virNWFilterDefineXML"/>
diff --git a/src/connect.c b/src/connect.c
index 1c27768..5e146a6 100644
--- a/src/connect.c
+++ b/src/connect.c
@@ -1069,6 +1069,35 @@ virtDBusConnectNetworkLookupByUUID(GVariant *inArgs,
 *outArgs = g_variant_new("(o)", path);
 }
 
+static void
+virtDBusConnectNodeDeviceCreateXML(GVariant *inArgs,
+   GUnixFDList *inFDs G_GNUC_UNUSED,
+   const gchar *objectPath G_GNUC_UNUSED,
+   gpointer userData,
+   GVariant **outArgs,
+   GUnixFDList **outFDs G_GNUC_UNUSED,
+   GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virNodeDevice) dev = NULL;
+g_autofree gchar *path = NULL;
+gchar *xml;
+guint flags;
+
+g_variant_get(inArgs, "(&su)", &xml, &flags);
+
+if (!virtDBusConnectOpen(connect, error))
+return;
+
+dev = virNodeDeviceCreateXML(connect->connection, xml, flags);
+if (!dev)
+return virtDBusUtilSetLastVirtError(error);
+
+path = virtDBusUtilBusPathForVirNodeDevice(dev, connect->devPath);
+
+*outArgs = g_variant_new("(o)", path);
+}
+
 static void
 virtDBusConnectNWFilterDefineXML(GVariant *inArgs,
  GUnixFDList *inFDs G_GNUC_UNUSED,
@@ -1687,6 +1716,7 @@ static virtDBusGDBusMethodTable 
virtDBusConnectMethodTable[] = {
 { "NetworkDefineXML", virtDBusConnectNetworkDefineXML },
 { "NetworkLookupByName", virtDBusConnectNetworkLookupByName },
 { "NetworkLookupByUUID", virtDBusConnectNetworkLookupByUUID },
+{ "NodeDeviceCreateXML", virtDBusConnectNodeDeviceCreateXML },
 { "NWFilterDefineXML", virtDBusConnectNWFilterDefineXML },
 { "NWFilterLookupByName", virtDBusConnectNWFilterLookupByName },
 { "NWFilterLookupByUUID", virtDBusConnectNWFilterLookupByUUID },
diff --git a/tests/libvirttest.py b/tests/libvirttest.py
index 6d1a9d0..6b6cb4c 100644
--- a/tests/libvirttest.py
+++ b/tests/libvirttest.py
@@ -217,6 +217,9 @@ class NetworkEvent(IntEnum):
 STARTED = 2
 STOPPED = 3
 
+class NodeDeviceEvent(IntEnum):
+CREATED = 0
+DELETED = 1
 
 class StoragePoolBuildFlags(IntEnum):
 NEW = 0
diff --git a/tests/test_connect.py b/tests/test_connect.py
index 57b385e..e532016 100755
--- a/tests/test_connect.py
+++ b/tests/test_connect.py
@@ -155,6 +155,20 @@ class TestConnect(libvirttest.BaseTestClass):
 path = getattr(self.connect, lookup_method_name)(prop)
 assert original_path == path
 
+def test_connect_node_device_create_xml(self):
+def node_device_created(path, event, _detail):
+if event != libvirttest.NodeDeviceEvent.CREATED:
+return
+assert isinstance(path, dbus.ObjectPath)
+self.loop.quit()
+
+self.connect.connect_to_signal('NodeDeviceEvent', node_device_created)
+
+path = 
self.connect.NodeDeviceCreateXML(xmldata.minimal_node_device_xml, 0)
+assert isinstance(path, dbus.ObjectPath)
+
+self.main_loop()
+
 def test_connect_node_get_cpu_stats(self):
 stats = self.connect.NodeGetCPUStats(0, 0)
 assert isinstance(stats, dbus.Dictionary)
diff --git a/tests/xmldata.py b/tests/xmldata.py
index 926bd2c..25bb089 100644
--- a/tests/xmldata.py
+++ b/tests/xmldata.py
@@ -26,6 +26,22 @@ minimal_network_xml = '''
 
 '''
 
+minimal_node_device_xml = '''
+
+  scsi_host22
+  scsi_host2
+  
+22
+22
+
+  200098765432
+  100098765432
+  200098769876
+
+  
+
+'''
+
 minimal_storage_pool_xml = '''
 
   foo
-- 
2.15.0

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


[libvirt] [dbus PATCH 00/14] Introduce NodeDevice APIs

2018-06-15 Thread Katerina Koukiou
Katerina Koukiou (14):
  Introduce NodeDevice Interface
  Implement ListNodeDevices method for Connect Interface
  Register NodeDevice Lifecycle Events
  Implement NodeDeviceCreateXML method for Connect Interface
  Implement Destroy method for NodeDevice Interface
  Implement Detach method for NodeDevice Interface
  Implement Name property for NodeDevice Interface
  Implement Parent property for NodeDevice Interface
  Implement GetXMLDesc property for NodeDevice Interface
  Implement ListCaps method for NodeDevicesInterface
  Implement NodeDeviceLookupByName method for Connect Interface
  Implement NodeDeviceLookupSCSIHostByWWN method for Connect Interface
  Implement ReAttach method for NodeDevice Interface
  Implement Reset method for NodeDevice Interface

 data/Makefile.am|   1 +
 data/org.libvirt.Connect.xml|  34 +
 data/org.libvirt.NodeDevice.xml |  45 +++
 src/Makefile.am |   1 +
 src/connect.c   | 148 +
 src/connect.h   |   2 +
 src/events.c|  42 ++
 src/nodedev.c   | 283 
 src/nodedev.h   |   9 ++
 src/util.c  |  35 +
 src/util.h  |  15 +++
 tests/Makefile.am   |   1 +
 tests/libvirttest.py|  13 ++
 tests/test_connect.py   |  27 
 tests/test_nodedev.py   |  52 
 tests/xmldata.py|  16 +++
 16 files changed, 724 insertions(+)
 create mode 100644 data/org.libvirt.NodeDevice.xml
 create mode 100644 src/nodedev.c
 create mode 100644 src/nodedev.h
 create mode 100755 tests/test_nodedev.py

-- 
2.15.0

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


Re: [libvirt] [PATCH v2 2/3] cmdDomblkinfo: add --all to show all block devices info

2018-06-15 Thread John Ferlan



On 06/15/2018 04:58 AM, Chen Hanxiao wrote:
> 
> 
> At 2018-06-15 05:41:48, "John Ferlan"  wrote:
>>
>>
>> On 06/11/2018 06:52 AM, Chen Hanxiao wrote:
>>> From: Chen Hanxiao 
>>>
> [...]
>>>
>>
>> Do you have networked disks in your domain configs? For a non running
>> guest, t; otherwise, you would have noted:
>>
>> # virsh domblkinfo $dom --all
>> Target CapacityAllocation  Physical
>> -
>> vda10737418240 2086727680  10737418240
>> error: internal error: missing storage backend for network files using
>> iscsi protocol
>>
> 
> Yes, I tested this cases.
> This issue already existed for the original domblkinfo, so I didn't change 
> this.
> Maybe we should fix it in another patch.
> 

True, but printing a partial list and then failing is I think worse than
the "singular error" that one would get normally, e.g.:

   # virsh domblkinfo $dom $networked-target
   error: internal error: missing storage backend for network files
using iscsi protocol
   #

The one thing that draws my attention though is using "internal error"
could make a consumer think they did something wrong or libvirt did
something wrong, I wonder if we change that to 'Operation not supported'
which may also help in the detection at [1]...

>>> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
>>> index daa86e8310..22c0b740c6 100644
> ...
>>> +if (device) {
>>> +if (virAsprintf(&cap, "%.3lf %s", val_cap, unit_cap) < 0)
>>> +goto cleanup;
>>> +
>>> +if (virAsprintf(&alloc, "%.3lf %s", val_alloc, unit_alloc) < 0)
>>> +goto cleanup;
>>> +
>>> +if (virAsprintf(&phy, "%.3lf %s", val_phy, unit_phy) < 0)
>>> +goto cleanup;
>>
>> it would be fine I think to do:
>>
>>if (virAsprintf(&cap, "%.3lf %s", val_cap, unit_cap) < 0 ||
>>virAsprintf(&alloc, "%.3lf %s", val_alloc, unit_alloc) < 0 ||
>>virAsprintf(&phy, "%.3lf %s", val_phy, unit_phy) < 0)
>>goto cleanup;
>>
>> But that's not required.
>>
> 
> Looks much better, will be changed in the next series.
> 
>>> +
>>> +vshPrint(ctl, "%-10s %-15s %-15s %-15s\n",
> [...]
> 
>>> +ctxt->node = disks[i];
>>> +target = virXPathString("string(./target/@dev)", ctxt);
>>> +
>>> +if (virDomainGetBlockInfo(dom, target, &info, 0) < 0)
>>> +goto cleanup;
>>
>> If the domain is not running, then it's possible to return an error for
>> a networked disk (e.g. )... This is
>> because qemuDomainGetBlockInfo calls qemuStorageLimitsRefresh which
>> calls qemuDomainStorageOpenStat and for non local storage the
>> virStorageFileInitAs will eventually fail in virStorageFileBackendForType.
>>
>> A couple of options come to mind...
>>
>> ... let the failure occur as is, so be it...
>>
>> ... check the last error message for code == VIR_ERR_INTERNAL_ERROR and
>> domain == VIR_FROM_STORAGE and we have a source protocol from an
>> inactive domain, then assume it's a we cannot get there from here.
>>
>> ... Other options?
>>
>> If we fail virDomainGetBlockInfo we could still display values as long
>> as there's an appropriately placed  memset(&info, 0, sizeof(info)). That
>> way we display only the name and 0's for everything else. Not optimal,
>> but easily described in the man page that an inactive guest, using
>> network protocol for storage may not be able to get the size values and
>> virsh will display as 0's instead... or get more creative and display
>> "-" for each size column.
> 
> I prefer this solutions.
> Also, I think domblkinfo DOM DEVICE should follow this if it's a network disk.
> 

...[1]

For the non --all case, you don't have the XML to check whether the
"protocol" field exists which makes the error recovery tricky because
internal error can be more than just this reason. Still taking the
earlier suggestion to change to VIR_ERR_OPERATION_UNSUPPORTED may help
in the detection. Using protocol also gave that extra reason since we
then know the device isn't local... Knowing that the error domain was
VIR_FROM_STORAGE and domain is not running lent more credence to the
"assumption" one could make about the error without checking the actual
message which is something we cannot do (think internationalization).


John

[...]

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


[libvirt] [PATCH] tests: add test file for smartcard database

2018-06-15 Thread Anya Harter
Add test case explicitly defining a smartcard host certificates
database via the following xml:


  /tmp/foo


This case is not currently covered in the test suite.

Signed-off-by: Anya Harter 
---
 .../smartcard-host-certificates-database.args | 28 +++
 .../smartcard-host-certificates-database.xml  | 21 +++
 tests/qemuxml2argvtest.c  |  2 ++
 .../smartcard-host-certificates-database.xml  | 35 +++
 tests/qemuxml2xmltest.c   |  1 +
 5 files changed, 87 insertions(+)
 create mode 100644 
tests/qemuxml2argvdata/smartcard-host-certificates-database.args
 create mode 100644 
tests/qemuxml2argvdata/smartcard-host-certificates-database.xml
 create mode 100644 
tests/qemuxml2xmloutdata/smartcard-host-certificates-database.xml

diff --git a/tests/qemuxml2argvdata/smartcard-host-certificates-database.args 
b/tests/qemuxml2argvdata/smartcard-host-certificates-database.args
new file mode 100644
index 00..bc159f234d
--- /dev/null
+++ b/tests/qemuxml2argvdata/smartcard-host-certificates-database.args
@@ -0,0 +1,28 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i686 \
+-name QEMUGuest1 \
+-S \
+-machine pc,accel=tcg,usb=off,dump-guest-core=off \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev 
socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-boot c \
+-device usb-ccid,id=ccid0,bus=usb.0,port=1 \
+-usb \
+-device ccid-card-emulated,backend=certificates,cert1=cert1,cert2=cert2,\
+cert3=cert3,db=/tmp/foo,id=smartcard0,bus=ccid0.0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/smartcard-host-certificates-database.xml 
b/tests/qemuxml2argvdata/smartcard-host-certificates-database.xml
new file mode 100644
index 00..fa35645736
--- /dev/null
+++ b/tests/qemuxml2argvdata/smartcard-host-certificates-database.xml
@@ -0,0 +1,21 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  1
+  
+hvm
+
+  
+  
+/usr/bin/qemu-system-i686
+
+  cert1
+  cert2
+  cert3
+  /tmp/foo
+
+
+  
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index f44cac9fef..194a8d2bab 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1404,6 +1404,8 @@ mymain(void)
 QEMU_CAPS_CCID_EMULATED);
 DO_TEST("smartcard-host-certificates",
 QEMU_CAPS_CCID_EMULATED);
+DO_TEST("smartcard-host-certificates-database",
+QEMU_CAPS_CCID_EMULATED);
 DO_TEST("smartcard-passthrough-tcp",
 QEMU_CAPS_CCID_PASSTHRU);
 DO_TEST("smartcard-passthrough-spicevmc",
diff --git a/tests/qemuxml2xmloutdata/smartcard-host-certificates-database.xml 
b/tests/qemuxml2xmloutdata/smartcard-host-certificates-database.xml
new file mode 100644
index 00..55d54a4355
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/smartcard-host-certificates-database.xml
@@ -0,0 +1,35 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-i686
+
+  
+
+
+
+
+  cert1
+  cert2
+  cert3
+  /tmp/foo
+  
+
+
+
+
+  
+
+  
+
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index f5805c7f50..4449954ad4 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1163,6 +1163,7 @@ mymain(void)
 
 DO_TEST("smartcard-host", NONE);
 DO_TEST("smartcard-host-certificates", NONE);
+DO_TEST("smartcard-host-certificates-database", NONE);
 DO_TEST("smartcard-passthrough-tcp", NONE);
 DO_TEST("smartcard-passthrough-spicevmc", NONE);
 DO_TEST("smartcard-controller", NONE);
-- 
2.17.1

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


[libvirt] [PATCH] qemuDomainDetachDeviceConfig: Don't free device from @dev

2018-06-15 Thread Michal Privoznik
https://bugzilla.redhat.com/show_bug.cgi?id=1591561

For reasons I don't understand my original patch of 75f0fd51124
freed not only the chardev from domain but also the one from
passed virDomainDeviceDefPtr. This caused no troubles until now,
because those two pointers were separate, but after I've
introduced virDomainDetachDeviceAlias() they became the same
resulting in double free on detach.

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

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 42069ee617..7fe86d4d2e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8241,8 +8241,6 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
 return -1;
 
 virDomainChrDefFree(chr);
-virDomainChrDefFree(dev->data.chr);
-dev->data.chr = NULL;
 break;
 
 case VIR_DOMAIN_DEVICE_FS:
-- 
2.16.4

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


Re: [libvirt] [PATCH 2/2] rpc: Fix segmentation fault when no worker pool is available

2018-06-15 Thread Marc Hartmayer
On Fri, Jun 15, 2018 at 01:39 PM +0200, Marc Hartmayer  
wrote:
> If srv->workers is a NULL pointer, as it is the case if there are no
> workers, then don't try to dereference it.
>
> Signed-off-by: Marc Hartmayer 
> Reviewed-by: Boris Fiuczynski 
> ---
>  src/rpc/virnetserver.c | 22 +++---
>  1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
> index 5ae809e372..be6f610880 100644
> --- a/src/rpc/virnetserver.c
> +++ b/src/rpc/virnetserver.c
> @@ -933,13 +933,21 @@ virNetServerGetThreadPoolParameters(virNetServerPtr srv,
>  size_t *jobQueueDepth)
>  {
>  virObjectLock(srv);
> -
> -*minWorkers = virThreadPoolGetMinWorkers(srv->workers);
> -*maxWorkers = virThreadPoolGetMaxWorkers(srv->workers);
> -*freeWorkers = virThreadPoolGetFreeWorkers(srv->workers);
> -*nWorkers = virThreadPoolGetCurrentWorkers(srv->workers);
> -*nPrioWorkers = virThreadPoolGetPriorityWorkers(srv->workers);
> -*jobQueueDepth = virThreadPoolGetJobQueueDepth(srv->workers);
> +if (srv->workers) {
> +*minWorkers = virThreadPoolGetMinWorkers(srv->workers);
> +*maxWorkers = virThreadPoolGetMaxWorkers(srv->workers);
> +*freeWorkers = virThreadPoolGetFreeWorkers(srv->workers);
> +*nWorkers = virThreadPoolGetCurrentWorkers(srv->workers);
> +*nPrioWorkers = virThreadPoolGetPriorityWorkers(srv->workers);
> +*jobQueueDepth = virThreadPoolGetJobQueueDepth(srv->workers);
> +} else {
> +*minWorkers = 0;
> +*maxWorkers = 0;
> +*freeWorkers = 0;
> +*nWorkers = 0;
> +*nPrioWorkers = 0;
> +*jobQueueDepth = 0;
> +}
>
>  virObjectUnlock(srv);
>  return 0;
> --
> 2.13.6

After thinking again it probably makes more sense (and the code more
beautiful) to initialize the worker pool even for maxworker=0 (within
virNetServerNew) (=> we'll have to adapt virNetServerDispatchNewMessage
as well). BTW, there is also a segmentation fault in
virThreadPoolSetParameters… And currently it’s not possible to start
with maxworkers set to 0 and then increase it via
virThreadPoolSetParameters. These problems would all be fixed if we
would initialize a thread pool for max_workers = 0. Shall I revise this
patch this way?

Beste Grüße / Kind regards
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


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

Re: [libvirt] [PATCH 2/2] conf: Forbid device alias change on device-update

2018-06-15 Thread John Ferlan



On 06/12/2018 11:04 AM, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1585108
> 
> When updating a live device users might pass different alias than
> the one the device has. Currently, this is silently ignored which
> goes against our behaviour for other parts of the device where we
> explicitly allow only certain changes and error out loudly on
> anything else.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/conf/domain_conf.c | 12 +++-
>  src/conf/domain_conf.h |  3 ++-
>  src/lxc/lxc_driver.c   |  6 +++---
>  src/qemu/qemu_driver.c | 16 
>  4 files changed, 24 insertions(+), 13 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 85f07af46e..91cac75c0a 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -28195,7 +28195,8 @@ virDomainDeviceInfoCheckBootIndex(virDomainDefPtr def 
> ATTRIBUTE_UNUSED,
>  int
>  virDomainDefCompatibleDevice(virDomainDefPtr def,
>   virDomainDeviceDefPtr dev,
> - virDomainDeviceDefPtr oldDev)
> + virDomainDeviceDefPtr oldDev,
> + bool live)
>  {
>  virDomainCompatibleDeviceData data = {
>  .newInfo = virDomainDeviceGetInfo(dev),
> @@ -28205,6 +28206,15 @@ virDomainDefCompatibleDevice(virDomainDefPtr def,
>  if (oldDev)
>  data.oldInfo = virDomainDeviceGetInfo(oldDev);
>  
> +if (live &&
> +((!!data.newInfo != !!data.oldInfo) ||
> + (data.newInfo && data.oldInfo &&
> +  STRNEQ(data.newInfo->alias, data.oldInfo->alias {
> +virReportError(VIR_ERR_OPERATION_DENIED, "%s",
> +   _("changing device alias is not allowed"));
> +return -1;
> +}
> +

Does this work for attach-device from the bz?  I just tried it and it
doesn't - probably because oldInfo would be NULL for attach.

Additionally, if the update/detach doesn't include an alias, then would
this fail too?

By comparison qemuDomainDiskChangeSupported only cares that the updated
disk definition has an alias before making check and of course that
would now be duplicitous to this.

Then there's qemuDomainChangeNet which at first I didn't understand why
it wasn't throwing up since it does check the alias. Then I noticed
something from the case, for the updated interface XML - if you include
the  from the live guest, but change the "", then you
get the error. However, if you remove the  from the update XML
things then succeed. That's because of this gem in the code:

if (!virDomainDeviceAddressIsValid(&newdev->info,

VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
virDomainDeviceInfoCopy(&newdev->info, &olddev->info) < 0) {
goto cleanup;
}

where virDomainDeviceInfoCopy essentially *overwrites* the
provided/changed newdev->info.alias.

So I think if you fix this part of the code, then you don't have the
need for the rest of this patch. Yes, that does possibly leave the
graphics devices as not checking alias updates, but that would seemingly
be fixable separately if it's considered a problem or not checked as I
didn't follow all the graphics update code.

John

NB:
Another inconsistency in the bz is that the add interface added uses
target dev='vnet0', but the live guest shows target dev='vnet12'. Then
the updated xml doesn't change target dev='vnet0', but does change the
alias and it works?  For me it failed with:

error: Operation not supported: cannot modify network device tap name

until I changed it to match the live xml target dev...


[...]

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


Re: [libvirt] [PATCH 1/2] qemuDomainUpdateDeviceFlags: Parse device as live if needed

2018-06-15 Thread John Ferlan



On 06/12/2018 11:04 AM, Michal Privoznik wrote:
> When updating device it's worth parsing live info too as users
> might want to update it as well.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_driver.c | 12 
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 

So this follows what DetachLiveAndConfig has done with altering
parse_flags to add the inactive mainly because it "makes sense" to
attempt to match whatever "live/active" device may exist to the fullest
possible extent including fields that wouldn't normally be parsed for an
inactive guest.

Took a bit to come to that conclusion ;-) because the commit message
above didn't give me that!

Reviewed-by: John Ferlan 

John

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


[libvirt] [PATCH 1/2] rpc: Fix deadlock if there is no worker pool available

2018-06-15 Thread Marc Hartmayer
@srv must be unlocked for the call virNetServerProcessMsg otherwise a
deadlock can occur.

Signed-off-by: Marc Hartmayer 
Reviewed-by: Boris Fiuczynski 
---
 src/rpc/virnetserver.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 5c7f7dd08f..5ae809e372 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -201,7 +201,7 @@ static void 
virNetServerDispatchNewMessage(virNetServerClientPtr client,
 virNetServerJobPtr job;
 
 if (VIR_ALLOC(job) < 0)
-goto error;
+goto error_unlock;
 
 job->client = client;
 job->msg = msg;
@@ -216,20 +216,23 @@ static void 
virNetServerDispatchNewMessage(virNetServerClientPtr client,
 virObjectUnref(client);
 VIR_FREE(job);
 virObjectUnref(prog);
-goto error;
+goto error_unlock;
 }
+virObjectUnlock(srv);
 } else {
+/* @srv must be unlocked for virNetServerProcessMsg */
+virObjectUnlock(srv);
 if (virNetServerProcessMsg(srv, client, prog, msg) < 0)
 goto error;
 }
 
-virObjectUnlock(srv);
 return;
 
+ error_unlock:
+virObjectUnlock(srv);
  error:
 virNetMessageFree(msg);
 virNetServerClientClose(client);
-virObjectUnlock(srv);
 }
 
 /**
-- 
2.13.6

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


[libvirt] [PATCH 0/2] Fixes for segfault and deadlock

2018-06-15 Thread Marc Hartmayer
One way to reproduce the bugs is to set admin_max_workers=0 in
libvirtd.conf, restart libvirtd, and then call:

 $ virt-admin server-threadpool-info admin

Marc Hartmayer (2):
  rpc: Fix deadlock if there is no worker pool available
  rpc: Fix segmentation fault when no worker pool is available

 src/rpc/virnetserver.c | 33 ++---
 1 file changed, 22 insertions(+), 11 deletions(-)

-- 
2.13.6

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


[libvirt] [PATCH 2/2] rpc: Fix segmentation fault when no worker pool is available

2018-06-15 Thread Marc Hartmayer
If srv->workers is a NULL pointer, as it is the case if there are no
workers, then don't try to dereference it.

Signed-off-by: Marc Hartmayer 
Reviewed-by: Boris Fiuczynski 
---
 src/rpc/virnetserver.c | 22 +++---
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 5ae809e372..be6f610880 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -933,13 +933,21 @@ virNetServerGetThreadPoolParameters(virNetServerPtr srv,
 size_t *jobQueueDepth)
 {
 virObjectLock(srv);
-
-*minWorkers = virThreadPoolGetMinWorkers(srv->workers);
-*maxWorkers = virThreadPoolGetMaxWorkers(srv->workers);
-*freeWorkers = virThreadPoolGetFreeWorkers(srv->workers);
-*nWorkers = virThreadPoolGetCurrentWorkers(srv->workers);
-*nPrioWorkers = virThreadPoolGetPriorityWorkers(srv->workers);
-*jobQueueDepth = virThreadPoolGetJobQueueDepth(srv->workers);
+if (srv->workers) {
+*minWorkers = virThreadPoolGetMinWorkers(srv->workers);
+*maxWorkers = virThreadPoolGetMaxWorkers(srv->workers);
+*freeWorkers = virThreadPoolGetFreeWorkers(srv->workers);
+*nWorkers = virThreadPoolGetCurrentWorkers(srv->workers);
+*nPrioWorkers = virThreadPoolGetPriorityWorkers(srv->workers);
+*jobQueueDepth = virThreadPoolGetJobQueueDepth(srv->workers);
+} else {
+*minWorkers = 0;
+*maxWorkers = 0;
+*freeWorkers = 0;
+*nWorkers = 0;
+*nPrioWorkers = 0;
+*jobQueueDepth = 0;
+}
 
 virObjectUnlock(srv);
 return 0;
-- 
2.13.6

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


Re: [libvirt] [PATCH v2 1/5] docker: Commit initial Dockerfiles

2018-06-15 Thread Andrea Bolognani
On Fri, 2018-06-15 at 11:28 +0100, Daniel P. Berrangé wrote:
> On Fri, Jun 15, 2018 at 12:09:39PM +0200, Andrea Bolognani wrote:
> > We *do* need to have the Dockerfiles committed *somewhere* for
> > Docker Hub automated builds to work.
> 
> Does docker hub only pull from git repos, or does it have other
> options for detching the files ?

It has to be a git repository, more specifically one hosted on
either GitHub or Bitbucket.

  https://docs.docker.com/docker-hub/builds/

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [jenkins-ci POC 2/2] guests: Add Dockerfile generator

2018-06-15 Thread Andrea Bolognani
On Fri, 2018-06-15 at 10:12 +0100, Daniel P. Berrangé wrote:
> On Fri, Jun 15, 2018 at 08:53:44AM +0200, Andrea Bolognani wrote:
> > This is basically the exact same algorithm used by the
> > Ansible playbooks to process package mappings, implemented
> > as a standalone script.
> > 
> > There's little to none error handling, and some information
> > is hardcoded instead of being configurable at runtime; more
> > importantly, before it can be considered for merging it
> > needs to be integrated into lcitool, which in turn requires
> > lcitool to be significantly reworked.
> 
> I'm not understanding why it needs to be integrated into
> lcitool ?  Generating dockerfiles has no dependancy /
> interaction with ansible updating the jenkins slaves.

Strictly speaking there is no interaction; however, in order to
generate Dockerfiles you need to poke into the Ansible inventory,
so the two are not completely independent either.

To a degree it's similar to how installation and setup of guests,
while performed in isolation from each other and using completely
different tools, both require information stored in the Ansible
inventory and are exposed by lcitool as top-level actions with a
consistent interface.

Plus it's a good excuse to go back and make lcitool somewhat less
of a horrible hack ;)

> Why won't we just run this script separately when needed,
> ideally as non-root ?  Looks good enough to commit now
> IMHO

lcitool is already designed to run as non-root, so no need to
change anything there.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH v2 1/5] docker: Commit initial Dockerfiles

2018-06-15 Thread Daniel P . Berrangé
On Fri, Jun 15, 2018 at 12:09:39PM +0200, Andrea Bolognani wrote:
> On Fri, 2018-06-15 at 10:05 +0100, Daniel P. Berrangé wrote:
> > On Fri, Jun 15, 2018 at 09:06:52AM +0200, Andrea Bolognani wrote:
> > > libvirt.git feels like a sensible enough place to store these
> > > files, especially considering that we've been storing pretty
> > > much the same information in .travis.yml up until now; that
> > > said, I don't love the idea of tracking what is ultimately
> > > generated data, so I'm open to creating a separate, ad-hoc
> > > repository (libvirt-dockerfiles.git?) instead.
> > 
> > They really should be just generated directly by a script in
> > the libvirt-jenkins-ci repo, we don't need to check the
> > generated files into git anywhere IMHO.
> 
> We *do* need to have the Dockerfiles committed *somewhere* for
> Docker Hub automated builds to work.

Does docker hub only pull from git repos, or does it have other
options for detching the files ?

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

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

Re: [libvirt] [PATCH v2 1/5] docker: Commit initial Dockerfiles

2018-06-15 Thread Andrea Bolognani
On Fri, 2018-06-15 at 10:05 +0100, Daniel P. Berrangé wrote:
> On Fri, Jun 15, 2018 at 09:06:52AM +0200, Andrea Bolognani wrote:
> > libvirt.git feels like a sensible enough place to store these
> > files, especially considering that we've been storing pretty
> > much the same information in .travis.yml up until now; that
> > said, I don't love the idea of tracking what is ultimately
> > generated data, so I'm open to creating a separate, ad-hoc
> > repository (libvirt-dockerfiles.git?) instead.
> 
> They really should be just generated directly by a script in
> the libvirt-jenkins-ci repo, we don't need to check the
> generated files into git anywhere IMHO.

We *do* need to have the Dockerfiles committed *somewhere* for
Docker Hub automated builds to work.

> >  .docker/buildenv-centos-7.Dockerfile   |  70 ++
> >  .docker/buildenv-debian-8.Dockerfile   |  76 +++
> >  .docker/buildenv-debian-9.Dockerfile   |  78 
> >  .docker/buildenv-debian-sid.Dockerfile |  78 
> >  .docker/buildenv-fedora-27.Dockerfile  |  78 
> >  .docker/buildenv-fedora-28.Dockerfile  |  78 
> >  .docker/buildenv-fedora-rawhide.Dockerfile | 102 +
> >  .docker/buildenv-ubuntu-16.Dockerfile  |  79 
> >  .docker/buildenv-ubuntu-18.Dockerfile  |  79 
> 
> Please put them in build-aux/docker, rather than a hidden
> directory.

Will do.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[libvirt] [RFCv2 PATCH 2/5] util: Add memory bandwidth support to resctrl

2018-06-15 Thread bing . niu
From: Bing Niu 

Add memory bandwidth allocation support basing on existing
virresctrl implementation. Two new structures virResctrlInfoMB
and virResctrlAllocMB are introduced.

virResctrlInfoMB is used to record host system MBA supporting
information, e.g., minimum bandwidth allowed, bandwidth
granularity, number of bandwidth controller (same as number of
last level cache).

virResctrlAllocMB is used for allocating memory bandwidth.
Following virResctrlAllocPerType, it also employs a nested
sparse array to indicate whether allocation is available for
particular llc.

Overall, the memory bandwidth allocation follows the same sequence
as existing virresctrl cache allocation model. It exposes
interfaces for probing host's memory bandwidth capability on
initialization time and memory bandwidth allocation on runtime.

Signed-off-by: Bing Niu 
---
 src/libvirt_private.syms |   2 +
 src/util/virresctrl.c| 383 ++-
 src/util/virresctrl.h|  14 ++
 3 files changed, 397 insertions(+), 2 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fc17059..6925062 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2628,6 +2628,7 @@ virResctrlAllocAddPID;
 virResctrlAllocCreate;
 virResctrlAllocDeterminePath;
 virResctrlAllocForeachCache;
+virResctrlAllocForeachMemory;
 virResctrlAllocFormat;
 virResctrlAllocGetID;
 virResctrlAllocGetUnused;
@@ -2635,6 +2636,7 @@ virResctrlAllocNew;
 virResctrlAllocRemove;
 virResctrlAllocSetID;
 virResctrlAllocSetSize;
+virResctrlSetMemoryBandwidth;
 virResctrlInfoGetCache;
 virResctrlInfoNew;
 
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index e62061f..de736b0 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -80,18 +80,21 @@ typedef virResctrlInfoPerType *virResctrlInfoPerTypePtr;
 typedef struct _virResctrlInfoPerLevel virResctrlInfoPerLevel;
 typedef virResctrlInfoPerLevel *virResctrlInfoPerLevelPtr;
 
+typedef struct _virResctrlInfoMB virResctrlInfoMB;
+typedef virResctrlInfoMB *virResctrlInfoMBPtr;
+
 typedef struct _virResctrlAllocPerType virResctrlAllocPerType;
 typedef virResctrlAllocPerType *virResctrlAllocPerTypePtr;
 
 typedef struct _virResctrlAllocPerLevel virResctrlAllocPerLevel;
 typedef virResctrlAllocPerLevel *virResctrlAllocPerLevelPtr;
 
-
+typedef struct _virResctrlAllocMB virResctrlAllocMB;
+typedef virResctrlAllocMB *virResctrlAllocMBPtr;
 /* Class definitions and initializations */
 static virClassPtr virResctrlInfoClass;
 static virClassPtr virResctrlAllocClass;
 
-
 /* virResctrlInfo */
 struct _virResctrlInfoPerType {
 /* Kernel-provided information */
@@ -116,11 +119,29 @@ struct _virResctrlInfoPerLevel {
 virResctrlInfoPerTypePtr *types;
 };
 
+/* Information about memory bandwidth allocation */
+struct _virResctrlInfoMB {
+/* minimum memory bandwidth allowed*/
+unsigned int min_bandwidth;
+/* bandwidth granularity */
+unsigned int bandwidth_granularity;
+/* Maximum number of simultaneous allocations */
+unsigned int max_allocation;
+/* level number of last level cache*/
+unsigned int last_level_cache;
+/* max id of last level cache, this is used to track
+ * how many last level cache available in host system
+ * */
+unsigned int max_id;
+};
+
 struct _virResctrlInfo {
 virObject parent;
 
 virResctrlInfoPerLevelPtr *levels;
 size_t nlevels;
+
+virResctrlInfoMBPtr mb_info;
 };
 
 
@@ -146,6 +167,7 @@ virResctrlInfoDispose(void *obj)
 VIR_FREE(level);
 }
 
+VIR_FREE(resctrl->mb_info);
 VIR_FREE(resctrl->levels);
 }
 
@@ -202,12 +224,23 @@ struct _virResctrlAllocPerLevel {
  * VIR_CACHE_TYPE_LAST number of items */
 };
 
+/*
+ * virResctrlAllocMB represents one memory bandwidth allocation. Since it can 
have
+ * multiple last level caches in a NUMA system, it is also represented as a 
nested
+ * sparse arrays as virRestrlAllocPerLevel
+ */
+struct _virResctrlAllocMB {
+unsigned int **bandwidth;
+size_t nsizes;
+};
+
 struct _virResctrlAlloc {
 virObject parent;
 
 virResctrlAllocPerLevelPtr *levels;
 size_t nlevels;
 
+virResctrlAllocMBPtr mba;
 /* The identifier (any unique string for now) */
 char *id;
 /* libvirt-generated path in /sys/fs/resctrl for this particular
@@ -251,6 +284,13 @@ virResctrlAllocDispose(void *obj)
 VIR_FREE(level);
 }
 
+if (alloc->mba) {
+virResctrlAllocMBPtr mba = alloc->mba;
+for (i = 0; i < mba->nsizes; i++)
+VIR_FREE(mba->bandwidth[i]);
+}
+
+VIR_FREE(alloc->mba);
 VIR_FREE(alloc->id);
 VIR_FREE(alloc->path);
 VIR_FREE(alloc->levels);
@@ -440,6 +480,61 @@ virResctrlGetCacheInfo(virResctrlInfoPtr resctrl, DIR 
*dirp)
 }
 
 
+static int
+virResctrlGetMemoryBandwidthInfo(virResctrlInfoPtr resctrl)
+{
+int ret = -1;
+int rv = -1;
+virResctrlInfoMBPtr i_mb = NULL;
+
+/* query memory bandwidth alloc

[libvirt] [RFCv2 PATCH 4/5] conf: Introduce cputune/memorytune to support memory bandwidth allocation

2018-06-15 Thread bing . niu
From: Bing Niu 

Introduce a new section memorytune to support memory bandwidth allocation.
This is consistent with existing cachetune  . As the example
below:





id--- on which node memory bandwidth to be set
bandwidth --- the memory bandwidth percent to set.

Signed-off-by: Bing Niu 
---
 src/conf/domain_conf.c | 260 +
 1 file changed, 260 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b3543f3..dbfd69f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19076,6 +19076,189 @@ virDomainCachetuneDefParse(virDomainDefPtr def,
 }
 
 
+static int
+virDomainMemorytuneDefParseMemory(xmlXPathContextPtr ctxt,
+  xmlNodePtr node,
+  virResctrlAllocPtr alloc)
+{
+xmlNodePtr oldnode = ctxt->node;
+unsigned int id;
+unsigned int bandwidth;
+char *tmp = NULL;
+int ret = -1;
+
+ctxt->node = node;
+
+tmp = virXMLPropString(node, "id");
+if (!tmp) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("Missing memorytune attribute 'id'"));
+goto cleanup;
+}
+if (virStrToLong_uip(tmp, NULL, 10, &id) < 0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("Invalid memorytune attribute 'id' value '%s'"),
+   tmp);
+goto cleanup;
+}
+VIR_FREE(tmp);
+
+tmp = virXMLPropString(node, "bandwidth");
+if (!tmp) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("Missing memorytune attribute 'bandwidth'"));
+goto cleanup;
+}
+if (virStrToLong_uip(tmp, NULL, 10, &bandwidth) < 0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("Invalid memorytune attribute 'bandwidth' value 
'%s'"),
+   tmp);
+goto cleanup;
+}
+VIR_FREE(tmp);
+if (virResctrlSetMemoryBandwidth(alloc, id, bandwidth) < 0)
+goto cleanup;
+
+ret = 0;
+ cleanup:
+ctxt->node = oldnode;
+VIR_FREE(tmp);
+return ret;
+}
+
+
+static int
+virDomainMemorytuneDefParse(virDomainDefPtr def,
+   xmlXPathContextPtr ctxt,
+   xmlNodePtr node,
+   unsigned int flags)
+{
+xmlNodePtr oldnode = ctxt->node;
+xmlNodePtr *nodes = NULL;
+virBitmapPtr vcpus = NULL;
+virResctrlAllocPtr alloc = NULL;
+virDomainRestuneDefPtr tmp_cachetune = NULL;
+char *tmp = NULL;
+char *vcpus_str = NULL;
+char *alloc_id = NULL;
+ssize_t i = 0;
+int n;
+int ret = -1;
+bool new_alloc = false;
+
+ctxt->node = node;
+
+if (VIR_ALLOC(tmp_cachetune) < 0)
+goto cleanup;
+
+vcpus_str = virXMLPropString(node, "vcpus");
+if (!vcpus_str) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("Missing memorytune attribute 'vcpus'"));
+goto cleanup;
+}
+if (virBitmapParse(vcpus_str, &vcpus, VIR_DOMAIN_CPUMASK_LEN) < 0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("Invalid memorytune attribute 'vcpus' value '%s'"),
+   vcpus_str);
+goto cleanup;
+}
+
+/* We need to limit the bitmap to number of vCPUs.  If there's nothing 
left,
+ * then we can just clean up and return 0 immediately */
+virBitmapShrink(vcpus, def->maxvcpus);
+
+if (virBitmapIsAllClear(vcpus)) {
+ret = 0;
+goto cleanup;
+}
+
+if ((n = virXPathNodeSet("./node", ctxt, &nodes)) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Cannot extract memory nodes under memorytune"));
+goto cleanup;
+}
+for (i = 0; i < def->nrestunes; i++) {
+/* vcpus group has been created, directly use the existing one.
+ * Just updating memory allocation information of that group
+ * */
+if (virBitmapEqual(def->restunes[i]->vcpus, vcpus)) {
+alloc = def->restunes[i]->alloc;
+break;
+}
+
+/* vcpus can not overlapping */
+if (virBitmapOverlaps(def->restunes[i]->vcpus, vcpus)) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("Overlapping vcpus in memorytunes"));
+goto cleanup;
+}
+}
+
+if (!alloc) {
+alloc = virResctrlAllocNew();
+new_alloc = true;
+}
+
+for (i = 0; i < n; i++) {
+if (virDomainMemorytuneDefParseMemory(ctxt, nodes[i], alloc) < 0)
+goto cleanup;
+}
+/*
+ * If this is a new allocation, format ID and append to restune, otherwise
+ * just update the existing alloc information
+ * */
+if (new_alloc) {
+if (virResctrlAllocIsEmpty(alloc)) {
+ret = 0;
+goto cleanup;
+}
+
+
+/* We need to format it back because we need to be consistent in the 

[libvirt] [RFCv2 PATCH 1/5] util: Rename and packing parts of virresctrl

2018-06-15 Thread bing . niu
From: Bing Niu 

Renaming some functions and packing some CAT logic into functions

Signed-off-by: Bing Niu 
---
 src/conf/domain_conf.c   |  2 +-
 src/libvirt_private.syms |  2 +-
 src/util/virresctrl.c| 93 +++-
 src/util/virresctrl.h| 16 -
 4 files changed, 70 insertions(+), 43 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 522e0c2..62c412e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -26882,7 +26882,7 @@ virDomainCachetuneDefFormat(virBufferPtr buf,
 int ret = -1;
 
 virBufferSetChildIndent(&childrenBuf, buf);
-virResctrlAllocForeachSize(cachetune->alloc,
+virResctrlAllocForeachCache(cachetune->alloc,
virDomainCachetuneDefFormatHelper,
&childrenBuf);
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ea24f28..fc17059 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2627,7 +2627,7 @@ virCacheTypeToString;
 virResctrlAllocAddPID;
 virResctrlAllocCreate;
 virResctrlAllocDeterminePath;
-virResctrlAllocForeachSize;
+virResctrlAllocForeachCache;
 virResctrlAllocFormat;
 virResctrlAllocGetID;
 virResctrlAllocGetUnused;
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index e492a63..e62061f 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -316,11 +316,9 @@ virResctrlUnlock(int fd)
 }
 
 
-/* virResctrlInfo-related definitions */
 static int
-virResctrlGetInfo(virResctrlInfoPtr resctrl)
+virResctrlGetCacheInfo(virResctrlInfoPtr resctrl, DIR *dirp)
 {
-DIR *dirp = NULL;
 char *endptr = NULL;
 char *tmp_str = NULL;
 int ret = -1;
@@ -332,12 +330,6 @@ virResctrlGetInfo(virResctrlInfoPtr resctrl)
 virResctrlInfoPerLevelPtr i_level = NULL;
 virResctrlInfoPerTypePtr i_type = NULL;
 
-rv = virDirOpenIfExists(&dirp, SYSFS_RESCTRL_PATH "/info");
-if (rv <= 0) {
-ret = rv;
-goto cleanup;
-}
-
 while ((rv = virDirRead(dirp, &ent, SYSFS_RESCTRL_PATH "/info")) > 0) {
 VIR_DEBUG("Parsing info type '%s'", ent->d_name);
 if (ent->d_name[0] != 'L')
@@ -443,12 +435,33 @@ virResctrlGetInfo(virResctrlInfoPtr resctrl)
 
 ret = 0;
  cleanup:
-VIR_DIR_CLOSE(dirp);
 VIR_FREE(i_type);
 return ret;
 }
 
 
+/* virResctrlInfo-related definitions */
+static int
+virResctrlGetInfo(virResctrlInfoPtr resctrl)
+{
+DIR *dirp = NULL;
+int ret = -1;
+
+ret = virDirOpenIfExists(&dirp, SYSFS_RESCTRL_PATH "/info");
+if (ret <= 0)
+goto cleanup;
+
+ret = virResctrlGetCacheInfo(resctrl, dirp);
+if (ret < 0)
+goto cleanup;
+
+ret = 0;
+ cleanup:
+VIR_DIR_CLOSE(dirp);
+return ret;
+}
+
+
 virResctrlInfoPtr
 virResctrlInfoNew(void)
 {
@@ -773,8 +786,8 @@ virResctrlAllocSetSize(virResctrlAllocPtr alloc,
 
 
 int
-virResctrlAllocForeachSize(virResctrlAllocPtr alloc,
-   virResctrlAllocForeachSizeCallback cb,
+virResctrlAllocForeachCache(virResctrlAllocPtr alloc,
+   virResctrlAllocForeachCacheCallback cb,
void *opaque)
 {
 int ret = 0;
@@ -835,17 +848,14 @@ virResctrlAllocGetID(virResctrlAllocPtr alloc)
 }
 
 
-char *
-virResctrlAllocFormat(virResctrlAllocPtr alloc)
+static int
+virResctrlAllocFormatCache(virResctrlAllocPtr alloc, virBufferPtr buf)
 {
-virBuffer buf = VIR_BUFFER_INITIALIZER;
+int ret = -1;
 unsigned int level = 0;
 unsigned int type = 0;
 unsigned int cache = 0;
 
-if (!alloc)
-return NULL;
-
 for (level = 0; level < alloc->nlevels; level++) {
 virResctrlAllocPerLevelPtr a_level = alloc->levels[level];
 
@@ -858,7 +868,7 @@ virResctrlAllocFormat(virResctrlAllocPtr alloc)
 if (!a_type)
 continue;
 
-virBufferAsprintf(&buf, "L%u%s:", level, 
virResctrlTypeToString(type));
+virBufferAsprintf(buf, "L%u%s:", level, 
virResctrlTypeToString(type));
 
 for (cache = 0; cache < a_type->nmasks; cache++) {
 virBitmapPtr mask = a_type->masks[cache];
@@ -868,20 +878,37 @@ virResctrlAllocFormat(virResctrlAllocPtr alloc)
 continue;
 
 mask_str = virBitmapToString(mask, false, true);
-if (!mask_str) {
-virBufferFreeAndReset(&buf);
-return NULL;
-}
+if (!mask_str)
+return ret;
 
-virBufferAsprintf(&buf, "%u=%s;", cache, mask_str);
+virBufferAsprintf(buf, "%u=%s;", cache, mask_str);
 VIR_FREE(mask_str);
 }
 
-virBufferTrim(&buf, ";", 1);
-virBufferAddChar(&buf, '\n');
+virBufferTrim(buf, ";", 1);
+virBufferAddChar(buf, '\n');
 }
 }
 
+ret = 0;
+virBufferCheckError(buf);
+return ret;
+}

[libvirt] [RFCv2 PATCH 3/5] conf: rename cachetune to restune

2018-06-15 Thread bing . niu
From: Bing Niu 

resctrl not only supports cache tuning, but also memory bandwidth
tuning. Rename cachetune to restune(resource tuning) to reflect
that.

Signed-off-by: Bing Niu 
---
 src/conf/domain_conf.c  | 44 ++--
 src/conf/domain_conf.h  | 10 +-
 src/qemu/qemu_process.c | 18 +-
 3 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 62c412e..b3543f3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2950,14 +2950,14 @@ virDomainLoaderDefFree(virDomainLoaderDefPtr loader)
 
 
 static void
-virDomainCachetuneDefFree(virDomainCachetuneDefPtr cachetune)
+virDomainRestuneDefFree(virDomainRestuneDefPtr restune)
 {
-if (!cachetune)
+if (!restune)
 return;
 
-virObjectUnref(cachetune->alloc);
-virBitmapFree(cachetune->vcpus);
-VIR_FREE(cachetune);
+virObjectUnref(restune->alloc);
+virBitmapFree(restune->vcpus);
+VIR_FREE(restune);
 }
 
 
@@ -3147,9 +3147,9 @@ void virDomainDefFree(virDomainDefPtr def)
 virDomainShmemDefFree(def->shmems[i]);
 VIR_FREE(def->shmems);
 
-for (i = 0; i < def->ncachetunes; i++)
-virDomainCachetuneDefFree(def->cachetunes[i]);
-VIR_FREE(def->cachetunes);
+for (i = 0; i < def->nrestunes; i++)
+virDomainRestuneDefFree(def->restunes[i]);
+VIR_FREE(def->restunes);
 
 VIR_FREE(def->keywrap);
 
@@ -18971,7 +18971,7 @@ virDomainCachetuneDefParse(virDomainDefPtr def,
 xmlNodePtr *nodes = NULL;
 virBitmapPtr vcpus = NULL;
 virResctrlAllocPtr alloc = virResctrlAllocNew();
-virDomainCachetuneDefPtr tmp_cachetune = NULL;
+virDomainRestuneDefPtr tmp_restune = NULL;
 char *tmp = NULL;
 char *vcpus_str = NULL;
 char *alloc_id = NULL;
@@ -18984,7 +18984,7 @@ virDomainCachetuneDefParse(virDomainDefPtr def,
 if (!alloc)
 goto cleanup;
 
-if (VIR_ALLOC(tmp_cachetune) < 0)
+if (VIR_ALLOC(tmp_restune) < 0)
 goto cleanup;
 
 vcpus_str = virXMLPropString(node, "vcpus");
@@ -19025,8 +19025,8 @@ virDomainCachetuneDefParse(virDomainDefPtr def,
 goto cleanup;
 }
 
-for (i = 0; i < def->ncachetunes; i++) {
-if (virBitmapOverlaps(def->cachetunes[i]->vcpus, vcpus)) {
+for (i = 0; i < def->nrestunes; i++) {
+if (virBitmapOverlaps(def->restunes[i]->vcpus, vcpus)) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("Overlapping vcpus in cachetunes"));
 goto cleanup;
@@ -19056,16 +19056,16 @@ virDomainCachetuneDefParse(virDomainDefPtr def,
 if (virResctrlAllocSetID(alloc, alloc_id) < 0)
 goto cleanup;
 
-VIR_STEAL_PTR(tmp_cachetune->vcpus, vcpus);
-VIR_STEAL_PTR(tmp_cachetune->alloc, alloc);
+VIR_STEAL_PTR(tmp_restune->vcpus, vcpus);
+VIR_STEAL_PTR(tmp_restune->alloc, alloc);
 
-if (VIR_APPEND_ELEMENT(def->cachetunes, def->ncachetunes, tmp_cachetune) < 
0)
+if (VIR_APPEND_ELEMENT(def->restunes, def->nrestunes, tmp_restune) < 0)
 goto cleanup;
 
 ret = 0;
  cleanup:
 ctxt->node = oldnode;
-virDomainCachetuneDefFree(tmp_cachetune);
+virDomainRestuneDefFree(tmp_restune);
 virObjectUnref(alloc);
 virBitmapFree(vcpus);
 VIR_FREE(alloc_id);
@@ -26874,7 +26874,7 @@ virDomainCachetuneDefFormatHelper(unsigned int level,
 
 static int
 virDomainCachetuneDefFormat(virBufferPtr buf,
-virDomainCachetuneDefPtr cachetune,
+virDomainRestuneDefPtr restune,
 unsigned int flags)
 {
 virBuffer childrenBuf = VIR_BUFFER_INITIALIZER;
@@ -26882,7 +26882,7 @@ virDomainCachetuneDefFormat(virBufferPtr buf,
 int ret = -1;
 
 virBufferSetChildIndent(&childrenBuf, buf);
-virResctrlAllocForeachCache(cachetune->alloc,
+virResctrlAllocForeachCache(restune->alloc,
virDomainCachetuneDefFormatHelper,
&childrenBuf);
 
@@ -26895,14 +26895,14 @@ virDomainCachetuneDefFormat(virBufferPtr buf,
 goto cleanup;
 }
 
-vcpus = virBitmapFormat(cachetune->vcpus);
+vcpus = virBitmapFormat(restune->vcpus);
 if (!vcpus)
 goto cleanup;
 
 virBufferAsprintf(buf, "alloc);
+const char *alloc_id = virResctrlAllocGetID(restune->alloc);
 if (!alloc_id)
 goto cleanup;
 
@@ -27023,8 +27023,8 @@ virDomainCputuneDefFormat(virBufferPtr buf,
  def->iothreadids[i]->iothread_id);
 }
 
-for (i = 0; i < def->ncachetunes; i++)
-virDomainCachetuneDefFormat(&childrenBuf, def->cachetunes[i], flags);
+for (i = 0; i < def->nrestunes; i++)
+virDomainCachetuneDefFormat(&childrenBuf, def->restunes[i], flags);
 
 if (virBufferCheckError(&childrenBuf) < 0)
 return -1;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 6344c02..9e71a0b 1006

[libvirt] [RFCv2 PATCH 5/5] conf: add memory bandwidth allocation capability of host

2018-06-15 Thread bing . niu
From: Bing Niu 

Add new XML section to report host's memory bandwidth allocation
capability. The format as below example:

 
 .







granularity    granularity of memory bandwidth, unit percentage.
min    minimum memory bandwidth allowed, unit percentage.
maxAllocs  maximum memory bandwidth allocation group supported.

Signed-off-by: Bing Niu 
---
 src/conf/capabilities.c | 108 
 src/conf/capabilities.h |  11 +
 src/util/virresctrl.c   |  20 +
 src/util/virresctrl.h   |  15 +++
 4 files changed, 154 insertions(+)

diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 7a810ef..3bba153 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -198,6 +198,16 @@ virCapabilitiesFreeNUMAInfo(virCapsPtr caps)
 }
 
 static void
+virCapsHostMemoryNodeFree(virCapsHostMemoryNodePtr ptr)
+{
+if (!ptr)
+return;
+
+virBitmapFree(ptr->cpus);
+VIR_FREE(ptr);
+}
+
+static void
 virCapabilitiesClearSecModel(virCapsHostSecModelPtr secmodel)
 {
 size_t i;
@@ -239,6 +249,11 @@ virCapsDispose(void *object)
 virCapsHostCacheBankFree(caps->host.caches[i]);
 VIR_FREE(caps->host.caches);
 
+for (i = 0; i < caps->host.nnodes; i++)
+virCapsHostMemoryNodeFree(caps->host.nodes[i]);
+VIR_FREE(caps->host.nodes);
+
+
 VIR_FREE(caps->host.netprefix);
 VIR_FREE(caps->host.pagesSize);
 virCPUDefFree(caps->host.cpu);
@@ -957,6 +972,58 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
 return 0;
 }
 
+static int
+virCapabilitiesFormatMemory(virBufferPtr buf,
+size_t nnodes,
+virCapsHostMemoryNodePtr *nodes)
+{
+size_t i = 0;
+virBuffer controlBuf = VIR_BUFFER_INITIALIZER;
+
+if (!nnodes)
+return 0;
+
+virBufferAddLit(buf, "\n");
+virBufferAdjustIndent(buf, 2);
+
+for (i = 0; i < nnodes; i++) {
+virCapsHostMemoryNodePtr node = nodes[i];
+virResctrlInfoPerNodePtr control = &node->control;
+char *cpus_str = virBitmapFormat(node->cpus);
+
+if (!cpus_str)
+return -1;
+
+virBufferAsprintf(buf,
+  "id, cpus_str);
+VIR_FREE(cpus_str);
+
+virBufferSetChildIndent(&controlBuf, buf);
+virBufferAsprintf(&controlBuf,
+  "\n",
+  control->granularity, control->min,
+  control->max_allocation);
+
+if (virBufferCheckError(&controlBuf) < 0)
+return -1;
+
+if (virBufferUse(&controlBuf)) {
+virBufferAddLit(buf, ">\n");
+virBufferAddBuffer(buf, &controlBuf);
+virBufferAddLit(buf, "\n");
+} else {
+virBufferAddLit(buf, "/>\n");
+}
+}
+
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, "\n");
+
+return 0;
+}
+
 /**
  * virCapabilitiesFormatXML:
  * @caps: capabilities to format
@@ -1060,6 +1127,10 @@ virCapabilitiesFormatXML(virCapsPtr caps)
 caps->host.caches) < 0)
 goto error;
 
+if (virCapabilitiesFormatMemory(&buf, caps->host.nnodes,
+caps->host.nodes) < 0)
+goto error;
+
 for (i = 0; i < caps->host.nsecModels; i++) {
 virBufferAddLit(&buf, "\n");
 virBufferAdjustIndent(&buf, 2);
@@ -1602,6 +1673,40 @@ virCapabilitiesInitResctrl(virCapsPtr caps)
 }
 
 
+static int
+virCapabilitiesInitResctrlMemory(virCapsPtr caps)
+{
+virCapsHostMemoryNodePtr node = NULL;
+size_t i = 0;
+int ret = -1;
+
+for (i = 0; i < caps->host.ncaches; i++) {
+virCapsHostCacheBankPtr bank = caps->host.caches[i];
+if (VIR_ALLOC(node) < 0)
+goto cleanup;
+
+if (virResctrlInfoGetMemory(caps->host.resctrl, bank->level, 
&node->control) > 0) {
+node->id = bank->id;
+if (!(node->cpus = virBitmapNewCopy(bank->cpus)))
+goto cleanup;
+
+if (VIR_APPEND_ELEMENT(caps->host.nodes,
+   caps->host.nnodes,
+   node) < 0) {
+goto cleanup;
+}
+}
+virCapsHostMemoryNodeFree(node);
+node = NULL;
+}
+
+ret = 0;
+ cleanup:
+virCapsHostMemoryNodeFree(node);
+return ret;
+}
+
+
 int
 virCapabilitiesInitCaches(virCapsPtr caps)
 {
@@ -1731,6 +1836,9 @@ virCapabilitiesInitCaches(virCapsPtr caps)
 qsort(caps->host.caches, caps->host.ncaches,
   sizeof(*caps->host.caches), virCapsHostCacheBankSorter);
 
+if (virCapabilitiesInitResctrlMemory(caps) < 0)
+goto cleanup;
+
 ret = 0;
  cleanup:
 VIR_FREE(type);
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index fe1b9ea..73e165e 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -15

[libvirt] [RFCv2 PATCH 0/5] Introduce RDT memory bandwidth allocation support

2018-06-15 Thread bing . niu
From: Bing Niu 

This series is to introduce RDT memory bandwidth allocation support by extending
current virresctrl implementation.

The Memory Bandwidth Allocation (MBA) feature provides indirect and approximate
control over memory bandwidth available per-core. This feature provides a 
method to
control applications which may be over-utilizing bandwidth relative to their 
priority
in environments such as the data-center. The details can be found in Intel's 
SDM 17.19.7.
Kernel supports MBA through resctrl file system same as CAT. Each resctrl group 
have a
MB parameter to control how much memory bandwidth it can utilize in unit of 
percentage.

In this series, MBA is enabled by enhancing existing virresctrl implementation. 
The
policy employed for MBA is similar with CAT: The sum of each MBA group's 
bandwidth
dose not exceed 100%. 

The enhancement of virresctrl include two parts:

Part 1:  Add two new structures virResctrlInfoMB and virResctrlAllocMB for 
collecting
 host system MBA capability and domain memory bandwidth allocation. 
Those two
 structures are the extension of existing virResctrlInfo and 
virResctrlAlloc. With
 them, virresctrl framework can support MBA and CAT concurrently. Each 
virResctrlAlloc
 represent a resource allocation including CAT, or MBA, or CAT&MBA. The 
policy of MBA is
 that: total memory bandwidth of each resctrl group, created by 
virresctrl, does not
 exceed to 100%.

Part 2:  On XML part, add new elements to host capabilities query and domain 
allocation to support
 memory bandwidth allocation.
 
--
  
 For host capabilities XML, new XML format like below example,
   
.
 
   
 
   
 
   
  
   granularity --- memory bandwidth granularity
   min --- minimum memory bandwidth allowed
   maxAllocs   --- maximum concurrent memory bandwidth allocation 
allowed.

 
--
  
 For domain XML, new format as below example
   
 .. 
 
   ..
   1024
   
 
   
 
 ..
   

  id --- node where memory bandwidth allocation will happen
  bandwidth  --- bandwidth allocated in percentage

 
--
  

Changelog:
v1->v2:
Jano's comment: 1. put renaming parts into separated patches.
2. set the initial return value as -1.
3. using full name in structure definition.
4. do not use VIR_CACHE_TYPE_LAST for memory 
bandwidth allocation formatting.

Pavel's comment: 1. add host capabilities XML for memory bandwidth 
allocation.
 2. do not mix use cachetune section in XML for 
memory bandwidth allocation in
domain XML. define a dedicated one for memory 
bandwidth allocation.

Bing Niu (5):
  util: Rename and packing parts of virresctrl
  util: Add memory bandwidth support to resctrl
  conf: rename cachetune to restune
  conf: Introduce  cputune/memorytune to support memory bandwidth
allocation
  conf: add memory bandwidth allocation capability of host

 src/conf/capabilities.c  | 112 +++
 src/conf/capabilities.h  |  11 ++
 src/conf/domain_conf.c   | 304 ++---
 src/conf/domain_conf.h   |  10 +-
 src/libvirt_private.syms |   4 +-
 src/qemu/qemu_process.c  |  18 +-
 src/util/virresctrl.c| 496 ---
 src/util/virresctrl.h|  35 +++-
 8 files changed, 918 insertions(+), 72 deletions(-)

-- 
2.7.4

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


Re: [libvirt] [jenkins-ci POC 2/2] guests: Add Dockerfile generator

2018-06-15 Thread Daniel P . Berrangé
On Fri, Jun 15, 2018 at 08:53:44AM +0200, Andrea Bolognani wrote:
> This is basically the exact same algorithm used by the
> Ansible playbooks to process package mappings, implemented
> as a standalone script.
> 
> There's little to none error handling, and some information
> is hardcoded instead of being configurable at runtime; more
> importantly, before it can be considered for merging it
> needs to be integrated into lcitool, which in turn requires
> lcitool to be significantly reworked.

I'm not understanding why it needs to be integrated into
lcitool ?  Generating dockerfiles has no dependancy /
interaction with ansible updating the jenkins slaves.

Why won't we just run this script separately when needed,
ideally as non-root ?  Looks good enough to commit now
IMHO


Reviewed-by: Daniel P. Berrangé 

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

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

Re: [libvirt] [jenkins-ci POC 1/2] guests: Remove OS detection

2018-06-15 Thread Daniel P . Berrangé
On Fri, Jun 15, 2018 at 08:53:43AM +0200, Andrea Bolognani wrote:
> We're going to need information about the OS outside of
> Ansible soon, so drop the OS detection code and start
> storing the information as host variables instead.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  guests/host_vars/libvirt-centos-7/main.yml|  4 ++
>  guests/host_vars/libvirt-debian-8/main.yml|  4 ++
>  guests/host_vars/libvirt-debian-9/main.yml|  4 ++
>  guests/host_vars/libvirt-debian-sid/main.yml  |  4 ++
>  guests/host_vars/libvirt-fedora-27/main.yml   |  4 ++
>  guests/host_vars/libvirt-fedora-28/main.yml   |  4 ++
>  .../host_vars/libvirt-fedora-rawhide/main.yml |  4 ++
>  guests/host_vars/libvirt-freebsd-10/main.yml  |  4 ++
>  guests/host_vars/libvirt-freebsd-11/main.yml  |  4 ++
>  .../libvirt-freebsd-current/main.yml  |  4 ++
>  guests/host_vars/libvirt-ubuntu-16/main.yml   |  4 ++
>  guests/host_vars/libvirt-ubuntu-18/main.yml   |  4 ++
>  guests/tasks/base.yml | 51 ---
>  13 files changed, 48 insertions(+), 51 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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

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

Re: [libvirt] [PATCH v2 5/5] travis: Add MinGW builds

2018-06-15 Thread Daniel P . Berrangé
On Fri, Jun 15, 2018 at 09:06:56AM +0200, Andrea Bolognani wrote:
> We build on Fedora Rawhide, same as on the CentOS CI
> environment.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  .travis.yml | 24 
>  1 file changed, 24 insertions(+)

Reviewed-by: Daniel P. Berrangé 


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

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

Re: [libvirt] [PATCH v2 4/5] travis: Add CentOS 7 build

2018-06-15 Thread Daniel P . Berrangé
On Fri, Jun 15, 2018 at 09:06:55AM +0200, Andrea Bolognani wrote:
> Now that we use pre-built Docker images, it's very easy
> to extend our test matrix; adding CentOS 7 is a good start.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  .travis.yml | 6 ++
>  1 file changed, 6 insertions(+)

Reviewed-by: Daniel P. Berrangé 


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

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

Re: [libvirt] [PATCH v2 3/5] travis: Use pre-built Docker images

2018-06-15 Thread Daniel P . Berrangé
On Fri, Jun 15, 2018 at 09:06:54AM +0200, Andrea Bolognani wrote:
> Instead of starting from the minimal Ubuntu 18.04 base
> image and installing all requirements at build time,
> use a Docker image that has been specifically tailored
> at building libvirt and thus already includes all
> required packages.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  .travis.yml | 75 ++---
>  1 file changed, 2 insertions(+), 73 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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

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

Re: [libvirt] [PATCH v2 1/5] docker: Commit initial Dockerfiles

2018-06-15 Thread Daniel P . Berrangé
On Fri, Jun 15, 2018 at 09:06:52AM +0200, Andrea Bolognani wrote:
> These have been generated from the build dependency data
> available in the libvirt-jenkins-ci repository, and will
> be refreshed periodically to keep them in sync the same
> way we've updated .travis.yml so far; my guess, based on
> that effort, is that we'll need to do so about once per
> release.
> 
> Automated builds will be set up on Docker Hub so that
> changes to the Dockerfiles will cause the images to be
> regenerated, and with that in place (a subset of) the
> resulting images will be used in the Travis CI pipeline,
> as well of course as being available to developers for
> testing and debugging purposes.
> 
> Signed-off-by: Andrea Bolognani 
> ---
> POC script used to generate the files:
> 
>   https://www.redhat.com/archives/libvir-list/2018-June/msg01238.html
> 
> Preview of what the images will look like:
> 
>   https://hub.docker.com/r/andreabolognani/buildenv-centos-7/
>   https://hub.docker.com/r/andreabolognani/buildenv-fedora-rawhide/
>   https://hub.docker.com/r/andreabolognani/buildenv-ubuntu-18/
> 
> libvirt.git feels like a sensible enough place to store these
> files, especially considering that we've been storing pretty
> much the same information in .travis.yml up until now; that
> said, I don't love the idea of tracking what is ultimately
> generated data, so I'm open to creating a separate, ad-hoc
> repository (libvirt-dockerfiles.git?) instead.

They really should be just generated directly by a script in
the libvirt-jenkins-ci repo, we don't need to check the
generated files into git anywhere IMHO.

But for short term, this patch is fine.

> 
>  .docker/buildenv-centos-7.Dockerfile   |  70 ++
>  .docker/buildenv-debian-8.Dockerfile   |  76 +++
>  .docker/buildenv-debian-9.Dockerfile   |  78 
>  .docker/buildenv-debian-sid.Dockerfile |  78 
>  .docker/buildenv-fedora-27.Dockerfile  |  78 
>  .docker/buildenv-fedora-28.Dockerfile  |  78 
>  .docker/buildenv-fedora-rawhide.Dockerfile | 102 +
>  .docker/buildenv-ubuntu-16.Dockerfile  |  79 
>  .docker/buildenv-ubuntu-18.Dockerfile  |  79 

Please put them in build-aux/docker, rather than a hidden
directory.

If that is changed:

  Reviewed-by: Daniel P. Berrangé 

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

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

Re: [libvirt] [PATCH v2 2/5] travis: Drop Ubuntu 16.04 build

2018-06-15 Thread Daniel P . Berrangé
On Fri, Jun 15, 2018 at 09:06:53AM +0200, Andrea Bolognani wrote:
> This will make further changes easier; all coverage
> lost due to this will be reintroduced later on.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  .travis.yml | 6 --
>  1 file changed, 6 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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

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

Re: [libvirt] [Qemu-devel] [PATCH RFC] hw/pc: set q35 as the default x86 machine

2018-06-15 Thread Daniel P . Berrangé
On Thu, Jun 14, 2018 at 11:50:56PM -0300, Eduardo Habkost wrote:
> On Thu, Jun 14, 2018 at 09:09:48AM +0100, Daniel P. Berrangé wrote:
> > On Wed, Jun 13, 2018 at 03:05:08PM -0300, Eduardo Habkost wrote:
> > > Getting back to this discussion:
> > > 
> > > On Tue, Jun 05, 2018 at 09:43:00AM +0100, Daniel P. Berrangé wrote:
> > > > On Tue, Jun 05, 2018 at 09:27:46AM +0200, Gerd Hoffmann wrote:
> > > > >   Hi,
> > > > > 
> > > > > > >   Add to that shortcuts like -cdrom
> > > > > > > stop working,
> > > > > > 
> > > > > > Maybe is fixable.
> > > > > 
> > > > > Already fixed for ages.
> > > > > 
> > > > > > I see marking Q35 as the default machine a first step.
> > > > > 
> > > > > Maybe the better option is to go the arm route:  Just don't define a
> > > > > default, so users have to specify pc or q35.  That will make them 
> > > > > notice
> > > > > there is a world beside 'pc', and we also avoid breaking things
> > > > > silently.
> > > > 
> > > > If QEMU removes the default, then libvirt will have to hardcode
> > > > 'pc' as the default to maintain back compatibility, so I don't
> > > > think that ends up as a net win
> > > 
> > > I believe there's consensus that applications blindly relying on
> > > the default machine-type when creating a domain is a bad idea.
> > > 
> > > That said, can we deprecate this feature in libvirt, encourage
> > > applications to always specify an explicit machine-type, thus
> > > making it possible to deprecate the i440fx machine-types one day?
> > 
> > Well from libvirt's POV this scenario arrives if a mgmt app simply omits
> > the relevant element/attribute from the XML config. Deprecating something
> > implies that in future we'd drop support for it, but we're never going
> > to make this mandatory in libvirt as that would be a regression in
> > behaviour from libvirt's POV. So I don't think it is something we would
> > deprecate.
> 
> Does libvirt really have an option, here?  I'm sure that sooner
> or later somebody will distribute QEMU binaries without "pc".

Sure if someone does that, we'll have no choice, but as long as 'pc' is
shipped we shouldn't gratuitously break apps by changing the default.


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

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

Re: [libvirt] [PATCH 4/5] Introcude VIR_CONNECT_GET_ALL_DOMAINS_STATS_BEST_EFFORT

2018-06-15 Thread Daniel P . Berrangé
On Fri, Jun 15, 2018 at 09:19:44AM +0200, Michal Privoznik wrote:
> On 06/14/2018 05:35 PM, Daniel P. Berrangé wrote:
> > On Thu, Jun 14, 2018 at 11:07:43AM +0200, Michal Privoznik wrote:
> >> On 06/13/2018 05:34 PM, John Ferlan wrote:
> >>>
> >>> $SUBJ: "Introduce" and "NO_WAIT"
> >>>
> >>>
> >>> On 06/07/2018 07:59 AM, Michal Privoznik wrote:
>  https://bugzilla.redhat.com/show_bug.cgi?id=1552092
> 
>  If there's a long running job it might cause us to wait 30
>  seconds before we give up acquiring job. This may cause trouble
> >>>
> >>> s/job/the job/
> >>>
> >>> s/may cause trouble/is problematic/
> >>>
>  to interactive applications that fetch stats repeatedly every few
>  seconds.
> 
>  Solution is to introduce
> >>>
> >>> The solution is...
> >>>
>  VIR_CONNECT_GET_ALL_DOMAINS_STATS_BEST_EFFORT flag which tries to
>  acquire job but does not wait if acquiring failed.
> 
>  Signed-off-by: Michal Privoznik 
>  ---
>   include/libvirt/libvirt-domain.h |  1 +
>   src/libvirt-domain.c | 10 ++
>   src/qemu/qemu_driver.c   | 15 ---
>   3 files changed, 23 insertions(+), 3 deletions(-)
> 
>  diff --git a/include/libvirt/libvirt-domain.h 
>  b/include/libvirt/libvirt-domain.h
>  index da773b76cb..1a1d34620d 100644
>  --- a/include/libvirt/libvirt-domain.h
>  +++ b/include/libvirt/libvirt-domain.h
>  @@ -2055,6 +2055,7 @@ typedef enum {
>   VIR_CONNECT_GET_ALL_DOMAINS_STATS_SHUTOFF = 
>  VIR_CONNECT_LIST_DOMAINS_SHUTOFF,
>   VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER = 
>  VIR_CONNECT_LIST_DOMAINS_OTHER,
>   
>  +VIR_CONNECT_GET_ALL_DOMAINS_STATS_BEST_EFFORT = 1 << 29, /* ignore 
>  stalled domains */
> >>>
> >>> "stalled"?  How about "don't wait on other jobs"
> >>
> >> Well, my hidden idea was also that we can "misuse" this flag to not wait
> >> on other places too. For instance, if we find out (somehow) that a
> >> domain is in D state, we would consider it stale even without any job
> >> running on it. Okay, we have no way of detecting if qemu is in D state
> >> right now, but you get my point. If we don't lock this flag down to just
> >> domain jobs (that not all drivers have btw), we can use it more widely.
> > 
> > I would suggest we call it  "NOWAIT" with explanation that we will only
> > report statistics that can be obtained immediately without any blocking,
> > whatever may be the cause.
> 
> Okay, works for me. I'll post v2 shortly.
> 
> > 
> > 
> > On a tangent, I think this problem really calls for a significantly
> > different design approach, medium term.
> > 
> > The bulk stats query APIs were a good step forward on what we had
> > before where users must call many libvirt APIs, but it is still
> > not very scalable. With huge numbers of guests, we're still having
> > to serialize stats query calls into 1000's of QEMU processes.
> > 
> > I think we must work with QEMU to define a better interface, taking
> > advantage of fact we're colocated on the same host. ie we tell QEMU
> > we we want stats exported in memory page , and QEMU will keep
> > that updated at all times.
> > 
> > When libvirt needs the info it can then just read it straight out of
> > the shared memory page, no blocking on any jobs, no QMP serialization,
> > etc.
> > 
> > For that matter, we can do a similar thing in libvirt API too. We can
> > export a shared memory region for applications to use, which we keep
> > updated on some regular interval that app requests. They can then always
> > access updated stats without calling libvirt APIs at all.
> 
> This is clever idea. But qemu is not the only source of stats we gather.
> We also fetch some data from CGroups, /proc, ovs bridge, etc. So libvirt
> would need to add its own stats for client to see. This means there will
> be a function that updates the shared mem every so often (as client
> tells us via some new API?). The same goes for qemu impl. Now imagine
> two clients wanting two different refresh rates with GCD 1 :-)

Yes, the shared mem areas used by qemu would have to be separate from
the one exposed by libvirt. Each QEMU would expose a separate area to
libvirt, and libvirt would aggregate stats from every QEMU, and also
the other places like cgroups, and then put them into its own single
shared mem area.

I would imagine we could be restrictive with refresh rate, eg require
5 seconds, or a multiple of 5 seconds, so we can easily reconcile
multiple client rates.

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

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

Re: [libvirt] [PATCH v2 2/3] cmdDomblkinfo: add --all to show all block devices info

2018-06-15 Thread Chen Hanxiao



At 2018-06-15 05:41:48, "John Ferlan"  wrote:
>
>
>On 06/11/2018 06:52 AM, Chen Hanxiao wrote:
>> From: Chen Hanxiao 
>> 
[...]
>> 
>
>Do you have networked disks in your domain configs? For a non running
>guest, t; otherwise, you would have noted:
>
># virsh domblkinfo $dom --all
>Target CapacityAllocation  Physical
>-
>vda10737418240 2086727680  10737418240
>error: internal error: missing storage backend for network files using
>iscsi protocol
>

Yes, I tested this cases.
This issue already existed for the original domblkinfo, so I didn't change this.
Maybe we should fix it in another patch.

>> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
>> index daa86e8310..22c0b740c6 100644
...
>> +if (device) {
>> +if (virAsprintf(&cap, "%.3lf %s", val_cap, unit_cap) < 0)
>> +goto cleanup;
>> +
>> +if (virAsprintf(&alloc, "%.3lf %s", val_alloc, unit_alloc) < 0)
>> +goto cleanup;
>> +
>> +if (virAsprintf(&phy, "%.3lf %s", val_phy, unit_phy) < 0)
>> +goto cleanup;
>
>it would be fine I think to do:
>
>if (virAsprintf(&cap, "%.3lf %s", val_cap, unit_cap) < 0 ||
>virAsprintf(&alloc, "%.3lf %s", val_alloc, unit_alloc) < 0 ||
>virAsprintf(&phy, "%.3lf %s", val_phy, unit_phy) < 0)
>goto cleanup;
>
>But that's not required.
>

Looks much better, will be changed in the next series.

>> +
>> +vshPrint(ctl, "%-10s %-15s %-15s %-15s\n",
[...]

>> +ctxt->node = disks[i];
>> +target = virXPathString("string(./target/@dev)", ctxt);
>> +
>> +if (virDomainGetBlockInfo(dom, target, &info, 0) < 0)
>> +goto cleanup;
>
>If the domain is not running, then it's possible to return an error for
>a networked disk (e.g. )... This is
>because qemuDomainGetBlockInfo calls qemuStorageLimitsRefresh which
>calls qemuDomainStorageOpenStat and for non local storage the
>virStorageFileInitAs will eventually fail in virStorageFileBackendForType.
>
>A couple of options come to mind...
>
>... let the failure occur as is, so be it...
>
>... check the last error message for code == VIR_ERR_INTERNAL_ERROR and
>domain == VIR_FROM_STORAGE and we have a source protocol from an
>inactive domain, then assume it's a we cannot get there from here.
>
>... Other options?
>
>If we fail virDomainGetBlockInfo we could still display values as long
>as there's an appropriately placed  memset(&info, 0, sizeof(info)). That
>way we display only the name and 0's for everything else. Not optimal,
>but easily described in the man page that an inactive guest, using
>network protocol for storage may not be able to get the size values and
>virsh will display as 0's instead... or get more creative and display
>"-" for each size column.

I prefer this solutions.
Also, I think domblkinfo DOM DEVICE should follow this if it's a network disk.

>
>
>> +
>> +cmdDomblkinfoPrint(ctl, &info, target, human, false);
>> +
>> +VIR_FREE(target);
>> +}
>> +} else {
>> +if (virDomainGetBlockInfo(dom, device, &info, 0) < 0)
>> +goto cleanup;
>> +
>> +cmdDomblkinfoPrint(ctl, &info, NULL, human, false);
>> +}
>>  
>>  ret = true;
>>  
>>   cleanup:
>>  virshDomainFree(dom);
>> +VIR_FREE(target);
>> +VIR_FREE(disks);
>> +xmlXPathFreeContext(ctxt);
>> +xmlFreeDoc(xmldoc);
>>  return ret;
>>  }
>
>Taking the handle the error path option and a bit of poetic license,
>here's some diffs...

Will do in v3.
>
> char *target = NULL;
>+char *protocol = NULL;
>...
> if (all) {
>+bool active = virDomainIsActive(dom) == 1;
>+int rc;
>+
>...
> for (i = 0; i < ndisks; i++) {
> ctxt->node = disks[i];
>+protocol = virXPathString("string(./source/@protocol)", ctxt);
> target = virXPathString("string(./target/@dev)", ctxt);
>...
>-if (virDomainGetBlockInfo(dom, target, &info, 0) < 0)
>-goto cleanup;
>+rc = virDomainGetBlockInfo(dom, target, &info, 0);
>+
>+if (rc < 0) {
>+if (protocol && !active &&
>+virGetLastErrorCode() == VIR_ERR_INTERNAL_ERROR &&
>+virGetLastErrorDomain() == VIR_FROM_STORAGE)
>+memset(&info, 0, sizeof(info));
>+else
>+goto cleanup;
>+}
>...
>+VIR_FREE(protocol);
> VIR_FREE(target);
>
>>  
>> diff --git a/tools/virsh.pod b/tools/virsh.pod
>> index 3f3314a87e..e273011037 100644
>> --- a/tools/virsh.pod
>> +++ b/tools/virsh.pod
>> @@ -949,13 +949,16 @@ B command says that a domain was paused due 
>> to I/O error.
>>  The B command lists all block devices in error state and
>>  the error seen on each of them.
>>  
>> -=item B I I [I<--human>]
>> +=item B I [I I<

Re: [libvirt] [PATCH 2/2] conf: Fix formatting of element in domain capabilities XML

2018-06-15 Thread Erik Skultety
On Thu, Jun 14, 2018 at 05:59:53PM +0200, Ján Tomko wrote:
> On Thu, Jun 14, 2018 at 02:28:03PM +0200, Erik Skultety wrote:
> > We only formatted the  element when QEMU supported the feature when
> > in fact we should always format the element to make clear that libvirt
> > knows about the feature and the fact whether it is or isn't supported
> > depends on QEMU version, in other words if QEMU doesn't support the
> > feature we're going to format the following into the domain capabilities
> > XML:
> >
> > 
> >
> > Signed-off-by: Erik Skultety 
> > ---
> > src/conf/domain_capabilities.c   | 20 
> > 
> > tests/domaincapsschemadata/basic.xml |  1 +
> > tests/domaincapsschemadata/bhyve_basic.x86_64.xml|  1 +
> > tests/domaincapsschemadata/bhyve_fbuf.x86_64.xml |  1 +
> > tests/domaincapsschemadata/bhyve_uefi.x86_64.xml |  1 +
> > tests/domaincapsschemadata/full.xml  |  1 +
> > tests/domaincapsschemadata/libxl-xenfv-usb.xml   |  1 +
> > tests/domaincapsschemadata/libxl-xenfv.xml   |  1 +
> > tests/domaincapsschemadata/libxl-xenpv-usb.xml   |  1 +
> > tests/domaincapsschemadata/libxl-xenpv.xml   |  1 +
> > tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml |  1 +
> > .../qemu_2.12.0-virt.aarch64.xml |  1 +
> > tests/domaincapsschemadata/qemu_2.12.0.ppc64.xml |  1 +
> > tests/domaincapsschemadata/qemu_2.12.0.s390x.xml |  1 +
> > tests/domaincapsschemadata/qemu_2.12.0.x86_64.xml|  1 +
> > .../domaincapsschemadata/qemu_2.6.0-virt.aarch64.xml |  1 +
> > tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml|  1 +
> > tests/domaincapsschemadata/qemu_2.6.0.ppc64.xml  |  1 +
> > tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml |  1 +
> > tests/domaincapsschemadata/qemu_2.7.0.s390x.xml  |  1 +
> > tests/domaincapsschemadata/qemu_2.8.0-tcg.x86_64.xml |  1 +
> > tests/domaincapsschemadata/qemu_2.8.0.s390x.xml  |  1 +
> > tests/domaincapsschemadata/qemu_2.8.0.x86_64.xml |  1 +
> > tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml |  1 +
> > tests/domaincapsschemadata/qemu_2.9.0-tcg.x86_64.xml |  1 +
> > tests/domaincapsschemadata/qemu_2.9.0.x86_64.xml |  1 +
> > 26 files changed, 37 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
> > index e5d943af50..9041e46622 100644
> > --- a/src/conf/domain_capabilities.c
> > +++ b/src/conf/domain_capabilities.c
> > @@ -559,16 +559,20 @@ static void
> > virDomainCapsFeatureSEVFormat(virBufferPtr buf,
> >   virSEVCapabilityPtr const sev)
> > {
> > -if (!sev)
> > -return;
> >
> > -virBufferAddLit(buf, "\n");
> > -virBufferAdjustIndent(buf, 2);
> > -virBufferAsprintf(buf, "%d\n", sev->cbitpos);
> > -virBufferAsprintf(buf, "%d\n",
> > +if (!sev) {
> > +virBufferAddLit(buf, "\n");
> > +} else {
> > +virBufferAddLit(buf, "\n");
> > +virBufferAdjustIndent(buf, 2);
> > +virBufferAsprintf(buf, "%d\n", sev->cbitpos);
> > +virBufferAsprintf(buf, 
> > "%d\n",
> >   sev->reduced_phys_bits);
>
> Same issue as in previous patch.
>
> > -virBufferAdjustIndent(buf, -2);
> > -virBufferAddLit(buf, "\n");
> > +virBufferAdjustIndent(buf, -2);
> > +virBufferAddLit(buf, "\n");
> > +}
>
> This may or may not be nicer with virXMLFormatElement. Thankfully we
> don't format anything if supported=no.
>
> With the reducedPhysBits change incorporated:
>
> Reviewed-by: Ján Tomko 

Thanks, I'll actually have to squash this patch into the previous one, since
either domaincapstest or virschematest complained (depending on the order of the
changes).
I'll make the adjustment before merging.

Erik

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

[libvirt] [PATCH v2 5/5] virsh: Introduce --nowait to domstats

2018-06-15 Thread Michal Privoznik
This new switch can be used to set
VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT flag for stats
fetching API.

Signed-off-by: Michal Privoznik 
---
 tools/virsh-domain-monitor.c |  7 +++
 tools/virsh.pod  | 16 +++-
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 8cbb3db37c..886f7f16b5 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1992,6 +1992,10 @@ static const vshCmdOptDef opts_domstats[] = {
  .type = VSH_OT_BOOL,
  .help = N_("add backing chain information to block stats"),
 },
+{.name = "nowait",
+ .type = VSH_OT_BOOL,
+ .help = N_("report only stats that are accessible instantly"),
+},
 VIRSH_COMMON_OPT_DOMAIN_OT_ARGV(N_("list of domains to get stats for"), 0),
 {.name = NULL}
 };
@@ -2087,6 +2091,9 @@ cmdDomstats(vshControl *ctl, const vshCmd *cmd)
 if (vshCommandOptBool(cmd, "backing"))
 flags |= VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING;
 
+if (vshCommandOptBool(cmd, "nowait"))
+flags |= VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT;
+
 if (vshCommandOptBool(cmd, "domain")) {
 if (VIR_ALLOC_N(domlist, 1) < 0)
 goto cleanup;
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 3f3314a87e..7cb8c8a6e4 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -968,11 +968,11 @@ that require a block device name (such as I or
 I for disk snapshots) will accept either target
 or unique source names printed by this command.
 
-=item B [I<--raw>] [I<--enforce>] [I<--backing>] [I<--state>]
-[I<--cpu-total>] [I<--balloon>] [I<--vcpu>] [I<--interface>] [I<--block>]
-[I<--perf>] [[I<--list-active>] [I<--list-inactive>] [I<--list-persistent>]
-[I<--list-transient>] [I<--list-running>] [I<--list-paused>]
-[I<--list-shutoff>] [I<--list-other>]] | [I ...]
+=item B [I<--raw>] [I<--enforce>] [I<--backing>] [I<--nowait>]
+[I<--state>] [I<--cpu-total>] [I<--balloon>] [I<--vcpu>] [I<--interface>]
+[I<--block>] [I<--perf>] [[I<--list-active>] [I<--list-inactive>]
+[I<--list-persistent>] [I<--list-transient>] [I<--list-running>]
+[I<--list-paused>] [I<--list-shutoff>] [I<--list-other>]] | [I ...]
 
 Get statistics for multiple or all domains. Without any argument this
 command prints all available statistics for all domains.
@@ -1123,6 +1123,12 @@ daemon supports the selected group of stats. Flag 
I<--enforce>
 forces the command to fail if the daemon doesn't support the
 selected group.
 
+When collecting stats libvirtd may wait for some time if there's
+already another job running on given domain for it to finish.
+This may cause unnecessary delay in delivering stats. Using
+I<--nowait> suppresses this behaviour. On the other hand
+some statistics might be missing for such domain.
+
 =item B I [I<--inactive>]
 
 Print a table showing the brief information of all virtual interfaces
-- 
2.16.4

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


[libvirt] [PATCH v2 2/5] qemuDomainObjBeginJobInternal: Remove spurious @ret assignment

2018-06-15 Thread Michal Privoznik
The variable is initialized to -1 already. There's no way it can
be overwritten by the time control gets to the line I'm removing.

Signed-off-by: Michal Privoznik 
Reviewed-by: John Ferlan 
---
 src/qemu/qemu_domain.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index be36a9e3e4..21d54938b6 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6490,7 +6490,6 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
 else
 blocker = priv->job.asyncOwnerAPI;
 
-ret = -1;
 if (errno == ETIMEDOUT) {
 if (blocker) {
 virReportError(VIR_ERR_OPERATION_TIMEOUT,
-- 
2.16.4

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


[libvirt] [PATCH v2 0/5] Introduce VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT

2018-06-15 Thread Michal Privoznik
v2 of:

https://www.redhat.com/archives/libvir-list/2018-June/msg00546.html

diff to v1:
- Rename to STATS_NOWAIT
- John's review worked in
- added comment to qemuDomainObjBeginJobNowait

Michal Privoznik (5):
  qemu_domain: Document qemuDomainObjBeginJob
  qemuDomainObjBeginJobInternal: Remove spurious @ret assignment
  qemu_domain: Introduce qemuDomainObjBeginJobNowait
  Introduce VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT
  virsh: Introduce --nowait to domstats

 include/libvirt/libvirt-domain.h |  2 ++
 src/libvirt-domain.c | 12 
 src/qemu/qemu_domain.c   | 60 +++-
 src/qemu/qemu_domain.h   |  4 +++
 src/qemu/qemu_driver.c   | 15 --
 tools/virsh-domain-monitor.c |  7 +
 tools/virsh.pod  | 16 +++
 7 files changed, 101 insertions(+), 15 deletions(-)

-- 
2.16.4

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


[libvirt] [PATCH v2 4/5] Introduce VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT

2018-06-15 Thread Michal Privoznik
https://bugzilla.redhat.com/show_bug.cgi?id=1552092

If there's a long running job it might cause us to wait 30
seconds before we give up acquiring the job. This is problematic
to interactive applications that fetch stats repeatedly every few
seconds.

The solution is to introduce
VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT flag which tries to
acquire job but does not wait if acquiring failed.

Signed-off-by: Michal Privoznik 
---
 include/libvirt/libvirt-domain.h |  2 ++
 src/libvirt-domain.c | 12 
 src/qemu/qemu_driver.c   | 15 ---
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 3ef7c24528..796f2e1408 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -2055,6 +2055,8 @@ typedef enum {
 VIR_CONNECT_GET_ALL_DOMAINS_STATS_SHUTOFF = 
VIR_CONNECT_LIST_DOMAINS_SHUTOFF,
 VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER = VIR_CONNECT_LIST_DOMAINS_OTHER,
 
+VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT = 1 << 29, /* report statistics 
that can be obtained
+   immediately without 
any blocking */
 VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING = 1 << 30, /* include backing 
chain for block stats */
 VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS = 1U << 31, /* enforce 
requested stats */
 } virConnectGetAllDomainStatsFlags;
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 4a899f31c8..c71f2e6877 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11502,6 +11502,12 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
  * fields for offline domains if the statistics are meaningful only for a
  * running domain.
  *
+ * Passing VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT in
+ * @flags means when libvirt is unable to fetch stats for any of
+ * the domains (for whatever reason) only a subset of statistics
+ * is returned for the domain.  That subset being statistics that
+ * don't involve querying the underlying hypervisor.
+ *
  * Similarly to virConnectListAllDomains, @flags can contain various flags to
  * filter the list of domains to provide stats for.
  *
@@ -11586,6 +11592,12 @@ virConnectGetAllDomainStats(virConnectPtr conn,
  * fields for offline domains if the statistics are meaningful only for a
  * running domain.
  *
+ * Passing VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT in
+ * @flags means when libvirt is unable to fetch stats for any of
+ * the domains (for whatever reason) only a subset of statistics
+ * is returned for the domain.  That subset being statistics that
+ * don't involve querying the underlying hypervisor.
+ *
  * Note that any of the domain list filtering flags in @flags may be rejected
  * by this function.
  *
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 42069ee617..35038889a3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20428,6 +20428,7 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
 virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE |
   VIR_CONNECT_LIST_DOMAINS_FILTERS_PERSISTENT |
   VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE |
+  VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT |
   VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING |
   VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS, -1);
 
@@ -20462,9 +20463,17 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
 
 virObjectLock(vm);
 
-if (HAVE_JOB(privflags) &&
-qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) == 0)
-domflags |= QEMU_DOMAIN_STATS_HAVE_JOB;
+if (HAVE_JOB(privflags)) {
+int rv;
+
+if (flags & VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT)
+rv = qemuDomainObjBeginJobNowait(driver, vm, QEMU_JOB_QUERY);
+else
+rv = qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY);
+
+if (rv == 0)
+domflags |= QEMU_DOMAIN_STATS_HAVE_JOB;
+}
 /* else: without a job it's still possible to gather some data */
 
 if (flags & VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING)
-- 
2.16.4

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


[libvirt] [PATCH v2 3/5] qemu_domain: Introduce qemuDomainObjBeginJobNowait

2018-06-15 Thread Michal Privoznik
The aim of this API is to allow the caller do best effort. Some
functions can work even when acquiring the job fails (e.g.
qemuConnectGetAllDomainStats()). But what they can't bear is
delay if they have to wait up to 30 seconds for each domain that
is processing some other job.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_domain.c | 43 ++-
 src/qemu/qemu_domain.h |  4 
 2 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 21d54938b6..a01067049e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6359,11 +6359,15 @@ qemuDomainJobAllowed(qemuDomainObjPrivatePtr priv, 
qemuDomainJob job)
  * @obj: domain object
  * @job: qemuDomainJob to start
  * @asyncJob: qemuDomainAsyncJob to start
+ * @nowait: don't wait trying to acquire @job
  *
  * Acquires job for a domain object which must be locked before
  * calling. If there's already a job running waits up to
  * QEMU_JOB_WAIT_TIME after which the functions fails reporting
- * an error.
+ * an error unless @nowait is set.
+ * If @nowait is true this function tries to acquire job and if
+ * it fails, then it returns immediately without waiting. No
+ * error is reported in this case.
  *
  * Returns: 0 on success,
  * -2 if unable to start job because of timeout or
@@ -6374,7 +6378,8 @@ static int ATTRIBUTE_NONNULL(1)
 qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
   virDomainObjPtr obj,
   qemuDomainJob job,
-  qemuDomainAsyncJob asyncJob)
+  qemuDomainAsyncJob asyncJob,
+  bool nowait)
 {
 qemuDomainObjPrivatePtr priv = obj->privateData;
 unsigned long long now;
@@ -6414,12 +6419,18 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
 }
 
 while (!nested && !qemuDomainNestedJobAllowed(priv, job)) {
+if (nowait)
+goto cleanup;
+
 VIR_DEBUG("Waiting for async job (vm=%p name=%s)", obj, 
obj->def->name);
 if (virCondWaitUntil(&priv->job.asyncCond, &obj->parent.lock, then) < 
0)
 goto error;
 }
 
 while (priv->job.active) {
+if (nowait)
+goto cleanup;
+
 VIR_DEBUG("Waiting for job (vm=%p name=%s)", obj, obj->def->name);
 if (virCondWaitUntil(&priv->job.cond, &obj->parent.lock, then) < 0)
 goto error;
@@ -6536,7 +6547,7 @@ int qemuDomainObjBeginJob(virQEMUDriverPtr driver,
   qemuDomainJob job)
 {
 if (qemuDomainObjBeginJobInternal(driver, obj, job,
-  QEMU_ASYNC_JOB_NONE) < 0)
+  QEMU_ASYNC_JOB_NONE, false) < 0)
 return -1;
 else
 return 0;
@@ -6551,7 +6562,7 @@ int qemuDomainObjBeginAsyncJob(virQEMUDriverPtr driver,
 qemuDomainObjPrivatePtr priv;
 
 if (qemuDomainObjBeginJobInternal(driver, obj, QEMU_JOB_ASYNC,
-  asyncJob) < 0)
+  asyncJob, false) < 0)
 return -1;
 
 priv = obj->privateData;
@@ -6580,9 +6591,31 @@ qemuDomainObjBeginNestedJob(virQEMUDriverPtr driver,
 
 return qemuDomainObjBeginJobInternal(driver, obj,
  QEMU_JOB_ASYNC_NESTED,
- QEMU_ASYNC_JOB_NONE);
+ QEMU_ASYNC_JOB_NONE,
+ false);
 }
 
+/**
+ * qemuDomainObjBeginJobNowait:
+ *
+ * @driver: qemu driver
+ * @obj: domain object
+ * @job: qemuDomainJob to start
+ *
+ * Acquires job for a domain object which must be locked before
+ * calling. If there's already a job running it returns
+ * immediately without any error reported.
+ *
+ * Returns: see qemuDomainObjBeginJobInternal
+ */
+int
+qemuDomainObjBeginJobNowait(virQEMUDriverPtr driver,
+virDomainObjPtr obj,
+qemuDomainJob job)
+{
+return qemuDomainObjBeginJobInternal(driver, obj, job,
+ QEMU_ASYNC_JOB_NONE, true);
+}
 
 /*
  * obj must be locked and have a reference before calling
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index fd8d9b5305..9e2da0a37c 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -510,6 +510,10 @@ int qemuDomainObjBeginNestedJob(virQEMUDriverPtr driver,
 virDomainObjPtr obj,
 qemuDomainAsyncJob asyncJob)
 ATTRIBUTE_RETURN_CHECK;
+int qemuDomainObjBeginJobNowait(virQEMUDriverPtr driver,
+virDomainObjPtr obj,
+qemuDomainJob job)
+ATTRIBUTE_RETURN_CHECK;
 
 void qemuDomainObjEndJob(virQEMUDriverPtr driver,
  virDomainObjPtr obj);
-- 
2.16.4

--
libvir-list mai

[libvirt] [PATCH v2 1/5] qemu_domain: Document qemuDomainObjBeginJob

2018-06-15 Thread Michal Privoznik
Provide a small comment on the function and its parameters.

Signed-off-by: Michal Privoznik 
Reviewed-by: John Ferlan 
---
 src/qemu/qemu_domain.c | 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 2119233907..be36a9e3e4 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6353,8 +6353,22 @@ qemuDomainJobAllowed(qemuDomainObjPrivatePtr priv, 
qemuDomainJob job)
 /* Give up waiting for mutex after 30 seconds */
 #define QEMU_JOB_WAIT_TIME (1000ull * 30)
 
-/*
- * obj must be locked before calling
+/**
+ * qemuDomainObjBeginJobInternal:
+ * @driver: qemu driver
+ * @obj: domain object
+ * @job: qemuDomainJob to start
+ * @asyncJob: qemuDomainAsyncJob to start
+ *
+ * Acquires job for a domain object which must be locked before
+ * calling. If there's already a job running waits up to
+ * QEMU_JOB_WAIT_TIME after which the functions fails reporting
+ * an error.
+ *
+ * Returns: 0 on success,
+ * -2 if unable to start job because of timeout or
+ *maxQueuedJobs limit,
+ * -1 otherwise.
  */
 static int ATTRIBUTE_NONNULL(1)
 qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
-- 
2.16.4

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


Re: [libvirt] [PATCH 1/2] schema: Fix the definition of SEV in domain capabilities schema

2018-06-15 Thread Erik Skultety
On Thu, Jun 14, 2018 at 05:56:15PM +0200, Ján Tomko wrote:
> On Thu, Jun 14, 2018 at 02:28:02PM +0200, Erik Skultety wrote:
> > The whole  element was optional which it shouldn't be as if the
> > platform doesn't support SEV we should format .
> >
> > Signed-off-by: Erik Skultety 
> > ---
> > docs/schemas/domaincaps.rng | 19 ++-
> > 1 file changed, 10 insertions(+), 9 deletions(-)
> >
> > diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
> > index e25201fc68..ed29823548 100644
> > --- a/docs/schemas/domaincaps.rng
> > +++ b/docs/schemas/domaincaps.rng
> > @@ -185,9 +185,7 @@
> > 
> > 
> > 
> > -
> > -  
> > -
> > +
> >   
> > 
> >   
> > @@ -213,12 +211,15 @@
> >
> >   
> > 
> > -  
> > -
> > -  
> > -  
> > -
> > -  
> > +  
> > +  
> > +
> > +  
> > +
> > +
>
> Actually, s/reduced-phys-bits/reducedPhysBits/

Wow, how did that one got in there?! I didn't even notice this bit to be honest
(since I basically only did a simple code movement), I guess it could have only
happened as part of resolving some merge conflicts with an older branch I got,
so thanks for pointing that out, I'll adjust.

Erik

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

Re: [libvirt] [PATCH v4 3/3] hw/vfio/display: add ramfb support

2018-06-15 Thread Gerd Hoffmann
> > Well, it's simliar to qxl vs. qxl-vga.  It's not qxl,vga={on,off} and
> > libvirt has no problems to deal with that ...
> >
> > Another more technical reason is (again) hotplug.  ramfb needs an fw_cfg
> > entry for configuration, and fw_cfg entries can't be hotplugged.  So
> > hotplugging vfio-pci with ramfb=on isn't going to fly.  So we need a
> > separate device with hotplug turned off.
> 
> Well if that's not supposed to work ever, libvirt's hotplug code could format
> the following FWIW:
> "-device vfio-pci [opts],ramfb=off"
> 
> As such, new device wouldn't be that of an issue for libvirt if vfio-pci and
> vfio-pci-ramfb are back to back compatible with all the device options that 
> are
> available for vfio-pci (I mean in terms of using an mdev). Because in that
> case, what libvirt could do is to look whether we're supposed to turn on the
> display, if so, then we need support for this in capabilities to query and 
> then
> we could prefer this new device over the "legacy" vfio-pci one. However, if we
> expect a case where QEMU would succeed to start with an mdev mapped to this
> new ramfb device but not with vfio-pci, then that's an issue.

vfio-pci and vfio-pci-ramfb are identical, except for the later having a
boot display (with display=on), and vfio-pci-ramfb not being
hotplugable.  So, yes, all pcu/mdev devices should work with both
vfio-pci variants.

> But I'm still curious about the ramfb=off possibility I asked above
> for hotplug nonetheless.

Well, the problem is introspection.  qemu can report via qmp whenever a
specific device supports hotplug.  A device which can both be
hot-pluggable or not hot-pluggable depending on some condition is a
problem here ...

cheers,
  Gerd

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


Re: [libvirt] [PATCH v2 0/5] travis: Use pre-built Docker images

2018-06-15 Thread Andrea Bolognani
On Fri, 2018-06-15 at 09:06 +0200, Andrea Bolognani wrote:
> Andrea Bolognani (5):
>   docker: Commit initial Dockerfiles
>   travis: Drop Ubuntu 16.04 build
>   travis: Use pre-built Docker images
>   travis: Add CentOS 7 build
>   travis: Add MinGW builds

This is what it looks like in action:

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

... which also proves it actually works ;)

-- 
Andrea Bolognani / Red Hat / Virtualization

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


Re: [libvirt] [PATCH 4/5] Introcude VIR_CONNECT_GET_ALL_DOMAINS_STATS_BEST_EFFORT

2018-06-15 Thread Michal Privoznik
On 06/14/2018 05:35 PM, Daniel P. Berrangé wrote:
> On Thu, Jun 14, 2018 at 11:07:43AM +0200, Michal Privoznik wrote:
>> On 06/13/2018 05:34 PM, John Ferlan wrote:
>>>
>>> $SUBJ: "Introduce" and "NO_WAIT"
>>>
>>>
>>> On 06/07/2018 07:59 AM, Michal Privoznik wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1552092

 If there's a long running job it might cause us to wait 30
 seconds before we give up acquiring job. This may cause trouble
>>>
>>> s/job/the job/
>>>
>>> s/may cause trouble/is problematic/
>>>
 to interactive applications that fetch stats repeatedly every few
 seconds.

 Solution is to introduce
>>>
>>> The solution is...
>>>
 VIR_CONNECT_GET_ALL_DOMAINS_STATS_BEST_EFFORT flag which tries to
 acquire job but does not wait if acquiring failed.

 Signed-off-by: Michal Privoznik 
 ---
  include/libvirt/libvirt-domain.h |  1 +
  src/libvirt-domain.c | 10 ++
  src/qemu/qemu_driver.c   | 15 ---
  3 files changed, 23 insertions(+), 3 deletions(-)

 diff --git a/include/libvirt/libvirt-domain.h 
 b/include/libvirt/libvirt-domain.h
 index da773b76cb..1a1d34620d 100644
 --- a/include/libvirt/libvirt-domain.h
 +++ b/include/libvirt/libvirt-domain.h
 @@ -2055,6 +2055,7 @@ typedef enum {
  VIR_CONNECT_GET_ALL_DOMAINS_STATS_SHUTOFF = 
 VIR_CONNECT_LIST_DOMAINS_SHUTOFF,
  VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER = 
 VIR_CONNECT_LIST_DOMAINS_OTHER,
  
 +VIR_CONNECT_GET_ALL_DOMAINS_STATS_BEST_EFFORT = 1 << 29, /* ignore 
 stalled domains */
>>>
>>> "stalled"?  How about "don't wait on other jobs"
>>
>> Well, my hidden idea was also that we can "misuse" this flag to not wait
>> on other places too. For instance, if we find out (somehow) that a
>> domain is in D state, we would consider it stale even without any job
>> running on it. Okay, we have no way of detecting if qemu is in D state
>> right now, but you get my point. If we don't lock this flag down to just
>> domain jobs (that not all drivers have btw), we can use it more widely.
> 
> I would suggest we call it  "NOWAIT" with explanation that we will only
> report statistics that can be obtained immediately without any blocking,
> whatever may be the cause.

Okay, works for me. I'll post v2 shortly.

> 
> 
> On a tangent, I think this problem really calls for a significantly
> different design approach, medium term.
> 
> The bulk stats query APIs were a good step forward on what we had
> before where users must call many libvirt APIs, but it is still
> not very scalable. With huge numbers of guests, we're still having
> to serialize stats query calls into 1000's of QEMU processes.
> 
> I think we must work with QEMU to define a better interface, taking
> advantage of fact we're colocated on the same host. ie we tell QEMU
> we we want stats exported in memory page , and QEMU will keep
> that updated at all times.
> 
> When libvirt needs the info it can then just read it straight out of
> the shared memory page, no blocking on any jobs, no QMP serialization,
> etc.
> 
> For that matter, we can do a similar thing in libvirt API too. We can
> export a shared memory region for applications to use, which we keep
> updated on some regular interval that app requests. They can then always
> access updated stats without calling libvirt APIs at all.

This is clever idea. But qemu is not the only source of stats we gather.
We also fetch some data from CGroups, /proc, ovs bridge, etc. So libvirt
would need to add its own stats for client to see. This means there will
be a function that updates the shared mem every so often (as client
tells us via some new API?). The same goes for qemu impl. Now imagine
two clients wanting two different refresh rates with GCD 1 :-)

Michal

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

[libvirt] [PATCH v2 5/5] travis: Add MinGW builds

2018-06-15 Thread Andrea Bolognani
We build on Fedora Rawhide, same as on the CentOS CI
environment.

Signed-off-by: Andrea Bolognani 
---
 .travis.yml | 24 
 1 file changed, 24 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index 0efa14a1c3..56d25b2ecd 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -19,6 +19,18 @@ matrix:
 - IMAGE="centos-7"
 - DISTCHECK_CONFIGURE_FLAGS="--with-init-script=upstart"
 - DOCKER_CMD="$LINUX_CMD"
+- services:
+- docker
+  env:
+- IMAGE="fedora-rawhide"
+- MINGW="mingw32"
+- DOCKER_CMD="$MINGW_CMD"
+- services:
+- docker
+  env:
+- IMAGE="fedora-rawhide"
+- MINGW="mingw64"
+- DOCKER_CMD="$MINGW_CMD"
 - compiler: clang
   language: c
   os: osx
@@ -32,6 +44,7 @@ script:
   -v $(pwd):/build
   -w /build
   -e VIR_TEST_DEBUG="$VIR_TEST_DEBUG"
+  -e MINGW="$MINGW"
   -e DISTCHECK_CONFIGURE_FLAGS="$DISTCHECK_CONFIGURE_FLAGS"
   "libvirt/buildenv-$IMAGE"
   /bin/sh -xc "$DOCKER_CMD"
@@ -53,6 +66,17 @@ env:
   exit 1
 )
   "
+- MINGW_CMD="
+NOCONFIGURE=1 ./autogen.sh &&
+\$MINGW-configure &&
+make -j3 ||
+(
+  echo '=== LOG FILE(S) START ===';
+  find -name test-suite.log | xargs cat;
+  echo '=== LOG FILE(S) END ===';
+  exit 1
+)
+  "
 # We can't run 'distcheck' or 'syntax-check' because they fail on
 # macOS, but doing 'install' and 'dist' gives us some useful coverage
 - MACOS_CMD="
-- 
2.17.1

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


[libvirt] [PATCH v2 4/5] travis: Add CentOS 7 build

2018-06-15 Thread Andrea Bolognani
Now that we use pre-built Docker images, it's very easy
to extend our test matrix; adding CentOS 7 is a good start.

Signed-off-by: Andrea Bolognani 
---
 .travis.yml | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index 7a90c4a251..0efa14a1c3 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -13,6 +13,12 @@ matrix:
 - IMAGE="ubuntu-18"
 - DISTCHECK_CONFIGURE_FLAGS="--with-init-script=systemd"
 - DOCKER_CMD="$LINUX_CMD"
+- services:
+- docker
+  env:
+- IMAGE="centos-7"
+- DISTCHECK_CONFIGURE_FLAGS="--with-init-script=upstart"
+- DOCKER_CMD="$LINUX_CMD"
 - compiler: clang
   language: c
   os: osx
-- 
2.17.1

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


[libvirt] [PATCH v2 0/5] travis: Use pre-built Docker images

2018-06-15 Thread Andrea Bolognani
Changes from [v1]:

* store Dockerfiles in git so that we can have automated builds
  on Docker Hub;

* add MinGW builds;

* change image naming convention, from eg. libvirt/build:fedora-28
  to libvirt/buildenv-fedora-28.


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

Andrea Bolognani (5):
  docker: Commit initial Dockerfiles
  travis: Drop Ubuntu 16.04 build
  travis: Use pre-built Docker images
  travis: Add CentOS 7 build
  travis: Add MinGW builds

 .docker/buildenv-centos-7.Dockerfile   |  70 ++
 .docker/buildenv-debian-8.Dockerfile   |  76 +++
 .docker/buildenv-debian-9.Dockerfile   |  78 
 .docker/buildenv-debian-sid.Dockerfile |  78 
 .docker/buildenv-fedora-27.Dockerfile  |  78 
 .docker/buildenv-fedora-28.Dockerfile  |  78 
 .docker/buildenv-fedora-rawhide.Dockerfile | 102 +
 .docker/buildenv-ubuntu-16.Dockerfile  |  79 
 .docker/buildenv-ubuntu-18.Dockerfile  |  79 
 .travis.yml| 101 ++--
 10 files changed, 745 insertions(+), 74 deletions(-)
 create mode 100644 .docker/buildenv-centos-7.Dockerfile
 create mode 100644 .docker/buildenv-debian-8.Dockerfile
 create mode 100644 .docker/buildenv-debian-9.Dockerfile
 create mode 100644 .docker/buildenv-debian-sid.Dockerfile
 create mode 100644 .docker/buildenv-fedora-27.Dockerfile
 create mode 100644 .docker/buildenv-fedora-28.Dockerfile
 create mode 100644 .docker/buildenv-fedora-rawhide.Dockerfile
 create mode 100644 .docker/buildenv-ubuntu-16.Dockerfile
 create mode 100644 .docker/buildenv-ubuntu-18.Dockerfile

-- 
2.17.1

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


[libvirt] [PATCH v2 1/5] docker: Commit initial Dockerfiles

2018-06-15 Thread Andrea Bolognani
These have been generated from the build dependency data
available in the libvirt-jenkins-ci repository, and will
be refreshed periodically to keep them in sync the same
way we've updated .travis.yml so far; my guess, based on
that effort, is that we'll need to do so about once per
release.

Automated builds will be set up on Docker Hub so that
changes to the Dockerfiles will cause the images to be
regenerated, and with that in place (a subset of) the
resulting images will be used in the Travis CI pipeline,
as well of course as being available to developers for
testing and debugging purposes.

Signed-off-by: Andrea Bolognani 
---
POC script used to generate the files:

  https://www.redhat.com/archives/libvir-list/2018-June/msg01238.html

Preview of what the images will look like:

  https://hub.docker.com/r/andreabolognani/buildenv-centos-7/
  https://hub.docker.com/r/andreabolognani/buildenv-fedora-rawhide/
  https://hub.docker.com/r/andreabolognani/buildenv-ubuntu-18/

libvirt.git feels like a sensible enough place to store these
files, especially considering that we've been storing pretty
much the same information in .travis.yml up until now; that
said, I don't love the idea of tracking what is ultimately
generated data, so I'm open to creating a separate, ad-hoc
repository (libvirt-dockerfiles.git?) instead.

 .docker/buildenv-centos-7.Dockerfile   |  70 ++
 .docker/buildenv-debian-8.Dockerfile   |  76 +++
 .docker/buildenv-debian-9.Dockerfile   |  78 
 .docker/buildenv-debian-sid.Dockerfile |  78 
 .docker/buildenv-fedora-27.Dockerfile  |  78 
 .docker/buildenv-fedora-28.Dockerfile  |  78 
 .docker/buildenv-fedora-rawhide.Dockerfile | 102 +
 .docker/buildenv-ubuntu-16.Dockerfile  |  79 
 .docker/buildenv-ubuntu-18.Dockerfile  |  79 
 9 files changed, 718 insertions(+)
 create mode 100644 .docker/buildenv-centos-7.Dockerfile
 create mode 100644 .docker/buildenv-debian-8.Dockerfile
 create mode 100644 .docker/buildenv-debian-9.Dockerfile
 create mode 100644 .docker/buildenv-debian-sid.Dockerfile
 create mode 100644 .docker/buildenv-fedora-27.Dockerfile
 create mode 100644 .docker/buildenv-fedora-28.Dockerfile
 create mode 100644 .docker/buildenv-fedora-rawhide.Dockerfile
 create mode 100644 .docker/buildenv-ubuntu-16.Dockerfile
 create mode 100644 .docker/buildenv-ubuntu-18.Dockerfile

diff --git a/.docker/buildenv-centos-7.Dockerfile 
b/.docker/buildenv-centos-7.Dockerfile
new file mode 100644
index 00..5d92bfabc5
--- /dev/null
+++ b/.docker/buildenv-centos-7.Dockerfile
@@ -0,0 +1,70 @@
+FROM centos:centos7
+ENV PACKAGES audit-libs-devel \
+ augeas \
+ autoconf \
+ automake \
+ avahi-devel \
+ bash \
+ bash-completion \
+ chrony \
+ cyrus-sasl-devel \
+ dbus-devel \
+ device-mapper-devel \
+ dnsmasq \
+ ebtables \
+ fuse-devel \
+ gcc \
+ gettext \
+ gettext-devel \
+ git \
+ glibc-common \
+ glibc-devel \
+ glusterfs-api-devel \
+ gnutls-devel \
+ iproute \
+ iscsi-initiator-utils \
+ libacl-devel \
+ libattr-devel \
+ libblkid-devel \
+ libcap-ng-devel \
+ libcurl-devel \
+ libnl3-devel \
+ libpcap-devel \
+ libpciaccess-devel \
+ librbd1-devel \
+ libselinux-devel \
+ libssh-devel \
+ libssh2-devel \
+ libtirpc-devel \
+ libtool \
+ libudev-devel \
+ libwsman-devel \
+ libxml2 \
+ libxml2-devel \
+ libxslt \
+ lvm2 \
+ make \
+ netcf-devel \
+ nfs-utils \
+ numactl-devel \
+ numad \
+ parted \
+ parted-devel \
+ patch \
+ perl \
+ pkgconfig \
+ polkit \
+ qemu-img \
+ radvd \
+ readline-devel \
+ rpm-build \
+ sanlock-devel \
+ screen \
+ scrub \
+ sudo \
+ systemtap-sdt-devel \
+ vim \
+ yajl-devel
+RUN yum install -y ${PACKAGES} && \
+yum autoremove -y && \
+yum clean all -y
diff --git a/.docker/buildenv-debian-8.Dockerfile 
b/.docker/buildenv-debian-8.Dockerfile
new file mode 100644
index 00..0766cc99e9
--- /dev/null
+++ b/.docker/buildenv-debian-8.Dockerfile
@@ -0,0 +1,76 @@
+FROM debian:8
+ENV PACKAGES augeas-tools \
+ autoconf \
+ automake \
+ autopoint \
+ bash \
+ bash-compl

[libvirt] [PATCH v2 3/5] travis: Use pre-built Docker images

2018-06-15 Thread Andrea Bolognani
Instead of starting from the minimal Ubuntu 18.04 base
image and installing all requirements at build time,
use a Docker image that has been specifically tailored
at building libvirt and thus already includes all
required packages.

Signed-off-by: Andrea Bolognani 
---
 .travis.yml | 75 ++---
 1 file changed, 2 insertions(+), 73 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 67ad155148..7a90c4a251 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -10,7 +10,7 @@ matrix:
 - services:
 - docker
   env:
-- IMAGE=ubuntu:18.04
+- IMAGE="ubuntu-18"
 - DISTCHECK_CONFIGURE_FLAGS="--with-init-script=systemd"
 - DOCKER_CMD="$LINUX_CMD"
 - compiler: clang
@@ -23,13 +23,11 @@ matrix:
 
 script:
   - docker run
-  --privileged
   -v $(pwd):/build
   -w /build
   -e VIR_TEST_DEBUG="$VIR_TEST_DEBUG"
-  -e PACKAGES="$PACKAGES"
   -e DISTCHECK_CONFIGURE_FLAGS="$DISTCHECK_CONFIGURE_FLAGS"
-  "$IMAGE"
+  "libvirt/buildenv-$IMAGE"
   /bin/sh -xc "$DOCKER_CMD"
 
 git:
@@ -39,8 +37,6 @@ env:
   global:
 - VIR_TEST_DEBUG=1
 - LINUX_CMD="
-apt-get update &&
-apt-get install -y \$PACKAGES &&
 ./autogen.sh &&
 make -j3 syntax-check &&
 make -j3 distcheck 
DISTCHECK_CONFIGURE_FLAGS=\"\$DISTCHECK_CONFIGURE_FLAGS\" ||
@@ -67,73 +63,6 @@ env:
   exit 1
 )
   "
-# Please keep this list sorted alphabetically
-- PACKAGES="
-augeas-tools
-autoconf
-automake
-autopoint
-bash-completion
-ccache
-dnsmasq-base
-dwarves
-ebtables
-gcc
-gettext
-git
-glusterfs-client
-libacl1-dev
-libapparmor-dev
-libattr1-dev
-libaudit-dev
-libavahi-client-dev
-libblkid-dev
-libc6-dev
-libcap-ng-dev
-libc-dev-bin
-libdbus-1-dev
-libdevmapper-dev
-libfuse-dev
-libgnutls28-dev
-libnetcf-dev
-libnl-3-dev
-libnl-route-3-dev
-libnuma-dev
-libopenwsman-dev
-libparted-dev
-libpcap-dev
-libpciaccess-dev
-librbd-dev
-libreadline-dev
-libsanlock-dev
-libsasl2-dev
-libselinux1-dev
-libssh2-1-dev
-libssh-dev
-libtirpc-dev
-libtool
-libudev-dev
-libxen-dev
-libxml2-dev
-libxml2-utils
-libyajl-dev
-lvm2
-make
-nfs-common
-open-iscsi
-parted
-patch
-perl
-pkgconf
-policykit-1
-qemu-utils
-radvd
-scrub
-sheepdog
-systemtap-sdt-dev
-xsltproc
-zfs-fuse
-  "
 
 notifications:
   irc:
-- 
2.17.1

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


[libvirt] [PATCH v2 2/5] travis: Drop Ubuntu 16.04 build

2018-06-15 Thread Andrea Bolognani
This will make further changes easier; all coverage
lost due to this will be reintroduced later on.

Signed-off-by: Andrea Bolognani 
---
 .travis.yml | 6 --
 1 file changed, 6 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index a4a0bbb072..67ad155148 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -13,12 +13,6 @@ matrix:
 - IMAGE=ubuntu:18.04
 - DISTCHECK_CONFIGURE_FLAGS="--with-init-script=systemd"
 - DOCKER_CMD="$LINUX_CMD"
-- services:
-- docker
-  env:
-- IMAGE=ubuntu:16.04
-- DISTCHECK_CONFIGURE_FLAGS="--with-init-script=upstart"
-- DOCKER_CMD="$LINUX_CMD"
 - compiler: clang
   language: c
   os: osx
-- 
2.17.1

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