- Devel - libvirt List Archives

[libvirt] [PATCH] qemu: Prevent crash of libvirtd when setting numa parameters
by ajia＠redhat.com 05 Jan '12

05 Jan '12

From: Alex Jia <ajia(a)redhat.com> It's a NULL pointer deref issue, which leads to libvirtd crash. This patch directly use 'params[i].value.s' value instead of derefing a NULL pointer on memcpy. * how to reproduce? % virsh numatune <domain> --nodeset 0 % service libvirtd status * src/qemu/qemu_driver.c (qemuDomainSetNumaParameters): avoid a NULL pointer deref. RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=771562 Signed-off-by: Alex Jia <ajia(a)redhat.com> --- src/qemu/qemu_driver.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 82bab67..1bd93f6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6721,14 +6721,12 @@ qemuDomainSetNumaParameters(virDomainPtr dom, } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - memcpy(oldnodemask, persistentDef->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN); + memcpy(oldnodemask, params[i].value.s, VIR_DOMAIN_CPUMASK_LEN); if (virDomainCpuSetParse(params[i].value.s, 0, persistentDef->numatune.memory.nodemask, VIR_DOMAIN_CPUMASK_LEN) < 0) { - memcpy(persistentDef->numatune.memory.nodemask, - oldnodemask, VIR_DOMAIN_CPUMASK_LEN); + memcpy(params[i].value.s, oldnodemask, VIR_DOMAIN_CPUMASK_LEN); ret = -1; continue; } -- 1.7.1

4 8

[libvirt] [PATCH] revert commit baade4cd2bf partly
by Hu Tao 05 Jan '12

05 Jan '12

