Commit 2025356 missed uses of PCI functions in the older HAL-related
code, probably because hal-devel is no longer available in latest Fedora.
* src/node_device/node_device_hal.c (gather_pci_cap): Reflect
function rename.
We had an easy way to iterate set bits, but not for iterating
cleared bits.
* src/util/virbitmap.h (virBitmapNextClearBit): New prototype.
* src/util/virbitmap.c (virBitmapNextClearBit): Implement it.
* src/libvirt_private.syms (bitmap.h): Export it.
* tests/virbitmaptest.c (test4): Test it.
The conditional setting of cmdout in networkBuildDhcpDaemonCommandLine()
caused Coverity to complain that 'cmd' could be leaked if !cmdout. Since
the function is local and only called with cmdout being passed those checks
have been removed.
Currently the activePciHostdevs, inactivePciHostdevsd and
activeUsbHostdevs lists are all implicitly protected by the
QEMU driver lock. Now that the lists all inherit from the
virObjectLockable, we can make the locking explicit, removing
the dependency on the QEMU driver lock for correctness.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
To allow modifications to the lists to be synchronized, convert
virPCIDeviceList and virUSBDeviceList into virObjectLockable
classes. The locking, however, will not be self-contained. The
users of these classes will have to call virObjectLock/Unlock
in the critical regions.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When iterating over USB host devices to setup cgroups, the
usbDevice object was leaked in both LXC and QEMU driers
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The QEMU driver struct has a 'qemuVersion' field that was previously
used to cache the version lookup from capabilities. With the recent
QEMU capabilities rewrite the caching happens at a lower level so
this field is pointless. Removing it avoids worries about locking
when updating it.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Switch virDomainObjList to inherit from virObjectLockable and
make all the APIs acquire/release the mutex when running. This
makes virDomainObjList completely self-locking and no longer
reliant on the hypervisor driver locks
The duplicate VM checking should be done atomically with
virDomainObjListAdd, so shoud not be a separate function.
Instead just use flags to indicate what kind of checks are
required.
This pair, used in virDomainCreateXML:
if (virDomainObjListIsDuplicate(privconn->domains, def, 1) < 0)
goto cleanup;
if (!(dom = virDomainObjListAdd(privconn->domains,
privconn->caps,
def, false)))
goto cleanup;
Changes to
if (!(dom = virDomainObjListAdd(privconn->domains,
privconn->caps,
def,
VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
NULL)))
goto cleanup;
This pair, used in virDomainRestoreFlags:
if (virDomainObjListIsDuplicate(privconn->domains, def, 1) < 0)
goto cleanup;
if (!(dom = virDomainObjListAdd(privconn->domains,
privconn->caps,
def, true)))
goto cleanup;
Changes to
if (!(dom = virDomainObjListAdd(privconn->domains,
privconn->caps,
def,
VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
NULL)))
goto cleanup;
This pair, used in virDomainDefineXML:
if (virDomainObjListIsDuplicate(privconn->domains, def, 0) < 0)
goto cleanup;
if (!(dom = virDomainObjListAdd(privconn->domains,
privconn->caps,
def, false)))
goto cleanup;
Changes to
if (!(dom = virDomainObjListAdd(privconn->domains,
privconn->caps,
def,
0, NULL)))
goto cleanup;
Otherwise, we get a lot of scary (but harmless) noise in the logs:
2013-02-05 15:35:48.555+0000: 8637: error : qemuMonitorJSONCheckError:353 : internal error unable to execute QEMU command 'add-fd': Parameter 'fdset-id' expects an existing fdset-id
one for every qemu 1.2 binary that we probe.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONAddFd): During
probe, avoid logging failures.
As a step towards making virDomainObjList thread-safe turn it
into an opaque virObject, preventing any direct access to its
internals.
As part of this a new method virDomainObjListForEach is
introduced to replace all existing usage of virHashForEach
Currently the virQEMUDriverPtr struct contains an wide variety
of data with varying access needs. Move all the static config
data into a dedicated virQEMUDriverConfigPtr object. The only
locking requirement is to hold the driver lock, while obtaining
an instance of virQEMUDriverConfigPtr. Once a reference is held
on the config object, it can be used completely lockless since
it is immutable.
NB, not all APIs correctly hold the driver lock while getting
a reference to the config object in this patch. This is safe
for now since the config is never updated on the fly. Later
patches will address this fully.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
If a compression binary prints something to stderr, currently
it is discarded. However, it can contain useful data from
debugging POV, so we should catch it.
If a decompression binary prints something to stderr, currently
it is discarded. However, it can contain useful data from
debugging POV, so we should catch it.
Commit 34e8f63a32 introduced support for catching errors from
libvirt iohelper. However, at those times there wasn't such fancy
API as virCommandDoAsyncIO(), so everything has to be implemented
on our own. But since we do have the API now, we can use it and
drop our implementation then.
Currently, if we want to feed stdin, or catch stdout or stderr of a
virCommand we have to use virCommandRun(). When using virCommandRunAsync()
we have to register FD handles by hand. This may lead to code duplication.
Hence, introduce an internal API, which does this automatically within
virCommandRunAsync(). The intended usage looks like this:
virCommandPtr cmd = virCommandNew*(...);
char *buf = NULL;
...
virCommandSetOutputBuffer(cmd, &buf);
virCommandDoAsyncIO(cmd);
if (virCommandRunAsync(cmd, NULL) < 0)
goto cleanup;
...
if (virCommandWait(cmd, NULL) < 0)
goto cleanup;
/* @buf now contains @cmd's stdout */
VIR_DEBUG("STDOUT: %s", NULLSTR(buf));
...
cleanup:
VIR_FREE(buf);
virCommandFree(cmd);
Note, that both stdout and stderr buffers may change until virCommandWait()
returns.
libvirt.c calls curl_global_init() if WITH_CURL is defined and thus it
should be linked with libcurl. This fixes link failure in case neither
xenapi nor esx driver is enabled (they are the only users of libcurl).
QEMU is fully capable of handling VDI images and we just refuse to
work with them. As qemu-img knows and supports this, there should be
no problem with this addition.
This is of course, just basic functionality, without searching for any
backing files, etc.
Some files have the magic shifted to some offset other than 0, so we
have to support that. I also cleaned up some lines to be more
readable and added missing magic for iso file format.
Commit 4445e16bfa changed the signature
of esxConnectToHost and esxConnectToVCenter by replacing the esxPrivate
pointer with virConnectPtr. The esxPrivate pointer was then retrieved
again from virConnectPtr's privateData. This resulted in a NULL pointer
dereference, because the privateData pointer was not yet initialized at
the point where esxConnectToHost and esxConnectToVCenter are called.
This was fixed in commit b126715a48 that
moved the initialization of privateData before the problematic calls.
Simplify the logic by making the call to esxFreePrivate unconditional and
changing esxConnectToHost and esxConnectToVCenter back to take a esxPrivate
pointer directly. This allows to assign esxPrivate to the virConnectPtr's
privateData pointer as one of the last steps in esxOpen making it more
obvious that it is not initialized during the earlier steps of esxOpen.
Commit 6094ad7b (0.9.3 release) promoted several functions from
internal to public, but forgot to fix the documentation generator
to provide details about those functions.
For an example of what this fixes, look at:
file:///path/to/libvirt/docs/html/libvirt-libvirt.html#virEventAddHandle
before and after the patch.
* docs/apibuild.py (ignored_functions): Don't ignore functions
that were turned into official API.
* src/util/virevent.c: Fix comments to pass through parser.
Add support for QEMU -add-fd command line parameter detection.
This intentionally rejects qemu 1.2, where 'add-fd' QMP did
not allow full control of set ids, and where there was no command
line counterpart, but accepts qemu 1.3.
Signed-off-by: Eric Blake <eblake@redhat.com>
Add entry points for calling the qemu 'add-fd' and 'remove-fd'
monitor commands. There is no entry point for 'query-fdsets';
the assumption is that a developer can use
virsh qemu-monitor-command domain '{"execute":"query-fdsets"}'
when debugging issues, and that meanwhile, libvirt is responsible
enough to remember what fds it associated with what fdsets.
Likewise, on the 'add-fd' command, it is assumed that libvirt
will always pass a set id, rather than letting qemu autogenerate
the next available id number.
* src/qemu/qemu_monitor.c (qemuMonitorAddFd, qemuMonitorRemoveFd):
New functions.
* src/qemu/qemu_monitor.h (qemuMonitorAddFd, qemuMonitorRemoveFd):
New prototypes.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONAddFd)
(qemuMonitorJSONRemoveFd): New functions.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONAddFd)
(qemuMonitorJSONRemoveFd): New prototypes.
Way back when I started making changes for Coverity messages my first set
were to a bunch of CHECKED_RETURN errors. In particular virAsprintf() had
a few callers that Coverity noted didn't check their return (although some
did check if the buffer being printed to was NULL or not).
It was suggested at the time as a further patch an ATTRIBUTE_RETURN_CHECK
should be added to virAsprintf(), see:
https://www.redhat.com/archives/libvir-list/2013-January/msg00120.html
This patch does that and fixes a few more instances not found by Coverity
that failed the check.
When a disk-only snapshot is requested the domain is treated as if it
was offline. This forbids to mix memory checkpoints with the DISK_ONLY
flag.
This patch improves the error message and mentions the restriction in
the virsh man page.
Commit 60b176c3d0 introduced a bug that
when editing an XML with cputune similar to this:
...
<vcpu placement='static' current='1'>2</vcpu>
<cputune>
<vcpupin vcpu="1" cpuset="0"/>
</cputune>
...
results in formatted XML that looks like this:
...
<vcpu placement='static' current='1'>2</vcpu>
<cputune>
</cputune>
...
That is caused by a condition depending on def->cputune.vcpupin being
set rather than checking def->cputune.nvcpupin. Notice that nvcpupin
can be 0 and vcpupin can still be allocated since it's a pointer to an
array, so no harm done there.
I also changed it on other places in the code where it depended on the
wrong variable.
Setting the log output prefix to 0 is not supported and in fact results
in the following message:
warning : virLogParseOutputs:1021 : Ignoring invalid log output setting.
https://bugzilla.redhat.com/show_bug.cgi?id=894723
Currently, if qemuProcessStart() succeeds, but it's decompression
binary that returns nonzero status, we don't kill the qemu process,
but remove it from internal domain list, leaving the qemu process
hanging around totally uncontrolled.
This patch resolves CVE-2013-0170:
https://bugzilla.redhat.com/show_bug.cgi?id=893450
When reading and dispatching of a message failed the message was freed
but wasn't removed from the message queue.
After that when the connection was about to be closed the pointer for
the message was still present in the queue and it was passed to
virNetMessageFree which tried to call the callback function from an
uninitialized pointer.
This patch removes the message from the queue before it's freed.
* rpc/virnetserverclient.c: virNetServerClientDispatchRead:
- avoid use after free of RPC messages
https://bugzilla.redhat.com/show_bug.cgi?id=892289
It seems like with new udev within guest OS, the tray is locked,
so we need to:
- 'eject'
- wait for tray to open
- 'change'
Moreover, even when doing bare 'eject', we should check for
'tray_open' as guest may have locked the tray. However, the
waiting phase shouldn't be unbounded, so I've chosen 10 retries
maximum, each per 500ms. This should give enough time for guest
to eject a media and open the tray.
Adjust the macros to free memory allocated during various calls to
perform the check if parameter is NULL prior to really freeing and to
set the pointer to NULL after done freeing.
Resolve a false positive from 'vboxIIDFromUUID_v2_x()'. The code sets
'iid->value = &iid->backing' unconditionally prior to calling 'nsIDFromChar()'.
The 'vboxIIDUnalloc_v2_x()' checks iid->value to not be &iid->backing. The
iid->backing is a static buffer within the initialized structure.
Since libxl provides the domain ID in the event handler callback,
find the domain object based on the ID. This approach prevents
processing the callback on a domain that has already been reaped.
Also, similar to the xl implementation, ignore the SUSPEND shutdown
reason. By calling libxl_domain_suspend(), we know a shutdown
event with SUSPEND reason will be generated, but it can be safely
ignored since any subsequent cleanup will be done by the callers.
libxlDoDomainSave() was removing non-persistent domains, but
required callers to have the virDomainObj locked. Callers could
potentially unlock an already freed virDomainObj. Move this
logic to the callers of libxlDoDomainSave().
I've noticed that libxl can invoke timeout reregister/modify hooks
after returning from libxl_ctx_free. Explicitly remove the
timeouts before freeing the libxl ctx to avoid executing hooks on
stale objects.
It is possible to destroy and cleanup a VM, resulting in freeing the
libxlDomainObjPrivate object and associated libxl ctx, before all fds and
timeouts have been deregistered and destroyed.
Fix this race by incrementing the reference count on libxlDomainObjPrivate
for each fd and timeout registration. Only when all fds and timeouts are
deregistered and destroyed will the libxlDomainObjPrivate be destroyed.
The libxl driver is racy in it's interactions with libxl and libvirt's
event loop. The event loop can invoke callbacks after libxl has
deregistered the event, and possibly access freed data associated with
the event.
This patch fixes the race by converting libxlDomainObjPrivate to a
virObjectLockable, and locking it while executing libxl upcalls and
libvirt event loop callbacks.
Note that using the virDomainObj lock is not satisfactory since it may
be desirable to hold the virDomainObj lock even when libxl events such
as reading and writing to xenstore need processed.
xen-unstable changeset 26469 makes changes wrt modifying and deregistering
timeouts.
First, timeout modify callbacks will only be invoked with an
abs_t of {0,0}, i.e. make the timeout fire immediately. Prior to this
commit, timeout modify callbacks were never invoked.
Second, timeout deregister hooks will no longer be called.
This patch makes changes in the libvirt libxl driver that should be
compatible before and after changeset 26469.
While at it, fix a potential overflow in the timeout register callback.
While working with a pmsuspend vs. snapshot issue, I noticed that
the state file in /var/run/libvirt/qemu/dom.xml contained a rather
suspicious "(null)" string, which does not round-trip well through
a libvirtd restart. Had I been on a platform other than glibc
where printf("%s",NULL) crashes instead of printing (null), we might
have noticed the problem much sooner.
And in fixing that problem, I also noticed that we had several
missing states, because we were #defining several *_LAST names
to a value _different_ than what they were already given as enums
in libvirt.h. Yuck. I got rid of default: labels in the case
statements, because they get in the way of gcc's -Wswitch helping
us ensure we cover all enum values.
* src/conf/domain_conf.c (virDomainStateReasonToString)
(virDomainStateReasonFromString): Fill in missing domain states;
rewrite case statement to let compiler enforce checking.
(VIR_DOMAIN_NOSTATE_LAST, VIR_DOMAIN_RUNNING_LAST)
(VIR_DOMAIN_BLOCKED_LAST, VIR_DOMAIN_PAUSED_LAST)
(VIR_DOMAIN_SHUTDOWN_LAST, VIR_DOMAIN_SHUTOFF_LAST)
(VIR_DOMAIN_CRASHED_LAST): Drop dead defines.
(VIR_DOMAIN_PMSUSPENDED_LAST): Drop dead define.
(virDomainPMSuspendedReason): Add missing enum function.
(virDomainRunningReason, virDomainPausedReason): Add missing enum
value.
* src/conf/domain_conf.h (virDomainPMSuspendedReason): Declare
missing functions.
* src/libvirt_private.syms (domain_conf.h): Export them.
With our code, we fail to query for tray-open attribute currently.
That's because in HMP it is 'tray-open' and in QMP it's 'tray_open'.
It always has been. However, we got it exactly the opposite.
A logic bug meant we reported KVM was possible for every
architecture, merely based on whether the query-kvm command
exists. We should instead have been doing it based on whether
the query-kvm command returns 'present: 1'
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently QEMU capabilities are initialized before the QEMU driver
sets ownership on its various directories. The upshot is that if
you change the user/group in the qemu.conf file, libvirtd will fail
to probe QEMU the first time it is run after the config change.
Moving QEMU capabilities initialization to after the chown() calls
fixes this
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This previous commit
commit 1a50ba2cb0
Author: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Date: Mon Nov 26 15:17:13 2012 +0100
qemu: Fix QMP Capabability Probing Failure
which attempted to make sure the QEMU process used for probing
ran as the right user id, caused serious performance regression
and unreliability in probing. The -daemonize switch in QEMU
guarantees that the monitor socket is present before the parent
process exits. This means libvirtd is guaranteed to be able to
connect immediately. By switching from -daemonize to the
virCommandDaemonize API libvirtd was no longer synchronized with
QEMU's startup process. The result was that the QEMU monitor
failed to open and went into its 200ms sleep loop. This happened
for all 25 binaries resulting in 5 seconds worth of sleeping
at libvirtd startup. In addition sometimes when libvirt connected,
QEMU would be partially initialized and crash causing total
failure to probe that binary.
This commit reverts the previous change, ensuring we do use the
-daemonize flag to QEMU. Startup delay is cut from 7 seconds
to 2 seconds on my machine, which is on a par with what it was
prior to the capabilities rewrite.
To deal with the fact that QEMU needs to be able to create the
pidfile, we switch pidfile location fron runDir to libDir, which
QEMU is guaranteed to be able to write to.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently, there is no reason to hold qemu driver locked
throughout whole API execution. Moreover, we can use the
new qemuDomObjFromDomain() internal API to lookup domain then.
Hosts for rbd are ceph monitor daemons. These have fixed IP addresses,
so they are often referenced by IP rather than hostname for
convenience, or to avoid relying on DNS. Using IPv4 addresses as the
host name works already, but IPv6 addresses require rbd-specific
escaping because the colon is used as an option separator in the
string passed to qemu.
Escape these colons, and enclose the IPv6 address in square brackets
so it is distinguished from the port, which is currently mandatory.
Acked-by: Osier Yang <jyang@redhat.com>
Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
https://bugzilla.redhat.com/show_bug.cgi?id=876829 complains that
if a guest is put into S3 state (such as via virsh dompmsuspend)
and then an external snapshot is taken, qemu forcefully transitions
the domain to paused, but libvirt doesn't reflect that change
internally. Thus, a user has to use 'virsh suspend' to get libvirt
back in sync with qemu state, and if the user doesn't know this
trick, then the guest appears hung.
* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateActiveExternal):
Track fact that qemu wakes up a suspended domain on migration.
https://bugzilla.redhat.com/show_bug.cgi?id=895882
virDomainSnapshot.getDomain() and virDomainSnapshot.getConnect()
wrappers around virDomainSnapshotGet{Domain,Connect} were not supposed
to be ever implemented. The class should contain proper domain() and
connect() accessors that fetch python objects stored internally within
the class. While domain() was already provided, connect() was missing.
This patch adds connect() method to virDomainSnapshot class and
reimplements getDomain() and getConnect() methods as aliases to domain()
and connect() for backward compatibility.
The previous fix to avoid leaking securityDriverNames forgot to
handle the case of securityDriverNames being NULL, leading to
a crash
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The autodestroy callback code has the following function
called from a hash iterator
qemuDriverCloseCallbackRun(void *payload,
const void *name,
void *opaque)
{
...
char *uuidstr = name
...
dom = closeDef->cb(data->driver, dom, data->conn);
if (dom)
virObjectUnlock(dom);
virHashRemoveEntry(data->driver->closeCallbacks, uuidstr);
}
The closeDef->cb function may well cause the current callback
to be removed, if it shuts down 'dom'. As such the use of
'uuidstr' in virHashRemoveEntry is accessing free'd memory.
We must make a copy of the uuid str before invoking the
callback to be safe.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When linuxNodeInfoCPUPopulate() method triggered use of an
uninitialize value, since it did not initialize the 'sockets'
field in the virNodeInfoPtr struct:
==30020== Conditional jump or move depends on uninitialised value(s)
==30020== at 0x5125DBD: linuxNodeInfoCPUPopulate (nodeinfo.c:513)
==30020== by 0x51261A0: nodeGetInfo (nodeinfo.c:884)
==30020== by 0x149B9B10: qemuCapsInit (qemu_capabilities.c:846)
==30020== by 0x14A11B25: qemuCreateCapabilities (qemu_driver.c:424)
==30020== by 0x14A12426: qemuStartup (qemu_driver.c:874)
==30020== by 0x512A7AF: virStateInitialize (libvirt.c:822)
==30020== by 0x40DE04: daemonRunStateInit (libvirtd.c:877)
==30020== by 0x50ADCE5: virThreadHelper (virthreadpthread.c:161)
==30020== by 0x328CA07D14: start_thread (pthread_create.c:308)
==30020== by 0x328C6F246C: clone (clone.S:114)
(happened twice)
if (socks > nodeinfo->sockets) <--- here
nodeinfo->sockets = socks;
Rather than doing this for each field, just make the caller memset
the entire struct to zero.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Commit 87b4c10c6c added code that may call
the virCapabilitiesClearHostNUMACellCPUTopology function with
uninitialized second argument. Although the value wouldn't be used some
compilers whine about that.
Be sure to VIR_FREE(accel) and moved virDomainVideoDefFree() within no_memory
label to be consistent
Resolve resource leak in parallelsApplyIfaceParams() when the 'oldnet' is
allocated locally. Also virCommandFree(cmd) as necessary.
This patch adds data gathering to the NUMA gathering files and adds
support for outputting the data. The test driver and xend driver need to
be adapted to fill sensible data to the structure in a future patch.
This will allow storing additional topology data in the NUMA topology
definition.
This patch changes the storage type and fixes fallout of the change
across the drivers using it.
This patch also changes semantics of adding new NUMA cell information.
Until now the data were re-allocated and copied to the topology
definition. This patch changes the addition function to steal the
pointer to a pre-allocated structure to simplify the code.
The way in that memory balloon suppression was handled for S390
is flawed for a number or reasons.
1. Just preventing the default balloon to be created in the case
of VIR_ARCH_S390[X] is not sufficient. An explicit memballoon
element in the guest definition will still be honored, resulting
both in a -balloon option and the allocation of a PCI bus address,
neither being supported.
2. Prohibiting balloon for S390 altogether at a domain_conf level
is no good solution either as there's work in progress on the QEMU
side to implement a virtio-balloon device, although in
conjunction with a new machine type. Suppressing the balloon
should therefore be done at the QEMU driver level depending
on the present capabilities.
Therefore we remove the conditional suppression of the default
balloon in domain_conf.c.
Further, we are claiming the memballoon device for virtio-s390
during device address assignment to prevent it from being considered
as a PCI device.
Finally, we suppress the generation of the balloon command line option
if this is a virtio-s390 machine.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Should have been done in commit 56fd513 already, but was missed
due to oversight: qemuDomainSendKey didn't release the driver lock
in its cleanup section. This fixes an issue introduced by commit
8c5d2ba.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
This patch changes the name of the @sep argument to @terminator and
clarifies it's usage. This patch also explicitly documents that
whitespace can't be used as @terminator as it is skipped multiple times
in the implementation.
https://bugzilla.redhat.com/show_bug.cgi?id=892079
One of my previous patches (f2a4e5f176) tried to fix crashing
libvirtd on domain detroy. However, we need to copy pattern from
qemuProcessHandleMonitorEOF() instead of decrementing reference
counter. The rationale for this is, if qemu process is dying due
to domain being destroyed, we obtain EOF on both the monitor and
agent sockets. However, if the exit is expected, qemuProcessStop
is called, which cleans both agent and monitor sockets up. We
want qemuAgentClose() to be called iff the EOF is not expected,
so we don't leak an FD and memory. Moreover, there could be race
with qemuProcessHandleMonitorEOF() which could have already
closed the agent socket, in which case we don't want to do
anything.
A followon to commit id: 68dceb635 - if iface->iname is NULL, then
neither virNetDevOpenvswitchRemovePort() nor virNetDevVethDelete()
should be called. Found by Coverity.
Although the nwfilter driver skips startup when running in a
session libvirtd, it did not skip reload or shutdown. This
caused errors to be reported when sending SIGHUP to libvirtd,
and caused an abort() in libdbus on shutdown due to trying
to remove a dbus filter that was never added
When building with static analysis enabled, we turn on attribute
nonnull checking. However, this caused the build to fail with:
../../src/util/virobject.c: In function 'virObjectOnceInit':
../../src/util/virobject.c:55:40: error: null argument where non-null required (argument 1) [-Werror=nonnull]
Creation of the virObject class is the one instance where the
parent class is allowed to be NULL. Making things conditional
will let us keep static analysis checking for all other .c file
callers, without breaking the build on this one exception.
* src/util/virobject.c: Define witness.
* src/util/virobject.h (virClassNew): Use it to force most callers
to pass non-null parameter.
Adds a "ram" attribute globally to the video.model element, that changes
the resulting qemu command line only if video.type == "qxl".
<video>
<model type='qxl' ram='65536' vram='65536' heads='1'/>
</video>
That attribute gets a default value of 64*1024. The schema is unchanged
for other video element types.
The resulting qemu command line change is the addition of
-global qxl-vga.ram_size=<ram>*1024
or
-global qxl.ram_size=<ram>*1024
For the main and secondary qxl devices respectively.
The default for the qxl ram bar is 64*1024 kilobytes (the same as the
default qxl vram bar size).
The Coverity static analyzer was generating many false positives for the
unary operation inside the VIR_FREE() definition as it was trying to evaluate
the else portion of the "?:" even though the if portion was (1).
Signed-off-by: Eric Blake <eblake@redhat.com>
The count of vCPUs for a domain is extracted as a usingned long variable
but is stored in a unsigned short. If the actual number was too large,
a faulty number was stored.
The local redefinition of PED_PARTITION_PROTECTED results in the error
but is not a problem especially if the built code doesn't have the latest
definitions.
Upon successful return of virNetClientStreamEventAddCallback() the
allocated cbdata field will be freed by virNetClientStreamEventRemoveCallback()
as cbOpaque using the free function remoteStreamCallbackFree().
This avoids "Event negative_returns: A negative constant "-1" is passed as
an argument to a parameter that cannot be negative.". The called function
uses -1 to determine whether it needs to traverse all the hostdevs.
The use of switch statements inside a bounded for loop resulted in some
false positives regarding the "default:" label which cannot be reached
since each of the other case statements use the possible for loop values.
A [dead_error_begin] was added before the default label.
Commit id ebdbe25a adjusted the algorithm and the caller guarantees that
the 'params' will have a '_' in the name being searched. Add the [returned_null]
tag to the two instances.
The use of switch statements inside a bounded for loop resulted in some
false positives regarding the "default:" label which cannot be reached
since each of the other case statements use the possible for loop values.
Commit id a994ef2d1 changed the mechanism to store/update the default
security label from using disk->seclabels[0] to allocating one on the
fly. That change allocated the label, but never saved it. This patch
will save the label. The new virDomainDiskDefAddSecurityLabelDef() is
a copy of the virDomainDefAddSecurityLabelDef().
The code is not reachable as of commit id: bb85f229. Removed
virKeepAliveStop() and virObjectUnref() because 'ka' cannot be
anything but NULL at the cleanup label.
Currently, whenever somebody calls saferead() on nonblocking FD
(safewrite() is totally interchangeable for purpose of this message)
he might get wrong return value. For instance, in the first iteration
some data is read. The number of bytes read is stored into local
variable 'nread'. However, in next iterations we can get -1 from
read() with errno == EAGAIN, in which case the -1 is returned despite
fact some data has already been read. So the caller gets confused.
Bare read() should be used for nonblocking FD.
The snapshot name is used to create path to the definition save file.
When the name contains slashes the creation of the file fails. Reject
such names.
When the snapshot definition can't be saved, the
qemuDomainSnapshotCreate function succeeded without filling some of the
fields in the internal definition.
This patch removes the snapshot and returns failure if the XML file
cannot be written.
When running virDomainDestroy, we need to make sure that no other
background thread cleans up the domain while we're doing our work.
This can happen if we release the domain object while in the
middle of work, because the monitor might detect EOF in this window.
For this reason we have a 'beingDestroyed' flag to stop the monitor
from doing its normal cleanup. Unfortunately this flag was only
being used to protect qemuDomainBeginJob, and not qemuProcessKill
This left open a race condition where either libvirtd could crash,
or alternatively report bogus error messages about the domain already
having been destroyed to the caller
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Working with virTypedParameters in clients written in C is ugly and
requires all clients to duplicate the same code. This set of APIs makes
this code for manipulating with virTypedParameters integral part of
libvirt so that all clients may benefit from it.
When virStorageBackendLogicalCreateVol() creates a snapshot for a
logical volume with backingStore element, it fails with the message
below:
2013-01-17 03:10:18.869+0000: 1967: error : virCommandWait:2345 :
internal error Child process (/sbin/lvcreate --name lvm-snapshot -L 51200K
-s=/dev/lvm-pool/lvm-volume) unexpected exit status 3: /sbin/lvcreate:
invalid option -- '=' Error during parsing of command line.
This is because virCommandAddArgPair() uses '=' to connect the two
parameters, it's unsuitable for -s option of the lvcreate.
Signed-off-by: Atsushi Kumagai <kumagai-atsushi@mxc.nes.nec.co.jp>
A build on FreeBSD failed with:
util/virportallocator.c:108: error: storage size of 'addr' isn't known
util/virportallocator.c:123: error: 'INADDR_ANY' undeclared (first use in this function)
It turns out that while POSIX allows sockaddr_in to leak in through
<arpa/inet.h> (the way Linux does it), it is not mandatory, and
conforming applications are required to get it through <netinet/in.h>.
* src/util/virportallocator.c: Include header for struct
sockaddr_in.
* tests/virportallocatortest.c: Likewise.
The fetch of 'ipdef' in networkRefreshDhcpDaemon() when the loop to fill
in ipv4def fails to find an ipv4 address with dhcp defined. The filled in
ipdef value was not used. Code was made unnecessary with commit it 2d5cd1.
The driver mutex was unlocked in qemuDomainModifyDeviceFlags before
entering qemuDomainObjBeginJobWithDriver where it will be unlocked once
more leaving it in an undefined state. The result was that two
threads were simultaneously looking up the domain hash table during
multiple parallel device attach/detach operations.
Luckily this triggered a virHashIterationError.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
When starting a VM, /var/log/messages was spammed with the following message:
xt_physdev: using --physdev-out in the OUTPUT, FORWARD and POSTROUTING chains for non-bridged traffic is not supported anymore.
With each extra VM I start, the messages get amplified
exponentially. This results in longer starting times every new VM,
relative the the previously started VM. When I ran a test with
starting 100 equal VM's, the first VM started in about 2 seconds, the
100th VM took 48 seconds to start. I'm running a vanilla 3.7.1 kernel,
but I have the same issue on VM hosts with kernel 3.2.28 or 3.2.0,
running libvirt 0.9.12 and 0.9.8 respectively.
Looking into the warning, it seemed that iptables need an extra argument,
--physdev-is-bridged, in commands like:
iptables -A libvirt-out -m physdev --physdev-is-bridged --physdev-out vnet99 -g FP-vnet99
With that, the warnings in /var/log/messages are gone and running the
test again proved the 100th VM started in 3.8 seconds.
This resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=895294
The symptom was that attempts to modify a network device using
virDomainUpdateDeviceFlags() would fail if the original device had a
<boot> element (e.g. "<boot order='1'/>"), even if the updated device
had the same <boot> element. Instead, the following error would be logged:
cannot modify network device boot index setting
It's true that it's not possible to change boot order (internally
known as bootIndex) of a live device; qemuDomainChangeNet checks for
that, but the problem was that the information it was checking was
incorrect.
Explanation:
When a complete domain is parsed, a global (to the domain) "bootMap"
is passed down to the parse for each device; the bootMap is used to
make sure that devices don't have conflicting settings for their boot
orders.
When a single device is parsed by itself (as in the case of
virDomainUpdateDeviceFlags), there is no global bootMap that would be
appropriate to send, so NULL is sent instead. However, although the
lowest level function that parses just the boot order *does* simply
skip the sanity check in that case, the next higher level
"virDomainDeviceInfoParseXML" function refuses to call down to the
lower "virDomainDeviceBootParseXML" if bootMap is NULL. So, the boot
order is never set in the "new" device object, and when it is compared
to the original (which does have a boot order), they don't match.
The fix is to patch virDomainDeviceInfoParseXML to not care about
bootMap, and just always call virDomainDeviceInfoBootParseXML whenever
there is a <boot> element. When we are only parsing a single device,
we don't care whether or not any specified boot order is consistent
with the rest of the domain; we will always do this check later (in
the current case, we do it by verifying that the net bootIndex exactly
matches the old bootIndex).
The bandwidth plug and unplug functions were assuming that an
interface's bandwidth setting was always specified directly in the
domain's <interface> definition, but that's not necessarily true - it
could have been obtained from a <portgroup> definition in the network
definition. This patch fixes those functions to use
virDomainNetGetActualBandwidth(), which gets the bandwidth pointer
from iface->data.network.actual if it exists, otherwise returns
iface->bandwidth.
Remove extraneous check for 'netdef' when dereferencing for vlan.nTags.
Prior code would already check if netdef was NULL.
Coverity complained about a path where the 'vlan' was potentially valid,
but a prior checks may not have allocated 'iface->data.network.actual',
so like other paths it needs to be allocated on the fly.
Move the copying of vlan up earlier in networkAllocateActualDevice, so
that actual.type gets properly set.
Since the first assignment to vlan is redundant except in the case of
jumping immediately to validate from the start of the function,
eliminate its initial setting at the top of the function in favor of
calling the helper function virDomainNetGetActualVlan() (which doesn't
depend on the local vlan pointer being initialized) down at validate:
Signed-off-by: Laine Stump <laine@redhat.com>
When creating the virClass object for virNetClient, we specified
virObject as the parent instead of virObjectLockable
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The QEMU driver default max port is 65535, but it then increments
this by 1 to 65536. This maps to 0 in an unsigned short :-( This
was apparently done so that for() loops could use "< max" instead
of "<= max". Remove this insanity and just make the loop do the
right thing.
that broke the build like:
CC libvirt_conf_la-domain_conf.lo
conf/domain_conf.c: In function 'virDomainVcpuPinAdd':
conf/domain_conf.c:11920:29: error: 'vpcupin' undeclared (first use in this function)
conf/domain_conf.c:11920:29: note: each undeclared identifier is reported only once for each function it appears in
make[3]: *** [libvirt_conf_la-domain_conf.lo] Error 1
Commit dfa1e1dd added functions whose definitions do not conform
to the style used in the libxl driver. Change these functions to
be consistent throughout the driver.
In commit c4bbaaf8, caps->arch was checked uninitialized, rendering the
whole check useless.
This patch moves the conditional setting of QEMU_CAPS_NO_ACPI to
qemuCapsInitQMP, and removes the no longer needed exception for S390.
It also clears the flag for all non-x86 archs instead of just S390 in
qemuCapsInitHelp.
The virDomainObj, qemuAgent, qemuMonitor, lxcMonitor classes
all require a mutex, so can be switched to use virObjectLockable
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Check status when attempting to set SO_REUSEADDR flag on outgoing connection
On failure, VIR_WARN(), but continue to connect. This code path is on the
sender side where the setting is just a hint and would only take effect if
the sender is overflowed with TCP connections. Inability to set doesn't mean
failure to establish a connection.
In virLockSpaceProtocolDispatchNew() the returned value of lockspace from
virLockDaemonFindLockSpace() is overwritten by the virLockSpaceNew() return.
Coverity complains that it's unused.
In virLockSpaceProtocolDispatchCreateLockSpace() lockspace is also overwritten
in a similar manner resulting in the same Coverity message.
After live change of cpu counts, the number of processor threads is
verified. This patch makes use of this approach to check if qemu ignored
the request for cpu hot-unplug and report an appropriate message.
A great many virObject instances require a mutex, so introduce
a convenient class for this which provides a mutex. This avoids
repeating the tedious init/destroy code
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently all classes must directly inherit from virObject.
This allows for arbitrarily deep hierarchy. There's not much
to this aside from chaining up the 'dispose' handlers from
each class & providing APIs to check types.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
libvirt lxc will fail to start when selinux is disabled.
error: Failed to start domain noroot
error: internal error guest failed to start: PATH=/bin:/sbin TERM=linux container=lxc-libvirt container_uuid=b9873916-3516-c199-8112-1592ff694a9e LIBVIRT_LXC_UUID=b9873916-3516-c199-8112-1592ff694a9e LIBVIRT_LXC_NAME=noroot /bin/sh
2013-01-09 11:04:05.384+0000: 1: info : libvirt version: 1.0.1
2013-01-09 11:04:05.384+0000: 1: error : lxcContainerMountBasicFS:546 : Failed to mkdir /sys/fs/selinux: No such file or directory
2013-01-09 11:04:05.384+0000: 7536: info : libvirt version: 1.0.1
2013-01-09 11:04:05.384+0000: 7536: error : virLXCControllerRun:1466 : error receiving signal from container: Input/output error
2013-01-09 11:04:05.404+0000: 7536: error : virCommandWait:2287 : internal error Child process (ip link del veth1) unexpected exit status 1: Cannot find device "veth1"
fix this problem by checking if selinuxfs is mounted
in host before we try to create dir /sys/fs/selinux.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
Commit 509eb51 added lxc_protocol.x; but without the initial
checkin of src/lxc_protocol-structs, 'make check' would fail for
anyone with pdwtags installed:
make[3]: *** No rule to make target `lxc_protocol-structs', needed by `check-protocol'. Stop.
* src/lxc_protocol-structs: New file.
The virDomainLxcOpenNamespace method needs to open every file
in /proc/$INITPID/ns and return the open file descriptor to the
client application.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Make cpuset local to the while loop and free it once done with it each
time through the loop. Add a sa_assert() to virBitmapParse() to keep Coverity
from believing there could be a negative return and possible resource leak.
Commit-id 'afc4631b' added the regfree(reg) to free resources alloc'd
during regcomp; however, reg still needed to be VIR_FREE()'d. The call
to regfree() also didn't account for possible NULL value. Reformatted
the call to be closer to usage.
Commit c308a9ae was incomplete; it resolved the configure failure,
but not a later build failure.
* src/util/virnetdevbridge.c: Include pre-req header.
* configure.ac (AC_CHECK_HEADERS): Prefer standard in.h over
non-standard ip6.h.
Some places missed the conversion from LIBCURL_{CFLAGS,LIBS} to
CURL_{CFLAGS,LIBS}, and a part of curl check was left in
configure.ac instead of m4/virt-curl.m4 by mistake
This patch introduces support for LXC specific public APIs. In
common with what was done for QEMU, this creates a libvirt_lxc.so
library and libvirt/libvirt-lxc.h header file.
The actual APIs are
int virDomainLxcOpenNamespace(virDomainPtr domain,
int **fdlist,
unsigned int flags);
int virDomainLxcEnterNamespace(virDomainPtr domain,
unsigned int nfdlist,
int *fdlist,
unsigned int *noldfdlist,
int **oldfdlist,
unsigned int flags);
which provide a way to use the setns() system call to move the
calling process into the container's namespace. It is not
practical to write in a generically applicable manner. The
nearest that we could get to such an API would be an API which
allows to pass a command + argv to be executed inside a
container. Even if we had such a generic API, this LXC specific
API is still useful, because it allows the caller to maintain
the current process context, in particular any I/O streams they
have open.
NB the virDomainLxcEnterNamespace() API is special in that it
runs client side, so does not involve the internal driver API.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This converts the libssh2 configure check to use LIBVIRT_CHECK_PKG.
Previously it would check version 1.0 and 1.3, but this simplifies
things to just require version 1.3
If addition of rules in networkAddIptablesRules() failed the real error
was masked by error reported when trying to clean up the remaining
rules.
With this patch the original error message is saved and set back after
the removal is complete.
Commit 0211fd6e04 introduced regression
where newly defined networks were not made persistent.
This patch makes the network persistent on each successful definition.
The phypUUIDTable_Push and phypUUIDTable_Pull leaked their file descriptors
on normal return. Each function had an unnecessary use of creating a buffer
to print conn->uri->user and needed a bit better flow control. I also noted
that the Read function had a cut-n-paste error from the write function on a
couple of VIR_WARN's.
The openSSHSession leaked the sock on the failure path. Additionally that
turns into the internal_socket in the phypOpen code. That was neither saved
nor closed on any path. So I used the connnection_data->sock field to save
the socket for eventual close. Of interest here is that phypExec used the
connection_data->sock field even though it had never been initialized.
I ran 'make dist' in the directory left over from ./autobuild.sh
(which was configured for a mingw cross build); the resulting
tarball had more files than 'make dist' on a normal Linux build.
I traced it to the fact that we were distributing a generated
file, but only when configure said the end user had to generate
the file in the first place. In the process, I noticed that
we had some difference in symbol file names; I added a comment
explaining why the difference exists (after first trying to
normalize the names and hitting VPATH build failures).
* configure.ac (LIBVIRT_QEMU_SYMBOL_FILE): Add some comments.
* src/Makefile.am (EXTRA_DIST): No need to ship a generated file;
particularly since which file is built depends on configure results.
There's no need to do lots of readlink() calls to canonicalize
a name if we're only going to use stat() on it, since stat()
already chases symlinks.
* src/util/virutil.c (virGetDeviceID): Let stat() do the symlink
chasing.
Pass stub driver name directly to pciDettachDevice and pciReAttachDevice to fit
for different libvirt drivers. For example, qemu driver prefers pci-stub, but
Xen prefers pciback.
Signed-off-by: Chunyan Liu <cyliu@suse.com>
Add an optional 'type' attribute to <target> element of serial port
device. There are two choices for its value, 'isa-serial' and
'usb-serial'. For backward compatibility, when attribute 'type' is
missing the 'isa-serial' will be chosen as before.
Libvirt XML sample
<serial type='pty'>
<target type='usb-serial' port='0'/>
<address type='usb' bus='0' port='1'/>
</serial>
qemu commandline:
qemu ${other_vm_args} \
-chardev pty,id=charserial0 \
-device usb-serial,chardev=charserial0,id=serial0,bus=usb.0,port=1
https://bugzilla.redhat.com/show_bug.cgi?id=892079
With current code, if user calls virDomainPMSuspendForDuration()
followed by virDomainDestroy(), the former API checks for qemu agent
presence, which will evaluate as true (if agent is configured). While
talking to qemu agent, the qemu driver is unlocked, so the latter API
starts executing. However, if machine dies meanwhile, libvirtd gets
EOF on the agent socket and qemuProcessHandleAgentEOF() is called. The
handler clears reference to qemu agent while the destroy API already
holding a reference to it. This leads to NULL dereferencing later in
the code. Therefore, the agent pointer should be set to NULL only if
we are the exclusive owner of it.
While OOM can have knock-on effects that trash a system, generally
the first symptom is one of memory thrashing.
* src/qemu/qemu_cgroup.c (qemuSetupCgroup): Reword slightly.
when we has no host's src mapped to container.
there is no .oldroot dir,so libvirt lxc will fail
to start when mouting meminfo.
in this case,the parameter srcprefix of function
lxcContainerMountProcFuse should be NULL.and make
this method handle NULL correctly.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
Perform all the appropriate plumbing.
When qemu/KVM VMs are paused manually through a monitor not-owned by libvirt,
libvirt will think of them as "paused" event after they are resumed and
effectively running. With this patch the discrepancy goes away.
This is meant to address bug 892791.
Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
gcc 4.1.2 on RHEL 5 warned:
conf/network_conf.c:3136: warning: 'foundIdx' may be used uninitialized in this function
The warning is spurious, but initializing the variable doesn't hurt.
* src/conf/network_conf.c (virNetworkDefUpdateDNSHost): Silence
unused variable warning.
POSIX does not guarantee whether uid_t and gid_t are signed or
unsigned, nor does it guarantee whether they are smaller, same
size, or larger than int (or even the same size as one another).
Therefore, it is possible to have platforms where '(uid_t)-1==-1'
is false or where 'uid = gid = -1' sets uid to the wrong value,
thanks to integer promotion rules. The only portable way to use
the placeholder value of these two types is to always use a cast.
Thankfully, the issue is mostly theoretical - sanlock only
compiles on Linux for now, and on Linux, these types do not
suffer from strange promotion problems.
* src/locking/lock_driver_sanlock.c
(virLockManagerSanlockSetupLockspace, virLockManagerSanlockInit)
(virLockManagerSanlockCreateLease): Cast -1 to proper type before
comparing with uid_t or gid_t.
Currently, if there's no hard memory limit defined for a domain,
libvirt tries to calculate one, based on domain definition and magic
equation and set it upon the domain startup. The rationale behind was,
if there's a memory leak or exploit in qemu, we should prevent the
host system trashing. However, the equation was too tightening, as it
didn't reflect what the kernel counts into the memory used by a
process. Since many hosts do have a swap, nobody hasn't noticed
anything, because if hard memory limit is reached, process can
continue allocating memory on a swap. However, if there is no swap on
the host, the process gets killed by OOM killer. In our case, the qemu
process it is.
To prevent this, we need to relax the hard RSS limit. Moreover, we
should reflect more precisely the kernel way of accounting the memory
for process. That is, even the kernel caches are counted within the
memory used by a process (within cgroups at least). Hence the magic
equation has to be changed:
limit = 1.5 * (domain memory + total video memory) + (32MB for cache
per each disk) + 200MB
This is the QEMU backend code for the SCLP console support.
It includes SCLP capability detection, QEMU command line generation
and a test case.
Signed-off-by: J.B. Joret <jb@linux.vnet.ibm.com>
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
The SCLP console is the native console type for s390 and is preferred
over the virtio console as it doesn't require special drivers and
is more efficient. Recent versions of QEMU come with SCLP support
which is hereby enabled.
The new target types 'sclp' and 'sclplm' can be used to specify a
SCLP console. Adding documentation, domain schema and XML processing
support.
Signed-off-by: J.B. Joret <jb@linux.vnet.ibm.com>
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
To avoid confusion between the LXC driver <-> controller
monitor RPC protocol and the libvirt-lxc.so <-> libvirtd public
RPC protocol, rename the former to lxc_monitor_protocol.x
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently the libvirt client can pass FDs to the server, but the
dispatch mechanism provides no way to return FDs back from the
server to the client. Tweak the dispatch code, such that if a
dispatcher returns '1', this indicates that it populated the
virNetMessagePtr with FDs to return
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
