Define two new RPC message types VIR_NET_CALL_WITH_FDS and
VIR_NET_REPLY_WITH_FDS. These message types are equivalent
to VIR_NET_CALL and VIR_NET_REPLY, except that between the
message header, and payload there is a 32-bit integer field
specifying how many file descriptors have been passed.
The actual file descriptors are sent/recv'd out of band.
* src/rpc/virnetmessage.c, src/rpc/virnetmessage.h,
src/libvirt_private.syms: Add support for handling
passed file descriptors
* src/rpc/virnetprotocol.x: Extend protocol for FD
passing
Add APIs to the virNetSocket object, to allow file descriptors
to be sent/received over UNIX domain socket connections
* src/rpc/virnetsocket.c, src/rpc/virnetsocket.h,
src/libvirt_private.syms: Add APIs for FD send/recv
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
Pull the call to gnutls_x509_crt_get_dn up into a higher function
so that the 'dname' variable will be available for probe points
* src/rpc/virnettlscontext.c: Pull gnutls_x509_crt_get_dn up
one level
If we receive an error on the stream, set the EOF marker so
that any further (bogus) incoming data is dropped.
* src/rpc/virnetclientstream.c: Set EOF on stream
If we send back an unknown program error for async messages,
we will confuse the client because they only expect replies
for method calls. Just log & drop any invalid async messages
* src/rpc/virnetserver.c: Don't send error for async messages
Commit 597fe3cee6 accidentally
introduced a deadlock when reporting an unknown RPC program.
The virNetServerDispatchNewMessage method is called with
the client locked, and must therefore not attempt to send
any RPC messages back to the client. Only once the incoming
message is passed off to the virNetServerHandleJob worker
is it safe to start sending messages back
* src/rpc/virnetserver.c: Delay checking for unknown RPC
program until in worker thread
Do not crash if virStreamFinish is called after error.
==11000== Invalid read of size 4
==11000== at 0x373A8099A0: pthread_mutex_lock (pthread_mutex_lock.c:51)
==11000== by 0x4C7CADE: virMutexLock (threads-pthread.c:85)
==11000== by 0x4D57C31: virNetClientStreamRaiseError (virnetclientstream.c:203)
==11000== by 0x4D385E4: remoteStreamFinish (remote_driver.c:3541)
==11000== by 0x4D182F9: virStreamFinish (libvirt.c:14157)
==11000== by 0x40FDC4: cmdScreenshot (virsh.c:3075)
==11000== by 0x42BA40: vshCommandRun (virsh.c:14922)
==11000== by 0x42ECCA: main (virsh.c:16381)
==11000== Address 0x59b86c0 is 16 bytes inside a block of size 216 free'd
==11000== at 0x4A06928: free (vg_replace_malloc.c:427)
==11000== by 0x4C69E2B: virFree (memory.c:310)
==11000== by 0x4D57B56: virNetClientStreamFree (virnetclientstream.c:184)
==11000== by 0x4D3DB7A: remoteDomainScreenshot (remote_client_bodies.h:1812)
==11000== by 0x4CFD245: virDomainScreenshot (libvirt.c:2903)
==11000== by 0x40FB73: cmdScreenshot (virsh.c:3029)
==11000== by 0x42BA40: vshCommandRun (virsh.c:14922)
==11000== by 0x42ECCA: main (virsh.c:16381)
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.
commit 984840a2c2 removed the
notification of waiting calls when VIR_NET_CONTINUE messages
arrive. This was to fix the case of a virStreamAbort() call
being prematurely notified of completion.
The problem is that sometimes there are dummy calls from a
virStreamRecv() call waiting that *do* need to be notified.
These dummy calls should have a status VIR_NET_CONTINUE. So
re-add the notification upon VIR_NET_CONTINUE, but only if
the waiter also has a status of VIR_NET_CONTINUE.
* src/rpc/virnetclient.c: Notify waiting call if stream data
arrives
* src/rpc/virnetclientstream.c: Mark dummy stream read packet
with status VIR_NET_CONTINUE
Libvirt special-cases a specific VIR_ERR_RPC from the remote driver
back into VIR_ERR_NO_SUPPORT on the client, so that clients can
handle missing rpc functions the same whether the hypervisor driver
is local or remote. However, commit c1b22644 introduced a regression:
VIR_FROM_THIS changed from VIR_FROM_REMOTE to VIR_FROM_RPC, so the
special casing no longer works if the server uses the newer error
domain.
* src/rpc/virnetclientprogram.c
(virNetClientProgramDispatchError): Also cater to 0.9.3 and newer.
* src/rpc/virnettlscontext.c: fix memory leak on
virNetTLSContextValidCertificate.
* Detected in valgrind run:
==25667==
==25667== 6,085 (44 direct, 6,041 indirect) bytes in 1 blocks are definitely
lost in loss record 326 of 351
==25667== at 0x4005447: calloc (vg_replace_malloc.c:467)
==25667== by 0x4F2791F3: _asn1_add_node_only (structure.c:53)
==25667== by 0x4F27997A: _asn1_copy_structure3 (structure.c:421)
==25667== by 0x4F276A50: _asn1_append_sequence_set (element.c:144)
==25667== by 0x4F2743FF: asn1_der_decoding (decoding.c:1194)
==25667== by 0x4F22B9CC: gnutls_x509_crt_import (x509.c:229)
==25667== by 0x805274B: virNetTLSContextCheckCertificate
(virnettlscontext.c:1009)
==25667== by 0x804DE32: testTLSSessionInit (virnettlscontexttest.c:693)
==25667== by 0x804F14D: virtTestRun (testutils.c:140)
==25667==
==25667== 23,188 (88 direct, 23,100 indirect) bytes in 11 blocks are definitely
lost in loss record 346 of 351
==25667== at 0x4005447: calloc (vg_replace_malloc.c:467)
==25667== by 0x4F22B841: gnutls_x509_crt_init (x509.c:50)
==25667== by 0x805272B: virNetTLSContextCheckCertificate
(virnettlscontext.c:1003)
==25667== by 0x804DDD1: testTLSSessionInit (virnettlscontexttest.c:673)
==25667== by 0x804F14D: virtTestRun (testutils.c:140)
* How to reproduce?
% cd libvirt && ./configure && make && make -C tests valgrind
or
% valgrind -v --leak-check=full ./tests/virnettlscontexttest
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.
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.
The bufferOffset has been initialized to zero in virNetMessageEncodePayloadRaw(),
so, we use bufferLength to represent the length of message which is going to be
sent to client side.
From: Matthias Bolte <matthias.bolte@googlemail.com>
Tested-by: Jim Fehlig <jfehlig@novell.com>
Matthias provided this patch to fix an issue I encountered in the
generator with APIs containing call-by-ref long type, e.g.
int virDomainMigrateGetMaxSpeed(virDomainPtr domain,
unsigned long *bandwidth,
unsigned int flags);
In case we add a new program in the future (we did that in the past and
we are going to do it again soon) current daemon will behave badly with
new client that wants to use the new program. Before the RPC rewrite we
used to just send an error reply to any request with unknown program.
With the RPC rewrite in 0.9.3 the daemon just closes the connection
through which such request was sent. This patch fixes this regression.
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.
If a client had initiated a stream abort, it will have a call
waiting for a reply in the queue. If more data continues to
arrive on the stream, the abort command could mistakenly get
signalled as complete. Remove the code from async data processing
that looked for waiting calls. Add a sanity check to ensure no
async call can ever be marked as needing a reply
* src/rpc/virnetclient.c: Ensure async data packets can't
trigger a reply
If a stream gets a server initiated abort, the client may still
send an abort request before it receives the server side abort.
This causes the server to send back another abort for the
stream. Since the protocol defines that abort is the last thing
to be sent, the client gets confused by this second abort from
the server. If the stream is already shutdown, just drop any
client requested abort, rather than sending back another message.
This fixes the regression from previous versions.
Tested as follows
In one virsh session
virsh # start foo
virsh # console foo
In other virsh session
virsh # destroy foo
The first virsh session should be able to continue issuing
commands without error. Prior to this patch it saw
virsh # list
error: Failed to list active domains
error: An error occurred, but the cause is unknown
virsh # list
error: Failed to list active domains
error: no call waiting for reply with prog 536903814 vers 1 serial 9
* src/rpc/virnetserverprogram.c: Drop abort requests
for streams which no longer exist
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
When trying to use any SASL authentication for TCP sockets by
setting auth_tls = "sasl" in libvirtd.conf on server side, the
client will hang because of the sasl session relocking other than
dropping the lock when exiting virNetSASLSessionExtKeySize()
* src/rpc/virnetsaslcontext.c: virNetSASLSessionExtKeySize drop the
lock on exit
This patch introduces a internal RPC API "virNetServerClose", which
is standalone with "virNetServerFree". it closes all the socket fds,
and unlinks the unix socket paths, regardless of whether the socket
is still referenced or not.
This is to address regression bug:
https://bugzilla.redhat.com/show_bug.cgi?id=725702
In virNetServerNew, Coverity didn't realize that srv->mdsnGroupName
can only be non-NULL if mdsnGroupName was non-NULL.
In virNetServerRun, Coverity didn't realize that the array is non-NULL
if the array count is non-zero.
* src/rpc/virnetserver.c (virNetServerNew): Use alternate pointer.
(virNetServerRun): Give coverity a hint.
Detected by Coverity. Freeing the wrong variable results in both
a memory leak and the likelihood of the caller dereferencing through
a freed pointer.
* src/rpc/virnettlscontext.c (virNetTLSSessionNew): Free correct
variable.
Detected by Coverity. We want to compare the result of fnmatch 'rv',
not our pre-set return value 'ret'.
* src/rpc/virnetsaslcontext.c (virNetSASLContextCheckIdentity):
Check correct variable.
Spotted by Coverity. Gnutls documents that buffer must be NULL
if gnutls_x509_crt_get_key_purpose_oid is to be used to determine
the correct size needed for allocating a buffer.
* src/rpc/virnettlscontext.c
(virNetTLSContextCheckCertKeyPurpose): Initialize buffer.
Spotted by coverity. If pipe2 fails, then we attempt to close
uninitialized fds, which may result in a double-close.
* src/rpc/virnetserver.c (virNetServerSignalSetup): Initialize fds.
Steps to reproduce this problem (vm1 is not running):
for i in `seq 50`; do virsh managedsave vm1& done; killall virsh
Pre-patch, virNetServerClientClose could end up setting client->sock
to NULL prior to other cleanup functions trying to use client->sock.
This fixes things by checking for NULL in more places, and by deferring
the cleanup until after all queued messages have been served.
* src/rpc/virnetserverclient.c (virNetServerClientRegisterEvent)
(virNetServerClientGetFD, virNetServerClientIsSecure)
(virNetServerClientLocalAddrString)
(virNetServerClientRemoteAddrString): Check for closed socket.
(virNetServerClientClose): Rearrange close sequence.
Analysis from Wen Congyang.
Without this, cygwin failed to compile:
In file included from ../src/rpc/virnetmessage.h:24,
from ../src/rpc/virnetclient.h:27,
from remote/remote_driver.c:31:
../src/rpc/virnetprotocol.h:9:21: error: rpc/rpc.h: No such file or directory
With that fixed, compilation warned:
rpc/virnetsocket.c: In function 'virNetSocketNewListenUNIX':
rpc/virnetsocket.c:347: warning: format '%d' expects type 'int', but argument 8 has type 'gid_t' [-Wformat]
rpc/virnetsocket.c: In function 'virNetSocketGetLocalIdentity':
rpc/virnetsocket.c:743: warning: pointer targets in passing argument 5 of 'getsockopt' differ in signedness
* src/Makefile.am (libvirt_driver_remote_la_CFLAGS)
(libvirt_net_rpc_client_la_CFLAGS)
(libvirt_net_rpc_server_la_CFLAGS): Include XDR_CFLAGS, for rpc
headers on cygwin.
* src/rpc/virnetsocket.c (virNetSocketNewListenUNIX)
(virNetSocketGetLocalIdentity): Avoid compiler warnings.
On RHEL 5, with gcc 4.1.2:
rpc/virnetsaslcontext.c: In function 'virNetSASLSessionUpdateBufSize':
rpc/virnetsaslcontext.c:396: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
* src/rpc/virnetsaslcontext.c (virNetSASLSessionUpdateBufSize):
Use a union to work around gcc warning.
When an incoming RPC message is ready for processing,
virNetServerClientDispatchRead()
will invoke the 'dispatchFunc' callback. This is set to
virNetServerDispatchNewMessage
This function puts the message + client in a queue for processing by the thread
pool. The thread pool worker function is
virNetServerHandleJob
The first thing this does is acquire an extra reference on the 'client'.
Unfortunately, between the time the message+client are put on the thread pool
queue, and the time the worker runs, the client object may have had its last
reference removed.
We clearly need to add the reference to the client object before putting the
client on the processing queue
* src/rpc/virnetserverclient.c: Add a reference to the client when
invoking the dispatch function
* src/rpc/virnetserver.c: Don't acquire a reference to the client
when in the worker thread
The virNetSASLContext, virNetSASLSession, virNetTLSContext and
virNetTLSSession classes previously relied in their owners
(virNetClient / virNetServer / virNetServerClient) to provide
locking protection for concurrent usage. When virNetSocket
gained its own locking code, this invalidated the implicit
safety the SASL/TLS modules relied on. Thus we need to give
them all explicit locking of their own via new mutexes.
* src/rpc/virnetsaslcontext.c, src/rpc/virnettlscontext.c: Add
a mutex per object
When setting up a server socket, we must skip EADDRINUSE errors
from bind, since the IPv6 socket bind may have already bound to
the IPv4 socket too. If we don't manage to bind to any sockets
at all though, we should then report the EADDRINUSE error as
normal.
This fixes the case where libvirtd would not exit if some other
program was listening on its TCP/TLS ports.
* src/rpc/virnetsocket.c: Report EADDRINUSE
When libvirtd starts it it will sanity check its own certs,
and before libvirt clients connect to a remote server they
will sanity check their own certs. This patch allows such
sanity checking to be skipped. There is no strong reason to
need to do this, other than to bypass possible libvirt bugs
in sanity checking, or for testing purposes.
libvirt.conf gains tls_no_sanity_certificate parameter to
go along with tls_no_verify_certificate. The remote driver
client URIs gain a no_sanity URI parameter
* daemon/test_libvirtd.aug, daemon/libvirtd.conf,
daemon/libvirtd.c, daemon/libvirtd.aug: Add parameter to
allow cert sanity checks to be skipped
* src/remote/remote_driver.c: Add no_sanity parameter to
skip cert checks
* src/rpc/virnettlscontext.c, src/rpc/virnettlscontext.h:
Add new parameter for skipping sanity checks independantly
of skipping session cert validation checks
There is some commonality between the code for sanity checking
certs when initializing libvirt and the code for validating
certs during a live TLS session handshake. This patchset splits
up the sanity checking function into several smaller functions
each doing a specific type of check. The cert validation code
is then updated to also call into these functions
* src/rpc/virnettlscontext.c: Refactor cert validation code
The gnutls_certificate_type_set_priority method is deprecated.
Since we already set the default gnutls priority, it was not
serving any useful purpose and can be removed
* src/rpc/virnettlscontext.c: Remove gnutls_certificate_type_set_priority
call
If the virStateInitialize call fails we must shutdown libvirtd
since drivers will not be available. Just free'ing the virNetServer
is not sufficient, we must send a SIGTERM to ourselves so that
we interrupt the event loop and trigger a orderly shutdown
* daemon/libvirtd.c: Kill ourselves if state init fails
* src/rpc/virnetserver.c: Add some debugging to event loop
The generator can handle everything except virDomainGetBlockJobInfo().
* src/remote/remote_protocol.x: provide defines for the new entry points
* src/remote/remote_driver.c daemon/remote.c: implement the client and
server side for virDomainGetBlockJobInfo.
* src/remote_protocol-structs: structure definitions for protocol verification
* src/rpc/gendispatch.pl: Permit some unsigned long parameters
The only 'void name(void)' style procedure in the protocol is 'close' that
is handled special, but also programming errors like a missing _args or
_ret suffix on the structs in the .x files can create such a situation by
accident. Making the generator aware of this avoids bogus errors from the
generator such as:
Use of uninitialized value in exists at ./rpc/gendispatch.pl line 967.
Also this allows to get rid of the -c option and the special case code for
the 'close' procedure, as the generator handles it now correctly.
Reported by Michal Privoznik
Though we prefer users to have SSH keys setup, virt-manager users still
depend on remote SSH connections to launch a password dialog. This fixes
launch ssh-askpass
Fix suggested by danpb
If a key purpose or usage field is marked as non-critical in the
certificate, then a data mismatch is not (ordinarily) a cause for
rejecting the connection
* src/rpc/virnettlscontext.c: Honour key usage/purpose criticality
If key usage or purpose data is not present in the cert, the
RFC recommends that access be allowed. Also fix checking of
key usage to include requirements for client/server certs,
and fix key purpose checking to treat data as a list of bits
Gnutls requires that certificates have basic constraints present
to be used as a CA certificate. OpenSSL doesn't add this data
by default, so add a sanity check to catch this situation. Also
validate that the key usage and key purpose constraints contain
correct data
* src/rpc/virnettlscontext.c: Add sanity checking of certificate
constraints
If the libvirt daemon or libvirt client is configured with bogus
certificates, it is very unhelpful to only find out about this
when a TLS connection is actually attempted. Not least because
the error messages you get back for failures are incredibly
obscure.
This adds some basic sanity checking of certificates at the
time the virNetTLSContext object is created. This is at libvirt
startup, or when creating a virNetClient instance.
This checks that the certificate expiry/start dates are valid
and that the certificate is actually signed by the CA that is
loaded.
* src/rpc/virnettlscontext.c: Add certificate sanity checks
Since the I/O callback registered against virNetSocket will
hold a reference on the virNetClient, we can't rely on the
virNetClientFree to be able to close the network connection.
The last reference will only go away when the event callback
fires (likely due to EOF from the server).
This is sub-optimal and can potentially cause a leak of the
virNetClient object if the server were to not explicitly
close the socket itself
* src/remote/remote_driver.c: Explicitly close the client
object when disconnecting
* src/rpc/virnetclient.c, src/rpc/virnetclient.h: Add a
virNetClientClose method
When unregistering an I/O callback from a virNetSocket object,
there is still a chance that an event may come in on the callback.
In this case it is possible that the virNetSocket might have been
freed already. Make use of a virFreeCallback when registering
the I/O callbacks and hold a reference for the entire time the
callback is set.
* src/rpc/virnetsocket.c: Register a free function for the
file handle watch
* src/rpc/virnetsocket.h, src/rpc/virnetserverservice.c,
src/rpc/virnetserverclient.c, src/rpc/virnetclient.c: Add
a free function for the socket I/O watches
Remove the need for a virNetSocket object to be protected by
locks from the object using it, by introducing its own native
locking and reference counting
* src/rpc/virnetsocket.c: Add locking & reference counting
If we get an I/O error in the async event callback for an RPC
client, we might not have consumed all pending data off the
wire. This could result in the callback being immediately
invoked again. At which point the same I/O might occur. And
we're invoked again. And again...And again...
Unregistering the async event callback if an error occurs is
a good safety net. The real error will be seen when the next
RPC method is invoked
* src/rpc/virnetclient.c: Unregister event callback on error
These typos are introduced by file renaming in commit b17b4afaf.
src/remote/qemu_protocol.x \
src/remote/remote_protocol.x \
src/rpc/gendispatch.pl:
s/remote_generator/gendispatch/
src/rpc/genprotocol.pl:
s/remote\/remote_protocol/remote_protocol/
If the server succesfully validates the client cert, it will send
back a single byte, under TLS. If it fails, it will close the
connection. In this case, we were just reporting the standard
I/O error. The original RPC code had a special case hack for the
GNUTLS_E_UNEXPECTED_PACKET_LENGTH error code to make us report
a more useful error message
* src/rpc/virnetclient.c: Return ENOMSG if we get
GNUTLS_E_UNEXPECTED_PACKET_LENGTH
* src/rpc/virnettlscontext.c: Report cert failure if we
see ENOMSG
Rather than trying to clean up the ssh child ourselves, and risk
subtle differences from the socket creation error path, we can
just use the new APIs.
* src/rpc/virnetsocket.c (virNetSocketFree): Use new function.
Continuation of commit 313ac7fd, and enforce things with a syntax
check.
Technically, virNetServerClientCalculateHandleMode is not printing
a mode_t, but rather a collection of VIR_EVENT_HANDLE_* bits;
however, these bits are < 8, so there is no different in the
output, and that was the easiest way to silence the new syntax check.
* cfg.mk (sc_flags_debug): New syntax check.
(exclude_file_name_regexp--sc_flags_debug): Add exemptions.
* src/fdstream.c (virFDStreamOpenFileInternal): Print flags in
hex, mode_t in octal.
* src/libvirt-qemu.c (virDomainQemuMonitorCommand)
(virDomainQemuAttach): Likewise.
* src/locking/lock_driver_nop.c (virLockManagerNopInit): Likewise.
* src/locking/lock_driver_sanlock.c (virLockManagerSanlockInit):
Likewise.
* src/locking/lock_manager.c: Likewise.
* src/qemu/qemu_migration.c: Likewise.
* src/qemu/qemu_monitor.c: Likewise.
* src/rpc/virnetserverclient.c
(virNetServerClientCalculateHandleMode): Print mode with %o.
When replacing the default SEGV/ABORT/BUS signal handlers you
can't rely on the process being terminated after your custom
handler runs. It is neccessary to manually restore the default
handler and then re-raise the signal
* src/rpc/virnetserver.c: Restore default handler and raise
signal
This tweaks the RPC generator to cope with some naming
conventions used for the QEMU specific APIs
* daemon/remote.c: Server side dispatcher
* src/remote/remote_driver.c: Client side dispatcher
* src/remote/qemu_protocol.x: Wire protocol definition
* src/rpc/gendispatch.pl: Use '$structprefix' in method
names, fix QEMU flags and fix dispatcher method names
Set StrictHostKeyChecking=no to auto-accept new ssh host keys if the
no_verify extra parameter was specified. This won't disable host key
checking for already known hosts. Includes a test and documentation.
The dispatch for the CLOSE RPC call was invoking the method
virNetServerClientClose(). This caused the client connection
to be immediately terminated. This meant the reply to the
final RPC message was never sent. Prior to the RPC rewrite
we merely flagged the connection for closing, and actually
closed it when the next RPC call dispatch had completed.
* daemon/remote.c: Flag connection for a delayed close
* daemon/stream.c: Update to use new API for closing
failed connection
* src/rpc/virnetserverclient.c, src/rpc/virnetserverclient.h:
Add support for a delayed connection close. Rename the
virNetServerClientMarkClose method to virNetServerClientImmediateClose
to clarify its semantics
When sending back the final OK or ERROR message on completion
of a stream, we were not decrementing the 'nrequests' tracker
on the client. With the default requests limit of '5', this
meant once a client had created 5 streams, they are unable to
process any further RPC calls. There was also a bug when
handling an error from decoding a message length header, which
meant a client connection would not immediately be closed.
* src/rpc/virnetserverclient.c: Fix release of request after
stream completion & mark client for close on error
In one exit path we forgot to free the virNetMessage object causing
a large memory leak for streams which send a lot of data. Some other
paths were calling VIR_FREE directly instead of virNetMessageFree
although this was (currently) harmless.
* src/rpc/virnetclientstream.c: Fix leak of msg object
* src/rpc/virnetclientprogram.c: Call virNetMessageFree instead
of VIR_FREE
The virNetTLSContextNew was being passed key/cert parameters in
the wrong order. This wasn't immediately visible because if
virNetTLSContextNewPath was used, a second bug reversed the order
of those parameters again.
Only if the paths were manually specified in /etc/libvirt/libvirtd.conf
did the bug appear
* src/rpc/virnettlscontext.c: Fix order of params passed to
virNetTLSContextNew
Coverity noted that 4 out of 5 calls to virNetClientStreamRaiseError
checked the return value. This case expects a particular value, so
warn if our expectations went wrong due to some bug elsewhere.
* src/rpc/virnetclient.c (virNetClientCallDispatchStream): Warn on
unexpected scenario.
Detected by Coverity. The leak is on an error path, but I'm not
sure whether that path is likely to be triggered in practice.
* src/rpc/virnetserverservice.c (virNetServerServiceAccept): Plug leak.
Spotted by Coverity. If we don't update tmp each time through
the loop, then if the filter being removed was not the head of
the list, we accidentally lose all filters prior to the one we
wanted to remove.
* src/rpc/virnetserverclient.c (virNetServerClientRemoveFilter):
Don't lose unrelated filters.
Detected by Coverity. No real harm in leaving these, but fixing
them cuts down on the noise for future analysis.
* src/rpc/virnetserver.c (virNetServerAddService): Delete unused
entry.
* src/util/sysinfo.c (virSysinfoRead): Delete dead assignment to
base.
Detected by Coverity. Both are instances of bad things happening
if pipe2 fails; the virNetClientNew failure could free garbage,
and virNetSocketNewConnectCommand could close random fds.
Note: POSIX doesn't guarantee the contents of fd[0] and fd[1]
after pipe failure: http://austingroupbugs.net/view.php?id=467
We may need to introduce a virPipe2 wrapper that guarantees
that on pipe failure, the fds are explicitly set to -1, rather
than our current state of assuming the fds are unchanged from
their value prior to the failed pipe call.
* src/rpc/virnetclient.c (virNetClientNew): Initialize variable.
* src/rpc/virnetsocket.c (virNetSocketNewConnectCommand):
Likewise.
We ignore any stream data packets which come in for streams which
are not registered, since these packets are async and do not have
a reply. If we get a stream control packet though we must send back
an actual error, otherwise a (broken) client may hang forever
making it hard to diagnose the client bug.
* src/rpc/virnetserverprogram.c: Send back error for unexpected
stream control messages
If a message packet for a invalid stream is received it is just
free'd. This is not good because it doesn't let the client RPC
request counter decrement. If a stream is shutdown with pending
packets the message also isn't released properly because of an
incorrect header type
* daemon/stream.c: Fix message header type
* src/rpc/virnetserverprogram.c: Send dummy reply instead of
free'ing ignored stream message
