Re: [libvirt] Expose vfio device display/migration to libvirt and above, was Re: [PATCH 0/3] sample: vfio mdev display devices.

[Erik Skultety]

On Thu, May 03, 2018 at 12:58:00PM -0600, Alex Williamson wrote:
> Hi,
>
> The previous discussion hasn't produced results, so let's start over.
> Here's the situation:
>
>  - We currently have kernel and QEMU support for the QEMU vfio-pci
>display option.
>
>  - The default for this option is 'auto', so the device will attempt to
>generate a display if the underlying device supports it, currently
>only GVTg and some future release of NVIDIA vGPU (plus Gerd's
>sample mdpy and mbochs).
>
>  - The display option is implemented via two different mechanism, a
>vfio region (NVIDIA, mdpy) or a dma-buf (GVTg, mbochs).
>
>  - Displays using dma-buf require OpenGL support, displays making
>use of region support do not.
>
>  - Enabling OpenGL support requires specific VM configurations, which
>libvirt /may/ want to facilitate.
>
>  - Probing display support for a given device is complicated by the
>fact that GVTg and NVIDIA both impose requirements on the process
>opening the device file descriptor through the vfio API:
>
>- GVTg requires a KVM association or will fail to allow the device
>  to be opened.

How exactly is this association checked?

>
>- NVIDIA requires that their vgpu-manager process can locate a UUID
>  for the VM via the process commandline.
>
>- These are both horrible impositions and prevent libvirt from
>  simply probing the device itself.

So I feel like we're trying to solve a problem coming from one layer on a bunch
of different layers which inherently prevents us to produce a viable long term
solution without dragging a significant amount of hacky nasty code and it is
not the missing sysfs attributes I have in mind. Why does NVIDIA's vgpu-manager
need to locate a UUID of a qemu VM? I assume that's to prevent multiple VM
instances trying to use the same mdev device, in which case can't the
vgpu-manager track references to how many "open" and "close" calls have been
made to the same device? This is just from a layman's perspective, but it would
allow the following:
- when libvirt starts, it initializes all its drivers (let's focus on
  QEMU)
- as part of this initialization, libvirt probes QEMU for capabilities and
  caches them in order to use them when spawning VMs

Now, if we (theoretically) can settle on easing the restrictions Alex has
mentioned, we in fact could introduce a QMP command to probe these devices and
provide libvirt with useful information at that point in time. Of course, since
the 3rd party vendor is "de-coupled" from qemu, libvirt would have no way to
find out that the driver has changed in the meantime, thus still using the old
information we gathered, ergo potentially causing the QEMU process to fail
eventually. But then again, there's very often a strong recommendation to reboot
your host after a driver update, especially in NVIDIA's case, which means this
fact wouldn't matter. However, there's also a significant drawback to my
proposal which probably renders it completely useless (but we can continue from
there...) and that is the devices would either have to be present already (not
an option) or QEMU would need to be enhanced in a way, that it would create a
dummy device during QMP probing, open it, collect the information libvirt
needs, close it and remove it. If the driver doesn't change in the meantime,
this should be sufficient for a VM to be successfully instantiated with a
display, right?

>
> The above has pressed the need for investigating some sort of
> alternative API through which libvirt might introspect a vfio device
> and with vfio device migration on the horizon, it's natural that some
> sort of support for migration state compatibility for the device need be
> considered as a second user of such an API.  However, we currently have
> no concept of migration compatibility on a per-device level as there
> are no migratable devices that live outside of the QEMU code base.
> It's therefore assumed that per device migration compatibility is
> encompassed by the versioned machine type for the overall VM.  We need
> participation all the way to the top of the VM management stack to
> resolve this issue and it's dragging down the (possibly) more simple
> question of how do we resolve the display situation.  Therefore I'm
> looking for alternatives for display that work within what we have
> available to us at the moment.
>
> Erik Skultety, who initially raised the display question, has identified
> one possible solution, which is to simply make the display configuration
> the user's problem (apologies if I've misinterpreted Erik).  I believe
> this would work something like:
>
>  - libvirt identifies a version of QEMU that includes 'display' support
>for vfio-pci devices and defaults to adding display=off for every
>vfio-pci device [have we chosen the wrong default (auto) in QEMU?].

>From libvirt's POV, having a new XML attribute display to the host device type
mdev should with

[libvirt] [dbus PATCH v2 1/9] Abandon usage of all *TypeToString functions in domain.c

[Katerina Koukiou]

Converting ENUMS to str can be user friendly though
it can be problematic in matters of backward compatibility.

In particular when some ENUM in libvirt API will introduce a
new constant, libvirt-dbus will fail with:

size of array ‘_GStaticAssertCompileTimeAssertion_5’ is negative

So using ints is preferable to avoid the previous issue.

Signed-off-by: Katerina Koukiou 
---
 data/org.libvirt.Domain.xml |  14 ++--
 src/domain.c| 172 
 tests/test_domain.py|   6 +-
 3 files changed, 25 insertions(+), 167 deletions(-)

diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
index db43b1c..6448a46 100644
--- a/data/org.libvirt.Domain.xml
+++ b/data/org.libvirt.Domain.xml
@@ -191,19 +191,19 @@
 value="See 
https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetBlockJobInfo"/>
   
   
-  
+  
 
 
   https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetControlInfo"/>
   
-  
+  
 
 
   https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetDiskErrors"/>
   
-  
+  
 
 
   https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetJobStats"/>
   
-  
+  
 
 
   https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetMetadata
Empty string can be used to pass a NULL as @uri argument."/>
-  
+  
   
   
   
@@ -319,7 +319,7 @@
 
   https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainInterfaceAddresses"/>
-  
+  
   
   
 
@@ -492,7 +492,7 @@
   https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainSetMetadata
  Empty string can be used to pass a NULL as @key or @uri 
argument."/>
-  
+  
   
   
   
diff --git a/src/domain.c b/src/domain.c
index e305fa3..40cf2f7 100644
--- a/src/domain.c
+++ b/src/domain.c
@@ -3,75 +3,6 @@
 
 #include 
 
