[libvirt] [PATCH 0/7] Address some Coverity found issues
by John Ferlan
The following Coverity issues are flagged in Coverity 7.6.1 compared
to not being seen in 7.5.1 - this series just addresses those issues
before 7.6.1 is installed in our upstream Jenkins environment:
http://jenkins-virt.usersys.redhat.com/job/libvirt-coverity/
It seems the bulk are essentially false positives, but we can do something
in order to work around them.
For the first 3 patches, Coverity complains that virVasprintfInternal
can set "*strp = NULL;" on return (in the error path), but the callers
don't associate the error path (< 0) with the 'buffer' and thus believe
there could be some path where a return 0 would occur. This would result
in the callers of the virPCIFile, virPCIDriverFile, and virPCIDriverDir
having an error. Rather than try to add sa_assert()'s for each, adjust
the calls and callers to return the buffer
For the fourth patch, the strtok_r call expects that the first call will
have a non-NULL value for the first parameter and subsequent calls over
the same string would pass a NULL value in the first parameter and a
non-NULL parameter in the third parameter to be used as the "point" in
the string to continue looking for the token. However, Coverity doesn't
realize this and thus flags the strtok_r call with a possible forward
NULL dereference because we initialize the third parameter to NULL.
By adding an 'sa_assert' for what is the first argument lets Coverity
know we know what we're doing. Additionally, VIR_STRDUP could return a
NULL string and a zero return, the assertion in openvz_conf.c is on
the passed value to be scanned.
The fifth patch is perhaps the only one fixing a real issue; however,
it's been around since 1.2.11, so perhaps it's a less traveled path.
The sixth patch seems to be a false positive, but since Coverity doesn't
make the connection between nstack and stack, it's just as safe to
add the protection in the Free routine against a strange path being hit.
For the last patch, this is a false positive, but it also shows up in
the 7.5.1 coverity build:
http://jenkins-virt.usersys.redhat.com/job/libvirt-coverity/128/
Essentially the issue is that virResizeN can "return 0;" prior to
the virExpandN and that causes Coverity some angst since it would
then be possible to not have anything returned in "*ret" which would
cause a NULL dereference. Stepping through the logic has no path
in which that could happen. In order to handle this, rather than
use VIR_RESIZE_N for 'n' times through the loop, let's first count
the matches, use VIR_ALLOC_N, and then rerun the same loop filling
in each address as before. The difference being 'n' VIR_RESIZE_N
versus 2*'n' STREQ calls - perhaps a small wash performance wise.
John Ferlan (7):
util: Resolve Coverity FORWARD_NULL
util: Resolve Coverity FORWARD_NULL
util: Resolve Coverity FORWARD_NULL
Avoid Coverity FORWARD_NULL prior to strtok_r calls
phyp: Resolve Coverity FORWARD_NULL
util: Resolve Coverity FORWARD_NULL
util: Avoid Coverity FORWARD_NULL
src/esx/esx_vi.c | 1 +
src/libxl/libxl_conf.c | 1 +
src/openvz/openvz_conf.c | 2 ++
src/phyp/phyp_driver.c | 3 +--
src/util/virdbus.c | 4 +++
src/util/virpci.c | 67 ++++++++++++++++++++++++-----------------------
src/util/virtypedparam.c | 14 ++++++----
src/xenapi/xenapi_utils.c | 1 +
8 files changed, 53 insertions(+), 40 deletions(-)
--
2.1.0
9 years, 9 months
[libvirt] [PATCH 0/2] some qemu_driver cleanups
by Pavel Hrdina
Pavel Hrdina (2):
qemu_driver: live/config checks cleanup
qemu_driver: remove duplicate code
src/qemu/qemu_driver.c | 85 ++++++--------------------------------------------
1 file changed, 9 insertions(+), 76 deletions(-)
--
2.4.5
9 years, 9 months
[libvirt] Avoid XSS vulnerability on the search engine
by Daniel Veillard
Raised by https://www.xssposed.org/incidents/69566/
Need to escape the user provided query before displaying it back
I pushed immediately as this was made public,
Daniel
diff --git a/docs/search.php.code.in b/docs/search.php.code.in
index df25cd6..84f8759 100644
--- a/docs/search.php.code.in
+++ b/docs/search.php.code.in
@@ -13,7 +13,7 @@
<form action="<?php echo $_SERVER['PHP_SELF'], "?query=", rawurlencode($query) ?>"
enctype="application/x-www-form-urlencoded" method="get">
- <input name="query" type="text" size="50" value="<?php echo $query?>"/>
+ <input name="query" type="text" size="50" value="<?php echo htmlspecialchars($query, ENT_QUOTES, 'UTF-8')?>"/>
<select name="scope">
<option value="any">Search All</option>
<option value="API" <?php if ($scope == 'API') print "selected='selected'"?>>Only the APIs</option>
--
Daniel Veillard | Open Source and Standards, Red Hat
veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | virtualization library http://libvirt.org/
9 years, 9 months
[libvirt] [PATCH] Fix nodeinfo output on PPC64 KVM hosts
by Shivaprasad G Bhat
The nodeinfo is reporting incorrect number of cpus and incorrect host
topology on PPC64 KVM hosts. The KVM hypervisor on PPC64 needs only
the primary thread in a core to be online, and the secondaries offlined.
While scheduling a guest in, the kvm scheduler wakes up the secondaries to
run in guest context.
The host scheduling of the guests happen at the core level(as only primary
thread is online). The kvm scheduler exploits as many threads of the core
as needed by guest. Further, starting POWER8, the processor allows splitting
a physical core into multiple subcores with 2 or 4 threads each. Again, only
the primary thread in a subcore is online in the host. The KVM-PPC
scheduler allows guests to exploit all the offline threads in the subcore,
by bringing them online when needed.
(Kernel patches on split-core http://www.spinics.net/lists/kvm-ppc/msg09121.html)
Recently with dynamic micro-threading changes in ppc-kvm, makes sure
to utilize all the offline cpus across guests, and across guests with
different cpu topologies.
(https://www.mail-archive.com/kvm@vger.kernel.org/msg115978.html)
Since the offline cpus are brought online in the guest context, it is safe
to count them as online. Nodeinfo today discounts these offline cpus from
cpu count/topology calclulation, and the nodeinfo output is not of any help
and the host appears overcommited when it is actually not.
The patch carefully counts those offline threads whose primary threads are
online. The host topology displayed by the nodeinfo is also fixed when the
host is in valid kvm state.
Test results with and without fix:
http://ur1.ca/mx25y
Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com>
---
src/nodeinfo.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 85 insertions(+)
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 2fafe2d..1a27080 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -32,6 +32,8 @@
#include <sys/utsname.h>
#include <sched.h>
#include "conf/domain_conf.h"
+#include <fcntl.h>
+#include <sys/ioctl.h>
#if defined(__FreeBSD__) || defined(__APPLE__)
# include <sys/time.h>
@@ -428,6 +430,10 @@ virNodeParseNode(const char *node,
unsigned int cpu;
int online;
int direrr;
+ int kvmfd = -1;
+ int lastonline;
+ int threads_per_subcore = 1;
+ bool valid_ppc64_kvmhost_mode = false;
*threads = 0;
*cores = 0;
@@ -464,6 +470,66 @@ virNodeParseNode(const char *node,
sock_max++;
+ if (ARCH_IS_PPC64(arch)) {
+ /* PPC-KVM needs the secondary threads of a core to be offline on the
+ * host. The kvm schedular brings the secondary threads online in the
+ * guest context. Moreover, P8 processor has split-core capability
+ * where, there can be 1,2 or 4 subcores per core. The primaries of the
+ * subcores alone will be online on the host for a subcore in the
+ * host. Even though the actual threads per core for P8 processor is 8,
+ * depending on the subcores_per_core = 1, 2 or 4, the threads per
+ * subcore will vary accordingly to 8, 4 and 2 repectively.
+ * So, On host threads_per_core what is arrived at from sysfs in the
+ * current logic is actually the subcores_per_core. Threads per subcore
+ * can only be obtained from the kvm device. For example, on P8 wih 1
+ * core having 8 threads, sub_cores_percore=4, the threads 0,2,4 & 6
+ * will be online. The sysfs reflects this and in the current logic
+ * variable 'threads' will be 4 which is nothing but subcores_per_core.
+ * If the user tampers the cpu online/offline states using chcpu or other
+ * means, then it is an unsupported configuration for kvm.
+ * The code below tries to keep in mind
+ * - when the libvirtd is run inside a KVM guest or Phyp based guest.
+ * - Or on the kvm host where user manually tampers the cpu states to
+ * offline/online randomly.
+ */
+# if HAVE_LINUX_KVM_H
+# include <linux/kvm.h>
+ kvmfd = open("/dev/kvm", O_RDONLY);
+ if (kvmfd >= 0) {
+# ifdef KVM_CAP_PPC_SMT
+ valid_ppc64_kvmhost_mode = true;
+ /* For Phyp and KVM based guests the ioctl for KVM_CAP_PPC_SMT
+ * returns zero and both primary and secondary threads will be
+ * online. Dont try to alter or interpret anything here.
+ */
+ threads_per_subcore = ioctl(kvmfd, KVM_CHECK_EXTENSION, KVM_CAP_PPC_SMT);
+ if (threads_per_subcore == 0)
+ valid_ppc64_kvmhost_mode = false;
+# endif
+ }
+ VIR_FORCE_CLOSE(kvmfd);
+# endif
+ rewinddir(cpudir);
+ lastonline = -1;
+ while (valid_ppc64_kvmhost_mode &&
+ ((direrr = virDirRead(cpudir, &cpudirent, node)) > 0)) {
+ if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
+ continue;
+ if ((online = virNodeGetCpuValue(node, cpu, "online", 1)) < 0)
+ goto cleanup;
+
+ if (online) {
+ if (lastonline != -1 &&
+ (cpu - lastonline) < threads_per_subcore) {
+ valid_ppc64_kvmhost_mode = false; /* if any of secondaries
+ * online */
+ break;
+ }
+ lastonline = cpu;
+ }
+ }
+ }
+
/* allocate cpu maps for each socket */
if (VIR_ALLOC_N(core_maps, sock_max) < 0)
goto cleanup;
@@ -473,6 +539,7 @@ virNodeParseNode(const char *node,
/* iterate over all CPU's in the node */
rewinddir(cpudir);
+ lastonline = -1;
while ((direrr = virDirRead(cpudir, &cpudirent, node)) > 0) {
if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
continue;
@@ -480,6 +547,17 @@ virNodeParseNode(const char *node,
if ((online = virNodeGetCpuValue(node, cpu, "online", 1)) < 0)
goto cleanup;
+ if (ARCH_IS_PPC64(arch) && valid_ppc64_kvmhost_mode) {
+ if (online) {
+ lastonline = cpu;
+ } else if (lastonline != -1 &&
+ (cpu-lastonline) < threads_per_subcore) {
+ processors++; /* Count only those secondaries whose primary
+ subcore is online */
+ continue;
+ }
+ }
+
if (!online) {
(*offline)++;
continue;
@@ -528,6 +606,13 @@ virNodeParseNode(const char *node,
*cores = core;
}
+ if (ARCH_IS_PPC64(arch) && valid_ppc64_kvmhost_mode) {
+ /* The actual host threads per core is
+ * multiple of the subcores_per_core(i.e variable *threads in this case)
+ * and threads_per_subcore.
+ */
+ *threads = *threads * threads_per_subcore;
+ }
ret = processors;
cleanup:
9 years, 9 months
[libvirt] [PATCH] virsh: report error if vcpu number exceed the guest maxvcpu number
by Luyao Huang
If usr pass a vcpu which exceed guest maxvcpu number, virsh client
will only output an header like this:
# virsh vcpupin test3 1000
VCPU: CPU Affinity
----------------------------------
After this patch:
# virsh vcpupin test3 1000
error: vcpu 1000 is out of range of cpu count 2
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
tools/virsh-domain.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 27d62e9..681fc1a 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6497,6 +6497,11 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
+ if (got_vcpu && vcpu >= ncpus) {
+ vshError(ctl, _("vcpu %d is out of range of cpu count %d"), vcpu, ncpus);
+ goto cleanup;
+ }
+
cpumaplen = VIR_CPU_MAPLEN(maxcpu);
cpumap = vshMalloc(ctl, ncpus * cpumaplen);
if ((ncpus = virDomainGetVcpuPinInfo(dom, ncpus, cpumap,
--
1.8.3.1
9 years, 9 months
[libvirt] Request for libvirt wiki account creation
by Prerna
Hi,
I am interested in contributing some content to the libvirt wiki; and so I
need a libvirt wiki account. Could you pls create an account with following
credentials:
username : prerna
Thanks,
Prerna Saxena
9 years, 9 months
[libvirt] Which symbols can be used in file paths in libvirt's XMLs?
by Dmitry Guryanov
Hello,
There is an absFilePatch type in docs/schemas/basictypes.rng rng schema:
<define name="absFilePath">
<data type="string">
<param
name="pattern">/[a-zA-Z0-9_\.\+\-\\&"'<>/%,:]+</param>
</data>
</define>
There are quite few symbols in this set of the ones, which linux allows
to have in file paths. For example spaces or curly braces are not
allowed. There are a lot of elements of this type in other rng schems,
for example:
<define name="diskSourceFile">
<optional>
<attribute name="type">
<value>file</value>
</attribute>
</optional>
<interleave>
<optional>
<element name="source">
<optional>
<attribute name="file">
<ref name="absFilePath"/>
</attribute>
There is no checks in libvirt if the the xml, given to, for example,
virDomainDefineXML function conforms to this schema. I've added a disk
to the domain with curly braces in path and there were no errors:
<disk type='file' device='disk'>
<driver name='qemu' type='qcow2'/>
<source file='/home/libvirt/{123}.qcow2'/>
<target dev='hda' bus='ide'/>
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
</disk>
The domain was successfully defined and virsh dumpxml shows this path in
output.
So the question is, where did this path pattern come from? Is it
possible to use file paths with another symbols or there will be some
problems?
--
Dmitry Guryanov
9 years, 9 months
[libvirt] [PATCHv3] virt-aa-helper: Fix permissions for vhost-user socket files
by Michal Dubiel
QEMU working in vhost-user mode communicates with the other end (i.e.
some virtual router application) via unix domain sockets. This requires
that permissions for the socket files are correctly written into
/etc/apparmor.d/libvirt/libvirt-UUID.files.
Signed-off-by: Michal Dubiel <md(a)semihalf.com>
---
Changes since v2:
- Removed curly braces from one line 'if' block (syntax-check claims)
src/security/virt-aa-helper.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 35423b5..13f8a6a 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -32,7 +32,6 @@
#include <unistd.h>
#include <errno.h>
#include <sys/types.h>
-#include <sys/stat.h>
#include <fcntl.h>
#include <getopt.h>
#include <sys/utsname.h>
@@ -542,7 +541,6 @@ array_starts_with(const char *str, const char * const *arr, const long size)
static int
valid_path(const char *path, const bool readonly)
{
- struct stat sb;
int npaths, opaths;
const char * const restricted[] = {
"/bin/",
@@ -590,20 +588,8 @@ valid_path(const char *path, const bool readonly)
if (STRNEQLEN(path, "/", 1))
return 1;
- if (!virFileExists(path)) {
+ if (!virFileExists(path))
vah_warning(_("path does not exist, skipping file type checks"));
- } else {
- if (stat(path, &sb) == -1)
- return -1;
-
- switch (sb.st_mode & S_IFMT) {
- case S_IFSOCK:
- return 1;
- break;
- default:
- break;
- }
- }
opaths = sizeof(override)/sizeof(*(override));
@@ -1101,6 +1087,18 @@ get_files(vahControl * ctl)
}
}
+ for (i = 0; i < ctl->def->nnets; i++) {
+ if (ctl->def->nets[i] &&
+ ctl->def->nets[i]->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER &&
+ ctl->def->nets[i]->data.vhostuser) {
+ virDomainChrSourceDefPtr vhu = ctl->def->nets[i]->data.vhostuser;
+
+ if (vah_add_file_chardev(&buf, vhu->data.nix.path, "rw",
+ vhu->type) != 0)
+ goto cleanup;
+ }
+ }
+
if (ctl->def->virtType == VIR_DOMAIN_VIRT_KVM) {
for (i = 0; i < ctl->def->nnets; i++) {
virDomainNetDefPtr net = ctl->def->nets[i];
--
1.9.1
9 years, 9 months
[libvirt] [PATCH 00/20] cpu_map.xml: Make CPU definitions easier to review
by Jiri Denemark
Inheritance among CPU model is cool but it makes reviewing CPU model
definitions and comparing them to CPU models from QEMU rather hard and
unpleasant. Let's define all CPU models from scratch with a sorted list
of features.
Jiri Denemark (20):
cpu_map.xml: Sort features in x86 CPU models
cpu_map.xml: Expand 486 CPU model
cpu_map.xml: Expand pentium CPU model
cpu_map.xml: Expand pentium2 CPU model
cpu_map.xml: Expand pentiumpro CPU model
cpu_map.xml: Expand coreduo CPU model
cpu_map.xml: Expand n270 CPU model
cpu_map.xml: Expand qemu32 CPU model
cpu_map.xml: Expand kvm32 CPU model
cpu_map.xml: Expand cpu64-rhel5 CPU model
cpu_map.xml: Expand kvm64 CPU model
cpu_map.xml: Expand Conroe CPU model
cpu_map.xml: Expand Penryn CPU model
cpu_map.xml: Expand Nehalem CPU model
cpu_map.xml: Expand Westmere CPU model
cpu_map.xml: Expand SandyBridge CPU model
cpu_map.xml: Expand Haswell-noTSX CPU model
cpu_map.xml: Expand Opteron_G1 CPU model
cpu_map.xml: Expand Opteron_G2 CPU model
cpu_map.xml: Expand Opteron_G4 CPU model
src/cpu/cpu_map.xml | 829 +++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 729 insertions(+), 100 deletions(-)
--
2.4.4
9 years, 9 months
[libvirt] How to define QEMU/SMBIOS Attributes in virsh
by Holger Schranz
Hello,
I have a "request for Help" please.
Myself:
I'm a native german language speaker and
I' work with qemu/virsh since 6 months now.
At the moment I try to define/configure qemu/smbios
attributes in a virsh xml-file.
My current problem:
I need to configure two on-board lan elements.
Thanks to Gerd Hoffmann (Qemu group). He gave me the hint for
smbios/acpi configuration. For this configuration I need a
type 41 record. I found on http://libvirt.org some descriptions
for smbios but only for type 0 and 1.
In qemu the solution is available:
[Qemu-devel] [PATCH 2/2] smbios: Allow device ID instead of PCI address
http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg02350.html
@@ -1336,9 +1338,10 @@ Specify SMBIOS type 0 fields Specify SMBIOS type
1 fields
@item -smbios
type=41,address@hidden,address@hidden,address@hidden,status=on|off][,address@hidden|n}]
address@hidden -smbios
type=41,address@hidden,address@hidden,address@hidden,status=on|off][,address@hidden|n}]
Add SMBIOS type 41 fields (Onboard Devices Extended Information). Could be
specified multiple times for different devices. Mandatory options are
address@hidden in form "[[<domain>:]<bus>:]<slot>.<func>" and
address@hidden in form "[[<domain>:]<bus>:]<slot>.<func>" or
@option{id}, and
@option{instance} number. @option{instance} shoud be in range [1, 255] and
should be unique within specified @option{device-type}.
@option{designation} is
an optional string that somehow designates device. @option{status} is
a device
@@ -1365,6 +1368,9 @@ qemu-i386 \
-netdev user,id=hostnet0 \
-device e1000,netdev=hostnet0,id=net0,bus=pci.0,addr=0x1f \
-smbios type=41,address=00:1f.0,instance=1,designation="NIC
1",device-type=ethernet \
+-netdev user,id=hostnet1 \
+-device e1000,netdev=hostnet1,id=net1 \
+-smbios type=41,id=net1,instance=2,designation="NIC
2",device-type=ethernet \
< ... other qemu options ... >
@end example
So my question: How can I configure such record in a virsh xml-file please.
Any help is appreciated.
Holger
---
Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft.
https://www.avast.com/antivirus
9 years, 9 months
