May 2012 - Devel - libvirt List Archives

[libvirt] [PATCH v3 0/2] Rework RPC message buffer
by Michal Privoznik 05 Jun '12

05 Jun '12

This patch set tries to fix corner cases where libvirt runs on huge system, e.g. 4K CPU monster. In these cases, capabilities XML is enormously big, as we are transferring info about each singe CPU core (to which NUMA node it belongs, etc.). This XML is bigger than our RPC limit, therefore users cannot get it as it is dropped on server, leaving them with inability to connect. Therefore we need to increase those limits (whole RPC message and RPC string). However, simple lifting up will work, but increase mem usage. Therefore I've reworked RPC buffer handling: changed it from 'statically' to dynamically allocated. So in most cases - when small messages are sent - this will even decrease our memory consumption. Leaving us flexible for corner cases described above. On the other hand, I realize we've had our history with RPC breakage. So I think I'll require more than 1 ACK before pushing. (I guess Eric's ACK still holds) diff to v2: -suggestions from Eric's review included diff to v1: -couple of fixes (1/2) -increased other limits as well (2/2) Michal Privoznik (2): rpc: Switch to dynamically allocated message buffer rpc: Size up RPC limits src/libvirt.c | 2 + src/remote/remote_protocol.x | 20 +- src/rpc/virnetclient.c | 16 ++- src/rpc/virnetmessage.c | 12 ++- src/rpc/virnetmessage.h | 5 +- src/rpc/virnetprotocol.x | 6 +- src/rpc/virnetserverclient.c | 24 +++- tests/virnetmessagetest.c | 393 +++++++++++++++++++++++------------------- 8 files changed, 281 insertions(+), 197 deletions(-) -- 1.7.8.5

2 4

[libvirt] [PATCH] build: fix 'make distcheck' issues
by Eric Blake 05 Jun '12

05 Jun '12

