Ever since commit 61ac8ce, Coverity complained about
remoteNetworkBuildEventLifecycle not checking for NULL failure
to build an event, compared to other calls in the code base.
But the problem is latent from copy and paste; all 17 of our
remote*BuildEvent* functions in remote_driver.c have the same
issue - if an OOM causes an event to not be built, we happily
pass NULL to remoteEventQueue(), but that function has marked
event as a nonnull parameter. We were getting lucky (the
event queue's first use of the event happened to be a call to
virIsObjectClass(), which acts gracefully on NULL, so there
was no way to crash); but this is a latent bug waiting to bite
us due to the disregard for the nonnull attribute, as well as
a waste of resources in the event queue. Better is to just
refuse to queue NULL. The discard is silent, since the problem
only happens on OOM, and since events are already best effort -
if we fail to get an event, it's not like we have any memory
left to report the issue, nor any idea of who would benefit
from knowing we couldn't create or queue the event.
* src/remote/remote_driver.c (remoteEventQueue): Ignore NULL event.
Signed-off-by: Eric Blake <eblake@redhat.com>
Commit cfd62c1 was incomplete; I found more cases where error
messages were being overwritten, and where the code between
the three registration/deregistration APIs was not consistent.
Since it is fairly easy to trigger an attempt to deregister an
unregistered object through public API, I also changed the error
message from VIR_ERR_INTERNAL_ERROR to VIR_ERR_INVALID_ARG.
* src/conf/object_event.c (virObjectEventCallbackListEventID):
Inline...
(virObjectEventStateEventID): ...into lone caller, and report
error on failure.
(virObjectEventCallbackListAddID, virObjectEventStateCallbackID)
(virObjectEventCallbackListRemoveID)
(virObjectEventCallbackListMarkDeleteID): Tweak error category.
* src/remote/remote_driver.c (remoteConnectDomainEventRegister):
Don't leak registration on failure.
(remoteConnectDomainEventDeregisterAny)
(remoteConnectNetworkEventDeregisterAny): Don't overwrite error.
Signed-off-by: Eric Blake <eblake@redhat.com>
While comparing network and domain events, I noticed that the
test driver had to do a cast in one place and not the other.
For consistency, we should hide the necessary casting as low
as possible in the stack, with everything else using saner
types.
* src/conf/network_event.h (virNetworkEventStateRegisterID): Alter
type.
* src/conf/network_event.c (virNetworkEventStateRegisterID): Hoist
cast here.
* src/test/test_driver.c (testConnectNetworkEventRegisterAny):
Simplify callers.
* src/remote/remote_driver.c
(remoteConnectNetworkEventRegisterAny): Likewise.
* src/network/bridge_driver.c
(networkConnectNetworkEventRegisterAny): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Since the introduction of network events, any driver that uses
a single event state object to track both domain and network
events should not include 'domain' in the name of that object.
* src/test/test_driver.c (_testConn):
s/domainEventState/eventState/, and fix all callers.
* src/remote/remote_driver.c (private_data): Likewise.
(remoteDomainEventQueue): Rename to remoteEventQueue.
(remoteDomainEvents): Rename to remoteEvents.
Signed-off-by: Eric Blake <eblake@redhat.com>
Prior to this patch, an attempt to register an event without an
event loop started results in the vague:
libvirt: Remote Driver error : adding cb to list
Now it gives the much nicer:
libvirt: error : internal error: could not initialize domain event timer
This also avoids hiding other reasonable error messages, such as
attempts to register a duplicate callback or OOM errors.
* src/remote/remote_driver.c (remoteConnectNetworkEventRegisterAny)
(remoteConnectDomainEventRegister)
(remoteConnectDomainEventRegisterAny): Preserve more detailed error.
Signed-off-by: Eric Blake <eblake@redhat.com>
While the public API & wire protocol included the 'detail'
arg for network lifecycle events, the internal event handling
code did not process it. This meant that if a future libvirtd
server starts sending non-0 'detail' args, the current libvirt
client will not process them.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
With some authentication mechanism (PLAIN for example), sasl_client_start()
can return SASL_OK, which translates to virNetSASLSessionClientStart()
returning VIR_NET_SASL_COMPLETE.
cyrus-sasl documentation is a bit vague as to what to do in such situation,
but upstream clarified this a bit in
http://asg.andrew.cmu.edu/archive/message.php?mailbox=archive.cyrus-sasl&msg=10104
When we got VIR_NET_SASL_COMPLETE after virNetSASLSessionClientStart() and
if the remote also tells us that authentication is complete, then we should
end the authentication procedure rather than forcing a call to
virNetSASLSessionClientStep(). Without this patch, when trying to use SASL
PLAIN, I get:
error :authentication failed : Failed to step SASL negotiation: -1
(SASL(-1): generic failure: Unable to find a callback: 32775)
This patch is based on a spice-gtk patch by Dietmar Maurer.
The array of sasl_callback_t callbacks which is passed to sasl_client_new()
must be kept alive as long as the created sasl_conn_t object is alive as
cyrus-sasl uses this structure internally for things like logging, so
the memory used for callbacks must only be freed after sasl_dispose() has
been called.
During testing of successful SASL logins with
virsh -c qemu+tls:///system list --all
I've been getting invalid read reports from valgrind
==9237== Invalid read of size 8
==9237== at 0x6E93B6F: _sasl_getcallback (common.c:1745)
==9237== by 0x6E95430: _sasl_log (common.c:1850)
==9237== by 0x16593D87: digestmd5_client_mech_dispose (digestmd5.c:4580)
==9237== by 0x6E91653: client_dispose (client.c:332)
==9237== by 0x6E9476A: sasl_dispose (common.c:851)
==9237== by 0x4E225A1: virNetSASLSessionDispose (virnetsaslcontext.c:678)
==9237== by 0x4CBC551: virObjectUnref (virobject.c:262)
==9237== by 0x4E254D1: virNetSocketDispose (virnetsocket.c:1042)
==9237== by 0x4CBC551: virObjectUnref (virobject.c:262)
==9237== by 0x4E2701C: virNetSocketEventFree (virnetsocket.c:1794)
==9237== by 0x4C965D3: virEventPollCleanupHandles (vireventpoll.c:583)
==9237== by 0x4C96987: virEventPollRunOnce (vireventpoll.c:652)
==9237== by 0x4C94730: virEventRunDefaultImpl (virevent.c:274)
==9237== by 0x12C7BA: vshEventLoop (virsh.c:2407)
==9237== by 0x4CD3D04: virThreadHelper (virthreadpthread.c:161)
==9237== by 0x7DAEF32: start_thread (pthread_create.c:309)
==9237== by 0x8C86EAC: clone (clone.S:111)
==9237== Address 0xe2d61b0 is 0 bytes inside a block of size 168 free'd
==9237== at 0x4A07577: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==9237== by 0x4C73827: virFree (viralloc.c:580)
==9237== by 0x4DE4BC7: remoteAuthSASL (remote_driver.c:4219)
==9237== by 0x4DE33D0: remoteAuthenticate (remote_driver.c:3639)
==9237== by 0x4DDBFAA: doRemoteOpen (remote_driver.c:832)
==9237== by 0x4DDC8DC: remoteConnectOpen (remote_driver.c:1031)
==9237== by 0x4D8595F: do_open (libvirt.c:1239)
==9237== by 0x4D863F3: virConnectOpenAuth (libvirt.c:1481)
==9237== by 0x12762B: vshReconnect (virsh.c:337)
==9237== by 0x12C9B0: vshInit (virsh.c:2470)
==9237== by 0x12E9A5: main (virsh.c:3338)
This commit changes virNetSASLSessionNewClient() to take ownership of the SASL
callbacks. Then we can free them in virNetSASLSessionDispose() after the corresponding
sasl_conn_t has been freed.
When testing SASL authentication over TLS with
virsh -c qemu+tls:///system list --all
I got this valgrind trace after entering wrong credentials:
==30540== 26,903 (88 direct, 26,815 indirect) bytes in 1 blocks are definitely lost in loss record 289 of 293
==30540== at 0x4A081D4: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==30540== by 0x4C7379A: virAllocVar (viralloc.c:558)
==30540== by 0x4CBC178: virObjectNew (virobject.c:190)
==30540== by 0x4CBC329: virObjectLockableNew (virobject.c:216)
==30540== by 0x4E2D003: virNetTLSContextNew (virnettlscontext.c:719)
==30540== by 0x4E2DC3F: virNetTLSContextNewPath (virnettlscontext.c:930)
==30540== by 0x4E2DD5B: virNetTLSContextNewClientPath (virnettlscontext.c:957)
==30540== by 0x4DDB618: doRemoteOpen (remote_driver.c:627)
==30540== by 0x4DDC8BA: remoteConnectOpen (remote_driver.c:1031)
==30540== by 0x4D8595F: do_open (libvirt.c:1239)
==30540== by 0x4D863F3: virConnectOpenAuth (libvirt.c:1481)
==30540== by 0x12762B: vshReconnect (virsh.c:337)
==30540== by 0x12C9B0: vshInit (virsh.c:2470)
==30540== by 0x12E9A5: main (virsh.c:3338)
Most of our code base uses space after comma but not before;
fix the remaining uses before adding a syntax check.
* src/lxc/lxc_container.c: Consistently use commas.
* src/openvz/openvz_driver.c: Likewise.
* src/openvz/openvz_util.c: Likewise.
* src/remote/remote_driver.c: Likewise.
* src/test/test_driver.c: Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Most of the usage of getuid()/getgid() is in cases where we are
considering what privileges we have. As such the code should be
using the effective IDs, not real IDs.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
We don't want setuid programs automatically spawning libvirtd,
so disable any use of autostart when setuid.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
We don't know enough about quality of external libraries used
for non-UNIX transports, nor do we want to spawn external
commands when setuid. Restrict to the bare minimum which is
UNIX transport for local usage. Users shouldn't need to be
running setuid if connecting to remote hypervisors in any
case.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Unconditional use of getenv is not secure in setuid env.
While not all libvirt code runs in a setuid env (since
much of it only exists inside libvirtd) this is not always
clear to developers. So make all the code paranoid, even
if it only ever runs inside libvirtd.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virConnectDomainXMLToNative API should require 'connect:write'
not 'connect:read', since it will trigger execution of the QEMU
binaries listed in the XML.
Also make virConnectDomainXMLFromNative API require a full
read-write connection and 'connect:write' permission. Although the
current impl doesn't trigger execution of QEMU, we should not
rely on that impl detail from an API permissioning POV.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Introduced by 7b87a3
When I quit the process which only register VIR_DOMAIN_EVENT_ID_REBOOT,
I got error like:
"libvirt: XML-RPC error : internal error: domain event 0 not registered".
Then I add the following code, it fixed.
Signed-off-by: Zhou Yimin <zhouyimin@huawei.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1012818
Commit 6d7d0b1869 (in 1.1.2) added bounds
checking to virDomainGetJobStats. But even at that time the API was able
to return 20 parameters while the limit was set to 16.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The polkit access driver used the wrong permission names for checks
on storage pools, volumes and node devices. This led to them always
being denied access.
The 'dettach' permission was also mis-spelt and should have been
'detach'. While permission names are ABI sensitive, the fact that
the code used the wrong object name for checking node device
permissions, means that no one could have used the mis-spelt
'dettach' permission.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The return values for the virConnectListAllSecrets call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The return values for the virConnectListAllNWFilters call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The return values for the virConnectListAllNodeDevices call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The return values for the virConnectListAllInterfaces call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The return values for the virConnectListAllNetworks call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The return values for the virStoragePoolListAllVolumes call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The return values for the virConnectListAllStoragePools call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The return values for the virConnectListAllDomains call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The return values for the virDomain{SnapshotListAllChildren,ListAllSnapshots}
calls were not bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The return values for the virDomainGetJobStats call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The parameters for the virDomainMigrate*Params RPC calls were
not bounds checks, meaning a malicious client can cause libvirtd
to consume arbitrary memory
This issue was introduced in the 1.1.0 release of libvirt
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Valgrind defects memory error:
==16759== 1 errors in context 1 of 8:
==16759== Invalid free() / delete / delete[] / realloc()
==16759== at 0x4A074C4: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==16759== by 0x83CD329: xdr_string (in /usr/lib64/libc-2.17.so)
==16759== by 0x4D93E4D: xdr_remote_nonnull_string (remote_protocol.c:31)
==16759== by 0x4D94350: xdr_remote_nonnull_domain (remote_protocol.c:58)
==16759== by 0x4D976C8: xdr_remote_domain_create_with_flags_ret (remote_protocol.c:1762)
==16759== by 0x83CC734: xdr_free (in /usr/lib64/libc-2.17.so)
==16759== by 0x4D7F1E0: remoteDomainCreateWithFlags (remote_driver.c:2441)
==16759== by 0x4D4BF17: virDomainCreateWithFlags (libvirt.c:9499)
==16759== by 0x13127A: cmdStart (virsh-domain.c:3376)
==16759== by 0x12BF83: vshCommandRun (virsh.c:1751)
==16759== by 0x126FFB: main (virsh.c:3205)
==16759== Address 0xe1394a0 is not stack'd, malloc'd or (recently) free'd
==16759== 1 errors in context 2 of 8:
==16759== Conditional jump or move depends on uninitialised value(s)
==16759== at 0x4A07477: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==16759== by 0x83CD329: xdr_string (in /usr/lib64/libc-2.17.so)
==16759== by 0x4D93E4D: xdr_remote_nonnull_string (remote_protocol.c:31)
==16759== by 0x4D94350: xdr_remote_nonnull_domain (remote_protocol.c:58)
==16759== by 0x4D976C8: xdr_remote_domain_create_with_flags_ret (remote_protocol.c:1762)
==16759== by 0x83CC734: xdr_free (in /usr/lib64/libc-2.17.so)
==16759== by 0x4D7F1E0: remoteDomainCreateWithFlags (remote_driver.c:2441)
==16759== by 0x4D4BF17: virDomainCreateWithFlags (libvirt.c:9499)
==16759== by 0x13127A: cmdStart (virsh-domain.c:3376)
==16759== by 0x12BF83: vshCommandRun (virsh.c:1751)
==16759== by 0x126FFB: main (virsh.c:3205)
==16759== Uninitialised value was created by a stack allocation
==16759== at 0x4D7F120: remoteDomainCreateWithFlags (remote_driver.c:2423)
How to reproduce?
# virsh start <domain> --paused
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=994855
Signed-off-by: Alex Jia <ajia@redhat.com>
In the following commit:
commit 03d813bbcd
Author: Marek Marczykowski <marmarek@invisiblethingslab.com>
Date: Thu May 23 02:01:30 2013 +0200
remote: fix dom->id after virDomainCreateWithFlags
The virDomainCreateWithFlags remote client helper was made to
invoke REMOTE_PROC_DOMAIN_LOOKUP_BY_UUID to refresh the 'id'
of the domain, following the pattern used in the previous
virDomainCreate method impl.
The remote protocol for virDomainCreateWithFlags though did
actually fix the design flaw in virDomainCreate, by directly
returning the new domain info. For some reason, this data was
never used. So we can just use that data now instead.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Since they make use of file descriptor passing, the remote protocol
methods for virDomainCreate{XML}WithFiles must be written by hand.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This patch enables the password authentication in the libssh2 connection
driver. There are a few benefits to this step:
1) Hosts with challenge response authentication will now be supported
with the libssh2 connection driver.
2) Credential for hosts can now be stored in the authentication
credential config file
Convert the type of loop iterators named 'i', 'j', k',
'ii', 'jj', 'kk', to be 'size_t' instead of 'int' or
'unsigned int', also santizing 'ii', 'jj', 'kk' to use
the normal 'i', 'j', 'k' naming
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Found while trying to cross-compile to mingw:
CC libvirt_driver_remote_la-remote_driver.lo
../../src/remote/remote_driver.c: In function 'doRemoteOpen':
../../src/remote/remote_driver.c:487:23: error: variable 'verify' set but not used [-Werror=unused-but-set-variable]
* src/remote/remote_driver.c (doRemoteOpen): Also ignore 'verify'.
Signed-off-by: Eric Blake <eblake@redhat.com>
Introduce annotations to all RPC messages to declare what
access control checks are required. There are two new
annotations defined:
@acl: <object>:<permission>
@acl: <object>:<permission>:<flagname>
Declare the access control requirements for the API. May be repeated
multiple times, if multiple rules are required.
<object> is one of 'connect', 'domain', 'network', 'storagepool',
'interface', 'nodedev', 'secret'.
<permission> is one of the permissions in access/viraccessperm.h
<flagname> indicates the rule only applies if the named flag
is set in the API call
@aclfilter: <object>:<permission>
Declare an access control filter that will be applied to a list
of objects being returned by an API. This allows the returned
list to be filtered to only show those the user has permissions
against
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Without the socket path explicitly specified, the remote driver tried to
connect to the "/system" instance socket even if "/session" was
specified in the uri. With this patch this configuration now produces an
error.
It is still possible to initiate a session connection with specifying
the path to the socket manually and also manually starting the session
daemon. This was also possible prior to this patch,
This is a minimal fix. We may decide to support remote session
connections using ssh but this will require changes to the remote driver
code so this fix shouldn't cause regressions in the case we decide to do
that.
The RPC limits for cpu maps didn't allow to use libvirt on ultra big
boxes. This patch increases size of the limits to support a maximum of
16384 cpus on the host with a maximum of 4096 cpus per guest.
The full cpu map of such a system takes 8 megabytes and the map for
vcpu pinning is 2 kilobytes long.
The same issue as (already fixed) in virDomainCreate -
REMOTE_PROC_DOMAIN_CREATE_WITH_FLAGS doesn't return new domain ID, only
-1 on error or 0 on success.
Besides this one fix it is more general problem - local domain object
ID can desynchronize with the real one, for example in case of another
client creates/destroys domain in the meantime. Perhaps virDomainGetID
should be called remotely (with all performance implications...)? Or
some event-based notification used?
Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
We have seen an issue on s390x platform where domain XMLs larger than 1MB
were used. The define command was finished successfully. The dumpxml command
was not successful (i.e. could not encode message payload).
Enlarged message related sizes (e.g. maximum string size, message size, etc.)
to handle larger system configurations used on s390x platform.
To improve handling of the RPC message size the allocation during encode process
is changed to a dynamic one (i.e. starting with 64kB initial size and increasing
that size in steps up to 16MB if the payload data is larger).
Signed-off-by: Daniel Hansel <daniel.hansel@linux.vnet.ibm.com>
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
The source code base needs to be adapted as well. Some files
include virutil.h just for the string related functions (here,
the include is substituted to match the new file), some include
virutil.h without any need (here, the include is removed), and
some require both.
This requires a custom function for remoteNodeDeviceDetachFlags,
because it is named *NodeDevice, but it goes through the hypervisor
driver rather than nodedevice driver, and so it uses privateData
instead of nodeDevicePrivateData. (It has to go through the hypervisor
driver, because that is the driver that knows about the backend drivers
that will perform the pci device assignment).
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>
It will simplify later work if the sub-drivers have dedicated
APIs / field names. ie virNetworkDriver should have
virDrvNetworkOpen and virDrvNetworkClose methods
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>
Ensure that the driver struct field names match the public
API names. For an API virXXXX we must have a driver struct
field xXXXX. ie strip the leading 'vir' and lowercase any
leading uppercase letters.
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>
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>
The last Viktor's effort to fix the race and memory corruption unfortunately
wasn't complete in the case the close callback was not registered in an
connection. At that time, the trail of event's that I'll describe later could
still happen and corrupt the memory or cause a crash of the client (including
the daemon in case of a p2p migration).
Consider the following prerequisities and trail of events:
Let's have a remote connection to a hypervisor that doesn't have a close
callback registered and the client is using the event loop. The crash happens in
cooperation of 2 threads. Thread E is the event loop and thread W is the worker
that does some stuff. R denotes the remote client.
1.) W - The client finishes everything and sheds the last reference on the client
2.) W - The virObject stuff invokes virConnectDispose that invokes doRemoteClose
3.) W - the remote close method invokes the REMOTE_PROC_CLOSE RPC method.
4.) W - The thread is preempted at this point.
5.) R - The remote side receives the close and closes the socket.
6.) E - poll() wakes up due to the closed socket and invokes the close callback
7.) E - The event loop is preempted right before remoteClientCloseFunc is called
8.) W - The worker now finishes, and frees the conn object.
9.) E - The remoteClientCloseFunc accesses the now-freed conn object in the
attempt to retrieve pointer for the real close callback.
10.) Kaboom, corrupted memory/segfault.
This patch tries to fix this by introducing a new object that survives the
freeing of the connection object. We can't increase the reference count on the
connection object itself or the connection would never be closed, as the
connection is closed only when the reference count reaches zero.
The new object - virConnectCloseCallbackData - is a lockable object that keeps
the pointers to the real user registered callback and ensures that the
connection callback is either not called if the connection was already freed or
that the connection isn't freed while this is being called.
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.
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.
Upon successful return of virNetClientStreamEventAddCallback() the
allocated cbdata field will be freed by virNetClientStreamEventRemoveCallback()
as cbOpaque using the free function remoteStreamCallbackFree().
This patch adds a new API, virDomainOpenChannel, that uses streams to
connect to a virtio channel on a guest. This creates a secure
communication channel between a guest and a libvirt client.
This behaves the same as virDomainOpenConsole, except on channels
instead of console/serial/parallel devices.
https://bugzilla.redhat.com/show_bug.cgi?id=866524
Since the virConnect object is not locked wholely when doing
virConenctDispose, a thread can get the lock and thus might
cause the race.
Detected by valgrind:
==23687== Invalid read of size 4
==23687== at 0x38BAA091EC: pthread_mutex_lock (pthread_mutex_lock.c:61)
==23687== by 0x3FBA919E36: remoteClientCloseFunc (remote_driver.c:337)
==23687== by 0x3FBA936BF2: virNetClientCloseLocked (virnetclient.c:688)
==23687== by 0x3FBA9390D8: virNetClientIncomingEvent (virnetclient.c:1859)
==23687== by 0x3FBA851AAE: virEventPollRunOnce (event_poll.c:485)
==23687== by 0x3FBA850846: virEventRunDefaultImpl (event.c:247)
==23687== by 0x40CD61: vshEventLoop (virsh.c:2128)
==23687== by 0x3FBA8626F8: virThreadHelper (threads-pthread.c:161)
==23687== by 0x38BAA077F0: start_thread (pthread_create.c:301)
==23687== by 0x33F68E570C: clone (clone.S:115)
==23687== Address 0x4ca94e0 is 144 bytes inside a block of size 312 free'd
==23687== at 0x4A0595D: free (vg_replace_malloc.c:366)
==23687== by 0x3FBA8588B8: virFree (memory.c:309)
==23687== by 0x3FBA86AAFC: virObjectUnref (virobject.c:145)
==23687== by 0x3FBA8EA767: virConnectClose (libvirt.c:1458)
==23687== by 0x40C8B8: vshDeinit (virsh.c:2584)
==23687== by 0x41071E: main (virsh.c:3022)
The above race is caused by the eventLoop thread tries to handle
the net client event by calling the callback set by:
virNetClientSetCloseCallback(priv->client,
remoteClientCloseFunc,
conn, NULL);
I.E. remoteClientCloseFunc, which lock/unlock the virConnect object.
This patch is to fix the bug by setting the callback to NULL when
doRemoteClose.
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>
The virStateInitialize method and several cgroups methods were
using an 'int privileged' parameter or similar for dual-state
values. These are better represented with the bool type.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
* src/remote/remote_protocol.x: message definition
* src/remote/remote_driver.c: Register driver function
* src/remote_protocol-structs: Test case
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This will simplify the refactoring of the ESX storage driver to support
a VMFS and an iSCSI backend.
One of the tasks the storage driver needs to do is to decide which backend
driver needs to be invoked for a given request. This approach extends
virStoragePool and virStorageVol to store extra parameters:
1. privateData: stores pointer to respective backend storage driver.
2. privateDataFreeFunc: stores cleanup function pointer.
virGetStoragePool and virGetStorageVol are modfied to accept these extra
parameters as user params. virStoragePoolDispose and virStorageVolDispose
checks for cleanup operation if available.
The private data pointer allows the ESX storage driver to store a pointer
to the used backend with each storage pool and volume. This avoids the need
to detect the correct backend in each storage driver function call.
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>
It turns out that calling virNodeGetCPUMap(conn, NULL, NULL, 0)
is both useful, and with Viktor's patches, common enough to
optimize. Since this interface hasn't been released yet, we
can change the RPC call.
A bit more background on the optimization - learning the cpu count
is a single file read (/sys/devices/system/cpu/possible), but
learning the number of online cpus can possibly trigger a file
read per cpu, depending on the age of the kernel, and all wasted
if the caller passed NULL for both arguments.
* src/nodeinfo.c (nodeGetCPUMap): Avoid bitmap when not needed.
* src/remote/remote_protocol.x (remote_node_get_cpu_map_args):
Supply two separate flags for needed arguments.
* src/remote/remote_driver.c (remoteNodeGetCPUMap): Update
caller.
* daemon/remote.c (remoteDispatchNodeGetCPUMap): Likewise.
* src/remote_protocol-structs: Regenerate.
- Defined the wire protocol format for virNodeGetCPUMap and its
arguments
- Implemented remote method invocation (remoteNodeGetCPUMap)
- Implemented method dispatcher (remoteDispatchNodeGetCPUMap)
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
This patch adds support for SUSPEND_DISK event; both lifecycle and
separated. The support is added for QEMU, machines are changed to
PMSUSPENDED, but as QEMU sends SHUTDOWN afterwards, the state changes
to shut-off. This and much more needs to be done in order for libvirt
to work with transient devices, wake-ups etc. This patch is not
aiming for that functionality.
When auto-probing hypervisor drivers, the conn->uri field will
initially be NULL. Care must be taken not to access members
when doing auth lookups in the config file
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/
Currently we search along the hard-coded names:
SBINDIR "/libvirtd"
SBINDIR "/libvirtd_dbg"
but if the environment variable $LIBVIRTD_PATH is set to the
name of the libvirtd binary, that is used instead. Fix the
error message so it accurately reflects current behaviour
($PATH is NOT searched).
This is very short, because almost everything is autogenerated. All
that's needed are:
* src/remote/remote_driver.c: add pointer to autogenerated
remoteNetworkUpdate to the function table for the remote
network driver.
* src/remote/remote_protocol.x: add the "args" struct and add one more
item to the remote_procedure enum for this function.
* src/remote_protocol-struct: update to match remote_protocol.x
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
The RPC generator doesn't support returning list of object yet, this patch
does the work manually.
* daemon/remote.c:
Implement the server side handler remoteDispatchConnectListAllSecrets.
* src/remote/remote_driver.c:
Add remote driver handler remoteConnectListAllSecrets.
* src/remote/remote_protocol.x:
New RPC procedure REMOTE_PROC_CONNECT_LIST_ALL_SECRETS and
structs to represent the args and ret for it.
* src/remote_protocol-structs: Likewise.
The RPC generator doesn't support returning list of object yet, this patch
do the work manually.
* daemon/remote.c:
Implemente the server side handler remoteDispatchConnectListAllNWFilters.
* src/remote/remote_driver.c:
Add remote driver handler remoteConnectListAllNWFilters.
* src/remote/remote_protocol.x:
New RPC procedure REMOTE_PROC_CONNECT_LIST_ALL_NWFILTERS and
structs to represent the args and ret for it.
* src/remote_protocol-structs: Likewise.
The RPC generator doesn't support returning list of object yet, this patch
does the work manually.
* daemon/remote.c:
Implemente the server side handler remoteDispatchConnectListAllNodeDevices.
* src/remote/remote_driver.c:
Add remote driver handler remoteConnectListAllNodeDevices.
* src/remote/remote_protocol.x:
New RPC procedure REMOTE_PROC_CONNECT_LIST_ALL_INTERFACES and
The RPC generator doesn't support returning list of object yet, this patch
do the work manually.
* daemon/remote.c:
Implemente the server side handler remoteDispatchConnectListAllInterfaces.
* src/remote/remote_driver.c:
Add remote driver handler remoteConnectListAllInterfaces.
* src/remote/remote_protocol.x:
New RPC procedure REMOTE_PROC_CONNECT_LIST_ALL_INTERFACES and
structs to represent the args and ret for it.
* src/remote_protocol-structs: Likewise.
The remote driver first looks at the libvirt auth config file to
fill in any credentials. It then invokes the auth callback for
any remaining credentials. It was accidentally invoking the
auth callback even if there were not any more credentials
required.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The RPC generator doesn't support returning list of object, this patch
do the work manually.
* daemon/remote.c:
Implemente the server side handler remoteDispatchConnectListAllNetworks.
* src/remote/remote_driver.c:
Add remote driver handler remoteConnectListAllNetworks.
* src/remote/remote_protocol.x:
New RPC procedure REMOTE_PROC_CONNECT_LIST_ALL_NETWORKS and
structs to represent the args and ret for it.
* src/remote_protocol-structs: Likewise.
The RPC generator doesn't returning support list of object, this
patch do the work manually.
* daemon/remote.c:
Implemente the server side handler remoteDispatchStoragePoolListAllVolumes
* src/remote/remote_driver.c:
Add remote driver handler remoteStoragePoolListAllVolumes
* src/remote/remote_protocol.x:
New RPC procedure REMOTE_PROC_STORAGE_POOL_LIST_ALL_VOLUMES and
structs to represent the args and ret for it.
* src/remote_protocol-structs: Likewise.
The RPC generator doesn't support returning list of object, this patch does
the work manually.
* daemon/remote.c:
Implement the server side handler remoteDispatchConnectListAllStoragePools
* src/remote/remote_driver.c:
Add remote driver handler remoteConnectListAllStoragePools.
* src/remote/remote_protocol.x:
New RPC procedure REMOTE_PROC_CONNECT_LIST_ALL_STORAGE_POOLS and
structs to represent the args and ret for it.
* src/remote_protocol-structs: Likewise.
The recent virDomainQemuAgentCommand addition is part of 0.10.0;
also, grouping all libvirt-qemu.so callbacks together makes them
easier to identify.
* src/libvirt_qemu.syms: Fix release symbol.
* src/qemu/qemu_driver.c (qemuDriver): Likewise.
* src/remote/remote_driver.c (remote_driver): Likewise.
* src/driver.h (_virDriver): Group qemu-specific callbacks.
Add qemuDomainAgentCommand() which is generated automatically,
for .qemuDomainArbitraryAgentCommand to remote driver.
Signed-off-by: MATSUDA Daiki <matsudadik@intellilink.co.jp>
Introduce 2 APIs to support emulator threads in remote driver.
1) remoteDomainPinEmulator: call driver api, such as qemudDomainPinEmulator.
2) remoteDomainGetEmulatorPinInfo: call driver api, such as qemudDomainGetEmulatorPinInfo.
They are similar to remoteDomainPinVcpuFlags and remoteDomainGetVcpuPinInfo.
Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
This patch adds URI options to support libssh2 transport in the remote
driver.
A new transport sceme is introduced eg. "qemu+libssh2://..." that
utilizes the libssh2 code added in previous patches.
The libssh2 code requires the authentication callback to be able to
perform keyboard-interactive authentication or to ask t passprhases or
add host keys to known hosts database.
Added URI components:
- known_hosts - path to a knownHosts file in OpenSSH format to check
for known ssh host keys
- known_hosts_verify - how to deal with server key verification:
* "normal" (default) - ask to add new keys
* "auto" - automaticaly add new keys
* "ignore" - don't validate host keys
- sshauth - authentication methods to use. Default is
"agent,privkey,keyboard-interactive". It's a comma separated
string of methods to try while authenticating. The order is
preserved. Some of the methods may require additional
parameters.
Locations of the known_hosts file and private keys are set to default
values if they're present. (~/.ssh/known_hosts, ~/.ssh/id_rsa,
~/.ssh/id_dsa)
This patch updates libvirt's API to allow applications to inspect the
full list of security labels of a domain.
Signed-off-by: Marcelo Cerri <mhcerri@linux.vnet.ibm.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>
Remove spaces before function calls and some other coding nits in some
parts of the remote driver and refactor getting of URI argument
components into variables used by libvirt later on.
The remote driver did not fill the required snapshot parent argument in
the RPC call structure that caused a client crash when trying to use
this new API.
With 0.10.0-rc0 out the door, we are committed to the next version
number.
* src/libvirt_public.syms (LIBVIRT_0.9.14): Rename...
(LIBVIRT_0.10.0): ...to this.
* docs/formatdomain.html.in: Fix fallout.
* src/openvz/openvz_driver.c (openvzDriver): Likewise.
* src/remote/remote_driver.c (remote_driver): Likewise.
The option 'srcSpec' to virsh command find-storage-pool-sources
is optional for logical type of storage pool, but mandatory for
netfs and iscsi type.
When missing the option for netfs and iscsi, libvirt reports XML
parsing error due to null string option srcSpec.
before
error: Failed to find any netfs pool sources
error: (storage_source_specification):1: Document is empty
(null)
after:
error: pool type 'iscsi' requires option --srcSpec for source discovery
Update the remote driver to use the virNetClient close callback
to trigger the virConnectPtr close callbacks
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Per the FSF address could be changed from time to time, and GNU
recommends the following now: (http://www.gnu.org/licenses/gpl-howto.html)
You should have received a copy of the GNU General Public License
along with Foobar. If not, see <http://www.gnu.org/licenses/>.
This patch removes the explicit FSF address, and uses above instead
(of course, with inserting 'Lesser' before 'General').
Except a bunch of files for security driver, all others are changed
automatically, the copyright for securify files are not complete,
that's why to do it manually:
src/security/security_selinux.h
src/security/security_driver.h
src/security/security_selinux.c
src/security/security_apparmor.h
src/security/security_apparmor.c
src/security/security_driver.c
Remote driver needs to make sure the driver lock is released before
entering client IO loop as that may block indefinitely in poll(). As a
direct consequence of not following this in stream APIs, tunneled
migration to a destination host which becomes non-responding may block
qemu driver. Luckily, if keepalive is turned for p2p migrations, both
remote and qemu drivers will get automagically unblocked after keepalive
timeout.
When the guest changes its memory balloon applications may want
to know what the new value is, without having to periodically
poll on XML / domain info. Introduce a "balloon change" event
to let apps see this
* include/libvirt/libvirt.h.in: Define the
virConnectDomainEventBalloonChangeCallback callback
and VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE constant
* python/libvirt-override-virConnect.py,
python/libvirt-override.c: Wire up helpers for new event
* daemon/remote.c: Helper for serializing balloon event
* examples/domain-events/events-c/event-test.c,
examples/domain-events/events-python/event-test.py: Add
example of balloon event usage
* src/conf/domain_event.c, src/conf/domain_event.h: Handling
of balloon events
* src/remote/remote_driver.c: Add handler of balloon events
* src/remote/remote_protocol.x: Define wire protocol for
balloon events
* src/remote_protocol-structs: Likewise.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The generator doesn't handle lists of virDomainSnapshotPtr, so
this commit requires a bit more work than some RPC additions.
* src/remote/remote_protocol.x
(REMOTE_PROC_DOMAIN_LIST_ALL_SNAPSHOTS)
(REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_ALL_CHILDREN): New RPC calls,
with corresponding structs.
* daemon/remote.c (remoteDispatchDomainListAllSnapshots)
(remoteDispatchDomainSnapshotListAllChildren): New functions.
* src/remote/remote_driver.c (remoteDomainListAllSnapshots)
(remoteDomainSnapshotListAllChildren): Likewise.
* src/remote_protocol-structs: Regenerate.
This patch wires up the RPC protocol handlers for
virConnectListAllDomains(). The RPC generator has no support for the way
how virConnectListAllDomains() returns the results so the handler code
had to be done manually.
The new api is handled by REMOTE_PROC_CONNECT_LIST_ALL_DOMAINS, with
number 273 and marked with high priority.
Since we are allocating RPC buffer dynamically, we can increase limits
for max. size of RPC message and RPC string. This is needed to cover
some corner cases where libvirt is run on such huge machines that their
capabilities XML is 4 times bigger than our current limit. This leaves
users with inability to even connect.
Remove the uid param from virGetUserConfigDirectory,
virGetUserCacheDirectory, virGetUserRuntimeDirectory,
and virGetUserDirectory
These functions were universally called with the
results of getuid() or geteuid(). To make it practical
to port to Win32, remove the uid parameter and hardcode
geteuid()
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The current unprivileged user libvirtd sockets are in the abstract
namespace. This has a number of problems
- You can't connect to them remotely using the nc/ssh tunnel
- This is not portable for OS-X, BSD & probably others
- Parent directory permissions don't apply
As defined in:
http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html
This offers a number of advantages:
* Allows sharing a home directory between different machines, or
sessions (eg. using NFS)
* Cleanly separates cache, runtime (eg. sockets), or app data from
user settings
* Supports performing smart or selective migration of settings
between different OS versions
* Supports reseting settings without breaking things
* Makes it possible to clear cache data to make room when the disk
is filling up
* Allows us to write a robust and efficient backup solution
* Allows an admin flexibility to change where data and settings are stored
* Dramatically reduces the complexity and incoherence of the
system for administrators
The docs for virConnectSetKeepAlive() advertise that this function
should be able to disable keepalives on negative or zero interval time.
This patch removes the check that prohibited this and adds code to
disable keepalives on negative/zero interval.
* src/libvirt.c: virConnectSetKeepAlive(): - remove check for negative
values
* src/rpc/virnetclient.c
* src/rpc/virnetclient.h: - add virNetClientKeepAliveStop() to disable
keepalive messages
* src/remote/remote_driver.c: remoteSetKeepAlive(): -add ability to
disable keepalives
The code is splattered with a mix of
sizeof foo
sizeof (foo)
sizeof(foo)
Standardize on sizeof(foo) and add a syntax check rule to
enforce it
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
A handful of places used %zd for format specifiers even
though the args was size_t, not ssize_t.
* src/remote/remote_driver.c, src/util/xml.c: s/%zd/%zu/
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Return statements with parameter enclosed in parentheses were modified
and parentheses were removed. The whole change was scripted, here is how:
List of files was obtained using this command:
git grep -l -e '\<return\s*([^()]*\(([^()]*)[^()]*\)*)\s*;' | \
grep -e '\.[ch]$' -e '\.py$'
Found files were modified with this command:
sed -i -e \
's_^\(.*\<return\)\s*(\(\([^()]*([^()]*)[^()]*\)*\))\s*\(;.*$\)_\1 \2\4_' \
-e 's_^\(.*\<return\)\s*(\([^()]*\))\s*\(;.*$\)_\1 \2\3_'
Then checked for nonsense.
The whole command looks like this:
git grep -l -e '\<return\s*([^()]*\(([^()]*)[^()]*\)*)\s*;' | \
grep -e '\.[ch]$' -e '\.py$' | xargs sed -i -e \
's_^\(.*\<return\)\s*(\(\([^()]*([^()]*)[^()]*\)*\))\s*\(;.*$\)_\1 \2\4_' \
-e 's_^\(.*\<return\)\s*(\([^()]*\))\s*\(;.*$\)_\1 \2\3_'
This patch introduces a new event type for the QMP event
SUSPEND:
VIR_DOMAIN_EVENT_ID_PMSUSPEND
The event doesn't take any data, but considering there might
be reason for wakeup in future, the callback definition is:
typedef void
(*virConnectDomainEventSuspendCallback)(virConnectPtr conn,
virDomainPtr dom,
int reason,
void *opaque);
"reason" is unused currently, always passes "0".
This patch introduces a new event type for the QMP event
WAKEUP:
VIR_DOMAIN_EVENT_ID_PMWAKEUP
The event doesn't take any data, but considering there might
be reason for wakeup in future, the callback definition is:
typedef void
(*virConnectDomainEventWakeupCallback)(virConnectPtr conn,
virDomainPtr dom,
int reason,
void *opaque);
"reason" is unused currently, always passes "0".
This patch introduces a new event type for the QMP event
DEVICE_TRAY_MOVED, which occurs when the tray of a removable
disk is moved (i.e opened or closed):
VIR_DOMAIN_EVENT_ID_TRAY_CHANGE
The event's data includes the device alias and the reason
for tray status' changing, which indicates why the tray
status was changed. Thus the callback definition for the event
is:
enum {
VIR_DOMAIN_EVENT_TRAY_CHANGE_OPEN = 0,
VIR_DOMAIN_EVENT_TRAY_CHANGE_CLOSE,
\#ifdef VIR_ENUM_SENTINELS
VIR_DOMAIN_EVENT_TRAY_CHANGE_LAST
\#endif
} virDomainEventTrayChangeReason;
typedef void
(*virConnectDomainEventTrayChangeCallback)(virConnectPtr conn,
virDomainPtr dom,
const char *devAlias,
int reason,
void *opaque);
