Coverity points out it's possible for one of the virCommand{Output|Error}*
API's to have not allocated 'output' and/or 'error' in which case the
strstr comparison will cause a NULL deref
Signed-off-by: John Ferlan <jferlan@redhat.com>
Build with gcc 4.8 fails with:
bhyve/bhyve_monitor.c: In function 'bhyveMonitorIO':
bhyve/bhyve_monitor.c:51:18: error: missing initializer for field 'tv_sec' of 'const struct timespec' [-Werror=missing-field-initializers]
const struct timespec zerowait = {};
Explicitly initialize zerowait to fix the build.
Fixes build problems on x86_64-cygwin host for aarch64 target:
CC lxc/libvirt_driver_lxc_impl_la-lxc_monitor_protocol.lo
In file included from lxc/lxc_monitor_protocol.c:7:0:
lxc/lxc_monitor_protocol.h:9:21: fatal error: rpc/rpc.h: No such file or directory
CC rpc/libvirt_setuid_rpc_client_la-virnetmessage.lo
In file included from rpc/virnetmessage.h:24:0,
from rpc/virnetmessage.c:26:
rpc/virnetprotocol.h:9:21: fatal error: rpc/rpc.h: No such file or directory
CC lxc/libvirt_lxc-lxc_monitor_protocol.o
In file included from lxc/lxc_monitor_protocol.c:7:0:
lxc/lxc_monitor_protocol.h:9:21: fatal error: rpc/rpc.h: No such file or directory
Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
If the firewalld backend wasn't available and libvirt decides to try
setting up a "direct" backend, it checks for the presence of iptables,
ip6tables, and ebtables. If they are not found, a message like this is logged:
error : virFirewallValidateBackend:193 : direct firewall backend
requested, but /usr/sbin/ip6tables is not available:
No such file or directory
But then at a later time if an attempt is made to use the virFirewall
API, failure will be indicated with:
error : virFirewallApply:936 : out of memory
This patch changes virFirewallApply to first check if a firewall
backend hadn't been successfully setup, and logs a slightly more
informative message in that case:
error : virFirewallApply:940 : internal error:
Failed to initialize a valid firewall backend
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1223876
As of netcf-0.2.8, netcf supports configuring multipl IPv4 addresses,
as well as simultaneously configuring dhcp and static IPv4 addresses,
on a single interface. This patch updates libvirt's interface.rng to
allow such configurations.
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1223688
If an SRIOV PF is offline, the kernel won't complain if you set the
mac address and vlan tag for a VF via this PF, and it will even let
you assign the VF to a guest using PCI device assignment or macvtap
passthrough. But in this case (the PF isn't online), the device won't
be usable in the guest.
Silently setting the PF online would solve the connectivity problem,
but as pointed out by Dan Berrange, when an interface is set online
with no associated config, the kernel will by default turn on IPv6
autoconf, which could create unexpected security problems for the
host. For this reason, this patch instead logs an error and fails the
operation.
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=893738
Originally filed against RHEL6, but present in every version of
libvirt until today.
commit ffc40b63b5 changed uniond _virNodeDevCapData into a typedef
named virNodeDevCapData with a struct that contains the union as well
as a type enum. This change necessitated changing every reference to
"caps->type" into "caps->data.type", but the author of that patch
failed to test a build "WITH_HAL". This patch fixes the one place in
the hal backend that needed changing.
Due to a kernel commit (b4b8f770e), cpuinfo format has changed on
ARMs. Firstly, 'Processor: ...' may not be reported, it's
replaced by 'model name: ...'. Secondly, the "Processor" string
may occur in CPU name, e.g. 'ARMv7 Processor rev 5 (v7l)'.
Therefore, we must firstly look for 'model name' and then for
'Processor' if not found.
Moreover, lines in the cpuinfo file are shuffled, so we better
not manipulate the pointer to start of internal buffer as we may
lost some info.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Not every chardev is plugged onto virtio-serial bus. However, the
code introduced in 89e991a2aa assumes that. Incorrectly.
With previous patches we have three options where a chardev can
be plugged: virtio-serial, USB and PCI. This commit fixes the
detach part. However, since we are not auto allocating USB
addresses yet, I'm just marking the place where appropriate code
should go.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Not every chardev is plugged onto virtio-serial bus. However, the
code introduced in 89e991a2aa assumes that. Incorrectly.
With previous patches we have three options where a chardev can
be plugged: virtio-serial, USB and PCI. This commit fixes the
attach part. However, since we are not auto allocating USB
addresses yet, I'm just marking the place where appropriate code
should go.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=998813
Implementation is pretty straight-forward. Of course, not all qemus
out there supports the device, so new capability is introduced and
checked prior each use of the device.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=998813
Like usb-serial, the pci-serial device allows a serial device to be
attached to PCI bus. An example XML looks like this:
<serial type='dev'>
<source path='/dev/ttyS2'/>
<target type='pci-serial' port='0'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
</serial>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Old compilers whine:
src/util/virutil.c: In function 'virMemoryMaxValue':
src/util/virutil.c:2612: error: declaration of 'ulong' shadows a global declaration [-Wshadow]
/usr/include/sys/types.h:151: error: shadowed declaration is here [-Wshadow]
s/ulong/capped/ to work around the problem
For setting passwords of users inside the domain.
With the VIR_DOMAIN_PASSWORD_ENCRYPTED flag set, the password
is assumed to be already encrypted by the method required
by the guest OS.
https://bugzilla.redhat.com/show_bug.cgi?id=1174177
Using joinable threads does not help anything, but it can lead to memory
leaks.
When a worker thread exits, it decreases nWorkers or nPrioWorkers and
once both nWorkers and nPrioWorkers are zero (i.e., the last worker is
gone), quit_cond is signaled. When freeing the pool we first tell all
threads to die and then we are waiting for both nWorkers and
nPrioWorkers to become zero. At this point we already know all threads
are gone. So the only reason for calling virThreadJoin of all workers is
to free the memory allocated for joinable threads. If we avoid
allocating this memory, we don't need to take care of freeing it.
Moreover, any memory associated with a worker thread which died before
we asked it to die (e.g., because virCondWait failed in the thread)
would be lost anyway since virThreadPoolFree calls virThreadJoin only
for threads which were running at the time virThreadPoolFree was called.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Most virDomainDiskIndexByName callers do not care about the index; what
they really want is a disk def pointer.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Sometimes the only thing we need is the pointer to virDomainDiskDef and
having to call virDomainDiskIndexBy* APIs, storing the disk index, and
looking it up in the disks array is ugly. After this patch, we can just
call virDomainDiskBy* and get the pointer in one step.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
When starting a domain, if a domain specifies security drivers we do not have
loaded, we fail. However we don't check for this during
reconnect, so any operation relying on security driver functionality would fail.
If someone e.g. starts a domain with selinux driver loaded, then they change
the security driver to 'none' in config, restart the daemon and call dump/save/..,
QEMU will return an error.
As we shouldn't kill the domain, we should at least log an error to let the
user know that domain reconnect wasn't completely clean.
https://bugzilla.redhat.com/show_bug.cgi?id=1183893
After parsing the memory device XML the function would not restore the
XML parser context causing invalid XPath starting point for the rest of
the elements. This is a regression since 3e4230d2.
The test case addition uses the <idmap> element that is currently unused
by qemu, but parsed after the memory device definition and formatted
always.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1223631
Signed-off-by: Luyao Huang <lhuang@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
virDomainParseMemory parses the size and then rounds up while converting
it to kibibytes. Since the number is limit-checked before the rounding
it's possible to use a number that would be correctly parsed the first
time, but not the second time. For numbers not limited to 32 bit systems
the magic is 9223372036854775807 bytes. That number then can't be parsed
back in kibibytes.
To solve the issue add a second overflow check for the few values that
would cause the problem. Since virDomainParseMemory is used in config
parsing, this avoids vanishing VMs.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1221504
So far, we are not reporting if numatune was even defined. The
value of zero is blindly returned (which maps onto
VIR_DOMAIN_NUMATUNE_MEM_STRICT). Unfortunately, we are making
decisions based on this value. Instead, we should not only return
the correct value, but report to the caller if the value is valid
at all.
For better viewing of this patch use '-w'.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=976387
For a domain configured using the host cdrom, we should taint the domain
due to problems encountered when the host and guest try to control the tray.
Since af2a1f0587,
qemuDomainGetNumaParameters() returns invalid value for a running
guest. The problem is that it is getting the information from cgroups,
but the parent cgroup is being left alone since the mentioned commit.
Since the running guest's XML is in sync with cgroups, there is no need
to look into cgroups (unless someone changes the configuration behind
libvirt's back). Returning the info from the definition fixes a bug and
is also a cleanup.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1221047
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
From xl.cfg950 man page:
spiceagent_mouse=BOOLEAN
Whether SPICE agent is used for client mouse mode. The default is
true (1) (turn on)
spicevdagent=BOOLEAN
Enables spice vdagent. The Spice vdagent is an optional component for
enhancing user experience and performing guest-oriented management
tasks. Its features includes: client mouse mode (no need to grab
mouse by client, no mouse lag), automatic adjustment of screen
resolution, copy and paste (text and image) between client and domU.
It also requires vdagent service installed on domU o.s. to work.
The default is 0.
spice_clipboard_sharing=BOOLEAN
Enables Spice clipboard sharing (copy/paste). It requires spicevdagent
enabled. The default is false (0).
So if spiceagent_mouse is enabled (client mouse mode) or
spice_clipboard_sharing is enabled, spicevdagent must be enabled.
Along with this change, s/spicedvagent/spicevdagent, set
spiceagent_mouse correctly, and add a test for these spice
features.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
The logic related to spicedisable_ticketing and spicepasswd was
inverted. As per man xl.cfg(5), 'spicedisable_ticketing = 1'
means no passwd is required. On the other hand, a passwd is
required if 'spicedisable_ticketing = 0'. Fix the logic and
produce and error if 'spicedisable_ticketing = 0' but spicepasswd
is not provided. Also fix the spice cfg test file.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Move formating of spice listenAddr to the section of code
where spice ports are formatted. It is more logical to
format address and ports together. Account for the change
in spice cfg test file by moving 'spicehost'.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Both the hal and udev drivers call virPCI*() functions to the the
SRIOV VF/PF info about PCI devices, and the UDEV backend calls
virPCI*() to get IOMMU group info. Since there is now a single
function call in node_device_linux_sysfs.c to do all of this, replace
all that code in the two backends with calls to
nodeDeviceSysfsGetPCIRelatedDevCaps().
Note that this results in the HAL driver (probably) unnecessarily
calling virPCIDevieAddressGetIOMMUGroupNum(), but in the case that the
host doesn't support IOMMU groups, that function turns into a NOP (it
returns -2, which causes the caller to skip the call to
virPCIDeviceAddressGetIOMMUGroupAddresses()). So in the worst case it
is a few extra cycles spent, and in the best case a mythical platform
that supported IOMMU groups but used HAL rather than UDEV would gain
proper reporting of IOMMU group info.
Because reloading a PF driver with a different number of VFs doesn't
result in any sort of event sent from udev to the libvirt node_device
driver, libvirt's cache of that info can be out of date when a request
arrives for the info about a device. To fix this, we refresh that data
at the time of the dumpxml request, similar to what is already done
for netdev link info and SCSI host capabilities.
Since the same is true for iommu group information (for example, some
other device in the same iommu group could have been detached from the
host), we also create a function to update the iommu group info from
sysfs, and a common function that does both. (a later patch will call
this common function from the udev and hal backends).
This resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=981546
The udev and hal drivers both already call the same functions as these
new functions added to node_device_linux_sysfs.c, but 1) we need to
call them from node_device_driver.c, and 2) it would be nice to
eliminate the duplicated code from the hal and udev backends.
This file contains only a single function, detect_scsi_host_caps(),
which is declared in node_device_driver.h and called from both the hal
and udev backends. Other things common to the hal and udev drivers
can be placed in that file though. As a prelude to adding further
functions, this patch renames the existing function to something
closer in line with other internal libvirt function names
(nodeDeviceSysfsGetSCSIHostCaps()), and puts the declarations into a
separate .h file.
For some reason a union (_virNodeDevCapData) that had only been
declared inside the toplevel struct virNodeDevCapsDef was being used
as an argument to functions all over the place. Since it was only a
union, the "type" attribute wasn't necessarily sent with it. While
this works, it just seems wrong.
This patch creates a toplevel typedef for virNodeDevCapData and
virNodeDevCapDataPtr, making it a struct that has the type attribute
as a member, along with an anonymous union of everything that used to
be in union _virNodeDevCapData. This way we only have to change the
following:
s/union _virNodeDevCapData */virNodeDevCapDataPtr /
and
s/caps->type/caps->data.type/
This will make me feel less guilty when adding functions that need a
pointer to one of these.
The option didn't have VSH_OT_INT type even thought it's expected
to be numeric, as shown by the fact that vshCommandOptInt() is later
used to retrieve its value.
Replace more than 30 ad-hoc error messages with a single, generic one
that contains the name of the option being processed and some hints
to help the user understand what could have gone wrong.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1207043
Test the support for enabling/disabling CPACF protected key management
operations for a guest.
Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Introduces two new -machine option parameters to the QEMU command to
enable/disable the CPACF protected key management operations for a guest:
aes-key-wrap='on|off'
dea-key-wrap='on|off'
The QEMU code maps the corresponding domain configuration elements to the
QEMU -machine option parameters to create the QEMU command:
<cipher name='aes' state='on'> --> aes-key-wrap=on
<cipher name='aes' state='off'> --> aes-key-wrap=off
<cipher name='dea' state='on'> --> dea-key-wrap=on
<cipher name='dea' state='off'> --> dea-key-wrap=off
Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
Signed-off-by: Daniel Hansel <daniel.hansel@linux.vnet.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Two new domain configuration XML elements are added to enable/disable
the protected key management operations for a guest:
<domain>
...
<keywrap>
<cipher name='aes|dea' state='on|off'/>
</keywrap>
...
</domain>
Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Signed-off-by: Daniel Hansel <daniel.hansel@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Currently, the libxl driver does not support any security drivers.
When the qemu driver has no security driver configued,
nodeGetSecurityModel succeeds but returns an empty virSecurityModel
object. Do the same in the libxl driver instead of reporting
this function is not supported by the connection driver:
virNodeGetSecurityModel
We have previously effectively ignored all <controller type='ide'>
elements in a domain definition.
On the i440fx-based machinetypes there is an IDE controller that is
included in the chipset and can't be removed (which is the ide
controller with index='0'>), so it makes sense to ignore that one
controller. However, if an i440fx domain definition has a 2nd
controller, nothing catches this error (unless you also have a disk
attached to it, in which case qemu will complain that you're trying to
use the ide controller named "ide1", which doesn't exist), and if any
other type of domain has even a single controller defined, it will be
incorrectly ignored.
Ignoring a bogus controller definition isn't such a big problem, as
long as an error is logged when any disk is attached to that
non-existent controller. But in the case of q35-based machinetypes,
the hardcoded id ("alias" in libvirt terms) of its builtin SATA
controller is "ide", which happens to be the same id as the builtin
IDE controller on i440fx machinetypes. So libvirt creates a
commandline believing that it is connecting the disk to the builtin
(but actually nonexistent) IDE controller, qemu thinks that libvirt
wanted that disk connected to the builtin SATA controller, and
everybody is happy.
Until you try to connect a 2nd disk to the IDE controller. Then qemu
will complain that you're trying to set unit=1 on a controller that
requires unit=0 (SATA controllers are organized differently than IDE
controllers).
After this patch, if a domain has an IDE controller defined for a
machinetype that has no IDE controllers, libvirt will log an error
about the controller itself as it is building the qemu commandline
(rather than a (possible) error from qemu about disks attached to that
controller). This is done by adding IDE to the list of controller
types that are handled in the loop that creates controller command
strings in qemuBuildCommandline() (previously it would *always* skip
IDE controllers). Then qemuBuildControllerDevStr() is modified to log
an appropriate error in the case of IDE controllers.
In the future, if we add support for extra IDE controllers (piix3-ide
and/or piix4-ide) we can just add it into the IDE case in
qemuBuildControllerDevStr(). For now, nobody seems anxious to add
extra support for an aging and very slow controller, when there are so
many better options available.
Resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1176071 (Fedora)
Back in 2013, commit 877bc089 added in some tests that made sure no
error was generated on a domain definition that had an automatically
added usb controller if that domain didn't have a PCI bus to attach
the usb controller to. This was done because, at that time, libvirt
was automatically adding a usb controller to *any* domain definition
that didn't have one. Along with permitting the controller, two
s390-specific tests were added to ensure this behavior was maintained
- one with <controller type='usb' model='none'/> and another (called
"s390-piix-controllers") that had both usb and ide controllers, but
nothing attached to them.
Then in February of this year, commit 09ab9dcc eliminated the annoying
auto-adding of a usb device for s390 and s390x machines, stating:
"Since s390 does not support usb the default creation of a usb
controller for a domain should not occur."
Although, as verified here, the s390 doesn't support usb, and usb
controllers aren't currently added to s390 domain definitions
automatically, there are likely still some domain definitions in the
wild that have a usb controller (which was added *by libvirt*, not by
the user), so we will keep the tests verifying that behavior for
now. But this patch changes the names of the tests to reflect that
they don't actually contain a valid s390 config; this way future
developers won't propagate the incorrect idea that an s390 virtual
machine can have a USB (or IDE) bus.
In the case of the IDE controller, though, libvirt has never
automatically added an IDE controller unless a user added an IDE disk
(which itself would have caused an error), and we specifically *do*
want to begin generating an error when someone tries to add an IDE
controller to a domain that can't support one. For that reason, while
renaming the sz390-piix-controllers patch, this patch removes the
<controller type='ide'...> from it (otherwise the upcoming patch would
break make check)
This makes sure that that the commandlines generated for devices and
controller devices are all using the alias that has been set in the
controller's object as the id of the controller, rather than
hardcoding a printf (or worse, encoding exceptions to the standard
${controller}${index} into the logic)
Since this "fixes" the controller name used for the sata controller,
the commandline arg for the sata controller in the sata test case had
to be adjusted to be "sata0" instead of "ahci0". All other tests
remain unchanged, verifying that the patch causes no other functional
change.
Because the function that finds a controller alias based on a device
def requires a pointer to the full domainDef in order to get the list
of controllers, the arglist of a few functions had to have this added.
