[libvirt] [PATCH v3 3/3] qemu: check presence of each disk and its backing file as well

2013-07-30 Thread Guannan Ren
For disk with startupPolicy support, such as cdrom and floppy
when its chain is broken, the startup policy will apply,
otherwise, report an error.
---
 src/qemu/qemu_domain.c  | 31 +--
 src/qemu/qemu_process.c |  6 --
 2 files changed, 13 insertions(+), 24 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index be77991..1e75adb 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2043,19 +2043,11 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr 
driver,
 break;
 
 case VIR_DOMAIN_STARTUP_POLICY_MANDATORY:
-virReportSystemError(errno,
- _(cannot access file '%s'),
- disk-src);
 goto error;
-break;
 
 case VIR_DOMAIN_STARTUP_POLICY_REQUISITE:
-if (cold_boot) {
-virReportSystemError(errno,
- _(cannot access file '%s'),
- disk-src);
+if (cold_boot)
 goto error;
-}
 break;
 
 case VIR_DOMAIN_STARTUP_POLICY_DEFAULT:
@@ -2064,6 +2056,7 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver,
 break;
 }
 
+virResetLastError();
 VIR_DEBUG(Dropping disk '%s' on domain '%s' (UUID '%s') 
   due to inaccessible source '%s',
   disk-dst, vm-def-name, uuid, disk-src);
@@ -2091,22 +2084,24 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
 virDomainDiskDefPtr disk;
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 
+VIR_DEBUG(Checking for disk presence);
 for (i = 0; i  vm-def-ndisks; i++) {
 disk = vm-def-disks[i];
 
-if (!disk-startupPolicy || !disk-src)
+if (!disk-src)
 continue;
 
-if (virFileAccessibleAs(disk-src, F_OK,
-cfg-user,
-cfg-group) = 0) {
-/* disk accessible */
-continue;
+if (qemuDomainDetermineDiskChain(driver, disk, false) = 0 
+qemuDiskChainCheckBroken(disk) = 0)
+continue;
+
+if (disk-startupPolicy) {
+if (qemuDomainCheckDiskStartupPolicy(driver, vm, disk,
+ cold_boot) = 0)
+continue;
 }
 
-if (qemuDomainCheckDiskStartupPolicy(driver, vm, disk,
- cold_boot)  0)
-goto cleanup;
+goto cleanup;
 }
 
 ret = 0;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e8e459e..61a897c 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3621,16 +3621,10 @@ int qemuProcessStart(virConnectPtr conn,
 if (qemuAssignDeviceAliases(vm-def, priv-qemuCaps)  0)
 goto cleanup;
 
-VIR_DEBUG(Checking for CDROM and floppy presence);
 if (qemuDomainCheckDiskPresence(driver, vm,
 flags  VIR_QEMU_PROCESS_START_COLD)  0)
 goto cleanup;
 
-for (i = 0; i  vm-def-ndisks; i++) {
-if (qemuDomainDetermineDiskChain(driver, vm-def-disks[i],
- false)  0)
-goto cleanup;
-}
 
 /* Get the advisory nodeset from numad if 'placement' of
  * either vcpu or numatune is 'auto'.
-- 
1.8.3.1

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


[libvirt] [PATCH] Fix minor typos in messages and docs

2013-07-30 Thread Yuri Chornoivan
From 1b8af1b915f3cce1d27576f651e99b6ce3a73e3a Mon Sep 17 00:00:00 2001
From: Yuri Chornoivan yurc...@ukr.net
Date: Tue, 30 Jul 2013 11:21:11 +0300
Subject: [PATCH] Fix minor typos in messages and docs

---
 docs/apibuild.py  |  2 +-
 docs/formatdomain.html.in |  6 ++---
 docs/hacking.html.in  |  2 +-
 docs/migration.html.in|  4 ++--
 docs/news.html.in | 60 +++
 src/esx/esx_vi.c  |  2 +-
 src/lxc/lxc_driver.c  |  4 ++--
 7 files changed, 40 insertions(+), 40 deletions(-)

diff --git a/docs/apibuild.py b/docs/apibuild.py
index e65c559..5c55d79 100755
--- a/docs/apibuild.py
+++ b/docs/apibuild.py
@@ -919,7 +919,7 @@ class CParser:
 l = lines[0]
 i = 0
 # Remove all leading '*', followed by at most one ' ' character
-# since we need to preserve correct identation of code examples
+# since we need to preserve correct indentation of code examples
 while i  len(l) and l[i] == '*':
 i = i + 1
 if i  0:
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 78e132e..49c7c8d 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1001,7 +1001,7 @@
 /dl
 
 p
-  Guest NUMA topology can be specifed using the codenuma/code element.
+  Guest NUMA topology can be specified using the codenuma/code element.
   span class=sinceSince 0.9.8/span
 /p
 
@@ -3034,7 +3034,7 @@
 
 p
   Provides direct attachment of the virtual machine's NIC to the given
-  physial interface of the host.
+  physical interface of the host.
   span class=sinceSince 0.7.7 (QEMU and KVM only)/spanbr/
   This setup requires the Linux macvtap
   driver to be available. span class=since(Since Linux 2.6.34.)/span
@@ -3439,7 +3439,7 @@ qemu-kvm -net nic,model=? /dev/null
 p
   If no target is specified, certain hypervisors will
   automatically generate a name for the created tun device. This
-  name can be manually specifed, however the name imust not
+  name can be manually specified, however the name imust not
   start with either 'vnet' or 'vif'/i, which are prefixes
   reserved by libvirt and certain hypervisors. Manually specified
   targets using these prefixes will be ignored.
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 700eb3a..8120b19 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -207,7 +207,7 @@
 ==4643==by 0x34D8C01569: ??? (in /usr/lib64/ld-2.15.so)
 
 /pre
-pIn this instance, it is acceptible to modify the
+pIn this instance, it is acceptable to modify the
codetests/.valgrind.supp/code file in order to add a
suppression filter. The filter should be unique enough to
not suppress real leaks, but it should be generic enough to
diff --git a/docs/migration.html.in b/docs/migration.html.in
index 2ea0969..6986cc0 100644
--- a/docs/migration.html.in
+++ b/docs/migration.html.in
@@ -9,7 +9,7 @@
 p
   Migration of guests between hosts is a complicated problem with many possible
   solutions, each with their own positive and negative points. For maximum
-  flexibility of both hypervisor integration, and adminsitrator deployment,
+  flexibility of both hypervisor integration, and administrator deployment,
   libvirt implements several options for migration.
 /p
 
@@ -187,7 +187,7 @@
 URI. In this case the management application should specify the
 hypervisor specific URI explicitly, using an IP address, or a
 correct hostname./li
-  liThe host has multiple network interaces. If a host has multiple
+  liThe host has multiple network interfaces. If a host has multiple
 network interfaces, it might be desirable for the migration data
 stream to be sent over a specific interface for either security
 or performance reasons. In this case the management application
diff --git a/docs/news.html.in b/docs/news.html.in
index 059b2bc..f176d30 100644
--- a/docs/news.html.in
+++ b/docs/news.html.in
@@ -460,7 +460,7 @@ and check the a href=http://libvirt.org/git/?p=libvirt.git;a=log;GIT log/a
   Add libvirt-daemon-vbox amp; libvirt-daemon-driver-vbox RPMs (Daniel P. Berrange),br/
   Include GNULIB mkdtemp module (Daniel P. Berrange),br/
   Set PKG_CONFIG_LIBDIR in autobuild.sh (Daniel P. Berrange),br/
-  qemu: report useful error failling to destroy domain gracefully (Guannan Ren),br/
+  qemu: report useful error failing to destroy domain gracefully (Guannan Ren),br/
   qemu: Check conflicts for shared scsi host device (Osier Yang),br/
   Re-add selinux/selinux.h to lxc_container.c (Daniel P. Berrange),br/
   schema: make source optional in volume XML (Ján Tomko),br/
@@ -1222,7 +1222,7 @@ and check the a href=http://libvirt.org/git/?p=libvirt.git;a=log;GIT log/a
   

Re: [libvirt] [PATCH] Configuring systemd to restart libvirt on abort

2013-07-30 Thread Mooli Tayer


- Original Message -
 On 07/29/2013 08:36 AM, Mooli Tayer wrote:
  From: Mooli Tayer mta...@redhat.com
  
  This will create a respawn behaviour in case libvirt
  process exits due to an uncaught signal not specified
  as a clean exit status.
  see http://www.freedesktop.org/software/systemd/man/systemd.service.html
  ---
   daemon/libvirtd.service.in | 1 +
   1 file changed, 1 insertion(+)
 
 Looks useful.  However, I think this is a feature, and as we are already
 in freeze for 1.1.1, I think this should wait until after the release
 before this goes in.

Does this patch needs to be resubmitted after the release? 
 
  
  diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in
  index aa5913b..b3c0849 100644
  --- a/daemon/libvirtd.service.in
  +++ b/daemon/libvirtd.service.in
  @@ -15,6 +15,7 @@ EnvironmentFile=-/etc/sysconfig/libvirtd
   ExecStart=@sbindir@/libvirtd $LIBVIRTD_ARGS
   ExecReload=/bin/kill -HUP $MAINPID
   KillMode=process
  +Restart=on-abort
   # Override the maximum number of opened files
   #LimitNOFILE=2048
   
  
 
 --
 Eric Blake   eblake redhat com+1-919-301-3266
 Libvirt virtualization library http://libvirt.org
 
 
 --
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list

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


[libvirt] [PATCH] tests: Put a mock library at the start of LD_PRELOAD

2013-07-30 Thread Jiri Denemark
This fixes vircgrouptest when run in a sandbox which already overrides
open() and others.
---
 tests/testutils.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/testutils.h b/tests/testutils.h
index 27af5da..8583747 100644
--- a/tests/testutils.h
+++ b/tests/testutils.h
@@ -87,8 +87,9 @@ int virtTestMain(int argc,
 perror(lib);\
 return EXIT_FAILURE;\
 }   \
-if (virAsprintf(newenv, %s%s%s, preload ? preload : ,  \
-preload ? : : , lib)  0) { \
+if (!preload) { \
+newenv = (char *) lib;  \
+} else if (virAsprintf(newenv, %s:%s, lib, preload)  0) {   \
 perror(virAsprintf);  \
 return EXIT_FAILURE;\
 }   \
-- 
1.8.3.2

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


Re: [libvirt] [PATCH] tests: Put a mock library at the start of LD_PRELOAD

2013-07-30 Thread Daniel P. Berrange
On Tue, Jul 30, 2013 at 11:29:11AM +0200, Jiri Denemark wrote:
 This fixes vircgrouptest when run in a sandbox which already overrides
 open() and others.

In general I think that having multiple LD_PRELOADS overriding the
same symbols may well lead to pain.

We try to make it at least chain up the libraries by using the
'RTLD_NEXT' handle when looking up the real symbol, which
should make it forward to the next preloaded library. From
the sound of it, your sandbox tool isn't using RTLD_NEXT,
causing the libvirt override to be skipped when the order was
reversed.

 ---
  tests/testutils.h | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)
 
 diff --git a/tests/testutils.h b/tests/testutils.h
 index 27af5da..8583747 100644
 --- a/tests/testutils.h
 +++ b/tests/testutils.h
 @@ -87,8 +87,9 @@ int virtTestMain(int argc,
  perror(lib);\
  return EXIT_FAILURE;\
  }   \
 -if (virAsprintf(newenv, %s%s%s, preload ? preload : ,  \
 -preload ? : : , lib)  0) { \
 +if (!preload) { \
 +newenv = (char *) lib;  \
 +} else if (virAsprintf(newenv, %s:%s, lib, preload)  0) {   \
  perror(virAsprintf);  \
  return EXIT_FAILURE;\
  }   \

ACK

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] Add flag to BaselineCPU API to return detailed CPU features

2013-07-30 Thread Daniel P. Berrange
On Mon, Jul 29, 2013 at 05:31:19PM -0600, Don Dugger wrote:
 
 Currently the virConnectBaselineCPU API does not expose the CPU features
 that are part of the CPU's model.  This patch adds a new flag,
 VIR_CONNECT_BASELINE_SHOW_MODEL, that causes the API to explictly
 list all features that are part of that model.

Nit-pick: I'd prefer the constant to be named

VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE

so that it is more precisely describing what it is doing,
and includes the word CPU there to match the API name.

 +static void
 +x86AddFeatures(virCPUDefPtr cpu,
 +   struct x86_map *map)
 +{
 +const struct x86_model *candidate;
 +const struct x86_feature *feature = map-features;
 +
 +candidate = map-models;
 +while (candidate != NULL) {
 +if (STREQ(cpu-model, candidate-name))
 +break;
 +candidate = candidate-next;
 +}
 +if (!candidate) {
 +VIR_WARN(Odd, %s not a known CPU model\n, cpu-model);

VIR_WARN shouldn't be used for errors which can occur in public API call
paths. It should virReportError() and return an error code '-1' to the
caller of the API

 +return;
 +}
 +while (feature != NULL) {
 +if (x86DataIsSubset(candidate-data, feature-data)) {
 +if (virCPUDefAddFeature(cpu, feature-name, 
 VIR_CPU_FEATURE_REQUIRE)  0) {
 +VIR_WARN(CPU model %s, no room for feature %s, cpu-model, 
 feature-name);
 +return;

Again this should be reporting an error  propagating it back to the
caller

Regards,
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


[libvirt] [PATCH] Configuring systemd to restart libvirt on abort

2013-07-30 Thread Mooli Tayer

- Original Message -
 On Mon, Jul 29, 2013 at 05:36:21PM +0300, Mooli Tayer wrote:
  From: Mooli Tayer mta...@redhat.com
  
  This will create a respawn behaviour in case libvirt
  process exits due to an uncaught signal not specified
  as a clean exit status.
  see http://www.freedesktop.org/software/systemd/man/systemd.service.html
  ---
   daemon/libvirtd.service.in | 1 +
   1 file changed, 1 insertion(+)
  
  diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in
  index aa5913b..b3c0849 100644
  --- a/daemon/libvirtd.service.in
  +++ b/daemon/libvirtd.service.in
  @@ -15,6 +15,7 @@ EnvironmentFile=-/etc/sysconfig/libvirtd
   ExecStart=@sbindir@/libvirtd $LIBVIRTD_ARGS
   ExecReload=/bin/kill -HUP $MAINPID
   KillMode=process
  +Restart=on-abort
   # Override the maximum number of opened files
   #LimitNOFILE=2048
 
 I'm wondering whether 'on-abort' is the best choice or if
 'on-failure' or 'always' are better. The systemd.service
 man page says
 
 [quote]
Takes one of no, on-success, on-failure,
on-abort, or always. If set to no (the
default) the service will not be restarted. If
set to on-success it will be restarted only
when the service process exits cleanly. In
this context, a clean exit means an exit code
of 0, or one of the signals SIGHUP, SIGINT,
SIGTERM, or SIGPIPE, and additionally, exit
statuses and signals specified in
SuccessExitStatus=. If set to on-failure the
service will be restarted when the process
exits with an nonzero exit code, is terminated
by a signal (including on core dump), when an
operation (such as service reload) times out,
and when the configured watchdog timeout is
triggered. If set to on-abort the service will
be restarted only if the service process exits
due to an uncaught signal not specified as a
clean exit status. If set to always the
service will be restarted regardless whether
it exited cleanly or not, got terminated
abnormally by a signal or hit a timeout.
 [/quote]
 
 I tend towards saying 'on-failure' here.

I agree. we defiantly want restart in the 'on-failure'
cases.
 
 
 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
 

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


[libvirt] Release of libvirt-1.1.1

2013-07-30 Thread Daniel Veillard
  As planned the release is out, tagged in git and available
at the usual place:

   ftp://libvirt.org/libvirt/

This is a rather large release, including around 380 commits (time
to take some vacations !) with a large set of bug fixes, including
3 CVE security ones, and large improvements over the code base.
The localization got updates for no less then 18 languages too,
with 8 of them being nearly 100% complete !

Features:
- Adding device removal or deletion events (Jiri Denemark)
- Introduce new domain create APIs to pass pre-opened FDs to LXC (Daniel P. 
Berrange)
- Add interface versions for Xen 4.3 (Stefan Bader)
- Add new public API virDomainSetMemoryStatsPeriod (John Ferlan)
- Various LXC improvements (Daniel P. Berrange and Gao feng)

Security:
- security: fix deadlock with prefork (Eric Blake)
- CVE-2013-4153 qemu: Fix double free of returned JSON array in 
qemuAgentGetVCPUs() (Peter Krempa)
- CVE-2013-4154 qemu: Prevent crash of libvirtd without guest agent 
configuration (Alex Jia)
- CVE-2013-2230 Fix crash when multiple event callbacks were registered (Ján 
Tomko)

Documentation:
- formatdomain.html.in: Document implementation limitation of QoS (Michal 
Privoznik)
- formatdomain.html.in: Correctly use code/ in #elementQoS (Michal Privoznik)
- Fix copy-paste-error in virNodeGetMemoryStats (Philipp Hahn)
- virsh: Mention --driver in man page for nodedev-detach (Peter Krempa)
- maint: tweak use of a in HACKING (Eric Blake)
- maint: fix typo in qemu error message (Eric Blake)
- daemon: Fix command example in libvirtd.sasl (Cole Robinson)
- Put virt-sanlock-cleanup into section 8 (Guido Günther)
- Document hypervisor drivers that support certain timer models (Peter Krempa)

Portability:
- build: fix shunloadtest breakage (Eric Blake)
- examples: fix mingw build vs. printf (Eric Blake)
- build: skip systemd mock on non-Linux (Eric Blake)
- Fix dbus message reading code on big endian hosts (Daniel P. Berrange)
- build: fix another virdbus issue on mingw (Eric Blake)
- build: fix virutil build on mingw (Eric Blake)
- build: fix virthread build on mingw (Eric Blake)
- build: fix virdbus build on mingw (Eric Blake)
- build: fix vircgroup build on mingw (Eric Blake)
- Conditionalize build of virCgroupValidateMachineGroup (Daniel P. Berrange)
- build: fix VPATH 'make check' (Eric Blake)
- cpu: Fix one compile error for PPC. (Li Zhang)
- virdbustest: Don't pass number of arguments as long long (Guido Günther)
- Fix virCgroupAvailable() w/o HAVE_GETMNTENT_R defined (Roman Bogorodskiy)
- Fix link_addr detection (Roman Bogorodskiy)
- build: work around broken kernel headers (Eric Blake)
- dbus: work with older dbus (Eric Blake)
- Use AC_LINK_IFELSE (Guido Günther)
- Check for link_addr more thoroughly (Guido Günther)
- Fix bridge routines detection on kFreeBSD (Roman Bogorodskiy)
- Fix build with clang (Ján Tomko)
- build: don't ship access syms files in tarball (Eric Blake)
- build: work around mingw header pollution (Eric Blake)
- build: avoid build failure without gnutls (Eric Blake)

Bug Fixes:
- Fix probing of legacy Xen driver to not leave URI set (Daniel P. Berrange)
- caps: use -device for primary video when qemu =1.6 (Guannan Ren)
- Resolve Coverity complaint in storagevolxml2argvtest (Ján Tomko)
- Don't check validity of missing attributes in DNS SRV XML (Ján Tomko)
- Set the number of elements 0 in virNetwork*Clear (Ján Tomko)
- conf:Fix a copy paste error (Alex Jia)
- virLXCMonitorClose: Unlock domain while closing monitor (Michal Privoznik)
- libxl: Correctly initialize vcpu bitmap (Stefan Bader)
- Add new virAuth symbols to private.syms (Ján Tomko)
- Use qemuOpenFile in qemu_driver.c (Martin Kletzander)
- Make qemuOpenFile aware of per-VM DAC seclabel. (Martin Kletzander)
- domain_event: Resolve memory leak found by Valgrind (John Ferlan)
- lxc: Resolve Coverity warning (John Ferlan)
- qemu: Take error path if acquiring of job fails in qemuDomainSaveInternal 
(Peter Krempa)
- build: fix make rpm failure (Laine Stump)
- virsh: fix change-media bug on disk block type (Guannan Ren)
- tests: Free test at the end of GetDeviceAliases JSON test (Jiri Denemark)
- vmware: Fix bogus CPU arch copy (Jiri Denemark)
- qemu: Shorten SCSI hostdev alias to avoid QEMU failure (Viktor Mihajlovski)
- Add virtio-scsi to fallback models of scsi controller (Martin Kletzander)
- qemuhotplugtest: Resolve some memleaks (Michal Privoznik)
- qemuDomainDetachChrDevice: Don't leak @charAlias (Michal Privoznik)
- Fix impl of virDomainCreateWithFlags remote client helper (Daniel P. Berrange)
- cgroup: reuse buffer for getline (Ján Tomko)
- Create directory for lease files if it's missing (Guido Günther)
- rbd: Do not free the secret if it is not set (Wido den Hollander)
- Make logical pools independent on target path (Martin Kletzander)
- qemu: add macvlan delete to qemuDomainAttachNetDevice cleanup (Matthew Rosato)
- qemuBuildChrDeviceCommandLine: Don't leak devstr (Michal Privoznik)
- conf: reject pci-root controllers with 

[libvirt] [PATCH] Delete obsolete / unused python test files

2013-07-30 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

The python/tests directory contains a number of so called
tests for the python API. These are all hardcoded to
look for Xen and cannot be run in any automated fashion,
and no one is ever manually running them. Given that they
don't meaningully contribute to the test coverage, delete
them.

For some reason these tests were also copied into the
filesystem as part of 'make install'. The change to the
RPM in commit 3347a4203278ec93d7b0ceb88b5ed10e4f14765c
caused a build failure, since it removed the code which
deleted these installed tests.

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 configure.ac |   2 +-
 python/Makefile.am   |   2 -
 python/tests/Makefile.am |  52 -
 python/tests/basic.py|  31 --
 python/tests/create.py   | 146 ---
 python/tests/error.py|  42 --
 python/tests/node.py |  34 ---
 python/tests/uuid.py |  41 -
 8 files changed, 1 insertion(+), 349 deletions(-)
 delete mode 100644 python/tests/Makefile.am
 delete mode 100755 python/tests/basic.py
 delete mode 100755 python/tests/create.py
 delete mode 100755 python/tests/error.py
 delete mode 100755 python/tests/node.py
 delete mode 100755 python/tests/uuid.py

diff --git a/configure.ac b/configure.ac
index 35a5d76..084d864 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2447,7 +2447,7 @@ AC_OUTPUT(Makefile src/Makefile include/Makefile 
docs/Makefile \
   libvirt.pc libvirt.spec mingw-libvirt.spec \
   po/Makefile.in \
  include/libvirt/Makefile include/libvirt/libvirt.h \
- python/Makefile python/tests/Makefile \
+ python/Makefile \
   daemon/Makefile \
   tools/Makefile \
   tests/Makefile \
diff --git a/python/Makefile.am b/python/Makefile.am
index 7eb42c6..925e1f4 100644
--- a/python/Makefile.am
+++ b/python/Makefile.am
@@ -16,8 +16,6 @@
 ## License along with this library.  If not, see
 ## http://www.gnu.org/licenses/.
 
-SUBDIRS= . tests
-
 INCLUDES = \
$(PYTHON_INCLUDES) \
-I$(top_builddir)/gnulib/lib \
diff --git a/python/tests/Makefile.am b/python/tests/Makefile.am
deleted file mode 100644
index 0fd3c78..000
--- a/python/tests/Makefile.am
+++ /dev/null
@@ -1,52 +0,0 @@
-## Copyright (C) 2005-2011, 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
-## http://www.gnu.org/licenses/.
-
-EXAMPLE_DIR = $(datadir)/doc/libvirt-python-$(VERSION)/examples
-
-PYTESTS=   \
-   basic.py\
-   create.py   \
-   uuid.py \
-   error.py\
-   node.py
-
-EXTRA_DIST = $(PYTESTS)
-
-if WITH_PYTHON
-tests: $(PYTESTS)
-   @echo ## running Python regression tests
-   -@(PYTHONPATH=..:../.libs:../src/.libs:$(srcdir)/../src:$$PYTHONPATH;\
-  export PYTHONPATH; \
-  LD_LIBRARY_PATH=$(top_builddir)/src/.libs:$$LD_LIBRARY_PATH ; \
-  export LD_LIBRARY_PATH; \
-  for test in $(PYTESTS) ; \
-  do log=`$(PYTHON) $(srcdir)/$$test` ; \
-  if [ `echo $$log | grep OK` =  ] ; then \
-  echo -- $$test ; echo $$log ; fi ; done)
-else
-tests:
-endif
-
-clean:
-   rm -f *.pyc core
-
-install-data-local:
-   $(mkinstalldirs) $(DESTDIR)$(EXAMPLE_DIR)
-   -(for test in $(PYTESTS); \
- do $(INSTALL) -m 0644 $(srcdir)/$$test $(DESTDIR)$(EXAMPLE_DIR) ; 
done)
-
-uninstall-local:
-   for test in $(PYTESTS); do rm -f $(DESTDIR)$(EXAMPLE_DIR)/$$test; done
diff --git a/python/tests/basic.py b/python/tests/basic.py
deleted file mode 100755
index c6dec81..000
--- a/python/tests/basic.py
+++ /dev/null
@@ -1,31 +0,0 @@
-#!/usr/bin/python -u
-import libvirt
-import sys
-import os
-
-if not os.access(/proc/xen, os.R_OK):
-print 'System is not running a Xen kernel'
-sys.exit(1)
-
-conn = libvirt.openReadOnly(None)
-if conn == None:
-print 'Failed to open connection to the hypervisor'
-sys.exit(1)
-
-# print conn
-
-try:
-dom0 = conn.lookupByName(Domain-0)
-except:
-print 'Failed to find the main domain'
-sys.exit(1)
-
-# print dom0
-
-print Domain 0: id %d running %s % (dom0.ID(), dom0.OSType())
-print dom0.info()
-del dom0
-del conn
-print OK
-
-sys.exit(0)
diff --git a/python/tests/create.py b/python/tests/create.py
deleted file 

Re: [libvirt] [PATCH] Delete obsolete / unused python test files

2013-07-30 Thread Jiri Denemark
On Tue, Jul 30, 2013 at 11:27:58 +0100, Daniel Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 The python/tests directory contains a number of so called
 tests for the python API. These are all hardcoded to
 look for Xen and cannot be run in any automated fashion,
 and no one is ever manually running them. Given that they
 don't meaningully contribute to the test coverage, delete
 them.
 
 For some reason these tests were also copied into the
 filesystem as part of 'make install'. The change to the
 RPM in commit 3347a4203278ec93d7b0ceb88b5ed10e4f14765c
 caused a build failure, since it removed the code which
 deleted these installed tests.
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com

ACK

Jirka

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


[libvirt] [PATCH] Support apparmor in RPM spec

2013-07-30 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

If libapparmor-devel happens to be installed when building the
RPM, it will failed due to unlisted virt-aa-helper in %files.
Add support for apparmor in the spec, so that we can explicitly
turn it on/off, defaulting to off in all distros. This causes
--without-apparmor to be given to configure, preventing the
build failures if the user happens to have libapparmor-devel
present.

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 libvirt.spec.in | 16 
 1 file changed, 16 insertions(+)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index a3a831f..6212b3c 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -100,6 +100,9 @@
 %define with_numactl  0%{!?_without_numactl:%{server_drivers}}
 %define with_selinux  0%{!?_without_selinux:%{server_drivers}}
 
+# Just hardcode to off, since few people ever have apparmor RPMs installed
+%define with_apparmor 0%{!?_without_apparmor:0}
+
 # A few optional bits off by default, we enable later
 %define with_polkit0%{!?_without_polkit:0}
 %define with_capng 0%{!?_without_capng:0}
@@ -472,6 +475,9 @@ BuildRequires: avahi-devel
 %if %{with_selinux}
 BuildRequires: libselinux-devel
 %endif
+%if %{with_apparmor}
+BuildRequires: libapparmor-devel
+%endif
 %if %{with_network}
 BuildRequires: dnsmasq = 2.41
 BuildRequires: iptables
@@ -1268,6 +1274,10 @@ of recent versions of Linux (and other OSes).
 %define _without_selinux --without-selinux
 %endif
 
+%if ! %{with_apparmor}
+%define _without_apparmor --without-apparmor
+%endif
+
 %if ! %{with_hal}
 %define _without_hal --without-hal
 %endif
@@ -1367,6 +1377,7 @@ of recent versions of Linux (and other OSes).
%{?_without_netcf} \
%{?_without_selinux} \
%{?_with_selinux_mount} \
+   %{?_without_apparmor} \
%{?_without_hal} \
%{?_without_udev} \
%{?_without_yajl} \
@@ -1835,6 +1846,11 @@ fi
 %endif
 
 %attr(0755, root, root) %{_libexecdir}/libvirt_iohelper
+
+%if %{with_apparmor}
+%attr(0755, root, root) %{_libexecdir}/virt-aa-helper
+%endif
+
 %attr(0755, root, root) %{_sbindir}/libvirtd
 %attr(0755, root, root) %{_sbindir}/virtlockd
 
-- 
1.8.3.1

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


[libvirt] [PATCH] spec: RHEL-7 does not have sanlock on i686

2013-07-30 Thread Jiri Denemark
---
 libvirt.spec.in | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 6236c08..e74f774 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -245,11 +245,16 @@
 %if 0%{?fedora} = 16
 %define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
 %endif
-%if 0%{?rhel} = 6
+%if 0%{?rhel} == 6
 %ifarch %{ix86} x86_64
 %define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
 %endif
 %endif
+%if 0%{?rhel} = 7
+%ifarch x86_64
+%define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
+%endif
+%endif
 
 # Enable libssh2 transport for new enough distros
 %if 0%{?fedora} = 17
-- 
1.8.3.2

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


[libvirt] [PATCH] spec: Disable libssh2 support for RHEL

2013-07-30 Thread Jiri Denemark
From: Peter Krempa pkre...@redhat.com

https://bugzilla.redhat.com/show_bug.cgi?id=905513

Libssh2 isn't reliable enough to support the libvirt transport using it.
The problems include mishandling of known_hosts files that may confuse
users.
---
 libvirt.spec.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index a3a831f..6236c08 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -252,7 +252,7 @@
 %endif
 
 # Enable libssh2 transport for new enough distros
-%if 0%{?fedora} = 17 || 0%{?rhel} = 6
+%if 0%{?fedora} = 17
 %define with_libssh2 0%{!?_without_libssh2:1}
 %endif
 
-- 
1.8.3.2

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


Re: [libvirt] [PATCH] Delete obsolete / unused python test files

2013-07-30 Thread Daniel Veillard
On Tue, Jul 30, 2013 at 12:57:36PM +0200, Jiri Denemark wrote:
 On Tue, Jul 30, 2013 at 11:27:58 +0100, Daniel Berrange wrote:
  From: Daniel P. Berrange berra...@redhat.com
  
  The python/tests directory contains a number of so called
  tests for the python API. These are all hardcoded to
  look for Xen and cannot be run in any automated fashion,
  and no one is ever manually running them. Given that they
  don't meaningully contribute to the test coverage, delete
  them.
  
  For some reason these tests were also copied into the
  filesystem as part of 'make install'. The change to the
  RPM in commit 3347a4203278ec93d7b0ceb88b5ed10e4f14765c
  caused a build failure, since it removed the code which
  deleted these installed tests.
  
  Signed-off-by: Daniel P. Berrange berra...@redhat.com
 
 ACK

  I'm fine seeing them go :-)

ACK too

Daniel

-- 
Daniel Veillard  | Open Source and Standards, Red Hat
veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/

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


[libvirt] [PATCH] Configuring systemd to restart libvirt on failure

2013-07-30 Thread Mooli Tayer
This will create a respawn behaviour in case libvirt
process exits due to nonzero exit code, is terminated
by a signal, an operation times out or the configured
watchdog timeout is triggered.
see http://www.freedesktop.org/software/systemd/man/systemd.service.html
---
 daemon/libvirtd.service.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in
index aa5913b..25979ef 100644
--- a/daemon/libvirtd.service.in
+++ b/daemon/libvirtd.service.in
@@ -15,6 +15,7 @@ EnvironmentFile=-/etc/sysconfig/libvirtd
 ExecStart=@sbindir@/libvirtd $LIBVIRTD_ARGS
 ExecReload=/bin/kill -HUP $MAINPID
 KillMode=process
+Restart=on-failure
 # Override the maximum number of opened files
 #LimitNOFILE=2048
 
-- 
1.8.3.1

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


Re: [libvirt] [PATCH] spec: Disable libssh2 support for RHEL

2013-07-30 Thread Eric Blake
On 07/30/2013 05:45 AM, Jiri Denemark wrote:
 From: Peter Krempa pkre...@redhat.com
 
 https://bugzilla.redhat.com/show_bug.cgi?id=905513
 
 Libssh2 isn't reliable enough to support the libvirt transport using it.
 The problems include mishandling of known_hosts files that may confuse
 users.
 ---
  libvirt.spec.in | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

ACK.

 
 diff --git a/libvirt.spec.in b/libvirt.spec.in
 index a3a831f..6236c08 100644
 --- a/libvirt.spec.in
 +++ b/libvirt.spec.in
 @@ -252,7 +252,7 @@
  %endif
  
  # Enable libssh2 transport for new enough distros
 -%if 0%{?fedora} = 17 || 0%{?rhel} = 6
 +%if 0%{?fedora} = 17
  %define with_libssh2 0%{!?_without_libssh2:1}
  %endif
  
 

-- 
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] spec: RHEL-7 does not have sanlock on i686

2013-07-30 Thread Eric Blake
On 07/30/2013 05:46 AM, Jiri Denemark wrote:
 ---
  libvirt.spec.in | 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

ACK.

  %if 0%{?fedora} = 16
  %define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
  %endif
 -%if 0%{?rhel} = 6
 +%if 0%{?rhel} == 6
  %ifarch %{ix86} x86_64
  %define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
  %endif
  %endif
 +%if 0%{?rhel} = 7
 +%ifarch x86_64
 +%define with_sanlock 0%{!?_without_sanlock:%{server_drivers}}
 +%endif
 +%endif

It feels a bit redundant, but I don't see any more compact way to
combine conditionals.

-- 
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] Support apparmor in RPM spec

2013-07-30 Thread Eric Blake
On 07/30/2013 05:06 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 If libapparmor-devel happens to be installed when building the
 RPM, it will failed due to unlisted virt-aa-helper in %files.
 Add support for apparmor in the spec, so that we can explicitly
 turn it on/off, defaulting to off in all distros. This causes
 --without-apparmor to be given to configure, preventing the
 build failures if the user happens to have libapparmor-devel
 present.

Makes sense - we want the spec file to be absolute regardless of the
dev's environment.

Question - is there a repo somewhere with libapparmor-devel built for
Fedora, or does it remain something where I'm best off testing on a
Debian-based build?

 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  libvirt.spec.in | 16 
  1 file changed, 16 insertions(+)

ACK.

-- 
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 01/20] conf: Export virDomainChrSourceDefClear()

2013-07-30 Thread Peter Krempa
---
 src/conf/domain_conf.c   | 2 +-
 src/conf/domain_conf.h   | 2 ++
 src/libvirt_private.syms | 1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 783df96..e3aec69 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1332,7 +1332,7 @@ void virDomainNetDefFree(virDomainNetDefPtr def)
 VIR_FREE(def);
 }

-static void ATTRIBUTE_NONNULL(1)
+void ATTRIBUTE_NONNULL(1)
 virDomainChrSourceDefClear(virDomainChrSourceDefPtr def)
 {
 switch (def-type) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index abf024c..de3b59c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2719,4 +2719,6 @@ int virDomainDefFindDevice(virDomainDefPtr def,
 bool virDomainDiskSourceIsBlockType(virDomainDiskDefPtr def)
 ATTRIBUTE_NONNULL(1);

+void virDomainChrSourceDefClear(virDomainChrSourceDefPtr def);
+
 #endif /* __DOMAIN_CONF_H */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d9615ea..7163350 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -123,6 +123,7 @@ virDomainChrInsert;
 virDomainChrRemove;
 virDomainChrSerialTargetTypeFromString;
 virDomainChrSerialTargetTypeToString;
+virDomainChrSourceDefClear;
 virDomainChrSourceDefCopy;
 virDomainChrSourceDefFree;
 virDomainChrSpicevmcTypeFromString;
-- 
1.8.3.2

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


[libvirt] [PATCH 02/20] qemu_agent: Output newline at the end of the sync JSON message

2013-07-30 Thread Peter Krempa
Although this isn't apparently needed for the guest agent itself, the
test I will be adding later depends on the newline as a separator of
messages to process.
---
 src/qemu/qemu_agent.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 72bf211..1607e88 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -918,7 +918,7 @@ qemuAgentGuestSync(qemuAgentPtr mon)

 if (virAsprintf(sync_msg.txBuffer,
 {\execute\:\guest-sync\, 
-\arguments\:{\id\:%llu}}, id)  0)
+\arguments\:{\id\:%llu}}\n, id)  0)
 return -1;

 sync_msg.txLength = strlen(sync_msg.txBuffer);
