[libvirt] [RFC v2 00/20] qmp: Report bus information on 'query-machines'

[Eduardo Habkost]

Changes v1 -> v2:
* v1 series subject was:
  "qmp: Report supported device types on 'query-machines'"
* Now we return additional bus information: bus ID, bus type,
  and the list of accepted device types on each bus
* Now hotplug can be covered because accepted-device-types can
  be set by individual bus instances if necessary
* Now the new field is optional, and every machine having the
  new field are now validated in "strict mode" on test code
  (meaning all buses must be present on the list)
  * TODO: only PC was converted, machines from other
architectures don't include the field yet
* Legacy-PCI vs PCIe is now handled:
  * PCIDeviceClass::is_express field was removed
  * Defined INTERFACE_LEGACY_PCI_DEVICE and INTERFACE_PCIE_DEVICE,
buses and devices now report appropriate interface names
  * Now PCIe buses can return the right information
because each bus instance can set its own
accepted-device-types list
  * TODO: PCIe pci-bridge classes need to be changed to not include
INTERFACE_LEGACY_PCI_DEVICE
  * TODO: replace q35 hack with appropriate code that set
Legacy-PCI rules in PCI code
* Removed patches from v1:
  * pc: Initialize default bus lists
  * s390x: Initialize default bus lists
  * arm: Initialize default bus lists
  * mips: Initialize default bus lists
  * ppc: Initialize default bus lists
  * qdev: Add device_class_set_bus_type() function
(validation is more difficult with the PCI/PCIe rules, plan
is to try to remove DeviceClass::bus_type field, se
"Limitations" below)
* See individual patches for additional details

The Problem
===

Currently management software has no way to find out which device
types can be plugged in a machine, unless the machine is already
initialized.

Even after the machine is initialized, there's no way to map
existing bus types to supported device types unless management
software hardcodes the mapping between bus types and device
types.

Example: floppy support on q35 vs i440fx


There's no way for libvirt to find out that there's no floppy
controller on pc-q35-* machine-types by default.

With this series, pc-i440fx-* will report "floppy" as a supported
device type, but pc-q35-* will not.

Example: Legacy PCI vs vs PCIe devices
--

Some devices require a PCIe bus to be available, others work on
both legacy PCI and PCIe, while others work only on a legacy PCI
bus.

Currently management software has no way to know which devices
can be added to a given machine, unless it hardcodes machine-type
names and device-types names.

Example: spapr and PCIe root bus


See the thread at:
  Subject: [RFC PATCH qemu] spapr_pci: Create PCI-express root bus by default

If we make new spapr machine-type versions create a PCIe root
bus, management software will need a way to find out:

1) The type of the default bus for the machine type;
2) The ID of the default bus for the machine type.

Otherwise, management software will have to hardcode it based on
machine-type version. The proposed interface should solve this
problem.

The Proposed Interface
==

Bus info on query-machines
--

This series adds a new field to the output of 'query-machines':
'always-available-buses'. It will contain a a list of
MachineBusInfo structs. MachineBusInfo will contain:
* bus-id: The bus ID
* bus-type: The bus type
* accepted-device-types: A list of accepted device types for the bus.

bus-id can be used to find out what's the right "bus" argument to
be used when adding a device to the machine configuration (e.g.
when using -device and device_add).

accepted-device-types can be used as the 'implements' argument on
the 'qom-list-types' command, to find out which device types can
be plugged on the bus.

accepted-device-types property on bus objects
-

This series also adds a 'accepted-device-types' property to bus
objects, so management software can check which kinds of devices
can be plugged at runtime.

Example output
--

TODO: update it.

Considered alternatives
===

Indirect mapping (machine => bus => device)
---

This RFC implements a mechanism to implement ax
  machine-type => accepted-device-types
mapping. An alternative solution I considered was to expose an
indirect mapping:
  machine-type => default-bus-types
followed by
  bus-type => accepted-device-types.

But some buses have no correct bus-type => accepted-device-types
mapping. PCIe buses, for example, may or may not accept legacy
PCI buses, depending on the machine and  the bus topology.

imposes less restrictions on how the device and bus type
hierarchy is implemented inside QEMU. There's still a
  machine-type => bus-type => device-type
mapping implemented internally, but it is an implementation
detail on the current version, and 

[libvirt] [RFC v2 07/20] qmp: Add 'always-available-buses' field to 'query-machines'

[Eduardo Habkost]

The new field will return a list MachineBusInfo structs,
containing information about the buses that are always created by
the machine (even if -nodefaults is used).

Note that some machine options may enable or disable some bus
types and affect the set of available buses. Introspection of
those options is out of the scope of this patch.

Includes a qtest test case that will validate the returned data
by actually running each machine-type and checking the list of
available buses.

As a TYPE_SYSTEM_BUS bus is always created, add it to the default
list on TYPE_MACHINE.

Cc: libvir-list@redhat.com
Cc: Laine Stump 
Signed-off-by: Eduardo Habkost 
---
Changes series v1 -> v2:
* Replacing patches from v1:
  * machine: Add MachineClass::default_buses field
  * qmp: Add 'supported-device-types' field to 'query-machines'
* Replace 'supported-device-types' string list with
  'always-available-buses' MachineBusInfo list
* Make the new field optional, so "strict mode" will
  be always enabled when the field is present
* Don't include sysbus on TYPE_MACHINE, as not
  all machines will return the field
* Test code changes:
  * Update to use the new 'supported-device-types' field
  * Use unittest.main() instead of custom main() function
* Add QTEST_LOG_LEVEL variable to control logging level
  * Include architecture on test case name
  * Simulate -nodefaults
  * Rewrote machine-type discovery hack
  * Run two test cases for each machine:
using -nodefaults and without -nodefaults
  * Blacklist known machines that won't work with
-nodefaults
  * Enable strict mode only when using -nodefaults
---
 hw/core/machine.c |  33 -
 include/hw/boards.h   |   7 ++
 qapi-schema.json  |  37 +-
 tests/Makefile.include|   2 +
 tests/qmp-machine-info.py | 173 ++
 vl.c  |   6 ++
 6 files changed, 256 insertions(+), 2 deletions(-)
 create mode 100755 tests/qmp-machine-info.py

diff --git a/hw/core/machine.c b/hw/core/machine.c
index b0fd91f..5be1297 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -19,6 +19,7 @@
 #include "sysemu/sysemu.h"
 #include "qemu/error-report.h"
 #include "qemu/cutils.h"
+#include "qapi/clone-visitor.h"
 
 static char *machine_get_accel(Object *obj, Error **errp)
 {
@@ -357,6 +358,32 @@ static void machine_init_notify(Notifier *notifier, void 
*data)
 foreach_dynamic_sysbus_device(error_on_sysbus_device, NULL);
 }
 
+
+/* Add an item to always_available_bus list
+ *
+ * The accepted_device_types field is automatically filled using
+ * BusClass::device_type.
+ */
+MachineBusInfo *machine_class_add_always_available_bus(MachineClass *mc,
+   const char *bus_id,
+   const char *bus_type)
+{
+BusClass *bc = BUS_CLASS(object_class_by_name(bus_type));
+MachineBusInfo *bi = g_new0(MachineBusInfo, 1);
+MachineBusInfoList *bl = g_new0(MachineBusInfoList, 1);
+
+bi->bus_id = g_strdup(bus_id);
+bi->bus_type = g_strdup(bus_type);
+bi->accepted_device_types = g_new0(strList, 1);
+bi->accepted_device_types->value = g_strdup(bc->device_type);
+
+bl->value = bi;
+bl->next = mc->always_available_buses;
+mc->always_available_buses = bl;
+
+return bi;
+}
+
 static void machine_class_init(ObjectClass *oc, void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
@@ -466,13 +493,17 @@ static void machine_class_init(ObjectClass *oc, void 
*data)
 
 static void machine_class_base_init(ObjectClass *oc, void *data)
 {
+MachineClass *mc = MACHINE_CLASS(oc);
+
 if (!object_class_is_abstract(oc)) {
-MachineClass *mc = MACHINE_CLASS(oc);
 const char *cname = object_class_get_name(oc);
 assert(g_str_has_suffix(cname, TYPE_MACHINE_SUFFIX));
 mc->name = g_strndup(cname,
 strlen(cname) - strlen(TYPE_MACHINE_SUFFIX));
 }
+
+mc->always_available_buses =
+QAPI_CLONE(MachineBusInfoList, mc->always_available_buses);
 }
 
 static void machine_initfn(Object *obj)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index a51da9c..915a46d 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -42,6 +42,10 @@ bool machine_dump_guest_core(MachineState *machine);
 bool machine_mem_merge(MachineState *machine);
 void machine_register_compat_props(MachineState *machine);
 
+MachineBusInfo *machine_class_add_always_available_bus(MachineClass *mc,
+   const char *bus_id,
+   const char *bus_type);
+
 /**
  * CPUArchId:
  * @arch_id - architecture-dependent CPU ID of present or possible CPU
@@ -92,6 +96,8 @@ typedef struct {
  *size than the target architecture's minimum. (Attempting to create
  *such a CPU will fail.) Note that changing this is a 

Re: [libvirt] QEMU soundcards vulnerable to jack retasking?

[bancfc]

On 2016-11-24 18:18, Daniel P. Berrange wrote:


This is better sent to the QEMU mailing list, rather than libvirt, 
since

the former is where the QEMU audio experts are...

Regards,
Daniel


OK done. Good call, thanks Dan.

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

Re: [libvirt] Libvirt domain event usage and consistency

[Michal Privoznik]

On 25.11.2016 18:33, Roman Mohr wrote:
> On Fri, Nov 25, 2016 at 6:04 PM, Michal Privoznik 
> wrote:
> 
>> On 25.11.2016 17:54, Roman Mohr wrote:
>>> On Fri, Nov 25, 2016 at 4:34 PM, Michal Privoznik 
>>> wrote:
>>>
 On 25.11.2016 14:38, Roman Mohr wrote:
> Hi,
>
> I recently started to use the libvirt domain events. With them I
>> increase
> the responsiveness of my VM state wachers.
> In general it works pretty well. I just listen to the events and do a
> periodic resync to cope with missed events.
>
> While watching the events I ran into a few interesting situations I
 wanted
> to share. The points 1-3 describe some minor issues or irregularities.
> Point 4 is about the fact that domain and state updates are not
>> versioned
> which makes it very hard to stay in sync with libvirt when using
>> events.
>
> My libvirt version is 1.2.18.4.

 This might be the root cause. I'm unable to see some of the scenarios
 you're seeing. Have you tried the latest release (or even git HEAD) to
 check whether all the scenarios you are describing still stand?

>>>
>>> Definitely better with latest HEAD but still it does not look completely
>>> right.
>>>

>
> 1) Event order seems to be weird on startup:
>
> When listening for VM lifecycle events I get this order:
>
> {"event_type": "Started", "timestamp": "2016-11-25T11:59:53.209326Z",
> "reason": "Booted", "domain_name": "generic", "domain_id":
> "8ff7047b-fb46-44ff-a4c6-7c20c73ab86e"}
> {"event_type": "Defined", "timestamp": "2016-11-25T11:59:53.435530Z",
> "reason": "Added", "domain_name": "generic", "domain_id":
> "8ff7047b-fb46-44ff-a4c6-7c20c73ab86e"}
>
> It is strange that a VM already boots before it is defined. Is this the
> intended order?

 I don't see this order so probable this is fixed upstream.

>>>
>>> On latest master a normal creation emits these events:
>>>
>>> event 'lifecycle' for domain testvm: Resumed Unpaused
>>> event 'lifecycle' for domain testvm: Started Booted
>>>
>>> The Resumed event looks wrong. Further I get no more Defined/Undefined
>>> events. Maybe they were removed?
>>
>> Yes, they were removed.
> 
> 
> Nice
> 
> 
>> The Resumed event comes from qemu actually,
>> because libvirt starts qemu in paused mode so that we can do some setup
>> (e.g. place vcpu threads into cgroups) and only after that we can resume
>> guest CPUs and in fact let guest start. Once this is done we
>> deliberately emit Started event.
>>
> 
> I would expect an event like this:
> 
> event 'lifecycle' for domain testvm: Suspended Bootstrapping
> 
> before the other two events. That takes the ambiguity from the Resumed
> event.
> 
> 
>>>
>>>

>
> 2) Defining a VM with VIR_DOMAIN_START_PAUSED gives me this event order

 I don't think you can define a domain with that flag. What's the actual
 action?

>>>
>>> That is the flag for the api, when using virsh using `--paused` does
>> that.
>>
>> Ah, that's for virsh create/start not virsh define. Anyway, this is no
>> longer the case with upstream, is it?
>>
>>
> Right
> 
> 
>>>
>>>

>
> {"event_type": "Defined", "timestamp": "2016-11-25T12:02:44.037817Z",
> "reason": "Added", "domain_name": "core_node", "domain_id":
> "b9906489-6d5b-40f8-a742-ca71b2b84277"}
> {"event_type": "Resumed", "timestamp": "2016-11-25T12:02:44.813104Z",
> "reason": "Unpaused", "domain_name": "core_node", "domain_id":
> "b9906489-6d5b-40f8-a742-ca71b2b84277"}
> {"event_type": "Started", "timestamp": "2016-11-25T12:02:44.813733Z",
> "reason": "Booted", "domain_name": "core_node", "domain_id":
> "b9906489-6d5b-40f8-a742-ca71b2b84277"}


 Interesting, so here is "defined" event delivered before the "started"
 event. Also - where is "suspended" event?

>>>
>>>
>>> With latest master the situation looks better. Now I see
>>>
>>> event 'lifecycle' for domain testvm: Started Booted
>>> event 'lifecycle' for domain testvm: Suspended Paused
>>
>> Again, both of these are deliberately emitted by libvirt and in fact I
>> think they reflect what is happening.
>>
>>
> Why is in this case  not
> 
>  event 'lifecycle' for domain testvm: Resumed Unpaused
> 
> event emitted?
> 
> I would expect
> 
> event 'lifecycle' for domain testvm: Resumed Unpaused

I fail to see why this is supposed to be emitted. The QEMU is started as
paused. If this even would be emitted, even if immediately followed by
Paused event, it might suggest that guest CPUs have run for a little
while. But that's not true.

> event 'lifecycle' for domain testvm: Started Booted
> event 'lifecycle' for domain testvm: Suspended Paused
> 
> 
> So the situation on master is much better but because of the
> Resumed/Unpaused event I still have the feeling that the most simple but
> powerful usecase, watching for CREATE, 

Re: [libvirt] [PATCH v2 13/31] qemu: Probe CPU models for KVM and TCG

[Jiri Denemark]

On Thu, Nov 24, 2016 at 19:10:18 +0100, Pavel Hrdina wrote:
> On Mon, Nov 21, 2016 at 12:21:09AM +0100, Jiri Denemark wrote:
> > CPU models (and especially some additional details which we will start
> > probing for later) differ depending on the accelerator. Thus we need to
> > call query-cpu-definitions in both KVM and TCG mode to get all data we
> > want.
> > 
> > Tests in tests/domaincapstest.c are temporarily switched to TCG to avoid
> > having to squash even more stuff into this single patch. They will all
> > be switched back later in separate commits.
> 
> That could be avoided by moving this patch after patch
> "tests: Update capabilities for QEMU 2.7.0".  Code from this patch is required
> to use qemucapsprobe to generate updated *.replies, but I think that the 
> updated
> replies can be pushed before this patch is introduced.
> 
> This is just a suggestion, so you don't have to do it, but it would probably
> made the patches cleaner.

Well, the updated replies could be theoretically moved before this
patch, although it would be a bit weired since this patch needs to be
applied to generate them. And what's even worse, all the associated
changes in test XML files from patches 14 to 28 would need to be
squashed in this patch.

That said, I think the approach I chose is a smaller mess :-)

Jirka

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

Re: [libvirt] [PATCH v3] qemu: Make QMP probing process reusable

[Jiri Denemark]

On Fri, Nov 25, 2016 at 19:56:23 +0100, Jiri Denemark wrote:
> The code that runs a new QEMU process to be used for probing
> capabilities is separated into four reusable functions so that any code
> that wants to probe a QEMU process may just follow a few simple steps:
> 
> cmd = virQEMUCapsInitQMPCommandNew(...);
> 
> mon = virQEMUCapsInitQMPCommandRun(cmd, ...);

Oops, and this needs to be changed to

virQEMUCapsInitQMPCommandRun(cmd);

> 
> /* talk to the running QEMU process using its QMP monitor */
> 
> if (reprobeIsRequired) {
> virQEMUCapsInitQMPCommandAbort(cmd, ...);
> mon = virQEMUCapsInitQMPCommandRun(cmd, ...);

And here as well.

> 
> /* talk to the running QEMU process again */
> }
> 
> virQEMUCapsInitQMPCommandFree(cmd);

Jirka

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

[libvirt] [PATCH v3] qemu: Make QMP probing process reusable

[Jiri Denemark]

The code that runs a new QEMU process to be used for probing
capabilities is separated into four reusable functions so that any code
that wants to probe a QEMU process may just follow a few simple steps:

cmd = virQEMUCapsInitQMPCommandNew(...);

mon = virQEMUCapsInitQMPCommandRun(cmd, ...);

/* talk to the running QEMU process using its QMP monitor */

if (reprobeIsRequired) {
virQEMUCapsInitQMPCommandAbort(cmd, ...);
mon = virQEMUCapsInitQMPCommandRun(cmd, ...);

/* talk to the running QEMU process again */
}

virQEMUCapsInitQMPCommandFree(cmd);

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_capabilities.c | 266 +--
 1 file changed, 180 insertions(+), 86 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index f9e39a34a..d6182ccb7 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4024,32 +4024,101 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
 return ret;
 }
 
-static int
-virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
-   const char *libDir,
-   uid_t runUid,
-   gid_t runGid,
-   char **qmperr)
-{
-int ret = -1;
-virCommandPtr cmd = NULL;
-qemuMonitorPtr mon = NULL;
-int status = 0;
+
+typedef struct _virQEMUCapsInitQMPCommand virQEMUCapsInitQMPCommand;
+typedef virQEMUCapsInitQMPCommand *virQEMUCapsInitQMPCommandPtr;
+struct _virQEMUCapsInitQMPCommand {
+char *binary;
+uid_t runUid;
+gid_t runGid;
+char **qmperr;
+char *monarg;
+char *monpath;
+char *pidfile;
+virCommandPtr cmd;
+qemuMonitorPtr mon;
 virDomainChrSourceDef config;
-char *monarg = NULL;
-char *monpath = NULL;
-char *pidfile = NULL;
-pid_t pid = 0;
-virDomainObjPtr vm = NULL;
-virDomainXMLOptionPtr xmlopt = NULL;
+pid_t pid;
+virDomainObjPtr vm;
+};
+
+
+static void
+virQEMUCapsInitQMPCommandAbort(virQEMUCapsInitQMPCommandPtr cmd)
+{
+if (cmd->mon)
+virObjectUnlock(cmd->mon);
+qemuMonitorClose(cmd->mon);
+cmd->mon = NULL;
+
+virCommandAbort(cmd->cmd);
+virCommandFree(cmd->cmd);
+cmd->cmd = NULL;
+
+if (cmd->monpath)
+ignore_value(unlink(cmd->monpath));
+
+virDomainObjEndAPI(>vm);
+
+if (cmd->pid != 0) {
+char ebuf[1024];
+
+VIR_DEBUG("Killing QMP caps process %lld", (long long) cmd->pid);
+if (virProcessKill(cmd->pid, SIGKILL) < 0 && errno != ESRCH)
+VIR_ERROR(_("Failed to kill process %lld: %s"),
+  (long long) cmd->pid,
+  virStrerror(errno, ebuf, sizeof(ebuf)));
+
+VIR_FREE(*cmd->qmperr);
+}
+if (cmd->pidfile)
+unlink(cmd->pidfile);
+cmd->pid = 0;
+}
+
+
+static void
+virQEMUCapsInitQMPCommandFree(virQEMUCapsInitQMPCommandPtr cmd)
+{
+if (!cmd)
+return;
+
+virQEMUCapsInitQMPCommandAbort(cmd);
+VIR_FREE(cmd->binary);
+VIR_FREE(cmd->monpath);
+VIR_FREE(cmd->monarg);
+VIR_FREE(cmd->pidfile);
+VIR_FREE(cmd);
+}
+
+
+static virQEMUCapsInitQMPCommandPtr
+virQEMUCapsInitQMPCommandNew(char *binary,
+ const char *libDir,
+ uid_t runUid,
+ gid_t runGid,
+ char **qmperr)
+{
+virQEMUCapsInitQMPCommandPtr cmd = NULL;
+
+if (VIR_ALLOC(cmd) < 0)
+goto error;
+
+if (VIR_STRDUP(cmd->binary, binary) < 0)
+goto error;
+
+cmd->runUid = runUid;
+cmd->runGid = runGid;
+cmd->qmperr = qmperr;
 
 /* the ".sock" sufix is important to avoid a possible clash with a qemu
  * domain called "capabilities"
  */
-if (virAsprintf(, "%s/%s", libDir, "capabilities.monitor.sock") < 
0)
-goto cleanup;
-if (virAsprintf(, "unix:%s,server,nowait", monpath) < 0)
-goto cleanup;
+if (virAsprintf(>monpath, "%s/%s", libDir,
+"capabilities.monitor.sock") < 0)
+goto error;
+if (virAsprintf(>monarg, "unix:%s,server,nowait", cmd->monpath) < 0)
+goto error;
 
 /* ".pidfile" suffix is used rather than ".pid" to avoid a possible clash
  * with a qemu domain called "capabilities"
@@ -4057,17 +4126,35 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
  * -daemonize we need QEMU to be allowed to create them, rather
  * than libvirtd. So we're using libDir which QEMU can write to
  */
-if (virAsprintf(, "%s/%s", libDir, "capabilities.pidfile") < 0)
-goto cleanup;
+if (virAsprintf(>pidfile, "%s/%s", libDir, "capabilities.pidfile") < 
0)
+goto error;
 
-memset(, 0, sizeof(config));
-config.type = VIR_DOMAIN_CHR_TYPE_UNIX;
-config.data.nix.path = monpath;
-config.data.nix.listen = false;
+virPidFileForceCleanupPath(cmd->pidfile);
 
-virPidFileForceCleanupPath(pidfile);
+

Re: [libvirt] [PATCH v2 01/31] qemu: Make QMP probing process reusable

[Jiri Denemark]

On Thu, Nov 24, 2016 at 14:54:48 +0100, Pavel Hrdina wrote:
> On Mon, Nov 21, 2016 at 12:20:57AM +0100, Jiri Denemark wrote:
> > The code that runs a new QEMU process to be used for probing
> > capabilities is separated into four reusable functions so that any code
> > that wants to probe a QEMU process may just follow a few simple steps:
> > 
> > cmd = virQEMUCapsInitQMPCommandNew(...);
> > 
> > mon = virQEMUCapsInitQMPCommandRun(cmd, ...);
> > 
> > /* talk to the running QEMU process using its QMP monitor */
> > 
> > if (reprobeIsRequired) {
> > virQEMUCapsInitQMPCommandAbort(cmd, ...);
> > mon = virQEMUCapsInitQMPCommandRun(cmd, ...);
> > 
> > /* talk to the running QEMU process again */
> > }
> > 
> > virQEMUCapsInitQMPCommandFree(cmd);
> > 
> > Signed-off-by: Jiri Denemark 
> > ---
> >  src/qemu/qemu_capabilities.c | 259 
> > +--
> >  1 file changed, 174 insertions(+), 85 deletions(-)
> > 
> > +static qemuMonitorPtr
> > +virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd,
> > + bool *qemuFailed)
> 
> That *bool* is not necessary, function virQEMUCapsInitQMPCommandRun can return
> simple *int* because qemuMonitorPtr is stored in *cmd*.  So this function can
> return -1 in case of fatal error.  In case return is 0, following code can
> easily depend on whether cmd->mon is set or not.

Indeed, although I think the function should rather return -1, 0, 1
instead of forcing the code to do magic based on cmd->mon.

> Otherwise this patch looks good, but I think that it would be better to send a
> new version with the suggested change.

Sure, since the patch is quite large and not exactly easy to read, the
following is the diff I will squash into v2.

Jirka

diff --git i/src/qemu/qemu_capabilities.c w/src/qemu/qemu_capabilities.c
index 1257bb0ff..d6182ccb7 100644
--- i/src/qemu/qemu_capabilities.c
+++ w/src/qemu/qemu_capabilities.c
@@ -4143,12 +4143,16 @@ virQEMUCapsInitQMPCommandNew(char *binary,
 }
 
 
-static qemuMonitorPtr
-virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd,
- bool *qemuFailed)
+/* Returns -1 on fatal error,
+ *  0 on success,
+ *  1 when probing QEMU failed
+ */
+static int
+virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd)
 {
 virDomainXMLOptionPtr xmlopt = NULL;
 int status = 0;
+int ret = -1;
 
 VIR_DEBUG("Try to probe capabilities of '%s' via QMP", cmd->binary);
 
@@ -4203,15 +4207,17 @@ 
virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd,
 
 virObjectLock(cmd->mon);
 
+ret = 0;
+
  cleanup:
 if (!cmd->mon)
 virQEMUCapsInitQMPCommandAbort(cmd);
 virObjectUnref(xmlopt);
 
-return cmd->mon;
+return ret;
 
  ignore:
-*qemuFailed = true;
+ret = 1;
 goto cleanup;
 }
 
