The RPC code assumed that the array returned by the driver would be
fully populated; that is, ncpus on entry resulted in ncpus * return
value on exit. However, while we don't support holes in the middle
of ncpus, we do want to permit the case of ncpus on entry being
longer than the array returned by the driver (that is, it should be
safe for the caller to pass ncpus=128 on entry, and the driver will
stop populating the array when it hits max_id).
Additionally, a successful return implies that the caller will then
use virTypedParamArrayClear on the entire array; for this to not
free uninitialized memory, the driver must ensure that all skipped
entries are explicitly zeroed (the RPC driver did this, but not
the qemu driver).
There are now three cases:
server 0.9.10 and client 0.9.10 or newer: No impact - there were no
hypervisor drivers that supported cpu stats
server 0.9.11 or newer and client 0.9.10: if the client calls with
ncpus beyond the max, then the rpc call will fail on the client side
and disconnect the client, but the server is no worse for the wear
server 0.9.11 or newer and client 0.9.11: the server can return a
truncated array and the client will do just fine
I reproduced the problem by using a host with 2 CPUs, and doing:
virsh cpu-stats $dom --start 1 --count 2
* daemon/remote.c (remoteDispatchDomainGetCPUStats): Allow driver
to omit tail of array.
* src/remote/remote_driver.c (remoteDomainGetCPUStats):
Accommodate driver that omits tail of array.
* src/libvirt.c (virDomainGetCPUStats): Document this.
* src/qemu/qemu_driver.c (qemuDomainGetPercpuStats): Clear all
unpopulated entries.
No thanks to 64-bit windows, with 64-bit pid_t, we have to avoid
constructs like 'int pid'. Our API in libvirt-qemu cannot be
changed without breaking ABI; but then again, libvirt-qemu can
only be used on systems that support UNIX sockets, which rules
out Windows (even if qemu could be compiled there) - so for all
points on the call chain that interact with this API decision,
we require a different variable name to make it clear that we
audited the use for safety.
Adding a syntax-check rule only solves half the battle; anywhere
that uses printf on a pid_t still needs to be converted, but that
will be a separate patch.
* cfg.mk (sc_correct_id_types): New syntax check.
* src/libvirt-qemu.c (virDomainQemuAttach): Document why we didn't
use pid_t for pid, and validate for overflow.
* include/libvirt/libvirt-qemu.h (virDomainQemuAttach): Tweak name
for syntax check.
* src/vmware/vmware_conf.c (vmwareExtractPid): Likewise.
* src/driver.h (virDrvDomainQemuAttach): Likewise.
* tools/virsh.c (cmdQemuAttach): Likewise.
* src/remote/qemu_protocol.x (qemu_domain_attach_args): Likewise.
* src/qemu_protocol-structs (qemu_domain_attach_args): Likewise.
* src/util/cgroup.c (virCgroupPidCode, virCgroupKillInternal):
Likewise.
* src/qemu/qemu_command.c(qemuParseProcFileStrings): Likewise.
(qemuParseCommandLinePid): Use pid_t for pid.
* daemon/libvirtd.c (daemonForkIntoBackground): Likewise.
* src/conf/domain_conf.h (_virDomainObj): Likewise.
* src/probes.d (rpc_socket_new): Likewise.
* src/qemu/qemu_command.h (qemuParseCommandLinePid): Likewise.
* src/qemu/qemu_driver.c (qemudGetProcessInfo, qemuDomainAttach):
Likewise.
* src/qemu/qemu_process.c (qemuProcessAttach): Likewise.
* src/qemu/qemu_process.h (qemuProcessAttach): Likewise.
* src/uml/uml_driver.c (umlGetProcessInfo): Likewise.
* src/util/virnetdev.h (virNetDevSetNamespace): Likewise.
* src/util/virnetdev.c (virNetDevSetNamespace): Likewise.
* tests/testutils.c (virtTestCaptureProgramOutput): Likewise.
* src/conf/storage_conf.h (_virStoragePerms): Use mode_t, uid_t,
and gid_t rather than int.
* src/security/security_dac.c (virSecurityDACSetOwnership): Likewise.
* src/conf/storage_conf.c (virStorageDefParsePerms): Avoid
compiler warning.
Function xmlParseURI does not remove square brackets around IPv6
address when parsing. One of the solutions is making wrappers around
functions working with xmlURI*. This assures that uri->server will be
always properly assigned and it doesn't have to be changed when used
on some new place in the code.
For this purpose, functions virParseURI and virSaveURI were
added. These function are wrappers around xmlParseURI and xmlSaveUri
respectively.
Also there is one new syntax check function to prohibit these functions
anywhere else.
File changes:
- src/util/viruri.h -- declaration
- src/util/viruri.c -- definition
- src/libvirt_private.syms -- symbol export
- src/Makefile.am -- added source and header files
- cfg.mk -- added sc_prohibit_xmlURI
- all others -- ID name and include fixes
Detected by valgrind. Leaks are introduced in commit c1b2264.
* src/remote/remote_driver.c (doRemoteOpen): free client program memory in failure path.
* How to reproduce?
% valgrind -v --leak-check=full virsh -c qemu:
* Actual result
==3969== 40 bytes in 1 blocks are definitely lost in loss record 8 of 28
==3969== at 0x4A04A28: calloc (vg_replace_malloc.c:467)
==3969== by 0x4C89C41: virAlloc (memory.c:101)
==3969== by 0x4D5A236: virNetClientProgramNew (virnetclientprogram.c:60)
==3969== by 0x4D47AB4: doRemoteOpen (remote_driver.c:658)
==3969== by 0x4D49FFF: remoteOpen (remote_driver.c:871)
==3969== by 0x4D13373: do_open (libvirt.c:1196)
==3969== by 0x4D14535: virConnectOpenAuth (libvirt.c:1422)
==3969== by 0x425627: main (virsh.c:18537)
==3969==
==3969== 40 bytes in 1 blocks are definitely lost in loss record 9 of 28
==3969== at 0x4A04A28: calloc (vg_replace_malloc.c:467)
==3969== by 0x4C89C41: virAlloc (memory.c:101)
==3969== by 0x4D5A236: virNetClientProgramNew (virnetclientprogram.c:60)
==3969== by 0x4D47AD7: doRemoteOpen (remote_driver.c:664)
==3969== by 0x4D49FFF: remoteOpen (remote_driver.c:871)
==3969== by 0x4D13373: do_open (libvirt.c:1196)
==3969== by 0x4D14535: virConnectOpenAuth (libvirt.c:1422)
==3969== by 0x425627: main (virsh.c:18537)
==3969==
==3969== LEAK SUMMARY:
==3969== definitely lost: 80 bytes in 2 blocks
Signed-off-by: Alex Jia <ajia@redhat.com>
This patch adds API to modify domain metadata for running and stopped
domains. The api supports changing description, title as well as the
newly added <metadata> element. The API has support for storing data in
the metadata element using xml namespaces.
* include/libvirt/libvirt.h.in
* src/libvirt_public.syms
- add function headers
- add enum to select metadata to operate on
- export functions
* src/libvirt.c
- add public api implementation
* src/driver.h
- add driver support
* src/remote/remote_driver.c
* src/remote/remote_protocol.x
- wire up the remote protocol
* include/libvirt/virterror.h
* src/util/virterror.c
- add a new error message note that metadata for domain are
missing
Using new function 'virTypedParameterArrayClear' to simplify block of codes.
* daemon/remote.c, src/remote/remote_driver.c: simplify codes.
Signed-off-by: Alex Jia <ajia@redhat.com>
Our existing virDomainBlockResize takes an unsigned long long
argument; if that command is later taught a DELTA and SHRINK flag,
we cannot change its type without breaking API (but at least such
a change would be ABI compatible). Meanwhile, the only time a
negative size makes sense is if both DELTA and SHRINK are used
together, but if we keep the argument unsigned, applications can
pass the positive delta amount by which they would like to shrink
the system, and have the flags imply the negative value. So,
since this API has not yet been released, and in the interest of
consistency with existing API, we swap virStorageVolResize to
always pass an unsigned value.
* include/libvirt/libvirt.h.in (virStorageVolResize): Use unsigned
argument.
* src/libvirt.c (virStorageVolResize): Likewise.
* src/driver.h (virDrvStorageVolUpload): Adjust clients.
* src/remote/remote_protocol.x (remote_storage_vol_resize_args):
Likewise.
* src/remote_protocol-structs: Regenerate.
Suggested by Daniel P. Berrange.
Unlike other users of virTypedParameter with RPC, this interface
can return zero-filled entries because the interface assumes
2 dimensional array. We compress these entries out from the
server when generating the over-the-wire contents, then reconstitute
them in the client.
Signed-off-by: Eric Blake <eblake@redhat.com>
This API allows a domain to be put into one of S# ACPI states.
Currently, S3 and S4 are supported. These states are shared
with virNodeSuspendForDuration.
However, for now we don't support any duration other than zero.
The same apply for flags.
Currently, we support only filling a volume with zeroes on wiping.
However, it is not enough as data might still be readable by
experienced and equipped attacker. Many technical papers have been
written, therefore we should support other wiping algorithms.
Add a new API virDomainShutdownFlags and define:
VIR_DOMAIN_SHUTDOWN_DEFAULT = 0,
VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN = (1 << 0),
VIR_DOMAIN_SHUTDOWN_GUEST_AGENT = (1 << 1),
Also define some flags for the reboot API
VIR_DOMAIN_REBOOT_DEFAULT = 0,
VIR_DOMAIN_REBOOT_ACPI_POWER_BTN = (1 << 0),
VIR_DOMAIN_REBOOT_GUEST_AGENT = (1 << 1),
Although these two APIs currently have the same flags, using
separate enums allows them to expand separately in the future.
Add stub impls of the new API for all existing drivers
Although this is a public API break, it only affects users that
were compiling against *_LAST values, and can be trivially
worked around without impacting compilation against older
headers, by the user defining VIR_ENUM_SENTINELS before using
libvirt.h. It is not an ABI break, since enum values do not
appear as .so entry points. Meanwhile, it prevents users from
using non-stable enum values without explicitly acknowledging
the risk of doing so.
See this list discussion:
https://www.redhat.com/archives/libvir-list/2012-January/msg00804.html
* include/libvirt/libvirt.h.in: Hide all sentinels behind
LIBVIRT_ENUM_SENTINELS, and add missing sentinels.
* src/internal.h (VIR_DEPRECATED): Allow inclusion after
libvirt.h.
(LIBVIRT_ENUM_SENTINELS): Expose sentinels internally.
* daemon/libvirtd.h: Use the sentinels.
* src/remote/remote_protocol.x (includes): Don't expose sentinels.
* python/generator.py (enum): Likewise.
* tests/cputest.c (cpuTestCompResStr): Silence compiler warning.
* tools/virsh.c (vshDomainStateReasonToString)
(vshDomainControlStateToString): Likewise.
I previously mentioned [1] a PolicyKit issue where libvirt would
proceed with authentication even though polkit-auth failed:
testusr xen134:~> virsh list --all
Attempting to obtain authorization for org.libvirt.unix.manage.
polkit-grant-helper: given auth type (8 -> yes) is bogus
Failed to obtain authorization for org.libvirt.unix.manage.
Id Name State
----------------------------------
0 Domain-0 running
- sles11sp1-pv shut off
AFAICT, libvirt attempts to obtain a privilege it already has,
causing polkit-auth to fail with above message. Instead of calling
obtain and then checking auth, IMO the workflow should be for the
server to check auth first, and if that fails ask the client to
obtain it and check again. This workflow also allows for checking
only successful exit of polkit-auth in virConnectAuthGainPolkit().
[1] https://www.redhat.com/archives/libvir-list/2011-December/msg00837.html
* daemon/remote.c: implement the server side support
* src/remote/remote_driver.c: implement the client side support
* src/remote/remote_protocol.x: definitions for the new entry points
* src/remote_protocol-structs: structure definitions
The lifetime of the virDomainEventState object is tied to
the lifetime of the driver, which in stateless drivers is
tied to the lifetime of the virConnectPtr.
If we add & remove a timer when allocating/freeing the
virDomainEventState object, we can get a situation where
the timer still triggers once after virDomainEventState
has been freed. The timeout callback can't keep a ref
on the event state though, since that would be a circular
reference.
The trick is to only register the timer when a callback
is registered with the event state & remove the timer
when the callback is unregistered.
The demo for the bug is to run
while true ; do date ; ../tools/virsh -q -c test:///default 'shutdown test; undefine test; dominfo test' ; done
prior to this fix, it will frequently hang and / or
crash, or corrupt memory
Currently all drivers using domain events need to provide a callback
for handling a timer to dispatch events in a clean stack. There is
no technical reason for dispatch to go via driver specific code. It
could trivially be dispatched directly from the domain event code,
thus removing tedious boilerplate code from all drivers
Also fix the libxl & xen drivers to pass 'true' when creating the
virDomainEventState, since they run inside the daemon & thus always
expect events to be present.
* src/conf/domain_event.c, src/conf/domain_event.h: Internalize
dispatch of events from timer callback
* src/libxl/libxl_driver.c, src/lxc/lxc_driver.c,
src/qemu/qemu_domain.c, src/qemu/qemu_driver.c,
src/remote/remote_driver.c, src/test/test_driver.c,
src/uml/uml_driver.c, src/vbox/vbox_tmpl.c,
src/xen/xen_driver.c: Remove all timer dispatch functions
When registering a callback for a particular event some callers
need to know how many callbacks already exist for that event.
While it is possible to ask for a count, this is not free from
race conditions when threaded. Thus the API for registering
callbacks should return the count of callbacks. Also rename
virDomainEventStateDeregisterAny to virDomainEventStateDeregisterID
* src/conf/domain_event.c, src/conf/domain_event.h,
src/libvirt_private.syms: Return count of callbacks when
registering callbacks
* src/libxl/libxl_driver.c, src/libxl/libxl_driver.c,
src/qemu/qemu_driver.c, src/remote/remote_driver.c,
src/remote/remote_driver.c, src/uml/uml_driver.c,
src/vbox/vbox_tmpl.c, src/xen/xen_driver.c: Update
for change in APIs
Currently if you try to connect to a local libvirtd when
libvirtd is not in $PATH, you'll get an error
error: internal error invalid use of command API
This is because remoteFindDaemonPath() returns NULL, which
causes us to pass NULL into virNetSocketConnectUNIX which
in turn causes us to pass NULL into virCommandNewArgList.
Adding missing error checks improves this to
error: internal error Unable to locate libvirtd daemon in $PATH
* src/remote/remote_driver.c: Report error if libvirtd
cannot be found
* src/rpc/virnetsocket.c: Report error if caller requested
spawning of daemon, but provided no binary path
Otherwise connections to older libvirt abort with:
$ virsh -c qemu+ssh://host.example.com/system list
error: invalid connection pointer in virDrvSupportsFeature
error: failed to connect to the hypervisor
Tested against 0.8.3 and 0.9.8-rc2.
Support Block I/O Throttle setting and query to remote driver.
Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Send and receive string typed parameters across RPC. This also
completes the back-compat mentioned in the previous patch - the
only time we have an older client talking to a newer server is
if RPC is in use, so filtering out strings during RPC prevents
returning an unknown type to the older client.
* src/remote/remote_protocol.x (remote_typed_param_value): Add
another union value.
* daemon/remote.c (remoteDeserializeTypedParameters): Handle
strings on rpc.
(remoteSerializeTypedParameters): Likewise; plus filter out
strings when replying to older clients. Adjust callers.
* src/remote/remote_driver.c (remoteFreeTypedParameters)
(remoteSerializeTypedParameters)
(remoteDeserializeTypedParameters): Handle strings on rpc.
* src/rpc/gendispatch.pl: Properly clean up typed arrays.
* src/remote_protocol-structs: Update.
Based on an initial patch by Hu Tao, with feedback from
Daniel P. Berrange.
Signed-off-by: Eric Blake <eblake@redhat.com>
Since it needs to access file descriptors passed in the msg,
the RPC driver for virDomainOpenGraphics needs to be manually
implemented.
* daemon/remote.c: RPC server dispatcher
* src/remote/remote_driver.c: RPC client dispatcher
* src/remote/remote_protocol.x: Define protocol
The RPC server classes are extended to allow FDs to be received
from clients with calls. There is not currently any way for a
procedure to pass FDs back to the client with replies
* daemon/remote.c, src/rpc/gendispatch.pl: Change virNetMessageHeaderPtr
param to virNetMessagePtr in dispatcher impls
* src/rpc/virnetserver.c, src/rpc/virnetserverclient.c,
src/rpc/virnetserverprogram.c, src/rpc/virnetserverprogram.h:
Extend to support FD passing
Extend the RPC client code to allow file descriptors to be sent
to the server with calls, and received back with replies.
* src/remote/remote_driver.c: Stub extra args
* src/libvirt_private.syms, src/rpc/virnetclient.c,
src/rpc/virnetclient.h, src/rpc/virnetclientprogram.c,
src/rpc/virnetclientprogram.h: Extend APIs to allow
FD passing
If a disk source gets dropped because it is not accessible,
mgmt application might want to be informed about this. Therefore
we need to emit an event. The event presented in this patch
is however a bit superset of what written above. The reason is simple:
an intention to be easily expanded, e.g. on 'user ejected disk
in guest' events. Therefore, callback gets source string and disk alias
(which should be unique among a domain) and reason (an integer);
This adds support for a libvirt client configuration file
either /etc/libvirt/libvirt.conf for privileged clients,
or $HOME/.libvirt/libvirt.conf for unprivileged clients.
It allows one parameter
uri_aliases = [
"hail=qemu+ssh://root@hail.cloud.example.com/system",
"sleet=qemu+ssh://root@sleet.cloud.example.com/system",
]
Any call to virConnectOpen with a non-NULL URI will first
attempt to match against the uri_aliases list. An application
can disable this by using VIR_CONNECT_NO_ALIASES
* docs/uri.html.in: Document URI aliases
* include/libvirt/libvirt.h.in: Add VIR_CONNECT_NO_ALIASES
* libvirt.spec.in, mingw32-libvirt.spec.in: Add /etc/libvirt/libvirt.conf
* src/Makefile.am: Install default config file
* src/libvirt.c: Add support for URI aliases
* src/remote/remote_driver.c: Don't try to handle URIs
with no scheme and which clearly are not paths
* src/util/conf.c: Don't raise error on virConfFree(NULL)
* src/xen/xen_driver.c: Don't raise error on URIs
with no scheme
Very mechanical. I'm so glad we've automated the generation of things,
compared to what it was in 0.8.x days, where this would be much longer.
* src/remote/remote_protocol.x
(REMOTE_PROC_DOMAIN_SNAPSHOT_NUM_CHILDREN)
(REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES): New rpcs.
(remote_domain_snapshot_num_children_args)
(remote_domain_snapshot_num_children_ret)
(remote_domain_snapshot_list_children_names_args)
(remote_domain_snapshot_list_children_names_ret): New structs.
* src/remote/remote_driver.c (remote_driver): Use it.
* src/remote_protocol-structs: Update.
Mostly straight-forward, although this is the first API that
returns a new snapshot based on a snapshot rather than a domain.
* src/remote/remote_protocol.x
(REMOTE_PROC_DOMAIN_SNAPSHOT_GET_PARENT): New rpc.
(remote_domain_snapshot_get_parent_args)
(remote_domain_snapshot_get_parent_ret): New structs.
* src/rpc/gendispatch.pl: Adjust generator.
* src/remote/remote_driver.c (remote_driver): Use it.
* src/remote_protocol-structs: Update.
Leak present since introduction of remoteDomainBuildEventGraphics
in commit 987e31e.
* src/remote/remote_driver.c: fix memory leak.
Signed-off-by: Alex Jia <ajia@redhat.com>
This patch annotates APIs with low or high priority.
In low set MUST be all APIs which might eventually access monitor
(and thus block indefinitely). Other APIs may be marked as high
priority. However, some must be (e.g. domainDestroy).
For high priority calls (HPC), there are some high priority workers
(HPW) created in the pool. HPW can execute only HPC, although normal
worker can process any call regardless priority. Therefore, only those
APIs which are guaranteed to end in reasonable small amount of time
can be marked as HPC.
The size of this HPC pool is static, because HPC are expected to end
quickly, therefore jobs assigned to this pool will be served quickly.
It can be configured in libvirtd.conf via prio_workers variable.
Default is set to 5.
To mark API with low or high priority, append priority:{low|high} to
it's comment in src/remote/remote_protocol.x. This is similar to
autogen|skipgen. If not marked, the generator assumes low as default.
If users wants to connect to remote unix socket, e.g.
'qemu+unix://<remote>/system' currently the <remote> part is ignored,
ending up connecting to localhost. Connecting to remote socket is not
supported and user should have used TLS/TCP/SSH instead.
