Re: [libvirt] [PATCHv6 8/8] blockjob: allow mirroring under SELinux

2012-05-07 Thread Eric Blake
On 04/23/2012 08:49 PM, Eric Blake wrote:
> This copies heavily from qemuDomainSnapshotCreateSingleDiskActive(),
> in order to set the SELinux label, obtain locking manager lease, and
> audit the fact that we hand a new file over to qemu.  Alas, releasing
> the lease and label at the end of the mirroring is a trickier
> prospect (we would have to know the backing chain of both source and
> destination, and be sure not to revoke rights to any part of the
> chain that is shared), so for now, virDomainBlockJobAbort still
> leaves things locked and labeled.
> 
> * src/qemu/qemu_driver.c (qemuDomainBlockCopy): Set up labeling.
> ---

Given today's fix for snapshot, and that this code heavily copied from
snapshot, I will be squashing this in:

diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index a2f88fd..da4ad7e 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -12031,6 +12031,7 @@ qemuDomainBlockCopy(virDomainPtr dom, const char
*path,
 char *mirrorFormat = NULL;
 char *origsrc = NULL;
 char *origdriver = NULL;
+virCgroupPtr cgroup = NULL;

 /* Preliminaries: find the disk we are editing, sanity checks */
 virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
@@ -12049,6 +12050,13 @@ qemuDomainBlockCopy(virDomainPtr dom, const
char *path,
 _("domain is not running"));
 goto cleanup;
 }
