[libvirt] [PATCH v3 3/3] qemu: check presence of each disk and its backing file as well
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
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
- 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
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
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
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
- 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
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
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
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
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
--- 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
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
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
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
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
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
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()
--- 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
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
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
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
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
--- 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
--- 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
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
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
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
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
--- 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
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
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
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
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
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
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
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
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
--- 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
--- 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
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()
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
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
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
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
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
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
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
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
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
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
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
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
- 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
%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
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
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.
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
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
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
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
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
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
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
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
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
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
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
* 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
[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.
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.
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
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
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
[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.
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
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
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
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
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
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