February 2013 - Devel - libvirt List Archives

[libvirt] [PATCH v4] add pci-bridge support
by liguang 19 Feb '13

19 Feb '13

Now, it's impossible to arrange devices into multi-pci-bus, for example: <sound model='ac97'> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </sound> <video> <model type='cirrus' vram='9216' heads='1'/> <address type='pci' domain='0x0000' bus='0x1' slot='0x02' function='0x0'/> </video> libvirt will complain about "bus != 0", fortunately, qemu supports pci-to-pci bridge, if we want to use multi-pci-bus, we can define 2 pci bridge controllers, then attach 1 to the other as a subordinate pci-bus, so, 2 pci-buses appear. for example: <controller type='pci-bridge' index='0'/> <controller type='pci-bridge' index='1'> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </controller> <sound model='ac97'> <address type='pci' domain='0x0000' bus='0x01' slot='0x02' function='0x0'/> </sound> <video> <model type='cirrus' vram='9216' heads='1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </video> src/conf/domain_conf.c | 98 ++++- src/conf/domain_conf.h | 1 + docs/schemas/domaincommon.rng | 1 + src/qemu/qemu_capabilities.c | 4 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 43 ++++++++++++++++++++----- 6 files changed, 126 insertions(+), 22 deletions(-)

1 4

[libvirt] [PATCH 0/6 v2] Fix problems on shared disk management
by Osier Yang 19 Feb '13

19 Feb '13

This fixes problems on shared disk management, inspired by the crash when changing the media of cdrom and floppy disk, and the source of cdrom and floppy disk is empty. v1 - v2: * v1 uses a bitmap to record whether the disk is added into the table when do qemuProcessStart. v2 added a array to the hash entry to record the names of domain which uses the shared disk. * Update the shared disk table when reconnecting qemu process. Osier Yang (6): qemu: Add checking in helpers for sgio setting qemu: Merge qemuCheckSharedDisk into qemuAddSharedDisk qemu: Record names of domain which uses the shared disk in hash table qemu: Update shared disk table when reconnecting qemu process qemu: Move the shared disk adding and sgio setting prior to attaching qemu: Remove the shared disk entry if the operation is ejecting src/qemu/qemu_conf.c | 187 ++++++++++++++++++++++++++++++++++++++++++---- src/qemu/qemu_conf.h | 22 +++++- src/qemu/qemu_driver.c | 84 ++++++++++++++------- src/qemu/qemu_hotplug.c | 10 ++- src/qemu/qemu_hotplug.h | 3 +- src/qemu/qemu_process.c | 81 +++++--------------- src/qemu/qemu_process.h | 3 - 7 files changed, 276 insertions(+), 114 deletions(-) -- 1.7.7.6

2 12

[libvirt] Fwd: There seems a deadlock in libvirt
by Chun-Hung Chen 19 Feb '13

19 Feb '13