+if (qemuCgroupControllerActive(driver,
VIR_CGROUP_CONTROLLER_DEVICES) &&
+virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0)) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+_("Unable to find cgroup for %s"),
+vm->def->name);
+goto cleanup;
+}

 device = qemuDiskPathToAlias(vm, path, &idx);
 if (!device) {
@@ -12154,8 +12162,15 @@ qemuDomainBlockCopy(virDomainPtr dom, const
char *path,

 if (virDomainLockDiskAttach(driver->lockManager, vm, disk) < 0)
 goto endjob;
+if (cgroup && qemuSetupDiskCgroup(driver, vm, cgroup, disk) < 0) {
+if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
+VIR_WARN("Unable to release lock on %s", dest);
+goto cleanup;
+}
 if (virSecurityManagerSetImageLabel(driver->securityManager, vm->def,
 disk) < 0) {
+if (cgroup && qemuTeardownDiskCgroup(driver, vm, cgroup, disk) < 0)
+VIR_WARN("Failed to teardown cgroup for disk path %s", dest);
 if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
 VIR_WARN("Unable to release lock on %s", dest);
 goto endjob;
@@ -12167,6 +12182,8 @@ qemuDomainBlockCopy(virDomainPtr dom, const char
*path,
 virDomainAuditDisk(vm, NULL, dest, "mirror", ret >= 0);
 qemuDomainObjExitMonitorWithDriver(driver, vm);
 if (ret < 0) {
+if (cgroup && qemuTeardownDiskCgroup(driver, vm, cgroup, disk) < 0)
+VIR_WARN("Failed to teardown cgroup for disk path %s", dest);
 if (virSecurityManagerRestoreImageLabel(driver->securityManager,
 vm->def, disk) < 0)
 VIR_WARN("Unable to restore security label on %s", dest);
@@ -12202,6 +12219,8 @@ endjob:
 }

 cleanup:
+if (cgroup)
+virCgroupFree(&cgroup);
 VIR_FREE(device);
 if (vm)
 virDomainObjUnlock(vm);

-- 
Eric Blake   ebl...@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] snapshot: allow block devices past cgroup

2012-05-07 Thread Eric Blake
It turns out that when cgroups are enabled, the use of a block device
for a snapshot target was failing with EPERM due to libvirt failing
to add the block device to the cgroup whitelist.  See also
https://bugzilla.redhat.com/show_bug.cgi?id=810200

* src/qemu/qemu_driver.c
(qemuDomainSnapshotCreateSingleDiskActive)
(qemuDomainSnapshotUndoSingleDiskActive): Account for cgroup.
(qemuDomainSnapshotCreateDiskActive): Update caller.
---

I still need to properly test this, but based on how disk hotplug
works, I think this should solve the problem at hand.

My recent patch series for virDomainBlockRebase needs the same
treatment; I'll do that as a separate patch.

This still does not address the fact that block pull should be
revoking rights to a backing file that is no longer in use; that
will require more infrastructure for libvirt properly tracking an
entire backing chain.

 src/qemu/qemu_driver.c |   26 --
 1 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2bec617..92535b9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9895,6 +9895,7 @@ cleanup:
 static int
 qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver,
  virDomainObjPtr vm,
+ virCgroupPtr cgroup,
  virDomainSnapshotDiskDefPtr snap,
  virDomainDiskDefPtr disk,
  virDomainDiskDefPtr persistDisk,
@@ -9948,8 +9949,15 @@ qemuDomainSnapshotCreateSingleDiskActive(struct 
qemud_driver *driver,

 if (virDomainLockDiskAttach(driver->lockManager, vm, disk) < 0)
 goto cleanup;
+if (cgroup && qemuSetupDiskCgroup(driver, vm, cgroup, disk) < 0) {
+if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
+VIR_WARN("Unable to release lock on %s", source);
+goto cleanup;
+}
 if (virSecurityManagerSetImageLabel(driver->securityManager, vm->def,
 disk) < 0) {
+if (cgroup && qemuTeardownDiskCgroup(driver, vm, cgroup, disk) < 0)
+VIR_WARN("Failed to teardown cgroup for disk path %s", source);
 if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
 VIR_WARN("Unable to release lock on %s", source);
 goto cleanup;
@@ -10009,6 +10017,7 @@ cleanup:
 static void
 qemuDomainSnapshotUndoSingleDiskActive(struct qemud_driver *driver,
virDomainObjPtr vm,
+   virCgroupPtr cgroup,
virDomainDiskDefPtr origdisk,
virDomainDiskDefPtr disk,
virDomainDiskDefPtr persistDisk,
@@ -10033,6 +10042,8 @@ qemuDomainSnapshotUndoSingleDiskActive(struct 
qemud_driver *driver,
 if (virSecurityManagerRestoreImageLabel(driver->securityManager,
 vm->def, disk) < 0)
 VIR_WARN("Unable to restore security label on %s", disk->src);
+if (cgroup && qemuTeardownDiskCgroup(driver, vm, cgroup, disk) < 0)
+VIR_WARN("Failed to teardown cgroup for disk path %s", disk->src);
 if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
 VIR_WARN("Unable to release lock on %s", disk->src);
 if (need_unlink && stat(disk->src, &st) == 0 &&
@@ -10084,6 +10095,7 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
 int thaw = 0; /* 1 if freeze succeeded, -1 if freeze failed */
 bool atomic = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC) != 0;
 bool reuse = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0;
+virCgroupPtr cgroup = NULL;

 if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0)
 return -1;
@@ -10094,6 +10106,14 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
 goto endjob;
 }

+if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES) &&
+virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0)) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+_("Unable to find cgroup for %s"),
+vm->def->name);
+goto endjob;
+}
+
 /* If quiesce was requested, then issue a freeze command, and a
  * counterpart thaw command, no matter what.  The command will
  * fail if the guest is paused or the guest agent is not
@@ -10156,7 +10176,7 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
 }
 }

-ret = qemuDomainSnapshotCreateSingleDiskActive(driver, vm,
+ret = qemuDomainSnapshotCreateSingleDiskActive(driver, vm, cgroup,
&snap->def->disks[i],
vm->def->di

[libvirt] [PATCH] build: work around mingw isatty issues

2012-05-07 Thread Eric Blake
Gnulib finally relaxed the isatty license, needed as first mentioned here:
https://www.redhat.com/archives/libvir-list/2012-February/msg01022.html

* .gnulib: Update to latest, for isatty.
* bootstrap.conf (gnulib_modules): Add isatty.
* bootstrap: Resync from gnulib.
---

Not worth including until after 0.9.12 is out, but I thought I'd
post it today, since I was investigating another possible gnulib issue.

* .gnulib bb2f564...170e1b4 (33):
  > isatty: relax license to LGPLv2+
  > stat-size: comment fix
  > Tests for module 'sh-quote'.
  > sh-quote: Improve shell_quote_argv's signature.
  > stdint: document issues with int_fast8_t etc.
  > nanosleep: Fix typo in comment.
  > nanosleep: Avoid guessing wrong when cross-compiling to Linux.
  > link-follow: Avoid guessing wrong when cross-compiling to glibc/Linux.
  > tzset: Avoid guessing wrong when cross-compiling to glibc systems.
  > d-ino: Avoid guessing "no" when cross-compiling to glibc/Linux systems.
  > fseeko-tests, ftello-tests: Avoid "guessing no" when cross-compiling.
  > signbit: Avoid "guessing no" when cross-compiling to glibc systems.
  > strerror: Avoid "guessing no" when cross-compiling to glibc systems.
  > canonicalize[-lgpl]: Avoid "guessing no" when cross-compiling to glibc.
  > gettimeofday: Avoid bad guess when cross-compiling to glibc systems.
  > Tweak last commit.
  > unistd_h: make it easier to avoid sys_types_h
  > lstat: Avoid "guessing no" when cross-compiling to glibc systems.
  > *alloc-gnu, eealloc: Avoid "guessing no" when cross-compiling to glibc.
  > getgroups: Avoid "guessing no" when cross-compiling to glibc systems.
  > chown: Avoid "guessing no" when cross-compiling to glibc systems.
  > Simplify last commit.
  > Avoid "guessing no" guesses when cross-compiling to glibc systems.
  > Say "guessing yes" or "guessing no" when cross-compiling.
  > relocatable-prog: Enable ELF ORIGIN trick also on GNU/kFreeBSD.
  > gnulib-tool: Remove transitional code.
  > getcwd: Fix misindentation.
  > exclude: process exclude and include directives in order
  > exclude: handle wildcards with FNM_NOESCAPE and with trailing \
  > _Noreturn: future-proof non-GNU and non-MSVC compilers
  > exclude: handle wildcards with FNM_EXTMATCH
  > gnulib-tool: Fix list of authors.
  > bootstrap: support Automake-NG in $buildreq

 .gnulib|2 +-
 bootstrap  |   18 +-
 bootstrap.conf |1 +
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/.gnulib b/.gnulib
index bb2f564..170e1b4 16
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit bb2f5640d5379c5b4eec2d62341413bbab1aa308
+Subproject commit 170e1b42590631eac8279664ccc0b99ee5a73fb7
diff --git a/bootstrap b/bootstrap
index 6b45868..c496d29 100755
--- a/bootstrap
+++ b/bootstrap
@@ -1,6 +1,6 @@
 #! /bin/sh
 # Print a version string.
-scriptversion=2012-04-25.17; # UTC
+scriptversion=2012-04-26.13; # UTC

 # Bootstrap this package from checked-out sources.

@@ -433,6 +433,22 @@ check_versions() {
 GZIP) ;; # Do not use $GZIP:  it contains gzip options.
 *) eval "app=\${$appvar-$app}" ;;
 esac
+
+# Handle the still-experimental Automake-NG programs specially.
+# They remain named as the mainstream Automake programs ("automake",
+# and "aclocal") to avoid gratuitous incompatibilities with
+# pre-existing usages (by, say, autoreconf, or custom autogen.sh
+# scripts), but correctly identify themselves (as being part of
+# "GNU automake-ng") when asked their version.
+case $app in
+  automake-ng|aclocal-ng)
+app=`echo "$app" | sed 's/-ng$//'`
+($app --version | grep '(GNU automake-ng)') >/dev/null 2>&1 || {
+  echo "$me: Error: '$app' not found or not from Automake-NG" >&2
+  ret=1
+  continue
+} ;;
+esac
 if [ "$req_ver" = "-" ]; then
   # Merely require app to exist; not all prereq apps are well-behaved
   # so we have to rely on $? rather than get_version.
diff --git a/bootstrap.conf b/bootstrap.conf
index c6620e5..9b42cbf 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -60,6 +60,7 @@ ignore-value
 inet_pton
 intprops
 ioctl
+isatty
 largefile
 listen
 maintainer-makefile
-- 
1.7.7.6

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


Re: [libvirt] [PATCH libvirt 1/3] build: do not build shunloadtest if pthread missing

2012-05-07 Thread Eric Blake
ping

On 05/02/2012 12:45 PM, Eric Blake wrote:
> On 04/20/2012 07:13 AM, Marc-André Lureau wrote:
>> Fixes build on Windows systems

> we've already conditionalized -pthread; so does this (shorter) patch
> work to fix the problem instead?
> 
> diff --git i/tests/Makefile.am w/tests/Makefile.am
> index c4d550f..b30d79c 100644
> --- i/tests/Makefile.am
> +++ w/tests/Makefile.am
> @@ -529,7 +529,7 @@ libshunload_la_LDFLAGS = -module -avoid-version
> -rpath /evil/libtool/hack/to/for
> 
>  shunloadtest_SOURCES = \
>   shunloadtest.c
> -shunloadtest_LDADD = -lpthread
> +shunloadtest_LDADD = $(LIB_PTHREAD)
>  shunloadtest_DEPENDENCIES = libshunload.la
> 
>  if WITH_CIL
> 

-- 
Eric Blake   ebl...@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] virsh: avoid heap corruption leading to virsh abort

2012-05-07 Thread Jim Meyering
Eric Blake wrote:
> On 05/07/2012 01:29 PM, Jim Meyering wrote:
>> Investigating a build problem reported by Laine,
>> I was surprised to see "make check" fail on F17 due to a
>> glibc invalid free abort.  Ok to push to master?
>>
>>>From 61a559e0b2f4bded3059c5be7c958e2276f7fd16 Mon Sep 17 00:00:00 2001
>> From: Jim Meyering 
>> Date: Mon, 7 May 2012 21:22:09 +0200
>> Subject: [PATCH] virsh: avoid heap corruption leading to virsh abort
>>
>> * tools/virsh.c (vshParseSnapshotDiskspec): Fix off-by-3 memmove
>> that would corrupt heap when parsing escaped --diskspec comma.
>> Bug introduced via commit v0.9.4-260-g35d52b5.
>> ---
>>  tools/virsh.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/virsh.c b/tools/virsh.c
>> index 1207ac9..dd9292a 100644
>> --- a/tools/virsh.c
>> +++ b/tools/virsh.c
>> @@ -16107,7 +16107,7 @@ vshParseSnapshotDiskspec(vshControl *ctl, 
>> virBufferPtr buf, const char *str)
>>  while ((tmp = strchr(tmp, ','))) {
>>  if (tmp[1] == ',') {
>>  /* Recognize ,, as an escape for a literal comma */
>> -memmove(&tmp[1], &tmp[2], len - (tmp - spec) + 2);
>> +memmove(&tmp[1], &tmp[2], len - (tmp - spec) - 2 + 1);
>
> ACK.