-- 
1.8.3.2

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


[libvirt] [PATCH 03/20] qemu_agent: Move updater function for VCPU hotplug into qemu_agent.c

2013-07-30 Thread Peter Krempa
To allow testing of the cpu updater function, this function needs to be
available separately. Export it from qemu_agent.c where it should
belong.
---
 src/qemu/qemu_agent.c  | 63 +
 src/qemu/qemu_agent.h  |  3 +++
 src/qemu/qemu_driver.c | 64 +-
 3 files changed, 67 insertions(+), 63 deletions(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 1607e88..fc85e3e 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1597,3 +1597,66 @@ cleanup:
 virJSONValueFree(cpus);
 return ret;
 }
+
+
+/* modify the cpu info structure to set the correct amount of cpus */
+int
+qemuAgentUpdateCPUInfo(unsigned int nvcpus,
+   qemuAgentCPUInfoPtr cpuinfo,
+   int ncpuinfo)
+{
+size_t i;
+int nonline = 0;
+int nofflinable = 0;
+
+/* count the active and offlinable cpus */
+for (i = 0; i  ncpuinfo; i++) {
+if (cpuinfo[i].online)
+nonline++;
+
+if (cpuinfo[i].offlinable  cpuinfo[i].online)
+nofflinable++;
+
+/* This shouldn't happen, but we can't trust the guest agent */
+if (!cpuinfo[i].online  !cpuinfo[i].offlinable) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Invalid data provided by guest agent));
+return -1;
+}
+}
+
+/* the guest agent reported less cpus than requested */
+if (nvcpus  ncpuinfo) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(guest agent reports less cpu than requested));
+return -1;
+}
+
+/* not enough offlinable CPUs to support the request */
+if (nvcpus  nonline - nofflinable) {
+virReportError(VIR_ERR_INVALID_ARG, %s,
+   _(Cannot offline enough CPUs));
+return -1;
+}
+
+for (i = 0; i  ncpuinfo; i++) {
+if (nvcpus  nonline) {
+/* unplug */
+if (cpuinfo[i].offlinable  cpuinfo[i].online) {
+cpuinfo[i].online = false;
+nonline--;
+}
+} else if (nvcpus  nonline) {
+/* plug */
+if (!cpuinfo[i].online) {
+cpuinfo[i].online = true;
+nonline++;
+}
+} else {
+/* done */
+break;
+}
+}
+
+return 0;
+}
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index cf70653..5fbacdb 100644
--- a/src/qemu/qemu_agent.h
+++ b/src/qemu/qemu_agent.h
@@ -94,4 +94,7 @@ struct _qemuAgentCPUInfo {

 int qemuAgentGetVCPUs(qemuAgentPtr mon, qemuAgentCPUInfoPtr *info);
 int qemuAgentSetVCPUs(qemuAgentPtr mon, qemuAgentCPUInfoPtr cpus, size_t 
ncpus);
+int qemuAgentUpdateCPUInfo(unsigned int nvcpus,
+   qemuAgentCPUInfoPtr cpuinfo,
+   int ncpuinfo);
 #endif /* __QEMU_AGENT_H__ */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5634abf..2daafa8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4090,68 +4090,6 @@ unsupported:


 static int
-qemuDomainPrepareAgentVCPUs(unsigned int nvcpus,
-qemuAgentCPUInfoPtr cpuinfo,
-int ncpuinfo)
-{
-size_t i;
-int nonline = 0;
-int nofflinable = 0;
-
-/* count the active and offlinable cpus */
-for (i = 0; i  ncpuinfo; i++) {
-if (cpuinfo[i].online)
-nonline++;
-
-if (cpuinfo[i].offlinable  cpuinfo[i].online)
-nofflinable++;
-
-/* This shouldn't happen, but we can't trust the guest agent */
-if (!cpuinfo[i].online  !cpuinfo[i].offlinable) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   _(Invalid data provided by guest agent));
-return -1;
-}
-}
-
-/* the guest agent reported less cpus than requested */
-if (nvcpus  ncpuinfo) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   _(guest agent reports less cpu than requested));
-return -1;
-}
-
-/* not enough offlinable CPUs to support the request */
-if (nvcpus  nonline - nofflinable) {
-virReportError(VIR_ERR_INVALID_ARG, %s,
-   _(Cannot offline enough CPUs));
-return -1;
-}
-
-for (i = 0; i  ncpuinfo; i++) {
-if (nvcpus  nonline) {
-/* unplug */
-if (cpuinfo[i].offlinable  cpuinfo[i].online) {
-cpuinfo[i].online = false;
-nonline--;
-}
-} else if (nvcpus  nonline) {
-/* plug */
-if (!cpuinfo[i].online) {
-cpuinfo[i].online = true;
-nonline++;
-}
-} else {
-/* done */
-break;
-}
-}
-
-return 0;
-}
-
-
-static int
 qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned 

[libvirt] [PATCH 00/20] tests: add qemu agent test

2013-07-30 Thread Peter Krempa
As promised earlier I'm sending a unit test for guest agent interaction.

This series contains a few refactors and additions of monitor test utils
and then add tests for all agent interaction functions. The refactors
done in this series will allow to do a more thorough testing on the json
monitor too.

The small drawback of this test suite is a 6 second run time introduced
to test timeout of a guest agent command. The test may be removed in
case it's too much.

Peter Krempa (20):
  conf: Export virDomainChrSourceDefClear()
  qemu_agent: Output newline at the end of the sync JSON message
  qemu_agent: Move updater function for VCPU hotplug into qemu_agent.c
  qemu_agent: Remove obvious comments
  qemumonitortestutils: Use consistent header style and line spacing
  qemumonitortestutils: Use VIR_DELETE_ELEMENT and VIR_APPEND_ELEMENT
  qemumonitortestutils: remove multiline function calls
  qemumonitortestutils: Don't crash on non fully initialized test
  qemumonitortestutils: Split up creation of the test to allow reuse
  qemumonitortestutils: Refactor the test helpers to allow reuse
  qemumonitortestutils: Split lines on \n instead of \r\n
  qemumonitortestutils: Add instrumentation for guest agent testing
  qemumonitortestutils: Improve error reporting from mock qemu monitor
  qemumonitortestutils: Add the ability to check arguments of commands
  tests: Add qemuagenttest
  qemuagenttest: Test the filesystem trimming
  qemuagenttest: Add testing of agent suspend modes
  qemuagenttest: Introduce testing of shutdown commands
  qemuagenttest: Test arbitrary qemu commands and timeouting of commands
  qemuagenttest: Add tests for CPU plug functions and helpers

 .gitignore   |   1 +
 src/conf/domain_conf.c   |   2 +-
 src/conf/domain_conf.h   |   2 +
 src/libvirt_private.syms |   1 +
 src/qemu/qemu_agent.c|  73 -
 src/qemu/qemu_agent.h|   3 +
 src/qemu/qemu_driver.c   |  64 +---
 tests/Makefile.am|  11 +-
 tests/qemuagenttest.c| 580 
 tests/qemumonitortestutils.c | 684 +--
 tests/qemumonitortestutils.h |  39 ++-
 11 files changed, 1231 insertions(+), 229 deletions(-)
 create mode 100644 tests/qemuagenttest.c

-- 
1.8.3.2

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


[libvirt] [PATCH 04/20] qemu_agent: Remove obvious comments

2013-07-30 Thread Peter Krempa
Most APIs in libvirt report errors, thus no need to state that
explictly.
---
 src/qemu/qemu_agent.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index fc85e3e..2cd0ccc 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -930,10 +930,8 @@ qemuAgentGuestSync(qemuAgentPtr mon)

 VIR_DEBUG(qemuAgentSend returned: %d, send_ret);

-if (send_ret  0) {
-/* error reported */
+if (send_ret  0)
 goto cleanup;
-}

 if (!sync_msg.rxObject) {
 virReportError(VIR_ERR_INTERNAL_ERROR, %s,
@@ -976,10 +974,8 @@ qemuAgentCommand(qemuAgentPtr mon,

 *reply = NULL;

-if (qemuAgentGuestSync(mon)  0) {
-/* helper reported the error */
+if (qemuAgentGuestSync(mon)  0)
 return -1;
-}

 memset(msg, 0, sizeof(msg));

-- 
1.8.3.2

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


[libvirt] [PATCH 05/20] qemumonitortestutils: Use consistent header style and line spacing

2013-07-30 Thread Peter Krempa
---
 tests/qemumonitortestutils.c | 62 +---
 1 file changed, 41 insertions(+), 21 deletions(-)

diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index 56368a2..ff90c5f 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -76,11 +76,13 @@ struct _qemuMonitorTest {

 static void qemuMonitorTestItemFree(qemuMonitorTestItemPtr item);

+
 /*
  * Appends data for a reply to the outgoing buffer
  */
-static int qemuMonitorTestAddReponse(qemuMonitorTestPtr test,
- const char *response)
+static int
+qemuMonitorTestAddReponse(qemuMonitorTestPtr test,
+  const char *response)
 {
 size_t want = strlen(response) + 2;
 size_t have = test-outgoingCapacity - test-outgoingLength;
@@ -107,8 +109,9 @@ static int qemuMonitorTestAddReponse(qemuMonitorTestPtr 
test,
  * Processes a single line, looking for a matching expected
  * item to reply with, else replies with an error
  */
-static int qemuMonitorTestProcessCommandJSON(qemuMonitorTestPtr test,
- const char *cmdstr)
+static int
+qemuMonitorTestProcessCommandJSON(qemuMonitorTestPtr test,
+  const char *cmdstr)
 {
 virJSONValuePtr val;
 const char *cmdname;
@@ -150,8 +153,9 @@ cleanup:
 }


-static int qemuMonitorTestProcessCommandText(qemuMonitorTestPtr test,
- const char *cmdstr)
+static int
+qemuMonitorTestProcessCommandText(qemuMonitorTestPtr test,
+  const char *cmdstr)
 {
 char *tmp;
 char *cmdname;
@@ -190,8 +194,10 @@ cleanup:
 return ret;
 }

-static int qemuMonitorTestProcessCommand(qemuMonitorTestPtr test,
- const char *cmdstr)
+
+static int
+qemuMonitorTestProcessCommand(qemuMonitorTestPtr test,
+  const char *cmdstr)
 {
 if (test-json)
 return qemuMonitorTestProcessCommandJSON(test ,cmdstr);
@@ -199,12 +205,14 @@ static int 
qemuMonitorTestProcessCommand(qemuMonitorTestPtr test,
 return qemuMonitorTestProcessCommandText(test ,cmdstr);
 }

+
 /*
  * Handles read/write of monitor data on the monitor server side
  */
-static void qemuMonitorTestIO(virNetSocketPtr sock,
-  int events,
-  void *opaque)
+static void
+qemuMonitorTestIO(virNetSocketPtr sock,
+  int events,
+  void *opaque)
 {
 qemuMonitorTestPtr test = opaque;
 bool err = false;
@@ -298,7 +306,8 @@ cleanup:
 }


-static void qemuMonitorTestWorker(void *opaque)
+static void
+qemuMonitorTestWorker(void *opaque)
 {
 qemuMonitorTestPtr test = opaque;

@@ -322,7 +331,9 @@ static void qemuMonitorTestWorker(void *opaque)
 return;
 }

-static void qemuMonitorTestItemFree(qemuMonitorTestItemPtr item)
+
+static void
+qemuMonitorTestItemFree(qemuMonitorTestItemPtr item)
 {
 if (!item)
 return;
@@ -335,13 +346,15 @@ static void 
qemuMonitorTestItemFree(qemuMonitorTestItemPtr item)


 static void
-qemuMonitorTestFreeTimer(int timer ATTRIBUTE_UNUSED, void *opaque 
ATTRIBUTE_UNUSED)
+qemuMonitorTestFreeTimer(int timer ATTRIBUTE_UNUSED,
+ void *opaque ATTRIBUTE_UNUSED)
 {
 /* nothing to be done here */
 }


-void qemuMonitorTestFree(qemuMonitorTestPtr test)
+void
+qemuMonitorTestFree(qemuMonitorTestPtr test)
 {
 size_t i;
 int timer = -1;
@@ -424,13 +437,16 @@ error:
 }


-static void qemuMonitorTestEOFNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
- virDomainObjPtr vm ATTRIBUTE_UNUSED)
+static void
+qemuMonitorTestEOFNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
+ virDomainObjPtr vm ATTRIBUTE_UNUSED)
 {
 }

-static void qemuMonitorTestErrorNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
-   virDomainObjPtr vm ATTRIBUTE_UNUSED)
+
+static void
+qemuMonitorTestErrorNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
+   virDomainObjPtr vm ATTRIBUTE_UNUSED)
 {
 }

@@ -440,12 +456,14 @@ static qemuMonitorCallbacks qemuCallbacks = {
 .errorNotify = qemuMonitorTestErrorNotify,
 };

+
 #define QEMU_JSON_GREETING {\QMP\: {\version\: {\qemu\: {\micro\: 1, 
\minor\: 0, \major\: 1}, \package\: \ (qemu-kvm-1.0.1)\}, 
\capabilities\: []}}
 /* We skip the normal handshake reply of {\execute\:\qmp_capabilities\} 
*/

 #define QEMU_TEXT_GREETING QEMU 1.0,1 monitor - type 'help' for more 
information

-qemuMonitorTestPtr qemuMonitorTestNew(bool json, virDomainXMLOptionPtr xmlopt)
+qemuMonitorTestPtr
+qemuMonitorTestNew(bool json, virDomainXMLOptionPtr xmlopt)
 {
 qemuMonitorTestPtr test = NULL;
 virDomainChrSourceDef src;
@@ -541,7 +559,9 @@ error:
 goto cleanup;
 }

-qemuMonitorPtr qemuMonitorTestGetMonitor(qemuMonitorTestPtr test)
+

[libvirt] [PATCH 07/20] qemumonitortestutils: remove multiline function calls

