Ensure that all drivers implementing public APIs use a
naming convention for their implementation that matches
the public API name.
eg for the public API virDomainCreate make sure QEMU
uses qemuDomainCreate and not qemuDomainStart
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The driver.h struct for node devices used an inconsistent
naming scheme 'DeviceMonitor' instead of the more usual
'NodeDeviceDriver'. Fix this everywhere it has leaked
out to.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
A number of the remote procedure names did not match the
corresponding API names. For example, many lacked the
word 'CONNECT', others re-arranged the names. Update the
procedures so their names exactly match the API names.
Then remove the special case handling of these APIs in
the generator
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
There are many declared options in gendispatch.pl that were
no longer used. Those which were used were obscure '-b', '-k'
and '-d'. Switch to use --mode={debug|client|server}.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently the RPC protocol files can contain annotations after
the protocol enum eg
REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247, /* autogen autogen priority:high */
This is not very extensible as the number of annotations grows.
Change it to use
/**
* @generate: both
* @priority: high
*/
REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247,
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
http://www.uhv.edu/ac/newsletters/writing/grammartip2009.07.01.htm
(and several other sites) give hints that 'onto' is best used if
you can also add 'up' just before it and still make sense. In many
cases in the code base, we really want the two-word form, or even
a simplification to just 'on' or 'to'.
* docs/hacking.html.in: Use correct 'on to'.
* python/libvirt-override.c: Likewise.
* src/lxc/lxc_controller.c: Likewise.
* src/util/virpci.c: Likewise.
* daemon/THREADS.txt: Use simpler 'on'.
* docs/formatdomain.html.in: Better usage.
* docs/internals/rpc.html.in: Likewise.
* src/conf/domain_event.c: Likewise.
* src/rpc/virnetclient.c: Likewise.
* tests/qemumonitortestutils.c: Likewise.
* HACKING: Regenerate.
Signed-off-by: Eric Blake <eblake@redhat.com>
Despite the comment stating virNetClientIncomingEvent handler should
never be called with either client->haveTheBuck or client->wantClose
set, there is a sequence of events that may lead to both booleans being
true when virNetClientIncomingEvent is called. However, when that
happens, we must not immediately close the socket as there are other
threads waiting for the buck and they would cause SIGSEGV once they are
woken up after the socket was closed. Another thing is we should clear
all remaining calls in the queue after closing the socket.
The situation that can lead to the crash involves three threads, one of
them running event loop and the other two calling libvirt APIs. The
event loop thread detects an event on client->sock and calls
virNetClientIncomingEvent handler. But before the handler gets a chance
to lock client, the other two threads (T1 and T2) start calling some
APIs. T1 gets the buck and detects EOF on client->sock while processing
its RPC call. Since T2 is waiting for its own call, T1 passes the buck
on to it and unlocks client. But before T2 gets the signal, the event
loop thread wakes up, does its job and closes client->sock. The crash
happens when T2 actually wakes up and tries to do its job using a closed
client->sock.
but libvirt is built with --with-selinux. In this case getpeercon
returns ENOPROTOOPT so don't return an error in that case but simply
don't set seccon.
The virNetSocket & virIdentity classes accidentally got some
conditionals using HAVE_SELINUX instead of WITH_SELINUX.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When dispatching an RPC API call, setup the current identity to
hold the identity of the network client associated with the
RPC message being dispatched. The setting is thread-local, so
only affects the API call in this thread
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Add APIs which allow creation of a virIdentity from the info
associated with a virNetServerClientPtr instance. This is done
based on the results of client authentication processes like
TLS, x509, SASL, SO_PEERCRED
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
A socket object has various pieces of security data associated
with it, such as the SELinux context, the SASL username and
the x509 distinguished name. Add new APIs to virNetServerClient
and related modules to access this data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The naming used in the RPC protocols for the LXC monitor and
lock daemon confused the script used to generate systemtap
helper functions. Rename the LXC monitor protocol symbols to
reduce confusion. Adapt the gensystemtap.pl script to cope
with the LXC monitor / lock daemon naming conversions.
This has no functional impact on RPC wire protocol, since
names are only used in the C layer
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When converting to virObject, the probes on the 'Free' functions
were removed on the basis that there is a probe on virObjectFree
that suffices. This puts a burden on people writing probe scripts
to identify which object is being dispose. This adds back probes
in the 'Dispose' functions and updates the rpc monitor systemtap
example to use them
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently the server determines whether authentication of clients
is complete, by checking whether an identity is set. This patch
removes that lame hack and replaces it with an explicit method
for changing the client auth code
* daemon/remote.c: Update for new APis
* src/libvirt_private.syms, src/rpc/virnetserverclient.c,
src/rpc/virnetserverclient.h: Remove virNetServerClientGetIdentity
and virNetServerClientSetIdentity, adding a new method
virNetServerClientSetAuth.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
To avoid a clash with daemon() libc API, rename the
'daemon' param in the header file to 'binary'. The
source file already uses the name 'binary'.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
remoteDeserializeTypedParameters can now be called with either
preallocated params array (size of which is announced by nparams) or it
can allocate params array according to the number of parameters received
from the server.
Don't allow interval to be > MAX_INT/1000 in virKeepAliveStart()
Guard against possible overflow in virKeepAliveTimeout() by setting the
timeout to be MAX_INT/1000 since the math following will multiply it by 1000.
We need to drop the server lock before calling virObjectUnlock(client)
since in case we had the last reference to the client, its dispose
callback would be called and that could possibly try to lock the server
and cause a deadlock. This is exactly what happens when there is only
one QEMU domain running and it is marked to be autodestroyed when the
connection dies. This results in qemuProcessAutoDestroy ->
qemuProcessStop -> virNetServerRemoveShutdownInhibition call sequence,
where the last function locks the server.
Like virNodeDeviceCreateXML, virNodeDeviceLookupSCSIHostByWWN
has to be treated specially when generating the RPC codes. Also
new rules are added in fixup_name to keep the name SCSIHostByWWN.
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
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.
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>
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.
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>
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>
A number of bugs handling file descriptors received from the
server caused the FDs to be lost and leaked.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This adds an implementation of virNetSocketGetUNIXIdentity()
using LOCAL_PEERCRED socket option and xucred struct, defined
in <sys/ucred.h> on systems that have it.
RHEL 6.3 uses dbus-devel-1.2.24, which lacked support for the
DBUS_TYPE_UNIX_FD define (contrast with Fedora 18 using 1.6.8).
But since it is an older dbus, it also lacks support for shutdown
inhibitions as provided by newer systemd.
Compilation failure introduced in commit 31330926.
* src/rpc/virnetserver.c (virNetServerAddShutdownInhibition):
Compile out if dbus is too old.
Use the freedesktop inhibition DBus service to prevent host
shutdown or session logout while any VMs are running.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently to deal with auto-shutdown libvirtd must periodically
poll all stateful drivers. Thus sucks because it requires
acquiring both the driver lock and locks on every single virtual
machine. Instead pass in a "inhibit" callback to virStateInitialize
which drivers can invoke whenever they want to inhibit shutdown
due to existance of active VMs.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When seeing a message
virNetSASLContextCheckIdentity:146 : SASL client admin not allowed in whitelist
it isn't immediately obvious that 'admin' is the identity
being checked. Quote the string to make it more obvious
On OOM, xdr_destroy got called even though it wasn't created yet.
Found by coverity:
Error: UNINIT (CWE-457):
libvirt-0.10.2/src/rpc/virnetmessage.c:214: var_decl: Declaring
variable "xdr" without initializer.
libvirt-0.10.2/src/rpc/virnetmessage.c:219: cond_true: Condition
"virReallocN(&msg->buffer, 1UL /* sizeof (*msg->buffer) */,
msg->bufferLength) < 0", taking true branch
libvirt-0.10.2/src/rpc/virnetmessage.c:221: goto: Jumping to label
"cleanup"
libvirt-0.10.2/src/rpc/virnetmessage.c:257: label: Reached label
"cleanup"
libvirt-0.10.2/src/rpc/virnetmessage.c:258: uninit_use: Using
uninitialized value "xdr.x_ops".
The libvirt coding standard is to use 'function(...args...)'
instead of 'function (...args...)'. A non-trivial number of
places did not follow this rule and are fixed in this patch.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Commit 246143b fixed a warning on older gcc, but caused a warning
on newer gcc.
../../src/rpc/virnetserverservice.c: In function 'virNetServerServiceNewPostExecRestart':
../../src/rpc/virnetserverservice.c:277:41: error: pointer targets in passing argument 3 of 'virJSONValueObjectGetNumberUint' differ in signedness [-Werror=pointer-sign]
* src/rpc/virnetserverservice.c: Use correct types.
With older gcc and 64-bit size_t, the compiler issues a real warning:
rpc/virnetserverservice.c:277: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
Introduced in commit 0cc79255. Depending on machine endianness,
this warning represents a real bug that could mis-interpret the
value by a factor of 2^32. I don't know why I couldn't get newer
gcc to report the same warning message.
* src/rpc/virnetserverservice.c
(virNetServerServiceNewPostExecRestart): Use temporary instead.
Add two new APIs virNetServerNewPostExecRestart and
virNetServerPreExecRestart which allow a virNetServerPtr
object to be created from a JSON object and saved to a
JSON object, for the purpose of re-exec'ing a process.
This includes serialization of all registered services
and clients
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Add two new APIs virNetServerClientNewPostExecRestart and
virNetServerClientPreExecRestart which allow a virNetServerClientPtr
object to be created from a JSON object and saved to a
JSON object, for the purpose of re-exec'ing a process.
This includes serialization of the connected socket associated
with the client
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Add two new APIs virNetServerServiceNewPostExecRestart and
virNetServerServicePreExecRestart which allow a virNetServerServicePtr
object to be created from a JSON object and saved to a
JSON object, for the purpose of re-exec'ing a process.
This includes serialization of the listening sockets associated
with the service
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Add two new APIs virNetSocketNewPostExecRestart and
virNetSocketPreExecRestart which allow a virNetSocketPtr
object to be created from a JSON object and saved to a
JSON object, for the purpose of re-exec'ing a process.
As well as saving the state in JSON format, the second
method will disable the O_CLOEXEC flag so that the open
file descriptors are preserved across the process re-exec()
Since it is not possible to serialize SASL or TLS encryption
state, an error will be raised if attempting to perform
serialization on non-raw sockets
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Continue consolidation of process functions by moving some
helpers out of command.{c,h} into virprocess.{c,h}
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
https://www.gnu.org/licenses/gpl-howto.html recommends that
the 'If not, see <url>.' phrase be a separate sentence.
* tests/securityselinuxhelper.c: Remove doubled line.
* tests/securityselinuxtest.c: Likewise.
* globally: s/; If/. If/
Relatively straightforward. Our decision to make block job
speed a long keeps haunting us on new API.
* src/remote/remote_protocol.x (remote_domain_block_commit_args):
New struct.
* src/remote/remote_driver.c (remote_driver): Enable it.
* src/remote_protocol-structs: Regenerate.
* src/rpc/gendispatch.pl (long_legacy): Exempt another bandwidth.
* src/rpc/gendispatch.pl: (virNodeSetMemoryParameters is the
the special one which needs a connection object as the first
argument, improve the generator to support it).
* daemon/remote.c: (Implement the server side handler for
virDomainGetMemoryParameters)
* src/remote/remote_driver.c: (Implement the client side handler
for virDomainGetMemoryParameters)
* src/remote/remote_protocol.x: (New RPC procedures for the two
new APIs and structs to represent the args and ret for it)
* src/remote_protocol-structs: Likewise
Fix for CVE-2012-4423.
When generating RPC protocol messages, it's strictly needed to have a
continuous line of numbers or RPC messages. However in case anyone
tries backporting some functionality and will skip a number, there is
a possibility to make the daemon segfault with newer virsh (version of
the library, rpc call, etc.) even unintentionally.
The problem is that the skipped numbers will get func filled with
NULLs, but there is no check whether these are set before the daemon
tries to run them. This patch very simply enhances one check and fixes
that.
I got an off-list report about a bad diagnostic:
Target network card mac 52:54:00:49:07:ccdoes not match source 52:54:00:49:07:b8
True to form, I've added a syntax check rule to prevent it
from recurring, and found several other offenders.
* cfg.mk (sc_require_whitespace_in_translation): New rule.
* src/conf/domain_conf.c (virDomainNetDefCheckABIStability): Add
space.
* src/esx/esx_util.c (esxUtil_ParseUri): Likewise.
* src/qemu/qemu_command.c (qemuCollectPCIAddress): Likewise.
* src/qemu/qemu_driver.c (qemuDomainSetMetadata)
(qemuDomainGetMetadata): Likewise.
* src/qemu/qemu_hotplug.c (qemuDomainChangeNetBridge): Likewise.
* src/rpc/virnettlscontext.c
(virNetTLSContextCheckCertDNWhitelist): Likewise.
* src/vmware/vmware_driver.c (vmwareDomainResume): Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc, vboxAttachDrives):
Avoid false negatives.
* tools/virsh-domain.c (info_save_image_dumpxml): Reword.
Based on a report by Luwen Su.
e5a1bee07 introduced a regression in Boxes: when Boxes is left idle
(it's still doing some libvirt calls in the background), the
libvirt connection gets closed after a few minutes. What happens is
that this code in virNetClientIOHandleOutput gets triggered:
if (!thecall)
return -1; /* Shouldn't happen, but you never know... */
and after the changes in e5a1bee07, this causes the libvirt connection
to be closed.
Upon further investigation, what happens is that
virNetClientIOHandleOutput is called from gvir_event_handle_dispatch
in libvirt-glib, which is triggered because the client fd became
writable. However, between the times gvir_event_handle_dispatch
is called, and the time the client lock is grabbed and
virNetClientIOHandleOutput is called, another thread runs and
completes the current call. 'thecall' is then NULL when the first
thread gets to run virNetClientIOHandleOutput.
After describing this situation on IRC, danpb suggested this:
11:37 < danpb> In that case I think the correct thing would be to change
'return -1' above to 'return 0' since that's not actually an
error - its a rare, but expected event
which is what this patch is doing. I've tested it against master
libvirt, and I didn't get disconnected in ~10 minutes while this
happens in less than 5 minutes without this patch.
Unfortunately libssh2 doesn't support all types of host keys that can be
saved in the known_hosts file. Also it does not report that parsing of
the file failed. This results into truncated known_hosts files where the
standard client stores keys also in other formats (eg.
ecdsa-sha2-nistp256).
This patch changes the default location of the known_hosts file into the
libvirt private configuration directory, where it will be only written
by the libssh2 layer itself. This prevents trashing user's known_host
file.
The libssh2 code wasn't supposed to create the known_hosts file, but
recent findings show, that we can't use the default created by OpenSSH
as libssh2 might damage it. We need to create a private known_hosts file
in the config path.
This patch adds support for skipping error if the known_hosts file is
not present and let libssh2 create a new one.
This patch adds a glue layer to enable using libssh2 code with the
network client code.
As in the original client implementation, shell code is sent to the
server to detect correct options for netcat and connect to libvirt's
unix socket.
This patch enables virNetSocket to be used as an ssh client when
properly configured.
This patch adds function virNetSocketNewConnectLibSSH2() that takes all
needed parameters and creates a libssh2 session and performs steps
needed to open the connection and then create a virNetSocket that
seamlesly encapsulates the communication.
This patch adds helper functions that enable us to use libssh2 in
conjunction with libvirt's virNetSockets for ssh transport instead of
spawning "ssh" client process.
This implemetation supports tunneled plaintext, keyboard-interactive,
private key, ssh agent based and null authentication. Libvirt's Auth
callback is used for interaction with the user. (Keyboard interactive
authentication, adding of host keys, private key passphrases). This
enables seamless integration into the application using libvirt. No
helpers as "ssh-askpass" are needed.
Reading and writing of OpenSSH style "known_hosts" files is supported.
Communication is done using SSH exec channel, where the user may specify
arbitrary command to be executed on the remote side and reads and writes
to/from stdin/out are sent through the ssh channel. Usage of stderr is
not (yet) supported.
In order to support systemd socket based activation, it needs to
be possible to create virNetSocketPtr and virNetServerServicePtr
instance from a pre-opened file descriptor
In preparation for adding further constructors, refactor
the virNetServerClientNew method to move most of the code
into a common virNetServerClientNewInternal helper API.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently the virNetServerDispatchNewClient both creates the
virNetServerClientPtr instance and registers it with the
virNetServerPtr internal state. Split the client registration
code out into a separate virNetServerAddClient method to
allow future reuse from other contexts
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently there is a hook function that is invoked when a
new client connection comes in, which allows an app to
setup private data. This setup will make it difficult to
serialize client state during process re-exec(). Change to
a model where the app registers a callback when creating
the virNetServerPtr instance, which is used to allocate
the client private data immediately during virNetClientPtr
construction.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently the virNetClientPtr constructor will always register
the async IO event handler and the keepalive objects. In the
case of the lock manager, there will be no event loop available
nor keepalive support required. Split this setup out of the
constructor and into separate methods.
The remote driver will enable async IO and keepalives, while
the LXC driver will only enable async IO
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently the virNetServerServicePtr is responsible for
creating the virNetServerClientPtr instance when accepting
a new connection. Change this so that the virNetServerServicePtr
merely gives virNetServerPtr a virNetSocketPtr instance. The
virNetServerPtr can then create the virNetServerClientPtr
as it desires
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
From man poll(2), poll does not set errno=EAGAIN on interrupt, however
it does set errno=EINTR. Have libvirt retry on the appropriate errno.
Under heavy load, a program of mine kept getting libvirt errors 'poll on
socket failed: Interrupted system call'. The signals were SIGCHLD from
processes forked by threads unrelated to those using libvirt.