This is not a memory leak. See line 8029 and 8030 of qemu_driver.c. To ensure this, I tested twice following these steps: 1. set bandwidth lively (--live) 2. query bandwidth (--live) 3. set bandwidth lively (--live) The first time libvirtd crashed at step 2. The second time on step 2 I got strage data, and libvirtd crashed at step 3. --- src/qemu/qemu_driver.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e93fe87..4be36f5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7864,7 +7864,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, virDomainDefPtr persistentDef = NULL; int ret = -1; virDomainNetDefPtr net = NULL, persistentNet = NULL; - virNetDevBandwidthPtr bandwidth = NULL, newBandwidth = NULL; + virNetDevBandwidthPtr bandwidth = NULL; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -7986,6 +7986,8 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, } if (flags & VIR_DOMAIN_AFFECT_LIVE) { + virNetDevBandwidthPtr newBandwidth = NULL; + if (VIR_ALLOC(newBandwidth) < 0) { virReportOOMError(); goto cleanup; @@ -8053,7 +8055,6 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, ret = 0; cleanup: virNetDevBandwidthFree(bandwidth); - virNetDevBandwidthFree(newBandwidth); virCgroupFree(&group); if (vm) virDomainObjUnlock(vm); -- 1.7.4.4

3 3

[libvirt] kvm-qemu emulator problem
by Pankaj Rawat 05 Jan '12

05 Jan '12

Hi all , I am trying to use fvd for my guest machine for that I refer the link http://sites.google.com/site/tangchq/qemu-fvd now all works fine except the last step which said that if libvirt is used then the xml configuration file need to be changed to include the modified emulator qemu-system-x86_64 The default path is /usr/libexec/qemu-kvm But the change in xml configuration file is of no use which is in /etc/libvirt/qemu directory. Since it is created when the guest is created using virt-install. Well I don't know how I can change the default path for emulator, so that qemu-system-x86_64 act as a default emulator can anyone help ? DISCLAIMER: ----------------------------------------------------------------------------------------------------------------------- The contents of this e-mail and any attachment(s) are confidential and intended for the named recipient(s) only. It shall not attach any liability on the originator or NECHCL or its affiliates. Any views or opinions presented in this email are solely those of the author and may not necessarily reflect the opinions of NECHCL or its affiliates. Any form of reproduction, dissemination, copying, disclosure, modification, distribution and / or publication of this message without the prior written consent of the author of this e-mail is strictly prohibited. If you have received this email in error please delete it and notify the sender immediately. . -----------------------------------------------------------------------------------------------------------------------

3 8

[libvirt] [PATCH] seclabel: fix regression in libvirtd restart
by Eric Blake 05 Jan '12

05 Jan '12

Commit b434329 has a logic bug: seclabel overrides don't set def->type, but the default value is 0 (aka static). Restarting libvirtd would thus reject the XML for any domain with an override of <seclabel relabel='no'/> (which happens quite easily if a disk image lives on NFS), with a message: 2012-01-04 22:29:40.949+0000: 6769: error : virSecurityLabelDefParseXMLHelper:2593 : XML error: security label is missing Fix the logic to never read from an override's def->type, and to allow a missing <label> subelement when relabel is no. There's a lot of stupid double-negatives in the code (!norelabel) because of the way that we want the zero-initialized defaults to behave. * src/conf/domain_conf.c (virSecurityLabelDefParseXMLHelper): Use type field from correct location. --- src/conf/domain_conf.c | 16 +++++++++------- 1 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 29966f1..dcf23fa 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1,7 +1,7 @@ /* * domain_conf.c: domain XML processing * - * Copyright (C) 2006-2011 Red Hat, Inc. + * Copyright (C) 2006-2012 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -2541,6 +2541,7 @@ virSecurityLabelDefParseXMLHelper(virSecurityLabelDefPtr def, char *p; xmlNodePtr save_ctxt = ctxt->node; int ret = -1; + int type = default_seclabel ? default_seclabel->type : def->type; ctxt->node = node; @@ -2567,14 +2568,15 @@ virSecurityLabelDefParseXMLHelper(virSecurityLabelDefPtr def, } VIR_FREE(p); if (!default_seclabel && - def->type == VIR_DOMAIN_SECLABEL_DYNAMIC && + type == VIR_DOMAIN_SECLABEL_DYNAMIC && def->norelabel) { - virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("dynamic label type must use resource relabeling")); + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("dynamic label type must use resource " + "relabeling")); goto cleanup; } } else { - if (!default_seclabel && def->type == VIR_DOMAIN_SECLABEL_STATIC) + if (!default_seclabel && type == VIR_DOMAIN_SECLABEL_STATIC) def->norelabel = true; else def->norelabel = false; @@ -2583,12 +2585,12 @@ virSecurityLabelDefParseXMLHelper(virSecurityLabelDefPtr def, /* Only parse label, if using static labels, or * if the 'live' VM XML is requested, or if this is a device override */ - if (def->type == VIR_DOMAIN_SECLABEL_STATIC || + if (type == VIR_DOMAIN_SECLABEL_STATIC || !(flags & VIR_DOMAIN_XML_INACTIVE) || (default_seclabel && !def->norelabel)) { p = virXPathStringLimit("string(./label[1])", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); - if (p == NULL) { + if (p == NULL && !(default_seclabel && def->norelabel)) { virDomainReportError(VIR_ERR_XML_ERROR, "%s", _("security label is missing")); goto cleanup; -- 1.7.7.5

3 2

[libvirt] [PATCH] Support Xen domctl v8
by Jim Fehlig 05 Jan '12

05 Jan '12

xen-unstable c/s 23874:651aed73b39c added another member to xen_domctl_getdomaininfo struct and bumped domctl version to 8. Add a corresponding domctl v8 struct in xen hypervisor sub-driver and detect domctl v8 during initialization. --- src/xen/xen_hypervisor.c | 129 +++++++++++++++++++++++++++++++++------------ 1 files changed, 94 insertions(+), 35 deletions(-) diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 941686f..ee3a59c 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -243,12 +243,30 @@ struct xen_v2d7_getdomaininfo { }; typedef struct xen_v2d7_getdomaininfo xen_v2d7_getdomaininfo; +struct xen_v2d8_getdomaininfo { + domid_t domain; /* the domain number */ + uint32_t flags; /* flags, see before */ + uint64_t tot_pages ALIGN_64; /* total number of pages used */ + uint64_t max_pages ALIGN_64; /* maximum number of pages allowed */ + uint64_t shr_pages ALIGN_64; /* number of shared pages */ + uint64_t paged_pages ALIGN_64; /* number of paged pages */ + uint64_t shared_info_frame ALIGN_64; /* MFN of shared_info struct */ + uint64_t cpu_time ALIGN_64; /* CPU time used */ + uint32_t nr_online_vcpus; /* Number of VCPUs currently online. */ + uint32_t max_vcpu_id; /* Maximum VCPUID in use by this domain. */ + uint32_t ssidref; + xen_domain_handle_t handle; + uint32_t cpupool; +}; +typedef struct xen_v2d8_getdomaininfo xen_v2d8_getdomaininfo; + union xen_getdomaininfo { struct xen_v0_getdomaininfo v0; struct xen_v2_getdomaininfo v2; struct xen_v2d5_getdomaininfo v2d5; struct xen_v2d6_getdomaininfo v2d6; struct xen_v2d7_getdomaininfo v2d7; + struct xen_v2d8_getdomaininfo v2d8; }; typedef union xen_getdomaininfo xen_getdomaininfo; @@ -258,6 +276,7 @@ union xen_getdomaininfolist { struct xen_v2d5_getdomaininfo *v2d5; struct xen_v2d6_getdomaininfo *v2d6; struct xen_v2d7_getdomaininfo *v2d7; + struct xen_v2d8_getdomaininfo *v2d8; }; typedef union xen_getdomaininfolist xen_getdomaininfolist; @@ -295,179 +314,211 @@ typedef struct xen_v2s5_availheap xen_v2s5_availheap; #define XEN_GETDOMAININFOLIST_ALLOC(domlist, size) \ (hv_versions.hypervisor < 2 ? \ (VIR_ALLOC_N(domlist.v0, (size)) == 0) : \ - (hv_versions.dom_interface >= 7 ? \ + (hv_versions.dom_interface >= 8 ? \ + (VIR_ALLOC_N(domlist.v2d8, (size)) == 0) : \ + (hv_versions.dom_interface == 7 ? \ (VIR_ALLOC_N(domlist.v2d7, (size)) == 0) : \ (hv_versions.dom_interface == 6 ? \ (VIR_ALLOC_N(domlist.v2d6, (size)) == 0) : \ (hv_versions.dom_interface == 5 ? \ (VIR_ALLOC_N(domlist.v2d5, (size)) == 0) : \ - (VIR_ALLOC_N(domlist.v2, (size)) == 0))))) + (VIR_ALLOC_N(domlist.v2, (size)) == 0)))))) #define XEN_GETDOMAININFOLIST_FREE(domlist) \ (hv_versions.hypervisor < 2 ? \ VIR_FREE(domlist.v0) : \ - (hv_versions.dom_interface >= 7 ? \ + (hv_versions.dom_interface >= 8 ? \ + VIR_FREE(domlist.v2d8) : \ + (hv_versions.dom_interface == 7 ? \ VIR_FREE(domlist.v2d7) : \ (hv_versions.dom_interface == 6 ? \ VIR_FREE(domlist.v2d6) : \ (hv_versions.dom_interface == 5 ? \ VIR_FREE(domlist.v2d5) : \ - VIR_FREE(domlist.v2))))) + VIR_FREE(domlist.v2)))))) #define XEN_GETDOMAININFOLIST_CLEAR(domlist, size) \ (hv_versions.hypervisor < 2 ? \ memset(domlist.v0, 0, sizeof(*domlist.v0) * size) : \ - (hv_versions.dom_interface >= 7 ? \ + (hv_versions.dom_interface >= 8 ? \ + memset(domlist.v2d8, 0, sizeof(*domlist.v2d8) * size) : \ + (hv_versions.dom_interface == 7 ? \ memset(domlist.v2d7, 0, sizeof(*domlist.v2d7) * size) : \ (hv_versions.dom_interface == 6 ? \ memset(domlist.v2d6, 0, sizeof(*domlist.v2d6) * size) : \ (hv_versions.dom_interface == 5 ? \ memset(domlist.v2d5, 0, sizeof(*domlist.v2d5) * size) : \ - memset(domlist.v2, 0, sizeof(*domlist.v2) * size))))) + memset(domlist.v2, 0, sizeof(*domlist.v2) * size)))))) #define XEN_GETDOMAININFOLIST_DOMAIN(domlist, n) \ (hv_versions.hypervisor < 2 ? \ domlist.v0[n].domain : \ - (hv_versions.dom_interface >= 7 ? \ + (hv_versions.dom_interface >= 8 ? \ + domlist.v2d8[n].domain : \ + (hv_versions.dom_interface == 7 ? \ domlist.v2d7[n].domain : \ (hv_versions.dom_interface == 6 ? \ domlist.v2d6[n].domain : \ (hv_versions.dom_interface == 5 ? \ domlist.v2d5[n].domain : \ - domlist.v2[n].domain)))) + domlist.v2[n].domain))))) #define XEN_GETDOMAININFOLIST_UUID(domlist, n) \ (hv_versions.hypervisor < 2 ? \ domlist.v0[n].handle : \ - (hv_versions.dom_interface >= 7 ? \ + (hv_versions.dom_interface >= 8 ? \ + domlist.v2d8[n].handle : \ + (hv_versions.dom_interface == 7 ? \ domlist.v2d7[n].handle : \ (hv_versions.dom_interface == 6 ? \ domlist.v2d6[n].handle : \ (hv_versions.dom_interface == 5 ? \ domlist.v2d5[n].handle : \ - domlist.v2[n].handle)))) + domlist.v2[n].handle))))) #define XEN_GETDOMAININFOLIST_DATA(domlist) \ (hv_versions.hypervisor < 2 ? \ (void*)(domlist->v0) : \ - (hv_versions.dom_interface >= 7 ? \ + (hv_versions.dom_interface >= 8 ? \ + (void*)(domlist->v2d8) : \ + (hv_versions.dom_interface == 7 ? \ (void*)(domlist->v2d7) : \ (hv_versions.dom_interface == 6 ? \ (void*)(domlist->v2d6) : \ (hv_versions.dom_interface == 5 ? \ (void*)(domlist->v2d5) : \ - (void*)(domlist->v2))))) + (void*)(domlist->v2)))))) #define XEN_GETDOMAININFO_SIZE \ (hv_versions.hypervisor < 2 ? \ sizeof(xen_v0_getdomaininfo) : \ - (hv_versions.dom_interface >= 7 ? \ + (hv_versions.dom_interface >= 8 ? \ + sizeof(xen_v2d8_getdomaininfo) : \ + (hv_versions.dom_interface == 7 ? \ sizeof(xen_v2d7_getdomaininfo) : \ (hv_versions.dom_interface == 6 ? \ sizeof(xen_v2d6_getdomaininfo) : \ (hv_versions.dom_interface == 5 ? \ sizeof(xen_v2d5_getdomaininfo) : \ - sizeof(xen_v2_getdomaininfo))))) + sizeof(xen_v2_getdomaininfo)))))) #define XEN_GETDOMAININFO_CLEAR(dominfo) \ (hv_versions.hypervisor < 2 ? \ memset(&(dominfo.v0), 0, sizeof(xen_v0_getdomaininfo)) : \ - (hv_versions.dom_interface >= 7 ? \ + (hv_versions.dom_interface >= 8 ? \ + memset(&(dominfo.v2d8), 0, sizeof(xen_v2d8_getdomaininfo)) : \ + (hv_versions.dom_interface == 7 ? \ memset(&(dominfo.v2d7), 0, sizeof(xen_v2d7_getdomaininfo)) : \ (hv_versions.dom_interface == 6 ? \ memset(&(dominfo.v2d6), 0, sizeof(xen_v2d6_getdomaininfo)) : \ (hv_versions.dom_interface == 5 ? \ memset(&(dominfo.v2d5), 0, sizeof(xen_v2d5_getdomaininfo)) : \ - memset(&(dominfo.v2), 0, sizeof(xen_v2_getdomaininfo)))))) + memset(&(dominfo.v2), 0, sizeof(xen_v2_getdomaininfo))))))) #define XEN_GETDOMAININFO_DOMAIN(dominfo) \ (hv_versions.hypervisor < 2 ? \ dominfo.v0.domain : \ - (hv_versions.dom_interface >= 7 ? \ + (hv_versions.dom_interface >= 8 ? \ + dominfo.v2d8.domain : \ + (hv_versions.dom_interface == 7 ? \ dominfo.v2d7.domain : \ (hv_versions.dom_interface == 6 ? \ dominfo.v2d6.domain : \ (hv_versions.dom_interface == 5 ? \ dominfo.v2d5.domain : \ - dominfo.v2.domain)))) + dominfo.v2.domain))))) #define XEN_GETDOMAININFO_CPUTIME(dominfo) \ (hv_versions.hypervisor < 2 ? \ dominfo.v0.cpu_time : \ - (hv_versions.dom_interface >= 7 ? \ + (hv_versions.dom_interface >= 8 ? \ + dominfo.v2d8.cpu_time : \ + (hv_versions.dom_interface == 7 ? \ dominfo.v2d7.cpu_time : \ (hv_versions.dom_interface == 6 ? \ dominfo.v2d6.cpu_time : \ (hv_versions.dom_interface == 5 ? \ dominfo.v2d5.cpu_time : \ - dominfo.v2.cpu_time)))) + dominfo.v2.cpu_time))))) #define XEN_GETDOMAININFO_CPUCOUNT(dominfo) \ (hv_versions.hypervisor < 2 ? \ dominfo.v0.nr_online_vcpus : \ - (hv_versions.dom_interface >= 7 ? \ + (hv_versions.dom_interface >= 8 ? \ + dominfo.v2d8.nr_online_vcpus : \ + (hv_versions.dom_interface == 7 ? \ dominfo.v2d7.nr_online_vcpus : \ (hv_versions.dom_interface == 6 ? \ dominfo.v2d6.nr_online_vcpus : \ (hv_versions.dom_interface == 5 ? \ dominfo.v2d5.nr_online_vcpus : \ - dominfo.v2.nr_online_vcpus)))) + dominfo.v2.nr_online_vcpus))))) -#define XEN_GETDOMAININFO_MAXCPUID(dominfo) \ +#define XEN_GETDOMAININFO_MAXCPUID(dominfo) \ (hv_versions.hypervisor < 2 ? \ dominfo.v0.max_vcpu_id : \ - (hv_versions.dom_interface >= 7 ? \ + (hv_versions.dom_interface >= 8 ? \ + dominfo.v2d8.max_vcpu_id : \ + (hv_versions.dom_interface == 7 ? \ dominfo.v2d7.max_vcpu_id : \ (hv_versions.dom_interface == 6 ? \ dominfo.v2d6.max_vcpu_id : \ (hv_versions.dom_interface == 5 ? \ dominfo.v2d5.max_vcpu_id : \ - dominfo.v2.max_vcpu_id)))) + dominfo.v2.max_vcpu_id))))) #define XEN_GETDOMAININFO_FLAGS(dominfo) \ (hv_versions.hypervisor < 2 ? \ dominfo.v0.flags : \ - (hv_versions.dom_interface >= 7 ? \ + (hv_versions.dom_interface >= 8 ? \ + dominfo.v2d8.flags : \ + (hv_versions.dom_interface == 7 ? \ dominfo.v2d7.flags : \ (hv_versions.dom_interface == 6 ? \ dominfo.v2d6.flags : \ (hv_versions.dom_interface == 5 ? \ dominfo.v2d5.flags : \ - dominfo.v2.flags)))) + dominfo.v2.flags))))) #define XEN_GETDOMAININFO_TOT_PAGES(dominfo) \ (hv_versions.hypervisor < 2 ? \ dominfo.v0.tot_pages : \ - (hv_versions.dom_interface >= 7 ? \ + (hv_versions.dom_interface >= 8 ? \ + dominfo.v2d8.tot_pages : \ + (hv_versions.dom_interface == 7 ? \ dominfo.v2d7.tot_pages : \ (hv_versions.dom_interface == 6 ? \ dominfo.v2d6.tot_pages : \ (hv_versions.dom_interface == 5 ? \ dominfo.v2d5.tot_pages : \ - dominfo.v2.tot_pages)))) + dominfo.v2.tot_pages))))) #define XEN_GETDOMAININFO_MAX_PAGES(dominfo) \ (hv_versions.hypervisor < 2 ? \ dominfo.v0.max_pages : \ - (hv_versions.dom_interface >= 7 ? \ + (hv_versions.dom_interface >= 8 ? \ + dominfo.v2d8.max_pages : \ + (hv_versions.dom_interface == 7 ? \ dominfo.v2d7.max_pages : \ (hv_versions.dom_interface == 6 ? \ dominfo.v2d6.max_pages : \ (hv_versions.dom_interface == 5 ? \ dominfo.v2d5.max_pages : \ - dominfo.v2.max_pages)))) + dominfo.v2.max_pages))))) #define XEN_GETDOMAININFO_UUID(dominfo) \ (hv_versions.hypervisor < 2 ? \ dominfo.v0.handle : \ - (hv_versions.dom_interface >= 7 ? \ + (hv_versions.dom_interface >= 8 ? \ + dominfo.v2d8.handle : \ + (hv_versions.dom_interface == 7 ? \ dominfo.v2d7.handle : \ (hv_versions.dom_interface == 6 ? \ dominfo.v2d6.handle : \ (hv_versions.dom_interface == 5 ? \ dominfo.v2d5.handle : \ - dominfo.v2.handle)))) + dominfo.v2.handle))))) static int @@ -2142,12 +2193,20 @@ xenHypervisorInit(struct xenHypervisorVersions *override_versions) /* Xen 4.1 * sysctl version 8 -> xen-unstable c/s 21118:28e5409e3fb3 * domctl version 7 -> xen-unstable c/s 21212:de94884a669c + * domctl version 8 -> xen-unstable c/s 23874:651aed73b39c */ hv_versions.sys_interface = 8; /* XEN_SYSCTL_INTERFACE_VERSION */ if (virXen_getdomaininfo(fd, 0, &info) == 1) { hv_versions.dom_interface = 7; /* XEN_DOMCTL_INTERFACE_VERSION */ - VIR_DEBUG("Using hypervisor call v2, sys ver8 dom ver7\n"); - goto done; + if (virXen_getvcpusinfo(fd, 0, 0, ipt, NULL, 0) == 0){ + VIR_DEBUG("Using hypervisor call v2, sys ver8 dom ver7"); + goto done; + } + hv_versions.dom_interface = 8; /* XEN_DOMCTL_INTERFACE_VERSION */ + if (virXen_getvcpusinfo(fd, 0, 0, ipt, NULL, 0) == 0){ + VIR_DEBUG("Using hypervisor call v2, sys ver8 dom ver8"); + goto done; + } } hv_versions.hypervisor = 1; -- 1.7.7

3 3

[libvirt] [PATCH] command: Discard FD_SETSIZE limit for opened files
by Michal Privoznik 05 Jan '12

05 Jan '12

Currently, virCommand implementation uses FD_ macros from sys/select.h. However, those cannot handle more opened files than FD_SETSIZE. Therefore switch to generalized implementation based on array of integers. --- src/util/command.c | 108 ++++++++++++++++++++++++++++++++++++++++------------ 1 files changed, 83 insertions(+), 25 deletions(-) diff --git a/src/util/command.c b/src/util/command.c index f5effdf..6a2a0eb 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -75,9 +75,10 @@ struct _virCommand { char *pwd; - /* XXX Use int[] if we ever need to support more than FD_SETSIZE fd's. */ - fd_set preserve; /* FDs to pass to child. */ - fd_set transfer; /* FDs to close in parent. */ + int *preserve; /* FDs to pass to child. */ + int preserve_size; + int *transfer; /* FDs to close in parent. */ + int transfer_size; unsigned int flags; @@ -130,6 +131,64 @@ static int virClearCapabilities(void) } # endif +/* + * virCommandFDIsSet: + * @fd: FD to test + * @set: the set + * @set_size: actual size of @set + * + * Check if FD is already in @set or not. + * + * Returns true if @set contains @fd, + * false otherwise. + */ +static bool +virCommandFDIsSet(int fd, + const int *set, + int set_size) +{ + int i = 0; + + while (i < set_size) + if (set[i++] == fd) + return true; + + return false; +} + +/* + * virCommandFDSet: + * @fd: FD to be put into @set + * @set: the set + * @set_size: actual size of @set + * + * This is practically generalized implementation + * of FD_SET() as we do not want to be limited + * by FD_SETSIZE. + * + * Returns true on success, false otherwise. + */ +static bool +virCommandFDSet(int fd, + int **set, + int *set_size) +{ + if (fd < 0 || !set || !set_size) + return false; + + if (virCommandFDIsSet(fd, *set, *set_size)) + return true; + + if (VIR_REALLOC_N(*set, *set_size + 1) < 0) { + virReportOOMError(); + return false; + } + + (*set)[*set_size] = fd; + (*set_size)++; + + return true; +} /** * virFork: @@ -303,7 +362,8 @@ prepareStdFd(int fd, int std) static int virExecWithHook(const char *const*argv, const char *const*envp, - const fd_set *keepfd, + const int *keepfd, + int keepfd_size, pid_t *retpid, int infd, int *outfd, int *errfd, unsigned int flags, @@ -430,7 +490,7 @@ virExecWithHook(const char *const*argv, for (i = 3; i < openmax; i++) { if (i == infd || i == childout || i == childerr) continue; - if (!keepfd || i >= FD_SETSIZE || !FD_ISSET(i, keepfd)) { + if (!keepfd || !virCommandFDIsSet(i, keepfd, keepfd_size)) { tmpfd = i; VIR_FORCE_CLOSE(tmpfd); } else if (virSetInherit(i, true) < 0) { @@ -619,7 +679,8 @@ virRun(const char *const *argv ATTRIBUTE_UNUSED, static int virExecWithHook(const char *const*argv ATTRIBUTE_UNUSED, const char *const*envp ATTRIBUTE_UNUSED, - const fd_set *keepfd ATTRIBUTE_UNUSED, + const int *keepfd ATTRIBUTE_UNUSED, + int keepfd_size ATTRIBUTE_UNUSED, pid_t *retpid ATTRIBUTE_UNUSED, int infd ATTRIBUTE_UNUSED, int *outfd ATTRIBUTE_UNUSED, @@ -687,8 +748,6 @@ virCommandNewArgs(const char *const*args) cmd->handshakeNotify[0] = -1; cmd->handshakeNotify[1] = -1; - FD_ZERO(&cmd->preserve); - FD_ZERO(&cmd->transfer); cmd->infd = cmd->outfd = cmd->errfd = -1; cmd->inpipe = -1; cmd->pid = -1; @@ -739,16 +798,16 @@ virCommandKeepFD(virCommandPtr cmd, int fd, bool transfer) if (!cmd) return fd > STDERR_FILENO; - if (fd <= STDERR_FILENO || FD_SETSIZE <= fd) { + if (fd <= STDERR_FILENO || + !virCommandFDSet(fd, &cmd->preserve, &cmd->preserve_size) || + (transfer && !virCommandFDSet(fd, &cmd->transfer, + &cmd->transfer_size))) { if (!cmd->has_error) cmd->has_error = -1; VIR_DEBUG("cannot preserve %d", fd); - return fd > STDERR_FILENO; + return true; } - FD_SET(fd, &cmd->preserve); - if (transfer) - FD_SET(fd, &cmd->transfer); return false; } @@ -2064,7 +2123,8 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid) ret = virExecWithHook((const char *const *)cmd->args, (const char *const *)cmd->env, - &cmd->preserve, + cmd->preserve, + cmd->preserve_size, &cmd->pid, cmd->infd, cmd->outfdptr, @@ -2077,13 +2137,11 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid) VIR_DEBUG("Command result %d, with PID %d", ret, (int)cmd->pid); - for (i = STDERR_FILENO + 1; i < FD_SETSIZE; i++) { - if (FD_ISSET(i, &cmd->transfer)) { - int tmpfd = i; - VIR_FORCE_CLOSE(tmpfd); - FD_CLR(i, &cmd->transfer); - } + for (i = 0; i < cmd->transfer_size; i++) { + VIR_FORCE_CLOSE(cmd->transfer[i]); } + cmd->transfer_size = 0; + VIR_FREE(cmd->transfer); if (ret == 0 && pid) *pid = cmd->pid; @@ -2448,11 +2506,8 @@ virCommandFree(virCommandPtr cmd) if (!cmd) return; - for (i = STDERR_FILENO + 1; i < FD_SETSIZE; i++) { - if (FD_ISSET(i, &cmd->transfer)) { - int tmpfd = i; - VIR_FORCE_CLOSE(tmpfd); - } + for (i = 0; i < cmd->transfer_size; i++) { + VIR_FORCE_CLOSE(cmd->transfer[i]); } VIR_FREE(cmd->inbuf); @@ -2482,5 +2537,8 @@ virCommandFree(virCommandPtr cmd) if (cmd->reap) virCommandAbort(cmd); + VIR_FREE(cmd->transfer); + VIR_FREE(cmd->preserve); + VIR_FREE(cmd); } -- 1.7.3.4

3 8

[libvirt] [PATCH v2][TCK] add test case for block job lifecycle testing
by Xiaoqiang Hu 05 Jan '12

05 Jan '12

v2: Add skip block for qemu only and 120s timeout for test v1: Add tests for block job lifecyle and the test flow is as follows: create 50M qed img with qed backing img-> block pull->abort block job->resume block pull->set block job speed-> wait to finish --- scripts/qemu/300-blockjob-lifecycle.t | 142 +++++++++++++++++++++++++++++++++ 1 files changed, 142 insertions(+), 0 deletions(-) create mode 100644 scripts/qemu/300-blockjob-lifecycle.t diff --git a/scripts/qemu/300-blockjob-lifecycle.t b/scripts/qemu/300-blockjob-lifecycle.t new file mode 100644 index 0000000..cdf604e --- /dev/null +++ b/scripts/qemu/300-blockjob-lifecycle.t @@ -0,0 +1,142 @@ +# -*- perl -*- +# +# Copyright (C) 2011-2012 Red Hat, Inc. +# Copyright (C) 2011 Xiaoqiang Hu <xhu(a)redhat.com> +# +# This program is free software; You can redistribute it and/or modify +# it under the GNU General Public License as published by the Free +# Software Foundation; either version 2, or (at your option) any +# later version +# +# The file "LICENSE" distributed along with this file provides full +# details of the terms and conditions +# + +=pod + +=head1 NAME + +qemu/300-blockjob-lifecycle.t - verify the lifecycle of block job: +block pull, set block job speed, get block job info and abort block job + +=head1 DESCRIPTION + +The test case validates that it is possible to block pull, set block job +speed, get block job info and abort block job for domain using qed img +with qed backing img + +=cut + +use strict; +use warnings; + +use Test::More tests => 16; + +use Sys::Virt::TCK; +use Test::Exception; +use File::Spec::Functions qw(catfile); +use File::stat; + +my $tck = Sys::Virt::TCK->new(); +my $conn = eval { $tck->setup(); }; +BAIL_OUT "failed to setup test harness: $@" if $@; +END { + $tck->cleanup if $tck; +} + +SKIP:{ + skip "Only relevant to QEMU driver", 16 unless $conn->get_type() eq "QEMU"; + my $xml = $tck->generic_pool("dir") + ->mode("0755")->as_xml; + + diag "Defining transient storage pool $xml"; + my $pool; + + ok_pool(sub { $pool = $conn->define_storage_pool($xml) }, "define transient storage pool"); + lives_ok(sub { $pool->build(0) }, "built storage pool"); + lives_ok(sub { $pool->create }, "started storage pool"); + + my $volbackxml = $tck->generic_volume("tck-back", "qed", 1024*1024*50)->allocation(0)->as_xml; + + my ($volback, $pathback); + diag "back $volbackxml"; + ok_volume(sub { $volback = $pool->create_volume($volbackxml) }, "create qed backing file volume"); + + my $st; + $pathback = xpath($volback, "string(/volume/target/path)"); + $st = stat($pathback); + + ok($st, "path $pathback exists"); + + ok($st->size < 1024*1024, "size is < 1M"); + + my $volmainxml = $tck->generic_volume("tck-main", "qed", 1024*1024*50) + ->backing_file($pathback) + ->backing_format("qed") + ->allocation(0)->as_xml; + + my ($volmain, $pathmain); + diag "main $volmainxml"; + ok_volume(sub { $volmain = $pool->create_volume($volmainxml) }, "create qed backing file volume"); + + $pathmain = xpath($volmain, "string(/volume/target/path)"); + $st = stat($pathmain); + + ok($st, "path $pathmain exists"); + + ok($st->size < 1024*1024, "size is < 1M"); + + # define the guest at a qed image + # and the backing store in this qed image. + $xml = $tck->generic_domain("tck") + ->disk(format => { name => "qemu", type => "qed" }, + type => "file", + src => $pathmain, + dst => "vdb") + ->as_xml; + + diag "Defining an inactive domain config $xml"; + my $dom; + ok_domain(sub { $dom = $conn->define_domain($xml) }, "defined persistent domain config"); + + diag "Starting inactive domain config"; + $dom->create; + ok($dom->get_id() > 0, "running domain has an ID > 0"); + + # start to block pull and bandwidth is 1MB/S + my ($bandwidth, $flags, $jobinfo, $timeout); + # 1MB/S + $bandwidth = 1; + $flags=0; + $dom->block_pull($pathmain, $bandwidth, $flags); + # $jobinfo is a hash reference summarising the execution state of the block job + # and it has four keys:cur, end, bandwidth, type + $jobinfo = $dom->get_block_job_info($pathmain, $flags); + ok($jobinfo->{bandwidth} == $bandwidth, "start to block pull and block job bandwidth is $bandwidth"."MB/S"); + + $dom->abort_block_job($pathmain, $flags); + $jobinfo = $dom->get_block_job_info($pathmain, $flags); + ok($jobinfo->{type} == 0, "abort block job"); + + $dom->block_pull($pathmain, $bandwidth, $flags); + $jobinfo = $dom->get_block_job_info($pathmain, $flags); + ok($jobinfo->{bandwidth} == $bandwidth, "continue to block pull and block job bandwidth is $bandwidth"."MB/S"); + + # set block job bandwidth to 2MB/S + $bandwidth = 2; + $dom->set_block_job_speed($pathmain, $bandwidth, $flags); + $jobinfo = $dom->get_block_job_info($pathmain, $flags); + ok($jobinfo->{bandwidth} == $bandwidth, "block job bandwidth is set to $bandwidth"."MB/S"); + + # wait for the end of block pull and timeout is 120s + $timeout = 120; + while($jobinfo->{cur} < $jobinfo->{end} && $jobinfo->{type} == 1 && $timeout > 0) { + sleep(1); + $jobinfo = $dom->get_block_job_info($pathmain, $flags); + $timeout--; + } + + diag "block pull is not finished in 120S" if $jobinfo->{type} == 1 && $timeout == 0; + ok($jobinfo->{type} == 0, "block pull is finished"); +} +# end -- 1.7.1

1 0

[libvirt] Implement gvir_config_domain_get_devices
by Christophe Fergeau 05 Jan '12

05 Jan '12

Hi, This is the second version of the patch series introducing gvir_config_domain_get_devices. The difference from the first version is the added 4/5 patch which adds a helper function to help with ignoring blank nodes while parsing the XML document. Christophe

2 10

[libvirt] Coverity automatic detection
by Alex Jia 05 Jan '12

05 Jan '12

This email is automatically generated. The test result is based on the following git commit: 49d8c8b Support Xen domctl v8 Analysis summary report: ------------------------ Files analyzed : 235 Total LoC input to cov-analyze : 330902 Functions analyzed : 8031 Paths analyzed : 940313 Defect occurrences found : 110 Total 7 ATOMICITY 5 CHECKED_RETURN 15 DEADCODE 6 FORWARD_NULL 13 LOCK 2 NEGATIVE_RETURNS 1 NULL_RETURNS 9 RESOURCE_LEAK 1 RETURN_LOCAL 12 REVERSE_INULL 3 SECURE_TEMP 2 STRING_NULL 12 TAINTED_SCALAR 4 TAINTED_STRING 12 TOCTOU 4 UNINIT 2 UNUSED_VALUE Exceeded path limit of 5000 paths in 0.47% of functions (normally up to 5% of functions encounter this limitation) For details, please see attachment. Regards, Alex

1 0

Re: [libvirt] update util-linux-ng -> util-linux
by Eric Blake 05 Jan '12

05 Jan '12

I saw this commit on the Fedora build of libvirt: On 12/29/2011 09:21 AM, Peter Robinson wrote: > commit f9de2f6bc6bb180f1511794b6a7fcaa06e451076 > Author: Peter Robinson <pbrobinson(a)gmail.com> > Date: Thu Dec 29 16:21:18 2011 +0000 > > update util-linux-ng -> util-linux > > libvirt.spec | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > --- > diff --git a/libvirt.spec b/libvirt.spec > index 0644e3a..bc062fc 100644 > --- a/libvirt.spec > +++ b/libvirt.spec > @@ -302,7 +302,7 @@ Requires: PolicyKit >= 0.6 > %if %{with_storage_fs} > Requires: nfs-utils > # For mkfs > -Requires: util-linux-ng > +Requires: util-linux I think it would be safer to make it conditional: depending on util-linux-ng if on fedora < N, and on util-linux for fedora >= N, but what's the right value of N? We should probably fold this into libvirt.spec.in before 0.9.9 is released. -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

1 0