2013-07-30 Thread Peter Krempa
---
 tests/qemumonitortestutils.c | 13 +++--
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index a3c5431..34ca1ae 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -94,12 +94,8 @@ qemuMonitorTestAddReponse(qemuMonitorTestPtr test,
 }

 want -= 2;
-memcpy(test-outgoing + test-outgoingLength,
-   response,
-   want);
-memcpy(test-outgoing + test-outgoingLength + want,
-   \r\n,
-   2);
+memcpy(test-outgoing + test-outgoingLength, response, want);
+memcpy(test-outgoing + test-outgoingLength + want, \r\n, 2);
 test-outgoingLength += want + 2;
 return 0;
 }
@@ -484,10 +480,7 @@ qemuMonitorTestNew(bool json, virDomainXMLOptionPtr xmlopt)
 if (!(test-vm = virDomainObjNew(xmlopt)))
 goto error;

-if (virNetSocketNewListenUNIX(path,
-  0700,
-  getuid(),
-  getgid(),
+if (virNetSocketNewListenUNIX(path, 0700, getuid(), getgid(),
   test-server)  0)
 goto error;

-- 
1.8.3.2

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


[libvirt] [PATCH 08/20] qemumonitortestutils: Don't crash on non fully initialized test

2013-07-30 Thread Peter Krempa
The qemumonitorjsontest crashed when one of the initialization steps
done before starting the worker thread failed. This patch fixes this by
trying to pthread_join() the thread only after it was created.
---
 tests/qemumonitortestutils.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index 34ca1ae..5ca569f 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -368,7 +368,8 @@ qemuMonitorTestFree(qemuMonitorTestPtr test)

 virObjectUnref(test-vm);

-virThreadJoin(test-thread);
+if (test-running)
+virThreadJoin(test-thread);

 if (timer != -1)
 virEventRemoveTimeout(timer);
-- 
1.8.3.2

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


[libvirt] [PATCH 13/20] qemumonitortestutils: Improve error reporting from mock qemu monitor

2013-07-30 Thread Peter Krempa
Use the JSON error messages to report errors back to the caller in
addition to erroring out. The error reported from the event loop from
the mock function of the monitor was later overwritten by the call to
the monitor/agent interaction API. This will also allow testing of error
reporting.
---
 tests/qemumonitortestutils.c | 56 
 1 file changed, 46 insertions(+), 10 deletions(-)

diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index 96cb054..f421b9e 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -103,6 +103,8 @@ qemuMonitorTestAddReponse(qemuMonitorTestPtr test,
 size_t want = strlen(response) + 2;
 size_t have = test-outgoingCapacity - test-outgoingLength;

+VIR_DEBUG(Adding response to monitor command: '%s, response);
+
 if (have  want) {
 size_t need = want - have;
 if (VIR_EXPAND_N(test-outgoing, test-outgoingCapacity, need)  0)
@@ -132,11 +134,48 @@ 
qemuMonitorTestAddUnexpectedErrorResponse(qemuMonitorTestPtr test)


 static int
+qemuMonitorReportError(qemuMonitorTestPtr test, const char *errmsg, ...)
+{
+va_list msgargs;
+char *msg = NULL;
+char *jsonmsg = NULL;
+int ret = -1;
+
+va_start(msgargs, errmsg);
+
+if (virVasprintf(msg, errmsg, msgargs)  0)
+goto cleanup;
+
+if (test-agent || test-json) {
+if (virAsprintf(jsonmsg, { \error\: 
+   { \desc\: \%s\, 
+ \class\: \UnexpectedCommand\ } },
+  msg)  0)
+goto cleanup;
+} else {
+if (virAsprintf(jsonmsg, error: '%s', msg)  0)
+goto cleanup;
+}
+
+ret = qemuMonitorTestAddReponse(test, jsonmsg);
+
+cleanup:
+va_end(msgargs);
+VIR_FREE(msg);
+VIR_FREE(jsonmsg);
+return ret;
+}
+
+
+
+static int
 qemuMonitorTestProcessCommand(qemuMonitorTestPtr test,
   const char *cmdstr)
 {
 int ret;

+VIR_DEBUG(Processing string from monitor handler: '%s, cmdstr);
+
 if (test-nitems == 0) {
 return qemuMonitorTestAddUnexpectedErrorResponse(test);
 } else {
@@ -399,8 +438,7 @@ qemuMonitorTestProcessCommandDefault(qemuMonitorTestPtr 
test,
 return -1;

 if (!(cmdname = virJSONValueObjectGetString(val, execute))) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   Missing command name in %s, cmdstr);
+ret = qemuMonitorReportError(test, Missing command name in %s, 
cmdstr);
 goto cleanup;
 }
 } else {
@@ -410,8 +448,9 @@ qemuMonitorTestProcessCommandDefault(qemuMonitorTestPtr 
test,
 cmdname = cmdcopy;

 if (!(tmp = strchr(cmdcopy, ' '))) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   Cannot find command name in '%s', cmdstr);
+ret = qemuMonitorReportError(test,
+ Cannot find command name in '%s',
+ cmdstr);
 goto cleanup;
 }
 *tmp = '\0';
@@ -465,8 +504,7 @@ qemuMonitorTestProcessGuestAgentSync(qemuMonitorTestPtr 
test,
 return -1;

 if (!(cmdname = virJSONValueObjectGetString(val, execute))) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   Missing guest-sync command name);
+ret = qemuMonitorReportError(test, Missing guest-sync command name);
 goto cleanup;
 }

@@ -476,14 +514,12 @@ qemuMonitorTestProcessGuestAgentSync(qemuMonitorTestPtr 
test,
 }

 if (!(args = virJSONValueObjectGet(val, arguments))) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   Missing arguments for guest-sync);
+ret = qemuMonitorReportError(test, Missing arguments for guest-sync);
 goto cleanup;
 }

 if (virJSONValueObjectGetNumberUlong(args, id, id)) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   Missing id for guest sync);
+ret = qemuMonitorReportError(test, Missing id for guest sync);
 goto cleanup;
 }

-- 
1.8.3.2

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


[libvirt] [PATCH 09/20] qemumonitortestutils: Split up creation of the test to allow reuse

2013-07-30 Thread Peter Krempa
The instrumentation for the monitor test can be hacked for qemu agent
testing. Split out the monitor specific stuff to allow using the code in
guest agent tests in the future.
---
 tests/qemumonitortestutils.c | 96 ++--
 1 file changed, 66 insertions(+), 30 deletions(-)

diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index 5ca569f..1785293 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -440,16 +440,11 @@ static qemuMonitorCallbacks qemuCallbacks = {
 };


-#define QEMU_JSON_GREETING {\QMP\: {\version\: {\qemu\: {\micro\: 1, 
\minor\: 0, \major\: 1}, \package\: \ (qemu-kvm-1.0.1)\}, 
\capabilities\: []}}
-/* We skip the normal handshake reply of {\execute\:\qmp_capabilities\} 
*/
-
-#define QEMU_TEXT_GREETING QEMU 1.0,1 monitor - type 'help' for more 
information
-
-qemuMonitorTestPtr
-qemuMonitorTestNew(bool json, virDomainXMLOptionPtr xmlopt)
+static qemuMonitorTestPtr
+qemuMonitorCommonTestNew(virDomainXMLOptionPtr xmlopt,
+ virDomainChrSourceDefPtr src)
 {
 qemuMonitorTestPtr test = NULL;
-virDomainChrSourceDef src;
 char *path = NULL;
 char *tmpdir_template = NULL;

@@ -477,7 +472,6 @@ qemuMonitorTestNew(bool json, virDomainXMLOptionPtr xmlopt)
 if (virAsprintf(path, %s/qemumonitorjsontest.sock, test-tmpdir)  0)
 goto error;

-test-json = json;
 if (!(test-vm = virDomainObjNew(xmlopt)))
 goto error;

@@ -485,29 +479,36 @@ qemuMonitorTestNew(bool json, virDomainXMLOptionPtr 
xmlopt)
   test-server)  0)
 goto error;

-memset(src, 0, sizeof(src));
-src.type = VIR_DOMAIN_CHR_TYPE_UNIX;
-src.data.nix.path = (char *)path;
-src.data.nix.listen = false;
+memset(src, 0, sizeof(*src));
+src-type = VIR_DOMAIN_CHR_TYPE_UNIX;
+src-data.nix.path = (char *)path;
+src-data.nix.listen = false;

 if (virNetSocketListen(test-server, 1)  0)
 goto error;

-if (!(test-mon = qemuMonitorOpen(test-vm,
-  src,
-  json,
-  qemuCallbacks)))
-goto error;
-virObjectLock(test-mon);
+cleanup:
+return test;
+
+error:
+VIR_FREE(tmpdir_template);
+qemuMonitorTestFree(test);
+test = NULL;
+goto cleanup;
+
+}
+
+
+static int
+qemuMonitorCommonTestInit(qemuMonitorTestPtr test)
+{
+if (!test)
+return -1;

 if (virNetSocketAccept(test-server, test-client)  0)
 goto error;
-if (!test-client)
-goto error;

-if (qemuMonitorTestAddReponse(test, json ?
-  QEMU_JSON_GREETING :
-  QEMU_TEXT_GREETING)  0)
+if (!test-client)
 goto error;

 if (virNetSocketAddIOCallback(test-client,
@@ -528,17 +529,52 @@ qemuMonitorTestNew(bool json, virDomainXMLOptionPtr 
xmlopt)
 test-running = true;
 virMutexUnlock(test-lock);

-cleanup:
-VIR_FREE(path);
-return test;
+return 0;

 error:
-VIR_FREE(tmpdir_template);
 qemuMonitorTestFree(test);
-test = NULL;
-goto cleanup;
+return -1;
 }

+#define QEMU_JSON_GREETING {\QMP\: {\version\: {\qemu\: {\micro\: 1, 
\minor\: 0, \major\: 1}, \package\: \ (qemu-kvm-1.0.1)\}, 
\capabilities\: []}}
+/* We skip the normal handshake reply of {\execute\:\qmp_capabilities\} 
*/
+
+#define QEMU_TEXT_GREETING QEMU 1.0,1 monitor - type 'help' for more 
information
+
+qemuMonitorTestPtr
+qemuMonitorTestNew(bool json, virDomainXMLOptionPtr xmlopt)
+{
+qemuMonitorTestPtr test = NULL;
+virDomainChrSourceDef src;
+
+if (!(test = qemuMonitorCommonTestNew(xmlopt, src)))
+goto error;
+
+test-json = json;
+if (!(test-mon = qemuMonitorOpen(test-vm,
+  src,
+  json,
+  qemuCallbacks)))
+goto error;
+
+virObjectLock(test-mon);
+
+if (qemuMonitorTestAddReponse(test, json ?
+  QEMU_JSON_GREETING :
+  QEMU_TEXT_GREETING)  0)
+goto error;
+
+if (qemuMonitorCommonTestInit(test)  0)
+goto error;
+
+virDomainChrSourceDefClear(src);
+
+return test;
+
+error:
+qemuMonitorTestFree(test);
+return NULL;
+}

 qemuMonitorPtr
 qemuMonitorTestGetMonitor(qemuMonitorTestPtr test)
-- 
1.8.3.2

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


[libvirt] [PATCH 06/20] qemumonitortestutils: Use VIR_DELETE_ELEMENT and VIR_APPEND_ELEMENT

2013-07-30 Thread Peter Krempa
Simplify the code using the existing helpers instead of open coding the
same functionality.
---
 tests/qemumonitortestutils.c | 32 +---
 1 file changed, 9 insertions(+), 23 deletions(-)

diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index ff90c5f..a3c5431 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -133,17 +133,11 @@ qemuMonitorTestProcessCommandJSON(qemuMonitorTestPtr test,
  { \desc\: \Unexpected command\, 
\class\: \UnexpectedCommand\ } 
});
 } else {
-ret = qemuMonitorTestAddReponse(test,
-test-items[0]-response);
+ret = qemuMonitorTestAddReponse(test, test-items[0]-response);
 qemuMonitorTestItemFree(test-items[0]);
-if (test-nitems == 1) {
-VIR_FREE(test-items);
-test-nitems = 0;
-} else {
-memmove(test-items,
-test-items + 1,
-sizeof(test-items[0]) * (test-nitems - 1));
-VIR_SHRINK_N(test-items, test-nitems, 1);
+if (VIR_DELETE_ELEMENT(test-items, 0, test-nitems)  0) {
+ret = -1;
+goto cleanup;
 }
 }

@@ -175,17 +169,11 @@ qemuMonitorTestProcessCommandText(qemuMonitorTestPtr test,
 ret = qemuMonitorTestAddReponse(test,
 unexpected command);
 } else {
-ret = qemuMonitorTestAddReponse(test,
-test-items[0]-response);
+ret = qemuMonitorTestAddReponse(test, test-items[0]-response);
 qemuMonitorTestItemFree(test-items[0]);
-if (test-nitems == 1) {
-VIR_FREE(test-items);
-test-nitems = 0;
-} else {
-memmove(test-items,
-test-items + 1,
-sizeof(test-items[0]) * (test-nitems - 1));
-VIR_SHRINK_N(test-items, test-nitems, 1);
+if (VIR_DELETE_ELEMENT(test-items, 0, test-nitems)  0) {
+ret = -1;
+goto cleanup;
 }
 }

@@ -421,12 +409,10 @@ qemuMonitorTestAddItem(qemuMonitorTestPtr test,
 goto error;

 virMutexLock(test-lock);
-if (VIR_EXPAND_N(test-items, test-nitems, 1)  0) {
+if (VIR_APPEND_ELEMENT(test-items, test-nitems, item)  0) {
 virMutexUnlock(test-lock);
 goto error;
 }
-test-items[test-nitems - 1] = item;
-
 virMutexUnlock(test-lock);

 return 0;
-- 
1.8.3.2

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


[libvirt] [PATCH 16/20] qemuagenttest: Test the filesystem trimming

2013-07-30 Thread Peter Krempa
---
 tests/qemuagenttest.c | 31 +++
 1 file changed, 31 insertions(+)

diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
index 7f22ff0..a3b8834 100644
--- a/tests/qemuagenttest.c
+++ b/tests/qemuagenttest.c
@@ -132,6 +132,36 @@ cleanup:


 static int
+testQemuAgentFSTrim(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt);
+int ret = -1;
+
+if (!test)
+return -1;
+
+if (qemuMonitorTestAddAgentSyncResponse(test)  0)
+goto cleanup;
+
+if (qemuMonitorTestAddItemParams(test, guest-fstrim,
+ { \return\ : {} },
+ minimum, 1337,
+ NULL)  0)
+goto cleanup;
+
+if (qemuAgentFSTrim(qemuMonitorTestGetAgent(test), 1337)  0)
+goto cleanup;
+
+ret = 0;
+
+cleanup:
+qemuMonitorTestFree(test);
+return ret;
+}
+
+
+static int
 mymain(void)
 {
 int ret = 0;
@@ -154,6 +184,7 @@ mymain(void)

 DO_TEST(FSFreeze);
 DO_TEST(FSThaw);
+DO_TEST(FSTrim);

 virObjectUnref(xmlopt);

-- 
1.8.3.2

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


[libvirt] [PATCH 10/20] qemumonitortestutils: Refactor the test helpers to allow reuse

2013-07-30 Thread Peter Krempa
Refactor the test helpers to allow adding callbacks to verify the
monitor responses instead of simple command name checking and clean up
various parts to prepare for adding guest agent tests.
---
 tests/qemumonitortestutils.c | 220 ---
 1 file changed, 121 insertions(+), 99 deletions(-)

diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index 1785293..941dfea 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -38,10 +38,14 @@

 typedef struct _qemuMonitorTestItem qemuMonitorTestItem;
 typedef qemuMonitorTestItem *qemuMonitorTestItemPtr;
+typedef int (*qemuMonitorTestResponseCallback)(qemuMonitorTestPtr test,
+   qemuMonitorTestItemPtr item,
+   const char *message);

 struct _qemuMonitorTestItem {
-char *command_name;
-char *response;
+qemuMonitorTestResponseCallback cb;
+void *opaque;
+virFreeCallback freecb;
 };

 struct _qemuMonitorTest {
@@ -74,7 +78,17 @@ struct _qemuMonitorTest {
 };


-static void qemuMonitorTestItemFree(qemuMonitorTestItemPtr item);
+static void
+qemuMonitorTestItemFree(qemuMonitorTestItemPtr item)
+{
+if (!item)
+return;
+
+if (item-freecb)
+(item-freecb)(item-opaque);
+
+VIR_FREE(item);
+}


 /*
@@ -101,95 +115,40 @@ qemuMonitorTestAddReponse(qemuMonitorTestPtr test,
 }


-/*
- * Processes a single line, looking for a matching expected
- * item to reply with, else replies with an error
- */
 static int
-qemuMonitorTestProcessCommandJSON(qemuMonitorTestPtr test,
-  const char *cmdstr)
+qemuMonitorTestAddUnexpectedErrorResponse(qemuMonitorTestPtr test)
 {
-virJSONValuePtr val;
-const char *cmdname;
-int ret = -1;
-
-if (!(val = virJSONValueFromString(cmdstr)))
-return -1;
-
-if (!(cmdname = virJSONValueObjectGetString(val, execute))) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   Missing command name in %s, cmdstr);
-goto cleanup;
-}
-
-if (test-nitems == 0 ||
-STRNEQ(test-items[0]-command_name, cmdname)) {
-ret = qemuMonitorTestAddReponse(test,
-{ \error\: 
- { \desc\: \Unexpected command\, 
-   \class\: \UnexpectedCommand\ } 
});
+if (test-json) {
+return qemuMonitorTestAddReponse(test,
+ { \error\: 
+  { \desc\: \Unexpected command\, 

+\class\: \UnexpectedCommand\ 
} });
 } else {
-ret = qemuMonitorTestAddReponse(test, test-items[0]-response);
-qemuMonitorTestItemFree(test-items[0]);
-if (VIR_DELETE_ELEMENT(test-items, 0, test-nitems)  0) {
-ret = -1;
-goto cleanup;
-}
+return qemuMonitorTestAddReponse(test, unexpected command);
 }
-
-cleanup:
-virJSONValueFree(val);
-return ret;
 }


 static int
-qemuMonitorTestProcessCommandText(qemuMonitorTestPtr test,
-  const char *cmdstr)
+qemuMonitorTestProcessCommand(qemuMonitorTestPtr test,
+  const char *cmdstr)
 {
-char *tmp;
-char *cmdname;
-int ret = -1;
+int ret;

-if (VIR_STRDUP(cmdname, cmdstr)  0)
-return -1;
-if (!(tmp = strchr(cmdname, ' '))) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   Cannot find command name in '%s', cmdstr);
-goto cleanup;
-}
-*tmp = '\0';
-
-if (test-nitems == 0 ||
-STRNEQ(test-items[0]-command_name, cmdname)) {
-ret = qemuMonitorTestAddReponse(test,
-unexpected command);
+if (test-nitems == 0) {
+return qemuMonitorTestAddUnexpectedErrorResponse(test);
 } else {
-ret = qemuMonitorTestAddReponse(test, test-items[0]-response);
-qemuMonitorTestItemFree(test-items[0]);
-if (VIR_DELETE_ELEMENT(test-items, 0, test-nitems)  0) {
-ret = -1;
-goto cleanup;
-}
+qemuMonitorTestItemPtr item = test-items[0];
+ret = (item-cb)(test, item, cmdstr);
+qemuMonitorTestItemFree(item);
+if (VIR_DELETE_ELEMENT(test-items, 0, test-nitems)  0)
+return -1;
 }

-cleanup:
-VIR_FREE(cmdname);
 return ret;
 }


-static int
-qemuMonitorTestProcessCommand(qemuMonitorTestPtr test,
-  const char *cmdstr)
-{
-if (test-json)
-return qemuMonitorTestProcessCommandJSON(test ,cmdstr);
-else
-return qemuMonitorTestProcessCommandText(test ,cmdstr);
-}
-
-
 /*
  * Handles read/write of monitor data on the monitor server side
  */
@@ -317,19 +276,6 @@ qemuMonitorTestWorker(void *opaque)


 static void

[libvirt] [PATCH 15/20] tests: Add qemuagenttest

2013-07-30 Thread Peter Krempa
Add a basic test framework with two simple tests to test guest agent
interaction.
---
 .gitignore|   1 +
 tests/Makefile.am |  11 +++-
 tests/qemuagenttest.c | 163 ++
 3 files changed, 174 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemuagenttest.c

diff --git a/.gitignore b/.gitignore
index 4c79de3..738c6ba 100644
--- a/.gitignore
+++ b/.gitignore
@@ -165,6 +165,7 @@
 /tests/object-locking-files.txt
 /tests/object-locking.cm[ix]
 /tests/openvzutilstest
+/tests/qemuagenttest
 /tests/qemuargv2xmltest
 /tests/qemuhelptest
 /tests/qemuhotplugtest
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 5ea806e..9c578fa 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -161,7 +161,8 @@ endif
 if WITH_QEMU
 test_programs += qemuxml2argvtest qemuxml2xmltest qemuxmlnstest \
qemuargv2xmltest qemuhelptest domainsnapshotxml2xmltest \
-   qemumonitortest qemumonitorjsontest qemuhotplugtest
+   qemumonitortest qemumonitorjsontest qemuhotplugtest \
+   qemuagenttest
 endif

 if WITH_LXC
@@ -418,6 +419,13 @@ qemumonitorjsontest_SOURCES = \
$(NULL)
 qemumonitorjsontest_LDADD = $(qemu_LDADDS) libqemumonitortestutils.la

+qemuagenttest_SOURCES = \
+   qemuagenttest.c \
+   testutils.c testutils.h \
+   testutilsqemu.c testutilsqemu.h \
+   $(NULL)
+qemuagenttest_LDADD = $(qemu_LDADDS) libqemumonitortestutils.la
+
 qemuhotplugtest_SOURCES = \
qemuhotplugtest.c \
testutils.c testutils.h \
@@ -434,6 +442,7 @@ EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c 
qemuargv2xmltest.c \
qemuxmlnstest.c qemuhelptest.c domainsnapshotxml2xmltest.c \
qemumonitortest.c testutilsqemu.c testutilsqemu.h \
qemumonitorjsontest.c qemuhotplugtest.c \
+   qemuagenttest.c \
$(QEMUMONITORTESTUTILS_SOURCES)
 endif

diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
new file mode 100644
index 000..7f22ff0
--- /dev/null
+++ b/tests/qemuagenttest.c
@@ -0,0 +1,163 @@
+/*
+ * 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
+ * http://www.gnu.org/licenses/.
+ *
+ */
+
+#include config.h
+
+#include testutils.h
+#include testutilsqemu.h
+#include qemumonitortestutils.h
+#include qemu/qemu_conf.h
+#include qemu/qemu_agent.h
+#include virthread.h
+#include virerror.h
+#include virstring.h
+
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+static int
+testQemuAgentFSFreeze(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt);
+int ret = -1;
+
+if (!test)
+return -1;
+
+if (qemuMonitorTestAddAgentSyncResponse(test)  0)
+goto cleanup;
+
+if (qemuMonitorTestAddItem(test, guest-fsfreeze-freeze,
+   { \return\ : 5 })  0)
+goto cleanup;
+
+if (qemuMonitorTestAddAgentSyncResponse(test)  0)
+goto cleanup;
+
+if (qemuMonitorTestAddItem(test, guest-fsfreeze-freeze,
+   { \return\ : 7 })  0)
+goto cleanup;
+
+if ((ret = qemuAgentFSFreeze(qemuMonitorTestGetAgent(test)))  0)
+goto cleanup;
+
+if (ret != 5) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   expected 5 frozen filesystems, got %d, ret);
+goto cleanup;
+}
+
+if ((ret = qemuAgentFSFreeze(qemuMonitorTestGetAgent(test)))  0)
+goto cleanup;
+
+if (ret != 7) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   expected 7 frozen filesystems, got %d, ret);
+goto cleanup;
+}
+
+ret = 0;
+
+cleanup:
+qemuMonitorTestFree(test);
+return ret;
+}
+
+
+static int
+testQemuAgentFSThaw(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt);
+int ret = -1;
+
+if (!test)
+return -1;
+
+if (qemuMonitorTestAddAgentSyncResponse(test)  0)
+goto cleanup;
+
+if (qemuMonitorTestAddItem(test, guest-fsfreeze-thaw,
+   { \return\ : 5 })  0)
+goto cleanup;
+
+if (qemuMonitorTestAddAgentSyncResponse(test)  0)
+goto cleanup;
+
+if (qemuMonitorTestAddItem(test, guest-fsfreeze-thaw,
+   { \return\ : 7 })  0)

[libvirt] [PATCH 14/20] qemumonitortestutils: Add the ability to check arguments of commands

2013-07-30 Thread Peter Krempa
This patch adds helpers that allow to check for argument values in
commands sent to the monitor.
---
 tests/qemumonitortestutils.c | 174 ---
 tests/qemumonitortestutils.h |   5 ++
 2 files changed, 170 insertions(+), 9 deletions(-)

diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index f421b9e..f383915 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -415,18 +415,47 @@ error:
 }