Thanks for the quick review.  Pushed.

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


Re: [libvirt] [PATCH] virsh: avoid heap corruption leading to virsh abort

2012-05-07 Thread Eric Blake
On 05/07/2012 01:29 PM, Jim Meyering wrote:
> Investigating a build problem reported by Laine,
> I was surprised to see "make check" fail on F17 due to a
> glibc invalid free abort.  Ok to push to master?
> 
>>From 61a559e0b2f4bded3059c5be7c958e2276f7fd16 Mon Sep 17 00:00:00 2001
> From: Jim Meyering 
> Date: Mon, 7 May 2012 21:22:09 +0200
> Subject: [PATCH] virsh: avoid heap corruption leading to virsh abort
> 
> * tools/virsh.c (vshParseSnapshotDiskspec): Fix off-by-3 memmove
> that would corrupt heap when parsing escaped --diskspec comma.
> Bug introduced via commit v0.9.4-260-g35d52b5.
> ---
>  tools/virsh.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 1207ac9..dd9292a 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -16107,7 +16107,7 @@ vshParseSnapshotDiskspec(vshControl *ctl, 
> virBufferPtr buf, const char *str)
>  while ((tmp = strchr(tmp, ','))) {
>  if (tmp[1] == ',') {
>  /* Recognize ,, as an escape for a literal comma */
> -memmove(&tmp[1], &tmp[2], len - (tmp - spec) + 2);
> +memmove(&tmp[1], &tmp[2], len - (tmp - spec) - 2 + 1);

ACK.

/me crawls in a hole for introducing that bug

-- 
Eric Blake   ebl...@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] virsh: avoid heap corruption leading to virsh abort

2012-05-07 Thread Jim Meyering
Investigating a build problem reported by Laine,
I was surprised to see "make check" fail on F17 due to a
glibc invalid free abort.  Ok to push to master?

>From 61a559e0b2f4bded3059c5be7c958e2276f7fd16 Mon Sep 17 00:00:00 2001
From: Jim Meyering 
Date: Mon, 7 May 2012 21:22:09 +0200
Subject: [PATCH] virsh: avoid heap corruption leading to virsh abort

* tools/virsh.c (vshParseSnapshotDiskspec): Fix off-by-3 memmove
that would corrupt heap when parsing escaped --diskspec comma.
Bug introduced via commit v0.9.4-260-g35d52b5.
---
 tools/virsh.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 1207ac9..dd9292a 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -16107,7 +16107,7 @@ vshParseSnapshotDiskspec(vshControl *ctl, virBufferPtr 
buf, const char *str)
 while ((tmp = strchr(tmp, ','))) {
 if (tmp[1] == ',') {
 /* Recognize ,, as an escape for a literal comma */
-memmove(&tmp[1], &tmp[2], len - (tmp - spec) + 2);
+memmove(&tmp[1], &tmp[2], len - (tmp - spec) - 2 + 1);
 len--;
 tmp++;
 continue;
--
1.7.10.1.457.g8275905

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


Re: [libvirt] [PATCHv3 0/4] util: fix libvirtd startup failure due to netlink error

2012-05-07 Thread Laine Stump
On 05/07/2012 06:14 AM, Gerhard Stenzel wrote:
> On Fri, 2012-05-04 at 14:51 -0400, Laine Stump wrote:
>> As before, I'm unable to fully test this myself, so I won't push
>> unless/until I get verification it works.
> We tried various test scenarios, and libvirtd->lldpad and
> lldpad->libvirtd communication appeared to be functional and stable.
>

Based on this report of successful testing of the 802.1Qbg macvtap code
and communication with lldpad, my own successful testing that multiple
restarts are reliable, and the reviews, I've pushed this series.

Thanks to everyone for their reviews and testing!

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


Re: [libvirt] [PATCH] numad: Set memory policy according to the advisory nodeset from numad

2012-05-07 Thread Eric Blake
On 05/07/2012 02:34 AM, Osier Yang wrote:
> Though numad will manage the memory allocation of task dynamically,
> but it wants management application (libvirt) to pre-set the memory
> policy according to the advisory nodeset returned from querying numad,
> (just like pre-bind CPU nodeset for domain process), and thus the
> performance could benifit much more from it.

s/benifit/benefit/

> 
> This patch introduces new XML tag 'placement', value 'auto' indicates
> whether to set the memory policy with the advisory nodeset from numad,
> and its value defaults to 'static'. e.g.
> 
>   
> 
>   
> 
> Just like what current "numatune" does, the 'auto' numa memory policy
> setting uses libnuma's API too.
> 
> So, to full drive numad, one needs to specify placement='auto' for
> both "" and "". It's a bit
> inconvenient, but makes sense from semantics' point of view.
> 
> ---
> An alternative way is to not introduce the new XML tag, and pre-set
> the memory policy implicitly with "4",
> but IMHO it implies too much, and I'd not like go this way unless
> the new XML tag is not accepted.

I think we need a hybrid approach.

Existing users wrote XML that used just  but
expecting full numad support. Normally, numad is most useful if it
controls _both_ memory () and vcpu placement together, but it
is feasible to control either one in isolation.

Therefore, I think what we need to do is specify that on input, we have
four situations:

1. /domain/vcpu@placement is missing
   /domain/numatune/memory@placement is missing
   We default to mode='static' in both places, and avoid numad

2. /domain/vcpu@placement is present
   /domain/numatune/memory@placement is missing
   We copy vcpu placement over to numa placement (and if
placement='auto', that means we use numad for everything)

3. /domain/vcpu@placement is missing
   /domain/numatune/memory@placement is present
   We copy numa placement over to vcpu placement (and if
placement='auto', that means we use numad for everything)

4. /domain/vcpu@placement is present
   /domain/numatune/memory@placement is present
   We use exactly what the user specifies (and if only one of the two
features is placement='auto', then the other feature avoids numad)

Mode 3 and 4 would be the new modes possible by the XML added in this
patch.  Mode 1 is the default for XML in use by guests before numad
integration, and Mode 2 is the XML for guests attempting to use numad in
the 0.9.11 release; I'm okay changing the semantics of mode 2 to be
easier to use (because you can use mode 4 if you really wanted the
unusual semantics of numad vcpu placement but not memory placement).

Then on output, we always output mode in both  and , as
in mode 4 (yeah, that probably means touching a lot of tests, but that's
life when we add new XML).

I'm still a bit torn on whether this must go in before 0.9.12, since
we're past rc1.  But since it looks like this is more of an oversight
(our numad usage in 0.9.11 is not very useful, since it missed memory
placement), this can be argued to be a bug fix rather than a new
feature, even though it is adding XML, so I will probably be okay with a
v2 patch going in before the final 0.9.12 release.

On to the actual code of v1:

> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index e1fe0c4..01b3124 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -580,9 +580,14 @@
>  The optional memory element specifies how to allocate 
> memory
>  for the domain process on a NUMA host. It contains two attributes,

s/two attributes,/several optional attributes./

>  attribute mode is either 'interleave', 'strict',

s/attribute/Attribute/

> -or 'preferred',
> -attribute nodeset specifies the NUMA nodes, it leads 
> same
> -syntax with attribute cpuset of element 
> vcpu.
> +or 'preferred', attribute nodeset specifies the NUMA 
> nodes,

s/'preferred', attribute/'preferred'.  Attribute/

> +it leads same syntax with attribute cpuset of element

s/it leads same syntax with/using the same syntax as/

> +vcpu, the optional attribute placement can 
> be

s/, the/.  The/

> +used to indicate the memory placement mode for domain process, its 
> value
> +can be either "static" or "auto", defaults to "static". "auto" 
> indicates

Given my above comments, this would default to the same placement as
used in .

> +the domain process will only allocate memory from the advisory 
> nodeset
> +returned from querying numad, and the value of attribute 
> nodeset
> +will be ignored if it's specified.
>  Since 0.9.3

Mention that attribute placement is since 0.9.12.

>
>  
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 8419ccc..9b509f1 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -562,16 +562,35 @@
>  
>
>  

Re: [libvirt] [PATCH] openvz: simplify openvzDomainDefineCmd by using virCommandPtr

2012-05-07 Thread Guido Günther
On Mon, May 07, 2012 at 08:51:50AM -0600, Eric Blake wrote:
> On 05/05/2012 03:30 AM, Guido Günther wrote:
> > ---
> > While working on setting filesystem quotas I came accross this and I
> > figured I'll send this in advance because of the diffstat.
> > Cheers,
> >  -- Guido
> 
> > +} else {
> > +cmd = openvzDomainDefineCmd(vmdef);
> > +if (cmd == NULL)
> >  goto cleanup;
> 
> This misses out on the virReportOOMError(); you could just delete this
> if clause, and directly proceed to:
> 
> > -}
> >  
> > -if (virRun(prog, NULL) < 0) {
> > +if (virCommandRun(cmd, NULL) < 0)
> 
> this, which will properly report the OOM in case cmd is NULL.
> 
> ACK with that simplification.
Pushed with that change. Thanks,
 -- Guido

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


Re: [libvirt] [Qemu-devel] [RFC 0/5] block: File descriptor passing using -open-hook-fd

2012-05-07 Thread Corey Bryant



On 05/01/2012 06:15 PM, Eric Blake wrote:

On 05/01/2012 03:53 PM, Anthony Liguori wrote:


I think (correct me if I'm wrong) libvirt should be aware of any file
that qemu
asks it to open. So from a security point of view, libvirt can prevent
opening a
file if it isn't affiliated with the guest.


Right, libvirt can maintain a whitelist of files QEMU is allowed to open
(which is already has because it needs to label these files).


Indeed.


  The only
complexity is that it's not a straight strcmp().  The path needs to be
(carefully) broken into components with '.' and '..' handled
appropriately.  But this shouldn't be that difficult to do.


Libvirt would probably canonicalize path names, both when sticking them
in the whitelist, and in validating the requests from qemu - agreed that
it's not difficult.

More importantly, libvirt needs to start tracking the backing chain of
any qcow2 or qed file as part of the domain XML; and operations like
'block-stream' would update not only the chain, but also the whitelist.
  In the drive-reopen case, this means that libvirt would have to be
careful when to change labeling - provide access to the new files before
drive-reopen, then revoke access to files after drive-reopen completes.
  In other words, having the -open-hook-fd client pass a command to
libvirt at the time it is closing an fd would help libvirt know when
qemu has quit using a file, which might make it easier to revoke SELinux
labels at that time.



If we were to go with this approach, I think the following updates would 
be required for libvirt.  Could you let me know if I'm missing anything?


libvirt tasks:
- Introduce a data structure to store file whitelist per guest
- Add -open-hook-fd option to QEMU command line and pass Unix
  domain socket fd to QEMU
- Create open() handler that handles requests from QEMU to open
  files and passes back fd
- Potentially also handle close requests from QEMU?  Would allow
  libvirt to update XML and whitelist (as well as SELinux labels).
- Canonicalize path names when putting them in whitelist and
  when validating requests from QEMU
- XML updates to track backing chain of qcow2 and qed files
- Update whitelist and XML chain when QEMU monitor commands are
  used to open new files: block-stream, drive-reopen, drive_add,
  savevm, snapshot_blkdev, change

Updates would also be required for SELinux and AppArmor policy to allow 
libvirt open of NFS files, and allow QEMU read/write (no open allowed) 
of NFS Files.


--
Regards,
Corey

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


Re: [libvirt] [PATCH 2/2] domain_conf: add "default" to list of valid spice channels

2012-05-07 Thread Eric Blake
On 05/07/2012 09:53 AM, Eric Blake wrote:
> On 05/07/2012 06:33 AM, Alon Levy wrote:
>> qemu's behavior in this case is to change the spice server behavior to
>> require secure connection to any channel not otherwise specified as
>> being in plaintext mode. libvirt doesn't currently allow requesting this
>> (via plaintext-channel=).
>>
>> RHBZ: 819499
>>
>> Signed-off-by: Alon Levy 
>> ---
>>  src/conf/domain_conf.c |3 ++-
>>  src/conf/domain_conf.h |1 +
>>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> Same complaints as for 1/2 (docs, RNG schema, tests).  Also, is it ever
> valid to mark the default channel for plaintext (meaning all channels
> not marked secure are plaintext), or must it only be permitted for
> secure channels?

Here's one of the existing tests:
tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml

   
  
  
  

I'm wondering if rather than adding a new , it might make more sense to hoist the default channel
security mode up one element.  Something like:


   
   
   

While it is obvious that usbredir must be a valid channel name, it's not
as obvious about 'default' being a channel name (since it is really more
of the catchall for all other channels not explicitly listed).

-- 
Eric Blake   ebl...@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 1/3] usb: create functions to search usb device accurately

2012-05-07 Thread Guannan Ren

On 05/05/2012 03:16 AM, Eric Blake wrote:

On 05/04/2012 02:25 AM, Guannan Ren wrote:

usbFindDevice():get usb device according to
 idVendor, idProduct, bus, device
 it is the exact match of the four parameters

usbFindDeviceByBus():get usb device according to bus, device
   it returns only one usb device same as usbFindDevice

usbFindDeviceByVendor():get usb device according to idVendor,idProduct
  it probably returns multiple usb devices.

usbDeviceSearch(): a helper function to do the actual search
---
  src/libvirt_private.syms |2 +
  src/qemu/qemu_hostdev.c  |   14 +++-
  src/qemu/qemu_hotplug.c  |   14 +++-
  src/util/hostusb.c   |  202 -
  src/util/hostusb.h   |   22 --
  5 files changed, 181 insertions(+), 73 deletions(-)


ACK.  Looks better now.



   Thanks and pushed.

   Guannan Ren

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


Re: [libvirt] [PATCH] openvz: simplify openvzDomainDefineCmd by using virCommandPtr

2012-05-07 Thread Eric Blake
On 05/05/2012 03:30 AM, Guido Günther wrote:
> ---
> While working on setting filesystem quotas I came accross this and I
> figured I'll send this in advance because of the diffstat.
> Cheers,
>  -- Guido

> +} else {
> +cmd = openvzDomainDefineCmd(vmdef);
> +if (cmd == NULL)
>  goto cleanup;

This misses out on the virReportOOMError(); you could just delete this
if clause, and directly proceed to:

> -}
>  
> -if (virRun(prog, NULL) < 0) {
> +if (virCommandRun(cmd, NULL) < 0)

this, which will properly report the OOM in case cmd is NULL.

ACK with that simplification.

-- 
Eric Blake   ebl...@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 1/2] domain_conf: add "usbredir" to list of valid spice channels

2012-05-07 Thread Eric Blake
On 05/07/2012 06:33 AM, Alon Levy wrote:
> Add "usbredir" channel to list of recognized spice channels.
> 
> RHBZ: 819498
> 
> Signed-off-by: Alon Levy 
> ---
>  src/conf/domain_conf.c |3 ++-
>  src/conf/domain_conf.h |1 +
>  2 files changed, 3 insertions(+), 1 deletion(-)

Missing a change to the documentation in docs/formatdomain.html.in, and
to the RelaxNG grammar in docs/schemas/domaincommon.rng.  It would also
be nice to see if there is an existing test in tests/qemuxml2argvdata/*
that can be enhanced to cover the new XML, and/or add a new test.

I'm not sure whether this counts as a simple enough addition to include
in 0.9.12, or whether we should wait until after the release to take it,
but the code patch itself looks reasonable.

-- 
Eric Blake   ebl...@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] rpm: Handle different source URLs for maint releases

2012-05-07 Thread Cole Robinson
On 05/07/2012 07:06 AM, Michal Privoznik wrote:
> On 06.05.2012 20:10, Cole Robinson wrote:
>>
>> Signed-off-by: Cole Robinson 
>> ---
>>  libvirt.spec.in |6 +-
>>  1 files changed, 5 insertions(+), 1 deletions(-)
>>
>> diff --git a/libvirt.spec.in b/libvirt.spec.in
>> index e7e0a55..95d8af4 100644
>> --- a/libvirt.spec.in
>> +++ b/libvirt.spec.in
>> @@ -276,10 +276,14 @@ Version: @VERSION@
>>  Release: 1%{?dist}%{?extra_release}
>>  License: LGPLv2+
>>  Group: Development/Libraries
>> -Source: http://libvirt.org/sources/libvirt-%{version}.tar.gz
>>  BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root
>>  URL: http://libvirt.org/
>>  
>> +%if %(echo %{version} | grep -o \\. | wc -l) == 3
>> +%define mainturl stable_updates/
>> +%endif
>> +Source: http://libvirt.org/sources/%{?mainturl}libvirt-%{version}.tar.gz
>> +
>>  %if %{with_libvirtd}
>>  Requires: libvirt-daemon = %{version}-%{release}
>>  %if %{with_network}
> 
> ACK
> 
> Michal

Thanks, pushed now.

- Cole

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


Re: [libvirt] [PATCH 2/2] domain_conf: add "default" to list of valid spice channels

2012-05-07 Thread Eric Blake
On 05/07/2012 06:33 AM, Alon Levy wrote:
> qemu's behavior in this case is to change the spice server behavior to
> require secure connection to any channel not otherwise specified as
> being in plaintext mode. libvirt doesn't currently allow requesting this
> (via plaintext-channel=).
> 
> RHBZ: 819499
> 
> Signed-off-by: Alon Levy 
> ---
>  src/conf/domain_conf.c |3 ++-
>  src/conf/domain_conf.h |1 +
>  2 files changed, 3 insertions(+), 1 deletion(-)

Same complaints as for 1/2 (docs, RNG schema, tests).  Also, is it ever
valid to mark the default channel for plaintext (meaning all channels
not marked secure are plaintext), or must it only be permitted for
secure channels?

-- 
Eric Blake   ebl...@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] [v5 PATCH] qemu: call usb search function for hostdev initialization and hotplug

2012-05-07 Thread Guannan Ren

On 05/07/2012 11:32 PM, Eric Blake wrote:

On 05/06/2012 08:55 AM, Guannan Ren wrote:

src/qemu/qemu_hostdev.c:
refactor qemuPrepareHostdevUSBDevices function, make it focus on
adding usb device to activeUsbHostdevs after check. After that,
the usb hotplug function qemuDomainAttachHostDevice also could use
it.
expand qemuPrepareHostUSBDevices to perform the usb search,
rollback on failure.

src/qemu/qemu_hotplug.c:
If there are multiple usb devices available with same vendorID and productID,
but with different value of "bus, device", we give an error to let user
use  to specify the desired one.
---
  src/qemu/qemu_hostdev.c |  120 ---
  src/qemu/qemu_hostdev.h |3 +-
  src/qemu/qemu_hotplug.c |   69 +++
  3 files changed, 122 insertions(+), 70 deletions(-)

ACK; this fixes my concerns.



   Thanks for the review.
   Pushed.

   Guannan Ren

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


Re: [libvirt] internal error hostname of destination resolved to localhost, but migration requires an FQDN

2012-05-07 Thread Jiri Denemark
On Mon, May 07, 2012 at 20:18:42 +0800, William Herry wrote:
> I use virsh to migrate kvm virtual machine, it show me this error
> 
> I already use ip address in my command:
> virsh migrate --live kvm-test qemu+ssh://root@my_ip:port/system
> 
> after a few tries I make it work by change the hostname of dest to
> something else rather than localhost,
> 
> this looks like a bug cause a already this virsh the ip address,
> 
> or any thing I misunderstand?

I think you forgot to paste the actual error you see. But anyway, the
following page should help you understand what's going on and how to solve it:

http://wiki.libvirt.org/page/Migration_fails_with_%22Unable_to_resolve_address%22_error

Feel free to come back if it doesn't help you :-)

Jirka

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


Re: [libvirt] [v5 PATCH] qemu: call usb search function for hostdev initialization and hotplug

2012-05-07 Thread Eric Blake
On 05/06/2012 08:55 AM, Guannan Ren wrote:
> src/qemu/qemu_hostdev.c:
> refactor qemuPrepareHostdevUSBDevices function, make it focus on
> adding usb device to activeUsbHostdevs after check. After that,
> the usb hotplug function qemuDomainAttachHostDevice also could use
> it.
> expand qemuPrepareHostUSBDevices to perform the usb search,
> rollback on failure.
> 
> src/qemu/qemu_hotplug.c:
> If there are multiple usb devices available with same vendorID and productID,
> but with different value of "bus, device", we give an error to let user
> use  to specify the desired one.
> ---
>  src/qemu/qemu_hostdev.c |  120 
> ---
>  src/qemu/qemu_hostdev.h |3 +-
>  src/qemu/qemu_hotplug.c |   69 +++
>  3 files changed, 122 insertions(+), 70 deletions(-)

ACK; this fixes my concerns.

-- 
Eric Blake   ebl...@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: Add 'maintenance releases' link in 'News' sidebar

2012-05-07 Thread Cole Robinson
On 05/07/2012 06:37 AM, Michal Privoznik wrote:
> On 06.05.2012 20:10, Cole Robinson wrote:
>>
>> Signed-off-by: Cole Robinson 
>> ---
>>  docs/sitemap.html.in |8 +++-
>>  1 files changed, 7 insertions(+), 1 deletions(-)
>>
> 
> ACK
> 
> Michal
> 

Thanks, pushed now.

- Cole

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


[libvirt] internal error hostname of destination resolved to localhost, but migration requires an FQDN

2012-05-07 Thread William Herry
HI

I use virsh to migrate kvm virtual machine, it show me this error

I already use ip address in my command:
virsh migrate --live kvm-test qemu+ssh://root@my_ip:port/system

after a few tries I make it work by change the hostname of dest to
something else rather than localhost,

this looks like a bug cause a already this virsh the ip address,

or any thing I misunderstand?


regards

-- 

===
William Herry

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

Re: [libvirt] [PATCH 2/2] qemu: Emit compatible XML when migrating a domain

2012-05-07 Thread Jiri Denemark
On Fri, May 04, 2012 at 17:28:39 -0600, Eric Blake wrote:
> On 05/04/2012 04:40 PM, Jiri Denemark wrote:
> > When we added the default USB controller into domain XML, we efficiently
> > broke migration to older versions of libvirt that didn't support USB
> > controllers at all (0.9.4 and earlier) even for domains that doesn't use
> 
> s/doesn't/don't/
> 
> > anything that the older libvirt can't provide. We still want to present
> > the default USB controller in any XML seen by a user/app but we can
> > safely remove it from the domain XML used during migration. If we are
> > migrating to a new enough libvirt, it will add the controller XML back,
> > while older libvirt won't be confused with it although it will still
> > tell qemu to create the controller.
> > 
> > Similar approach can be used in the future whenever we find out we
> > always enabled some kind of device without properly advertising it in
> > domain XML.
> 
> Memballoon would be such a device, if we care about migration to even
> older libvirt.

Right.

> > ---
> >  src/qemu/qemu_domain.c|   60 
> > 
> >  src/qemu/qemu_domain.h|   10 +--
> >  src/qemu/qemu_driver.c|   21 +--
> >  src/qemu/qemu_migration.c |   10 ---
> >  src/qemu/qemu_process.c   |8 +++---
> >  5 files changed, 83 insertions(+), 26 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> > index b105644..3752ddf 100644
> > --- a/src/qemu/qemu_domain.c
> > +++ b/src/qemu/qemu_domain.c
> > @@ -1227,11 +1227,14 @@ int
> >  qemuDomainDefFormatBuf(struct qemud_driver *driver,
> > virDomainDefPtr def,
> > unsigned int flags,
> > +   bool compatible,
> > virBuffer *buf)
> 
> Rather than adding a new 'compatible' flag and having to adjust _every_
> caller, could we instead add a new VIR_DOMAIN_XML_INTERNAL flag to be
> or'd in to flags only for the callers that care?  On the other hand,
> we'd have to filter that flag back out before calling into domain_conf,
> so I guess this approach is as good as any.

Yeah, we would need to filter it out and we would need to either move the
internal flags enum to a more public place (it's internal to domain_conf.c
now) or make another enum and make sure neither of them conflicts with the
real flags enum :-) I decided to introduce new bool parameter since the
compiler checked are places I need to change for me and it's more explicit so
that any new caller of this api needs to explicitly think about the value to
pass for compatibility.

> > +if (usb && usb->idx == 0 && usb->model == -1) {
> 
> Just like memballoon, if we ever want to explicitly represent an XML
> with no usb controller, we will have to provide  mode='none'>; but there, the model would not be -1, so this would
> properly be migrated.

Right and we already have a request to do just this. BTW, I don't explicitly
check the PCI address assigned to the controller because the default
automatically added USB1 controller must always have PCI address 0:0:1.2. When
a domains is configured with different address assigned to the default USB
controller, libvirt will refuse to start it.

> ACK.

Thanks. I pushed this series.

Jirka

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


[libvirt] [PATCH 2/2] domain_conf: add "default" to list of valid spice channels

2012-05-07 Thread Alon Levy
qemu's behavior in this case is to change the spice server behavior to
require secure connection to any channel not otherwise specified as
being in plaintext mode. libvirt doesn't currently allow requesting this
(via plaintext-channel=).

RHBZ: 819499

Signed-off-by: Alon Levy 
---
 src/conf/domain_conf.c |3 ++-
 src/conf/domain_conf.h |1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 10b023e..140bc63 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -429,7 +429,8 @@ VIR_ENUM_IMPL(virDomainGraphicsSpiceChannelName,
   "playback",
   "record",
   "smartcard",
-  "usbredir");
+  "usbredir",
+  "default");
 
 VIR_ENUM_IMPL(virDomainGraphicsSpiceChannelMode,
   VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_LAST,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 6581fea..6189efa 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1098,6 +1098,7 @@ enum virDomainGraphicsSpiceChannelName {
 VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_RECORD,
 VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_SMARTCARD,
 VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_USBREDIR,
+VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_DEFAULT,
 
 VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST
 };
-- 
1.7.10.1

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


[libvirt] [PATCH 1/2] domain_conf: add "usbredir" to list of valid spice channels

2012-05-07 Thread Alon Levy
Add "usbredir" channel to list of recognized spice channels.

RHBZ: 819498

Signed-off-by: Alon Levy 
---
 src/conf/domain_conf.c |3 ++-
 src/conf/domain_conf.h |1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3fce7e5..10b023e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -428,7 +428,8 @@ VIR_ENUM_IMPL(virDomainGraphicsSpiceChannelName,
   "cursor",
   "playback",
   "record",
-  "smartcard");
+  "smartcard",
+  "usbredir");
 
 VIR_ENUM_IMPL(virDomainGraphicsSpiceChannelMode,
   VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_LAST,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 5aa8fc1..6581fea 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1097,6 +1097,7 @@ enum virDomainGraphicsSpiceChannelName {
 VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_PLAYBACK,
 VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_RECORD,
 VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_SMARTCARD,
+VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_USBREDIR,
 
 VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST
 };
-- 
1.7.10.1

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


[libvirt] [PATCH 0/3] Fix output of virsh vcpuinfo after cpu hotplug

2012-05-07 Thread Peter Krempa
The informations about virtual processors stored by libvirt were not
updated after a cpu hotplug.

Peter Krempa (3):
  qemu: Refactor qemuDomainSetVcpusFlags
  qemu: Re-detect virtual cpu threads after cpu hot (un)plug.
  qemu: Don't skip detection of virtual cpu's on non KVM targets

 src/qemu/qemu_driver.c  |   48 +++---
 src/qemu/qemu_process.c |   12 ---
 2 files changed, 32 insertions(+), 28 deletions(-)

-- 
1.7.3.4

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


[libvirt] [PATCH 2/3] qemu: Re-detect virtual cpu threads after cpu hot (un)plug.

2012-05-07 Thread Peter Krempa
After a cpu hotplug the qemu driver did not refresh information about
virtual procesors used by qemu and their corresponding threads. This
patch forces a re-detection as is done on start of QEMU.

This ensures that correct informations are reported by the
virDomainGetVcpus API and "virsh vcpuinfo".
---
 src/qemu/qemu_driver.c |   24 
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7b1d1b6..fcaf0d2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3343,6 +3343,8 @@ static int qemudDomainHotplugVcpus(struct qemud_driver 
*driver,
 int ret = -1;
 int oldvcpus = vm->def->vcpus;
 int vcpus = oldvcpus;
+pid_t *cpupids = NULL;
+int ncpupids;

 qemuDomainObjEnterMonitor(driver, vm);

@@ -3373,8 +3375,30 @@ static int qemudDomainHotplugVcpus(struct qemud_driver 
*driver,
 }
 }

+/* after hotplugging the cpu we need to re-detect threads for the virtual
+ * cpus */
+if ((ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids)) < 0)
+goto cleanup;
+
 ret = 0;

+/* Treat failure to get VCPU<->PID mapping as non-fatal */
+if (ncpupids == 0)
+goto cleanup;
+
+if (ncpupids != vcpus) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+_("got wrong number of vCPU pids from QEMU monitor. "
+  "got %d, wanted %d"),
+ncpupids, vcpus);
+VIR_FREE(cpupids);
+ret = -1;
+goto cleanup;
+}
+
+priv->nvcpupids = ncpupids;
+priv->vcpupids = cpupids;
+
 cleanup:
 qemuDomainObjExitMonitor(driver, vm);
 vm->def->vcpus = vcpus;
