Refactor some code to create a static function virHostdevIsPCINetDevice
which will detect whether the hostdev is a pci net device or not.
Signed-off-by: Huanle Han <hanxueluo@gmail.com>
Commit 1268820a removed obsolete index() function and replaced it by
strchr. Few versions of gcc has a bug and reports a warning about
strchr:
../../src/util/virstring.c:1006: error: logical '&&' with non-zero
constant will always evaluate as true [-Wlogical-op]
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Commit 2a530a3e5 is not portable to mingw, which intentionally
avoids declaring the obsolete index(). See also:
https://bugzilla.redhat.com/show_bug.cgi?id=1214605
* src/util/virstring.c (virStringStripControlChars): Use strchr.
Signed-off-by: Eric Blake <eblake@redhat.com>
When running on FreeBSD, there's a bug in virCommandProcessIO
polling that is triggered by the commandtest.
A test that triggers EPIPE in commandtest (named "test20") hungs
forever on FreeBSD.
Apparently, this happens because FreeBSD sets POLLHUP flag on revents
when stdin in closed. And as the current implementation only checks for
POLLOUT and POLLERR, it ends up looping forever inside
virCommandProcessIO and not trying to do one more write() that would
trigger EPIPE.
To fix that check for the POLLHUP flag along with POLLOUT and POLLERR.
When a user would specify a backing chain index that is above the start
point libvirt would report a rather unhelpful error:
invalid argument: could not find backing store 1 in chain for 'sub/link2'
This patch adds an explicit check that the index is below start point in
the backing store and reports the following error if not:
invalid argument: requested backing store index 1 is above 'sub/../qcow2' in chain for 'sub/link2'
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177062
Some storage protocols allow to have the @path field in struct
virStorageSource set to NULL. Add NULLSTR() wrappers to handle this
possibility until I finish the storage source error formatter.
Build fails on non-Linux systems with this error:
CC util/libvirt_util_la-virnetdev.lo
util/virnetdev.c:364:1: error: unused function 'virNetDevReplaceMacAddress' [-Werror,-Wunused-function]
virNetDevReplaceMacAddress(const char *linkdev,
^
util/virnetdev.c:406:1: error: unused function 'virNetDevRestoreMacAddress' [-Werror,-Wunused-function]
virNetDevRestoreMacAddress(const char *linkdev,
^
2 errors generated.
The virNetDev{Restore,Replace}MacAddress() functions are only used
by VF-related routines that are available on Linux only. So move these
functions under the same #ifdef.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1113474
When we set the MAC address of a network device as a part of setting
up macvtap "passthrough" mode (where the domain has an emulated netdev
connected to a host macvtap device that has exclusive use of the
physical device, and sets the device MAC address to match its own,
i.e. "<interface type='direct'> <source mode='passthrough' .../>"), we
use ioctl(SIOCSIFHWADDR) giving it the name of that device. This is
true even if it is an SRIOV Virtual Function (VF).
But, when we are setting the MAC address / vlan ID of a VF in
preparation for "hostdev network" passthrough (this is where we set
the MAC address and vlan id of the VF after detaching the host net
driver and before assigning the device to the domain with PCI
passthrough, i.e. "<interface type='hostdev'>", we do the setting via
a netlink RTM_SETLINK message for that VF's Physical Function (PF),
telling it the VF# we want to change. This sets an "administratively
changed MAC" flag for that VF in the PF's driver, and from that point
on (until the PF driver is reloaded, *not* merely the VF driver) that
VF's MAC address can't be changed using ioctl(SIOCSIFHWADDR) - the
only way to change it is via the PF with RTM_SETLINK.
This means that if a VF is used for hostdev passthrough, it will have
the admin flag set, and future attempts to use that VF for macvtap
passthrough will fail.
The solution to this problem is to check if the device being used for
macvtap passthrough is actually a VF; if so, we use the netlink
RTM_SETLINK message to the PF to set the VF's mac address instead of
ioctl(SIOCSIFHWADDR) directly to the VF; if not, behavior does not
change from previously.
There are three pieces to making this work:
1) virNetDevMacVLan(Create|Delete)WithVPortProfile() now call
virNetDev(Replace|Restore)NetConfig() rather than
virNetDev(Replace|Restore)MacAddress() (simply passing -1 for VF#
and vlanid).
2) virNetDev(Replace|Restore)NetConfig() check to see if the device is
a VF. If so, they find the PF's name and VF#, allowing them to call
virNetDev(Replace|Restore)VfConfig().
3) To prevent mixups when detaching a macvtap passthrough device that
had been attached while running an older version of libvirt,
virNetDevRestoreVfConfig() is potentially given the preserved name
of the VF, and if the proper statefile for a VF can't be found in
the stateDir (${stateDir}/${pfname}_vf${vfid}),
virNetDevRestoreMacAddress() is called instead (which will look in
the file named ${stateDir}/${vfname}).
This problem has existed in every version of libvirt that has both
macvtap passthrough and interface type='hostdev'. Fortunately people
seem to use one or the other though, so it hasn't caused any real
world problem reports.
This is a simple wrapper around virNetDevBandwidthManipulateFilter() that
will update the desired filter on an interface (usually a network bridge)
with a new MAC address. Although, the MAC address in question usually
refers to some other interface - the one that the filter is constructed
for. Yeah, hard to parse. Thing is, our NATed network has a bridge where
some part of QoS takes place. And vNICs from guests are plugged into
the bridge. However, if a guest decides to change the MAC of its vNIC,
the corresponding qemu process emits an event which we can use to
update the QoS configuration based on the new MAC address.. However,
our QoS hierarchy is currently not notified, therefore it falls apart.
This function (when called in response to the aforementioned event)
will update our QoS hierarchy and duct tape it together again.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Not only this simplifies the code a bit, it prepares the
environment for upcoming patches. The new
virNetDevBandwidthManipulateFilter() function is capable of both
removing a filter and adding a new one. At the same time! Yeah,
this is not currently used anywhere but look at the next commit
where you'll see it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Currently, when constructing traffic shaping rules, the ingress
filter is created without any priority specified on the command
line. This makes kernel to make up one. While this works, it
simplifies things a bit if we provide the filter priority. In
this case, since it's the root filter lets give it the highest
priority of number 1.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The 802.11 interfaces can not be moved by themselves, their Phy has to move too.
If there are other interfaces, they have to move too -- hopefully it's not too
confusing. This is a less-invasive alternative to defining a new hostdev type
for PHYs.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
On rhel-6 is broken gcc that reports this warning:
util/virbuffer.c:500: error: logical '&&' with non-zero constant will
always evaluate as true [-Wlogical-op]
Move the pragma directive before function virBufferEscapeString because
since commit aeb5262e this function uses 'strchr' too.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
ts.tv_nsec was off by a factor of 1000, making timeouts less than a
second in the future often expiring immediately.
Signed-off-by: Michael Chapman <mike@very.puzzling.org>
The comment is describing arguments passed to the function.
However, there's no @ifmac argument. In 955af4d4 it was replaced
with @ifmac_ptr. Unfortunately, the comment wasn't updated.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Add virStringHasControlChars that checks if the string has
any control characters other than \t\r\n,
and virStringStripControlChars that removes them in-place.
Throughout the code, we have several places need to construct a path
somewhere in /sys/class/net/... They are not consistent and nearly
each code piece invents its own way how to do it. So unify this by:
1) use virNetDevSysfsFile() wherever possible
2) At least use common macro SYSFS_NET_DIR declared in virnetdev.h at
the rest of places which can't go with 1)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
If a virAsprintf() within the function fails, we call VIR_FREE()
over @rundir variable and jump onto cleanup label, where it is
freed again. It doesn't hurt, but not make much sense too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When acquiring resource via sanlock fails, we would report it as
VIR_ERR_INTERNAL_ERROR, which is not very friendly to applications using
libvirt. Moreover, the lockd driver would report the same failure as
VIR_ERR_RESOURCE_BUSY, which looks better.
Unfortunately, in sanlock driver we don't really know if acquiring the
resource failed because it was already locked or there was another
reason behind. But the end result is the same and I think using
VIR_ERR_RESOURCE_BUSY reason for all acquire failures is still better
than what we have now.
https://bugzilla.redhat.com/show_bug.cgi?id=1165119
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Commit 49ed6cff is broken on mingw and other non-linux platforms:
CCLD libvirt.la
Cannot export virNetDevSysfsFile: symbol not defined
collect2: error: ld returned 1 exit status
* src/util/virnetdev.c: Provide virNetDevSysfsFile fallback.
Signed-off-by: Eric Blake <eblake@redhat.com>
Found by ./autobuild.sh during a mingw cross-compile:
Commit 8a96e87 was not innocuous - glibc happens to leak the
definition of time() through other headers, so that even without
<sys/select.h>, virrandom.c compiled just fine. But on mingw,
we were not so lucky; <sys/select.h> was important for its side
effect of dragging in <time.h>, and we now have nothing providing
the declaration of time():
../../src/util/virrandom.c: In function 'virRandomOnceInit':
../../src/util/virrandom.c:65:5: error: implicit declaration of function 'time' [-Werror=implicit-function-declaration]
unsigned int seed = time(NULL) ^ getpid();
^
../../src/util/virrandom.c:65:5: error: nested extern declaration of 'time' [-Werror=nested-externs]
Signed-off-by: Eric Blake <eblake@redhat.com>
The variable 'last_processed_hostdev_vf' indicates index of the last
successfully configed vf. When resetvfnetconfig because of failure,
hostdevs[last_processed_hostdev_vf] should also be reset.
Signed-off-by: Huanle Han <hanxueluo@gmail.com>
This patch adds checks for empty bitmaps right after the calls of
virBitmapParse. These only include spots where set API's are called and
where domain's XML is parsed.
Also, it partially reverts commit 983f5a which added a check for
invalid nodeset "0,^0" into virBitmapParse function. This change broke
the logic, as an empty bitmap should not cause an error.
https://bugzilla.redhat.com/show_bug.cgi?id=1210545
Add static virNetDevGetifaddrsAddress to attempt to get the interface
IP address. If getifaddrs is not supported, fall back to
virNetDevGetIPv4AddressIoctl to get the IP address.
This allows IPv6 addresses to be used for <listen type='network>
with device-backed networks.
https://bugzilla.redhat.com/show_bug.cgi?id=1192318
Signed-off-by: Luyao Huang <lhuang@redhat.com>
Signed-off-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Rename it to virNetDevGetIPv4AddressIoctl and make
virNetDevGetIPAddress a wrapper around it, allowing
other ways of getting the address to be implemented,
and still falling back to the old method.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Create a new common API to replace the virCgroupNew{Vcpu|Emulator|IOThread}
API's using an emum to generate the cgroup name
Signed-off-by: John Ferlan <jferlan@redhat.com>
lxc-enter-namespace stopped working on recent kernels (at least 3.19+)
due to /proc/PID/ns/* file descriptors being opened RW. From outside
the namespace these can only be opened RO.
rfc3986 states that the separator in URI path is a single slash.
Multiple slashes may potentially lead to different resources and thus we
should not remove them.
This new internal API checks if given CGroup controller is
available. It is going to be needed later when we need to make a
decision whether pin domain memory onto NUMA nodes using cpuset
CGroup controller or using numa_set_membind().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The close callbacks hash are keyed by a UUID-string, but
virCloseCallbacksRun was attempting to remove them by raw UUID. This
patch ensures the callback entries are removed by UUID-string as well.
This bug caused problems when guest migrations were abnormally aborted:
# timeout --signal KILL 1 \
virsh migrate example qemu+tls://remote/system \
--verbose --compressed --live --auto-converge \
--abort-on-error --unsafe --persistent \
--undefinesource --copy-storage-all --xml example.xml
Killed
# virsh migrate example qemu+tls://remote/system \
--verbose --compressed --live --auto-converge \
--abort-on-error --unsafe --persistent \
--undefinesource --copy-storage-all --xml example.xml
error: Requested operation is not valid: domain 'example' is not being migrated
Signed-off-by: Michael Chapman <mike@very.puzzling.org>
The nodedev-detach can report the name of the domain using the device
just the way nodedev-reattach does it.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Dependant is flagged as wrong in US dictionary (only valid in UK
dictionary, and even then, it has only the financial sense and not the
inter-relatedness sense that we are more prone to be wanting throughout
code).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
'virPCIDeviceList' is actually an array. Removing one element makes the
rest of the element move.
Use while loop, increase index only when not virPCIDeviceListDel(pcidevs, dev)
Signed-off-by: Huanle Han <hanxueluo@gmail.com>
In the order of appearance:
* MAX_LISTEN - never used
added by 23ad665c (qemud) and addec57 (lock daemon)
* NEXT_FREE_CLASS_ID - never used, added by 07d1b6b
* virLockError - never used, added by eb8268a4
* OPENVZ_MAX_ARG, CMDBUF_LEN, CMDOP_LEN
unused since the removal of ADD_ARG_LIT in d8b31306
* QEMU_NB_PER_CPU_STAT_PARAM - unused since 897808e
* QEMU_CMD_PROMPT, QEMU_PASSWD_PROMPT - unused since 1dc10a7
* TEST_MODEL_WORDSIZE - unused since c25c18f7
* TEMPDIR - never used, added by 714bef5
* NSIG - workaround around old headers
added by commit 60ed1d2
unused since virExec was moved by commit 02e8691
* DO_TEST_PARSE - never used, added by 9afa006
* DIFF_MSEC, GETTIMEOFDAY - unused since eee6eb6
Throughout our code, the virCgroupController enum is used in two ways.
First as an index to an array of cgroup controllers:
struct virCgroup {
char *path;
struct virCgroupController controllers[VIR_CGROUP_CONTROLLER_LAST];
};
Second way is that when calling virCgroupNew() a bitmask of the enum
items can be passed to selectively detect only some controllers. For
instance:
int
virCgroupNewVcpu(virCgroupPtr domain,
int vcpuid,
bool create,
virCgroupPtr *group)
{
...
controllers = ((1 << VIR_CGROUP_CONTROLLER_CPU) |
(1 << VIR_CGROUP_CONTROLLER_CPUACCT) |
(1 << VIR_CGROUP_CONTROLLER_CPUSET));
if (virCgroupNew(-1, name, domain, controllers, group) < 0)
goto cleanup;
}
Even though it's highly unlikely that so many new controllers will be
invented so that we would overflow when constructing the bitmask, it
doesn't hurt to check at compile time either.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When creating new internal representation of cgroups, all passed
arguments are logged. Well, except for two: pid and pointer for
return value. Lets log them too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The function has no argument named @name rather than @path
instead. The comment is, however, referring to @name while it
should have been referring to @path really.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Commit id '2dbfa716' exposed virCgroupDetectMountsFromFile, but did not
add the corresponding entry in the "#else /* !VIR_CGROUP_SUPPORTED */"
section of the module.
Commit id 'ba1dfc5' added virCgroupSetCpusetMemoryMigrate and
virCgroupGetCpusetMemoryMigrate, but did not add the corresponding
entry points into the "#else /* !VIR_CGROUP_SUPPORTED */" section
Commint 0473b45cc introduced new function virNetlinkDelLink, but in
it's counterpart for non-linux platform there should be ATTRIBUTE_UNUSED
instead of ATTRIBUTE_UNSUPPORTED.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Just as it is possible to delete a bridge device with the netlink
RTM_DELLINK message, one can be created with the RTM_NEWLINK
message. Because of differences in the format of the message, it's not
as straightforward as with virNetlinkDelLink() to create a single
utility function that can be used to create any type of interface, so
the new netlink version of virNetDevBridgeCreate() does its own
construction of the netlink message and calls virNetlinkCommand()
itself.
This doesn't provide any extra functionality, just provides symmetry
with the previous commit.
NB: We *could* alter the API of virNetDevBridgeCreate() to take a MAC
address, and directly program that mac address into the bridge (by
adding an IFLA_ADDRESS attribute, as is done in
virNetDevMacVLanCreate()) rather than separately creating the "dummy
tap" (e.g. virbr0-nic) to maintain a fixed mac address on the bridge,
but the commit history of virnetdevbridge.c shows that the presence of
this dummy tap is essential in some older versions of the kernel
(between 2.6.39 and 3.1 or 3.2, possibly?) to proper operation of IPv6
DAD, and I don't want to take the chance of breaking something that I
don't have the time/setup to test (my RHEL6 box is at kernel
2.6.32-544, and the next lowest kernel I have is 3.17)
https://bugzilla.redhat.com/show_bug.cgi?id=1125755
reported that a stray bridge device was left on the system when a
libvirt network failed to start due to an illegal iptables rule caused
by bad config. Apparently the reason this was happening was that
NetworkManager was noticing immediately when the bridge device was
created and automatically setting it IFF_UP. libvirt would then try to
setup the iptables rules, get an error back, and since libvirt had
never IFF_UPed the bridge, it didn't expect that it needed to set it
~IFF_UP before deleting it during the cleanup process. But the
ioctl(SIOCBRDELBR) ioctl will fail to delete a bridge if it is IFF_UP.
Since that bug was reported, NetworkManager has gotten a bit more
polite in this respect, but just in case something similar happens in
the future, this patch switches to using the netlink RTM_DELLINK
message to delete the bridge - unlike SIOCBRDELBR, it will delete the
requested bridge no matter what the setting of IFF_UP.
These two functions are identical, so no sense in having the
duplication. I resisted the temptation to replace calls to
virNetDevMacVLanDelete() with calls to virNetlinkDelLink() just in
case some mythical future platform has macvtap devices that aren't
managed with netlink (or in case we some day need to do more than just
tell the kernel to delete the device).
libvirt has always used the netlink RTM_DELLINK message to delete
macvtap/macvlan devices, but it can actually be used to delete other
types of network devices, such as bonds and bridges. This patch makes
virNetDevMacVLanDelete() available as a generic function so it can
intelligibly be called to delete these other types of interfaces.
The current auto-indentation buffer code applies indentation only on
complete strings. To allow adding a string containing newlines and
having it properly indented this patch adds virBufferAddStr.
Don't unref the old identity unless we set the new one correctly and
unref the new one on failure to set it so that we don't leak any
references or use invalid pointers.
Every thread created as a worker thread within a pool gets a name
according to virThreadPoolJobFunc name.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Automatically assign a job to every thread created by virThreadCreate.
The name of the virThreadFunc function passed to virThreadCreate is used
as the job or worker name in case no name is explicitly passed.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Each thread can use a thread local variable to keep the name of a job
which is currently running in the job.
The virThreadJobSetWorker API is supposed to be called once by any
thread which is used as a worker, i.e., it is waiting in a pool, woken
up to do a job, and returned back to the pool.
The virThreadJobSet/virThreadJobClear APIs are to be called at the
beginning/end of each job.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Wikipedia's list of common misspellings [1] has a machine-readable
version. This patch fixes those misspellings mentioned in the list
which don't have multiple right variants (as e.g. "accension", which can
be both "accession" and "ascension"), such misspellings are left
untouched. The list of changes was manually re-checked for false
positives.
[1] https://en.wikipedia.org/wiki/Wikipedia:Lists_of_common_misspellings/For_machines
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Investigation of a problem with creating passthrough macvtap devices
(https://bugzilla.redhat.com/show_bug.cgi?id=1185501) has shown that
this slightly more verbose failure message is useful. In particular,
the mac address can be used to determine the domain. You could also
figure this out by looking at preceding messages in a debug log, but
this gets it in a single place.
Use the utilities introduced in the previous patches so the qemu
driver is able to create tap devices that are bound (and unbound
on domain destroyal) to Midonet virtual ports.
Signed-off-by: Antoni Segura Puimedon <toni+libvirt@midokura.com>
Midonet is an opensource virtual networking that over lays the IP
network between hypervisors. Currently, such networks can be made
with the openvswitch virtualport type.
This patch, defines the schema and documentation that will serve
as basis for the follow up patches that will add support to libvirt
for using Midonet virtual ports for its interfaces. The schema
definition requires that the port profile expresses its interfaceid
as part of the port profile. For that reason, this is part of the
patch too.
Signed-off-by: Antoni Segura Puimedon <toni+libvirt@midokura.com>
Adds the port type definitions and methods that will be used to bind
interfaces to the Midonet virtual ports.
virtnetdevmidonet.c adds the way to bind and unbind the ports by
calling into the Midonet Host Agent control command line (installed
with the midolman package).
Signed-off-by: Antoni Segura Puimedon <toni+libvirt@midokura.com>
If the storage device type is parsed as network our parser still allows
it to omit the <source> element. The empty drive check would not trigger
on such device as it expects that every network storage source is valid.
Use VIR_STORAGE_NET_PROTOCOL_NONE as a marker that the storage source is
empty.
Valgrind complained:
==3770== Syscall param ioctl(SIOCETHTOOL) points to uninitialised byte(s)
==3770== at 0x919D407: ioctl (syscall-template.S:81)
==3770== by 0x530FE7E: rpl_ioctl (ioctl.c:42)
==3770== by 0x50CB433: virNetDevFeatureAvailable (virnetdev.c:2764)
==3770== by 0x50CB6A7: virNetDevGetFeatures (virnetdev.c:2830)
==3770== by 0x1F0E5347: udevProcessNetworkInterface (node_device_udev.c:722)
==3770== by 0x1F0E689F: udevGetDeviceDetails (node_device_udev.c:1300)
==3770== by 0x1F0E6E06: udevAddOneDevice (node_device_udev.c:1422)
==3770== by 0x1F0E6FB8: udevProcessDeviceListEntry (node_device_udev.c:1464)
==3770== by 0x1F0E70CF: udevEnumerateDevices (node_device_udev.c:1494)
==3770== by 0x1F0E7BB4: nodeStateInitialize (node_device_udev.c:1806)
==3770== by 0x51B4303: virStateInitialize (libvirt.c:777)
==3770== by 0x11DEE7: daemonRunStateInit (libvirtd.c:906)
==3770== Address 0x228e38d4 is on thread 12's stack
==3770== in frame #2, created by virNetDevFeatureAvailable (virnetdev.c:2750)
* src/util/virnetdev.c (virNetDevFeatureAvailable): Initialize all
bytes of ifr.
Signed-off-by: Eric Blake <eblake@redhat.com>
Introduced by f6a2f97e
Problem Description:
After multiple times of migrating a domain, which has an ovs interface with no portData set,
with non-shared disk, nbd ports got overflowed.
The steps to reproduce the problem:
1 define and start a domain with its network configured as:
<interface type='bridge'>
<source bridge='br0'/>
<virtualport type='openvswitch'>
</virtualport>
<model type='virtio'/>
<driver name='vhost' queues='4'/>
</interface>
2 do not set the network's portData.
3 migrate(ToURI2) it with flag 91(1011011), which means:
VIR_MIGRATE_LIVE
VIR_MIGRATE_PEER2PEER
VIR_MIGRATE_PERSIST_DEST
VIR_MIGRATE_UNDEFINE_SOURCE
VIR_MIGRATE_NON_SHARED_DISK
4 migrate success, but we got an error log in libvirtd.log:
error : virCommandWait:2423 : internal error: Child process (ovs-vsctl --timeout=5 get Interface
vnet1 external_ids:PortData) unexpected exit status 1: ovs-vsctl: no key "PortData" in Interface
record "vnet1" column external_ids
5 migrate it back, migrate it , migrate it back, .......
6 nbd port got overflowed.
The reasons for the problem is :
1 virNetDevOpenvswitchGetMigrateData() takes it as wrong if no portData is available for the ovs
interface of a domain. (We think it's not appropriate, as portData is just OPTIONAL)
2 in func qemuMigrationBakeCookie(), it fails in qemuMigrationCookieAddNetwork(), and returns with -1.
qemuMigrationCookieAddNBD() is not called thereafter, and mig->nbd is still NULL.
3 However, qemuMigrationRun() just *WARN* if qemuMigrationBakeCookie() fails, migration still successes.
cookie is NULL, it's not baked on the src side.
4 On the destination side, it would alloc a port first and then free the nbd port in COOKIE.
But the cookie is NULL due to qemuMigrationCookieAddNetwork() failure at src side. thus the nbd port
is not freed.
In this patch, we add "--if-exists" option to make ovs-vsctl not raise error if there's no portData available.
Further more, because portData may be NULL in the cookie at the dest side, check it before setting portData.
Signed-off-by: Zhou Yimin <zhouyimin@huawei.com>
Signed-off-by: Zhang Bo <oscar.zhangbo@huawei.com>
A helper that never returns an error and treats bits out of bitmap range
as false.
Use it everywhere we use ignore_value on virBitmapGetBit, or loop over
the bitmap size.
Commit c9027d8f added a detection of NIC HW features, but some of them
are not available in old kernel. Very old kernels lack enum
ethtool_flags and even if this enum is present, not all values are
available for all kernels. To be sure that we have everything in kernel
that we need, we must check for existence of most of that flags, because
only few of them were defined at first.
Also to successfully build libvirt with older kernel we need to include
<linux/types.h> before <linux/ethtool.h> to have __u32 and friends
defined.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
There was a mess in the way how we store unlimited value for memory
limits and how we handled values provided by user. Internally there
were two possible ways how to store unlimited value: as 0 value or as
VIR_DOMAIN_MEMORY_PARAM_UNLIMITED. Because we chose to store memory
limits as unsigned long long, we cannot use -1 to represent unlimited.
It's much easier for us to say that everything greater than
VIR_DOMAIN_MEMORY_PARAM_UNLIMITED means unlimited and leave 0 as valid
value despite that it makes no sense to set limit to 0.
Remove unnecessary function virCompareLimitUlong. The update of test
is to prevent the 0 to be miss-used as unlimited in future.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1146539
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
The first one is to truncate the memory limit to
VIR_DOMAIN_MEMORY_PARAM_UNLIMITED if the value is greater and the second
one is to decide whether the memory limit is set or not, unlimited means
that it's not set.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Implement virCommandPassFDGetFDIndex to determine the index a given
file descriptor will have when passed to the child process.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Adding functionality to libvirt that will allow it
query the ethtool interface for the availability
of certain NIC HW offload features
Here is an example of the feature XML definition:
<device>
<name>net_eth4_90_e2_ba_5e_a5_45</name>
<path>/sys/devices/pci0000:00/0000:00:03.0/0000:08:00.1/net/eth4</path>
<parent>pci_0000_08_00_1</parent>
<capability type='net'>
<interface>eth4</interface>
<address>90:e2:ba:5e:a5:45</address>
<link speed='10000' state='up'/>
<feature name='rx'/>
<feature name='tx'/>
<feature name='sg'/>
<feature name='tso'/>
<feature name='gso'/>
<feature name='gro'/>
<feature name='rxvlan'/>
<feature name='txvlan'/>
<feature name='rxhash'/>
<capability type='80203'/>
</capability>
</device>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
The gluster volume name extraction code was copied from the XML parser
without changing the VIR_ERR_XML_ERROR error code. Use
VIR_ERR_CONFIG_UNSUPPORTED instead.
Similar to commit fdb80ed4f6 libvirtd
would crash if a gluster URI without path would be used in the backing
chain of a volume. The crash happens in the gluster specific part of the
parser that extracts the gluster volume name from the path.
Fix the crash by checking that the PATH is NULL.
This patch does not contain a test case as it's not possible to test it
with the current infrastructure as the test suite would attempt to
contact the gluster server in the URI. I'm working on the test suite
addition but that will be post-release material.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1196528
Previously this function relied on having ATTRIBUTE_NONNULL(1) in its
prototype rather than explicitly checking for a null
ifname. Unfortunately, ATTRIBUTE_NONNULL is just a hint to the
optimizer and code analyzers like Coverity, it doesn't actually check
anything at execution time, so the result was possible warnings from
Coverity, along with the possibility of null dereferences when ifname
wasn't available.
This patch removes the ATTRIBUTE_NONNULL from the prototype, and
checks ifname inside the function, logging an error if it's NULL (once
we've determined that the user really is trying to set a bandwidth).
libvirt was unconditionally calling virNetDevBandwidthClear() for
every interface (and network bridge) of a type that supported
bandwidth, whether it actually had anything set or not. This doesn't
hurt anything (unless ifname == NULL!), but is wasteful.
This patch makes sure that all calls to virNetDevBandwidthClear() are
qualified by checking that the interface really had some bandwidth
setup done, and checks for a null ifname inside
virNetDevBandwidthClear(), silently returning success if it is null
(as well as removing the ATTRIBUTE_NONNULL from that function's
prototype, since we can't guarantee that it is never null,
e.g. sometimes a type='ethernet' interface has no ifname as it is
provided on the fly by qemu).
This API joins the following two lines:
char *s = virBufferContentAndReset(buf1);
virBufferAdd(buf2, s, -1);
into one:
virBufferAddBuffer(buf2, buf1);
With one exception: there's no re-indentation applied to @buf1.
The idea is, that in general both can have different indentation
(like the test I'm adding proves)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Not all files we want to find using virFileFindResource{,Full} are
generated when libvirt is built, some of them (such as RNG schemas) are
distributed with sources. The current API was not able to find source
files if libvirt was built in VPATH.
Both RNG schemas and cpu_map.xml are distributed in source tarball.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Commit b6a2828e introduced new functions to set process scheduler. There
is a small typo in ELSE path for systems where scheduler is not
available.
Also some of the definitions were introduced later in kernel. For
example RHEL-5 is running on kernel 2.6.18, but SCHED_IDLE was introduces
in 2.6.23 [1] and SCHED_BATCH in 2.6.16 [1]. We should not count only on
existence of function sched_setscheduler(), we must also check for
existence of used macros as they might not be defined.
[1] see 'man 7 sched'
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
This function uses sched_setscheduler() function so it works with
processes and threads as well (even threads not created by us, which is
what we'll need in the future).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Rather than have a dummy waitpid loop and return of the failure status
from recvfd, adjust the logic to save the recvfd error & fd and then
in priority order:
- if waitpid failed, use that errno value
- waitpid succeeded, but if the child exited abnormally, report failure
(use EACCES to report as return failure, since either EACCES or EPERM is
what caused us to fall into the fork+setuid path)
- waitpid succeeded, but if the child reported non-zero status, report
failure (use the errno value that the child encoded into exit status)
- waitpid succeeded, but if recvfd failed, report recvfd_errno
- waitpid and recvfd succeeded, use the fd
NOTE: Original logic to retry the open and force owner mode was
"documented" as only being attempted if we had already tried opening
with the fork+setuid, but checked flags vs. VIR_FILE_OPEN_NOFORK which
is counter to how we would get to that point. So that code was removed.
It is often helpful to know which version of libvirt and QEMU
was present when a guest was first launched. Ensure this info
is written into the QEMU log file for each guest.
If a storage file would be backed with a NBD device without path
(nbd://localhost) libvirt would crash when parsing the backing path for
the disk as the URI structure's path element is NULL in such case but
the NBD parser would access it shamelessly.
Commit b38da584 introduced two new functions to get a page size but it
won't work on Windows. We should take care of this.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Commit e562a61a introduced new function to get/set interface state but
there was misuse of ATTRIBUTE_NONNULL on non-pointer attributes and also
we need to wrap that functions by #ifdef to not break mingw build.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Some code paths have special logic depending on the page size
reported by sysconf, which in turn affects the test results.
We must mock this so tests always have a consistent page size.
e562a61a07 added these two new helper functions and only used them
within virnetdev.c, but declared them in the .h file. If some
currently unsupported interface flags need to be accessed in the
future, it will make more sense to write the appropriate higher level
function rather than require us to artificially define IFF_* on some
mythical platform that doesn't have SIOC[SG]IFFLAGS (and therefore
doesn't have IFF_*) just so we can call virNetDevSetIFFFlags() to
return an error.
To help someone in not going down the wrong road, this patch makes the
two helper functions static, hopefully making it less likely that
someone will want to use them outside of virnetdev.c.
This helper eases iterating all key=value pairs stored in a JSON
object. Usually we pick only certain known keys from a JSON object, but
this will allow to walk complete objects and have the callback act on
those.
To be able to easily represent nodesets and other data stored in
virBitmaps in libvirt, this patch introduces a set of helpers that allow
to convert the bitmap to and from JSON value objects.
This patch provides the utility functions needed to synchronize
the rxfilter changes made to a guest domain with the corresponding
macvtap devices on the host:
* Get/set PROMISC flag
* Get/set ALLMULTI, MULTICAST
Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
A gnulib change (commit id 'beae0bdc') causes ENOTCONN to be returned
from recvfd which causes us to fall into the throwaway waitpid() call
and return ENOTCONN to the caller, this then gets displayed during
a 'virsh save' when using a root squashed NFS environment that's trying
to save the file as something other than root:root.
This patch will add the additional check for ENOTCONN to force the code
into the waitpid loop looking for the actual status from the _exit()'d
child fork.
Signed-off-by: John Ferlan <jferlan@redhat.com>
When compiling without WITH_MACVTAP, we can get:
'unsupported flags (0x1) in function
virNetDevMacVLanCreateWithVPortProfile'
on an attempt to start a domain.
Remove the flag check to reach the more helpful error:
Cannot create macvlan devices on this platform
https://bugzilla.redhat.com/show_bug.cgi?id=1186928
In many cases where we invoke virSystemdTerminateMachine the
process(es) will have already gone away on their own accord.
In these cases we log an error message that the machine does
not exist. We should catch this particular error and simply
ignore it, so we don't pollute the logs.
Coverity reports that my commit af1c98e introduced
two memory leaks:
the cpumap if ncpus == 0 in virCgroupGetPercpuStats
and the params array in the test of the function.
The virDBusMethodCall method has a DBusError as one of its
parameters. If the caller wants to pass a non-NULL value
for this, it immediately makes the calling code require
DBus at build time. This has led to breakage of non-DBus
builds several times. It is desirable that only the virdbus.c
file should need WITH_DBUS conditionals, so we must ideally
remove the DBusError parameter from the method.
We can't simply raise a libvirt error, since the whole point
of this parameter is to give the callers a way to check if
the error is one they want to ignore, without having the logs
polluted with an error message. So, we add a virErrorPtr
parameter which the caller can then either ignore or raise
using the new virReportErrorObject method.
This new method is distinct from virSetError in that it
ensures the logging hooks are run.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Per-cpu stats are only shown for present CPUs in the cgroups,
but we were only parsing the largest CPU number from
/sys/devices/system/cpu/present and looking for stats even for
non-present CPUs.
This resulted in:
internal error: cpuacct parse error
Every dtrace/systemd probe also include a libvirt log message.
These are logged at level DEBUG currently, which means if you
want to see all probes they are drowned by the rest of the
DEBUG messages. Since we don't really use the INFO log level
for much, it seems reasonable to suggest we log all probes at
level INFO.
When debugging libvirt it is helpful to set probes around RPC
calls. We already have probes for libvirt's native RPC layer,
so it makes sense to add them for the DBus RPC layer too.
systemd-machined introduced a new method CreateMachineWithNetwork
that obsoletes CreateMachine. It expects to be given a list of
VETH/TAP device indexes for the host side device(s) associated
with a container/machine.
This falls back to the old CreateMachine method when the new
one is not supported.
The virsh start <domain> fails with qemu error when the hostdevices of the
same iommu group are used actively by other vms. It is not clear which
hostdev from the same iommu group is used by any of the running guests.
User has to go through every guest xml to figure out who is using the
hostdev of same iommu group.
Solution:
Iterate the iommu group of the hostdev and error our neatly in case a
device in the same iommu group is busy. Reattach code also does the same
kind of check, remove duplicate code as well.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Basically a getter function which is implemented for accessing the
address fields in virPCIDevice.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Found this one by inspection... The API claims to "own" the input
value even in the case of error. However, in the initial entry
to the API if the value exists, was STRING, but without a str value
it just returned without freeing the 'value' which it claims to now
own. So I added the virConfFreeValue() call in order to resolve.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Remove the resize flag and use the same code path for all callers.
This flag was added by commit 18f0316 to allow virStorageFileResize
use 'safezero' while preserving the behavior.
Explicitly return -2 when a fallback to a different method should
be done, to make the code path more obvious.
Fail immediately when ftruncate fails in the mmap method,
as we did before commit 18f0316.
Commit 4dc04d3a added virNetlinkGetErrorCode, but forgot to provide
a fallback, which kills the build on mingw (among others):
CCLD libvirt.la
Cannot export virNetlinkGetErrorCode: symbol not defined
collect2: error: ld returned 1 exit status
* src/util/virnetlink.c (virNetlinkGetErrorCode): Provide fallback.
Signed-off-by: Eric Blake <eblake@redhat.com>
Add a default implementation of virNetDevSetIPv4Address using netlink
and libnl. This avoids requiring /usr/sbin/ip or /usr/sbin/ifconfig
external binaries.
Some of the nwfilter tests are now failing since --concurrent shows
up in the ebtables command. To avoid this, implement a function
preventing the probing for lock support in the eb/iptables tools
and use it in the tests.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
There is one problem that causes various errors in the daemon. When
domain is waiting for a job, it is unlocked while waiting on the
condition. However, if that domain is for example transient and being
removed in another API (e.g. cancelling incoming migration), it get's
unref'd. If the first call, that was waiting, fails to get the job, it
unref's the domain object, and because it was the last reference, it
causes clearing of the whole domain object. However, when finishing the
call, the domain must be unlocked, but there is no way for the API to
know whether it was cleaned or not (unless there is some ugly temporary
variable, but let's scratch that).
The root cause is that our APIs don't ref the objects they are using and
all use the implicit reference that the object has when it is in the
domain list. That reference can be removed when the API is waiting for
a job. And because each domain doesn't do its ref'ing, it results in
the ugly checking of the return value of virObjectUnref() that we have
everywhere.
This patch changes qemuDomObjFromDomain() to ref the domain (using
virDomainObjListFindByUUIDRef()) and adds qemuDomObjEndAPI() which
should be the only function in which the return value of
virObjectUnref() is checked. This makes all reference counting
deterministic and makes the code a bit clearer.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Commit 1a80b97d, which added the virCgroupHasEmptyTasks() function
forgot that the parameter @cgroup may be NULL and did not check that.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
The code assumes that def->vcpus == nvcpupids, so when we setup
fake CPU pids for old QEMU with nvcpupids == 1, we cause the
later code to read off the end of the array. This has fun results
like sche_setaffinity(0, ...) which changes libvirtd's own CPU
affinity, or even better sched_setaffinity($RANDOM, ...) which
changes the affinity of a random OS process.
On a system with 160 CPUs the /proc/cpuinfo size grows beyond
the currently set limit of 10KB causing an internal error.
This patch increases the buffer size to 1MB.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Right now, grabbing blockinfo always calls stat on the disk, then
opens the image to determine the capacity, using a throw-away
virStorageSourcePtr. This has a couple of drawbacks:
1. We are calling stat and opening a file on every invocation of
the API. However, there are cases where the stats should NOT be
changing between successive calls (if a domain is running, no
one should be changing the physical size of a block device or raw
image behind our backs; capacity of read-only files should not
be changing; and we are the gateway to the block-resize command
to know when the capacity of read-write files should be changing).
True, we still have to use stat in some cases (a sparse raw file
changes allocation if it is read-write and the amount of holes is
changing, and a read-write qcow2 image stored in a file changes
physical size if it was not fully pre-allocated). But for
read-only images, even this should be something we can remember
from the previous time, rather than repeating every call.
2. We want to enhance the power of virDomainListGetStats, by
sharing code. But we already have a virStorageSourcePtr for
each disk, and it would be easier to reuse the common structure
than to have to worry about the one-off virDomainBlockInfoPtr.
While this patch does not optimize reuse of information in point
1, it does get us closer to being able to do so; by updating a
structure that survives between consecutive calls.
* src/util/virstoragefile.h (_virStorageSource): Add physical, to
mirror virDomainBlockInfo; rearrange fields to match public struct.
(virStorageSourceCopy): Copy the new field.
* src/qemu/qemu_driver.c (qemuDomainGetBlockInfo): Store into
storage source, then copy to block info.
Signed-off-by: Eric Blake <eblake@redhat.com>
When any of the functions modified in commit 214c687b took false branch,
the function itself used none of its parameters resulting in "unused
parameter" error. Rewriting these functions to the stubs we use
elsewhere should fix the problem.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Currently virStorageFileResize() function uses build conditionals to
choose either the posix_fallocate() or syscall(SYS_fallocate) with no
fallback in order to preallocate the space in the newly resized file.
Since the safezero code has a similar set of conditionals modify the
resize and safezero code in order to allow the resize logic to make use
of safezero to unify the look/feel of the code paths.
Add a new boolean (resize) to safezero() to make the optional decision
whether to try syscall(SYS_fallocate) if the posix_fallocate fails because
HAVE_POSIX_FALLOCATE is not defined (eg, return -1 and errno == 0).
Create a local safezero_sys_fallocate in order to handle the resize
code paths that support that. If not present, the set errno = ENOSYS
in order to allow the caller to handle the failure scenarios.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Currently build conditionals decide which of two safezero() functions
should be built - either the posix_fallocate() or mmap() with a fallback
to a slower safewrite() algorithm in order to preallocate space in a raw file.
This patch will refactor safezero to utilize static functions for either
posix_fallocate or mmap/safewrite. The build conditional still exist, but
are only for shorter sections of code.
The posix_fallocate path will make use of the ret/errno setting to contain
the logic for safezero to decide whether it needs to fallback to other
algorithms. A return of -1 with errno not changed will indicate the conditional
is not present; otherwise, a return of -1 with errno change indicates the
call was made and it failed (no functional difference to current algorithm).
The mmap/safewrite option changes only slightly to handle the ftruncate
failure for mmap. That is, previously if the ftruncate failed, there was
no fallback to the slow safewrite option.
Signed-off-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1160995
In our config files users are expected to pass several integer values
for different configuration knobs. However, majority of them expect a
nonnegative number and only a few of them accept a negative number too
(notably keepalive_interval in libvirtd.conf).
Therefore, a new type to config value is introduced: VIR_CONF_ULONG
that is set whenever an integer is positive or zero. With this
approach knobs accepting VIR_CONF_LONG should accept VIR_CONF_ULONG
too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
There's no need for condition of the following form:
if (str && STREQ(str, dst))
since we have STREQ_NULLABLE macro that handles NULL cases.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When calling virCgroupAllowAllDevices we get these invalid entries
in the device cgroup config.
b -1:-1 rw
c -1:-1 rw
Check for positive values before outputting the major and minor to
avoid that.
Currently, MAC registration occurs during device creation, which is
early enough that, during live migration, you end up with duplicate
MAC addresses on still-running source and target devices, even though
the target device isn't actually being used yet.
This patch proposes to defer MAC registration until right before
the guest can actually use the device -- In other words, right
before starting guest CPUs.
Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
Signed-off-by: Laine Stump <laine@laine.org>
https://bugzilla.redhat.com/show_bug.cgi?id=1172015
The refactoring done as part of commit id '59446096' caused a regression
for the multi initiator IQN commit '6aabcb5b' because the sendtargets was
not done on/for the initiator IQN prior to login (or trying to disable
autologin)
Prior to that commit, the paths were essentially
virStorageBackendISCSIStartPool
virStorageBackendISCSILogin
virStorageBackendISCSIConnection
if initiatoriqn
virStorageBackendCreateIfaceIQN
Issue sendtargets
Perform --login
else
Issue sendtargets
Perform --login
After that commit:
virStorageBackendISCSIStartPool
Issue sendtargets
Call virStorageBackendISCSIConnection
If initiatoriqn
virStorageBackendCreateIfaceIQN
Perform --login
else
Perform --login
So for non initiator IQN paths, nothing changed. For the initiator path,
the --login fails as does any attempts to change autologin via "--op update
--name node.startup --value manual".
These two functions use netlink RTM_NEWNEIGH and RTM_DELNEIGH messages
to add and delete entries from a bridge's fdb. The bridge itself is
not referenced in the arguments to the functions, only the name of the
device that is attached to the bridge (since a device can only be
attached to one bridge at a time, and must be attached for this
function to make sense, the kernel easily infers which bridge's fdb is
being modified by looking at the device name/index).
When user tries to insert element metadata providing a namespace
declaration as well, currently we insert the element without any validation
check for XML prefix (if provided). The next VM start would then
fail with parse error. This patch fixes this issue by adding a call to
xmlValidateNCName function to check for illegal characters in the
prefix.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1143921
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.
The helper checks whether a string contains only whitespace or is NULL.
This will be helpful to skip cases where a user string is optional, but
may be provided empty with the same meaning.
Newer versions of Debian use '/run/initctl' instead of '/dev/initctl'.
This patch updates the code to search for the FIFO from a list of
well-known locations.
Build with clang fails with:
CC util/libvirt_util_la-virsocketaddr.lo
util/virsocketaddr.c:904:17: error: cast from 'struct sockaddr *' to
'struct sockaddr_in *' increases required alignment from 1 to 4
[-Werror,-Wcast-align]
inet4 = (struct sockaddr_in*) res->ai_addr;
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
util/virsocketaddr.c:909:17: error: cast from 'struct sockaddr *' to
'struct sockaddr_in6 *' increases required alignment from 1 to 4
[-Werror,-Wcast-align]
inet6 = (struct sockaddr_in6*) res->ai_addr;
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 errors generated.
Fix that by replacing virSocketAddrParseInternal() call with
virSocketAddrParse() in the virSocketAddrIsNumericLocalhost() function.
virSocketAddrParse stores an address in virSocketAddr.
virSocketAddr uses a union to store an address, so it doesn't
need casting.
The JSON structure constructor has an option to add JSON arrays to the
constructed object. The description is inaccurate as it can add any json
object even a dict. Change the docs to cover this option and reject
adding NULL objects.
Our qemu monitor code has a converter from key-value pairs to a json
value object. I want to re-use the code later and having it part of the
monitor command generator is inflexible. Split it out into a separate
helper.
if specifying migration_host to an Ipv6 address without brackets,
it was resolved to an incorrect address, such as:
tcp:2001:0DB8::1428:4444,
but the correct address should be:
tcp:[2001:0DB8::1428]:4444
so we should add brackets when parsing it.
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
The actual origin of this so called typo are two commits. The first one
was commit 72f8a7f that came up with the following condition:
if ((i == 8) & (flags & VIR_QEMU_PROCESS_KILL_FORCE))
Fortunately this succeeded thanks to bool being (int)1 and
VIR_QEMU_PROCESS_KILL_FORCE having the value of 1 << 0. The check was
then moved and altered in 8fd3823117 to
current state:
if ((i == 50) & force)
that will work again (both sides of '&' being booleans), but since this
was missed so many times, it may pose a problem in the future in case it
gets copy-pasted again.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
This same structure will be used to retrieve RX filter info for
interfaces on the host via netlink messages, and RX filter info for
interfaces on the guest via the qemu "query-rx-filter" command.
Since commit 8eb55d782a2b9afacc7938694891cc6fad7b42a5 libxml2 removes
two slashes from the URI when there is no server part. This is fixed
with beb7281055dbf0ed4d041022a67c6c5cfd126f25, but only if the calling
application calls xmlSaveUri() on URI that xmlURIParse() parsed. And
that is not the case in virURIFormat(). virURIFormat() accepts
virURIPtr that can be created without parsing it and we do that when we
format network storage paths for gluster for example. Even though
virStorageSourceParseBackingURI() uses virURIParse(), it throws that data
structure right away.
Since we want to format URIs as URIs and not absolute URIs or opaque
URIs (see RFC 3986), we can specify that with a special hack thanks to
commit beb7281055dbf0ed4d041022a67c6c5cfd126f25, by setting port to -1.
This fixes qemuxml2argvtest test where the disk-drive-network-gluster
case was failing.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
A cygwin build of 1.2.9 fails with:
util/virprocess.c:87:27: fatal error: sys/syscall.h: No such file or directory
# include <sys/syscall.h>
But in reality, the ONLY user of setns() is lxc, which is Linux-only.
It's easiest to just limit the setns workarounds to Linux.
* src/util/virprocess.c (setns): Limit definition to Linux.
Signed-off-by: Eric Blake <eblake@redhat.com>
If we don't properly clean up all processes in the
machine-<vmname>.scope systemd won't remove the cgroup and subsequent vm
starts fail with
'CreateMachine: File exists'
Additional processes can e.g. be added via
echo $PID > /sys/fs/cgroup/systemd/machine.slice/machine-${VMNAME}.scope/tasks
but there are other cases like
http://bugs.debian.org/761521
Invoke TerminateMachine to be on the safe side since systemd tracks the
cgroup anyway. This is a noop if all processes have terminated already.
The commit 1b854c76 introduced a new function 'virPolkitCheckAuth' and
in the #else section when you don't have polkit all attributes should be
follwed by ATTRIBUTE_UNUSED.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Spawning the pkcheck program every time a permission check is
required is hugely expensive on CPU. The pkcheck program is just
a dumb wrapper for the DBus API, so rewrite the code to use the
DBus API directly. This also simplifies error handling a bit.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Update virNetServerClientCreateIdentity and virIdentityGetSystem
to use the new typesafe APIs for setting identity attributes
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
There are now two places in libvirt which use polkit. Currently
they use pkexec, which is set to be replaced by direct DBus API
calls. Add a common API which they will both be able to use for
this purpose.
No tests are added at this time, since the impl will be gutted
in favour of a DBus API call shortly.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Clean up all _virDomainInterfaceStats.
Signed-off-by: Wang Yufei <james.wangyufei@huawei.com>
Signed-off-by: Wang Rui <moon.wangrui@huawei.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
virStorageSourceInitChainElement initializes a new storage chain element
for use as a new disk source. If the new element doesn't contain the
driver name, copy it from the old source.
This fixes issue where a disk would forget the driver after a snapshot.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140984
Currently, the setns() wrapper is supported only for x86_64 and i686
which leaves us failing to build on other platforms like arm, aarch64
and so on. This means, that the wrapper needs to be extended to those
platforms and make to fail on runtime not compile time.
The syscall numbers for other platforms was fetched using this
command:
kernel.git $ git grep "define.*__NR_setns" | grep -e arm -e powerpc -e s390
arch/arm/include/uapi/asm/unistd.h:#define __NR_setns (__NR_SYSCALL_BASE+375)
arch/arm64/include/asm/unistd32.h:#define __NR_setns 375
arch/powerpc/include/uapi/asm/unistd.h:#define __NR_setns 350
arch/s390/include/uapi/asm/unistd.h:#define __NR_setns 339
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The backing store string location offset 0 determines that the file
isn't present. The string size shouldn't be then checked:
from qemu.git/docs/specs/qcow2.txt
== Header ==
The first cluster of a qcow2 image contains the file header:
Byte 0 - 3: magic
QCOW magic string ("QFI\xfb")
4 - 7: version
Version number (valid values are 2 and 3)
8 - 15: backing_file_offset
Offset into the image file at which the backing file name
is stored (NB: The string is not null terminated). 0 if the
image doesn't have a backing file.
16 - 19: backing_file_size
Length of the backing file name in bytes. Must not be
longer than 1023 bytes. Undefined if the image doesn't have
a backing file. ^^^^^^^^^
This patch intentionally leaves the backing file string size check in
place in case a malformatted file would be presented to libvirt. Also
according to the docs the string size is maximum 1023 bytes, thus this
patch adds a check to verify that.
I was also able to verify that the check was done the same way in the
legacy qcow fromat (in qemu's code).
Coverity complains that because of how 'offset' is initialized to
0 (zero), the resulting math and comparison on rem is pointless.
According to the origin commit id '3ec128989', the code is a
replacement for gmtime(), but without the localtime() or GMT
calculations - so just remove this code and add a comment
indicating the removal
With the virGetGroupList() change in place - Coverity further complains
that if we fail to virFork(), the groups will be leaked - which aha seems
to be the case. Adjust the logic to save off the -errno, free the groups,
and then return the value we saved
Signed-off-by: John Ferlan <jferlan@redhat.com>
This ends up being a very bizarre false positive. With an assist from
eblake, the claim is that mgetgroups() could return a -1 value, but yet
still have a groups buffer allocated, yet the example shown doesn't
seem to prove that.
Rather than fret about it, by adding a well placed sa_assert() on the
returned *list value we can "assure" ourselves that the mgetgroups()
failure path won't signal this condition.
Signed-off-by: John Ferlan <jferlan@redhat.com>
To express empty drive we historically use storage source with empty
path. Unfortunately NBD disks may be declared without a path.
Add a helper to wrap this logic.
Test suites using the port allocator don't want to have different
behaviour depending on whether a port is in use on the host. Add
a VIR_PORT_ALLOCATOR_SKIP_BIND_CHECK which test suites can use
to skip the bind() test. The port allocator will thus only track
ports in use by the test suite process itself. This is fine when
using the port allocator to generate guest configs which won't
actually be launched
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Perhaps a false positive, but since Coverity doesn't understand the
relationship between the 'count' and the 'strings', rather than leave
the chance the on input 'strings' is NULL and causes a deref - just
check for it and return
Signed-off-by: John Ferlan <jferlan@redhat.com>