-struct qemuMonitorTestDefaultHandlerData {
-const char *command_name;
-const char *response;
+typedef struct _qemuMonitorTestCommandArgs qemuMonitorTestCommandArgs;
+typedef qemuMonitorTestCommandArgs *qemuMonitorTestCommandArgsPtr;
+struct _qemuMonitorTestCommandArgs {
+char *argname;
+char *argval;
 };


+struct qemuMonitorTestHandlerData {
+char *command_name;
+char *response;
+size_t nargs;
+qemuMonitorTestCommandArgsPtr args;
+};
+
+static void
+qemuMonitorTestHandlerDataFree(void *opaque)
+{
+struct qemuMonitorTestHandlerData *data = opaque;
+size_t i;
+
+if (!data)
+return;
+
+for (i = 0; i  data-nargs; i++) {
+VIR_FREE(data-args[i].argname);
+VIR_FREE(data-args[i].argval);
+}
+
+VIR_FREE(data-command_name);
+VIR_FREE(data-response);
+VIR_FREE(data-args);
+VIR_FREE(data);
+}
+
 static int
 qemuMonitorTestProcessCommandDefault(qemuMonitorTestPtr test,
  qemuMonitorTestItemPtr item,
  const char *cmdstr)
 {
-struct qemuMonitorTestDefaultHandlerData *data = item-opaque;
+struct qemuMonitorTestHandlerData *data = item-opaque;
 virJSONValuePtr val = NULL;
 char *cmdcopy = NULL;
 const char *cmdname;
@@ -473,18 +502,20 @@ qemuMonitorTestAddItem(qemuMonitorTestPtr test,
const char *command_name,
const char *response)
 {
-struct qemuMonitorTestDefaultHandlerData *data;
+struct qemuMonitorTestHandlerData *data;

 if (VIR_ALLOC(data)  0)
 return -1;

-data-command_name = command_name;
-data-response = response;
+if (VIR_STRDUP(data-command_name, command_name)  0 ||
+VIR_STRDUP(data-response, response)  0) {
+qemuMonitorTestHandlerDataFree(data);
+return -1;
+}

 return qemuMonitorTestAddHandler(test,
  qemuMonitorTestProcessCommandDefault,
- data,
- free);
+ data, qemuMonitorTestHandlerDataFree);
 }


@@ -551,6 +582,131 @@ qemuMonitorTestAddAgentSyncResponse(qemuMonitorTestPtr 
test)
 }


+static int
+qemuMonitorTestProcessCommandWithArgs(qemuMonitorTestPtr test,
+  qemuMonitorTestItemPtr item,
+  const char *cmdstr)
+{
+struct qemuMonitorTestHandlerData *data = item-opaque;
+virJSONValuePtr val = NULL;
+virJSONValuePtr args;
+virJSONValuePtr argobj;
+char *argstr = NULL;
+const char *cmdname;
+size_t i;
+int ret = -1;
+
+if (!(val = virJSONValueFromString(cmdstr)))
+return -1;
+
+if (!(cmdname = virJSONValueObjectGetString(val, execute))) {
+ret = qemuMonitorReportError(test, Missing command name in %s, 
cmdstr);
+goto cleanup;
+}
+
+if (STRNEQ(data-command_name, cmdname)) {
+ret = qemuMonitorTestAddUnexpectedErrorResponse(test);
+goto cleanup;
+}
+
+if (!(args = virJSONValueObjectGet(val, arguments))) {
+ret = qemuMonitorReportError(test,
+ Missing arguments section for command 
'%s',
+ data-command_name);
+goto cleanup;
+}
+
+/* validate the args */
+for (i = 0; i  data-nargs; i++) {
+qemuMonitorTestCommandArgsPtr arg = data-args[i];
+if (!(argobj = virJSONValueObjectGet(args, arg-argname))) {
+ret = qemuMonitorReportError(test,
+ Missing argument '%s' for command 
'%s',
+ arg-argname, data-command_name);
+goto cleanup;
+}
+
+/* convert the argument to string */
+if (!(argstr = virJSONValueToString(argobj, false)))
+goto cleanup;
+
+/* verify that the argument value is expected */
+if (STRNEQ(argstr, arg-argval)) {
+ret = qemuMonitorReportError(test,
+ Invalid value of argument '%s' 
+ of command '%s': 
+ expected '%s' got '%s',
+ arg-argname, data-command_name,
+ arg-argval, argstr);
+goto cleanup;
+}
+
+VIR_FREE(argstr);
+}
+

[libvirt] [PATCH 19/20] qemuagenttest: Test arbitrary qemu commands and timeouting of commands

2013-07-30 Thread Peter Krempa
Test arbitrary qemu commands and timeouting of the guest agent
synchronisation.
---
 tests/qemuagenttest.c | 101 ++
 1 file changed, 101 insertions(+)

diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
index e314bb0..81c24f2 100644
--- a/tests/qemuagenttest.c
+++ b/tests/qemuagenttest.c
@@ -326,6 +326,105 @@ cleanup:


 static int
+qemuAgentTimeoutTestMonitorHandler(qemuMonitorTestPtr test ATTRIBUTE_UNUSED,
+   qemuMonitorTestItemPtr item 
ATTRIBUTE_UNUSED,
+   const char *cmdstr ATTRIBUTE_UNUSED)
+{
+return 0;
+}
+
+static const char testQemuAgentArbitraryCommandResponse[] =
+{\return\:\bla\};
+
+static int
+testQemuAgentArbitraryCommand(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt);
+int ret = -1;
+char *reply = NULL;
+
+if (!test)
+return -1;
+
+if (qemuMonitorTestAddAgentSyncResponse(test)  0)
+goto cleanup;
+
+if (qemuMonitorTestAddItem(test, ble,
+   testQemuAgentArbitraryCommandResponse)  0)
+goto cleanup;
+
+if (qemuAgentArbitraryCommand(qemuMonitorTestGetAgent(test),
+  {\execute\:\ble\},
+  reply,
+  VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK)  0)
+goto cleanup;
+
+if (STRNEQ(reply, testQemuAgentArbitraryCommandResponse)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   invalid processing of guest agent reply: 
+   got '%s' expected '%s',
+   reply, testQemuAgentArbitraryCommandResponse);
+goto cleanup;
+}
+
+VIR_FREE(reply);
+
+/* test timeout */
+if (qemuMonitorTestAddAgentSyncResponse(test)  0)
+goto cleanup;
+
+if (qemuMonitorTestAddHandler(test, qemuAgentTimeoutTestMonitorHandler,
+  NULL, NULL)  0)
+goto cleanup;
+
+if (qemuAgentArbitraryCommand(qemuMonitorTestGetAgent(test),
+  {\execute\:\ble\},
+  reply,
+  1) != -2) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   agent command didn't time out);
+goto cleanup;
+}
+
+ret = 0;
+
+cleanup:
+VIR_FREE(reply);
+qemuMonitorTestFree(test);
+return ret;
+}
+
+
+static int
+testQemuAgentTimeout(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt);
+int ret = -1;
+
+if (!test)
+return -1;
+
+if (qemuMonitorTestAddHandler(test, qemuAgentTimeoutTestMonitorHandler,
+  NULL, NULL)  0)
+goto cleanup;
+
+if (qemuAgentFSFreeze(qemuMonitorTestGetAgent(test)) != -1) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   agent command should have failed);
+goto cleanup;
+}
+
+ret = 0;
+
+cleanup:
+qemuMonitorTestFree(test);
+return ret;
+}
+
+
+static int
 mymain(void)
 {
 int ret = 0;
@@ -351,6 +450,8 @@ mymain(void)
 DO_TEST(FSTrim);
 DO_TEST(Suspend);
 DO_TEST(Shutdown);
+DO_TEST(ArbitraryCommand);
+DO_TEST(Timeout);

 virObjectUnref(xmlopt);

-- 
1.8.3.2

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


[libvirt] [PATCH 11/20] qemumonitortestutils: Split lines on \n instead of \r\n

2013-07-30 Thread Peter Krempa
The normal monitor uses windows line endings, where the agent monitor
uses only newlines. Change this to tolerate both approaches and allow to
use the utilities for guest agent tests.
---
 tests/qemumonitortestutils.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index 941dfea..6dc430e 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -207,7 +207,7 @@ qemuMonitorTestIO(virNetSocketPtr sock,
  * if so, handle that command
  */
 t1 = test-incoming;
-while ((t2 = strstr(t1, \r\n))) {
+while ((t2 = strstr(t1, \n))) {
 *t2 = '\0';

 if (qemuMonitorTestProcessCommand(test, t1)  0) {
@@ -215,7 +215,7 @@ qemuMonitorTestIO(virNetSocketPtr sock,
 goto cleanup;
 }

-t1 = t2 + 2;
+t1 = t2 + 1;
 }
 used = t1 - test-incoming;
 memmove(test-incoming, t1, test-incomingLength - used);
-- 
1.8.3.2

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


[libvirt] [PATCH 18/20] qemuagenttest: Introduce testing of shutdown commands

2013-07-30 Thread Peter Krempa
This patch exports a few utility functions and adds testing of shutdown
commands of the guest agent.
---
 tests/qemuagenttest.c| 119 +++
 tests/qemumonitortestutils.c |  21 
 tests/qemumonitortestutils.h |  28 --
 3 files changed, 153 insertions(+), 15 deletions(-)

diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
index d0b450e..e314bb0 100644
--- a/tests/qemuagenttest.c
+++ b/tests/qemuagenttest.c
@@ -207,6 +207,124 @@ cleanup:
 }


+struct qemuAgentShutdownTestData {
+const char *mode;
+qemuAgentEvent event;
+};
+
+
+static int
+qemuAgentShutdownTestMonitorHandler(qemuMonitorTestPtr test,
+qemuMonitorTestItemPtr item,
+const char *cmdstr)
+{
+struct qemuAgentShutdownTestData *data;
+virJSONValuePtr val = NULL;
+virJSONValuePtr args;
+const char *cmdname;
+const char *mode;
+int ret = -1;
+
+data = qemuMonitorTestItemGetPrivateData(item);
+
+if (!(val = virJSONValueFromString(cmdstr)))
+return -1;
+
+if (!(cmdname = virJSONValueObjectGetString(val, execute))) {
+ret = qemuMonitorReportError(test, Missing command name in %s, 
cmdstr);
+goto cleanup;
+}
+
+if (STRNEQ(cmdname, guest-shutdown)) {
+ret = qemuMonitorTestAddUnexpectedErrorResponse(test);
+goto cleanup;
+}
+
+if (!(args = virJSONValueObjectGet(val, arguments))) {
+ret = qemuMonitorReportError(test,
+ Missing arguments section);
+goto cleanup;
+}
+
+if (!(mode = virJSONValueObjectGetString(args, mode))) {
+ret = qemuMonitorReportError(test, Missing shutdown mode);
+goto cleanup;
+}
+
+/* now don't reply but return a qemu agent event */
+qemuAgentNotifyEvent(qemuMonitorTestGetAgent(test),
+ data-event);
+
+ret = 0;
+
+cleanup:
+virJSONValueFree(val);
+return ret;
+
+}
+
+
+static int
+testQemuAgentShutdown(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt);
+struct qemuAgentShutdownTestData priv;
+int ret = -1;
+
+if (!test)
+return -1;
+
+if (qemuMonitorTestAddAgentSyncResponse(test)  0)
+goto cleanup;
+
+priv.event = QEMU_AGENT_EVENT_SHUTDOWN;
+priv.mode = shutdown;
+
+if (qemuMonitorTestAddHandler(test, qemuAgentShutdownTestMonitorHandler,
+  priv, NULL)  0)
+goto cleanup;
+
+if (qemuAgentShutdown(qemuMonitorTestGetAgent(test),
+  QEMU_AGENT_SHUTDOWN_HALT)  0)
+goto cleanup;
+
+if (qemuMonitorTestAddAgentSyncResponse(test)  0)
+goto cleanup;
+
+priv.event = QEMU_AGENT_EVENT_SHUTDOWN;
+priv.mode = powerdown;
+
+if (qemuMonitorTestAddHandler(test, qemuAgentShutdownTestMonitorHandler,
+  priv, NULL)  0)
+goto cleanup;
+
+if (qemuAgentShutdown(qemuMonitorTestGetAgent(test),
+  QEMU_AGENT_SHUTDOWN_POWERDOWN)  0)
+goto cleanup;
+
+if (qemuMonitorTestAddAgentSyncResponse(test)  0)
+goto cleanup;
+
+priv.event = QEMU_AGENT_EVENT_RESET;
+priv.mode = reboot;
+
+if (qemuMonitorTestAddHandler(test, qemuAgentShutdownTestMonitorHandler,
+  priv, NULL)  0)
+goto cleanup;
+
+if (qemuAgentShutdown(qemuMonitorTestGetAgent(test),
+  QEMU_AGENT_SHUTDOWN_REBOOT)  0)
+goto cleanup;
+
+ret = 0;
+
+cleanup:
+qemuMonitorTestFree(test);
+return ret;
+}
+
+
 static int
 mymain(void)
 {
@@ -232,6 +350,7 @@ mymain(void)
 DO_TEST(FSThaw);
 DO_TEST(FSTrim);
 DO_TEST(Suspend);
+DO_TEST(Shutdown);

 virObjectUnref(xmlopt);

diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index f383915..32217a3 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -37,12 +37,6 @@

 #define VIR_FROM_THIS VIR_FROM_NONE

-typedef struct _qemuMonitorTestItem qemuMonitorTestItem;
-typedef qemuMonitorTestItem *qemuMonitorTestItemPtr;
-typedef int (*qemuMonitorTestResponseCallback)(qemuMonitorTestPtr test,
-   qemuMonitorTestItemPtr item,
-   const char *message);
-
 struct _qemuMonitorTestItem {
 qemuMonitorTestResponseCallback cb;
 void *opaque;
@@ -96,7 +90,7 @@ qemuMonitorTestItemFree(qemuMonitorTestItemPtr item)
 /*
  * Appends data for a reply to the outgoing buffer
  */
-static int
+int
 qemuMonitorTestAddReponse(qemuMonitorTestPtr test,
   const char *response)
 {
@@ -119,7 +113,7 @@ qemuMonitorTestAddReponse(qemuMonitorTestPtr test,
 }


-static int
+int
 qemuMonitorTestAddUnexpectedErrorResponse(qemuMonitorTestPtr test)

Re: [libvirt] [PATCH] Fix minor typos in messages and docs

2013-07-30 Thread Eric Blake
On 07/30/2013 02:24 AM, Yuri Chornoivan wrote:
 
 0001-Fix-minor-typos-in-messages-and-docs.patch
 
 
From 1b8af1b915f3cce1d27576f651e99b6ce3a73e3a Mon Sep 17 00:00:00 2001
 From: Yuri Chornoivan yurc...@ukr.net
 Date: Tue, 30 Jul 2013 11:21:11 +0300
 Subject: [PATCH] Fix minor typos in messages and docs
 
 ---
  docs/apibuild.py  |  2 +-
  docs/formatdomain.html.in |  6 ++---
  docs/hacking.html.in  |  2 +-
  docs/migration.html.in|  4 ++--
  docs/news.html.in | 60 
 +++
  src/esx/esx_vi.c  |  2 +-
  src/lxc/lxc_driver.c  |  4 ++--
  7 files changed, 40 insertions(+), 40 deletions(-)
 

ACK and pushed.  (Too bad this missed the release; some of the typos are
in error messages now baked into .po files.)

 +++ b/docs/news.html.in
 @@ -460,7 +460,7 @@ and check the a 
 href=http://libvirt.org/git/?p=libvirt.git;a=log;GIT log/a
Add libvirt-daemon-vbox amp; libvirt-daemon-driver-vbox RPMs (Daniel 
 P. Berrange),br/
Include GNULIB mkdtemp module (Daniel P. Berrange),br/
Set PKG_CONFIG_LIBDIR in autobuild.sh (Daniel P. Berrange),br/
 -  qemu: report useful error failling to destroy domain gracefully 
 (Guannan Ren),br/
 +  qemu: report useful error failing to destroy domain gracefully 
 (Guannan Ren),br/

These typos are extracted from 'git short-log' history; I'm hoping that
they don't get reinstated when DV does releases.

-- 
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 12/20] qemumonitortestutils: Add instrumentation for guest agent testing

2013-07-30 Thread Peter Krempa
Add helper functions to open guest agent connections and a handler for
replying to the guest-sync command.
---
 tests/qemumonitortestutils.c | 133 ++-
 tests/qemumonitortestutils.h |   6 ++
 2 files changed, 137 insertions(+), 2 deletions(-)

diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index 6dc430e..96cb054 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -28,6 +28,7 @@

 #include virthread.h
 #include qemu/qemu_monitor.h
+#include qemu/qemu_agent.h
 #include rpc/virnetsocket.h
 #include viralloc.h
 #include virlog.h
@@ -68,6 +69,7 @@ struct _qemuMonitorTest {
 virNetSocketPtr client;

 qemuMonitorPtr mon;
+qemuAgentPtr agent;

 char *tmpdir;

@@ -118,7 +120,7 @@ qemuMonitorTestAddReponse(qemuMonitorTestPtr test,
 static int
 qemuMonitorTestAddUnexpectedErrorResponse(qemuMonitorTestPtr test)
 {
-if (test-json) {
+if (test-agent || test-json) {
 return qemuMonitorTestAddReponse(test,
  { \error\: 
   { \desc\: \Unexpected command\, 

@@ -312,6 +314,11 @@ qemuMonitorTestFree(qemuMonitorTestPtr test)
 qemuMonitorClose(test-mon);
 }

+if (test-agent) {
+virObjectUnlock(test-agent);
+qemuAgentClose(test-agent);
+}
+
 virObjectUnref(test-vm);

 if (test-running)
@@ -387,7 +394,7 @@ qemuMonitorTestProcessCommandDefault(qemuMonitorTestPtr 
test,
 char *tmp;
 int ret = -1;

-if (test-json) {
+if (test-agent || test-json) {
 if (!(val = virJSONValueFromString(cmdstr)))
 return -1;

@@ -442,6 +449,72 @@ qemuMonitorTestAddItem(qemuMonitorTestPtr test,
 }


+static int
+qemuMonitorTestProcessGuestAgentSync(qemuMonitorTestPtr test,
+ qemuMonitorTestItemPtr item 
ATTRIBUTE_UNUSED,
+ const char *cmdstr)
+{
+virJSONValuePtr val = NULL;
+virJSONValuePtr args;
+unsigned long long id;
+const char *cmdname;
+char *retmsg = NULL;
+int ret = -1;
+
+if (!(val = virJSONValueFromString(cmdstr)))
+return -1;
+
+if (!(cmdname = virJSONValueObjectGetString(val, execute))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   Missing guest-sync command name);
+goto cleanup;
+}
+
+if (STRNEQ(cmdname, guest-sync)) {
+ret = qemuMonitorTestAddUnexpectedErrorResponse(test);
+goto cleanup;
+}
+
+if (!(args = virJSONValueObjectGet(val, arguments))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   Missing arguments for guest-sync);
+goto cleanup;
+}
+
+if (virJSONValueObjectGetNumberUlong(args, id, id)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   Missing id for guest sync);
+goto cleanup;
+}
+
+if (virAsprintf(retmsg, {\return\:%llu}, id)  0)
+goto cleanup;
+
+
+ret = qemuMonitorTestAddReponse(test, retmsg);
+
+cleanup:
+virJSONValueFree(val);
+VIR_FREE(retmsg);
+return ret;
+}
+
+
+int
+qemuMonitorTestAddAgentSyncResponse(qemuMonitorTestPtr test)
+{
+if (!test-agent) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   This test is not an agent test);
+return -1;
+}
+
+return qemuMonitorTestAddHandler(test,
+ qemuMonitorTestProcessGuestAgentSync,
+ NULL, NULL);
+}
+
+
 static void
 qemuMonitorTestEOFNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
  virDomainObjPtr vm ATTRIBUTE_UNUSED)
@@ -462,6 +535,24 @@ static qemuMonitorCallbacks qemuMonitorTestCallbacks = {
 };


+static void
+qemuMonitorTestAgentEOFNotify(qemuAgentPtr agent ATTRIBUTE_UNUSED,
+  virDomainObjPtr vm ATTRIBUTE_UNUSED)
+{
+}
+
+static void
+qemuMonitorTestAgentErrorNotify(qemuAgentPtr agent ATTRIBUTE_UNUSED,
+virDomainObjPtr vm ATTRIBUTE_UNUSED)
+{
+}
+
+static qemuAgentCallbacks qemuMonitorTestAgentCallbacks = {
+.eofNotify = qemuMonitorTestAgentEOFNotify,
+.errorNotify = qemuMonitorTestAgentErrorNotify,
+};
+
+
 static qemuMonitorTestPtr
 qemuMonitorCommonTestNew(virDomainXMLOptionPtr xmlopt,
  virDomainChrSourceDefPtr src)
@@ -594,12 +685,50 @@ qemuMonitorTestNew(bool json, virDomainXMLOptionPtr 
xmlopt)
 return test;

 error:
+virDomainChrSourceDefClear(src);
+qemuMonitorTestFree(test);
+return NULL;
+}
+
+qemuMonitorTestPtr
+qemuMonitorTestNewAgent(virDomainXMLOptionPtr xmlopt)
+{
+qemuMonitorTestPtr test = NULL;
+virDomainChrSourceDef src;
+
+if (!(test = qemuMonitorCommonTestNew(xmlopt, src)))
+goto error;
+
+if (!(test-agent = qemuAgentOpen(test-vm,
+  src,
+  

[libvirt] [PATCH 20/20] qemuagenttest: Add tests for CPU plug functions and helpers

2013-07-30 Thread Peter Krempa
---
 tests/qemuagenttest.c | 119 ++
 1 file changed, 119 insertions(+)

diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
index 81c24f2..33a6d73 100644
--- a/tests/qemuagenttest.c
+++ b/tests/qemuagenttest.c
@@ -423,6 +423,124 @@ cleanup:
 return ret;
 }

+static const char testQemuAgentCPUResponse[] =
+{\return\: 
+   [
+   {\online\: true,
+\can-offline\: false,
+\logical-id\: 0
+   },
+   {\online\: true,
+\can-offline\: true,
+\logical-id\: 1
+   },
+   {\online\: true,
+\can-offline\: true,
+\logical-id\: 2
+},
+   {\online\: false,
+\can-offline\: true,
+\logical-id\: 3
+   }
+   ]
+};
+
+static const char testQemuAgentCPUArguments1[] =
+[{\logical-id\:0,\online\:true},
+ {\logical-id\:1,\online\:false},
+ {\logical-id\:2,\online\:true},
+ {\logical-id\:3,\online\:false}];
+
+static const char testQemuAgentCPUArguments2[] =
+[{\logical-id\:0,\online\:true},
+ {\logical-id\:1,\online\:true},
+ {\logical-id\:2,\online\:true},
+ {\logical-id\:3,\online\:true}];
+
+static int
+testQemuAgentCPU(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt);
+qemuAgentCPUInfoPtr cpuinfo = NULL;
+int nvcpus;
+int ret = -1;
+
+if (!test)
+return -1;
+
+if (qemuMonitorTestAddAgentSyncResponse(test)  0)
+goto cleanup;
+
+if (qemuMonitorTestAddItem(test, guest-get-vcpus,
+   testQemuAgentCPUResponse)  0)
+goto cleanup;
+
+/* get cpus */
+if ((nvcpus = qemuAgentGetVCPUs(qemuMonitorTestGetAgent(test),
+cpuinfo))  0)
+goto cleanup;
+
+if (nvcpus != 4) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   Expected '4' cpus, got '%d', nvcpus);
+goto cleanup;
+}
+
+/* try to unplug one */
+if (qemuAgentUpdateCPUInfo(2, cpuinfo, nvcpus)  0)
+goto cleanup;
+
+if (qemuMonitorTestAddAgentSyncResponse(test)  0)
+goto cleanup;
+
+if (qemuMonitorTestAddItemParams(test, guest-set-vcpus,
+ { \return\ : 4 },
+ vcpus, testQemuAgentCPUArguments1,
+ NULL)  0)
+goto cleanup;
+
+if ((nvcpus = qemuAgentSetVCPUs(qemuMonitorTestGetAgent(test),
+cpuinfo, nvcpus))  0)
+goto cleanup;
+
+if (nvcpus != 4) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   Expected '4' cpus updated , got '%d', nvcpus);
+goto cleanup;
+}
+
+/* try to hotplug two */
+if (qemuMonitorTestAddAgentSyncResponse(test)  0)
+goto cleanup;
+
+if (qemuMonitorTestAddItemParams(test, guest-set-vcpus,
+ { \return\ : 4 },
+ vcpus, testQemuAgentCPUArguments2,
+ NULL)  0)
+goto cleanup;
+
+if (qemuAgentUpdateCPUInfo(4, cpuinfo, nvcpus)  0)
+goto cleanup;
+
+if ((nvcpus = qemuAgentSetVCPUs(qemuMonitorTestGetAgent(test),
+cpuinfo, nvcpus))  0)
+goto cleanup;
+
+if (nvcpus != 4) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   Expected '4' cpus updated , got '%d', nvcpus);
+goto cleanup;
+}
+
+ret = 0;
+
+cleanup:
+VIR_FREE(cpuinfo);
+qemuMonitorTestFree(test);
+return ret;
+}
+

 static int
 mymain(void)
@@ -452,6 +570,7 @@ mymain(void)
 DO_TEST(Shutdown);
 DO_TEST(ArbitraryCommand);
 DO_TEST(Timeout);
+DO_TEST(CPU);

 virObjectUnref(xmlopt);

-- 
1.8.3.2

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


[libvirt] [PATCH 17/20] qemuagenttest: Add testing of agent suspend modes

2013-07-30 Thread Peter Krempa
---
 tests/qemuagenttest.c | 47 +++
 1 file changed, 47 insertions(+)

diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
index a3b8834..d0b450e 100644
--- a/tests/qemuagenttest.c
+++ b/tests/qemuagenttest.c
@@ -162,6 +162,52 @@ cleanup:


 static int
+testQemuAgentSuspend(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt);
+int ret = -1;
+size_t i;
+
+if (!test)
+return -1;
+
+if (qemuMonitorTestAddAgentSyncResponse(test)  0)
+goto cleanup;
+
+if (qemuMonitorTestAddItem(test, guest-suspend-ram,
+   { \return\ : {} })  0)
+goto cleanup;
+
+if (qemuMonitorTestAddAgentSyncResponse(test)  0)
+goto cleanup;
+
+if (qemuMonitorTestAddItem(test, guest-suspend-disk,
+   { \return\ : {} })  0)
+goto cleanup;
+
+if (qemuMonitorTestAddAgentSyncResponse(test)  0)
+goto cleanup;
+
+if (qemuMonitorTestAddItem(test, guest-suspend-hybrid,
+   { \return\ : {} })  0)
+goto cleanup;
+
+/* try the commands - fail if ordering changes */
+for (i = 0; i  VIR_NODE_SUSPEND_TARGET_LAST; i++) {
+if (qemuAgentSuspend(qemuMonitorTestGetAgent(test), i)  0)
+goto cleanup;
+}
+
+ret = 0;
+
+cleanup:
+qemuMonitorTestFree(test);
+return ret;
+}
+
+
+static int
 mymain(void)
 {
 int ret = 0;
@@ -185,6 +231,7 @@ mymain(void)
 DO_TEST(FSFreeze);
 DO_TEST(FSThaw);
 DO_TEST(FSTrim);
+DO_TEST(Suspend);

 virObjectUnref(xmlopt);

-- 
1.8.3.2

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


Re: [libvirt] [PATCH] Support apparmor in RPM spec

2013-07-30 Thread Daniel P. Berrange
On Tue, Jul 30, 2013 at 06:59:16AM -0600, Eric Blake wrote:
 On 07/30/2013 05:06 AM, Daniel P. Berrange wrote:
  From: Daniel P. Berrange berra...@redhat.com
  
  If libapparmor-devel happens to be installed when building the
  RPM, it will failed due to unlisted virt-aa-helper in %files.
  Add support for apparmor in the spec, so that we can explicitly
  turn it on/off, defaulting to off in all distros. This causes
  --without-apparmor to be given to configure, preventing the
  build failures if the user happens to have libapparmor-devel
  present.
 
 Makes sense - we want the spec file to be absolute regardless of the
 dev's environment.
 
 Question - is there a repo somewhere with libapparmor-devel built for
 Fedora, or does it remain something where I'm best off testing on a
 Debian-based build?

I took the SUSE libapparmor  source RPM and hacked it until
it would build on Fedora. Of course you can't actually
use it due to missing kernel support in Fedora, but it was
good enough to compile libvirt against.

Unfortunately I lost the hacked src.rpm I had when one of my
dev machines suffered fielsystem data loss :-(

It wasn't all that hard to hack the suse RPM though, so ought
to be fairly easy to recreate...

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 01/20] conf: Export virDomainChrSourceDefClear()

2013-07-30 Thread Eric Blake
On 07/30/2013 07:05 AM, Peter Krempa wrote:
 ---
  src/conf/domain_conf.c   | 2 +-
  src/conf/domain_conf.h   | 2 ++
  src/libvirt_private.syms | 1 +
  3 files changed, 4 insertions(+), 1 deletion(-)

ACK.

-- 
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 02/20] qemu_agent: Output newline at the end of the sync JSON message

2013-07-30 Thread Eric Blake
On 07/30/2013 07:05 AM, Peter Krempa wrote:
 Although this isn't apparently needed for the guest agent itself, the
 test I will be adding later depends on the newline as a separator of
 messages to process.

Agreed - absence of whitespace like newlines is inconsequential in
strict JSON, but using it to simplify tests is worthwhile; it also makes
it easier to see what is sent across a line when tracing debug messages.

 ---
  src/qemu/qemu_agent.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

ACK.

 
 diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
 index 72bf211..1607e88 100644
 --- a/src/qemu/qemu_agent.c
 +++ b/src/qemu/qemu_agent.c
 @@ -918,7 +918,7 @@ qemuAgentGuestSync(qemuAgentPtr mon)
 
  if (virAsprintf(sync_msg.txBuffer,
  {\execute\:\guest-sync\, 
 -\arguments\:{\id\:%llu}}, id)  0)
 +\arguments\:{\id\:%llu}}\n, id)  0)
  return -1;
 
  sync_msg.txLength = strlen(sync_msg.txBuffer);
 

