Re: [libvirt] [v0.9.12-maint 0/8] Backport changes for CVE-2013-6458 to v0.9.12-maint

[Guido Günther]

On Wed, Jan 15, 2014 at 01:43:54PM -0700, Eric Blake wrote:
> On 01/11/2014 07:27 AM, Guido Günther wrote:
> > Hi,
> > attached patches backport the fixes for CVE-2013-6458 to v0.9.12-maint. I
> > decided to cherry-pick the introduction of VIR_STRDUP and virReportError
> > as well to ease backporting of future fixes. I'd be happy about any review.
> 
> Looks correct to me.  I'll let you push to 0.9.12-maint since you
> already did that work; I already pushed to all the branches 0.10.2 and
> later.  When porting to 0.10.2, I chose to just inline the call to
> strdup() instead of backporting VIR_STRDUP, for fewer patches but more
> conflict resolution; but either approach seems acceptable.

Thanks for the review!

> 
> Is anyone still using v0.9.11-maint?  The CVE extends back to 0.9.8, so
> we could argue that we should either fix the 0.9.11 branch, or add
> another commit to the branch that explicitly marks it as end-of-life
> because no one appears to be relying on it.  Fedora 18 is now
> end-of-life, so from Fedora's perspective, I only care about 0.10.2
> (RHEL and CentOS 6), 1.0.5 (F19), 1.1.3 (F20) and soon 1.2.1 (rawhide),
> although I didn't mind touching all the intermediate branches on my way
> down to 0.10.2.  RHEL 5 is also vulnerable to CVE-2013-6458, but as we
> don't have an upstream v0.8.2-maint branch (thank goodness!), that's
> something for Red Hat to worry about.

I'd say let's close 0.9.11. We have 0.8.3 in Debian oldstable but I'm
not going to open a maint branch for this but deal with it in the
package itself.
Cheers,
 -- Guido

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

Re: [libvirt] [PATCH] virsh nodecpustats returns incorrect stats of cpu on Linux when the number of online cpu exceed 9.

[Bing Bu Cao]

On 01/14/2014 09:30 PM, Ján Tomko wrote:

On 01/13/2014 11:28 AM, m...@linux.vnet.ibm.com wrote:

From: Bing Bu Cao 

To retrieve node cpu statistics on Linux system, the
linuxNodeGetCPUstats function use STRPREFIX() to match the
cpuid with the cpuid read from /proc/cpustat, it will cause
obvious error.

For example:
'virsh nodecpustats 12' will display stats of cpu1 if the latter is online.

This patch fixes this bug.

Signed-off-by: Bing Bu Cao 
Signed-off-by: Pradipta Kr. Banerjee 
---
  src/nodeinfo.c |5 -
  1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 05bc038..6d7926c 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -715,8 +715,9 @@ linuxNodeGetCPUStats(FILE *procstat,

  while (fgets(line, sizeof(line), procstat) != NULL) {
  char *buf = line;
+char **buf_header = virStringSplit(buf, " ", 2);

-if (STRPREFIX(buf, cpu_header)) { /* aka logical CPU time */
+if (STREQ(buf_header[0], cpu_header)) { /* aka logical CPU time */
  size_t i;

  if (sscanf(buf,


I think it would be simpler to just add a space at the end of cpu_header and
keep using STRPREFIX than splitting the string.

Good point, thank you.


Jan




--
Best Regards,
Bing Bu Cao

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

Re: [libvirt] [PATCH v2] Add helper program to create custom leases

[Doug Goldstein]

On Jan 14, 2014, at 2:09 PM, Nehal J Wani  wrote:
> 
> Introduce helper program to catch events from dnsmasq and maintain a custom
> lease file per network. It supports DHCPv4 and DHCPv6. The file is saved as
> ".status".
> 
> The format of each lease is:
>  
> 

I feel like I'm bikesheding but is it the best idea to have a custom file 
format? I know our string handling code makes this really easy to do but it 
just has a slight code smell to make our own format. We link to stuff like yajl 
and libxml for JSON/XML support and its really simple to do so we could easily 
write out a JSON/XML file and read it in. 

Definitely don't rework the patch based on my comments because we'll 99.9% go 
with this way, I'm just asking a question I felt should be asked.

> Example of custom leases file content:
> 1385245780 52:54:00:2f:ba:76 * 192.168.150.153 * *
> 1385245781 52:54:00:2f:ba:76 3127926 2001:db8:ca2:2:1::6c * 
> 00:04:76:00:cf:ae:b3:0b:fc:cd:0e:22:2e:97:76:65:74:ec
> 1385245964 52:54:00:44:7c:d7 * 192.168.150.219 iiit-ad885e4aa1 
> 01:52:54:00:44:7c:d7
> 1385245964 52:54:00:44:7c:d7 * 192.168.150.219 * 01:52:54:00:44:7c:d7
> 1385246016 52:54:00:5d:99:92 * 192.168.150.212 iiit-ad885e4aa1 
> 01:52:54:00:5d:99:92
> 1385246041 52:54:00:3b:16:e0 * 192.168.150.207 * *
> 1385246081 52:54:00:db:dd:98 * 192.168.150.234 * *
> 1385246088 52:54:00:db:dd:98 14409112 2001:db8:ca2:2:1::6d * 
> 00:04:76:00:cf:ae:b3:0b:fc:cd:0e:22:2e:97:76:65:74:ec
> 
> ---
> As danpb suggested, I have split the previous patch into helper program & API
> Refer: https://www.redhat.com/archives/libvir-list/2013-December/msg00694.html
> Once this get ACKed, I'll send in the patches for the Leases API v6
> 
> src/Makefile.am |  20 
> src/network/bridge_driver.c |   4 +
> src/util/leaseshelper.c | 225 
> 3 files changed, 249 insertions(+)
> create mode 100644 src/util/leaseshelper.c
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 57e163f..6e5b03c 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -838,6 +838,9 @@ STORAGE_HELPER_DISK_SOURCES =\
> UTIL_IO_HELPER_SOURCES =\
>util/iohelper.c
> 
> +UTIL_LEASES_HELPER_SOURCES =\
> +util/leaseshelper.c
> +
> # Network filters
> NWFILTER_DRIVER_SOURCES =\
>nwfilter/nwfilter_driver.h nwfilter/nwfilter_driver.c\
> @@ -2408,6 +2411,23 @@ libvirt_iohelper_CFLAGS = \
>$(NULL)
> endif WITH_LIBVIRTD
> 
> +if WITH_LIBVIRTD
> +libexec_PROGRAMS += libvirt_leaseshelper
> +libvirt_leaseshelper_SOURCES = $(UTIL_LEASES_HELPER_SOURCES)
> +libvirt_leaseshelper_LDFLAGS = \
> +   $(NULL)
> +libvirt_leaseshelper_LDADD =   \
> +   libvirt_util.la \
> +   ../gnulib/lib/libgnu.la
> +if WITH_DTRACE_PROBES
> +libvirt_leaseshelper_LDADD += libvirt_probes.lo
> +endif WITH_DTRACE_PROBES
> +
> +libvirt_leaseshelper_CFLAGS = \
> +   $(PIE_CFLAGS) \
> +   $(NULL)
> +endif WITH_LIBVIRTD
> +
> if WITH_STORAGE_DISK
> if WITH_LIBVIRTD
> libexec_PROGRAMS += libvirt_parthelper
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 95e4b65..2278dba 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -1063,6 +1063,10 @@ networkBuildDhcpDaemonCommandLine(virNetworkObjPtr 
> network,
> 
> cmd = virCommandNew(dnsmasqCapsGetBinaryPath(caps));
> virCommandAddArgFormat(cmd, "--conf-file=%s", configfile);
> +
> +/* This helper is used to create cutom leases file for libvirt */
> +virCommandAddArgFormat(cmd, "--dhcp-script=%s", LIBEXECDIR 
> "/libvirt_leaseshelper");
> +
> *cmdout = cmd;
> ret = 0;
> cleanup:
> diff --git a/src/util/leaseshelper.c b/src/util/leaseshelper.c
> new file mode 100644
> index 000..486ebe3
> --- /dev/null
> +++ b/src/util/leaseshelper.c
> @@ -0,0 +1,225 @@
> +/*
> + * leasehelper.c: Helper program to create custom leases file
> + *
> + * Copyright (C) 2013 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library.  If not, see
> + * .
> + *
> + * Author: Nehal J Wani 
> + *
> + */
> +
> +#include 
> +
> +#include 
> +#include 
> +
> +#include "virutil.h"
> +#include "virthread.h"
> +#include "virfile.h"
> +#include "vi

Re: [libvirt] [PATCH 2/2] spice: expose the QEMU disable file transfer option

[Doug Goldstein]

On Jan 15, 2014, at 10:33 AM, Francesco Romani  wrote:
> 
> spice-server offers an API to disable file transfer messages
> on the agent channel between the client and the guest.
> This is supported in qemu through the disable-agent-file-xfer option.
> 
> This patch exposes this option to libvirt.
> Adds a new element 'filetransfer', with one property,
> 'filetransfer', which accepts a boolean setting.
> Default is enabled.

Haven't reviewed the code but the commit message is wrong. The property is 
'enable' from the schema change.



> 
> Depends on the capability exported in the first patch of the series.
> 
> Signed-off-by: Francesco Romani 
> ---
> docs/formatdomain.html.in  |  8 +
> docs/schemas/domaincommon.rng  | 11 ++
> src/conf/domain_conf.c | 31 -
> src/conf/domain_conf.h | 10 ++
> src/libvirt_private.syms   |  2 ++
> src/qemu/qemu_command.c|  9 +
> ...emuxml2argv-graphics-spice-agent-file-xfer.args |  9 +
> ...qemuxml2argv-graphics-spice-agent-file-xfer.xml | 40 ++
> .../qemuxml2argv-graphics-spice.args   |  5 +--
> .../qemuxml2argv-graphics-spice.xml|  1 +
> tests/qemuxml2argvtest.c   |  9 -
> 11 files changed, 131 insertions(+), 4 deletions(-)
> create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.args
> create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.xml
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 68860ef..c11a7d3 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -4042,6 +4042,7 @@ qemu-kvm -net nic,model=? /dev/null
> 
> 
> 
> +
>   
> 
>   Spice supports variable compression settings for audio,
> @@ -4081,6 +4082,13 @@ qemu-kvm -net nic,model=? /dev/null
>   since 0.9.11. If no mode is
>   specified, the qemu default will be used (client mode).
> 
> +
> +  File transfer functionality (via Spice agent) is set using the
> +  filetransfer element.
> +  It is enabled by default, and can be disabled by setting the
> +  enable property to no ,
> +  since since 1.2.2.
> +
>   
>   "rdp"
>   
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index a69f6b6..9ddb772 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -2474,6 +2474,17 @@
> 
>   
> 
> +
> +  
> +
> +  
> +yes
> +no
> +  
> +
> +
> +  
> +
>   
> 
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index c1dd598..7d6c9ba 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -604,6 +604,12 @@ VIR_ENUM_IMPL(virDomainGraphicsSpiceClipboardCopypaste,
>   "yes",
>   "no");
> 
> +VIR_ENUM_IMPL(virDomainGraphicsSpiceAgentFileTransfer,
> +  VIR_DOMAIN_GRAPHICS_SPICE_AGENT_FILE_TRANSFER_LAST,
> +  "default",
> +  "yes",
> +  "no");
> +
> VIR_ENUM_IMPL(virDomainHostdevMode, VIR_DOMAIN_HOSTDEV_MODE_LAST,
>   "subsystem",
>   "capabilities")
> @@ -8519,6 +8525,26 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
> VIR_FREE(copypaste);
> 
> def->data.spice.copypaste = copypasteVal;
> +} else if (xmlStrEqual(cur->name, BAD_CAST "filetransfer")) {
> +char *enable = virXMLPropString(cur, "enable");
> +int enableVal;
> +
> +if (!enable) {
> +virReportError(VIR_ERR_XML_ERROR, "%s",
> +   _("spice filetransfer missing 
> enable"));
> +goto error;
> +}
> +
> +if ((enableVal =
> + 
> virDomainGraphicsSpiceAgentFileTransferTypeFromString(enable)) <= 0) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   _("unknown enable value '%s'"), 
> enable);
> +VIR_FREE(enable);
> +goto error;
> +}
> +VIR_FREE(enable);
> +
> +def->data.spice.filetransfer = enableVal;
> } else if (xmlStrEqual(cur->name, BAD_CAST "mouse")

Re: [libvirt] [PATCHv2 0/6] remaining cleanups to libvirt.c

[John Ferlan]

On 01/15/2014 04:07 PM, Eric Blake wrote:
> On 01/15/2014 07:32 AM, John Ferlan wrote:
>>
>> I don't see a reason other than proximity to release date to preclude
>> inclusion in 1.2.1
> 
> So more to the point, should I push now, or wait until post-release?
> 

There's bug fixes and code cleanup - it seems safe enough for me...
Still muttering to myself about the missed if (info) check...  :-)

John

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

Re: [libvirt] [PATCH 2/2] qemu: Don't fail if the SCSI host device is shareable between domains

[John Ferlan]

On 01/08/2014 09:51 AM, Osier Yang wrote:
> It doesn't make sense to fail if the SCSI host device is specified
> as "shareable" explicitly between domains (NB, it works if and only
> if the device is specified as "shareable" for *all* domains,
> otherwise it fails).
> 
> To fix the problem, this patch introduces an array for virSCSIDevice
> struct, which records all the names of domain which are using the
> device (note that the recorded domains must specifiy the device as
> shareable).  And the change on the data struct brings on many
> subsequent changes in the code.
> 
> * src/util/virscsi.h:
>   - Remove virSCSIDeviceGetUsedBy
>   - Change definition of virSCSIDeviceGetUsedBy and virSCSIDeviceListDel
>   - Add virSCSIDeviceIsAvailable
> 
> * src/util/virscsi.c:
>   - struct virSCSIDevice: Change "used_by" to be an array; Add
> "n_used_by" as the array count
>   - virSCSIDeviceGetUsedBy: Removed
>   - virSCSIDeviceFree: frees the "used_by" array
>   - virSCSIDeviceSetUsedBy: Copy the domain name to avoid potential
> memory corruption
>   - virSCSIDeviceIsAvailable: New
>   - virSCSIDeviceListDel: Change the logic, for device which is already
> in the list, just remove the corresponding entry in "used_by"
>   - Copyright updating
> 
> * src/libvirt_private.sys:
>   - virSCSIDeviceGetUsedBy: Remove
>   - virSCSIDeviceIsAvailable: New
> 
> * src/qemu/qemu_hostdev.c:
>   - qemuUpdateActiveScsiHostdevs: Check if the device existing before
> adding it to the list;
>   - qemuPrepareHostdevSCSIDevices: Error out if the not all domains
> use the device as "shareable"; Also don't try to add the device
> to the activeScsiHostdevs list if it already there.
>   - qemuDomainReAttachHostScsiDevices: Change the logic according
> to the changes on helpers.
> ---
>  src/libvirt_private.syms |  2 +-
>  src/qemu/qemu_hostdev.c  | 75 
> ++--
>  src/util/virscsi.c   | 48 +--
>  src/util/virscsi.h   |  7 +++--
>  4 files changed, 84 insertions(+), 48 deletions(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 65d1bde..bd5f466 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1677,7 +1677,7 @@ virSCSIDeviceGetSgName;
>  virSCSIDeviceGetShareable;
>  virSCSIDeviceGetTarget;
>  virSCSIDeviceGetUnit;
> -virSCSIDeviceGetUsedBy;
> +virSCSIDeviceIsAvailable;
>  virSCSIDeviceListAdd;
>  virSCSIDeviceListCount;
>  virSCSIDeviceListDel;
> diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
> index 86a463a..9d81e94 100644
> --- a/src/qemu/qemu_hostdev.c
> +++ b/src/qemu/qemu_hostdev.c
> @@ -250,13 +250,14 @@ qemuUpdateActiveScsiHostdevs(virQEMUDriverPtr driver,
>  virDomainHostdevDefPtr hostdev = NULL;
>  size_t i;
>  int ret = -1;
> +virSCSIDevicePtr scsi = NULL;
> +virSCSIDevicePtr tmp = NULL;
>  
>  if (!def->nhostdevs)
>  return 0;
>  
>  virObjectLock(driver->activeScsiHostdevs);
>  for (i = 0; i < def->nhostdevs; i++) {
> -virSCSIDevicePtr scsi = NULL;
>  hostdev = def->hostdevs[i];
>  
>  if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
> @@ -271,11 +272,18 @@ qemuUpdateActiveScsiHostdevs(virQEMUDriverPtr driver,
>hostdev->shareable)))
>  goto cleanup;
>  
> -virSCSIDeviceSetUsedBy(scsi, def->name);
> -
> -if (virSCSIDeviceListAdd(driver->activeScsiHostdevs, scsi) < 0) {
> +if ((tmp = virSCSIDeviceListFind(driver->activeScsiHostdevs, scsi))) 
> {
> +if (virSCSIDeviceSetUsedBy(tmp, def->name) < 0) {
> +virSCSIDeviceFree(scsi);
> +goto cleanup;
> +}
>  virSCSIDeviceFree(scsi);
> -goto cleanup;
> +} else {
> +if (virSCSIDeviceSetUsedBy(scsi, def->name) < 0 ||
> +virSCSIDeviceListAdd(driver->activeScsiHostdevs, scsi) < 0) {
> +virSCSIDeviceFree(scsi);
> +goto cleanup;
> +}

It took some thinking, but I convinced myself that this path doesn't
need the shared check since it's only called from qemuProcessReconnect;
however, if something else did call it some day then that check may be
necessary. It may be wise to add it anyway... I have no strong opinion
about it being required for this change.

>  }
>  }
>  ret = 0;
> @@ -1118,24 +1126,26 @@ qemuPrepareHostdevSCSIDevices(virQEMUDriverPtr driver,
>  for (i = 0; i < count; i++) {
>  virSCSIDevicePtr scsi = virSCSIDeviceListGet(list, i);
>  if ((tmp = virSCSIDeviceListFind(driver->activeScsiHostdevs, scsi))) 
> {
> -const char *other_name = virSCSIDeviceGetUsedBy(tmp);
> -
> -if (other_name)
> -virReportError(VIR_ERR_OPERATION_INVALID,
> -   _("SCSI device %s is in use by domain %s"),
> - 

Re: [libvirt] [PATCH 1/2] util: Add "shareable" field for virSCSIDevice struct

[John Ferlan]

On 01/08/2014 09:51 AM, Osier Yang wrote:
> Unlike the host devices of other types, SCSI host device XML supports
> "shareable" tag. This patch introduces it for the virSCSIDevice struct
> for a later patch use (to detect if the SCSI device is shareable when
> preparing the SCSI host device in QEMU driver).
> ---
>  src/libvirt_private.syms |  1 +
>  src/qemu/qemu_cgroup.c   |  3 ++-
>  src/qemu/qemu_hostdev.c  |  9 ++---
>  src/security/security_apparmor.c |  3 ++-
>  src/security/security_dac.c  |  6 --
>  src/security/security_selinux.c  |  6 --
>  src/util/virscsi.c   | 11 ++-
>  src/util/virscsi.h   |  4 +++-
>  8 files changed, 32 insertions(+), 11 deletions(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index fbc9e11..65d1bde 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1674,6 +1674,7 @@ virSCSIDeviceGetDevName;
>  virSCSIDeviceGetName;
>  virSCSIDeviceGetReadonly;
>  virSCSIDeviceGetSgName;
> +virSCSIDeviceGetShareable;
>  virSCSIDeviceGetTarget;
>  virSCSIDeviceGetUnit;
>  virSCSIDeviceGetUsedBy;
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index a18955e..10b1131 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -295,7 +295,8 @@ qemuSetupHostdevCGroup(virDomainObjPtr vm,
>   dev->source.subsys.u.scsi.bus,
>   dev->source.subsys.u.scsi.target,
>   dev->source.subsys.u.scsi.unit,
> - dev->readonly)) == NULL)
> + dev->readonly,
> + dev->shareable)) == NULL)
>  goto cleanup;
>  
>  if (virSCSIDeviceFileIterate(scsi,
> diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
> index dee61e7..86a463a 100644
> --- a/src/qemu/qemu_hostdev.c
> +++ b/src/qemu/qemu_hostdev.c
> @@ -267,7 +267,8 @@ qemuUpdateActiveScsiHostdevs(virQEMUDriverPtr driver,
>hostdev->source.subsys.u.scsi.bus,
>hostdev->source.subsys.u.scsi.target,
>hostdev->source.subsys.u.scsi.unit,
> -  hostdev->readonly)))
> +  hostdev->readonly,
> +  hostdev->shareable)))
>  goto cleanup;
>  
>  virSCSIDeviceSetUsedBy(scsi, def->name);
> @@ -1097,7 +1098,8 @@ qemuPrepareHostdevSCSIDevices(virQEMUDriverPtr driver,
>hostdev->source.subsys.u.scsi.bus,
>hostdev->source.subsys.u.scsi.target,
>hostdev->source.subsys.u.scsi.unit,
> -  hostdev->readonly)))
> +  hostdev->readonly,
> +  hostdev->shareable)))
>  goto cleanup;
>  
>  if (scsi && virSCSIDeviceListAdd(list, scsi) < 0) {
> @@ -1395,7 +1397,8 @@ qemuDomainReAttachHostScsiDevices(virQEMUDriverPtr 
> driver,
>hostdev->source.subsys.u.scsi.bus,
>hostdev->source.subsys.u.scsi.target,
>hostdev->source.subsys.u.scsi.unit,
> -  hostdev->readonly))) {
> +  hostdev->readonly,
> +  hostdev->shareable))) {
>  VIR_WARN("Unable to reattach SCSI device %s:%d:%d:%d on domain 
> %s",
>   hostdev->source.subsys.u.scsi.adapter,
>   hostdev->source.subsys.u.scsi.bus,
> diff --git a/src/security/security_apparmor.c 
> b/src/security/security_apparmor.c
> index a9f04d2..86a033f 100644
> --- a/src/security/security_apparmor.c
> +++ b/src/security/security_apparmor.c
> @@ -833,7 +833,8 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr,
>   dev->source.subsys.u.scsi.bus,
>   dev->source.subsys.u.scsi.target,
>   dev->source.subsys.u.scsi.unit,
> - dev->readonly);
> + dev->readonly,
> + dev->shareable);
>  
>   if (!scsi)
>   goto done;
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index cb7d322..0952df9 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -536,7 +536,8 @@ 
> virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr,
>   dev->source.subsys.u.scsi.bus,
>   dev->source.subsys.u.scsi.