We have a distributed file depending on a generated file, which is a no-no for a VPATH build from a read-only source tree (no wonder 'make distcheck' gives us precisely that situation): File `libvirt_driver_remote.la' does not exist. File `libvirt_driver_remote_la-remote_driver.lo' does not exist. Prerequisite `libvirt_probes.h' is newer than target `../../src/remote/remote_protocol.h'. Must remake target `../../src/remote/remote_protocol.h'. Invoking recipe from Makefile:7464 to update target `../../src/remote/remote_protocol.h'. make[3]: Entering directory `/home/remote/eblake/libvirt-tmp2/build/libvirt-0.9.12/_build/src' GEN ../../src/remote/remote_protocol.h cannot create ../../src/remote/remote_protocol.h: Permission denied at ../../src/rpc/genprotocol.pl line 31. make[3]: *** [../../src/remote/remote_protocol.h] Error 13 Rather than making distributed .c files depend on generated files, we really want to ensure that compilation into .lo files is not attempted until the generated files are present, done by this patch. After that fix, the next issue was that make treats './foo' and 'foo' differently in determining whether an implicit %foo rule is applicable. Also, the output for using the .aug test files was a bit verbose. After fixing that, the next error is related to the docs directory, where the tarball is missing a stamp file and thus tries to regenerate files that are already present: GEN ../../docs/apibuild.py.stamp Traceback (most recent call last): File "../../docs/apibuild.py", line 2511, in <module> rebuild("libvirt") File "../../docs/apibuild.py", line 2495, in rebuild builder.serialize() File "../../docs/apibuild.py", line 2424, in serialize output = open(filename, "w") IOError: [Errno 13] Permission denied: '../../docs/libvirt-api.xml' make[5]: *** [../../docs/apibuild.py.stamp] Error 1 and fixing that exposed another case of a distributed file (generated html) depending on a built file (libvirt.h). * src/Makefile.am ($(srcdir)/remote/remote_driver.c): Change... (libvirt_driver_remote_la-remote_driver.lo): ...to the real dependency. ($(builddir)/locking/%-sanlock.conf): Drop $(builddir), so that rule gets run in time for test_libvirt_sanlock.aug. (test_libvir*.aug): Cater to silent build. (conf_DATA): Don't ship qemu-sanlock.conf in the tarball, since it is trivial to regenerate. * docs/Makefile.am (EXTRA_DIST): Ship our stamp file. ($(APIBUILD_STAMP)): Don't depend on generated file. --- Painfully tested by running 'make distcheck' for each tweak until I got it to pass. However, I'd like a review before pushing. docs/Makefile.am | 5 ++++- src/Makefile.am | 15 ++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index a03ca3e..88407b1 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -202,11 +202,14 @@ python_generated_files = \ APIBUILD=$(srcdir)/apibuild.py APIBUILD_STAMP=$(APIBUILD).stamp +EXTRA_DIST += $(APIBUILD_STAMP) $(python_generated_files): $(APIBUILD_STAMP) $(APIBUILD_STAMP): $(srcdir)/apibuild.py \ - $(srcdir)/../include/libvirt/*.h \ + $(srcdir)/../include/libvirt/libvirt.h.in \ + $(srcdir)/../include/libvirt/libvirt-qemu.h \ + $(srcdir)/../include/libvirt/virterror.h \ $(srcdir)/../src/libvirt.c \ $(srcdir)/../src/libvirt-qemu.c \ $(srcdir)/../src/util/virterror.c diff --git a/src/Makefile.am b/src/Makefile.am index 781c547..7b21317 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -649,7 +649,8 @@ libvirt_driver_remote_la_LIBADD = $(GNUTLS_LIBS) \ libvirt-net-rpc.la libvirt_driver_remote_la_SOURCES = $(REMOTE_DRIVER_SOURCES) -$(srcdir)/remote/remote_driver.c: $(REMOTE_DRIVER_GENERATED) +REMOTE_DRIVER_PREREQS = $(REMOTE_DRIVER_GENERATED) +%remote_driver.lo: $(REMOTE_DRIVER_PREREQS) endif WITH_REMOTE @@ -1125,7 +1126,7 @@ EXTRA_DIST += $(top_srcdir)/build-aux/augeas-gentest.pl if WITH_QEMU test_libvirtd_qemu.aug: qemu/test_libvirtd_qemu.aug.in \ $(srcdir)/qemu/qemu.conf $(AUG_GENTEST) - $(AUG_GENTEST) $(srcdir)/qemu/qemu.conf $< $@ + $(AM_V_GEN)$(AUG_GENTEST) $(srcdir)/qemu/qemu.conf $< $@ check-augeas-qemu: test_libvirtd_qemu.aug $(AM_V_GEN)if test -x '$(AUGPARSE)'; then \ @@ -1138,7 +1139,7 @@ endif if WITH_LXC test_libvirtd_lxc.aug: lxc/test_libvirtd_lxc.aug.in \ $(srcdir)/lxc/lxc.conf $(AUG_GENTEST) - $(AUG_GENTEST) $(srcdir)/lxc/lxc.conf $< $@ + $(AM_V_GEN)$(AUG_GENTEST) $(srcdir)/lxc/lxc.conf $< $@ check-augeas-lxc: test_libvirtd_lxc.aug $(AM_V_GEN)if test -x '$(AUGPARSE)'; then \ @@ -1150,7 +1151,7 @@ endif test_libvirt_sanlock.aug: locking/test_libvirt_sanlock.aug.in \ locking/qemu-sanlock.conf $(AUG_GENTEST) - $(AUG_GENTEST) locking/qemu-sanlock.conf $< $@ + $(AM_V_GEN)$(AUG_GENTEST) locking/qemu-sanlock.conf $< $@ check-augeas-sanlock: test_libvirt_sanlock.aug $(AM_V_GEN)if test -x '$(AUGPARSE)'; then \ @@ -1287,7 +1288,7 @@ libvirt_la_BUILT_LIBADD += libvirt_probes.lo libvirt_la_DEPENDENCIES += libvirt_probes.lo libvirt_probes.o nodist_libvirt_la_SOURCES = libvirt_probes.h if WITH_REMOTE -$(REMOTE_DRIVER_GENERATED): libvirt_probes.h +REMOTE_DRIVER_PREREQS += libvirt_probes.h endif WITH_REMOTE BUILT_SOURCES += libvirt_probes.h libvirt_probes.stp libvirt_functions.stp @@ -1361,12 +1362,12 @@ augeas_DATA += locking/libvirt_sanlock.aug augeastest_DATA += test_libvirt_sanlock.aug CLEANFILES += test_libvirt_sanlock.aug -$(builddir)/locking/%-sanlock.conf: $(srcdir)/locking/sanlock.conf +locking/%-sanlock.conf: $(srcdir)/locking/sanlock.conf $(AM_V_GEN)$(MKDIR_P) locking ; \ cp $< $@ if WITH_QEMU -conf_DATA += locking/qemu-sanlock.conf +nodist_conf_DATA = locking/qemu-sanlock.conf BUILT_SOURCES += locking/qemu-sanlock.conf DISTCLEANFILES += locking/qemu-sanlock.conf endif -- 1.7.7.6

2 2

[libvirt] [PATCH] Fix for parallel port passtrough for QEMU
by Beat.Joerg＠ssatr.ch 05 Jun '12

05 Jun '12

Hi As requested by Dave, I send this to the list. I came across a bug that the command line generated for passtrough of the host parallel port /dev/parport0 by libvirt for QEMU is incorrect. It currently produces: -chardev tty,id=charparallel0,path=/dev/parport0 -device isa-parallel,chardev=charparallel0,id=parallel0 The first parameter is "tty". It sould be "parport". If I launch qemu with -chardev parport,... it works as expected. I have already filled a bug report ( https://bugzilla.redhat.com/show_bug.cgi?id=823879 ), the topic was already on the list some months ago: https://www.redhat.com/archives/libvirt-users/2011-September/msg00095.html Not sure if this is fix is very clean, but for my case it works. Perhaps it should also be checked if similar problems / patches are required for other chardev backends. Kind regards Beat diff -Naur libvirt-0.9.4.orig/src/qemu/qemu_command.c libvirt-0.9.4/src/qemu/qemu_command.c --- libvirt-0.9.4.orig/src/qemu/qemu_command.c 2012-05-22 13:40:16.788633656 +0200 +++ libvirt-0.9.4/src/qemu/qemu_command.c 2012-05-22 13:52:18.453608557 +0200 @@ -2357,8 +2357,14 @@ break; case VIR_DOMAIN_CHR_TYPE_DEV: - virBufferAsprintf(&buf, "tty,id=char%s,path=%s", alias, - dev->data.file.path); + if (STRPREFIX(dev->data.file.path, "/dev/parport")) { + virBufferAsprintf(&buf, "parport,id=char%s,path=%s", alias, + dev->data.file.path); + } + else { + virBufferAsprintf(&buf, "tty,id=char%s,path=%s", alias, + dev->data.file.path); + } break; case VIR_DOMAIN_CHR_TYPE_FILE:

2 1

[libvirt] [PATCH] Fixes for check and rpm builds without sanlock (and qemu)
by Martin Kletzander 04 Jun '12

04 Jun '12

Apart from the non-sanlock check build, there is also a little fix for qemu (EXTRA_DIST had qemu.conf and others inside even if the build was supposed to be without qemu). --- libvirt.spec.in | 5 +++++ src/Makefile.am | 6 +++++- 2 files changed, 10 insertions(+), 1 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index d4e43f6..3c544f1 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1232,6 +1232,11 @@ rm -f $RPM_BUILD_ROOT%{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug %endif %find_lang %{name} +%if ! %{with_sanlock} +rm -f $RPM_BUILD_ROOT%{_datadir}/augeas/lenses/libvirt_sanlock.aug +rm -f $RPM_BUILD_ROOT%{_datadir}/augeas/lenses/tests/test_libvirt_sanlock.aug +%endif + %if ! %{with_lxc} rm -f $RPM_BUILD_ROOT%{_datadir}/augeas/lenses/libvirtd_lxc.aug rm -f $RPM_BUILD_ROOT%{_datadir}/augeas/lenses/tests/test_libvirtd_lxc.aug diff --git a/src/Makefile.am b/src/Makefile.am index 23747d7..e56a8ef 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -785,9 +785,9 @@ augeas_DATA += qemu/libvirtd_qemu.aug augeastest_DATA += qemu/test_libvirtd_qemu.aug CLEANFILES += qemu/test_libvirtd_qemu.aug -endif EXTRA_DIST += qemu/qemu.conf qemu/libvirtd_qemu.aug \ qemu/test_libvirtd_qemu.aug.in qemu/THREADS.txt +endif if WITH_LXC @@ -1147,6 +1147,7 @@ else check-augeas-lxc: endif +if HAVE_SANLOCK test_libvirt_sanlock.aug: locking/test_libvirt_sanlock.aug.in \ locking/qemu-sanlock.conf $(AUG_GENTEST) $(AUG_GENTEST) locking/qemu-sanlock.conf $< $@ @@ -1155,6 +1156,9 @@ check-augeas-sanlock: test_libvirt_sanlock.aug $(AM_V_GEN)if test -x '$(AUGPARSE)'; then \ '$(AUGPARSE)' -I $(srcdir)/locking test_libvirt_sanlock.aug; \ fi +else +check-augeas-sanlock: +endif # # Build our version script. This is composed of three parts: -- 1.7.8.6

3 10

[libvirt] Authentication
by Jagannath Timma 31 May '12

31 May '12

Are there any plans to add support for the default authentication method of the server? """ http://libvirt.org/drvhyperv.html Authentication In order to perform any useful operation the driver needs to log into the Hyper-V server. Therefore, only virConnectOpenAuth can be used to connect to an Hyper-V server, virConnectOpen and virConnectOpenReadOnly don't work. To log into an Hyper-V server the driver will request credentials using the callback passed to the virConnectOpenAuth function. The driver passes the hostname as challenge parameter to the callback. Note: Currently only Basic authentication is supported by libvirt. This method is disabled by default on the Hyper-V server and can be enabled via the WinRM commandline tool. "" Regards, Jagannath

1 0

[libvirt] [PATCH] SNMP: include inactive domains in guest table
by Jonathan Daugherty 31 May '12

31 May '12

Hi, I've written a patch to include inactive domains in the guest table available via the libvirt-snmp subagent. This makes it possible to start domains via SNMP (a state transition which AFAICT was previously impossible, because inactive domains never appeared in the SNMP result). The code now collects the IDs of running domains along with the names of declared (but not running) domains and normalizes them into a list of domain pointers, which is then processed by the preexisting SNMP container row allocation code. The change was fairly straightforward, but certainly there might be Good Reasons it wasn't already implemented. As part of this change, the other (noteworthy) change I made was to move calls to virDomainFree() to the end of libvirtSnmpLoadGuests() since I have to free all of the used domain pointers there anyway. Some other issues which may need to be addressed: - This will be a breaking change for anyone who assumes that the old behavior is still in place (i.e., that the guest table only includes non-shutdown domains). However, it might not be *that* bad because the domain rows already included a 'state' that one could already filter on. - I don't know if there exists other code in libvirt-snmp that assumes inactive domains can't appear in the table. - I have not tested the inactive -> active state transition (because I don't want to enable writable SNMP OIDs on my server). Please let me know if there is anything further I need to do to make the patch acceptable. Thanks for libvirt-snmp! -- Jonathan Daugherty Software Engineer Galois, Inc.

3 4

[libvirt] [PATCH v2 0/2] Make virsh *edit more bearable
by Michal Privoznik 31 May '12

31 May '12

If there's an error in XML, all changes made by user are lost. Users (read /me) tends to get angry with this. Allow them to get back and re-edit. diff to v1: -Eric's review suggestions included Michal Privoznik (2): virsh: Switch from generated cmd*Edit commands to nongenerated virsh: Allow users to reedit rejected XML cfg.mk | 4 +- po/POTFILES.in | 1 + tools/Makefile.am | 40 +---- tools/console.c | 40 +++-- tools/console.h | 2 + tools/virsh-edit.c | 138 +++++++++++++++ tools/virsh.c | 483 ++++++++++++++++++++++------------------------------ 7 files changed, 371 insertions(+), 337 deletions(-) create mode 100644 tools/virsh-edit.c -- 1.7.8.5

2 5

[libvirt] [PATCH] tools: make virt-pki-validate work with acls and xattrs
by Martin Kletzander 31 May '12

31 May '12

This patch makes virt-pki-validate work with certificates that have acl or xattr set. Otherwise it failing due to wrong permissions. --- tools/virt-pki-validate.in | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/virt-pki-validate.in b/tools/virt-pki-validate.in index 01825d1..4164758 100755 --- a/tools/virt-pki-validate.in +++ b/tools/virt-pki-validate.in @@ -170,7 +170,8 @@ then else echo Found client private key $LIBVIRTP/clientkey.pem OWN=`ls -l "$LIBVIRTP/clientkey.pem" | awk '{ print $3 }'` - MOD=`ls -l "$LIBVIRTP/clientkey.pem" | awk '{ print $1 }'` + # The substr($1, 1, 10) gets rid of acl and xattr markers + MOD=`ls -l "$LIBVIRTP/clientkey.pem" | awk '{ print substr($1, 1, 10) }'` if [ "$OWN" != "root" ] then echo The client private key should be owned by root @@ -222,7 +223,8 @@ then else echo Found server private key $LIBVIRTP/serverkey.pem OWN=`ls -l "$LIBVIRTP/serverkey.pem" | awk '{ print $3 }'` - MOD=`ls -l "$LIBVIRTP/serverkey.pem" | awk '{ print $1 }'` + # The substr($1, 1, 10) gets rid of acl and xattr markers + MOD=`ls -l "$LIBVIRTP/serverkey.pem" | awk '{ print substr($1, 1, 10) }'` if [ "$OWN" != "root" ] then echo The server private key should be owned by root -- 1.7.8.6

3 4

[libvirt] [PATCHv2 0/4] double close() fixes
by Eric Blake 31 May '12

31 May '12

Updated version of Wen's and my patches. In this version, I fixed review comments, split Wen's patch into 3, and tracked down when each bug was introduced. Eric Blake (2): command: avoid double close bugs fdstream: avoid double close bug Wen Congyang (2): command: check for fork error before closing fd qemu: avoid closing fd more than once src/fdstream.c | 7 +++---- src/qemu/qemu_migration.c | 4 +++- src/util/command.c | 24 ++++++++++-------------- 3 files changed, 16 insertions(+), 19 deletions(-) -- 1.7.7.6

1 5

[libvirt] [PATCH 1/3] avoid closing fd more than once in virFDStreamOpenFileInternal()
by Wen Congyang 31 May '12

31 May '12

When we assign fd and childfd to elements of fds[], we will close these fds more than once on error. So we should assign fds[] to -1 after we assign fd and childfd to elements of fds[]. We should also close childfd on error. --- src/fdstream.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/src/fdstream.c b/src/fdstream.c index fca0f41..0de3b04 100644 --- a/src/fdstream.c +++ b/src/fdstream.c @@ -581,6 +581,7 @@ virFDStreamOpenFileInternal(virStreamPtr st, struct stat sb; virCommandPtr cmd = NULL; int errfd = -1; + int childfd = -1; VIR_DEBUG("st=%p path=%s oflags=%x offset=%llu length=%llu mode=%o", st, path, oflags, offset, length, mode); @@ -619,7 +620,6 @@ virFDStreamOpenFileInternal(virStreamPtr st, if ((st->flags & VIR_STREAM_NONBLOCK) && (!S_ISCHR(sb.st_mode) && !S_ISFIFO(sb.st_mode))) { - int childfd; if ((oflags & O_ACCMODE) == O_RDWR) { streamsReportError(VIR_ERR_INTERNAL_ERROR, @@ -650,6 +650,7 @@ virFDStreamOpenFileInternal(virStreamPtr st, fd = fds[1]; virCommandSetInputFD(cmd, childfd); } + fds[0] = fds[1] = -1; virCommandSetErrorFD(cmd, &errfd); if (virCommandRunAsync(cmd, NULL) < 0) @@ -668,6 +669,7 @@ error: VIR_FORCE_CLOSE(fds[0]); VIR_FORCE_CLOSE(fds[1]); VIR_FORCE_CLOSE(fd); + VIR_FORCE_CLOSE(childfd); VIR_FORCE_CLOSE(errfd); if (oflags & O_CREAT) unlink(path); -- 1.7.1

1 2