-- 
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 03/20] qemu_agent: Move updater function for VCPU hotplug into qemu_agent.c

2013-07-30 Thread Eric Blake
On 07/30/2013 07:05 AM, Peter Krempa wrote:
 To allow testing of the cpu updater function, this function needs to be
 available separately. Export it from qemu_agent.c where it should
 belong.
 ---
  src/qemu/qemu_agent.c  | 63 +
  src/qemu/qemu_agent.h  |  3 +++
  src/qemu/qemu_driver.c | 64 
 +-
  3 files changed, 67 insertions(+), 63 deletions(-)

ACK.

-- 
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] Configuring systemd to restart libvirt on abort

2013-07-30 Thread Dan Kenigsberg
On Tue, Jul 30, 2013 at 05:43:16AM -0400, Mooli Tayer wrote:
 
 - Original Message -
  On Mon, Jul 29, 2013 at 05:36:21PM +0300, Mooli Tayer wrote:
   From: Mooli Tayer mta...@redhat.com
   
   This will create a respawn behaviour in case libvirt
   process exits due to an uncaught signal not specified
   as a clean exit status.
   see http://www.freedesktop.org/software/systemd/man/systemd.service.html
   ---
daemon/libvirtd.service.in | 1 +
1 file changed, 1 insertion(+)
   
   diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in
   index aa5913b..b3c0849 100644
   --- a/daemon/libvirtd.service.in
   +++ b/daemon/libvirtd.service.in
   @@ -15,6 +15,7 @@ EnvironmentFile=-/etc/sysconfig/libvirtd
ExecStart=@sbindir@/libvirtd $LIBVIRTD_ARGS
ExecReload=/bin/kill -HUP $MAINPID
KillMode=process
   +Restart=on-abort
# Override the maximum number of opened files
#LimitNOFILE=2048
  
  I'm wondering whether 'on-abort' is the best choice or if
  'on-failure' or 'always' are better. The systemd.service
  man page says
  
  [quote]
 Takes one of no, on-success, on-failure,
 on-abort, or always. If set to no (the
 default) the service will not be restarted. If
 set to on-success it will be restarted only
 when the service process exits cleanly. In
 this context, a clean exit means an exit code
 of 0, or one of the signals SIGHUP, SIGINT,
 SIGTERM, or SIGPIPE, and additionally, exit
 statuses and signals specified in
 SuccessExitStatus=. If set to on-failure the
 service will be restarted when the process
 exits with an nonzero exit code, is terminated
 by a signal (including on core dump), when an
 operation (such as service reload) times out,
 and when the configured watchdog timeout is
 triggered. If set to on-abort the service will
 be restarted only if the service process exits
 due to an uncaught signal not specified as a
 clean exit status. If set to always the
 service will be restarted regardless whether
 it exited cleanly or not, got terminated
 abnormally by a signal or hit a timeout.
  [/quote]
  
  I tend towards saying 'on-failure' here.
 
 I agree. we defiantly want restart in the 'on-failure'
 cases.

Would 'on-failure' mean that libvirtd is to be restarted if it has
exited with an error due to wrong configuration? This may spell nasty
thrashing of systemd.

Dan.

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


Re: [libvirt] [PATCH 04/20] qemu_agent: Remove obvious comments

2013-07-30 Thread Eric Blake
On 07/30/2013 07:05 AM, Peter Krempa wrote:
 Most APIs in libvirt report errors, thus no need to state that
 explictly.

s/explictly/explicitly/

ACK.

-- 
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 05/20] qemumonitortestutils: Use consistent header style and line spacing

2013-07-30 Thread Eric Blake
On 07/30/2013 07:05 AM, Peter Krempa wrote:
 ---
  tests/qemumonitortestutils.c | 62 
 +---
  1 file changed, 41 insertions(+), 21 deletions(-)

Thanks for separating cosmetic from functional changes.

ACK.

 @@ -440,12 +456,14 @@ static qemuMonitorCallbacks qemuCallbacks = {
  .errorNotify = qemuMonitorTestErrorNotify,
  };
 
 +
  #define QEMU_JSON_GREETING {\QMP\: {\version\: {\qemu\: {\micro\: 
 1, \minor\: 0, \major\: 1}, \package\: \ (qemu-kvm-1.0.1)\}, 
 \capabilities\: []}}

Might be worth splitting this macro long line with \-newline, and
relying on C's implicit concatenation of strings separated only by
whitespace.

-- 
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] Configuring systemd to restart libvirt on abort

2013-07-30 Thread Daniel P. Berrange
On Tue, Jul 30, 2013 at 04:27:37PM +0300, Dan Kenigsberg wrote:
 On Tue, Jul 30, 2013 at 05:43:16AM -0400, Mooli Tayer wrote:
  
  - Original Message -
   On Mon, Jul 29, 2013 at 05:36:21PM +0300, Mooli Tayer wrote:
From: Mooli Tayer mta...@redhat.com

This will create a respawn behaviour in case libvirt
process exits due to an uncaught signal not specified
as a clean exit status.
see http://www.freedesktop.org/software/systemd/man/systemd.service.html
---
 daemon/libvirtd.service.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in
index aa5913b..b3c0849 100644
--- a/daemon/libvirtd.service.in
+++ b/daemon/libvirtd.service.in
@@ -15,6 +15,7 @@ EnvironmentFile=-/etc/sysconfig/libvirtd
 ExecStart=@sbindir@/libvirtd $LIBVIRTD_ARGS
 ExecReload=/bin/kill -HUP $MAINPID
 KillMode=process
+Restart=on-abort
 # Override the maximum number of opened files
 #LimitNOFILE=2048
   
   I'm wondering whether 'on-abort' is the best choice or if
   'on-failure' or 'always' are better. The systemd.service
   man page says
   
   [quote]
  Takes one of no, on-success, on-failure,
  on-abort, or always. If set to no (the
  default) the service will not be restarted. If
  set to on-success it will be restarted only
  when the service process exits cleanly. In
  this context, a clean exit means an exit code
  of 0, or one of the signals SIGHUP, SIGINT,
  SIGTERM, or SIGPIPE, and additionally, exit
  statuses and signals specified in
  SuccessExitStatus=. If set to on-failure the
  service will be restarted when the process
  exits with an nonzero exit code, is terminated
  by a signal (including on core dump), when an
  operation (such as service reload) times out,
  and when the configured watchdog timeout is
  triggered. If set to on-abort the service will
  be restarted only if the service process exits
  due to an uncaught signal not specified as a
  clean exit status. If set to always the
  service will be restarted regardless whether
  it exited cleanly or not, got terminated
  abnormally by a signal or hit a timeout.
   [/quote]
   
   I tend towards saying 'on-failure' here.
  
  I agree. we defiantly want restart in the 'on-failure'
  cases.
 
 Would 'on-failure' mean that libvirtd is to be restarted if it has
 exited with an error due to wrong configuration? This may spell nasty
 thrashing of systemd.

You could get thrashing no matter what setting you use here - eg it
could be core dumping during startup which would hit with on-abort.

It doesn't matter though because systemd has rate limiting. By default
services which are started more often than 5 times within 10s are not
permitted to start any more times until the 10s interval ends.

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 06/20] qemumonitortestutils: Use VIR_DELETE_ELEMENT and VIR_APPEND_ELEMENT

2013-07-30 Thread Eric Blake
On 07/30/2013 07:05 AM, Peter Krempa wrote:
 Simplify the code using the existing helpers instead of open coding the
 same functionality.
 ---
  tests/qemumonitortestutils.c | 32 +---
  1 file changed, 9 insertions(+), 23 deletions(-)

ACK.

-- 
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 07/20] qemumonitortestutils: remove multiline function calls

2013-07-30 Thread Eric Blake
On 07/30/2013 07:05 AM, Peter Krempa wrote:
 ---
  tests/qemumonitortestutils.c | 13 +++--
  1 file changed, 3 insertions(+), 10 deletions(-)

ACK.

-- 
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 08/20] qemumonitortestutils: Don't crash on non fully initialized test

2013-07-30 Thread Eric Blake
On 07/30/2013 07:05 AM, Peter Krempa wrote:
 The qemumonitorjsontest crashed when one of the initialization steps
 done before starting the worker thread failed. This patch fixes this by
 trying to pthread_join() the thread only after it was created.
 ---
  tests/qemumonitortestutils.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

ACK.

 
 diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
 index 34ca1ae..5ca569f 100644
 --- a/tests/qemumonitortestutils.c
 +++ b/tests/qemumonitortestutils.c
 @@ -368,7 +368,8 @@ qemuMonitorTestFree(qemuMonitorTestPtr test)
 
  virObjectUnref(test-vm);
 
 -virThreadJoin(test-thread);
 +if (test-running)
 +virThreadJoin(test-thread);
 
  if (timer != -1)
  virEventRemoveTimeout(timer);
 

-- 
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 09/20] qemumonitortestutils: Split up creation of the test to allow reuse

2013-07-30 Thread Eric Blake
On 07/30/2013 07:05 AM, Peter Krempa wrote:
 The instrumentation for the monitor test can be hacked for qemu agent
 testing. Split out the monitor specific stuff to allow using the code in
 guest agent tests in the future.
 ---
  tests/qemumonitortestutils.c | 96 
 ++--
  1 file changed, 66 insertions(+), 30 deletions(-)
 
 diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
 index 5ca569f..1785293 100644
 --- a/tests/qemumonitortestutils.c
 +++ b/tests/qemumonitortestutils.c
 @@ -440,16 +440,11 @@ static qemuMonitorCallbacks qemuCallbacks = {
  };
 
 
 -#define QEMU_JSON_GREETING {\QMP\: {\version\: {\qemu\: {\micro\: 
 1, \minor\: 0, \major\: 1}, \package\: \ (qemu-kvm-1.0.1)\}, 
 \capabilities\: []}}

Ah, as long as you are refactoring here, you don't have to tweak the
earlier patch in the series where I suggested breaking the long line.

 
 +#define QEMU_JSON_GREETING {\QMP\: {\version\: {\qemu\: {\micro\: 
 1, \minor\: 0, \major\: 1}, \package\: \ (qemu-kvm-1.0.1)\}, 
 \capabilities\: []}}

But here, you SHOULD break up the long line :)

ACK with that fixed.

-- 
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] spec: Use --enable-werror on RHEL

2013-07-30 Thread Jiri Denemark
As RHEL provides a stable tool chain, we don't have to worry about
frequent changes in reported compiler warnings (which prevents us from
enabling -Werror unconditionally).
---
 libvirt.spec.in | 9 +
 1 file changed, 9 insertions(+)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index e74f774..6d6203b 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -352,6 +352,14 @@
 %endif
 
 
+# RHEL releases provide stable tool chains and so it is safe to turn
+# compiler warning into errors without being worried about frequent
+# changes in reported warnings
+%if 0%{?rhel}
+%define enable_werror --enable-werror
+%endif
+
+
 Summary: Library providing a simple virtualization API
 Name: libvirt
 Version: @VERSION@
@@ -1386,6 +1394,7 @@ of recent versions of Linux (and other OSes).
%{with_packager_version} \
--with-qemu-user=%{qemu_user} \
--with-qemu-group=%{qemu_group} \
+   %{?enable_werror} \
%{init_scripts}
 make %{?_smp_mflags}
 gzip -9 ChangeLog
-- 
1.8.3.2

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


Re: [libvirt] [PATCH] Configuring systemd to restart libvirt on abort

2013-07-30 Thread Mooli Tayer


- Original Message -
 On Tue, Jul 30, 2013 at 04:27:37PM +0300, Dan Kenigsberg wrote:
  On Tue, Jul 30, 2013 at 05:43:16AM -0400, Mooli Tayer wrote:
   
   - Original Message -
On Mon, Jul 29, 2013 at 05:36:21PM +0300, Mooli Tayer wrote:
 From: Mooli Tayer mta...@redhat.com
 
 This will create a respawn behaviour in case libvirt
 process exits due to an uncaught signal not specified
 as a clean exit status.
 see
 http://www.freedesktop.org/software/systemd/man/systemd.service.html
 ---
  daemon/libvirtd.service.in | 1 +
  1 file changed, 1 insertion(+)
 
 diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in
 index aa5913b..b3c0849 100644
 --- a/daemon/libvirtd.service.in
 +++ b/daemon/libvirtd.service.in
 @@ -15,6 +15,7 @@ EnvironmentFile=-/etc/sysconfig/libvirtd
  ExecStart=@sbindir@/libvirtd $LIBVIRTD_ARGS
  ExecReload=/bin/kill -HUP $MAINPID
  KillMode=process
 +Restart=on-abort
  # Override the maximum number of opened files
  #LimitNOFILE=2048

I'm wondering whether 'on-abort' is the best choice or if
'on-failure' or 'always' are better. The systemd.service
man page says

[quote]
   Takes one of no, on-success, on-failure,
   on-abort, or always. If set to no (the
   default) the service will not be restarted. If
   set to on-success it will be restarted only
   when the service process exits cleanly. In
   this context, a clean exit means an exit code
   of 0, or one of the signals SIGHUP, SIGINT,
   SIGTERM, or SIGPIPE, and additionally, exit
   statuses and signals specified in
   SuccessExitStatus=. If set to on-failure the
   service will be restarted when the process
   exits with an nonzero exit code, is terminated
   by a signal (including on core dump), when an
   operation (such as service reload) times out,
   and when the configured watchdog timeout is
   triggered. If set to on-abort the service will
   be restarted only if the service process exits
   due to an uncaught signal not specified as a
   clean exit status. If set to always the
   service will be restarted regardless whether
   it exited cleanly or not, got terminated
   abnormally by a signal or hit a timeout.
[/quote]

I tend towards saying 'on-failure' here.
   
   I agree. we defiantly want restart in the 'on-failure'
   cases.
  
  Would 'on-failure' mean that libvirtd is to be restarted if it has
  exited with an error due to wrong configuration? This may spell nasty
  thrashing of systemd.
 
 You could get thrashing no matter what setting you use here - eg it
 could be core dumping during startup which would hit with on-abort.
 
 It doesn't matter though because systemd has rate limiting. By default
 services which are started more often than 5 times within 10s are not
 permitted to start any more times until the 10s interval ends.
 

It is also possible to exclude certain exit codes or signals
from the restart behavior using RestartPreventExitStatus=

 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


[libvirt] [PATCH] spec: Don't mix commands with macro definitions

2013-07-30 Thread Jiri Denemark
%build section should first define all required macros and then run
commands. Interleaving them makes it harder to spot what commands are
run.
---
 libvirt.spec.in | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 6d6203b..0fdba54 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1333,10 +1333,6 @@ of recent versions of Linux (and other OSes).
 %define init_scripts --with-init_script=redhat
 %endif
 
-%if 0%{?enable_autotools}
- autoreconf -if
-%endif
-
 %if %{with_selinux}
 %if 0%{?fedora} = 17 || 0%{?rhel} = 7
 %define with_selinux_mount --with-selinux-mount=/sys/fs/selinux
@@ -1345,6 +1341,11 @@ of recent versions of Linux (and other OSes).
 %endif
 %endif
 
+
+%if 0%{?enable_autotools}
+ autoreconf -if
+%endif
+
 %configure %{?_without_xen} \
%{?_without_qemu} \
%{?_without_openvz} \
-- 
1.8.3.2

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


Re: [libvirt] [PATCH 0/4] Support for integrating cgroups with systemd

2013-07-30 Thread Daniel P. Berrange
On Fri, Jul 26, 2013 at 04:48:20PM +0100, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 This is a much changed / expanded version of my previous work to
 create cgroups via systemd. The difference is that this time it
 actually works :-)
 
 I'm not proposing this for merge until after the 1.1.1 release.

This is now ready for review...

 
 Daniel P. Berrange (4):
   Add APIs for formatting systemd slice/scope names
   Add support for systemd cgroup mount
   Cope with races while killing processes
   Enable support for systemd-machined in cgroups creation
 
  src/libvirt_private.syms |   2 +
  src/lxc/lxc_process.c|  10 +-
  src/qemu/qemu_cgroup.c   |   1 +
  src/util/vircgroup.c | 270 
 +--
  src/util/vircgroup.h |   2 +
  src/util/virsystemd.c|  96 -
  src/util/virsystemd.h|   5 +
  tests/vircgrouptest.c|   9 ++
  tests/virsystemdtest.c   |  48 +
  9 files changed, 403 insertions(+), 40 deletions(-)


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] spec: Don't mix commands with macro definitions

2013-07-30 Thread Eric Blake
On 07/30/2013 08:07 AM, Jiri Denemark wrote:
 %build section should first define all required macros and then run
 commands. Interleaving them makes it harder to spot what commands are
 run.
 ---
  libvirt.spec.in | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)

ACK with one observation:

 
 diff --git a/libvirt.spec.in b/libvirt.spec.in
 index 6d6203b..0fdba54 100644
 --- a/libvirt.spec.in
 +++ b/libvirt.spec.in
 @@ -1333,10 +1333,6 @@ of recent versions of Linux (and other OSes).
  %define init_scripts --with-init_script=redhat
  %endif
  
 -%if 0%{?enable_autotools}
 - autoreconf -if
 -%endif
 -
  %if %{with_selinux}
  %if 0%{?fedora} = 17 || 0%{?rhel} = 7
  %define with_selinux_mount --with-selinux-mount=/sys/fs/selinux
 @@ -1345,6 +1341,11 @@ of recent versions of Linux (and other OSes).
  %endif
  %endif
  

Perhaps it deserves a comment here, along the lines of:

# place macros above and build commands below this comment

 +
 +%if 0%{?enable_autotools}
 + autoreconf -if
 +%endif
 +
  %configure %{?_without_xen} \
 %{?_without_qemu} \
 %{?_without_openvz} \
 

-- 
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] virt-login-shell joins users into lxc container.

2013-07-30 Thread Daniel P. Berrange
On Fri, Jul 26, 2013 at 12:26:36PM -0400, Daniel J Walsh wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1
 
 On 07/26/2013 07:40 AM, Daniel P. Berrange wrote:
  On Fri, Jul 26, 2013 at 07:38:31AM -0400, Daniel J Walsh wrote:
  -BEGIN PGP SIGNED MESSAGE- Hash: SHA1
  
  I can't seem to get the error reporting to turn on, what am I doing
  wrong.,
  
  if (virInitialize()  0) { fprintf(stderr, _(Failed to initialize
  libvirt)); return EXIT_FAILURE; }
  
  if (virErrorInitialize()  0) { fprintf(stderr, _(Failed to initialize
  libvirt Error Handling)); return EXIT_FAILURE; }
  
  virSetErrorFunc(NULL, NULL);
  
  
  virReportSystemError(EINVAL, %s, _(Test));
  
  And I get no output, I thought I would get error on stderr?
  
  You would, except that you just turned off printing to stderr by calling
  virSetErrorFunc in that way.
  
  
  Daniel
  
 Am I misreading this?
 * virSetErrorFunc:
 * @userData: pointer to the user data provided in the handler callback
 * @handler: the function to get called in case of error or NULL
 *
 * Set a library global error handling function, if @handler is NULL,
 * it will reset to default printing on stderr. The error raised there
 * are those for which no handler at the connection level could caught.
 */
 Looks like setting handler to Null reset default printing on stderr?
 But I am getting no output whether or not I set this.

Oh, whoops, I'm back to front.

The actual reason you're having trouble is that you're using
virReportError from outside the context of any public API
calls.

virReportError does not actually print any errors, it just
records them. It requires a seprate call to virDispatchError
to trigger the print to stderr.  This is usually taken care
of by the public API methods.

You'll have to take care of it yourself though. If you
have a centralized error handling return point eg

...success...
 return 0;

   error:
  return -1;
   }

Then you'd put a virDispatchError call just after the
'error:' label

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] spec: Use --enable-werror on RHEL

2013-07-30 Thread Eric Blake
On 07/30/2013 08:03 AM, Jiri Denemark wrote:
 As RHEL provides a stable tool chain, we don't have to worry about
 frequent changes in reported compiler warnings (which prevents us from
 enabling -Werror unconditionally).
 ---
  libvirt.spec.in | 9 +
  1 file changed, 9 insertions(+)

ACK.

-- 
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] spec: Don't mix commands with macro definitions

2013-07-30 Thread Jiri Denemark
On Tue, Jul 30, 2013 at 08:55:53 -0600, Eric Blake wrote:
 On 07/30/2013 08:07 AM, Jiri Denemark wrote:
  %build section should first define all required macros and then run
  commands. Interleaving them makes it harder to spot what commands are
  run.
  ---
   libvirt.spec.in | 9 +
   1 file changed, 5 insertions(+), 4 deletions(-)
 
 ACK with one observation:

  @@ -1345,6 +1341,11 @@ of recent versions of Linux (and other OSes).
   %endif
   %endif
   
 
 Perhaps it deserves a comment here, along the lines of:
 
 # place macros above and build commands below this comment

Done and pushed, thanks.

Jirka

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


Re: [libvirt] [PATCH 10/20] qemumonitortestutils: Refactor the test helpers to allow reuse