-- 
1.7.3.4

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


[libvirt] [PATCH 3/3] qemu: Don't skip detection of virtual cpu's on non KVM targets

2012-05-07 Thread Peter Krempa
QEMU adapted the info cpus command from the kvm branch. This patch
removes the check for the KVM domain type to detect virtualprocessors
also while using software emulation.

The output of the "info cpus" command may vary across different targets
but informations that are parsed by libvirt (cpu ID and thread pid) are
(should be) present on those other targets too.
---
 src/qemu/qemu_process.c |   12 
 1 files changed, 0 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f1401e1..b25cecb 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1613,18 +1613,6 @@ qemuProcessDetectVcpuPIDs(struct qemud_driver *driver,
 int ncpupids;
 qemuDomainObjPrivatePtr priv = vm->privateData;

-if (vm->def->virtType != VIR_DOMAIN_VIRT_KVM) {
-priv->nvcpupids = 1;
-if (VIR_ALLOC_N(priv->vcpupids, priv->nvcpupids) < 0) {
-virReportOOMError();
-return -1;
-}
-priv->vcpupids[0] = vm->pid;
-return 0;
-}
-
-/* What follows is now all KVM specific */
-
 qemuDomainObjEnterMonitorWithDriver(driver, vm);
 if ((ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids)) < 0) {
 qemuDomainObjExitMonitorWithDriver(driver, vm);
-- 
1.7.3.4

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


[libvirt] [PATCH 1/3] qemu: Refactor qemuDomainSetVcpusFlags

2012-05-07 Thread Peter Krempa
---
 src/qemu/qemu_driver.c |   24 
 1 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c100a1a..7b1d1b6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3463,8 +3463,7 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int 
nvcpus,
 goto endjob;
 }