[libvirt] [PATCH maint branches] event: filter global events by domain:getattr ACL [CVE-2014-0028]

[Eric Blake]

Ever since ACL filtering was added in commit 7639736 (v1.1.1), a
user could still use event registration to obtain access to a
domain that they could not normally access via virDomainLookup*
or virConnectListAllDomains and friends.  We already have the
framework in the RPC generator for creating the filter, and
previous cleanup patches got us to the point that we can now
wire the filter through the entire object event stack.

Furthermore, whether or not domain:getattr is honored, use of
global events is a form of obtaining a list of networks, which
is covered by connect:search_domains added in a93cd08 (v1.1.0).
Ideally, we'd have a way to enforce connect:search_domains when
doing global registrations while omitting that check on a
per-domain registration.  But this patch just unconditionally
requires connect:search_domains, even when no list could be
obtained, based on the following observations:
1. Administrators are unlikely to grant domain:getattr for one
or all domains while still denying connect:search_domains - a
user that is able to manage domains will want to be able to
manage them efficiently, but efficient management includes being
able to list the domains they can access.  The idea of denying
connect:search_domains while still granting access to individual
domains is therefore not adding any real security, but just
serves as a layer of obscurity to annoy the end user.
2. In the current implementation, domain events are filtered
on the client; the server has no idea if a domain filter was
requested, and must therefore assume that all domain event
requests are global.  Even if we fix the RPC protocol to
allow for server-side filtering for newer client/server combos,
making the connect:serach_domains ACL check conditional on
whether the domain argument was NULL won't benefit older clients.
Therefore, we choose to document that connect:search_domains
is a pre-requisite to any domain event management.

Network events need the same treatment, with the obvious
change of using connect:search_networks and network:getattr.

* src/access/viraccessperm.h
(VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS)
(VIR_ACCESS_PERM_CONNECT_SEARCH_NETWORKS): Document additional
effect of the permission.
* src/conf/domain_event.h (virDomainEventStateRegister)
(virDomainEventStateRegisterID): Add new parameter.
* src/conf/network_event.h (virNetworkEventStateRegisterID):
Likewise.
* src/conf/object_event_private.h (virObjectEventStateRegisterID):
Likewise.
* src/conf/object_event.c (_virObjectEventCallback): Track a filter.
(virObjectEventDispatchMatchCallback): Use filter.
(virObjectEventCallbackListAddID): Register filter.
* src/conf/domain_event.c (virDomainEventFilter): New function.
(virDomainEventStateRegister, virDomainEventStateRegisterID):
Adjust callers.
* src/conf/network_event.c (virNetworkEventFilter): New function.
(virNetworkEventStateRegisterID): Adjust caller.
* src/remote/remote_protocol.x
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER)
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER_ANY)
(REMOTE_PROC_CONNECT_NETWORK_EVENT_REGISTER_ANY): Generate a
filter, and require connect:search_domains instead of weaker
connect:read.
* src/test/test_driver.c (testConnectDomainEventRegister)
(testConnectDomainEventRegisterAny)
(testConnectNetworkEventRegisterAny): Update callers.
* src/remote/remote_driver.c (remoteConnectDomainEventRegister)
(remoteConnectDomainEventRegisterAny): Likewise.
* src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister)
(xenUnifiedConnectDomainEventRegisterAny): Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc): Likewise.
* src/libxl/libxl_driver.c (libxlConnectDomainEventRegister)
(libxlConnectDomainEventRegisterAny): Likewise.
* src/qemu/qemu_driver.c (qemuConnectDomainEventRegister)
(qemuConnectDomainEventRegisterAny): Likewise.
* src/uml/uml_driver.c (umlConnectDomainEventRegister)
(umlConnectDomainEventRegisterAny): Likewise.
* src/network/bridge_driver.c
(networkConnectNetworkEventRegisterAny): Likewise.
* src/lxc/lxc_driver.c (lxcConnectDomainEventRegister)
(lxcConnectDomainEventRegisterAny): Likewise.

Signed-off-by: Eric Blake 
(cherry picked from commit f9f56340539d609cdc2e9d4ab812b9f146c3f100)

Conflicts:
src/conf/object_event.c - not backporting event refactoring
src/conf/object_event_private.h - likewise
src/conf/network_event.c - not backporting network events
src/conf/network_event.h - likewise
src/network/bridge_driver.c - likewise
src/access/viraccessperm.h - likewise
src/remote/remote_protocol.x - likewise
src/conf/domain_event.c - includes code that upstream has in 
object_event
src/conf/domain_event.h - context
src/libxl/libxl_driver.c - context
src/lxc/lxc_driver.c - context
src/remote/remote_driver.c - context, not backporting network events
src/test/test_driver.c - context, not backporting network events
src/uml/uml_driver.c - context
src/xen/xen_driver.c -

Re: [libvirt] [PATCH 3/5] event: prepare client to track domain callbackID

[Eric Blake]

On 01/15/2014 04:23 PM, Eric Blake wrote:
> We want to convert over to server-side events, even for older
> APIs.  To do that, the client side of the remote driver wants
> to distinguish between legacy virConnectDomainEventRegister and
> normal virConnectDomainEventRegisterAny, while knowing the
> client callbackID and the server's serverID for both types of
> registration.  The client also needs to probe whether the
> server supports server-side filtering.  However, for ease of
> review, we don't actually use the new RPCs until a later patch.
> 
> * src/conf/object_event_private.h (virObjectEventStateCallbackID):
> Add parameter.
> * src/conf/object_event.c (virObjectEventCallbackListAddID)
> (virObjectEventStateRegisterID): Separate legacy from callbackID.
> (virObjectEventStateCallbackID): Pass through parameter.
> * src/conf/network_event.c (virNetworkEventStateRegisterID):
> Update caller.
> * src/conf/domain_event.c (virDomainEventStateRegister)
> (virDomainEventStateRegisterID, virDomainEventStateDeregister):
> Likewise.
> (virDomainEventStateRegisterClient)
> (virDomainEventStateCallbackID): Implement new functions.
> * src/conf/domain_event.h (virDomainEventStateRegisterClient)
> (virDomainEventStateCallbackID): New prototypes.
> * src/remote/remote_driver.c (private_data): Add field.
> (doRemoteOpen): Probe server feature.
> (remoteConnectDomainEventRegister)
> (remoteConnectDomainEventRegisterAny): Use new function.

Based on my additional testing, I need to squash this in for global
domain lifecycle events (lookup of a remoteID should match the same
conditions as virObjectEventCallbackListCount, which ignores legacy).
Thankfully, no impact to 1.2.1 (network events are never marked legacy).

diff --git i/src/conf/object_event.c w/src/conf/object_event.c
index c5770bf..de45257 100644
--- i/src/conf/object_event.c
+++ w/src/conf/object_event.c
@@ -334,13 +334,13 @@ virObjectEventCallbackLookup(virConnectPtr conn,
 if (cb->klass == klass &&
 cb->eventID == eventID &&
 cb->conn == conn &&
-cb->legacy == legacy &&
 ((uuid && cb->uuid_filter &&
   memcmp(cb->uuid, uuid, VIR_UUID_BUFLEN) == 0) ||
  (!uuid && !cb->uuid_filter))) {
 if (remoteID)
 *remoteID = cb->remoteID;
-if (cb->cb == callback)
+if (cb->legacy == legacy &&
+cb->cb == callback)
 return cb->callbackID;
 }
 }


-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

[libvirt] [PATCH 4/5] event: client RPC protocol tweaks for domain lifecycle events

[Eric Blake]

The counterpart to the server RPC additions; here, a single
function can serve both old and new calls, while incoming
events must be serviced by two different functions.  Again,
some wise choices in our XDR makes event sharing easier.

While this only supports lifecycle events, it covers the
harder part of how Register and RegisterAny interact; the
remaining 15 events will be a mechanical change in a later
patch.  For Register, we now have a callbackID locally for
more efficient cleanup if the RPC fails; we also prefer to
use the newer RPC where we know it is supported (the older
RPC must be used if we don't know if RegisterAny is
supported).

* src/remote/remote_driver.c (remoteEvents): Register new RPC
event handler.
(remoteDomainBuildEventLifecycle): Move guts...
(remoteDomainBuildEventLifecycleHelper): ...here.
(remoteDomainBuildEventCallbackLifecycle): New function.
(remoteConnectDomainEventRegister)
(remoteConnectDomainEventDeregister)
(remoteConnectDomainEventRegisterAny)
(remoteConnectDomainEventDeregisterAny): Use new RPC when supported.
---
 src/remote/remote_driver.c | 191 +
 1 file changed, 157 insertions(+), 34 deletions(-)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index b0257c2..35baaeb 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -209,6 +209,11 @@ remoteDomainBuildEventLifecycle(virNetClientProgramPtr 
prog ATTRIBUTE_UNUSED,
 virNetClientPtr client ATTRIBUTE_UNUSED,
 void *evdata, void *opaque);
 static void
+remoteDomainBuildEventCallbackLifecycle(virNetClientProgramPtr prog 
ATTRIBUTE_UNUSED,
+virNetClientPtr client 
ATTRIBUTE_UNUSED,
+void *evdata, void *opaque);
+
+static void
 remoteDomainBuildEventReboot(virNetClientProgramPtr prog ATTRIBUTE_UNUSED,
  virNetClientPtr client ATTRIBUTE_UNUSED,
  void *evdata, void *opaque);
@@ -345,10 +350,23 @@ static virNetClientProgramEvent remoteEvents[] = {
   remoteDomainBuildEventDeviceRemoved,
   sizeof(remote_domain_event_device_removed_msg),
   (xdrproc_t)xdr_remote_domain_event_device_removed_msg },
+/* All events above here are legacy events, missing the callback
+ * ID, which means the server has a single global registration and
+ * we do full filtering in the client.  If the server lacks
+ * VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK, those are the only
+ * events we should ever receive.  Conversely, all events below
+ * here should only be triggered by modern servers, and all
+ * contain a callbackID.  Although we have to duplicate the first
+ * 16 domain events in both styles for back-compat, any future
+ * domain event additions should only use the modern style.  */
 { REMOTE_PROC_NETWORK_EVENT_LIFECYCLE,
   remoteNetworkBuildEventLifecycle,
   sizeof(remote_network_event_lifecycle_msg),
   (xdrproc_t)xdr_remote_network_event_lifecycle_msg },