2013-07-30 Thread Eric Blake
On 07/30/2013 07:05 AM, Peter Krempa wrote:
 Refactor the test helpers to allow adding callbacks to verify the
 monitor responses instead of simple command name checking and clean up
 various parts to prepare for adding guest agent tests.
 ---
  tests/qemumonitortestutils.c | 220 
 ---
  1 file changed, 121 insertions(+), 99 deletions(-)
 

 @@ -512,7 +534,7 @@ qemuMonitorCommonTestInit(qemuMonitorTestPtr test)
  goto error;
 
  if (virNetSocketAddIOCallback(test-client,
 -  VIR_EVENT_HANDLE_WRITABLE,
 +  test-outgoingLength  0 ? 
 VIR_EVENT_HANDLE_WRITABLE : VIR_EVENT_HANDLE_READABLE,

Long line.

Quite a bit of code, but looks like a reasonable refactor.  ACK.

-- 
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] spec: Disable libssh2 support for RHEL

2013-07-30 Thread Jiri Denemark
On Tue, Jul 30, 2013 at 06:50:28 -0600, Eric Blake wrote:
 On 07/30/2013 05:45 AM, Jiri Denemark wrote:
  From: Peter Krempa pkre...@redhat.com
  
  https://bugzilla.redhat.com/show_bug.cgi?id=905513
  
  Libssh2 isn't reliable enough to support the libvirt transport using it.
  The problems include mishandling of known_hosts files that may confuse
  users.
  ---
   libvirt.spec.in | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
 
 ACK.

Pushed, thanks.

Jirka

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


Re: [libvirt] [PATCH] tests: Put a mock library at the start of LD_PRELOAD

2013-07-30 Thread Jiri Denemark
On Tue, Jul 30, 2013 at 10:37:40 +0100, Daniel Berrange wrote:
 On Tue, Jul 30, 2013 at 11:29:11AM +0200, Jiri Denemark wrote:
  This fixes vircgrouptest when run in a sandbox which already overrides
  open() and others.
 
 In general I think that having multiple LD_PRELOADS overriding the
 same symbols may well lead to pain.
 
 We try to make it at least chain up the libraries by using the
 'RTLD_NEXT' handle when looking up the real symbol, which
 should make it forward to the next preloaded library. From
 the sound of it, your sandbox tool isn't using RTLD_NEXT,
 causing the libvirt override to be skipped when the order was
 reversed.

I did some investigation and the sandbox actually uses RTLD_NEXT but
unfortunately searches for full symbols including their versions. Thus,
e.g., our implementation of fopen() is ignored because the sandbox wants
fopen@GLIBC_2.2.5.

 ACK

Thanks and pushed.

Jirka

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


Re: [libvirt] [PATCH] spec: RHEL-7 does not have sanlock on i686

2013-07-30 Thread Jiri Denemark
On Tue, Jul 30, 2013 at 06:53:19 -0600, Eric Blake wrote:
 On 07/30/2013 05:46 AM, Jiri Denemark wrote:
  ---
   libvirt.spec.in | 7 ++-
   1 file changed, 6 insertions(+), 1 deletion(-)
 
 ACK.

Pushed, thanks.

Jirka

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


Re: [libvirt] [PATCH] spec: Use --enable-werror on RHEL

2013-07-30 Thread Jiri Denemark
On Tue, Jul 30, 2013 at 09:52:34 -0600, Eric Blake wrote:
 On 07/30/2013 08:03 AM, Jiri Denemark wrote:
  As RHEL provides a stable tool chain, we don't have to worry about
  frequent changes in reported compiler warnings (which prevents us from
  enabling -Werror unconditionally).
  ---
   libvirt.spec.in | 9 +
   1 file changed, 9 insertions(+)
 
 ACK.

Pushed, thanks.

Jirka

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


Re: [libvirt] [PATCH 11/20] qemumonitortestutils: Split lines on \n instead of \r\n

2013-07-30 Thread Eric Blake
On 07/30/2013 07:05 AM, Peter Krempa wrote:
 The normal monitor uses windows line endings, where the agent monitor
 uses only newlines. Change this to tolerate both approaches and allow to
 use the utilities for guest agent tests.

windows line endings are also the proscribed line endings for use in
telnet; it's not an accident that the qemu monitor was designed to be
usable via a telnet connection, and I'm actually a bit surprised that
the qemu agent didn't follow suit.  Might be worth reporting this as an
upstream issue to the qemu list.

 ---
  tests/qemumonitortestutils.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

ACK.

-- 
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 12/20] qemumonitortestutils: Add instrumentation for guest agent testing

2013-07-30 Thread Eric Blake
On 07/30/2013 07:05 AM, Peter Krempa wrote:
 Add helper functions to open guest agent connections and a handler for
 replying to the guest-sync command.
 ---
  tests/qemumonitortestutils.c | 133 
 ++-
  tests/qemumonitortestutils.h |   6 ++
  2 files changed, 137 insertions(+), 2 deletions(-)
 

 @@ -462,6 +535,24 @@ static qemuMonitorCallbacks qemuMonitorTestCallbacks = {
  };
 
 
 +static void
 +qemuMonitorTestAgentEOFNotify(qemuAgentPtr agent ATTRIBUTE_UNUSED,
 +  virDomainObjPtr vm ATTRIBUTE_UNUSED)
 +{
 +}
 +
 +static void
 +qemuMonitorTestAgentErrorNotify(qemuAgentPtr agent ATTRIBUTE_UNUSED,
 +virDomainObjPtr vm ATTRIBUTE_UNUSED)
 +{
 +}

Since these are the same impl, is it worth simplifying slightly to have:

static void
qemuMonitorTestAgentNofify(...) { }

 +
 +static qemuAgentCallbacks qemuMonitorTestAgentCallbacks = {
 +.eofNotify = qemuMonitorTestAgentEOFNotify,
 +.errorNotify = qemuMonitorTestAgentErrorNotify,

and have both callback pointers initialized to the same function
pointer?  But that's a minor space optimization.

  qemuMonitorTestFree(test);
  return NULL;
  }
 
 +
  qemuMonitorPtr

Was this newline missed in your earlier patch that sanitized the
whitespace between functions?

ACK.

-- 
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 14/20] qemumonitortestutils: Add the ability to check arguments of commands