@@ -4224,21 +4230,20 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
char **qmperr)
 {
 virQEMUCapsInitQMPCommandPtr cmd = NULL;
-qemuMonitorPtr mon = NULL;
-bool qemuFailed = false;
 int ret = -1;
+int rc;
 
 if (!(cmd = virQEMUCapsInitQMPCommandNew(qemuCaps->binary, libDir,
  runUid, runGid, qmperr)))
 goto cleanup;
 
-if (!(mon = virQEMUCapsInitQMPCommandRun(cmd, ))) {
-if (qemuFailed)
+if ((rc = virQEMUCapsInitQMPCommandRun(cmd)) != 0) {
+if (rc == 1)
 ret = 0;
 goto cleanup;
 }
 
-if (virQEMUCapsInitQMPMonitor(qemuCaps, mon) < 0)
+if (virQEMUCapsInitQMPMonitor(qemuCaps, cmd->mon) < 0)
 goto cleanup;
 
 ret = 0;

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

Re: [libvirt] [PATCH] qemu: capabilities: Don't partially reprope caps on process reconnect

[Jiri Denemark]

On Fri, Nov 25, 2016 at 17:14:48 +0100, Peter Krempa wrote:
> Thanks to the complex capability caching code virQEMUCapsProbeQMP was
> never called when we were starting a new qemu VM. On the other hand,
> when we are reconnecting to the qemu process we reload the capability
> list from the status XML file. This means that the flag preventing the
> function being called was not set and thus we partially reprobed some of
> the capabilities.
> 
> The recent addition of CPU hotplug clears the
> QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS if the machine does not support it.
> The partial re-probe on reconnect results into attempting to call the
> unsupported command and then killing the VM.
> 
> Remove the partial reprobe and depend on the stored capabilities. If it
> will be necessary to reprobe the capabilities in the future, we should
> do a full reprobe rather than this partial one.
> ---
>  src/qemu/qemu_capabilities.c | 17 -
>  src/qemu/qemu_capabilities.h |  3 ---
>  src/qemu/qemu_process.c  |  4 
>  3 files changed, 24 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 8901e7b..37e5302 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -2937,23 +2937,6 @@ virQEMUCapsProbeQMPGICCapabilities(virQEMUCapsPtr 
> qemuCaps,
>  return 0;
>  }
> 
> -int virQEMUCapsProbeQMP(virQEMUCapsPtr qemuCaps,
> -qemuMonitorPtr mon)
> -{
> -VIR_DEBUG("qemuCaps=%p mon=%p", qemuCaps, mon);
> -
> -if (qemuCaps->usedQMP)
> -return 0;
> -
> -if (virQEMUCapsProbeQMPCommands(qemuCaps, mon) < 0)
> -return -1;
> -
> -if (virQEMUCapsProbeQMPEvents(qemuCaps, mon) < 0)
> -return -1;
> -
> -return 0;
> -}
> -

Oh, that's some seriously ancient piece of code which should have been
removed ages ago.

ACK

Jirka

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

Re: [libvirt] Libvirt domain event usage and consistency

[Roman Mohr]

On Fri, Nov 25, 2016 at 6:04 PM, Michal Privoznik 
wrote:

> On 25.11.2016 17:54, Roman Mohr wrote:
> > On Fri, Nov 25, 2016 at 4:34 PM, Michal Privoznik 
> > wrote:
> >
> >> On 25.11.2016 14:38, Roman Mohr wrote:
> >>> Hi,
> >>>
> >>> I recently started to use the libvirt domain events. With them I
> increase
> >>> the responsiveness of my VM state wachers.
> >>> In general it works pretty well. I just listen to the events and do a
> >>> periodic resync to cope with missed events.
> >>>
> >>> While watching the events I ran into a few interesting situations I
> >> wanted
> >>> to share. The points 1-3 describe some minor issues or irregularities.
> >>> Point 4 is about the fact that domain and state updates are not
> versioned
> >>> which makes it very hard to stay in sync with libvirt when using
> events.
> >>>
> >>> My libvirt version is 1.2.18.4.
> >>
> >> This might be the root cause. I'm unable to see some of the scenarios
> >> you're seeing. Have you tried the latest release (or even git HEAD) to
> >> check whether all the scenarios you are describing still stand?
> >>
> >
> > Definitely better with latest HEAD but still it does not look completely
> > right.
> >
> >>
> >>>
> >>> 1) Event order seems to be weird on startup:
> >>>
> >>> When listening for VM lifecycle events I get this order:
> >>>
> >>> {"event_type": "Started", "timestamp": "2016-11-25T11:59:53.209326Z",
> >>> "reason": "Booted", "domain_name": "generic", "domain_id":
> >>> "8ff7047b-fb46-44ff-a4c6-7c20c73ab86e"}
> >>> {"event_type": "Defined", "timestamp": "2016-11-25T11:59:53.435530Z",
> >>> "reason": "Added", "domain_name": "generic", "domain_id":
> >>> "8ff7047b-fb46-44ff-a4c6-7c20c73ab86e"}
> >>>
> >>> It is strange that a VM already boots before it is defined. Is this the
> >>> intended order?
> >>
> >> I don't see this order so probable this is fixed upstream.
> >>
> >
> > On latest master a normal creation emits these events:
> >
> > event 'lifecycle' for domain testvm: Resumed Unpaused
> > event 'lifecycle' for domain testvm: Started Booted
> >
> > The Resumed event looks wrong. Further I get no more Defined/Undefined
> > events. Maybe they were removed?
>
> Yes, they were removed.


Nice


> The Resumed event comes from qemu actually,
> because libvirt starts qemu in paused mode so that we can do some setup
> (e.g. place vcpu threads into cgroups) and only after that we can resume
> guest CPUs and in fact let guest start. Once this is done we
> deliberately emit Started event.
>

I would expect an event like this:

event 'lifecycle' for domain testvm: Suspended Bootstrapping

before the other two events. That takes the ambiguity from the Resumed
event.


> >
> >
> >>
> >>>
> >>> 2) Defining a VM with VIR_DOMAIN_START_PAUSED gives me this event order
> >>
> >> I don't think you can define a domain with that flag. What's the actual
> >> action?
> >>
> >
> > That is the flag for the api, when using virsh using `--paused` does
> that.
>
> Ah, that's for virsh create/start not virsh define. Anyway, this is no
> longer the case with upstream, is it?
>
>
Right


> >
> >
> >>
> >>>
> >>> {"event_type": "Defined", "timestamp": "2016-11-25T12:02:44.037817Z",
> >>> "reason": "Added", "domain_name": "core_node", "domain_id":
> >>> "b9906489-6d5b-40f8-a742-ca71b2b84277"}
> >>> {"event_type": "Resumed", "timestamp": "2016-11-25T12:02:44.813104Z",
> >>> "reason": "Unpaused", "domain_name": "core_node", "domain_id":
> >>> "b9906489-6d5b-40f8-a742-ca71b2b84277"}
> >>> {"event_type": "Started", "timestamp": "2016-11-25T12:02:44.813733Z",
> >>> "reason": "Booted", "domain_name": "core_node", "domain_id":
> >>> "b9906489-6d5b-40f8-a742-ca71b2b84277"}
> >>
> >>
> >> Interesting, so here is "defined" event delivered before the "started"
> >> event. Also - where is "suspended" event?
> >>
> >
> >
> > With latest master the situation looks better. Now I see
> >
> > event 'lifecycle' for domain testvm: Started Booted
> > event 'lifecycle' for domain testvm: Suspended Paused
>
> Again, both of these are deliberately emitted by libvirt and in fact I
> think they reflect what is happening.
>
>
Why is in this case  not

 event 'lifecycle' for domain testvm: Resumed Unpaused

event emitted?

I would expect

event 'lifecycle' for domain testvm: Resumed Unpaused
event 'lifecycle' for domain testvm: Started Booted
event 'lifecycle' for domain testvm: Suspended Paused


So the situation on master is much better but because of the
Resumed/Unpaused event I still have the feeling that the most simple but
powerful usecase, watching for CREATE, UPDATE, DELETE is very hard because
you can't know if the Resumed/Unpaused is the indicator for CREATE or
UPDATE.

What do you think?


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

Re: [libvirt] [PATCH] Pass GPG_TTY env var to the ssh binary

[Guilhem Moulin]

And I didn't test this carefully, my apologies :-(  Whether gpg-agent
can prompt the password depends on the pinentry program in use, but for
pinentry-curses this also requires to pass TERM.  Patch modified
accordingly.

 
From: Guilhem Moulin 
Subject: [PATCH] Pass GPG_TTY env var to the ssh binary

gpg-agent(1) can emulate the OpenSSH Agent protocol (which provides
pubkey-authentication using an authentication-capable OpenPGP key, in
addition to the usual identity files).  However for a console-based
password prompt (such as pinentry-curses) to work, the ‘GPG_TTY’
environment variable needs to be set to the current TTY.

Using gpg-agent's ssh-agent implementation is currently not possible for
SSH remote URIs, because the environment is cleaned before calling the
ssh(1) binary.  The enclosed patches adds ‘GPG_TTY’ to the list of
environment variables passed to the child.

References: http://bugs.debian.org/843863
---
src/rpc/virnetsocket.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 325a7c7..8d20074 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -839,6 +839,8 @@ int virNetSocketNewConnectSSH(const char *nodename,
 virCommandAddEnvPassBlockSUID(cmd, "KRB5CCNAME", NULL);
 virCommandAddEnvPassBlockSUID(cmd, "SSH_AUTH_SOCK", NULL);
 virCommandAddEnvPassBlockSUID(cmd, "SSH_ASKPASS", NULL);
+virCommandAddEnvPassBlockSUID(cmd, "GPG_TTY", NULL);
+virCommandAddEnvPassBlockSUID(cmd, "TERM", NULL);
 virCommandAddEnvPassBlockSUID(cmd, "DISPLAY", NULL);
 virCommandAddEnvPassBlockSUID(cmd, "XAUTHORITY", NULL);
 virCommandClearCaps(cmd);

-- 
Guilhem.


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

Re: [libvirt] Libvirt domain event usage and consistency

[Michal Privoznik]

On 25.11.2016 17:54, Roman Mohr wrote:
> On Fri, Nov 25, 2016 at 4:34 PM, Michal Privoznik 
> wrote:
> 
>> On 25.11.2016 14:38, Roman Mohr wrote:
>>> Hi,
>>>
>>> I recently started to use the libvirt domain events. With them I increase
>>> the responsiveness of my VM state wachers.
>>> In general it works pretty well. I just listen to the events and do a
>>> periodic resync to cope with missed events.
>>>
>>> While watching the events I ran into a few interesting situations I
>> wanted
>>> to share. The points 1-3 describe some minor issues or irregularities.
>>> Point 4 is about the fact that domain and state updates are not versioned
>>> which makes it very hard to stay in sync with libvirt when using events.
>>>
>>> My libvirt version is 1.2.18.4.
>>
>> This might be the root cause. I'm unable to see some of the scenarios
>> you're seeing. Have you tried the latest release (or even git HEAD) to
>> check whether all the scenarios you are describing still stand?
>>
> 
> Definitely better with latest HEAD but still it does not look completely
> right.
> 
>>
>>>
>>> 1) Event order seems to be weird on startup:
>>>
>>> When listening for VM lifecycle events I get this order:
>>>
>>> {"event_type": "Started", "timestamp": "2016-11-25T11:59:53.209326Z",
>>> "reason": "Booted", "domain_name": "generic", "domain_id":
>>> "8ff7047b-fb46-44ff-a4c6-7c20c73ab86e"}
>>> {"event_type": "Defined", "timestamp": "2016-11-25T11:59:53.435530Z",
>>> "reason": "Added", "domain_name": "generic", "domain_id":
>>> "8ff7047b-fb46-44ff-a4c6-7c20c73ab86e"}
>>>
>>> It is strange that a VM already boots before it is defined. Is this the
>>> intended order?
>>
>> I don't see this order so probable this is fixed upstream.
>>
> 
> On latest master a normal creation emits these events:
> 
> event 'lifecycle' for domain testvm: Resumed Unpaused
> event 'lifecycle' for domain testvm: Started Booted
> 
> The Resumed event looks wrong. Further I get no more Defined/Undefined
> events. Maybe they were removed?

Yes, they were removed. The Resumed event comes from qemu actually,
because libvirt starts qemu in paused mode so that we can do some setup
(e.g. place vcpu threads into cgroups) and only after that we can resume
guest CPUs and in fact let guest start. Once this is done we
deliberately emit Started event.

> 
> 
>>
>>>
>>> 2) Defining a VM with VIR_DOMAIN_START_PAUSED gives me this event order
>>
>> I don't think you can define a domain with that flag. What's the actual
>> action?
>>
> 
> That is the flag for the api, when using virsh using `--paused` does that.

Ah, that's for virsh create/start not virsh define. Anyway, this is no
longer the case with upstream, is it?

> 
> 
>>
>>>
>>> {"event_type": "Defined", "timestamp": "2016-11-25T12:02:44.037817Z",
>>> "reason": "Added", "domain_name": "core_node", "domain_id":
>>> "b9906489-6d5b-40f8-a742-ca71b2b84277"}
>>> {"event_type": "Resumed", "timestamp": "2016-11-25T12:02:44.813104Z",
>>> "reason": "Unpaused", "domain_name": "core_node", "domain_id":
>>> "b9906489-6d5b-40f8-a742-ca71b2b84277"}
>>> {"event_type": "Started", "timestamp": "2016-11-25T12:02:44.813733Z",
>>> "reason": "Booted", "domain_name": "core_node", "domain_id":
>>> "b9906489-6d5b-40f8-a742-ca71b2b84277"}
>>
>>
>> Interesting, so here is "defined" event delivered before the "started"
>> event. Also - where is "suspended" event?
>>
> 
> 
> With latest master the situation looks better. Now I see
> 
> event 'lifecycle' for domain testvm: Started Booted
> event 'lifecycle' for domain testvm: Suspended Paused

Again, both of these are deliberately emitted by libvirt and in fact I
think they reflect what is happening.

Michal

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

Re: [libvirt] Libvirt domain event usage and consistency

[Roman Mohr]

On Fri, Nov 25, 2016 at 4:34 PM, Michal Privoznik 
wrote:

> On 25.11.2016 14:38, Roman Mohr wrote:
> > Hi,
> >
> > I recently started to use the libvirt domain events. With them I increase
> > the responsiveness of my VM state wachers.
> > In general it works pretty well. I just listen to the events and do a
> > periodic resync to cope with missed events.
> >
> > While watching the events I ran into a few interesting situations I
> wanted
> > to share. The points 1-3 describe some minor issues or irregularities.
> > Point 4 is about the fact that domain and state updates are not versioned
> > which makes it very hard to stay in sync with libvirt when using events.
> >
> > My libvirt version is 1.2.18.4.
>
> This might be the root cause. I'm unable to see some of the scenarios
> you're seeing. Have you tried the latest release (or even git HEAD) to
> check whether all the scenarios you are describing still stand?
>

Definitely better with latest HEAD but still it does not look completely
right.

>
> >
> > 1) Event order seems to be weird on startup:
> >
> > When listening for VM lifecycle events I get this order:
> >
> > {"event_type": "Started", "timestamp": "2016-11-25T11:59:53.209326Z",
> > "reason": "Booted", "domain_name": "generic", "domain_id":
> > "8ff7047b-fb46-44ff-a4c6-7c20c73ab86e"}
> > {"event_type": "Defined", "timestamp": "2016-11-25T11:59:53.435530Z",
> > "reason": "Added", "domain_name": "generic", "domain_id":
> > "8ff7047b-fb46-44ff-a4c6-7c20c73ab86e"}
> >
> > It is strange that a VM already boots before it is defined. Is this the
> > intended order?
>
> I don't see this order so probable this is fixed upstream.
>

On latest master a normal creation emits these events:

event 'lifecycle' for domain testvm: Resumed Unpaused
event 'lifecycle' for domain testvm: Started Booted

The Resumed event looks wrong. Further I get no more Defined/Undefined
events. Maybe they were removed?


>
> >
> > 2) Defining a VM with VIR_DOMAIN_START_PAUSED gives me this event order
>
> I don't think you can define a domain with that flag. What's the actual
> action?
>

That is the flag for the api, when using virsh using `--paused` does that.


>
> >
> > {"event_type": "Defined", "timestamp": "2016-11-25T12:02:44.037817Z",
> > "reason": "Added", "domain_name": "core_node", "domain_id":
> > "b9906489-6d5b-40f8-a742-ca71b2b84277"}
> > {"event_type": "Resumed", "timestamp": "2016-11-25T12:02:44.813104Z",
> > "reason": "Unpaused", "domain_name": "core_node", "domain_id":
> > "b9906489-6d5b-40f8-a742-ca71b2b84277"}
> > {"event_type": "Started", "timestamp": "2016-11-25T12:02:44.813733Z",
> > "reason": "Booted", "domain_name": "core_node", "domain_id":
> > "b9906489-6d5b-40f8-a742-ca71b2b84277"}
>
>
> Interesting, so here is "defined" event delivered before the "started"
> event. Also - where is "suspended" event?
>


With latest master the situation looks better. Now I see

event 'lifecycle' for domain testvm: Started Booted
event 'lifecycle' for domain testvm: Suspended Paused


>
> >
> > This boot-order makes it hard to track active domains by listening to
> > life-cycle events. One could theoretically still always fetch the VM
> state
> > in the event callback and check the state, but if the state is not
> > immediately transferred with the event itself, it can already be
> outdated,
> > so this might be racy (intransparent for the libvirt bindings user), and
> as
> > described in (3) currently not even possible. In general the real
> existing
> > events seem to differ quite significantly from the described life-cycle
> in
> > [1].
>
> Again, in the upstream I see something different:
> event 'lifecycle' for domain $domain: Started Booted
> event 'lifecycle' for domain $domain: Suspended Paused
>
>
On master I see that too when I start the VM with `virsh create --paused`.


>
> >
> > 3) "Defined" event is triggered before the domain is completely defined
> >
> > {"event_type": "Defined", "timestamp": "2016-11-25T12:02:44.037817Z",
> > "reason": "Added", "domain_name": "core_node", "domain_id":
> > "b9906489-6d5b-40f8-a742-ca71b2b84277"}
> > {"event_type": "Resumed", "timestamp": "2016-11-25T12:02:44.813104Z",
> > "reason": "Unpaused", "domain_name": "core_node", "domain_id":
> > "b9906489-6d5b-40f8-a742-ca71b2b84277"}
> > {"event_type": "Started", "timestamp": "2016-11-25T12:02:44.813733Z",
> > "reason": "Booted", "domain_name": "core_node", "domain_id":
> > "b9906489-6d5b-40f8-a742-ca71b2b84277"}
> >
> > When I try to process the first event and do a xmldump I get:
> >
> >Event: [Code-42] [Domain-10] Domain not found: no domain with matching
> > uuid 'b9906489-6d5b-40f8-a742-ca71b2b84277' (core_node)
> >
> > So it seems like I get the event before the domain is completely ready.
>
> You know that you shouldn't be calling libvirt APIs from event callbacks?


No, good to know. Anyway, just tried to work around the above problems.


So if the Defined/Undefined events were removed 

[libvirt] [PATCH] qemu: capabilities: Don't partially reprope caps on process reconnect

[Peter Krempa]

Thanks to the complex capability caching code virQEMUCapsProbeQMP was
never called when we were starting a new qemu VM. On the other hand,
when we are reconnecting to the qemu process we reload the capability
list from the status XML file. This means that the flag preventing the
function being called was not set and thus we partially reprobed some of
the capabilities.

The recent addition of CPU hotplug clears the
QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS if the machine does not support it.
The partial re-probe on reconnect results into attempting to call the
unsupported command and then killing the VM.

Remove the partial reprobe and depend on the stored capabilities. If it
will be necessary to reprobe the capabilities in the future, we should
do a full reprobe rather than this partial one.
---
 src/qemu/qemu_capabilities.c | 17 -
 src/qemu/qemu_capabilities.h |  3 ---
 src/qemu/qemu_process.c  |  4 
 3 files changed, 24 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 8901e7b..37e5302 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2937,23 +2937,6 @@ virQEMUCapsProbeQMPGICCapabilities(virQEMUCapsPtr 
qemuCaps,
 return 0;
 }

-int virQEMUCapsProbeQMP(virQEMUCapsPtr qemuCaps,
-qemuMonitorPtr mon)
-{
-VIR_DEBUG("qemuCaps=%p mon=%p", qemuCaps, mon);
-
-if (qemuCaps->usedQMP)
-return 0;
-
-if (virQEMUCapsProbeQMPCommands(qemuCaps, mon) < 0)
-return -1;
-
-if (virQEMUCapsProbeQMPEvents(qemuCaps, mon) < 0)
-return -1;
-
-return 0;
-}
-

 static bool
 virQEMUCapsCPUFilterFeatures(const char *name,
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 5255815..be71507 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -403,9 +403,6 @@ virQEMUCapsPtr virQEMUCapsNew(void);
 int virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
   qemuMonitorPtr mon);

-int virQEMUCapsProbeQMP(virQEMUCapsPtr qemuCaps,
-qemuMonitorPtr mon);
-
 void virQEMUCapsSet(virQEMUCapsPtr qemuCaps,
 virQEMUCapsFlags flag) ATTRIBUTE_NONNULL(1);

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ab0c2c8..90f1101 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1723,10 +1723,6 @@ qemuConnectMonitor(virQEMUDriverPtr driver, 
virDomainObjPtr vm, int asyncJob,
 if (qemuMonitorSetCapabilities(priv->mon) < 0)
 goto cleanup;

-if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MONITOR_JSON) &&
-virQEMUCapsProbeQMP(priv->qemuCaps, priv->mon) < 0)
-goto cleanup;
-
 if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT) &&
 qemuMonitorSetMigrationCapability(priv->mon,
   QEMU_MONITOR_MIGRATION_CAPS_EVENTS,
-- 
2.10.2

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

Re: [libvirt] [PATCH V2] qemu: Redefine the "unlimited" memory limits one more time

[Daniel P. Berrange]

On Fri, Nov 25, 2016 at 05:12:41PM +0100, Martin Kletzander wrote:
> On Fri, Nov 25, 2016 at 03:39:17PM +, Daniel P. Berrange wrote:
> > On Fri, Nov 25, 2016 at 04:35:14PM +0100, Martin Kletzander wrote:
> > > On Fri, Nov 25, 2016 at 03:19:54PM +0100, Viktor Mihajlovski wrote:
> > > > On 18.11.2016 17:44, Viktor Mihajlovski wrote:
> > > > > With kernel 3.18 (since commit 
> > > > > 3e32cb2e0a12b6915056ff04601cf1bb9b44f967) the
> > > > > "unlimited" value for cgroup memory limits has changed once again as 
> > > > > its byte
> > > > > value is now computed from a page counter.
> > > > > The new "unlimited" value reported by the cgroup fs is therefore 
> > > > > 2**51-1 pages
> > > > > which is (VIR_DOMAIN_MEMORY_PARAM_UNLIMITED - 3072). This results 
> > > > > e.g. in virsh
> > > > > memtune displaying 9007199254740988 instead of unlimited for the 
> > > > > limits.
> > > > >
> > > 
> > > Ah, I wonder hoe many times we'll have to deal with this.
> > > 
> > > Anyway, this means it is not enough to shift by 2 if the system hugepage
> > > is more than 4k (e.g. 2M).  I remember we had to change something due to
> > > such hosts.  virGetSystemPageSize(KB) should help with that.
> > > 
> > > We could also make it 2^50 - pagesize just to make sure it will work for
> > > a while, but some might not like it.
> > 
> > I guess I'm not understanding how this code copes with the fact that
> > we now have 3 different "unlimited" values to deal with.
> > 
> > Could we not simply record the unlimited values and pick the right
> > one based on kernel version we detect ?
> > 
> 
> We have one, which we return for Get APIs that means that the setting
> was not restricted.  We would need to have a cgroup which we set to
> some huge value and then read it, store it and compare it all the time.
> It _should_ work as far as I can tell.

No, I meant can we use some constants

 #define UNLIMTED_VALUE_KERNEL_A_B_C 2343243245325
 #define UNLIMTED_VALUE_KERNEL_D_E_F 3253253253253253
 #define UNLIMTED_VALUE_KERNEL_G_H_I 325325325325232


 if kver == A_B_C && value == UNLIMTED_VALUE_KERNEL_A_B_C
  value = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED
 else if kver == D_E_F && value == UNLIMTED_VALUE_KERNEL_D_E_F
  value = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED
 else if kver == G_H_I && value == UNLIMTED_VALUE_KERNEL_G_H_I
  value = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

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

Re: [libvirt] [PATCH V2] qemu: Redefine the "unlimited" memory limits one more time

[Martin Kletzander]

On Fri, Nov 25, 2016 at 03:39:17PM +, Daniel P. Berrange wrote:

On Fri, Nov 25, 2016 at 04:35:14PM +0100, Martin Kletzander wrote:

On Fri, Nov 25, 2016 at 03:19:54PM +0100, Viktor Mihajlovski wrote:
> On 18.11.2016 17:44, Viktor Mihajlovski wrote:
> > With kernel 3.18 (since commit 3e32cb2e0a12b6915056ff04601cf1bb9b44f967) the
> > "unlimited" value for cgroup memory limits has changed once again as its 
byte
> > value is now computed from a page counter.
> > The new "unlimited" value reported by the cgroup fs is therefore 2**51-1 
pages
> > which is (VIR_DOMAIN_MEMORY_PARAM_UNLIMITED - 3072). This results e.g. in 
virsh
> > memtune displaying 9007199254740988 instead of unlimited for the limits.
> >

Ah, I wonder hoe many times we'll have to deal with this.

Anyway, this means it is not enough to shift by 2 if the system hugepage
is more than 4k (e.g. 2M).  I remember we had to change something due to
such hosts.  virGetSystemPageSize(KB) should help with that.

We could also make it 2^50 - pagesize just to make sure it will work for
a while, but some might not like it.


I guess I'm not understanding how this code copes with the fact that
we now have 3 different "unlimited" values to deal with.

Could we not simply record the unlimited values and pick the right
one based on kernel version we detect ?



We have one, which we return for Get APIs that means that the setting
was not restricted.  We would need to have a cgroup which we set to
some huge value and then read it, store it and compare it all the time.
It _should_ work as far as I can tell.


Regards,
Daniel
--
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|


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

Re: [libvirt] [PATCH] Revert "vz: fixed race in vzDomainAttach/DettachDevice"

[Maxim Nestratov]

22-Nov-16 15:05, Nikolay Shirokovskiy пишет:


ACK


Pushed now. Thanx.

Maxim

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

Re: [libvirt] [REPOST PATCH v2 5/9] qemu: Adjust various bool BlockIoTune set_ values into mask

[Erik Skultety]

On Mon, Nov 21, 2016 at 06:35:50PM -0500, John Ferlan wrote:
> Rather than have multiple bool values, create a single enum with bits
> representing what can be set.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/qemu/qemu_driver.c | 113 
> +++--
>  1 file changed, 54 insertions(+), 59 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 87d219f..73b58d0 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -17338,34 +17338,38 @@ qemuDomainOpenGraphicsFD(virDomainPtr dom,
>  return ret;
>  }
>  
> +typedef enum {
> +QEMU_BLOCK_IOTUNE_SET_BYTES= 1 << 0,
> +QEMU_BLOCK_IOTUNE_SET_IOPS = 1 << 1,
> +QEMU_BLOCK_IOTUNE_SET_BYTES_MAX= 1 << 2,
> +QEMU_BLOCK_IOTUNE_SET_IOPS_MAX = 1 << 3,
> +QEMU_BLOCK_IOTUNE_SET_SIZE_IOPS= 1 << 4,
> +QEMU_BLOCK_IOTUNE_SET_BYTES_MAX_LENGTH = 1 << 5,
> +QEMU_BLOCK_IOTUNE_SET_IOPS_MAX_LENGTH  = 1 << 6,
> +} qemuBlockIoTuneSetFlags;
> +
>  
>  /* If the user didn't specify bytes limits, inherit previous values;
>   * likewise if the user didn't specify iops limits.  */
>  static void
>  qemuDomainSetBlockIoTuneDefaults(virDomainBlockIoTuneInfoPtr newinfo,
>   virDomainBlockIoTuneInfoPtr oldinfo,
> - bool set_bytes,
> - bool set_iops,
> - bool set_bytes_max,
> - bool set_iops_max,
> - bool set_size_iops,
> - bool set_bytes_max_length,
> - bool set_iops_max_length)
> + qemuBlockIoTuneSetFlags set_flag)