-switch (flags) {
-case VIR_DOMAIN_AFFECT_CONFIG:
+if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
 if (maximum) {
 persistentDef->maxvcpus = nvcpus;
 if (nvcpus < persistentDef->vcpus)
@@ -3472,24 +3471,17 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int 
nvcpus,
 } else {
 persistentDef->vcpus = nvcpus;
 }
-ret = 0;
-break;

-case VIR_DOMAIN_AFFECT_LIVE:
-ret = qemudDomainHotplugVcpus(driver, vm, nvcpus);
-break;
+if (virDomainSaveConfig(driver->configDir, persistentDef) < 0)
+goto endjob;
+}

-case VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG:
-ret = qemudDomainHotplugVcpus(driver, vm, nvcpus);
-if (ret == 0) {
-persistentDef->vcpus = nvcpus;
-}
-break;
+if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+if (qemudDomainHotplugVcpus(driver, vm, nvcpus) < 0)
+goto endjob;
 }

-/* Save the persistent config to disk */
-if (flags & VIR_DOMAIN_AFFECT_CONFIG)
-ret = virDomainSaveConfig(driver->configDir, persistentDef);
+ret = 0;

 endjob:
 if (qemuDomainObjEndJob(driver, vm) == 0)
-- 
1.7.3.4

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


