- Devel - libvirt List Archives

[libvirt] [PATCH 0/7] Address some Coverity found issues
by John Ferlan 03 Jul '15

03 Jul '15

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

3 19

[libvirt] [PATCH 0/2] some qemu_driver cleanups
by Pavel Hrdina 03 Jul '15

03 Jul '15

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

2 5

[libvirt] Avoid XSS vulnerability on the search engine
by Daniel Veillard 03 Jul '15

03 Jul '15

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/

1 0

[libvirt] [PATCH] Fix nodeinfo output on PPC64 KVM hosts
by Shivaprasad G Bhat 03 Jul '15

03 Jul '15

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:

3 4

[libvirt] [PATCH] virsh: report error if vcpu number exceed the guest maxvcpu number
by Luyao Huang 03 Jul '15

03 Jul '15

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

4 5

[libvirt] Request for libvirt wiki account creation
by Prerna 03 Jul '15

03 Jul '15

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

2 2

[libvirt] Which symbols can be used in file paths in libvirt's XMLs?
by Dmitry Guryanov 02 Jul '15

02 Jul '15

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

2 2

[libvirt] [PATCHv3] virt-aa-helper: Fix permissions for vhost-user socket files
by Michal Dubiel 02 Jul '15

02 Jul '15

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

3 2

[libvirt] [PATCH 00/20] cpu_map.xml: Make CPU definitions easier to review
by Jiri Denemark 02 Jul '15

02 Jul '15

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

3 49

[libvirt] How to define QEMU/SMBIOS Attributes in virsh
by Holger Schranz 02 Jul '15

02 Jul '15

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

2 3