Just a cosmetic "nit", I spent a few seconds looking at the name "set_flag"
confusingly (probably 'cause it's Friday). Maybe something like
set_map|set_mask|mask|bitmap or something alike would sound better, but then,
who am I to judge with my history of 'brilliant' function naming :D.

Patch looks good though, I'll leave it to you.

Erik


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

Re: [libvirt] [PATCH V2] qemu: Redefine the "unlimited" memory limits one more time

[Daniel P. Berrange]

On Fri, Nov 25, 2016 at 04:35:14PM +0100, Martin Kletzander wrote:
> On Fri, Nov 25, 2016 at 03:19:54PM +0100, Viktor Mihajlovski wrote:
> > On 18.11.2016 17:44, Viktor Mihajlovski wrote:
> > > With kernel 3.18 (since commit 3e32cb2e0a12b6915056ff04601cf1bb9b44f967) 
> > > the
> > > "unlimited" value for cgroup memory limits has changed once again as its 
> > > byte
> > > value is now computed from a page counter.
> > > The new "unlimited" value reported by the cgroup fs is therefore 2**51-1 
> > > pages
> > > which is (VIR_DOMAIN_MEMORY_PARAM_UNLIMITED - 3072). This results e.g. in 
> > > virsh
> > > memtune displaying 9007199254740988 instead of unlimited for the limits.
> > > 
> 
> Ah, I wonder hoe many times we'll have to deal with this.
> 
> Anyway, this means it is not enough to shift by 2 if the system hugepage
> is more than 4k (e.g. 2M).  I remember we had to change something due to
> such hosts.  virGetSystemPageSize(KB) should help with that.
> 
> We could also make it 2^50 - pagesize just to make sure it will work for
> a while, but some might not like it.

I guess I'm not understanding how this code copes with the fact that
we now have 3 different "unlimited" values to deal with.

Could we not simply record the unlimited values and pick the right
one based on kernel version we detect ?

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

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

Re: [libvirt] [PATCH V2] qemu: Redefine the "unlimited" memory limits one more time

[Martin Kletzander]

On Fri, Nov 25, 2016 at 03:19:54PM +0100, Viktor Mihajlovski wrote:

On 18.11.2016 17:44, Viktor Mihajlovski wrote:

With kernel 3.18 (since commit 3e32cb2e0a12b6915056ff04601cf1bb9b44f967) the
"unlimited" value for cgroup memory limits has changed once again as its byte
value is now computed from a page counter.
The new "unlimited" value reported by the cgroup fs is therefore 2**51-1 pages
which is (VIR_DOMAIN_MEMORY_PARAM_UNLIMITED - 3072). This results e.g. in virsh
memtune displaying 9007199254740988 instead of unlimited for the limits.



Ah, I wonder hoe many times we'll have to deal with this.

Anyway, this means it is not enough to shift by 2 if the system hugepage
is more than 4k (e.g. 2M).  I remember we had to change something due to
such hosts.  virGetSystemPageSize(KB) should help with that.

We could also make it 2^50 - pagesize just to make sure it will work for
a while, but some might not like it.

Also some comment for the condition would be nice to have for others
reading the code in the future.


This patch deals with the rounding issue by scaling the byte values reported
by the kernel and the PARAM_UNLIMITED value to page size and comparing those.

See also libvirt commit 231656bbeb9e4d3bedc44362784c35eee21cf0f4 for the
history for kernel 3.12 and before.




ping ... it would be nice if this or an equivalent patch (comparison on
4k boundaries) could be applied. Besides causing the goofy virsh memtune
output, this breaks applications that compare current limits against
VIR_DOMAIN_MEMORY_PARAM_UNLIMITED.

Thanks!

--

Mit freundlichen Grüßen/Kind Regards
  Viktor Mihajlovski

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

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


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

Re: [libvirt] Libvirt domain event usage and consistency

[Michal Privoznik]

On 25.11.2016 14:38, Roman Mohr wrote:
> Hi,
> 
> I recently started to use the libvirt domain events. With them I increase
> the responsiveness of my VM state wachers.
> In general it works pretty well. I just listen to the events and do a
> periodic resync to cope with missed events.
> 
> While watching the events I ran into a few interesting situations I wanted
> to share. The points 1-3 describe some minor issues or irregularities.
> Point 4 is about the fact that domain and state updates are not versioned
> which makes it very hard to stay in sync with libvirt when using events.
> 
> My libvirt version is 1.2.18.4.

This might be the root cause. I'm unable to see some of the scenarios
you're seeing. Have you tried the latest release (or even git HEAD) to
check whether all the scenarios you are describing still stand?

> 
> 1) Event order seems to be weird on startup:
> 
> When listening for VM lifecycle events I get this order:
> 
> {"event_type": "Started", "timestamp": "2016-11-25T11:59:53.209326Z",
> "reason": "Booted", "domain_name": "generic", "domain_id":
> "8ff7047b-fb46-44ff-a4c6-7c20c73ab86e"}
> {"event_type": "Defined", "timestamp": "2016-11-25T11:59:53.435530Z",
> "reason": "Added", "domain_name": "generic", "domain_id":
> "8ff7047b-fb46-44ff-a4c6-7c20c73ab86e"}
> 
> It is strange that a VM already boots before it is defined. Is this the
> intended order?

I don't see this order so probable this is fixed upstream.

> 
> 2) Defining a VM with VIR_DOMAIN_START_PAUSED gives me this event order

I don't think you can define a domain with that flag. What's the actual
action?

> 
> {"event_type": "Defined", "timestamp": "2016-11-25T12:02:44.037817Z",
> "reason": "Added", "domain_name": "core_node", "domain_id":
> "b9906489-6d5b-40f8-a742-ca71b2b84277"}
> {"event_type": "Resumed", "timestamp": "2016-11-25T12:02:44.813104Z",
> "reason": "Unpaused", "domain_name": "core_node", "domain_id":
> "b9906489-6d5b-40f8-a742-ca71b2b84277"}
> {"event_type": "Started", "timestamp": "2016-11-25T12:02:44.813733Z",
> "reason": "Booted", "domain_name": "core_node", "domain_id":
> "b9906489-6d5b-40f8-a742-ca71b2b84277"}


Interesting, so here is "defined" event delivered before the "started"
event. Also - where is "suspended" event?

> 
> This boot-order makes it hard to track active domains by listening to
> life-cycle events. One could theoretically still always fetch the VM state
> in the event callback and check the state, but if the state is not
> immediately transferred with the event itself, it can already be outdated,
> so this might be racy (intransparent for the libvirt bindings user), and as
> described in (3) currently not even possible. In general the real existing
> events seem to differ quite significantly from the described life-cycle in
> [1].

Again, in the upstream I see something different:
event 'lifecycle' for domain $domain: Started Booted
event 'lifecycle' for domain $domain: Suspended Paused


> 
> 3) "Defined" event is triggered before the domain is completely defined
> 
> {"event_type": "Defined", "timestamp": "2016-11-25T12:02:44.037817Z",
> "reason": "Added", "domain_name": "core_node", "domain_id":
> "b9906489-6d5b-40f8-a742-ca71b2b84277"}
> {"event_type": "Resumed", "timestamp": "2016-11-25T12:02:44.813104Z",
> "reason": "Unpaused", "domain_name": "core_node", "domain_id":
> "b9906489-6d5b-40f8-a742-ca71b2b84277"}
> {"event_type": "Started", "timestamp": "2016-11-25T12:02:44.813733Z",
> "reason": "Booted", "domain_name": "core_node", "domain_id":
> "b9906489-6d5b-40f8-a742-ca71b2b84277"}
> 
> When I try to process the first event and do a xmldump I get:
> 
>Event: [Code-42] [Domain-10] Domain not found: no domain with matching
> uuid 'b9906489-6d5b-40f8-a742-ca71b2b84277' (core_node)
> 
> So it seems like I get the event before the domain is completely ready.

You know that you shouldn't be calling libvirt APIs from event callbacks?

> 
> 4) There libvirt domain description is not versioned
> 
> I would expect that every time I update a domainxml (update from third
> party entity), or an event is generated (update from libvirt), that the
> resource version of a Domain is increased and that I get this resource
> version when I do a xmldump or when I get an event. Without this there is
> afaik no way to stay in sync with libvirt, even if you do regular polling
> of all domains. The main issue here is that I can never know if events in
> the queue arrived before my latest domain resync or after it.
> 
> Also not that this is not about delivery guarantees of events. It is just
> about having a consistent view of a VM and the individual event. If I have
> resource versions, I can decide if an event is still interesting for me or
> not, which is exactly what I need to solve the syncing problem above.
> When I do a complete relisting of all domains to syn, I know which version
> I got and I can then see on every event if it is newer or older.
> 
> If along side with the event, the 

Re: [libvirt] [REPOST PATCH v2 3/9] qemu: Adjust maxparams logic for qemuDomainGetBlockIoTune

[Erik Skultety]

On Mon, Nov 21, 2016 at 06:35:48PM -0500, John Ferlan wrote:
> Rather than using negative logic and setting the maxparams to a lesser
> value based on which capabilities exist, alter the logic to modify the
> maxparams based on a base value plus the found capabilities. Reduces the
> chance that some backported feature produces an incorrect value.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/qemu/qemu_driver.c | 23 ++-
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index d039255..87d219f 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -112,9 +112,12 @@ VIR_LOG_INIT("qemu.qemu_driver");
>  
>  #define QEMU_NB_MEM_PARAM  3
>  
> -#define QEMU_NB_BLOCK_IO_TUNE_PARAM  6
> -#define QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX  13
> -#define QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX_LENGTH 19
> +#define QEMU_NB_BLOCK_IO_TUNE_BASE_PARAMS 6
> +#define QEMU_NB_BLOCK_IO_TUNE_MAX_PARAMS 7
> +#define QEMU_NB_BLOCK_IO_TUNE_LENGTH_PARAMS 6
> +#define QEMU_NB_BLOCK_IO_TUNE_ALL_PARAMS (QEMU_NB_BLOCK_IO_TUNE_BASE_PARAMS 
> + \
> +  QEMU_NB_BLOCK_IO_TUNE_MAX_PARAMS + 
> \
> +  
> QEMU_NB_BLOCK_IO_TUNE_LENGTH_PARAMS)
>  
>  #define QEMU_NB_NUMA_PARAM 2
>  
> @@ -17719,7 +17722,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
>  virDomainBlockIoTuneInfo reply;
>  char *device = NULL;
>  int ret = -1;
> -int maxparams = QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX_LENGTH;
> +int maxparams;
>  
>  virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
>VIR_DOMAIN_AFFECT_CONFIG |
> @@ -17753,11 +17756,13 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
>  goto endjob;
>  }
>  
> -if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX))
> -maxparams = QEMU_NB_BLOCK_IO_TUNE_PARAM;
> -else if (!virQEMUCapsGet(priv->qemuCaps,
> - QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH))
> -maxparams = QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX;
> +maxparams = QEMU_NB_BLOCK_IO_TUNE_BASE_PARAMS;
> +if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX))
> +maxparams += QEMU_NB_BLOCK_IO_TUNE_MAX_PARAMS;
> +if (virQEMUCapsGet(priv->qemuCaps, 
> QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH))
> +maxparams += QEMU_NB_BLOCK_IO_TUNE_LENGTH_PARAMS;
> +} else {
> +maxparams = QEMU_NB_BLOCK_IO_TUNE_ALL_PARAMS;
>  }
>  

I'm curious about the else branch. Can you elaborate more on why do you assume
qemu supports all the features when you retrieve a persistentdef vs livedef?

From what I understand, this is one of the APIs that you have to call twice,
since the RPC layer does not allocate a structure of an appropriate size
automatically. So with the first run, you say, please allocate a structure of
size maxparams equal _ALL_PARAMS (without knowing that qemu actually supports 
all these features) and rerun.
Then during the second run, you run BLOCK_IOTUNE_ASSIGN on all of them, what am
I missing?

Erik

>  if (*nparams == 0) {
> -- 
> 2.7.4
> 
> --
> 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] Pass GPG_TTY env var to the ssh binary

[Martin Kletzander]

On Mon, Nov 14, 2016 at 11:13:22AM +0100, Guilhem Moulin wrote:

Hi Daniel,

On Mon, 14 Nov 2016 at 10:02:55 +, Daniel P. Berrange wrote:

On Sat, Nov 12, 2016 at 02:19:37PM +0100, Guido Günther wrote:

This came in via the Debian BTS:

http://bugs.debian.org/43863


This seems to be the wrong bug number.


Yup, it's #843863 actually: http://bugs.debian.org/843863


Can you explain what functional effect a GPG setting has on SSH ?!?!?!?


Quoting myself from the Debian bug #843863:

   gpg-agent(1) can emulate the OpenSSH Agent protocol (which provides
   pubkey-authentication using an authentication-capable OpenPGP key,
   in addition to the usual identity files).  However for a
   console-based password prompt (such as pinentry-curses) to work, the
   ‘GPG_TTY’ environment variable needs to be set to the current TTY.

   Using gpg-agent's ssh-agent implementation is currently not possible
   for SSH remote URIs, because the environment is cleaned before
   calling the ssh(1) binary.  The enclosed patches adds ‘GPG_TTY’ to
   the list of environment variables passed to the child.



Yeah, I use it as well, without GPG_TTY it fallbacks.  We need to pass
it together with SSH_AUTH_SOCK and others.

From me it's an ACK if you fix the bug number.



Cheers,
--
Guilhem.





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


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

Re: [libvirt] [PATCH 3/3] Add support for preallocated fd memory - doc

[Martin Kletzander]

On Tue, Nov 01, 2016 at 12:11:16PM +, Jaroslav Safka wrote:

Add html documentation for memoryBacking element.


We usually merge documentation with the conf changes.


---
docs/formatdomain.html.in | 10 ++
1 file changed, 10 insertions(+)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index c70377b..ed15eb5 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -903,6 +903,9 @@
/hugepages
nosharepages/
locked/
+source type='file|anonymous'/
+access mode='shared|private'
+allocation mode='immediate|ondemand'/
  /memoryBacking
  ...
/domain
@@ -942,6 +945,13 @@
most of the host's memory). Doing so may be dangerous to both the
domain and the host itself since the host's kernel may run out of
memory. Since 1.0.6
+  source
+If the option "file" is selected, then token 
"mem-path=/var/lib/libvirt/qemu" will be added to object argument.


You should say it is the type attribute.  Also  talking
about tokens being added to object argument makes no sense for the
reader.  And I'm not talking about hypervisors other than QEMU where it
doesn't make sense even to developers.


+  access
+Specify if memory is shared or private. This can be overridden by 
numa cell element memAccess.


overridden *per node*.  We enclose element and attribute names inside
 so that others can see that you are referring to something.


+If share is selected then token "share=yes" will be added to object 
memory-backend-file.


Again, this makes no sense in high-lieve documentation.


+  allocation
+Immediate - preallocate the memory immediately


^^ no comment.


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

Re: [libvirt] [PATCH 2/3] Add support for preallocated fd memory

[Martin Kletzander]

On Tue, Nov 01, 2016 at 12:11:15PM +, Jaroslav Safka wrote:

This second change introduces support for preallocated
shared file descriptor based memory backing.
It allows vhost-user to be used without hugepages.

Also token memAccess in numa cell is used (if not present, default value
from memoryBacking is used)

Used xml elements:

 
 
 