Hi, all, We were running OpenStack with Ubuntu and libvirt 0.9.10. We found that libvirt monitor command not working well. There were a lot of error in libvirtd.log like this 2013-02-07 06:07:39.000+0000: 18112: error : qemuDomainObjBeginJobInternal:773 : Timed out during operation: cannot acquire state change lock We dig into libvirtd by strace and find one of the thread only have the following command futex(0x7f69ac0ec0ec, FUTEX_WAIT_PRIVATE, 2717, NULL It seems this thread waiting for reply but nothing came back thus other threads would wait for it. We also saw there is a function called virCondWaitUntil(). Is it safe for us to modify the code from virCondWait() to virCondWaitUntil() to prevent such deadlock scenario? Thanks. Following is the gdb -p 'libvirt.pid' and 'thread id' and 'bt full' #0 0x00007f69c8c1dd84 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib/x86_64-linux-gnu/libpthread.so.0 No symbol table info available. #1 0x00007f69c9ee884a in virCondWait (c=<optimized out>, m=<optimized out>) at util/threads-pthread.c:117 ret = <optimized out> #2 0x000000000049c749 in qemuMonitorSend (mon=0x7f69ac0ec0c0, msg=<optimized out>) at qemu/qemu_monitor.c:826 ret = -1 __func__ = "qemuMonitorSend" __FUNCTION__ = "qemuMonitorSend" #3 0x00000000004ac8ed in qemuMonitorJSONCommandWithFd (mon=0x7f69ac0ec0c0, cmd=0x7f6998028280, scm_fd=-1, reply=0x7f69c57829f8) at qemu/qemu_monitor_json.c:230 ret = -1 msg = {txFD = -1, txBuffer = 0x7f69980e9b00 "{\"execute\":\"query-balloon\",\"id\":\"libvirt-1359\"}\r\n", txOffset = 49, txLength = 49, rxBuffer = 0x0, rxLength = 0, rxObject = 0x0, finished = false, passwordHandler = 0, passwordOpaque = 0x0} cmdstr = 0x7f69980ef2f0 "{\"execute\":\"query-balloon\",\"id\":\"libvirt-1359\"}" id = 0x7f69980b0a20 "libvirt-1359" exe = <optimized out> __FUNCTION__ = "qemuMonitorJSONCommandWithFd" __func__ = "qemuMonitorJSONCommandWithFd" #4 0x00000000004ae794 in qemuMonitorJSONGetBalloonInfo (mon=0x7f69ac0ec0c0, currmem=0x7f69c5782a48) at qemu/qemu_monitor_json.c:1190 ret = <optimized out> cmd = 0x7f6998028280 reply = 0x0 __FUNCTION__ = "qemuMonitorJSONGetBalloonInfo" #5 0x0000000000457451 in qemudDomainGetInfo (dom=<optimized out>, info=0x7f69c5782b50) at qemu/qemu_driver.c:2181 priv = 0x7f69a0093b00 driver = 0x7f69b80ca8e0 vm = 0x7f69a0093370 ret = -1 err = <optimized out> balloon = <optimized out> __FUNCTION__ = "qemudDomainGetInfo" #6 0x00007f69c9f63eda in virDomainGetInfo (domain=0x7f69980e3650, info=0x7f69c5782b50) at libvirt.c:4230 ret = <optimized out> conn = <optimized out> __func__ = "virDomainGetInfo" __FUNCTION__ = "virDomainGetInfo" #7 0x0000000000439bca in remoteDispatchDomainGetInfo (ret=0x7f6998000c20, args=<optimized out>, rerr=0x7f69c5782c50, client=0x157e730, server=<optimized out>, msg=<optimized out>) at remote_dispatch.h:1640 rv = -1 tmp = {state = 1 '\001', maxMem = 2097152, memory = 0, nrVirtCpu = 0, cpuTime = 5981880000000} dom = 0x7f69980e3650 priv = <optimized out> #8 remoteDispatchDomainGetInfoHelper (server=<optimized out>, client=0x157e730, msg=<optimized out>, rerr=0x7f69c5782c50, args=<optimized out>, ret=0x7f6998000c20) at remote_dispatch.h:1616 __func__ = "remoteDispatchDomainGetInfoHelper" #9 0x00007f69c9fbb915 in virNetServerProgramDispatchCall (msg=0x1689cc0, client=0x157e730, server=0x1577c90, prog=0x15825d0) at rpc/virnetserverprogram.c:416 ret = 0x7f6998000c20 "" rv = -1 i = <optimized out> arg = 0x7f6998027950 "\360e\n\230i\177" dispatcher = 0x73de40 rerr = {code = 0, domain = 0, message = 0x0, level = 0, dom = 0x0, str1 = 0x0, str2 = 0x0, str3 = 0x0, int1 = 0, int2 = 0, net = 0x0} #10 virNetServerProgramDispatch (prog=0x15825d0, server=0x1577c90, client=0x157e730, msg=0x1689cc0) at rpc/virnetserverprogram.c:289 ret = -1 rerr = {code = 0, domain = 0, message = 0x0, level = 0, dom = 0x0, str1 = 0x0, str2 = 0x0, str3 = 0x0, int1 = 0, int2 = 0, net = 0x0} __func__ = "virNetServerProgramDispatch" __FUNCTION__ = "virNetServerProgramDispatch" #11 0x00007f69c9fb6461 in virNetServerHandleJob (jobOpaque=<optimized out>, opaque=0x1577c90) at rpc/virnetserver.c:164 srv = 0x1577c90 job = 0x155dfa0 __func__ = "virNetServerHandleJob" #12 0x00007f69c9ee8e3e in virThreadPoolWorker (opaque=<optimized out>) at util/threadpool.c:144 data = 0x0 pool = 0x1577d80 cond = 0x1577de0 priority = false job = 0x162dd20 #13 0x00007f69c9ee84e6 in virThreadHelper (data=<optimized out>) at util/threads-pthread.c:161 args = 0x0 local = {func = 0x7f69c9ee8d00 <virThreadPoolWorker>, opaque = 0x1559f90} #14 0x00007f69c8c19e9a in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0 No symbol table info available. #15 0x00007f69c89474bd in clone () from /lib/x86_64-linux-gnu/libc.so.6 No symbol table info available. #16 0x0000000000000000 in ?? () No symbol table info available. Regards, Chun-Hung

2 1

[libvirt] [PATCH V6 0/3] Add support for QEMU file descriptor sets
by Stefan Berger 19 Feb '13

19 Feb '13

The following patch series adds initial support for QEMU file descriptor sets implementing support for creating the proper command line. Some devices are using the sets now. Regards, Stefan

3 15

[libvirt] [PATCH] build: force correct gcc syntax for attribute_nonnull
by Eric Blake 19 Feb '13

19 Feb '13

Gcc lets you do: int ATTRIBUTE_NONNULL(1) foo(void *param); int foo(void *param) ATTRIBUTE_NONNULL(1); int ATTRIBUTE_NONNULL(1) foo(void *param) { ... } but chokes on: int foo(void *param) ATTRIBUTE_NONNULL(1) { ... } However, since commit eefb881, we have intentionally been disabling ATTRIBUTE_NONNULL because of lame gcc handling of the attribute (that is, gcc doesn't do decent warning reporting, then compiles code that mysteriously fails if you break the contract of the attribute, which is surprisingly easy to do), leaving it on only for Coverity (which does a much better job of improved static analysis when the attribute is present). But completely eliding the macro makes it too easy to write code that uses the fourth syntax option, if you aren't using Coverity. So this patch forces us to avoid syntax errors, even when not using the attribute under gcc. It also documents WHY we disable the warning under gcc, rather than forcing you to find the commit log. * src/internal.h (ATTRIBUTE_NONNULL): Expand to empty attribute, rather than nothing, when on gcc. --- src/internal.h | 17 +++++++++++++++-- 1 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/internal.h b/src/internal.h index ebc91c7..2cf4731 100644 --- a/src/internal.h +++ b/src/internal.h @@ -181,9 +181,22 @@ # endif # endif +/* gcc's handling of attribute nonnull is less than stellar - it does + * NOT improve diagnostics, and merely allows gcc to optimize away + * null code checks even when the caller manages to pass null in spite + * of the attribute, leading to weird crashes. Coverity, on the other + * hand, knows how to do better static analysis based on knowing + * whether a parameter is nonnull. Make this attribute conditional + * based on whether we are compiling for real or for analysis, while + * still requiring correct gcc syntax when it is turned off. See also + * http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17308 */ # ifndef ATTRIBUTE_NONNULL -# if __GNUC_PREREQ (3, 3) && STATIC_ANALYSIS -# define ATTRIBUTE_NONNULL(m) __attribute__((__nonnull__(m))) +# if __GNUC_PREREQ (3, 3) +# if STATIC_ANALYSIS +# define ATTRIBUTE_NONNULL(m) __attribute__((__nonnull__(m))) +# else +# define ATTRIBUTE_NONNULL(m) __attribute__(()) +# endif # else # define ATTRIBUTE_NONNULL(m) # endif -- 1.7.1

2 2

[libvirt] [PATCH 0/4] interface: udev backend bond support
by Doug Goldstein 19 Feb '13

19 Feb '13

This patchset primarily aims to add support for bond devices which was lacking from the exiting code. This closes out support for all interface device types. The patchset also refactors to code to make it a bit easier to follow hopefully. Doug Goldstein (4): interface: Refactor udev bridge to helper func interface: Refactor interface vlan to helper func interface: Refactor udev backend device type id interface: add bond support to udev backend src/interface/interface_backend_udev.c | 456 ++++++++++++++++++++++++++------- 1 file changed, 359 insertions(+), 97 deletions(-) -- 1.7.12.4

2 8

[libvirt] [PATCH] qemu: pass "-1" as uid/gid for unprivileged qemu
by Guido Günther 18 Feb '13

18 Feb '13

so we don't try to change uid/git to 0 when probing capabilities. --- On Fri, Feb 15, 2013 at 11:20:17PM -0600, Doug Goldstein wrote: > The following error bisect's down to this commit when running out of > my local checkout for testing. > > 2013-02-16 05:16:55.102+0000: 29992: error : virCommandWait:2270 : > internal error Child process (LC_ALL=C > LD_LIBRARY_PATH=/home/cardoe/work/libvirt/src/.libs > PATH=/usr/local/bin:/usr/bin:/bin:/opt/bin:/usr/x86_64-pc-linux-gnu/gcc-bin/4.6.3:/usr/games/bin > HOME=/home/cardoe USER=cardoe LOGNAME=cardoe /usr/bin/qemu-kvm -help) > unexpected exit status 1: libvir: error : internal error cannot apply > process capabilities -1 This is due to: debug : virExec:641 : Setting child uid:gid to 0:0 with caps 0 We're trying to change capabilities even on qemu:///session. This unbreaks the libvirt-tck jenkins test suite as well. src/qemu/qemu_driver.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 23499ef..dc35b91 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -556,6 +556,8 @@ qemuStartup(bool privileged, char *membase = NULL; char *mempath = NULL; virQEMUDriverConfigPtr cfg; + uid_t run_uid = -1; + gid_t run_gid = -1; if (VIR_ALLOC(qemu_driver) < 0) return -1; @@ -707,11 +709,13 @@ qemuStartup(bool privileged, cfg->snapshotDir, cfg->user, cfg->group); goto error; } + run_uid = cfg->user; + run_gid = cfg->group; } qemu_driver->qemuCapsCache = virQEMUCapsCacheNew(cfg->libDir, - cfg->user, - cfg->group); + run_uid, + run_gid); if (!qemu_driver->qemuCapsCache) goto error; -- 1.7.10.4

2 2

[libvirt] [PATCH] Add capabilities bit for -no-kvm-pit-reinjection
by Doug Goldstein 18 Feb '13

18 Feb '13

The conversion to qemuCaps dropped the ability with qemu{,-kvm} 1.2 and newer to set the lost tick policy for the PIT. While the -no-kvm-pit-reinjection option is depreacated, it is still supported at least through 1.4, it is better to not lose the functionality. --- src/qemu/qemu_capabilities.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 51fc9dc..af52bbf 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2284,6 +2284,7 @@ virQEMUCapsInitQMPBasic(virQEMUCapsPtr qemuCaps) virQEMUCapsSet(qemuCaps, QEMU_CAPS_NO_USER_CONFIG); virQEMUCapsSet(qemuCaps, QEMU_CAPS_NETDEV_BRIDGE); virQEMUCapsSet(qemuCaps, QEMU_CAPS_SECCOMP_SANDBOX); + virQEMUCapsSet(qemuCaps, QEMU_CAPS_NO_KVM_PIT); } -- 1.7.12.4

3 4

[libvirt] Questions about the schemas
by Gene Czarcinski 18 Feb '13

18 Feb '13

This is not in any way critical but simply trying to get a better idea of the purposes of docs/schemas/* I recently submitted a patch which needed me to look at and modify some of the schema files [adding a client-id specification to dhcp/host]. As I looked them over I wondered what the purpose was of the schemas. If you ran virt-xml-validate (which uses xmllint) and the result is that your file is not validated, then there is a problem but it is not clear just what the problem is. On the other hand, if you run virt-xml-validate and it says the your file is OK (validated), that only means that it did not find a problem but not that the file is OK. For example, the schema says it is valid to specify an IPv6 address in an IPv4 definition. The implementing software if kick it out but the schema says it is OK. Another example is the (host) name under the dhcp/host definition. The current schema says this is required but dnsmasq does not require a host-name specification [I submitted a trivial patch to make it optional]. So, if the purpose is to kick out obvious errors, this it is OK as is. If the purpose to to be definitiive, then more work needs to be done. My opinion is that the current situation is "good enough" as long as valid elements/specifications are not treated as errors. Gene

2 2

[libvirt] [PATCHv2 00/15] Permit setting capabilities for uid!=0 processes
by Laine Stump 18 Feb '13

18 Feb '13

(and also properly clear capabilities bounding set for child processes) This replaces the earlier version of the same patches: https://www.redhat.com/archives/libvir-list/2013-February/msg00446.html Many of these patches were already ACKed in the first version. If I haven't modified them (other than rebasing) I note that in the subject. Other patches have been modified based on Dan and Eric's reviews of V1. Also, since I first posted the series, we've had a talk with some kernel people who are amenable to modify the semantics of CAP_COMPROMISE_KERNEL such that it's only checked on open(), not on read or write. If this is done, we will (thankfully!) no longer need to set CAP_COMPROMISE_KERNEL for the qemu process. However, the first 14 patches in this series are still useful. Two big changes are: 1) it allows setting CAP_SYS_RAWIO when needed for generic scsi command passthrough, and 2) the bounding set of child processes will now be properly cleared (it previously wasn't, even though the code looked like it *should* have been doing so). Here is the blurb from V1's intro: There are a bunch of patches here, but each is small and single-purpose to make reviewing easier (and also so that any potential regressions can be easily bisected). The original purpose of the patches was to permit setting CAP_COMPROMISE_KERNEL for non-root qemu processes, since Fedora 18 now requires that in order for generic PCI passthrough to work (the alternative was to always run qemu as root). Although we may not actually want to do that part (if we can convince kernel people to implement CAP_COMPROMISE_KERNEL such that it's only required when *opening* the necessary sysfs file (done by libvirt), rather than for every read/write (done by qemu), then we will not need CAP_COMPROMISE_KERNEL for qemu), but that is just a couple lines in the final patch, and the rest of the series is still useful, as it make dropping/keeping caps truly work for non-root child processes - this has never before been the case. (for example, CAP_SYS_RAWIO is needed for generic scsi passthrough to work, and until now the only way to have that was to run *all* qemus as root). A bit higher level description of what I've done with all the patches: 1) remove the programmable "hook" from virExecWithHook(), since that function was only called from one place, and always with the same hook function. Rename virExecWithHook() to virExec(), and replace the call to that hook with inline code. 2) give virCommand an API to set the intended uid/gid of the command that's going to be run, and use that instead of a "user hook" where appropriate (in the process completely eliminating two hook functions). 3) Also add an API to virCommand to do the final "set the process label" step for selinux/apparmor. 4) Add a new API to the security driver (and use it from qemu) called virSecurityManagerSetChildProcessLabel() which a) is called prior to virCommandRun() rather than from a command "hook" function, b) takes a virCommand, and c) rather than immediately performing the operation (as virSecurityManagerSetProcessLabel() did), merely stores the necessary information in the virCommand so that virExec can perform the operation (setting selinux label, setuid/gid, etc) 5) make a new function combining the setting of uid/gid and maintaining of capabilities, because that is the only way you can set uid!=0 and still maintain capabilities. Use this in virExec() 6) *Finally* set the CAP_COMPROMISE_KERNEL capability unconditionally for all qemu processes. (If we really do have to do this, we may want to consider making it a qemu.conf setting). Laine Stump (15): util: eliminate generic hook from virExecWithHook util: eliminate extra args from virExec util: refactor virCommandHook into virExec and virCommandHandshakeChild util: add virCommandSetUID and virCommandSetGID util: make virSetUIDGID a NOP only when uid or gid is -1 qemu: replace exec hook with virCommandSetUID/GID in qemuCaps* qemu: replace exec hook with virCommandSetUID/GID in storage_backend build: define SECDRIVER_LIBS in Makefile.am util: add security label setting to virCommand security: add new virSecurityManagerSetChildProcessLabel API qemu: let virCommand set child process security labels/uid/gid util: drop capabilities immediately after changing uid/gid of child util: virSetUIDGIDWithCaps - change uid while keeping caps util: maintain caps when running command with uid != 0 qemu: set CAP_COMPROMISE_KERNEL so that pci passthrough works src/Makefile.am | 36 ++- src/libvirt_private.syms | 6 + src/qemu/qemu_capabilities.c | 64 ++--- src/qemu/qemu_process.c | 23 +- src/security/security_apparmor.c | 42 +++- src/security/security_dac.c | 24 +- src/security/security_driver.h | 6 +- src/security/security_manager.c | 13 +- src/security/security_manager.h | 6 +- src/security/security_nop.c | 10 +- src/security/security_selinux.c | 32 +++ src/security/security_stack.c | 20 +- src/storage/storage_backend.c | 28 +-- src/util/vircommand.c | 523 ++++++++++++++++++++------------------- src/util/vircommand.h | 12 +- src/util/virutil.c | 115 ++++++++- src/util/virutil.h | 1 + 17 files changed, 595 insertions(+), 366 deletions(-) -- 1.8.1

6 36