Most severe here is a latent (but currently untriggered) memory leak
if any hypervisor ever adds a string interface property; the
remainder are mainly cosmetic.
* include/libvirt/libvirt.h.in (VIR_DOMAIN_BANDWIDTH_*): Move
macros closer to interface that uses them, and document type.
* src/libvirt.c (virDomainSetInterfaceParameters)
(virDomainGetInterfaceParameters): Formatting tweaks.
* daemon/remote.c (remoteDispatchDomainGetInterfaceParameters):
Avoid memory leak.
* src/libvirt_public.syms (LIBVIRT_0.9.9): Sort lines.
* src/libvirt_private.syms (domain_conf.h): Likewise.
* src/qemu/qemu_driver.c (qemuDomainSetInterfaceParameters): Fix
comments, break long lines.
* 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
Valgrind detected a pipe fd leak before the parent exits on success,
introduced in commit 4296cea; by itself, the leak is not bad, since
we immediately called _exit(), but we might as well be clean to make
valgrind analysis easier. Meanwhile, if the daemon grandchild detects
an error, the parent failed to flush the error message before exiting.
Also, we had the possibility of both parent and child returning to the
caller, such that the user could see duplicated reports of failure
from the two return paths. And we might as well be robust to the
(unlikely) situation of being started with stdin closed.
* daemon/libvirtd.c (daemonForkIntoBackground): Use exit if an
error message was generated, avoid fd leaks for valgrind's sake,
avoid returning to caller in both parent and child, and don't
close a just-dup'd stdin.
Based on a report by Alex Jia.
* How to reproduce?
% service libvirtd stop
% valgrind -v --track-fds=yes /usr/sbin/libvirtd --daemon
* Actual valgrind result:
==16804== FILE DESCRIPTORS: 7 open at exit.
==16804== Open file descriptor 7:
==16804== at 0x321FAD8B87: pipe (in /lib64/libc-2.12.so)
==16804== by 0x41F34D: daemonForkIntoBackground (libvirtd.c:186)
==16804== by 0x4207A0: main (libvirtd.c:1420)
Signed-off-by: Alex Jia <ajia@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
The RPC code had several latent memory leaks and an attempt to
free the wrong string, but thankfully nothing triggered them
(blkiotune was the only one returning a string, and always as
the last parameter). Also, our cleanups for rpcgen ended up
nuking a line of code that renders VIR_TYPED_PARAM_INT broken,
because it was the only use of 'i' in a function, even though
it was a member usage rather than a standalone declaration.
* daemon/remote.c (remoteSerializeTypedParameters): Free the
correct array element.
(remoteDispatchDomainGetSchedulerParameters)
(remoteDispatchDomainGetSchedulerParametersFlags)
(remoteDispatchDomainBlockStatsFlags)
(remoteDispatchDomainGetMemoryParameters): Don't leak strings.
* src/rpc/genprotocol.pl: Don't nuke member-usage of 'buf' or 'i'.
Commit 4d9e51f6 fixed a 'make uninstall' failure, but failed
to follow other conventions already present in src/Makefile.am.
In particular, we prefer MKDIR_P over mkdir -p, and should
have a matching rmdir during uninstall for every directory
created during install (the idea being that uninstall in a
DESTDIR should be clean, while installation in the final
system should not fail with non-empty directories left behind).
* tools/Makefile.am (install-sysconfig, install-initscript)
(install-systemd): Use MKDIR_P.
(uninstall-sysconfig, uninstall-initscript, uninstall-systemd):
Also remove directories.
* daemon/Makefile.am (install-data-local, install-data-polkit)
(install-logrotate, install-sysconfig, install-sysctl)
(install-init-redhat, install-init-upstart, install-init-systemd)
(install-data-sasl): Use MKDIR_P.
(uninstall-data-polkit, uninstall-sysconfig, uninstall-sysctl)
(uninstall-init-redhat, uninstall-init-upstart)
(uninstall-init-systemd): Also remove directory.
(uninstall-logrotate): New rule.
(uninstall-local): Add uninstall-logrotate.
Make uninstall currently fails with the following message:
rmdir /etc/sasl2/
rmdir: failed to remove `/etc/sasl2/': Directory not empty
That's fine (correct in fact) so force the command to return success
with || :
https://bugzilla.redhat.com/show_bug.cgi?id=648855 mentioned a
misuse of 'an' where 'a' is proper; that has since been fixed,
but a search found other problems (some were a spelling error for
'and', while most were fixed by 'a').
* daemon/stream.c: Fix grammar.
* src/conf/domain_conf.c: Likewise.
* src/conf/domain_event.c: Likewise.
* src/esx/esx_driver.c: Likewise.
* src/esx/esx_vi.c: Likewise.
* src/rpc/virnetclient.c: Likewise.
* src/rpc/virnetserverprogram.c: Likewise.
* src/storage/storage_backend_fs.c: Likewise.
* src/util/conf.c: Likewise.
* src/util/dnsmasq.c: Likewise.
* src/util/iptables.c: Likewise.
* src/xen/xen_hypervisor.c: Likewise.
* src/xen/xend_internal.c: Likewise.
* src/xen/xs_internal.c: Likewise.
* tools/virsh.c: Likewise.
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>
This patch adds support for a systemd init service for libvirtd
and libvirt-guests. The libvirtd.service is *not* written to use
socket activation, since we want libvirtd to start on boot so it
can do guest auto-start.
The libvirt-guests.service is pretty lame, just exec'ing the
original init script for now. Ideally we would factor out the
functionality, into some shared tool.
Instead of
./configure --with-init-script=redhat
You can now do
./configure --with-init-script=systemd
Or better still:
./configure --with-init-script=systemd+redhat
We can also now support install of the upstart init script
* configure.ac: Add systemd, and systemd+redhat options to
--with-init-script option
* daemon/Makefile.am: Install systemd services
* daemon/libvirtd.sysconf: Add note about unused env variable
with systemd
* daemon/libvirtd.service.in: libvirtd systemd service unit
* libvirt.spec.in: Add scripts to installing systemd services
and migrating from legacy init scripts
* tools/Makefile.am: Install systemd services
* tools/libvirt-guests.init.sh: Rename to tools/libvirt-guests.init.in
* tools/libvirt-guests.service.in: systemd service unit
Very occasionally the sequence of events from poll would result
in getting a HANGUP on its own, instead of a HANGUP+READABLE
at the same time. In the former case we would send back an error
event to the client, but never send the empty packet to indicate
EOF.
I missed adding libvirt_driver_remote.la to libvirtd_LDADD in
commit b8adfcc6, which didn't cause a problem in 0.9.6 but
results in this build error in 0.9.7
libvirtd-remote.o: In function `remoteDispatchAuthPolkit':
remote.c:(.text+0x188dd): undefined reference to `virNetServerGetDBusConn'
The src/util/network.c file is a dumping ground for many different
APIs. Split it up into 5 pieces, along functional lines
- src/util/virnetdevbandwidth.c: virNetDevBandwidth type & helper APIs
- src/util/virnetdevvportprofile.c: virNetDevVPortProfile type & helper APIs
- src/util/virsocketaddr.c: virSocketAddr and APIs
- src/conf/netdev_bandwidth_conf.c: XML parsing / formatting
for virNetDevBandwidth
- src/conf/netdev_vport_profile_conf.c: XML parsing / formatting
for virNetDevVPortProfile
* src/util/network.c, src/util/network.h: Split into 5 pieces
* src/conf/netdev_bandwidth_conf.c, src/conf/netdev_bandwidth_conf.h,
src/conf/netdev_vport_profile_conf.c, src/conf/netdev_vport_profile_conf.h,
src/util/virnetdevbandwidth.c, src/util/virnetdevbandwidth.h,
src/util/virnetdevvportprofile.c, src/util/virnetdevvportprofile.h,
src/util/virsocketaddr.c, src/util/virsocketaddr.h: New pieces
* daemon/libvirtd.h, daemon/remote.c, src/conf/domain_conf.c,
src/conf/domain_conf.h, src/conf/network_conf.c,
src/conf/network_conf.h, src/conf/nwfilter_conf.h,
src/esx/esx_util.h, src/network/bridge_driver.c,
src/qemu/qemu_conf.c, src/rpc/virnetsocket.c,
src/rpc/virnetsocket.h, src/util/dnsmasq.h, src/util/interface.h,
src/util/iptables.h, src/util/macvtap.c, src/util/macvtap.h,
src/util/virnetdev.h, src/util/virnetdevtap.c,
tools/virsh.c: Update include files
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
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);
Based on a report by Coverity. waitpid() can leak resources if it
fails with EINTR, so it should never be used without checking return
status. But we already have a helper function that does that, so
use it in more places.
* src/lxc/lxc_container.c (lxcContainerAvailable): Use safer
virWaitPid.
* daemon/libvirtd.c (daemonForkIntoBackground): Likewise.
* tests/testutils.c (virtTestCaptureProgramOutput, virtTestMain):
Likewise.
* src/libvirt.c (virConnectAuthGainPolkit): Simplify with virCommand.
This is a regression introduced by new RPC codes, previously
we advertise the service via ssh even if the daemon doesn't
listen on TLS port (TCP is not choosed). Now the service is
only advertised when it listens on TLS or TCP port. This breaks
upper layer apps which intends to discover the service, such
as virt-manager.
The libvirtd daemon had a few crude system tap probes. Some of
these were broken during the RPC rewrite. The new modular RPC
code is structured in a way that allows much more effective
tracing. Instead of trying to hook up the original probes,
define a new set of probes for the RPC and event code.
The master probes file is now src/probes.d. This contains
probes for virNetServerClientPtr, virNetClientPtr, virSocketPtr
virNetTLSContextPtr and virNetTLSSessionPtr modules. Also add
probes for the poll event loop.
The src/dtrace2systemtap.pl script can convert the probes.d
file into a libvirt_probes.stp file to make use from systemtap
much simpler.
The src/rpc/gensystemtap.pl script can generate a set of
systemtap functions for translating RPC enum values into
printable strings. This works for all RPC header enums (program,
type, status, procedure) and also the authentication enum
The PROBE macro will automatically generate a VIR_DEBUG
statement, so any place with a PROBE can remove any existing
manual DEBUG statements.
* daemon/libvirtd.stp, daemon/probes.d: Remove obsolete probing
* daemon/libvirtd.h: Remove probe macros
* daemon/Makefile.am: Remove all probe buildings/install
* daemon/remote.c: Update authentication probes
* src/dtrace2systemtap.pl, src/rpc/gensystemtap.pl: Scripts
to generate STP files
* src/internal.h: Add probe macros
* src/probes.d: Master list of probes
* src/rpc/virnetclient.c, src/rpc/virnetserverclient.c,
src/rpc/virnetsocket.c, src/rpc/virnettlscontext.c,
src/util/event_poll.c: Insert probe points, removing any
DEBUG statements that duplicate the info
To avoid static linking libvirtd to the RPC server code, which
then prevents sane introduction of DTrace probes, put it all
in the libvirt.so, and export it
* daemon/Makefile.am: Don't link to RPC libraries
* src/Makefile.am: Link all RPC libraries to libvirt.so
* src/libvirt_private.syms: Export all RPC functions
https://bugzilla.redhat.com/show_bug.cgi?id=740899 documents that
if qemu uses aio=native for its disks, then it consumes 128 aio
requests per disk. On a host with multiple guests, this can quickly
run out of kernel aio requests with the default aio-max-nr of
65536. Kernel developers have confirmed that there is no up-front
cost to raising this limit (a larger limit merely implies that more
aio requests can be issued in parallel, which in turn will result
in more kernel memory allocation, only if the system really does use
that many requests). Since the system default limit prevents 256
disks, which is well within libvirt's current scalability, this
patch installs a file to raise the limit and document it in case a
system administrator has further cause to tune the limit. The
install only works on platforms new enough to source /etc/sysctl.d/*
alongside /etc/sysctl.conf (F14 and RHEL 6).
* daemon/libvirtd.sysctl: New file.
* daemon/Makefile.am (EXTRA_DIST): Ship it.
(install-init, uninstall-init): Install it.
* libvirt.spec.in (%files): Include it in rpm.
Bug introduced in commit 675464b. On an OOM, this would try to
dereference a char* and free the contents as a pointer, which is
doomed to failure.
Adding a syntax check will prevent mistakes like this in the future.
* cfg.mk (sc_prohibit_internal_functions): New syntax check.
(exclude_file_name_regexp--sc_prohibit_internal_functions): Add
exemptions.
* daemon/remote.c (remoteRelayDomainEventIOError)
(remoteRelayDomainEventIOErrorReason)
(remoteRelayDomainEventGraphics, remoteRelayDomainEventBlockJob):
Use correct free function.
remoteRelayDomainEventBlockJob, remoteRelayDomainEventIOError,
remoteRelayDomainEventIOErrorReason and remoteRelayDomainEventGraphics
were using const string directly in rpc structure, before calling
remoteDispatchDomainEventSend(). But that routine now frees up all
the pointed allocated memory from the rpc structure and we end up
with a double free.
This now strdup() all the strings passed and provide mem_error goto
labels to be used when an allocation error occurs.
Note that the cleanup isn't completely finished because all relaying
function also call make_nonnull_domain() which also allocate a string
and never handle the error case. This patches doesn't try to address
this as this is only error correctness a priori and touches far more
functions in this module:
* daemon/remote.c: fix string allocations and memory error handling
for remoteRelayDomainEventBlockJob, remoteRelayDomainEventIOError,
remoteRelayDomainEventIOErrorReason and remoteRelayDomainEventGraphics
This is a bit painful for example when starting virt-manager
it tends to clutter libvirtd.log with invalid operation on cpu pinning
for defined but not running domains. A priori those kind of errors
don't indicate an error when executing the command but on a precondition
for running the API, and honnestly while the application should report
it, logging it as an error in libvirtd.log is not really useful,
Related bug: https://bugzilla.redhat.com/show_bug.cgi?id=590807
* daemon/libvirtd.c: extend daemonErrorLogFilter() to filter out
errors of type VIR_ERR_OPERATION_INVALID
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.
When libvirtd is running at non-root user, it won't create ${HOME}/.libvirt.
It will show error message:
17:44:16.838: 7035: error : virPidFileAcquirePath:322 : Failed to open pid file
Signed-off-by: Xu He Jie <xuhj@linux.vnet.ibm.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Commit 2c85644b0b attempted to
fix a problem with tracking RPC messages from streams by doing
- if (msg->header.type == VIR_NET_REPLY) {
+ if (msg->header.type == VIR_NET_REPLY ||
+ (msg->header.type == VIR_NET_STREAM &&
+ msg->header.status != VIR_NET_CONTINUE)) {
client->nrequests--;
In other words any stream packet, with status NET_OK or NET_ERROR
would cause nrequests to be decremented. This is great if the
packet from from a synchronous virStreamFinish or virStreamAbort
API call, but wildly wrong if from a server initiated abort.
The latter resulted in 'nrequests' being decremented below zero.
This then causes all I/O for that client to be stopped.
Instead of trying to infer whether we need to decrement the
nrequests field, from the message type/status, introduce an
explicit 'bool tracked' field to mark whether the virNetMessagePtr
object is subject to tracking.
Also add a virNetMessageClear function to allow a message
contents to be cleared out, without adversely impacting the
'tracked' field as a naive memset() would do
* src/rpc/virnetmessage.c, src/rpc/virnetmessage.h: Add
a 'bool tracked' field and virNetMessageClear() API
* daemon/remote.c, daemon/stream.c, src/rpc/virnetclientprogram.c,
src/rpc/virnetclientstream.c, src/rpc/virnetserverclient.c,
src/rpc/virnetserverprogram.c: Switch over to use
virNetMessageClear() and pass in the 'bool tracked' value
when creating messages.
When sending outbound stream RPC messages, a callback is
used to re-enable stream data transmission. If the stream
aborts while one of these messages is outstanding, the
stream may have been free'd by the time it is invoked. This
results in a use-after-free error
* daemon/stream.c: Ref-count streams to avoid use-after-free
When dispatching domain events we will create an XDR struct
containing the event info. Some of this data may be allocated
on the heap and so must be freed. The graphics event dispatcher
had a broken attempt to free one field, but missed others. All
the events have a dom->name string that needs freeing. The code
should have used the xdr_free() procedure for doing all this
* daemon/remote.c: Use xdr_free after dispatching events
Systemtap 1.2 <sys/sdt.h> tried to expand STAP_PROBE3 into an
initialization:
volatile __typeof__(arg) foo = arg;
but that fails if arg was declared as 'char arg[100]'.
Rather than make all callers to PROBE deal with the stupidity
of <sys/sdt.h>, we instead make PROBE cast away the problem.
Some of this preprocessor abuse copies ideas in src/libvirt.c.
* daemon/libvirtd.h (PROBE): Add casts to all arguments, using...
(VIR_ADD_CASTS, VIR_ADD_CAST, VIR_ADD_CAST2, VIR_ADD_CAST3)
(VIR_ADD_CAST_EXPAND, VIR_ADD_CAST_PASTE, VIR_COUNT_ARGS)
(VIR_ARG5, PROBE_EXPAND): New macros.
Reported by Wen Congyang.
When spice_tls is set but listen_tls is not, we don't initialize
GnuTLS library. So any later gnutls call (e.g. during migration,
where we initialize a certificate) will access uninitialized GnuTLS
internal structs and throws an error.
Although, we might now initialize GnuTLS twice, it is safe according
to the documentation:
This function can be called many times,
but will only do something the first time.
This patch creates 2 functions: virNetTLSInit and virNetTLSDeinit
with respect to written above.
The I/O event callback processes incoming packets first, and then
does outgoing packets. If the incoming packet caused the stream to
close, then the attempt to process outgoing data resulted in an
error. This caused libvirt to then send an error back to the client,
but the stream had already been stopped. This confused the client
since it sees 2 error events.
* daemon/stream.c: Don't attempt read if stream is closed
Every active stream results in a reference being held on the
virNetServerClientPtr object. This meant that if a client quit
with any streams active, although all I/O was stopped the
virNetServerClientPtr object would leak. This causes libvirtd
to leak any file handles associated with open streams when a
client quit
To fix this, when we call virNetServerClientClose there is a
callback invoked which lets the daemon release the streams
and thus the extra references
* daemon/remote.c: Add a hook to close all streams
* daemon/stream.c, daemon/stream.h: Add API for releasing
all streams
* src/rpc/virnetserverclient.c, src/rpc/virnetserverclient.h:
Allow registration of a hook to trigger when closing client
Early errors during start of libvirtd didn't have
an error reporting mechanism and caused libvirtd
to exit silently (only the return value indicated
an error).
Libvirt logging is initialized very early using
enviroment variables and the internal error reporting
API is used to report early errors.
v2 changes:
- print errors unconditionaly before logging starts
- fix message to US spelling
v2.5 changes:
- initialize logging from enviroment
- log all early errors using VIR_ERROR
v3 changes:
- move virSetLogFromEnv() after virInitialize()
fixes: https://bugzilla.redhat.com/show_bug.cgi?id=728654
