Coverity complains that many other callers to return err from
virGetLastError() will check if err is not NULL before dereferencing
it. Just do the same here for safety.
virReportSystemError is reserved for reporting system errors, calling it
with VIR_ERR_* error codes produces error messages that do not make any
sense, such as
internal error: guest failed to start: Kernel doesn't support user
namespace: Link has been severed
We should prohibit wrong usage with a syntax-check rule.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
This reverts commit 433b427ff8.
The patch was added in order to overcome a bug in iproute2 and since it
was properly identified as a bug, particularly in openSUSE 13.2, and it
is being worked on [1], the best solution for libvirt seems to be to
keep the old behaviour.
[1] https://bugzilla.novell.com/show_bug.cgi?id=907093
Commit 2aa167ca tried to fix the DBus interaction code to allow
callers to use native types instead of 4-byte bools. But in
fixing the issue, I missed the case of an arrayref; Conrad Meyer
shows the following valid complaint issued by clang:
CC util/libvirt_util_la-virdbus.lo
util/virdbus.c:956:13: error: cast from 'bool *' to 'dbus_bool_t *' (aka 'unsigned int *') increases required alignment from 1 to 4 [-Werror,-Wcast-align]
GET_NEXT_VAL(dbus_bool_t, bool_val, bool, "%d");
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
util/virdbus.c:858:17: note: expanded from macro 'GET_NEXT_VAL'
x = (dbustype *)(*xptrptr + (*narrayptr - 1)); \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1 error generated.
But fixing that points out that we have NEVER supported arrayrefs
of sub-int types (byte, i16, u16, and now bool). Again, while raw
types promote, arrays do not; so the macros HAVE to deal with both
size possibilities rather than assuming that an arrayref uses the
same sizing as the promoted raw type.
Obviously, our testsuite wasn't covering as much as it should have.
* src/util/virdbus.c (GET_NEXT_VAL): Also fix array cases.
(SET_NEXT_VAL): Fix uses of sub-int arrays.
* tests/virdbustest.c (testMessageArray, testMessageArrayRef):
Test it.
Signed-off-by: Eric Blake <eblake@redhat.com>
Due to a change (or bug?) in ip link implementation, the command
'ip link add vnet0...'
is forced into
'ip link add name vnet0...'
The changed command also works on older versions of iproute2, just the
'name' parameter has been made mandatory.
To be able to express some use cases of the RBD backing with libvirt, we
need to be able to specify a config file for the RBD client to qemu as
that is one of the commonly used options.
Some storage systems have internal support for snapshots. Libvirt should
be able to select a correct snapshot when starting a VM.
This patch adds a XML element to select a storage source snapshot for
the RBD protocol which supports this feature.
As we now have a common function to parse backing store string for RBD
backing store we can reuse it in the backing store walker so that we
don't fail on files backed by RBD storage.
This patch also adds a few tests to verify that the parsing works as
expected.
To allow reuse this non-trivial parser code in the backing store parser
this part of the command line parser needs to be split out into a
separate funciton.
If there are no hosts for a storage source virStorageSourceCopy and
virStorageSourceNewFromBackingRelative would try to copy them anyways.
As the success of virStorageNetHostDefCopy is determined by returning
a pointer and malloc of 0 elements might return NULL according to the
implementation, the result of the copy function may vary.
Fix this by copying the hosts array only if there are hosts defined.
As we now have a deep copy function for struct virStorageSource add a
notice that extensions of the structure require also appropriate changes
to the virStorageSourceCopy func.
When creating a disk image snapshot the libvirt code would blindly copy
the parents label to the newly created image. This runs into problems
when you start a VM from an image hosted on NFS (or other storage system
that doesn't support selinux labels) and the snapshot destination is on
a storage system that does support selinux labels. Libvirt's code in
that case generates a different security label for the image hosted on
NFS. This label is valid only for NFS images and doesn't allow access in
case of a locally stored image.
To fix this issue libvirt needs to refrain from copying security
information in cases where the default domain seclabel is a better
choice.
This patch repurposes the now unused @force argument of
virStorageSourceInitChainElement to denote whether a copy of the
security labelling stuff should be attempted or not. This allows to
fine-control the copy operation for cases where we need to keep the
label of the old disk vs. the cases where we need to keep the label
unset to use the default domain imagelabel.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1151718
Commit c0e7022 breaks on a machine that lacks dbus headers:
In file included from util/virdbus.c:24:0:
util/virdbuspriv.h:31:3: error: unknown type name 'dbus_int16_t'
* src/util/virdbuspriv.h (DBusBasicValue): Only provide fallback
when dbus is compiled.
Signed-off-by: Eric Blake <eblake@redhat.com>
Compilation on a RHEL 5 host failed, due to the older dbus headers
present on that machine, and triggered by commit 2aa167ca:
util/virdbus.c: In function 'virDBusMessageIterDecode':
util/virdbus.c:952: error: 'DBusBasicValue' undeclared (first use in this function)
* m4/virt-dbus.m4 (LIBVIRT_CHECK_DBUS): Check for DBusBasicValue.
* src/util/virdbuspriv.h (DBusBasicValue): Provide fallback.
Signed-off-by: Eric Blake <eblake@redhat.com>
Commit dc33e6e4 caused older platforms like Fedora 20 to emit
scary log messages at startup:
2014-11-19 23:12:58.800+0000: 28906: error : virCommandWait:2532 : internal error: Child process (/usr/sbin/iptables -w -L -n) unexpected exit status 2: iptables v1.4.19.1: unknown option "-w"
Try `iptables -h' or 'iptables --help' for more information.
Since we are probing and expect to handle the case where -w is not
supported, we should not let virCommand log it as an error.
* src/util/virfirewall.c (virFirewallCheckUpdateLock): Handle
non-zero status ourselves.
Signed-off-by: Eric Blake <eblake@redhat.com>
I noticed this while working on qemuDomainGetBlockInfo. Assigning
a bool value to an int variable compiles fine, but raises red flags
on the maintenance front as it becomes too easy to assign -1 or 2
or any other non-bool value to the same variable.
* cfg.mk (sc_prohibit_int_assign_bool): New rule.
* src/conf/snapshot_conf.c (virDomainSnapshotRedefinePrep): Fix
offenders.
* src/qemu/qemu_driver.c (qemuDomainGetBlockInfo)
(qemuDomainSnapshotCreateXML): Likewise.
* src/test/test_driver.c (testDomainSnapshotAlignDisks):
Likewise.
* src/util/vircgroup.c (virCgroupSupportsCpuBW): Likewise.
* src/util/virpci.c (virPCIDeviceBindToStub): Likewise.
* src/util/virutil.c (virIsCapableVport): Likewise.
* tools/virsh-domain-monitor.c (cmdDomMemStat): Likewise.
* tools/virsh-domain.c (cmdBlockResize, cmdScreenshot)
(cmdInjectNMI, cmdSendKey, cmdSendProcessSignal)
(cmdDetachInterface): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Use of an 'int' to represent a 'bool' value is confusing. Just
because dbus made the mistake of cementing their 4-byte wire
format of dbus_bool_t into their API doesn't mean we have to
repeat the mistake. With a little bit of finesse, we can
guarantee that we provide a large-enough value to the DBus
code, while still copying only the relevant one-byte bool
to the client code, and isolate the rest of our code base from
the DBus stupidity.
* src/util/virdbus.c (GET_NEXT_VAL): Add parameter.
(virDBusMessageIterDecode): Adjust all clients.
* src/util/virpolkit.c (virPolkitCheckAuth): Use nicer type.
* tests/virdbustest.c (testMessageSimple, testMessageStruct):
Test new behavior.
Signed-off-by: Eric Blake <eblake@redhat.com>
Ethernet interfaces in libvirt currently do not support bandwidth setting.
For example, following xml file for an interface will not apply these
settings to corresponding qdiscs.
<interface type="ethernet">
<mac address="02:36:1d:18:2a:e4"/>
<model type="virtio"/>
<script path=""/>
<target dev="tap361d182a-e4"/>
<bandwidth>
<inbound average="984" peak="1024" burst="64"/>
<outbound average="2000" peak="2048" burst="128"/>
</bandwidth>
</interface>
Signed-off-by: Anirban Chakraborty <abchak@juniper.net>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
A previous commit introduced use of locking with invocation
of iptables in the viriptables.c module
commit ba95426d6f
Author: Serge Hallyn <serge.hallyn@ubuntu.com>
Date: Fri Nov 1 12:36:59 2013 -0500
util: use -w flag when calling iptables
This only ever had effect with the virtual network driver,
as it was not wired up into the nwfilter driver. Unfortunately
in the firewall refactoring the use of the -w flag was
accidentally lost.
This patch introduces it to the virfirewall.c module so that
both the virtual network and nwfilter drivers will be using
it. It also ensures that the equivalent --concurrent flag
to ebtables is used.
This patch fixes the following issues.
1) When an invalid wwn is introduced, libvirt reports
"Malformed wwn: %s". The template won't be replaced.
2) "target" option for dompmsuspend and "xml" option for
save-image-define are required options and should use
VSH_OT_DATA instead of VSH_OT_STRING as an option type.
3) A typo.
Signed-off-by: Hao Liu <hliu@redhat.com>
Introduced by commit c63ef0452b, when nodeset is NULL, validation will
pass in virNumaSetupMemoryPolicy, but virBitmapNextSetBit must ensure
bitmap is not NULL, otherwise that might cause a segmentation fault.
This patch fixes it.
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
This adds support for PowerPC Little Endian architecture.,
and allows libvirt to spawn VMs based on 'ppc64le' architecture.
Signed-off-by: Pradipta Kr. Banerjee <bpradip@in.ibm.com>
Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
One of the latest patches (9a8fc3efc2) introduced call of
geteuid(). However, not all systems have the function
implemented, e.g. mingw. Therefore, we fail to build on those
system. The fix consist of including virutil.h which defines
geteuid in needed. Sigh.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When compiled without full numa support, the stub function for
virNumaNodeIsAvailable() just checks whether specified node is in range
<0, max); where max is maximum NUMA node available on the host. But
because the maximum node number is the highest usabe number (and not the
count of nodes), the check is incorrect as it should check whether the
specified node is in range <0, max> instead.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
This is a reaction to Michal's fix [1] for non-NUMA systems that also
splits out conf/ out of util/ because libvirt_util shouldn't require
libvirt_conf if it is the other way around. This particular use case
worked, but we're trying to avoid it as mentioned [2], many times.
The only functions from virnuma.c that needed numatune_conf were
virDomainNumatuneNodesetIsAvailable() and virNumaSetupMemoryPolicy().
The first one should be in numatune_conf as it works with
virDomainNumatune, the second one just needs nodeset and mode, both of
which can be passed without the need of numatune_conf.
Apart from fixing that, this patch also fixes recently added
code (between commits d2460f85^..5c8515620) that doesn't support
non-contiguous nodesets. It uses new function
virNumaNodesetIsAvailable(), which doesn't need a stub as it doesn't use
any libnuma functions, to check if every specified nodeset is available.
[1] https://www.redhat.com/archives/libvir-list/2014-November/msg00118.html
[2] http://www.redhat.com/archives/libvir-list/2011-June/msg01040.html
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Patch 43b67f2e disallowed network tuning only with qemu driver, however
this patch moved the check for root privileges into
virNetDevBandwidthSet function, so the call should now
fail in all possible cases. A mock function was created so that the test
suite doesn't fail because of unsufficient privileges.
Coverity found out the very obvious problem in the code. That is that
virPidFileReleasePath() was called only if
virPidFileAcquirePath() returned 0. But virPidFileAcquirePath() doesn't
return only 0 on success, but the FD that needs to be closed.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
There was no check for 'nodeset' attribute in numatune-related
elements. This patch adds validation that any nodeset specified does
not exceed maximum host node.
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
This function is used to cleanup a pidfile doing whatever it takes, even
killing the owning process.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
This macro seems to be defined only on linux/unix and it fails during
mingw build. Its value is '16' (taken from net/if.h) so define it if
it's not defined.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
The virGetSCSIHostNumber function return type is int, however
its stubbed version returns NULL. That results in a build fail
on systems that use the stubbed version. Fix by using a proper
return type.
Currently, build fails on FreeBSD because its struct ifreq does not
have ifr_hwaddr member. In order to fix that, check if this member
is present, otherwise fall back to the stub version of the
virNetDev{Add,Del}Multi functions.
The complaint is that if cleanup is called when virFileReadAll fails,
then mcast->entries is NULL and could be dereferenced in the clear
function. After following the code some - I saw that the caller to
the function (virNetDevGetMulticastTable) will also call
virNetDevMcastListClear if this function returns -1, so this
isn't necessary, so I removed the call.
Coverity complains that because the for loop is from 0 to 5 (max tokens)
and the impending switch/case statements used each of the #define values
that the 'default' wouldn't reachable. This patch will convert the #define's
into enum's and add the obligatory dead_error_begin marker for these type
situations.
Signed-off-by: John Ferlan <jferlan@redhat.com>
The code that parses the schema from the URI touches the "hosts[0]"
member of the storage file source structure in case the URI contains a
schema. The hosts array was not yet allocated at the point in the code
where the transport protocol was parsed and set. This lead to a crash of
libvirtd.
Fix the code by allocating the "hosts" array upfront and add a test case
to verify this scenario. (Unfortunately this requires shuffling the test
case numbers too).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1156288
We weren't ever using the value for anything other than being non-zero.
* src/util/viraudit.h (virAuditLog): Change signature.
* src/util/viraudit.c (virAuditLog): Update user.
* daemon/libvirtd.c (main): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
This patch provides the utility functions to needed to synchronize the
changes made to a guest domain network device's multicast filter
with the corresponding macvtap device's filter on the host:
* Get/add/remove multicast MAC addresses
* Get the macvtap device's RX filter list
Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
Signed-off-by: Laine Stump <laine@laine.org>
virNetDevLinkDump() gets a message from netlink into "resp", then
calls nlmsg_parse() to fill the table "tb" with pointers into resp. It
then returns tb to its caller, but not before freeing the buffer at
resp. That means that all the callers of virNetDevLinkDump() are
examining memory that has already been freed. This can be verified by
filling the buffer at resp with garbage prior to freeing it (or, I
suppose, just running libvirtd under valgrind) then performing some
operation that calls virNetDevLinkDump().
The code has been like this ever since virNetDevLinkDump() was written
- the original author didn't notice it, and neither did later
additional users of the function. It has only been pure luck (or maybe
a lack of heavy load, and/or maybe an allocation algorithm in malloc()
that delays re-use of just-freed memory) that has kept this from
causing errors, for example when configuring a PCI passthrough or
macvtap passthrough network interface.
The solution taken in this patch is the simplest - just return resp to
the caller along with tb, then have the caller free it after they are
finished using the data (pointers) in tb. I alternately could have
made a cleaner interface by creating a new struct that put tb and resp
together along with a vir*Free() function for it, but this function is
only used in a couple places, and I'm not sure there will be
additional new uses of virNetDevLinkDump(), so the value of adding a
new type, extra APIs, etc. is dubious.
