When initializing qemu guest capabilities, we should ignore qemu
binaries that we are not able to extract version/help info from since
they will be unusable for creating domains anyway. Ignoring them is also
much better than letting initialization of qemu driver fail.
Even with -Wuninitialized (which is part of autobuild.sh
--enable-compile-warnings=error), gcc does NOT catch this
use of an uninitialized variable:
{
if (cond)
goto error;
int a = 1;
error:
printf("%d", a);
}
which prints 0 (supposing the stack started life wiped) if
cond was true. Clang will catch it, but we don't use clang
as often. Using gcc -Wjump-misses-init catches it, but also
gives false positives:
{
if (cond)
goto error;
int a = 1;
return a;
error:
return 0;
}
Here, a was never used in the scope of the error block, so
declaring it after goto is technically fine (and clang agrees).
However, given that our HACKING already documents a preference
to C89 decl-before-statement, the false positive warning is
enough of a prod to comply with HACKING.
[Personally, I'd _really_ rather use C99 decl-after-statement
to minimize scope, but until gcc can efficiently and reliably
catch scoping and uninitialized usage bugs, I'll settle with
the compromise of enforcing a coding standard that happens to
reject false positives if it can also detect real bugs.]
* acinclude.m4 (LIBVIRT_COMPILE_WARNINGS): Add -Wjump-misses-init.
* src/util/util.c (__virExec): Adjust offenders.
* src/conf/domain_conf.c (virDomainTimerDefParseXML): Likewise.
* src/remote/remote_driver.c (doRemoteOpen): Likewise.
* src/phyp/phyp_driver.c (phypGetLparNAME, phypGetLparProfile)
(phypGetVIOSFreeSCSIAdapter, phypVolumeGetKey)
(phypGetStoragePoolDevice)
(phypVolumeGetPhysicalVolumeByStoragePool)
(phypVolumeGetPath): Likewise.
* src/vbox/vbox_tmpl.c (vboxNetworkUndefineDestroy)
(vboxNetworkCreate, vboxNetworkDumpXML)
(vboxNetworkDefineCreateXML): Likewise.
* src/xenapi/xenapi_driver.c (getCapsObject)
(xenapiDomainDumpXML): Likewise.
* src/xenapi/xenapi_utils.c (createVMRecordFromXml): Likewise.
* src/security/security_selinux.c (SELinuxGenNewContext):
Likewise.
* src/qemu/qemu_command.c (qemuBuildCommandLine): Likewise.
* src/qemu/qemu_hotplug.c (qemuDomainChangeEjectableMedia):
Likewise.
* src/qemu/qemu_process.c (qemuProcessWaitForMonitor): Likewise.
* src/qemu/qemu_monitor_text.c (qemuMonitorTextGetPtyPaths):
Likewise.
* src/qemu/qemu_driver.c (qemudDomainShutdown)
(qemudDomainBlockStats, qemudDomainMemoryPeek): Likewise.
* src/storage/storage_backend_iscsi.c
(virStorageBackendCreateIfaceIQN): Likewise.
* src/node_device/node_device_udev.c (udevProcessPCI): Likewise.
If strdup("x509dname") or strdup("saslUsername") success, but
strdup(x509dname) or strdup(saslUsername) failed, subject->nidentity
is not the num elements of subject->identities, and we will leak some
memory.
qemu driver uses a 4K buffer for reading qemu log file. This is enough
when only qemu's output is present in the log file. However, when
debugging messages are turned on, intermediate libvirt process fills the
log with a bunch of debugging messages before it executes qemu binary.
In such a case the buffer may become too small. However, we are not
really interested in libvirt messages so they can be filtered out from
the buffer.
It throws errors as long as the cgroup controller is not available,
regardless of whether we really want to use it to do setup or not,
which is not what we want, fixing it with throwing error when need
to use the controller.
And change "VIR_WARN" to "qemuReportError" for memory controller
incidentally.
We create a temporary file to save memory, and we will remove it after reading
memory to buffer. But we free the variable that contains the temporary filename
before we remove it. So we should free tmp after unlinking it.
strcase{cmp/str} have the drawback of being sensitive to the global
locale; this is unacceptable in a library setting. Prefer a
hard-coded C locale alternative for all but virsh, which is user
facing and where the global locale isn't changing externally.
* .gnulib: Update to latest, for c-strcasestr change.
* bootstrap.conf (gnulib_modules): Drop strcasestr, add c-strcase
and c-strcasestr.
* cfg.mk (sc_avoid_strcase): New rule.
(exclude_file_name_regexp--sc_avoid_strcase): New exception.
* src/internal.h (STRCASEEQ, STRCASENEQ, STRCASEEQLEN)
(STRCASENEQLEN): Adjust offenders.
* src/qemu/qemu_monitor_text.c (qemuMonitorTextEjectMedia):
Likewise.
* tools/virsh.c (namesorter): Document exception.
If qemu quited unexpectedly when we call qemuMonitorJSONHMP(),
libvirt will crash.
Steps to reproduce this bug:
1. use gdb to attach libvirtd, and set a breakpoint in the function
qemuMonitorSetCapabilities()
2. start a vm
3. let the libvirtd to run until qemuMonitorJSONSetCapabilities() returns.
4. kill the qemu process
5. continue running libvirtd
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
If the monitor met a error, and we will call qemuProcessHandleMonitorEOF().
But we may try to send monitor command after qemuProcessHandleMonitorEOF()
returned. Then libvirtd will be blocked in qemuMonitorSend().
Steps to reproduce this bug:
1. use gdb to attach libvirtd, and set a breakpoint in the function
qemuConnectMonitor()
2. start a vm
3. let the libvirtd to run until qemuMonitorOpen() returns.
4. kill the qemu process
5. continue running libvirtd
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Currently libvirt's default logging is limited and it is difficult to
determine what was happening when a proglem occurred (especially on a
machines where one don't know the detail.) This patch helps to do that
by making additional logging available for the following events:
creating/defining/undefining domains
creating/defining/undefining/starting/stopping networks
creating/defining/undefining/starting/stopping storage pools
creating/defining/undefining/starting/stopping storage volumes.
* AUTHORS: add Naoya Horiguchi
* src/network/bridge_driver.c src/qemu/qemu_driver.c
src/storage/storage_driver.c: provide more VIR_INFO logging
When domain startup, setting cpu affinity and cpu shares according
to the cputune xml specified in domain xml.
Modify "qemudDomainPinVcpu" to update domain config for vcpupin,
and modify "qemuSetSchedulerParameters" to update domain config
for cpu shares.
v1 - v2:
* Use "VIR_ALLOC_N" instead of "VIR_ALLOC_VAR"
* But keep raising error when it fails on adding vcpupin xml
entry, as I still don't have a better idea yet.
My earlier testing for commit 34fa0de0 was done while starting
just-built libvirt from an unconfined_t shell, where the fds happened
to work when transferring to qemu. But when installed and run under
virtd_t, failure to label the raw file (with no compression) or the
pipe (with compression) triggers SELinux failures when passing fds
over SCM_RIGHTS to svirt_t qemu.
* src/qemu/qemu_migration.c (qemuMigrationToFile): When passing
FDs, make sure they are labeled.
First fallout of fd: migration - it looks like SELinux enforcing
_does_ require fd labeling (running uninstalled libvirtd from an
unconstrained shell had no problems, but once faked out by doing
chcon `stat -c %C /usr/sbin/libvirtd` daemon/libvirtd
run_init $PWD/daemon/libvirtd
to run it with the same context as an init script service, and with
SELinux enforcing, I got a rather confusing failure:
error: Failed to save domain fedora_12 to fed12.img
error: internal error unable to send TAP file handle: No file descriptor supplied via SCM_RIGHTS
This fixes the error message, then I need to figure out a subsequent
patch that does the fsetfilecon() necessary to keep things happy.
It also appears that libvirtd hangs on a failed fd transfer; I don't
know if that needs an independent fix.
* src/qemu/qemu_monitor_text.c (qemuMonitorTextSendFileHandle):
Improve message, since TAP is no longer only client.
The O_NONBLOCK flag doesn't work as desired on plain files
or block devices. Introduce an I/O helper program that does
the blocking I/O operations, communicating over a pipe that
can support O_NONBLOCK
* src/fdstream.c, src/fdstream.h: Add non-blocking I/O
on plain files/block devices
* src/Makefile.am, src/util/iohelper.c: I/O helper program
* src/qemu/qemu_driver.c, src/lxc/lxc_driver.c,
src/uml/uml_driver.c, src/xen/xen_driver.c: Update for
streams API change
Spawn the compressor ourselves, instead of requiring the shell.
* src/qemu/qemu_migration.c (qemuMigrationToFile): Spawn
compression helper process when needed.
SELinux labeling and cgroup ACLs aren't required if we hand a
pre-opened fd to qemu. All the more reason to love fd: migration.
* src/qemu/qemu_migration.c (qemuMigrationToFile): Skip steps
that are irrelevant in fd migration.
This points out that core dumps (still) don't work for root-squash
NFS, since the fd is not opened correctly. This patch should not
introduce any functionality change, it is just a refactoring to
avoid duplicated code.
* src/qemu/qemu_migration.h (qemuMigrationToFile): New prototype.
* src/qemu/qemu_migration.c (qemuMigrationToFile): New function.
* src/qemu/qemu_driver.c (qemudDomainSaveFlag, doCoreDump): Use
it.
Direct access to an open file is so much simpler than passing
everything through a pipe!
* src/qemu/qemu_driver.c (qemudOpenAsUID)
(qemudDomainSaveImageClose): Delete.
(qemudDomainSaveImageOpen): Rename...
(qemuDomainSaveImageOpen): ...and drop read_pid argument. Use
virFileOpenAs instead of qemudOpenAsUID.
(qemudDomainSaveImageStartVM, qemudDomainRestore)
(qemudDomainObjRestore): Rename...
(qemuDomainSaveImageStartVM, qemuDomainRestore)
(qemDomainObjRestore): ...and simplify accordingly.
(qemudDomainObjStart, qemuDriver): Update callers.
This patch intentionally doesn't change indentation, in order to
make it easier to review the real changes.
* src/util/util.h (VIR_FILE_OP_RETURN_FD, virFileOperationHook):
Delete.
(virFileOperation): Rename...
(virFileOpenAs): ...and reduce parameters.
* src/util/util.c (virFileOperationNoFork, virFileOperation):
Rename and simplify.
* src/qemu/qemu_driver.c (qemudDomainSaveFlag): Adjust caller.
* src/storage/storage_backend.c (virStorageBackendCreateRaw):
Likewise.
* src/libvirt_private.syms: Reflect rename.
This allows direct saves (no compression, no root-squash NFS) to use
the more efficient fd: migration, which in turn avoids a race where
qemu exec: migration can sometimes fail because qemu does a generic
waitpid() that conflicts with the pclose() used by exec:. Further
patches will solve compression and root-squash NFS.
* src/qemu/qemu_driver.c (qemudDomainSaveFlag): Use new function
when there is no compression.
Latent bug introduced in commit 2d6a581960 (Aug 2009), but not exposed
until commit 1859939a (Jan 2011). Basically, when virExec creates a
pipe, it always marks libvirt's side as cloexec. If libvirt then
wants to hand that pipe to another child process, things work great if
the fd is dup2()'d onto stdin or stdout (as with stdin: or exec:
migration), but if the pipe is instead used as-is (such as with fd:
migration) then qemu sees EBADF because the fd was closed at exec().
This is a minimal fix for the problem at hand; it is slightly racy,
but no more racy than the rest of libvirt fd handling, including the
case of uncompressed save images. A more invasive fix, but ultimately
safer at avoiding leaking unintended fds, would be to _always and
atomically_ open all fds as cloexec in libvirt (thanks to primitives
like open(O_CLOEXEC), pipe2(), accept4(), ...), then teach virExec to
clear that bit for all fds explicitly marked to be handed to the child
only after forking.
* src/qemu/qemu_command.c (qemuBuildCommandLine): Clear cloexec
flag.
* tests/qemuxml2argvtest.c (testCompareXMLToArgvFiles): Tweak test.
Otherwise, if something like doStopVcpus fails after the first
restore, a second restore is attempted and throws a useless
warning.
* src/qemu/qemu_driver.c (qemudDomainSaveFlag): Avoid second
restore of state label.
Add missing open curly brace between function declaration of non-linux
variant of qemudDomainInterfaceStats() and its body.
Signed-off-by: Philipp Hahn <hahn@univention.de>
Hotpluging host usb device by text mode will fail, because the monitor
command 'device_add' outputs 'husb: using...' if it succeeds, but we
think the command should not output anything.
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Add the compiler attribute to ensure we don't introduce any more
ref bugs like were just patched in commit 9741f34, then explicitly
mark the remaining places in code that are safe.
* src/qemu/qemu_monitor.h (qemuMonitorUnref): Mark
ATTRIBUTE_RETURN_CHECK.
* src/conf/domain_conf.h (virDomainObjUnref): Likewise.
* src/conf/domain_conf.c (virDomainObjParseXML)
(virDomainLoadStatus): Fix offenders.
* src/openvz/openvz_conf.c (openvzLoadDomains): Likewise.
* src/vmware/vmware_conf.c (vmwareLoadDomains): Likewise.
* src/qemu/qemu_domain.c (qemuDomainObjBeginJob)
(qemuDomainObjBeginJobWithDriver)
(qemuDomainObjExitRemoteWithDriver): Likewise.
* src/qemu/qemu_monitor.c (QEMU_MONITOR_CALLBACK): Likewise.
Suggested by Daniel P. Berrange.
This simplifies several callers that were repeating checks already
guaranteed by util.c, and makes other callers more robust to now
reject directories. remote_driver.c was over-strict - access(,R_OK)
is only needed to execute a script file; a binary only needs
access(,X_OK) (besides, it's unusual to see a file with x but not
r permissions, whether script or binary).
* cfg.mk (sc_prohibit_access_xok): New syntax-check rule.
(exclude_file_name_regexp--sc_prohibit_access_xok): Exempt one use.
* src/network/bridge_driver.c (networkStartRadvd): Fix offenders.
* src/qemu/qemu_capabilities.c (qemuCapsProbeMachineTypes)
(qemuCapsInitGuest, qemuCapsInit, qemuCapsExtractVersionInfo):
Likewise.
* src/remote/remote_driver.c (remoteFindDaemonPath): Likewise.
* src/uml/uml_driver.c (umlStartVMDaemon): Likewise.
* src/util/hooks.c (virHookCheck): Likewise.
Steps to reproduce this bug:
1. virsh attach-disk domain --source diskimage --target sdb --sourcetype file --driver qemu --subdriver qcow2
error: Failed to attach disk
error: operation failed: adding scsi-disk,bus=scsi0.0,scsi-id=1,drive=drive-scsi0-0-1,id=scsi0-0-1 device failed: Property 'scsi-disk.drive' can't find value 'drive-scsi0-0-1'
2. service libvirtd restart
Stopping libvirtd daemon: [ OK ]
Starting libvirtd daemon: [ OK ]
3. virsh attach-disk domain --source diskimage --target sdb --sourcetype file --driver qemu --subdriver raw
error: Failed to attach disk
error: operation failed: adding lsi,id=scsi0,bus=pci.0,addr=0x6 device failed: Duplicate ID 'scsi0' for device
The reason is that we create a new scsi controller but we do not update
/var/run/libvirt/qemu/domain.xml.
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Steps to reproduce this bug:
# cat usb.xml
<hostdev mode='subsystem' type='usb'>
<source>
<address bus='0x001' device='0x003'/>
</source>
</hostdev>
# virsh attach-device vm1 usb.xml
error: Failed to attach device from usb.xml
error: server closed connection:
The reason of this bug is that we set data.cgroup to NULL, and this will cause
libvirtd crashed.
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
A future patch will change reference counting idioms; consolidating
this pattern now makes the next patch smaller (touch only the new
macro rather than every caller).
* src/qemu/qemu_monitor.c (QEMU_MONITOR_CALLBACK): New helper.
(qemuMonitorGetDiskSecret, qemuMonitorEmitShutdown)
(qemuMonitorEmitReset, qemuMonitorEmitPowerdown)
(qemuMonitorEmitStop, qemuMonitorEmitRTCChange)
(qemuMonitorEmitWatchdog, qemuMonitorEmitIOError)
(qemuMonitorEmitGraphics): Use it to reduce duplication.
Enhance the QEMU migration monitoring loop, so that it can get
a signal to change migration speed on the fly
* src/qemu/qemu_domain.h: Add signal for changing speed on the fly
* src/qemu/qemu_driver.c: Wire up virDomainMigrateSetSpeed driver
* src/qemu/qemu_migration.c: Support signal for changing speed
It is possible to set a migration speed limit when starting
migration. This new API allows the speed limit to be changed
on the fly to adjust to changing conditions
* src/driver.h, src/libvirt.c, src/libvirt_public.syms,
include/libvirt/libvirt.h.in: Add virDomainMigrateSetMaxSpeed
* src/esx/esx_driver.c, src/lxc/lxc_driver.c,
src/opennebula/one_driver.c, src/openvz/openvz_driver.c,
src/phyp/phyp_driver.c, src/qemu/qemu_driver.c,
src/remote/remote_driver.c, src/test/test_driver.c,
src/uml/uml_driver.c, src/vbox/vbox_tmpl.c,
src/vmware/vmware_driver.c, src/xen/xen_driver.c,
src/libxl/libxl_driver.c: Stub new API
Fix for bug https://bugzilla.redhat.com/show_bug.cgi?id=618970
The "prepare" hook is called very early in the VM statup process
before device labeling, so that it can allocate ressources not
managed by libvirt, such as DRBD, or for instance create missing
bridges and vlan interfaces.
* src/util/hooks.c src/util/hooks.h: add definitions for new hooks
VIR_HOOK_QEMU_OP_PREPARE and VIR_HOOK_QEMU_OP_RELEASE
* src/qemu/qemu_process.c: use them in qemuProcessStart and
qemuProcessStop()
With only a single caller to these two monitor commands, I
didn't need to wrap a new WithFds version, but just change
the command itself.
* src/qemu/qemu_monitor.h (qemuMonitorAddNetdev)
(qemuMonitorAddHostNetwork): Add parameters.
* src/qemu/qemu_monitor.c (qemuMonitorAddNetdev)
(qemuMonitorAddHostNetwork): Add support for fd passing.
* src/qemu/qemu_hotplug.c (qemuDomainAttachNetDevice): Use it to
simplify code.
This is also a bug fix - on the error path, qemu_hotplug would
leave the configfd file leaked into qemu. At least the next
attempt to hotplug a PCI device would reuse the same fdname,
and when the qemu getfd monitor command gets a new fd by the
same name as an earlier one, it closes the earlier one, so there
is no risk of qemu running out of fds.
* src/qemu/qemu_monitor.h (qemuMonitorAddDeviceWithFd): New
prototype.
* src/qemu/qemu_monitor.c (qemuMonitorAddDevice): Move guts...
(qemuMonitorAddDeviceWithFd): ...to new function, and add support
for fd passing.
* src/qemu/qemu_hotplug.c (qemuDomainAttachHostPciDevice): Use it
to simplify code.
Suggested by Daniel P. Berrange.
qemu_monitor was already returning -1 and setting errno to EINVAL
on any attempt to send an fd without a unix socket, but this was
a silent failure in the case of qemuDomainAttachHostPciDevice.
Meanwhile, qemuDomainAttachNetDevice was doing some sanity checking
for a better error message; it's better to consolidate that to a
central point in the API.
* src/qemu/qemu_hotplug.c (qemuDomainAttachNetDevice): Move sanity
checking...
* src/qemu/qemu_monitor.c (qemuMonitorSendFileHandle): ...into
central location.
Suggested by Chris Wright.
THREADS.txt states that the contents of vm should not be read or
modified while the vm lock is not held, but that the lock must not
be held while performing a monitor command. This fixes all the
offenders that I could find.
* src/qemu/qemu_process.c (qemuProcessStartCPUs)
(qemuProcessInitPasswords, qemuProcessStart): Don't modify or
refer to vm state outside lock.
* src/qemu/qemu_driver.c (qemudDomainHotplugVcpus): Likewise.
* src/qemu/qemu_hotplug.c (qemuDomainChangeGraphicsPasswords):
Likewise.
Steps to reproduce this bug:
# cat test.sh
#! /bin/bash -x
virsh start domain
sleep 5
virsh qemu-monitor-command domain 'cpu_set 2 online' --hmp
# while true; do ./test.sh ; done
Then libvirtd will crash.
The reason is that:
we add a reference of obj when we open the monitor. We will reduce this
reference when we free the monitor.
If the reference of monitor is 0, we will free monitor automatically and
the reference of obj is reduced.
But in the function qemuDomainObjExitMonitorWithDriver(), we reduce this
reference again when the reference of monitor is 0.
It will cause the obj be freed in the function qemuDomainObjEndJob().
Then we start the domain again, and libvirtd will crash in the function
virDomainObjListSearchName(), because we pass a null pointer(obj->def->name)
to strcmp().
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
This bug was reported by Shi Jin(jinzishuai@gmail.com):
=============
# virsh attach-disk RHEL6RC /var/lib/libvirt/images/test3.img vdb \
--driver file --subdriver qcow2
Disk attached successfully
# virsh save RHEL6RC /var/lib/libvirt/images/memory.save
Domain RHEL6RC saved to /var/lib/libvirt/images/memory.save
# virsh restore /var/lib/libvirt/images/memory.save
error: Failed to restore domain from /var/lib/libvirt/images/memory.save
error: internal error unsupported driver name 'file'
for disk '/var/lib/libvirt/images/test3.img'
=============
We check the driver name when we start or restore VM, but we do
not check it while attaching a disk. This adds the same check on disk
driverName used in qemuBuildCommandLine to qemudDomainAttachDevice.
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Steps to reproduce this bug:
# virsh qemu-monitor-command domain 'cpu_set 2 online' --hmp
The domain has 2 cpus, and we try to set the third cpu online.
The qemu crashes, and this command will hang.
The reason is that the refs is not 1 when we unwatch the monitor.
We lock the monitor, but we do not unlock it. So virCondWait()
will be blocked.
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
* Correct the documentation for cgroup: the swap_hard_limit indicates
mem+swap_hard_limit.
* Change cgroup private apis to: virCgroupGet/SetMemSwapHardLimit
Signed-off-by: Nikunj A. Dadhania <nikunj@linux.vnet.ibm.com>
POSIX states about dd:
If the bs=expr operand is specified and no conversions other than
sync, noerror, or notrunc are requested, the data returned from each
input block shall be written as a separate output block; if the read
returns less than a full block and the sync conversion is not
specified, the resulting output block shall be the same size as the
input block. If the bs=expr operand is not specified, or a conversion
other than sync, noerror, or notrunc is requested, the input shall be
processed and collected into full-sized output blocks until the end of
the input is reached.
Since we aren't using conv=sync, there is no zero-padding, but our
use of bs= means that a short read results in a short write. If
instead we use ibs= and obs=, then short reads are collected and dd
only has to do a single write, which can make dd more efficient.
* src/qemu/qemu_monitor.c (qemuMonitorMigrateToFile):
Avoid 'dd bs=', since it can cause short writes.
Outgoing migration still uses a Unix socket and or exec netcat until
the next patch.
* src/qemu/qemu_migration.c (qemuMigrationPrepareTunnel):
Replace Unix socket with simpler pipe.
Suggested by Paolo Bonzini.
The newly added call to qemuAuditNetDevice in qemuPhysIfaceConnect was
assuming that res_ifname (the name of the macvtap device) was always
valid, but this isn't the case. If openMacvtapTap fails, it always
returns NULL, which would result in a segv.
Since the audit log only needs a record of devices that are actually
sent to qemu, and a failure to open the macvtap device means that no
device will be sent to qemu, we can solve this problem by only doing
the audit if openMacvtapTap is successful (in which case res_ifname is
guaranteed valid).
This patch introduces a new libvirt API (virDomainSetMemoryFlags) and
a flag (virDomainMemoryModFlags).
Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
Opening raw network devices with the intent of passing those fds to
qemu is worth an audit point. This makes a multi-part audit: first,
we audit the device(s) that libvirt opens on behalf of the MAC address
of a to-be-created interface (which can independently succeed or
fail), then we audit whether qemu actually started the network device
with the same MAC (so searching backwards for successful audits with
the same MAC will show which fd(s) qemu is actually using). Note that
it is possible for the fd to be successfully opened but no attempt
made to pass the fd to qemu (for example, because intermediate
nwfilter operations failed) - no interface start audit will occur in
that case; so the audit for a successful opened fd does not imply
rights given to qemu unless there is a followup audit about the
attempt to start a new interface.
Likewise, when a network device is hot-unplugged, there is only one
audit message about the MAC being discontinued; again, searching back
to the earlier device open audits will show which fds that qemu quits
using (and yes, I checked via /proc/<qemu-pid>/fd that qemu _does_
close out the fds associated with an interface on hot-unplug). The
code would require much more refactoring to be able to definitively
state which device(s) were discontinued at that point, since we
currently don't record anywhere in the XML whether /dev/vhost-net was
opened for a given interface.
* src/qemu/qemu_audit.h (qemuAuditNetDevice): New prototype.
* src/qemu/qemu_audit.c (qemuAuditNetDevice): New function.
* src/qemu/qemu_command.h (qemuNetworkIfaceConnect)
(qemuPhysIfaceConnect, qemuOpenVhostNet): Adjust prototype.
* src/qemu/qemu_command.c (qemuNetworkIfaceConnect)
(qemuPhysIfaceConnect, qemuOpenVhostNet): Add audit points and
adjust parameters.
(qemuBuildCommandLine): Adjust caller.
* src/qemu/qemu_hotplug.c (qemuDomainAttachNetDevice): Likewise.
Since libvirt always passes /dev/net/tun to qemu via fd, we should
never trigger the cases where qemu tries to directly open the
device. Therefore, it is safer to deny the cgroup device ACL.
* src/qemu/qemu_cgroup.c (defaultDeviceACL): Remove /dev/net/tun.
* src/qemu/qemu.conf (cgroup_device_acl): Reflect this change.
qemu driver in libvirt gained support for creating domain snapshots
almost a year ago in libvirt 0.8.0. Since then we enabled QMP support
for qemu >= 0.13.0 but QMP equivalents of {save,load,del}vm commands are
not implemented in current qemu (0.14.0) so the domain snapshot support
is not very useful.
This patch detects when the appropriate QMP command is not implemented
and tries to use human-monitor-command (aka HMP passthrough) to run
it's HMP equivalent.
JSON monitor command implementation can now just directly call text
monitor implementation and it will be automatically encapsulated into
QMP's human-monitor-command.
Some qemu monitor event handlers were issuing inadequate warning when
virDomainSaveStatus() failed. They copied the message from I/O error
handler without customizing it to provide better information on why
virDomainSaveStatus() was called.
Adding audit points showed that we were granting too much privilege
to qemu; it should not need any mknod rights to recreate any
devices. On the other hand, lxc should have all device privileges.
The solution is adding a flag parameter.
This also lets us restrict write access to read-only disks.
* src/util/cgroup.h (virCgroup*Device*): Adjust prototypes.
* src/util/cgroup.c (virCgroupAllowDevice)
(virCgroupAllowDeviceMajor, virCgroupAllowDevicePath)
(virCgroupDenyDevice, virCgroupDenyDeviceMajor)
(virCgroupDenyDevicePath): Add parameter.
* src/qemu/qemu_driver.c (qemudDomainSaveFlag): Update clients.
* src/lxc/lxc_controller.c (lxcSetContainerResources): Likewise.
* src/qemu/qemu_cgroup.c: Likewise.
(qemuSetupDiskPathAllow): Also, honor read-only disks.
Although the cgroup device ACL controller path can be worked out
by researching the code, it is more efficient to include that
information directly in the audit message.
* src/util/cgroup.h (virCgroupPathOfController): New prototype.
* src/util/cgroup.c (virCgroupPathOfController): Export.
* src/libvirt_private.syms: Likewise.
* src/qemu/qemu_audit.c (qemuAuditCgroup): Use it.
Device names can be manipulated, so it is better to also log
the major/minor device number corresponding to the cgroup ACL
changes that libvirt made. This required some refactoring
of the relatively new qemu cgroup audit code.
Also, qemuSetupChardevCgroup was only auditing on failure, not success.
* src/qemu/qemu_audit.h (qemuDomainCgroupAudit): Delete.
(qemuAuditCgroup, qemuAuditCgroupMajor, qemuAuditCgroupPath): New
prototypes.
* src/qemu/qemu_audit.c (qemuDomainCgroupAudit): Rename...
(qemuAuditCgroup): ...and drop a parameter.
(qemuAuditCgroupMajor, qemuAuditCgroupPath): New functions, to
allow listing device major/minor in audit.
(qemuAuditGetRdev): New helper function.
* src/qemu/qemu_driver.c (qemudDomainSaveFlag): Adjust callers.
* src/qemu/qemu_cgroup.c (qemuSetupDiskPathAllow)
(qemuSetupHostUsbDeviceCgroup, qemuSetupCgroup)
(qemuTeardownDiskPathDeny): Likewise.
(qemuSetupChardevCgroup): Likewise, fixing missing audit.
* src/qemu/qemu_audit.c (qemuDomainHostdevAudit): Avoid use of
"type", which has a pre-defined meaning.
(qemuDomainCgroupAudit): Likewise, as well as "item".
virRun gives pretty useful error output, let's not overwrite it unless there
is a good reason. Some places were providing more information about what
the commands were _attempting_ to do, however that's usually less useful from
a debugging POV than what actually happened.
The way to detach a USB disk is the same as that to detach a SCSI
disk. Rename this function and we can use it to detach a USB disk.
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
For qemu names the primary vga as "qxl-vga":
1) if vram is specified for 2nd qxl device:
-vga qxl -global qxl-vga.vram_size=$SIZE \
-device qxl,id=video1,vram_size=$SIZE,...
2) if vram is not specified for 2nd qxl device, (use the default
set by global):
-vga qxl -global qxl-vga.vram_size=$SIZE \
-device qxl,id=video1,...
For qemu names all qxl devices as "qxl":
1) if vram is specified for 2nd qxl device:
-vga qxl -global qxl.vram_size=$SIZE \
-device qxl,id=video1,vram_size=$SIZE ...
2) if vram is not specified for 2nd qxl device:
-vga qxl -global qxl-vga.vram_size=$SIZE \
-device qxl,id=video1,...
"-global" is the only way to define vram_size for the primary qxl
device, regardless of how qemu names it, (It's not good a good
way, as original idea of "-global" is to set a global default for
a driver property, but to specify vram for first qxl device, we
have to use it).
For other qxl devices, as they are represented by "-device", could
specify it directly and seperately for each, and it overrides the
default set by "-global" if specified.
v1 - v2:
* modify "virDomainVideoDefaultRAM" so that it returns 16M as the
default vram_size for qxl device.
* vram_size * 1024 (qemu accepts bytes for vram_size).
* apply default vram_size for qxl device for which vram_size is
not specified.
* modify "graphics-spice" tests (more sensiable vram_size)
* Add an argument of virDomainDefPtr type for qemuBuildVideoDevStr,
to use virDomainVideoDefaultRAM in qemuBuildVideoDevStr).
v2 - v3:
* Modify default video memory size for qxl device from 16M to 24M
* Update codes to be consistent with changes on qemu_capabilities.*
This was also found while investigating
https://bugzilla.redhat.com/show_bug.cgi?id=670848
An EOF on a domain's monitor socket results in an event being queued
to handle the EOF. The handler calls qemuProcessHandleMonitorEOF. If
it is a transient domain, this leads to a call to
virDomainRemoveInactive, which removes the domain from the driver's
hashtable and unref's it. Nowhere in this code is the qemu driver lock
acquired.
However, all modifications to the driver's domain hashtable *must* be
done while holding the driver lock, otherwise the hashtable can become
corrupt, and (even more likely) another thread could call a different
hashtable function and acquire a pointer to the domain that is in the
process of being destroyed.
To prevent such a disaster, qemuProcessHandleMonitorEOF must get the
qemu driver lock *before* it gets the DomainObj's lock, and hold it
until it is finished with the DomainObj. This guarantees that nobody
else modifies the hashtable at the same time, and that anyone who had
already gotten the DomainObj from the hashtable prior to this call has
finished with it before we remove/destroy it.
This was found while researching the root cause of:
https://bugzilla.redhat.com/show_bug.cgi?id=670848
virDomainUnref should only be called with the lock held for the
virDomainObj in question. However, when a transient qemu domain gets
EOF on its monitor socket, it queues an event which frees the monitor,
which unref's the virDomainObj without first locking it. If another
thread has already locked the virDomainObj, the modification of the
refcount could potentially be corrupted. In an extreme case, it could
also be potentially unlocked by virDomainObjFree, thus left open to
modification by anyone else who would have otherwise waited for the
lock (not to mention the fact that they would be accessing freed
data!).
The solution is to have qemuMonitorFree lock the domain object right
before unrefing it. Since the caller to qemuMonitorFree doesn't expect
this lock to be held, if the refcount doesn't go all the way to 0,
qemuMonitorFree must unlock it after the unref.
qemudDomainSaveImageStartVM was evil - it closed the incoming fd
argument on some, but not all, code paths, without informing the
caller about that action. No wonder that this resulted in
double-closes: https://bugzilla.redhat.com/show_bug.cgi?id=672725
* src/qemu/qemu_driver.c (qemudDomainSaveImageStartVM): Alter
signature, to avoid double-close.
(qemudDomainRestore, qemudDomainObjRestore): Update callers.
When a SPICE or VNC graphics controller is present, and sound is
piggybacked over a channel to the graphics device rather than
directly accessing host hardware, then there is no need to grant
host hardware access to that qemu process.
* src/qemu/qemu_cgroup.c (qemuSetupCgroup): Prevent sound with
spice, and with vnc when vnc_allow_host_audio is 0.
Reported by Daniel Berrange.
Relax the restriction that the hash table key must be a string
by allowing an arbitrary hash code generator + comparison func
to be provided
* util/hash.c, util/hash.h: Allow any pointer as a key
* internal.h: Include stdbool.h as standard.
* conf/domain_conf.c, conf/domain_conf.c,
conf/nwfilter_params.c, nwfilter/nwfilter_gentech_driver.c,
nwfilter/nwfilter_gentech_driver.h, nwfilter/nwfilter_learnipaddr.c,
qemu/qemu_command.c, qemu/qemu_driver.c,
qemu/qemu_process.c, uml/uml_driver.c,
xen/xm_internal.c: s/char */void */ in hash callbacks
Since the deallocator is passed into the constructor of
a hash table it is not desirable to pass it into each
function again. Remove it from all functions, but provide
a virHashSteal to allow a item to be removed from a hash
table without deleteing it.
* src/util/hash.c, src/util/hash.h: Remove deallocator
param from all functions. Add virHashSteal
* src/libvirt_private.syms: Add virHashSteal
* src/conf/domain_conf.c, src/conf/nwfilter_params.c,
src/nwfilter/nwfilter_learnipaddr.c,
src/qemu/qemu_command.c, src/xen/xm_internal.c: Update
for changed hash API
This is done for two reasons:
- we are getting very close to 64 flags which is the maximum we can use
with unsigned long long
- by using LL constants in enum we already violates C99 constraint that
enum values have to fit into int
Steps to reproduce this bug:
1. virsh attach-disk domain --source imagefile --target sdb --sourcetype file --driver qemu --subdriver raw
2. virsh detach-device controller.xml # remove scsi controller 0
3. virsh detach-disk domain sdb
error: Failed to detach disk
error: operation failed: detaching scsi0-0-1 device failed: Device 'scsi0-0-1' not found
I think we should not detach a controller when it is used by some other device.
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Done mechanically with:
$ git grep -l '\bDEBUG0\? *(' | xargs -L1 sed -i 's/\bDEBUG0\? *(/VIR_&/'
followed by manual deletion of qemudDebug in daemon/libvirtd.c, along
with a single 'make syntax-check' fallout in the same file, and the
actual deletion in src/util/logging.h.
* src/util/logging.h (DEBUG, DEBUG0): Delete.
* daemon/libvirtd.h (qemudDebug): Likewise.
* global: Change remaining clients over to VIR_DEBUG counterpart.
Now that the virHash handling functions call virReportOOMError by
themselves when needed, users of the virHash API no longer need to
do it by themselves. Since users of the virHash API were not
consistently calling virReportOOMError after memory failures from
the virHash code, this has the added benefit of making OOM
reporting from this code more consistent and reliable.
When we attach a disk, but we specify a wrong format of disk image,
qemu monitor command drive_add will fail, but libvirt does not detect
this error.
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
An upcoming patch has a use for a tap device to be created that
doesn't need to be actually put into the "up" state, and keeping it
"down" keeps the output of ifconfig from being unnecessarily cluttered
(ifconfig won't show down interfaces unless you add "-a").
bridge.[ch]: add "up" as an arg to brAddTap()
uml_conf.c, qemu_command.c: add "up" (set to "true") to brAddTap() call.
This is in response to:
https://bugzilla.redhat.com/show_bug.cgi?id=629662
Explanation
qemu's virtio-net-pci driver allows setting the algorithm used for tx
packets to either "bh" or "timer". This is done by adding ",tx=bh" or
",tx=timer" to the "-device virtio-net-pci" commandline option.
'bh' stands for 'bottom half'; when this is set, packet tx is all done
in an iothread in the bottom half of the driver. (In libvirt, this
option is called the more descriptive "iothread".)
'timer' means that tx work is done in qemu, and if there is more tx
data than can be sent at the present time, a timer is set before qemu
moves on to do other things; when the timer fires, another attempt is
made to send more data. (libvirt retains the name "timer" for this
option.)
The resulting difference, according to the qemu developer who added
the option is:
bh makes tx more asynchronous and reduces latency, but potentially
causes more processor bandwidth contention since the cpu doing the
tx isn't necessarily the cpu where the guest generated the
packets.
Solution
This patch provides a libvirt domain xml knob to change the option on
the qemu commandline, by adding a new attribute "txmode" to the
<driver> element that can be placed inside any <interface> element in
a domain definition. It's use would be something like this:
<interface ...>
...
<model type='virtio'/>
<driver txmode='iothread'/>
...
</interface>
I chose to put this setting as an attribute to <driver> rather than as
a sub-element to <tune> because it is specific to the virtio-net
driver, not something that is generally usable by all network drivers.
(note that this is the same placement as the "driver name=..."
attribute used to choose kernel vs. userland backend for the
virtio-net driver.)
Actually adding the tx=xxx option to the qemu commandline is only done
if the version of qemu being used advertises it in the output of
qemu -device virtio-net-pci,?
If a particular txmode is requested in the XML, and the option isn't
listed in that help output, an UNSUPPORTED_CONFIG error is logged, and
the domain fails to start.
When the <driver> element (and its "name" attribute) was added to the
domain XML's interface element, a "backend" enum was simply added to
the toplevel of the virDomainNetDef struct.
Ignoring the naming inconsistency ("name" vs. "backend"), this is fine
when there's only a single item contained in the driver element of the
XML, but doesn't scale well as we add more attributes that apply to
the backend of the virtio-net driver, or add attributes applicable to
other drivers.
This patch changes virDomainNetDef in two ways:
1) Rename the item in the struct from "backend" to "name", so that
it's the same in the XML and in the struct, hopefully avoiding
confusion for someone unfamiliar with the function of the
attribute.
2) Create a "driver" union within virDomainNetDef, and a "virtio"
struct in that struct, which contains the "name" enum value.
3) Move around the virDomainNetParse and virDomainNetFormat functions
to allow for simple plugin of new attributes without disturbing
existing code. (you'll note that this results in a seemingly
redundant if() in the format function, but that will no longer be
the case as soon as a 2nd attribute is added).
In the future, new attributes for the virtio driver backend can be
added to the "virtio" struct, and any other network device backend that
needs an attribute will have its own struct added to the "driver"
union.
The introduction of the v3 migration protocol, along with
support for migration cookies, will significantly expand
the size of the migration code. Move it all to a separate
file to make it more manageable
The functions are not moved 100%. The API entry points
remain in the main QEMU driver, but once the public
virDomainPtr is resolved to the internal virDomainObjPtr,
all following code is moved.
This will allow the new v3 API entry points to call into the
same shared internal migration functions
* src/qemu/qemu_domain.c, src/qemu/qemu_domain.h: Add
qemuDomainFormatXML helper method
* src/qemu/qemu_driver.c: Remove all migration code
* src/qemu/qemu_migration.c, src/qemu/qemu_migration.h: Add
all migration code.
Move the qemudStartVMDaemon and qemudShutdownVMDaemon
methods into a separate file, renaming them to
qemuProcessStart, qemuProcessStop. All helper methods
called by these are also moved & renamed to match
* src/Makefile.am: Add qemu_process.c/.h
* src/qemu/qemu_command.c: Add qemuDomainAssignPCIAddresses
* src/qemu/qemu_command.h: Add VNC port min/max
* src/qemu/qemu_domain.c, src/qemu/qemu_domain.h: Add
domain event queue helpers
* src/qemu/qemu_driver.c, src/qemu/qemu_driver.h: Remove
all QEMU process startup/shutdown functions
* src/qemu/qemu_process.c, src/qemu/qemu_process.h: Add
all QEMU process startup/shutdown functions
"qemudDomainSaveFlag" goto wrong label "endjob", which will cause
error when security manager trying to restore label (regression).
As it's more reasonable to check if vm is shutoff immediately, and
return right away if it is, remove the checking in "qemudDomainSaveFlag",
and add checking in "qemudDomainSave".
* src/qemu/qemu_driver.c
The code expected that host CPU architecture matches the architecture on
which libvirt runs. This is normally true but not in tests, where host
CPU is faked to produce consistent results.
The processWatchdogEvent fix is real, although it can only trigger
on OOM, since bad things happen if doCoreDump is called with a NULL
pathname argument. The other fixes silence clang, but aren't a real
bug because virReportErrorHelper tolerates a NULL format string even
though *printf does not.
* src/qemu/qemu_driver.c (processWatchdogEvent): Exit on OOM.
(qemuDomainIsActive, qemuDomainIsPersistent, qemuDomainIsUpdated):
Provide valid message.
Commit 9962e406c6 introduced a
problem where if the VM failed to startup, it would not be
correctly cleaned up. Amongst other things the SELinux
security label would not be removed, which prevents the VM
from ever starting again.
The virDomainIsActive() check at the start of qemudShutdownVMDaemon
checks for vm->def->id not being -1. By moving the assignment of the
VM id to the start of qemudStartVMDaemon, we can ensure cleanup will
occur on failure
* src/qemu/qemu_driver.c: Move initialization of 'vm->def->id'
so that qemudShutdownVMDaemon() will process the shutdown
Suspending a VM which contains shell meta characters doesn't work with
libvirt-0.8.7:
/var/log/libvirt/qemu/andreas_231-ne\ doch\ nicht.log:
sh: -c: line 0: syntax error near unexpected token `doch'
sh: -c: line 0: `cat | { dd bs=4096 seek=1 if=/dev/null && dd bs=1048576; }
Although target="andreas_231-ne doch nicht" contains shell meta
characters (here: blanks), they are not properly escaped by
src/qemu/qemu_monitor_{json,text}.c#qemuMonitor{JSON,Text}MigrateToFile()
First, the filename needs to be properly escaped for the shell, than
this command line has to be properly escaped for qemu again.
For this to work, remove the old qemuMonitorEscapeArg() wrapper, rename
qemuMonitorEscape() to it removing the handling for shell=TRUE, and
implement a new qemuMonitorEscapeShell() returning strings using single
quotes.
Using double quotes or escaping special shell characters with backslashes
would also be possible, but the set of special characters heavily
depends on the concrete shell (dsh, bash, zsh) and its setting (history
expansion, interactive use, ...)
Signed-off-by: Philipp Hahn <hahn@univention.de>
QEMUD_CMD_FLAG_PCI_MULTIBUS should be set in the function
qemuCapsExtractVersionInfo()
The flag QEMUD_CMD_FLAG_PCI_MULTIBUS is used in the function
qemuBuildDeviceAddressStr(). All callers get qemuCmdFlags
by the function qemuCapsExtractVersionInfo() except that
testCompareXMLToArgvFiles() in qemuxml2argvtest.c.
So we should set QEMUD_CMD_FLAG_PCI_MULTIBUS in the function
qemuCapsExtractVersionInfo() instead of qemuBuildCommandLine()
because the function qemuBuildCommandLine() does not be called
when we attach a pci device.
tests: set QEMUD_CMD_FLAG_PCI_MULTIBUS in testCompareXMLToArgvFiles()
set QEMUD_CMD_FLAG_PCI_MULTIBUS before calling qemuBuildCommandLine()
as the flags is not set by qemuCapsExtractVersionInfo().
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Quite a few hosts don't have cgroups mounted and so see warnings
from libvirt logged, which then cause bug reports, etc. Reduce
the log level to INFO so they're not visible by default
* src/qemu/qemu_driver.c: Reduce log level for cgroups
When probing machine types if the QEMU binary does not exist
we get a hard to diagnose error, due to the execve() in the
child failing
error: internal error Child process exited with status 1.
Add an explicit check so that we get
error: Cannot find QEMU binary /usr/libexec/qem3u-kvm: No such file or directory
* src/qemu/qemu_capabilities.c: Check for QEMU binary
qemu 0.13.0 (at least as built for Fedora 14, and also backported to
RHEL 6.0 qemu) supported an older syntax for a spicevmc channel; it's
not as flexible (it has an implicit name and hides the chardev
aspect), but now that we support spicevmc, we might as well target
both variants.
* src/qemu/qemu_capabilities.h (QEMUD_CMD_FLAG_DEVICE_SPICEVMC):
New flag.
* src/qemu/qemu_capabilities.c (qemuCapsParseDeviceStr): Set it
correctly.
* src/qemu/qemu_command.h (qemuBuildVirtioSerialPortDevStr): Drop
declaration.
* src/qemu/qemu_command.c (qemuBuildVirtioSerialPortDevStr): Alter
signature, check flag.
(qemuBuildCommandLine): Adjust caller and check flag.
* tests/qemuhelptest.c (mymain): Update test.
* tests/qemuxml2argvtest.c (mymain): New test.
* tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc-old.xml:
New file.
* tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc-old.args:
Likewise.
Adds <smartcard mode='passthrough' type='spicevmc'/>, which uses the
new <channel name='smartcard'/> of <graphics type='spice'>.
* docs/schemas/domain.rng: Support new XML.
* docs/formatdomain.html.in: Document it.
* src/conf/domain_conf.h (virDomainGraphicsSpiceChannelName): New
enum value.
(virDomainChrSpicevmcName): New enum.
(virDomainChrSourceDef): Distinguish spicevmc types.
* src/conf/domain_conf.c (virDomainGraphicsSpiceChannelName): Add
smartcard.
(virDomainSmartcardDefParseXML): Parse it.
(virDomainChrDefParseXML, virDomainSmartcardDefParseXML): Set
spicevmc name.
(virDomainChrSpicevmc): New enum conversion functions.
* src/libvirt_private.syms: Export new functions.
* src/qemu/qemu_command.c (qemuBuildChrChardevStr): Conditionalize
name.
* tests/qemuxml2argvtest.c (domain): New test.
* tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.args:
New file.
* tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.xml:
Likewise.
Inspired by https://bugzilla.redhat.com/show_bug.cgi?id=615757
Add a new character device backend for virtio serial channels that
activates the QEMU spice agent on the main channel using the vdagent
spicevmc connection. The <target> must be type='virtio', and supports
an optional name that specifies how the guest will see the channel
(for now, name must be com.redhat.spice.0).
<channel type='spicevmc'>
<target type='virtio'/>
<address type='virtio-serial' controller='1' bus='0' port='3'/>
</channel>
* docs/schemas/domain.rng: Support new XML.
* docs/formatdomain.html.in: Document it.
* src/conf/domain_conf.h (virDomainChrType): New enum value.
* src/conf/domain_conf.c (virDomainChr): Add spicevmc.
(virDomainChrDefParseXML, virDomainChrSourceDefParseXML)
(virDomainChrDefParseTargetXML): Parse and enforce proper use.
(virDomainChrSourceDefFormat, virDomainChrDefFormat): Format.
* src/qemu/qemu_command.c (qemuBuildChrChardevStr)
(qemuBuildCommandLine): Add qemu support.
* tests/qemuxml2argvtest.c (domain): New test.
* tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml: New
file.
* tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args:
Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Qemu smartcard/spicevmc support exists on branches (such as
http://cgit.freedesktop.org/~alon/qemu/commit/?h=usb_ccid.v15&id=024a37b)
but is not yet upstream. The added -help output matches a scratch build
that will be close to the RHEL 6.1 qemu-kvm.
* src/qemu/qemu_capabilities.h (QEMUD_CMD_FLAG_CCID_EMULATED)
(QEMUD_CMD_FLAG_CCID_PASSTHRU, QEMUD_CMD_FLAG_CHARDEV_SPICEVMC):
New flags.
* src/qemu/qemu_capabilities.c (qemuCapsComputeCmdFlags)
(qemuCapsParseDeviceStr): Check for smartcard capabilities.
(qemuCapsExtractVersionInfo): Tweak comment.
* tests/qemuhelptest.c (mymain): New test.
* tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel61: New file.
* tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel61-device: Likewise.
Currently users who want to use virDomainQemuMonitorCommand() API or
it's virsh equivalent has to use the same protocol as libvirt uses for
communication to qemu. Since the protocol is QMP with current qemu and
HMP much more usable for humans, one ends up typing something like the
following:
virsh qemu-monitor-command DOM \
'{"execute":"human-monitor-command","arguments":{"command-line":"info kvm"}}'
which is not a very convenient way of debugging qemu.
This patch introduces --hmp option to qemu-monitor-command, which says
that the provided command is in HMP. If libvirt uses QMP to talk with
qemu, the command will automatically be converted into QMP. So the
example above is simplified to just
virsh qemu-monitor-command --hmp DOM "info kvm"
Also the result is converted from
{"return":"kvm support: enabled\r\n"}
to just plain HMP:
kvm support: enabled
If libvirt talks to qemu in HMP, --hmp flag is obviously a noop.
* src/qemu/qemu_driver.c (qemudShutdownVMDaemon): Check that vm is
still active.
Reported by Wen Congyang as follows:
Steps to reproduce this bug:
1. use gdb to debug libvirtd, and set breakpoint in the function
qemuConnectMonitor()
2. start a vm, and the libvirtd will be stopped in qemuConnectMonitor()
3. kill -STOP $(cat /var/run/libvirt/qemu/<domain>.pid)
4. continue to run libvirtd in gdb, and libvirtd will be blocked in the
function qemuMonitorSetCapabilities()
5. kill -9 $(cat /var/run/libvirt/qemu/<domain>.pid)
Here is log of the qemu:
=========
LC_ALL=C PATH=/sbin:/usr/sbin:/bin:/usr/bin ...
char device redirected to /dev/pts/3
2011-01-27 09:38:48.101: shutting down
2011-01-27 09:41:26.401: shutting down
=========
The vm is shut down twice. I do not know whether this behavior has
side effect, but I think we should shutdown the vm only once.
Depending if the qemu binary supports multiple pci-busses, the device
options will contain "bus=pci" or "bus=pci.0".
Only x86_64 and i686 seem to have support for multiple PCI-busses. When
a guest of these architectures is started, set the
QEMUD_CMD_FLAG_PCI_MULTIBUS flag.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Command line building for incoming tunneled migration is missed,
as a result, all the tunneled migration will fail with "unknown
migration protocol".
* src/qemu/qemu_command.c
Use it in all places where a memory or storage request size is converted
to a larger granularity. This avoids requesting too small memory or storage
sizes that could result from the truncation done by a simple division.
This extends the round up fix in 6002e0406c
to the whole codebase.
Instead of reporting errors for odd values in the VMX code round them up.
Update the QEMU Argv tests accordingly as the original memory size 219200
isn't a even multiple of 1024 and is rounded up to 215 megabyte now. Change
it to 219100 and 219136. Use two different values intentionally to make
sure that rounding up works.
Update virsh.pod accordingly, as rounding down and rejecting are replaced
by rounding up.
Fixes test failure that was overlooked after commit 1e1f7a8950.
* daemon/Makefile.am (check-local): Let 'make check' fail on error.
* daemon/test_libvirtd.aug: Move qemu-specific option...
* src/qemu/test_libvirtd_qemu.aug: ...into correct test.
* src/qemu/libvirtd_qemu.aug: Parse new option.
qemu allows the user to choose what io storage api should be used,
either the default (threads) or native (linux aio) which in the latter
case can result in better performance.
Based on a patch originally by Matthias Dahl.
Red Hat Bugzilla #591703
Signed-off-by: Eric Blake <eblake@redhat.com>
The refactoring of QEMU command startup was comitted with
a couple of VIR_WARN lines left in from debugging.
* src/qemu/qemu_driver.c: Remove log warning lines
When qemuMonitorSetCapabilities() fails, there is no need to
call qemuMonitorClose(), because the caller will already see
the error code and tear down the entire VM. The extra call to
qemuMonitorClose resulted in a double-free due to it removing
a ref count prematurely.
* src/qemu/qemu_driver.c: Remove premature close of monitor
Regression in commit caa805ea let a lot of bad messages slip in.
* cfg.mk (msg_gen_function): Fix function name.
* src/qemu/qemu_cgroup.c (qemuRemoveCgroup): Fix fallout from
'make syntax-check'.
* src/qemu/qemu_driver.c (qemudDomainGetInfo)
(qemuDomainWaitForMigrationComplete, qemudStartVMDaemon)
(qemudDomainSaveFlag, qemudDomainAttachDevice)
(qemuDomainUpdateDeviceFlags): Likewise.
* src/qemu/qemu_hotplug.c (qemuDomainAttachHostUsbDevice)
(qemuDomainDetachPciDiskDevice, qemuDomainDetachSCSIDiskDevice):
Likewise.
When attaching device from a xml file and the device is mis-configured,
virsh gives mis-leading message "out of memory". This patch fixes this.
Signed-off-by: Eric Blake <eblake@redhat.com>
* src/qemu/qemu_command.c (qemuBuildChrChardevStr): Alter the
chardev alias.
(qemuBuildCommandLine): Output an id for the chardev counterpart.
* tests/qemuxml2argvdata/*: Update tests to match.
Reported by Daniel P. Berrange.
Steps to reproduce this bug:
1. service libvirtd start
2. virsh start <domain>
3. kill -STOP $(cat /var/run/libvirt/qemu/<domain>.pid)
4. service libvirtd restart
5. kill -9 $(cat /var/run/libvirt/qemu/<domain>.pid)
Then libvirtd will core dump or be in deadlock state.
Make sure that json is built into libvirt and the version
of qemu is newer than 0.13.0.
The reason of libvirtd cores dump is that:
We add vm->refs when we alloc the memory, and decrease it
in the function qemuHandleMonitorEOF() in other thread.
We add vm->refs in the function qemuConnectMonitor() and
decrease it when the vm is inactive.
The libvirtd will block in the function qemuMonitorSetCapabilities()
because the vm is stopped by signal SIGSTOP. Now the vm->refs is 2.
Then we kill the vm by signal SIGKILL. The function
qemuMonitorSetCapabilities() failed, and then we will decrease vm->refs
in the function qemuMonitorClose().
In another thread, mon->fd is broken and the function
qemuHandleMonitorEOF() is called.
If qemuHandleMonitorEOF() decreases vm->refs before qemuConnectMonitor()
returns, vm->refs will be decrease to 0 and the memory is freed.
We will call qemudShutdownVMDaemon() as qemuConnectMonitor() failed.
The memory has been freed, so qemudShutdownVMDaemon() is too dangerous.
We will reference NULL pointer in the function virDomainConfVMNWFilterTeardown():
=============
void
virDomainConfVMNWFilterTeardown(virDomainObjPtr vm) {
int i;
if (nwfilterDriver != NULL) {
for (i = 0; i < vm->def->nnets; i++)
virDomainConfNWFilterTeardown(vm->def->nets[i]);
}
}
============
vm->def->nnets is not 0 but vm->def->nets is NULL(We don't set vm->def->nnets
to 0 when we free vm).
We should add an extra reference of vm to avoid vm to be deleted if
qemuConnectMonitor() failed.
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
If a guest image is saved in compressed format, and the restore fails
in some way after the intermediate process used to uncompress the
image has been started, but before qemu has been started to hook up to
the uncompressor, libvirt will endlessly wait for the uncompressor to
finish, but it never will because it's still waiting to have something
hooked up to drain its output.
The solution is to close the pipes on both sides of the uncompressor,
then send a SIGTERM before calling waitpid on it (only if the restore
has failed, of course).
This patch is a partial resolution to the following bug:
https://bugzilla.redhat.com/show_bug.cgi?id=667756
(to complete the fix, an updated selinux-policy package is required,
to add the policy that allows libvirt to set the context of a fifo,
which was previously not allowed).
Explanation : When an incoming migration is over a pipe (for example,
if the image was compressed and is being fed through gzip, or was on a
root-squash nfs server, so needed to be opened by a child process
running as a different uid), qemu cannot read it unless the selinux
context label for the pipe has been set properly.
The solution is to check the fd used as the source of the migration
just before passing it to qemu; if it's a fifo (implying that it's a
pipe), we call the newly added virSecurityManagerSetFDLabel() function
to set the context properly.
The problem was introduced by commit 4303c91, which removed the checking
of domain state, this patch is to fix it.
Otherwise, improper error will be thrown, e.g.
error: Failed to save domain rhel6 state
error: cannot resolve symlink /var/lib/libvirt/qemu/save/rhel6.save: No such
file or directory
In QEMU, the card itself is a PCI device, but it requires a codec
(either -device hda-output or -device hda-duplex) to actually output
sound. Specifying <sound model='ich6'/> gives us -device intel-hda
-device hda-duplex I think it's important that a simple <sound model='ich6'/>
sets up a useful codec, to have consistent behavior with all other sound cards.
This is basically Dan's proposal of
<sound model='ich6'>
<codec type='output' slot='0'/>
<codec type='duplex' slot='3'/>
</sound>
without the codec bits implemented.
The important thing is to keep a consistent API here, we don't want some
<sound> devs require tweaking codecs but not others. Steps I see to
accomplishing this:
- every <sound> device has a <codec type='default'/> (unless codecs are
manually specified)
- <codec type='none'/> is required to specify 'no codecs'
- new audio settings like mic=on|off could then be exposed in
<sound> or <codec> in a consistent manner for all sound models
v2:
Use model='ich6'
v3:
Use feature detection, from eblake
Set codec id, bus, and cad values
v4:
intel-hda isn't supported if -device isn't available
v5:
Comment spelling fixes
If vnc_auto_unix_socket is enabled, any VNC devices without a hardcoded
listen or socket value will be setup to serve over a unix socket in
/var/lib/libvirt/qemu/$vmname.vnc.
We store the generated socket path in the transient VM definition at
CLI build time.
QEMU supports serving VNC over a unix domain socket rather than traditional
TCP host/port. This is specified with:
<graphics type='vnc' socket='/foo/bar/baz'/>
This provides better security access control than VNC listening on
127.0.0.1, but will cause issues with tools that rely on the lax security
(virt-manager in fedora runs as regular user by default, and wouldn't be
able to access a socket owned by 'qemu' or 'root').
Also not currently supported by any clients, though I have patches for
virt-manager, and virt-viewer should be simple to update.
v2:
schema: Make listen vs. socket a <choice>
This will allow us to record transient runtime state in vm->def, like
default VNC parameters. Accomplish this by adding an extra 'live' parameter
to SetDefTransient, with similar semantics to the 'live' flag for
AssignDef.
When restoring a saved qemu instance via JSON monitor, the vm is
left in a paused state. Turns out the 'cont' cmd was failing with
"MigrationExpected" error class and "An incoming migration is
expected before this command can be executed" error description
due to migration (restore) not yet complete.
Detect if 'cont' cmd fails with "MigrationExpecte" error class and
retry 'cont' cmd.
V2: Fix potential double-free noted by Laine Stump
Report VIR_ERR_CONFIG_UNSUPPORTED instead of VIR_ERR_INTERNAL_ERROR,
as it's valid in our domain schema, just unsupported by hypervisor
here.
* src/qemu/qemu_command.c
The code which set VNC passwords correctly had fallback for
the set_password command, but was lacking it for the
expire_password command. This made it impossible to start
a guest. It also failed to check whether QEMU was still
running after the initial 'set_password' command completed
* src/qemu/qemu_hotplug.c: Fix error handling when
password expiry fails
* src/qemu/qemu_monitor_json.c, src/qemu/qemu_monitor_text.c: Fix
return code for missing expire_password command
Avoid overwriting the real error message with a generic
OOM failure message, when machine type probe fails
* src/qemu/qemu_driver.c: Don't overwrite error
The function virUnrefConnect() may call virReleaseConnect() to release
the dest connection, and the function virReleaseConnect() will call
conn->driver->close().
So the function virUnrefConnect() should be surrounded by
qemuDomainObjEnterRemoteWithDriver() and
qemuDomainObjExitRemoteWithDriver() to prevent possible deadlock between
two communicating libvirt daemons.
See commit f0c8e1cb37 for further details.
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
In some circumstances, libvirtd would issue two STOPPED events after it
stopped a domain. This was because an EOF event can arrive after a qemu
process is killed but before qemuMonitorClose() is called.
qemuHandleMonitorEOF() should ignore EOF when the domain is not running.
I wasn't able to reproduce this bug directly, only after adding an
artificial sleep() into qemudShutdownVMDaemon().
Fixes regression introduced in commit 2211518, where all qemu 0.12.x
fails to start, as does qemu 0.13.x lacking the pci-assign device.
Prior to 2211518, the code was just ignoring a non-zero exit status
from the qemu child, but the virCommand code checked this to avoid
masking any other issues, which means the real bug of provoking
non-zero exit status has been latent for a longer time.
* src/qemu/qemu_capabilities.c (qemuCapsExtractVersionInfo): Check
for -device driver,? support.
(qemuCapsExtractDeviceStr): Avoid failure if all probed devices
are unsupported.
Reported by Ken Congyang.
https://bugzilla.redhat.com/show_bug.cgi?id=620363
When using -incoming stdio or -incoming exec:, qemu keeps the
stdin fd open long after the migration is complete. Not to
mention that exec:cat is horribly inefficient, by doubling the
I/O and going through a popen interface in qemu.
The new -incoming fd: of qemu 0.12.0 closes the fd after using
it, and allows us to bypass an intermediary cat process for
less I/O.
* src/qemu/qemu_command.h (qemuBuildCommandLine): Add parameter.
* src/qemu/qemu_command.c (qemuBuildCommandLine): Support
migration via fd: when possible. Consolidate migration handling
into one spot, now that it is more complex.
* src/qemu/qemu_driver.c (qemudStartVMDaemon): Update caller.
* tests/qemuxml2argvtest.c (mymain): Likewise.
* tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.args: New file.
* tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.xml: Likewise.
Display or set unlimited values for memory parameters. Unlimited is
represented by INT64_MAX in memory cgroup.
Signed-off-by: Nikunj A. Dadhania <nikunj@linux.vnet.ibm.com>
Reported-by: Justin Clift <jclift@redhat.com>
This is in response to a request in:
https://bugzilla.redhat.com/show_bug.cgi?id=665293
In short, under heavy load, it's possible for qemu's networking to
lock up due to the tap device's default 1MB sndbuf being
inadequate. adding "sndbuf=0" to the qemu commandline -netdevice
option will alleviate this problem (sndbuf=0 actually sets it to
0xffffffff).
Because we must be able to explicitly specify "0" as a value, the
standard practice of "0 means not specified" won't work here. Instead,
virDomainNetDef also has a sndbuf_specified, which defaults to 0, but
is set to 1 if some value was given.
The sndbuf value is put inside a <tune> element of each <interface> in
the domain. The intent is that further tunable settings will also be
placed inside this element.
<interface type='network'>
...
<tune>
<sndbuf>0</sndbuf>
...
</tune>
</interface>
This patch is in response to
https://bugzilla.redhat.com/show_bug.cgi?id=643050
The existing libvirt support for the vhost-net backend to the virtio
network driver happens automatically - if the vhost-net device is
available, it is always enabled, otherwise the standard userland
virtio backend is used.
This patch makes it possible to force whether or not vhost-net is used
with a bit of XML. Adding a <driver> element to the interface XML, eg:
<interface type="network">
<model type="virtio"/>
<driver name="vhost"/>
will force use of vhost-net (if it's not available, the domain will
fail to start). if driver name="qemu", vhost-net will not be used even
if it is available.
If there is no <driver name='xxx'/> in the config, libvirt will revert
to the pre-existing automatic behavior - use vhost-net if it's
available, and userland backend if vhost-net isn't available.
We try to use that command first when setting a VNC/SPICE password. If
that doesn't work we fallback to the legacy VNC only password
Allow an expiry time to be set, if that doesn't work, throw an error
if they try to use SPICE.
Change since v1:
- moved qemuInitGraphicsPasswords to qemu_hotplug, renamed
to qemuDomainChangeGraphicsPasswords.
- updated what looks like a typo (that appears to work anyway) in
initial patch from Daniel:
- ret = qemuInitGraphicsPasswords(driver, vm,
- VIR_DOMAIN_GRAPHICS_TYPE_SPICE,
- &vm->def->graphics[0]->data.vnc.auth,
- driver->vncPassword);
+ ret = qemuInitGraphicsPasswords(driver, vm,
+ VIR_DOMAIN_GRAPHICS_TYPE_SPICE,
+ &vm->def->graphics[0]->data.spice.auth,
+ driver->spicePassword);
Based on patch by Daniel P. Berrange <berrange@redhat.com>.
* src/qemu/qemu_capabilities.h (qemuCapsParseDeviceStr): New
prototype.
* src/qemu/qemu_capabilities.c (qemuCapsParsePCIDeviceStrs)
Rename and split...
(qemuCapsExtractDeviceStr, qemuCapsParseDeviceStr): ...to make it
easier to add and test device-specific checks.
(qemuCapsExtractVersionInfo): Update caller.
* tests/qemuhelptest.c (testHelpStrParsing): Also test parsing of
device-related flags.
(mymain): Update expected flags.
* tests/qemuhelpdata/qemu-0.12.1-device: New file.
* tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60-device: New file.
* tests/qemuhelpdata/qemu-kvm-0.12.3-device: New file.
* tests/qemuhelpdata/qemu-kvm-0.13.0-device: New file.
* src/qemu/qemu_capabilities.c (qemuCapsProbeMachineTypes)
(qemuCapsProbeCPUModels, qemuCapsParsePCIDeviceStrs)
(qemuCapsExtractVersionInfo): Use virCommand rather than virExec.
If the emulator doesn't support SDL graphic, we should reject
the use of SDL graphic xml with error messages, but not ignore
it silently, and pretend things are fine.
"-sdl" flag was exposed explicitly by qemu since 0.10.0, more detail:
http://www.redhat.com/archives/libvir-list/2011-January/msg00442.html
And we already have capability flag "QEMUD_CMD_FLAG_0_10", which
could be used to prevent the patch affecting the older versions
of QEMU.
* src/qemu/qemu_command.c
Skip IB700 when assigning PCI slots.
Note: the I6300ESB watchdog _is_ a PCI device.
To test this: I applied this patch to libvirt-0.8.3-2.fc14 (rebasing
it slightly: qemu_command.c didn't exist in that version) and
installed this on my machine, then tested that I could successfully
add an ib700 watchdog device to a guest, start the guest, and the
ib700 was available to the guest. I also added an i6300esb (PCI)
watchdog to another guest, and verified that libvirt assigned a PCI
device to it, that the guest could be started, and that i6300esb was
present in the guest.
Note that if you previously had a domain with a ib700 watchdog, it
would have had an <address type='pci' .../> clause added to it in the
libvirt configuration. This patch does not attempt to remove this.
You cannot start such a domain -- qemu gives an error if you try.
With this patch you are able to remove the bogus address element
without libvirt adding it back.
Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
The current security driver usage requires horrible code like
if (driver->securityDriver &&
driver->securityDriver->domainSetSecurityHostdevLabel &&
driver->securityDriver->domainSetSecurityHostdevLabel(driver->securityDriver,
vm, hostdev) < 0)
This pair of checks for NULL clutters up the code, making the driver
calls 2 lines longer than they really need to be. The goal of the
patchset is to change the calling convention to simply
if (virSecurityManagerSetHostdevLabel(driver->securityDriver,
vm, hostdev) < 0)
The first check for 'driver->securityDriver' being NULL is removed
by introducing a 'no op' security driver that will always be present
if no real driver is enabled. This guarentees driver->securityDriver
!= NULL.
The second check for 'driver->securityDriver->domainSetSecurityHostdevLabel'
being non-NULL is hidden in a new abstraction called virSecurityManager.
This separates the driver callbacks, from main internal API. The addition
of a virSecurityManager object, that is separate from the virSecurityDriver
struct also allows for security drivers to carry state / configuration
information directly. Thus the DAC/Stack drivers from src/qemu which
used to pull config from 'struct qemud_driver' can now be moved into
the 'src/security' directory and store their config directly.
* src/qemu/qemu_conf.h, src/qemu/qemu_driver.c: Update to
use new virSecurityManager APIs
* src/qemu/qemu_security_dac.c, src/qemu/qemu_security_dac.h
src/qemu/qemu_security_stacked.c, src/qemu/qemu_security_stacked.h:
Move into src/security directory
* src/security/security_stack.c, src/security/security_stack.h,
src/security/security_dac.c, src/security/security_dac.h: Generic
versions of previous QEMU specific drivers
* src/security/security_apparmor.c, src/security/security_apparmor.h,
src/security/security_driver.c, src/security/security_driver.h,
src/security/security_selinux.c, src/security/security_selinux.h:
Update to take virSecurityManagerPtr object as the first param
in all callbacks
* src/security/security_nop.c, src/security/security_nop.h: Stub
implementation of all security driver APIs.
* src/security/security_manager.h, src/security/security_manager.c:
New internal API for invoking security drivers
* src/libvirt.c: Add missing debug for security APIs
When dynamic_ownership=0, saved images must be owned by the same uid
as is used to run the qemu process, otherwise restore won't work. To
accomplish this, qemuSecurityDACRestoreSavedStateLabel() needs to
simply return when it's called.
This fix is in response to:
https://bugzilla.redhat.com/show_bug.cgi?id=661720
While doing some testing with Qemu and creating huge logfiles I encountered the case where the VM could not start anymore due to the lseek() to the end of the Qemu VM's log file failing. The patch below fixes the problem by replacing the previously used 'int' with 'off_t'.
To reproduce this error, you could do the following:
dd if=/dev/zero of=/var/log/libvirt/qemu/<name of VM>.log bs=1024 count=$((1024*2048))
and you should get an error like this:
error: Failed to start domain <name of VM>
error: Unable to seek to -2147482651 in /var/log/libvirt/qemu/<name of VM>.log: Success
This patch fixes https://bugzilla.redhat.com/show_bug.cgi?id=664406
If qemu is run as a different uid, it has been unable to access mode
0660 files that are owned by a different user, but with a group that
the qemu is a member of (aside from the one group listed in the passwd
file), because initgroups() is not being called prior to the
exec. initgroups will change the group membership of the process (and
its children) to match the new uid.
To make this happen, the setregid()/setreuid() code in
qemuSecurityDACSetProcessLabel has been replaced with a call to
virSetUIDGID(), which does both of those, plus calls initgroups.
Similar, but not identical, code in qemudOpenAsUID() has been replaced
with virSetUIDGID(). This not only consolidates the functionality to a
single location, but also potentially fixes some as-yet unreported
bugs.
Shorten qemuDomainSnapshotWriteSnapshotMetadata function name
and make it take a snapshot pointer instead of dealing with
the current snapshot. Update other functions accordingly.
Add a qemuDomainSnapshotReparentChildren hash iterator to
reparent the children of a snapshot that is being deleted. Use
qemuDomainSnapshotWriteMetadata to write updated metadata
to disk.
This fixes a problem where outdated parent information breaks
the snapshot tree and hinders the deletion of child snapshots.
Reported by Philipp Hahn.