---
src/qemu/qemu_command.c| 156 -
src/qemu/qemu_command.h|   4 +
.../qemuxml2argv-fd-memory-no-numa-topology.args   |  33 +
.../qemuxml2argv-fd-memory-no-numa-topology.xml|  91 
.../qemuxml2argv-fd-memory-numa-topology.args  |  33 +
.../qemuxml2argv-fd-memory-numa-topology.xml   |  94 +
.../qemuxml2argv-fd-memory-numa-topology2.args |  36 +
.../qemuxml2argv-fd-memory-numa-topology2.xml  |  95 +
.../qemuxml2argv-fd-memory-numa-topology3.args |  39 ++
.../qemuxml2argv-fd-memory-numa-topology3.xml  |  96 +
.../qemuxml2argv-memorybacking-set.xml |  32 +
.../qemuxml2argv-memorybacking-unset.xml   |  32 +
tests/qemuxml2argvtest.c   |  42 ++
13 files changed, 750 insertions(+), 33 deletions(-)
create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-fd-memory-no-numa-topology.args
create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-fd-memory-no-numa-topology.xml
create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology.args
create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology.xml
create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology2.args
create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology2.xml
create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology3.args
create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology3.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memorybacking-set.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memorybacking-unset.xml

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b68da3d..7710864 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3283,15 +3283,11 @@ qemuBuildMemoryBackendStr(unsigned long long size,
if (!(props = virJSONValueNewObject()))
return -1;

-if (pagesize) {
-if (qemuGetHupageMemPath(cfg, pagesize, _path) < 0)
-goto cleanup;
-
+if (def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE) {
*backendType = "memory-backend-file";

if (virJSONValueObjectAdd(props,
-  "b:prealloc", true,
-  "s:mem-path", mem_path,
+  "s:mem-path", VIR_DOMAIN_MEMORY_DEFAULT_PATH,
  NULL) < 0)
goto cleanup;

@@ -3307,18 +3303,61 @@ qemuBuildMemoryBackendStr(unsigned long long size,
break;

case VIR_NUMA_MEM_ACCESS_DEFAULT:
+if (def->mem.access == VIR_DOMAIN_MEMORY_ACCESS_SHARED) {
+if (virJSONValueObjectAdd(props, "b:share", true, NULL) < 0)
+goto cleanup;
+}
+break;
+
case VIR_NUMA_MEM_ACCESS_LAST:
break;
}
+
+force = true;
} else {
-if (memAccess) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("Shared memory mapping is supported "
- "only with hugepages"));
-goto cleanup;
-}
+if (pagesize) {
+if (qemuGetHupageMemPath(cfg, pagesize, _path) < 0)
+goto cleanup;

-*backendType = "memory-backend-ram";
+*backendType = "memory-backend-file";
+
+if (virJSONValueObjectAdd(props,
+  "b:prealloc", true,
+  "s:mem-path", mem_path,
+  NULL) < 0)
+goto cleanup;
+
+switch (memAccess) {
+case VIR_NUMA_MEM_ACCESS_SHARED:
+if (virJSONValueObjectAdd(props, "b:share", true, NULL) < 0)
+goto cleanup;
+break;
+
+case VIR_NUMA_MEM_ACCESS_PRIVATE:
+if (virJSONValueObjectAdd(props, "b:share", false, NULL) < 0)
+goto cleanup;
+break;
+
+case VIR_NUMA_MEM_ACCESS_DEFAULT:
+if (def->mem.access == VIR_DOMAIN_MEMORY_ACCESS_SHARED) {
+if (virJSONValueObjectAdd(props, "b:share", true, NULL) < 
0)
+goto cleanup;
+}
+break;
+
+case VIR_NUMA_MEM_ACCESS_LAST:
+break;
+}
+} else {
+if (memAccess) {
+

Re: [libvirt] [PATCH] libvirt: qemu: Fix domain termination caused by query-hotpluggable-cpus not enabled

[Peter Krempa]

On Fri, Nov 25, 2016 at 14:25:33 +0100, Boris Fiuczynski wrote:
> On 11/25/2016 10:07 AM, Peter Krempa wrote:
> > On Fri, Nov 25, 2016 at 10:03:38 +0100, Peter Krempa wrote:
> > > On Fri, Nov 25, 2016 at 09:19:18 +0100, Boris Fiuczynski wrote:
> > 
> > [...]
> 
> Peter,
> looking at your commit message of 920bbe5c it reads as follows:
> qemu: capabilities: Extract availability of new cpu hotplug for machine
> types
> 
> QEMU reports whether 'query-hotpluggable-cpus' is supported for a given
> machine type. Extract and cache the information using the capability
> cache.
> 
> When copying the capabilities for a new start of qemu, mask out the
> presence of QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS if the machine type
> doesn't support hotpluggable cpus.
> 
> The loaded XML has the 'query-hotpluggable-cpus' capability set since the
> qmp command exists in the list returned by the qmp command query-commands by
> the qemu binary.

In the global capabilities cache, the QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS
is set if the command is supported by qemu.

Once a VM is started we create a copy of the given capability set and
possibly filter out stuff that won't work and then store the filtered
capability set in the domain object.

> The machine type dependent masking of QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS you
> are doing for a new start of qemu seems therefore also required to be done
> for reconnecting to this running qemu domain. Are you saying that this is

No. You can't do that at reconnect time. Once you start the VM you can
completely replace the original binary and thus you don't have data that
would allow to do such operation at reconnect time.

That is the main reason why we store the capabilities in the status XML
rather than reloading it from the cache.

> wrong and the machine type dependent masking result of the
> 'query-hotpluggable-cpus' capability should be stored in the XML?

Yes, exactly. As said, the capability set should not be re-detected for
the reasons stated above.

Said the above, there's something fishy going on. I compiled a qemu
binary that specifically doesn't support cpu hotplug and started a VM.
The status XML file does not show the flag:

# grep query-hotpluggable-cpus /var/run/libvirt/qemu/upstream.xml
#

But an attempt to restart libvirtd indeed ends up in the VM being
killed:

016-11-25 14:09:12.909+: 2610599: error :
qemuMonitorJSONCheckError:387 : internal error: unable to execute QEMU
command 'query-hotpluggable-cpus': The feature 'query-hotpluggable-cpus'
is not enabled

This means that the capabilities are not properly restored.

Peter



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

Re: [libvirt] [Qemu-devel] [PATCH v5 05/12] target-i386: Make plus_features/minus_features QOM-based

[Markus Armbruster]

Eduardo Habkost  writes:

> Instead of using custom feature name lookup code for
> plus_features/minus_features, save the property names used in
> "[+-]feature" and use object_property_set_bool() to set them.
>
> We don't need a feat2prop() call because we now have alias
> properties for the old names containing underscores.
>
> Signed-off-by: Eduardo Habkost 
> ---
> Changes v4 -> v5:
> * Removed feat2prop() call, as we now have property aliases for
>   the old names containing underscores
>
> Changes series v3 -> v4:
> * New patch added to series
> ---
>  target-i386/cpu.c | 106 
> +++---
>  1 file changed, 20 insertions(+), 86 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 6b5bf59..cef4ecd 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
[...]
> @@ -2047,10 +1967,12 @@ static void x86_cpu_parse_featurestr(const char 
> *typename, char *features,
>  
>  /* Compatibility syntax: */
>  if (featurestr[0] == '+') {
> -add_flagname_to_bitmaps(featurestr + 1, plus_features, 
> _err);
> +plus_features = g_list_append(plus_features,
> +  g_strdup(featurestr + 1));
>  continue;
>  } else if (featurestr[0] == '-') {
> -add_flagname_to_bitmaps(featurestr + 1, minus_features, 
> _err);
> +minus_features = g_list_append(minus_features,
> +   g_strdup(featurestr + 1));
>  continue;
>  }
>  

Since removes the only assignments to local_err other than its
initialization to null, it can't become non-null anymore.  Coverity
points out:

*** CID 1365201:  Possible Control flow issues  (DEADCODE)
/target-i386/cpu.c: 2050 in x86_cpu_parse_featurestr()
2044 prop->value = g_strdup(val);
2045 prop->errp = _fatal;
2046 qdev_prop_register_global(prop);
2047 }
2048 
2049 if (local_err) {
>>> CID 1365201:  Possible Control flow issues  (DEADCODE)
>>> Execution cannot reach this statement: "error_propagate(errp, 
local...".
2050 error_propagate(errp, local_err);
2051 }
2052 }
2053 
2054 static void x86_cpu_load_features(X86CPU *cpu, Error **errp);
2055 static int x86_cpu_filter_features(X86CPU *cpu);

[...]

Please clean this up.

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

Re: [libvirt] [PATCH 1/3] Add support for preallocated fd memory

[Martin Kletzander]

On Tue, Nov 01, 2016 at 12:11:14PM +, Jaroslav Safka wrote:

This first change introduces xml parsing support for preallocated
shared file descriptor based memory backing.
It allows vhost-user to be used without hugepages.

New xml elements:

 
 
 

---
docs/schemas/domaincommon.rng  |  30 +
src/conf/domain_conf.c | 138 -
src/conf/domain_conf.h |  33 +
.../qemuxml2xmlout-memorybacking-set.xml   |  40 ++
.../qemuxml2xmlout-memorybacking-unset.xml |  40 ++
tests/qemuxml2xmltest.c|   3 +
6 files changed, 251 insertions(+), 33 deletions(-)
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-memorybacking-set.xml
create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-memorybacking-unset.xml



Tests will fail after this patch, the source files for the tests are missing.


diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 03506cb..97ef769 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -830,6 +830,21 @@ VIR_ENUM_IMPL(virDomainDiskMirrorState, 
VIR_DOMAIN_DISK_MIRROR_STATE_LAST,
  "abort",
  "pivot")

+VIR_ENUM_IMPL(virDomainMemorySource, VIR_DOMAIN_MEMORY_SOURCE_LAST,
+  "none",
+  "file",
+  "anonymous")
+
+VIR_ENUM_IMPL(virDomainMemoryAccess, VIR_DOMAIN_MEMORY_ACCESS_LAST,
+  "none",
+  "shared",
+  "private")
+


This is the same as virNumaMemAccess from numa_conf.c, you should change that
one to this type so it's not duplicated.


+VIR_ENUM_IMPL(virDomainMemoryAllocation, VIR_DOMAIN_MEMORY_ALLOCATION_LAST,
+  "none",
+  "immediate",
+  "ondemand")
+
VIR_ENUM_IMPL(virDomainLoader,
  VIR_DOMAIN_LOADER_TYPE_LAST,
  "rom",
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 24aa79c..f50b575 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -567,6 +567,32 @@ typedef enum {
VIR_DOMAIN_DISK_MIRROR_STATE_LAST
} virDomainDiskMirrorState;

+# define VIR_DOMAIN_MEMORY_DEFAULT_PATH "/var/lib/libvirt/qemu"
+


This is wrong.  You should use domain's priv->libDir which is per-domain
directory which is properly labelled and configurable.  Also it is
handled so that it works on upgrades, etc.


+typedef enum {
+VIR_DOMAIN_MEMORY_SOURCE_NONE = 0,  /* No memory source defined */
+VIR_DOMAIN_MEMORY_SOURCE_FILE,  /* Memory source is set as file */
+VIR_DOMAIN_MEMORY_SOURCE_ANONYMOUS, /* Memory source is set as anonymous */
+
+VIR_DOMAIN_MEMORY_SOURCE_LAST,
+} virDomainMemorySource;
+
+typedef enum {
+VIR_DOMAIN_MEMORY_ACCESS_NONE = 0,  /*  No memory access defined */
+VIR_DOMAIN_MEMORY_ACCESS_SHARED,/* Memory access is set as shared */
+VIR_DOMAIN_MEMORY_ACCESS_PRIVATE,   /* Memory access is set as private */
+
+VIR_DOMAIN_MEMORY_ACCESS_LAST,
+} virDomainMemoryAccess;
+


Same here for the virNumaMemAccess, of course.


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

Re: [libvirt] [PATCH] Pass GPG_TTY env var to the ssh binary

[Guido Günther]

Hi Daniel,
On Mon, Nov 14, 2016 at 10:02:55AM +, Daniel P. Berrange wrote:
> On Sat, Nov 12, 2016 at 02:19:37PM +0100, Guido Günther wrote:
> > This came in via the Debian BTS:
> > 
> > http://bugs.debian.org/43863
> 
> This seems to be the wrong bug number.

I've updated the commit message and added the correct bugnumber as
reference. Does this look better:

From: Guilhem Moulin 
Subject: [PATCH] Pass GPG_TTY env var to the ssh binary

gpg-agent(1) can emulate the OpenSSH Agent protocol (which provides
pubkey-authentication using an authentication-capable OpenPGP key, in
addition to the usual identity files).  However for a console-based
password prompt (such as pinentry-curses) to work, the ‘GPG_TTY’
environment variable needs to be set to the current TTY.

Using gpg-agent's ssh-agent implementation is currently not possible for
SSH remote URIs, because the environment is cleaned before calling the
ssh(1) binary.  The enclosed patches adds ‘GPG_TTY’ to the list of
environment variables passed to the child.

References: http://bugs.debian.org/843863
---
 src/rpc/virnetsocket.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 325a7c7..8d20074 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -848,6 +848,7 @@ int virNetSocketNewConnectSSH(const char *nodename,
 virCommandAddEnvPassBlockSUID(cmd, "KRB5CCNAME", NULL);
 virCommandAddEnvPassBlockSUID(cmd, "SSH_AUTH_SOCK", NULL);
 virCommandAddEnvPassBlockSUID(cmd, "SSH_ASKPASS", NULL);
+virCommandAddEnvPassBlockSUID(cmd, "GPG_TTY", NULL);
 virCommandAddEnvPassBlockSUID(cmd, "DISPLAY", NULL);
 virCommandAddEnvPassBlockSUID(cmd, "XAUTHORITY", NULL);
 virCommandClearCaps(cmd);
-- 
2.10.2

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

Re: [libvirt] [Qemu-ppc] [RFC PATCH qemu] spapr_pci: Create PCI-express root bus by default

[Andrea Bolognani]

On Wed, 2016-11-23 at 16:02 +1100, David Gibson wrote:
> > > The change from OHCI to XHCI only affected the *default* USB
> > > controller, which libvirt tries its best not to use anyway:
> > > instead, it will prefer to use '-M ...,usb=off' along with
> > > '-device ...' and set both the controller model and its PCI
> > > address explicitly, partially to shield its users from such
> > > changes in QEMU.
> > 
> > Ok. Always likes this approach really. We should start moving to this
> > direction with PHB - stop adding the default PHB at all when -nodefaults is
> > passed (or -machine pseries,pci=off ?) and let libvirt manage PHBs itself
> > (and provide another spapr-phb type like spapr-pcie-host-bridge or add a
> > "pcie_root_bus_type" property to the existing PHB type).
> > 
> > What will be wrong with this approach?
> 
> Hm, that's a good point.  If were removing the default PHB entirely,
> that I would consider a possible case for a new machine type.  Putting
> construction of the PHBs into libvirt's hand could make life simpler
> there.  Although it would make it a bit of a pain for people starting
> qemu by hand.
> 
> I think this option is worth some thought.

Note that libvirt always runs QEMU with -nodefaults, so we
could just remove the default PHB if that flag is present,
and leave it in if that's not the case.

The idea itself sounds good, but of course it will require
more work from the libvirt side than simply making the PCIe
machine type behave like q35 and mach-virt.

Moreover, we already have an RFE for supporting additional
PHBs, we could solve both issues in one fell swoop and have
the libvirt guest XML be a more faithful representation of
the actual virtual hardware, which is always a win in my
book.

That will be even more important if it turns out that the
rules for PCIe device assignment (eg. what device/controller
can be plugged into which slot) are different for PCIe PHBs
than they are for q35/mach-virt PCIe Root Bus. I've asked
for clarifications about this elsewhere in the thread.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH] Qemu : Do not distinguish a 'hangup' from an 'eof'

[Martin Kletzander]

On Fri, Oct 28, 2016 at 12:33:28PM -0700, Prerna Saxena wrote:

An errno=ECONNRESET received on a monitor socket reflects that the
guest may have closed the socket.


How would they close it?  Does that happen when the process is dying?


Today, we just mark it as a 'hangup' and do not trigger
the eof callback.



I'm guessing that's because the process is not quitting and killing the
domain would be weird.


I've been looking at a slew of such messages in libvirt logs. If
the monitor socket indicated an ECONNRESET, it would possibly make sense
to mark the socket closed so that the "eof" callback may
then be invoked.



It depends.  What are all the ways you can call ECONNRESET on such
socket?


Is there a subtle corner case I'm missing here ?

Signed-off-by: Prerna Saxena 
---
src/qemu/qemu_monitor.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index a5e14b2..dcafa5a 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -690,10 +690,11 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque)

if (events & VIR_EVENT_HANDLE_HANGUP) {
hangup = true;
+eof = true;
if (!error) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
   _("End of file from qemu monitor"));
-eof = true;
+
events &= ~VIR_EVENT_HANDLE_HANGUP;
}
}
--
1.8.1.2

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


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

Re: [libvirt] [PATCH V2] qemu: Redefine the "unlimited" memory limits one more time

[Viktor Mihajlovski]

On 18.11.2016 17:44, Viktor Mihajlovski wrote:
> With kernel 3.18 (since commit 3e32cb2e0a12b6915056ff04601cf1bb9b44f967) the
> "unlimited" value for cgroup memory limits has changed once again as its byte
> value is now computed from a page counter.
> The new "unlimited" value reported by the cgroup fs is therefore 2**51-1 pages
> which is (VIR_DOMAIN_MEMORY_PARAM_UNLIMITED - 3072). This results e.g. in 
> virsh
> memtune displaying 9007199254740988 instead of unlimited for the limits.
> 
> This patch deals with the rounding issue by scaling the byte values reported
> by the kernel and the PARAM_UNLIMITED value to page size and comparing those.
> 
> See also libvirt commit 231656bbeb9e4d3bedc44362784c35eee21cf0f4 for the
> history for kernel 3.12 and before.
> 
> 

ping ... it would be nice if this or an equivalent patch (comparison on
4k boundaries) could be applied. Besides causing the goofy virsh memtune
output, this breaks applications that compare current limits against
VIR_DOMAIN_MEMORY_PARAM_UNLIMITED.

Thanks!

-- 

Mit freundlichen Grüßen/Kind Regards
   Viktor Mihajlovski

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

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

[libvirt] [PATCH V3 1/2] util: Allow to query the presence of host CPU bitmaps

[Viktor Mihajlovski]

The functions to retrieve online and present host CPU information
are only supported on Linux for the time being.

This leads to runtime errors if these function are used on other
platforms. To avoid that, code in higher levels using the functions
must replicate the conditional compilation in higher level which
is error prone (and is plainly spoken ugly).

Adding a function virHostCPUHasBitmap that can be used to check
for host CPU bitmap support.

NB: There are other functions including the host CPU count that
are lacking support on all platforms, but they are too essential
in order to be bypassed.

Signed-off-by: Viktor Mihajlovski 
---
 src/libvirt_private.syms |  1 +
 src/util/virhostcpu.c| 10 ++
 src/util/virhostcpu.h|  1 +
 3 files changed, 12 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b7d26fd..7468623 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1118,6 +1118,7 @@ virHostCPUGetOnlineBitmap;
 virHostCPUGetPresentBitmap;
 virHostCPUGetStats;
 virHostCPUGetThreadsPerSubcore;
+virHostCPUHasBitmap;
 virHostCPUStatsAssign;
 virHostMemAllocPages;
 virHostMemGetCellsFree;
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index f68176f..ec2469d 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -1112,6 +1112,16 @@ virHostCPUGetCount(void)
 #endif
 }
 
+bool
+virHostCPUHasBitmap(void)
+{
+#ifdef __linux__
+return true;
+#else
+return false;
+#endif
+}
+
 virBitmapPtr
 virHostCPUGetPresentBitmap(void)
 {
diff --git a/src/util/virhostcpu.h b/src/util/virhostcpu.h
index b048704..39f7cf8 100644
--- a/src/util/virhostcpu.h
+++ b/src/util/virhostcpu.h
@@ -35,6 +35,7 @@ int virHostCPUGetStats(int cpuNum,
int *nparams,
unsigned int flags);
 
+bool virHostCPUHasBitmap(void);
 virBitmapPtr virHostCPUGetPresentBitmap(void);
 virBitmapPtr virHostCPUGetOnlineBitmap(void);
 int virHostCPUGetCount(void);
-- 
1.9.1

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

[libvirt] [PATCH V3 0/2] Allow hot plugged host CPUs

[Viktor Mihajlovski]

This is a rework of the original patch allowing to use all host
CPUs if the cpuset controller is not configured by libvirt.

The major enhancement is that we won't try to retrieve the online
host CPU map on platforms where this is not supported and just
fall back to the old behavior.

V3:
- Added new function (in seperate patch) to find out whether the host
  CPU online map can be retrieved gracefully

V2:
- Avoid memory leak

Viktor Mihajlovski (2):
  util: Allow to query the presence of host CPU bitmaps
  qemu: Allow use of hot plugged host CPUs if no affinity set

 src/libvirt_private.syms |  1 +
 src/qemu/qemu_process.c  | 33 -
 src/util/virhostcpu.c| 10 ++
 src/util/virhostcpu.h|  1 +
 4 files changed, 36 insertions(+), 9 deletions(-)

-- 
1.9.1

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

[libvirt] [PATCH V3 2/2] qemu: Allow use of hot plugged host CPUs if no affinity set

[Viktor Mihajlovski]

If the cpuset cgroup controller is disabled in /etc/libvirt/qemu.conf
QEMU virtual machines can in principle use all host CPUs, even if they
are hot plugged, if they have no explicit CPU affinity defined.

However, there's libvirt code supposed to handle the situation where
the libvirt daemon itself is not using all host CPUs. The code in
qemuProcessInitCpuAffinity attempts to set an affinity mask including
all defined host CPUs. Unfortunately, the resulting affinity mask for
the process will not contain the offline CPUs. See also the
sched_setaffinity(2) man page.

That means that even if the host CPUs come online again, they won't be
used by the QEMU process anymore. The same is true for newly hot
plugged CPUs. So we are effectively preventing that QEMU uses all
processors instead of enabling it to use them.

It only makes sense to set the QEMU process affinity if we're able
to actually grow the set of usable CPUs, i.e. if the process affinity
is a subset of the online host CPUs.

There's still the chance that for some reason the deliberately chosen
libvirtd affinity matches the online host CPU mask by accident. In this
case the behavior remains as it was before (CPUs offline while setting
the affinity will not be used if they show up later on).

Signed-off-by: Viktor Mihajlovski 
Tested-by: Matthew Rosato 
---
 src/qemu/qemu_process.c | 33 -
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 4758c49..9d1bfa4 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2202,6 +2202,7 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm)
 int ret = -1;
 virBitmapPtr cpumap = NULL;
 virBitmapPtr cpumapToSet = NULL;
+virBitmapPtr hostcpumap = NULL;
 qemuDomainObjPrivatePtr priv = vm->privateData;
 
 if (!vm->pid) {
@@ -2223,21 +2224,34 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm)
  * the spawned QEMU instance to all pCPUs if no map is given in
  * its config file */
 int hostcpus;
+cpumap = virProcessGetAffinity(vm->pid);
 
-/* setaffinity fails if you set bits for CPUs which
- * aren't present, so we have to limit ourselves */
-if ((hostcpus = virHostCPUGetCount()) < 0)
+if (virHostCPUHasBitmap())
+hostcpumap = virHostCPUGetOnlineBitmap();
+
+if (hostcpumap && cpumap && virBitmapEqual(hostcpumap, cpumap)) {
+/* we're using all available CPUs, no reason to set
+ * mask. If libvirtd is running without explicit
+ * affinity, we can use hotplugged CPUs for this VM */
+ret = 0;
 goto cleanup;
+} else {
+/* setaffinity fails if you set bits for CPUs which
+ * aren't present, so we have to limit ourselves */
+if ((hostcpus = virHostCPUGetCount()) < 0)
+goto cleanup;
 
-if (hostcpus > QEMUD_CPUMASK_LEN)
-hostcpus = QEMUD_CPUMASK_LEN;
+if (hostcpus > QEMUD_CPUMASK_LEN)
+hostcpus = QEMUD_CPUMASK_LEN;
 
-if (!(cpumap = virBitmapNew(hostcpus)))
-goto cleanup;
+virBitmapFree(cpumap);
+if (!(cpumap = virBitmapNew(hostcpus)))
+goto cleanup;
 
-virBitmapSetAll(cpumap);
+virBitmapSetAll(cpumap);
 
-cpumapToSet = cpumap;
+cpumapToSet = cpumap;
+}
 }
 }
 
@@ -2248,6 +2262,7 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm)
 
  cleanup:
 virBitmapFree(cpumap);
+virBitmapFree(hostcpumap);
 return ret;
 }
 
-- 
1.9.1

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

Re: [libvirt] [Qemu-ppc] [RFC PATCH qemu] spapr_pci: Create PCI-express root bus by default

[Andrea Bolognani]

On Wed, 2016-11-23 at 16:00 +1100, David Gibson wrote:
> > Existing libvirt versions assume that pseries guests have
> > a legacy PCI root bus, and will base their PCI address
> > allocation / PCI topology decisions on that fact: they
> > will, for example, use legacy PCI bridges.
> 
> Um.. yeah.. trouble is libvirt's PCI-E address allocation probably
> won't work for spapr PCI-E either, because of the weird PCI-E without
> root complex presentation we get in PAPR.

So, would the PCIe Root Bus in a pseries guest behave
differently than the one in a q35 or mach-virt guest?

Does it have a different number of slots, do we have to
plug different controllers into them, ...?

Regardless of how we decide to move forward with the
PCIe-enabled pseries machine type, libvirt will have to
know about this so it can behave appropriately.

> > > I believe after we introduced the very first
> > > pseries-pcie-X.Y, we will just stop adding new pseries-X.Y.
> > 
> > Isn't i440fx still being updated despite the fact that q35
> > exists? Granted, there are a lot more differences between
> > those two machine types than just the root bus type.
> 
> Right, there are heaps of differences between i440fx and q35, and
> reasons to keep both updated.  For pseries we have neither the impetus
> nor the resources to maintain two different machine type variant,
> where the only difference is between legacy PCI and weirdly presented
> PCI-E.

Calling the PCIe machine type either pseries-2.8 or
pseries-pcie-2.8 would result in the very same amount of
work, and in both cases it would be understood that the
legacy PCI machine type is no longer going to be updated,
but can still be used to run existing guests.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[libvirt] Libvirt domain event usage and consistency

[Roman Mohr]

Hi,

I recently started to use the libvirt domain events. With them I increase
the responsiveness of my VM state wachers.
In general it works pretty well. I just listen to the events and do a
periodic resync to cope with missed events.

While watching the events I ran into a few interesting situations I wanted
to share. The points 1-3 describe some minor issues or irregularities.
Point 4 is about the fact that domain and state updates are not versioned
which makes it very hard to stay in sync with libvirt when using events.

My libvirt version is 1.2.18.4.

1) Event order seems to be weird on startup:

When listening for VM lifecycle events I get this order:

{"event_type": "Started", "timestamp": "2016-11-25T11:59:53.209326Z",
"reason": "Booted", "domain_name": "generic", "domain_id":
"8ff7047b-fb46-44ff-a4c6-7c20c73ab86e"}
{"event_type": "Defined", "timestamp": "2016-11-25T11:59:53.435530Z",
"reason": "Added", "domain_name": "generic", "domain_id":
"8ff7047b-fb46-44ff-a4c6-7c20c73ab86e"}

It is strange that a VM already boots before it is defined. Is this the
intended order?

2) Defining a VM with VIR_DOMAIN_START_PAUSED gives me this event order

{"event_type": "Defined", "timestamp": "2016-11-25T12:02:44.037817Z",
"reason": "Added", "domain_name": "core_node", "domain_id":
"b9906489-6d5b-40f8-a742-ca71b2b84277"}
{"event_type": "Resumed", "timestamp": "2016-11-25T12:02:44.813104Z",
"reason": "Unpaused", "domain_name": "core_node", "domain_id":
"b9906489-6d5b-40f8-a742-ca71b2b84277"}
{"event_type": "Started", "timestamp": "2016-11-25T12:02:44.813733Z",
"reason": "Booted", "domain_name": "core_node", "domain_id":
"b9906489-6d5b-40f8-a742-ca71b2b84277"}

This boot-order makes it hard to track active domains by listening to
life-cycle events. One could theoretically still always fetch the VM state
in the event callback and check the state, but if the state is not
immediately transferred with the event itself, it can already be outdated,
so this might be racy (intransparent for the libvirt bindings user), and as
described in (3) currently not even possible. In general the real existing
events seem to differ quite significantly from the described life-cycle in
[1].

3) "Defined" event is triggered before the domain is completely defined

{"event_type": "Defined", "timestamp": "2016-11-25T12:02:44.037817Z",
"reason": "Added", "domain_name": "core_node", "domain_id":
"b9906489-6d5b-40f8-a742-ca71b2b84277"}
{"event_type": "Resumed", "timestamp": "2016-11-25T12:02:44.813104Z",
"reason": "Unpaused", "domain_name": "core_node", "domain_id":
"b9906489-6d5b-40f8-a742-ca71b2b84277"}
{"event_type": "Started", "timestamp": "2016-11-25T12:02:44.813733Z",
"reason": "Booted", "domain_name": "core_node", "domain_id":
"b9906489-6d5b-40f8-a742-ca71b2b84277"}

When I try to process the first event and do a xmldump I get:

   Event: [Code-42] [Domain-10] Domain not found: no domain with matching
uuid 'b9906489-6d5b-40f8-a742-ca71b2b84277' (core_node)

So it seems like I get the event before the domain is completely ready.

4) There libvirt domain description is not versioned

I would expect that every time I update a domainxml (update from third
party entity), or an event is generated (update from libvirt), that the
resource version of a Domain is increased and that I get this resource
version when I do a xmldump or when I get an event. Without this there is
afaik no way to stay in sync with libvirt, even if you do regular polling
of all domains. The main issue here is that I can never know if events in
the queue arrived before my latest domain resync or after it.

Also not that this is not about delivery guarantees of events. It is just
about having a consistent view of a VM and the individual event. If I have
resource versions, I can decide if an event is still interesting for me or
not, which is exactly what I need to solve the syncing problem above.
When I do a complete relisting of all domains to syn, I know which version
I got and I can then see on every event if it is newer or older.

If along side with the event, the domain xml, the VM state, and the
resource version would be sent to a client, it would be even better. Then,
whenever there is a new event for a VM in the queue, I can be sure that
this domainxml I see is the one which triggered the event. This xml is then
a complete representation for this revision number.


Would be nice to hear your thoughts to these points.

Best Regards,
Roman

[1]
https://wiki.libvirt.org/page/VM_lifecycle#States_that_a_guest_domain_can_be_in
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] libvirt: qemu: Fix domain termination caused by query-hotpluggable-cpus not enabled

[Boris Fiuczynski]

On 11/25/2016 10:07 AM, Peter Krempa wrote:

On Fri, Nov 25, 2016 at 10:03:38 +0100, Peter Krempa wrote:

On Fri, Nov 25, 2016 at 09:19:18 +0100, Boris Fiuczynski wrote:


[...]


 src/qemu/qemu_process.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f8f379a..675f5b5 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3349,8 +3349,7 @@ qemuProcessReconnect(void *opaque)
 /* If upgrading from old libvirtd we won't have found any
  * caps in the domain status, so re-query them


At reconnect the capabilities are taken from the status XML file, where
they are saved for every instance specifically. This code is supposed to
run


only when a very old version of libvirt did not save the capability
flags into the status XML. It's even explained in the comment above.




  */