2013-07-30 Thread Eric Blake
On 07/30/2013 07:05 AM, Peter Krempa wrote:
 This patch adds helpers that allow to check for argument values in
 commands sent to the monitor.
 ---
  tests/qemumonitortestutils.c | 174 
 ---
  tests/qemumonitortestutils.h |   5 ++
  2 files changed, 170 insertions(+), 9 deletions(-)

 + Missing arguments section for command 
 '%s',
 + data-command_name);
 +goto cleanup;
 +}
 +
 +/* validate the args */
 +for (i = 0; i  data-nargs; i++) {

Indentation of the comment is off.

 +++ b/tests/qemumonitortestutils.h
 @@ -34,6 +34,11 @@ qemuMonitorTestAddItem(qemuMonitorTestPtr test,
 
  int qemuMonitorTestAddAgentSyncResponse(qemuMonitorTestPtr test);
 
 +int qemuMonitorTestAddItemParams(qemuMonitorTestPtr test,
 + const char *cmdname,
 + const char *response,
 + ...);

Might be worth adding an ATTRIBUTE_SENTINEL on the prototype of this
function, so that gcc ensures that a caller ends the list with a NULL.

ACK with those fixes.

-- 
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] maint: fix typo for SENTINEL

2013-07-30 Thread Eric Blake
* src/openvz/openvz_driver.c: Use correct spelling.
* src/vmware/vmware_conf.c: Likewise.
* src/vmware/vmware_conf.h: Likewise.
* src/vmware/vmware_driver.c: Likewise.

Signed-off-by: Eric Blake ebl...@redhat.com
---

Pushing under the trivial rule.

 src/openvz/openvz_driver.c | 24 
 src/vmware/vmware_conf.c   |  4 ++--
 src/vmware/vmware_conf.h   |  2 +-
 src/vmware/vmware_driver.c | 18 +-
 4 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index de4e4ff..d268647 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -564,12 +564,12 @@ cleanup:
  * key value. This lets us declare the argv on the
  * stack and just splice in the domain name after
  */
-#define PROGRAM_SENTINAL ((char *)0x1)
+#define PROGRAM_SENTINEL ((char *)0x1)
 static void openvzSetProgramSentinal(const char **prog, const char *key)
 {
 const char **tmp = prog;
 while (tmp  *tmp) {
-if (*tmp == PROGRAM_SENTINAL) {
+if (*tmp == PROGRAM_SENTINEL) {
 *tmp = key;
 break;
 }
@@ -580,7 +580,7 @@ static void openvzSetProgramSentinal(const char **prog, 
const char *key)
 static int openvzDomainSuspend(virDomainPtr dom) {
 struct openvz_driver *driver = dom-conn-privateData;
 virDomainObjPtr vm;
-const char *prog[] = {VZCTL, --quiet, chkpnt, PROGRAM_SENTINAL, 
--suspend, NULL};
+const char *prog[] = {VZCTL, --quiet, chkpnt, PROGRAM_SENTINEL, 
--suspend, NULL};
 int ret = -1;

 openvzDriverLock(driver);
@@ -618,7 +618,7 @@ cleanup:
 static int openvzDomainResume(virDomainPtr dom) {
   struct openvz_driver *driver = dom-conn-privateData;
   virDomainObjPtr vm;
-  const char *prog[] = {VZCTL, --quiet, chkpnt, PROGRAM_SENTINAL, 
--resume, NULL};
+  const char *prog[] = {VZCTL, --quiet, chkpnt, PROGRAM_SENTINEL, 
--resume, NULL};
   int ret = -1;

   openvzDriverLock(driver);
@@ -658,7 +658,7 @@ openvzDomainShutdownFlags(virDomainPtr dom,
   unsigned int flags) {
 struct openvz_driver *driver = dom-conn-privateData;
 virDomainObjPtr vm;
-const char *prog[] = {VZCTL, --quiet, stop, PROGRAM_SENTINAL, NULL};
+const char *prog[] = {VZCTL, --quiet, stop, PROGRAM_SENTINEL, NULL};
 int ret = -1;
 int status;

@@ -721,7 +721,7 @@ static int openvzDomainReboot(virDomainPtr dom,
 {
 struct openvz_driver *driver = dom-conn-privateData;
 virDomainObjPtr vm;
-const char *prog[] = {VZCTL, --quiet, restart, PROGRAM_SENTINAL, NULL};
+const char *prog[] = {VZCTL, --quiet, restart, PROGRAM_SENTINEL, NULL};
 int ret = -1;
 int status;

@@ -1041,7 +1041,7 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml,
 virDomainDefPtr vmdef = NULL;
 virDomainObjPtr vm = NULL;
 virDomainPtr dom = NULL;
-const char *progstart[] = {VZCTL, --quiet, start, PROGRAM_SENTINAL, 
NULL};
+const char *progstart[] = {VZCTL, --quiet, start, PROGRAM_SENTINEL, 
NULL};

 virCheckFlags(0, NULL);

@@ -1126,7 +1126,7 @@ openvzDomainCreateWithFlags(virDomainPtr dom, unsigned 
int flags)
 {
 struct openvz_driver *driver = dom-conn-privateData;
 virDomainObjPtr vm;
-const char *prog[] = {VZCTL, --quiet, start, PROGRAM_SENTINAL, NULL };
+const char *prog[] = {VZCTL, --quiet, start, PROGRAM_SENTINEL, NULL };
 int ret = -1;
 int status;

@@ -1180,7 +1180,7 @@ openvzDomainUndefineFlags(virDomainPtr dom,
 {
 struct openvz_driver *driver = dom-conn-privateData;
 virDomainObjPtr vm;
-const char *prog[] = { VZCTL, --quiet, destroy, PROGRAM_SENTINAL, NULL 
};
+const char *prog[] = { VZCTL, --quiet, destroy, PROGRAM_SENTINEL, NULL 
};
 int ret = -1;
 int status;

@@ -1228,7 +1228,7 @@ openvzDomainSetAutostart(virDomainPtr dom, int autostart)
 {
 struct openvz_driver *driver = dom-conn-privateData;
 virDomainObjPtr vm;
-const char *prog[] = { VZCTL, --quiet, set, PROGRAM_SENTINAL,
+const char *prog[] = { VZCTL, --quiet, set, PROGRAM_SENTINEL,
--onboot, autostart ? yes : no,
--save, NULL };
 int ret = -1;
@@ -1326,7 +1326,7 @@ static int openvzDomainSetVcpusInternal(virDomainObjPtr 
vm,
 unsigned int nvcpus)
 {
 charstr_vcpus[32];
-const char *prog[] = { VZCTL, --quiet, set, PROGRAM_SENTINAL,
+const char *prog[] = { VZCTL, --quiet, set, PROGRAM_SENTINEL,
--cpus, str_vcpus, --save, NULL };
 unsigned int pcpus;
 pcpus = openvzGetNodeCPUs();
@@ -1685,7 +1685,7 @@ openvzDomainSetMemoryInternal(virDomainObjPtr vm,
   unsigned long long mem)
 {
 char str_mem[16];
-const char *prog[] = { VZCTL, --quiet, set, PROGRAM_SENTINAL,
+const char *prog[] = { VZCTL, --quiet, set, PROGRAM_SENTINEL,
 --kmemsize, str_mem, --save, NULL
 };

diff --git 

Re: [libvirt] [PATCH] maint: fix typo for SENTINEL

2013-07-30 Thread Daniel P. Berrange
On Tue, Jul 30, 2013 at 10:51:04AM -0600, Eric Blake wrote:
 * src/openvz/openvz_driver.c: Use correct spelling.
 * src/vmware/vmware_conf.c: Likewise.
 * src/vmware/vmware_conf.h: Likewise.
 * src/vmware/vmware_driver.c: Likewise.
 
 Signed-off-by: Eric Blake ebl...@redhat.com
 ---
 
 Pushing under the trivial rule.
 
  src/openvz/openvz_driver.c | 24 
  src/vmware/vmware_conf.c   |  4 ++--
  src/vmware/vmware_conf.h   |  2 +-
  src/vmware/vmware_driver.c | 18 +-
  4 files changed, 24 insertions(+), 24 deletions(-)

oooh, remaining uses of virRun.  We're not actually that far off
being able to kill virRun in favour of virCommand


src/lxc/lxc_driver.c:if (virRun(argv, ip_rc)  0 ||
src/openvz/openvz_driver.c:if (virRun(prog, NULL)  0) {
src/openvz/openvz_driver.c:  if (virRun(prog, NULL)  0) {
src/openvz/openvz_driver.c:if (virRun(prog, NULL)  0)
src/openvz/openvz_driver.c:if (virRun(prog, NULL)  0)
src/openvz/openvz_driver.c:if (virRun(progstart, NULL)  0) {
src/openvz/openvz_driver.c:if (virRun(prog, NULL)  0) {
src/openvz/openvz_driver.c:if (virRun(prog, NULL)  0) {
src/openvz/openvz_driver.c:if (virRun(prog, NULL)  0) {
src/openvz/openvz_driver.c:if (virRun(prog, NULL)  0) {
src/openvz/openvz_driver.c:if (virRun(prog, NULL)  0) {
src/qemu/qemu_domain.c:if (virRun(qemuimgarg, NULL)  0) {
src/security/security_apparmor.c:if (virRun(argv, NULL) == 0)
src/security/virt-aa-helper.c:if ((ret = virRun(argv, status)) != 0 ||
src/util/virebtables.c:if (virRun((const char **)argv, NULL)  0) {
src/util/virnetdev.c:rc = virRun(argv, NULL);
src/util/virnetdevveth.c:if (virRun(argv, NULL)  0) {
src/util/virnetdevveth.c:return virRun(argv, NULL);
src/util/virpci.c:if (virRun(probecmd, NULL)  0) {
src/util/virutil.c:if (virRun(settleprog, exitstatus)  0)
src/vmware/vmware_conf.c:if (virRun(cmdmv, NULL)  0) {
src/vmware/vmware_driver.c:if (virRun(cmd, NULL)  0) {
src/vmware/vmware_driver.c:if (virRun(cmd, NULL)  0) {
src/vmware/vmware_driver.c:if (virRun(cmd, NULL)  0)
src/vmware/vmware_driver.c:if (virRun(cmd, NULL)  0)
src/vmware/vmware_driver.c:if (virRun(cmd, NULL)  0)


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 15/20] tests: Add qemuagenttest

2013-07-30 Thread Eric Blake
On 07/30/2013 07:05 AM, Peter Krempa wrote:
 Add a basic test framework with two simple tests to test guest agent
 interaction.
 ---
  .gitignore|   1 +
  tests/Makefile.am |  11 +++-
  tests/qemuagenttest.c | 163 
 ++
  3 files changed, 174 insertions(+), 1 deletion(-)
  create mode 100644 tests/qemuagenttest.c

ACK.  (Doesn't quite match reality, in that executing back-to-back
freeze with no thaw between is never what you'd do to a real agent - but
this test is about testing the libvirt code paths, not real
back-and-forth agent sequences)

-- 
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 18/20] qemuagenttest: Introduce testing of shutdown commands

2013-07-30 Thread Eric Blake
On 07/30/2013 07:05 AM, Peter Krempa wrote:
 This patch exports a few utility functions and adds testing of shutdown
 commands of the guest agent.
 ---
  tests/qemuagenttest.c| 119 
 +++
  tests/qemumonitortestutils.c |  21 
  tests/qemumonitortestutils.h |  28 --
  3 files changed, 153 insertions(+), 15 deletions(-)
 

 +++ b/tests/qemumonitortestutils.c
 @@ -37,12 +37,6 @@
 
  #define VIR_FROM_THIS VIR_FROM_NONE
 
 -typedef struct _qemuMonitorTestItem qemuMonitorTestItem;
 -typedef qemuMonitorTestItem *qemuMonitorTestItemPtr;
 -typedef int (*qemuMonitorTestResponseCallback)(qemuMonitorTestPtr test,
 -   qemuMonitorTestItemPtr item,
 -   const char *message);
 -

Hmm - this series is what added qemuMonitorTestResponseCallback, so this
feels like churn.  You should probably amend patch 10/20 to declare
qemuMonitorTestResponseCallback in the .h in the first place, and move
the typedefs at that time.

 @@ -167,7 +161,6 @@ cleanup:
  }
 
 
 -
  static int

Definitely some churn going on.

 @@ -414,6 +407,12 @@ error:
  return -1;
  }
 
 +void *
 +qemuMonitorTestItemGetPrivateData(qemuMonitorTestItemPtr item)

Missing the typical two blank lines.

Even after you hoist hunks to the proper earlier patches, this still
feels like two patches rolled into one - I'd split it into the exporting
of useful helper functions, then the new agent test that takes advantage
of the helpers.

At any rate, the test addition itself seems sane, and my complaints are
only about presentation of the commit contents.  ACK to the end result,
and rebasing seems like something you can safely do without me needing
to see a v2, as long as you test that each step compiles and passes
'make check'.

-- 
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 13/20] qemumonitortestutils: Improve error reporting from mock qemu monitor

2013-07-30 Thread Eric Blake
On 07/30/2013 07:05 AM, Peter Krempa wrote:
 Use the JSON error messages to report errors back to the caller in
 addition to erroring out. The error reported from the event loop from
 the mock function of the monitor was later overwritten by the call to
 the monitor/agent interaction API. This will also allow testing of error
 reporting.
 ---
  tests/qemumonitortestutils.c | 56 
 
  1 file changed, 46 insertions(+), 10 deletions(-)
 

 +}
 +
 +
 +
 +static int
  qemuMonitorTestProcessCommand(qemuMonitorTestPtr test,

Extra blank line.

ACK.

-- 
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 17/20] qemuagenttest: Add testing of agent suspend modes

2013-07-30 Thread Eric Blake
On 07/30/2013 07:05 AM, Peter Krempa wrote:
 ---
  tests/qemuagenttest.c | 47 +++
  1 file changed, 47 insertions(+)
 

ACK.

-- 
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 19/20] qemuagenttest: Test arbitrary qemu commands and timeouting of commands

2013-07-30 Thread Eric Blake
On 07/30/2013 07:05 AM, Peter Krempa wrote:
 Test arbitrary qemu commands and timeouting of the guest agent

s/timeouting/timeouts/ (also in the subject)

 synchronisation.
 ---
  tests/qemuagenttest.c | 101 
 ++
  1 file changed, 101 insertions(+)

Hmm, I wonder if it's worth adding some sort of escape hatch, maybe if
'VIR_TEST_FAST' is defined to non-empty in the environment, then we skip
this test; developers that don't like the long wait can then export that
variable as 1, whereas the spec file can ensure it is 0.  That could be
a followup patch, though, and it might be worth getting more feedback
than just mine on whether the new long-running test needs tweaking to
allow developers to avoid waiting, while still avoiding bit-rotting of
the test at release time.

ACK.

-- 
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 20/20] qemuagenttest: Add tests for CPU plug functions and helpers

2013-07-30 Thread Eric Blake
On 07/30/2013 07:05 AM, Peter Krempa wrote:
 ---
  tests/qemuagenttest.c | 119 
 ++
  1 file changed, 119 insertions(+)
 

ACK.

-- 
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 16/20] qemuagenttest: Test the filesystem trimming

2013-07-30 Thread Eric Blake
On 07/30/2013 07:05 AM, Peter Krempa wrote:
 ---
  tests/qemuagenttest.c | 31 +++
  1 file changed, 31 insertions(+)
 

ACK.

-- 
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 13/20] qemumonitortestutils: Improve error reporting from mock qemu monitor

2013-07-30 Thread Eric Blake
On 07/30/2013 07:05 AM, Peter Krempa wrote:
 Use the JSON error messages to report errors back to the caller in
 addition to erroring out. The error reported from the event loop from
 the mock function of the monitor was later overwritten by the call to
 the monitor/agent interaction API. This will also allow testing of error
 reporting.
 ---
  tests/qemumonitortestutils.c | 56 
 
  1 file changed, 46 insertions(+), 10 deletions(-)

My version of gcc complains:

qemumonitortestutils.c: In function 'qemuMonitorReportError':
qemumonitortestutils.c:140:5: error: function might be possible
candidate for 'gnu_printf' format attribute
[-Werror=suggest-attribute=format]
 if (virVasprintf(msg, errmsg, msgargs)  0)
 ^

 @@ -132,11 +134,48 @@ 
 qemuMonitorTestAddUnexpectedErrorResponse(qemuMonitorTestPtr test)
 
 
  static int
 +qemuMonitorReportError(qemuMonitorTestPtr test, const char *errmsg, ...)

You'll need to add ATTRIBUTE_FMT_PRINTF(2, 3) to shut up the warning.

-- 
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 1/4] Add APIs for formatting systemd slice/scope names

2013-07-30 Thread Eric Blake
On 07/26/2013 09:48 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 There are some interesting escaping rules to consider when dealing
 with systemd slice/scope names. Thus it is helpful to have APIs
 for formatting names
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/libvirt_private.syms |  2 ++
  src/util/virsystemd.c| 91 
 ++--
  src/util/virsystemd.h|  5 +++
  tests/virsystemdtest.c   | 48 +
  4 files changed, 144 insertions(+), 2 deletions(-)
 

 +
 +#define VALID_CHARS \
 +0123456789\
 +abcdefghijklmnopqrstuvwxyz\
 +ABCDEFGHIJKLMNOPQRSTUVWXYZ\
 +:-_.\\

If you would remove - and \\ from this list...

 +
 +if (*name == '.') {
 +ESCAPE(*name);
 +name++;
 +}
 +
 +while (*name) {
 +if (*name == '/')
 +virBufferAddChar(buf, '-');
 +else if (*name == '-' ||
 + *name == '\\' ||
 + !strchr(VALID_CHARS, *name))

...then this could be simplified to just !strchr().

 +
 +
 +char *virSystemdMakeScopeName(const char *name,

Is it worth using the style:

char *
virSystemdMakeScopeName(...

 +TEST_SCOPE(demo, /machine/eng-dept/testing!stuff, 
 machine-eng\\x2ddept-testing\\x21stuff-lxc\\x2ddemo.scope);

Worth wrapping the long line.

ACK.

-- 
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 2/4] Add support for systemd cgroup mount

2013-07-30 Thread Eric Blake
On 07/26/2013 09:48 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 Systemd uses a named cgroup mount for tracking processes. Add
 it as another type of controller, albeit one which we have to
 special case in a number of places. In particular we must
 never create/delete directories there, nor add tasks. Essentially
 the systemd mount is to be considered read-only for libvirt.
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/util/vircgroup.c  | 68 
 +++
  src/util/vircgroup.h  |  1 +
  tests/vircgrouptest.c |  9 +++
  3 files changed, 63 insertions(+), 15 deletions(-)
 

 @@ -524,13 +541,16 @@ static int virCgroupDetect(virCgroupPtr group,
  return -1;
  }
  
 -if (parent || path[0] == '/') {
 -if (virCgroupCopyPlacement(group, path, parent)  0)
 -return -1;
 -} else {
 -if (virCgroupDetectPlacement(group, pid, path)  0)
 -return -1;

This previously called only one of the two functions...

 -}
 +/* In some cases we can copy part of the placement info
 + * based on the parent cgroup...
 + */
 +if ((parent || path[0] == '/') 
 +virCgroupCopyPlacement(group, path, parent)  0)
 +return -1;
 +
 +/* ... but use /proc/cgroups to fill in the rest */
 +if (virCgroupDetectPlacement(group, pid, path)  0)

...now, if virCgroupCopyPlacement returns 0, it calls both functions.
Is that intentional?

 @@ -822,6 +842,12 @@ static int virCgroupMakeGroup(virCgroupPtr parent,
  for (i = 0; i  VIR_CGROUP_CONTROLLER_LAST; i++) {
  char *path = NULL;
  
 +/* We must never mkdir() in systemd's hierachy */

s/hierachy/hierarchy/

 +if (i == VIR_CGROUP_CONTROLLER_SYSTEMD) {
 +VIR_DEBUG(Not creating systemd controller group);
 +continue;
 +}
 +
  /* Skip over controllers that aren't mounted */
  if (!group-controllers[i].mountPoint) {
  VIR_DEBUG(Skipping unmounted controller %s,
 @@ -1026,6 +1052,10 @@ int virCgroupRemove(virCgroupPtr group)
  if (!group-controllers[i].mountPoint)
  continue;
  
 +/* We must never rmdir() in systemd's hiearchy */

s/hiearchy/hierarchy/

(hmm, you copied-and-pasted, but ended up with a different typo between
the two comments?)

 +if (i == VIR_CGROUP_CONTROLLER_SYSTEMD)
 +continue;
 +
  /* Don't delete the root group, if we accidentally
 ended up in it for some reason */
  if (STREQ(group-controllers[i].placement, /))
 @@ -1065,6 +1095,10 @@ int virCgroupAddTask(virCgroupPtr group, pid_t pid)
  if (!group-controllers[i].mountPoint)
  continue;
  
 +/* We must never add tasks in systemd's hiearchy */

s/hiearchy/hierarchy/

 +if (i == VIR_CGROUP_CONTROLLER_SYSTEMD)
 +continue;
 +
  if (virCgroupSetValueU64(group, i, tasks, (unsigned long long)pid) 
  0)
  goto cleanup;
  }
 @@ -1166,6 +1200,10 @@ int virCgroupMoveTask(virCgroupPtr src_group, 
 virCgroupPtr dest_group)
  !dest_group-controllers[i].mountPoint)
  continue;
  
 +/* We must never move tasks in systemd's hiearchy */

s/hiearchy/hierarchy/

Conditional ACK if you can address my question and fix the typos.

-- 
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 1/4] Add APIs for formatting systemd slice/scope names

2013-07-30 Thread Daniel P. Berrange
On Tue, Jul 30, 2013 at 12:05:51PM -0600, Eric Blake wrote:
 On 07/26/2013 09:48 AM, Daniel P. Berrange wrote:
  From: Daniel P. Berrange berra...@redhat.com
  
  There are some interesting escaping rules to consider when dealing
  with systemd slice/scope names. Thus it is helpful to have APIs
  for formatting names
  
  Signed-off-by: Daniel P. Berrange berra...@redhat.com
  ---
   src/libvirt_private.syms |  2 ++
   src/util/virsystemd.c| 91 
  ++--
   src/util/virsystemd.h|  5 +++
   tests/virsystemdtest.c   | 48 +
   4 files changed, 144 insertions(+), 2 deletions(-)
  
 
  +
  +#define VALID_CHARS \
  +0123456789\
  +abcdefghijklmnopqrstuvwxyz\
  +ABCDEFGHIJKLMNOPQRSTUVWXYZ\
  +:-_.\\
 
 If you would remove - and \\ from this list...

This is the set that systemd uses, and we need to match it
exactly, otherwise we won't detect the cgroups correctly.

 
  +
  +if (*name == '.') {
  +ESCAPE(*name);
  +name++;
  +}
  +
  +while (*name) {
  +if (*name == '/')
  +virBufferAddChar(buf, '-');
  +else if (*name == '-' ||
  + *name == '\\' ||
  + !strchr(VALID_CHARS, *name))
 
 ...then this could be simplified to just !strchr().
 
  +
  +
  +char *virSystemdMakeScopeName(const char *name,
 
 Is it worth using the style:
 
 char *
 virSystemdMakeScopeName(...
 
  +TEST_SCOPE(demo, /machine/eng-dept/testing!stuff, 
  machine-eng\\x2ddept-testing\\x21stuff-lxc\\x2ddemo.scope);
 
 Worth wrapping the long line.
 
 ACK.


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 2/4] Add support for systemd cgroup mount

2013-07-30 Thread Daniel P. Berrange
On Tue, Jul 30, 2013 at 12:13:58PM -0600, Eric Blake wrote:
 On 07/26/2013 09:48 AM, Daniel P. Berrange wrote:
  From: Daniel P. Berrange berra...@redhat.com
  
  Systemd uses a named cgroup mount for tracking processes. Add
  it as another type of controller, albeit one which we have to
  special case in a number of places. In particular we must
  never create/delete directories there, nor add tasks. Essentially
  the systemd mount is to be considered read-only for libvirt.
  
  Signed-off-by: Daniel P. Berrange berra...@redhat.com
  ---
   src/util/vircgroup.c  | 68 
  +++
   src/util/vircgroup.h  |  1 +
   tests/vircgrouptest.c |  9 +++
   3 files changed, 63 insertions(+), 15 deletions(-)
  
 
  @@ -524,13 +541,16 @@ static int virCgroupDetect(virCgroupPtr group,
   return -1;
   }
   
  -if (parent || path[0] == '/') {
  -if (virCgroupCopyPlacement(group, path, parent)  0)
  -return -1;
  -} else {
  -if (virCgroupDetectPlacement(group, pid, path)  0)
  -return -1;
 
 This previously called only one of the two functions...
 
  -}
  +/* In some cases we can copy part of the placement info
  + * based on the parent cgroup...
  + */
  +if ((parent || path[0] == '/') 
  +virCgroupCopyPlacement(group, path, parent)  0)
  +return -1;
  +
  +/* ... but use /proc/cgroups to fill in the rest */
  +if (virCgroupDetectPlacement(group, pid, path)  0)
 
 ...now, if virCgroupCopyPlacement returns 0, it calls both functions.
 Is that intentional?

Yes, when we use CopyPlacement it sets up all mounts, except for
the systemd mount, which we must always probe for. The DetectPlacement
method was changed so that it only fills in data, not already filled
in by CopyPlacement.


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 19/20] qemuagenttest: Test arbitrary qemu commands and timeouting of commands

2013-07-30 Thread Daniel P. Berrange
On Tue, Jul 30, 2013 at 11:27:49AM -0600, Eric Blake wrote:
 On 07/30/2013 07:05 AM, Peter Krempa wrote:
  Test arbitrary qemu commands and timeouting of the guest agent
 
 s/timeouting/timeouts/ (also in the subject)
 
  synchronisation.
  ---
   tests/qemuagenttest.c | 101 
  ++
   1 file changed, 101 insertions(+)
 
 Hmm, I wonder if it's worth adding some sort of escape hatch, maybe if
 'VIR_TEST_FAST' is defined to non-empty in the environment, then we skip
 this test; developers that don't like the long wait can then export that
 variable as 1, whereas the spec file can ensure it is 0.  That could be
 a followup patch, though, and it might be worth getting more feedback
 than just mine on whether the new long-running test needs tweaking to
 allow developers to avoid waiting, while still avoiding bit-rotting of
 the test at release time.

IMHO we don't want any of the tests doing multi-second timeouts by
default. IOW, rather than VIR_TEST_FAST, we should have a
VIR_TEST_ALLOW_SLEEP=1, and ensure that libvirt.spec.in sets that
when doing 'make check' and also make sure that autobuild.sh sets
it. So all automated builds fully exercise the tests, but day to
day usage isn't delayed

 ACK.

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 1/4] Add APIs for formatting systemd slice/scope names

2013-07-30 Thread Eric Blake
On 07/30/2013 12:47 PM, Daniel P. Berrange wrote:
 On Tue, Jul 30, 2013 at 12:05:51PM -0600, Eric Blake wrote:
 On 07/26/2013 09:48 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com

 There are some interesting escaping rules to consider when dealing
 with systemd slice/scope names. Thus it is helpful to have APIs
 for formatting names

 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/libvirt_private.syms |  2 ++
  src/util/virsystemd.c| 91 
 ++--
  src/util/virsystemd.h|  5 +++
  tests/virsystemdtest.c   | 48 +
  4 files changed, 144 insertions(+), 2 deletions(-)


 +
 +#define VALID_CHARS \
 +0123456789\
 +abcdefghijklmnopqrstuvwxyz\
 +ABCDEFGHIJKLMNOPQRSTUVWXYZ\
 +:-_.\\

 If you would remove - and \\ from this list...
 
 This is the set that systemd uses, and we need to match it
 exactly, otherwise we won't detect the cgroups correctly.

My point is that your only use of VALID_CHARS was in the strchr() to see
which characters need escaping.  systemd is using the full set for
characters present _after_ escaping, whereas you are only using it
_before_ escaping, and have to special-case your escaping as a result.

But I'm not too concerned about the efficiency to insist on that
micro-optimization.

-- 
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 2/4] Add support for systemd cgroup mount

2013-07-30 Thread Eric Blake
On 07/30/2013 12:49 PM, Daniel P. Berrange wrote:
 On Tue, Jul 30, 2013 at 12:13:58PM -0600, Eric Blake wrote:
 On 07/26/2013 09:48 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com

 Systemd uses a named cgroup mount for tracking processes. Add
 it as another type of controller, albeit one which we have to
 special case in a number of places. In particular we must
 never create/delete directories there, nor add tasks. Essentially
 the systemd mount is to be considered read-only for libvirt.

 Signed-off-by: Daniel P. Berrange berra...@redhat.com

 -}
 +/* In some cases we can copy part of the placement info
 + * based on the parent cgroup...
 + */
 +if ((parent || path[0] == '/') 
 +virCgroupCopyPlacement(group, path, parent)  0)
 +return -1;
 +
 +/* ... but use /proc/cgroups to fill in the rest */
 +if (virCgroupDetectPlacement(group, pid, path)  0)

 ...now, if virCgroupCopyPlacement returns 0, it calls both functions.
 Is that intentional?
 
 Yes, when we use CopyPlacement it sets up all mounts, except for
 the systemd mount, which we must always probe for. The DetectPlacement
 method was changed so that it only fills in data, not already filled
 in by CopyPlacement.

That explains it, then.  Probably worth mentioning in the commit
message, so the next reader won't miss it like I did.  Condition has
been met, ACK is now granted with the typo fixes.

-- 
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 19/20] qemuagenttest: Test arbitrary qemu commands and timeouting of commands

2013-07-30 Thread Eric Blake
On 07/30/2013 12:52 PM, Daniel P. Berrange wrote:

 Hmm, I wonder if it's worth adding some sort of escape hatch, maybe if
 'VIR_TEST_FAST' is defined to non-empty in the environment, then we skip
 this test; developers that don't like the long wait can then export that
 variable as 1, whereas the spec file can ensure it is 0.  That could be
 a followup patch, though, and it might be worth getting more feedback
 than just mine on whether the new long-running test needs tweaking to
 allow developers to avoid waiting, while still avoiding bit-rotting of
 the test at release time.
 
 IMHO we don't want any of the tests doing multi-second timeouts by
 default. IOW, rather than VIR_TEST_FAST, we should have a
 VIR_TEST_ALLOW_SLEEP=1, and ensure that libvirt.spec.in sets that
 when doing 'make check' and also make sure that autobuild.sh sets
 it. So all automated builds fully exercise the tests, but day to
 day usage isn't delayed

For that matter, 'make check' for day-to-day usage should be able to
skip the gnulib subdirectory - the results in gnulib/tests will only
change if you upgrade the gnulib submodule, glibc, or some other core
component, which is not what we change on a day-to-day basis when
hacking gnulib, but is also something an autobuilder should be running
always.  I'll see if I can hack something up to speed up 'make check'
for normal users on the gnulib front, which we can then extend into
skipping Peter's new test.

GNU coreutils calls its variable RUN_EXPENSIVE_TESTS, defaulting to no,
but set to yes in autobuilders.  Sounds like the best type of naming
(maybe VIR_TEST_EXPENSIVE, to keep it in the VIR_ namespace).  Anyone
else want to chime in with a bikeshed color?

-- 
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 3/4] Cope with races while killing processes

2013-07-30 Thread Eric Blake
On 07/26/2013 09:48 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 When systemd is involved in managing processes, it may start
 killing off  tearing down croups associated with the process
 while we're still doing virCgroupKillPainfully. We must
 explicitly check for ENOENT and treat it as if we had finished
 killing processes
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/util/vircgroup.c | 15 ++-
  1 file changed, 14 insertions(+), 1 deletion(-)

ACK.

-- 
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 4/4] Enable support for systemd-machined in cgroups creation

2013-07-30 Thread Eric Blake
On 07/26/2013 09:48 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 Make the virCgroupNewMachine method try to use systemd-machined
 first. If that fails, then fallback to using the traditional
 cgroup setup code path.
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/lxc/lxc_process.c  |  10 +--
  src/qemu/qemu_cgroup.c |   1 +
  src/util/vircgroup.c   | 187 
 -
  src/util/vircgroup.h   |   1 +
  src/util/virsystemd.c  |   5 +-
  5 files changed, 182 insertions(+), 22 deletions(-)
 

 +/*
 + * Retujrns 0 on success, -1 on fatal error, -2 on systemd not available
 + */

s/Retujrns/Returns/

  
 -*group = NULL;
 +path = (init)-controllers[VIR_CGROUP_CONTROLLER_SYSTEMD].placement;
 +(init)-controllers[VIR_CGROUP_CONTROLLER_SYSTEMD].placement = NULL;

s/(init)/init/ (looks like you refactored, and had (*init) at one point)


 +++ b/src/util/virsystemd.c
 @@ -225,8 +225,9 @@ int virSystemdCreateMachine(const char *name,
iscontainer ? container : vm,
(unsigned int)pidleader,
rootdir ? rootdir : ,
 -  1, Slice, s,
 -  slicename)  0) {
 +  1,
 +  Slice, s, slicename
 +  )  0) {

Spurious reformatting?

ACK with nits fixed.

-- 
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] Add patches to allow users to join running containers.

2013-07-30 Thread dwalsh
[PATCH 1/2] Add virGetUserDirectoryByUID to retrieve users homedir
[PATCH 2/2] virt-login-shell joins users into lxc container.

This patch implements most of the changes suggested by Dan Berrange and 
Eric Blake.

Some replies to suggested changes.

Removed mingw-libvirt.spec.in changes since virt lxc probably can not be 
supported in Windows.  Not sure if I need to make changes so my code will not 
build on that platform.

Did not make the changes to install virt-login-shell as 4755 automatically.
I guess I want a more firm, make that change request...

I did not make a helper function to parse a list of strings out of conf file.

The getuid and getgid calls return the user that executed the program, when the 
app is setuid geteuid and getegid return 0.  I believe getuid and getgid are 
correct.

Added virt-login-shell --help, not sure what --program would do?

The program is hard coded to LXC because there is no way that I know of for a ZZ
process to join a running qemu instance.

I have heard back from one security review from Miloslav Trmac, who had similar 
comments as Eric.

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


[libvirt] [PATCH 1/2] Add virGetUserDirectoryByUID to retrieve users homedir based on UID.

2013-07-30 Thread dwalsh
From: Dan Walsh dwa...@redhat.com

This function is needed for virt-login-shell.  Also modify virGirUserDirectory
to use the new function, to simplify the code.
---
 src/libvirt_private.syms | 1 +
 src/util/virutil.c   | 9 +++--
 src/util/virutil.h   | 1 +
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d9615ea..201b2cb 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2044,6 +2044,7 @@ virGetUnprivSGIOSysfsPath;
 virGetUserCacheDirectory;
 virGetUserConfigDirectory;
 virGetUserDirectory;
+virGetUserDirectoryByUID;
 virGetUserID;
 virGetUserName;
 virGetUserRuntimeDirectory;
diff --git a/src/util/virutil.c b/src/util/virutil.c
index adcdb3c..99812ca 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -750,13 +750,18 @@ static char *virGetGroupEnt(gid_t gid)
 return ret;
 }
 
-char *virGetUserDirectory(void)
+char *virGetUserDirectoryByUID(uid_t uid)
 {
 char *ret;
-virGetUserEnt(geteuid(), NULL, NULL, ret);
+virGetUserEnt(uid, NULL, NULL, ret);
 return ret;
 }
 
+char *virGetUserDirectory(void)
+{
+return virGetUserDirectoryByUID(geteuid());
+}
+
 static char *virGetXDGDirectory(const char *xdgenvname, const char *xdgdefdir)
 {
 const char *path = getenv(xdgenvname);
diff --git a/src/util/virutil.h b/src/util/virutil.h
index 526c610..4b06992 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -111,6 +111,7 @@ static inline int getgid (void) { return 0; }
 char *virGetHostname(void);
 
 char *virGetUserDirectory(void);
+char *virGetUserDirectoryByUID(uid_t uid);
 char *virGetUserConfigDirectory(void);
 char *virGetUserCacheDirectory(void);
 char *virGetUserRuntimeDirectory(void);
-- 
1.8.3.1

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


[libvirt] [PATCH 2/2] virt-login-shell joins users into lxc container.

2013-07-30 Thread dwalsh
From: Dan Walsh dwa...@redhat.com

Openshift wants to have their gears stuck into a container when they login
to the system.  virt-login-shell will join a running gear with the username of
the person running it, or attempt to start the container if it is not running.
(Currently containers do not exist if they are not running, so I can not test
this feature. But the code is there).

This tool needs to be setuid since joining a container (nsjoin) requires privs.
The root user is not allowed to execute this command. When this tool is
run by a normal user it will only join the users container.

Only users who are listed as valid_users in /etc/libvirt/virt-login-shell.conf
are allowed to join containers using this tool. By default no users are allowed.
---
 .gitignore  |   1 +
 libvirt.spec.in |   3 +
 po/POTFILES.in  |   1 +
 tools/Makefile.am   |  30 +++-
 tools/virt-login-shell.c| 363 
 tools/virt-login-shell.conf |  26 
 tools/virt-login-shell.pod  |  62 
 7 files changed, 485 insertions(+), 1 deletion(-)
 create mode 100644 tools/virt-login-shell.c
 create mode 100644 tools/virt-login-shell.conf
 create mode 100644 tools/virt-login-shell.pod

diff --git a/.gitignore b/.gitignore
index 4c79de3..b09463a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -214,6 +214,7 @@
 /tools/libvirt-guests.init
 /tools/libvirt-guests.service
 /tools/libvirt-guests.sh
+/tools/virt-login-shell
 /tools/virsh
 /tools/virsh-*-edit.c
 /tools/virt-*-validate
diff --git a/libvirt.spec.in b/libvirt.spec.in
index a3a831f..e69e87d 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1974,14 +1974,17 @@ fi
 %doc AUTHORS ChangeLog.gz NEWS README COPYING COPYING.LESSER TODO
 
 %config(noreplace) %{_sysconfdir}/libvirt/libvirt.conf
+%config(noreplace) %{_sysconfdir}/libvirt/virt-login-shell.conf
 %{_mandir}/man1/virsh.1*
 %{_mandir}/man1/virt-xml-validate.1*
 %{_mandir}/man1/virt-pki-validate.1*
 %{_mandir}/man1/virt-host-validate.1*
+%{_mandir}/man1/virt-login-shell.1*
 %{_bindir}/virsh
 %{_bindir}/virt-xml-validate
 %{_bindir}/virt-pki-validate
 %{_bindir}/virt-host-validate
+%attr(4755, root, root) %{_bindir}/virt-login-shell
 %{_libdir}/lib*.so.*
 
 %if %{with_dtrace}
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 1fd84af..884b70a 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -231,3 +231,4 @@ tools/virt-host-validate-common.c
 tools/virt-host-validate-lxc.c
 tools/virt-host-validate-qemu.c
 tools/virt-host-validate.c
+tools/virt-login-shell.c
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 644a86d..00c582a 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -37,6 +37,7 @@ EXTRA_DIST = \
virt-pki-validate.in\
virt-sanlock-cleanup.in \
virt-sanlock-cleanup.8  \
+   virt-login-shell.pod\
virsh.pod   \
libvirt-guests.sysconf  \
virsh-edit.c\
@@ -52,8 +53,11 @@ EXTRA_DIST = \
 
 DISTCLEANFILES =
 
+confdir = $(sysconfdir)/libvirt
+conf_DATA = virt-login-shell.conf
+
 bin_SCRIPTS = virt-xml-validate virt-pki-validate
-bin_PROGRAMS = virsh virt-host-validate
+bin_PROGRAMS = virsh virt-host-validate virt-login-shell
 libexec_SCRIPTS = libvirt-guests.sh
 
 if WITH_SANLOCK
@@ -65,6 +69,7 @@ dist_man1_MANS = \
virt-host-validate.1 \
virt-pki-validate.1 \
virt-xml-validate.1 \
+   virt-login-shell.1 \
virsh.1
 if WITH_SANLOCK
 dist_man8_MANS = virt-sanlock-cleanup.8
@@ -128,6 +133,24 @@ virt_host_validate_CFLAGS = \
$(COVERAGE_CFLAGS)  \
$(NULL)
 
+virt_login_shell_SOURCES = \
+   virt-login-shell.conf   \
+   virt-login-shell.c
+
+virt_login_shell_LDFLAGS = $(COVERAGE_LDFLAGS)
+virt_login_shell_LDADD =   \
+   $(STATIC_BINARIES)  \
+   $(PIE_LDFLAGS)  \
+   $(RELRO_LDFLAGS) \
+   ../src/libvirt.la   \
+   ../src/libvirt-lxc.la   \
+   ../gnulib/lib/libgnu.la
+
+virt_login_shell_CFLAGS =  \
+   $(WARN_CFLAGS)  \
+   $(PIE_CFLAGS)   \
+   $(COVERAGE_CFLAGS)
+
 virsh_SOURCES =\
console.c console.h \
virsh.c virsh.h \
@@ -189,6 +212,11 @@ virsh_win_icon.$(OBJEXT): 

[libvirt] [PATCHv3] build: avoid -lgcrypt with newer gnutls

2013-07-30 Thread Eric Blake
https://bugzilla.redhat.com/show_bug.cgi?id=951637

Newer gnutls uses nettle, rather than gcrypt, which is a lot nicer
regarding initialization.  Yet we were unconditionally initializing
gcrypt even when gnutls wouldn't be using it, and having two crypto
libraries linked into libvirt.so is pointless, but mostly harmless
(it doesn't crash, but does interfere with certification efforts).

There are three distinct version ranges to worry about when
determining which crypto lib gnutls uses, per these gnutls mails:
2.12: http://lists.gnu.org/archive/html/gnutls-devel/2011-03/msg00034.html
3.0: http://lists.gnu.org/archive/html/gnutls-devel/2011-07/msg00035.html

If pkg-config can prove version numbers and/or list the crypto
library used for static linking, we have our proof; if not, it
is safer (even if pointless) to continue to use gcrypt ourselves.

* configure.ac (WITH_GNUTLS): Probe whether to add -lgcrypt, and
define a witness WITH_GNUTLS_GCRYPT.
* src/libvirt.c (virTLSMutexInit, virTLSMutexDestroy)
(virTLSMutexLock, virTLSMutexUnlock, virTLSThreadImpl)
(virGlobalInit): Honor the witness.
* libvirt.spec.in (BuildRequires): Make gcrypt usage conditional,
no longer needed in Fedora 19.

Signed-off-by: Eric Blake ebl...@redhat.com
---

v3: configure logic is enhanced to try harder to get the
right answer for gentoo.  I tested with Fedora 19 and RHEL 6,
but need feedback from a gentoo tester before this can go in.

 configure.ac|   37 ++---
 libvirt.spec.in |2 ++
 src/libvirt.c   |   10 ++
 3 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/configure.ac b/configure.ac
index 1817126..e3d148c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1076,12 +1076,26 @@ if test x$with_gnutls != xno; then
   LIBS=$LIBS $GNUTLS_LIBS

   GNUTLS_FOUND=no
+  GNUTLS_GCRYPT=unknown
   if test -x $PKG_CONFIG ; then
+dnl Triple probe: gnutls  2.12 only used gcrypt, gnutls = 3.0 uses
+dnl only nettle, and versions in between had a configure option.
+dnl Our goal is to avoid gcrypt if we can prove gnutls uses nettle,
+dnl but it is a safe fallback to use gcrypt if we can't prove anything.
+if $PKG_CONFIG --exists 'gnutls = 3.0'; then
+  GNUTLS_GCRYPT=no
+elif $PKG_CONFIG --exists 'gnutls = 2.12'; then
+  GNUTLS_GCRYPT=probe
+else
+  GNUTLS_GCRYPT=yes
+fi
 PKG_CHECK_MODULES(GNUTLS, gnutls = $GNUTLS_REQUIRED,
   [GNUTLS_FOUND=yes], [GNUTLS_FOUND=no])
   fi
   if test $GNUTLS_FOUND = no; then
+dnl pkg-config couldn't help us, assume gcrypt is necessary
 fail=0
+GNUTLS_GCRYPT=yes
 AC_CHECK_HEADER([gnutls/gnutls.h], [], [fail=1])
 AC_CHECK_LIB([gnutls], [gnutls_handshake],[], [fail=1], [-lgcrypt])

@@ -1098,13 +1112,22 @@ if test x$with_gnutls != xno; then
   AC_MSG_ERROR([You must install the GnuTLS library in order to compile 
and run libvirt])
 fi
   else
-dnl Not all versions of gnutls include -lgcrypt, and so we add
-dnl it explicitly for the calls to gcry_control/check_version
-GNUTLS_LIBS=$GNUTLS_LIBS -lgcrypt
-
-dnl We're not using gcrypt deprecated features so define
-dnl GCRYPT_NO_DEPRECATED to avoid deprecated warnings
-GNUTLS_CFLAGS=$GNUTLS_CFLAGS -DGCRYPT_NO_DEPRECATED
+dnl See comments above about when to use gcrypt.
+if test $GNUTLS_GCRYPT = probe; then
+  case `$PKG_CONFIG --libs --static gnutls` in
+*gcrypt*) GNUTLS_GCRYPT=yes ;;
+*nettle*) GNUTLS_GCRYPT=no  ;;
+*)GNUTLS_GCRYPT=unknown ;;
+  esac
+fi
+if test $GNUTLS_GCRYPT = yes || test $GNUTLS_GCRYPT = unknown; then
+  GNUTLS_LIBS=$GNUTLS_LIBS -lgcrypt
+  dnl We're not using gcrypt deprecated features so define
+  dnl GCRYPT_NO_DEPRECATED to avoid deprecated warnings
+  GNUTLS_CFLAGS=$GNUTLS_CFLAGS -DGCRYPT_NO_DEPRECATED
+  AC_DEFINE_UNQUOTED([WITH_GNUTLS_GCRYPT], 1,
+[set to 1 if it is known or assumed that GNUTLS uses gcrypt])
+fi

 dnl gnutls 3.x moved some declarations to a new header
 AC_CHECK_HEADERS([gnutls/crypto.h], [], [], [[
diff --git a/libvirt.spec.in b/libvirt.spec.in
index fce7f91..7fd3c85 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -438,7 +438,9 @@ BuildRequires: readline-devel
 BuildRequires: ncurses-devel
 BuildRequires: gettext
 BuildRequires: libtasn1-devel
+%if (0%{?rhel}  0%{?rhel}  7) || (0%{?fedora}  0%{?fedora}  19)
 BuildRequires: libgcrypt-devel
+%endif
 BuildRequires: gnutls-devel
 BuildRequires: libattr-devel
 %if %{with_libvirtd}
diff --git a/src/libvirt.c b/src/libvirt.c
index 8157488..66e8248 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -55,7 +55,9 @@
 #include intprops.h
 #include virconf.h
 #if WITH_GNUTLS
-# include gcrypt.h
+# if WITH_GNUTLS_GCRYPT
+#  include gcrypt.h
+# endif
 # include rpc/virnettlscontext.h
 #endif
 #include vircommand.h
@@ -270,7 +272,7 @@ winsock_init(void)
 #endif


-#ifdef WITH_GNUTLS
+#ifdef WITH_GNUTLS_GCRYPT
 static int 

Re: [libvirt] [PATCH 19/20] qemuagenttest: Test arbitrary qemu commands and timeouting of commands

2013-07-30 Thread Eric Blake
On 07/30/2013 11:27 AM, Eric Blake wrote:
 On 07/30/2013 07:05 AM, Peter Krempa wrote:
 Test arbitrary qemu commands and timeouting of the guest agent
 
 s/timeouting/timeouts/ (also in the subject)
 
 synchronisation.
 ---
  tests/qemuagenttest.c | 101 
 ++
  1 file changed, 101 insertions(+)
 
 Hmm, I wonder if it's worth adding some sort of escape hatch, maybe if
 'VIR_TEST_FAST' is defined to non-empty in the environment, then we skip
 this test; developers that don't like the long wait can then export that
 variable as 1, whereas the spec file can ensure it is 0.  That could be
 a followup patch, though, and it might be worth getting more feedback
 than just mine on whether the new long-running test needs tweaking to
 allow developers to avoid waiting, while still avoiding bit-rotting of
 the test at release time.

In testing this patch, I'll point out that I was using new enough
automake that gives us parallel testing, and a command like 'make -j3
check' did not see ANY appreciable delay - we spend more than six
seconds on other tests, such that the sleep time of this new test was
mostly offset by the other tests running in parallel.  Still, it's
probably worth an option for pruning this and other sleeps from the
testsuite when run by a developer.

-- 
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 v2] Add flag to BaselineCPU API to return detailed CPU features

2013-07-30 Thread Don Dugger
[V2 - per review comments change the flag name to be more
descriptive and change errors from warnings to propogated
errors.]

Currently the virConnectBaselineCPU API does not expose the CPU features
that are part of the CPU's model.  This patch adds a new flag,
VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE, that causes the API to explictly
list all features that are part of that model.

Signed-off-by: Don Dugger donald.d.dug...@intel.com
---
 include/libvirt/libvirt.h.in|9 ++
 src/cpu/cpu.c   |   12 
 src/cpu/cpu.h   |   12 +---
 src/cpu/cpu_arm.c   |3 +-
 src/cpu/cpu_generic.c   |3 +-
 src/cpu/cpu_powerpc.c   |6 ++--
 src/cpu/cpu_s390.c  |3 +-
 src/cpu/cpu_x86.c   |   43 +--
 src/qemu/qemu_driver.c  |4 +--
 tests/cputest.c |   30 ++-
 tests/cputestdata/x86-baseline-3-result.xml |   35 ++
 tests/cputestdata/x86-baseline-3.xml|7 +
 tools/virsh-domain.c|   11 ++-
 13 files changed, 144 insertions(+), 34 deletions(-)
 create mode 100644 tests/cputestdata/x86-baseline-3-result.xml
 create mode 100644 tests/cputestdata/x86-baseline-3.xml

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 1804c93..b377707 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -3890,6 +3890,15 @@ int virConnectCompareCPU(virConnectPtr conn,
 
 
 /**
+ * virConnectBaselineCPUFlags
+ *
+ * Flags when getting XML description of a computed CPU
+ */
+typedef enum {
+VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE  = (1  0),  /* show model 
features*/
+} virConnectBaselineCPUFlags;
+
+/**
  * virConnectBaselineCPU:
  *
  * @conn: virConnect connection
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 68125a5..c96f669 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -167,7 +167,7 @@ cpuDecode(virCPUDefPtr cpu,
 return -1;
 }
 
-return driver-decode(cpu, data, models, nmodels, preferred);
+return driver-decode(cpu, data, models, nmodels, preferred, 0);
 }
 
 
@@ -277,7 +277,8 @@ char *
 cpuBaselineXML(const char **xmlCPUs,
unsigned int ncpus,
const char **models,
-   unsigned int nmodels)
+   unsigned int nmodels,
+   unsigned int flags)
 {
 xmlDocPtr doc = NULL;
 xmlXPathContextPtr ctxt = NULL;
@@ -324,7 +325,7 @@ cpuBaselineXML(const char **xmlCPUs,
 doc = NULL;
 }
 
-if (!(cpu = cpuBaseline(cpus, ncpus, models, nmodels)))
+if (!(cpu = cpuBaseline(cpus, ncpus, models, nmodels, flags)))
 goto error;
 
 cpustr = virCPUDefFormat(cpu, 0);
@@ -353,7 +354,8 @@ virCPUDefPtr
 cpuBaseline(virCPUDefPtr *cpus,
 unsigned int ncpus,
 const char **models,
-unsigned int nmodels)
+unsigned int nmodels,
+unsigned int flags)
 {
 struct cpuArchDriver *driver;
 unsigned int i;
@@ -395,7 +397,7 @@ cpuBaseline(virCPUDefPtr *cpus,
 return NULL;
 }
 
-return driver-baseline(cpus, ncpus, models, nmodels);
+return driver-baseline(cpus, ncpus, models, nmodels, flags);
 }
 
 
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index cba7149..9148871 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -49,7 +49,8 @@ typedef int
  const union cpuData *data,
  const char **models,
  unsigned int nmodels,
- const char *preferred);
+ const char *preferred,
+ unsigned int flags);
 
 typedef int
 (*cpuArchEncode)(const virCPUDefPtr cpu,
@@ -76,7 +77,8 @@ typedef virCPUDefPtr
 (*cpuArchBaseline)  (virCPUDefPtr *cpus,
  unsigned int ncpus,
  const char **models,
- unsigned int nmodels);
+ unsigned int nmodels,
+ unsigned int /* flags */);
 
 typedef int
 (*cpuArchUpdate)(virCPUDefPtr guest,
@@ -145,13 +147,15 @@ extern char *
 cpuBaselineXML(const char **xmlCPUs,
unsigned int ncpus,
const char **models,
-   unsigned int nmodels);
+   unsigned int nmodels,
+   unsigned int /* flags */);
 
 extern virCPUDefPtr
 cpuBaseline (virCPUDefPtr *cpus,
  unsigned int ncpus,
  const char **models,
- unsigned int nmodels);
+ unsigned int nmodels,
+ unsigned int /* flags */);
 
 extern int
 cpuUpdate   (virCPUDefPtr guest,
diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
index cfe1a23..af1309c 100644
--- a/src/cpu/cpu_arm.c
+++ b/src/cpu/cpu_arm.c
@@ -48,7 +48,8 @@ ArmDecode(virCPUDefPtr cpu ATTRIBUTE_UNUSED,
   

Re: [libvirt] [PATCH 1/2] Add virGetUserDirectoryByUID to retrieve users homedir based on UID.

2013-07-30 Thread Eric Blake
On 07/30/2013 01:55 PM, dwa...@redhat.com wrote:
 From: Dan Walsh dwa...@redhat.com

Subject line was a little long; I trimmed it to:

util: add virGetUserDirectoryByUID

 
 This function is needed for virt-login-shell.  Also modify virGirUserDirectory
 to use the new function, to simplify the code.
 ---
  src/libvirt_private.syms | 1 +
  src/util/virutil.c   | 9 +++--
  src/util/virutil.h   | 1 +
  3 files changed, 9 insertions(+), 2 deletions(-)

Failed to build on mingw; here's what I squashed in, then pushed:

diff --git i/src/util/virutil.c w/src/util/virutil.c
index 99812ca..aef63c7 100644
--- i/src/util/virutil.c
+++ w/src/util/virutil.c
@@ -647,6 +647,11 @@ cleanup:
 return result;
 }

+char *virGetUserDirectory(void)
+{
+return virGetUserDirectoryByUID(geteuid());
+}
+
 #ifdef HAVE_GETPWUID_R
 /* Look up fields from the user database for the given user.  On
  * error, set errno, report the error, and return -1.  */
@@ -757,11 +762,6 @@ char *virGetUserDirectoryByUID(uid_t uid)
 return ret;
 }

-char *virGetUserDirectory(void)
-{
-return virGetUserDirectoryByUID(geteuid());
-}
-
 static char *virGetXDGDirectory(const char *xdgenvname, const char
*xdgdefdir)
 {
 const char *path = getenv(xdgenvname);
@@ -1097,8 +1097,11 @@ virGetWin32DirectoryRoot(char **path)


 char *
-virGetUserDirectory(void)
+virGetUserDirectoryByUID(uid_t uid ATTRIBUTE_UNUSED)
 {
+/* Since Windows lacks setuid, and since we already fake
+ * geteuid(), we can safely assume that this is only called when
+ * querying about the current user */
 const char *dir;
 char *ret;

@@ -1182,7 +1185,7 @@ virGetUserRuntimeDirectory(void)

 # else /* !HAVE_GETPWUID_R  !WIN32 */
 char *
-virGetUserDirectory(void)
+virGetUserDirectoryByUID(uid_t uid ATTRIBUTE_UNUSED)
 {
 virReportError(VIR_ERR_INTERNAL_ERROR,
%s, _(virGetUserDirectory is not available));


-- 
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 v2] Add flag to BaselineCPU API to return detailed CPU features

2013-07-30 Thread Eric Blake
On 07/30/2013 03:09 PM, Don Dugger wrote:
 [V2 - per review comments change the flag name to be more
 descriptive and change errors from warnings to propogated
 errors.]

This aside belongs...

 
 Currently the virConnectBaselineCPU API does not expose the CPU features
 that are part of the CPU's model.  This patch adds a new flag,
 VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE, that causes the API to explictly
 list all features that are part of that model.
 
 Signed-off-by: Don Dugger donald.d.dug...@intel.com
 ---

...here, after the '---', so that 'git am' won't turn it into a
permanent part of libvirt.git history.  Your patch didn't apply as is
(are you properly rebasing to the latest libvirt.git?), but the conflict
resolution in qemu_driver.c seemed pretty trivial.  You also failed
'make syntax-check':

flags_usage
src/cpu/cpu_arm.c:48:  unsigned int flags ATTRIBUTE_UNUSED)
src/cpu/cpu_generic.c:117:unsigned int flags
ATTRIBUTE_UNUSED)
src/cpu/cpu_powerpc.c:308:  unsigned int flags ATTRIBUTE_UNUSED)
src/cpu/cpu_powerpc.c:382:unsigned int flags ATTRIBUTE_UNUSED)
src/cpu/cpu_s390.c:52:   unsigned int flags ATTRIBUTE_UNUSED)
maint.mk: flags should be checked with virCheckFlags

When adding a flags parameter, we intentionally update callers to
explicitly check for a 0 argument if they are unprepared to handle
non-zero flags; this leaves the door open for extensions that use new
flag bits, rather than being stuck with undefined behavior when passing
a new bit to an old binary.  So, all of these sites need to use either
virCheckFlags(0, NULL) to state they don't handle the flag, or
virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE, NULL) to state
that the flag is valid (and does not impact the output for that
architecture).

  include/libvirt/libvirt.h.in|9 ++

Needs a corresponding doc patch in src/libvirt.c that mentions use of
the new flag.

  tools/virsh-domain.c|   11 ++-

Likewise, this needs a corresponding patch to tools/virsh.pod to update
the man page to mention the new virsh option.

 +++ b/include/libvirt/libvirt.h.in
 @@ -3890,6 +3890,15 @@ int virConnectCompareCPU(virConnectPtr conn,
  
  
  /**
 + * virConnectBaselineCPUFlags
 + *
 + * Flags when getting XML description of a computed CPU
 + */
 +typedef enum {
 +VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE  = (1  0),  /* show model 
 features*/

Space before */

I know this is the name Dan suggested, but I can't help but wonder if
_EXPAND_FEATURES sounds nicer than EXPAND_FEATURE, since pretty much
every cpu name has more than one feature listed when expanded.

 +++ b/src/cpu/cpu.c
 @@ -167,7 +167,7 @@ cpuDecode(virCPUDefPtr cpu,
  return -1;
  }
  
 -return driver-decode(cpu, data, models, nmodels, preferred);
 +return driver-decode(cpu, data, models, nmodels, preferred, 0);
  }
  
  
 @@ -277,7 +277,8 @@ char *
  cpuBaselineXML(const char **xmlCPUs,
 unsigned int ncpus,
 const char **models,
 -   unsigned int nmodels)
 +   unsigned int nmodels,
 +   unsigned int flags)
  {

Needs to make sure we reject invalid flags:
virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE, -1)

 +++ b/src/cpu/cpu.h
 @@ -49,7 +49,8 @@ typedef int
   const union cpuData *data,
   const char **models,
   unsigned int nmodels,
 - const char *preferred);
 + const char *preferred,
 + unsigned int flags);
  
  typedef int
  (*cpuArchEncode)(const virCPUDefPtr cpu,
 @@ -76,7 +77,8 @@ typedef virCPUDefPtr
  (*cpuArchBaseline)  (virCPUDefPtr *cpus,
   unsigned int ncpus,
   const char **models,
 - unsigned int nmodels);
 + unsigned int nmodels,
 + unsigned int /* flags */);

No need to comment out the parameter name (multiple instances */.

 +++ b/src/cpu/cpu_x86.c
 @@ -1296,13 +1296,46 @@ x86GuestData(virCPUDefPtr host,
  return x86Compute(host, guest, data, message);
  }
  
 +static int
 +x86AddFeatures(virCPUDefPtr cpu,
 +   struct x86_map *map)
 +{
 +const struct x86_model *candidate;
 +const struct x86_feature *feature = map-features;
 +
 +candidate = map-models;
 +while (candidate != NULL) {
 +if (STREQ(cpu-model, candidate-name))
 +break;
 +candidate = candidate-next;
 +}
 +if (!candidate) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   %s not a known CPU model\n, cpu-model);
 +return -1;
 +}
 +while (feature != NULL) {
 +if (x86DataIsSubset(candidate-data, feature-data)) {
 +if (virCPUDefAddFeature(cpu, feature-name, 
 VIR_CPU_FEATURE_REQUIRE)  0) {

This function already outputs a decent error message on failure...

 +  

Re: [libvirt] [PATCH v2] Add flag to BaselineCPU API to return detailed CPU features

2013-07-30 Thread Eric Blake
On 07/30/2013 03:59 PM, Eric Blake wrote:
 On 07/30/2013 03:09 PM, Don Dugger wrote:
 [V2 - per review comments change the flag name to be more
 descriptive and change errors from warnings to propogated
 errors.]
 
 This aside belongs...
 

 Currently the virConnectBaselineCPU API does not expose the CPU features
 that are part of the CPU's model.  This patch adds a new flag,
 VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE, that causes the API to explictly
 list all features that are part of that model.

 Signed-off-by: Don Dugger donald.d.dug...@intel.com
 ---
 
 ...here, after the '---', so that 'git am' won't turn it into a
 permanent part of libvirt.git history.  Your patch didn't apply as is
 (are you properly rebasing to the latest libvirt.git?), but the conflict
 resolution in qemu_driver.c seemed pretty trivial.  You also failed
 'make syntax-check':

You also failed regular 'make':

  CC   libvirt_cpu_la-cpu_x86.lo
cpu/cpu_x86.c: In function 'x86DecodeCPUData':
cpu/cpu_x86.c:1467:5: error: too few arguments to function 'x86Decode'
 return x86Decode(cpu, data-data.x86, models, nmodels, preferred);
 ^
cpu/cpu_x86.c:1356:1: note: declared here
 x86Decode(virCPUDefPtr cpu,
 ^
cpu/cpu_x86.c: At top level:
cpu/cpu_x86.c:1948:5: error: initialization from incompatible pointer
type [-Werror]
 .decode = x86DecodeCPUData,
 ^
cpu/cpu_x86.c:1948:5: error: (near initialization for
'cpuDriverX86.decode') [-Werror]
cpu/cpu_x86.c: In function 'x86DecodeCPUData':
cpu/cpu_x86.c:1468:1: error: control reaches end of non-void function
[-Werror=return-type]
 }
 ^

-- 
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 V5] add console support in libxl

2013-07-30 Thread Jim Fehlig

On 07/26/2013 10:46 AM, Jim Fehlig wrote:

Bamvor Jian Zhang wrote:

this patch introduce the console api in libxl driver for both pv and
hvm guest.  and import and update the libxlMakeChrdevStr function
which was deleted in commit dfa1e1dd.
   

ACK, looks good to me now.  Unless there are other review comments, I'll
commit this once 1.1.1 is released.


Pushed now that 1.1.1 is released.  Thanks!

Regards,
Jim

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


Re: [libvirt] [PATCH v2] Add flag to BaselineCPU API to return detailed CPU features

2013-07-30 Thread Don Dugger
Sigh.  Rebase, I've heard of that, maybe I should remember to do it :-(

The failures are related to the rebasing, I'm working on that and
I'll incorporate your other fixes (with a couple of comments in line).

Only open issue is the flag name, I have no problems with
_EXPAND_FEATURES, unless there's an objection I'll do that.

On Tue, Jul 30, 2013 at 03:59:38PM -0600, Eric Blake wrote:
...
 
  +++ b/src/cpu/cpu.h
  @@ -49,7 +49,8 @@ typedef int
const union cpuData *data,
const char **models,
unsigned int nmodels,
  - const char *preferred);
  + const char *preferred,
  + unsigned int flags);
   
   typedef int
   (*cpuArchEncode)(const virCPUDefPtr cpu,
  @@ -76,7 +77,8 @@ typedef virCPUDefPtr
   (*cpuArchBaseline)  (virCPUDefPtr *cpus,
unsigned int ncpus,
const char **models,
  - unsigned int nmodels);
  + unsigned int nmodels,
  + unsigned int /* flags */);
 
 No need to comment out the parameter name (multiple instances */.

This was to avoid compile failures with unused parameters but
this becomes moot when I put in the code to explicitly check
that flag.

...

  @@ -1383,6 +1416,9 @@ x86Decode(virCPUDefPtr cpu,
   goto out;
   }
   
  +if (flags  VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE)
  +if (x86AddFeatures(cpuModel, map)  0)
  +goto out;
 
 You could write this with fewer nested if:
 
 if ((flags  VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE) 
 x86AddFeatures(cpuModel, map)  0)
 goto out;

I actually prefer the nested ifs over the logical expression (to
me that's more obvious) but I don't mind changing if that's your
preferred coding style.

...
  @@ -6049,7 +6057,8 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd)
   list[i] = vshStrdup(ctl, (const char *)xmlBufferContent(xml_buf));
   }
   
  -result = virConnectBaselineCPU(ctl-conn, list, count, 0);
  +result = virConnectBaselineCPU(ctl-conn, list, count, flags);
  +vshPrint(ctl, result - %p\n, result);
 
 Leftover debugging?

Yep, shoot me, I should know better.  Sorry about that.

-- 
Don Dugger
Censeo Toto nos in Kansa esse decisse. - D. Gale
n0...@n0ano.com
Ph: 303/443-3786

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


Re: [libvirt] [PATCH] Rename VIR_DOMAIN_PAUSED_GUEST_PANICKED to VIR_DOMAIN_PAUSED_CRASHED

2013-07-30 Thread chenfan
On Mon, 2013-07-29 at 17:56 +0100, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 The VIR_DOMAIN_PAUSED_GUEST_PANICKED constant is badly named,
 leaking the QEMU event name. Elsewhere in the API we use
 'CRASHED' rather than 'PANICKED', and the addition of 'GUEST'
 is redundant since all events are guest related.
 
 Thus rename it to VIR_DOMAIN_PAUSED_CRASHED, which matches
 with VIR_DOMAIN_RUNNING_CRASHED and VIR_DOMAIN_EVENT_CRASHED.
 
 It was added in commit 14e7e0ae8db9843aea80245a3d9e6cf5f2ef720d
 which post-dates v1.1.0, so is safe to rename before 1.1.1
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com

ACK.

Thanks,
Chen

 ---
  include/libvirt/libvirt.h.in | 2 +-
  src/qemu/qemu_monitor.c  | 2 +-
  src/qemu/qemu_process.c  | 2 +-
  tools/virsh-domain-monitor.c | 4 ++--
  4 files changed, 5 insertions(+), 5 deletions(-)
 
 diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
 index 4eae7bf..7bd3559 100644
 --- a/include/libvirt/libvirt.h.in
 +++ b/include/libvirt/libvirt.h.in
 @@ -181,7 +181,7 @@ typedef enum {
  VIR_DOMAIN_PAUSED_FROM_SNAPSHOT = 7, /* paused after restoring from 
 snapshot */
  VIR_DOMAIN_PAUSED_SHUTTING_DOWN = 8, /* paused during shutdown process */
  VIR_DOMAIN_PAUSED_SNAPSHOT = 9,  /* paused while creating a snapshot 
 */
 -VIR_DOMAIN_PAUSED_GUEST_PANICKED = 10, /* paused due to a guest panicked 
 event */
 +VIR_DOMAIN_PAUSED_CRASHED = 10, /* paused due to a guest crash */
  
  #ifdef VIR_ENUM_SENTINELS
  VIR_DOMAIN_PAUSED_LAST
 diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
 index 0b73411..5b2fb04 100644
 --- a/src/qemu/qemu_monitor.c
 +++ b/src/qemu/qemu_monitor.c
 @@ -3335,7 +3335,7 @@ int qemuMonitorVMStatusToPausedReason(const char 
 *status)
  return VIR_DOMAIN_PAUSED_WATCHDOG;
  
  case QEMU_MONITOR_VM_STATUS_GUEST_PANICKED:
 -return VIR_DOMAIN_PAUSED_GUEST_PANICKED;
 +return VIR_DOMAIN_PAUSED_CRASHED;
  
  /* unreachable from this point on */
  case QEMU_MONITOR_VM_STATUS_LAST:
 diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
 index e8e459e..d631a6f 100644
 --- a/src/qemu/qemu_process.c
 +++ b/src/qemu/qemu_process.c
 @@ -2724,7 +2724,7 @@ qemuProcessUpdateState(virQEMUDriverPtr driver, 
 virDomainObjPtr vm)
  newState = VIR_DOMAIN_SHUTDOWN;
  newReason = VIR_DOMAIN_SHUTDOWN_UNKNOWN;
  ignore_value(VIR_STRDUP_QUIET(msg, shutdown));
 -} else if (reason == VIR_DOMAIN_PAUSED_GUEST_PANICKED) {
 +} else if (reason == VIR_DOMAIN_PAUSED_CRASHED) {
  newState = VIR_DOMAIN_CRASHED;
  newReason = VIR_DOMAIN_CRASHED_PANICKED;
  ignore_value(VIR_STRDUP_QUIET(msg, crashed));
 diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
 index 98fe7fe..b29b82a 100644
 --- a/tools/virsh-domain-monitor.c
 +++ b/tools/virsh-domain-monitor.c
 @@ -228,8 +228,8 @@ vshDomainStateReasonToString(int state, int reason)
  return N_(shutting down);
  case VIR_DOMAIN_PAUSED_SNAPSHOT:
  return N_(creating snapshot);
 -case VIR_DOMAIN_PAUSED_GUEST_PANICKED:
 -return N_(guest panicked);
 +case VIR_DOMAIN_PAUSED_CRASHED:
 +return N_(crashed);
  case VIR_DOMAIN_PAUSED_UNKNOWN:
  case VIR_DOMAIN_PAUSED_LAST:
  ;


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


  1   2   >