+{ REMOTE_PROC_DOMAIN_EVENT_CALLBACK_LIFECYCLE,
+  remoteDomainBuildEventCallbackLifecycle,
+  sizeof(remote_domain_event_callback_lifecycle_msg),
+  (xdrproc_t)xdr_remote_domain_event_callback_lifecycle_msg },
 };

 enum virDrvOpenRemoteFlags {
@@ -4452,16 +4470,38 @@ remoteConnectDomainEventRegister(virConnectPtr conn,

VIR_DOMAIN_EVENT_ID_LIFECYCLE,

VIR_DOMAIN_EVENT_CALLBACK(callback),
opaque, freecb, true,
-   &callbackID, false)) < 0)
+   &callbackID,
+   priv->serverEventFilter)) < 
0)
  goto done;

 if (count == 1) {
 /* Tell the server when we are the first callback registering */
-if (call(conn, priv, 0, REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER,
- (xdrproc_t) xdr_void, (char *) NULL,
- (xdrproc_t) xdr_void, (char *) NULL) == -1) {
-virDomainEventStateDeregister(conn, priv->eventState, callback);
-goto done;
+if (priv->serverEventFilter) {
+remote_connect_domain_event_callback_register_any_args args;
+remote_connect_domain_event_callback_register_any_ret ret;
+
+args.eventID = VIR_DOMAIN_EVENT_ID_LIFECYCLE;
+args.dom = NULL;
+
+memset(&ret, 0, sizeof(ret));
+if (call(conn, priv, 0,
+ REMOTE_PROC_CONNECT_DOMAIN_EVENT_CALLBACK_REGISTER_ANY,
+ (xdrproc_t) 
xdr_remote_connect_domain_event_callback_register_any_args, (char *) &args,
+ (xdrproc_t) 
xdr_remote_connect_domain_event_callback_register_any_ret, (cha

[libvirt] [PATCH 3/5] event: prepare client to track domain callbackID

[Eric Blake]

We want to convert over to server-side events, even for older
APIs.  To do that, the client side of the remote driver wants
to distinguish between legacy virConnectDomainEventRegister and
normal virConnectDomainEventRegisterAny, while knowing the
client callbackID and the server's serverID for both types of
registration.  The client also needs to probe whether the
server supports server-side filtering.  However, for ease of
review, we don't actually use the new RPCs until a later patch.

* src/conf/object_event_private.h (virObjectEventStateCallbackID):
Add parameter.
* src/conf/object_event.c (virObjectEventCallbackListAddID)
(virObjectEventStateRegisterID): Separate legacy from callbackID.
(virObjectEventStateCallbackID): Pass through parameter.
* src/conf/network_event.c (virNetworkEventStateRegisterID):
Update caller.
* src/conf/domain_event.c (virDomainEventStateRegister)
(virDomainEventStateRegisterID, virDomainEventStateDeregister):
Likewise.
(virDomainEventStateRegisterClient)
(virDomainEventStateCallbackID): Implement new functions.
* src/conf/domain_event.h (virDomainEventStateRegisterClient)
(virDomainEventStateCallbackID): New prototypes.
* src/remote/remote_driver.c (private_data): Add field.
(doRemoteOpen): Probe server feature.
(remoteConnectDomainEventRegister)
(remoteConnectDomainEventRegisterAny): Use new function.

Signed-off-by: Eric Blake 
---
 src/conf/domain_event.c | 78 +++--
 src/conf/domain_event.h | 22 
 src/conf/network_event.c|  6 ++--
 src/conf/object_event.c | 31 
 src/conf/object_event_private.h |  6 ++--
 src/remote/remote_driver.c  | 36 +++
 6 files changed, 152 insertions(+), 27 deletions(-)

diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index f56aed3..29fc525 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -1307,6 +1307,8 @@ virDomainEventStateRegister(virConnectPtr conn,
 void *opaque,
 virFreeCallback freecb)
 {
+int callbackID;
+
 if (virDomainEventsInitialize() < 0)
 return -1;

@@ -1315,7 +1317,8 @@ virDomainEventStateRegister(virConnectPtr conn,
  filter, virDomainEventClass,
  VIR_DOMAIN_EVENT_ID_LIFECYCLE,
  VIR_OBJECT_EVENT_CALLBACK(callback),
- opaque, freecb, NULL, false);
+ opaque, freecb,
+ true, &callbackID, false);
 }


@@ -1355,7 +1358,75 @@ virDomainEventStateRegisterID(virConnectPtr conn,
  filter ? virDomainEventFilter : NULL,
  filter, virDomainEventClass, eventID,
  VIR_OBJECT_EVENT_CALLBACK(cb),
- opaque, freecb, callbackID, false);
+ opaque, freecb,
+ false, callbackID, false);
+}
+
+
+/**
+ * virDomainEventStateRegisterClient:
+ * @conn: connection to associate with callback
+ * @state: object event state
+ * @dom: optional domain for filtering the event
+ * @eventID: ID of the event type to register for
+ * @cb: function to invoke when event fires
+ * @opaque: data blob to pass to @callback
+ * @freecb: callback to free @opaque
+ * @legacy: true if callback is tracked by function instead of callbackID
+ * @callbackID: filled with callback ID
+ * @remoteID: true if server supports filtering
+ *
+ * Register the function @cb with connection @conn, from @state, for
+ * events of type @eventID, and return the registration handle in
+ * @callbackID.  This version is intended for use on the client side
+ * of RPC.
+ *
+ * Returns: the number of callbacks now registered, or -1 on error
+ */
+int
+virDomainEventStateRegisterClient(virConnectPtr conn,
+  virObjectEventStatePtr state,
+  virDomainPtr dom,
+  int eventID,
+  virConnectDomainEventGenericCallback cb,
+  void *opaque,
+  virFreeCallback freecb,
+  bool legacy,
+  int *callbackID,
+  bool remoteID)
+{
+if (virDomainEventsInitialize() < 0)
+return -1;
+
+return virObjectEventStateRegisterID(conn, state, dom ? dom->uuid : NULL,
+ NULL, NULL,
+ virDomainEventClass, eventID,
+ VIR_OBJECT_EVENT_CALLBACK(cb),
+ opaque, freecb,
+

[libvirt] [PATCH 2/5] event: server RPC protocol tweaks for domain lifecycle events

[Eric Blake]

This patch adds some new RPC call numbers, but for ease of review,
they sit idle until a later patch adds the client counterpart to
drive the new RPCs.  Also for ease of review, I limited this patch
to just the lifecycle event; although converting the remaining
15 domain events will be quite mechanical.  On the server side,
we have to have a function per RPC call, largely with duplicated
bodies (the key difference being that we store in our callback
opaque pointer whether events should be fired with old or new
style); meanwhile, a single function can drive multiple RPC
messages, along with a strategic choice of XDR struct layout,
makes the event generation code for both styles fairly compact.

I debated about adding a tri-state witness variable per
connection (values 'unknown', 'legacy', 'modern').  It would start
as 'unknown', move to 'legacy' if any RPC call is made to a legacy
event call, and move to 'modern' if the feature probe is made;
then the event code could issue an error if the witness state is
incorrect (a legacy RPC call while in 'modern', a modern RPC call
while in 'unknown' or 'legacy', and a feature probe while in
'legacy' or 'modern').  But while it might prevent odd behavior
caused by protocol fuzzing, I don't see that it would prevent
any security holes, so I considered it bloat.

* src/libvirt_internal.h (VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK):
New feature.
* src/remote/remote_protocol.x
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_CALLBACK_REGISTER_ANY)
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_CALLBACK_DEREGISTER_ANY)
(REMOTE_PROC_DOMAIN_EVENT_CALLBACK_LIFECYCLE): New RPCs.
* daemon/remote.c (daemonClientCallback): Add field.
(remoteDispatchConnectDomainEventCallbackRegisterAny)
(remoteDispatchConnectDomainEventCallbackDeregisterAny): New
functions.
(remoteDispatchConnectDomainEventRegisterAny)
(remoteDispatchConnectDomainEventDeregisterAny): Mark legacy use.
(remoteRelayDomainEventLifecycle): Change message based on legacy
or new use.
(remoteDispatchConnectSupportsFeature): Advertise new feature.
* src/remote_protocol-structs: Regenerate.

Signed-off-by: Eric Blake 
---
 daemon/remote.c  | 173 ---
 src/libvirt_internal.h   |   7 +-
 src/remote/remote_protocol.x |  39 +-
 src/remote_protocol-structs  |  17 +
 4 files changed, 225 insertions(+), 11 deletions(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index f0b9922..05fb708 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -76,6 +76,7 @@ struct daemonClientEventCallback {
 virNetServerClientPtr client;
 int eventID;
 int callbackID;
+bool legacy;
 };

 static virDomainPtr get_nonnull_domain(virConnectPtr conn, 
remote_nonnull_domain domain);
@@ -140,8 +141,8 @@ remoteRelayDomainEventLifecycle(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 if (callback->callbackID < 0)
 return -1;

-VIR_DEBUG("Relaying domain lifecycle event %d %d, callback %d",
-  event, detail, callback->callbackID);
+VIR_DEBUG("Relaying domain lifecycle event %d %d, callback %d legacy %d",
+  event, detail, callback->callbackID, callback->legacy);

 /* build return data */
 memset(&data, 0, sizeof(data));
@@ -149,9 +150,20 @@ remoteRelayDomainEventLifecycle(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 data.event = event;
 data.detail = detail;

-remoteDispatchObjectEventSend(callback->client, remoteProgram,
-  REMOTE_PROC_DOMAIN_EVENT_LIFECYCLE,
-  
(xdrproc_t)xdr_remote_domain_event_lifecycle_msg, &data);
+if (callback->legacy) {
+remoteDispatchObjectEventSend(callback->client, remoteProgram,
+  REMOTE_PROC_DOMAIN_EVENT_LIFECYCLE,
+  
(xdrproc_t)xdr_remote_domain_event_lifecycle_msg,
+  &data);
+} else {
+remote_domain_event_callback_lifecycle_msg msg = { 
callback->callbackID,
+   data };
+
+remoteDispatchObjectEventSend(callback->client, remoteProgram,
+  
REMOTE_PROC_DOMAIN_EVENT_CALLBACK_LIFECYCLE,
+  
(xdrproc_t)xdr_remote_domain_event_callback_lifecycle_msg,
+  &msg);
+}

 return 0;
 }
@@ -3206,6 +3218,7 @@ remoteDispatchConnectDomainEventRegister(virNetServerPtr 
server ATTRIBUTE_UNUSED
 callback->client = client;
 callback->eventID = VIR_DOMAIN_EVENT_ID_LIFECYCLE;
 callback->callbackID = -1;
+callback->legacy = true;
 ref = callback;
 if (VIR_APPEND_ELEMENT(priv->domainEventCallbacks,
priv->ndomainEventCallbacks,
@@ -3394,6 +3407,12 @@ cleanup:
 return rv;
 }

+
+/* Due to back-compat reasons, two RPC calls map to the same libvirt
+ * API of virConnectDomainEventRegisterAny.  A client should only use
+ * the ne

[libvirt] [PATCH 0/5] server-side filtering of domain events

[Eric Blake]

This work was originally done with the thought that the fix
for CVE-2014-0028 would require server-side filtering to
make the check of connect:search_domains conditional on
whether the user passed NULL or a domain when registering
for an event.  The final version of the CVE fix no longer
needs the conditional behavior, so there is no longer a rush
to get this in to 1.2.1; but for 1.2.2, the code changes
offer a nice efficiency gain for the use case of libvirtd
managing lots of domains while a client only cares about
events from a small subset of domains.

While this will not be in 1.2.1 proper, I also tested that
the entire series can be backported without breaking the .so
versioning, if any downstream distro wants to include the
efficiency gain as part of their value added maintenance of
an older version.

Eric Blake (5):
  event: dynamically manage server-side RPC domain events
  event: server RPC protocol tweaks for domain lifecycle events
  event: prepare client to track domain callbackID
  event: client RPC protocol tweaks for domain lifecycle events
  event: convert remaining domain events to new style

 daemon/libvirtd.h   |   3 +-
 daemon/remote.c | 801 +---
 src/conf/domain_event.c |  78 +++-
 src/conf/domain_event.h |  22 ++
 src/conf/network_event.c|   6 +-
 src/conf/object_event.c |  31 +-
 src/conf/object_event_private.h |   6 +-
 src/libvirt_internal.h  |   7 +-
 src/remote/remote_driver.c  | 784 ---
 src/remote/remote_protocol.x| 192 +-
 src/remote_protocol-structs |  92 +
 11 files changed, 1649 insertions(+), 373 deletions(-)

-- 
1.8.4.2

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

[libvirt] [PATCH 1/5] event: dynamically manage server-side RPC domain events

[Eric Blake]

This patch continues the earlier conversion made for network
events, with a goal of introducing server-side event filtering
in a later patch.  Actual behavior is unchanged without
further RPC changes.

* daemon/libvirtd.h (daemonClientPrivate): Alter the tracking of
domain events.
* daemon/remote.c (remoteClientInitHook, remoteClientFreeFunc)
(remoteRelayDomainEvent*)
(remoteDispatchConnectDomainEventRegister)
(remoteDispatchConnectDomainEventRegisterAny): Track domain
callbacks dynamically.
---
 daemon/libvirtd.h |   3 +-
 daemon/remote.c   | 436 +-
 2 files changed, 266 insertions(+), 173 deletions(-)

diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h
index a260ea1..c4f1f27 100644
--- a/daemon/libvirtd.h
+++ b/daemon/libvirtd.h
@@ -51,7 +51,8 @@ struct daemonClientPrivate {
 /* Hold while accessing any data except conn */
 virMutex lock;

-int domainEventCallbackID[VIR_DOMAIN_EVENT_ID_LAST];
+daemonClientEventCallbackPtr *domainEventCallbacks;
+size_t ndomainEventCallbacks;
 daemonClientEventCallbackPtr *networkEventCallbacks;
 size_t nnetworkEventCallbacks;

diff --git a/daemon/remote.c b/daemon/remote.c
index d2aafbe..f0b9922 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -127,19 +127,21 @@ remoteEventCallbackFree(void *opaque)
 VIR_FREE(opaque);
 }

-static int remoteRelayDomainEventLifecycle(virConnectPtr conn ATTRIBUTE_UNUSED,
-   virDomainPtr dom,
-   int event,
-   int detail,
-   void *opaque)
+static int
+remoteRelayDomainEventLifecycle(virConnectPtr conn ATTRIBUTE_UNUSED,
+virDomainPtr dom,
+int event,
+int detail,
+void *opaque)
 {
-virNetServerClientPtr client = opaque;
+daemonClientEventCallbackPtr callback = opaque;
 remote_domain_event_lifecycle_msg data;

-if (!client)
+if (callback->callbackID < 0)
 return -1;

-VIR_DEBUG("Relaying domain lifecycle event %d %d", event, detail);
+VIR_DEBUG("Relaying domain lifecycle event %d %d, callback %d",
+  event, detail, callback->callbackID);

 /* build return data */
 memset(&data, 0, sizeof(data));
@@ -147,30 +149,32 @@ static int remoteRelayDomainEventLifecycle(virConnectPtr 
conn ATTRIBUTE_UNUSED,
 data.event = event;
 data.detail = detail;

-remoteDispatchObjectEventSend(client, remoteProgram,
+remoteDispatchObjectEventSend(callback->client, remoteProgram,
   REMOTE_PROC_DOMAIN_EVENT_LIFECYCLE,
   
(xdrproc_t)xdr_remote_domain_event_lifecycle_msg, &data);

 return 0;
 }

-static int remoteRelayDomainEventReboot(virConnectPtr conn ATTRIBUTE_UNUSED,
-virDomainPtr dom,
-void *opaque)
+static int
+remoteRelayDomainEventReboot(virConnectPtr conn ATTRIBUTE_UNUSED,
+ virDomainPtr dom,
+ void *opaque)
 {
-virNetServerClientPtr client = opaque;
+daemonClientEventCallbackPtr callback = opaque;
 remote_domain_event_reboot_msg data;

-if (!client)
+if (callback->callbackID < 0)
 return -1;

-VIR_DEBUG("Relaying domain reboot event %s %d", dom->name, dom->id);
+VIR_DEBUG("Relaying domain reboot event %s %d, callback %d",
+  dom->name, dom->id, callback->callbackID);

 /* build return data */
 memset(&data, 0, sizeof(data));
 make_nonnull_domain(&data.dom, dom);

-remoteDispatchObjectEventSend(client, remoteProgram,
+remoteDispatchObjectEventSend(callback->client, remoteProgram,
   REMOTE_PROC_DOMAIN_EVENT_REBOOT,
   
(xdrproc_t)xdr_remote_domain_event_reboot_msg, &data);

@@ -178,25 +182,27 @@ static int remoteRelayDomainEventReboot(virConnectPtr 
conn ATTRIBUTE_UNUSED,
 }


-static int remoteRelayDomainEventRTCChange(virConnectPtr conn ATTRIBUTE_UNUSED,
-   virDomainPtr dom,
-   long long offset,
-   void *opaque)
+static int
+remoteRelayDomainEventRTCChange(virConnectPtr conn ATTRIBUTE_UNUSED,
+virDomainPtr dom,
+long long offset,
+void *opaque)
 {
-virNetServerClientPtr client = opaque;
+daemonClientEventCallbackPtr callback = opaque;
 remote_domain_event_rtc_change_msg data;

-if (!client)
+if (callback->callbackID < 0)
 return -1;

-VIR_DEBUG("Relaying domain rtc change event %s %d %lld", dom->name, 
dom->id, offset);
+VIR_DEBUG("R

[libvirt] [PATCH 4/4] event: filter global events by domain:getattr ACL [CVE-2014-0028]

[Eric Blake]

Ever since ACL filtering was added in commit 7639736 (v1.1.1), a
user could still use event registration to obtain access to a
domain that they could not normally access via virDomainLookup*
or virConnectListAllDomains and friends.  We already have the
framework in the RPC generator for creating the filter, and
previous cleanup patches got us to the point that we can now
wire the filter through the entire object event stack.

Furthermore, whether or not domain:getattr is honored, use of
global events is a form of obtaining a list of networks, which
is covered by connect:search_domains added in a93cd08 (v1.1.0).
Ideally, we'd have a way to enforce connect:search_domains when
doing global registrations while omitting that check on a
per-domain registration.  But this patch just unconditionally
requires connect:search_domains, even when no list could be
obtained, based on the following observations:
1. Administrators are unlikely to grant domain:getattr for one
or all domains while still denying connect:search_domains - a
user that is able to manage domains will want to be able to
manage them efficiently, but efficient management includes being
able to list the domains they can access.  The idea of denying
connect:search_domains while still granting access to individual
domains is therefore not adding any real security, but just
serves as a layer of obscurity to annoy the end user.
2. In the current implementation, domain events are filtered
on the client; the server has no idea if a domain filter was
requested, and must therefore assume that all domain event
requests are global.  Even if we fix the RPC protocol to
allow for server-side filtering for newer client/server combos,
making the connect:serach_domains ACL check conditional on
whether the domain argument was NULL won't benefit older clients.
Therefore, we choose to document that connect:search_domains
is a pre-requisite to any domain event management.

Network events need the same treatment, with the obvious
change of using connect:search_networks and network:getattr.

* src/access/viraccessperm.h
(VIR_ACCESS_PERM_CONNECT_SEARCH_DOMAINS)
(VIR_ACCESS_PERM_CONNECT_SEARCH_NETWORKS): Document additional
effect of the permission.
* src/conf/domain_event.h (virDomainEventStateRegister)
(virDomainEventStateRegisterID): Add new parameter.
* src/conf/network_event.h (virNetworkEventStateRegisterID):
Likewise.
* src/conf/object_event_private.h (virObjectEventStateRegisterID):
Likewise.
* src/conf/object_event.c (_virObjectEventCallback): Track a filter.
(virObjectEventDispatchMatchCallback): Use filter.
(virObjectEventCallbackListAddID): Register filter.
* src/conf/domain_event.c (virDomainEventFilter): New function.
(virDomainEventStateRegister, virDomainEventStateRegisterID):
Adjust callers.
* src/conf/network_event.c (virNetworkEventFilter): New function.
(virNetworkEventStateRegisterID): Adjust caller.
* src/remote/remote_protocol.x
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER)
(REMOTE_PROC_CONNECT_DOMAIN_EVENT_REGISTER_ANY)
(REMOTE_PROC_CONNECT_NETWORK_EVENT_REGISTER_ANY): Generate a
filter, and require connect:search_domains instead of weaker
connect:read.
* src/test/test_driver.c (testConnectDomainEventRegister)
(testConnectDomainEventRegisterAny)
(testConnectNetworkEventRegisterAny): Update callers.
* src/remote/remote_driver.c (remoteConnectDomainEventRegister)
(remoteConnectDomainEventRegisterAny): Likewise.
* src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister)
(xenUnifiedConnectDomainEventRegisterAny): Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc): Likewise.
* src/libxl/libxl_driver.c (libxlConnectDomainEventRegister)
(libxlConnectDomainEventRegisterAny): Likewise.
* src/qemu/qemu_driver.c (qemuConnectDomainEventRegister)
(qemuConnectDomainEventRegisterAny): Likewise.
* src/uml/uml_driver.c (umlConnectDomainEventRegister)
(umlConnectDomainEventRegisterAny): Likewise.
* src/network/bridge_driver.c
(networkConnectNetworkEventRegisterAny): Likewise.
* src/lxc/lxc_driver.c (lxcConnectDomainEventRegister)
(lxcConnectDomainEventRegisterAny): Likewise.

Signed-off-by: Eric Blake 
---
 src/access/viraccessperm.h  |  6 +++---
 src/conf/domain_event.c | 34 --
 src/conf/domain_event.h | 10 +++---
 src/conf/network_event.c| 31 ++-
 src/conf/network_event.h|  6 --
 src/conf/object_event.c | 23 +++
 src/conf/object_event_private.h | 20 ++--
 src/libxl/libxl_driver.c|  2 ++
 src/lxc/lxc_driver.c|  2 ++
 src/network/bridge_driver.c |  1 +
 src/qemu/qemu_driver.c  |  2 ++
 src/remote/remote_driver.c  |  4 ++--
 src/remote/remote_protocol.x| 11 +++
 src/test/test_driver.c  |  6 +++---
 src/uml/uml_driver.c|  2 ++
 src/vbox/vbox_tmpl.c|  4 ++--
 src/xen/xen_driver.c|  2 ++
 17 files changed, 138 insertions(+), 28 del

[libvirt] [PATCH 0/4] CVE-2014-0028: domain events vs. ACL filtering

[Eric Blake]

I have pushed the following series to the master branch, as well as
the the backport of patch 4 to all branches impacted by the CVE
(v1.1.0 onwards).  Basically, when ACLs permit fine-grained control
of what domains a user can manage, a user that was denied
domain:getattr for a particular domain, or denied
connect:search_domains in general, could use the event registration
API to gain access to domains that should have been hidden from that
user.  The patch was reviewed offlist during the time when the
vulnerability was under embargo.

In the process of fixing this, I made quite a few improvements to
the underlying mechanisms for events.  Among other things, I
want to switch libvirt over to using server-side filtering rather
than the current implementation of client-side filtering, for
increased efficiency in the case where a hypervisor hosts many
guests but the client only cares about events on a small subset of
those guests.  The existing RPC calls for domain events did not
allow this, but the brand new network events had not yet had their
RPC baked with a formal release.  At one point, I had tried making
the use of connect:search_networks conditional on whether a
non-NULL network had been requested, which requires server-side
filtering.  The final incarnation of the CVE fix no longer bypasses
connect:search_networks for a NULL network, so the first three
patches are now technically unrelated to the CVE fix; but as the
work is already done and reviewed and as it is easier to avoid
bloat in the RPC protocol by getting it right from the beginning,
I still pushed those patches to the master branch.  NOTE: if you
were testing network events with libvirt.git or with the 1.2.1
release candidates, you must ensure that you match your client's
use of libvirt.so with the libvirtd - early users of network
events are unable to communicate with the RPC wire representation
that will actually be in 1.2.1 as a result of this series.

I will also be posting a followup series, for application after
1.2.1 is released, which adds server-side filtering of domain
events, as the counterpart of the network event filtering
added in this series.  There, we already have existing RPC code
baked into releases, so there is no longer a rush to get the
patches in before a release freezes a mistake.

Eric Blake (4):
  event: track callbackID on daemon side of RPC
  event: add notion of remoteID for filtering client network events
  event: wire up RPC for server-side network event filtering
  event: filter global events by domain:getattr ACL [CVE-2014-0028]

 daemon/libvirtd.h   |   7 +-
 daemon/remote.c | 131 +
 src/access/viraccessperm.h  |   6 +-
 src/conf/domain_event.c |  38 ++-
 src/conf/domain_event.h |  10 +-
 src/conf/network_event.c|  69 -
 src/conf/network_event.h|  18 +++-
 src/conf/object_event.c | 212 
 src/conf/object_event.h |  30 +++---
 src/conf/object_event_private.h |  31 ++
 src/libvirt_private.syms|   1 -
 src/libxl/libxl_driver.c|   2 +
 src/lxc/lxc_driver.c|   2 +
 src/network/bridge_driver.c |   1 +
 src/qemu/qemu_driver.c  |   2 +
 src/remote/remote_driver.c  |  86 +---
 src/remote/remote_protocol.x|  23 ++---
 src/remote_protocol-structs |   9 +-
 src/test/test_driver.c  |   6 +-
 src/uml/uml_driver.c|   2 +
 src/vbox/vbox_tmpl.c|   4 +-
 src/xen/xen_driver.c|   2 +
 22 files changed, 527 insertions(+), 165 deletions(-)

-- 
1.8.4.2

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

[libvirt] [PATCH 2/4] event: add notion of remoteID for filtering client network events

[Eric Blake]

In order to mirror a server with per-object filtering, the client
needs to track which server callbackID is servicing the client
callback.  This patch introduces the notion of a serverID, as
well as the plumbing to use it for network events, although the
actual complexity of using per-object filtering in the remote
driver is deferred to a later patch.

* src/conf/object_event.h (virObjectEventStateEventID): Add parameter.
(virObjectEventStateQueueRemote, virObjectEventStateSetRemote):
New prototypes.
(virObjectEventStateRegisterID): Move...
* src/conf/object_event_private.h: ...here, and add parameter.
(_virObjectEvent): Add field.
* src/conf/network_event.h (virNetworkEventStateRegisterClient): New
prototype.
* src/conf/object_event.c (_virObjectEventCallback): Add field.
(virObjectEventStateSetRemote): New function.
(virObjectEventStateQueue): Make wrapper around...
(virObjectEventStateQueueRemote): New function.
(virObjectEventCallbackListCount): Tweak return count when remote
id matching is used.
(virObjectEventCallbackLookup, virObjectEventStateRegisterID):
Tweak registration when remote id matching will be used.
(virObjectEventNew): Default to no remote id.
(virObjectEventCallbackListAddID): Likewise, but set remote id
when one is available.
(virObjectEventCallbackListRemoveID)
(virObjectEventCallbackListMarkDeleteID): Adjust return value when
remote id was set.
(virObjectEventStateEventID): Query existing id.
(virObjectEventDispatchMatchCallback): Require matching event id.
(virObjectEventStateCallbackID): Adjust caller.
* src/conf/network_event.c (virNetworkEventStateRegisterClient): New
function.
(virNetworkEventStateRegisterID): Update caller.
* src/conf/domain_event.c (virDomainEventStateRegister)
(virDomainEventStateRegisterID): Update callers.
* src/remote/remote_driver.c
(remoteConnectNetworkEventRegisterAny)
(remoteConnectNetworkEventDeregisterAny)
(remoteConnectDomainEventDeregisterAny): Likewise.
(remoteEventQueue): Hoist earlier to avoid forward declaration,
and add parameter.  Adjust all callers.
* src/libvirt_private.syms (conf/object_event.h): Drop function.

Signed-off-by: Eric Blake 
---
 src/conf/domain_event.c |   4 +-
 src/conf/network_event.c|  42 -
 src/conf/network_event.h|  16 +++-
 src/conf/object_event.c | 189 
 src/conf/object_event.h |  30 ---
 src/conf/object_event_private.h |  15 
 src/libvirt_private.syms|   1 -
 src/remote/remote_driver.c  |  67 +++---
 8 files changed, 280 insertions(+), 84 deletions(-)

diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 35212ef..b934cc7 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -1288,7 +1288,7 @@ virDomainEventStateRegister(virConnectPtr conn,
  virDomainEventClass,
  VIR_DOMAIN_EVENT_ID_LIFECYCLE,
  VIR_OBJECT_EVENT_CALLBACK(callback),
- opaque, freecb, NULL);
+ opaque, freecb, NULL, false);
 }


@@ -1325,7 +1325,7 @@ virDomainEventStateRegisterID(virConnectPtr conn,
 return virObjectEventStateRegisterID(conn, state, dom ? dom->uuid : NULL,
  virDomainEventClass, eventID,
  VIR_OBJECT_EVENT_CALLBACK(cb),
- opaque, freecb, callbackID);
+ opaque, freecb, callbackID, false);
 }


diff --git a/src/conf/network_event.c b/src/conf/network_event.c
index 3cd884d..d9c47b8 100644
--- a/src/conf/network_event.c
+++ b/src/conf/network_event.c
@@ -154,7 +154,47 @@ virNetworkEventStateRegisterID(virConnectPtr conn,
 return virObjectEventStateRegisterID(conn, state, net ? net->uuid : NULL,
  virNetworkEventClass, eventID,
  VIR_OBJECT_EVENT_CALLBACK(cb),
- opaque, freecb, callbackID);
+ opaque, freecb, callbackID, false);
+}
+
+
+/**
+ * virNetworkEventStateRegisterClient:
+ * @conn: connection to associate with callback
+ * @state: object event state
+ * @net: network to filter on or NULL for all networks
+ * @eventID: ID of the event type to register for
+ * @cb: function to invoke when event occurs
+ * @opaque: data blob to pass to @callback
+ * @freecb: callback to free @opaque
+ * @callbackID: filled with callback ID
+ *
+ * Register the function @cb with connection @conn, from @state, for
+ * events of type @eventID, and return the registration handle in
+ * @callbackID.  This version is intended for use on the client side
+ * of RPC.
+ *
+ * Returns: the number of callbacks now registered, or -1 on error
+ */
+int
+virNetworkEventStateRegisterClient(virConnectPtr 

[libvirt] [PATCH 1/4] event: track callbackID on daemon side of RPC

[Eric Blake]

Right now, the daemon side of RPC events is hard-coded to at most
one callback per eventID.  But when there are hundreds of domains
or networks coupled and multiple conections, then sending every
event to every connection that wants an event, even for the
connections that only care about events for a particular object,
is inefficient.  In order to track more than one callback in the
server, we need to store callbacks by more than just their
eventID.  This patch rearranges the daemon side to store network
callbacks in a dynamic array, which can eventually be used for
multiple callbacks of the same eventID, although actual behavior
is unchanged without further patches to the RPC protocol.  For
ease of review, domain events are saved for a later patch, as
they touch more code.

While at it, fix a bug where a malicious client could send a
negative eventID to cause network event registration to access
outside of array bounds (thankfully not a CVE, since domain
events were already doing the bounds check, and since network
events have not been released).

* daemon/libvirtd.h (daemonClientPrivate): Alter the tracking of
network events.
* daemon/remote.c (daemonClientEventCallback): New struct.
(remoteEventCallbackFree): New function.
(remoteClientInitHook, remoteRelayNetworkEventLifecycle)
(remoteClientFreeFunc)
(remoteDispatchConnectNetworkEventRegisterAny): Track network
callbacks differently.
(remoteDispatchConnectNetworkEventDeregisterAny): Enforce bounds.

Signed-off-by: Eric Blake 
---
 daemon/libvirtd.h |   7 +++-
 daemon/remote.c   | 113 ++
 2 files changed, 85 insertions(+), 35 deletions(-)

diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h
index 47f2589..a260ea1 100644
--- a/daemon/libvirtd.h
+++ b/daemon/libvirtd.h
@@ -1,7 +1,7 @@
 /*
  * libvirtd.h: daemon data structure definitions
  *
- * Copyright (C) 2006-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2014 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -43,6 +43,8 @@ typedef struct daemonClientStream daemonClientStream;
 typedef daemonClientStream *daemonClientStreamPtr;
 typedef struct daemonClientPrivate daemonClientPrivate;
 typedef daemonClientPrivate *daemonClientPrivatePtr;
+typedef struct daemonClientEventCallback daemonClientEventCallback;
+typedef daemonClientEventCallback *daemonClientEventCallbackPtr;

 /* Stores the per-client connection state */
 struct daemonClientPrivate {
@@ -50,7 +52,8 @@ struct daemonClientPrivate {
 virMutex lock;

 int domainEventCallbackID[VIR_DOMAIN_EVENT_ID_LAST];
-int networkEventCallbackID[VIR_NETWORK_EVENT_ID_LAST];
+daemonClientEventCallbackPtr *networkEventCallbacks;
+size_t nnetworkEventCallbacks;

 # if WITH_SASL
 virNetSASLSessionPtr sasl;
diff --git a/daemon/remote.c b/daemon/remote.c
index 6ce9ec1..ae318b2 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1,7 +1,7 @@
 /*
  * remote.c: handlers for RPC method calls
  *
- * Copyright (C) 2007-2013 Red Hat, Inc.
+ * Copyright (C) 2007-2014 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -72,6 +72,12 @@
 # define HYPER_TO_ULONG(_to, _from) (_to) = (_from)
 #endif

+struct daemonClientEventCallback {
+virNetServerClientPtr client;
+int eventID;
+int callbackID;
+};
+
 static virDomainPtr get_nonnull_domain(virConnectPtr conn, 
remote_nonnull_domain domain);
 static virNetworkPtr get_nonnull_network(virConnectPtr conn, 
remote_nonnull_network network);
 static virInterfacePtr get_nonnull_interface(virConnectPtr conn, 
remote_nonnull_interface iface);
@@ -115,6 +121,12 @@ remoteDispatchObjectEventSend(virNetServerClientPtr client,
   xdrproc_t proc,
   void *data);

+static void
+remoteEventCallbackFree(void *opaque)
+{
+VIR_FREE(opaque);
+}
+
 static int remoteRelayDomainEventLifecycle(virConnectPtr conn ATTRIBUTE_UNUSED,
virDomainPtr dom,
int event,
@@ -654,19 +666,21 @@ static virConnectDomainEventGenericCallback 
domainEventCallbacks[] = {

 verify(ARRAY_CARDINALITY(domainEventCallbacks) == VIR_DOMAIN_EVENT_ID_LAST);

-static int remoteRelayNetworkEventLifecycle(virConnectPtr conn 
ATTRIBUTE_UNUSED,
-virNetworkPtr net,
-int event,
-int detail,
-void *opaque)
+static int
+remoteRelayNetworkEventLifecycle(virConnectPtr conn ATTRIBUTE_UNUSED,
+ virNetworkPtr net,
+ int event,
+ int detail,
+ void *opaque)
 {
-virNetServerClientPtr client = 

[libvirt] [PATCH 3/4] event: wire up RPC for server-side network event filtering

[Eric Blake]

We haven't had a release with network events yet, so we are free
to fix the RPC so that it actually does what we want.  Doing
client-side filtering of per-network events is inefficient if a
connection is only interested in events on a single network out
of hundreds available on the server.  But to do server-side
per-network filtering, the server needs to know which network
to filter on - so we need to pass an optional network over on
registration.  Furthermore, it is possible to have a client with
both a global and per-network filter; in the existing code, the
server sends only one event and the client replicates to both
callbacks.  But with server-side filtering, the server will send
the event twice, so we need a way for the client to know which
callbackID is sending an event, to ensure that the client can
filter out events from a registration that does not match the
callbackID from the server.  Likewise, the existing style of
deregistering by eventID alone is fine; but in the new style,
we have to remember which callbackID to delete.

This patch fixes the RPC wire definition to contain all the
needed pieces of information, and hooks into the server and
client side improvements of the previous patches, in order to
switch over to full server-side filtering of network events.
Also, since we fixed this in time, all released versions of
libvirtd that support network events also support per-network
filtering, so we can hard-code that assumption into
network_event.c.

Converting domain events to server-side filtering will require
the introduction of new RPC numbers, as well as a server
feature bit that the client can use to tell whether to use
old-style (server only supports global events) or new-style
(server supports filtered events), so that is deferred to a
later set of patches.

* src/conf/network_event.c (virNetworkEventStateRegisterClient):
Assume server-side filtering.
* src/remote/remote_protocol.x
(remote_connect_network_event_register_any_args): Add network
argument.
(remote_connect_network_event_register_any_ret): Return callbackID
instead of count.
(remote_connect_network_event_deregister_any_args): Pass
callbackID instead of eventID.
(remote_connect_network_event_deregister_any_ret): Drop unused
type.
(remote_network_event_lifecycle_msg): Add callbackID.
* daemon/remote.c
(remoteDispatchConnectNetworkEventDeregisterAny): Drop unused arg,
and deal with callbackID from client.
(remoteRelayNetworkEventLifecycle): Pass callbackID.
(remoteDispatchConnectNetworkEventRegisterAny): Likewise, and
recognize non-NULL network.
* src/remote/remote_driver.c
(remoteConnectNetworkEventRegisterAny): Pass network, and track
server side id.
(remoteConnectNetworkEventDeregisterAny): Deregister by callback id.
(remoteNetworkBuildEventLifecycle): Pass remote id to event queue.
* src/remote_protocol-structs: Regenerate.

Signed-off-by: Eric Blake 
---
 daemon/remote.c  | 34 +-
 src/conf/network_event.c |  4 +---
 src/remote/remote_driver.c   | 17 ++---
 src/remote/remote_protocol.x | 12 +---
 src/remote_protocol-structs  |  9 -
 5 files changed, 41 insertions(+), 35 deletions(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index ae318b2..d2aafbe 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -685,6 +685,7 @@ remoteRelayNetworkEventLifecycle(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 /* build return data */
 memset(&data, 0, sizeof(data));
 make_nonnull_network(&data.net, net);
+data.callbackID = callback->callbackID;
 data.event = event;
 data.detail = detail;

@@ -5285,7 +5286,7 @@ 
remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server ATTRIBUTE_UN
  virNetMessagePtr msg 
ATTRIBUTE_UNUSED,
  virNetMessageErrorPtr rerr 
ATTRIBUTE_UNUSED,
  
remote_connect_network_event_register_any_args *args,
- 
remote_connect_network_event_register_any_ret *ret ATTRIBUTE_UNUSED)
+ 
remote_connect_network_event_register_any_ret *ret)
 {
 int callbackID;
 int rv = -1;
@@ -5293,6 +5294,7 @@ 
remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server ATTRIBUTE_UN
 daemonClientEventCallbackPtr ref;
 struct daemonClientPrivate *priv =
 virNetServerClientGetPrivateData(client);
+virNetworkPtr net = NULL;

 if (!priv->conn) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
@@ -5301,6 +5303,10 @@ 
remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server ATTRIBUTE_UN

 virMutexLock(&priv->lock);

+if (args->net &&
+!(net = get_nonnull_network(priv->conn, *args->net)))
+goto cleanup;
+
 if (args->eventID >= VIR_NETWORK_EVENT_ID_LAST || args->eventID < 0) {
 virReportError(VIR_ERR_INTERNAL_ERRO

Re: [libvirt] [PATCHv2 0/6] remaining cleanups to libvirt.c

[Eric Blake]

On 01/15/2014 07:32 AM, John Ferlan wrote:
> 
> 
> On 01/14/2014 04:43 PM, Eric Blake wrote:
>> v2 of my patch series started here, after applying all patches
>> already reviewed in that thread:
>> https://www.redhat.com/archives/libvir-list/2013-December/msg01284.html
>>
>> Patches 1-3 can be considered bug fixes (particularly patch 3),
>> so I'd like them in 1.2.1 if they get a favorable review.  Patches
>> 4-6 are less urgent, but might as well finish the work all within
>> one release.
>>
>> Patch 1 comes from 5/24 in v1

and I squashed in the fix pointed out by Jan.

> 
> Patch 3 seems to be the one where there would be a couple of migration
> fixes that could use a backport, especially the change in
> virDomainMigrateVersion3Full() where the code now actually honors the
> condition where the uri was not set rather than just playing dumb. I
> also keep looking the changed "single" (!ret) to a "multi-state" (!ret
> && !xxx) condition and keep asking myself could we run into a situation
> where (!ret and xxx) is true where previously we'd fail on the !ret, but
> now wouldn't because xxx was true (where xxx is retcode or cancelled).

Yes - the scenario is as follows:

pre-patch:
public entry point virDomainMigrate()
 -> static virDomainMigrateVersion2()
   -> during prepare phase, fails to get URI, dispatches an error then
goes to finish
   -> in finish phase, calls domainMigrateFinish2(cancelled = 1)
  -> MigrateFinish2() will return NULL (because cancelled was set -
if it doesn't return NULL, that's a bug, where I added VIR_WARN to flag
it), but without setting an error
  -> because it returns NULL, we goto the error label
  -> the error label tries to dispatch the error

post-patch, we realize that if we are calling MigrateFinish2() on the
cleanup path of an aborted migration attempt (if cancelled/retcode is
non-zero), then the error message earlier in the 5-step process that
caused migration to be cancelled is the important error, and we really
don't care what happens in the finish step.  That is, we want to return
NULL without raising an error if we know we are calling the finish step
on a cleanup path, and only use the 'goto error' label in the API when
we are in the finish step with all earlier steps of migration successful
so far.

> 
> Another "step" to consider for patch 5 would be to make common the
> repetitive code that follows the virDriverCheckTabMaxReturn(), e.g.
> 
> VIR_DEBUG('string')
> table[count] = driver
> return count++

I didn't think it was worth the further compression.

> 
> I don't see a reason other than proximity to release date to preclude
> inclusion in 1.2.1

So more to the point, should I push now, or wait until post-release?

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [v0.9.12-maint 0/8] Backport changes for CVE-2013-6458 to v0.9.12-maint

[Eric Blake]

On 01/15/2014 01:43 PM, Eric Blake wrote:
> On 01/11/2014 07:27 AM, Guido Günther wrote:
>> Hi,
>> attached patches backport the fixes for CVE-2013-6458 to v0.9.12-maint. I
>> decided to cherry-pick the introduction of VIR_STRDUP and virReportError
>> as well to ease backporting of future fixes. I'd be happy about any review.
> 
> Looks correct to me.  I'll let you push to 0.9.12-maint since you
> already did that work; I already pushed to all the branches 0.10.2 and
> later.  When porting to 0.10.2, I chose to just inline the call to
> strdup() instead of backporting VIR_STRDUP, for fewer patches but more
> conflict resolution; but either approach seems acceptable.

Oh, and I also pushed the two patches for CVE-2014-1447 to all branches
back to 0.10.2.  Since that also exists in 0.9.8, you'll want to include
those two patches in your push to 0.9.12.  There's a conflict resolution
needed in the first of the two patches, if you want to borrow from
0.10.2-maint.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [v0.9.12-maint 0/8] Backport changes for CVE-2013-6458 to v0.9.12-maint

[Eric Blake]

On 01/11/2014 07:27 AM, Guido Günther wrote:
> Hi,
> attached patches backport the fixes for CVE-2013-6458 to v0.9.12-maint. I
> decided to cherry-pick the introduction of VIR_STRDUP and virReportError
> as well to ease backporting of future fixes. I'd be happy about any review.

Looks correct to me.  I'll let you push to 0.9.12-maint since you
already did that work; I already pushed to all the branches 0.10.2 and
later.  When porting to 0.10.2, I chose to just inline the call to
strdup() instead of backporting VIR_STRDUP, for fewer patches but more
conflict resolution; but either approach seems acceptable.

Is anyone still using v0.9.11-maint?  The CVE extends back to 0.9.8, so
we could argue that we should either fix the 0.9.11 branch, or add
another commit to the branch that explicitly marks it as end-of-life
because no one appears to be relying on it.  Fedora 18 is now
end-of-life, so from Fedora's perspective, I only care about 0.10.2
(RHEL and CentOS 6), 1.0.5 (F19), 1.1.3 (F20) and soon 1.2.1 (rawhide),
although I didn't mind touching all the intermediate branches on my way
down to 0.10.2.  RHEL 5 is also vulnerable to CVE-2013-6458, but as we
don't have an upstream v0.8.2-maint branch (thank goodness!), that's
something for Red Hat to worry about.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH] BSD: implement nodeGetCPUStats

[Roman Bogorodskiy]

  Daniel P. Berrange wrote:

> On Tue, Jan 14, 2014 at 12:47:20PM +0400, Roman Bogorodskiy wrote:
> > I had doubts how to implement that. Looks like the current
> > implementation is tied to Linux CPU metrics:
> > 
> >   user nice system idle iowait
> > 
> > That list is hardcoded into virsh-host.c. FreeBSD has a slightly
> > different set of metrics:
> > 
> >   user nice system intr idle
> > 
> > I.e. it's interrupt time instead of i/o time.
> 
> That's not a problem. Just define a new API constant for INTR and
> don't use the IOWAIT constant in your impl. No impl is required to
> support every single possible value.

I've just uploaded new version of the patch.

Meanwhile, I was thinking if it would be reasonable just to count INTR
as system load without introducing a new variable.

Roman Bogorodskiy

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

[libvirt] [PATCH v2] BSD: implement nodeGetCPUStats

[Roman Bogorodskiy]

Implementation obtains CPU usage information using
kern.cp_time and kern.cp_times sysctl(8)s and reports
CPU utilization.
---
 include/libvirt/libvirt.h.in |   8 
 src/nodeinfo.c   | 104 +++
 tools/virsh-host.c   |  11 -
 3 files changed, 121 insertions(+), 2 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 018a5ce..88afe20 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -692,6 +692,14 @@ typedef enum {
 #define VIR_NODE_CPU_STATS_IOWAIT "iowait"
 
 /**
+ * VIR_NODE_CPU_STATS_INTR:
+ *
+ * The cumulative interrupt CPU time,
+ * since the node booting up (in nanoseconds).
+ */
+#define VIR_NODE_CPU_STATS_INTR "intr"
+
+/**
  * VIR_NODE_CPU_STATS_UTILIZATION:
  *
  * The CPU utilization of a node.
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 05bc038..fd2f8c8 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -34,8 +34,10 @@
 #include "conf/domain_conf.h"
 
 #if defined(__FreeBSD__) || defined(__APPLE__)
+# include 
 # include 
 # include 
+# include 
 #endif
 
 #include "c-ctype.h"
@@ -99,8 +101,108 @@ appleFreebsdNodeGetMemorySize(unsigned long *memory)
 #endif /* defined(__FreeBSD__) || defined(__APPLE__) */
 
 #ifdef __FreeBSD__
+# define BSD_CPU_STATS_ALL 4
 # define BSD_MEMORY_STATS_ALL 4
 
+# define TICK_TO_NSEC (1000ull * 1000ull * 1000ull / (stathz ? stathz : hz))
+
+static int
+freebsdNodeGetCPUStats(int cpuNum,
+   virNodeCPUStatsPtr params,
+   int *nparams)
+{
+const char *sysctl_name;
+long *cpu_times;
+struct clockinfo clkinfo;
+size_t i, j, cpu_times_size, clkinfo_size;
+int cpu_times_num, offset, hz, stathz, ret = -1;
+struct field_cpu_map {
+const char *field;
+int idx[CPUSTATES];
+} cpu_map[] = {
+{VIR_NODE_CPU_STATS_KERNEL, {CP_SYS}},
+{VIR_NODE_CPU_STATS_USER, {CP_USER, CP_NICE}},
+{VIR_NODE_CPU_STATS_IDLE, {CP_IDLE}},
+{VIR_NODE_CPU_STATS_INTR, {CP_INTR}},
+{NULL, {0}}
+};
+
+if ((*nparams) == 0) {
+*nparams = BSD_CPU_STATS_ALL;
+return 0;
+}
+
+if ((*nparams) != BSD_CPU_STATS_ALL) {
+virReportInvalidArg(*nparams,
+_("nparams in %s must be equal to %d"),
+__FUNCTION__, BSD_CPU_STATS_ALL);
+return -1;
+}
+
+clkinfo_size = sizeof(clkinfo);
+if (sysctlbyname("kern.clockrate", &clkinfo, &clkinfo_size, NULL, 0) < 0) {
+virReportSystemError(errno,
+ _("sysctl failed for '%s'"),
+ "kern.clockrate");
+return -1;
+}
+
+stathz = clkinfo.stathz;
+hz = clkinfo.hz;
+
+if (cpuNum == VIR_NODE_CPU_STATS_ALL_CPUS) {
+sysctl_name = "kern.cp_time";
+cpu_times_num = 1;
+offset = 0;
+} else {
+sysctl_name = "kern.cp_times";
+cpu_times_num = appleFreebsdNodeGetCPUCount();
+
+if (cpuNum >= cpu_times_num) {
+virReportInvalidArg(cpuNum,
+_("Invalid cpuNum in %s"),
+__FUNCTION__);
+return -1;
+}
+
+offset = cpu_times_num * CPUSTATES;
+}
+
+cpu_times_size = sizeof(long) * cpu_times_num * CPUSTATES;
+
+if (VIR_ALLOC_N(cpu_times, cpu_times_num * CPUSTATES) < 0)
+goto cleanup;
+
+if (sysctlbyname(sysctl_name, cpu_times, &cpu_times_size, NULL, 0) < 0) {
+virReportSystemError(errno,
+ _("sysctl failed for '%s'"),
+ sysctl_name);
+goto cleanup;
+}
+
+for (i = 0; cpu_map[i].field != NULL; i++) {
+virNodeCPUStatsPtr param = ¶ms[i];
+
+if (virStrcpyStatic(param->field, cpu_map[i].field) == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Field '%s' too long for destination"),
+   cpu_map[i].field);
+goto cleanup;
+}
+
+param->value = 0;
+for (j = 0; j < ARRAY_CARDINALITY(cpu_map[i].idx); j++)
+param->value += cpu_times[offset + cpu_map[i].idx[j]] * 
TICK_TO_NSEC;
+}
+
+ret = 0;
+
+cleanup:
+VIR_FREE(cpu_times);
+
+return ret;
+}
+
 static int
 freebsdNodeGetMemoryStats(virNodeMemoryStatsPtr params,
int *nparams)
@@ -1066,6 +1168,8 @@ int nodeGetCPUStats(int cpuNum ATTRIBUTE_UNUSED,
 
 return ret;
 }
+#elif defined(__FreeBSD__)
+return freebsdNodeGetCPUStats(cpuNum, params, nparams);
 #else
 virReportError(VIR_ERR_NO_SUPPORT, "%s",
_("node CPU stats not implemented on this platform"));
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index 1d1bb97..ac41177 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -347,9 +347,10 @@ cmdNodeCpuStats(vshControl *ctl, const vshCmd *cm

Re: [libvirt] [PATCH 2/2] qemu: Avoid operations on NULL monitor if VM fails early

[Peter Krempa]

On 01/15/14 17:45, Michal Privoznik wrote:
> On 14.01.2014 19:31, Peter Krempa wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1047659
>>
>> If a VM dies very early during an attempted connect to the guest agent
>> while the locks are down the domain monitor object will be freed. The
>> object is then accessed later as any failure during guest agent startup
>> isn't considered fatal.
>>
>> In the current upstream version this doesn't lead to a crash as
>> virObjectLock called when entering the monitor in
>> qemuProcessDetectVcpuPIDs checks the pointer before attempting to
>> dereference (lock) it. The NULL pointer is then caught in the monitor
>> helper code.
>>
>> Before the introduction of virObjectLockable - observed on 0.10.2 - the
>> pointer is locked directly via virMutexLock leading to a crash.
>>
>> To avoid this problem we need to differentiate between the guest agent
>> not being present and the VM quitting when the locks were down. The fix
>> reorganizes the code in qemuConnectAgent to add the check and then adds
>> special handling to the callers.
>> ---
>>  src/qemu/qemu_process.c | 34 +-
>>  1 file changed, 25 insertions(+), 9 deletions(-)
>>
> 
> ACK and safe for 1.2.1.
> 
> Michal
> 

Pushed; Thanks.

Peter




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

[libvirt] [PATCH RESEND v4 0/2] Libvirt Wireshark Dissector

[Yuto KAWAMURA(kawamuray)]

From: "Yuto KAWAMURA(kawamuray)" 

Changes from version 3:
* Merge tools/wireshark/.gitignore into ROOT/.gitignore
* Fix dissect_xdr_pointer() bug which pointed by Michal
* Added -module option to libvirt_la_LDFLAGS respecting official build process

Introduce Wireshark dissector plugin which adds support to Wireshark
for dissecting libvirt RPC protocol.

This feature was presented by Michal Privoznik year before last[1].
But it did only support dissecting packet headers.
This time I enhanced that dissector to support dissecting packet
payload. Furthermore, I provide code generator of dissector. So you
can get fresh build of dissector from libvirt RPC specification file
at any version you like.

[1] http://www.redhat.com/archives/libvir-list/2011-October/msg00301.html

Yuto KAWAMURA(kawamuray) (2):
  Introduce Libvirt Wireshark dissector
  Add sample output of Wireshark dissector

 .gitignore  |2 +
 Makefile.am |3 +-
 cfg.mk  |8 +-
 configure.ac|   72 +-
 tools/wireshark/Makefile.am |   22 +
 tools/wireshark/README.md   |   31 +
 tools/wireshark/samples/libvirt-sample.pdml |  206 ++
 tools/wireshark/src/Makefile.am |   42 ++
 tools/wireshark/src/packet-libvirt.c|  512 ++
 tools/wireshark/src/packet-libvirt.h|  128 
 tools/wireshark/util/genxdrstub.pl  | 1011 +++
 tools/wireshark/util/make-dissector-reg |  198 ++
 12 files changed, 2229 insertions(+), 6 deletions(-)
 create mode 100644 tools/wireshark/Makefile.am
 create mode 100644 tools/wireshark/README.md
 create mode 100644 tools/wireshark/samples/libvirt-sample.pdml
 create mode 100644 tools/wireshark/src/Makefile.am
 create mode 100644 tools/wireshark/src/packet-libvirt.c
 create mode 100644 tools/wireshark/src/packet-libvirt.h
 create mode 100755 tools/wireshark/util/genxdrstub.pl
 create mode 100755 tools/wireshark/util/make-dissector-reg

-- 
1.8.3.2

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

[libvirt] [PATCH RESEND v4 2/2] Add sample output of Wireshark dissector

[Yuto KAWAMURA(kawamuray)]

From: "Yuto KAWAMURA(kawamuray)" 

Add directory tools/wireshark/samples/ and
libvirt-sample.pdml which is sample output of dissector.
---
 tools/wireshark/samples/libvirt-sample.pdml | 206 
 1 file changed, 206 insertions(+)
 create mode 100644 tools/wireshark/samples/libvirt-sample.pdml

diff --git a/tools/wireshark/samples/libvirt-sample.pdml 
b/tools/wireshark/samples/libvirt-sample.pdml
new file mode 100644
index 000..f6a4c28
--- /dev/null
+++ b/tools/wireshark/samples/libvirt-sample.pdml
@@ -0,0 +1,206 @@
+
+
+
+
+
+
+
+  
+
+
+
+
+
+
+
+  
+
+
+  
+
+
+
+
+
+
+
+
+  
+
+  
+
+  
+
+
+
+
+  
+
+
+
+
+
+
+
+
+  
+  
+
+  
+
+
+  
+
+
+
+
+
+
+
+  
+
+
+
+
+  
+
+
+
+
+
+
+
+
+  
+
+  
+
+
+  
+
+
+
+
+
+
+
+
+  
+
+
+
+  
+
+  
+
+
+
+
+  
+
+
+
+
+
+
+
+
+  
+  
+  
+
+  
+
+
+  
+
+
+
+
+
+
+
+
+  
+  
+  
+
+  
+
+
+
+
+  
+
+
+
+
+
+
+
+
+  
+
+
+
+  
+  
+  
+
+  
+
+
+  
+
+
+
+
+
+
+
+
+  
+
+  
+  
+
+
+  
+  
+
+  
+  
+
+  
+
+
+
+
+  
+
+
+
+
+
+
+
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
+  
+
+
-- 
1.8.3.2

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

Re: [libvirt] [PATCH 2/2] qemu: Avoid operations on NULL monitor if VM fails early

[Michal Privoznik]

On 14.01.2014 19:31, Peter Krempa wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1047659
> 
> If a VM dies very early during an attempted connect to the guest agent
> while the locks are down the domain monitor object will be freed. The
> object is then accessed later as any failure during guest agent startup
> isn't considered fatal.
> 
> In the current upstream version this doesn't lead to a crash as
> virObjectLock called when entering the monitor in
> qemuProcessDetectVcpuPIDs checks the pointer before attempting to
> dereference (lock) it. The NULL pointer is then caught in the monitor
> helper code.
> 
> Before the introduction of virObjectLockable - observed on 0.10.2 - the
> pointer is locked directly via virMutexLock leading to a crash.
> 
> To avoid this problem we need to differentiate between the guest agent
> not being present and the VM quitting when the locks were down. The fix
> reorganizes the code in qemuConnectAgent to add the check and then adds
> special handling to the callers.
> ---
>  src/qemu/qemu_process.c | 34 +-
>  1 file changed, 25 insertions(+), 9 deletions(-)
> 

ACK and safe for 1.2.1.

Michal

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

[libvirt] [PATCH 2/2] spice: expose the QEMU disable file transfer option

[Francesco Romani]

spice-server offers an API to disable file transfer messages
on the agent channel between the client and the guest.
This is supported in qemu through the disable-agent-file-xfer option.

This patch exposes this option to libvirt.
Adds a new element 'filetransfer', with one property,
'filetransfer', which accepts a boolean setting.
Default is enabled.

Depends on the capability exported in the first patch of the series.

Signed-off-by: Francesco Romani 
---
 docs/formatdomain.html.in  |  8 +
 docs/schemas/domaincommon.rng  | 11 ++
 src/conf/domain_conf.c | 31 -
 src/conf/domain_conf.h | 10 ++
 src/libvirt_private.syms   |  2 ++
 src/qemu/qemu_command.c|  9 +
 ...emuxml2argv-graphics-spice-agent-file-xfer.args |  9 +
 ...qemuxml2argv-graphics-spice-agent-file-xfer.xml | 40 ++
 .../qemuxml2argv-graphics-spice.args   |  5 +--
 .../qemuxml2argv-graphics-spice.xml|  1 +
 tests/qemuxml2argvtest.c   |  9 -
 11 files changed, 131 insertions(+), 4 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 68860ef..c11a7d3 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4042,6 +4042,7 @@ qemu-kvm -net nic,model=? /dev/null
 
 
 
+
   
 
   Spice supports variable compression settings for audio,
@@ -4081,6 +4082,13 @@ qemu-kvm -net nic,model=? /dev/null
   since 0.9.11. If no mode is
   specified, the qemu default will be used (client mode).
 
+
+  File transfer functionality (via Spice agent) is set using the
+  filetransfer element.
+  It is enabled by default, and can be disabled by setting the
+  enable property to no ,
+  since since 1.2.2.
+
   
   "rdp"
   
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index a69f6b6..9ddb772 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2474,6 +2474,17 @@
 
   
 
+
+  
+
+  
+yes
+no
+  
+
+
+  
+
   
 
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c1dd598..7d6c9ba 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -604,6 +604,12 @@ VIR_ENUM_IMPL(virDomainGraphicsSpiceClipboardCopypaste,
   "yes",
   "no");
 
+VIR_ENUM_IMPL(virDomainGraphicsSpiceAgentFileTransfer,
+  VIR_DOMAIN_GRAPHICS_SPICE_AGENT_FILE_TRANSFER_LAST,
+  "default",
+  "yes",
+  "no");
+
 VIR_ENUM_IMPL(virDomainHostdevMode, VIR_DOMAIN_HOSTDEV_MODE_LAST,
   "subsystem",
   "capabilities")
@@ -8519,6 +8525,26 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
 VIR_FREE(copypaste);
 
 def->data.spice.copypaste = copypasteVal;
+} else if (xmlStrEqual(cur->name, BAD_CAST "filetransfer")) {
+char *enable = virXMLPropString(cur, "enable");
+int enableVal;
+
+if (!enable) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("spice filetransfer missing enable"));
+goto error;
+}
+
+if ((enableVal =
+ 
virDomainGraphicsSpiceAgentFileTransferTypeFromString(enable)) <= 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown enable value '%s'"), enable);
+VIR_FREE(enable);
+goto error;
+}
+VIR_FREE(enable);
+
+def->data.spice.filetransfer = enableVal;
 } else if (xmlStrEqual(cur->name, BAD_CAST "mouse")) {
 char *mode = virXMLPropString(cur, "mode");
 int modeVal;
@@ -16423,7 +16449,7 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
 if (!children && (def->data.spice.image || def->data.spice.jpeg ||
   def->data.spice.zlib || def->data.spice.playback ||
   def->data.spice.streami

[libvirt] [PATCH 1/2] spice: detect if qemu can disable file transfer

[Francesco Romani]

spice-server offers an API to disable file transfer messages
on the agent channel between the client and the guest.
This is supported in qemu through the disable-agent-file-xfer option.

This detects if QEMU supports this option, and add
a capability if does.

Signed-off-by: Francesco Romani 
---
 src/qemu/qemu_capabilities.c  | 2 ++
 src/qemu/qemu_capabilities.h  | 1 +
 tests/qemucapabilitiesdata/caps_1.6.0-1.caps  | 1 +
 tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 +
 4 files changed, 5 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 0538115..8420047 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -247,6 +247,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "boot-strict", /* 160 */
   "pvpanic",
   "enable-fips",
+  "spice-file-xfer-disable"
 );
 
 struct _virQEMUCaps {
@@ -2286,6 +2287,7 @@ static struct virQEMUCapsCommandLineProps 
virQEMUCapsCommandLine[] = {
 { "realtime", "mlock", QEMU_CAPS_MLOCK },
 { "boot-opts", "strict", QEMU_CAPS_BOOT_STRICT },
 { "boot-opts", "reboot-timeout", QEMU_CAPS_REBOOT_TIMEOUT },
+{ "spice", "disable-agent-file-xfer", QEMU_CAPS_SPICE_FILE_XFER_DISABLE },
 };
 
 static int
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index efb3f43..23dccce 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -201,6 +201,7 @@ enum virQEMUCapsFlags {
 QEMU_CAPS_BOOT_STRICT= 160, /* -boot strict */
 QEMU_CAPS_DEVICE_PANIC   = 161, /* -device pvpanic */
 QEMU_CAPS_ENABLE_FIPS= 162, /* -enable-fips */
+QEMU_CAPS_SPICE_FILE_XFER_DISABLE = 163, /* -spice disable-agent-file-xfer 
*/
 
 QEMU_CAPS_LAST,   /* this must always be the last item */
 };
diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps 
b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
index 2d50cf9..61542a8 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
@@ -139,4 +139,5 @@
 
 
 
+
   
diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps 
b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
index ba64177..8ce17aa 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
@@ -137,4 +137,5 @@
 
 
 
+
   
-- 
1.8.4.2

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

[libvirt] [PATCH 0/2] v5: spice: expose the disable file transfer option

[Francesco Romani]

Changes v4:
Addressed Christophe's comment and changed the error value.
Addressed Michal's comment and changed the capability is detected
(thanks for the hint on query-command-line-output, I was looking
in the wrong place).

Changes v5:
rebased and squashed (hopefully right this time).

Francesco Romani (2):
  spice: detect if qemu can disable file transfer
  spice: expose the QEMU disable file transfer option

 docs/formatdomain.html.in  |  8 +
 docs/schemas/domaincommon.rng  | 11 ++
 src/conf/domain_conf.c | 31 -
 src/conf/domain_conf.h | 10 ++
 src/libvirt_private.syms   |  2 ++
 src/qemu/qemu_capabilities.c   |  2 ++
 src/qemu/qemu_capabilities.h   |  1 +
 src/qemu/qemu_command.c|  9 +
 tests/qemucapabilitiesdata/caps_1.6.0-1.caps   |  1 +
 tests/qemucapabilitiesdata/caps_1.6.50-1.caps  |  1 +
 ...emuxml2argv-graphics-spice-agent-file-xfer.args |  9 +
 ...qemuxml2argv-graphics-spice-agent-file-xfer.xml | 40 ++
 .../qemuxml2argv-graphics-spice.args   |  5 +--
 .../qemuxml2argv-graphics-spice.xml|  1 +
 tests/qemuxml2argvtest.c   |  9 -
 15 files changed, 136 insertions(+), 4 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.xml

-- 
1.8.4.2

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

Re: [libvirt] [PATCHv2] tests: be more explicit on qcow2 versions in virstoragetest

[Eric Blake]

On 01/15/2014 05:49 AM, Ján Tomko wrote:
> On 01/15/2014 01:19 AM, Eric Blake wrote:
>> While working on v1.0.5-maint (the branch in use on Fedora 19)
>> with the host at Fedora 20, I got a failure in virstoragetest.
> ...
>>
>> * tests/virstoragetest.c (testPrepImages): Simplify creation of
>> raw file; check if qemu supports compat and if so use it.
>>
>> Signed-off-by: Eric Blake 
>> ---
>>
>> v2: rebase to latest; still asking for inclusion in 1.2.1
>>
>>  tests/virstoragetest.c | 32 +++-
>>  1 file changed, 23 insertions(+), 9 deletions(-)
>>
> 
> ACK

Thanks; pushed.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH] docs: mention maintenance branches

[Eric Blake]

On 01/15/2014 07:32 AM, Michal Privoznik wrote:
> On 14.01.2014 17:53, Eric Blake wrote:
>> Mitre tried to assign us two separate CVEs for the fix for
>> https://bugzilla.redhat.com/show_bug.cgi?id=1047577, on the
>> grounds that the fixes were separated by more than an hour
>> and thus triggered different hourly snapshots.  But we
>> explicitly do NOT want to treat transient security bugs as
>> CVEs if they can only be triggered by patches in libvirt.git
>> but where the problem is cleaned up before a formal release.
>>
>> Meanwhile, I noticed that while our wiki mentioned maintenance
>> branches and releases, our formal documentation did not.
>>
>> * docs/downloads.html.in: Contrast hourly snapshots with
>> maintenance branches.
>>
>> Signed-off-by: Eric Blake 
>> ---
>>
>> Doc only, so suitable for 1.2.1 if it gets reviewed in time.


> 
> ACK & safe for the upcoming release.

Thanks; pushed.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH] Fix docs for PMWakeup/PMSuspend callback types

[Claudio Bley]

At Wed, 15 Jan 2014 06:49:10 -0700,
Eric Blake wrote:
> 
> On 01/15/2014 12:28 AM, Claudio Bley wrote:
> > s/is waken up/is woken up/
> > 
> > A registered PMSuspendCallback is called when the domain is suspended, not
> > when it is woken up.
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> > It's just a trivial fix, but I'd like some educated comment on the
> > grammar fix. IMO it is correct, or is there a better way to express
> > this?
> 
> ACK; as a native speaker, your patch sounds right.
> 
> > 
> >  include/libvirt/libvirt.h.in |4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> > index 018a5ce..5bdb2bc 100644
> > --- a/include/libvirt/libvirt.h.in
> > +++ b/include/libvirt/libvirt.h.in
> > @@ -4849,7 +4849,7 @@ typedef void 
> > (*virConnectDomainEventTrayChangeCallback)(virConnectPtr conn,
> >   *  always passes 0
> >   * @opaque: application specified data
> >   *
> > - * This callback occurs when the guest is waken up.
> > + * This callback occurs when the guest is woken up.
> 
> If you want an alternative wording:
> 
> This callback occurs when the guest is awakened.
> 
> But I don't care which of the two wordings you use.

OK, I pushed as presented. Thanks!

Claudio
-- 
AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany
Phone: +49 341 265 310 19
Web:

Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076)
Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern

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

Re: [libvirt] [PATCH v2] vbox: add support for v4.2.20+ and v4.3.4+

[Matthias Bolte]

2014/1/14 Manuel VIVES :
> Hi,
>
> While working on adding virDomain*Stats support to the vbox driver, we
> found bugs in the VirtualBox API C bindings. These bugs have been fixed
> in versions 4.2.20 and 4.3.4.
> However, the changes in the C bindings are incompatible with the
> vbox_CAPI_v4_2.h
> and vbox_CAPI_v4_3.h files which are bundled in libvirt source code. This is
> why the
> following patch adds vbox_CAPI_v4_2_20.h and vbox_CAPI_v4_3_4.h.
>
> As stated by Matthias Bolte, the actual underlying problem here is that
> libvirt assumes that VirtualBox API can only change between release versions
> (4.2 -> 4.3), but we have a case here where it changed (or got fixed) between
> minor versions (4.2.18 -> 4.2.20).
>
> This patch makes the VBOX_API_VERSION represent the full API
> version number (i.e 4002 => 4002000) so there are specific version
> numbers for Vbox 4.2.20 (4002020) and 4.3.4 (4003004)
>
> As the patch is too big for the mailing list, it is publicly available
> at http://git-lab.diateam.net/cots/libvirt.git/ with the branch name
> 'vbox-4.2.20-4.3.4-support-v2'
>
> Regards,
> Manuel VIVES

ACK to the content of the patch. But the commit message itself lacks
the explanation for this patch that is given in your mail. This should
be included. Also as libvirt is currently in freeze for the upcoming
release I suggest to delay pushing this patch until after the release,
as the patch doesn't fix any current bug in the VirtualBox driver, but
is a prerequisite for future patches.

-- 
Matthias Bolte
http://photron.blogspot.com

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

Re: [libvirt] [PATCHv2 1/6] maint: don't leave garbage on early API exit

[Ján Tomko]

On 01/14/2014 10:43 PM, Eric Blake wrote:
> Several APIs clear out a user input buffer before attempting to
> populate it; but in a few cases we missed this memset if we
> detect a reason for an early exit.  Note that these APIs
> check for non-NULL arguments, and exit early with an error
> message when NULL is passed in; which means that we must be
> careful to avoid a NULL deref in order to get to that error
> message.  Also, we were inconsistent on the use of
> sizeof(virType) vs. sizeof(expression); the latter is more
> robust if we ever change the type of the expression (although
> such action is unlikely since these types are part of our
> public API).
> 
> * src/libvirt.c (virDomainGetInfo, virDomainGetBlockInfo)
> (virStoragePoolGetInfo, virStorageVolGetInfo)
> (virDomainGetJobInfo, virDomainGetBlockJobInfo): Move memset
> before any returns.
> 
> Signed-off-by: Eric Blake 
> ---
> 
> v2 avoid null deref, prefer sizeof(expr)
> 
>  src/libvirt.c | 29 +
>  1 file changed, 17 insertions(+), 12 deletions(-)
> 

> @@ -8449,12 +8450,12 @@ virDomainGetBlockInfo(virDomainPtr domain, const char 
> *disk,
> 
>  virResetLastError();
> 

if (info)
> +memset(info, 0, sizeof(*info));
> +


>  virCheckDomainReturn(domain, -1);
>  virCheckNonNullArgGoto(disk, error);
>  virCheckNonNullArgGoto(info, error);
> 
> -memset(info, 0, sizeof(virDomainBlockInfo));
> -
>  conn = domain->conn;
> 
>  if (conn->driver->domainGetBlockInfo) {

Jan



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

Re: [libvirt] [PATCH] docs: mention maintenance branches

[Michal Privoznik]

On 14.01.2014 17:53, Eric Blake wrote:
> Mitre tried to assign us two separate CVEs for the fix for
> https://bugzilla.redhat.com/show_bug.cgi?id=1047577, on the
> grounds that the fixes were separated by more than an hour
> and thus triggered different hourly snapshots.  But we
> explicitly do NOT want to treat transient security bugs as
> CVEs if they can only be triggered by patches in libvirt.git
> but where the problem is cleaned up before a formal release.
> 
> Meanwhile, I noticed that while our wiki mentioned maintenance
> branches and releases, our formal documentation did not.
> 
> * docs/downloads.html.in: Contrast hourly snapshots with
> maintenance branches.
> 
> Signed-off-by: Eric Blake 
> ---
> 
> Doc only, so suitable for 1.2.1 if it gets reviewed in time.
> 
>  docs/downloads.html.in | 25 -
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/downloads.html.in b/docs/downloads.html.in
> index 83b8751..ef03567 100644
> --- a/docs/downloads.html.in
> +++ b/docs/downloads.html.in
> @@ -22,7 +22,9 @@
>  
>Once an hour, an automated snapshot is made from the git server
>source tree. These snapshots should be usable, but we make no 
> guarantees
> -  about their stability:
> +  about their stability; furthermore, they should NOT be
> +  considered formal releases, and they may have transient security
> +  problems that will not be assigned a CVE:
>  
> 
>  
> @@ -30,6 +32,27 @@
> href="http://libvirt.org/sources/libvirt-git-snapshot.tar.gz";>libvirt.org 
> HTTP server
>  
> 
> +Maintenance releases
> +
> +  In the git repository are several stable maintenance branches,
> +  matching the
> +  pattern vmajor.minor.micro-maint;
> +  these branches are forked off the corresponding
> +  vmajor.minor.micro formal
> +  release, and may have further releases of the
> +  form vmajor.minor.micro.rel.
> +  These maintenance branches should only contain bug fixes, and no
> +  new features, backported from the master branch, and are
> +  supported.  These maintenance branches are considered during
> +  CVE analysis.
> +
> +
> +
> +  For more details about contents of maintenance releases, see
> +  http://wiki.libvirt.org/page/Maintenance_Releases";>the
> +  wiki page.
> +
> +
>  GIT source repository
> 
>  
> 

ACK & safe for the upcoming release.

Michal

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

Re: [libvirt] [PATCHv2 0/6] remaining cleanups to libvirt.c

[John Ferlan]

On 01/14/2014 04:43 PM, Eric Blake wrote:
> v2 of my patch series started here, after applying all patches
> already reviewed in that thread:
> https://www.redhat.com/archives/libvir-list/2013-December/msg01284.html
> 
> Patches 1-3 can be considered bug fixes (particularly patch 3),
> so I'd like them in 1.2.1 if they get a favorable review.  Patches
> 4-6 are less urgent, but might as well finish the work all within
> one release.
> 
> Patch 1 comes from 5/24 in v1
> Patch 2 is new
> Patch 3 and 4 come from 23/24 in v1
> Patch 5 is new
> Patch 6 comes from 24/24 in v1
> 
> Eric Blake (6):
>   maint: don't leave garbage on early API exit
>   maint: avoid nested use of virConnect{Ref,Close}
>   maint: don't lose error on canceled migration
>   maint: clean up error reporting in migration
>   maint: simplify driver registration at startup
>   maint: replace remaining virLib*Error with better names
> 
>  cfg.mk   |  10 --
>  src/libvirt.c| 397 
> ---
>  src/lxc/lxc_process.c|  11 +-
>  src/qemu/qemu_driver.c   |  12 +-
>  src/qemu/qemu_migration.c|  14 +-
>  src/qemu/qemu_process.c  |  10 +-
>  src/storage/storage_driver.c |   5 +-
>  src/uml/uml_driver.c |   3 +-
>  8 files changed, 209 insertions(+), 253 deletions(-)
> 


Patch 3 seems to be the one where there would be a couple of migration
fixes that could use a backport, especially the change in
virDomainMigrateVersion3Full() where the code now actually honors the
condition where the uri was not set rather than just playing dumb. I
also keep looking the changed "single" (!ret) to a "multi-state" (!ret
&& !xxx) condition and keep asking myself could we run into a situation
where (!ret and xxx) is true where previously we'd fail on the !ret, but
now wouldn't because xxx was true (where xxx is retcode or cancelled).

Another "step" to consider for patch 5 would be to make common the
repetitive code that follows the virDriverCheckTabMaxReturn(), e.g.

VIR_DEBUG('string')
table[count] = driver
return count++

I don't see a reason other than proximity to release date to preclude
inclusion in 1.2.1

ACK series in general though.

John

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

Re: [libvirt] [PATCH] Fix docs for PMWakeup/PMSuspend callback types

[Eric Blake]

On 01/15/2014 12:28 AM, Claudio Bley wrote:
> s/is waken up/is woken up/
> 
> A registered PMSuspendCallback is called when the domain is suspended, not
> when it is woken up.
> 
> Signed-off-by: Claudio Bley 
> ---
> It's just a trivial fix, but I'd like some educated comment on the
> grammar fix. IMO it is correct, or is there a better way to express
> this?

ACK; as a native speaker, your patch sounds right.

> 
>  include/libvirt/libvirt.h.in |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 018a5ce..5bdb2bc 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -4849,7 +4849,7 @@ typedef void 
> (*virConnectDomainEventTrayChangeCallback)(virConnectPtr conn,
>   *  always passes 0
>   * @opaque: application specified data
>   *
> - * This callback occurs when the guest is waken up.
> + * This callback occurs when the guest is woken up.

If you want an alternative wording:

This callback occurs when the guest is awakened.

But I don't care which of the two wordings you use.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH] add nocow feature option to vol-create

[Michal Privoznik]

On 24.12.2013 09:56, Chunyan Liu wrote:
> Btrfs has terrible performance when hosting VM images, even more when the 
> guest
> in those VM are also using btrfs as file system. One way to mitigate this bad
> performance is to turn off COW attributes on VM files (since having copy on
> write for this kind of data is not useful).
> 
> According to 'chattr' manpage, NOCOW could be set to new or empty file only on
> btrfs, so this patch tries to add nocow feature option in volume xml and 
> handle
> it in vol-create, so that users could have a chance to set NOCOW to a new
> volume if that happens on a btrfs like file system.
> 
> Signed-off-by: Chunyan Liu 
> 
> ---
> This is a revised version to:
>   http://www.redhat.com/archives/libvir-list/2013-December/msg00303.html
> 
> Changes:
>   * fix Daniel's comments
> 
> ---
>  docs/formatstorage.html.in   |   12 +---
>  docs/schemas/storagefilefeatures.rng |3 ++
>  src/conf/storage_conf.c  |9 --
>  src/storage/storage_backend.c|4 +-
>  src/storage/storage_backend_fs.c |   48 
> ++
>  src/util/virstoragefile.c|1 +
>  src/util/virstoragefile.h|1 +
>  7 files changed, 69 insertions(+), 9 deletions(-)

Can you add a testcase that (at least) checks the RNG scheme? The more
your test tests the better.

> 
> diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
> index a089a31..3de1a2b 100644
> --- a/docs/formatstorage.html.in
> +++ b/docs/formatstorage.html.in
> @@ -385,6 +385,7 @@
>1.1
>
>  
> +
>
>  
>  
> @@ -423,11 +424,14 @@
>  Since 1.1.0
>
>features
> -  Format-specific features. Only used for qcow2 now.
> -Valid sub-elements are:
> -
> +  Format-specific features. Valid sub-elements are:
> +  
> - allow delayed reference
> -  counter updates. Since 1.1.0
> +  counter updates. Only used for qcow2 now.
> +  Since 1.1.0
> +   - turn off copy-on-write. Only 
> valid
> +  to volume on btrfs, can improve performance.
> +  Since 1.2.2
>  
>
>  
> diff --git a/docs/schemas/storagefilefeatures.rng 
> b/docs/schemas/storagefilefeatures.rng
> index 424b4e2..0cf3513 100644
> --- a/docs/schemas/storagefilefeatures.rng
> +++ b/docs/schemas/storagefilefeatures.rng
> @@ -17,6 +17,9 @@
>
>  
>
> +  
> +
> +  
>  
>
>  
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index 22e38c1..b6409a6 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -1398,9 +1398,6 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
>  if ((n = virXPathNodeSet("./target/features/*", ctxt, &nodes)) < 0)
>  goto error;
>  
> -if (!ret->target.compat && VIR_STRDUP(ret->target.compat, "1.1") < 0)
> -goto error;
> -
>  if (!(ret->target.features = 
> virBitmapNew(VIR_STORAGE_FILE_FEATURE_LAST)))
>  goto error;
>  
> @@ -1412,6 +1409,12 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
> (const char*)nodes[i]->name);
>  goto error;
>  }
> +
> +if (f == VIR_STORAGE_FILE_FEATURE_LAZY_REFCOUNTS) {
> +if (!ret->target.compat && VIR_STRDUP(ret->target.compat, 
> "1.1") < 0)
> +goto error;
> +}
> +
>  ignore_value(virBitmapSetBit(ret->target.features, f));
>  }
>  VIR_FREE(nodes);
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index b08d646..b4ab866 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -423,7 +423,7 @@ virStorageBackendCreateRaw(virConnectPtr conn 
> ATTRIBUTE_UNUSED,
>  operation_flags |= VIR_FILE_OPEN_FORK;
>  
>  if ((fd = virFileOpenAs(vol->target.path,
> -O_RDWR | O_CREAT | O_EXCL,
> +O_RDWR | O_CREAT,

I don't think this is safe. I see why are you doing this [2], but what
if user hadn't specified  nocow feature? Then we might overwrite an
existing file. And we want to do that.

>  vol->target.perms.mode,
>  vol->target.perms.uid,
>  vol->target.perms.gid,
> @@ -729,7 +729,7 @@ virStorageBackendCreateQemuImgOpts(char **opts,
>  break;
>  
>  /* coverity[dead_error_begin] */
> -case VIR_STORAGE_FILE_FEATURE_LAST:
> +default:

No, please no. The whole intent of having the enum enumerated explicitly
is: whenever a new item is added to a enum compiler will find a

Re: [libvirt] [PATCHv2] tests: be more explicit on qcow2 versions in virstoragetest

[Ján Tomko]

On 01/15/2014 01:19 AM, Eric Blake wrote:
> While working on v1.0.5-maint (the branch in use on Fedora 19)
> with the host at Fedora 20, I got a failure in virstoragetest.
...
> 
> * tests/virstoragetest.c (testPrepImages): Simplify creation of
> raw file; check if qemu supports compat and if so use it.
> 
> Signed-off-by: Eric Blake 
> ---
> 
> v2: rebase to latest; still asking for inclusion in 1.2.1
> 
>  tests/virstoragetest.c | 32 +++-
>  1 file changed, 23 insertions(+), 9 deletions(-)
> 

ACK

Jan




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

Re: [libvirt] [PATCH] qemu: remove memset params array to zero in qemuDomainGetPercpuStats

[Daniel P. Berrange]

On Wed, Jan 15, 2014 at 04:47:21PM +0800, Gao feng wrote:
> the array params is allocated by VIR_ALLOC_N in cmdCPUStats.
> it had been set to zero. No need to reset it to zero again,
> and this reset here is incorrect too, nparams * ncpus is the
> array length not the size of params array.

'cmdCPUStats' is virsh client code. The QEMU driver is
running server side in libvirtd. So whatever memory was
allocated in cmdCPUStats isn't the same as the memory
used in qemu_driver.c

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH 0/2] v4: spice: expose the disable file transfer option

[Francesco Romani]

- Original Message -
> From: "Francesco Romani" 
> To: libvir-list@redhat.com
> Cc: "Francesco Romani" 
> Sent: Wednesday, January 15, 2014 11:04:26 AM
> Subject: [PATCH 0/2] v4: spice: expose the disable file transfer option
> 
> Changes:
> Addressed Christophe's comment and changed the error value.
> Addressed Michal's comment and changed the capability is detected
> (thanks for the hint on query-command-line-output, I was looking
> in the wrong place).

Sorry, this ended up badly squashed. I'll fix and resend later.

Best regards,

-- 
Francesco Romani

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

[libvirt] [PATCH 1/2] spice: detect if qemu can disable file transfer

[Francesco Romani]

spice-server offers an API to disable file transfer messages
on the agent channel between the client and the guest.
This is supported in qemu through the disable-agent-file-xfer option.

This detects if QEMU supports this option, and add
a capability if does.
---
 src/qemu/qemu_capabilities.c  | 5 -
 src/qemu/qemu_capabilities.h  | 1 +
 tests/qemucapabilitiesdata/caps_1.6.0-1.caps  | 1 +
 tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 +
 4 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 0538115..c3f2e65 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -247,6 +247,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "boot-strict", /* 160 */
   "pvpanic",
   "enable-fips",
+  "spice-file-xfer-disable"
 );
 
 struct _virQEMUCaps {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index efb3f43..23dccce 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -201,6 +201,7 @@ enum virQEMUCapsFlags {
 QEMU_CAPS_BOOT_STRICT= 160, /* -boot strict */
 QEMU_CAPS_DEVICE_PANIC   = 161, /* -device pvpanic */
 QEMU_CAPS_ENABLE_FIPS= 162, /* -enable-fips */
+QEMU_CAPS_SPICE_FILE_XFER_DISABLE = 163, /* -spice disable-agent-file-xfer 
*/
 
 QEMU_CAPS_LAST,   /* this must always be the last item */
 };
diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps 
b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
index 2d50cf9..61542a8 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
@@ -139,4 +139,5 @@
 
 
 
+
   
diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps 
b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
index ba64177..8ce17aa 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
@@ -137,4 +137,5 @@
 
 
 
+
   
-- 
1.8.4.2

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

[libvirt] [PATCH 2/2] spice: expose the QEMU disable file transfer option

[Francesco Romani]

spice-server offers an API to disable file transfer messages
on the agent channel between the client and the guest.
This is supported in qemu through the disable-agent-file-xfer option.

This patch exposes this option to libvirt.
Adds a new element 'filetransfer', with one property,
'filetransfer', which accepts a boolean setting.
Default is enabled.

Depends on the capability exported in the first patch of the series.
---
 docs/formatdomain.html.in  |  8 +
 docs/schemas/domaincommon.rng  | 11 ++
 src/conf/domain_conf.c | 31 -
 src/conf/domain_conf.h | 10 ++
 src/libvirt_private.syms   |  2 ++
 src/qemu/qemu_capabilities.c   |  5 ++-
 src/qemu/qemu_command.c|  9 +
 ...emuxml2argv-graphics-spice-agent-file-xfer.args |  9 +
 ...qemuxml2argv-graphics-spice-agent-file-xfer.xml | 40 ++
 .../qemuxml2argv-graphics-spice.args   |  5 +--
 .../qemuxml2argv-graphics-spice.xml|  1 +
 tests/qemuxml2argvtest.c   |  9 -
 12 files changed, 133 insertions(+), 7 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 68860ef..c11a7d3 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4042,6 +4042,7 @@ qemu-kvm -net nic,model=? /dev/null
 
 
 
+
   
 
   Spice supports variable compression settings for audio,
@@ -4081,6 +4082,13 @@ qemu-kvm -net nic,model=? /dev/null
   since 0.9.11. If no mode is
   specified, the qemu default will be used (client mode).
 
+
+  File transfer functionality (via Spice agent) is set using the
+  filetransfer element.
+  It is enabled by default, and can be disabled by setting the
+  enable property to no ,
+  since since 1.2.2.
+
   
   "rdp"
   
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index a69f6b6..9ddb772 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2474,6 +2474,17 @@
 
   
 
+
+  
+
+  
+yes
+no
+  
+
+
+  
+
   
 
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c1dd598..7d6c9ba 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -604,6 +604,12 @@ VIR_ENUM_IMPL(virDomainGraphicsSpiceClipboardCopypaste,
   "yes",
   "no");
 
+VIR_ENUM_IMPL(virDomainGraphicsSpiceAgentFileTransfer,
+  VIR_DOMAIN_GRAPHICS_SPICE_AGENT_FILE_TRANSFER_LAST,
+  "default",
+  "yes",
+  "no");
+
 VIR_ENUM_IMPL(virDomainHostdevMode, VIR_DOMAIN_HOSTDEV_MODE_LAST,
   "subsystem",
   "capabilities")
@@ -8519,6 +8525,26 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
 VIR_FREE(copypaste);
 
 def->data.spice.copypaste = copypasteVal;
+} else if (xmlStrEqual(cur->name, BAD_CAST "filetransfer")) {
+char *enable = virXMLPropString(cur, "enable");
+int enableVal;
+
+if (!enable) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("spice filetransfer missing enable"));
+goto error;
+}
+
+if ((enableVal =
+ 
virDomainGraphicsSpiceAgentFileTransferTypeFromString(enable)) <= 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown enable value '%s'"), enable);
+VIR_FREE(enable);
+goto error;
+}
+VIR_FREE(enable);
+
+def->data.spice.filetransfer = enableVal;
 } else if (xmlStrEqual(cur->name, BAD_CAST "mouse")) {
 char *mode = virXMLPropString(cur, "mode");
 int modeVal;
@@ -16423,7 +16449,7 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
 if (!children && (def->data.spice.image || def->data.spice.jpeg ||
   def->data.spice.zlib || def->data.spice.playback ||
   

[libvirt] [PATCH 0/2] v4: spice: expose the disable file transfer option

[Francesco Romani]

Changes:
Addressed Christophe's comment and changed the error value.
Addressed Michal's comment and changed the capability is detected
(thanks for the hint on query-command-line-output, I was looking
in the wrong place).

Francesco Romani (2):
  spice: detect if qemu can disable file transfer
  spice: expose the QEMU disable file transfer option

 docs/formatdomain.html.in  |  8 +
 docs/schemas/domaincommon.rng  | 11 ++
 src/conf/domain_conf.c | 31 -
 src/conf/domain_conf.h | 10 ++
 src/libvirt_private.syms   |  2 ++
 src/qemu/qemu_capabilities.c   |  2 ++
 src/qemu/qemu_capabilities.h   |  1 +
 src/qemu/qemu_command.c|  9 +
 tests/qemucapabilitiesdata/caps_1.6.0-1.caps   |  1 +
 tests/qemucapabilitiesdata/caps_1.6.50-1.caps  |  1 +
 ...emuxml2argv-graphics-spice-agent-file-xfer.args |  9 +
 ...qemuxml2argv-graphics-spice-agent-file-xfer.xml | 40 ++
 .../qemuxml2argv-graphics-spice.args   |  5 +--
 .../qemuxml2argv-graphics-spice.xml|  1 +
 tests/qemuxml2argvtest.c   |  9 -
 15 files changed, 136 insertions(+), 4 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-agent-file-xfer.xml

-- 
1.8.4.2

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

Re: [libvirt] [PATCH 6/6] Widening API change - accept empty path for virDomainBlockStats

[Gao feng]

On 01/15/2014 07:23 AM, Thorsten Behrens wrote:
> And provide domain summary stats in that case, for lxc backend.
> Use case is a container domain using passthrough bind mounts of the
> host filesystem, which is a common case for lxc.
> ---
>  src/libvirt.c  |  1 -
>  src/lxc/lxc_driver.c   | 10 ++
>  src/qemu/qemu_driver.c |  2 ++
>  src/remote/remote_driver.c |  2 ++
>  src/test/test_driver.c |  2 ++
>  src/xen/xen_driver.c   |  2 ++
>  6 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 87a4d46..14ffca0 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -7804,7 +7804,6 @@ virDomainBlockStats(virDomainPtr dom, const char *disk,
>  virResetLastError();
>  
>  virCheckDomainReturn(dom, -1);
> -virCheckNonNullArgGoto(disk, error);
>  virCheckNonNullArgGoto(stats, error);
>  if (size > sizeof(stats2)) {
>  virReportInvalidArg(size,
> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index 1d2a457..fba9c12 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -2044,6 +2044,16 @@ lxcDomainBlockStats(virDomainPtr dom,
>  goto cleanup;
>  }
>  
> +if (!path || !*path) {
> +/* empty/NULL path - return entire domain blkstats instead */
> +ret = virCgroupGetBlkioIoServiced(priv->cgroup,
> +  &stats->rd_bytes,
> +  &stats->wr_bytes,
> +  &stats->rd_req,
> +  &stats->wr_req);
> +goto cleanup;
> +}
> +

I'm ok with this one, Let's see if others will object.
but you should check if we can use the NEW API as
I mehtioned in prev thread, and change the manpage of virsh domblkstat.

>  if ((idx = virDomainDiskIndexByName(vm->def, path, false)) < 0) {
>  virReportError(VIR_ERR_INVALID_ARG,
> _("invalid path: %s"), path);
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 2d92873..4dcf12b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -9021,6 +9021,8 @@ qemuDomainBlockStats(virDomainPtr dom,
>  virDomainDiskDefPtr disk = NULL;
>  qemuDomainObjPrivatePtr priv;
>  
> +virCheckNonNullArgReturn(path, -1);
> +
>  if (!(vm = qemuDomObjFromDomain(dom)))
>  goto cleanup;
>  
> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> index da9c1c9..160bdd4 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -1713,6 +1713,8 @@ remoteDomainBlockStatsFlags(virDomainPtr domain,
>  remote_domain_block_stats_flags_ret ret;
>  struct private_data *priv = domain->conn->privateData;
>  
> +virCheckNonNullArgReturn(path, -1);
> +
>  remoteDriverLock(priv);
>  
>  make_nonnull_domain(&args.dom, domain);
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index b724f82..7c637bb 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -3362,6 +3362,8 @@ static int testDomainBlockStats(virDomainPtr domain,
>  unsigned long long statbase;
>  int ret = -1;
>  
> +virCheckNonNullArgReturn(path, -1);
> +
>  testDriverLock(privconn);
>  privdom = virDomainObjListFindByName(privconn->domains,
>   domain->name);
> diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
> index c45d10f..2b9ac21 100644
> --- a/src/xen/xen_driver.c
> +++ b/src/xen/xen_driver.c
> @@ -2217,6 +2217,8 @@ xenUnifiedDomainBlockStats(virDomainPtr dom, const char 
> *path,
>  virDomainDefPtr def = NULL;
>  int ret = -1;
>  
> +virCheckNonNullArgReturn(path, -1);
> +
>  if (!(def = xenGetDomainDefForDom(dom)))
>  goto cleanup;
>  
> 

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

Re: [libvirt] [PATCH 5/6] Implemet lxcDomainBlockStats for lxc driver

[Gao feng]

On 01/15/2014 07:23 AM, Thorsten Behrens wrote:
> ---
>  src/lxc/lxc_driver.c | 51 +++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index 1e9c77a..1d2a457 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -2021,6 +2021,56 @@ lxcDomainGetSchedulerParameters(virDomainPtr domain,
>  
>  
>  static int
> +lxcDomainBlockStats(virDomainPtr dom,
> +const char *path,
> +struct _virDomainBlockStats *stats)
> +{
> +int ret = -1, idx;
> +virDomainObjPtr vm;
> +virDomainDiskDefPtr disk = NULL;
> +virLXCDomainObjPrivatePtr priv;
> +
> +if (!(vm = lxcDomObjFromDomain(dom)))
> +return ret;
> +
> +priv = vm->privateData;
> +
> +if (virDomainBlockStatsEnsureACL(dom->conn, vm->def) < 0)
> +goto cleanup;
> +
> +if (!virDomainObjIsActive(vm)) {
> +virReportError(VIR_ERR_OPERATION_INVALID,
> +   "%s", _("domain is not running"));
> +goto cleanup;
> +}
> +
> +if ((idx = virDomainDiskIndexByName(vm->def, path, false)) < 0) {
> +virReportError(VIR_ERR_INVALID_ARG,
> +   _("invalid path: %s"), path);
> +goto cleanup;
> +}
> +disk = vm->def->disks[idx];
> +
> +if (!disk->info.alias) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("missing disk device alias name for %s"), 
> disk->dst);
> +goto cleanup;
> +}
> +
> +ret = virCgroupGetBlkioIoDeviceServiced(priv->cgroup,
> +disk->info.alias,
> +&stats->rd_bytes,
> +&stats->wr_bytes,
> +&stats->rd_req,
> +&stats->wr_req);
> +cleanup:
> +if (vm)
> +virObjectUnlock(vm);
> +return ret;
> +}
> +
> +
> +static int
>  lxcDomainSetBlkioParameters(virDomainPtr dom,
>  virTypedParameterPtr params,
>  int nparams,
> @@ -4962,6 +5012,7 @@ static virDriver lxcDriver = {
>  .domainGetSchedulerParametersFlags = 
> lxcDomainGetSchedulerParametersFlags, /* 0.9.2 */
>  .domainSetSchedulerParameters = lxcDomainSetSchedulerParameters, /* 
> 0.5.0 */
>  .domainSetSchedulerParametersFlags = 
> lxcDomainSetSchedulerParametersFlags, /* 0.9.2 */
> +.domainBlockStats = lxcDomainBlockStats, /* 0.4.1 */

Just one question.
Can't we implement the new API domainBlockStatsFlags?

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

[libvirt] [PATCH] qemu: remove memset params array to zero in qemuDomainGetPercpuStats

[Gao feng]

the array params is allocated by VIR_ALLOC_N in cmdCPUStats.
it had been set to zero. No need to reset it to zero again,
and this reset here is incorrect too, nparams * ncpus is the
array length not the size of params array.

Signed-off-by: Gao feng 
---
 src/qemu/qemu_driver.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9f71160..7a329f0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15820,7 +15820,6 @@ qemuDomainGetPercpuStats(virDomainObjPtr vm,
 if (virCgroupGetCpuacctPercpuUsage(priv->cgroup, &buf))
 goto cleanup;
 pos = buf;
-memset(params, 0, nparams * ncpus);
 
 /* return percpu cputime in index 0 */
 param_idx = 0;
-- 
1.8.4.2

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

Re: [libvirt] [PATCH 4/6] Implement domainGetCPUStats for lxc driver.

[Gao feng]

On 01/15/2014 07:23 AM, Thorsten Behrens wrote:
> ---
>  src/lxc/lxc_driver.c | 132 
> +++
>  1 file changed, 132 insertions(+)
> 
> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index 9f586af..1e9c77a 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -75,6 +75,8 @@
>  
>  
>  #define LXC_NB_MEM_PARAM  3
> +#define LXC_NB_PER_CPU_STAT_PARAM 1
> +
>  
>  static int lxcStateInitialize(bool privileged,
>virStateInhibitCallback callback,
> @@ -4775,6 +4777,135 @@ cleanup:
>  }
>  
>  
> +static int
> +lxcDomainGetPercpuStats(virDomainObjPtr vm,
> +virTypedParameterPtr params,
> +unsigned int nparams,
> +int start_cpu,
> +unsigned int ncpus)
> +{
> +int rv = -1;
> +size_t i;
> +int id, max_id;
> +char *pos;
> +char *buf = NULL;
> +unsigned int n = 0;
> +virLXCDomainObjPrivatePtr priv = vm->privateData;
> +virTypedParameterPtr ent;
> +int param_idx;
> +unsigned long long cpu_time;
> +
> +/* TODO: share api contract code with other drivers here */
> +
> +/* return the number of supported params */
> +if (nparams == 0 && ncpus != 0)
> +return LXC_NB_PER_CPU_STAT_PARAM;
> +
> +/* To parse account file, we need to know how many cpus are present.  */
> +max_id = nodeGetCPUCount();
> +if (max_id < 0)
> +return rv;
> +
> +if (ncpus == 0) { /* returns max cpu ID */
> +rv = max_id;
> +goto cleanup;
> +}
> +
> +if (start_cpu > max_id) {
> +virReportError(VIR_ERR_INVALID_ARG,
> +   _("start_cpu %d larger than maximum of %d"),
> +   start_cpu, max_id);
> +goto cleanup;
> +}
> +
> +/* we get percpu cputime accounting info. */
> +if (virCgroupGetCpuacctPercpuUsage(priv->cgroup, &buf))
> +goto cleanup;
> +pos = buf;
> +memset(params, 0, nparams * ncpus);

this is unnecessary. we alreay filled params to zero when we allocate it
by VIR_ALLOC_N.

> +
> +/* return percpu cputime in index 0 */
> +param_idx = 0;
> +
> +/* number of cpus to compute */
> +if (start_cpu >= max_id - ncpus)
> +id = max_id - 1;
> +else
> +id = start_cpu + ncpus - 1;
> +
> +for (i = 0; i <= id; i++) {
> +if (virStrToLong_ull(pos, &pos, 10, &cpu_time) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("cpuacct parse error"));
> +goto cleanup;
> +} else {
> +n++;

n is useless.

> +}
> +if (i < start_cpu)
> +continue;
> +ent = ¶ms[(i - start_cpu) * nparams + param_idx];
> +if (virTypedParameterAssign(ent, VIR_DOMAIN_CPU_STATS_CPUTIME,
> +VIR_TYPED_PARAM_ULLONG, cpu_time) < 0)
> +goto cleanup;
> +}
> +
> +rv = nparams;
> +
> +cleanup:
> +VIR_FREE(buf);
> +return rv;
> +}
> +
> +
> +static int
> +lxcDomainGetCPUStats(virDomainPtr dom,
> + virTypedParameterPtr params,
> + unsigned int nparams,
> + int start_cpu,
> + unsigned int ncpus,
> + unsigned int flags)
> +{
> +virDomainObjPtr vm = NULL;
> +int ret = -1;
> +bool isActive;
> +virLXCDomainObjPrivatePtr priv;
> +
> +virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
> +
> +if (!(vm = lxcDomObjFromDomain(dom)))
> +return ret;
> +
> +priv = vm->privateData;
> +
> +if (virDomainGetCPUStatsEnsureACL(dom->conn, vm->def) < 0)
> +goto cleanup;
> +
> +isActive = virDomainObjIsActive(vm);
> +if (!isActive) {
> +virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +   _("domain is not running"));
> +goto cleanup;
> +}
> +
> +if (!virCgroupHasController(priv->cgroup, 
> VIR_CGROUP_CONTROLLER_CPUACCT)) {
> +virReportError(VIR_ERR_OPERATION_INVALID,
> +   "%s", _("cgroup CPUACCT controller is not mounted"));
> +goto cleanup;
> +}
> +
> +if (start_cpu == -1)
> +ret = virCgroupGetDomainTotalCPUStats(priv->cgroup,
> +  params, nparams);
> +else
> +ret = lxcDomainGetPercpuStats(vm, params, nparams,
> +  start_cpu, ncpus);
> +cleanup:
> +if (vm)
> +virObjectUnlock(vm);
> +return ret;
> +}
> +
> +
>  /* Function Tables */
>  static virDriver lxcDriver = {
>  .no = VIR_DRV_LXC,
> @@ -4852,6 +4983,7 @@ static virDriver lxcDriver = {
>  .nodeSuspendForDuration = lxcNodeSuspendForDuration, /* 0.9.8 */
>  .domainSetMetadata = lxcDomainSetMetadata, /* 1.1.3 */
>  .domainGetMetadata = lxcDomainGetMetadata, /* 1.1.3 */
> +.domainGetCPUStats =