Re: [libvirt] [PATCH] rpm: Handle different source URLs for maint releases

2012-05-07 Thread Michal Privoznik
On 06.05.2012 20:10, Cole Robinson wrote:
> 
> Signed-off-by: Cole Robinson 
> ---
>  libvirt.spec.in |6 +-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index e7e0a55..95d8af4 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -276,10 +276,14 @@ Version: @VERSION@
>  Release: 1%{?dist}%{?extra_release}
>  License: LGPLv2+
>  Group: Development/Libraries
> -Source: http://libvirt.org/sources/libvirt-%{version}.tar.gz
>  BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root
>  URL: http://libvirt.org/
>  
> +%if %(echo %{version} | grep -o \\. | wc -l) == 3
> +%define mainturl stable_updates/
> +%endif
> +Source: http://libvirt.org/sources/%{?mainturl}libvirt-%{version}.tar.gz
> +
>  %if %{with_libvirtd}
>  Requires: libvirt-daemon = %{version}-%{release}
>  %if %{with_network}

ACK

Michal

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


Re: [libvirt] [PATCH] docs: Add 'maintenance releases' link in 'News' sidebar

2012-05-07 Thread Michal Privoznik
On 06.05.2012 20:10, Cole Robinson wrote:
> 
> Signed-off-by: Cole Robinson 
> ---
>  docs/sitemap.html.in |8 +++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
> 