-if (!priv->qemuCaps &&
-!(priv->qemuCaps = virQEMUCapsCacheLookupCopy(caps,
+if (!(priv->qemuCaps = virQEMUCapsCacheLookupCopy(caps,
   driver->qemuCapsCache,
   obj->def->emulator,
   obj->def->os.machine)))


NACK, this certainly is not the right fix. Does the status XML have the
'query-hotpluggable-cpus' capability set? If it's so then it was
probably mis-detected at start of the VM and should be fixed there.

If there is no such capability, then the reconnect code is broken
somehow.

At any rate we should not re-detect the capabilities if they were
reloaded properly from the XML.

Peter


Peter,
looking at your commit message of 920bbe5c it reads as follows:
qemu: capabilities: Extract availability of new cpu hotplug for 
machine types


QEMU reports whether 'query-hotpluggable-cpus' is supported for a given
machine type. Extract and cache the information using the capability
cache.

When copying the capabilities for a new start of qemu, mask out the
presence of QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS if the machine type
doesn't support hotpluggable cpus.

The loaded XML has the 'query-hotpluggable-cpus' capability set since 
the qmp command exists in the list returned by the qmp command 
query-commands by the qemu binary.
The machine type dependent masking of QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS 
you are doing for a new start of qemu seems therefore also required to 
be done for reconnecting to this running qemu domain. Are you saying 
that this is wrong and the machine type dependent masking result of the 
'query-hotpluggable-cpus' capability should be stored in the XML?



--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

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

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

Re: [libvirt] [PATCH 1/1] reset vcpu pin info from config for zero mask

[Martin Kletzander]

On Wed, Oct 19, 2016 at 02:39:46PM +0300, Konstantin Neumoin wrote:

The option for removing vcpu pinning information from config was added
in:
'7ea9778 vcpupin: add vcpupin resetting feature to qemu driver'


By the way, this hash is ambiguous, 7ea9778c is better, but I rather use
7ea9778c8a3d =)


and removed in:
'a02a161 qemu: libxl: vcpupin: Don't reset pinning when pinning to all pcpus'
by some reasons.



The reason is written in the config.  Pinning to all pCPUs is sometimes
wanted.  Also you cannot easily reset the pinning on a live domain.  You
can pin to all the CPUs, but after CPU is hotplugged (or becomes online)
in the host, you are yet again not pinned to all of them, and more
importantly, you are returning inconsistent information.


So, for now there is no way to remove vcpu pinning from config.


Well, there is.  But it's just a workaround.  You can pin it to all and
then edit the config.  I agree that it's not fun to do, though.


This patch returns options for remove vcpu/emulator pinning settings
from both configs if zero mask(mask filled by zeros) was specified.



This is not documented anywhere.  You should add that in the docs.
Although I would rather prefer maplength == 0 to trigger this, or
cpumask == NULL.  But that would have to be taken care of in all
drivers, so it's probably not a good idea.

Bottom line for me is, that I don't think we should allow an API for
which we cannot guarantee the results.  Until there is a way to reset
pinning reliably that the kernel provides, that is.

Martin


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

[libvirt] [PATCH v2 04/10] admin: Introduce virAdmConnectGetLoggingOutputs

[Erik Skultety]

Enable libvirt users to query logging output settings.

Signed-off-by: Erik Skultety 
---
 daemon/admin.c  | 37 +
 include/libvirt/libvirt-admin.h |  4 
 src/admin/admin_protocol.x  | 16 +++-
 src/admin/admin_remote.c| 35 +++
 src/admin_protocol-structs  |  8 
 src/libvirt-admin.c | 40 
 src/libvirt_admin_private.syms  |  2 ++
 src/libvirt_admin_public.syms   |  5 +
 8 files changed, 146 insertions(+), 1 deletion(-)

diff --git a/daemon/admin.c b/daemon/admin.c
index a3c8b89..4fa3851 100644
--- a/daemon/admin.c
+++ b/daemon/admin.c
@@ -383,4 +383,41 @@ adminDispatchServerSetClientLimits(virNetServerPtr server 
ATTRIBUTE_UNUSED,
 virObjectUnref(srv);
 return rv;
 }
+
+/* Returns the number of outputs stored in @outputs */
+static int
+adminConnectGetLoggingOutputs(char **outputs, unsigned int flags)
+{
+char *tmp = NULL;
+
+virCheckFlags(0, -1);
+
+if (!(tmp = virLogGetOutputs()))
+return -1;
+
+*outputs = tmp;
+return virLogGetNbOutputs();
+}
+
+static int
+adminDispatchConnectGetLoggingOutputs(virNetServerPtr server ATTRIBUTE_UNUSED,
+  virNetServerClientPtr client 
ATTRIBUTE_UNUSED,
+  virNetMessagePtr msg ATTRIBUTE_UNUSED,
+  virNetMessageErrorPtr rerr,
+  admin_connect_get_logging_outputs_args 
*args,
+  admin_connect_get_logging_outputs_ret 
*ret)
+{
+char *outputs = NULL;
+int noutputs = 0;
+
+if ((noutputs = adminConnectGetLoggingOutputs(, args->flags) < 0)) 
{
+virNetMessageSaveError(rerr);
+return -1;
+}
+
+VIR_STEAL_PTR(ret->outputs, outputs);
+ret->noutputs = noutputs;
+
+return 0;
+}
 #include "admin_dispatch.h"
diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
index c810be3..46d2155 100644
--- a/include/libvirt/libvirt-admin.h
+++ b/include/libvirt/libvirt-admin.h
@@ -404,6 +404,10 @@ int virAdmServerSetClientLimits(virAdmServerPtr srv,
 int nparams,
 unsigned int flags);
 
+int virAdmConnectGetLoggingOutputs(virAdmConnectPtr conn,
+   char **outputs,
+   unsigned int flags);
+
 # ifdef __cplusplus
 }
 # endif
diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x
index 5963233..237632a 100644
--- a/src/admin/admin_protocol.x
+++ b/src/admin/admin_protocol.x
@@ -183,6 +183,15 @@ struct admin_server_set_client_limits_args {
 unsigned int flags;
 };
 
+struct admin_connect_get_logging_outputs_args {
+unsigned int flags;
+};
+
+struct admin_connect_get_logging_outputs_ret {
+admin_nonnull_string outputs;
+unsigned int noutputs;
+};
+
 /* Define the program number, protocol version and procedure numbers here. */
 const ADMIN_PROGRAM = 0x06900690;
 const ADMIN_PROTOCOL_VERSION = 1;
@@ -268,5 +277,10 @@ enum admin_procedure {
 /**
  * @generate: none
  */
-ADMIN_PROC_SERVER_SET_CLIENT_LIMITS = 13
+ADMIN_PROC_SERVER_SET_CLIENT_LIMITS = 13,
+
+/**
+ * @generate: none
+ */
+ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 14
 };
diff --git a/src/admin/admin_remote.c b/src/admin/admin_remote.c
index f37ff5e..e472484 100644
--- a/src/admin/admin_remote.c
+++ b/src/admin/admin_remote.c
@@ -425,3 +425,38 @@ remoteAdminServerSetClientLimits(virAdmServerPtr srv,
 virObjectUnlock(priv);
 return rv;
 }
+
+static int
+remoteAdminConnectGetLoggingOutputs(virAdmConnectPtr conn,
+char **outputs,
+unsigned int flags)
+{
+int rv = -1;
+remoteAdminPrivPtr priv = conn->privateData;
+admin_connect_get_logging_outputs_args args;
+admin_connect_get_logging_outputs_ret ret;
+
+args.flags = flags;
+
+memset(, 0, sizeof(ret));
+virObjectLock(priv);
+
+if (call(conn,
+ 0,
+ ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS,
+ (xdrproc_t) xdr_admin_connect_get_logging_outputs_args,
+ (char *) ,
+ (xdrproc_t) xdr_admin_connect_get_logging_outputs_ret,
+ (char *) ) == -1)
+goto done;
+
+if (outputs)
+VIR_STEAL_PTR(*outputs, ret.outputs);
+
+rv = ret.noutputs;
+xdr_free((xdrproc_t) xdr_admin_connect_get_logging_outputs_ret, (char *) 
);
+
+ done:
+virObjectUnlock(priv);
+return rv;
+}
diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs
index 1437d9e..096bf5a 100644
--- a/src/admin_protocol-structs
+++ b/src/admin_protocol-structs
@@ -127,6 +127,13 @@ struct admin_server_set_client_limits_args {
 } params;
 u_int 

[libvirt] [PATCH v2 00/10] admin: Introduce runtime logging APIs

[Erik Skultety]

v1 here https://www.redhat.com/archives/libvir-list/2016-November/msg9.html

since v1:
- incorporated notes raised during review
- allowed passing of NULL via the public APIs
* behaves the same way as an empty string, but lead to even more code
  reduction in 'daemonSetupLogging' for practically no extra code to enable
  such feature
- added forgotten documentation
- updated NEWS file

Erik Skultety (10):
  virlog: Introduce virLog{Get,Set}DefaultOutput
  admin: Allow passing NULL to virLogSetFilters
  daemon: Hook up the virLog{Get,Set}DefaultOutput to the daemon's init
routine
  admin: Introduce virAdmConnectGetLoggingOutputs
  admin: Introduce virAdmConnectGetLoggingFilters
  admin: Introduce virAdmConnectSetLoggingOutputs
  admin: Introduce virAdmConnectSetLoggingFilters
  virt-admin: Wire-up the logging APIs
  admin: Add an example demonstrating how to use the logging APIs
  admin: Update the news file to include the new logging features

 .gitignore  |   1 +
 daemon/admin.c  | 104 ++
 daemon/libvirtd.c   |  96 
 docs/news.html.in   |   4 +
 examples/Makefile.am|   3 +-
 examples/admin/logging.c| 102 ++
 include/libvirt/libvirt-admin.h |  16 
 src/admin/admin_protocol.x  |  50 -
 src/admin/admin_remote.c|  70 ++
 src/admin_protocol-structs  |  26 +++
 src/libvirt-admin.c | 157 
 src/libvirt_admin_private.syms  |   6 ++
 src/libvirt_admin_public.syms   |   8 ++
 src/libvirt_private.syms|   2 +
 src/locking/lock_daemon.c   |  90 ---
 src/logging/log_daemon.c|  92 ---
 src/util/virlog.c   | 100 -
 src/util/virlog.h   |   4 +-
 tools/virt-admin.c  | 120 ++
 tools/virt-admin.pod|  90 +++
 20 files changed, 899 insertions(+), 242 deletions(-)
 create mode 100644 examples/admin/logging.c

-- 
2.5.5

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

[libvirt] [PATCH v2 07/10] admin: Introduce virAdmConnectSetLoggingFilters

[Erik Skultety]

Enable libvirt users to modify logging filters of a daemon from outside.

Signed-off-by: Erik Skultety 
---
 daemon/admin.c  | 10 ++
 include/libvirt/libvirt-admin.h |  4 
 src/admin/admin_protocol.x  | 12 +++-
 src/admin_protocol-structs  |  5 +
 src/libvirt-admin.c | 38 ++
 src/libvirt_admin_private.syms  |  1 +
 src/libvirt_admin_public.syms   |  1 +
 7 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/daemon/admin.c b/daemon/admin.c
index 16a3453..c5678bb 100644
--- a/daemon/admin.c
+++ b/daemon/admin.c
@@ -426,6 +426,16 @@ adminConnectSetLoggingOutputs(virNetDaemonPtr dmn 
ATTRIBUTE_UNUSED,
 }
 
 static int
+adminConnectSetLoggingFilters(virNetDaemonPtr dmn ATTRIBUTE_UNUSED,
+  const char *filters,
+  unsigned int flags)
+{
+virCheckFlags(0, -1);
+
+return virLogSetFilters(filters);
+}
+
+static int
 adminDispatchConnectGetLoggingOutputs(virNetServerPtr server ATTRIBUTE_UNUSED,
   virNetServerClientPtr client 
ATTRIBUTE_UNUSED,
   virNetMessagePtr msg ATTRIBUTE_UNUSED,
diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
index aa33fef..161727e 100644
--- a/include/libvirt/libvirt-admin.h
+++ b/include/libvirt/libvirt-admin.h
@@ -416,6 +416,10 @@ int virAdmConnectSetLoggingOutputs(virAdmConnectPtr conn,
const char *outputs,
unsigned int flags);
 
+int virAdmConnectSetLoggingFilters(virAdmConnectPtr conn,
+   const char *filters,
+   unsigned int flags);
+
 # ifdef __cplusplus
 }
 # endif
diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x
index 1d2116d..d19d132 100644
--- a/src/admin/admin_protocol.x
+++ b/src/admin/admin_protocol.x
@@ -206,6 +206,11 @@ struct admin_connect_set_logging_outputs_args {
 unsigned int flags;
 };
 
+struct admin_connect_set_logging_filters_args {
+admin_string filters;
+unsigned int flags;
+};
+
 /* Define the program number, protocol version and procedure numbers here. */
 const ADMIN_PROGRAM = 0x06900690;
 const ADMIN_PROTOCOL_VERSION = 1;
@@ -306,5 +311,10 @@ enum admin_procedure {
 /**
  * @generate: both
  */
-ADMIN_PROC_CONNECT_SET_LOGGING_OUTPUTS = 16
+ADMIN_PROC_CONNECT_SET_LOGGING_OUTPUTS = 16,
+
+/**
+ * @generate: both
+ */
+ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 17
 };
diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs
index 04bbd7a..a6c5380 100644
--- a/src/admin_protocol-structs
+++ b/src/admin_protocol-structs
@@ -145,6 +145,10 @@ struct admin_connect_set_logging_outputs_args {
 admin_string   outputs;
 u_int  flags;
 };
+struct admin_connect_set_logging_filters_args {
+admin_string   filters;
+u_int  flags;
+};
 enum admin_procedure {
 ADMIN_PROC_CONNECT_OPEN = 1,
 ADMIN_PROC_CONNECT_CLOSE = 2,
@@ -162,4 +166,5 @@ enum admin_procedure {
 ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 14,
 ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS = 15,
 ADMIN_PROC_CONNECT_SET_LOGGING_OUTPUTS = 16,
+ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 17,
 };
diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c
index 2bc8b2c..67d9b6b 100644
--- a/src/libvirt-admin.c
+++ b/src/libvirt-admin.c
@@ -1203,3 +1203,41 @@ virAdmConnectSetLoggingOutputs(virAdmConnectPtr conn,
 virDispatchError(NULL);
 return -1;
 }
+
+/**
+ * virAdmConnectSetLoggingFilters:
+ * @conn: pointer to an active admin connection
+ * @filters: pointer to a string containing a list of filters to be defined
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Redefine the existing (set of) filter(s) with a new one specified in
+ * @filters. If multiple filters are specified, they need to be delimited by
+ * spaces. The format of each filter must conform to the format described in
+ * daemon's configuration file (e.g. libvirtd.conf).
+ *
+ * To clear the currently defined (set of) filter(s), pass either an empty
+ * string ("") or NULL in @filters.
+ *
+ * Returns 0 if the new filter or the set of filters has been defined
+ * successfully, or -1 in case of an error.
+ */
+int
+virAdmConnectSetLoggingFilters(virAdmConnectPtr conn,
+   const char *filters,
+   unsigned int flags)
+{
+int ret = -1;
+
+VIR_DEBUG("conn=%p, flags=%x", conn, flags);
+
+virResetLastError();
+virCheckAdmConnectReturn(conn, -1);
+
+if ((ret = remoteAdminConnectSetLoggingFilters(conn, filters, flags)) < 0)
+goto error;
+
+return ret;
+ error:
+virDispatchError(NULL);
+return -1;
+}
diff 

[libvirt] [PATCH v2 02/10] admin: Allow passing NULL to virLogSetFilters

[Erik Skultety]

Along with an empty string, it should also be possible for users to pass NULL
to the public APIs which in turn would trigger a routine (future work)
responsible for defining an appropriate default logging output given the current
circumstances.

Signed-off-by: Erik Skultety 
---
 daemon/libvirtd.c | 2 +-
 src/locking/lock_daemon.c | 2 +-
 src/logging/log_daemon.c  | 2 +-
 src/util/virlog.c | 8 +++-
 src/util/virlog.h | 2 +-
 5 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index cd25b50..3902a8b 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -693,7 +693,7 @@ daemonSetupLogging(struct daemonConfig *config,
 if (virLogGetNbFilters() == 0)
 virLogSetFilters(config->log_filters);
 
-if (config->log_outputs && virLogGetNbOutputs() == 0)
+if (virLogGetNbOutputs() == 0)
 virLogSetOutputs(config->log_outputs);
 
 /*
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 02745be..9ee818e 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -478,7 +478,7 @@ virLockDaemonSetupLogging(virLockDaemonConfigPtr config,
 if (virLogGetNbFilters() == 0)
 virLogSetFilters(config->log_filters);
 
-if (config->log_outputs && virLogGetNbOutputs() == 0)
+if (virLogGetNbOutputs() == 0)
 virLogSetOutputs(config->log_outputs);
 
 /*
diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c
index 04bb836..a9aebdb 100644
--- a/src/logging/log_daemon.c
+++ b/src/logging/log_daemon.c
@@ -406,7 +406,7 @@ virLogDaemonSetupLogging(virLogDaemonConfigPtr config,
 if (virLogGetNbFilters() == 0)
 virLogSetFilters(config->log_filters);
 
-if (config->log_outputs && virLogGetNbOutputs() == 0)
+if (virLogGetNbOutputs() == 0)
 virLogSetOutputs(config->log_outputs);
 
 /*
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 9b52e66..acd2285 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1818,6 +1818,8 @@ virLogParseFilters(const char *src, virLogFilterPtr 
**filters)
  * @outputs: string defining a (set of) output(s)
  *
  * Replaces the current set of defined outputs with a new set of outputs.
+ * Should the set be empty or NULL, a default output is used according to the
+ * daemon's runtime attributes.
  *
  * Returns 0 on success or -1 in case of an error.
  */
@@ -1826,12 +1828,16 @@ virLogSetOutputs(const char *src)
 {
 int ret = -1;
 int noutputs = 0;
+const char *outputstr = virLogDefaultOutput;
 virLogOutputPtr *outputs = NULL;
 
 if (virLogInitialize() < 0)
 return -1;
 
-if ((noutputs = virLogParseOutputs(src, )) < 0)
+if (src && *src)
+outputstr = src;
+
+if ((noutputs = virLogParseOutputs(outputstr, )) < 0)
 goto cleanup;
 
 if (virLogDefineOutputs(outputs, noutputs) < 0)
diff --git a/src/util/virlog.h b/src/util/virlog.h
index b4ffeca..cc09f48 100644
--- a/src/util/virlog.h
+++ b/src/util/virlog.h
@@ -187,7 +187,7 @@ void virLogOutputFree(virLogOutputPtr output);
 void virLogOutputListFree(virLogOutputPtr *list, int count);
 void virLogFilterFree(virLogFilterPtr filter);
 void virLogFilterListFree(virLogFilterPtr *list, int count);
-int virLogSetOutputs(const char *outputs) ATTRIBUTE_NONNULL(1);
+int virLogSetOutputs(const char *outputs);
 int virLogSetFilters(const char *filters);
 char *virLogGetDefaultOutput(void);
 int virLogSetDefaultOutput(const char *fname, bool godaemon, bool privileged);
-- 
2.5.5

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

[libvirt] [PATCH v2 10/10] admin: Update the news file to include the new logging features

[Erik Skultety]

Signed-off-by: Erik Skultety 
---
 docs/news.html.in | 4 
 1 file changed, 4 insertions(+)

diff --git a/docs/news.html.in b/docs/news.html.in
index 59bf20d..f2a65f5 100644
--- a/docs/news.html.in
+++ b/docs/news.html.in
@@ -31,6 +31,10 @@
   Add the capability to pass through a scsi_host HBA and the
   associated LUNs to the guest.
   
+  admin-logging: Add support for runtime logging settings 
adjustment
+  Logging-related settings like log outputs and filters can now be
+  adjusted during runtime without the necessity of the daemon's 
restart.
+  
 
   
   Improvements
-- 
2.5.5

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

[libvirt] [PATCH v2 09/10] admin: Add an example demonstrating how to use the logging APIs

[Erik Skultety]

Provide a simple C example demonstrating the use of both query APIs as well as
setter APIs.

Signed-off-by: Erik Skultety 
---
 .gitignore   |   1 +
 examples/Makefile.am |   3 +-
 examples/admin/logging.c | 102 +++
 3 files changed, 105 insertions(+), 1 deletion(-)
 create mode 100644 examples/admin/logging.c

diff --git a/.gitignore b/.gitignore
index 879ec24..984ad07 100644
--- a/.gitignore
+++ b/.gitignore
@@ -79,6 +79,7 @@
 /examples/admin/client_limits
 /examples/admin/list_clients
 /examples/admin/list_servers
+/examples/admin/logging
 /examples/admin/threadpool_params
 /examples/object-events/event-test
 /examples/dominfo/info1
diff --git a/examples/Makefile.am b/examples/Makefile.am
index 7cb8258..2956e14 100644
--- a/examples/Makefile.am
+++ b/examples/Makefile.am
@@ -43,7 +43,7 @@ noinst_PROGRAMS=dominfo/info1 dommigrate/dommigrate 
domsuspend/suspend \
domtop/domtop hellolibvirt/hellolibvirt object-events/event-test \
openauth/openauth rename/rename admin/list_servers admin/list_clients \
admin/threadpool_params admin/client_limits admin/client_info \
-   admin/client_close
+   admin/client_close admin/logging
 
 dominfo_info1_SOURCES = dominfo/info1.c
 dommigrate_dommigrate_SOURCES = dommigrate/dommigrate.c
@@ -65,6 +65,7 @@ admin_threadpool_params_SOURCES = admin/threadpool_params.c
 admin_client_limits_SOURCES = admin/client_limits.c
 admin_client_info_SOURCES = admin/client_info.c
 admin_client_close_SOURCES = admin/client_close.c
+admin_logging_SOURCES = admin/logging.c
 
 if WITH_APPARMOR_PROFILES
 apparmordir = $(sysconfdir)/apparmor.d/
diff --git a/examples/admin/logging.c b/examples/admin/logging.c
new file mode 100644
index 000..e28c7d5
--- /dev/null
+++ b/examples/admin/logging.c
@@ -0,0 +1,102 @@
+#include
+#include
+#include
+
+#include "config.h"
+#include
+#include
+#include
+
+static void printHelp(const char *argv0)
+{
+fprintf(stderr,
+("Usage:\n"
+  "  %s [options]\n"
+  "\n"
+  "Options:\n"
+  "  -h  Print this message.\n"
+  "  -o [string] Specify new log outputs.\n"
+  "  -f [string] Specify new log filters.\n"
+  "\n"),
+argv0);
+}
+
+int main(int argc, char **argv)
+{
+int ret, c;
+virAdmConnectPtr conn = NULL;
+char *get_outputs = NULL;
+char *get_filters = NULL;
+const char *set_outputs = NULL;
+const char *set_filters = NULL;
+
+ret = c = -1;
+opterr = 0;
+
+while ((c = getopt(argc, argv, ":hpo:f:")) > 0) {
+switch (c) {
+case 'h':
+printHelp(argv[0]);
+exit(EXIT_SUCCESS);
+case 'o':
+set_outputs = optarg;
+break;
+case 'f':
+set_filters = optarg;
+break;
+case ':':
+fprintf(stderr, "Missing argument for option -%c\n", optopt);
+exit(EXIT_FAILURE);
+case '?':
+fprintf(stderr, "Unrecognized option '-%c'\n", optopt);
+exit(EXIT_FAILURE);
+}
+}
+
+/* first, open a connection to the daemon */
+if (!(conn = virAdmConnectOpen(NULL, 0)))
+goto cleanup;
+
+/* get the currently defined log outputs and filters */
+if (virAdmConnectGetLoggingOutputs(conn, _outputs, 0) < 0 ||
+virAdmConnectGetLoggingFilters(conn, _filters, 0) < 0)
+goto cleanup;
+
+fprintf(stdout,
+"Current settings:\n"
+" outputs: %s\n"
+" filters: %s\n"
+"\n",
+get_outputs, get_filters ? get_filters : "None");
+
+free(get_outputs);
+free(get_filters);
+
+/* now, try to change the redefine the current log output and filters */
+if (virAdmConnectSetLoggingOutputs(conn, set_outputs, 0) < 0)
+goto cleanup;
+
+if (virAdmConnectSetLoggingFilters(conn, set_filters, 0) < 0)
+goto cleanup;
+
+/* get the currently defined log outputs and filters */
+if (virAdmConnectGetLoggingOutputs(conn, _outputs, 0) < 0 ||
+virAdmConnectGetLoggingFilters(conn, _filters, 0) < 0)
+goto cleanup;
+
+fprintf(stdout,
+"New settings:\n"
+" outputs: %s\n"
+" filters: %s\n"
+"\n",
+get_outputs ? get_outputs : "Default",
+get_filters ? get_filters : "None");
+
+free(get_outputs);
+free(get_filters);
+
+ret = 0;
+ cleanup:
+virAdmConnectClose(conn);
+return ret;
+}
-- 
2.5.5

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

[libvirt] [PATCH v2 01/10] virlog: Introduce virLog{Get, Set}DefaultOutput

[Erik Skultety]

These helpers will manage the log destination defaults (fetch/set). The reason
for this is to stay consistent with the current daemon's behaviour with respect
to /etc/libvirt/.conf file, since both assignment of an empty string
or not setting the log output variable at all trigger the daemon's decision on
the default log destination which depends on whether the daemon runs daemonized
or not.
This patch also changes the logic of the selection of the default
logging output compared to how it is done now. The main difference though is
that we should only really care if we're running daemonized or not, disregarding
the fact of (not) having a TTY completely (introduced by commit eba36a3878) as
that should be of the libvirtd's parent concern (what FD it will pass to it).

 Before:
 if (godaemon || !hasTTY):
 if (journald):
 use journald

 if (godaemon):
 if (privileged):
 use SYSCONFIG/libvirtd.log
 else:
 use XDG_CONFIG_HOME/libvirtd.log
 else:
 use stderr

 After:
 if (godaemon):
 if (journald):
 use journald

 else:
 if (privileged):
 use SYSCONFIG/libvirtd.log
 else:
 use XDG_CONFIG_HOME/libvirtd.log
 else:
 use stderr

Signed-off-by: Erik Skultety 
---
 src/libvirt_private.syms |  2 ++
 src/util/virlog.c| 92 
 src/util/virlog.h|  2 ++
 3 files changed, 96 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a04ba23..803458f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1883,6 +1883,7 @@ virLogFilterFree;
 virLogFilterListFree;
 virLogFilterNew;
 virLogFindOutput;
+virLogGetDefaultOutput;
 virLogGetDefaultPriority;
 virLogGetFilters;
 virLogGetNbFilters;
@@ -1901,6 +1902,7 @@ virLogParseOutputs;
 virLogPriorityFromSyslog;
 virLogProbablyLogMessage;
 virLogReset;
+virLogSetDefaultOutput;
 virLogSetDefaultPriority;
 virLogSetFilters;
 virLogSetFromEnv;
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 8f831fc..9b52e66 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -50,6 +50,7 @@
 #include "virtime.h"
 #include "intprops.h"
 #include "virstring.h"
+#include "configmake.h"
 
 /* Journald output is only supported on Linux new enough to expose
  * htole64.  */
@@ -105,6 +106,7 @@ struct _virLogOutput {
 char *name;
 };
 
+static char *virLogDefaultOutput;
 static virLogOutputPtr *virLogOutputs;
 static size_t virLogNbOutputs;
 
@@ -147,6 +149,96 @@ virLogUnlock(void)
 }
 
 
+static int
+virLogSetDefaultOutputToStderr(void)
+{
+return virAsprintf(, "%d:stderr",
+   virLogDefaultPriority);
+}
+
+
+static int
+virLogSetDefaultOutputToJournald(void)
+{
+virLogPriority priority = virLogDefaultPriority;
+
+/* By default we don't want to log too much stuff into journald as
+ * it may employ rate limiting and thus block libvirt execution. */
+if (priority == VIR_LOG_DEBUG)
+priority = VIR_LOG_INFO;
+
+return virAsprintf(, "%d:journald", priority);
+}
+
+
+static int
+virLogSetDefaultOutputToFile(const char *filename, bool privileged)
+{
+int ret = -1;
+char *logdir = NULL;
+mode_t old_umask;
+
+if (privileged) {
+if (virAsprintf(,
+"%d:file:%s/log/libvirt/%s", virLogDefaultPriority,
+LOCALSTATEDIR, filename) < 0)
+goto cleanup;
+} else {
+if (!(logdir = virGetUserCacheDirectory()))
+goto cleanup;
+
+old_umask = umask(077);
+if (virFileMakePath(logdir) < 0) {
+umask(old_umask);
+goto cleanup;
+}
+umask(old_umask);
+
+if (virAsprintf(, "%d:file:%s/%s",
+virLogDefaultPriority, logdir, filename) < 0)
+goto cleanup;
+}
+
+ret = 0;
+ cleanup:
+VIR_FREE(logdir);
+return ret;
+}
+
+
+/*
+ * virLogSetDefaultOutput:
+ * @filename: the file that the output should be redirected to (only needed
+ *when @godaemon equals true
+ * @godaemon: whether we're running daemonized
+ * @privileged: whether we're running with root privileges or not (session)
+ *
+ * Decides on what the default output (journald, file, stderr) should be
+ * according to @filename, @godaemon, @privileged. This function should be run
+ * exactly once at daemon startup, so no locks are used.
+ *
+ * Returns 0 on success, -1 in case of a failure.
+ */
+int
+virLogSetDefaultOutput(const char *filename, bool godaemon, bool privileged)
+{
+if (!godaemon)
+return virLogSetDefaultOutputToStderr();
+
+if (access("/run/systemd/journal/socket", W_OK) >= 0)
+return virLogSetDefaultOutputToJournald();
+
+return virLogSetDefaultOutputToFile(filename, privileged);
+}
+
+
+char *
+virLogGetDefaultOutput(void)
+{
+return virLogDefaultOutput;
+}
+
+
 static const char *
 virLogPriorityString(virLogPriority 

[libvirt] [PATCH v2 06/10] admin: Introduce virAdmConnectSetLoggingOutputs

[Erik Skultety]

Enable libvirt users to modify daemon's logging output settings from outside.
If either an empty string or NULL is passed, a default logging output will be
used the same way as it would be in case writing an empty string to the
libvirtd.conf

Signed-off-by: Erik Skultety 
---
 daemon/admin.c  | 10 ++
 include/libvirt/libvirt-admin.h |  4 
 src/admin/admin_protocol.x  | 12 +++-
 src/admin_protocol-structs  |  5 +
 src/libvirt-admin.c | 38 ++
 src/libvirt_admin_private.syms  |  1 +
 src/libvirt_admin_public.syms   |  1 +
 7 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/daemon/admin.c b/daemon/admin.c
index f3bc1de..16a3453 100644
--- a/daemon/admin.c
+++ b/daemon/admin.c
@@ -416,6 +416,16 @@ adminConnectGetLoggingFilters(char **filters, unsigned int 
flags)
 }
 
 static int
+adminConnectSetLoggingOutputs(virNetDaemonPtr dmn ATTRIBUTE_UNUSED,
+  const char *outputs,
+  unsigned int flags)
+{
+virCheckFlags(0, -1);
+
+return virLogSetOutputs(outputs);
+}
+
+static int
 adminDispatchConnectGetLoggingOutputs(virNetServerPtr server ATTRIBUTE_UNUSED,
   virNetServerClientPtr client 
ATTRIBUTE_UNUSED,
   virNetMessagePtr msg ATTRIBUTE_UNUSED,
diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
index d76ac20..aa33fef 100644
--- a/include/libvirt/libvirt-admin.h
+++ b/include/libvirt/libvirt-admin.h
@@ -412,6 +412,10 @@ int virAdmConnectGetLoggingFilters(virAdmConnectPtr conn,
char **filters,
unsigned int flags);
 
+int virAdmConnectSetLoggingOutputs(virAdmConnectPtr conn,
+   const char *outputs,
+   unsigned int flags);
+
 # ifdef __cplusplus
 }
 # endif
diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x
index 8316bc4..1d2116d 100644
--- a/src/admin/admin_protocol.x
+++ b/src/admin/admin_protocol.x
@@ -201,6 +201,11 @@ struct admin_connect_get_logging_filters_ret {
 unsigned int nfilters;
 };
 
+struct admin_connect_set_logging_outputs_args {
+admin_string outputs;
+unsigned int flags;
+};
+
 /* Define the program number, protocol version and procedure numbers here. */
 const ADMIN_PROGRAM = 0x06900690;
 const ADMIN_PROTOCOL_VERSION = 1;
@@ -296,5 +301,10 @@ enum admin_procedure {
 /**
  * @generate: none
  */
-ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS = 15
+ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS = 15,
+
+/**
+ * @generate: both
+ */
+ADMIN_PROC_CONNECT_SET_LOGGING_OUTPUTS = 16
 };
diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs
index c172557..04bbd7a 100644
--- a/src/admin_protocol-structs
+++ b/src/admin_protocol-structs
@@ -141,6 +141,10 @@ struct admin_connect_get_logging_filters_ret {
 admin_string   filters;
 u_int  nfilters;
 };
+struct admin_connect_set_logging_outputs_args {
+admin_string   outputs;
+u_int  flags;
+};
 enum admin_procedure {
 ADMIN_PROC_CONNECT_OPEN = 1,
 ADMIN_PROC_CONNECT_CLOSE = 2,
@@ -157,4 +161,5 @@ enum admin_procedure {
 ADMIN_PROC_SERVER_SET_CLIENT_LIMITS = 13,
 ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 14,
 ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS = 15,
+ADMIN_PROC_CONNECT_SET_LOGGING_OUTPUTS = 16,
 };
diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c
index 29754a8..2bc8b2c 100644
--- a/src/libvirt-admin.c
+++ b/src/libvirt-admin.c
@@ -1165,3 +1165,41 @@ virAdmConnectGetLoggingFilters(virAdmConnectPtr conn,
 virDispatchError(NULL);
 return -1;
 }
+
+/**
+ * virAdmConnectSetLoggingOutputs:
+ * @conn: pointer to an active admin connection
+ * @outputs: pointer to a string containing a list of outputs to be defined
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Redefine the existing (set of) outputs(s) with a new one specified in
+ * @outputs. If multiple outputs are specified, they need to be delimited by
+ * spaces. The format of each output must conform to the format described in
+ * daemon's configuration file (e.g. libvirtd.conf).
+ *
+ * To reset the existing (set of) output(s) to libvirt's defaults, an empty
+ * string ("") or NULL should be passed in @outputs.
+ *
+ * Returns 0 if the new output or the set of outputs has been defined
+ * successfully, or -1 in case of an error.
+ */
+int
+virAdmConnectSetLoggingOutputs(virAdmConnectPtr conn,
+   const char *outputs,
+   unsigned int flags)
+{
+int ret = -1;
+
+VIR_DEBUG("conn=%p, outputs=%s, flags=%x", conn, outputs, flags);
+
+virResetLastError();
+

[libvirt] [PATCH v2 05/10] admin: Introduce virAdmConnectGetLoggingFilters

[Erik Skultety]

Enable libvirt users to query logging filter settings.

Signed-off-by: Erik Skultety 
---
 daemon/admin.c  | 47 +
 include/libvirt/libvirt-admin.h |  4 
 src/admin/admin_protocol.x  | 16 +-
 src/admin/admin_remote.c| 35 ++
 src/admin_protocol-structs  |  8 +++
 src/libvirt-admin.c | 41 +++
 src/libvirt_admin_private.syms  |  2 ++
 src/libvirt_admin_public.syms   |  1 +
 8 files changed, 153 insertions(+), 1 deletion(-)

diff --git a/daemon/admin.c b/daemon/admin.c
index 4fa3851..f3bc1de 100644
--- a/daemon/admin.c
+++ b/daemon/admin.c
@@ -399,6 +399,22 @@ adminConnectGetLoggingOutputs(char **outputs, unsigned int 
flags)
 return virLogGetNbOutputs();
 }
 
+/* Returns the number of defined filters or -1 in case of an error */
+static int
+adminConnectGetLoggingFilters(char **filters, unsigned int flags)
+{
+char *tmp = NULL;
+int ret = 0;
+
+virCheckFlags(0, -1);
+
+if ((ret = virLogGetNbFilters()) > 0 && !(tmp = virLogGetFilters()))
+return -1;
+
+*filters = tmp;
+return ret;
+}
+
 static int
 adminDispatchConnectGetLoggingOutputs(virNetServerPtr server ATTRIBUTE_UNUSED,
   virNetServerClientPtr client 
ATTRIBUTE_UNUSED,
@@ -420,4 +436,35 @@ adminDispatchConnectGetLoggingOutputs(virNetServerPtr 
server ATTRIBUTE_UNUSED,
 
 return 0;
 }
+
+static int
+adminDispatchConnectGetLoggingFilters(virNetServerPtr server ATTRIBUTE_UNUSED,
+  virNetServerClientPtr client 
ATTRIBUTE_UNUSED,
+  virNetMessagePtr msg ATTRIBUTE_UNUSED,
+  virNetMessageErrorPtr rerr,
+  admin_connect_get_logging_filters_args 
*args,
+  admin_connect_get_logging_filters_ret 
*ret)
+{
+char *filters = NULL;
+int nfilters = 0;
+
+if ((nfilters = adminConnectGetLoggingFilters(, args->flags)) < 0) 
{
+virNetMessageSaveError(rerr);
+return -1;
+}
+
+if (nfilters == 0) {
+ret->filters = NULL;
+} else {
+char **ret_filters = NULL;
+if (VIR_ALLOC(ret_filters) < 0)
+return -1;
+
+*ret_filters = filters;
+ret->filters = ret_filters;
+}
+ret->nfilters = nfilters;
+
+return 0;
+}
 #include "admin_dispatch.h"
diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
index 46d2155..d76ac20 100644
--- a/include/libvirt/libvirt-admin.h
+++ b/include/libvirt/libvirt-admin.h
@@ -408,6 +408,10 @@ int virAdmConnectGetLoggingOutputs(virAdmConnectPtr conn,
char **outputs,
unsigned int flags);
 
+int virAdmConnectGetLoggingFilters(virAdmConnectPtr conn,
+   char **filters,
+   unsigned int flags);
+
 # ifdef __cplusplus
 }
 # endif
diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x
index 237632a..8316bc4 100644
--- a/src/admin/admin_protocol.x
+++ b/src/admin/admin_protocol.x
@@ -192,6 +192,15 @@ struct admin_connect_get_logging_outputs_ret {
 unsigned int noutputs;
 };
 
+struct admin_connect_get_logging_filters_args {
+unsigned int flags;
+};
+
+struct admin_connect_get_logging_filters_ret {
+admin_string filters;
+unsigned int nfilters;
+};
+
 /* Define the program number, protocol version and procedure numbers here. */
 const ADMIN_PROGRAM = 0x06900690;
 const ADMIN_PROTOCOL_VERSION = 1;
@@ -282,5 +291,10 @@ enum admin_procedure {
 /**
  * @generate: none
  */
-ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 14
+ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 14,
+
+/**
+ * @generate: none
+ */
+ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS = 15
 };
diff --git a/src/admin/admin_remote.c b/src/admin/admin_remote.c
index e472484..b29d109 100644
--- a/src/admin/admin_remote.c
+++ b/src/admin/admin_remote.c
@@ -460,3 +460,38 @@ remoteAdminConnectGetLoggingOutputs(virAdmConnectPtr conn,
 virObjectUnlock(priv);
 return rv;
 }
+
+static int
+remoteAdminConnectGetLoggingFilters(virAdmConnectPtr conn,
+char **filters,
+unsigned int flags)
+{
+int rv = -1;
+remoteAdminPrivPtr priv = conn->privateData;
+admin_connect_get_logging_filters_args args;
+admin_connect_get_logging_filters_ret ret;
+
+args.flags = flags;
+
+memset(, 0, sizeof(ret));
+virObjectLock(priv);
+
+if (call(conn,
+ 0,
+ ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS,
+ (xdrproc_t) xdr_admin_connect_get_logging_filters_args,
+ (char *) ,
+ (xdrproc_t) xdr_admin_connect_get_logging_filters_ret,
+

[libvirt] [PATCH v2 08/10] virt-admin: Wire-up the logging APIs

[Erik Skultety]

Finally, now that all APIs have been introduced, wire them up to virt-admin
and introduce daemon-log-outputs and daemon-log-filters commands.

Signed-off-by: Erik Skultety 
---
 tools/virt-admin.c   | 120 +++
 tools/virt-admin.pod |  90 ++
 2 files changed, 210 insertions(+)

diff --git a/tools/virt-admin.c b/tools/virt-admin.c
index 70b0e51..0b9945a 100644
--- a/tools/virt-admin.c
+++ b/tools/virt-admin.c
@@ -971,6 +971,114 @@ cmdSrvClientsSet(vshControl *ctl, const vshCmd *cmd)
 goto cleanup;
 }
 
+/* --
+ * Command daemon-log-filters
+ * --
+ */
+static const vshCmdInfo info_daemon_log_filters[] = {
+{.name = "help",
+ .data = N_("fetch or set the currently defined set of logging filters on "
+"daemon")
+},
+{.name = "desc",
+ .data = N_("Depending on whether run with or without options, the command 
"
+"fetches or redefines the existing active set of filters on "
+"daemon.")
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_daemon_log_filters[] = {
+{.name = "filters",
+ .type = VSH_OT_STRING,
+ .help = N_("redefine the existing set of logging filters"),
+ .flags = VSH_OFLAG_EMPTY_OK
+},
+{.name = NULL}
+};
+
+static bool
+cmdDaemonLogFilters(vshControl *ctl, const vshCmd *cmd)
+{
+int nfilters;
+char *filters = NULL;
+vshAdmControlPtr priv = ctl->privData;
+
+if (vshCommandOptBool(cmd, "filters")) {
+if ((vshCommandOptStringReq(ctl, cmd, "filters",
+(const char **) ) < 0 ||
+ virAdmConnectSetLoggingFilters(priv->conn, filters, 0) < 0)) {
+vshError(ctl, _("Unable to change daemon logging settings"));
+return false;
+}
+} else {
+if ((nfilters = virAdmConnectGetLoggingFilters(priv->conn,
+   , 0)) < 0) {
+vshError(ctl, _("Unable to get daemon logging filters 
information"));
+return false;
+}
+
+vshPrintExtra(ctl, " %-15s", _("Logging filters: "));
+vshPrint(ctl, "%s\n", filters ? filters : "");
+}
+
+return true;
+}
+
+/* --
+ * Command daemon-log-outputs
+ * --
+ */
+static const vshCmdInfo info_daemon_log_outputs[] = {
+{.name = "help",
+ .data = N_("fetch or set the currently defined set of logging outputs on "
+"daemon")
+},
+{.name = "desc",
+ .data = N_("Depending on whether run with or without options, the command 
"
+"fetches or redefines the existing active set of outputs on "
+"daemon.")
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_daemon_log_outputs[] = {
+{.name = "outputs",
+ .type = VSH_OT_STRING,
+ .help = N_("redefine the existing set of logging outputs"),
+ .flags = VSH_OFLAG_EMPTY_OK
+},
+{.name = NULL}
+};
+
+static bool
+cmdDaemonLogOutputs(vshControl *ctl, const vshCmd *cmd)
+{
+int noutputs;
+char *outputs = NULL;
+vshAdmControlPtr priv = ctl->privData;
+
+if (vshCommandOptBool(cmd, "outputs")) {
+if ((vshCommandOptStringReq(ctl, cmd, "outputs",
+(const char **) ) < 0 ||
+ virAdmConnectSetLoggingOutputs(priv->conn, outputs, 0) < 0)) {
+vshError(ctl, _("Unable to change daemon logging settings"));
+return false;
+}
+} else {
+if ((noutputs = virAdmConnectGetLoggingOutputs(priv->conn,
+   , 0)) < 0) {
+vshError(ctl, _("Unable to get daemon logging filters 
information"));
+return false;
+}
+
+vshPrintExtra(ctl, " %-15s", _("Logging outputs: "));
+vshPrint(ctl, "%s\n", outputs ? outputs : "");
+}
+
+return true;
+}
+
 static void *
 vshAdmConnectionHandler(vshControl *ctl)
 {
@@ -1341,6 +1449,18 @@ static const vshCmdDef managementCmds[] = {
  .info = info_srv_clients_set,
  .flags = 0
 },
+{.name = "daemon-log-filters",
+ .handler = cmdDaemonLogFilters,
+ .opts = opts_daemon_log_filters,
+ .info = info_daemon_log_filters,
+ .flags = 0
+},
+{.name = "daemon-log-outputs",
+ .handler = cmdDaemonLogOutputs,
+ .opts = opts_daemon_log_outputs,
+ .info = info_daemon_log_outputs,
+ .flags = 0
+},
 {.name = NULL}
 };
 
diff --git a/tools/virt-admin.pod b/tools/virt-admin.pod
index 443730e..d3f1a35 100644
--- a/tools/virt-admin.pod
+++ b/tools/virt-admin.pod
@@ -155,6 +155,96 @@ change its internal configuration.
 Lists all manageable servers contained within the daemon the client is
 currently connected to.
 
+=item B [I<--outputs> B] [I<--filters> B]
+
+Print the 

[libvirt] [PATCH v2 03/10] daemon: Hook up the virLog{Get, Set}DefaultOutput to the daemon's init routine

[Erik Skultety]

Now that virLog{Get,Set}DefaultOutput routines are introduced we can wire them
up to the daemon's logging initialization code. Also, change the order of
operations a bit so that we still strictly honor our precedence of settings:
cmdline > env > config now that outputs and filters are not appended anymore.

Signed-off-by: Erik Skultety 
---
 daemon/libvirtd.c | 96 +++
 src/locking/lock_daemon.c | 90 +++-
 src/logging/log_daemon.c  | 92 +++--
 3 files changed, 40 insertions(+), 238 deletions(-)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 3902a8b..b2e89fd 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -675,103 +675,33 @@ daemonSetupLogging(struct daemonConfig *config,
  * Libvirtd's order of precedence is:
  * cmdline > environment > config
  *
- * In order to achieve this, we must process configuration in
- * different order for the log level versus the filters and
- * outputs. Because filters and outputs append, we have to look at
- * the environment first and then only check the config file if
- * there was no result from the environment. The default output is
- * then applied only if there was no setting from either of the
- * first two. Because we don't have a way to determine if the log
- * level has been set, we must process variables in the opposite
+ * The default output is applied only if there was no setting from either
+ * the config or the environment. Because we don't have a way to determine
+ * if the log level has been set, we must process variables in the opposite
  * order, each one overriding the previous.
  */
 if (config->log_level != 0)
 virLogSetDefaultPriority(config->log_level);
 
+if (virLogSetDefaultOutput("libvirtd.log", godaemon, privileged) < 0)
+return -1;
+
+/* In case the config is empty, the filters become empty and outputs will
+ * be set to default
+ */
+virLogSetFilters(config->log_filters);
+virLogSetOutputs(config->log_outputs);
+
+/* If there are some environment variables defined, use those instead */
 virLogSetFromEnv();
 
-if (virLogGetNbFilters() == 0)
-virLogSetFilters(config->log_filters);
-
-if (virLogGetNbOutputs() == 0)
-virLogSetOutputs(config->log_outputs);
-
 /*
  * Command line override for --verbose
  */
 if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO))
 virLogSetDefaultPriority(VIR_LOG_INFO);
 
-/*
- * If no defined outputs, and either running
- * as daemon or not on a tty, then first try
- * to direct it to the systemd journal
- * (if it exists)
- */
-if (virLogGetNbOutputs() == 0 &&
-(godaemon || !isatty(STDIN_FILENO))) {
-char *tmp;
-if (access("/run/systemd/journal/socket", W_OK) >= 0) {
-virLogPriority priority = virLogGetDefaultPriority();
-
-/* By default we don't want to log too much stuff into journald as
- * it may employ rate limiting and thus block libvirt execution. */
-if (priority == VIR_LOG_DEBUG)
-priority = VIR_LOG_INFO;
-
-if (virAsprintf(, "%d:journald", priority) < 0)
-goto error;
-virLogSetOutputs(tmp);
-VIR_FREE(tmp);
-}
-}
-
-/*
- * otherwise direct to libvirtd.log when running
- * as daemon. Otherwise the default output is stderr.
- */
-if (virLogGetNbOutputs() == 0) {
-char *tmp = NULL;
-
-if (godaemon) {
-if (privileged) {
-if (virAsprintf(, "%d:file:%s/log/libvirt/libvirtd.log",
-virLogGetDefaultPriority(),
-LOCALSTATEDIR) == -1)
-goto error;
-} else {
-char *logdir = virGetUserCacheDirectory();
-mode_t old_umask;
-
-if (!logdir)
-goto error;
-
-old_umask = umask(077);
-if (virFileMakePath(logdir) < 0) {
-umask(old_umask);
-goto error;
-}
-umask(old_umask);
-
-if (virAsprintf(, "%d:file:%s/libvirtd.log",
-virLogGetDefaultPriority(), logdir) == -1) {
-VIR_FREE(logdir);
-goto error;
-}
-VIR_FREE(logdir);
-}
-} else {
-if (virAsprintf(, "%d:stderr", virLogGetDefaultPriority()) < 0)
-goto error;
-}
-virLogSetOutputs(tmp);
-VIR_FREE(tmp);
-}
-
 return 0;
-
- error:
-return -1;
 }
 
 
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 9ee818e..881af11 

Re: [libvirt] [PATCH 2/2] docs: news: Mention changes in memory slot number allocation

[Andrea Bolognani]

On Fri, 2016-11-25 at 13:44 +0100, Peter Krempa wrote:
> > You'll also need to close the  element properly, or
> > building the documentation will fail.
> 
> Right. The format of the file, where we wrap the text back to the column
> where the tag starts rather than wrapping it to the column where the
> text starts makes it very hard to visually spot this kind of mistake.
> 
> Is there a particular reason to do it in such a weird way?

Not really, I mostly went along with the formatting of the
existing news.html.in and hacked together a XSLT stylesheet
that happened to produce a decent output based on it.

Me and Martin have discussed some ways to improve on that,
but it'll have to wait for a bit. Please just be patient in
the meantime :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH v4 0/9] Implementation of QEMU vhost-scsi

[Andrea Bolognani]

On Fri, 2016-11-25 at 07:30 -0500, John Ferlan wrote:
> > > This patch series provides a libvirt implementation of the vhost-scsi
> > > interface in QEMU.
> > 
> > Can you please provide a release notes entry briefly
> > describing this change? You can look at existing entries
> > in docs/news.html.in to see how it should look. Thanks!
> 
> More than what I put in my reply to 9/9?
> 
> ... I'll also generate the news.html.in entry:
> 
>   vhost-scsi: Add support scsi_host hostdev passthrough
>   Add the capability to pass through a scsi_host HBA and the
>   associated LUNs to the guest.
>   

Nope, just missed it :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 2/2] docs: news: Mention changes in memory slot number allocation

[Peter Krempa]

On Fri, Nov 25, 2016 at 13:36:01 +0100, Andrea Bolognani wrote:
> On Fri, 2016-11-25 at 12:14 +0100, Peter Krempa wrote:
> > ---
> >  docs/news.html.in | 4 
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/docs/news.html.in b/docs/news.html.in
> > index 0a4b767..2ba25f5 100644
> > --- a/docs/news.html.in
> > +++ b/docs/news.html.in
> > @@ -34,6 +34,10 @@
> >qemu: Users can now enable debug logging for native gluster
> >volumes in qemu using the "gluster_debug_level" option in 
> >qemu.conf
> >
> > +  memory hotplug: Slot numbers for memory devices are now
> > +  automatically alocated and thus persistent. In addition slot 
> > numbers
> 
> s/alocated/allocated/
> 
> > +  can be specified without providing a base address, which 
> > simplifies
> > +  user configuration.
> 
> Please drop the period at the end of the sentence.
> 
> You'll also need to close the  element properly, or
> building the documentation will fail.

Right. The format of the file, where we wrap the text back to the column
where the tag starts rather than wrapping it to the column where the
text starts makes it very hard to visually spot this kind of mistake.

Is there a particular reason to do it in such a weird way?

Peter


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

Re: [libvirt] [PATCH 2/2] docs: news: Mention changes in memory slot number allocation

[Andrea Bolognani]

On Fri, 2016-11-25 at 12:14 +0100, Peter Krempa wrote:
> ---
>  docs/news.html.in | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/docs/news.html.in b/docs/news.html.in
> index 0a4b767..2ba25f5 100644
> --- a/docs/news.html.in
> +++ b/docs/news.html.in
> @@ -34,6 +34,10 @@
>qemu: Users can now enable debug logging for native gluster
>volumes in qemu using the "gluster_debug_level" option in qemu.conf
>
> +  memory hotplug: Slot numbers for memory devices are now
> +  automatically alocated and thus persistent. In addition slot 
> numbers

s/alocated/allocated/

> +  can be specified without providing a base address, which simplifies
> +  user configuration.

Please drop the period at the end of the sentence.

You'll also need to close the  element properly, or
building the documentation will fail.

>  
>
>Improvements

ACK with those issues fixed.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 1/2] docs: NEWS: Mention 'gluster_debug_level' qemu.conf option in the news

[Andrea Bolognani]

On Fri, 2016-11-25 at 12:14 +0100, Peter Krempa wrote:
> ---
>  docs/news.html.in | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/docs/news.html.in b/docs/news.html.in
> index 54eb8ad..0a4b767 100644
> --- a/docs/news.html.in
> +++ b/docs/news.html.in
> @@ -31,6 +31,9 @@
>Add the capability to pass through a scsi_host HBA and the
>associated LUNs to the guest.
>
> +  qemu: Users can now enable debug logging for native gluster
> +  volumes in qemu using the "gluster_debug_level" option in qemu.conf
> +  
>  
>
>Improvements

ACK

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH v4 0/9] Implementation of QEMU vhost-scsi

[John Ferlan]

On 11/25/2016 05:50 AM, Andrea Bolognani wrote:
> On Mon, 2016-11-21 at 22:58 -0500, Eric Farman wrote:
>> This patch series provides a libvirt implementation of the vhost-scsi
>> interface in QEMU.
> 
> Can you please provide a release notes entry briefly
> describing this change? You can look at existing entries
> in docs/news.html.in to see how it should look. Thanks!

More than what I put in my reply to 9/9?

... I'll also generate the news.html.in entry:

  vhost-scsi: Add support scsi_host hostdev passthrough
  Add the capability to pass through a scsi_host HBA and the
  associated LUNs to the guest.
  

John
> 
> -- 
> Andrea Bolognani / Red Hat / Virtualization
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 

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

Re: [libvirt] virsh domjobinfo during storage migration

[Ruben Kerkhof]

On Fri, Nov 25, 2016 at 11:53 AM, Jiri Denemark  wrote:
>> How do you want to handle this, would you like me to open an issue in 
>> bugzilla?
>
> Yes please. Unless you want to provide the patches :-)

I'd love to, but it is a bit outside of my area of expertise.

I opened https://bugzilla.redhat.com/1398599

>
> Jirka

Thanks Jirka!

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

[libvirt] [PATCH 0/2] NEWS updates for my recent changes

[Peter Krempa]

Peter Krempa (2):
  docs: NEWS: Mention 'gluster_debug_level' qemu.conf option in the news
  docs: news: Mention changes in memory slot number allocation

 docs/news.html.in | 7 +++
 1 file changed, 7 insertions(+)

-- 
2.10.2

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

[libvirt] [PATCH 2/2] docs: news: Mention changes in memory slot number allocation

[Peter Krempa]

---
 docs/news.html.in | 4 
 1 file changed, 4 insertions(+)

diff --git a/docs/news.html.in b/docs/news.html.in
index 0a4b767..2ba25f5 100644
--- a/docs/news.html.in
+++ b/docs/news.html.in
@@ -34,6 +34,10 @@
   qemu: Users can now enable debug logging for native gluster
   volumes in qemu using the "gluster_debug_level" option in qemu.conf
   
+  memory hotplug: Slot numbers for memory devices are now
+  automatically alocated and thus persistent. In addition slot numbers
+  can be specified without providing a base address, which simplifies
+  user configuration.
 
   
   Improvements
-- 
2.10.2

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

[libvirt] [PATCH 1/2] docs: NEWS: Mention 'gluster_debug_level' qemu.conf option in the news

[Peter Krempa]

---
 docs/news.html.in | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/docs/news.html.in b/docs/news.html.in
index 54eb8ad..0a4b767 100644
--- a/docs/news.html.in
+++ b/docs/news.html.in
@@ -31,6 +31,9 @@
   Add the capability to pass through a scsi_host HBA and the
   associated LUNs to the guest.
   
+  qemu: Users can now enable debug logging for native gluster
+  volumes in qemu using the "gluster_debug_level" option in qemu.conf
+  
 
   
   Improvements
-- 
2.10.2

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

Re: [libvirt] [PATCH] virstring: Unify string list function names

[Andrea Bolognani]

On Fri, 2016-11-25 at 09:28 +0100, Michal Privoznik wrote:
> We have couple of functions that operate over NULL terminated
> lits of strings. However, our naming sucks:
> 
> virStringJoin
> virStringFreeList
> virStringFreeListCount
> virStringArrayHasString
> virStringGetFirstWithPrefix
> 
> We can do better:
> 
> virStringListJoin
> virStringListFree
> virStringListFreeCount
> virStringListHasString
> virStringListGetFirstWithPrefix
> 
> Signed-off-by: Michal Privoznik 

[...]
> @@ -227,7 +227,7 @@ virStringArrayHasString(const char **strings,
>  }
>  
>  char *
> -virStringGetFirstWithPrefix(char **strings, const char *prefix)
> +virStringListGetFirstWithPrefix(char **strings, const char *prefix)

Since you're changing the definition anyway, you can use
the chance to move the second argument to a separate line.

[...]
> @@ -37,15 +37,15 @@ char **virStringSplit(const char *string,
>size_t max_tokens)
>  ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
>  
> -char *virStringJoin(const char **strings,
> -const char *delim)
> +char *virStringListJoin(const char **strings,
> +const char *delim)
>  ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
>  
> -void virStringFreeList(char **strings);
> -void virStringFreeListCount(char **strings, size_t count);
> +void virStringListFree(char **strings);
> +void virStringListFreeCount(char **strings, size_t count);
>  
> -bool virStringArrayHasString(const char **strings, const char *needle);
> -char *virStringGetFirstWithPrefix(char **strings, const char *prefix)
> +bool virStringListHasString(const char **strings, const char *needle);
> +char *virStringListGetFirstWithPrefix(char **strings, const char *prefix)
>  ATTRIBUTE_NONNULL(2);

Same here.


I'm assuming you changed the callers in an automated fashion,
the diff looks sane from a very quick look and the code still
compiles and passes all checks.

ACK

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] virsh domjobinfo during storage migration

[Jiri Denemark]

On Fri, Nov 25, 2016 at 11:38:36 +0100, Ruben Kerkhof wrote:
> Hi Jiri,
> 
> On Fri, Nov 25, 2016 at 9:26 AM, Jiri Denemark  wrote:
> > I guess it should be possible to also check the progress of all running
> > block jobs so that we can report statistics about ongoing storage
> > migration even when NBD is used.
> 
> That would be great, since I won't be able to upgrade to a newer
> version of QEMU yet.
> How do you want to handle this, would you like me to open an issue in 
> bugzilla?

Yes please. Unless you want to provide the patches :-)

Jirka

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

[libvirt] [PATCH 3/3] docs: Update news

[Jiri Denemark]

Signed-off-by: Jiri Denemark 
---
 docs/news.html.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/docs/news.html.in b/docs/news.html.in
index 59bf20da3..3a0fea65e 100644
--- a/docs/news.html.in
+++ b/docs/news.html.in
@@ -35,6 +35,8 @@
   
   Improvements
 
+  docs: Better documentation for migration APIs and flags
+  
   vbox: Address thread safety issues
   
   virsh: Add support for passing an alternative persistent XML
-- 
2.11.0.rc2

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

[libvirt] [PATCH 2/3] Consolidate documentation of virDomainMigrate{, ToURI}{, 2, 3}

[Jiri Denemark]

Only the latest APIs are fully documented and the documentation of the
older variants (which are just limited versions of the new APIs anyway)
points to the newest APIs.

Signed-off-by: Jiri Denemark 
---
 src/libvirt-domain.c | 348 ++-
 1 file changed, 36 insertions(+), 312 deletions(-)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index f9f5a4696..e0c69dbe7 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3519,77 +3519,9 @@ virDomainMigrateUnmanaged(virDomainPtr domain,
  * Migrate the domain object from its current host to the destination
  * host given by dconn (a connection to the destination host).
  *
- * Flags may be one of more of the following:
- *   VIR_MIGRATE_LIVE  Do not pause the VM during migration
- *   VIR_MIGRATE_PEER2PEER Direct connection between source & destination hosts
- *   VIR_MIGRATE_TUNNELLED Tunnel migration data over the libvirt RPC channel
- *   VIR_MIGRATE_PERSIST_DEST If the migration is successful, persist the 
domain
- *on the destination host.
- *   VIR_MIGRATE_UNDEFINE_SOURCE If the migration is successful, undefine the
- *   domain on the source host.
- *   VIR_MIGRATE_PAUSEDLeave the domain suspended on the remote side.
- *   VIR_MIGRATE_NON_SHARED_DISK Migration with non-shared storage with full
- *   disk copy
- *   VIR_MIGRATE_NON_SHARED_INC  Migration with non-shared storage with
- *   incremental disk copy
- *   VIR_MIGRATE_CHANGE_PROTECTION Protect against domain configuration
- * changes during the migration process (set
- * automatically when supported).
- *   VIR_MIGRATE_UNSAFEForce migration even if it is considered unsafe.
- *   VIR_MIGRATE_OFFLINE Migrate offline
- *   VIR_MIGRATE_POSTCOPY Enable (but do not start) post-copy
- *
- * VIR_MIGRATE_TUNNELLED requires that VIR_MIGRATE_PEER2PEER be set.
- * Applications using the VIR_MIGRATE_PEER2PEER flag will probably
- * prefer to invoke virDomainMigrateToURI, avoiding the need to
- * open connection to the destination host themselves.
- *
- * If a hypervisor supports renaming domains during migration,
- * then you may set the dname parameter to the new name (otherwise
- * it keeps the same name).  If this is not supported by the
- * hypervisor, dname must be NULL or else you will get an error.
- *
- * If the VIR_MIGRATE_PEER2PEER flag is set, the uri parameter
- * must be a valid libvirt connection URI, by which the source
- * libvirt driver can connect to the destination libvirt. If
- * omitted, the dconn connection object will be queried for its
- * current URI.
- *
- * If the VIR_MIGRATE_PEER2PEER flag is NOT set, the URI parameter
- * takes a hypervisor specific format. The hypervisor capabilities
- * XML includes details of the support URI schemes. If omitted
- * the dconn will be asked for a default URI.
- *
- * If you want to copy non-shared storage within migration you
- * can use either VIR_MIGRATE_NON_SHARED_DISK or
- * VIR_MIGRATE_NON_SHARED_INC as they are mutually exclusive.
- *
- * In either case it is typically only necessary to specify a
- * URI if the destination host has multiple interfaces and a
- * specific interface is required to transmit migration data.
- *
- * The maximum bandwidth (in MiB/s) that will be used to do migration
- * can be specified with the bandwidth parameter.  If set to 0,
- * libvirt will choose a suitable default.  Some hypervisors do
- * not support this feature and will return an error if bandwidth
- * is not 0.
- *
- * Users should note that implementation of VIR_MIGRATE_OFFLINE
- * flag in some drivers does not copy storage or any other file
- * based storage (e.g. UEFI variable storage).
- *
- * Enabling the VIR_MIGRATE_POSTCOPY flag tells libvirt to enable post-copy
- * migration.  Use virDomainMigrateStartPostCopy to switch migration into
- * the post-copy mode.  See virDomainMigrateStartPostCopy for more details
- * about post-copy.
- *
- * To see which features are supported by the current hypervisor,
- * see virConnectGetCapabilities, /capabilities/host/migration_features.
- *
- * There are many limitations on migration imposed by the underlying
- * technology - for example it may not be possible to migrate between
- * different processors even with the same architecture, or between
- * different types of hypervisor.
+ * This function is similar to virDomainMigrate3, but it only supports a fixed
+ * set of parameters: @dname corresponds to VIR_MIGRATE_PARAM_DEST_NAME, @uri
+ * is VIR_MIGRATE_PARAM_URI, and @bandwidth is VIR_MIGRATE_PARAM_BANDWIDTH.
  *
  * virDomainFree should be used to free the resources after the
  * returned domain object is no longer needed.
@@ -3740,89 +3672,10 @@ virDomainMigrate(virDomainPtr domain,
  * Migrate the domain object from its current host 

[libvirt] [PATCH 0/3] Better documentation of migration APIs and flags

[Jiri Denemark]

We have 6 public APIs for migration: virDomainMigrate,
virDomainMigrate2, virDomainMigrate3, virDomainMigrateToURI,
virDomainMigrateToURI2, and virDomainMigrateToURI3. Each of them was
documented separately, but the documentation was not consistent. Some
APIs documented individual flags, some didn't do that. Some notes about
specific behavior were not added to all APIs. And so on. Since the newer
versions provide a superset of the functionality of the older ones
mainly adding new parameters, it doesn't really make sense to keep 6
(ideally) almost identical copies of the same documentation.

To get out of this mess this patch set moves documentation of individual
migration flags directly to libvirt-domain.h, where the flags are
defined. And the documentation of virDomainMigrate{,ToURI}{,2} is
reduced to point to the *3 APIs with a description of how old parameters
are mapped to the new ones.

Jiri Denemark (3):
  Enhance documentation of virDomainMigrateFlags
  Consolidate documentation of virDomainMigrate{,ToURI}{,2,3}
  docs: Update news

 docs/news.html.in|   2 +
 include/libvirt/libvirt-domain.h | 153 ++---
 src/libvirt-domain.c | 348 ---
 3 files changed, 170 insertions(+), 333 deletions(-)

-- 
2.11.0.rc2

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

[libvirt] [PATCH 1/3] Enhance documentation of virDomainMigrateFlags

[Jiri Denemark]

The enhanced documentation of VIR_MIGRATE_RDMA_PIN_ALL fixes
https://bugzilla.redhat.com/show_bug.cgi?id=1373783

Signed-off-by: Jiri Denemark 
---
 include/libvirt/libvirt-domain.h | 153 +--
 1 file changed, 132 insertions(+), 21 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 5f5066074..a8435ab16 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -683,27 +683,138 @@ typedef enum {
 
 /* Domain migration flags. */
 typedef enum {
-VIR_MIGRATE_LIVE  = (1 << 0), /* live migration */
-VIR_MIGRATE_PEER2PEER = (1 << 1), /* direct source -> dest host 
control channel */
-/* Note the less-common spelling that we're stuck with:
-   VIR_MIGRATE_TUNNELLED should be VIR_MIGRATE_TUNNELED */
-VIR_MIGRATE_TUNNELLED = (1 << 2), /* tunnel migration data over 
libvirtd connection */
-VIR_MIGRATE_PERSIST_DEST  = (1 << 3), /* persist the VM on the 
destination */
-VIR_MIGRATE_UNDEFINE_SOURCE   = (1 << 4), /* undefine the VM on the source 
*/
-VIR_MIGRATE_PAUSED= (1 << 5), /* pause on remote side */
-VIR_MIGRATE_NON_SHARED_DISK   = (1 << 6), /* migration with non-shared 
storage with full disk copy */
-VIR_MIGRATE_NON_SHARED_INC= (1 << 7), /* migration with non-shared 
storage with incremental copy */
-  /* (same base image shared 
between source and destination) */
-VIR_MIGRATE_CHANGE_PROTECTION = (1 << 8), /* protect for changing domain 
configuration through the
-   * whole migration process; this 
will be used automatically
-   * when supported */
-VIR_MIGRATE_UNSAFE= (1 << 9), /* force migration even if it is 
considered unsafe */
-VIR_MIGRATE_OFFLINE   = (1 << 10), /* offline migrate */
-VIR_MIGRATE_COMPRESSED= (1 << 11), /* compress data during 
migration */
-VIR_MIGRATE_ABORT_ON_ERROR= (1 << 12), /* abort migration on I/O 
errors happened during migration */
-VIR_MIGRATE_AUTO_CONVERGE = (1 << 13), /* force convergence */
-VIR_MIGRATE_RDMA_PIN_ALL  = (1 << 14), /* RDMA memory pinning */
-VIR_MIGRATE_POSTCOPY  = (1 << 15), /* enable (but do not start) 
post-copy migration */
+/* Do not pause the domain during migration. The domain's memory will
+ * be transferred to the destination host while the domain is running.
+ * The migration may never converge if the domain is changing its memory
+ * faster then it can be transferred. The domain can be manually paused
+ * anytime during migration using virDomainSuspend.
+ */
+VIR_MIGRATE_LIVE  = (1 << 0),
+
+/* Tell the source libvirtd to connect directly to the destination host.
+ * Without this flag the client (e.g., virsh) connects to both hosts and
+ * controls the migration process. In peer-to-peer mode, the source
+ * libvirtd controls the migration by calling the destination daemon
+ * directly.
+ */
+VIR_MIGRATE_PEER2PEER = (1 << 1),
+
+/* Tunnel migration data over libvirtd connection. Without this flag the
+ * source hypervisor sends migration data directly to the destination
+ * hypervisor. This flag can only be used when VIR_MIGRATE_PEER2PEER is
+ * set as well.
+ *
+ * Note the less-common spelling that we're stuck with:
+ * VIR_MIGRATE_TUNNELLED should be VIR_MIGRATE_TUNNELED.
+ */
+VIR_MIGRATE_TUNNELLED = (1 << 2),
+
+/* Define the domain as persistent on the destination host after successful
+ * migration. If the domain was persistent on the source host and
+ * VIR_MIGRATE_UNDEFINE_SOURCE is not used, it will end up persistent on
+ * both hosts.
+ */
+VIR_MIGRATE_PERSIST_DEST  = (1 << 3),
+
+/* Undefine the domain on the source host once migration successfully
+ * finishes.
+ */
+VIR_MIGRATE_UNDEFINE_SOURCE   = (1 << 4),
+
+/* Leave the domain suspended on the destination host. virDomainResume (on
+ * the virDomainPtr returned by the migration API) has to be called
+ * explicitly to resume domain's virtual CPUs.
+ */
+VIR_MIGRATE_PAUSED= (1 << 5),
+
+/* Migrate full disk images in addition to domain's memory. By default
+ * only non-shared non-readonly disk images are transferred. The
+ * VIR_MIGRATE_PARAM_MIGRATE_DISKS parameter can be used to specify which
+ * disks should be migrated.
+ *
+ * This flag and VIR_MIGRATE_NON_SHARED_INC are mutually exclusive.
+ */
+VIR_MIGRATE_NON_SHARED_DISK   = (1 << 6),
+
+/* Migrate disk images in addition to domain's memory. This is similar to
+ * VIR_MIGRATE_NON_SHARED_DISK, but only the top level of each disk's
+ * backing chain is copied. That 

Re: [libvirt] [PATCH v4 0/9] Implementation of QEMU vhost-scsi

[Andrea Bolognani]

On Mon, 2016-11-21 at 22:58 -0500, Eric Farman wrote:
> This patch series provides a libvirt implementation of the vhost-scsi
> interface in QEMU.

Can you please provide a release notes entry briefly
describing this change? You can look at existing entries
in docs/news.html.in to see how it should look. Thanks!

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] virsh domjobinfo during storage migration

[Ruben Kerkhof]

Hi Jiri,

On Fri, Nov 25, 2016 at 9:26 AM, Jiri Denemark  wrote:
> On Thu, Nov 24, 2016 at 22:54:10 +0100, Ruben Kerkhof wrote:
>> Hi all,
>>
>> Running virsh domjobinfo on my CentOS 6 systems during a migration
>> with --copy-storage-all, the output used to look like this:
>>
>> Job type: Unbounded
>> Time elapsed: 1830632  ms
>> Data processed:   37.212 GiB
>> Data remaining:   1.025 GiB
>> Data total:   16.016 GiB
>> Memory processed: 37.212 GiB
>> Memory remaining: 1.025 GiB
>> Memory total: 16.016 GiB
>> Memory bandwidth: 100.018 MiB/s
>> Constant pages:   618279
>> Normal pages: 9734623
>> Normal data:  37.135 GiB
>> Expected downtime: 1118 ms
>> Setup time:   61   ms
>>
>> But when I run the same command on CentOS 7, libvirt-1.2.17-13 and
>> qemu-kvm-1.5.3-105, I just get this:
>>
>> $ sudo virsh domjobinfo 58358fec-c35c-4a7a-a4dd-18ec2e1327bf
>> Job type: Unbounded
>> Time elapsed: 209616   ms
>>
>> I tried to debug this myself, but I'm a bit stuck. I'd expected
>> libvirt to send query-migrate commands to qemu every 50 ms or so. To
>> verify I attached gdb and put a breakpoint at
>> qemuMonitorJSONGetMigrationStatus, but the breakpoint only hits after
>> the complete disk is mirrored.
>
> That's expected since QEMU finally implemented a migration event, which
> allowed us to stop polling every 50 ms. Libvirt will only call
> query-migrate at the end of a migration or when virsh domjobinfo is
> called.
>
> The reason why you don't see anything interesting in the output is NBD
> storage migration. With new enough QEMU libvirt will first migrate
> storage using QEMU's integrated NBD server and then "migrate" QMP
> command will called. With old QEMU storage migration was done by the
> "migrate" command itself. Thus calling query-migrate while storage is
> migrated (i.e., before migrate was called) does not provide anything.

That makes sense, thanks for the explanation.

> I guess it should be possible to also check the progress of all running
> block jobs so that we can report statistics about ongoing storage
> migration even when NBD is used.

That would be great, since I won't be able to upgrade to a newer
version of QEMU yet.
How do you want to handle this, would you like me to open an issue in bugzilla?

>
> Jirka

Kind regards,

Ruben Kerkhof

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

Re: [libvirt] [PATCH v2 0/4] qemu: assign vfio devices to PCIe addresses when appropriate

[Andrea Bolognani]

On Mon, 2016-11-21 at 00:01 -0500, Laine Stump wrote:
> Patches 1 and 4 were originally added to the end of the "more PCIe
> less legacy PCI" patchset in its final incarnation, but all the other
> patches were ACKed and pushed, while this needed a bit more work,
> resulting in this "faux V2" - although it's the 2nd time I've posted
> these patches, their "V1" was really inside the "V11666" of a larger
> series.

Forgot to mention yesterday: the NEWS file will need to
contain an entry for this. You can add a single entry that
covers both this series and the previous PCIe-related
series, or just document the previous one if this one
somehow doesn't make it into the release.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[libvirt] [PATCH] NEWS: Add some missing entries

[Andrea Bolognani]

Catch up with changes that have been pushed but didn't include
updates to the NEWS file themselves.
---
Pushed under the "this NEWS file thing will need some time
to catch on" rule.

 docs/news.html.in | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/docs/news.html.in b/docs/news.html.in
index 54eb8ad..59bf20d 100644
--- a/docs/news.html.in
+++ b/docs/news.html.in
@@ -46,6 +46,11 @@
   List user-visible changes instead of single commits for a better
   high-level overview of differences between libvirt releases
   
+  website: Modernize layout and branding
+  The libvirt website looked very cluttered and outdated; it has now
+  been completely overhauled, resulting in a design that's better
+  organized and more pleasant to look at
+  
 
   
   Bug fixes
@@ -54,6 +59,8 @@
   
   Forbid newline character in names of some libvirt objects.
   
+  Fix compilation on macOS
+  
 
   
 
-- 
2.7.4

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

Re: [libvirt] [PATCH] virhostcpu: fix calculation of total CPU sockets

[Daniel P. Berrange]

On Fri, Nov 25, 2016 at 04:20:18PM +0800, Zhang Zhuoyu wrote:
> CPU sockets calculation is inconsistent with physical sockets when
> Host machine has more than one node. It only calculate the maximum
> socket number of all CPU nodes instead of summing up.
> 
> For example:
> 
> Architecture:  x86_64
> CPU op-mode(s):32-bit, 64-bit
> Byte Order:Little Endian
> CPU(s):32
> On-line CPU(s) list:   0-31
> Thread(s) per core:2
> Core(s) per socket:8
> Socket(s): 2  <-- Host machine has 2 sockets
> NUMA node(s):  2
> Vendor ID: GenuineIntel
> CPU family:6
> Model: 62
> Model name:Intel(R) Xeon(R) CPU E5-2650 v2 @ 2.60GHz
> Stepping:  4
> CPU MHz:   3074.296
> BogoMIPS:  5205.76
> Virtualization:VT-x
> L1d cache: 32K
> L1i cache: 32K
> L2 cache:  256K
> L3 cache:  20480K
> NUMA node0 CPU(s): 0-7,16-23
> NUMA node1 CPU(s): 8-15,24-31
> 
> CPU model:   x86_64
> CPU(s):  32
> CPU frequency:   3021 MHz
> CPU socket(s):   1<- Should be 2 sockets
> Core(s) per socket:  8
> Thread(s) per core:  2
> NUMA cell(s):2
> Memory size: 131833636 KiB
> 
> "lscpu" shows host machine has 2 sockets, however "virsh nodeinfo"
> only calculate the maximum socket number of all CPU nodes,
> This patch fix it by summing sockets in all nodes up.

No, this is wrong interpretation of the data. 'virsh nodeinfo' is
actually reporting sockets *per* NUMA cell.

The nodeinfo data is in fact broken if you have a different
number of sockets in each cell. So we recommend that apps use
the capabilities XML as the accurate socket data.


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

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

[libvirt] [PATCH v5 1/2] conf: List only online cpus for virsh vcpupin

[Nitesh Konkar]

Currently when the vcpu placement is static
and cpuset is not specified, CPU Affinity
under virsh vcpupin shows 0..CPUMAX. This
patchset will result in display of only
online CPU's under CPU Affinity on linux.

Signed-off-by: Nitesh Konkar 
---
 src/conf/domain_conf.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e7517d9..f25cf63 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -56,6 +56,7 @@
 #include "virstring.h"
 #include "virnetdev.h"
 #include "virhostdev.h"
+#include "virhostcpu.h"
 
 #define VIR_FROM_THIS VIR_FROM_DOMAIN
 
@@ -1549,10 +1550,15 @@ virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def,
 if (hostcpus < 0)
 return -1;
 
+#ifdef __linux__
+if (!(allcpumap = virHostCPUGetOnlineBitmap()))
+return -1;
+#else
 if (!(allcpumap = virBitmapNew(hostcpus)))
 return -1;
 
 virBitmapSetAll(allcpumap);
+#endif
 
 for (i = 0; i < maxvcpus && i < ncpumaps; i++) {
 virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(def, i);
-- 
2.1.0

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

[libvirt] [PATCH v5 0/2] List only online cpus for vcpupin/emulatorpin when vcpu placement static

[Nitesh Konkar]

Currently when the vcpu placement is static and
cpuset is not specified, CPU Affinity shows 0..
CPUMAX. This patchset will result in display of
only online CPU's under CPU Affinity on linux.

Fixes the following Bug:

virsh dumpxml Fedora


  Fedora
  aecf3e5e-6f9a-42a3-9d6a-223a75569a66
  3145728
  524288
  524288
  160
  
/machine
  
 .
...
.

  
  

  
  
+0:+0
+0:+0
  


lscpu
Architecture:  x86_64
CPU op-mode(s):32-bit, 64-bit
Byte Order:Little Endian
CPU(s):8
On-line CPU(s) list:   0-2,4-7
Off-line CPU(s) list:  3
Thread(s) per core:1
Core(s) per socket:7
Socket(s): 1
..
..
NUMA node0 CPU(s): 0-2,4-7
NUMA node1 CPU(s):

cat /sys/devices/system/cpu/online
0-2,4-7

Before Patch

virsh vcpupin Fedora

VCPU: CPU Affinity
--
   0: 0-7
   1: 0-7
...
...
 158: 0-7
 159: 0-7

virsh emulatorpin Fedora
emulator: CPU Affinity
--
   *: 0-7


After  Patch

virsh vcpupin Fedora

VCPU: CPU Affinity
--
   0: 0-2,4-7
   1: 0-2,4-7
...
...
 158: 0-2,4-7
 159: 0-2,4-7

virsh emulatorpin Fedora
emulator: CPU Affinity
--
   *: 0-2,4-7

Nitesh Konkar (2):
  conf: List only online cpus for virsh vcpupin
  conf: List only online cpus for virsh emulatorpin

 src/conf/domain_conf.c | 6 ++
 src/qemu/qemu_driver.c | 5 +
 2 files changed, 11 insertions(+)

-- 
2.1.0

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

[libvirt] [PATCH v5 2/2] conf: List only online cpus for virsh emulatorpin

[Nitesh Konkar]

Currently when the vcpu placement is static
and cpuset is not specified, CPU Affinity
under virsh emulatorpin shows 0..CPUMAX. This
patchset will result in display of only
online CPU's under CPU Affinity on linux.

Signed-off-by: Nitesh Konkar 
---
 src/qemu/qemu_driver.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fdfe912..e69d92d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5435,9 +5435,14 @@ qemuDomainGetEmulatorPinInfo(virDomainPtr dom,
autoCpuset) {
 cpumask = autoCpuset;
 } else {
+#ifdef __linux__
+if (!(bitmap = virHostCPUGetOnlineBitmap()))
+goto cleanup;
+#else
 if (!(bitmap = virBitmapNew(hostcpus)))
 goto cleanup;
 virBitmapSetAll(bitmap);
+#endif
 cpumask = bitmap;
 }
 
-- 
2.1.0

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

Re: [libvirt] [PATCH] libvirt: qemu: Fix domain termination caused by query-hotpluggable-cpus not enabled

[Peter Krempa]

On Fri, Nov 25, 2016 at 10:03:38 +0100, Peter Krempa wrote:
> On Fri, Nov 25, 2016 at 09:19:18 +0100, Boris Fiuczynski wrote:

[...]

> >  src/qemu/qemu_process.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> > index f8f379a..675f5b5 100644
> > --- a/src/qemu/qemu_process.c
> > +++ b/src/qemu/qemu_process.c
> > @@ -3349,8 +3349,7 @@ qemuProcessReconnect(void *opaque)
> >  /* If upgrading from old libvirtd we won't have found any
> >   * caps in the domain status, so re-query them
> 
> At reconnect the capabilities are taken from the status XML file, where
> they are saved for every instance specifically. This code is supposed to
> run

only when a very old version of libvirt did not save the capability
flags into the status XML. It's even explained in the comment above.

> 
> >   */
> > -if (!priv->qemuCaps &&
> > -!(priv->qemuCaps = virQEMUCapsCacheLookupCopy(caps,
> > +if (!(priv->qemuCaps = virQEMUCapsCacheLookupCopy(caps,
> >
> > driver->qemuCapsCache,
> >obj->def->emulator,
> >
> > obj->def->os.machine)))
> 
> NACK, this certainly is not the right fix. Does the status XML have the
> 'query-hotpluggable-cpus' capability set? If it's so then it was
> probably mis-detected at start of the VM and should be fixed there.
> 
> If there is no such capability, then the reconnect code is broken
> somehow.
> 
> At any rate we should not re-detect the capabilities if they were
> reloaded properly from the XML.
> 
> Peter



> --
> 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] libvirt: qemu: Fix domain termination caused by query-hotpluggable-cpus not enabled

[Peter Krempa]

On Fri, Nov 25, 2016 at 09:19:18 +0100, Boris Fiuczynski wrote:
> (Re-)Starting libvirt on a system with running qemu domains which earlier
> had been successfully started by libvirt results in the error
> 
> internal error: unable to execute QEMU command 'query-hotpluggable-cpus':
>   The feature 'query-hotpluggable-cpus' is not enabled
> 
> if the qemu binary does not support the qmp command 'query-hotpluggable-cpus'.
> 
> As libvirt tries to reconnect to the running qemu domains it reads in the
> capabilities but in qemuProcessReconnect misses to run
> virQEMUCapsCacheLookupCopy and not clearing the query-hotpluggable-cpus
> capability in virQEMUCapsFilterByMachineType which was introduced with
> commit 920bbe5c.
> Libvirt therefore issues the qmp command and qemu responds with the error
> 'The feature 'query-hotpluggable-cpus' is not enabled'.
> As a consequence libvirt terminates the running qemu process since it
> determines that it cannot reconnect to the domain.
> 
> Signed-off-by: Boris Fiuczynski 
> Reviewed-by: Marc Hartmayer 
> ---
> Due to the severity of this issue I recommend to backport this fix
> into all maintenance releases up to v2.2.0.
> 
>  src/qemu/qemu_process.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index f8f379a..675f5b5 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3349,8 +3349,7 @@ qemuProcessReconnect(void *opaque)
>  /* If upgrading from old libvirtd we won't have found any
>   * caps in the domain status, so re-query them

At reconnect the capabilities are taken from the status XML file, where
they are saved for every instance specifically. This code is supposed to
run

>   */
> -if (!priv->qemuCaps &&
> -!(priv->qemuCaps = virQEMUCapsCacheLookupCopy(caps,
> +if (!(priv->qemuCaps = virQEMUCapsCacheLookupCopy(caps,
>driver->qemuCapsCache,
>obj->def->emulator,
>obj->def->os.machine)))

NACK, this certainly is not the right fix. Does the status XML have the
'query-hotpluggable-cpus' capability set? If it's so then it was
probably mis-detected at start of the VM and should be fixed there.

If there is no such capability, then the reconnect code is broken
somehow.

At any rate we should not re-detect the capabilities if they were
reloaded properly from the XML.

Peter


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

[libvirt] [PATCH] cpu: Add support for pku and ospke Intel features for Memory Protection Keys

[Lin Ma]

qemu commit: f74eefe0
https://lwn.net/Articles/667156/

Signed-off-by: Lin Ma 
---
 src/cpu/cpu_map.xml | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index 6da8321..dca5720 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -255,6 +255,13 @@
   
 
 
+
+  
+
+
+  
+
+
 
 
   
-- 
2.9.2

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

Re: [libvirt] [PATCH 1/2] qemu: fix internal error: NUMA isn't available on this host

[Martin Kletzander]

On Thu, Nov 24, 2016 at 01:28:00PM +0100, Boris Fiuczynski wrote:

If libvirt is compiled without NUMACTL support starting libvirtd
reports a libvirt internal error "NUMA isn't available on this host"
without checking if NUMA support is compiled into the libvirt binaries.
This patch adds the missing NUMA support check to prevent the internal error.
It also includes a check if the cgroup controller cpuset is available before
using it.

The error was noticed when libvirtd was restarted with running domains and
on libvirtd start the qemuConnectCgroup gets called during qemuProcessReconnect.

Signed-off-by: Boris Fiuczynski 
Reviewed-by: Bjoern Walk 
---
src/qemu/qemu_cgroup.c | 4 
1 file changed, 4 insertions(+)



ACK, will push in a while, thanks.


diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 211d0b5..0baa2b3 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -939,6 +939,10 @@ qemuRestoreCgroupState(virDomainObjPtr vm)
virBitmapPtr all_nodes;
virCgroupPtr cgroup_temp = NULL;

+if (!virNumaIsAvailable() ||
+!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET))
+return;
+
if (!(all_nodes = virNumaGetHostMemoryNodeset()))
goto error;

--
2.5.5

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


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

Re: [libvirt] [PATCH 1/2] qemu: Create hugepage path on per domain basis

[Martin Kletzander]

On Tue, Nov 22, 2016 at 01:45:42PM +0100, Michal Privoznik wrote:

If you've ever tried running a huge page backed guest under
different user than root, you probably failed. Problem is even


Surely you mean different than the default user from qemu.conf.


though we have corresponding APIs in the security drivers,
there's no implementation and thus we don't relabel the huge page
path. But even if we did, so far all of the domains share the
same path:

  /hugepageMount/libvirt/qemu

Our only option there would be to set 0777 mode on the qemu dir
which is totally unsafe. Therefore, we can create dir on
per-domain basis, i.e.:

  /hugepageMount/libvirt/qemu/domainName

and chown domainName dir to the user that domain is configured to
run under.

Signed-off-by: Michal Privoznik 
---
src/qemu/qemu_command.c|  4 +-
src/qemu/qemu_conf.c   | 45 --
src/qemu/qemu_conf.h   | 16 +---
src/qemu/qemu_driver.c | 19 +++--
src/qemu/qemu_process.c| 25 +++-
.../qemuxml2argv-hugepages-numa.args   |  4 +-
.../qemuxml2argv-hugepages-pages.args  | 14 +++
.../qemuxml2argv-hugepages-pages2.args |  2 +-
.../qemuxml2argv-hugepages-pages3.args |  2 +-
.../qemuxml2argv-hugepages-pages5.args |  2 +-
.../qemuxml2argv-hugepages-shared.args | 12 +++---
tests/qemuxml2argvdata/qemuxml2argv-hugepages.args |  2 +-
.../qemuxml2argv-memory-hotplug-dimm-addr.args |  4 +-
.../qemuxml2argv-memory-hotplug-dimm.args  |  4 +-
14 files changed, 97 insertions(+), 58 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 0ed88f5..942ad86 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1468,8 +1468,26 @@ qemuGetHugepagePath(virHugeTLBFSPtr hugepage)
}


+char *
+qemuGetDomainHugepagePath(const virDomainDef *def,
+  virHugeTLBFSPtr hugepage)
+{
+char *base = qemuGetBaseHugepagePath(hugepage);
+char *ret;
+
+if (!base ||
+virAsprintf(, "%s/%s", base, def->name) < 0) {
+VIR_FREE(base);
+return NULL;
+}
+
+return ret;
+}
+


You can't simply user the name because our restrictions for the name are
too lax.  You should get unique directory name usable for this using
virDomainObjGetShortName() to make sure the creation doesn't fail.

However, that reminds me that you might need to deal with similar thing
I had to deal with when adding per-domain subdirectories for private
domain paths.  You should save the path (or at least the information
that the newer path is used) in the domain object and save/restore it
in/from the state XML.  The way it's implemented now will break for
example hotplug of hugepage-backed memory after libvirt upgrade.

I know this (and others) are mostly corner-cases, this won't have that
many problems as the per-domain path had, but still, not fixing it right
away will just increase the complexity of the fix.

Martin


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

[libvirt] [PATCH] virstring: Unify string list function names

[Michal Privoznik]

We have couple of functions that operate over NULL terminated
lits of strings. However, our naming sucks:

virStringJoin
virStringFreeList
virStringFreeListCount
virStringArrayHasString
virStringGetFirstWithPrefix

We can do better:

virStringListJoin
virStringListFree
virStringListFreeCount
virStringListHasString
virStringListGetFirstWithPrefix

Signed-off-by: Michal Privoznik 
---
 daemon/remote.c|  2 +-
 src/bhyve/bhyve_command.c  |  2 +-
 src/bhyve/bhyve_parse_command.c| 14 +++---
 src/conf/domain_capabilities.c |  2 +-
 src/cpu/cpu_ppc64.c|  2 +-
 src/cpu/cpu_x86.c  |  2 +-
 src/libvirt_private.syms   | 10 +-
 src/lxc/lxc_container.c|  2 +-
 src/lxc/lxc_native.c   | 24 
 src/openvz/openvz_conf.c   |  2 +-
 src/qemu/qemu_agent.c  |  4 ++--
 src/qemu/qemu_capabilities.c   | 20 ++--
 src/qemu/qemu_conf.c   | 10 +-
 src/qemu/qemu_domain.c |  6 +++---
 src/qemu/qemu_driver.c |  2 +-
 src/qemu/qemu_monitor_json.c   | 18 +-
 src/qemu/qemu_monitor_text.c   |  6 +++---
 src/qemu/qemu_parse_command.c  | 22 +++---
 src/qemu/qemu_process.c|  8 
 src/remote/remote_driver.c |  2 +-
 src/storage/storage_backend_sheepdog.c |  6 +++---
 src/storage/storage_backend_zfs.c  | 12 ++--
 src/storage/storage_driver.c   |  2 +-
 src/util/vircgroup.c   |  8 
 src/util/vircommand.c  |  2 +-
 src/util/virconf.c |  4 ++--
 src/util/virfile.c |  6 +++---
 src/util/virfirewall.c |  2 +-
 src/util/virfirmware.c |  4 ++--
 src/util/virlog.c  |  8 
 src/util/virpolkit.c   |  2 +-
 src/util/virprocess.c  |  2 +-
 src/util/virstoragefile.c  | 16 
 src/util/virstring.c   | 32 
 src/util/virstring.h   | 12 ++--
 src/util/viruri.c  |  2 +-
 src/vbox/vbox_common.c | 10 +-
 src/vbox/vbox_snapshot_conf.c  | 18 +-
 src/vz/vz_sdk.c|  2 +-
 tests/domainsnapshotxml2xmltest.c  |  2 +-
 tests/qemumonitorjsontest.c| 14 +++---
 tests/qemuxml2argvtest.c   |  2 +-
 tests/vboxsnapshotxmltest.c|  2 +-
 tests/virconftest.c|  2 +-
 tests/virfiletest.c|  2 +-
 tests/virpolkittest.c  |  2 +-
 tests/virstringtest.c  | 14 +++---
 tools/virsh-domain.c   | 12 ++--
 tools/virsh-nodedev.c  |  6 +++---
 tools/virsh-pool.c |  4 ++--
 tools/virsh-snapshot.c |  4 ++--
 tools/virt-host-validate-common.c  |  6 +++---
 tools/virt-login-shell.c   |  6 +++---
 tools/vsh.c|  4 ++--
 54 files changed, 196 insertions(+), 196 deletions(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index e414f92..46773da 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -5425,7 +5425,7 @@ remoteDispatchConnectGetCPUModelNames(virNetServerPtr 
server ATTRIBUTE_UNUSED,
  cleanup:
 if (rv < 0)
 virNetMessageSaveError(rerr);
-virStringFreeList(models);
+virStringListFree(models);
 return rv;
 }
 
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index bb5c45c..8a29977 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -323,7 +323,7 @@ virAppendBootloaderArgs(virCommandPtr cmd, virDomainDefPtr 
def)
 /* XXX: Handle quoted? */
 blargs = virStringSplit(def->os.bootloaderArgs, " ", 0);
 virCommandAddArgSet(cmd, (const char * const *)blargs);
-virStringFreeList(blargs);
+virStringListFree(blargs);
 }
 
 static virCommandPtr
diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c
index 0ae7a83..6190042 100644
--- a/src/bhyve/bhyve_parse_command.c
+++ b/src/bhyve/bhyve_parse_command.c
@@ -246,7 +246,7 @@ bhyveCommandLineToArgv(const char *nativeConfig,
 } else {
 /* To prevent a use-after-free here, only free the argument list
  * when it is definitely not going to be used */
-virStringFreeList(arglist);
+virStringListFree(arglist);
 }
 }
 
@@ -254,13 +254,13 @@ bhyveCommandLineToArgv(const char *nativeConfig,
 if (!(*bhyve_argv = _bhyve_argv))
 goto error;
 
-virStringFreeList(lines);
+virStringListFree(lines);
 return 0;
 
  error:
 VIR_FREE(_loader_argv);
 VIR_FREE(_bhyve_argv);
- 

Re: [libvirt] virsh domjobinfo during storage migration

[Jiri Denemark]

On Thu, Nov 24, 2016 at 22:54:10 +0100, Ruben Kerkhof wrote:
> Hi all,
> 
> Running virsh domjobinfo on my CentOS 6 systems during a migration
> with --copy-storage-all, the output used to look like this:
> 
> Job type: Unbounded
> Time elapsed: 1830632  ms
> Data processed:   37.212 GiB
> Data remaining:   1.025 GiB
> Data total:   16.016 GiB
> Memory processed: 37.212 GiB
> Memory remaining: 1.025 GiB
> Memory total: 16.016 GiB
> Memory bandwidth: 100.018 MiB/s
> Constant pages:   618279
> Normal pages: 9734623
> Normal data:  37.135 GiB
> Expected downtime: 1118 ms
> Setup time:   61   ms
> 
> But when I run the same command on CentOS 7, libvirt-1.2.17-13 and
> qemu-kvm-1.5.3-105, I just get this:
> 
> $ sudo virsh domjobinfo 58358fec-c35c-4a7a-a4dd-18ec2e1327bf
> Job type: Unbounded
> Time elapsed: 209616   ms
> 
> I tried to debug this myself, but I'm a bit stuck. I'd expected
> libvirt to send query-migrate commands to qemu every 50 ms or so. To
> verify I attached gdb and put a breakpoint at
> qemuMonitorJSONGetMigrationStatus, but the breakpoint only hits after
> the complete disk is mirrored.

That's expected since QEMU finally implemented a migration event, which
allowed us to stop polling every 50 ms. Libvirt will only call
query-migrate at the end of a migration or when virsh domjobinfo is
called.

The reason why you don't see anything interesting in the output is NBD
storage migration. With new enough QEMU libvirt will first migrate
storage using QEMU's integrated NBD server and then "migrate" QMP
command will called. With old QEMU storage migration was done by the
"migrate" command itself. Thus calling query-migrate while storage is
migrated (i.e., before migrate was called) does not provide anything. I
guess it should be possible to also check the progress of all running
block jobs so that we can report statistics about ongoing storage
migration even when NBD is used.

Jirka

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

[libvirt] [PATCH] libvirt: qemu: Fix domain termination caused by query-hotpluggable-cpus not enabled

[Boris Fiuczynski]

(Re-)Starting libvirt on a system with running qemu domains which earlier
had been successfully started by libvirt results in the error

internal error: unable to execute QEMU command 'query-hotpluggable-cpus':
  The feature 'query-hotpluggable-cpus' is not enabled

if the qemu binary does not support the qmp command 'query-hotpluggable-cpus'.

As libvirt tries to reconnect to the running qemu domains it reads in the
capabilities but in qemuProcessReconnect misses to run
virQEMUCapsCacheLookupCopy and not clearing the query-hotpluggable-cpus
capability in virQEMUCapsFilterByMachineType which was introduced with
commit 920bbe5c.
Libvirt therefore issues the qmp command and qemu responds with the error
'The feature 'query-hotpluggable-cpus' is not enabled'.
As a consequence libvirt terminates the running qemu process since it
determines that it cannot reconnect to the domain.

Signed-off-by: Boris Fiuczynski 
Reviewed-by: Marc Hartmayer 
---
Due to the severity of this issue I recommend to backport this fix
into all maintenance releases up to v2.2.0.

 src/qemu/qemu_process.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f8f379a..675f5b5 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3349,8 +3349,7 @@ qemuProcessReconnect(void *opaque)
 /* If upgrading from old libvirtd we won't have found any
  * caps in the domain status, so re-query them
  */
-if (!priv->qemuCaps &&
-!(priv->qemuCaps = virQEMUCapsCacheLookupCopy(caps,
+if (!(priv->qemuCaps = virQEMUCapsCacheLookupCopy(caps,
   driver->qemuCapsCache,
   obj->def->emulator,
   obj->def->os.machine)))
-- 
2.5.5

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

Re: [libvirt] [PATCH 2/2] security: Implement virSecurityManagerSetHugepages

[Martin Kletzander]

On Tue, Nov 22, 2016 at 01:45:43PM +0100, Michal Privoznik wrote:

Since its introduction in 2012 this internal API did nothing. Now
it's finally the right time to put it into a good use. It's
implementation is fairly simple and exactly the same as
virSecurityDomainSetPathLabel.



Then why do you redefine the function?  Why not just say

 .domainSetSecurityHugepages = …DomainSetPathLabel,

or simply:

s/virSecurityManagerDomainSetHugepages/virSecurityManagerDomainSetPathLabel/g

and zap the former function, qemuProcessPrepareHost() is the only caller
anyway.

ACK to any of those two (I prefer the latter).


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

Re: [libvirt] [Qemu-devel] [RFC 00/15] qmp: Report supported device types on 'query-machines'

[Markus Armbruster]

Marcel Apfelbaum  writes:

> On 11/24/2016 05:41 PM, Markus Armbruster wrote:
>> Marcel Apfelbaum  writes:
>>
>>> On 11/24/2016 03:34 PM, Markus Armbruster wrote:
 Eduardo Habkost  writes:

> On Wed, Nov 23, 2016 at 06:43:16PM +0200, Marcel Apfelbaum wrote:
>> On 11/22/2016 03:11 AM, Eduardo Habkost wrote:
>>> The Problem
>
>>>
>>> [...]
>>>
 Our decision to have hybrid PCI/PCIe devices and buses breeds
 considerable complexity.  I wish we had avoided them, but I believe it's
 too late to change now.

>> This still does not solve the problem that some devices makes
>> sense only on a specific arch.

>>>
>>> Hi Markus,
>>>
 Examples?

>>>
>>> One quick example would be that we don't want to see
>>> Intel's IOH 3420 PCIe Root Port in an ARM machine,
>>> or a pxb on a Q35 machine (in this case we want pxb-pcie)
>>
>> Such a device would be weird.  But would it be wrong?
>
> Define wrong :)

I do:

>  Wrong enough for
>> QEMU to reject it?
>
> QEMU accepts them and they even function correctly as far as I know.
>
>> Unless QEMU rejects it, there's no reason not to
>> list it as pluggable.
>>
>
> This is the gray area I can't argue. I do think that Eduardo's
> work may present an opportunity to change QEMU's mantra:
> "everything goes as long as it works" to "here is what this configuration 
> supports".

I guess you argument is that in reality, the devices you quoted are
always part of specific chipsets, so "we dont want to see" them
elsewhere.

If I understand you correctly, there are two cases you don't want to see
elsewhere:

(1) The real PCI device only ever exists as function of another device.

(2) The real PCI device only ever exists on certain boards.

We accept these devices elsewhere due to the way we model them.

Because we conflate PCI functions and devices, we can't model (1)
correctly.  I think the appropriate solution would be modelling
functions separate from devices, then provide the functions in question
only as part of the devices where you want to see them, by making them
not user-pluggable.

Because we model a board's chipset as a set of independent devices
instead of a composite device, we can't model (2) correctly.  I think
the appropriate solution to (2) is modelling composite chipset devices,
then provide the devices in question only as part of the chipset devices
where you want to see them, by making them not user-pluggable.

Adding a bunch of special types to QOM so that introspection claims "you
can't plug that thing" (even though you actually could) would be an
inappropriate solution.  As long as you can plug them, QEMU should be
honest about it, even if we think you shouldn't plug them.  "Can't" is
mechanism.  "Shouldn't" is policy.  Baking policy into introspection by
making it lie doesn't strike me as a good idea.

[...]

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