-VIRT_DBUS_ENUM_DECL(virtDBusDomainBlockJob)
-VIRT_DBUS_ENUM_IMPL(virtDBusDomainBlockJob,
-VIR_DOMAIN_BLOCK_JOB_TYPE_LAST,
-"unknown",
-"pull",
-"copy",
-"commit",
-"active-commit")
-
-VIRT_DBUS_ENUM_DECL(virtDBusDomainControlErrorReason)
-VIRT_DBUS_ENUM_IMPL(virtDBusDomainControlErrorReason,
-VIR_DOMAIN_CONTROL_ERROR_REASON_LAST,
-"none",
-"unknown",
-"monitor",
-"internal")
-
-VIRT_DBUS_ENUM_DECL(virtDBusDomainControlState)
-VIRT_DBUS_ENUM_IMPL(virtDBusDomainControlState,
-VIR_DOMAIN_CONTROL_LAST,
-"ok",
-"job",
-"occupied",
-"error")
-
-VIRT_DBUS_ENUM_DECL(virtDBusDomainDiskError)
-VIRT_DBUS_ENUM_IMPL(virtDBusDomainDiskError,
-VIR_DOMAIN_DISK_ERROR_LAST,
-"none",
-"unspec",
-"no-space")
-
-VIRT_DBUS_ENUM_DECL(virtDBusDomainInterfaceAddressesSource)
-VIRT_DBUS_ENUM_IMPL(virtDBusDomainInterfaceAddressesSource,
-VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LAST,
-"lease",
-"agent")
-
-VIRT_DBUS_ENUM_DECL(virtDBusDomainJob)
-VIRT_DBUS_ENUM_IMPL(virtDBusDomainJob,
-VIR_DOMAIN_JOB_LAST,
-"none",
-"bounded",
-"unbounded",
-"completed",
-"failed",
-"canceled")
-
-VIRT_DBUS_ENUM_DECL(virtDBusDomainMemoryStat)
-VIRT_DBUS_ENUM_IMPL(virtDBusDomainMemoryStat,
-VIR_DOMAIN_MEMORY_STAT_LAST,
-"swap_in",
-"swap_out",
-"major_fault",
-"minor_fault",
-"unused",
-"available",
-"actual_baloon",
-"rss",
-"usable",
-"last_update")
-
-VIRT_DBUS_ENUM_DECL(virtDBusDomainMetadata)
-VIRT_DBUS_ENUM_IMPL(virtDBusDomainMetadata,
-VIR_DOMAIN_METADATA_LAST,
-"description",
-"title",
-"element")
-
 struct _virtDBusDomainFSInfoList {
 virDomainFSInfoPtr *info;
 gint count;
@@ -137,12 +68,8 @@ virtDBusDomainMemoryStatsToGVariant(virDomainMemoryStatPtr 
stats,
 
 g_variant_builder_init(&builder, G_VARIANT_TYPE("a{st}"));
 
-for (gint i = 0; i < nr_stats; i++) {
-const gchar *memoryStat = 
virtDBusDomainMemoryStatTypeToString(stats[i].tag);
-if (!memoryStat)
-continue;
-g_variant_builder_add(&builder, "{st}", memoryStat, stats[i].val);
-}
+for (gint i = 0; i < nr_stats; i++)
+g_variant_builder_add(&builder, "{ut}", stats[i].

[libvirt] [dbus PATCH v2 0/9] Remove all enum->string and string->enum code

[Katerina Koukiou]

Changes from v1:
 * Added the three last patches

Katerina Koukiou (9):
  Abandon usage of all *TypeToString functions in domain.c
  Abandon usage of all *TypeToString functions in connect.c
  Abandon usage of all *TypeToString functions in network.c
  Change DomainEvent argument from string to unsigned int
  Change NetworkEvent argument from string to unsigned int
  Remove virtDBusUtilEnum{From,From}String functions
  Remove reason to string translation in virtDBusEventsDomainTrayChange
  Remove state to string translation in virtDBusDomainGetState
  Remove reason to string translation in virtDBusEventsDomainDiskChange

 data/org.libvirt.Connect.xml |   6 +-
 data/org.libvirt.Domain.xml  |  20 ++---
 data/org.libvirt.Network.xml |   6 +-
 src/connect.c|  18 +---
 src/domain.c | 203 ---
 src/events.c |  72 ++-
 src/network.c|  66 +-
 src/util.c   |  27 --
 src/util.h   |  28 --
 tests/libvirttest.py |  32 +++
 tests/test_connect.py|  24 +++--
 tests/test_domain.py |  46 ++
 tests/test_network.py|  20 +++--
 13 files changed, 131 insertions(+), 437 deletions(-)

-- 
2.15.0

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

[libvirt] [dbus PATCH v2 3/9] Abandon usage of all *TypeToString functions in network.c

[Katerina Koukiou]

Converting ENUMS to str can be user friendly though
it can be problematic in matters of backward compatibility.

In particular when some ENUM in libvirt API will introduce a
new constant, libvirt-dbus will fail with:

size of array ‘_GStaticAssertCompileTimeAssertion_5’ is negative

So using ints is preferable to avoid the previous issue.

Signed-off-by: Katerina Koukiou 
---
 data/org.libvirt.Network.xml |  6 ++--
 src/network.c| 66 +++-
 tests/test_network.py|  2 +-
 3 files changed, 8 insertions(+), 66 deletions(-)

diff --git a/data/org.libvirt.Network.xml b/data/org.libvirt.Network.xml
index cf05062..e8c1506 100644
--- a/data/org.libvirt.Network.xml
+++ b/data/org.libvirt.Network.xml
@@ -43,7 +43,7 @@
Empty string will be returned in output for NULL variables."/>
   
   
-  
+  
 
 
   
   https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkUpdate"/>
-  
-  
+  
+  
   
   
   
diff --git a/src/network.c b/src/network.c
index 4d00dfe..256940f 100644
--- a/src/network.c
+++ b/src/network.c
@@ -3,38 +3,6 @@
 
 #include 
 
-VIRT_DBUS_ENUM_DECL(virtDBusNetworkIPAddr)
-VIRT_DBUS_ENUM_IMPL(virtDBusNetworkIPAddr,
-VIR_IP_ADDR_TYPE_LAST,
-"ipv4",
-"ipv6")
-
-VIRT_DBUS_ENUM_DECL(virtDBusNetworkUpdateCommand)
-VIRT_DBUS_ENUM_IMPL(virtDBusNetworkUpdateCommand,
-VIR_NETWORK_UPDATE_COMMAND_LAST,
-"none",
-"modify",
-"delete",
-"add-last",
-"add-first")
-
-VIRT_DBUS_ENUM_DECL(virtDBusNetworkUpdateSection)
-VIRT_DBUS_ENUM_IMPL(virtDBusNetworkUpdateSection,
-VIR_NETWORK_SECTION_LAST,
-"none",
-"bridge",
-"domain",
-"ip",
-"ip-dhcp-host",
-"ip-dhcp-range",
-"forward",
-"forward-interface",
-"forward-pf",
-"portgroup",
-"dns-host",
-"dns-txt",
-"dns-srv")
-
 static void
 virtDBusNetworkDHCPLeaseListFree(virNetworkDHCPLeasePtr *leases)
 {
@@ -284,20 +252,11 @@ virtDBusNetworkGetDHCPLeases(GVariant *inArgs,
 
 g_variant_builder_init(&builder, G_VARIANT_TYPE("a(stuss)"));
 for (gint i = 0; i < nleases; i++) {
-const gchar *typeStr;
-
 virNetworkDHCPLeasePtr lease = leases[i];
 
-typeStr = virtDBusNetworkIPAddrTypeToString(lease->type);
-if (!typeStr) {
-g_set_error(error, VIRT_DBUS_ERROR, VIRT_DBUS_ERROR_LIBVIRT,
-"Can't format virIPAddrType '%d' to string.", 
lease->type);
-return;
-}
-
-g_variant_builder_add(&builder, "(stuss)",
+g_variant_builder_add(&builder, "(stusssuss)",
   lease->iface, lease->expirytime,
-  typeStr, lease->mac,
+  lease->type, lease->mac,
   lease->iaid ? lease->iaid : "" ,
   lease->ipaddr, lease->prefix,
   lease->hostname ? lease->hostname : "",
@@ -366,33 +325,16 @@ virtDBusNetworkUpdate(GVariant *inArgs,
 {
 virtDBusConnect *connect = userData;
 g_autoptr(virNetwork) network = NULL;
-const gchar *commandStr;
 gint command;
-const gchar *sectionStr;
 gint section;
 gint parentIndex;
 const gchar *xml;
 guint flags;
 
-g_variant_get(inArgs, "(&s&si&su)",
-  &commandStr, §ionStr,
+g_variant_get(inArgs, "(uui&su)",
+  &command, §ion,
   &parentIndex, &xml, &flags);
 
-command = virtDBusNetworkUpdateCommandTypeFromString(commandStr);
-if (command < 0) {
-g_set_error(error, VIRT_DBUS_ERROR, VIRT_DBUS_ERROR_LIBVIRT,
-"Can't get valid virNetworkUpdateCommand from string 
'%s'.",
-commandStr);
-return;
-}
-section = virtDBusNetworkUpdateSectionTypeFromString(sectionStr);
-if (section < 0) {
-g_set_error(error, VIRT_DBUS_ERROR, VIRT_DBUS_ERROR_LIBVIRT,
-"Can't get valid virNetworkUpdateSection from string 
'%s'.",
-sectionStr);
-return;
-}
-
 network = virtDBusNetworkGetVirNetwork(connect, objectPath, error);
 if (!network)
 return;
diff --git a/tests/test_network.py b/tests/test_network.py
index 2c1bd21..1340d95 100755
--- a/tests/test_network.py
+++ b/tests/test_network.py
@@ -80,7 +80,7 @@ class TestNetwork(libvirttest.BaseTestClass):
 self.main_loop()
 
 @pytest.mark.parametrize("command, section, parentIndex, xml_str, flags", [
-('add-first', 'ip-dhcp-hos

[libvirt] [dbus PATCH v2 5/9] Change NetworkEvent argument from string to unsigned int

[Katerina Koukiou]

Modify the relevant tests to comply with the new signal.

Note: argument matching in connect_to_signal method is
only usable with string and thus had to be refactored.

Signed-off-by: Katerina Koukiou 
---
 data/org.libvirt.Connect.xml |  2 +-
 src/events.c | 14 +-
 tests/libvirttest.py |  7 +++
 tests/test_connect.py| 12 
 tests/test_network.py| 18 --
 5 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index 386a7bb..8272da6 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -176,7 +176,7 @@
   https://libvirt.org/html/libvirt-libvirt-network.html#virConnectNetworkEventLifecycleCallback"/>
   
-  
+  
 
   
 
diff --git a/src/events.c b/src/events.c
index 601d898..1b584f7 100644
--- a/src/events.c
+++ b/src/events.c
@@ -146,14 +146,6 @@ virtDBusEventsDomainDiskChange(virConnectPtr connection 
G_GNUC_UNUSED,
 return 0;
 }
 
-VIRT_DBUS_ENUM_DECL(virtDBusEventsNetworkEvent)
-VIRT_DBUS_ENUM_IMPL(virtDBusEventsNetworkEvent,
-VIR_NETWORK_EVENT_LAST,
-"Defined",
-"Undefined",
-"Started",
-"Stopped")
-
 static gint
 virtDBusEventsNetworkLifecycle(virConnectPtr connection G_GNUC_UNUSED,
virNetworkPtr network,
@@ -163,10 +155,6 @@ virtDBusEventsNetworkLifecycle(virConnectPtr connection 
G_GNUC_UNUSED,
 {
 virtDBusConnect *connect = opaque;
 g_autofree gchar *path = NULL;
-const gchar *eventStr = virtDBusEventsNetworkEventTypeToString(event);
-
-if (!eventStr)
-return 0;
 
 path = virtDBusUtilBusPathForVirNetwork(network, connect->networkPath);
 
@@ -175,7 +163,7 @@ virtDBusEventsNetworkLifecycle(virConnectPtr connection 
G_GNUC_UNUSED,
   connect->connectPath,
   VIRT_DBUS_CONNECT_INTERFACE,
   "NetworkEvent",
-  g_variant_new("(os)", path, eventStr),
+  g_variant_new("(ou)", path, event),
   NULL);
 
 return 0;
diff --git a/tests/libvirttest.py b/tests/libvirttest.py
index f7f48ed..0e84a94 100644
--- a/tests/libvirttest.py
+++ b/tests/libvirttest.py
@@ -98,3 +98,10 @@ class DomainEvent(IntEnum):
 SHUTDOWN = 6
 PMSUSPENDED = 7
 CRASHED = 8
+
+
+class NetworkEvent(IntEnum):
+DEFINED = 0
+UNDEFINED = 1
+STARTED = 2
+STOPPED = 3
diff --git a/tests/test_connect.py b/tests/test_connect.py
index 41cc134..7748822 100755
--- a/tests/test_connect.py
+++ b/tests/test_connect.py
@@ -125,11 +125,13 @@ class TestConnect(libvirttest.BaseTestClass):
 assert isinstance(self.connect.GetCPUModelNames(arch, 0), dbus.Array)
 
 def test_connect_network_create_xml(self):
-def network_started(path, _event):
+def network_started(path, event):
+if event != libvirttest.NetworkEvent.STARTED:
+return
 assert isinstance(path, dbus.ObjectPath)
 self.loop.quit()
 
-self.connect.connect_to_signal('NetworkEvent', network_started, 
arg1='Started')
+self.connect.connect_to_signal('NetworkEvent', network_started)
 
 path = self.connect.NetworkCreateXML(self.minimal_network_xml)
 assert isinstance(path, dbus.ObjectPath)
@@ -137,11 +139,13 @@ class TestConnect(libvirttest.BaseTestClass):
 self.main_loop()
 
 def test_connect_network_define_xml(self):
-def network_defined(path, _event):
+def network_defined(path, event):
+if event != libvirttest.NetworkEvent.DEFINED:
+return
 assert isinstance(path, dbus.ObjectPath)
 self.loop.quit()
 
-self.connect.connect_to_signal('NetworkEvent', network_defined, 
arg1='Defined')
+self.connect.connect_to_signal('NetworkEvent', network_defined)
 
 path = self.connect.NetworkDefineXML(self.minimal_network_xml)
 assert isinstance(path, dbus.ObjectPath)
diff --git a/tests/test_network.py b/tests/test_network.py
index 1340d95..f34c081 100755
--- a/tests/test_network.py
+++ b/tests/test_network.py
@@ -34,11 +34,13 @@ class TestNetwork(libvirttest.BaseTestClass):
 assert autostart_current == dbus.Boolean(autostart_expected)
 
 def test_network_create(self):
-def domain_started(path, _event):
+def domain_started(path, event):
+if event != libvirttest.NetworkEvent.STARTED:
+return
 assert isinstance(path, dbus.ObjectPath)
 self.loop.quit()
 
-self.connect.connect_to_signal('NetworkEvent', domain_started, 
arg1='Started')
+self.connect.connect_to_signal('NetworkEvent', domain_started)
 
 _,test_network = self.test_network

[libvirt] [dbus PATCH v2 8/9] Remove state to string translation in virtDBusDomainGetState

[Katerina Koukiou]

Adjust tests to comply with the new type.

Signed-off-by: Katerina Koukiou 
---
 data/org.libvirt.Domain.xml |  2 +-
 src/domain.c| 31 +--
 tests/libvirttest.py| 12 
 tests/test_domain.py| 10 +-
 4 files changed, 19 insertions(+), 36 deletions(-)

diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
index 98c018c..3627b1b 100644
--- a/data/org.libvirt.Domain.xml
+++ b/data/org.libvirt.Domain.xml
@@ -32,7 +32,7 @@
   https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetSchedulerType"/>
 
-
+
   https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetState"/>
 
diff --git a/src/domain.c b/src/domain.c
index 40cf2f7..059cd3e 100644
--- a/src/domain.c
+++ b/src/domain.c
@@ -251,7 +251,6 @@ virtDBusDomainGetState(const gchar *objectPath,
 virtDBusConnect *connect = userData;
 g_autoptr(virDomain) domain = NULL;
 gint state = 0;
-const gchar *string;
 
 domain = virtDBusDomainGetVirDomain(connect, objectPath, error);
 if (!domain)
@@ -260,35 +259,7 @@ virtDBusDomainGetState(const gchar *objectPath,
 if (virDomainGetState(domain, &state, NULL, 0) < 0)
 return virtDBusUtilSetLastVirtError(error);
 
-switch (state) {
-case VIR_DOMAIN_NOSTATE:
-default:
-string = "nostate";
-break;
-case VIR_DOMAIN_RUNNING:
-string = "running";
-break;
-case VIR_DOMAIN_BLOCKED:
-string = "blocked";
-break;
-case VIR_DOMAIN_PAUSED:
-string = "paused";
-break;
-case VIR_DOMAIN_SHUTDOWN:
-string = "shutdown";
-break;
-case VIR_DOMAIN_SHUTOFF:
-string = "shutoff";
-break;
-case VIR_DOMAIN_CRASHED:
-string = "crashed";
-break;
-case VIR_DOMAIN_PMSUSPENDED:
-string = "pmsuspended";
-break;
-}
-
-*value = g_variant_new("s", string);
+*value = g_variant_new("u", state);
 }
 
 static void
diff --git a/tests/libvirttest.py b/tests/libvirttest.py
index 0e84a94..06ac0e4 100644
--- a/tests/libvirttest.py
+++ b/tests/libvirttest.py
@@ -100,6 +100,18 @@ class DomainEvent(IntEnum):
 CRASHED = 8
 
 
+class DomainState(IntEnum):
+NOSTATE = 0
+RUNNING = 1
+BLOCKED = 2
+PAUSED = 3
+SHUTDOWN = 4
+SHUTOFF = 5
+CRASHED = 6
+PMSUSPENDED = 7
+LAST = 8
+
+
 class NetworkEvent(IntEnum):
 DEFINED = 0
 UNDEFINED = 1
diff --git a/tests/test_domain.py b/tests/test_domain.py
index 2def6c1..c7e09cd 100755
--- a/tests/test_domain.py
+++ b/tests/test_domain.py
@@ -19,7 +19,7 @@ class TestDomain(libvirttest.BaseTestClass):
 assert any([isinstance(props['SchedulerType'], dbus.Struct),
 isinstance(props['SchedulerType'][0], dbus.String),
 isinstance(props['SchedulerType'][1], dbus.Int32)])
-assert isinstance(props['State'], dbus.String)
+assert isinstance(props['State'], dbus.UInt32)
 assert isinstance(props['Updated'], dbus.Boolean)
 assert isinstance(props['UUID'], dbus.String)
 
@@ -59,7 +59,7 @@ class TestDomain(libvirttest.BaseTestClass):
 domain.ManagedSave(0)
 assert domain.HasManagedSaveImage(0) == dbus.Boolean(True)
 state = obj.Get('org.libvirt.Domain', 'State', 
dbus_interface=dbus.PROPERTIES_IFACE)
-assert state == 'shutoff'
+assert state == libvirttest.DomainState.SHUTOFF
 domain.ManagedSaveRemove(0)
 assert domain.HasManagedSaveImage(0) == dbus.Boolean(False)
 
@@ -87,7 +87,7 @@ class TestDomain(libvirttest.BaseTestClass):
 domain.Resume()
 
 state = obj.Get('org.libvirt.Domain', 'State', 
dbus_interface=dbus.PROPERTIES_IFACE)
-assert state == 'running'
+assert state == libvirttest.DomainState.RUNNING
 
 self.main_loop()
 
@@ -104,7 +104,7 @@ class TestDomain(libvirttest.BaseTestClass):
 domain.Shutdown(0)
 
 state = obj.Get('org.libvirt.Domain', 'State', 
dbus_interface=dbus.PROPERTIES_IFACE)
-assert state == 'shutoff'
+assert state == libvirttest.DomainState.SHUTOFF
 
 self.main_loop()
 
@@ -121,7 +121,7 @@ class TestDomain(libvirttest.BaseTestClass):
 domain.Suspend()
 
 state = obj.Get('org.libvirt.Domain', 'State', 
dbus_interface=dbus.PROPERTIES_IFACE)
-assert state == 'paused'
+assert state == libvirttest.DomainState.PAUSED
 
 self.main_loop()
 
-- 
2.15.0

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

[libvirt] [dbus PATCH v2 9/9] Remove reason to string translation in virtDBusEventsDomainDiskChange

[Katerina Koukiou]

Signed-off-by: Katerina Koukiou 
---
 data/org.libvirt.Domain.xml |  2 +-
 src/events.c| 17 ++---
 2 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
index 3627b1b..a09e868 100644
--- a/data/org.libvirt.Domain.xml
+++ b/data/org.libvirt.Domain.xml
@@ -572,7 +572,7 @@
   
   
   
-  
+  
 
 
   domainPath);
 
-switch (reason) {
-case VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START:
-reasonstr = "missing-on-start";
-break;
-case VIR_DOMAIN_EVENT_DISK_DROP_MISSING_ON_START:
-reasonstr = "missing-on-start";
-break;
-default:
-reasonstr = "";
-break;
-}
-
 g_dbus_connection_emit_signal(connect->bus,
   NULL,
   path,
   VIRT_DBUS_DOMAIN_INTERFACE,
   "DiskChange",
-  g_variant_new("()", old_src_path,
-new_src_path, device, 
reasonstr),
+  g_variant_new("(sssu)", old_src_path,
+new_src_path, device, reason),
   NULL);
 
 return 0;
-- 
2.15.0

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

[libvirt] [dbus PATCH v2 6/9] Remove virtDBusUtilEnum{From, From}String functions

[Katerina Koukiou]

Signed-off-by: Katerina Koukiou 
---
 src/util.c | 27 ---
 src/util.h | 28 
 2 files changed, 55 deletions(-)

diff --git a/src/util.c b/src/util.c
index a9d130e..4efa3ec 100644
--- a/src/util.c
+++ b/src/util.c
@@ -214,33 +214,6 @@ virtDBusUtilVirDomainListFree(virDomainPtr *domains)
 g_free(domains);
 }
 
-const gchar *
-virtDBusUtilEnumToString(const gchar *const *types,
- guint ntypes,
- gint type)
-{
-if (type < 0 || (guint)type >= ntypes)
-return NULL;
-
-return types[type];
-}
-
-gint
-virtDBusUtilEnumFromString(const gchar *const *types,
-   guint ntypes,
-   const gchar *type)
-{
-guint i;
-if (!type)
-return -1;
-
-for (i = 0; i < ntypes; i++)
-if (g_str_equal(types[i], type))
-return i;
-
-return -1;
-}
-
 virNetworkPtr
 virtDBusUtilVirNetworkFromBusPath(virConnectPtr connection,
   const gchar *path,
diff --git a/src/util.h b/src/util.h
index 4a2138a..3309803 100644
--- a/src/util.h
+++ b/src/util.h
@@ -57,34 +57,6 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainPtr, 
virtDBusUtilVirDomainListFree);
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainStatsRecordPtr, 
virDomainStatsRecordListFree);
 
-gint
-virtDBusUtilEnumFromString(const gchar *const *types,
-   guint ntypes,
-   const gchar *type) G_GNUC_PURE;
-
-const gchar *
-virtDBusUtilEnumToString(const gchar *const *types,
- guint ntypes,
- gint type) G_GNUC_PURE;
-
-#define VIRT_DBUS_ENUM_IMPL(name, lastVal, ...) \
-static const gchar *const name ##TypeList[] = { __VA_ARGS__ }; \
-G_STATIC_ASSERT(G_N_ELEMENTS(name ##TypeList) == lastVal); \
-const gchar *name ##TypeToString(gint type) { \
-return virtDBusUtilEnumToString(name ##TypeList, \
-G_N_ELEMENTS(name ##TypeList), \
-type); \
-} \
-gint name ##TypeFromString(const gchar *type) { \
-return virtDBusUtilEnumFromString(name ##TypeList, \
-  G_N_ELEMENTS(name ##TypeList), \
-  type); \
-}
-
-#define VIRT_DBUS_ENUM_DECL(name) \
-const gchar *name ##TypeToString(gint type) G_GNUC_PURE; \
-gint name ##TypeFromString(const gchar *type) G_GNUC_PURE;
-
 virNetworkPtr
 virtDBusUtilVirNetworkFromBusPath(virConnectPtr connection,
  const gchar *path,
-- 
2.15.0

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

[libvirt] [dbus PATCH v2 4/9] Change DomainEvent argument from string to unsigned int

[Katerina Koukiou]

Modify the relevant tests to comply with the new signal.

Note: argument matching in connect_to_signal method is
only usable with string and thus had to be refactored.

Signed-off-by: Katerina Koukiou 
---
 data/org.libvirt.Connect.xml |  2 +-
 src/events.c | 26 +-
 tests/libvirttest.py | 13 +
 tests/test_connect.py| 12 
 tests/test_domain.py | 30 --
 5 files changed, 43 insertions(+), 40 deletions(-)

diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index 7249fa4..386a7bb 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -170,7 +170,7 @@
   https://libvirt.org/html/libvirt-libvirt-domain.html#virConnectDomainEventCallback"/>
   
-  
+  
 
 
   
 
-VIRT_DBUS_ENUM_DECL(virtDBusEventsDomainEvent)
-VIRT_DBUS_ENUM_IMPL(virtDBusEventsDomainEvent,
-VIR_DOMAIN_EVENT_LAST,
-"Defined",
-"Undefined",
-"Started",
-"Suspended",
-"Resumed",
-"Stopped",
-"Shutdown",
-"PMSuspended",
-"Crashed")
-
-static const gchar *
-virtDBusEventsDomainEventToString(gint event)
-{
-const gchar *str = virtDBusEventsDomainEventTypeToString(event);
-return str ? str : "unknown";
-}
-
 static gint
 virtDBusEventsDomainLifecycle(virConnectPtr connection G_GNUC_UNUSED,
   virDomainPtr domain,
@@ -33,10 +13,6 @@ virtDBusEventsDomainLifecycle(virConnectPtr connection 
G_GNUC_UNUSED,
 {
 virtDBusConnect *connect = opaque;
 g_autofree gchar *path = NULL;
-const gchar *eventStr = virtDBusEventsDomainEventToString(event);
-
-if (!eventStr)
-return 0;
 
 path = virtDBusUtilBusPathForVirDomain(domain, connect->domainPath);
 
@@ -45,7 +21,7 @@ virtDBusEventsDomainLifecycle(virConnectPtr connection 
G_GNUC_UNUSED,
   connect->connectPath,
   VIRT_DBUS_CONNECT_INTERFACE,
   "DomainEvent",
-  g_variant_new("(os)", path, eventStr),
+  g_variant_new("(ou)", path, event),
   NULL);
 
 return 0;
diff --git a/tests/libvirttest.py b/tests/libvirttest.py
index d1b71cc..f7f48ed 100644
--- a/tests/libvirttest.py
+++ b/tests/libvirttest.py
@@ -1,3 +1,4 @@
+from enum import IntEnum
 from dbus.mainloop.glib import DBusGMainLoop
 from gi.repository import GLib
 import dbus
@@ -85,3 +86,15 @@ class BaseTestClass():
 path = self.connect.ListNetworks(0)[0]
 obj = self.bus.get_object('org.libvirt', path)
 return path, obj
+
+
+class DomainEvent(IntEnum):
+DEFINED = 0
+UNDEFINED = 1
+STARTED = 2
+SUSPENDED = 3
+RESUMED = 4
+STOPPED = 5
+SHUTDOWN = 6
+PMSUSPENDED = 7
+CRASHED = 8
diff --git a/tests/test_connect.py b/tests/test_connect.py
index 57f0658..41cc134 100755
--- a/tests/test_connect.py
+++ b/tests/test_connect.py
@@ -31,11 +31,13 @@ class TestConnect(libvirttest.BaseTestClass):
 '''
 
 def test_connect_domain_create_xml(self):
-def domain_started(path, _event):
+def domain_started(path, event):
+if event != libvirttest.DomainEvent.STARTED:
+return
 assert isinstance(path, dbus.ObjectPath)
 self.loop.quit()
 
-self.connect.connect_to_signal('DomainEvent', domain_started, 
arg1='Started')
+self.connect.connect_to_signal('DomainEvent', domain_started)
 
 path = self.connect.DomainCreateXML(self.minimal_domain_xml, 0)
 assert isinstance(path, dbus.ObjectPath)
@@ -43,11 +45,13 @@ class TestConnect(libvirttest.BaseTestClass):
 self.main_loop()
 
 def test_comnect_domain_define_xml(self):
-def domain_defined(path, _event):
+def domain_defined(path, event):
+if event != libvirttest.DomainEvent.DEFINED:
+return
 assert isinstance(path, dbus.ObjectPath)
 self.loop.quit()
 
-self.connect.connect_to_signal('DomainEvent', domain_defined, 
arg1='Defined')
+self.connect.connect_to_signal('DomainEvent', domain_defined)
 
 path = self.connect.DomainDefineXML(self.minimal_domain_xml)
 assert isinstance(path, dbus.ObjectPath)
diff --git a/tests/test_domain.py b/tests/test_domain.py
index 0e83f72..2def6c1 100755
--- a/tests/test_domain.py
+++ b/tests/test_domain.py
@@ -47,11 +47,13 @@ class TestDomain(libvirttest.BaseTestClass):
 assert autostart_current == dbus.Boolean(autostart_expected)
 
 def test_domain_managed_save(self):
-def domain_stopped(path, _event):
+def domain_stopped(path, event):
+if event != libvirttest.Do

[libvirt] [dbus PATCH v2 2/9] Abandon usage of all *TypeToString functions in connect.c

[Katerina Koukiou]

Converting ENUMS to str can be user friendly though
it can be problematic in matters of backward compatibility.

In particular when some ENUM in libvirt API will introduce a
new constant, libvirt-dbus will fail with:

size of array ‘_GStaticAssertCompileTimeAssertion_5’ is negative

So using ints is preferable to avoid the previous issue.

Signed-off-by: Katerina Koukiou 
---
 data/org.libvirt.Connect.xml |  2 +-
 src/connect.c| 18 +-
 2 files changed, 2 insertions(+), 18 deletions(-)

diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index ee7bfdc..7249fa4 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -37,7 +37,7 @@
 value="See 
https://libvirt.org/html/libvirt-libvirt-host.html#virConnectCompareCPU"/>
   
   
-  
+  
 
 
   
 
-VIRT_DBUS_ENUM_DECL(virtDBusConnectCPUCompareResult)
-VIRT_DBUS_ENUM_IMPL(virtDBusConnectCPUCompareResult,
-VIR_CPU_COMPARE_LAST,
-"incompatible",
-"identical",
-"superset")
-
 static gint virtDBusConnectCredType[] = {
 VIR_CRED_AUTHNAME,
 VIR_CRED_ECHOPROMPT,
@@ -263,7 +256,6 @@ virtDBusConnectCompareCPU(GVariant *inArgs,
 const gchar *xmlDesc;
 guint flags;
 gint compareResult;
-const gchar* compareResultStr;
 
 g_variant_get(inArgs, "(&su)", &xmlDesc, &flags);
 
@@ -274,15 +266,7 @@ virtDBusConnectCompareCPU(GVariant *inArgs,
 if (compareResult < 0)
 return virtDBusUtilSetLastVirtError(error);
 
-compareResultStr = 
virtDBusConnectCPUCompareResultTypeToString(compareResult);
-if (!compareResultStr) {
-g_set_error(error, VIRT_DBUS_ERROR, VIRT_DBUS_ERROR_LIBVIRT,
-"Can't format virCPUCompareResult '%d' to string.",
-compareResult);
-return;
-}
-
-*outArgs = g_variant_new("(s)", compareResultStr);
+*outArgs = g_variant_new("(u)", compareResult);
 }
 
 static void
-- 
2.15.0

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

[libvirt] [dbus PATCH v2 7/9] Remove reason to string translation in virtDBusEventsDomainTrayChange

[Katerina Koukiou]

Signed-off-by: Katerina Koukiou 
---
 data/org.libvirt.Domain.xml |  2 +-
 src/events.c| 15 +--
 2 files changed, 2 insertions(+), 15 deletions(-)

diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
index 6448a46..98c018c 100644
--- a/data/org.libvirt.Domain.xml
+++ b/data/org.libvirt.Domain.xml
@@ -578,7 +578,7 @@
   https://libvirt.org/html/libvirt-libvirt-domain.html#virConnectDomainEventTrayChangeCallback"/>
   
-  
+  
 
   
 
diff --git a/src/events.c b/src/events.c
index 1b584f7..d4c7145 100644
--- a/src/events.c
+++ b/src/events.c
@@ -80,28 +80,15 @@ virtDBusEventsDomainTrayChange(virConnectPtr connection 
G_GNUC_UNUSED,
 {
 virtDBusConnect *connect = opaque;
 g_autofree gchar *path = NULL;
-const gchar *reasonstr;
 
 path = virtDBusUtilBusPathForVirDomain(domain, connect->domainPath);
 
-switch (reason) {
-case VIR_DOMAIN_EVENT_TRAY_CHANGE_OPEN:
-reasonstr = "open";
-break;
-case VIR_DOMAIN_EVENT_TRAY_CHANGE_CLOSE:
-reasonstr = "close";
-break;
-default:
-reasonstr = "";
-break;
-}
-
 g_dbus_connection_emit_signal(connect->bus,
   NULL,
   path,
   VIRT_DBUS_DOMAIN_INTERFACE,
   "TrayChange",
-  g_variant_new("(ss)", device, reasonstr),
+  g_variant_new("(su)", device, reason),
   NULL);
 
 return 0;
-- 
2.15.0

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

Re: [libvirt] [PATCH 0/3] sample: vfio mdev display devices.

[Erik Skultety]

On Fri, Apr 27, 2018 at 12:15:01AM +0530, Kirti Wankhede wrote:
>
>
> On 4/26/2018 1:22 AM, Dr. David Alan Gilbert wrote:
> > * Alex Williamson (alex.william...@redhat.com) wrote:
> >> On Wed, 25 Apr 2018 21:00:39 +0530
> >> Kirti Wankhede  wrote:
> >>
> >>> On 4/25/2018 4:29 AM, Alex Williamson wrote:
>  On Wed, 25 Apr 2018 01:20:08 +0530
>  Kirti Wankhede  wrote:
> 
> > On 4/24/2018 3:10 AM, Alex Williamson wrote:
> >> On Wed, 18 Apr 2018 12:31:53 -0600
> >> Alex Williamson  wrote:
> >>
> >>> On Mon,  9 Apr 2018 12:35:10 +0200
> >>> Gerd Hoffmann  wrote:
> >>>
>  This little series adds three drivers, for demo-ing and testing vfio
>  display interface code.  There is one mdev device for each interface
>  type (mdpy.ko for region and mbochs.ko for dmabuf).
> >>>
> >>> Erik Skultety brought up a good question today regarding how libvirt 
> >>> is
> >>> meant to handle these different flavors of display interfaces and
> >>> knowing whether a given mdev device has display support at all.  It
> >>> seems that we cannot simply use the default display=auto because
> >>> libvirt needs to specifically configure gl support for a dmabuf type
> >>> interface versus not having such a requirement for a region interface,
> >>> perhaps even removing the emulated graphics in some cases (though I
> >>> don't think we have boot graphics through either solution yet).
> >>> Additionally, GVT-g seems to need the x-igd-opregion support
> >>> enabled(?), which is a non-starter for libvirt as it's an experimental
> >>> option!
> >>>
> >>> Currently the only way to determine display support is through the
> >>> VFIO_DEVICE_QUERY_GFX_PLANE ioctl, but for libvirt to probe that on
> >>> their own they'd need to get to the point where they could open the
> >>> vfio device and perform the ioctl.  That means opening a vfio
> >>> container, adding the group, setting the iommu type, and getting the
> >>> device.  I was initially a bit appalled at asking libvirt to do that,
> >>> but the alternative is to put this information in sysfs, but doing 
> >>> that
> >>> we risk that we need to describe every nuance of the mdev device
> >>> through sysfs and it becomes a dumping ground for every possible
> >>> feature an mdev device might have.
> >> ...
> >>> So I was ready to return and suggest that maybe libvirt should probe
> >>> the device to know about these ancillary configuration details, but
> >>> then I remembered that both mdev vGPU vendors had external 
> >>> dependencies
> >>> to even allow probing the device.  KVMGT will fail to open the device
> >>> if it's not associated with an instance of KVM and NVIDIA vGPU, I
> >>> believe, will fail if the vGPU manager process cannot find the QEMU
> >>> instance to extract the VM UUID.  (Both of these were bad ideas)
> >>
> >> Here's another proposal that's really growing on me:
> >>
> >>  * Fix the vendor drivers!  Allow devices to be opened and probed
> >>without these external dependencies.
> >>  * Libvirt uses the existing vfio API to open the device and probe the
> >>necessary ioctls, if it can't probe the device, the feature is
> >>unavailable, ie. display=off, no migration.
> >>
> >
> > I'm trying to think simpler mechanism using sysfs that could work for
> > any feature and knowing source-destination migration compatibility check
> > by libvirt before initiating migration.
> >
> > I have another proposal:
> > * Add a ioctl VFIO_DEVICE_PROBE_FEATURES
> > struct vfio_device_features {
> > __u32 argsz;
> > __u32 features;
> > }
> >
> > Define bit for each feature:
> > #define VFIO_DEVICE_FEATURE_DISPLAY_REGION  (1 << 0)
> > #define VFIO_DEVICE_FEATURE_DISPLAY_DMABUF  (1 << 1)
> > #define VFIO_DEVICE_FEATURE_MIGRATION   (1 << 2)
> >
> > * Vendor driver returns bitmask of supported features during
> > initialization phase.
> >
> > * In vfio core module, trap this ioctl for each device  in
> > vfio_device_fops_unl_ioctl(),
> 
>  Whoops, chicken and egg problem, VFIO_GROUP_GET_DEVICE_FD is our
>  blocking point with mdev drivers, we can't get a device fd, so we can't
>  call an ioctl on the device fd.
> 
> >>>
> >>> I'm sorry, I thought we could expose features when QEMU initialize, but
> >>> libvirt needs to know supported features before QEMU initialize.
> >>>
> >>>
> > check features bitmask returned by vendor
> > driver and add a sysfs file if feature is supported that device. This
> > sysfs file would return 0/1.
> 
>  I don't understand why we have an ioctl interface, if the user can get
>  to the device fd then we have existing interfaces to probe these
>  things, it seems like you're just

Re: [libvirt] Expose vfio device display/migration to libvirt and above, was Re: [PATCH 0/3] sample: vfio mdev display devices.

[Daniel P . Berrangé]

On Thu, May 03, 2018 at 12:58:00PM -0600, Alex Williamson wrote:
> Hi,
> 
> The previous discussion hasn't produced results, so let's start over.
> Here's the situation:
> 
>  - We currently have kernel and QEMU support for the QEMU vfio-pci
>display option.
> 
>  - The default for this option is 'auto', so the device will attempt to
>generate a display if the underlying device supports it, currently
>only GVTg and some future release of NVIDIA vGPU (plus Gerd's
>sample mdpy and mbochs).
> 
>  - The display option is implemented via two different mechanism, a
>vfio region (NVIDIA, mdpy) or a dma-buf (GVTg, mbochs).
> 
>  - Displays using dma-buf require OpenGL support, displays making
>use of region support do not.
> 
>  - Enabling OpenGL support requires specific VM configurations, which
>libvirt /may/ want to facilitate.
> 
>  - Probing display support for a given device is complicated by the
>fact that GVTg and NVIDIA both impose requirements on the process
>opening the device file descriptor through the vfio API:
> 
>- GVTg requires a KVM association or will fail to allow the device
>  to be opened.
> 
>- NVIDIA requires that their vgpu-manager process can locate a UUID
>  for the VM via the process commandline.
> 
>- These are both horrible impositions and prevent libvirt from
>  simply probing the device itself.

Agreed, these requirements are just horrific. Probing for features
should not require this kind of level environmental setup. I can
just about understand & accept how we ended up here, because this
scenario is not one that was strongly considered when the first impls
were being done. I don't think we should accept it as a long term
requirement though.

> Erik Skultety, who initially raised the display question, has identified
> one possible solution, which is to simply make the display configuration
> the user's problem (apologies if I've misinterpreted Erik).  I believe
> this would work something like:
> 
>  - libvirt identifies a version of QEMU that includes 'display' support
>for vfio-pci devices and defaults to adding display=off for every
>vfio-pci device [have we chosen the wrong default (auto) in QEMU?].
> 
>  - New XML support would allow a user to enable display support on the
>vfio device.
> 
>  - Resolving any OpenGL dependencies of that change would be left to
>the user.
> 
> A nice aspect of this is that policy decisions are left to the user and
> clearly no interface changes are necessary, perhaps with the exception
> of deciding whether we've made the wrong default choice for vfio-pci
> devices in QEMU.

Unless I'm mis-understanding this isn't really a solution to the
problem, rather it is us simply giving up and telling someone else
to try to fix the problem. The 'user' here is not a human - it is
simply the next level up in the mgmt stack, eg OpenStack or oVirt.
If we can't solve it acceptably in libvirt code, I don't have much
hope that OpenStack can solve it in their code, since they have
even stronger need to automate everything.

> On the other hand, if we do want to give libvirt a mechanism to probe
> the display support for a device, we can make a simplified QEMU
> instance be the mechanism through which we do that.  For example the
> script[1] can be provided with either a PCI device or sysfs path to an
> mdev device and run a minimal VM instance meeting the requirements of
> both GVTg and NVIDIA to report the display support and GL requirements
> for a device.  There are clearly some unrefined and atrocious bits of
> this script, but it's only a proof of concept, the process management
> can be improved and we can decide whether we want to provide qmp
> mechanism to introspect the device rather than grep'ing error
> messages.  The goal is simply to show that we could choose to embrace
> QEMU and use it not as a VM, but simply a tool for poking at a device
> given the restrictions the mdev vendor drivers have already imposed.

Feels like a pretty heavy weight solution, that just encourages the
drivers to continue down the undesirable path they're already on,
possibly making the situation even worse over time.


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] [sandbox PATCH] maint: Drop autobuild.sh

[Daniel P . Berrangé]

On Thu, May 03, 2018 at 06:28:13PM +0200, Andrea Bolognani wrote:
> On Thu, 2018-05-03 at 16:59 +0100, Daniel P. Berrangé wrote:
> > On Thu, May 03, 2018 at 05:58:07PM +0200, Andrea Bolognani wrote:
> > > Renaming the scripts sounds good to me. Just come up with
> > > a reasonable name and I'll go ahead with another round of
> > > patches.
> > 
> > Perhaps just call it 'make-release.sh' or similar ?
> 
> How does prepare-release.sh sound?

Sure that, or check-release.sh, is fine with me.

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 PATCH] projects: Run libvirt's 'make check' on all machines

[Daniel P . Berrangé]

On Thu, May 03, 2018 at 06:25:19PM +0200, Andrea Bolognani wrote:
> Up until now, we had to skip it on FreeBSD due to some
> portability issues in the test suite, but as of libvirt
> commit 0b86e23d2569 they've all been fixed.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  projects/libvirt.yaml | 7 ---
>  1 file changed, 7 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

[libvirt] [PATCH 05/12] vshReadlineParse: Ignore vshReadlineOptionsGenerator for VSH_OT_ARGV options

[Lin Ma]

Currently the VSH_OT_ARGV options don't support complete, But some of
VSH_OT_ARGV options are gonna support complete in upcoming patches.

Once applied the upcoming completion patches for VSH_OT_ARGV options, If
we don't ignore VSH_OT_ARGV here, The vshReadlineOptionsGenerator will
be called, Hence complete output will consist of the result by command
completer + the result by option completer, It's confusing.
e.g.
$ virsh domstats --domain 
--backing --interface  --list-paused  --perf  --vcpu
--balloon leap42.3 --list-persistent  --raw   win10
--block   --list-active--list-running sles12sp3
--cpu-total   --list-inactive  --list-shutoff sles15
--enforce --list-other --list-transient   --state

After this patch and the upcoming completion patches:
$ virsh domstats --domain 
leap42.3sles12sp3sles15win10

Signed-off-by: Lin Ma 
---
 tools/vsh.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index 38058c874a..5a9916cbb0 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -2824,7 +2824,9 @@ vshReadlineParse(const char *text, int state)
 if (!cmd) {
 list = vshReadlineCommandGenerator(text);
 } else {
-if (!opt || (opt->type != VSH_OT_DATA && opt->type != 
VSH_OT_STRING))
+if (!opt || (opt->type != VSH_OT_DATA &&
+ opt->type != VSH_OT_STRING &&
+ opt->type != VSH_OT_ARGV))
 list = vshReadlineOptionsGenerator(text, cmd, partial);
 
 if (opt && opt->completer) {
-- 
2.15.1

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

[libvirt] [PATCH 02/12] virsh: Conditionally Ignore the first entry in list of completions

[Lin Ma]

The first entry in the returned array is the substitution for TEXT. It
causes unnecessary output if other commands or options share the same
prefix, e.g.

$ virsh des
des  desc destroy

or

$ virsh domblklist --d
--d--details  --domain

This patch fixes the above issue.

Signed-off-by: Lin Ma 
---
 tools/vsh.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index 73ec007e56..38058c874a 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -3458,6 +3458,7 @@ cmdComplete(vshControl *ctl, const vshCmd *cmd)
 const vshCmdOpt *opt = NULL;
 char **matches = NULL, **iter;
 virBuffer buf = VIR_BUFFER_INITIALIZER;
+int n;
 
 if (vshCommandOptStringQuiet(ctl, cmd, "string", &arg) <= 0)
 goto cleanup;
@@ -3493,8 +3494,11 @@ cmdComplete(vshControl *ctl, const vshCmd *cmd)
 if (!(matches = vshReadlineCompletion(arg, 0, 0)))
 goto cleanup;
 
-for (iter = matches; *iter; iter++)
+for (n =0, iter = matches; *iter; iter++, n++) {
+if (n == 0 && matches[1])
+continue;
 printf("%s\n", *iter);
+}
 
 ret = true;
  cleanup:
-- 
2.15.1

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

[libvirt] [PATCH 07/12] virsh: Apply macro for current VSH_OT_ARGV "domain" options

[Lin Ma]

Signed-off-by: Lin Ma 
---
 tools/virsh-domain-monitor.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index e4a21534cb..1e1ee60198 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -2000,11 +2000,7 @@ static const vshCmdOptDef opts_domstats[] = {
  .type = VSH_OT_BOOL,
  .help = N_("add backing chain information to block stats"),
 },
-{.name = "domain",
- .type = VSH_OT_ARGV,
- .flags = VSH_OFLAG_NONE,
- .help = N_("list of domains to get stats for"),
-},
+VIRSH_COMMON_OPT_DOMAIN_OT_ARGV(N_("list of domains to get stats for"), 0),
 {.name = NULL}
 };
 
-- 
2.15.1

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

[libvirt] [PATCH 03/12] virsh: Create macros for VSH_OT_STRING "domain" option

[Lin Ma]

Signed-off-by: Lin Ma 
---
 tools/virsh-domain-monitor.c | 3 +++
 tools/virsh-domain.c | 3 +++
 tools/virsh-snapshot.c   | 3 +++
 tools/virsh.h| 8 
 4 files changed, 17 insertions(+)

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 8e071779b4..8ad651626b 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -43,6 +43,9 @@
 #define VIRSH_COMMON_OPT_DOMAIN_FULL(cflags) \
 VIRSH_COMMON_OPT_DOMAIN(N_("domain name, id or uuid"), cflags)
 
+#define VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(cflags) \
+VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("domain name, id or uuid"), cflags)
+
 VIR_ENUM_DECL(virshDomainIOError)
 VIR_ENUM_IMPL(virshDomainIOError,
   VIR_DOMAIN_DISK_ERROR_LAST,
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 6601f94a12..8a63761fab 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -68,6 +68,9 @@
 #define VIRSH_COMMON_OPT_DOMAIN_FULL(cflags) \
 VIRSH_COMMON_OPT_DOMAIN(N_("domain name, id or uuid"), cflags)
 
+#define VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(cflags) \
+VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("domain name, id or uuid"), cflags)
+
 #define VIRSH_COMMON_OPT_DOMAIN_PERSISTENT \
 {.name = "persistent", \
  .type = VSH_OT_BOOL, \
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index e4908eea70..3d86ac84d1 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -45,6 +45,9 @@
 #define VIRSH_COMMON_OPT_DOMAIN_FULL(cflags) \
 VIRSH_COMMON_OPT_DOMAIN(N_("domain name, id or uuid"), cflags)
 
+#define VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(cflags) \
+VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("domain name, id or uuid"), cflags)
+
 /* Helper for snapshot-create and snapshot-create-as */
 static bool
 virshSnapshotCreate(vshControl *ctl, virDomainPtr dom, const char *buffer,
diff --git a/tools/virsh.h b/tools/virsh.h
index f2213ebb57..9dcf104cc4 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -107,6 +107,14 @@
  .help = _helpstr \
 }
 
+# define VIRSH_COMMON_OPT_DOMAIN_OT_STRING(_helpstr, cflags) \
+{.name = "domain", \
+ .type = VSH_OT_STRING, \
+ .help = _helpstr, \
+ .completer = virshDomainNameCompleter, \
+ .completer_flags = cflags, \
+}
+
 typedef struct _virshControl virshControl;
 typedef virshControl *virshControlPtr;
 
-- 
2.15.1

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

[libvirt] [PATCH 08/12] vshReadlineOptionsGenerator: Add already provided VSH_OT_ARGV options to list

[Lin Ma]

It's helpful for users while they type certain kind of VSH_OT_ARGV options.
e.g.

$ virsh domstats --domain sles12sp3 --d

Signed-off-by: Lin Ma 
---
 tools/vsh.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index 5a9916cbb0..9cfd4d92b8 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -2685,7 +2685,7 @@ vshReadlineOptionsGenerator(const char *text,
 }
 
 while (opt) {
-if (STREQ(opt->def->name, name)) {
+if (STREQ(opt->def->name, name) && opt->def->type != VSH_OT_ARGV) {
 exists = true;
 break;
 }
-- 
2.15.1

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

[libvirt] [PATCH 06/12] virsh: Create macros for VSH_OT_ARGV "domain" option

[Lin Ma]

Signed-off-by: Lin Ma 
---
 tools/virsh-domain-monitor.c | 3 +++
 tools/virsh-domain.c | 3 +++
 tools/virsh-snapshot.c   | 3 +++
 tools/virsh.h| 9 +
 4 files changed, 18 insertions(+)

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 8ad651626b..e4a21534cb 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -46,6 +46,9 @@
 #define VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(cflags) \
 VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("domain name, id or uuid"), cflags)
 
+#define VIRSH_COMMON_OPT_DOMAIN_OT_ARGV_FULL(cflags) \
+VIRSH_COMMON_OPT_DOMAIN_OT_ARGV(N_("domain name, id or uuid"), cflags)
+
 VIR_ENUM_DECL(virshDomainIOError)
 VIR_ENUM_IMPL(virshDomainIOError,
   VIR_DOMAIN_DISK_ERROR_LAST,
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 689f9d686b..89aefbf86a 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -71,6 +71,9 @@
 #define VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(cflags) \
 VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("domain name, id or uuid"), cflags)
 
+#define VIRSH_COMMON_OPT_DOMAIN_OT_ARGV_FULL(cflags) \
+VIRSH_COMMON_OPT_DOMAIN_OT_ARGV(N_("domain name, id or uuid"), cflags)
+
 #define VIRSH_COMMON_OPT_DOMAIN_PERSISTENT \
 {.name = "persistent", \
  .type = VSH_OT_BOOL, \
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index 3d86ac84d1..0c86b6c950 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -48,6 +48,9 @@
 #define VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(cflags) \
 VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("domain name, id or uuid"), cflags)
 
+#define VIRSH_COMMON_OPT_DOMAIN_OT_ARGV_FULL(cflags) \
+VIRSH_COMMON_OPT_DOMAIN_OT_ARGV(N_("domain name, id or uuid"), cflags)
+
 /* Helper for snapshot-create and snapshot-create-as */
 static bool
 virshSnapshotCreate(vshControl *ctl, virDomainPtr dom, const char *buffer,
diff --git a/tools/virsh.h b/tools/virsh.h
index 9dcf104cc4..a33d108b2d 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -115,6 +115,15 @@
  .completer_flags = cflags, \
 }
 
+# define VIRSH_COMMON_OPT_DOMAIN_OT_ARGV(_helpstr, cflags) \
+{.name = "domain", \
+ .type = VSH_OT_ARGV, \
+ .flags = VSH_OFLAG_NONE, \
+ .help = _helpstr, \
+ .completer = virshDomainNameCompleter, \
+ .completer_flags = cflags, \
+}
+
 typedef struct _virshControl virshControl;
 typedef virshControl *virshControlPtr;
 
-- 
2.15.1

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

[libvirt] [PATCH 11/12] domain_event: Introduce function virDomainEventGetName

[Lin Ma]

It will be used in next patch for event name completion.

Signed-off-by: Lin Ma 
---
 src/conf/domain_event.c  | 62 
 src/conf/domain_event.h  |  3 +++
 src/libvirt_private.syms |  1 +
 3 files changed, 66 insertions(+)

diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 97520706c9..f8bd457b34 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -1968,6 +1968,68 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn,
 }
 
 
+const char *
+virDomainEventGetName(int event)
+{
+virResetLastError();
+
+switch (event) {
+case VIR_DOMAIN_EVENT_ID_LIFECYCLE:
+return VIR_DOMAIN_EVENT_LIFECYCLE;
+case VIR_DOMAIN_EVENT_ID_REBOOT:
+return VIR_DOMAIN_EVENT_REBOOT;
+case VIR_DOMAIN_EVENT_ID_RTC_CHANGE:
+return VIR_DOMAIN_EVENT_RTC_CHANGE;
+case VIR_DOMAIN_EVENT_ID_WATCHDOG:
+return VIR_DOMAIN_EVENT_WATCHDOG;
+case VIR_DOMAIN_EVENT_ID_IO_ERROR:
+return VIR_DOMAIN_EVENT_IO_ERROR;
+case VIR_DOMAIN_EVENT_ID_GRAPHICS:
+return VIR_DOMAIN_EVENT_GRAPHICS;
+case VIR_DOMAIN_EVENT_ID_IO_ERROR_REASON:
+return VIR_DOMAIN_EVENT_IO_ERROR_REASON;
+case VIR_DOMAIN_EVENT_ID_CONTROL_ERROR:
+return VIR_DOMAIN_EVENT_CONTROL_ERROR;
+case VIR_DOMAIN_EVENT_ID_BLOCK_JOB:
+return VIR_DOMAIN_EVENT_BLOCK_JOB;
+case VIR_DOMAIN_EVENT_ID_DISK_CHANGE:
+return VIR_DOMAIN_EVENT_DISK_CHANGE;
+case VIR_DOMAIN_EVENT_ID_TRAY_CHANGE:
+return VIR_DOMAIN_EVENT_TRAY_CHANGE;
+case VIR_DOMAIN_EVENT_ID_PMWAKEUP:
+return VIR_DOMAIN_EVENT_PMWAKEUP;
+case VIR_DOMAIN_EVENT_ID_PMSUSPEND:
+return VIR_DOMAIN_EVENT_PMSUSPEND;
+case VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE:
+return VIR_DOMAIN_EVENT_BALLOON_CHANGE;
+case VIR_DOMAIN_EVENT_ID_PMSUSPEND_DISK:
+return VIR_DOMAIN_EVENT_PMSUSPEND_DISK;
+case VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED:
+return VIR_DOMAIN_EVENT_DEVICE_REMOVED;
+case VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2:
+return VIR_DOMAIN_EVENT_BLOCK_JOB_2;
+case VIR_DOMAIN_EVENT_ID_TUNABLE:
+return VIR_DOMAIN_EVENT_TUNABLE;
+case VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE:
+return VIR_DOMAIN_EVENT_AGENT_LIFECYCLE;
+case VIR_DOMAIN_EVENT_ID_DEVICE_ADDED:
+return VIR_DOMAIN_EVENT_DEVICE_ADDED;
+case VIR_DOMAIN_EVENT_ID_MIGRATION_ITERATION:
+return VIR_DOMAIN_EVENT_MIGRATION_ITERATION;
+case VIR_DOMAIN_EVENT_ID_JOB_COMPLETED:
+return VIR_DOMAIN_EVENT_JOB_COMPLETED;
+case VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED:
+return VIR_DOMAIN_EVENT_DEVICE_REMOVAL_FAILED;
+case VIR_DOMAIN_EVENT_ID_METADATA_CHANGE:
+return VIR_DOMAIN_EVENT_METADATA_CHANGE;
+case VIR_DOMAIN_EVENT_ID_BLOCK_THRESHOLD:
+return VIR_DOMAIN_EVENT_BLOCK_THRESHOLD;
+default:
+return NULL;
+}
+}
+
+
 virObjectEventPtr
 virDomainQemuMonitorEventNew(int id,
  const char *name,
diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h
index 3992a29c58..9c3a1e3b88 100644
--- a/src/conf/domain_event.h
+++ b/src/conf/domain_event.h
@@ -327,4 +327,7 @@ virDomainQemuMonitorEventNew(int id,
  const char *details)
 ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
 
+const char *
+virDomainEventGetName(int event);
+
 #endif
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 92b5e0fa2b..0b6c414b66 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -641,6 +641,7 @@ virDomainEventWatchdogNewFromDom;
 virDomainEventWatchdogNewFromObj;
 virDomainQemuMonitorEventNew;
 virDomainQemuMonitorEventStateRegisterID;
+virDomainEventGetName;
 
 
 # conf/domain_nwfilter.h
-- 
2.15.1

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

[libvirt] [PATCH 10/12] virsh: Introduce some VIR_DOMAIN_EVENT_* macros representing domain event names

[Lin Ma]

Signed-off-by: Lin Ma 
---
 include/libvirt/libvirt-domain.h | 201 +++
 tools/virsh-domain.c |  50 +-
 2 files changed, 226 insertions(+), 25 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 12fd34037e..f7dd510f7f 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -4424,6 +4424,207 @@ typedef enum {
 } virDomainEventID;
 
 
+/**
+ * VIR_DOMAIN_EVENT_LIFECYCLE:
+ *
+ * Macro for the event name "lifecycle"
+ */
+
+# define VIR_DOMAIN_EVENT_LIFECYCLE "lifecycle"
+
+/**
+ * VIR_DOMAIN_EVENT_REBOOT:
+ *
+ * Macro for the event name "reboot"
+ */
+
+# define VIR_DOMAIN_EVENT_REBOOT "reboot"
+
+/**
+ * VIR_DOMAIN_EVENT_RTC_CHANGE:
+ *
+ * Macro for the event name "rtc-change"
+ */
+
+# define VIR_DOMAIN_EVENT_RTC_CHANGE "rtc-change"
+
+/**
+ * VIR_DOMAIN_EVENT_WATCHDOG:
+ *
+ * Macro for the event name "watchdog"
+ */
+
+# define VIR_DOMAIN_EVENT_WATCHDOG "watchdog"
+
+/**
+ * VIR_DOMAIN_EVENT_IO_ERROR:
+ *
+ * Macro for the event name "io-error"
+ */
+
+# define VIR_DOMAIN_EVENT_IO_ERROR "io-error"
+
+/**
+ * VIR_DOMAIN_EVENT_GRAPHICS:
+ *
+ * Macro for the event name "graphics"
+ */
+
+# define VIR_DOMAIN_EVENT_GRAPHICS "graphics"
+
+/**
+ * VIR_DOMAIN_EVENT_IO_ERROR_REASON:
+ *
+ * Macro for the event name "io-error-reason"
+ */
+
+# define VIR_DOMAIN_EVENT_IO_ERROR_REASON "io-error-reason"
+
+/**
+ * VIR_DOMAIN_EVENT_CONTROL_ERROR:
+ *
+ * Macro for the event name "control-error"
+ */
+
+# define VIR_DOMAIN_EVENT_CONTROL_ERROR "control-error"
+
+/**
+ * VIR_DOMAIN_EVENT_BLOCK_JOB:
+ *
+ * Macro for the event name "block-job"
+ */
+
+# define VIR_DOMAIN_EVENT_BLOCK_JOB "block-job"
+
+/**
+ * VIR_DOMAIN_EVENT_DISK_CHANGE:
+ *
+ * Macro for the event name "disk-change"
+ */
+
+# define VIR_DOMAIN_EVENT_DISK_CHANGE "disk-change"
+
+/**
+ * VIR_DOMAIN_EVENT_TRAY_CHANGE:
+ *
+ * Macro for the event name "tray-change"
+ */
+
+# define VIR_DOMAIN_EVENT_TRAY_CHANGE "tray-change"
+
+/**
+ * VIR_DOMAIN_EVENT_PMWAKEUP:
+ *
+ * Macro for the event name "pm-wakeup"
+ */
+
+# define VIR_DOMAIN_EVENT_PMWAKEUP "pm-wakeup"
+
+/**
+ * VIR_DOMAIN_EVENT_PMSUSPEND:
+ *
+ * Macro for the event name "pm-suspend"
+ */
+
+# define VIR_DOMAIN_EVENT_PMSUSPEND "pm-suspend"
+
+/**
+ * VIR_DOMAIN_EVENT_BALLOON_CHANGE:
+ *
+ * Macro for the event name "balloon-change"
+ */
+
+# define VIR_DOMAIN_EVENT_BALLOON_CHANGE "balloon-change"
+
+/**
+ * VIR_DOMAIN_EVENT_PMSUSPEND_DISK:
+ *
+ * Macro for the event name "pm-suspend-disk"
+ */
+
+# define VIR_DOMAIN_EVENT_PMSUSPEND_DISK "pm-suspend-disk"
+
+/**
+ * VIR_DOMAIN_EVENT_DEVICE_REMOVED:
+ *
+ * Macro for the event name "device-removed"
+ */
+
+# define VIR_DOMAIN_EVENT_DEVICE_REMOVED "device-removed"
+
+/**
+ * VIR_DOMAIN_EVENT_BLOCK_JOB_2:
+ *
+ * Macro for the event name "block-job-2"
+ */
+
+# define VIR_DOMAIN_EVENT_BLOCK_JOB_2 "block-job-2"
+
+/**
+ * VIR_DOMAIN_EVENT_TUNABLE:
+ *
+ * Macro for the event name "tunable"
+ */
+
+# define VIR_DOMAIN_EVENT_TUNABLE "tunable"
+
+/**
+ * VIR_DOMAIN_EVENT_AGENT_LIFECYCLE:
+ *
+ * Macro for the event name "agent-lifecycle"
+ */
+
+# define VIR_DOMAIN_EVENT_AGENT_LIFECYCLE "agent-lifecycle"
+
+/**
+ * VIR_DOMAIN_EVENT_DEVICE_ADDED:
+ *
+ * Macro for the event name "device-added"
+ */
+
+# define VIR_DOMAIN_EVENT_DEVICE_ADDED "device-added"
+
+/**
+ * VIR_DOMAIN_EVENT_MIGRATION_ITERATION:
+ *
+ * Macro for the event name "migration-iteration"
+ */
+
+# define VIR_DOMAIN_EVENT_MIGRATION_ITERATION "migration-iteration"
+
+/**
+ * VIR_DOMAIN_EVENT_JOB_COMPLETED:
+ *
+ * Macro for the event name "job-completed"
+ */
+
+# define VIR_DOMAIN_EVENT_JOB_COMPLETED "job-completed"
+
+/**
+ * VIR_DOMAIN_EVENT_DEVICE_REMOVAL_FAILED:
+ *
+ * Macro for the event name "device-removal-failed"
+ */
+
+# define VIR_DOMAIN_EVENT_DEVICE_REMOVAL_FAILED "device-removal-failed"
+
+/**
+ * VIR_DOMAIN_EVENT_METADATA_CHANGE:
+ *
+ * Macro for the event name "metadata-change"
+ */
+
+# define VIR_DOMAIN_EVENT_METADATA_CHANGE "metadata-change"
+
+/**
+ * VIR_DOMAIN_EVENT_BLOCK_THRESHOLD:
+ *
+ * Macro for the event name "block-threshold"
+ */
+
+# define VIR_DOMAIN_EVENT_BLOCK_THRESHOLD "block-threshold"
+
+
 /* Use VIR_DOMAIN_EVENT_CALLBACK() to cast the 'cb' parameter  */
 int virConnectDomainEventRegisterAny(virConnectPtr conn,
  virDomainPtr dom, /* Optional, to filter 
*/
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index b35c9adaaa..e21ba0117b 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -13283,53 +13283,53 @@ virshEventBlockThresholdPrint(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 
 
 static vshEventCallback vshEventCallbacks[] = {
-{ "lifecycle",
+{ VIR_DOMAIN_EVENT_LIFECYCLE,
   VIR_DOMAIN_EVENT_CALLBACK(virshEventLifecyclePrint), },
-{ "reboot", virshEventGenericPrint, },
-{ "rtc-change",
+{ VIR_DOMAIN_EVENT_REBOOT, virshEventGenericPrint,

[libvirt] [PATCH 04/12] virsh: Apply macro for current VSH_OT_STRING "domain" options

[Lin Ma]

These VSH_OT_STRING "domain" options support domain name completion now.

Signed-off-by: Lin Ma 
---
 tools/virsh-domain.c | 18 +-
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 8a63761fab..689f9d686b 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9601,10 +9601,8 @@ static const vshCmdInfo info_qemu_monitor_event[] = {
 };
 
 static const vshCmdOptDef opts_qemu_monitor_event[] = {
-{.name = "domain",
- .type = VSH_OT_STRING,
- .help = N_("filter by domain name, id or uuid")
-},
+VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("filter by domain name, id or uuid"),
+  0),
 {.name = "event",
  .type = VSH_OT_STRING,
  .help = N_("filter by event name")
@@ -10157,11 +10155,7 @@ static const vshCmdOptDef opts_domxmltonative[] = {
  .flags = VSH_OFLAG_REQ,
  .help = N_("target config data type format")
 },
-{.name = "domain",
- .type = VSH_OT_STRING,
- .flags = VSH_OFLAG_REQ_OPT,
- .help = N_("domain name, id or uuid")
-},
+VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(0),
 {.name = "xml",
  .type = VSH_OT_STRING,
  .help = N_("xml data file to export from")
@@ -13348,10 +13342,8 @@ static const vshCmdInfo info_event[] = {
 };
 
 static const vshCmdOptDef opts_event[] = {
-{.name = "domain",
- .type = VSH_OT_STRING,
- .help = N_("filter by domain name, id, or uuid")
-},
+VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("filter by domain name, id or uuid"),
+  0),
 {.name = "event",
  .type = VSH_OT_STRING,
  .help = N_("which event type to wait for")
-- 
2.15.1

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

[libvirt] [PATCH 01/12] virsh: Add domain name completion to 'migrate-postcopy' command

[Lin Ma]

Signed-off-by: Lin Ma 
---
 tools/virsh-domain.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 2b775fc4cc..6601f94a12 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11219,11 +11219,7 @@ static const vshCmdInfo info_migrate_postcopy[] = {
 };
 
 static const vshCmdOptDef opts_migrate_postcopy[] = {
-{.name = "domain",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
- .help = N_("domain name, id or uuid")
-},
+VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
 {.name = NULL}
 };
 
-- 
2.15.1

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

[libvirt] [PATCH 09/12] virsh: Enable multiple --event flags to 'event' command

[Lin Ma]

Signed-off-by: Lin Ma 
---
 tools/virsh-domain.c | 76 +++-
 1 file changed, 39 insertions(+), 37 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 89aefbf86a..b35c9adaaa 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -13347,10 +13347,6 @@ static const vshCmdInfo info_event[] = {
 static const vshCmdOptDef opts_event[] = {
 VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("filter by domain name, id or uuid"),
   0),
-{.name = "event",
- .type = VSH_OT_STRING,
- .help = N_("which event type to wait for")
-},
 {.name = "all",
  .type = VSH_OT_BOOL,
  .help = N_("wait for all events instead of just one type")
@@ -13371,6 +13367,10 @@ static const vshCmdOptDef opts_event[] = {
  .type = VSH_OT_BOOL,
  .help = N_("show timestamp for each printed event")
 },
+{.name = "event",
+ .type = VSH_OT_ARGV,
+ .help = N_("which event type to wait for")
+},
 {.name = NULL}
 };
 
@@ -13382,12 +13382,14 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
 int timeout = 0;
 virshDomEventData *data = NULL;
 size_t i;
-const char *eventName = NULL;
+int *eventIdxList = NULL;
+size_t nevents = 0;
 int event = -1;
 bool all = vshCommandOptBool(cmd, "all");
 bool loop = vshCommandOptBool(cmd, "loop");
 bool timestamp = vshCommandOptBool(cmd, "timestamp");
 int count = 0;
+const vshCmdOpt *opt = NULL;
 virshControlPtr priv = ctl->privData;
 
 if (vshCommandOptBool(cmd, "list")) {
@@ -13396,15 +13398,25 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
 return true;
 }
 
-if (vshCommandOptStringReq(ctl, cmd, "event", &eventName) < 0)
-return false;
-if (eventName) {
-for (event = 0; event < VIR_DOMAIN_EVENT_ID_LAST; event++)
-if (STREQ(eventName, vshEventCallbacks[event].name))
-break;
-if (event == VIR_DOMAIN_EVENT_ID_LAST) {
-vshError(ctl, _("unknown event type %s"), eventName);
-return false;
+if (vshCommandOptBool(cmd, "event")) {
+if (VIR_ALLOC_N(eventIdxList, 1) < 0)
+goto cleanup;
+nevents = 1;
+while ((opt = vshCommandOptArgv(ctl, cmd, opt))) {
+if (opt->data) {
+for (event = 0; event < VIR_DOMAIN_EVENT_ID_LAST; event++)
+if (STREQ(opt->data, vshEventCallbacks[event].name))
+break;
+if (event == VIR_DOMAIN_EVENT_ID_LAST) {
+vshError(ctl, _("unknown event type %s"), opt->data);
+return false;
+}
+if (VIR_INSERT_ELEMENT(eventIdxList,
+   nevents - 1,
+   nevents,
+   event) < 0)
+goto cleanup;
+}
 }
 } else if (!all) {
 vshError(ctl, "%s",
@@ -13412,26 +13424,15 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
 return false;
 }
 
-if (all) {
-if (VIR_ALLOC_N(data, VIR_DOMAIN_EVENT_ID_LAST) < 0)
-goto cleanup;
-for (i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i++) {
-data[i].ctl = ctl;
-data[i].loop = loop;
-data[i].count = &count;
-data[i].timestamp = timestamp;
-data[i].cb = &vshEventCallbacks[i];
-data[i].id = -1;
-}
-} else {
-if (VIR_ALLOC_N(data, 1) < 0)
-goto cleanup;
-data[0].ctl = ctl;
-data[0].loop = vshCommandOptBool(cmd, "loop");
-data[0].count = &count;
-data[0].timestamp = timestamp;
-data[0].cb = &vshEventCallbacks[event];
-data[0].id = -1;
+if (VIR_ALLOC_N(data, all ? VIR_DOMAIN_EVENT_ID_LAST : nevents - 1) < 0)
+goto cleanup;
+for (i = 0; i < (all ? VIR_DOMAIN_EVENT_ID_LAST : nevents - 1); i++) {
+data[i].ctl = ctl;
+data[i].loop = loop;
+data[i].count = &count;
+data[i].timestamp = timestamp;
+data[i].cb = &vshEventCallbacks[all ? i : eventIdxList[i]];
+data[i].id = -1;
 }
 if (vshCommandOptTimeoutToMs(ctl, cmd, &timeout) < 0)
 goto cleanup;
@@ -13441,9 +13442,9 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
 if (vshEventStart(ctl, timeout) < 0)
 goto cleanup;
 
-for (i = 0; i < (all ? VIR_DOMAIN_EVENT_ID_LAST : 1); i++) {
+for (i = 0; i < (all ? VIR_DOMAIN_EVENT_ID_LAST : nevents - 1); i++) {
 if ((data[i].id = virConnectDomainEventRegisterAny(priv->conn, dom,
-   all ? i : event,
+   all ? i : 
eventIdxList[i],
data[i].cb->cb,
 

[libvirt] [PATCH 12/12] virsh: Add event name completion to 'event' command

[Lin Ma]

Signed-off-by: Lin Ma 
---
 tools/virsh-completer.c | 36 
 tools/virsh-completer.h |  3 +++
 tools/virsh-domain.c|  1 +
 3 files changed, 40 insertions(+)

diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index e3b8234b41..a188a8d7ab 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -30,6 +30,7 @@
 #include "viralloc.h"
 #include "virstring.h"
 #include "virxml.h"
+extern const char *virDomainEventGetName(int event);
 
 
 char **
@@ -522,3 +523,38 @@ virshSnapshotNameCompleter(vshControl *ctl,
 virshDomainFree(dom);
 return NULL;
 }
+
+
+char **
+virshEventNameCompleter(vshControl *ctl,
+const vshCmd *cmd ATTRIBUTE_UNUSED,
+unsigned int flags)
+{
+virshControlPtr priv = ctl->privData;
+size_t i = 0;
+char **ret = NULL;
+
+virCheckFlags(0, NULL);
+
+if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
+return NULL;
+
+if (VIR_ALLOC_N(ret, VIR_DOMAIN_EVENT_ID_LAST + 1) < 0)
+goto error;
+
+for (i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i++) {
+const char *name = virDomainEventGetName(i);
+
+if (name == NULL)
+goto error;
+
+if (VIR_STRDUP(ret[i], name) < 0)
+goto error;
+}
+
+return ret;
+
+error:
+VIR_FREE(ret);
+return NULL;
+}
diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h
index fa443d3ad7..27d78dc7ac 100644
--- a/tools/virsh-completer.h
+++ b/tools/virsh-completer.h
@@ -69,5 +69,8 @@ char ** virshSecretUUIDCompleter(vshControl *ctl,
 char ** virshSnapshotNameCompleter(vshControl *ctl,
const vshCmd *cmd,
unsigned int flags);
+char ** virshEventNameCompleter(vshControl *ctl,
+const vshCmd *cmd,
+unsigned int flags);
 
 #endif
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index e21ba0117b..ce72d414b9 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -13369,6 +13369,7 @@ static const vshCmdOptDef opts_event[] = {
 },
 {.name = "event",
  .type = VSH_OT_ARGV,
+ .completer = virshEventNameCompleter,
  .help = N_("which event type to wait for")
 },
 {.name = NULL}
-- 
2.15.1

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

[libvirt] [PATCH 7/7] qemu: stats: Display net count, type and source even if domain is inactive

[Lin Ma]

Signed-off-by: Lin Ma 
---
 src/qemu/qemu_driver.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7a9a2bcf97..33ae68129d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19804,9 +19804,6 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver 
ATTRIBUTE_UNUSED,
 struct _virDomainInterfaceStats tmp;
 int ret = -1;
 
-if (!virDomainObjIsActive(dom))
-return 0;
-
 QEMU_ADD_COUNT_PARAM(record, maxparams, "net", dom->def->nnets);
 
 /* Check the path is one of the domain's network interfaces. */
@@ -19814,6 +19811,14 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver 
ATTRIBUTE_UNUSED,
 virDomainNetDefPtr net = dom->def->nets[i];
 virDomainNetType actualType;
 
+QEMU_ADD_NAME_PARAM(record, maxparams,
+"net", "type", i, 
virDomainNetTypeToString(net->type));
+QEMU_ADD_NAME_PARAM(record, maxparams,
+"net", "source", i, net->data.bridge.brname);
+
+if (!virDomainObjIsActive(dom))
+return 0;
+
 if (!net->ifname)
 continue;
 
@@ -19823,10 +19828,6 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver 
ATTRIBUTE_UNUSED,
 
 QEMU_ADD_NAME_PARAM(record, maxparams,
 "net", "name", i, net->ifname);
-QEMU_ADD_NAME_PARAM(record, maxparams,
-"net", "type", i, 
virDomainNetTypeToString(net->type));
-QEMU_ADD_NAME_PARAM(record, maxparams,
-"net", "source", i, net->data.bridge.brname);
 
 if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
 if (virNetDevOpenvswitchInterfaceStats(net->ifname, &tmp) < 0) {
-- 
2.15.1

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

[libvirt] [PATCH 3/7] po: fix typo: remove a redundant Chinese character

[Lin Ma]

Signed-off-by: Lin Ma 
---
 po/zh_CN.mini.po | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/po/zh_CN.mini.po b/po/zh_CN.mini.po
index c5574cf1df..0e47184c93 100644
--- a/po/zh_CN.mini.po
+++ b/po/zh_CN.mini.po
@@ -15340,7 +15340,7 @@ msgid "entry was missing 'type'"
 msgstr "条目缺少 'type' "
 
 msgid "enumerate devices on this host"
-msgstr "这台主机中中的枚举设备"
+msgstr "这台主机中的枚举设备"
 
 msgid "error"
 msgstr "错误"
-- 
2.15.1

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

[libvirt] [PATCH 2/7] virsh: Error out while domain not found for 'event' command

[Lin Ma]

Signed-off-by: Lin Ma 
---
 tools/virsh-domain.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 3b2a34f936..1f3ea0c939 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -13445,7 +13445,9 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
 goto cleanup;
 
 if (vshCommandOptBool(cmd, "domain"))
-dom = virshCommandOptDomain(ctl, cmd, NULL);
+if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+goto cleanup;
+
 if (vshEventStart(ctl, timeout) < 0)
 goto cleanup;
 
-- 
2.15.1

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

[libvirt] [PATCH 4/7] virsh: Simplify control flow for 'desc' command

[Lin Ma]

Just like the commit 8941c800, It does the similar thing.

Signed-off-by: Lin Ma 
---
 tools/virsh-domain.c | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 1f3ea0c939..65170225a7 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8330,7 +8330,6 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd)
 char *tmpstr;
 const vshCmdOpt *opt = NULL;
 virBuffer buf = VIR_BUFFER_INITIALIZER;
-bool pad = false;
 bool ret = false;
 unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
 
@@ -8348,18 +8347,16 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd)
 if ((state = virshDomainState(ctl, dom, NULL)) < 0)
 goto cleanup;
 
-while ((opt = vshCommandOptArgv(ctl, cmd, opt))) {
-if (pad)
-virBufferAddChar(&buf, ' ');
-pad = true;
-virBufferAdd(&buf, opt->data, -1);
-}
-
 if (title)
 type = VIR_DOMAIN_METADATA_TITLE;
 else
 type = VIR_DOMAIN_METADATA_DESCRIPTION;
 
+while ((opt = vshCommandOptArgv(ctl, cmd, opt)))
+virBufferAsprintf(&buf, "%s ", opt->data);
+
+virBufferTrim(&buf, " ", -1);
+
 if (virBufferError(&buf)) {
 vshError(ctl, "%s", _("Failed to collect new description/title"));
 goto cleanup;
-- 
2.15.1

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

[libvirt] [PATCH 1/7] virsh: Error out while domain not found for 'qemu-monitor-event' command

[Lin Ma]

Signed-off-by: Lin Ma 
---
 tools/virsh-domain.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 2b775fc4cc..3b2a34f936 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9661,7 +9661,9 @@ cmdQemuMonitorEvent(vshControl *ctl, const vshCmd *cmd)
 return false;
 
 if (vshCommandOptBool(cmd, "domain"))
-dom = virshCommandOptDomain(ctl, cmd, NULL);
+if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+goto cleanup;
+
 if (vshEventStart(ctl, timeout) < 0)
 goto cleanup;
 
-- 
2.15.1

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

[libvirt] [PATCH 6/7] qemu: stats: Display the net type and net source in bulk stats

[Lin Ma]

Signed-off-by: Lin Ma 
---
 src/libvirt-domain.c   | 2 ++
 src/qemu/qemu_driver.c | 4 
 tools/virsh.pod| 2 ++
 3 files changed, 8 insertions(+)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 2d86e48979..e317ca00d0 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11310,6 +11310,8 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
  * "net.count" - number of network interfaces on this domain
  *   as unsigned int.
  * "net..name" - name of the interface  as string.
+ * "net..type" - type of the interface  as string.
+ * "net..source" - source of the interface  as string.
  * "net..rx.bytes" - bytes received as unsigned long long.
  * "net..rx.pkts" - packets received as unsigned long long.
  * "net..rx.errs" - receive errors as unsigned long long.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 83fc191085..7a9a2bcf97 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19823,6 +19823,10 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver 
ATTRIBUTE_UNUSED,
 
 QEMU_ADD_NAME_PARAM(record, maxparams,
 "net", "name", i, net->ifname);
+QEMU_ADD_NAME_PARAM(record, maxparams,
+"net", "type", i, 
virDomainNetTypeToString(net->type));
+QEMU_ADD_NAME_PARAM(record, maxparams,
+"net", "source", i, net->data.bridge.brname);
 
 if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
 if (virNetDevOpenvswitchInterfaceStats(net->ifname, &tmp) < 0) {
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 929958a953..7ec57c0b4b 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -974,6 +974,8 @@ I<--interface> returns:
 
  "net.count" - number of network interfaces on this domain
  "net..name" - name of the interface 
+ "net..type" - type of the interface 
+ "net..source" - source of the interface 
  "net..rx.bytes" - number of bytes received
  "net..rx.pkts" - number of packets received
  "net..rx.errs" - number of receive errors
-- 
2.15.1

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

[libvirt] [PATCH 0/7] Couple of tiny fixes for virsh and translation.

[Lin Ma]

Lin Ma (7):
  virsh: Error out while domain not found for 'qemu-monitor-event'
command
  virsh: Error out while domain not found for 'event' command
  po: fix typo: remove a redundant Chinese character
  virsh: Simplify control flow for 'desc' command
  virsh: Simplify control flow for 'qemu-agent-command' command
  qemu: stats: Display the net type and net source in bulk stats
  qemu: stats: Display net count,type and source even if domain is
inactive

 po/zh_CN.mini.po   |  2 +-
 src/libvirt-domain.c   |  2 ++
 src/qemu/qemu_driver.c | 11 ---
 tools/virsh-domain.c   | 33 -
 tools/virsh.pod|  2 ++
 5 files changed, 29 insertions(+), 21 deletions(-)

-- 
2.15.1

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

[libvirt] [PATCH 5/7] virsh: Simplify control flow for 'qemu-agent-command' command

[Lin Ma]

Signed-off-by: Lin Ma 
---
 tools/virsh-domain.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 65170225a7..598d2fa4a4 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9796,19 +9796,17 @@ cmdQemuAgentCommand(vshControl *ctl, const vshCmd *cmd)
 unsigned int flags = 0;
 const vshCmdOpt *opt = NULL;
 virBuffer buf = VIR_BUFFER_INITIALIZER;
-bool pad = false;
 virJSONValuePtr pretty = NULL;
 
 dom = virshCommandOptDomain(ctl, cmd, NULL);
 if (dom == NULL)
 goto cleanup;
 
-while ((opt = vshCommandOptArgv(ctl, cmd, opt))) {
-if (pad)
-virBufferAddChar(&buf, ' ');
-pad = true;
-virBufferAdd(&buf, opt->data, -1);
-}
+while ((opt = vshCommandOptArgv(ctl, cmd, opt)))
+virBufferAsprintf(&buf, "%s ", opt->data);
+
+virBufferTrim(&buf, " ", -1);
+
 if (virBufferError(&buf)) {
 vshError(ctl, "%s", _("Failed to collect command"));
 goto cleanup;
-- 
2.15.1

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

[libvirt] [dbus PATCH] Add detail argument to DomainEvent signal

[Katerina Koukiou]

Adjust all DomainEvent tests to do detail type checking.

Signed-off-by: Katerina Koukiou 
---
This commit is rebased on top of unmerged patches for removing enum<->string
translation.

 data/org.libvirt.Connect.xml |  1 +
 src/events.c |  4 ++--
 tests/libvirttest.py | 55 
 tests/test_connect.py|  6 +++--
 tests/test_domain.py | 15 
 5 files changed, 72 insertions(+), 9 deletions(-)

diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index 8272da6..0f1456f 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -171,6 +171,7 @@
 value="See 
https://libvirt.org/html/libvirt-libvirt-domain.html#virConnectDomainEventCallback"/>
   
   
+  
 
 
   connectPath,
   VIRT_DBUS_CONNECT_INTERFACE,
   "DomainEvent",
-  g_variant_new("(ou)", path, event),
+  g_variant_new("(ouu)", path, event, detail),
   NULL);
 
 return 0;
diff --git a/tests/libvirttest.py b/tests/libvirttest.py
index 06ac0e4..eee67a0 100644
--- a/tests/libvirttest.py
+++ b/tests/libvirttest.py
@@ -100,6 +100,61 @@ class DomainEvent(IntEnum):
 CRASHED = 8
 
 
+class DomainEventDefinedDetailType(IntEnum):
+ADDED = 0
+UPDATED = 1
+RENAMED = 2
+FROM_SNAPSHOT = 3
+LAST = 4
+
+
+class DomainEventResumedDetailType(IntEnum):
+UNPAUSED = 0
+MIGRATED = 1
+FROM_SNAPSHOT = 2
+POSTCOPY = 3
+LAST = 4
+
+
+class DomainEventStartedDetailType(IntEnum):
+BOOTED = 0
+MIGRATED = 1
+RESTORED = 2
+FROM_SNAPSHOT = 3
+WAKEUP = 4
+LAST = 5
+
+
+class DomainEventStoppedDetailType(IntEnum):
+SHUTDOWN = 0
+DESTROYED = 1
+CRASHED = 2
+MIGRATED = 3
+SAVED = 4
+FAILED = 5
+FROM_SNAPSHOT = 6
+LAST = 7
+
+
+class DomainEventSuspendedDetailType(IntEnum):
+PAUSED = 0
+MIGRATED   = 1
+IOERROR = 2
+WATCHDOG   = 3
+RESTORED = 4
+FROM_SNAPSHOT = 5
+API_ERROR = 6
+POSTCOPY = 7
+POSTCOPY_FAILED = 8
+LAST = 9
+
+
+class DomainEventUndefinedDetailType(IntEnum):
+REMOVED = 0
+RENAMED = 1
+LAST = 2
+
+
 class DomainState(IntEnum):
 NOSTATE = 0
 RUNNING = 1
diff --git a/tests/test_connect.py b/tests/test_connect.py
index 7748822..a2bd17f 100755
--- a/tests/test_connect.py
+++ b/tests/test_connect.py
@@ -31,9 +31,10 @@ class TestConnect(libvirttest.BaseTestClass):
 '''
 
 def test_connect_domain_create_xml(self):
-def domain_started(path, event):
+def domain_started(path, event, detail):
 if event != libvirttest.DomainEvent.STARTED:
 return
+assert detail == libvirttest.DomainEventStartedDetailType.BOOTED
 assert isinstance(path, dbus.ObjectPath)
 self.loop.quit()
 
@@ -45,9 +46,10 @@ class TestConnect(libvirttest.BaseTestClass):
 self.main_loop()
 
 def test_comnect_domain_define_xml(self):
-def domain_defined(path, event):
+def domain_defined(path, event, detail):
 if event != libvirttest.DomainEvent.DEFINED:
 return
+assert detail == libvirttest.DomainEventDefinedDetailType.ADDED
 assert isinstance(path, dbus.ObjectPath)
 self.loop.quit()
 
diff --git a/tests/test_domain.py b/tests/test_domain.py
index c7e09cd..dfa19ed 100755
--- a/tests/test_domain.py
+++ b/tests/test_domain.py
@@ -47,9 +47,10 @@ class TestDomain(libvirttest.BaseTestClass):
 assert autostart_current == dbus.Boolean(autostart_expected)
 
 def test_domain_managed_save(self):
-def domain_stopped(path, event):
+def domain_stopped(path, event, detail):
 if event != libvirttest.DomainEvent.STOPPED:
 return
+assert detail == libvirttest.DomainEventStoppedDetailType.SAVED
 assert isinstance(path, dbus.ObjectPath)
 self.loop.quit()
 
@@ -74,9 +75,10 @@ class TestDomain(libvirttest.BaseTestClass):
 assert description_expected == 
domain.GetMetadata(metadata_description, "", 0)
 
 def test_resume(self):
-def domain_resumed(path, event):
+def domain_resumed(path, event, detail):
 if event != libvirttest.DomainEvent.RESUMED:
 return
+assert detail == libvirttest.DomainEventResumedDetailType.UNPAUSED
 assert isinstance(path, dbus.ObjectPath)
 self.loop.quit()
 
@@ -92,9 +94,10 @@ class TestDomain(libvirttest.BaseTestClass):
 self.main_loop()
 
 def test_shutdown(self):
-def domain_stopped(path, event):
+def domain_stopped(path, event, detail):
 if event != libvirttest.DomainEvent.STOPPED:
 return
+assert detail == libvirttes

Re: [libvirt] [PATCH 3/7] po: fix typo: remove a redundant Chinese character

[Peter Krempa]

On Fri, May 04, 2018 at 17:28:50 +0800, Lin Ma wrote:
> Signed-off-by: Lin Ma 
> ---

NACK

This needs to be fixed in Zanata where we pull the translations from
since it would be overwritten by the next sync.


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

Re: [libvirt] [PATCH 6/7] qemu: stats: Display the net type and net source in bulk stats

[Peter Krempa]

On Fri, May 04, 2018 at 17:28:53 +0800, Lin Ma wrote:
> Signed-off-by: Lin Ma 
> ---

Could you justify this? This is really configuration and will not change
during the lifetime of the interface, thus there's no pressing need to
report it in the stats which should report only data which is changing.


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

Re: [libvirt] [PATCH 7/7] qemu: stats: Display net count, type and source even if domain is inactive

[Peter Krempa]

On Fri, May 04, 2018 at 17:28:54 +0800, Lin Ma wrote:
> Signed-off-by: Lin Ma 
> ---
>  src/qemu/qemu_driver.c | 15 ---
>  1 file changed, 8 insertions(+), 7 deletions(-)

The network is not enabled so there's no stats to report when the VM is
offline so I don't think this is justified.


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

Re: [libvirt] [PATCH 3/7] po: fix typo: remove a redundant Chinese character

[Daniel P . Berrangé]

On Fri, May 04, 2018 at 12:07:39PM +0200, Peter Krempa wrote:
> On Fri, May 04, 2018 at 17:28:50 +0800, Lin Ma wrote:
> > Signed-off-by: Lin Ma 
> > ---
> 
> NACK
> 
> This needs to be fixed in Zanata where we pull the translations from
> since it would be overwritten by the next sync.

To save time, I will fix this mistake in Zanata, so it gets synced.


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 10/12] virsh: Introduce some VIR_DOMAIN_EVENT_* macros representing domain event names

[Peter Krempa]

On Fri, May 04, 2018 at 17:25:31 +0800, Lin Ma wrote:
> Signed-off-by: Lin Ma 
> ---
>  include/libvirt/libvirt-domain.h | 201 
> +++
>  tools/virsh-domain.c |  50 +-
>  2 files changed, 226 insertions(+), 25 deletions(-)
> 
> diff --git a/include/libvirt/libvirt-domain.h 
> b/include/libvirt/libvirt-domain.h
> index 12fd34037e..f7dd510f7f 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -4424,6 +4424,207 @@ typedef enum {
>  } virDomainEventID;
>  
>  
> +/**
> + * VIR_DOMAIN_EVENT_LIFECYCLE:
> + *
> + * Macro for the event name "lifecycle"
> + */
> +
> +# define VIR_DOMAIN_EVENT_LIFECYCLE "lifecycle"

This would make all of these public API and I'm not persuaded this is a
good idea. Specifically since they are only used in virsh. The library
always refers to the events by number. Also some of the events have
multiple implementations which don't usually need to be referred to
publically depending on the use case of the event. Virsh implements all
of them just for the sake of completness.

NACK



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

[libvirt] [dbus PATCH v2] Implement SetGuestVcpus method for Domain Interface

[Katerina Koukiou]

Signed-off-by: Katerina Koukiou 
---
Fixed conditions since v1 was creating incorrect vcpumap.
If all vcpus will be passed False the API will not work, since '' is not
valid cpumap but this scenario should not be used.

 data/org.libvirt.Domain.xml |  7 +
 src/domain.c| 62 +
 2 files changed, 69 insertions(+)

diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
index db43b1c..eae6d97 100644
--- a/data/org.libvirt.Domain.xml
+++ b/data/org.libvirt.Domain.xml
@@ -463,6 +463,13 @@
   
   
 
+
+  https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainSetGuestVcpus"/>
+  
+  
+  
+
 
   https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainSetInterfaceParameters"/>
diff --git a/src/domain.c b/src/domain.c
index e305fa3..28cbe76 100644
--- a/src/domain.c
+++ b/src/domain.c
@@ -72,6 +72,39 @@ VIRT_DBUS_ENUM_IMPL(virtDBusDomainMetadata,
 "title",
 "element")
 
+static gchar *
+virtDBusDomainConvertBoolArrayToGuestVcpumap(GVariantIter *iter)
+{
+g_autoptr(GVariantIter) tmpIter = NULL;
+gint intervalCnt = 0;
+guint intervalStart = 0;
+gboolean set;
+gboolean setPrev = 0;
+g_autofree GString *ret = NULL;
+
+ret = g_string_new("");
+for (guint i = 0; ; i++) {
+gboolean next = g_variant_iter_loop(iter, "b", &set);
+
+if (next && set && !setPrev)
+intervalStart = i;
+else if ((!next && setPrev) || (!set && setPrev)) {
+if (intervalCnt > 0)
+g_string_append_printf(ret, ",");
+if (intervalStart != i - 1)
+g_string_append_printf(ret, "%d-%d", intervalStart, i - 1);
+else
+g_string_append_printf(ret, "%d", intervalStart);
+intervalCnt++;
+}
+setPrev = set;
+if (!next)
+break;
+}
+
+return ret->str;
+}
+
 struct _virtDBusDomainFSInfoList {
 virDomainFSInfoPtr *info;
 gint count;
@@ -2490,6 +2523,34 @@ virtDBusDomainSetBlockIOTune(GVariant *inArgs,
 }
 }
 
+static void
+virtDBusDomainSetGuestVcpus(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(virDomain) domain = NULL;
+g_autoptr(GVariantIter) iter = NULL;
+gint state;
+guint flags;
+g_autofree gchar *ret = NULL;
+
+g_variant_get(inArgs, "(abiu)", &iter, &state, &flags);
+
+domain = virtDBusDomainGetVirDomain(connect, objectPath, error);
+if (!domain)
+return;
+
+ret = virtDBusDomainConvertBoolArrayToGuestVcpumap(iter);
+
+if (virDomainSetGuestVcpus(domain, ret, state, flags) < 0)
+virtDBusUtilSetLastVirtError(error);
+}
+
 static void
 virtDBusDomainSetInterfaceParameters(GVariant *inArgs,
  GUnixFDList *inFDs G_GNUC_UNUSED,
@@ -2988,6 +3049,7 @@ static virtDBusGDBusMethodTable 
virtDBusDomainMethodTable[] = {
 { "SendProcessSignal", virtDBusDomainSendProcessSignal },
 { "SetBlkioParameters", virtDBusDomainSetBlkioParameters },
 { "SetBlockIOTune", virtDBusDomainSetBlockIOTune },
+{ "SetGuestVcpus", virtDBusDomainSetGuestVcpus },
 { "SetInterfaceParameters", virtDBusDomainSetInterfaceParameters },
 { "SetVcpus", virtDBusDomainSetVcpus },
 { "SetMemory", virtDBusDomainSetMemory },
-- 
2.15.0

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

Re: [libvirt] [PATCH 11/12] domain_event: Introduce function virDomainEventGetName

[Peter Krempa]

On Fri, May 04, 2018 at 17:25:32 +0800, Lin Ma wrote:
> It will be used in next patch for event name completion.
> 
> Signed-off-by: Lin Ma 
> ---
>  src/conf/domain_event.c  | 62 
> 
>  src/conf/domain_event.h  |  3 +++
>  src/libvirt_private.syms |  1 +
>  3 files changed, 66 insertions(+)
> 
> diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
> index 97520706c9..f8bd457b34 100644
> --- a/src/conf/domain_event.c
> +++ b/src/conf/domain_event.c
> @@ -1968,6 +1968,68 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn,
>  }
>  
>  
> +const char *
> +virDomainEventGetName(int event)
> +{
> +virResetLastError();

This should not be necessary. This function shall not touch the error
since it does not report any. The caller needs to handle it properly.

> +
> +switch (event) {

Typecast event to the proper type so that compiler moans when new events
are added.

> +case VIR_DOMAIN_EVENT_ID_LIFECYCLE:
> +return VIR_DOMAIN_EVENT_LIFECYCLE;
> +case VIR_DOMAIN_EVENT_ID_REBOOT:
> +return VIR_DOMAIN_EVENT_REBOOT;

[...]


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

Re: [libvirt] [PATCH 12/12] virsh: Add event name completion to 'event' command

[Peter Krempa]

On Fri, May 04, 2018 at 17:25:33 +0800, Lin Ma wrote:
> Signed-off-by: Lin Ma 
> ---
>  tools/virsh-completer.c | 36 
>  tools/virsh-completer.h |  3 +++
>  tools/virsh-domain.c|  1 +
>  3 files changed, 40 insertions(+)
> 
> diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
> index e3b8234b41..a188a8d7ab 100644
> --- a/tools/virsh-completer.c
> +++ b/tools/virsh-completer.c
> @@ -30,6 +30,7 @@
>  #include "viralloc.h"
>  #include "virstring.h"
>  #include "virxml.h"
> +extern const char *virDomainEventGetName(int event);

Please don't use externs. If you need the function export it.

>  
>  
>  char **
> @@ -522,3 +523,38 @@ virshSnapshotNameCompleter(vshControl *ctl,
>  virshDomainFree(dom);
>  return NULL;
>  }
> +
> +
> +char **
> +virshEventNameCompleter(vshControl *ctl,
> +const vshCmd *cmd ATTRIBUTE_UNUSED,
> +unsigned int flags)
> +{
> +virshControlPtr priv = ctl->privData;
> +size_t i = 0;
> +char **ret = NULL;
> +
> +virCheckFlags(0, NULL);
> +
> +if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
> +return NULL;
> +
> +if (VIR_ALLOC_N(ret, VIR_DOMAIN_EVENT_ID_LAST + 1) < 0)
> +goto error;
> +
> +for (i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i++) {
> +const char *name = virDomainEventGetName(i);
> +
> +if (name == NULL)
> +goto error;
> +
> +if (VIR_STRDUP(ret[i], name) < 0)
> +goto error;
> +}
> +
> +return ret;
> +
> +error:
> +VIR_FREE(ret);

This does not free the members in ret allocated by the strdup above.

> +return NULL;
> +}


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

Re: [libvirt] [PATCH 06/12] virsh: Create macros for VSH_OT_ARGV "domain" option

[Peter Krempa]

On Fri, May 04, 2018 at 17:25:27 +0800, Lin Ma wrote:
> Signed-off-by: Lin Ma 
> ---
>  tools/virsh-domain-monitor.c | 3 +++
>  tools/virsh-domain.c | 3 +++
>  tools/virsh-snapshot.c   | 3 +++
>  tools/virsh.h| 9 +
>  4 files changed, 18 insertions(+)
> 
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index 8ad651626b..e4a21534cb 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -46,6 +46,9 @@
>  #define VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(cflags) \
>  VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("domain name, id or uuid"), cflags)
>  
> +#define VIRSH_COMMON_OPT_DOMAIN_OT_ARGV_FULL(cflags) \
> +VIRSH_COMMON_OPT_DOMAIN_OT_ARGV(N_("domain name, id or uuid"), cflags)

All of these defined here are unused (even in the other patches in this
series), so what's the point of adding them?

> +
>  VIR_ENUM_DECL(virshDomainIOError)
>  VIR_ENUM_IMPL(virshDomainIOError,
>VIR_DOMAIN_DISK_ERROR_LAST,
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 689f9d686b..89aefbf86a 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -71,6 +71,9 @@
>  #define VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(cflags) \
>  VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("domain name, id or uuid"), cflags)
>  
> +#define VIRSH_COMMON_OPT_DOMAIN_OT_ARGV_FULL(cflags) \
> +VIRSH_COMMON_OPT_DOMAIN_OT_ARGV(N_("domain name, id or uuid"), cflags)
> +
>  #define VIRSH_COMMON_OPT_DOMAIN_PERSISTENT \
>  {.name = "persistent", \
>   .type = VSH_OT_BOOL, \
> diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
> index 3d86ac84d1..0c86b6c950 100644
> --- a/tools/virsh-snapshot.c
> +++ b/tools/virsh-snapshot.c
> @@ -48,6 +48,9 @@
>  #define VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(cflags) \
>  VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("domain name, id or uuid"), cflags)
>  
> +#define VIRSH_COMMON_OPT_DOMAIN_OT_ARGV_FULL(cflags) \
> +VIRSH_COMMON_OPT_DOMAIN_OT_ARGV(N_("domain name, id or uuid"), cflags)
> +
>  /* Helper for snapshot-create and snapshot-create-as */

Also all of them have the same docs string so I really don't know why we
need to define them in every file separately ...

>  static bool
>  virshSnapshotCreate(vshControl *ctl, virDomainPtr dom, const char *buffer,
> diff --git a/tools/virsh.h b/tools/virsh.h
> index 9dcf104cc4..a33d108b2d 100644
> --- a/tools/virsh.h
> +++ b/tools/virsh.h
> @@ -115,6 +115,15 @@
>   .completer_flags = cflags, \
>  }
>  
> +# define VIRSH_COMMON_OPT_DOMAIN_OT_ARGV(_helpstr, cflags) \
> +{.name = "domain", \
> + .type = VSH_OT_ARGV, \
> + .flags = VSH_OFLAG_NONE, \
> + .help = _helpstr, \
> + .completer = virshDomainNameCompleter, \
> + .completer_flags = cflags, \
> +}
> +
>  typedef struct _virshControl virshControl;
>  typedef virshControl *virshControlPtr;
>  
> -- 
> 2.15.1
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list


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

Re: [libvirt] [PATCH 03/12] virsh: Create macros for VSH_OT_STRING "domain" option

[Peter Krempa]

On Fri, May 04, 2018 at 17:25:24 +0800, Lin Ma wrote:
> Signed-off-by: Lin Ma 
> ---
>  tools/virsh-domain-monitor.c | 3 +++
>  tools/virsh-domain.c | 3 +++
>  tools/virsh-snapshot.c   | 3 +++
>  tools/virsh.h| 8 
>  4 files changed, 17 insertions(+)
> 
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index 8e071779b4..8ad651626b 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -43,6 +43,9 @@
>  #define VIRSH_COMMON_OPT_DOMAIN_FULL(cflags) \
>  VIRSH_COMMON_OPT_DOMAIN(N_("domain name, id or uuid"), cflags)
>  
> +#define VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(cflags) \
> +VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("domain name, id or uuid"), cflags)
> +
>  VIR_ENUM_DECL(virshDomainIOError)
>  VIR_ENUM_IMPL(virshDomainIOError,
>VIR_DOMAIN_DISK_ERROR_LAST,
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 6601f94a12..8a63761fab 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -68,6 +68,9 @@
>  #define VIRSH_COMMON_OPT_DOMAIN_FULL(cflags) \
>  VIRSH_COMMON_OPT_DOMAIN(N_("domain name, id or uuid"), cflags)
>  
> +#define VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(cflags) \
> +VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("domain name, id or uuid"), cflags)
> +
>  #define VIRSH_COMMON_OPT_DOMAIN_PERSISTENT \
>  {.name = "persistent", \
>   .type = VSH_OT_BOOL, \
> diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
> index e4908eea70..3d86ac84d1 100644
> --- a/tools/virsh-snapshot.c
> +++ b/tools/virsh-snapshot.c
> @@ -45,6 +45,9 @@
>  #define VIRSH_COMMON_OPT_DOMAIN_FULL(cflags) \
>  VIRSH_COMMON_OPT_DOMAIN(N_("domain name, id or uuid"), cflags)
>  
> +#define VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(cflags) \
> +VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("domain name, id or uuid"), cflags)

Only one of the three definitions above is actually used ...

> +
>  /* Helper for snapshot-create and snapshot-create-as */
>  static bool
>  virshSnapshotCreate(vshControl *ctl, virDomainPtr dom, const char *buffer,
> diff --git a/tools/virsh.h b/tools/virsh.h
> index f2213ebb57..9dcf104cc4 100644
> --- a/tools/virsh.h
> +++ b/tools/virsh.h
> @@ -107,6 +107,14 @@
>   .help = _helpstr \
>  }

... since all of them share the docs string, why isn't it declared here?

>  
> +# define VIRSH_COMMON_OPT_DOMAIN_OT_STRING(_helpstr, cflags) \
> +{.name = "domain", \
> + .type = VSH_OT_STRING, \
> + .help = _helpstr, \
> + .completer = virshDomainNameCompleter, \
> + .completer_flags = cflags, \
> +}
> +
>  typedef struct _virshControl virshControl;
>  typedef virshControl *virshControlPtr;
>  
> -- 
> 2.15.1
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list


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

Re: [libvirt] [PATCH 02/12] virsh: Conditionally Ignore the first entry in list of completions

[Peter Krempa]

On Fri, May 04, 2018 at 17:25:23 +0800, Lin Ma wrote:
> The first entry in the returned array is the substitution for TEXT. It
> causes unnecessary output if other commands or options share the same
> prefix, e.g.
> 
> $ virsh des
> des  desc destroy
> 
> or
> 
> $ virsh domblklist --d
> --d--details  --domain
> 
> This patch fixes the above issue.
> 
> Signed-off-by: Lin Ma 
> ---
>  tools/vsh.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/vsh.c b/tools/vsh.c
> index 73ec007e56..38058c874a 100644
> --- a/tools/vsh.c
> +++ b/tools/vsh.c
> @@ -3458,6 +3458,7 @@ cmdComplete(vshControl *ctl, const vshCmd *cmd)
>  const vshCmdOpt *opt = NULL;
>  char **matches = NULL, **iter;
>  virBuffer buf = VIR_BUFFER_INITIALIZER;
> +int n;
>  
>  if (vshCommandOptStringQuiet(ctl, cmd, "string", &arg) <= 0)
>  goto cleanup;
> @@ -3493,8 +3494,11 @@ cmdComplete(vshControl *ctl, const vshCmd *cmd)
>  if (!(matches = vshReadlineCompletion(arg, 0, 0)))
>  goto cleanup;
>  
> -for (iter = matches; *iter; iter++)
> +for (n =0, iter = matches; *iter; iter++, n++) {

Fails 'make syntax-check':

Spacing around '=' or '==':
tools/vsh.c:3499: for (n =0, iter = matches; *iter; iter++, n++) {
maint.mk: incorrect formatting


> +if (n == 0 && matches[1])
> +continue;
>  printf("%s\n", *iter);
> +}
>  
>  ret = true;
>   cleanup:
> -- 
> 2.15.1
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list


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

Re: [libvirt] [PATCH 09/12] virsh: Enable multiple --event flags to 'event' command

[Peter Krempa]

On Fri, May 04, 2018 at 17:25:30 +0800, Lin Ma wrote:
> Signed-off-by: Lin Ma 
> ---
>  tools/virsh-domain.c | 76 
> +++-
>  1 file changed, 39 insertions(+), 37 deletions(-)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 89aefbf86a..b35c9adaaa 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -13347,10 +13347,6 @@ static const vshCmdInfo info_event[] = {
>  static const vshCmdOptDef opts_event[] = {
>  VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("filter by domain name, id or 
> uuid"),
>0),
> -{.name = "event",
> - .type = VSH_OT_STRING,
> - .help = N_("which event type to wait for")
> -},
>  {.name = "all",
>   .type = VSH_OT_BOOL,
>   .help = N_("wait for all events instead of just one type")
> @@ -13371,6 +13367,10 @@ static const vshCmdOptDef opts_event[] = {
>   .type = VSH_OT_BOOL,
>   .help = N_("show timestamp for each printed event")
>  },
> +{.name = "event",
> + .type = VSH_OT_ARGV,
> + .help = N_("which event type to wait for")
> +},
>  {.name = NULL}
>  };
>  
> @@ -13382,12 +13382,14 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
>  int timeout = 0;
>  virshDomEventData *data = NULL;
>  size_t i;
> -const char *eventName = NULL;
> +int *eventIdxList = NULL;
> +size_t nevents = 0;
>  int event = -1;
>  bool all = vshCommandOptBool(cmd, "all");
>  bool loop = vshCommandOptBool(cmd, "loop");
>  bool timestamp = vshCommandOptBool(cmd, "timestamp");
>  int count = 0;
> +const vshCmdOpt *opt = NULL;
>  virshControlPtr priv = ctl->privData;
>  
>  if (vshCommandOptBool(cmd, "list")) {
> @@ -13396,15 +13398,25 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
>  return true;
>  }
>  
> -if (vshCommandOptStringReq(ctl, cmd, "event", &eventName) < 0)
> -return false;
> -if (eventName) {
> -for (event = 0; event < VIR_DOMAIN_EVENT_ID_LAST; event++)
> -if (STREQ(eventName, vshEventCallbacks[event].name))
> -break;
> -if (event == VIR_DOMAIN_EVENT_ID_LAST) {
> -vshError(ctl, _("unknown event type %s"), eventName);
> -return false;
> +if (vshCommandOptBool(cmd, "event")) {
> +if (VIR_ALLOC_N(eventIdxList, 1) < 0)
> +goto cleanup;

This is not necessary, VIR_APPEND_ELEMENT does that

> +nevents = 1;
> +while ((opt = vshCommandOptArgv(ctl, cmd, opt))) {
> +if (opt->data) {
> +for (event = 0; event < VIR_DOMAIN_EVENT_ID_LAST; event++)
> +if (STREQ(opt->data, vshEventCallbacks[event].name))
> +break;
> +if (event == VIR_DOMAIN_EVENT_ID_LAST) {
> +vshError(ctl, _("unknown event type %s"), opt->data);
> +return false;

This leaks the eventIdxList array. Also it would be preferrable just to
use one array for the case when events are enumerated and when all are
used ... 

> +}
> +if (VIR_INSERT_ELEMENT(eventIdxList,

Use VIR_APPEND_ELEMENT instead.

> +   nevents - 1,
> +   nevents,
> +   event) < 0)
> +goto cleanup;
> +}
>  }
>  } else if (!all) {
>  vshError(ctl, "%s",
> @@ -13412,26 +13424,15 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
>  return false;
>  }
>  
> -if (all) {
> -if (VIR_ALLOC_N(data, VIR_DOMAIN_EVENT_ID_LAST) < 0)
> -goto cleanup;
> -for (i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i++) {
> -data[i].ctl = ctl;
> -data[i].loop = loop;
> -data[i].count = &count;
> -data[i].timestamp = timestamp;
> -data[i].cb = &vshEventCallbacks[i];
> -data[i].id = -1;
> -}
> -} else {
> -if (VIR_ALLOC_N(data, 1) < 0)
> -goto cleanup;
> -data[0].ctl = ctl;
> -data[0].loop = vshCommandOptBool(cmd, "loop");
> -data[0].count = &count;
> -data[0].timestamp = timestamp;
> -data[0].cb = &vshEventCallbacks[event];
> -data[0].id = -1;
> +if (VIR_ALLOC_N(data, all ? VIR_DOMAIN_EVENT_ID_LAST : nevents - 1) < 0)
> +goto cleanup;
> +for (i = 0; i < (all ? VIR_DOMAIN_EVENT_ID_LAST : nevents - 1); i++) {
> +data[i].ctl = ctl;
> +data[i].loop = loop;
> +data[i].count = &count;
> +data[i].timestamp = timestamp;
> +data[i].cb = &vshEventCallbacks[all ? i : eventIdxList[i]];

No ternaries, please use the same array, just fill it differently.

> +data[i].id = -1;

You can refactor the above to use VIR_APPEND_ELEMENT too so that the
same array can be used.

>  }
>  if (vshCommandOptTime

Re: [libvirt] [dbus PATCH] Implement SetGuestVcpus method for Domain Interface

[Pavel Hrdina]

On Wed, May 02, 2018 at 04:51:42PM +0200, Katerina Koukiou wrote:
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Domain.xml |  7 +
>  src/domain.c| 62 
> +
>  2 files changed, 69 insertions(+)
> 
> diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
> index db43b1c..eae6d97 100644
> --- a/data/org.libvirt.Domain.xml
> +++ b/data/org.libvirt.Domain.xml
> @@ -463,6 +463,13 @@
>
>
>  
> +
> +   +  value="See 
> https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainSetGuestVcpus"/>
> +  
> +  
> +  
> +
>  
>  value="See 
> https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainSetInterfaceParameters"/>
> diff --git a/src/domain.c b/src/domain.c
> index e305fa3..bbc4ead 100644
> --- a/src/domain.c
> +++ b/src/domain.c
> @@ -72,6 +72,39 @@ VIRT_DBUS_ENUM_IMPL(virtDBusDomainMetadata,
>  "title",
>  "element")
>  
> +static gchar *
> +virtDBusDomainConvertBoolArrayToGuestVcpumap(GVariantIter *iter)
> +{
> +g_autoptr(GVariantIter) tmpIter = NULL;

tmpIter is not used

> +gboolean set;
> +gint intervalCnt = 0;
> +guint intervalStart = 0;
> +gboolean setPrev = 0;
> +g_autofree GString *ret = NULL;
> +
> +ret = g_string_new("");
> +for (guint i = 0; ; i++) {
> +gboolean next = g_variant_iter_loop(iter, "b", &set);

I would rename this variable to stop and invert the logic, the return
value tells you whether there was some value to unpack or not.

> +
> +if (set && !setPrev)
> +intervalStart = i;
> +else if ((!set && setPrev) || next) {

Here you should remove the '|| next' part, otherwise the 'else if'
section is called for every unset CPU in the array of boolean mask.

> +if (intervalCnt > 0)
> +g_string_append_printf(ret, ",");

I would change it to 'boolean first' instead of intervalCnt to make it
clear what's the purpose of this variable.

> +if (intervalStart != i - 1)
> +g_string_append_printf(ret, "%d-%d", intervalStart, i - 1);
> +else
> +g_string_append_printf(ret, "%d", intervalStart);
> +intervalCnt++;
> +}
> +setPrev = set;
> +if (!next)
> +break;
> +}
> +
> +return ret->str;
> +}
> +
>  struct _virtDBusDomainFSInfoList {
>  virDomainFSInfoPtr *info;
>  gint count;
> @@ -2490,6 +2523,34 @@ virtDBusDomainSetBlockIOTune(GVariant *inArgs,
>  }
>  }
>  
> +static void
> +virtDBusDomainSetGuestVcpus(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(virDomain) domain = NULL;
> +g_autoptr(GVariantIter) iter = NULL;
> +gint state;
> +guint flags;
> +g_autofree gchar *ret = NULL;

s/ret/cpumap/

Reviewed-by: Pavel Hrdina 

with the following diff applied, to make the suggested changes clear:

diff --git a/src/domain.c b/src/domain.c
index bbc4ead..8210a04 100644
--- a/src/domain.c
+++ b/src/domain.c
@@ -75,30 +75,32 @@ VIRT_DBUS_ENUM_IMPL(virtDBusDomainMetadata,
 static gchar *
 virtDBusDomainConvertBoolArrayToGuestVcpumap(GVariantIter *iter)
 {
-g_autoptr(GVariantIter) tmpIter = NULL;
 gboolean set;
-gint intervalCnt = 0;
+gboolean first = TRUE;
 guint intervalStart = 0;
 gboolean setPrev = 0;
 g_autofree GString *ret = NULL;

 ret = g_string_new("");
 for (guint i = 0; ; i++) {
-gboolean next = g_variant_iter_loop(iter, "b", &set);
+gboolean stop = !g_variant_iter_loop(iter, "b", &set);

-if (set && !setPrev)
+if (set && !setPrev) {
 intervalStart = i;
-else if ((!set && setPrev) || next) {
-if (intervalCnt > 0)
+} else if (!set && setPrev) {
+if (!first)
 g_string_append_printf(ret, ",");
+else
+first = FALSE;
+
 if (intervalStart != i - 1)
 g_string_append_printf(ret, "%d-%d", intervalStart, i - 1);
 else
 g_string_append_printf(ret, "%d", intervalStart);
-intervalCnt++;
 }
 setPrev = set;
-if (!next)
+
+if (stop)
 break;
 }

@@ -2537,7 +2539,7 @@ virtDBusDomainSetGuestVcpus(GVariant *inArgs,
 g_autoptr(GVariantIter) iter = NULL;
 gint state;
 guint flags;
-g_autofree gchar *ret = NULL;
+g_autofree gchar *cpumap = NULL;

 g_variant_get(inArgs, "(abiu)", &iter, &state, &flags);

@@ -25

Re: [libvirt] [dbus PATCH 3/4] Annotate properties for which we will not emit changed signal

[Pavel Hrdina]

On Thu, May 03, 2018 at 03:21:01PM +0200, Katerina Koukiou wrote:
> On Thu, 2018-05-03 at 14:46 +0200, Pavel Hrdina wrote:
> > For some of these properties there is no libvirt event to detect the
> > change and for properties where we could somehow detect the change
> > let's annotate them as well.
> > 
> > We could change the properties to methods but with the annotation we
> > can keep them as properties in order to allow to get them by single
> > D-Bus call.
> > 
> > Signed-off-by: Pavel Hrdina 
> > ---
> >  data/org.libvirt.Connect.xml | 3 +++
> >  data/org.libvirt.Domain.xml  | 9 +
> >  data/org.libvirt.Network.xml | 3 +++
> >  3 files changed, 15 insertions(+)
> > 
> 
> Maybe it would be nicer to set `false` as the default annotation for
> EmitsChangedSignal in the enclosing interface element and explicitly
> mark the rest. But I am ok with this as well.

Good point, I missed that in D-Bus specification that the annotation can
be for the whole interface.  I'll sent v2.

Thanks,
Pavel


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

Re: [libvirt] [PATCH 0/7] Couple of tiny fixes for virsh and translation.

[Peter Krempa]

On Fri, May 04, 2018 at 17:28:47 +0800, Lin Ma wrote:
> Lin Ma (7):
>   virsh: Error out while domain not found for 'qemu-monitor-event'
> command
>   virsh: Error out while domain not found for 'event' command
>   po: fix typo: remove a redundant Chinese character
>   virsh: Simplify control flow for 'desc' command
>   virsh: Simplify control flow for 'qemu-agent-command' command
>   qemu: stats: Display the net type and net source in bulk stats
>   qemu: stats: Display net count,type and source even if domain is
> inactive

Patches 1, 2, 4, and 5 are ACKed and pushed.


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

Re: [libvirt] [PATCH 1/2] util: adding virHasLastError and virGetLastErrorCode/Domain

[Erik Skultety]

On Fri, May 04, 2018 at 04:46:47AM +0100, ramyelkest wrote:
> Many places in the code call virGetLastError() just to check the
> raised error code, or domain. However virGetLastError() can return
> NULL, so the code has to check for that as well.

s/as well/first.

>
> So Instead we create functions virGetLastErrorCode and virGetLastErrorDomain
> (in addition to the existing virGetLastErrorMessage) that always return a
> valid error code/domain/message, to simplify callers.

No paragraph, how about "This patch therefore introduces virGetLasErrorCode
function which always returns a valid error code, thus dropping the need to
perform any checks on the error object".

>
> Also created virHasLastErrorCode for completion
>
> My first commit, for:
> https://wiki.libvirt.org/page/BiteSizedTasks#Add_and_use_virGetLastErrorCode.28.29
>

You mentioned this in the cover letter already, no need to have it in the commit
messages too ;).

> Signed-off-by: Ramy Elkest 
> ---
>  include/libvirt/virterror.h |  3 +++
>  src/libvirt_public.syms |  7 +
>  src/util/virerror.c | 63 
> +
>  3 files changed, 73 insertions(+)
>
> diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
> index 3e7c7a0..8336258 100644
> --- a/include/libvirt/virterror.h
> +++ b/include/libvirt/virterror.h
> @@ -344,6 +344,9 @@ voidvirResetLastError   (void);
>  voidvirResetError   (virErrorPtr err);
>  voidvirFreeError(virErrorPtr err);
>
> +int virHasLastError (void);
> +int virGetLastErrorCode (void);
> +int virGetLastErrorDomain   (void);
>  const char *virGetLastErrorMessage  (void);
>
>  virErrorPtr virConnGetLastError (virConnectPtr conn);
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index 95df3a0..3a641a3 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -785,4 +785,11 @@ LIBVIRT_4.1.0 {
>  virStoragePoolLookupByTargetPath;
>  } LIBVIRT_3.9.0;
>
> +LIBVIRT_4.4.0 {
> +global:
> +virHasLastError;
> +virGetLastErrorCode;
> +virGetLastErrorDomain;
> +} LIBVIRT_4.1.0;
> +
>  #  define new API here using predicted next version number 
> diff --git a/src/util/virerror.c b/src/util/virerror.c
> index c000b00..818af2e 100644
> --- a/src/util/virerror.c
> +++ b/src/util/virerror.c
> @@ -272,6 +272,69 @@ virGetLastError(void)
>
>
>  /**
> + * virHasLastError:
> + *
> + * Check for a reported last error
> + *
> + * The error object is kept in thread local storage, so separate
> + * threads can safely access this concurrently.
> + *
> + * Returns 1 if there is an error and the error code is not VIR_ERR_OK,
> +   otherwise returns 0
> + */
> +int
> +virHasLastError(void)
> +{
> +virErrorPtr err = virLastErrorObject();
> +if (!err || err->code == VIR_ERR_OK)
> +return 0;
> +return 1;
> +}

As you noted in the cover letter, ^this one is essentially identical to
virGetLastErrorCode, minus the "1 vs err->code" and since VIR_ERR_OK == 0, you
can use virGetLastErrorCode the same way as you used virHasLastError as a
replacement in many occurrences of virGetLastError(), so I don't see the need
for this function.

> +
> +
> +/**
> + * virGetLastErrorCode:
> + *
> + * Get the most recent error code
> + *
> + * The error object is kept in thread local storage, so separate
> + * threads can safely access this concurrently.
> + *
> + * Returns the most recent error code in this thread,
> + * or VIR_ERR_OK if none is set
> + */
> +int
> +virGetLastErrorCode(void)
> +{
> +virErrorPtr err = virLastErrorObject();
> +if (!err)
> +return VIR_ERR_OK;
> +return err->code;
> +}
> +
> +
> +/**
> + * virGetLastErrorDomain:
> + *
> + * Get the most recent error domain
> + *
> + * The error object is kept in thread local storage, so separate
> + * threads can safely access this concurrently.
> + *
> + * Returns the most recent error code in this thread,
> + * or VIR_FROM_NONE if none is set
> + */
> +int
> +virGetLastErrorDomain(void)
> +{
> +virErrorPtr err = virLastErrorObject();
> +if (!err)
> +return VIR_FROM_NONE;
> +return err->domain;
> +}

I can see a need for ^this function, even though we don't have an immediate use
case for it internally, it's still a public API which someone else might
consume, otherwise they'd have to query the object itself.

Erik

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

[libvirt] [perl PATCH v2 0/3] maint: Rename autobuild.sh to prepare-release.sh

[Andrea Bolognani]

Changes from [v1]:

* rename the script instead of dropping it;

* perform some further clean ups.


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

Andrea Bolognani (3):
  maint: Rename autobuild.sh to prepare-release.sh
  prepare-release: Drop references to Test::AutoBuild
  prepare-release: Drop references to Debian packages

 autobuild.sh => prepare-release.sh | 22 +++---
 1 file changed, 3 insertions(+), 19 deletions(-)
 rename autobuild.sh => prepare-release.sh (67%)

-- 
2.17.0

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

[libvirt] [perl PATCH v2 1/3] maint: Rename autobuild.sh to prepare-release.sh

[Andrea Bolognani]

The script was originally used by the Test::AutoBuild
project to perform periodic automatic builds; however, that
effort has been abandoned a long time ago, and these days
libvirt-perl CI builds are happening on the Jenkins-based
CentOS CI environment under the libvirt umbrella[1], where
build recipes are maintained separately from the projects
themselves.

The script is still used to prepare releases, so it can't
be dropped from the repository: rename it so that its
purpose is more clearly communicated instead.

[1] https://ci.centos.org/view/libvirt/

Signed-off-by: Andrea Bolognani 
---
 autobuild.sh => prepare-release.sh | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename autobuild.sh => prepare-release.sh (100%)

diff --git a/autobuild.sh b/prepare-release.sh
similarity index 100%
rename from autobuild.sh
rename to prepare-release.sh
-- 
2.17.0

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

[libvirt] [perl PATCH v2 2/3] prepare-release: Drop references to Test::AutoBuild

[Andrea Bolognani]

They are no longer relevant and misleading.

Signed-off-by: Andrea Bolognani 
---
 prepare-release.sh | 14 +++---
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/prepare-release.sh b/prepare-release.sh
index 8a6d102..22f9155 100755
--- a/prepare-release.sh
+++ b/prepare-release.sh
@@ -1,21 +1,17 @@
 #!/bin/sh
-#
-# This script is used to Test::AutoBuild (http://www.autobuild.org)
-# to perform automated builds of the Sys-Virt module
 
 NAME=Sys-Virt
 
 set -e
 
 test -n "$1" && RESULTS=$1 || RESULTS=results.log
-: ${AUTOBUILD_INSTALL_ROOT=$HOME/builder}
 
 make -k realclean ||:
 rm -rf MANIFEST blib pm_to_blib
 
 export TEST_MAINTAINER=1
 
-perl Makefile.PL  PREFIX=$AUTOBUILD_INSTALL_ROOT
+perl Makefile.PL PREFIX=$HOME/builder
 
 rm -f MANIFEST
 
@@ -56,12 +52,8 @@ rm -f $NAME-*.tar.gz
 make dist
 
 if [ -f /usr/bin/rpmbuild ]; then
-  if [ -n "$AUTOBUILD_COUNTER" ]; then
-EXTRA_RELEASE=".auto$AUTOBUILD_COUNTER"
-  else
-NOW=`date +"%s"`
-EXTRA_RELEASE=".$USER$NOW"
-  fi
+  NOW=`date +"%s"`
+  EXTRA_RELEASE=".$USER$NOW"
   rpmbuild --nodeps -ta --define "extra_release $EXTRA_RELEASE" --clean 
$NAME-*.tar.gz
 fi
 
-- 
2.17.0

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

[libvirt] [perl PATCH v2 3/3] prepare-release: Drop references to Debian packages

[Andrea Bolognani]

The Debian packaging was never included in the git repository
to begin with.

Signed-off-by: Andrea Bolognani 
---
 prepare-release.sh | 8 
 1 file changed, 8 deletions(-)

diff --git a/prepare-release.sh b/prepare-release.sh
index 22f9155..9de67fe 100755
--- a/prepare-release.sh
+++ b/prepare-release.sh
@@ -56,11 +56,3 @@ if [ -f /usr/bin/rpmbuild ]; then
   EXTRA_RELEASE=".$USER$NOW"
   rpmbuild --nodeps -ta --define "extra_release $EXTRA_RELEASE" --clean 
$NAME-*.tar.gz
 fi
-
-# Skip debian pkg for now
-exit 0
-
-if [ -f /usr/bin/fakeroot ]; then
-  fakeroot debian/rules clean
-  fakeroot debian/rules DESTDIR=$HOME/packages/debian binary
-fi
-- 
2.17.0

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

[libvirt] [dbus PATCH v2 0/2] improve annotation of properties

[Pavel Hrdina]

Pavel Hrdina (2):
  Change the default annotation for emitting changed properties to false
  Annotate properties that will never change during the object lifecycle

 data/org.libvirt.Connect.xml | 3 +++
 data/org.libvirt.Domain.xml  | 2 ++
 data/org.libvirt.Network.xml | 3 +++
 3 files changed, 8 insertions(+)

-- 
2.14.3

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

[libvirt] [dbus PATCH v2 2/2] Annotate properties that will never change during the object lifecycle

[Pavel Hrdina]

These can be annotated as 'const' properties because they will never
change.

Reviewed-by: Katerina Koukiou 
Signed-off-by: Pavel Hrdina 
---
 data/org.libvirt.Connect.xml | 2 ++
 data/org.libvirt.Domain.xml  | 1 +
 data/org.libvirt.Network.xml | 2 ++
 3 files changed, 5 insertions(+)

diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index 69bbc84..d7a9b5f 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -8,6 +8,7 @@
   https://libvirt.org/html/libvirt-libvirt-host.html#virConnectIsEncrypted
Note that monitoring of traffic on the D-Bus message bus is out 
of the scope of this property"/>
+  
 
 
   https://libvirt.org/html/libvirt-libvirt-host.html#virConnectIsSecure
Note that monitoring of traffic on the D-Bus message bus is out 
of the scope of this property"/>
+  
 
 
   
   https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetUUIDString"/>
+  
 
 
   
   https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkGetName"/>
+  
 
 
   
   https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkGetUUIDString"/>
+  
 
 
   https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [dbus PATCH v2 1/2] Change the default annotation for emitting changed properties to false

[Pavel Hrdina]

For some of these properties there is no libvirt event to detect the
change.  For some of the remaining properties we could somehow detect
the change but it would be a lot of code for nothing and it can be
added later if someone asks for that.

We could change the properties to methods but with the annotation we
can keep them as properties in order to allow to get them by single
D-Bus call.

Signed-off-by: Pavel Hrdina 
---

changes in v2:
- annotation was moved to  scope to make it the default
  one

 data/org.libvirt.Connect.xml | 1 +
 data/org.libvirt.Domain.xml  | 1 +
 data/org.libvirt.Network.xml | 1 +
 3 files changed, 3 insertions(+)

diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index 3bda461..69bbc84 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -3,6 +3,7 @@
 
 
   
+
 
   https://libvirt.org/html/libvirt-libvirt-host.html#virConnectIsEncrypted
diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
index eae6d97..089b896 100644
--- a/data/org.libvirt.Domain.xml
+++ b/data/org.libvirt.Domain.xml
@@ -3,6 +3,7 @@
 
 
   
+
 
   https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainIsActive"/>
diff --git a/data/org.libvirt.Network.xml b/data/org.libvirt.Network.xml
index 81bf081..5b6823e 100644
--- a/data/org.libvirt.Network.xml
+++ b/data/org.libvirt.Network.xml
@@ -3,6 +3,7 @@
 
 
   
+
 
   https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkIsActive"/>
-- 
2.14.3

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

Re: [libvirt] [dbus PATCH v2 0/2] improve annotation of properties

[Katerina Koukiou]

On Fri, 2018-05-04 at 14:28 +0200, Pavel Hrdina wrote:
> Pavel Hrdina (2):
>   Change the default annotation for emitting changed properties to
> false
>   Annotate properties that will never change during the object
> lifecycle
> 
>  data/org.libvirt.Connect.xml | 3 +++
>  data/org.libvirt.Domain.xml  | 2 ++
>  data/org.libvirt.Network.xml | 3 +++
>  3 files changed, 8 insertions(+)

ACK both patches in the series.

Katerina

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

Re: [libvirt] [perl PATCH v2 2/3] prepare-release: Drop references to Test::AutoBuild

[Daniel P . Berrangé]

On Fri, May 04, 2018 at 02:20:25PM +0200, Andrea Bolognani wrote:
> They are no longer relevant and misleading.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  prepare-release.sh | 14 +++---
>  1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/prepare-release.sh b/prepare-release.sh
> index 8a6d102..22f9155 100755
> --- a/prepare-release.sh
> +++ b/prepare-release.sh
> @@ -1,21 +1,17 @@
>  #!/bin/sh
> -#
> -# This script is used to Test::AutoBuild (http://www.autobuild.org)
> -# to perform automated builds of the Sys-Virt module
>  
>  NAME=Sys-Virt
>  
>  set -e
>  
>  test -n "$1" && RESULTS=$1 || RESULTS=results.log
> -: ${AUTOBUILD_INSTALL_ROOT=$HOME/builder}
>  
>  make -k realclean ||:
>  rm -rf MANIFEST blib pm_to_blib
>  
>  export TEST_MAINTAINER=1
>  
> -perl Makefile.PL  PREFIX=$AUTOBUILD_INSTALL_ROOT
> +perl Makefile.PL PREFIX=$HOME/builder
>  
>  rm -f MANIFEST
>  
> @@ -56,12 +52,8 @@ rm -f $NAME-*.tar.gz
>  make dist
>  
>  if [ -f /usr/bin/rpmbuild ]; then
> -  if [ -n "$AUTOBUILD_COUNTER" ]; then
> -EXTRA_RELEASE=".auto$AUTOBUILD_COUNTER"
> -  else
> -NOW=`date +"%s"`
> -EXTRA_RELEASE=".$USER$NOW"
> -  fi
> +  NOW=`date +"%s"`
> +  EXTRA_RELEASE=".$USER$NOW"
>rpmbuild --nodeps -ta --define "extra_release $EXTRA_RELEASE" --clean 
> $NAME-*.tar.gz
>  fi

We can actually chop the 'extra_release' bit out of the RPM spec
entirely, as it is only useful for automated CI builds.

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] [perl PATCH v2 3/3] prepare-release: Drop references to Debian packages

[Daniel P . Berrangé]

On Fri, May 04, 2018 at 02:20:26PM +0200, Andrea Bolognani wrote:
> The Debian packaging was never included in the git repository
> to begin with.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  prepare-release.sh | 8 
>  1 file changed, 8 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] [perl PATCH v2 1/3] maint: Rename autobuild.sh to prepare-release.sh

[Daniel P . Berrangé]

On Fri, May 04, 2018 at 02:20:24PM +0200, Andrea Bolognani wrote:
> The script was originally used by the Test::AutoBuild
> project to perform periodic automatic builds; however, that
> effort has been abandoned a long time ago, and these days
> libvirt-perl CI builds are happening on the Jenkins-based
> CentOS CI environment under the libvirt umbrella[1], where
> build recipes are maintained separately from the projects
> themselves.
> 
> The script is still used to prepare releases, so it can't
> be dropped from the repository: rename it so that its
> purpose is more clearly communicated instead.
> 
> [1] https://ci.centos.org/view/libvirt/
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  autobuild.sh => prepare-release.sh | 0
>  1 file changed, 0 insertions(+), 0 deletions(-)
>  rename autobuild.sh => prepare-release.sh (100%)

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] Qemu driver: Support network-backed pflash disks.

[John Ferlan]

On 05/02/2018 01:24 PM, Prerna wrote:
> 
> 
> On Sat, Apr 28, 2018 at 1:11 AM, John Ferlan  > wrote:
> 
> 
> 

[...]

>  
> 
> This will need a v2 anyway because the patch has too much going on and
> needs to be split up more.
> 
> 
> Will do. I should have properly mentioned this was an RFC rather than a
> ready-to-merge patch, and that this currently excluded test and
> documentation fixes.
>  

Even with an RFC - it's really more pleasant to break things up a bit.
Makes it easier to follow flow rather than one large patch where
multiple things are happening and you're left trying to figure that all
out.  But I also understand that sometimes when posting as RFC that
things languish longer than posting as v1, so it's a gamble as to which
to use...

> 
> 
> You need to break out the domain_conf, docs, etc. changes into one
> patch. Much of what you put into the cover that describes the XML
> should 
> 
> 
> Got it.
>  
> 
> go into this patch's commit message. There should be xml2xml test
> changes as well as adjustments to formatdomain.html.in
>  to describe the
> new options. The part of the cover that says automatically reformatting
> to use the storage source cannot happen. There's precedence for that 
> 
> when the  and  moved *into* the storage source we
> still have to accommodate for them outside. Internally, it can be placed
> as expected, but when formatting, we have to format as we read. After
> 
> 
> Ok. I had explicitly asked around on IRC if it was okay "breaking" the
> existing XML  semantics. Peter had mentioned it was okay to have the XML
> read as its old semantic. The formatter could later "translate" the old
> -style absolute filepaths into the "new-style" source-description that
> is introduced.
> I had kept that in mind while implementing this patch. If that is not to
> be done, I will need to redo parts of this patch.
>  

I see you pinged on IRC today - I had this marked as get back to because
it appears there's questions. Juggling lots of different series and your
response only came on Wed afternoon for me. It's now Friday morning.

In any case, I think that contradicts what was required of me to be done
when moving  and  into  from . See
commits 37537a7c and 8002d3c which handle formatting as the XML was read.

Now if someone *changes* the read XML to use , then
that's a different story. But if you find:

/usr/lib/xen/boot/hvmloader

then you should format that.

If you find


  


then you format that.

Hopefully the two examples provided give you an idea of the "accepted
mechanism" used in a previous change of XML format.

> 
> that, perhaps add the security changes in another, the cgroup in
> another, and finally the qemu adjustments in the last.  Not even sure if
> you need a capability as well.
> 
> 
> Why do you think we'd need a capability for this?  I'd be keen to
> understand why changes to XML spec  is not enough.
>  

Depends on the command line generated I guess - cannot recall right now
if that was clear while I was looking at the existing changes. In the
long run, you have to decide whether the arguments provided to QEMU have
existed since QEMU 1.5. If they have, then no capability, but if there's
some argument provided on the QEMU command line that was introduced in
(for example) 2.9 in order to allow usage of a networked path, then
you'd need a capability.

> 
> Finally this doesn't even compile for me.  You removed @path from
> _virDomainLoaderDef but neglected to adjust all consumers. I suggest
> using cscope and egrep'ing on "os.*loader->path" as well as ->nvram
> since you changed it's type.
> 
> 
> I know why. I had run and tested this to work, but my build
> configuration included the qemu driver and excluded every other driver.
> Given that this element extends to other drivers, I can understand the
> build issues. Again, should have mentioned this was an RFC.
>  
> 
> 
> I assume you've considered that network storage types need to deal with
> authentication and encryption passphrases, right?  What about using a
> srcpool storage source?
> 
> 
> Erm, no. This patch does not include support for
> encryption/authenticaton. I will need to add those.
>  

At least a storage source provides the capability to storage, parse,
format that data...

> 
> I'll briefly scan the rest.
> 

[...]

> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 35666c1..c80f9d9 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -2883,8 +2883,8 @@ virDomainLoaderDefFree(virDomainLoaderDefPtr
> loader)
> >      if (!loader)
> >          return;
> > 
> > -    VIR_FREE(loader->path);
> > -    VIR_FREE(loader->nvram);
> > +    virStorageSourceFree(loader->loader_src);
> 
> loader_src is redundant to loader isn't it?
> 
> 
> Should this 

[libvirt] [jenkins-ci PATCH] guests: Update vault

[Pavel Hrdina]

Renaming node in Jenkins changes the secret as well so we cannot reuse
the same secret as for the old node.

This updates the libvirt-fedora-28 secret to correct one.

Signed-off-by: Pavel Hrdina 
---

Pushed under trivial rule.

 guests/vars/vault.yml | 90 +--
 1 file changed, 45 insertions(+), 45 deletions(-)

diff --git a/guests/vars/vault.yml b/guests/vars/vault.yml
index 403bd9b..5bf89dc 100644
--- a/guests/vars/vault.yml
+++ b/guests/vars/vault.yml
@@ -1,46 +1,46 @@
 $ANSIBLE_VAULT;1.1;AES256
-3766396131623832643164623861363938613135376161343231383739663836313130363538
-3136326161376136376332636534643437626530316439650a636134636561333962376564626635
-31306539613534653133383161313834633133316361303539646234356335386639303332323063
-3937626336326131650a373737396639373063316131633634393961393634663961613532353134
-3262306537343833656164353866653665323536643730386364653162323939616131313634
-38653936663937613765663465353166656363356336636362353432636537633836623930653437
-3035323930343561353663686436336638636462626537623766343237376331643334633433
-37353531616466386234376639306261303561373035383338383366646537373231386362313562
-396536643838383365363138623635616365383530366466653932636238336234383865
-3962353561636165383862616430626634373237626638373131303439356164646361613838
-64333165393935636162613638626461313661623963626664366361366364643136313236306636
-61633665393630356133306338343563636332386539626366383133326662633638306330323437
-34613034326465346338633538363231386430363932656430346634663437333734363234346533
-35303962333032626435343030653338656133323932386264346562323133326164623961623466
-6264376563336362666439653038653335373531303930313239393166623539386239323337
-3939333731356365613164613060313230366462306636363535386663666131613662363566
-63643030633262336263316134636136616430636536363464386436616530336264396264393762
-3664653163623834376337643866616337616661323130636363353662316635643738683665
-37353962383238346132633230373666366638386162663263663134303936633034643737386362
-36393537373864323632376632376134303866393966336536376139393334613163356234623863
-36353837616535313266326538376663316366616561613833393037316264396532306430623338
-33636339623861313536323835326464636265386536646263643765306432626462363533373434
-32343961613563613734633736396139623463653230303337333862393836393533363637626662
-35376236613436323736643533613436646136373436363862383933323231663237343838313262
-34343337343031326139366636346632303738366439346433633738313464366438366235356462
-6537616539653262353335396332333635346661613431343237366166376462393732373532
-62616631643564653865386133663666313366313336323432316236336165386234323165366461
-35343238336235386364363732343862393039326234653539643861383832393465343536353739
-33633134333836393965343935316564653866363130343234633566643463343331306135386334
-39316166616463383964613432373437623964343164383638666531303064313063663330366566
-3364393039623262626639663436656463316837373062636131623463613061656138363534
-39383830626136636563663233376237626565643661656430336462626436386235643239646365
-34303938323237626362613765326138376130666466373033626338363161383432623465353131
-39373230336330666337393161306437313035373164366330333533636662313266336333626261
-37353638623362643138356434323864346265613835306665383564323139333539336231323534
-38323664396438663664343436333133656336663633396234303034633163643534346139663632
-34633364373066303363306366653566353338326566616138643761343166343536333239653966
-3962663938376265343964363166316130323330313063336532303531393830646532343261
-6536343437353865333730623238653465313266643838333862616337323063626432393939
-34663838363931353561396164623138366538376230333738373962356337366533643866343863
-3763336632333432393630396265346235383138626233626339633366336164623766653862
-32653262373433343136386363386231373938636661373363393331663365636333633164353563
-3864396637326135613166643231393431303836303430616237326235366630393564303935
-62383432633763623864323263616632396166363165336331363434316430626334616537376132
-6365336362373265363462343666356434323365633632626632666130356431
+66646139336431653964363035353762363664313738393832356132333835616363373032656335
+3439633266356532316564643762623935643736613839390a616631363336393962633464326264
+31373235323236303134356165386638663865353331353362386538386233363438623035343462
+3864656136376430360a64623537373463373934616463363234316262376534633865616632
+32643566633531323162366134333562363463336237646130393965646335316561393565663664
+36343164373564336233363963393031363937336137303433396265323261663632373534613365
+31613838363562366564646333646638303837626638323065326237623665646264633233633537
+3937343236396239646332353235343462363138376334623434346233356538366439393835
+30356437393232363432313334616462346164393437303330343562356461626464633935376239
+396264643166623337366439626239

Re: [libvirt] [PATCH 2/2] all: replacing virGetLastError with virHas/GetLastErrorCode/Domain

[Erik Skultety]

On Fri, May 04, 2018 at 04:46:48AM +0100, ramyelkest wrote:
> Many places in the code call virGetLastError() just to check the
> raised error code, or domain. However virGetLastError() can return
> NULL, so the code has to check for that as well.
>
> So Instead we create functions virGetLastErrorCode and virGetLastErrorDomain
> (in addition to the existing virGetLastErrorMessage) that always return a
> valid error code/domain/message, to simplify callers.
>
> Also created virHasLastErrorCode for completion
>
> for:
> https://wiki.libvirt.org/page/BiteSizedTasks#Add_and_use_virGetLastErrorCode.28.29
>
> Notes:
> * There are a few instances of virGetLastErrorCode() left where we use 
> multiple
> fields from the error, which makes sense to keep it.
> * I didn't manage to use virGetLastErrorDomain() (due to the above) so I'm 
> inclined
> to remove it.

Same comments to the commit message as for patch 1.

...
>
> Signed-off-by: Ramy Elkest 
> ---
>  src/locking/lock_driver_lockd.c |  3 +--
>  src/lxc/lxc_controller.c|  4 +---
>  src/qemu/qemu_agent.c   |  3 +--
>  src/qemu/qemu_conf.c|  3 +--
>  src/qemu/qemu_domain.c  |  2 +-
>  src/qemu/qemu_driver.c  | 12 ++--
>  src/qemu/qemu_hotplug.c |  2 +-
>  src/qemu/qemu_migration.c   |  4 ++--
>  src/qemu/qemu_monitor.c |  5 ++---
>  src/qemu/qemu_monitor_json.c|  2 +-
>  src/qemu/qemu_process.c |  2 +-
>  src/remote/remote_driver.c  |  3 +--
>  src/rpc/virnetclient.c  |  2 +-
>  src/util/virfilecache.c |  3 +--
>  src/util/virxml.c   |  4 ++--
>  tests/commandtest.c |  2 +-
>  tests/testutils.c   |  6 ++
>  tests/virhostcputest.c  |  2 +-
>  tests/virstoragetest.c  |  8 
>  tools/virsh-domain-monitor.c|  7 +++
>  tools/virsh-domain.c|  4 +---
>  tools/virsh-util.c  |  3 +--
>  tools/vsh.c |  2 +-
>  23 files changed, 37 insertions(+), 51 deletions(-)

I found 2 more occurrences in src/util/virnetlibsshsession.c (l.502) and
src/util/virmodule.c (l.126) respectively, which can be optimized too.

...
>  if (!(loadData = cache->handlers.loadFile(file, name, cache->priv))) {
> -virErrorPtr err = virGetLastError();
>  VIR_WARN("Failed to load cached data from '%s' for '%s': %s",
> - file, name, err ? NULLSTR(err->message) : "unknown error");
> + file, name, virGetLastErrorMessage());

Good catch, although, this should be a separate patch IMO, let's say patch 1
and then the rest of the series. On a side note, I looked whether there aren't
other places which could use the virGetLastErrorMessage, sadly no, since the
interface isn't really flexible (among other problems with the whole error
subsystem) so that when there's no error we report "unknown error" which is
good as nothing, so providing a custom string in case there's no error or we
can't identify it would probably have been a smarter choice, oh well...

Other than that I'm fine with the changes (v2 shouldn't be a problem).

Erik

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

Re: [libvirt] [perl PATCH v2 2/3] prepare-release: Drop references to Test::AutoBuild

[Andrea Bolognani]

On Fri, 2018-05-04 at 13:38 +0100, Daniel P. Berrangé wrote:
> On Fri, May 04, 2018 at 02:20:25PM +0200, Andrea Bolognani wrote:
[...]
> >  if [ -f /usr/bin/rpmbuild ]; then
> > -  if [ -n "$AUTOBUILD_COUNTER" ]; then
> > -EXTRA_RELEASE=".auto$AUTOBUILD_COUNTER"
> > -  else
> > -NOW=`date +"%s"`
> > -EXTRA_RELEASE=".$USER$NOW"
> > -  fi
> > +  NOW=`date +"%s"`
> > +  EXTRA_RELEASE=".$USER$NOW"
> >rpmbuild --nodeps -ta --define "extra_release $EXTRA_RELEASE" --clean 
> > $NAME-*.tar.gz
> >  fi
> 
> We can actually chop the 'extra_release' bit out of the RPM spec
> entirely, as it is only useful for automated CI builds.

Neat. I assume you meant that we can avoid defining it when calling
rpmbuild in the script, not that we want to remove it from the spec
altogether, right? The latter can still be useful.

Can I just squash in the following diff and push, or do you want me
to respin?


diff --git a/prepare-release.sh b/prepare-release.sh
index 22f9155..25a5cda 100755
--- a/prepare-release.sh
+++ b/prepare-release.sh
@@ -52,9 +52,7 @@ rm -f $NAME-*.tar.gz
 make dist

 if [ -f /usr/bin/rpmbuild ]; then
-  NOW=`date +"%s"`
-  EXTRA_RELEASE=".$USER$NOW"
-  rpmbuild --nodeps -ta --define "extra_release $EXTRA_RELEASE" --clean 
$NAME-*.tar.gz
+  rpmbuild --nodeps -ta --clean $NAME-*.tar.gz
 fi

 # Skip debian pkg for now
-- 
Andrea Bolognani / Red Hat / Virtualization

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

[libvirt] [PATCH] lxc: convert to typesafe virConf accessors in lxc_native.c

[Prafull]

From: Prafullkumar Tale 

Signed-off-by: Prafullkumar Tale 
---
 src/lxc/lxc_native.c | 141 +--
 1 file changed, 70 insertions(+), 71 deletions(-)

diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
index 55ea774..35077e1 100644
--- a/src/lxc/lxc_native.c
+++ b/src/lxc/lxc_native.c
@@ -199,19 +199,15 @@ lxcSetRootfs(virDomainDefPtr def,
  virConfPtr properties)
 {
 int type = VIR_DOMAIN_FS_TYPE_MOUNT;
-virConfValuePtr value;
+char *value = NULL;
 
-if (!(value = virConfGetValue(properties, "lxc.rootfs")) ||
-!value->str) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("Missing lxc.rootfs configuration"));
+if (virConfGetValueString(properties, "lxc.rootfs", &value) <= 0)
 return -1;
-}
 
-if (STRPREFIX(value->str, "/dev/"))
+if (STRPREFIX(value, "/dev/"))
 type = VIR_DOMAIN_FS_TYPE_BLOCK;
 
-if (lxcAddFSDef(def, type, value->str, "/", false, 0) < 0)
+if (lxcAddFSDef(def, type, value, "/", false, 0) < 0)
 return -1;
 
 return 0;
@@ -684,17 +680,17 @@ lxcConvertNetworkSettings(virDomainDefPtr def, virConfPtr 
properties)
 static int
 lxcCreateConsoles(virDomainDefPtr def, virConfPtr properties)
 {
-virConfValuePtr value;
+char *value = NULL;
 int nbttys = 0;
 virDomainChrDefPtr console;
 size_t i;
 
-if (!(value = virConfGetValue(properties, "lxc.tty")) || !value->str)
+if (virConfGetValueString(properties, "lxc.tty", &value) <= 0)
 return 0;
 
-if (virStrToLong_i(value->str, NULL, 10, &nbttys) < 0) {
+if (virStrToLong_i(value, NULL, 10, &nbttys) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to parse int: '%s'"),
-   value->str);
+   value);
 return -1;
 }
 
@@ -761,89 +757,91 @@ lxcIdmapWalkCallback(const char *name, virConfValuePtr 
value, void *data)
 static int
 lxcSetMemTune(virDomainDefPtr def, virConfPtr properties)
 {
-virConfValuePtr value;
+char *value = NULL;
 unsigned long long size = 0;
 
-if ((value = virConfGetValue(properties,
-"lxc.cgroup.memory.limit_in_bytes")) &&
-value->str && STRNEQ(value->str, "-1")) {
-if (lxcConvertSize(value->str, &size) < 0)
-return -1;
+if (virConfGetValueString(properties,
+  "lxc.cgroup.memory.limit_in_bytes",
+  &value) > 0) {
+if (lxcConvertSize(value, &size) < 0)
+goto error;
 size = size / 1024;
 virDomainDefSetMemoryTotal(def, size);
 def->mem.hard_limit = virMemoryLimitTruncate(size);
 }
 
-if ((value = virConfGetValue(properties,
-"lxc.cgroup.memory.soft_limit_in_bytes")) &&
-value->str && STRNEQ(value->str, "-1")) {
-if (lxcConvertSize(value->str, &size) < 0)
-return -1;
-
+if (virConfGetValueString(properties,
+  "lxc.cgroup.memory.soft_limit_in_bytes",
+  &value) > 0) {
+if (lxcConvertSize(value, &size) < 0)
+goto error;
 def->mem.soft_limit = virMemoryLimitTruncate(size / 1024);
 }
 
-if ((value = virConfGetValue(properties,
-"lxc.cgroup.memory.memsw.limit_in_bytes")) &&
-value->str && STRNEQ(value->str, "-1")) {
-if (lxcConvertSize(value->str, &size) < 0)
-return -1;
-
+if (virConfGetValueString(properties,
+  "lxc.cgroup.memory.memsw.limit_in_bytes",
+  &value) > 0) {
+if (lxcConvertSize(value, &size) < 0)
+goto error;
 def->mem.swap_hard_limit = virMemoryLimitTruncate(size / 1024);
 }
 return 0;
+
+ error:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("failed to parse integer: '%s'"), value);
+return -1;
+
 }
 
 static int
 lxcSetCpuTune(virDomainDefPtr def, virConfPtr properties)
 {
-virConfValuePtr value;
+char *value = NULL;
 
-if ((value = virConfGetValue(properties, "lxc.cgroup.cpu.shares")) &&
-value->str) {
-if (virStrToLong_ull(value->str, NULL, 10, &def->cputune.shares) < 0)
+if (virConfGetValueString(properties, "lxc.cgroup.cpu.shares",
+  &value) > 0) {
+if (virStrToLong_ull(value, NULL, 10, &def->cputune.shares) < 0)
 goto error;
 def->cputune.sharesSpecified = true;
 }
 
-if ((value = virConfGetValue(properties,
- "lxc.cgroup.cpu.cfs_quota_us")) &&
-value->str && virStrToLong_ll(value->str, NULL, 10,
-  &def->cputune.quota) < 0)
-goto error;
+if (virConfGetValueString(properties, "lxc.cgroup.cpu.cfs_quota_us",
+  &value) > 0) {
+if (virStrT

Re: [libvirt] [perl PATCH v2 2/3] prepare-release: Drop references to Test::AutoBuild

[Daniel P . Berrangé]

On Fri, May 04, 2018 at 03:14:04PM +0200, Andrea Bolognani wrote:
> On Fri, 2018-05-04 at 13:38 +0100, Daniel P. Berrangé wrote:
> > On Fri, May 04, 2018 at 02:20:25PM +0200, Andrea Bolognani wrote:
> [...]
> > >  if [ -f /usr/bin/rpmbuild ]; then
> > > -  if [ -n "$AUTOBUILD_COUNTER" ]; then
> > > -EXTRA_RELEASE=".auto$AUTOBUILD_COUNTER"
> > > -  else
> > > -NOW=`date +"%s"`
> > > -EXTRA_RELEASE=".$USER$NOW"
> > > -  fi
> > > +  NOW=`date +"%s"`
> > > +  EXTRA_RELEASE=".$USER$NOW"
> > >rpmbuild --nodeps -ta --define "extra_release $EXTRA_RELEASE" --clean 
> > > $NAME-*.tar.gz
> > >  fi
> > 
> > We can actually chop the 'extra_release' bit out of the RPM spec
> > entirely, as it is only useful for automated CI builds.
> 
> Neat. I assume you meant that we can avoid defining it when calling
> rpmbuild in the script, not that we want to remove it from the spec
> altogether, right? The latter can still be useful.

I did actually mean removing it from the RPM spec entirely - Fedora
reviewers have complained about it existing multiple times, but at
least in past I could justify it for CI purposes.

> 
> Can I just squash in the following diff and push, or do you want me
> to respin?
> 
> 
> diff --git a/prepare-release.sh b/prepare-release.sh
> index 22f9155..25a5cda 100755
> --- a/prepare-release.sh
> +++ b/prepare-release.sh
> @@ -52,9 +52,7 @@ rm -f $NAME-*.tar.gz
>  make dist
> 
>  if [ -f /usr/bin/rpmbuild ]; then
> -  NOW=`date +"%s"`
> -  EXTRA_RELEASE=".$USER$NOW"
> -  rpmbuild --nodeps -ta --define "extra_release $EXTRA_RELEASE" --clean 
> $NAME-*.tar.gz
> +  rpmbuild --nodeps -ta --clean $NAME-*.tar.gz
>  fi
> 
>  # Skip debian pkg for now
> -- 
> Andrea Bolognani / Red Hat / Virtualization

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] [perl PATCH v2 2/3] prepare-release: Drop references to Test::AutoBuild

[Andrea Bolognani]

On Fri, 2018-05-04 at 14:20 +0100, Daniel P. Berrangé wrote:
> On Fri, May 04, 2018 at 03:14:04PM +0200, Andrea Bolognani wrote:
> > On Fri, 2018-05-04 at 13:38 +0100, Daniel P. Berrangé wrote:
> > > We can actually chop the 'extra_release' bit out of the RPM spec
> > > entirely, as it is only useful for automated CI builds.
> > 
> > Neat. I assume you meant that we can avoid defining it when calling
> > rpmbuild in the script, not that we want to remove it from the spec
> > altogether, right? The latter can still be useful.
> 
> I did actually mean removing it from the RPM spec entirely - Fedora
> reviewers have complained about it existing multiple times, but at
> least in past I could justify it for CI purposes.

Not sure I agree, but I don't feel strongly enough to oppose the
change either :)

That IMHO puts it squarely into separate patch territory though,
so if you're okay with it I'll push this one as originally posted
and follow up with another that drops extra_release completely.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [perl PATCH v2 2/3] prepare-release: Drop references to Test::AutoBuild

[Daniel P . Berrangé]

On Fri, May 04, 2018 at 03:37:50PM +0200, Andrea Bolognani wrote:
> On Fri, 2018-05-04 at 14:20 +0100, Daniel P. Berrangé wrote:
> > On Fri, May 04, 2018 at 03:14:04PM +0200, Andrea Bolognani wrote:
> > > On Fri, 2018-05-04 at 13:38 +0100, Daniel P. Berrangé wrote:
> > > > We can actually chop the 'extra_release' bit out of the RPM spec
> > > > entirely, as it is only useful for automated CI builds.
> > > 
> > > Neat. I assume you meant that we can avoid defining it when calling
> > > rpmbuild in the script, not that we want to remove it from the spec
> > > altogether, right? The latter can still be useful.
> > 
> > I did actually mean removing it from the RPM spec entirely - Fedora
> > reviewers have complained about it existing multiple times, but at
> > least in past I could justify it for CI purposes.
> 
> Not sure I agree, but I don't feel strongly enough to oppose the
> change either :)
> 
> That IMHO puts it squarely into separate patch territory though,
> so if you're okay with it I'll push this one as originally posted
> and follow up with another that drops extra_release completely.

Sure, consider it 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 REBASE 2/5] qemu: monitor: set error flag even in OOM conditions

[John Ferlan]

On 05/03/2018 03:35 AM, Nikolay Shirokovskiy wrote:
> 
> 
> On 01.05.2018 01:03, John Ferlan wrote:
>>
>>
>> On 04/18/2018 10:44 AM, Nikolay Shirokovskiy wrote:
>>> lastError.code is used as flag in qemu monitor. Unfortunately
>>> due to temporary OOM conditions (very unlikely through as thread local
>>> error is allocated on first use) we can fail to set this flag
>>> in case of monitor eofs/errors. This can cause hangs.
>>>
>>> Let's make sure flag is always set in case eofs/errors.
>>>
>>> Signed-off-by: Nikolay Shirokovskiy 
>>> ---
>>>  src/qemu/qemu_monitor.c | 5 +
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
>>> index f642d9a..906a320 100644
>>> --- a/src/qemu/qemu_monitor.c
>>> +++ b/src/qemu/qemu_monitor.c
>>> @@ -757,6 +757,11 @@ qemuMonitorIO(int watch, int fd, int events, void 
>>> *opaque)
>>>  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>> _("Error while processing monitor IO"));
>>>  virCopyLastError(&mon->lastError);
>>> +
>>> +/* set error code if due to OOM conditions we fail to set it 
>>> before */
>>> +if (mon->lastError.code == VIR_ERR_OK)
>>
>> So this can only happen if virCopyLastError fails, right?  And code == 0
> 
> Nope, virCopyLastError fail to set code field only in one case - if last
> error is NULL which can be only if in next lines even reporting fails:
> 
> virErrorPtr err = virGetLastError();  
>   
> if (!err) 
>   
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",  
>   
>_("Error while processing monitor IO")); 
> 

I read the code as if pthread_getspecific in virThreadLocalGet returns
NULL, then virLastErrorObject will try to VIR_ALLOC_QUIET and return
NULL if it fails. If that succeeds, then virThreadLocalSet is called to
pthread_setspecific and if it fails, then @err (from VIR_ALLOC_QUIET) is
VIR_FREE'd and a NULL is returned.

So NULL could be returned if VIR_ALLOC_QUIET or virThreadLocalSet fail.

>> because of memset(..., 0, ...).  Wouldn't you only be working around the
>> issue for one of the callers? and only setting *.code and not *.domain?
> 
> We use mon->lastError for 2 purposes:
> 
> - as flag, and it is critical that we set this flag appropriately. We
>   use only code field for this purpuse

Understood, but still ->domain would be 0; however, if you set it to
something (such as VIR_FROM_THREAD), then perhaps that too could be used
as a marker of sorts (e.g. code == value, thread == value, and message
== NULL means something fairly specific).

> 
> - as error message that client can retrieve in case if API call returns
>   error, it is no critical if we can not provide error message, moreover
>   we can fail to pass error message in RPC for example due to temporary OOM
>   conditions
> 
> I even think to remove flag usage from mon->lastError and introduce bool
> flag but this looks ugly like 2 fields to keep error. 
> 

In any case, perhaps you misinterpreted my remark. Only adjusting this
one caller fixes one condition, but if there are possibly more then
changing virCopyLastError instead of here would allow those others to
also have the adjustment...

>>
>> BTW: I feel we've been this way in rather recent history... See commit
>> 'e711a391' - I recall we had varying opinions on that one.
> 
> Yes, but this fix does not relate to usage mon->lastError as flag, rather
> then to second usage in the above distinction.
> 
>>
>> Anyway, assuming CopyLastError is the issue, then perhaps the right
>> place to fix this is there and I would think at least the following
>> would need to be set in the error path:
>>
>>to->code = VIR_ERR_NO_MEMORY;
>>to->domain = VIR_FROM_THREAD;
> 
> It can be misleading. For example if we forget to set last error
> and then try to copy it.
> 
>>
>> We cannot create a to->message...
>>
>> Of course, how can we be "sure" that the failure is because the
>> VIR_ALLOC_QUIET in virLastErrorObject caused the issue other than
>> changing it's return values or passing a "bool &memfail" that's only set
>> when the VIR_ALLOC_QUIET fails allowing the caller to decide what to do.
>> It's local, so change is limited. Thus virGetLastErrorMessage and
>> virCopyLastError could make use of the returned, while other callers
>> could just pass "&dummy".
>>
>> In the long run, it's a somewhat interesting corner case problem and we
>> may have some varying opinions over the "best way" to resolve.  Suffice
>> to say, libvirt probably isn't going to "survive" much longer, but
>> unless others read this response and have varying opinions and/or we
>> come to a resolution - can this be posted/consider on it's own merits?> 
>> Tks -
>>
>> John
> 
> May be it would be just more simple to not to use mon->lastError as flag
> in m

[libvirt] [perl PATCH] spec: Drop %{extra_release}

[Andrea Bolognani]

It was mainly meant to be used for automatic builds through
Test::AutoBuild, so it can be removed now.

Signed-off-by: Andrea Bolognani 
---
 perl-Sys-Virt.spec.PL | 2 +-
 prepare-release.sh| 4 +---
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/perl-Sys-Virt.spec.PL b/perl-Sys-Virt.spec.PL
index dbb749d..ffa6c86 100644
--- a/perl-Sys-Virt.spec.PL
+++ b/perl-Sys-Virt.spec.PL
@@ -24,7 +24,7 @@ __DATA__
 
 Name:   perl-Sys-Virt
 Version:@VERSION@
-Release:1%{?dist}%{?extra_release}
+Release:1%{?dist}
 Summary:Represent and manage a libvirt hypervisor connection
 License:GPLv2+ or Artistic
 Group:  Development/Libraries
diff --git a/prepare-release.sh b/prepare-release.sh
index 9de67fe..fea03f4 100755
--- a/prepare-release.sh
+++ b/prepare-release.sh
@@ -52,7 +52,5 @@ rm -f $NAME-*.tar.gz
 make dist
 
 if [ -f /usr/bin/rpmbuild ]; then
-  NOW=`date +"%s"`
-  EXTRA_RELEASE=".$USER$NOW"
-  rpmbuild --nodeps -ta --define "extra_release $EXTRA_RELEASE" --clean 
$NAME-*.tar.gz
+  rpmbuild --nodeps -ta --clean $NAME-*.tar.gz
 fi
-- 
2.17.0

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

Re: [libvirt] [perl PATCH] spec: Drop %{extra_release}

[Daniel P . Berrangé]

On Fri, May 04, 2018 at 03:50:57PM +0200, Andrea Bolognani wrote:
> It was mainly meant to be used for automatic builds through
> Test::AutoBuild, so it can be removed now.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  perl-Sys-Virt.spec.PL | 2 +-
>  prepare-release.sh| 4 +---
>  2 files changed, 2 insertions(+), 4 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 REBASE 3/5] utils: export virCopyError

[John Ferlan]

On 05/03/2018 03:39 AM, Nikolay Shirokovskiy wrote:
> 
> 
> On 01.05.2018 01:03, John Ferlan wrote:
>>
>>
>> On 04/18/2018 10:44 AM, Nikolay Shirokovskiy wrote:
>>> Signed-off-by: Nikolay Shirokovskiy 
>>> ---
>>>  src/libvirt_private.syms |  1 +
>>>  src/util/virerror.c  | 12 +---
>>>  src/util/virerror.h  |  1 +
>>>  3 files changed, 11 insertions(+), 3 deletions(-)
>>>
>>
>> NACK, you should be using virErrorCopyNew
>>
>> John
> 
> I introduced monError in qemuDomainObjPrivatePtr in next patch and it is not 
> a pointer so
> I need this function. I did not make monError pointer for the same reason it 
> is not pointer
> in monitor object - I use monError both as flag and as error message.
> 

I saw what you did - the fact is virCopyError is coded as private to the
module. Just making it global because you have a use for it is I believe
incorrect.

Ironically in the next patch you have:

+virErrorPtr err = qemuMonitorLastError(mon);
+
+virCopyError(err, &priv->monError);


The first call isn't guaranteed to set @err and all you're essentially
doing is wanting to copy from mon->lastError to priv->monError or
perhaps similar to virCopyLastError.

Maybe some new API in virerror.c should handle that.

John

>>
>>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>>> index b31f599..6bbbf77 100644
>>> --- a/src/libvirt_private.syms
>>> +++ b/src/libvirt_private.syms
>>> @@ -1702,6 +1702,7 @@ ebtablesRemoveForwardAllowIn;
>>>  
>>>  
>>>  # util/virerror.h
>>> +virCopyError;
>>>  virDispatchError;
>>>  virErrorCopyNew;
>>>  virErrorInitialize;
>>> diff --git a/src/util/virerror.c b/src/util/virerror.c
>>> index c000b00..2ff8a3e 100644
>>> --- a/src/util/virerror.c
>>> +++ b/src/util/virerror.c
>>> @@ -188,10 +188,16 @@ virErrorGenericFailure(virErrorPtr err)
>>>  }
>>>  
>>>  
>>> -/*
>>> - * Internal helper to perform a deep copy of an error
>>> +/**
>>> + * virCopyError:
>>> + * @from: error to copy from
>>> + * @to: error to copy to
>>> + *
>>> + * Copy error fields from @from to @to.
>>> + *
>>> + * Returns 0 on success, -1 on failure.
>>>   */
>>> -static int
>>> +int
>>>  virCopyError(virErrorPtr from,
>>>   virErrorPtr to)
>>>  {
>>> diff --git a/src/util/virerror.h b/src/util/virerror.h
>>> index 760bfa4..90ac619 100644
>>> --- a/src/util/virerror.h
>>> +++ b/src/util/virerror.h
>>> @@ -192,6 +192,7 @@ void virReportOOMErrorFull(int domcode,
>>>  
>>>  int virSetError(virErrorPtr newerr);
>>>  virErrorPtr virErrorCopyNew(virErrorPtr err);
>>> +int virCopyError(virErrorPtr from, virErrorPtr to);
>>>  void virDispatchError(virConnectPtr conn);
>>>  const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen);
>>>  
>>>

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

Re: [libvirt] [dbus PATCH v2 2/9] Abandon usage of all *TypeToString functions in connect.c

[Pavel Hrdina]

On Fri, May 04, 2018 at 10:38:28AM +0200, Katerina Koukiou wrote:
> Converting ENUMS to str can be user friendly though
> it can be problematic in matters of backward compatibility.
> 
> In particular when some ENUM in libvirt API will introduce a
> new constant, libvirt-dbus will fail with:
> 
> size of array ‘_GStaticAssertCompileTimeAssertion_5’ is negative
> 
> So using ints is preferable to avoid the previous issue.
> 
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Connect.xml |  2 +-
>  src/connect.c| 18 +-
>  2 files changed, 2 insertions(+), 18 deletions(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [dbus PATCH v2 4/9] Change DomainEvent argument from string to unsigned int

[Pavel Hrdina]

On Fri, May 04, 2018 at 10:38:30AM +0200, Katerina Koukiou wrote:
> Modify the relevant tests to comply with the new signal.
> 
> Note: argument matching in connect_to_signal method is
> only usable with string and thus had to be refactored.
> 
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Connect.xml |  2 +-
>  src/events.c | 26 +-
>  tests/libvirttest.py | 13 +
>  tests/test_connect.py| 12 
>  tests/test_domain.py | 30 --
>  5 files changed, 43 insertions(+), 40 deletions(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [dbus PATCH v2 1/9] Abandon usage of all *TypeToString functions in domain.c

[Pavel Hrdina]

On Fri, May 04, 2018 at 10:38:27AM +0200, Katerina Koukiou wrote:
> Converting ENUMS to str can be user friendly though
> it can be problematic in matters of backward compatibility.
> 
> In particular when some ENUM in libvirt API will introduce a
> new constant, libvirt-dbus will fail with:
> 
> size of array ‘_GStaticAssertCompileTimeAssertion_5’ is negative
> 
> So using ints is preferable to avoid the previous issue.
> 
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Domain.xml |  14 ++--
>  src/domain.c| 172 
> 
>  tests/test_domain.py|   6 +-
>  3 files changed, 25 insertions(+), 167 deletions(-)

[...]

> diff --git a/src/domain.c b/src/domain.c
> index e305fa3..40cf2f7 100644
> --- a/src/domain.c
> +++ b/src/domain.c

[...]

> @@ -137,12 +68,8 @@ 
> virtDBusDomainMemoryStatsToGVariant(virDomainMemoryStatPtr stats,
>  
>  g_variant_builder_init(&builder, G_VARIANT_TYPE("a{st}"));
>  
> -for (gint i = 0; i < nr_stats; i++) {
> -const gchar *memoryStat = 
> virtDBusDomainMemoryStatTypeToString(stats[i].tag);
> -if (!memoryStat)
> -continue;
> -g_variant_builder_add(&builder, "{st}", memoryStat, stats[i].val);
> -}
> +for (gint i = 0; i < nr_stats; i++)
> +g_variant_builder_add(&builder, "{ut}", stats[i].tag, stats[i].val);

MemoryStats method needs to be updated in the introspection file.

>  
>  return g_variant_builder_end(&builder);
>  }

[...]

> @@ -1355,7 +1252,6 @@ virtDBusDomainGetJobInfo(GVariant *inArgs G_GNUC_UNUSED,
>  virtDBusConnect *connect = userData;
>  g_autoptr(virDomain) domain = NULL;
>  g_autofree virDomainJobInfoPtr jobInfo = NULL;
> -const gchar *jobTypeStr;
>  
>  domain = virtDBusDomainGetVirDomain(connect, objectPath, error);
>  if (!domain)
> @@ -1365,13 +1261,7 @@ virtDBusDomainGetJobInfo(GVariant *inArgs 
> G_GNUC_UNUSED,
>  if (virDomainGetJobInfo(domain, jobInfo) < 0)
>  return virtDBusUtilSetLastVirtError(error);
>  
> -jobTypeStr = virtDBusDomainJobTypeToString(jobInfo->type);
> -if (!jobTypeStr) {
> -g_set_error(error, VIRT_DBUS_ERROR, VIRT_DBUS_ERROR_LIBVIRT,
> -"Can't format virDomainJobType '%d' to string.", 
> jobInfo->type);
> -return;
> -}
> -*outArgs = g_variant_new("((sttt))", jobTypeStr,
> +*outArgs = g_variant_new("((uttt))", jobInfo->type,

GetJobInfo method needs to be updated in the introspection file.

>   jobInfo->timeElapsed, jobInfo->timeRemaining,
>   jobInfo->dataTotal, jobInfo->dataProcessed,
>   jobInfo->dataRemaining, jobInfo->memTotal,

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [dbus PATCH v2 3/9] Abandon usage of all *TypeToString functions in network.c

[Pavel Hrdina]

On Fri, May 04, 2018 at 10:38:29AM +0200, Katerina Koukiou wrote:
> Converting ENUMS to str can be user friendly though
> it can be problematic in matters of backward compatibility.
> 
> In particular when some ENUM in libvirt API will introduce a
> new constant, libvirt-dbus will fail with:
> 
> size of array ‘_GStaticAssertCompileTimeAssertion_5’ is negative
> 
> So using ints is preferable to avoid the previous issue.
> 
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Network.xml |  6 ++--
>  src/network.c| 66 
> +++-
>  tests/test_network.py|  2 +-
>  3 files changed, 8 insertions(+), 66 deletions(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [dbus PATCH v2 5/9] Change NetworkEvent argument from string to unsigned int

[Pavel Hrdina]

On Fri, May 04, 2018 at 10:38:31AM +0200, Katerina Koukiou wrote:
> Modify the relevant tests to comply with the new signal.
> 
> Note: argument matching in connect_to_signal method is
> only usable with string and thus had to be refactored.
> 
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Connect.xml |  2 +-
>  src/events.c | 14 +-
>  tests/libvirttest.py |  7 +++
>  tests/test_connect.py| 12 
>  tests/test_network.py| 18 --
>  5 files changed, 29 insertions(+), 24 deletions(-)

Reviewed-by: Pavel Hrdina 


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

[libvirt] [tck PATCH v2 3/3] spec: Drop %{extra_release}

[Andrea Bolognani]

It was mainly meant to be used for automatic builds through
Test::AutoBuild, so it can be removed now.

Signed-off-by: Andrea Bolognani 
---
 perl-Sys-Virt-TCK.spec.PL | 3 +--
 prepare-release.sh| 4 +---
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/perl-Sys-Virt-TCK.spec.PL b/perl-Sys-Virt-TCK.spec.PL
index 9694227..aac6cf6 100644
--- a/perl-Sys-Virt-TCK.spec.PL
+++ b/perl-Sys-Virt-TCK.spec.PL
@@ -32,12 +32,11 @@ __DATA__
 %define perlversion %(perl -e 'use Config; print $Config{version}')
 
 %define appname Sys-Virt-TCK
-%define _extra_release %{?extra_release:%{extra_release}}
 
 Summary: Sys::Virt::TCK - libvirt Technology Compatibility Kit
 Name: perl-%{appname}
 Version: @VERSION@
-Release: 1%{_extra_release}
+Release: 1
 License: GPLv2
 Group: Development/Tools
 Source: http://libvirt.org/sources/tck/%{appname}-v%{version}.tar.gz
diff --git a/prepare-release.sh b/prepare-release.sh
index 7282123..75777a8 100755
--- a/prepare-release.sh
+++ b/prepare-release.sh
@@ -42,9 +42,7 @@ rm -f $NAME-*.tar.gz
 ./Build dist
 
 if [ -f /usr/bin/rpmbuild ]; then
-  NOW=`date +"%s"`
-  EXTRA_RELEASE=".$USER$NOW"
-  rpmbuild -ta --define "extra_release $EXTRA_RELEASE" --clean $NAME-*.tar.gz
+  rpmbuild -ta --clean $NAME-*.tar.gz
 fi
 
 exit 0
-- 
2.17.0

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

[libvirt] [tck PATCH v2 1/3] maint: Rename autobuild.sh to prepare-release.sh

[Andrea Bolognani]

The script was originally used by the Test::AutoBuild
project to perform periodic automatic builds; however, that
effort has been abandoned a long time ago, and these days
libvirt-tck CI builds are happening on the Jenkins-based
CentOS CI environment under the libvirt umbrella[1], where
build recipes are maintained separately from the projects
themselves.

The script is still used to prepare releases, so it can't
be dropped from the repository: rename it so that its
purpose is more clearly communicated instead.

[1] https://ci.centos.org/view/libvirt/

Signed-off-by: Andrea Bolognani 
---
 autobuild.sh => prepare-release.sh | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename autobuild.sh => prepare-release.sh (100%)

diff --git a/autobuild.sh b/prepare-release.sh
similarity index 100%
rename from autobuild.sh
rename to prepare-release.sh
-- 
2.17.0

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

[libvirt] [tck PATCH v2 2/3] prepare-release: Drop references to Test::AutoBuild

[Andrea Bolognani]

They are misleading, and no longer relevant anyway.

Signed-off-by: Andrea Bolognani 
---
 prepare-release.sh | 15 +++
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/prepare-release.sh b/prepare-release.sh
index 005993d..7282123 100755
--- a/prepare-release.sh
+++ b/prepare-release.sh
@@ -1,8 +1,5 @@
 #!/bin/sh
 #
-# This script is used to Test::AutoBuild (http://www.autobuild.org)
-# to perform automated builds of the Sys-Virt module
-#
 # Copyright (C) 2009 Red Hat, Inc.
 # Copyright (C) 2009 Daniel P. Berrange
 #
@@ -21,9 +18,7 @@ set -e
 
 rm -rf MANIFEST blib _build Build
 
-test -z "$AUTOBUILD_INSTALL_ROOT" && AUTOBUILD_INSTALL_ROOT=$HOME/builder
-
-perl Build.PL install_base=$AUTOBUILD_INSTALL_ROOT
+perl Build.PL install_base=$HOME/builder
 
 ./Build
 ./Build manifest
@@ -47,12 +42,8 @@ rm -f $NAME-*.tar.gz
 ./Build dist
 
 if [ -f /usr/bin/rpmbuild ]; then
-  if [ -n "$AUTOBUILD_COUNTER" ]; then
-EXTRA_RELEASE=".auto$AUTOBUILD_COUNTER"
-  else
-NOW=`date +"%s"`
-EXTRA_RELEASE=".$USER$NOW"
-  fi
+  NOW=`date +"%s"`
+  EXTRA_RELEASE=".$USER$NOW"
   rpmbuild -ta --define "extra_release $EXTRA_RELEASE" --clean $NAME-*.tar.gz
 fi
 
-- 
2.17.0

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

[libvirt] [tck PATCH v2 0/3] Remove Test::AutoBuild leftovers

[Andrea Bolognani]

Changes from [v1]:

* rename the script instead of dropping it;

* perform some further clean ups.


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

Andrea Bolognani (3):
  maint: Rename autobuild.sh to prepare-release.sh
  prepare-release: Drop references to Test::AutoBuild
  spec: Drop %{extra_release}

 perl-Sys-Virt-TCK.spec.PL  |  3 +--
 autobuild.sh => prepare-release.sh | 15 ++-
 2 files changed, 3 insertions(+), 15 deletions(-)
 rename autobuild.sh => prepare-release.sh (64%)

-- 
2.17.0

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

Re: [libvirt] [dbus PATCH v2 6/9] Remove virtDBusUtilEnum{From, From}String functions

[Pavel Hrdina]

On Fri, May 04, 2018 at 10:38:32AM +0200, Katerina Koukiou wrote:
> Signed-off-by: Katerina Koukiou 
> ---
>  src/util.c | 27 ---
>  src/util.h | 28 
>  2 files changed, 55 deletions(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [dbus PATCH v2 7/9] Remove reason to string translation in virtDBusEventsDomainTrayChange

[Pavel Hrdina]

On Fri, May 04, 2018 at 10:38:33AM +0200, Katerina Koukiou wrote:
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Domain.xml |  2 +-
>  src/events.c| 15 +--
>  2 files changed, 2 insertions(+), 15 deletions(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [dbus PATCH v2 8/9] Remove state to string translation in virtDBusDomainGetState

[Pavel Hrdina]

On Fri, May 04, 2018 at 10:38:34AM +0200, Katerina Koukiou wrote:
> Adjust tests to comply with the new type.
> 
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Domain.xml |  2 +-
>  src/domain.c| 31 +--
>  tests/libvirttest.py| 12 
>  tests/test_domain.py| 10 +-
>  4 files changed, 19 insertions(+), 36 deletions(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [dbus PATCH v2 9/9] Remove reason to string translation in virtDBusEventsDomainDiskChange

[Pavel Hrdina]

On Fri, May 04, 2018 at 10:38:35AM +0200, Katerina Koukiou wrote:
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Domain.xml |  2 +-
>  src/events.c| 17 ++---
>  2 files changed, 3 insertions(+), 16 deletions(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [dbus PATCH] Add detail argument to DomainEvent signal

[Pavel Hrdina]

On Fri, May 04, 2018 at 11:45:59AM +0200, Katerina Koukiou wrote:
> Adjust all DomainEvent tests to do detail type checking.
> 
> Signed-off-by: Katerina Koukiou 
> ---
> This commit is rebased on top of unmerged patches for removing enum<->string
> translation.
> 
>  data/org.libvirt.Connect.xml |  1 +
>  src/events.c |  4 ++--
>  tests/libvirttest.py | 55 
> 
>  tests/test_connect.py|  6 +++--
>  tests/test_domain.py | 15 
>  5 files changed, 72 insertions(+), 9 deletions(-)

Reviewed-by: Pavel Hrdina 

Similar thing should be done for Domain.State property.


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

Re: [libvirt] [PATCH REBASE 4/5] qemu: fix domain object wait to handle monitor errors

[John Ferlan]

On 05/03/2018 03:54 AM, Nikolay Shirokovskiy wrote:
> 
> 
> On 01.05.2018 01:03, John Ferlan wrote:
>>
>>
>> On 04/18/2018 10:44 AM, Nikolay Shirokovskiy wrote:
>>> Block job abort operation can not handle properly qemu crashes
>>> when waiting for abort/pivot completion. Deadlock scenario is next:
>>>
>>> - qemuDomainBlockJobAbort waits for pivot/abort completion
>>> - qemu crashes, then qemuProcessBeginStopJob broadcasts for VM condition and
>>>   then waits for job condition (taken by qemuDomainBlockJobAbort)
>>> - qemuDomainBlockJobAbort awakes but nothing really changed, VM is still
>>>   active (vm->def->id != -1) so thread starts waiting for completion again.
>>>   Now two threads are in deadlock.
>>>
>>> First let's add some condition besides domain active status so that waiting
>>> thread can check it when awakes. Second let's move signalling to the place
>>> where condition is set, namely monitor eof/error handlers. Having signalling
>>> in qemuProcessBeginStopJob is not useful.
>>>
>>> The patch copies monitor error to domain state because at time
>>> waiting thread awakes there can be no monitor and it is useful to
>>> report monitor error to user.
>>>
>>> The patch has a drawback that on destroying a domain when waiting for
>>> event from monitor we get not very convinient error message like
>>
>> convenient
>>
>>> 'EOF from monitor' from waiting API. On the other hand if qemu crashes
>>> this is more useful then 'domain is not running'. The first case
>>> will be addressed in another patch.
>>>
>>> The patch also fixes other places where we wait for event from qemu.
>>> Namely handling device removal and tray ejection. Other places which
>>> used virDomainObjWait (dump, migration, save) were good because of
>>> async jobs which allow concurrent destroy job.
>>>
>>> Signed-off-by: Nikolay Shirokovskiy 
>>> ---
>>>  src/conf/domain_conf.c| 43 ---
>>>  src/conf/domain_conf.h|  3 ---
>>>  src/libvirt_private.syms  |  2 --
>>>  src/qemu/qemu_domain.c| 45 
>>> +
>>>  src/qemu/qemu_domain.h|  5 -
>>>  src/qemu/qemu_driver.c|  6 +++---
>>>  src/qemu/qemu_hotplug.c   |  4 ++--
>>>  src/qemu/qemu_migration.c | 12 ++--
>>>  src/qemu/qemu_process.c   | 27 ++-
>>>  9 files changed, 82 insertions(+), 65 deletions(-)
>>>
>>
>> This no longer git am -3 applies and based on previous patch reviews, I
>> think perhaps this needs to be redone.
> 
> I'll resend as soon as we come to agreement on series. Now I'm not
> convinced to change much (except for using distinct flag to indicate error
> as mentioned in 2nd patch discussion, then I don't need 3d patch).
> 
> See comments below.
> 
>>
>> I don't believe moving/renaming virDomainObjWait is good/right, but I'm
>> sure there would be other opinions on that.  Yes, QEMU is currently the
>> only consumer... If it were to move, it should move to virdomainobjlist
>> since that's where innards of virDomainObjPtr are managed. The fact that
>> we look at ->parent.lock, well, ...
> 
> I would not move the function without reason. It gets new name 
> qemuDomainObjWait
> becase it use qemu specific data, namely monError.

Today only qemu uses this generic virDomainObjWait which is generic
without the need to have/use qemu specific things. Other domain
consumers could use it if they chose.

I'm not in favor of moving, renaming, repurposing for a very specific
issue for what is a generically used function. Maybe that's just me - so
you could try to get a different reviewer if you want.


> 
>>
>> Anyway, you're attempting to special case something and perhaps you just
>> need to create a qemuDomainObjWait that would call the timeout version
>> of the virDomainObjWait and be able to handle whether some other error
>> occurred.  Wouldn't the LastError before the current wait return NULL
>> (as in no error), then during the timeout period if LastError is
>> something couldn't that indicate the failure you're looking for.
> 
> I introduced qemuDomainObjWait not to put virDomainObjWait and 
> virDomainObjWaitUntil
> in the first place but to check monError. Then rather then keeping too 
> functions
> it's look more simple to use only one and branch on given timeout.
> 

I would think if the goal was to have specific code for a specific
purpose for specific functions, then introduce the qemuDomainObjWait,
but have it call virDomainObjWait[Until] based on the need you have.
Which in this case appears to fiddle with monError in some way.

Again - that's just my opinion

John

>>
>> I didn't spend a lot of time thinking about alternatives and how the
>> code should change, but when you mention the patch has a drawback - that
>> immediately raises concern.
> 
> But ... I addressed this issue in next patch as I wrote.
> 


[...]

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

Re: [libvirt] [PATCH REBASE 5/5] qemu: fix races in beingDestroyed usage

[John Ferlan]

On 05/03/2018 05:26 AM, Nikolay Shirokovskiy wrote:
> 
> 
> On 01.05.2018 01:03, John Ferlan wrote:
>>
>>
>> On 04/18/2018 10:44 AM, Nikolay Shirokovskiy wrote:
>>> Clearing beingDestroyed right after acquiring job condition is racy.
>>> It is not known when EOF will be delivired. Let's keep this flag
>>
>> delivered
>>
>>> set. This makes possible to make a clear distinction between monitor
>>> errors/eofs and domain being destroyed in qemuDomainObjWait.
>>>
>>> Also let's move setting destroyed flag out of qemuProcessBeginStopJob
>>> as the function is called from eof handler too.
>>>
>>> Signed-off-by: Nikolay Shirokovskiy 
>>> ---
>>>  src/qemu/qemu_domain.c  |  4 ++--
>>>  src/qemu/qemu_domain.h  |  2 +-
>>>  src/qemu/qemu_driver.c  |  8 +++-
>>>  src/qemu/qemu_process.c | 24 
>>>  4 files changed, 22 insertions(+), 16 deletions(-)
>>>
>>
>> This one didn't git am -3 apply as well, but I see you're changing
>> DomainObjWait so that makes sense as to why.
>>
>> I do recall looking at this code at one time, but I cannot recall my
>> exact conclusion given how qemuDomainObjBeginJobInternal allows the
>> QEMU_JOB_DESTROY to be run, but then waits for whatever is taking place
>> now to finish.
>>
>>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>>> index 1f40ff1..431901c 100644
>>> --- a/src/qemu/qemu_domain.c
>>> +++ b/src/qemu/qemu_domain.c
>>> @@ -11913,9 +11913,9 @@ qemuDomainObjWait(virDomainObjPtr vm, unsigned long 
>>> long until)
>>>  return -1;
>>>  }
>>>  
>>> -if (!virDomainObjIsActive(vm)) {
>>> +if (priv->destroyed) {
>>>  virReportError(VIR_ERR_OPERATION_FAILED, "%s",
>>> -   _("domain is not running"));
>>> +   _("domain is destroyed"));
>>>  return -1;
>>>  }
>>>  
>>> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
>>> index 494ed35..69112ea 100644
>>> --- a/src/qemu/qemu_domain.h
>>> +++ b/src/qemu/qemu_domain.h
>>> @@ -269,7 +269,7 @@ struct _qemuDomainObjPrivate {
>>>  bool agentError;
>>>  
>>>  bool gotShutdown;
>>> -bool beingDestroyed;
>>> +bool destroyed;
>>>  char *pidfile;
>>>  
>>>  virDomainPCIAddressSetPtr pciaddrs;
>>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>>> index 03969d8..4356c0d 100644
>>> --- a/src/qemu/qemu_driver.c
>>> +++ b/src/qemu/qemu_driver.c
>>> @@ -2227,7 +2227,13 @@ qemuDomainDestroyFlags(virDomainPtr dom,
>>>  state = virDomainObjGetState(vm, &reason);
>>>  starting = (state == VIR_DOMAIN_PAUSED &&
>>>  reason == VIR_DOMAIN_PAUSED_STARTING_UP &&
>>> -!priv->beingDestroyed);
>>> +!priv->destroyed);
>>> +
>>> +/* We need to prevent monitor EOF callback from doing our work (and
>>> + * sending misleading events) while the vm is unlocked inside
>>> + * BeginJob/ProcessKill API
>>> + */
>>> +priv->destroyed = true;
>>
>> would this be the right place anyway?  especially since you don't clear
>> it in error paths after setting it.  Once the job starts and we either
>> goto cleanup or endjob on failure - how does a future call distinguish?
> 
> We send SIGTERM/SIGKILL right away after this flag is set so even if
> this API fails eventually keeping destroyed flag set looks good because we
> send signal to qemu and good chances are qemu will die because of that.
> 
> I guess it will be nicer then to move setting the flag to 
> qemuProcessBeginStopJob
> function to keep setting the flag and killing domain together.
> 
> Anyway we can clear the flag on failure too.
> 
>>
>> Not sure this works conceptually.  At least with the current model if
>> DestroyFlags finally gets the job, it checks the domain active state,
>> and goes to endjob after printing a message.  So if while waiting, EOF
>> occurred, there's no qemuProcessStop
> 
> Not true. After sending signal to qemu to terminate EOF will occur but
> handler will return right away because of beingDestroyed/destroyed flag
> check and then after destroyFlags gets the job it will call qemuProcessStop.
> 
> This is not the place I'm addressing with the patch. It is rather if 
> destroyFlags
> is called when we are migrating for example then the interrupted
> API call can report 'domain is not active'/'some monitor error' rather 
> then much nicer 'domain is destroyed' without this patch. See the
> scenario below.
> 
>>
>> Perhaps the only thing I recall wondering is whether we clear
>> beingDestroyed too soon... If we're successful, we're still destroying
>> but not until destroy is successful should the flag then be cleared
>> while still holding the job.
> 
> It does not matter if we clear the flag at the begin or the end of time
> we holding the job because during the job nobody else who needs the job
> too can progress.
> 
> I propose to prolong setting the flag after destroy job is finished (and
> thus rename flag too). Consider next scenario:
> 
> - migrate is

Re: [libvirt] [PATCH 0/8] vfio-ccw passthrough support

[John Ferlan]

On 04/26/2018 03:59 AM, Bjoern Walk wrote:
> Shalini Chellathurai Saroja  [2018-04-11, 05:49PM 
> +0200]:
>> Let us support the basic channel I/O passthrough infrastructure based on
>> vfio, which have been introduced in QEMU 2.10. The current focus is to
>> support dasd-eckd (cu_type/dev_type = 0x3990/0x3390) as the target
>> device.
>>
>> Shalini Chellathurai Saroja (8):
>>   qemu: introduce capability for virtual-css-bridge
>>   qemu: introduce vfio-ccw capability
>>   util: virhostdev: add virHostdevIsMdevDevice()
>>   qemu: vfio-ccw device address generation
>>   qemu: command line generation for vfio-ccw device
>>   tests: tests for vfio-ccw passthrough
>>   docs: documentation for vfio-ccw passthrough
>>   news: documentation of new feature
> 
> Any chance, we get at least a review before 4.3 hits? Would be
> appreciated.
> 
> 

So obviously this did not make the 4.3.0 and the series will need a
refresh due to the volume of change in qemu_capabilities.{c,h}.

I've reviewed a number of patches recently and made a similar comment in
all of them - when changing qemu_capabilities.{c,h} and updating the
various qemucapabilitiesdata/caps_*.xml files - do so in a separate
patch. That way if someone doesn't review the code right away, it's
actually fairly simple to recreate at least the capability for a
reviewer. Having it mixed in one patch with other qemu, conf, test, etc.
changes causes git am -3 to fail and thus makes review harder especially
when you don't get to reviews as soon as patches hit the list.

Someone may also want to consider creating a s390 specific version of
what Peter did for x86_64 for VIR_TEST_CAPS_LATEST in order to then
have/use the "latest" capabilities instead of adding bits to xml2argv
tests. I'm curious why the xml2xml test needed the bit adjustment - did
something fail?  Since there were no xml output data changes, that would
seem to indicate there isn't a need to modify the xml2xml test source.

John

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

Re: [libvirt] [PATCH] lxc: convert to typesafe virConf accessors in lxc_native.c

[Cedric Bosdonnat]

On Fri, 2018-05-04 at 18:46 +0530, Prafull wrote:
> From: Prafullkumar Tale 
> 
> Signed-off-by: Prafullkumar Tale 
> ---
>  src/lxc/lxc_native.c | 141 
> +--
>  1 file changed, 70 insertions(+), 71 deletions(-)
> 
> diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
> index 55ea774..35077e1 100644
> --- a/src/lxc/lxc_native.c
> +++ b/src/lxc/lxc_native.c
> @@ -199,19 +199,15 @@ lxcSetRootfs(virDomainDefPtr def,
>   virConfPtr properties)
>  {
>  int type = VIR_DOMAIN_FS_TYPE_MOUNT;
> -virConfValuePtr value;
> +char *value = NULL;
>  
> -if (!(value = virConfGetValue(properties, "lxc.rootfs")) ||
> -!value->str) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -   _("Missing lxc.rootfs configuration"));
> +if (virConfGetValueString(properties, "lxc.rootfs", &value) <= 0)
>  return -1;
> -}
>  
> -if (STRPREFIX(value->str, "/dev/"))
> +if (STRPREFIX(value, "/dev/"))
>  type = VIR_DOMAIN_FS_TYPE_BLOCK;
>  
> -if (lxcAddFSDef(def, type, value->str, "/", false, 0) < 0)
> +if (lxcAddFSDef(def, type, value, "/", false, 0) < 0)
>  return -1;
>  
>  return 0;
> @@ -684,17 +680,17 @@ lxcConvertNetworkSettings(virDomainDefPtr def, 
> virConfPtr properties)
>  static int
>  lxcCreateConsoles(virDomainDefPtr def, virConfPtr properties)
>  {
> -virConfValuePtr value;
> +char *value = NULL;
>  int nbttys = 0;
>  virDomainChrDefPtr console;
>  size_t i;
>  
> -if (!(value = virConfGetValue(properties, "lxc.tty")) || !value->str)
> +if (virConfGetValueString(properties, "lxc.tty", &value) <= 0)
>  return 0;
>  
> -if (virStrToLong_i(value->str, NULL, 10, &nbttys) < 0) {
> +if (virStrToLong_i(value, NULL, 10, &nbttys) < 0) {
>  virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to parse int: 
> '%s'"),
> -   value->str);
> +   value);
>  return -1;
>  }
>  
> @@ -761,89 +757,91 @@ lxcIdmapWalkCallback(const char *name, virConfValuePtr 
> value, void *data)
>  static int
>  lxcSetMemTune(virDomainDefPtr def, virConfPtr properties)
>  {
> -virConfValuePtr value;
> +char *value = NULL;
>  unsigned long long size = 0;
>  
> -if ((value = virConfGetValue(properties,
> -"lxc.cgroup.memory.limit_in_bytes")) &&
> -value->str && STRNEQ(value->str, "-1")) {
> -if (lxcConvertSize(value->str, &size) < 0)
> -return -1;
> +if (virConfGetValueString(properties,
> +  "lxc.cgroup.memory.limit_in_bytes",
> +  &value) > 0) {
> +if (lxcConvertSize(value, &size) < 0)
> +goto error;
>  size = size / 1024;
>  virDomainDefSetMemoryTotal(def, size);
>  def->mem.hard_limit = virMemoryLimitTruncate(size);
>  }
>  
> -if ((value = virConfGetValue(properties,
> -"lxc.cgroup.memory.soft_limit_in_bytes")) &&
> -value->str && STRNEQ(value->str, "-1")) {
> -if (lxcConvertSize(value->str, &size) < 0)
> -return -1;
> -
> +if (virConfGetValueString(properties,
> +  "lxc.cgroup.memory.soft_limit_in_bytes",
> +  &value) > 0) {
> +if (lxcConvertSize(value, &size) < 0)
> +goto error;
>  def->mem.soft_limit = virMemoryLimitTruncate(size / 1024);
>  }
>  
> -if ((value = virConfGetValue(properties,
> -"lxc.cgroup.memory.memsw.limit_in_bytes")) &&
> -value->str && STRNEQ(value->str, "-1")) {
> -if (lxcConvertSize(value->str, &size) < 0)
> -return -1;
> -
> +if (virConfGetValueString(properties,
> +  "lxc.cgroup.memory.memsw.limit_in_bytes",
> +  &value) > 0) {
> +if (lxcConvertSize(value, &size) < 0)
> +goto error;
>  def->mem.swap_hard_limit = virMemoryLimitTruncate(size / 1024);
>  }
>  return 0;
> +
> + error:
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("failed to parse integer: '%s'"), value);
> +return -1;
> +
>  }
>  
>  static int
>  lxcSetCpuTune(virDomainDefPtr def, virConfPtr properties)
>  {
> -virConfValuePtr value;
> +char *value = NULL;
>  
> -if ((value = virConfGetValue(properties, "lxc.cgroup.cpu.shares")) &&
> -value->str) {
> -if (virStrToLong_ull(value->str, NULL, 10, &def->cputune.shares) < 0)
> +if (virConfGetValueString(properties, "lxc.cgroup.cpu.shares",
> +  &value) > 0) {
> +if (virStrToLong_ull(value, NULL, 10, &def->cputune.shares) < 0)
>  goto error;
>  def->cputune.sharesSpecified = true;
>  }
>  
> -if ((value = virConfGetValue(properties,
> - "l

Re: [libvirt] Expose vfio device display/migration to libvirt and above, was Re: [PATCH 0/3] sample: vfio mdev display devices.

[Alex Williamson]

On Fri, 4 May 2018 09:49:44 +0200
Erik Skultety  wrote:

> On Thu, May 03, 2018 at 12:58:00PM -0600, Alex Williamson wrote:
> > Hi,
> >
> > The previous discussion hasn't produced results, so let's start over.
> > Here's the situation:
> >
> >  - We currently have kernel and QEMU support for the QEMU vfio-pci
> >display option.
> >
> >  - The default for this option is 'auto', so the device will attempt to
> >generate a display if the underlying device supports it, currently
> >only GVTg and some future release of NVIDIA vGPU (plus Gerd's
> >sample mdpy and mbochs).
> >
> >  - The display option is implemented via two different mechanism, a
> >vfio region (NVIDIA, mdpy) or a dma-buf (GVTg, mbochs).
> >
> >  - Displays using dma-buf require OpenGL support, displays making
> >use of region support do not.
> >
> >  - Enabling OpenGL support requires specific VM configurations, which
> >libvirt /may/ want to facilitate.
> >
> >  - Probing display support for a given device is complicated by the
> >fact that GVTg and NVIDIA both impose requirements on the process
> >opening the device file descriptor through the vfio API:
> >
> >- GVTg requires a KVM association or will fail to allow the device
> >  to be opened.  
> 
> How exactly is this association checked?

The intel_vgpu_open() callback for the mdev device registers a vfio
group notifier for VFIO_GROUP_NOTIFY_SET_KVM events. The KVM pointer is
already registered via the addition of the vfio group to the vfio-kvm
pseudo device, so the registration synchronously triggers the notifier
callback and the result is tested slightly later in the open path in
kvmgt_guest_init().
 
> >
> >- NVIDIA requires that their vgpu-manager process can locate a
> > UUID for the VM via the process commandline.
> >
> >- These are both horrible impositions and prevent libvirt from
> >  simply probing the device itself.  
> 
> So I feel like we're trying to solve a problem coming from one layer
> on a bunch of different layers which inherently prevents us to
> produce a viable long term solution without dragging a significant
> amount of hacky nasty code and it is not the missing sysfs attributes
> I have in mind. Why does NVIDIA's vgpu-manager need to locate a UUID
> of a qemu VM? I assume that's to prevent multiple VM instances trying
> to use the same mdev device, in which case can't the vgpu-manager
> track references to how many "open" and "close" calls have been made

Hard to say, NVIDIA hasn't been terribly forthcoming about this
requirement, but probably not multiple users of the same mdev device
as that's already prevented through vfio in general.  Intel has
discussed that their requirement is to be able to track VM page table
updates so they can update their shadow tables, so effectively rather
than mediating interactions directly with the device, they're using a
KVM back channel to manage the DMA translation address space for the
device.

The flip side is that while these requirements are annoying and hard
for non-VM users to deal with, is there a next logical point in the
interaction with the vfio device where the vendor driver can reasonably
impose those requirements?  For instance, both vendors expose a
vfio-pci interface, so they could prevent the user driver from enabling
bus master in the PCI command register, but that's a fairly subtle
failure, typically drivers wouldn't even bother to read back after a
write to the bus master bit to see if it sticks and this sort of
enabling is done by the guest, not the hypervisor.  There's really no
error path for a write to the device.

> to the same device? This is just from a layman's perspective, but it
> would allow the following:
> - when libvirt starts, it initializes all its drivers (let's
> focus on QEMU)
> - as part of this initialization, libvirt probes QEMU for
> capabilities and caches them in order to use them when spawning VMs
> 
> Now, if we (theoretically) can settle on easing the restrictions Alex
> has mentioned, we in fact could introduce a QMP command to probe
> these devices and provide libvirt with useful information at that
> point in time. Of course, since the 3rd party vendor is "de-coupled"
> from qemu, libvirt would have no way to find out that the driver has
> changed in the meantime, thus still using the old information we
> gathered, ergo potentially causing the QEMU process to fail
> eventually. But then again, there's very often a strong
> recommendation to reboot your host after a driver update, especially
> in NVIDIA's case, which means this fact wouldn't matter. However,
> there's also a significant drawback to my proposal which probably
> renders it completely useless (but we can continue from there...) and
> that is the devices would either have to be present already (not an
> option) or QEMU would need to be enhanced in a way, that it would
> create a dummy device during QMP probing, open it, collect the
> information libvirt 

Re: [libvirt] [PATCH 0/5] Another batch of old capability cleanups

[John Ferlan]

On 05/03/2018 06:35 AM, Ján Tomko wrote:
> Ján Tomko (5):
>   qemu: remove qemuBuildObsoleteAccelArg
>   qemuBuildMachineCommandLine: use a switch for virDomainVirtType
>   Deprecate QEMU_CAPS_NO_KVM_PIT
>   Depreacte QEMU_CAPS_TDF
>   Deprecate QEMU_CAPS_NESTING
> 
>  src/qemu/qemu_capabilities.c   |   6 +-
>  src/qemu/qemu_capabilities.h   |   6 +-
>  src/qemu/qemu_command.c| 101 
> +++--
>  tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml   |   1 -
>  tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml   |   1 -
>  tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml   |   1 -
>  tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml   |   1 -
>  tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml  |   1 -
>  tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |   1 -
>  tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml   |   1 -
>  tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml   |   1 -
>  tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml   |   1 -
>  tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml   |   1 -
>  tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml   |   1 -
>  tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml   |   1 -
>  tests/qemuxml2argvdata/clock-hpet-off.args |   1 -
>  tests/qemuxml2argvdata/hugepages-numa.args |   1 -
>  tests/qemuxml2argvdata/no-kvm-pit-device.args  |  28 --
>  tests/qemuxml2argvdata/no-kvm-pit-device.xml   |  29 --
>  tests/qemuxml2argvdata/q35-virt-manager-basic.args |   1 -
>  tests/qemuxml2argvtest.c   |   3 +-
>  tests/qemuxml2xmltest.c|   1 -
>  22 files changed, 40 insertions(+), 149 deletions(-)
>  delete mode 100644 tests/qemuxml2argvdata/no-kvm-pit-device.args
>  delete mode 100644 tests/qemuxml2argvdata/no-kvm-pit-device.xml
> 

After fixing the 2 issues noted in patch 4 and 5,

(series)

Reviewed-by: John Ferlan 

John

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

Re: [libvirt] [PATCH 4/5] Depreacte QEMU_CAPS_TDF

[John Ferlan]

$subj:  Deprecate



On 05/03/2018 06:35 AM, Ján Tomko wrote:
> This capability is unused since we stopped parsing -help output.
> 
> Signed-off-by: Ján Tomko 
> ---
>  src/qemu/qemu_capabilities.h | 2 +-
>  src/qemu/qemu_command.c  | 5 +
>  2 files changed, 2 insertions(+), 5 deletions(-)
> 

[...]

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

Re: [libvirt] [PATCH 5/5] Deprecate QEMU_CAPS_NESTING

[John Ferlan]

On 05/03/2018 06:35 AM, Ján Tomko wrote:
> Unused since commit .
> 
> Signed-off-by: Ján Tomko 
> ---
>  src/qemu/qemu_capabilities.h | 2 +-
>  src/qemu/qemu_command.c  | 3 ---
>  2 files changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 0cf7f12230..07dd9ff4ec 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -105,7 +105,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
> syntax-check */
>  
>  /* 40 */
>  X_QEMU_CAPS_FSDEV, /* -fstype filesystem passthrough */
> -QEMU_CAPS_NESTING, /* -enable-nesting (SVM/VMX) */
> +X_QEMU_CAPS_NESTING, /* -enable-nesting (SVM/VMX) */
>  X_QEMU_CAPS_NAME_PROCESS, /* Is -name process= available */
>  X_QEMU_CAPS_DRIVE_READONLY, /* -drive readonly=on|off */
>  X_QEMU_CAPS_SMBIOS_TYPE, /* Is -smbios type= available */
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 099265da17..e8f5f97fa1 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6870,9 +6870,6 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
>  if (cpu) {
>  virCommandAddArg(cmd, "-cpu");
>  virCommandAddArgFormat(cmd, "%s%s", cpu, cpu_flags ? cpu_flags : "");
> -
> -if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NESTING) && hasHwVirt)

Did you clip too much here?   hasHWVirt is defined and set, but not used
according to my compiler

John
> -virCommandAddArg(cmd, "-enable-nesting");
>  }
>  
>  ret = 0;
> 

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

Re: [libvirt] [PATCH] rpc: fixing compilation error due to deprecated ssh_get_publickey().

[John Ferlan]

On 05/01/2018 12:21 PM, Julio Faracco wrote:
> After 0.7.5 release, libssh deprecated ssh_get_publickey() method to
> introduce ssh_get_server_publickey(). This commit check the current
> version of libssh and use the proper method during the compilation.
> See the error:
> 
> make[3]: Entering directory '/home/julio/Desktop/virt/libvirt/src'
>   CC   rpc/libvirt_net_rpc_la-virnetlibsshsession.lo
> rpc/virnetlibsshsession.c:217:9: error: 'ssh_get_publickey' is deprecated 
> [-Werror,-Wdeprecated-declarations]
> if (ssh_get_publickey(sess->session, &key) != SSH_OK) {
> ^
> /usr/include/libssh/libssh.h:489:1: note: 'ssh_get_publickey' has been 
> explicitly marked deprecated here
> SSH_DEPRECATED LIBSSH_API int ssh_get_publickey(ssh_session session, ssh_key 
> *key);
> ^
> /usr/include/libssh/libssh.h:99:40: note: expanded from macro 'SSH_DEPRECATED'
>^
> 1 error generated.
> Makefile:8604: recipe for target 
> 'rpc/libvirt_net_rpc_la-virnetlibsshsession.lo' failed
> 
> Signed-off-by: Julio Faracco 
> ---
>  src/rpc/virnetlibsshsession.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/rpc/virnetlibsshsession.c b/src/rpc/virnetlibsshsession.c
> index 309e8a9340..96c5bc0882 100644
> --- a/src/rpc/virnetlibsshsession.c
> +++ b/src/rpc/virnetlibsshsession.c
> @@ -214,7 +214,11 @@ virLibsshServerKeyAsString(virNetLibsshSessionPtr sess)
>  size_t keyhashlen;
>  char *str;
>  
> +#if LIBSSH_VERSION_INT > 0x0705 /* 0.7.5 */
> +if (ssh_get_server_publickey(sess->session, &key) != SSH_OK) {
> +#else
>  if (ssh_get_publickey(sess->session, &key) != SSH_OK) {
> +#endif

Usually this involves changes to some m4/* file that would do the
version check and existence of some API and set a WITH_xxx or HAVE_xxx
type conditional which would then be used.

I typically try to avoid m4/* files and build stuff, so not my area of
expertise, but m4/virt-libssh.m4 is perhaps where you should start.
Perhaps usage of AC_CHECK_FUNCS

Look up HAVE_GNUTLS_CIPHER_ENCRYPT and how m4/virt-gnutls.m4 will check
for gnutls_cipher_encrypt for at least one example that comes to my mind.

John

>  virReportError(VIR_ERR_LIBSSH, "%s",
> _("failed to get the key of the current "
>   "session"));
> 

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

Re: [libvirt] [PATCH] rpc: fixing compilation error due to deprecated ssh_get_publickey().

[Daniel P . Berrangé]

On Fri, May 04, 2018 at 12:53:14PM -0400, John Ferlan wrote:
> 
> 
> On 05/01/2018 12:21 PM, Julio Faracco wrote:
> > After 0.7.5 release, libssh deprecated ssh_get_publickey() method to
> > introduce ssh_get_server_publickey(). This commit check the current
> > version of libssh and use the proper method during the compilation.
> > See the error:
> > 
> > make[3]: Entering directory '/home/julio/Desktop/virt/libvirt/src'
> >   CC   rpc/libvirt_net_rpc_la-virnetlibsshsession.lo
> > rpc/virnetlibsshsession.c:217:9: error: 'ssh_get_publickey' is deprecated 
> > [-Werror,-Wdeprecated-declarations]
> > if (ssh_get_publickey(sess->session, &key) != SSH_OK) {
> > ^
> > /usr/include/libssh/libssh.h:489:1: note: 'ssh_get_publickey' has been 
> > explicitly marked deprecated here
> > SSH_DEPRECATED LIBSSH_API int ssh_get_publickey(ssh_session session, 
> > ssh_key *key);
> > ^
> > /usr/include/libssh/libssh.h:99:40: note: expanded from macro 
> > 'SSH_DEPRECATED'
> >^
> > 1 error generated.
> > Makefile:8604: recipe for target 
> > 'rpc/libvirt_net_rpc_la-virnetlibsshsession.lo' failed
> > 
> > Signed-off-by: Julio Faracco 
> > ---
> >  src/rpc/virnetlibsshsession.c | 4 
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/src/rpc/virnetlibsshsession.c b/src/rpc/virnetlibsshsession.c
> > index 309e8a9340..96c5bc0882 100644
> > --- a/src/rpc/virnetlibsshsession.c
> > +++ b/src/rpc/virnetlibsshsession.c
> > @@ -214,7 +214,11 @@ virLibsshServerKeyAsString(virNetLibsshSessionPtr sess)
> >  size_t keyhashlen;
> >  char *str;
> >  
> > +#if LIBSSH_VERSION_INT > 0x0705 /* 0.7.5 */
> > +if (ssh_get_server_publickey(sess->session, &key) != SSH_OK) {
> > +#else
> >  if (ssh_get_publickey(sess->session, &key) != SSH_OK) {
> > +#endif
> 
> Usually this involves changes to some m4/* file that would do the
> version check and existence of some API and set a WITH_xxx or HAVE_xxx
> type conditional which would then be used.

That all depends on whether we care about handling the possibility of
distros backporting the function to older versions of libssh. This
kind of backporting happens alot for some projects, like QEMU, but
not for others. IMHO this check against LIBSSH_VERSION_INT is fine.

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] Expose vfio device display/migration to libvirt and above, was Re: [PATCH 0/3] sample: vfio mdev display devices.

[Alex Williamson]

On Fri, 4 May 2018 10:16:09 +0100
Daniel P. Berrangé  wrote:

> On Thu, May 03, 2018 at 12:58:00PM -0600, Alex Williamson wrote:
> > Hi,
> > 
> > The previous discussion hasn't produced results, so let's start over.
> > Here's the situation:
> > 
> >  - We currently have kernel and QEMU support for the QEMU vfio-pci
> >display option.
> > 
> >  - The default for this option is 'auto', so the device will attempt to
> >generate a display if the underlying device supports it, currently
> >only GVTg and some future release of NVIDIA vGPU (plus Gerd's
> >sample mdpy and mbochs).
> > 
> >  - The display option is implemented via two different mechanism, a
> >vfio region (NVIDIA, mdpy) or a dma-buf (GVTg, mbochs).
> > 
> >  - Displays using dma-buf require OpenGL support, displays making
> >use of region support do not.
> > 
> >  - Enabling OpenGL support requires specific VM configurations, which
> >libvirt /may/ want to facilitate.
> > 
> >  - Probing display support for a given device is complicated by the
> >fact that GVTg and NVIDIA both impose requirements on the process
> >opening the device file descriptor through the vfio API:
> > 
> >- GVTg requires a KVM association or will fail to allow the device
> >  to be opened.
> > 
> >- NVIDIA requires that their vgpu-manager process can locate a UUID
> >  for the VM via the process commandline.
> > 
> >- These are both horrible impositions and prevent libvirt from
> >  simply probing the device itself.  
> 
> Agreed, these requirements are just horrific. Probing for features
> should not require this kind of level environmental setup. I can
> just about understand & accept how we ended up here, because this
> scenario is not one that was strongly considered when the first impls
> were being done. I don't think we should accept it as a long term
> requirement though.
> 
> > Erik Skultety, who initially raised the display question, has identified
> > one possible solution, which is to simply make the display configuration
> > the user's problem (apologies if I've misinterpreted Erik).  I believe
> > this would work something like:
> > 
> >  - libvirt identifies a version of QEMU that includes 'display' support
> >for vfio-pci devices and defaults to adding display=off for every
> >vfio-pci device [have we chosen the wrong default (auto) in QEMU?].
> > 
> >  - New XML support would allow a user to enable display support on the
> >vfio device.
> > 
> >  - Resolving any OpenGL dependencies of that change would be left to
> >the user.
> > 
> > A nice aspect of this is that policy decisions are left to the user and
> > clearly no interface changes are necessary, perhaps with the exception
> > of deciding whether we've made the wrong default choice for vfio-pci
> > devices in QEMU.  
> 
> Unless I'm mis-understanding this isn't really a solution to the
> problem, rather it is us simply giving up and telling someone else
> to try to fix the problem. The 'user' here is not a human - it is
> simply the next level up in the mgmt stack, eg OpenStack or oVirt.
> If we can't solve it acceptably in libvirt code, I don't have much
> hope that OpenStack can solve it in their code, since they have
> even stronger need to automate everything.

But to solve this at any level other than the user suggests there is
one "right" answer to automatically configuring the device.  Is there?
If a device supports a display, does the user necessarily want to
enable it?  If there's a difference between enabling a display for a
local user or a remote user, is there any reasonable expectation that
we can automatically make that determination?

> > On the other hand, if we do want to give libvirt a mechanism to probe
> > the display support for a device, we can make a simplified QEMU
> > instance be the mechanism through which we do that.  For example the
> > script[1] can be provided with either a PCI device or sysfs path to an
> > mdev device and run a minimal VM instance meeting the requirements of
> > both GVTg and NVIDIA to report the display support and GL requirements
> > for a device.  There are clearly some unrefined and atrocious bits of
> > this script, but it's only a proof of concept, the process management
> > can be improved and we can decide whether we want to provide qmp
> > mechanism to introspect the device rather than grep'ing error
> > messages.  The goal is simply to show that we could choose to embrace
> > QEMU and use it not as a VM, but simply a tool for poking at a device
> > given the restrictions the mdev vendor drivers have already imposed.  
> 
> Feels like a pretty heavy weight solution, that just encourages the
> drivers to continue down the undesirable path they're already on,
> possibly making the situation even worse over time.

I'm not getting the impression that the vendor drivers are considering
a change, or necessarily can change.  The NVIDIA UUID requirement
certainly seems arbitrary