ACK

Michal

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


Re: [libvirt] [PATCH] esx: Fix memory leaks in error paths related to transferred ownership

2012-05-07 Thread Michal Privoznik
On 06.05.2012 19:39, Matthias Bolte wrote:
> Appending an item to a list transfers ownership of that item to the
> list owner. But an error can occur in between item allocation and
> appending it to the list. In this case the item has to be freed
> explicitly. This was not done in some special cases resulting in
> possible memory leaks.
> 
> Reported by Coverity.
> ---
> 
> This is actually a v2 for this patch
> 
> https://www.redhat.com/archives/libvir-list/2012-May/msg00115.html
> 
>  src/esx/esx_driver.c |   16 ++--
>  src/esx/esx_vi.c |   63 
> ++
>  2 files changed, 66 insertions(+), 13 deletions(-)
> 


ACK

Michal

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


Re: [libvirt] [PATCHv3 0/4] util: fix libvirtd startup failure due to netlink error

2012-05-07 Thread Gerhard Stenzel
On Fri, 2012-05-04 at 14:51 -0400, Laine Stump wrote:
> As before, I'm unable to fully test this myself, so I won't push
> unless/until I get verification it works.

We tried various test scenarios, and libvirtd->lldpad and
lldpad->libvirtd communication appeared to be functional and stable.

