[libvirt] [PATCH]lxc: improve readability of lxcContainer[Send|Waitfor]Continue
From: Chen Hanxiao chenhanx...@cn.fujitsu.com Currently, lxcContainer[Send|Waitfor]Continue only tell us 'fd', but we had to deal with the interaction between lxc_[container|controller|process]. This patch adds parameters to identify the caller. Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- src/lxc/lxc_container.c | 16 +--- src/lxc/lxc_container.h | 4 ++-- src/lxc/lxc_controller.c | 6 +++--- src/lxc/lxc_process.c| 2 +- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index ed1fe29..c7371ca 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -351,19 +351,20 @@ cleanup: /** * lxcContainerSendContinue: * @control: control FD to child + * @caller: function invoker * * Sends the continue message via the socket pair stored in the vm * structure. * * Returns 0 on success or -1 in case of error */ -int lxcContainerSendContinue(int control) +int lxcContainerSendContinue(int control, const char *caller) { int rc = -1; lxc_message_t msg = LXC_CONTINUE_MSG; int writeCount = 0; -VIR_DEBUG(Send continue on fd %d, control); +VIR_DEBUG(Send continue on fd %d by %s, control, caller); writeCount = safewrite(control, msg, sizeof(msg)); if (writeCount != sizeof(msg)) { goto error_out; @@ -377,6 +378,7 @@ error_out: /** * lxcContainerWaitForContinue: * @control: Control FD from parent + * @caller: function invoker * * This function will wait for the container continue message from the * parent process. It will send this message on the socket pair stored in @@ -384,14 +386,14 @@ error_out: * * Returns 0 on success or -1 in case of error */ -int lxcContainerWaitForContinue(int control) +int lxcContainerWaitForContinue(int control, const char *caller) { lxc_message_t msg; int readLen; -VIR_DEBUG(Wait continue on fd %d, control); +VIR_DEBUG(%s wait continue on fd %d, caller, control); readLen = saferead(control, msg, sizeof(msg)); -VIR_DEBUG(Got continue on fd %d %d, control, readLen); +VIR_DEBUG(%s got continue on fd %d %d, caller, control, readLen); if (readLen != sizeof(msg)) { if (readLen = 0) errno = EIO; @@ -1813,7 +1815,7 @@ static int lxcContainerChild(void *data) /* Wait for controller to finish setup tasks, including * things like move of network interfaces, uid/gid mapping */ -if (lxcContainerWaitForContinue(argv-monitor) 0) { +if (lxcContainerWaitForContinue(argv-monitor, __func__) 0) { virReportSystemError(errno, %s, _(Failed to read the container continue message)); goto cleanup; @@ -1880,7 +1882,7 @@ static int lxcContainerChild(void *data) if (lxcContainerDropCapabilities(!!hasReboot) 0) goto cleanup; -if (lxcContainerSendContinue(argv-handshakefd) 0) { +if (lxcContainerSendContinue(argv-handshakefd, __func__) 0) { virReportSystemError(errno, %s, _(Failed to send continue signal to controller)); goto cleanup; diff --git a/src/lxc/lxc_container.h b/src/lxc/lxc_container.h index e74a7d7..77fb2a0 100644 --- a/src/lxc/lxc_container.h +++ b/src/lxc/lxc_container.h @@ -49,8 +49,8 @@ enum { # define LXC_DEV_MAJ_FUSE10 # define LXC_DEV_MIN_FUSE229 -int lxcContainerSendContinue(int control); -int lxcContainerWaitForContinue(int control); +int lxcContainerSendContinue(int control, const char *caller); +int lxcContainerWaitForContinue(int control, const char *caller); int lxcContainerStart(virDomainDefPtr def, virSecurityManagerPtr securityDriver, diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 851a773..25dbbec 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -319,7 +319,7 @@ static int virLXCControllerConsoleSetNonblocking(virLXCControllerConsolePtr cons static int virLXCControllerDaemonHandshake(virLXCControllerPtr ctrl) { -if (lxcContainerSendContinue(ctrl-handshakeFd) 0) { +if (lxcContainerSendContinue(ctrl-handshakeFd, __func__) 0) { virReportSystemError(errno, %s, _(error sending continue signal to daemon)); return -1; @@ -2176,13 +2176,13 @@ virLXCControllerRun(virLXCControllerPtr ctrl) if (virLXCControllerMoveInterfaces(ctrl) 0) goto cleanup; -if (lxcContainerSendContinue(control[0]) 0) { +if (lxcContainerSendContinue(control[0], __func__) 0) { virReportSystemError(errno, %s, _(Unable to send container continue message)); goto cleanup; } -if (lxcContainerWaitForContinue(containerhandshake[0]) 0) { +if (lxcContainerWaitForContinue(containerhandshake[0], __func__) 0) { virReportSystemError(errno, %s, _(error receiving signal from
Re: [libvirt] [PATCHv2 15/23] qemu: Clean up check of maximum cpu count supported by a machine type
On 10/15/13 15:33, Daniel P. Berrange wrote: On Tue, Oct 15, 2013 at 02:30:44PM +0200, Peter Krempa wrote: Reword the error message and change formating to avoid exceeding 80 columns of text. --- src/qemu/qemu_process.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) ... To be honest I'm not really a fan of making these kind of changes. IMHO 80 column limit is something that shouldn't be strictly enforced when it causes splitting of messages which are only marginally over. I definitely agree. Splitting lines just due to this historic limitation seemed strange to me especially when it hinders code readability (especially in condition statements). Dropping this patch. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-glib PATCH] Add filterref and filterref parameter support.
On Tue, Oct 15, 2013 at 12:05:02PM -0700, Ian Main wrote: This patch adds support for setting filterref's on interfaces. Also supported are parameters to the filterref's. This mostly looks good, some comments below. Signed-off-by: Ian Main im...@redhat.com --- examples/config-demo.py| 9 +- libvirt-gconfig/Makefile.am| 4 + ...-gconfig-domain-interface-filterref-parameter.c | 101 ++ ...-gconfig-domain-interface-filterref-parameter.h | 75 ++ .../libvirt-gconfig-domain-interface-filterref.c | 155 + .../libvirt-gconfig-domain-interface-filterref.h | 76 ++ libvirt-gconfig/libvirt-gconfig-domain-interface.c | 41 ++ libvirt-gconfig/libvirt-gconfig-domain-interface.h | 5 + libvirt-gconfig/libvirt-gconfig.h | 2 + libvirt-gconfig/libvirt-gconfig.sym| 18 +++ 10 files changed, 485 insertions(+), 1 deletion(-) create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-interface-filterref-parameter.c create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-interface-filterref-parameter.h create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-interface-filterref.c create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-interface-filterref.h diff --git a/examples/config-demo.py b/examples/config-demo.py index 09b9e89..0566d66 100644 --- a/examples/config-demo.py +++ b/examples/config-demo.py @@ -28,13 +28,20 @@ disk.set_type(LibvirtGConfig.DomainDiskType.FILE) disk.set_guest_device_type(LibvirtGConfig.DomainDiskGuestDeviceType.DISK) disk.set_source(/tmp/foo/bar) disk.set_driver_name(qemu) -disk.set_driver_format(LibvirtGConfig.DriverType.QCOW2) +disk.set_driver_format(LibvirtGConfig.DomainDiskFormat.QCOW2) Can you split that one line change to a separate patch? disk.set_target_bus(LibvirtGConfig.DomainDiskBus.IDE) disk.set_target_dev(hda) domain.add_device(disk) interface = LibvirtGConfig.DomainInterfaceNetwork.new() interface.set_source(default) +filterref = LibvirtGConfig.DomainInterfaceFilterref.new() +filterref.set_filter(clean-traffic) +parameter = LibvirtGConfig.DomainInterfaceFilterrefParameter.new() +parameter.set_name(IP) +parameter.set_value(205.23.12.40) +filterref.add_parameter(parameter) +interface.set_filterref(filterref) domain.add_device(interface) interface = LibvirtGConfig.DomainInterfaceUser.new() diff --git a/libvirt-gconfig/Makefile.am b/libvirt-gconfig/Makefile.am index 35dc978..48f0547 100644 --- a/libvirt-gconfig/Makefile.am +++ b/libvirt-gconfig/Makefile.am @@ -49,6 +49,8 @@ GCONFIG_HEADER_FILES = \ libvirt-gconfig-domain-interface-bridge.h \ libvirt-gconfig-domain-interface-network.h \ libvirt-gconfig-domain-interface-user.h \ + libvirt-gconfig-domain-interface-filterref.h \ + libvirt-gconfig-domain-interface-filterref-parameter.h \ Can you move it right after -bridge.h so that it's alphabetically sorted? libvirt-gconfig-domain-memballoon.h \ libvirt-gconfig-domain-os.h \ libvirt-gconfig-domain-parallel.h \ @@ -131,6 +133,8 @@ GCONFIG_SOURCE_FILES = \ libvirt-gconfig-domain-interface-bridge.c \ libvirt-gconfig-domain-interface-network.c \ libvirt-gconfig-domain-interface-user.c \ + libvirt-gconfig-domain-interface-filterref.c \ + libvirt-gconfig-domain-interface-filterref-parameter.c \ Same here. libvirt-gconfig-domain-memballoon.c \ libvirt-gconfig-domain-os.c \ libvirt-gconfig-domain-parallel.c \ diff --git a/libvirt-gconfig/libvirt-gconfig-domain-interface-filterref-parameter.c b/libvirt-gconfig/libvirt-gconfig-domain-interface-filterref-parameter.c new file mode 100644 index 000..e697e86 --- /dev/null +++ b/libvirt-gconfig/libvirt-gconfig-domain-interface-filterref-parameter.c @@ -0,0 +1,101 @@ +/* + * libvirt-gconfig-domain-interface-filterref-parameter.c: + * libvirt filterref parameters + * + * 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
Re: [libvirt] [PATCHv2 16/23] cpu: Export few x86-specific APIs
On 10/15/13 15:37, Daniel P. Berrange wrote: On Tue, Oct 15, 2013 at 02:30:45PM +0200, Peter Krempa wrote: From: Jiri Denemark jdene...@redhat.com This makes virCPUx86DataAddCPUID, virCPUx86DataFree, and virCPUx86MakeData available for direct usage outside of cpu driver. Presumably these are exported for use by test suites ? If so then I think we'd be better having them in a cpu_x86_priv.h to make it clear they're not actually part of the main API. The json monitor code will use those too, so we should export them in a normal fashion. Peter 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] storage: document existing pools
On 10/16/2013 01:18 AM, Eric Blake wrote: We forgot to document several pool types. Furthermore, the RNG grammar did not allow arbitrary interleaving, which makes it harder than necessary to create a new pool from XML. * docs/formatstorage.html.in: Add docs for scsi, mpath, rbd, and sheepdog. * docs/schemas/storagepool.rng: Allow interleaving. * tests/storagepoolxml2xmlin/pool-sheepdog.xml: Test interleave. * tests/storagepoolxml2xmlin/pool-iscsi-auth.xml: Likewise. These would fit better in a separate commit IMO. Signed-off-by: Eric Blake ebl...@redhat.com --- docs/formatstorage.html.in | 44 +-- docs/schemas/storagepool.rng | 356 ++--- tests/storagepoolxml2xmlin/pool-iscsi-auth.xml | 12 +- tests/storagepoolxml2xmlin/pool-sheepdog.xml | 6 +- 4 files changed, 238 insertions(+), 180 deletions(-) @@ -128,18 +134,19 @@ dtcodeauth/code/dt ddIf present, the codeauth/code element provides the authentication credentials needed to access the source by the -setting of the codetype/code attribute. The codetype/code -must be either chap or ceph. Additionally a mandatory attribute +setting of the codetype/code attribute (iscsi, +rbd). The codetype/code In this case the sentence wasn't mentioning pools. Would (for iscsi and rbd pools) be more clear? And/or wrap all these pool names in code? +must be either chap or ceph. Use ceph for +Ceph RBD (Rados Block Device) network sources and use iscsi for CHAP +(Challenge-Handshake Authentication Protocol) iSCSI +targets. Additionally a mandatory attribute codeusername/code identifies the username to use during authentication as well as a sub-element codesecret/code with ACK either way. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/6 v2] Add virFileIsMountPoint function
From: Daniel P. Berrange berra...@redhat.com Add a function for efficiently checking if a path is a filesystem mount point. NB will not work for bind mounts, only true filesystem mounts. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/libvirt_private.syms | 1 + src/util/virfile.c | 50 src/util/virfile.h | 2 ++ 3 files changed, 53 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 84c1c28..fff91ca 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1187,6 +1187,7 @@ virFileIsAbsPath; virFileIsDir; virFileIsExecutable; virFileIsLink; +virFileIsMountPoint; virFileLinkPointsTo; virFileLock; virFileLoopDeviceAssociate; diff --git a/src/util/virfile.c b/src/util/virfile.c index e10de5a..b93fa87 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -1513,6 +1513,56 @@ virFileIsExecutable(const char *file) return false; } + +/* + * Check that a file refers to a mount point. Trick is that for + * a mount point, the st_dev field will differ from the parent + * directory. + * + * Note that this will not detect bind mounts of dirs/files, + * only true filesystem mounts. + */ +int virFileIsMountPoint(const char *file) +{ +char *parent = NULL; +int ret = -1; +struct stat sb1, sb2; + +if (!(parent = mdir_name(file))) { +virReportOOMError(); +goto cleanup; +} + +VIR_DEBUG(Comparing '%s' to '%s', file, parent); + +if (stat(file, sb1) 0) { +if (errno == ENOENT) +ret = 0; +else +virReportSystemError(errno, + _(Cannot stat '%s'), + file); +goto cleanup; +} + +if (stat(parent, sb2) 0) { +virReportSystemError(errno, + _(Cannot stat '%s'), + parent); +goto cleanup; +} + +if (!S_ISDIR(sb1.st_mode)) +return false; + +ret = sb1.st_dev != sb2.st_dev; +VIR_DEBUG(Is mount %d, ret); + + cleanup: +VIR_FREE(parent); +return ret; +} + #ifndef WIN32 /* Check that a file is accessible under certain * user gid. diff --git a/src/util/virfile.h b/src/util/virfile.h index 72d35ce..ff84719 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -156,6 +156,8 @@ bool virFileIsDir (const char *file) ATTRIBUTE_NONNULL(1); bool virFileExists(const char *file) ATTRIBUTE_NONNULL(1); bool virFileIsExecutable(const char *file) ATTRIBUTE_NONNULL(1); +int virFileIsMountPoint(const char *file) ATTRIBUTE_NONNULL(1); + char *virFileSanitizePath(const char *path); enum { -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/6] Skip any files which are not mounted on the host
On Tue, Oct 08, 2013 at 09:24:50PM -0600, Eric Blake wrote: On 10/07/2013 07:06 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Currently the LXC container tries to skip selinux/securityfs mounts if the directory does not exist in the filesystem, or if SELinux is disabled. The former check is flawed because the /sys/fs/selinux or /sys/kernel/securityfs directories may exist in sysfs even if the mount type is disabled. Instead of just doing an access() check, use an virFileIsMounted() to see if the FS is actually present in the host OS. This also avoids the need to check is_selinux_enabled(). Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/lxc/lxc_container.c | 37 +++-- 1 file changed, 23 insertions(+), 14 deletions(-) static const virLXCBasicMountInfo lxcBasicMounts[] = { -{ proc, /proc, proc, MS_NOSUID|MS_NOEXEC|MS_NODEV, false }, -{ /proc/sys, /proc/sys, NULL, MS_BIND|MS_RDONLY, false }, -{ sysfs, /sys, sysfs, MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, false }, -{ securityfs, /sys/kernel/security, securityfs, MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, true }, +{ proc, /proc, proc, MS_NOSUID|MS_NOEXEC|MS_NODEV, false, false }, +{ /proc/sys, /proc/sys, NULL, MS_BIND|MS_RDONLY, false, false }, +{ sysfs, /sys, sysfs, MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, false, false }, +{ securityfs, /sys/kernel/security, securityfs, MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, true, true }, #if WITH_SELINUX -{ SELINUX_MOUNT, SELINUX_MOUNT, selinuxfs, MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, true }, +{ SELINUX_MOUNT, SELINUX_MOUNT, selinuxfs, MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, true, true }, All your bools are either both false or both true; does it make sense to consolidate them into one, or will a future patch expose a need for a different combo? I don't have a pending use for it, but logically I just prefer to keep it separate. 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] build: use the gnulib version of the .m4 files when present
Giuseppe Scrivano gscri...@redhat.com writes: prevent aclocal to prefer .m4 files under m4/ to the version provided by gnulib. I have noticed this after './configure --help' gave me two different versions of --enable-threads. This was caused by aclocal that preferred the version of lock.m4 provided by autopoint instead of using the newer version distributed with gnulib. Signed-off-by: Giuseppe Scrivano gscri...@redhat.com ping -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] Introduce new OOM testing support
From: Daniel P. Berrange berra...@redhat.com The previous OOM testing support would re-run the entire main method each iteration, failing a different malloc each time. When a test suite has 'n' allocations, the number of repeats requires is (n * (n + 1) ) / 2. This gets very large, very quickly. This new OOM testing support instead integrates at the virtTestRun level, so each individual test case gets repeated, instead of the entire test suite. This means the values of 'n' are orders of magnitude smaller. The simple usage is $ VIR_TEST_OOM=1 ./qemuxml2argvtest ... 29) QEMU XML-2-ARGV clock-utc ... OK Test OOM for nalloc=36 OK 30) QEMU XML-2-ARGV clock-localtime ... OK Test OOM for nalloc=36 OK 31) QEMU XML-2-ARGV clock-france ... OK Test OOM for nalloc=38 .. OK ... the second lines reports how many mallocs have to be failed, and thus how many repeats of the test will be run. If it crashes, then running under valgrind will often show the problem $ VIR_TEST_OOM=1 ../run valgrind ./qemuxml2argvtest When debugging problems it is also helpful to select an individual test case $ VIR_TEST_RANGE=30 VIR_TEST_OOM=1 ../run valgrind ./qemuxml2argvtest When things get really tricky, it is possible to request that just specific allocs are failed. eg to fail allocs 5 - 12, use $ VIR_TEST_RANGE=30 VIR_TEST_OOM=1:5-12 ../run valgrind ./qemuxml2argvtest In the worse case, you might want to know the stack trace of the alloc which was failed then VIR_TEST_OOM_TRACE can be set. If it is set to 1 then it will only print if it thinks a mistake happened. This is often not reliable, so setting it to 2 will make it print the stack trace for every alloc that is failed. $ VIR_TEST_OOM_TRACE=2 VIR_TEST_RANGE=30 VIR_TEST_OOM=1:5-5 ../run valgrind ./qemuxml2argvtest 30) QEMU XML-2-ARGV clock-localtime ... OK Test OOM for nalloc=36 !virAllocN /home/berrange/src/virt/libvirt/src/util/viralloc.c:180 virHashCreateFull /home/berrange/src/virt/libvirt/src/util/virhash.c:144 virDomainDefParseXML /home/berrange/src/virt/libvirt/src/conf/domain_conf.c:11745 virDomainDefParseNode /home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12646 virDomainDefParse /home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12590 testCompareXMLToArgvFiles /home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:106 virtTestRun /home/berrange/src/virt/libvirt/tests/testutils.c:250 mymain /home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:418 (discriminator 2) virtTestMain /home/berrange/src/virt/libvirt/tests/testutils.c:750 ?? ??:0 _start ??:? FAILED Signed-off-by: Daniel P. Berrange berra...@redhat.com --- docs/internals/oomtesting.html.in | 213 ++ docs/sitemap.html.in | 4 + tests/qemuargv2xmltest.c | 14 +-- tests/qemuxml2argvtest.c | 12 ++- tests/qemuxmlnstest.c | 18 ++-- tests/testutils.c | 195 +- tests/testutils.h | 2 + 7 files changed, 437 insertions(+), 21 deletions(-) create mode 100644 docs/internals/oomtesting.html.in diff --git a/docs/internals/oomtesting.html.in b/docs/internals/oomtesting.html.in new file mode 100644 index 000..652aff6 --- /dev/null +++ b/docs/internals/oomtesting.html.in @@ -0,0 +1,213 @@ +?xml version=1.0 encoding=UTF-8? +!DOCTYPE html PUBLIC -//W3C//DTD XHTML 1.0 Strict//EN http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd; +html xmlns=http://www.w3.org/1999/xhtml; + body +h1Out of memory testing/h1 + +ul id=toc/ul + + +p + This page describes how to use the test suite todo out of memory + testing. +/p + +h2Building with OOM testing/h2 + +p + Since OOM testing requires hooking into the malloc APIs, it is + not enabled by default. The flag code--enable-test-oom/code + must be given to codeconfigure/code. When this is done the + libvirt allocation APIs will have some hooks enabled. +/p + +pre +$ ./configure --enable-test-oom +/pre + + +h2a name=basicoomBasic OOM testing support/a/h2 + +p + The first step in validating OOM usage is to run a test suite + with full OOM testing enabled. This is done by setting the + codeVIR_TEST_OOM=1/code environment variable. The way this + works is that it runs the test once normally to prime any + static memory allocations. Then it runs it once more counting + the total number of memory allocations. Then it runs it in a + loop failing a different memory allocation each time. For every + memory allocation failure triggered, it expects the test case +
[libvirt] [PATCH] qemu: Fix the wrong logic of building cmd line for volume type disk
It builds a libiscsi uri if the volume type is file and disk-auth.username is specified, but I even see no reason to build the libiscsi uri for a file type volume, and the logic to check whether disk-auth.username is specified is also beyond understanding. It produces unexpected result like below: Disk config (pool is of 'fs' type): disk type='volume' device='disk' driver name='qemu' type='raw'/ auth username='myuser' /auth source pool='default' volume='f18.img'/ target dev='sda' bus='scsi'/ address type='drive' controller='0' bus='0' target='0' unit='0'/ /disk % virsh start f18 error: Failed to start domain f18 error: internal error: iscsi accepts only one host --- src/qemu/qemu_command.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index abb62e9..9b6a27d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3806,13 +3806,7 @@ qemuBuildVolumeString(virConnectPtr conn, } break; case VIR_STORAGE_VOL_FILE: -if (disk-auth.username) { -if (qemuBuildISCSIString(conn, disk, opt) 0) -goto cleanup; -virBufferAddChar(opt, ','); -} else { -virBufferEscape(opt, ',', ,, file=%s,, disk-src); -} +virBufferEscape(opt, ',', ,, file=%s,, disk-src); break; case VIR_STORAGE_VOL_NETWORK: /* Keep the compiler quite, qemuTranslateDiskSourcePool already -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: use the gnulib version of the .m4 files when present
On Mon, Oct 07, 2013 at 11:44:45PM +0200, Giuseppe Scrivano wrote: prevent aclocal to prefer .m4 files under m4/ to the version provided by gnulib. I have noticed this after './configure --help' gave me two different versions of --enable-threads. This was caused by aclocal that preferred the version of lock.m4 provided by autopoint instead of using the newer version distributed with gnulib. Signed-off-by: Giuseppe Scrivano gscri...@redhat.com --- bootstrap.conf | 5 - 1 file changed, 5 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index 68c4a89..e7353e2 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -191,11 +191,6 @@ gnulib_tool_option_extras=\ local_gl_dir=gnulib/local -# Convince bootstrap to use multiple m4 directories. -: ${ACLOCAL=aclocal} -ACLOCAL=$ACLOCAL -I m4 -export ACLOCAL - Makefile.am has ACLOCAL_AMFLAGS = -I m4 -I gnulib/m4 and looking at what is run by autogen.sh with strace, it seems to use 'aclocal -I m4 -I gnulib/m4'. So this bootstrap.conf ACLOCAL setting doesn't appear to have any effect. None the less removing it does change the contents of 'configure' I think this needs Eric's review. 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] qemu: Fix the wrong logic of building cmd line for volume type disk
On Wed, Oct 16, 2013 at 08:18:28PM +0800, Osier Yang wrote: It builds a libiscsi uri if the volume type is file and disk-auth.username is specified, but I even see no reason to build the libiscsi uri for a file type volume, and the logic to check whether disk-auth.username is specified is also beyond understanding. It produces unexpected result like below: Disk config (pool is of 'fs' type): disk type='volume' device='disk' driver name='qemu' type='raw'/ auth username='myuser' /auth source pool='default' volume='f18.img'/ target dev='sda' bus='scsi'/ address type='drive' controller='0' bus='0' target='0' unit='0'/ /disk % virsh start f18 error: Failed to start domain f18 error: internal error: iscsi accepts only one host --- src/qemu/qemu_command.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) Test case 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 v2] rpc: Correct the wrong payload size checking
On Tue, Oct 15, 2013 at 11:15:32PM +0800, Osier Yang wrote: ... /* Size of message length field. Not counted in VIR_NET_MESSAGE_MAX * and VIR_NET_MESSAGE_INITIAL. */ const VIR_NET_MESSAGE_LEN_MAX = 4; /... However, msg-bufferLength includes the length word. The wrong checking was introduced by commit e914dcfd. * src/rpc/virnetmessage.c: - Correct the checking in virNetMessageEncodePayloadRaw - Use a new variable to track the new payload length in virNetMessageEncodePayloadRaw --- src/rpc/virnetmessage.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/rpc/virnetmessage.c b/src/rpc/virnetmessage.c index 8f4e4bc..d60366b 100644 --- a/src/rpc/virnetmessage.c +++ b/src/rpc/virnetmessage.c @@ -346,15 +346,16 @@ int virNetMessageEncodePayload(virNetMessagePtr msg, /* Try to encode the payload. If the buffer is too small increase it. */ while (!(*filter)(xdr, data)) { -if ((msg-bufferLength - VIR_NET_MESSAGE_LEN_MAX) * 4 VIR_NET_MESSAGE_MAX) { +unsigned int newlen = (msg-bufferLength - VIR_NET_MESSAGE_LEN_MAX) * 4; + +if (newlen VIR_NET_MESSAGE_MAX) { virReportError(VIR_ERR_RPC, %s, _(Unable to encode message payload)); goto error; } xdr_destroy(xdr); -msg-bufferLength = (msg-bufferLength - VIR_NET_MESSAGE_LEN_MAX) * 4 + -VIR_NET_MESSAGE_LEN_MAX; +msg-bufferLength = newlen + VIR_NET_MESSAGE_LEN_MAX; if (VIR_REALLOC_N(msg-buffer, msg-bufferLength) 0) goto error; @@ -426,10 +427,15 @@ int virNetMessageEncodePayloadRaw(virNetMessagePtr msg, /* If the message buffer is too small for the payload increase it accordingly. */ if ((msg-bufferLength - msg-bufferOffset) len) { -if ((msg-bufferOffset + len) VIR_NET_MESSAGE_MAX) { +if ((msg-bufferOffset + len) +(VIR_NET_MESSAGE_MAX + VIR_NET_MESSAGE_LEN_MAX)) { virReportError(VIR_ERR_RPC, - _(Stream data too long to send (%zu bytes needed, %zu bytes available)), - len, (VIR_NET_MESSAGE_MAX - msg-bufferOffset)); + _(Stream data too long to send + (%zu bytes needed, %zu bytes available)), + len, + VIR_NET_MESSAGE_MAX + + VIR_NET_MESSAGE_LEN_MAX - + msg-bufferOffset); return -1; } 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 v2] rpc: Correct the wrong payload size checking
On 16/10/13 20:31, Daniel P. Berrange wrote: On Tue, Oct 15, 2013 at 11:15:32PM +0800, Osier Yang wrote: ... /* Size of message length field. Not counted in VIR_NET_MESSAGE_MAX * and VIR_NET_MESSAGE_INITIAL. */ const VIR_NET_MESSAGE_LEN_MAX = 4; /... However, msg-bufferLength includes the length word. The wrong checking was introduced by commit e914dcfd. * src/rpc/virnetmessage.c: - Correct the checking in virNetMessageEncodePayloadRaw - Use a new variable to track the new payload length in virNetMessageEncodePayloadRaw --- src/rpc/virnetmessage.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/rpc/virnetmessage.c b/src/rpc/virnetmessage.c index 8f4e4bc..d60366b 100644 --- a/src/rpc/virnetmessage.c +++ b/src/rpc/virnetmessage.c @@ -346,15 +346,16 @@ int virNetMessageEncodePayload(virNetMessagePtr msg, /* Try to encode the payload. If the buffer is too small increase it. */ while (!(*filter)(xdr, data)) { -if ((msg-bufferLength - VIR_NET_MESSAGE_LEN_MAX) * 4 VIR_NET_MESSAGE_MAX) { +unsigned int newlen = (msg-bufferLength - VIR_NET_MESSAGE_LEN_MAX) * 4; + +if (newlen VIR_NET_MESSAGE_MAX) { virReportError(VIR_ERR_RPC, %s, _(Unable to encode message payload)); goto error; } xdr_destroy(xdr); -msg-bufferLength = (msg-bufferLength - VIR_NET_MESSAGE_LEN_MAX) * 4 + -VIR_NET_MESSAGE_LEN_MAX; +msg-bufferLength = newlen + VIR_NET_MESSAGE_LEN_MAX; if (VIR_REALLOC_N(msg-buffer, msg-bufferLength) 0) goto error; @@ -426,10 +427,15 @@ int virNetMessageEncodePayloadRaw(virNetMessagePtr msg, /* If the message buffer is too small for the payload increase it accordingly. */ if ((msg-bufferLength - msg-bufferOffset) len) { -if ((msg-bufferOffset + len) VIR_NET_MESSAGE_MAX) { +if ((msg-bufferOffset + len) +(VIR_NET_MESSAGE_MAX + VIR_NET_MESSAGE_LEN_MAX)) { virReportError(VIR_ERR_RPC, - _(Stream data too long to send (%zu bytes needed, %zu bytes available)), - len, (VIR_NET_MESSAGE_MAX - msg-bufferOffset)); + _(Stream data too long to send + (%zu bytes needed, %zu bytes available)), + len, + VIR_NET_MESSAGE_MAX + + VIR_NET_MESSAGE_LEN_MAX - + msg-bufferOffset); return -1; } ACK Pushed. Thanks. Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: use the gnulib version of the .m4 files when present
Daniel P. Berrange berra...@redhat.com writes: and looking at what is run by autogen.sh with strace, it seems to use 'aclocal -I m4 -I gnulib/m4'. So this bootstrap.conf ACLOCAL setting doesn't appear to have any effect. thanks to have checked it. It seems that using directly the bootstrap script makes a difference, I had assumed it doesn't; and also it makes a difference if I run git clean -fdx before autogen.sh (so maybe it is a separate issue?). I'll investigate this better if Eric hasn't already an answer. Regards, Giuseppe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] remote-driver: Fix 'leav' typo in comment
--- Pushed under the trivial rule. src/remote/remote_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 96ccb99..87ef5a9 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -3721,7 +3721,7 @@ static int remoteAuthCredSASL2Vir(int vircred) * * Build up the SASL callback structure. We register one callback for * each credential type that the libvirt client indicated they support. - * We explicitly leav the callback function pointer at NULL though, + * We explicitly leave the callback function pointer at NULL though, * because we don't actually want to get SASL callbacks triggered. * Instead, we want the start/step functions to return SASL_INTERACT. * This lets us give the libvirt client a list of all required -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: better error checking for LOCAL_PEERCRED
On 15/10/2013 12:16, Daniel P. Berrange wrote: Unfortunately your patch does not apply since your mail client has messed up line wrapping. Also there have been conflicting changes to the code since your patch. I would fix it myself, but I don't have ability to compile test code on BSD platforms. Can you update your patch re-send. Sorry about that. Updated patch uploaded to https://bugzilla.redhat.com/show_bug.cgi?id=1019929 where Thunderbird can't mangle it. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: document existing pools
On 10/16/2013 05:03 AM, Ján Tomko wrote: On 10/16/2013 01:18 AM, Eric Blake wrote: We forgot to document several pool types. Furthermore, the RNG grammar did not allow arbitrary interleaving, which makes it harder than necessary to create a new pool from XML. * docs/formatstorage.html.in: Add docs for scsi, mpath, rbd, and sheepdog. * docs/schemas/storagepool.rng: Allow interleaving. * tests/storagepoolxml2xmlin/pool-sheepdog.xml: Test interleave. * tests/storagepoolxml2xmlin/pool-iscsi-auth.xml: Likewise. These would fit better in a separate commit IMO. Indeed; I've split it into two. +setting of the codetype/code attribute (iscsi, +rbd). The codetype/code In this case the sentence wasn't mentioning pools. Would (for iscsi and rbd pools) be more clear? And/or wrap all these pool names in code? Good idea. I modified the patch to call out (pool types codex/code, codey/code, codez/code) in each situation. +must be either chap or ceph. Use ceph for +Ceph RBD (Rados Block Device) network sources and use iscsi for CHAP +(Challenge-Handshake Authentication Protocol) iSCSI +targets. Additionally a mandatory attribute codeusername/code identifies the username to use during authentication as well as a sub-element codesecret/code with ACK either way. Thanks; pushed with those improvements. -- 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] build: use the gnulib version of the .m4 files when present
On 10/16/2013 07:38 AM, Giuseppe Scrivano wrote: Daniel P. Berrange berra...@redhat.com writes: and looking at what is run by autogen.sh with strace, it seems to use 'aclocal -I m4 -I gnulib/m4'. So this bootstrap.conf ACLOCAL setting doesn't appear to have any effect. thanks to have checked it. It seems that using directly the bootstrap script makes a difference, I had assumed it doesn't; and also it makes a difference if I run git clean -fdx before autogen.sh (so maybe it is a separate issue?). I'll investigate this better if Eric hasn't already an answer. Looking at it now; but I think we can just go one step further, and use a single m4 directory for everything. Stay tuned... -- 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] build: use the gnulib version of the .m4 files when present
On 10/07/2013 03:44 PM, Giuseppe Scrivano wrote: prevent aclocal to prefer .m4 files under m4/ to the version provided by gnulib. s/to prefer/from preferring/; s/to the/over the/ I have noticed this after './configure --help' gave me two different versions of --enable-threads. This was caused by aclocal that preferred the version of lock.m4 provided by autopoint instead of using the newer version distributed with gnulib. Basically, xgettext was honoring configure's AC_CONFIG_MACRO_DIR designation of m4/ for where to stick the (often older) versions of its macros, while gnulib was honoring bootstrap.conf and trying to stick all gnulib macros into gnulib/m4/, and with lock.m4 installed into two locations, we get weird effects. Signed-off-by: Giuseppe Scrivano gscri...@redhat.com --- bootstrap.conf | 5 - 1 file changed, 5 deletions(-) I think we can go one step further, by squashing in this, to use only one directory in the first place (so no matter what xgettext installs, gnulib installs over the same location so we always end up with just one version of the file): diff --git i/bootstrap.conf w/bootstrap.conf index e7353e2..8b37217 100644 --- i/bootstrap.conf +++ w/bootstrap.conf @@ -175,11 +175,11 @@ fi # Tell gnulib to: # require LGPLv2+ # apply any local diffs in gnulib/local/ dir -# put *.m4 files in new gnulib/m4/ dir +# put *.m4 files in m4/ dir # put *.[ch] files in new gnulib/lib/ dir # import gnulib tests in new gnulib/tests/ dir gnulib_name=libgnu -m4_base=gnulib/m4 +m4_base=m4 source_base=gnulib/lib tests_base=gnulib/tests gnulib_tool_option_extras=\ diff --git i/Makefile.am w/Makefile.am index 66cb677..f3b5cd2 100644 --- i/Makefile.am +++ w/Makefile.am @@ -24,7 +24,7 @@ SUBDIRS = . gnulib/lib include src daemon tools docs gnulib/tests \ examples/dominfo examples/domsuspend examples/python examples/apparmor \ examples/xml/nwfilter examples/openauth examples/systemtap -ACLOCAL_AMFLAGS = -I m4 -I gnulib/m4 +ACLOCAL_AMFLAGS = -I m4 XML_EXAMPLES = \ $(patsubst $(srcdir)/%,%,$(wildcard $(addprefix $(srcdir)/examples/xml/, \ ACK with that change, and pushed on your behalf, after testing that 'make distcheck' still works with the new layout. -- 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]virsh: fix a typo in virsh-domain
From: Chen Hanxiao chenhanx...@cn.fujitsu.com s/it's/its Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- tools/virsh-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 6d241db..9017cac 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -3036,7 +3036,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) ctxt))) goto error; -/* tokenize the string from user and save it's parts into an array */ +/* tokenize the string from user and save its parts into an array */ if (vol_string (nvol_list = vshStringToArray(vol_string, vol_list)) 0) goto error; -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Migration issue php-libvirt
HI, I am using KVM Br. Umar On Tue, Oct 15, 2013 at 4:11 PM, Daniel P. Berrange berra...@redhat.comwrote: On Sun, Oct 13, 2013 at 03:20:31PM +0500, Umar Draz wrote: Hi All I am trying to migrate offline domain on other URI but its not working due to this error Failure!Libvirt last error: Requested operation is not valid: domain is not running I tried to use this option but not working VIR_MIGRATE_OFFLINE Please anybody help me? You don't mention what hypervisor you're using. IIRC only the QEMU driver supports the VIR_MIGRATE_OFFLINE flag. 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:| -- Umar Draz Network Architect -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH]virsh: improve usability of '--print-xml' flag for attach-disk command
From: Chen Hanxiao chenhanx...@cn.fujitsu.com '--print-xml' option is very useful for doing some test. But we had to specify a real domain for it. This patch could enable us to specify a fake domain when using --print-xml option. Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- tools/virsh-domain.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 2aed9f9..565966d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -528,13 +528,6 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) if (live) flags |= VIR_DOMAIN_AFFECT_LIVE; -if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) -return false; - -if (persistent -virDomainIsActive(dom) == 1) -flags |= VIR_DOMAIN_AFFECT_LIVE; - if (vshCommandOptStringReq(ctl, cmd, source, source) 0 || vshCommandOptStringReq(ctl, cmd, target, target) 0 || vshCommandOptStringReq(ctl, cmd, driver, driver) 0 || @@ -672,6 +665,13 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) goto cleanup; } +if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) +return false; + +if (persistent +virDomainIsActive(dom) == 1) +flags |= VIR_DOMAIN_AFFECT_LIVE; + if (flags) ret = virDomainAttachDeviceFlags(dom, xml, flags); else -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list