Appending an item to a list transfers ownership of that item to the
list owner. But an error can occur in between item allocation and
appending it to the list. In this case the item has to be freed
explicitly. This was not done in some special cases resulting in
possible memory leaks.
Reported by Coverity.
(cherry picked from commit 3b9a12958d)
This patch lifts the limit of calling thread detection code only on KVM
guests. With upstream qemu the thread mappings are reported also on
non-KVM machines.
QEMU adopted the thread_id information from the kvm branch.
To remain compatible with older upstream versions of qemu the check is
attempted but the failure to detect threads (or even run the monitor
command - on older versions without SMP support) is treated non-fatal
and the code reports one vCPU with pid of the hypervisor (in same
fashion this was done on non-KVM guests).
(cherry picked from commit c833526924)
After a cpu hotplug the qemu driver did not refresh information about
virtual processors used by qemu and their corresponding threads. This
patch forces a re-detection as is done on start of QEMU.
This ensures that correct information is reported by the
virDomainGetVcpus API and "virsh vcpuinfo".
A failure to obtain the thread<->vcpu mapping is treated non-fatal and
the mapping is not updated in a case of failure as not all versions of
QEMU report this in the info cpus command.
(cherry picked from commit 3163682b58)
This patch changes a switch statement into ifs when handling live vs.
configuration modifications getting rid of redundant code in case when
both live and persistent configuration gets changed.
(cherry picked from commit e99ad93d02)
when failing to attach another usb device to a domain for some reason
which has one use device attached before, the libvirtd crashed.
The crash is caused by null-pointer dereference error in invoking
usbDeviceListSteal passed in NULL value usb variable.
commit 05abd1507d introduces the bug.
(cherry picked from commit ab5fb8f34c)
Commit 97010eb1f forgot to change the other side of an #ifdef.
* src/qemu/qemu_process.c (qemuProcessInitNumaMemoryPolicy): Add
argument.
(cherry picked from commit 5c650b98ce)
No useful error was being reported when an invalid character device
target type is specified in the domainXML. E.g.
...
<console type="pty">
<source path="/dev/pts/2"/>
<target type="kvm" port="0"/>
</console>
...
resulted in
error: Failed to define domain from x.xml
error: An error occurred, but the cause is unknown
With this small patch, the error is more helpful
error: Failed to define domain from x.xml
error: XML error: unknown target type 'kvm' specified for character device
(cherry picked from commit 10a87145f7)
It turns out that when cgroups are enabled, the use of a block device
for a snapshot target was failing with EPERM due to libvirt failing
to add the block device to the cgroup whitelist. See also
https://bugzilla.redhat.com/show_bug.cgi?id=810200
* src/qemu/qemu_driver.c
(qemuDomainSnapshotCreateSingleDiskActive)
(qemuDomainSnapshotUndoSingleDiskActive): Account for cgroup.
(qemuDomainSnapshotCreateDiskActive): Update caller.
(cherry picked from commit 8be304ecb9)
Until now, the nl_pid of the source address of every message sent by
virNetlinkCommand has been set to the value of getpid(). Most of the
time this doesn't matter, and in the one case where it does
(communication with lldpad), it previously was the proper thing to do,
because the netlink event service (which listens on a netlink socket
for unsolicited messages from lldpad) coincidentally always happened
to bind with a local nl_pid == getpid().
With the fix for:
https://bugzilla.redhat.com/show_bug.cgi?id=816465
that particular nl_pid is now effectively a reserved value, so the
netlink event service will always bind to something else
(coincidentally "getpid() + (1 << 22)", but it really could be
anything). The result is that communication between lldpad and
libvirtd is broken (lldpad gets a "disconnected" error when it tries
to send a directed message).
The solution to this problem caused by a solution, is to query the
netlink event service's nlhandle for its "local_port", and send that
as the source nl_pid (but only when sending to lldpad, of course - in
other cases we maintain the old behavior of sending getpid()).
There are two cases where a message is being directed at lldpad - one
in virNetDevLinkDump, and one in virNetDevVPortProfileOpSetLink.
The case of virNetDevVPortProfileOpSetLink is simplest to explain -
only if !nltarget_kernel, i.e. the message isn't targetted for the
kernel, is the dst_pid set (by calling
virNetDevVPortProfileGetLldpadPid()), so only in that case do we call
virNetlinkEventServiceLocalPid() to set src_pid.
For virNetDevLinkDump, it's a bit more complicated. The call to
virNetDevVPortProfileGetLldpadPid() was effectively up one level (in
virNetDevVPortProfileOpCommon), although obscured by an unnecessary
passing of a function pointer. This patch removes the function
pointer, and calls virNetDevVPortProfileGetLldpadPid() directly in
virNetDevVPortProfileOpCommon - if it's doing this, it knows that it
should also call virNetlinkEventServiceLocalPid() to set src_pid too;
then it just passes src_pid and dst_pid down to
virNetDevLinkDump. Since (src_pid == 0 && dst_pid == 0) implies that
the kernel is the destination, there is no longer any need to send
nltarget_kernel as an arg to virNetDevLinkDump, so it's been removed.
The disparity between src_pid being int and dst_pid being uint32_t may
be a bit disconcerting to some, but I didn't want to complicate
virNetlinkEventServiceLocalPid() by having status returned separately
from the value.
(cherry picked from commit cc0737713a)
This value will be needed to set the src_pid when sending netlink
messages to lldpad. It is part of the solution to:
https://bugzilla.redhat.com/show_bug.cgi?id=816465
Note that libnl's port generation algorithm guarantees that the
nl_socket_get_local_port() will always be > 0 (since it is "getpid() +
(n << 22>" where n is always < 1024), so it is okay to cast the
uint32_t to int (thus allowing us to use -1 as an error sentinel).
(cherry picked from commit c99e93758d)
Until now, virNetlinkCommand has assumed that the nl_pid in the source
address of outgoing netlink messages should always be the return value
of getpid(). In most cases it actually doesn't matter, but in the case
of communication with lldpad, lldpad saves this info and later uses it
to send netlink messages back to libvirt. A recent patch to fix Bug
816465 changed the order of the universe such that the netlink event
service socket is no longer bound with nl_pid == getpid(), so lldpad
could no longer send unsolicited messages to libvirtd. Adding src_pid
as an argument to virNetlinkCommand() is the first step in notifying
lldpad of the proper address of the netlink event service socket.
(cherry picked from commit cca7bb1fb5)
This is part of the solution to the problem detailed in:
https://bugzilla.redhat.com/show_bug.cgi?id=816465
and further detailed in
https://www.redhat.com/archives/libvir-list/2012-May/msg00202.htm
A short explanation is included in the comments of the patch itself.
Note that this patch by itself breaks communication between lldpad and
libvirtd, so the other 3 patches in the series must be applied at the
same time as this patch.
(cherry picked from commit 642973135c)
Conflicts:
daemon/libvirtd.c
src/qemu/qemu_hostdev.c:
refactor qemuPrepareHostdevUSBDevices function, make it focus on
adding usb device to activeUsbHostdevs after check. After that,
the usb hotplug function qemuDomainAttachHostDevice also could use
it.
expand qemuPrepareHostUSBDevices to perform the usb search,
rollback on failure.
src/qemu/qemu_hotplug.c:
If there are multiple usb devices available with same vendorID and productID,
but with different value of "bus, device", we give an error to let user
use <address> to specify the desired one.
(cherry picked from commit 05abd1507d)
usbFindDevice():get usb device according to
idVendor, idProduct, bus, device
it is the exact match of the four parameters
usbFindDeviceByBus():get usb device according to bus, device
it returns only one usb device same as usbFindDevice
usbFindDeviceByVendor():get usb device according to idVendor,idProduct
it probably returns multiple usb devices.
usbDeviceSearch(): a helper function to do the actual search
(cherry picked from commit 9914477efc)
When we added the default USB controller into domain XML, we efficiently
broke migration to older versions of libvirt that didn't support USB
controllers at all (0.9.4 and earlier) even for domains that don't use
anything that the older libvirt can't provide. We still want to present
the default USB controller in any XML seen by a user/app but we can
safely remove it from the domain XML used during migration. If we are
migrating to a new enough libvirt, it will add the controller XML back,
while older libvirt won't be confused with it although it will still
tell qemu to create the controller.
Similar approach can be used in the future whenever we find out we
always enabled some kind of device without properly advertising it in
domain XML.
(cherry picked from commit 409b5f5495)
Commit 4c82f09e added a capability check for qemu per-device io
throttling, but only applied it to domain startup. As mentioned
in the previous commit (98cec05), the user can still get an 'internal
error' message during a hotplug attempt, when the monitor command
doesn't exist. It is confusing to allow tuning on inactive domains
only to then be rejected when starting the domain.
* src/qemu/qemu_driver.c (qemuDomainSetBlockIoTune): Reject
offline tuning if online can't match it.
(cherry picked from commit 13f9a19326)
If you have a qemu build that lacks the blockio tune monitor command,
then this command:
$ virsh blkdeviotune rhel6u2 hda --total_bytes_sec 1000
error: Unable to change block I/O throttle
error: internal error Unexpected error
fails as expected (well, the error message is lousy), but the next
dumpxml shows that the domain was modified anyway. Worse, that means
if you save the domain then restore it, the restore will likely fail
due to throttling being unsupported, even though no throttling should
even be active because the monitor command failed in the first place.
* src/qemu/qemu_driver.c (qemuDomainSetBlockIoTune): Check for
error before making modification permanent.
(cherry picked from commit 98cec05288)
These two functions are called from main() on all platforms, and
always return success on platforms that don't support libnl. They
still log an error message, though, which doesn't make sense - they
should just be NOPs on those platforms. (Per a suggestion during
review, I've turned the logs into debug messages rather than removing
them completely).
(cherry picked from commit bae4ff282b)
Error: STRING_NULL:
/libvirt/src/node_device/node_device_linux_sysfs.c:80:
string_null_argument: Function "saferead" does not terminate string "*buf".
/libvirt/src/util/util.c:101:
string_null_argument: Function "read" fills array "*buf" with a non-terminated string.
/libvirt/src/node_device/node_device_linux_sysfs.c:87:
string_null: Passing unterminated string "buf" to a function expecting a null-terminated string.
(cherry picked from commit 43d1616ff5)
Error: STRING_NULL:
/libvirt/src/util/uuid.c:273:
string_null_argument: Function "getDMISystemUUID" does not terminate string "*dmiuuid".
/libvirt/src/util/uuid.c:241:
string_null_argument: Function "saferead" fills array "*uuid" with a non-terminated string.
/libvirt/src/util/util.c:101:
string_null_argument: Function "read" fills array "*buf" with a non-terminated string.
/libvirt/src/util/uuid.c:274:
string_null: Passing unterminated string "dmiuuid" to a function expecting a null-terminated string.
/libvirt/src/util/uuid.c:138:
var_assign_parm: Assigning: "cur" = "uuidstr". They now point to the same thing.
/libvirt/src/util/uuid.c:164:
string_null_sink_loop: Searching for null termination in an unterminated array "cur".
(cherry picked from commit b4586051ec)
Error: RESOURCE_LEAK:
/libvirt/src/qemu/qemu_driver.c:6968:
alloc_fn: Calling allocation function "calloc".
/libvirt/src/qemu/qemu_driver.c:6968:
var_assign: Assigning: "nodeset" = storage returned from "calloc(1UL, 1UL)".
/libvirt/src/qemu/qemu_driver.c:6977:
noescape: Variable "nodeset" is not freed or pointed-to in function "virTypedParameterAssign".
/libvirt/src/qemu/qemu_driver.c:6997:
leaked_storage: Variable "nodeset" going out of scope leaks the storage it points to.
(cherry picked from commit c0774482ff)
Error: RESOURCE_LEAK:
/libvirt/src/vmx/vmx.c:2431:
alloc_fn: Calling allocation function "calloc".
/libvirt/src/vmx/vmx.c:2431:
var_assign: Assigning: "networkName" = storage returned from "calloc(1UL, 1UL)".
/libvirt/src/vmx/vmx.c:2495:
leaked_storage: Variable "networkName" going out of scope leaks the storage it points to.
(cherry picked from commit 409a637eb1)
Error: RESOURCE_LEAK:
/builddir/build/BUILD/libvirt-0.9.10/src/nodeinfo.c:629: alloc_fn: Calling allocation function "fopen".
/builddir/build/BUILD/libvirt-0.9.10/src/nodeinfo.c:629: var_assign: Assigning: "cpuinfo" = storage returned from "fopen("/proc/cpuinfo", "r")".
/builddir/build/BUILD/libvirt-0.9.10/src/nodeinfo.c:638: leaked_storage: Variable "cpuinfo" going out of scope leaks the storage it points to.
(cherry picked from commit 739cfc3161)
Error: RESOURCE_LEAK:
/builddir/build/BUILD/libvirt-0.9.10/src/test/test_driver.c:1041: alloc_arg: Calling allocation function "virXPathNodeSet" on "devs".
/builddir/build/BUILD/libvirt-0.9.10/src/util/xml.c:621: alloc_arg: "virAllocN" allocates memory that is stored into "*list".
/builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:129: alloc_fn: Storage is returned from allocation function "calloc".
/builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:129: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(count, size)".
/builddir/build/BUILD/libvirt-0.9.10/src/util/xml.c:625: noescape: Variable "*list" is not freed or pointed-to in function "memcpy".
/builddir/build/BUILD/libvirt-0.9.10/src/test/test_driver.c:1098: leaked_storage: Variable "devs" going out of scope leaks the storage it points to.
(cherry picked from commit ad4d4ad036)
Coverity logs:
Error: RESOURCE_LEAK:
/builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_inotify.c:103: alloc_fn: Calling allocation function "xenDaemonLookupByUUID".
/builddir/build/BUILD/libvirt-0.9.10/src/xen/xend_internal.c:2534: alloc_fn: Storage is returned from allocation function "virGetDomain".
/builddir/build/BUILD/libvirt-0.9.10/src/datatypes.c:191: alloc_arg: "virAlloc" allocates memory that is stored into "ret".
/builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:101: alloc_fn: Storage is returned from allocation function "calloc".
/builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:101: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(1UL, size)".
/builddir/build/BUILD/libvirt-0.9.10/src/datatypes.c:210: return_alloc: Returning allocated memory "ret".
/builddir/build/BUILD/libvirt-0.9.10/src/xen/xend_internal.c:2534: var_assign: Assigning: "ret" = "virGetDomain(conn, name, uuid)".
/builddir/build/BUILD/libvirt-0.9.10/src/xen/xend_internal.c:2541: return_alloc: Returning allocated memory "ret".
/builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_inotify.c:103: var_assign: Assigning: "dom" = storage returned from "xenDaemonLookupByUUID(conn, rawuuid)".
/builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_inotify.c:126: leaked_storage: Variable "dom" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK:
/builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_hypervisor.c:2742: alloc_fn: Calling allocation function "fopen".
/builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_hypervisor.c:2742: var_assign: Assigning: "cpuinfo" = storage returned from "fopen("/proc/cpuinfo", "r")".
/builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_hypervisor.c:2763: noescape: Variable "cpuinfo" is not freed or pointed-to in function "xenHypervisorMakeCapabilitiesInternal".
/builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_hypervisor.c:2574:45: noescape: "xenHypervisorMakeCapabilitiesInternal" does not free or save its pointer parameter "cpuinfo".
/builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_hypervisor.c:2768: leaked_storage: Variable "cpuinfo" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK:
/builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_hypervisor.c:2752: alloc_fn: Calling allocation function "fopen".
/builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_hypervisor.c:2752: var_assign: Assigning: "capabilities" = storage returned from "fopen("/sys/hypervisor/properties/capabilities", "r")".
/builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_hypervisor.c:2763: noescape: Variable "capabilities" is not freed or pointed-to in function "xenHypervisorMakeCapabilitiesInternal".
/builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_hypervisor.c:2574:60: noescape: "xenHypervisorMakeCapabilitiesInternal" does not free or save its pointer parameter "capabilities".
/builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_hypervisor.c:2768: leaked_storage: Variable "capabilities" going out of scope leaks the storage it points to.
(cherry picked from commit e91e0ab604)
Coverity logs:
Error: RESOURCE_LEAK:
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:523: alloc_fn: Calling allocation function "fopen".
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:523: var_assign: Assigning: "fd" = storage returned from "fopen(local_file, "rb")".
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:540: noescape: Variable "fd" is not freed or pointed-to in function "fread".
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:542: noescape: Variable "fd" is not freed or pointed-to in function "feof".
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:575: leaked_storage: Variable "fd" going out of scope leaks the storage it points to.
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:585: leaked_storage: Variable "fd" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK:
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2088: alloc_fn: Calling allocation function "phypVolumeLookupByName".
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2026: alloc_fn: Storage is returned from allocation function "virGetStorageVol".
/builddir/build/BUILD/libvirt-0.9.10/src/datatypes.c:724: alloc_arg: "virAlloc" allocates memory that is stored into "ret".
/builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:101: alloc_fn: Storage is returned from allocation function "calloc".
/builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:101: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(1UL, size)".
/builddir/build/BUILD/libvirt-0.9.10/src/datatypes.c:753: return_alloc: Returning allocated memory "ret".
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2026: var_assign: Assigning: "vol" = "virGetStorageVol(pool->conn, pool->name, volname, key)".
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2030: return_alloc: Returning allocated memory "vol".
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2088: leaked_storage: Failing to save storage allocated by "phypVolumeLookupByName(pool, voldef->name)" leaks it.
Error: RESOURCE_LEAK:
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2725: alloc_fn: Calling allocation function "phypGetStoragePoolLookUpByUUID".
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2689: alloc_fn: Storage is returned from allocation function "virGetStoragePool".
/builddir/build/BUILD/libvirt-0.9.10/src/datatypes.c:592: alloc_arg: "virAlloc" allocates memory that is stored into "ret".
/builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:101: alloc_fn: Storage is returned from allocation function "calloc".
/builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:101: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(1UL, size)".
/builddir/build/BUILD/libvirt-0.9.10/src/datatypes.c:610: return_alloc: Returning allocated memory "ret".
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2689: var_assign: Assigning: "sp" = "virGetStoragePool(conn, pools[i], uuid)".
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2694: return_alloc: Returning allocated memory "sp".
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2725: leaked_storage: Failing to save storage allocated by "phypGetStoragePoolLookUpByUUID(conn, def->uuid)" leaks it.
Error: RESOURCE_LEAK:
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2719: alloc_fn: Calling allocation function "phypStoragePoolLookupByName".
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2254: alloc_fn: Storage is returned from allocation function "virGetStoragePool".
/builddir/build/BUILD/libvirt-0.9.10/src/datatypes.c:592: alloc_arg: "virAlloc" allocates memory that is stored into "ret".
/builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:101: alloc_fn: Storage is returned from allocation function "calloc".
/builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:101: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(1UL, size)".
/builddir/build/BUILD/libvirt-0.9.10/src/datatypes.c:610: return_alloc: Returning allocated memory "ret".
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2254: return_alloc_fn: Directly returning storage allocated by "virGetStoragePool".
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2719: leaked_storage: Failing to save storage allocated by "phypStoragePoolLookupByName(conn, def->name)" leaks it.
Error: RESOURCE_LEAK:
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2270: alloc_fn: Calling allocation function "phypStoragePoolLookupByName".
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2254: alloc_fn: Storage is returned from allocation function "virGetStoragePool".
/builddir/build/BUILD/libvirt-0.9.10/src/datatypes.c:592: alloc_arg: "virAlloc" allocates memory that is stored into "ret".
/builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:101: alloc_fn: Storage is returned from allocation function "calloc".
/builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:101: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(1UL, size)".
/builddir/build/BUILD/libvirt-0.9.10/src/datatypes.c:610: return_alloc: Returning allocated memory "ret".
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2254: return_alloc_fn: Directly returning storage allocated by "virGetStoragePool".
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2270: var_assign: Assigning: "sp" = storage returned from "phypStoragePoolLookupByName(vol->conn, vol->pool)".
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2324: leaked_storage: Variable "sp" going out of scope leaks the storage it points to.
/builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2327: leaked_storage: Variable "sp" going out of scope leaks the storage it points t
(cherry picked from commit cff0d342ad)
qemuDomainNetsRestart indents with 3 spaces.
This patch is to correct it.
Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com>
(cherry picked from commit 0d631e9182)
The ATTRIBUTE_NONNULL(m) macro normally resolves to the gcc builtin
__attribute__((__nonnull__(m))). The effect of this in gcc is
unfortunately only to make gcc believe that "m" can never possibly be
NULL, *not* to add in any checks to guarantee that it isn't ever NULL
(i.e. it is an optimization aid, *not* something to verify code
correctness.) - see the following gcc bug report for more details:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17308
Static source analyzers such as clang and coverity apparently can use
ATTRIBUTE_NONNULL(), though, to detect dead code (in the case that the
arg really is guaranteed non-NULL), as well as situations where an
obviously NULL arg is given to the function.
https://bugzilla.redhat.com/show_bug.cgi?id=815270 is a good example
of a bug caused by erroneous application of ATTRIBUTE_NONNULL().
Several people spent a long time staring at this code and not finding
the problem, because the problem wasn't in the function itself, but in
the prototype that specified ATTRIBUTE_NONNULL() for an arg that
actually *wasn't* always non-NULL, and caused a segv when dereferenced
(even though the code that dereferenced the pointer was inside an if()
that checked for a NULL pointer, that code was optimized out by gcc).
There may be some very small gain to be had from the optimizations
that can be inferred from ATTRIBUTE_NONNULL(), but it seems safer to
err on the side of generating code that behaves as expected, while
turning on the attribute for static analyzers.
(cherry picked from commit eefb881d46)
Once lxcContainerSetStdio is invoked, logging will not work as
expected in libvirt_lxc. So make sure this is the last thing to
be called, in particular after setting the security process label
(cherry picked from commit 07cf96ecc7)
The virLogSetFromEnv call was done too late in startup to
catch many log messages (eg from security driver initialization).
To assist debugging also explicitly log the security details
at startup
(cherry picked from commit 43ee987312)
The driver->securityDriverName field may be NULL, if automatic
probing is used to determine security driver. This meant that
unless selinux was explicitly requested in lxc.conf, it was
not being sent to the libvirt_lxc process.
The driver->securityManager field is guaranteed non-NULL, since
there will always be the 'none' security driver present if
nothing else exists. So use that to set the driver name for
libvirt_lxc
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit 3746b070e7)
Currently the libvirt_lxc process uses VIR_DOMAIN_XML_INACTIVE
when loading the XML for the container. This means it loses
any dynamic data such as the, just allocated, SELinux label.
Further there is an inconsistency in the libvirt LXC driver
whereby it saves the live config XML and then later overwrites
the file with the live status XML instead. Add a comment about
this for future reference.
* src/lxc/lxc_controller.c: Remove VIR_DOMAIN_XML_INACTIVE
when loading XML
* src/lxc/lxc_driver.c: Add comment about inconsistent
config file formats
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit eb06375a84)
This works with newer qemu that doesn't allow escaping spaces.
It's backwards compatible as well.
Signed-off-by: Josh Durgin <josh.durgin@dreamhost.com>
(cherry picked from commit d50cae3335)
In fact, the 'tapfd' is always NULL, the function 'virNetDevTapCreate()' hasn't
assign 'fd' to 'tapfd', when the function 'virNetDevSetMAC()' is failed then
goto 'error' label, finally, the VIR_FORCE_CLOSE() will deref a NULL 'tapfd'.
* util/virnetdevtap.c (virNetDevTapCreateInBridgePort): fix a NULL pointer derefing.
* How to reproduce?
$ cat > /tmp/net.xml <<EOF
<network>
<name>test</name>
<forward mode='nat'/>
<bridge name='br1' stp='off' delay='1' />
<mac address='00:00:00:00:00:00'/>
<ip address='192.168.100.1' netmask='255.255.255.0'>
<dhcp>
<range start='192.168.100.2' end='192.168.100.254' />
</dhcp>
</ip>
</network>
EOF
$ virsh net-define /tmp/net.xml
$ virsh net-start test
error: Failed to start network brTest
error: End of file while reading data: Input/output error
Signed-off-by: Alex Jia <ajia@redhat.com>
(cherry picked from commit 5ee18aaa57)
It doesn't break out the "for" loop even if duplicate pool is
found, and thus the "matchpool" could be overriden as NULL again
if there is different pool afterwards.
To address the problem in libvirt-user list:
https://www.redhat.com/archives/libvirt-users/2012-April/msg00150.html
(cherry picked from commit 980f12be72)
When libvirtd is started, we create "libvirt/qemu" directories under
hugetlbfs mount point. Only the "qemu" subdirectory is chowned to qemu
user and "libvirt" remains owned by root. If umask was too restrictive
when libvirtd started, qemu user may lose access to "qemu"
subdirectory. Let's explicitly grant search permissions to "libvirt"
directory for all users.
(cherry picked from commit 9d2ac5453e)
Currently, qemu GA is not providing 'desc' field for errors like
we are used to from qemu monitor. Therefore, we fall back to this
general 'unknown error' string. However, GA is reporting 'class' which
is not perfect, but much more helpful than generic error string.
Thus we should fall back to class firstly and if even no class
is presented, then we can fall back to that generic string.
Before this patch:
virsh # dompmsuspend --target mem f16
error: Domain f16 could not be suspended
error: internal error unable to execute QEMU command
'guest-suspend-ram': unknown QEMU command error
After this patch:
virsh # dompmsuspend --target mem f16
error: Domain f16 could not be suspended
error: internal error unable to execute QEMU command
'guest-suspend-ram': The command has not been found
(cherry picked from commit 378031088f)
More bug extermination in the category of:
Error: CHECKED_RETURN:
/libvirt/src/conf/network_conf.c:595:
check_return: Calling function "virAsprintf" without checking return value (as is done elsewhere 515 out of 543 times).
/libvirt/src/qemu/qemu_process.c:2780:
unchecked_value: No check of the return value of "virAsprintf(&msg, "was paused (%s)", virDomainPausedReasonTypeToString(reason))".
/libvirt/tests/commandtest.c:809:
check_return: Calling function "setsid" without checking return value (as is done elsewhere 4 out of 5 times).
/libvirt/tests/commandtest.c:830:
unchecked_value: No check of the return value of "virTestGetDebug()".
/libvirt/tests/commandtest.c:831:
check_return: Calling function "virTestGetVerbose" without checking return value (as is done elsewhere 41 out of 42 times).
/libvirt/tests/commandtest.c:833:
check_return: Calling function "virInitialize" without checking return value (as is done elsewhere 18 out of 21 times).
One note about the error in commandtest line 809: setsid() seems to fail when running the test -- could be removed ?
(cherry picked from commit 59b935f5ae)
Error: UNINIT:
/libvirt/src/lxc/lxc_driver.c:1412:
var_decl: Declaring variable "fd" without initializer.
/libvirt/src/lxc/lxc_driver.c:1460:
uninit_use_in_call: Using uninitialized value "fd" when calling "virFileClose".
/libvirt/src/util/virfile.c:50:
read_parm: Reading a parameter value.
Error: DEADCODE:
/libvirt/src/lxc/lxc_controller.c:960:
dead_error_condition: On this path, the condition "ret == 4" cannot be true.
/libvirt/src/lxc/lxc_controller.c:959:
at_most: After this line, the value of "ret" is at most -1.
/libvirt/src/lxc/lxc_controller.c:959:
new_values: Noticing condition "ret < 0".
/libvirt/src/lxc/lxc_controller.c:961:
dead_error_line: Execution cannot reach this statement "continue;".
Error: UNINIT:
/libvirt/src/lxc/lxc_controller.c:1104:
var_decl: Declaring variable "consoles" without initializer.
/libvirt/src/lxc/lxc_controller.c:1237:
uninit_use: Using uninitialized value "consoles".
(cherry picked from commit f74471de4f)
This reverts commit 372a14c673.
We shoul not have cherry-picked 28ae4ff0 without also cherry-picking
2223ea98, but the latter is too complex for a stable branch.
https://bugzilla.redhat.com/show_bug.cgi?id=816662 pointed out
that attempting 'virsh blockpull' on an offline domain gave a
misleading error message about qemu lacking support for the
operation, even when qemu was specifically updated to support it.
The real problem is that we have several capabilities that are
only determined when starting a domain, and therefore are still
clear when first working with an inactive domain (namely, any
capability set by qemuMonitorJSONCheckCommands).
While this patch was able to hoist an existing check in one of the
three culprits, it had to add redundant checks in the other two
places (because you always have to check for an active domain after
obtaining a VM job lock, but the capability bits were being checked
prior to obtaining the job lock).
Someday it would be nice to patch libvirt to cache the set of
capabilities per qemu binary (as determined by inode and timestamp),
rather than re-probing the binary every time a domain is started,
and to teach the cache how to query the monitor during the one
time the probe is made rather than having to wait until a guest
is started; then, a capability probe would succeed even for offline
guests because it just refers to the cache, and the single check for
an active domain after grabbing the job lock would be sufficient.
But since that will involve a lot more coding, I'm happy to go
with this simpler solution for an immediate solution.
* src/qemu/qemu_driver.c (qemuDomainPMSuspendForDuration)
(qemuDomainSnapshotCreateXML, qemuDomainBlockJobImpl): Check for
offline state before checking an online-only cap.
Conflicts:
src/qemu/qemu_driver.c
This patch addresses the following coverity findings:
/libvirt/src/conf/nwfilter_params.c:390:
var_assigned: Assigning: "varValue" = null return value from "virHashLookup".
/libvirt/src/conf/nwfilter_params.c:392:
dereference: Dereferencing a pointer that might be null "varValue" when calling "virNWFilterVarValueGetNthValue".
/libvirt/src/conf/nwfilter_params.c:399:
dereference: Dereferencing a pointer that might be null "tmp" when calling "virNWFilterVarValueGetNthValue".
This patch addresses the following coverity findings:
/libvirt/src/conf/nwfilter_params.c:157:
deref_parm: Directly dereferencing parameter "val".
/libvirt/src/conf/nwfilter_params.c:473:
negative_returns: Using variable "iterIndex" as an index to array "res->iter".
/libvirt/src/nwfilter/nwfilter_ebiptables_driver.c:2891:
unchecked_value: No check of the return value of "virAsprintf(&protostr, "-d 01:80:c2:00:00:00 ")".
/libvirt/src/nwfilter/nwfilter_ebiptables_driver.c:2894:
unchecked_value: No check of the return value of "virAsprintf(&protostr, "-p 0x%04x ", l3_protocols[protoidx].attr)".
/libvirt/src/nwfilter/nwfilter_ebiptables_driver.c:3590:
var_deref_op: Dereferencing null variable "inst".
Some of the error messages in this function should have been
virReportSystemError (since they have an errno they want to log), but
were mistakenly written as netlinkError, which expects a libvirt error
code instead. The result was that when one of the errors was
encountered, "No error message provided" would be printed instead of
something meaningful (see
https://bugzilla.redhat.com/show_bug.cgi?id=816465 for an example).
Once qemu monitor reports migration has completed, we just closed our
end of the pipe and let migration tunnel die. This generated bogus error
in case we did so before the thread saw EOF on the pipe and migration
was aborted even though it was in fact successful.
With this patch we first wake up the tunnel thread and once it has read
all data from the pipe and finished the stream we close the
filedescriptor.
A small additional bonus of this patch is that real errors reported
inside qemuMigrationIOFunc are not overwritten by virStreamAbort any
more.
When QEMU reported failed or canceled migration, we correctly detected
it but didn't really consider it as an error condition and migration
protocol just went on. Luckily, some of the subsequent steps eventually
failed end we reported an (unrelated and mostly random) error back to
the caller.
Currently, non-blocking calls are either sent immediately or discarded
in case sending would block. This was implemented based on the
assumption that the non-blocking keepalive call is not needed as there
are other calls in the queue which would keep the connection alive.
However, if those calls are no-reply calls (such as those carrying
stream data), the remote party knows the connection is alive but since
we don't get any reply from it, we think the connection is dead.
This is most visible in tunnelled migration. If it happens to be longer
than keepalive timeout (30s by default), it may be unexpectedly aborted
because the connection is considered to be dead.
With this patch, we only discard non-blocking calls when the last call
with a thread is completed and thus there is no thread left to keep
sending the remaining non-blocking calls.
In some cases (spotted with broken connection during tunneled migration)
we were overwriting the original error with worse or even misleading
errors generated when we were cleaning up after failed migration.
This patch resolves https://bugzilla.redhat.com/show_bug.cgi?id=815270
The function virNetDevMacVLanVPortProfileRegisterCallback() takes an
arg "virtPortProfile", and was checking it for non-NULL before using
it. However, the prototype for
virNetDevMacVLanPortProfileRegisterCallback had marked that arg with
ATTRIBUTE_NONNULL(). Contrary to what one may think,
ATTRIBUTE_NONNULL() does not provide any guarantee that an arg marked
as such really is always non-null; the only effect to the code
generated by gcc, is that gcc *assumes* it is non-NULL; this results
in, for example, the check for a non-NULL value being optimized out.
(Unfortunately, this code removal only occurs when optimization is
enabled, and I am in the habit of doing local builds with optimization
off to ease debugging, so the bug didn't show up in my earlier local
testing).
In general, virPortProfile might always be NULL, so it shouldn't be
marked as ATTRIBUTE_NONNULL. One other function prototype made this
same error, so this patch fixes it as well.
vboxArray is not castable to a COM item type. vboxArray is a
wrapper around the XPCOM and MSCOM specific array handling.
In this case we can avoid passing NULL as an empty array to
IMachine::Delete by passing a dummy IMedium* array with a single
NULL item.
virThreadSelf tries to access the virThreadPtr stored in TLS for the
current thread via TlsGetValue. When virThreadSelf is called on a thread
that was not created via virThreadCreate (e.g. the main thread) then
TlsGetValue returns NULL as TlsAlloc initializes TLS slots to NULL.
virThreadSelf can be called on the main thread via this call chain from
virsh
vshDeinit
virEventAddTimeout
virEventPollAddTimeout
virEventPollInterruptLocked
virThreadIsSelf
triggering a segfault as virThreadSelf unconditionally dereferences the
return value of TlsGetValue.
Fix this by making virThreadSelf check the TLS slot value for NULL and
setting the given virThreadPtr accordingly.
Reported by Marcel Müller.
POSIX says that sa_sigaction is only safe to use if sa_flags
includes SA_SIGINFO; conversely, sa_handler is only safe to
use when flags excludes that bit. Gnulib doesn't guarantee
an implementation of SA_SIGINFO, but does guarantee that
if SA_SIGINFO is undefined, we can safely define it to 0 as
long as we don't dereference the 2nd or 3rd argument of
any handler otherwise registered via sa_sigaction.
Based on a report by Wen Congyang.
* src/rpc/virnetserver.c (SA_SIGINFO): Stub for mingw.
(virNetServerSignalHandler): Avoid bogus dereference.
(virNetServerFatalSignal, virNetServerNew): Set flags properly.
(virNetServerAddSignalHandler): Drop unneeded #ifdef.
https://bugzilla.redhat.com/show_bug.cgi?id=617711 reported that
even with my recent patched to allow <memory unit='G'>1</memory>,
people can still get away with trying <memory>1G</memory> and
silently get <memory unit='KiB'>1</memory> instead. While
virt-xml-validate catches the error, our C parser did not.
Not to mention that it's always fun to fix bugs while reducing
lines of code. :)
* src/conf/domain_conf.c (virDomainParseMemory): Check for parse error.
(virDomainDefParseXML): Avoid strtoll.
* src/conf/storage_conf.c (virStorageDefParsePerms): Likewise.
* src/util/xml.c (virXPathLongBase, virXPathULongBase)
(virXPathULongLong, virXPathLongLong): Likewise.
Commit 78345c68 makes at least gcc 4.1.2 on RHEL 5 complain:
cc1: warnings being treated as errors
In file included from vbox/vbox_V4_0.c:13:
vbox/vbox_tmpl.c: In function 'vboxDomainUndefineFlags':
vbox/vbox_tmpl.c:5298: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
* src/vbox/vbox_tmpl.c (vboxDomainUndefineFlags): Use union to
avoid compiler warning.
Currently upon a migration a callback is created when a 802.1qbg link
is set to PREASSOCIATE, this should not happen because this is a no-op
on most switches, and does not lead to an ASSOCIATE state. This patch
only creates callbacks when CREATE or RESTORE is requested. Migration
and libvirtd restart scenarios are already handled elsewhere.
Signed-off-by: D. Herrendoerfer <d.herrendoerfer@herrendoerfer.name>
The below patch fixes the following memory leak.
==20624== 24 bytes in 2 blocks are definitely lost in loss record 532 of 1,867
==20624== at 0x4A05E46: malloc (vg_replace_malloc.c:195)
==20624== by 0x38EC27FC01: strdup (strdup.c:43)
==20624== by 0x4EB6BA3: virDomainChrSourceDefCopy (domain_conf.c:1122)
==20624== by 0x495D76: qemuProcessFindCharDevicePTYs (qemu_process.c:1497)
==20624== by 0x498321: qemuProcessWaitForMonitor (qemu_process.c:1258)
==20624== by 0x49B5F9: qemuProcessStart (qemu_process.c:3652)
==20624== by 0x468B5C: qemuDomainObjStart (qemu_driver.c:4753)
==20624== by 0x469171: qemuDomainStartWithFlags (qemu_driver.c:4810)
==20624== by 0x4F21735: virDomainCreate (libvirt.c:8153)
==20624== by 0x4302BF: remoteDispatchDomainCreateHelper (remote_dispatch.h:852)
==20624== by 0x4F72C14: virNetServerProgramDispatch (virnetserverprogram.c:416)
==20624== by 0x4F6D690: virNetServerHandleJob (virnetserver.c:164)
==20624== by 0x4E8F43D: virThreadPoolWorker (threadpool.c:144)
==20624== by 0x4E8EAB5: virThreadHelper (threads-pthread.c:161)
==20624== by 0x38EC606CCA: start_thread (pthread_create.c:301)
==20624== by 0x38EC2E0C2C: clone (clone.S:115)
So that a domain xml which doesn't have "placement" specified, but
"cpuset" is specified, could be parsed. And in this case, the
"placement" mode will be set as "static".
If console[0] is an alias for serial[0], do not enforce the former to
have a PTY source type. This breaks serial consoles on stdio and makes
no sense.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
When using the xm/xend stack to manage instances there is a bug
that causes the emulated interfaces to be unusable when the vif
config contains type=ioemu.
The current code already has a special quirk to not use this
keyword if no specific model is given for the emulated NIC
(defaulting to rtl8139).
Essentially it works because regardless of the type argument,i
the Xen stack always creates emulated and paravirt interfaces and
lets the guest decide which one to use. So neither xl nor xm stack
actually require the type keyword for emulated NICs.
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
lvcreate want's the parent pool's name, not the pool path
lvchange and lvremove want lv specified as $vgname/$lvname
This largely worked before because these commands strip off a
starting /dev. But https://bugzilla.redhat.com/show_bug.cgi?id=714986
is from a user using a 'nested VG' that was having problems.
I couldn't find any info on nested LVM and the reporter never responded,
but I reproduced with XML that specified a valid source name, and
set target path to a symlink.
As explained in previous patch, numad will balance the affinity
dynamically, so reflecting the cpuset from numad at the first
time doesn't make much case, and may just could cause confusion.
(cherry picked from commit 8fb2164cff)
Instead of returning a CPUs list, numad returns NUMA node
list instead, this patch is to convert the node list to
cpumap before affinity setting. Otherwise, the domain
processes will be pinned only to CPU[$numa_cell_num],
which will cause significiant performance losses.
Also because numad will balance the affinity dynamically,
reflecting the cpuset from numad back doesn't make much
sense then, and it may just could produce confusion for
the users. Thus the better way is not to reflect it back
to XML. And in this case, it's better to ignore the cpuset
when parsing XML.
The codes to update the cpuset is removed in this patch
incidentally, and there will be a follow up patch to ignore
the manually specified "cpuset" if "placement" is "auto",
and document will be updated too.
(cherry picked from commit ccf80e3630)
The linux-2.6.32 kernel header does not yet define IFLA_VF_MAX and others,
which breaks compiling a new libvirt on old systems like Debian Squeeze.
(I also have to add --without-macvtap --disable-werror --without-virtualport to
./configure to get it to compile.)
Signed-off-by: Philipp Hahn <hahn@univention.de>
(cherry picked from commit d7451bddc5)
Although it should be harmless to do:
disk = disk = def->disks[i]
some not-so-wise compilers may fool around.
Besides, such assignment is useless here.
(cherry picked from commit e14d6571c1)
On newer xend (v3.x and after) there is no state and domid reported
for inactive domains. When initially creating connections this is
handled in various places by assigning domain->id = -1.
But once an instance has been running, the id is set to the current
domain id. And it does not change when the instance is shut down.
So when querying the domain info, the hypervisor driver, which gets
asked first will indicate it cannot find information, then the
xend driver is asked and will set the status to NOSTATE because it
checks for the -1 domain id.
Checking domain/status for 0 seems to be more reliable for that.
One note: I am not sure whether the domain->id also should get set
back to -1 whenever any sub-driver thinks the instance is no longer
running.
BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=746007
BugLink: http://bugs.launchpad.net/bugs/929626
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
(cherry picked from commit 26e9ef4762)
This patch adds a netlink callback when migrating a VEPA enabled
virtual machine. It fixes a Bug where a VM would not request a port
association when it was cleared by lldpad.
This patch requires the latest git version of lldpad to work.
Signed-off-by: D. Herrendoerfer <d.herrendoerfer@herrendoerfer.name>
(cherry picked from commit 997366ca7d)
If dynamic_ownership is off and we are creating a file on NFS
we force chown. This will fail as chown/chmod are not supported
on NFS. However, with no dynamic_ownership we are not required
to do any chown.
(cherry picked from commit b1256816ff)
The new safe console handling introduced a possibility to deadlock the
qemu driver when a new console connection forcibly disconnects a
previous console stream that belongs to an already closed connection.
The virStreamFree function calls subsequently a the virReleaseConnect
function that tries to lock the driver while discarding the connection,
but the driver was already locked in qemuDomainOpenConsole.
Backtrace of the deadlocked thread:
0 0x00007f66e5aa7f14 in __lll_lock_wait () from /lib64/libpthread.so.0
1 0x00007f66e5aa3411 in _L_lock_500 () from /lib64/libpthread.so.0
2 0x00007f66e5aa322a in pthread_mutex_lock () from/lib64/libpthread.so.0
3 0x0000000000462bbd in qemudClose ()
4 0x00007f66e6e178eb in virReleaseConnect () from/usr/lib64/libvirt.so.0
5 0x00007f66e6e19c8c in virUnrefStream () from /usr/lib64/libvirt.so.0
6 0x00007f66e6e3d1de in virStreamFree () from /usr/lib64/libvirt.so.0
7 0x00007f66e6e09a5d in virConsoleHashEntryFree () from/usr/lib64/libvirt.so.0
8 0x00007f66e6db7282 in virHashRemoveEntry () from/usr/lib64/libvirt.so.0
9 0x00007f66e6e09c4e in virConsoleOpen () from /usr/lib64/libvirt.so.0
10 0x00000000004526e9 in qemuDomainOpenConsole ()
11 0x00007f66e6e421f1 in virDomainOpenConsole () from/usr/lib64/libvirt.so.0
12 0x00000000004361e4 in remoteDispatchDomainOpenConsoleHelper ()
13 0x00007f66e6e80375 in virNetServerProgramDispatch () from/usr/lib64/libvirt.so.0
14 0x00007f66e6e7ae11 in virNetServerHandleJob () from/usr/lib64/libvirt.so.0
15 0x00007f66e6da897d in virThreadPoolWorker () from/usr/lib64/libvirt.so.0
16 0x00007f66e6da7ff6 in virThreadHelper () from/usr/lib64/libvirt.so.0
17 0x00007f66e5aa0c5c in start_thread () from /lib64/libpthread.so.0
18 0x00007f66e57e7fcd in clone () from /lib64/libc.so.6
* src/qemu/qemu_driver.c: qemuDomainOpenConsole()
-- unlock the qemu driver right after acquiring the domain
object
(cherry picked from commit 3d3de46a67)
I noticed these compiler warnings when building for the s390 architecture.
* src/node_device/node_device_udev.c (udevDeviceMonitorStartup):
Mark unused variable.
* src/nodeinfo.c (linuxNodeInfoCPUPopulate): Avoid unused variable.
(cherry picked from commit 9011a494ac)
Leak introduced in commit 0436d32. If we allocate an actions array,
but fail early enough to never consume it with the qemu monitor
transaction call, we leaked memory.
But our semantics of making the transaction command free the caller's
memory is awkward; avoiding the memory leak requires making every
intermediate function in the call chain check for error. It is much
easier to fix things so that the function that allocates also frees,
while the call chain leaves the caller's data intact. To do that,
I had to hack our JSON data structure to make it easy to protect a
portion of an arbitrary JSON tree from being freed.
* src/util/json.h (virJSONType): Name the enum.
(_virJSONValue): New field.
* src/util/json.c (virJSONValueFree): Use it to protect a portion
of an array.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONTransaction): Avoid
freeing caller's data.
* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateDiskActive):
Free actions array on failure.
(cherry picked from commit 1413560966)
We can tell qemuDomainSnapshotFSThaw if we want it to report errors or
not. However, if we don't want to and an error has been already set by
previous qemuReportError() we must keep copy of that error not just a
pointer to it. Otherwise, it get overwritten if FSThaw reports an error.
(cherry picked from commit 650da0e99c)
Detected by valgrind. Leaks are introduced in commit b22eaa7.
* src/conf/domain_conf.c (virDomainDiskDefParseXML): fix memory leaks.
How to reproduce?
% make && make -C tests check TESTS=qemuxml2argvtest
% cd tests && valgrind -v --leak-check=full ./qemuxml2argvtest
actual result:
==2143== 12 bytes in 2 blocks are definitely lost in loss record 74 of 179
==2143== at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==2143== by 0x39D90A67DD: xmlStrndup (xmlstring.c:45)
==2143== by 0x4F5EC0: virDomainDiskDefParseXML (domain_conf.c:3438)
==2143== by 0x502F00: virDomainDefParseXML (domain_conf.c:8304)
==2143== by 0x505FE3: virDomainDefParseNode (domain_conf.c:9080)
==2143== by 0x5069AE: virDomainDefParse (domain_conf.c:9030)
==2143== by 0x41CBF4: testCompareXMLToArgvHelper (qemuxml2argvtest.c:105)
==2143== by 0x41E5DD: virtTestRun (testutils.c:145)
==2143== by 0x416FA3: mymain (qemuxml2argvtest.c:399)
==2143== by 0x41DCB7: virtTestMain (testutils.c:700)
==2143== by 0x39CF01ECDC: (below main) (libc-start.c:226)
Signed-off-by: Alex Jia <ajia@redhat.com>
(cherry picked from commit 80d476a92f)
If the daemon is restarted it will lose list of active
USB devices assigned to active domains. Therefore we need
to rebuild this list on qemuProcessReconnect().
(cherry picked from commit ea3bc548ac)
To prevent assigning one USB device to two domains,
we keep a list of assigned USB devices. On domain
startup - qemuProcessStart() - we insert devices
used by domain into the list but remove them only
on detach-device. Devices are, however, released
on qemuProcessStop() as well.
(cherry picked from commit e2f5dd6134)
Originally, qemuDomainCheckEjectableMedia was entering monitor with qemu
driver lock. Commit 2067e31bf9, which I
made to fix that, revealed another issue we had (but didn't notice it
since the driver was locked): we didn't set nested job when
qemuDomainCheckEjectableMedia is called during migration. Thus the
original fix I made was wrong.
XenD-3.1 introduced managed domains. HV-domains have rtc_timeoffset
(hgd24f37b31030 from 2007-04-03), which tracks the offset between the
hypervisors clock and the domains RTC, and is persisted by XenD.
In combination with localtime=1 this had a bug until XenD-3.4
(hg5d701be7c37b from 2009-04-01) (I'm not 100% sure how that bug
manifests, but at least for me in TZ=Europe/Berlin I see the previous
offset relative to utc being applied to localtime again, which manifests
in an extra hour being added)
XenD implements the following variants for clock/@offset:
- PV domains don't have a RTC → 'localtime' | 'utc'
- <3.1: no managed domains → 'localtime' | 'utc'
- ≥3.1: the offset is tracked for HV → 'variable'
due to the localtime=1 bug → 'localtime' | 'utc'
- ≥3.4: the offset is tracked for HV → 'variable'
Current libvirtd still thinks XenD only implements <clock offset='utc'/>
and <clock offset='localtime'/>, which is wrong, since the semantic of
'utc' and 'localtime' specifies, that the offset will be reset on
domain-restart, while with 'variable' the offset is kept. (keeping the
offset over "virsh edit" is important, since otherwise the clock might
jump, which confuses certain guest OSs)
xendConfigVersion was last incremented to 4 by the xen-folks for
xen-3.1.0. I know of no way to reliably detect the version of XenD
(user space tools), which may be different from the version of the
hypervisor (kernel) version! Because of this only the change from
'utc'/'localtime' to 'variable' in XenD-3.1 is handled, not the buggy
behaviour of XenD-3.1 until XenD-3.4.
For backward compatibility with previous versions of libvirt Xen-HV
still accepts 'utc' and 'localtime', but they are returned as 'variable'
on the next read-back from Xend to libvirt, since this is what XenD
implements: The RTC is NOT reset back to the specified time on next
restart, but the previous offset is kept.
This behaviour can be turned off by adding the additional attribute
adjustment='reset', in which case libvirt will report an error instead
of doing the conversion. The attribute can also be used as a shortcut to
offset='variable' with basis='...'.
With these changes, it is also necessary to adjust the xen tests:
"localtime = 0" is always inserted, because otherwise on updates the
value is not changed within XenD.
adjustment='reset' is inserted for all cases, since they're all <
XEND_CONFIG_VERSION_3_1_0, only 3.1 introduced persistent
rtc_timeoffset.
Some statements change their order because code was moved around.
Signed-off-by: Philipp Hahn <hahn@univention.de>
Since Xen 3.1 the clock=variable semantic is supported. In addition to
qemu/kvm Xen also knows about a variant where the offset is relative to
'localtime' instead of 'utc'.
Extends the libvirt structure with a flag 'basis' to specify, if the
offset is relative to 'localtime' or 'utc'.
Extends the libvirt structure with a flag 'reset' to force the reset
behaviour of 'localtime' and 'utc'; this is needed for backward
compatibility with previous versions of libvirt, since they report
incorrect XML.
Adapt the only user 'qemu' to the new name.
Extend the RelaxNG schema accordingly.
Document the new 'basis' attribute in the HTML documentation.
Adapt test for the new attribute.
Signed-off-by: Philipp Hahn <hahn@univention.de>
https://bugzilla.redhat.com/show_bug.cgi?id=808979
The leak is really in virProcessInfoGetAffinity, as shown in the
valgrind output given in the above bug report - it calls CPU_ALLOC(),
but then fails to call CPU_FREE().
This leak has existed in every version of libvirt since 0.7.5.
Commit 1b1402b introduced a regression. Since older libvirt versions
would silently round memory up (until the previous patch), but populated
current memory based on querying the guest, it was possible to have
dumpxml show cur > max by the amount of the rounding. For example, if
a user requested 1048570 KiB memory (just shy of 1GiB), the qemu
driver would actually run with 1048576 KiB, and libvirt 0.9.10 would
output a current that was 6KiB larger than the maximum. Situations
where this could have an impact include, but are not limited to,
migration from old to new libvirt, managedsave in old libvirt and
start in new libvirt, snapshot creation in old libvirt and revert in
new libvirt - without this patch, the new libvirt would reject the
VM because of the rounding discrepancy.
Fix things by adding a fuzz factor, and silently clamp current down to
maximum in that case, rather than failing to reparse XML for an existing
VM. From a practical standpoint, this has no user impact: 'virsh
dumpxml' will continue to query the running guest rather than rely on
the incoming xml, which will see the currect current value, and even if
clamping down occurs during parsing, it will be by at most the fuzz
factor of a megabyte alignment, and rounded back up when passed back to
the hypervisor.
Meanwhile, we continue to reject cur > max if the difference is beyond
the fuzz factor of nearest megabyte. But this is not a real change in
behavior, since with 0.9.10, even though the parser allowed it, later
in the processing stream we would reject it at the qemu layer; so
rejecting it in the parser just moves error detection to a nicer place.
* src/conf/domain_conf.c (virDomainDefParseXML): Don't reject
existing XML.
Based on a report by Zhou Peng.
If we round up a user's memory request, we should update the XML
to reflect the actual value in use by the VM, rather than giving
an artificially small value back to the user.
* src/qemu/qemu_command.c (qemuBuildNumaArgStr)
(qemuBuildCommandLine): Reflect rounding back to XML.
This patch was created to resolve this upstream bug:
https://bugzilla.redhat.com/show_bug.cgi?id=784767
and is at least a partial solution to this RHEL RFE:
https://bugzilla.redhat.com/show_bug.cgi?id=805071
Previously the only attribute of a network device that could be
modified by virUpdateDeviceFlags() ("virsh update-device") was the
link state; attempts to change any other attribute would log an error
and fail.
This patch adds recognition of a change in bridge device name, and
supports reconnecting the guest's interface to the new device.
Standard audit logs for detaching and attaching a network device are
also generated. Although the current auditing function doesn't log the
bridge being attached to, this will later be changed in a separate
patch.