-- 
Best regards, 

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

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

[libvirt] [PATCH] numad: Set memory policy according to the advisory nodeset from numad

2012-05-07 Thread Osier Yang
Though numad will manage the memory allocation of task dynamically,
but it wants management application (libvirt) to pre-set the memory
policy according to the advisory nodeset returned from querying numad,
(just like pre-bind CPU nodeset for domain process), and thus the
performance could benifit much more from it.

This patch introduces new XML tag 'placement', value 'auto' indicates
whether to set the memory policy with the advisory nodeset from numad,
and its value defaults to 'static'. e.g.

  

  

Just like what current "numatune" does, the 'auto' numa memory policy
setting uses libnuma's API too.

So, to full drive numad, one needs to specify placement='auto' for
both "" and "". It's a bit
inconvenient, but makes sense from semantics' point of view.

---
An alternative way is to not introduce the new XML tag, and pre-set
the memory policy implicitly with "4",
but IMHO it implies too much, and I'd not like go this way unless
the new XML tag is not accepted.
---
 docs/formatdomain.html.in  |   11 ++-
 docs/schemas/domaincommon.rng  |   39 +++---
 libvirt.spec.in|1 +
 src/conf/domain_conf.c |   96 
 src/conf/domain_conf.h |9 ++
 src/libvirt_private.syms   |2 +
 src/qemu/qemu_process.c|   79 +++
 tests/qemuxml2argvdata/qemuxml2argv-numad.args |4 +
 tests/qemuxml2argvdata/qemuxml2argv-numad.xml  |   31 
 tests/qemuxml2argvtest.c   |1 +
 10 files changed, 194 insertions(+), 79 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numad.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numad.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index e1fe0c4..01b3124 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -580,9 +580,14 @@
 The optional memory element specifies how to allocate 
memory
 for the domain process on a NUMA host. It contains two attributes,
 attribute mode is either 'interleave', 'strict',
-or 'preferred',
-attribute nodeset specifies the NUMA nodes, it leads same
-syntax with attribute cpuset of element vcpu.
+or 'preferred', attribute nodeset specifies the NUMA 
nodes,
+it leads same syntax with attribute cpuset of element
+vcpu, the optional attribute placement can be
+used to indicate the memory placement mode for domain process, its 
value
+can be either "static" or "auto", defaults to "static". "auto" 
indicates
+the domain process will only allocate memory from the advisory nodeset
+returned from querying numad, and the value of attribute 
nodeset
+will be ignored if it's specified.
 Since 0.9.3
   
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 8419ccc..9b509f1 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -562,16 +562,35 @@
 
   
 
-  
-
-  strict
-  preferred
-  interleave
-
-  
-  
-
-  
+  
+
+  
+
+  strict
+  preferred
+  interleave
+
+  
+  
+
+  static
+  auto
+
+  
+
+
+  
+
+  strict
+  preferred
+  interleave
+
+  
+  
+
+  
+
+  
 
   
 
diff --git a/libvirt.spec.in b/libvirt.spec.in
index e7e0a55..0b119c2 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -454,6 +454,7 @@ BuildRequires: scrub
 
 %if %{with_numad}
 BuildRequires: numad
+BuildRequires: numactl-devel
 %endif
 
 %description
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3fce7e5..b728cb6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -640,6 +640,11 @@ VIR_ENUM_IMPL(virDomainDiskTray, VIR_DOMAIN_DISK_TRAY_LAST,
   "closed",
   "open");
 
+VIR_ENUM_IMPL(virDomainNumatuneMemPlacementMode,
+  VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_LAST,
+  "static",
+  "auto");
+
 #define virDomainReportError(code, ...)  \
 virReportErrorHelper(VIR_FROM_DOMAIN, code, __FILE__,\
  __FUNCTION__, __LINE__, __VA_ARGS__)
@@ -8023,30 +8028,22