https://bugzilla.redhat.com/show_bug.cgi?id=971485
As of d7f9d82753 we copy the listen
address from the qemu.conf config file in case none has been provided
via XML. But later, when migrating, we should not include such listen
address in the migratable XML as it is something autogenerated, not
requested by user. Moreover, the binding to the listen address will
likely fail, unless the address is '0.0.0.0' or its IPv6 equivalent.
This patch introduces a new boolean attribute to virDomainGraphicsListenDef
to distinguish autofilled listen addresses. However, we must keep the
attribute over libvirtd restarts, so it must be kept within status XML.
This patch fixes changes done in commit 29c1e913e4
that was pushed without implementing review feedback.
The flag introduced by the patch is changed to VIR_DOMAIN_VCPU_GUEST and
documentation makes the difference between regular hotplug and this new
functionality more explicit.
The virsh options that enable the use of the new flag are changed to
"--guest" and the documentation is fixed too.
Currently, there's a path to use the ncpuinfo variable uninitialized,
which leads to a compiler warning:
qemu/qemu_driver.c: In function 'qemuDomainGetVcpusFlags':
qemu/qemu_driver.c:4573:9: error: 'ncpuinfo' may be used
uninitialized in this function [-Werror=maybe-uninitialized]
for (i = 0; i < ncpuinfo; i++) {
^
This patch implements support for the "cpu-add" QMP command that plugs
CPUs into a live guest. The "cpu-add" command was introduced in QEMU
1.5. For the hotplug to work machine type "pc-i440fx-1.5" is required.
The qemu guest agent allows to online and offline CPUs from the
perspective of the guest. This patch adds helpers that call
'guest-get-vcpus' and 'guest-set-vcpus' guest agent functions and
convert the data for internal libvirt usage.
https://bugzilla.redhat.com/show_bug.cgi?id=964177
Though both libvirt and QEMU's document say RTC_CHANGE returns
the offset from the host UTC, qemu actually returns the offset
from the specified date instead when specific date is provided
(-rtc base=$date).
It's not safe for qemu to fix it in code, it worked like that
for 3 years, changing it now may break other QEMU use cases.
What qemu tries to do is to fix the document:
http://lists.gnu.org/archive/html/qemu-devel/2013-05/msg04782.html
And in libvirt side, instead of replying on the value from qemu,
this converts the offset returned from qemu to the offset from
host UTC, by:
/*
* a: the offset from qemu RTC_CHANGE event
* b: The specified date (-rtc base=$date)
* c: the host date when libvirt gets the RTC_CHANGE event
* offset: What libvirt will report
*/
offset = a + (b - c);
The specified date (-rtc base=$date) is recorded in clock's def as
an internal only member (may be useful to exposed outside?).
Internal only XML tag "basetime" is introduced to not lose the
guest's basetime after libvirt restarting/reloading:
<clock offset='variable' adjustment='304' basis='utc' basetime='1370423588'/>
Currently, a listen address for a SPICE server can be specified. Later,
when the domain is migrated, we need to relocate the graphics which
involves telling new destination to the SPICE server. However, we can't
just assume the listen address is the new location, because the listen
address can be ANYCAST (0.0.0.0 for IPv4, :: for IPv6). In which case,
we want to pass the remote hostname. But there are some troubles with
ANYCAST. In both IPv4 and IPv6 it has many ways for specifying such
address. For instance, in IPv4: 0, 0.0, 0.0.0, 0.0.0.0. The number of
variations gets bigger in IPv6 world. Hence, in order to check for
ANYCAST address sanely, we should take the provided listen address,
parse it and format back in it's full form. Which is exactly what this
patch does.
The work was done at the time of snapshot xmlstring parsing
if (offline && def->memory &&
def->memory != VIR_DOMAIN_SNAPSHOT_LOCATION_NONE) {
virReportError(...);
}
This should resolve:
https://bugzilla.redhat.com/show_bug.cgi?id=959191
The problem was that qemuUpdateActivePciHostdevs was returning 0
(success) when no hostdevs were present, but would otherwise return -1
(failure) even when it completed successfully. It is only called from
qemuProcessReconnect(), and when qemuProcessReconnect got back an
error, it would not only stop reconnecting, but would terminate the
guest qemu process "to remove danger of it ending up running twice if
user tries to start it again later".
(This bug was introduced in commit 011cf7ad, which was pushed between
v1.0.2 and v1.0.3, so all maintenance branches from v1.0.3 up to 1.0.5
will need this one line patch applied.)
A literal IPv6 must be escaped, otherwise migration fails with:
unable to execute QEMU command 'drive-mirror': address resolution failed
for f0::0d:5901: Servname not supported for ai_socktype
since QEMU treats everything after the first ':' as the port.
If snapshot creation failed for example due to invalid use of the
"REUSE_EXTERNAL" flag, libvirt killed access to the original image file
instead of the new image file. On machines with selinux this kills the
whole VM as the selinux context is enforced immediately.
* qemu_driver.c:qemuDomainSnapshotUndoSingleDiskActive():
- Kill access to the new image file instead of the old one.
Partially resolves: https://bugzilla.redhat.com/show_bug.cgi?id=906639
A bug in Cygwin [1] and poor error messages from gcc [2] lead
to this confusing compilation error:
qemu/qemu_monitor.c:418:9: error: passing argument 2 of 'sendmsg' from incmpatible pointer type
/usr/include/sys/socket.h:42:11: note: expected 'const struct msghdr *' but argument is of type 'struct msghdr *'
[1] http://cygwin.com/ml/cygwin/2013-05/msg00451.html
[2] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57475
* src/qemu/qemu_monitor.c (includes): Include <sys/socket.h>
before <sys/un.h>.
Signed-off-by: Eric Blake <eblake@redhat.com>
This is a recurring problem for cygwin :)
For example, see commit 23a4df88.
qemu/qemu_driver.c: In function 'qemuStateInitialize':
qemu/qemu_driver.c:691:13: error: format '%d' expects type 'int', but argument 8 has type 'uid_t' [-Wformat]
* src/qemu/qemu_driver.c (qemuStateInitialize): Add casts.
* daemon/remote.c (remoteDispatchAuthList): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
A cygwin build of the qemu driver fails with:
qemu/qemu_process.c: In function 'qemuPrepareCpumap':
qemu/qemu_process.c:1803:31: error: 'CPU_SETSIZE' undeclared (first use in this function)
CPU_SETSIZE is a Linux extension in <sched.h>; a bit more portable
is using sysconf if _SC_NPROCESSORS_CONF is defined (several platforms
have it, including Cygwin). Ultimately, I would have preferred to
use gnulib's 'nproc' module, but it is currently under an incompatible
license.
* src/qemu/qemu_conf.h (QEMUD_CPUMASK_LEN): Provide definition on
cygwin.
Signed-off-by: Eric Blake <eblake@redhat.com>
Currently, if there's an error opening /dev/vhost-net (e.g. because
it doesn't exist) but it's not required we proceed with vhostfd array
filled with -1 and vhostfdSize unchanged. Later, when constructing
the qemu command line only non-negative items within vhostfd array
are taken into account. This means, vhostfdSize may be greater than
the actual count of non-negative items in vhostfd array. This results
in improper command line arguments being generated, e.g.:
-netdev tap,fd=21,id=hostnet0,vhost=on,vhostfd=(null)
If we are just ejecting media, ret == -1 even after the retry loop
determines that the tray is open, as requested. This means media
disconnect always report's error.
Fix it, and fix some other mini issues:
- Don't overwrite the 'eject' error message if the retry loop fails
- Move the retries decrement inside the loop, otherwise the final loop
might succeed, yet retries == 0 and we will raise error
- Setting ret = -1 in the disk->src check is unneeded
- Fix comment typos
cc: mprivozn@redhat.com
Currently qemuDomainReboot() does reboot in two phases:
qemuMonitorSystemPowerdown() and qemuProcessFakeReboot().
qemuMonitorSystemPowerdown() shutdowns the domain and saves domain
state/reason as VIR_DOMAIN_SHUTDOWN_UNKNOWN.
qemuProcessFakeReboot() sets domain state/reason to
VIR_DOMAIN_RESUMED_UNPAUSED but does not save domain state changes.
Subsequent restart of libvirtd leads to restoring domain state/reason to
saved that is VIR_DOMAIN_SHUTDOWN_UNKNOWN and to automatic shutdown of
the domain. This commit adds virDomainSaveStatus() into
qemuProcessFakeReboot() to avoid unexpected shutdowns.
With previous patch, we accept negative value as length of string to
duplicate. So there is no need to pass strlen(src) in case we want to do
duplicate the whole string.
Function qemuDomainSetBlockIoTune() was checking QEMU capabilities
even when !(flags & VIR_DOMAIN_AFFECT_LIVE) and the domain was
shutoff, resulting in the following problem:
virsh # domstate asdf; blkdeviotune asdf vda --write-bytes-sec 100
shut off
error: Unable to change block I/O throttle
error: unsupported configuration: block I/O throttling not supported with this QEMU binary
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=965016
Since f03dcc5 we use [::] as the listening address both on qemu
command line in -incoming and in nbd-server-start QMP command.
However the latter requires just :: without the braces.
In order to learn libvirt multiqueue several things must be done:
1) The '/dev/net/tun' device needs to be opened multiple times with
IFF_MULTI_QUEUE flag passed to ioctl(fd, TUNSETIFF, &ifr);
2) Similarly, '/dev/vhost-net' must be opened as many times as in 1)
in order to keep 1:1 ratio recommended by qemu and kernel folks.
3) The command line construction code needs to switch from 'fd=X' to
'fds=X:Y:...:Z' and from 'vhostfd=X' to 'vhostfds=X:Y:...:Z'.
4) The monitor handling code needs to learn to pass multiple FDs.
In 84c59ffa I've tried to fix changing ejectable media process. The
process should go like this:
1) we need to call 'eject' on the monitor
2) we should wait for 'DEVICE_TRAY_MOVED' event
3) now we can issue 'change' command
However, while waiting in step 2) the domain monitor was locked. So
even if qemu reported the desired event, the proper callback was not
called immediately. The monitor handling code needs to lock the
monitor in order to read the event. So that's the first lock we must
not hold while waiting. The second one is the domain lock. When
monitor handling code reads an event, the appropriate callback is
called then. The first thing that each callback does is locking the
corresponding domain as a domain or its device is about to change
state. So we need to unlock both monitor and VM lock. Well, holding
any lock while sleep()-ing is not the best thing to do anyway.
Since 0d70656afd, it starts to access the sysfs files to build
the qemu command line (by virSCSIDeviceGetSgName, which is to find
out the scsi generic device name by adpater🚌target:unit), there
is no way to work around, qemu wants to see the scsi generic device
like "/dev/sg6" anyway.
And there might be other places which need to access sysfs files
when building qemu command line in future.
Instead of increasing the arguments of qemuBuildCommandLine, this
introduces a new callback for qemuBuildCommandLine, and thus tests
can register their own callbacks for sysfs test input files accessing.
* src/qemu/qemu_command.h: (New callback struct
qemuBuildCommandLineCallbacks;
extern buildCommandLineCallbacks)
* src/qemu/qemu_command.c: (wire up the callback struct)
* src/qemu/qemu_driver.c: (Use the new syntax of qemuBuildCommandLine)
* src/qemu/qemu_hotplug.c: Likewise
* src/qemu/qemu_process.c: Likewise
* tests/testutilsqemu.[ch]: (Helper testSCSIDeviceGetSgName;
callback struct testCallbacks;)
* tests/qemuxml2argvtest.c: (Use testCallbacks)
* src/tests/qemuxmlnstest.c: (Like above)
Resolves:https://bugzilla.redhat.com/show_bug.cgi?id=927620
#kill -STOP `pidof qemu-kvm`
#virsh destroy $guest --graceful
error: Failed to destroy domain testVM
error: An error occurred, but the cause is unknown
With --graceful, SIGTERM always is emitted to kill driver
process, but it won't success till burning out waiting time
in case of process being stopped.
But domain destroy without --graceful can work, SIGKILL will
be emitted to the stopped process after 10 secs which always
kills a process even one that is currently stopped.
So report an error after burning out waiting time in this case.
Change bbe97ae968 caused the
QEMU driver to ignore ENOENT errors from cgroups, in order
to cope with missing /proc/cgroups. This is not good though
because many other things can cause ENOENT and should not
be ignored. The callers expect to see ENXIO when cgroups
are not present, so adjust the code to report that errno
when /proc/cgroups is missing
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Commit 632f78c introduced a regression which causes schedinfo being
unable to set some parameters. When migrating to priv->cgroup there
was missing variable left out and due to passed NULL to underlying
function, the setting failed.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=963592
This adds the shared device entry when starting domain (more
exactly, when preparing host devices), and remove the entry
when destroying domain (when reattaching host devices).
This changes the helpers qemu{Add,Remove}SharedDisk into
qemu{Add,Remove}SharedDevice, as most of the code in the helpers
can be reused for scsi host device.
To track the shared scsi host device, first it finds out the
device path (e.g. /dev/s[dr]*) which is mapped to the sg device,
and use device ID of the found device path (/dev/s[dr]*) as the
hash key. This is because of the device ID is not unique between
between /dev/s[dr]* and /dev/sg*, e.g.
% sg_map
/dev/sg0 /dev/sda
/dev/sg1 /dev/sr0
% ls -l /dev/sda
brw-rw----. 1 root disk 8, 0 May 2 19:26 /dev/sda
%ls -l /dev/sg0
crw-rw----. 1 root disk 21, 0 May 2 19:26 /dev/sg0
"Shared disk" is not only the thing we should care about after "scsi
hostdev" is introduced. A same scsi device can be used as "disk" for
one domain, and as "scsi hostdev" for another domain at the same time.
That's why this patch renames qemu_driver->sharedDisks. Related functions
and structs are also renamed.
Commit 7f15ebc7a2 introduced a bug
happening when guests without a <graphics> element are migrated.
The initialization of listenAddress happens unconditionally
from the cookie even if the cookie->graphics pointer was NULL.
Moved the initialization to where it is safe.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
QEMU introduced "discard" option for drive since commit a9384aff53,
<...>
@var{discard} is one of "ignore" (or "off") or "unmap" (or "on") and
controls whether @dfn{discard} (also known as @dfn{trim} or @dfn{unmap})
requests are ignored or passed to the filesystem. Some machine types
may not support discard requests.
</...>
This patch exposes the support in libvirt.
QEMU supported "discard" for "-drive" since v1.5.0-rc0:
% git tag --contains a9384aff53
contains
v1.5.0-rc0
v1.5.0-rc1
So this only detects the capability bit using virQEMUCapsProbeQMPCommandLine.
During building of the qemu command line determine whether to add/use the
'-no-reboot' option only if each of the 'on' events want to to destroy
the domain; otherwise, use the '-no-shutdown' option.
Prior to this change both could be on the command line, which while allowed
could be construed as a conflict.
Adding a VNC WebSocket support for QEMU driver. This functionality is
in upstream qemu from commit described as v1.3.0-982-g7536ee4, so the
capability is being recognized based on QEMU version for now.
QEMU introduced command line "-mem-merge=on|off" (defaults to on) to
enable/disable the memory merge (KSM) at guest startup. This exposes
it by new XML:
<memoryBacking>
<nosharepages/>
</memoryBacking>
The XML tag is same with what we used internally for old RHEL.
* src/qemu/qemu_capabilities.h: New capability bit.
* src/qemu/qemu_capabilities.c (virQEMUCapsProbeQMPCommandLine): New
function, based on qemuMonitorGetCommandLineOptionParameters, which was
introduced by commit bd56d0d813; use it to set new capability bit.
(virQEMUCapsInitQMP): Use new function.
The QEMU command line syntax for RBD disks is
file=rbd:pool/image:opt1=val1:opt2=val2...
There is no way to escape the ':' if it appears in the
pool or image name. Thus it must be explicitly forbidden
if it occurs in the libvirt XML. People are known to
be abusing the lack of escaping in current libvirt to
pass arbitrary args to QEMU.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Commit bd56d0d8 could lead to freeing an uninitialized pointer:
qemu/qemu_monitor_json.c: In function 'qemuMonitorJSONGetCommandLineOptionParameters':
qemu/qemu_monitor_json.c:4284: warning: 'cmd' may be used uninitialized in this function
* src/qemu/qemu_monitor_json.c
(qemuMonitorJSONGetCommandLineOptionParameters): Initialize variable.
Signed-off-by: Eric Blake <eblake@redhat.com>
Ever since the conversion to using only QMP for probing features
of qemu 1.2 and newer, we have been unable to detect features
that are added only by additional command line options. For
example, we'd like to know if '-machine mem-merge=on' (added
in qemu 1.5) is present. To do this, we will take advantage
of qemu 1.5's query-command-line-parameters QMP call [1].
This patch wires up the framework for probing the command results;
if the QMP command is missing, or if a particular command line
option does not output any parameters (for example, -net uses
a polymorphic parser, which showed up as no parameters as of qemu
1.5), we silently treat that command as having no results.
[1] https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg05180.html
* src/qemu/qemu_monitor.h (qemuMonitorGetOptions)
(qemuMonitorSetOptions)
(qemuMonitorGetCommandLineOptionParameters): New functions.
* src/qemu/qemu_monitor_json.h
(qemuMonitorJSONGetCommandLineOptionParameters): Likewise.
* src/qemu/qemu_monitor.c (_qemuMonitor): Add cache field.
(qemuMonitorDispose): Clean it.
(qemuMonitorGetCommandLineOptionParameters): Implement new function.
* src/qemu/qemu_monitor_json.c
(qemuMonitorJSONGetCommandLineOptionParameters): Likewise.
(testQemuMonitorJSONGetCommandLineParameters): Test it.
Signed-off-by: Eric Blake <eblake@redhat.com>
No need to open code a string list cleanup, if we are nice
to the caller by guaranteeing a NULL-terminated result.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONGetCPUDefinitions)
(qemuMonitorJSONGetCommands, qemuMonitorJSONGetEvents)
(qemuMonitorJSONGetObjectTypes, qemuMonitorJSONGetObjectProps):
Use simpler cleanup.
Signed-off-by: Eric Blake <eblake@redhat.com>
This adds both attachment and detachment support for scsi host
device.
Signed-off-by: Han Cheng <hanc.fnst@cn.fujitsu.com>
Signed-off-by: Osier Yang <jyang@redhat>
Found that I was unable to start existing domains after updating
to a kernel with no cgroups support
# zgrep CGROUP /proc/config.gz
# CONFIG_CGROUPS is not set
# virsh start test
error: Failed to start domain test
error: Unable to initialize /machine cgroup: Cannot allocate memory
virCgroupPartitionNeedsEscaping() correctly returns errno (ENOENT) when
attempting to open /proc/cgroups on such a system, but it was being
dropped in virCgroupSetPartitionSuffix().
Change virCgroupSetPartitionSuffix() to propagate errors returned by
its callees. Also check for ENOENT in qemuInitCgroup() when determining
if cgroups support is available.
It's better to put the usb related codes into qemuDomainAttachHostUsbDevice
instead of qemuDomainAttachHostDevice.
And in the old qemuDomainAttachHostDevice, just stealing the "usb" from
driver->activeUsbHostdevs leaks the memory.
Although virtio-scsi supports SCSI PR (Persistent Reservations),
the device on host may do not support it. To avoid losing data,
Just like PCI and USB pass through devices, only one live guest
is allowed per SCSI host pass through device."
Signed-off-by: Han Cheng <hanc.fnst@cn.fujitsu.com>
The <filesystem> element can now accept a <driver type='nbd'/>
as an alternative to 'loop'. The benefit of NBD is support
for non-raw disk image formats.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Extend the <driver> element in filesystem devices to
allow a storage format to be set. The new attribute
uses 'format' to reflect the storage format. This is
different from the <driver> element in disk devices
which use 'type' to reflect the storage format. This
is because the 'type' attribute on filesystem devices
is already used for the driver backend, for which the
disk devices use the 'name' attribute. Arggggh.
Anyway for disks we have
<driver name="qemu" type="raw"/>
And for filesystems this change means we now have
<driver type="loop" format="raw"/>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This adds the scsi-generic device into the device controller's
whitelist, so that it's allowed to used by the qemu process.
Signed-off-by: Han Cheng <hanc.fnst@cn.fujitsu.com>
Signed-off-by: Osier Yang <jyang@redhat.com>
Except the scsi host device's controller is "lsilogic", mapping
between the libvirt attributes and scsi-generic properties is:
libvirt qemu
-----------------------------------------
controller bus ($libvirt_controller.0)
bus channel
target scsi-id
unit lun
For scsi host device with "lsilogic" controller, the mapping is:
('target (libvirt)' must be 0, as it's not used; 'unit (libvirt)
must <= 7).
libvirt qemu
----------------------------------------------------------
controller && bus bus ($libvirt_controller.$libvirt_bus)
unit scsi-id
It's not good to hardcode/hard-check limits of these attributes,
and even worse, these limits are not documented, one has to find
out by either testing or reading the qemu code, I'm looking forward
to qemu expose limits like these one day). For example, exposing
"max_target", "max_lun" for megasas:
static const struct SCSIBusInfo megasas_scsi_info = {
.tcq = true,
.max_target = MFI_MAX_LD,
.max_lun = 255,
.transfer_data = megasas_xfer_complete,
.get_sg_list = megasas_get_sg_list,
.complete = megasas_command_complete,
.cancel = megasas_command_cancel,
};
Example of the qemu command line (lsilogic controller):
-drive file=/dev/sg2,if=none,id=drive-hostdev-scsi_host7-0-0-0 \
-device scsi-generic,bus=scsi0.0,scsi-id=8,\
drive=drive-hostdev-scsi_host7-0-0-0,id=hostdev-scsi_host7-0-0-0
Example of the qemu command line (virtio-scsi controller):
-drive file=/dev/sg2,if=none,id=drive-hostdev-scsi_host7-0-0-0 \
-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=128,lun=128,\
drive=drive-hostdev-scsi_host7-0-0-0,id=hostdev-scsi_host7-0-0-0
Signed-off-by: Han Cheng <hanc.fnst@cn.fujitsu.com>
Signed-off-by: Osier Yang <jyang@redhat.com>
Adding two cap flags for scsi-generic:
QEMU_CAPS_SCSI_GENERIC
QEMU_CAPS_SCSI_GENERIC_BOOTINDEX
Signed-off-by: Han Cheng <hanc.fnst@cn.fujitsu.com>
Signed-off-by: Osier Yang <jyang@redhat.com>
It is possible to build a kernel without swap cgroup controls
present. This causes a fatal error when querying memory
parameters. Treat missing swap controls as meaning "unlimited".
The fatal error remains if the user tries to actually change
the limit.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=851411https://bugzilla.redhat.com/show_bug.cgi?id=955500
The first problem was that virFileOpenAs was returning fd (-1) in one
of the error cases rather than ret (-errno), so the caller thought
that the error was EPERM rather than ENOENT.
The second problem was that some log messages in the general purpose
qemuOpenFile() function would always say "Failed to create" even if
the caller hadn't included O_CREAT (i.e. they were trying to open an
existing file).
This fixes virFileOpenAs to jump down to the error return (which
returns ret instead of fd) in the previously mentioned incorrect
failure case of virFileOpenAs(), removes all error logging from
virFileOpenAs() (since the callers report it), and modifies
qemuOpenFile to appropriately use "open" or "create" in its log
messages.
NB: I seriously considered removing logging from all callers of
virFileOpenAs(), but there is at least one case where the caller
doesn't want virFileOpenAs() to log any errors, because it's just
going to try again (qemuOpenFile()). We can't simply make a silent
variation of virFileOpenAs() though, because qemuOpenFile() can't make
the decision about whether or not it wants to retry until after
virFileOpenAs() has already returned an error code.
Likewise, I also considered changing virFileOpenAs() to return -1 with
errno set on return, and may still do that, but only as a separate
patch, as it obscures the intent of this patch too much.
The LXC, QEMU, and LibXL drivers have all merged their handling of
the attach/update/modify device APIs into one large
'xxxxDomainModifyDeviceFlags'
which then does a 'switch()' based on the actual API being invoked.
While this saves some lines of code, it is not really all that
significant in the context of the driver API impls as a whole.
This merger of the handling of different APIs creates pain when
wanting to automated analysis of the code and do things which
are specific to individual APIs. The slight duplication of code
from unmerged the API impls, is preferrable to allow for easier
automated analysis.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently the parsing of XML is pushed down into the various
migration helper APIs. This makes it difficult to insert the
correct access control checks, since one helper API services
many public APIs. Pull the parsing of XML up to the top level
of the QEMU driver APIs
Several APIs allow for custom XML to be passed in. This is
checked for ABI stability, which will ensure the UUID is
not being changed. There isn't validation that the name
did not change though. This could allow renaming of guests
via the backdoor, which in turn could allow for bypassing
access control restrictions based on names.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The individual hypervisor drivers were directly referencing
APIs in virnodesuspend.c in their virDriverPtr struct. Separate
these methods, so there is always a wrapper in the hypervisor
driver. This allows the unused virConnectPtr args to be removed
from the virnodesuspend.c file. Again this will ensure that
ACL checks will only be performed on invocations that are
directly associated with public API usage.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The individual hypervisor drivers were directly referencing
APIs in src/nodeinfo.c in their virDriverPtr struct. Separate
these methods, so there is always a wrapper in the hypervisor
driver. This allows the unused virConnectPtr args to be
removed from the nodeinfo.c file. Again this will ensure that
ACL checks will only be performed on invocations that are
directly associated with public API usage.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently the virGetHostname() API has a bogus virConnectPtr
parameter. This is because virtualization drivers directly
reference this API in their virDriverPtr tables, tieing its
API design to the public virConnectGetHostname API design.
This also causes problems for access control checks since
these must only be done for invocations from the public
API, not internal invocation.
Remove the bogus virConnectPtr parameter, and make each
hypervisor driver provide a dedicated function for the
driver API impl. This will allow access control checks
to be easily inserted later.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
VFIO device assignment requires a cgroup ACL to be setup for access to
the /dev/vfio/nn "group" device for any devices that will be assigned
to a guest. In the case of a host device that is allocated from a
pool, it was being allocated during qemuBuildCommandLine(), which is
called by qemuProcessStart() *after* the all-encompassing
qemuSetupCgroup() was called, meaning that the standard Cgroup ACL
setup wasn't creating ACLs for these devices allocated from pools.
One possible solution was to manually add a single ACL down inside
qemuBuildCommandLine() when networkAllocateActualDevice() is called,
but that has two problems: 1) the function that adds the cgroup ACL
requires a virDomainObjPtr, which isn't available in
qemuBuildCommandLine(), and 2) we really shouldn't be doing network
device setup inside qemuBuildCommandLine() anyway.
Instead, I've created a new function called
qemuNetworkPrepareDevices() which is called just before
qemuPrepareHostDevices() during qemuProcessStart() (explanation of
ordering in the comments), i.e. well before the call to
qemuSetupCgroup(). To minimize code churn in a patch that will be
backported to 1.0.5-maint, qemuNetworkPrepareDevices only does
networkAllocateActualDevice() and the bare amount of setup required
for type='hostdev network devices, but it eventually should do *all*
device setup for guest network devices.
Note that some of the code that was previously needed in
qemuBuildCommandLine() is no longer required when
networkAllocateActualDevice() is called earlier:
* qemuAssignDeviceHostdevAlias() is already done further down in
qemuProcessStart().
* qemuPrepareHostdevPCIDevices() is called by
qemuPrepareHostDevices() which is called after
qemuNetworkPrepareDevices() in qemuProcessStart().
As hinted above, this new function should be moved into a separate
qemu_network.c (or similarly named) file along with
qemuPhysIfaceConnect(), qemuNetworkIfaceConnect(), and
qemuOpenVhostNet(), and expanded to call those functions as well, then
the nnets loop in qemuBuildCommandLine() should be reduced to only
build the commandline string (which itself can be in a separate
qemuInterfaceBuilldCommandLine() function as suggested by
Michal). However, this will require storing away an array of tapfd and
vhostfd that are needed for the commandline, so I would rather do that
in a separate patch and leave this patch at the minimum to fix the
bug.
On architectures not supporting the Intel specific programmable interval
timer, like e.g. S390, starting a domain with a clock definition containing
a pit timer results in the error "Option no-kvm-pit-reinjection not supported
for this target".
By moving the capability enablement for -no-kvm-pit-reinjection from the
InitQMPBasic section into the x86_64 and i686 only enablement section all
other architectures are no longer automatically enabled. In addition
architecture related capabilities enablements have refactored into a new
architecture bound capabilities initialization function.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
When attempting to generate the native command line from an XML file
that uses graphics port auto allocation, the generated commandline
wouldn't be valid.
This patch adds fake autoallocation of ports as done when starting the
actual machine.
I must have looked at this a couple dozen times before I noticed it
had "!=" instead of "==". Not doing this setup prevented qemu from
doing anything with the vfio group device.
The QEMU migration code unconditionally sets the 'persistent'
cookie flag on the source host. The dest host, however, only
allows it during parsing if VIR_MIGRATE_PERSIST_DEST was
set. Make the source host only set it if this flag is
present.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
POSIX says pthread_t is opaque. We can't guarantee if it is scaler
or a pointer, nor what size it is; and BSD differs from Linux.
We've also had reports of gcc complaining on attempts to cast it,
if we use a cast to the wrong type (for example, pointers have to be
cast to void* or intptr_t before being narrowed; while casting a
function return of scalar pthread_t to void* triggers a different
warning).
Give up on casts, and use unions to get at decent bits instead. And
rather than futz around with figuring which 32 bits of a potentially
64-bit pointer are most likely to be unique, convert the rest of
the code base to use 64-bit values when using a debug id.
Based on a report by Guido Günther against kFreeBSD, but with a
fix that doesn't regress commit 4d970fd29 for FreeBSD.
* src/util/virthreadpthread.c (virThreadSelfID, virThreadID): Use
union to get at a decent bit representation of thread_t bits.
* src/util/virthread.h (virThreadSelfID, virThreadID): Alter
signature.
* src/util/virthreadwin32.c (virThreadSelfID, virThreadID):
Likewise.
* src/qemu/qemu_domain.h (qemuDomainJobObj): Alter type of owner.
* src/qemu/qemu_domain.c (qemuDomainObjTransferJob)
(qemuDomainObjSetJobPhase, qemuDomainObjReleaseAsyncJob)
(qemuDomainObjBeginNestedJob, qemuDomainObjBeginJobInternal): Fix
clients.
* src/util/virlog.c (virLogFormatString): Likewise.
* src/util/vireventpoll.c (virEventPollInterruptLocked):
Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
The source code base needs to be adapted as well. Some files
include virutil.h just for the string related functions (here,
the include is substituted to match the new file), some include
virutil.h without any need (here, the include is removed), and
some require both.
It's not desired to force users imagine path for a socket they
are not even supposed to connect to. On the other hand, we
already have a release where the qemu agent socket path is
exposed to XML, so we cannot silently drop it from there.
The new path is generated in form:
$LOCALSTATEDIR/lib/libvirt/qemu/channel/target/$domain.$name
for qemu system mode, and
$XDG_CONFIG_HOME/qemu/lib/channel/target/$domain.$name
for qemu session mode.
virPCIDeviceReattach and virPCIDeviceUnbindFromStub (called by
virPCIDeviceReattach) had previously required the name of the stub
driver as input. This is unnecessary, because the name of the driver
the device is currently bound to can be found by looking at the link:
/sys/bus/pci/dddd:bb:ss.ff/driver
Instead of requiring that the name of the expected stub driver name
and only unbinding if that one name is matched, we no longer take a
driver name in the arglist for either of these
functions. virPCIDeviceUnbindFromStub just compares the name of the
currently bound driver to a list of "well known" stubs (right now
contains "pci-stub" and "vfio-pci" for qemu, and "pciback" for xen),
and only performs the unbind if it's one of those devices.
This allows virsh nodedevice-reattach to work properly across a
libvirtd restart, and fixes a couple of cases where we were
erroneously still hard-coding "pci-stub" as the drive name.
For some unknown reason, virPCIDeviceReattach had been calling
modprobe on the stub driver prior to unbinding the device. This was
problematic because we no longer know the name of the stub driver in
that function. However, it is pointless to probe for the stub driver
at that time anyway - because the device is bound to the stub driver,
we are guaranteed that it is already loaded, and so that call to
modprobe has been removed.
For s390 we don't want to have a default USB device generated even
if QEMU is silently tolerating -usb on the command line. This may change
in the future.
Another reason to avoid the USB controller is that it implies a PCI
bus which might cause a regression at some later point in time.
The following change will set the USB controller model to 'none'
unless a model or address has been specified, which can be the case
if a legacy definition is loaded or the XML writer knows what
she/he's doing.
Requiring the user to explicitly disable USB on systems not supporting
it seems cumbersome.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Commit eca3fdf inadvertantly caused a failure to start for any domain
with the following in its config:
<graphics type='spice' autoport='yes'/>
The problem is that when tlsPort == 0 and defaultMode == "any" (which
is the default for defaultMode), this would be flagged in the code as
"needTLSPort", and if there was then no spice tls config, the new
error+fail would happen.
This patch checks for the case of defaultMode == "any", and in that
case simply doesn't allocate a TLS port (since that's probably not
what the user wanted, and it would have failed later anyway.). It does
leave the error in place for cases when the user specifically asked to
use tls in one way or another, though.
As a result of commit id '19c345f2', 'make -C tests valgrind' has the
following for qemuxml2argvtest:
==22482== 197 (80 direct, 117 indirect) bytes in 1 blocks are definitely lost in loss record 101 of 120
==22482== at 0x4A06B6F: calloc (vg_replace_malloc.c:593)
==22482== by 0x4C6F301: virAlloc (viralloc.c:124)
==22482== by 0x4C840FC: virSaveLastError (virerror.c:308)
==22482== by 0x431882: qemuBuildCommandLine (qemu_command.c:8204)
==22482== by 0x41E8F0: testCompareXMLToArgvHelper (qemuxml2argvtest.c:155)
==22482== by 0x41FE9F: virtTestRun (testutils.c:157)
==22482== by 0x419DEB: mymain (qemuxml2argvtest.c:654)
==22482== by 0x4204DA: virtTestMain (testutils.c:719)
==22482== by 0x39D0821A04: (below main) (libc-start.c:225)
==22482==
qemuBuildMemballoonDevStr returns NULL if memballoon doesn't have
the right address type, but it doesn't report an error, leading to:
error: An error occurred, but the cause is unknown
Report a helpful error message instead, e.g.:
error: XML error: memballoon unsupported with address type 'usb'
When a user requests auto-allocation of the spice TLS port but spice TLS
is disabled in qemu.conf, we start the machine and let qemu fail instead
of erroring out sooner.
Add an error message so that this doesn't happen.
The USB-specific cgroup setup had been inserted inline in
qemuDomainAttachHostUsbDevice and qemuSetupCgroup, but now there is a
common cgroup setup function called for all hostdevs, so it makes sens
to put the usb-specific setup there and just rely on that function
being called.
The one thing I'm uncertain of here (and a reason for not pushing
until after release) is that previously hostdev->missing was checked
only when starting a domain (and cgroup setup for the device skipped
if missing was true), but with this consolidation, it is now checked
in the case of hotplug as well. I don't know if this will have any
practical effect (does it make sense to hotplug a "missing" usb
device?)
PCIO device assignment using VFIO requires read/write access by the
qemu process to /dev/vfio/vfio, and /dev/vfio/nn, where "nn" is the
VFIO group number that the assigned device belongs to (and can be
found with the function virPCIDeviceGetVFIOGroupDev)
/dev/vfio/vfio can be accessible to any guest without danger
(according to vfio developers), so it is added to the static ACL.
The group device must be dynamically added to the cgroup ACL for each
vfio hostdev in two places:
1) for any devices in the persistent config when the domain is started
(done during qemuSetupCgroup())
2) at device attach time for any hotplug devices (done in
qemuDomainAttachHostDevice)
The group device must be removed from the ACL when a device it
"hot-unplugged" (in qemuDomainDetachHostDevice())
Note that USB devices are already doing their own cgroup setup and
teardown in the hostdev-usb specific function. I chose to make the new
functions generic and call them in a common location though. We can
then move the USB-specific code (which is duplicated in two locations)
to this single location. I'll be posting a followup patch to do that.
Don't reserve slot 2 for video if the machine has no PCI buses.
Error out when the user specifies a video device without
a PCI address when there are no PCI buses.
(This wouldn't work on a machine with no PCI bus anyway since
we do add PCI addresses for video devices to the command line)
In the past we automatically added a USB controller and assigned
it a PCI address (0:0:1.2) even on machines without a PCI bus.
This didn't break machines with no PCI bus because the command
line for it is just '-usb', with no mention of the PCI bus.
The implicit IDE controller (reserved address 0:0:1.1) has
no command line at all.
Commit b33eb0dc removed the ability to reserve PCI addresses
on machines without a PCI bus. This made them stop working,
since there would always be the implicit USB controller.
Skip the reservation of addresses for these controllers when
there is no PCI bus, instead of failing.
This isn't strictly speaking a bugfix, but I realized I'd gotten a bit
too verbose when I chose the names for
VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_*. This shortens them all a bit.
<source type='bridge'> uses a helper application to do the necessary
TUN/TAP setup to use an existing network bridge, thus letting
unprivileged users use TUN/TAP interfaces.
However, libvirt should be preventing QEMU from running any setuid
programs at all, which would include this helper program. From
a security POV, any setuid helper needs to be run by libvirtd itself,
not QEMU.
This is what this patch does. libvirt now invokes the setuid helper,
gets the TAP fd and then passes it to QEMU in the normal manner.
The path to the helper is specified in qemu.conf.
As a small advantage, this adds a <target dev='tap0'/> element to the
XML of an active domain using <interface type='bridge'>.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
VFIO requires all of the guest's memory and IO space to be lockable in
RAM. The domain's max_balloon is the maximum amount of memory the
domain can have (in KiB). We add a generous 1GiB to that for IO space
(still much better than KVM device assignment, where the KVM module
actually *ignores* the process limits and locks everything anyway),
and convert from KiB to bytes.
In the case of hotplug, we are changing the limit for the already
existing qemu process (prlimit() is used under the hood), and for
regular commandline additions of vfio devices, we schedule a call to
setrlimit() that will happen after the qemu process is forked.
These were previously being set in a custom hook function, but now
that virCommand directly supports setting them, we can eliminate that
part of the hook and call the APIs directly.
The differences from virNodeDeviceDettach are very minor:
1) Check that the flags are 0.
2) Set the virPCIDevice's stubDriver according to the driverName that
is passed in.
3) Call virPCIDeviceDetach with a NULL stubDriver, indicating it
should get the name of the stub driver from the virPCIDevice
object.
If the config for a device has specified <driver name='vfio'/>,
"backend" in the pci part of the hostdev object will be set to
..._VFIO. In this case, when creating a virPCIDevice set the
stubDriver to "vfio-pci", otherwise set it to "pci-stub". We will rely
on the lower levels to report an error if the vfio driver isn't
loaded.
The detach/attach functions in virpci.c will pay attention to the
stubDriver setting in the device, and bind/unbind the appropriate
driver when preparing hostdevs for the domain.
Note that we don't yet attempt to do anything to mark active any other
devices in the same vfio "group" as a single device that is being
marked active. We do need to do that, but in order to get basic VFIO
functionality testing sooner rather than later, initially we'll just
live with more cryptic errors when someone tries to do that.
The device option for vfio-pci is nearly identical to that for
pci-assign - only the configfd parameter isn't supported (or needed).
Checking for presence of the bootindex parameter is done separately
from constructing the commandline, similar to how it is done for
pci-assign.
This patch contains tests to check for proper commandline
construction. It also includes tests for parser-formatter-parser
roundtrips (xml2xml), because those tests use the same data files, and
would have failed had they been included before now.
qemu: xml/args tests for VFIO hostdev and <interface type='hostdev'/>
These should be squashed in with the patch that adds commandline
handling of vfio (they would fail at any earlier time).
There will soon be other items related to pci hostdevs that need to be
in the same part of the hostdevsubsys union as the pci address (which
is currently a single member called "pci". This patch replaces the
single member named pci with a struct named pci that contains a single
member named "addr".
QEMU_CAPS_DEVICE_VFIO_PCI is set if the device named "vfio-pci" is
supported in the qemu binary.
QEMU_CAPS_VFIO_PCI_BOOTINDEX is set if the vfio-pci device supports
the "bootindex" parameter; for some reason, the bootindex parameter
wasn't included in early versions of vfio support (qemu 1.4) so we
have to check for it separately from vfio itself.
Jim Fehlig reported on IRC that older gcc/glibc triggers this warning:
cc1: warnings being treated as errors
qemu/qemu_domain.c: In function 'qemuDomainDefFormatBuf':
qemu/qemu_domain.c:1297: error: declaration of 'remove' shadows a global declaration [-Wshadow]
/usr/include/stdio.h:157: error: shadowed declaration is here [-Wshadow]
make[3]: *** [libvirt_driver_qemu_impl_la-qemu_domain.lo] Error 1
Fix it like we have done in the past (such as commit 2e6322a).
* src/qemu/qemu_domain.c (qemuDomainDefFormatBuf): Avoid shadowing
a function name.
Signed-off-by: Eric Blake <eblake@redhat.com>
After 9d6e56db the syntax-check was unhappy due to wrong whitespacing:
src/qemu/qemu_command.c:1637: for ( ; a.slot < QEMU_PCI_ADDRESS_SLOT_LAST; a.slot++) {
maint.mk: incorrect whitespace around brackets, see HACKING for rules
make: *** [bracket-spacing-check] Error 1
After 78d7c3c5 we are strdup()-ing path to qemu-bridge-helper.
However, the check for its return value is missing. So it is
possible we've ignored the OOM error silently.
Add a "dry run" address allocation to figure out how many bridges
will be needed for all the devices without explicit addresses.
Auto-add just enough bridges to put all the devices on, or up to the
bridge with the largest specified index.
<controller type='pci' index='0' model='pci-root'/>
is auto-added to pc* machine types.
Without this controller PCI bus 0 is not available and
no PCI addresses are assigned by default.
Since older libvirt supported PCI bus 0 even without
this controller, it is removed from the XML when migrating.
Now we set the default disk driver name when parsing
the qemu command line too, hence all the test changes.
Assume format type is 'auto' when none is specified on
qemu command line.
Currently, if there has been an error in building command line
process after virtual interfaces has been created, the flow jumps
to 'error' label, where virDomainConfNWFilterTeardown() is
called. This may report an error as well, but should not
overwrite the original cause why we jumped to 'error' label.
Instead of making a choice between the underscore and camelCase, this
simply changes "num_queues" into "queues", which is also consistent
with Michal's multiple queue support for interface.
Improve error reporting and generating of SPICE command line arguments
according to the need to enable TLS. If TLS is disabled, there's no need
to pass the certificate dir to qemu.
This patch resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=953126
Ensure that all drivers implementing public APIs use a
naming convention for their implementation that matches
the public API name.
eg for the public API virDomainCreate make sure QEMU
uses qemuDomainCreate and not qemuDomainStart
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Ensure that the driver struct field names match the public
API names. For an API virXXXX we must have a driver struct
field xXXXX. ie strip the leading 'vir' and lowercase any
leading uppercase letters.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Switch the function from a bunch of ifs to a switch statement with
correct type and reflow some code.
Also fix comment in enum describing possible graphics types
Decrease size of qemuBuildGraphicsCommandLine() by splitting out
spice-related code into qemuBuildGraphicsVNCCommandLine().
This patch also fixes 2 possible memory leaks on error path in the code
that was split-out. The buffer containing the already generated options
and a listen address string could be leaked.
Also break a few very long lines and reflow code that fits now.
Decrease size of qemuBuildGraphicsCommandLine() by splitting out
spice-related code into qemuBuildGraphicsSPICECommandLine().
This patch also fixes 2 possible memory leaks on error path in the code
that was split-out. The buffer containing the already generated options
and a listen address string could be leaked.
Also break a few very long lines.
