Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Currently we have issues like [1] on libvirtd shutdown as we cleanup while RPC
and other threads are still running. Let's finish all threads other then main
before cleanup.
The approach to finish threads is suggested in [2]. In order to finish RPC
threads serving API calls we let the event loop run but stop accepting new API
calls and block processing any pending API calls. We also inform all drivers of
shutdown so they can prepare for shutdown too. Then we wait for all RPC threads
and driver's background thread to finish. If finishing takes more then 15s we
just exit as we can't safely cleanup in time.
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1828207
[2] https://www.redhat.com/archives/libvir-list/2020-April/msg01328.html
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
virNetServerClose and virNetServerShutdownWait are used to start net server
threads shutdown and wait net server threads to actually finish respectively
during net daemon shutdown procedure.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The function is used to set shutdown prepare and wait callbacks. Prepare
callback is used to inform other threads of the daemon that the daemon will be
closed soon so that they can start to shutdown. Wait callback is used to wait
for other threads to actually finish.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Second unref was added in [1]. We don't need it actually as
we pass free callback to virNetSocketAddIOCallback thus
when we call virNetSocketRemoveIOCallback the extra ref for
callback will be dropped without extra efforts.
[1] 355d8f470f: virNetServerServiceClose: Don't leak sockets
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Currently, we are mixing: #if HAVE_BLAH with #if WITH_BLAH.
Things got way better with Pavel's work on meson, but apparently,
mixing these two lead to confusing and easy to miss bugs (see
31fb929eca for instance). While we were forced to use HAVE_
prefix with autotools, we are free to chose our own prefix with
meson and since WITH_ prefix appears to be more popular let's use
it everywhere.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use https: links for websites that support them.
The URIs which are used as namespace identifiers
are left alone.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Many of our functions start with a DEBUG statement.
Move the statements after declarations to appease
our coding style.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Split those initializations that depend on a statement
above them.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
These leaks were introduced in commit 15d280fa97, use g_autofree for all
cert_path pointers.
Signed-off-by: Jin Yan <jinyan12@huawei.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The source ID number is an alternative way to identify a source that has
been added to a GMainContext. Internally when a source ID is given, glib
will lookup the corresponding GSource and use that. The use of a source
ID is racy in some cases though, because it is invalid to continue to
use an ID number after the GSource has been removed. It is thus safer
to use the GSource object directly and have full control over the ref
counting and thus cleanup.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
EXTRA_DIST is not relevant because meson makes a git copy when creating
dist archive so everything tracked by git is part of dist tarball.
The remaining ones are not converted to meson files as they are
automatically tracked by meson.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
In 9536379da4 and 8b0cb0e666 I've tried to call
virNetSocketCheckProtocolByLookup() only if we are suspecting the
host is IPv4 or IPv6 capable (because we've found an interface
with such address). However, the code was missing dereference of
the boolean variables and thus was comparing pointers against
NULL.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Historically, if we found an interface with an IPv6 address we
did not blindly trust that host is IPv6 capable (as in we can
successfully translate IPv4 addresses) but used getaddrinfo() to
confirm it. Turns out, we have use the same argument for IPv4.
For instance, in an namespace created by the following steps,
getaddrinfo("127.0.0.1", ...) fails (demonstrating by "Socket
TCP/IPv4 Accept" test case failing in virnetsockettest):
unshare -n
ip link set lo up
ip link add dummy0 type dummy
ip link set dummy0 up
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There is not much sense trying to disprove host is IPv6 capable
if we know after first round (getifaddrs()) that is is not.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virNetSocketCheckProtocols() function is supposed to tell
caller whether IPv4 and/or IPv6 is supported on the system. In
the initial round, it uses getifaddrs() to see if an interface
has IPv4/IPv6 address assigned and then to double check IPv6 it
uses getaddrinfo() to lookup IPv6 loopback address. Separate out
this latter code because it is going to be reused.
Since the original code lived under an #ifdef and the new
function doesn't it is marked as unused - because on some systems
it may be so.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Historically, we've used security_context_t for variables passed
to libselinux APIs. But almost 7 years ago, libselinux developers
admitted in their API that in fact, it's just a 'char *' type
[1]. Ever since then the APIs accept 'char *' instead, but they
kept the old alias just for API stability. Well, not anymore [2].
1: 9eb9c93275
2: 7a124ca275
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The term "access control list" better describes the concept involved.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Memory allocated using g_object_new must never be released using
VIR_FREE/g_free because g_object_new uses a special allocation
strategy internally.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
In a few places we use 0 and false, or 1 and true interchangeably
even though the variable or return type in question is boolean.
Fix those places.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
There are few functions that currently return an integer but in
fact they always return the same integer (zero). Make them void.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Since it's introduction in v0.9.7-147-gf4324e3292 the
virNetServerClientInitKeepAlive() function returned nothing than
a negative one. Fortunately, this did not pose any problem
because we ignored the retval happily. Well, it's time to check
for the retval because the function might fail regularly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The node device APIs are a little unusual because we don't use a
"remote_nonnull_node_device" object on the wire, instead we just
have a "remote_string" for the device name. This meant dispatcher
code generation needed special cases. In doing so we mistakenly
used the virNodeDeviceLookupByName() API which gets dispatched
into the driver, instead of get_nonnull_node_device() which
directly populates a virNodeDevicePtr object.
This wasn't a problem with monolithic libvirtd, as the
virNodeDeviceLookupByName() API call was trivially satisfied
by the registered driver, albeit with an extra (undesirable)
authentication check. With the split daemons, the call to
virNodeDeviceLookupByName() fails in virtqemud, because the
node device driver obviously doesn't exist in that daemon.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Despite their names, the following APIs:
virNodeDeviceDettach
virNodeDeviceDetachFlags
virNodeDeviceReAttach
virNodeDeviceReset
are all handled by the virt drivers, not the node device driver.
A bug in the RPC generator meant that these APIs were sent to
the nodedev driver for handling. This caused breakage with the
split daemons, since nothing was available to process them.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Prevent the handshake function from reading 'tlsCtxt' while
updating 'tlsCtxt'.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Zhang Bo <oscar.zhangbo@huawei.com>
Signed-off-by: Wu Qingliang <wuqingliang4@huawei.com>
Add an API to update server's tls context.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Zhang Bo <oscar.zhangbo@huawei.com>
Signed-off-by: Wu Qingliang <wuqingliang4@huawei.com>
If only IPv6 is configured on the host, getaddrinfo with AI_ADDRCONFIG
in hints would return EAI_ADDRFAMILY for nodenames that resolve to IPv4.
Also pass AI_V4MAPPED to accept IPv4-mapped addresses on IPv6-only
systems.
Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
[rewrote the commit message - jtomko]
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Historically threads are given a name based on the C function,
and this name is just used inside libvirt. With OS level thread
naming this name is now visible to debuggers, but also has to
fit in 15 characters on Linux, so function names are too long
in some cases.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Make it obvious that the function always returns a valid pointer and fix
all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Historically, this file was a dump for most of our helper
functions and needed almost everywhere.
With the introduction of virfile.h and virstring.h,
and more importantly, virenum.h and the introduction
of GLib, that is no longer true.
Remove its include from C files that don't even use it.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This deletes all trace of gnulib from libvirt. We still
have the keycodemapdb submodule to deal with. The simple
solution taken was to update it when running autogen.sh.
Previously gnulib could auto-trigger refresh when running
'make' too. We could figure out a solution for this, but
with the pending meson rewrite it isn't worth worrying
about, given how infrequently keycodemapdb changes.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
To eliminate the dependancy on GNULIB's poll impl, we need
to change the RPC client code to use GMainLoop. We don't
really want to use GIOChannel, but it provides the most
convenient way to do socket event watches with Windows
portability. The other alternative would be to use GSocket
but that is a much more complex change affecting libvirt
more broadly.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This lets it generate the remote dispatch for StorageVolGetInfoFlags.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This addreses portability to Windows and standardizes
error reporting. This fixes a number of places which
failed to set O_CLOEXEC or failed to report errors.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Most code now uses the virProcess / virCommand APIs, so
the need for sys/wait.h is quite limited. Removing this
include removes the dependency on GNULIB providing a
dummy sys/wait.h for Windows.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Use the glib allocation function that never returns NULL and remove the
now dead-code checks from all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Always trim the full specified suffix.
All of the callers outside of tests were passing either
strlen or the actual length of the string.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
* chown: use is conditionally compiled
* configmake: functionality imported to Makefile.am
* getaddrinfo: we have no portability problems
with Windows impl
* getpass: simplified impl is imported
* mgetgroups: getgrouplist is used directly
* net_if: header includes are conditionalized
* netdb: header includes are conditionalized
* passfd: simplified impl is imported
* posix-shell: functionality was unused & removed
* sigaction: usage is conditionalized
* sigpipe: usage is conditionalized
* stat-time: struct stat is used directly
* strchrnul: usage is eliminated
* strtok_r: usage is not a portability problem
* sys_stat: usage is conditionalized
* uname: rewritten to use native Win32 function to
get host arch
* waitpid: usage is conditionalized
* wcwidth: rewritten using g_unichar APIs
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The EAI_ADDRFAMILY constant has been removed from FreeBSD
headers, supposedly because it is deprecated by new RFC
drafts.
Previously GNULIB was providing a replacement because
MinGW lacked it too. The replacement provided for MinGW
was thus being used on FreeBSD too, but with a completely
bogus integer value.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
There are a large number of different header files that
are related to the sockets APIs. The virsocket.h header
includes all of the relevant headers for Windows and UNIX
in one convenient place. If virsocketaddr.h is already
included, then there's no need for virsocket.h
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The F_DUPFD_CLOEXEC functionality is not available on
some platformms. We must thus explicitly call the
virSetCloexec function once we remove GNULIB's equiv
fix for this.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Cygwin is not a supported build platform for libvirt and
has no testing coverage in our CI systems. Stop pretending
the code is usable and remove it so there is less to port
to Meson.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The Windows platform does not have the signal handling
support we need, so it must be disabled in several parts
of the codebase.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This is a simplified variant of gnulib's passfd module
without the portability code that we do not require.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Since commit <60d9ad6f1e42618fce10baeb0f02c35e5ebd5b24> we require
GnuTLS and since commit <ac0d21c762351f58dd5d2dafa2014ed48a8b49f3>
we can actually drop the usage of WITH_GNUTLS.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Convert to use socket wrappers. Aside from the header file
include change, this requires changing close -> closesocket
since our portability isn't trying to replace the close
function.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The last_component() method is a GNULIB custom function
that returns a pointer to the base name in the path.
This is similar to g_path_get_basename() but without the
malloc. The extra malloc is no trouble for libvirt's
needs so we can use g_path_get_basename().
Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The g_pattern_match function_simple is an acceptably close
approximation of fnmatch for libvirt's needs.
In contrast to fnmatch(), the '/' character can be matched
by the wildcards, there are no '[...]' character ranges and
'*' and '?' can not be escaped to include them literally in
a pattern.
Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
As pointed out by Ján Tomko, "no_memory seems suspicious in the times of
abort()".
As libvirt decided to take the path to not report OOM and simply abort
when it happens, let's get rid of the no_memory labels and simplify the
code around them.
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virGetUserDirectory() *never* *ever* returns NULL, making the checks for
it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This patch introduces virNetServerGetProgramLocked. It's a function to
determine which program has to be used for a given @msg. This function
will be reused in the next patch.
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Use the return value of virObjectRef directly. This way, it's easier
for another reader to identify the reason why the additional reference
is required.
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Replace all the uses passing a single parameter as the length.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
As part of a goal to eliminate Perl from libvirt build tools,
rewrite the gensystemtap.pl tool in Python.
This was a straight conversion, manually going line-by-line to
change the syntax from Perl to Python. Thus the overall structure
of the file and approach is the same.
Tested-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When dispatching a message read from client it is first passed
through registered filters. If one of the filters consumes the
message no further processing of the message is done. However,
the filter callbacks are called with the client object locked.
This breaks lock ordering in case of virStream filter, we always
acquire stream private data lock without the client object
locked. In other words, the daemonStreamFilter() does not follow
the lock ordering.
Signed-off-by: LanceLiu <liu.lance.89@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When parsing allowed authentication methods for the native ssh lib
transports we used strsep. Since we have virStringSplit helper let's use
that one.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This affects more than src/Makefile.am as the rule to generate source
files for protocols is generic for all sub-directories.
Affected files are:
src/admin/admin_protocol.{h,c}
src/locking/lock_protocol.{h,c}
src/logging/log_protocol.{h,c}
src/lxc/lxc_monitor_protocol.{h,c}
src/remote/{lxc,qemu,remote}_protocol.{h,c}
src/rpc/{virkeepalive,virnet}protocol.{h,c}
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The function now does not return an error so we can drop it fully.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
After conversion to g_strdup, the helpers now always return success.
Remove the return value to simplify the callers.
Note that many occurrences of these is in the code generated by
gendispatch.pl. Since gendispatch aggregates many cases together an
incremental conversion would require more invasive changes to
gendispatch for the time of conversion which doesn't make sense.
Also in many cases the helper was the last place where the 'error:'
label was used and thus also those conversions must be included in this
patch.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
ACKed-by: Eric Blake <eblake@redhat.com>
Replace all occurrences of
if (VIR_STRDUP(a, b) < 0)
/* effectively dead code */
with:
a = g_strdup(b);
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Replace all the occurrences of
ignore_value(VIR_STRDUP(a, b));
with
a = g_strdup(b);
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use G_GNUC_UNUSED from GLib instead of ATTRIBUTE_UNUSED.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The usleep function was missing on older mingw versions, but we can rely
on it existing everywhere these days. It may only support times upto 1
second in duration though, so we'll prefer to use g_usleep instead.
The commandhelper program is not changed since that can't link to glib.
Fortunately it doesn't need to build on Windows platforms either.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Converting from virObject to GObject is reasonably straightforward,
as illustrated by this patch for virIdentity
In the header file
- Remove
typedef struct _virIdentity virIdentity
- Add
#define VIR_TYPE_IDENTITY virIdentity_get_type ()
G_DECLARE_FINAL_TYPE (virIdentity, vir_identity, VIR, IDENTITY, GObject);
Which provides the typedef we just removed, and class
declaration boilerplate and various other constants/macros.
In the source file
- Change 'virObject parent' to 'GObject parent' in the struct
- Remove the virClass variable and its initializing call
- Add
G_DEFINE_TYPE(virIdentity, vir_identity, G_TYPE_OBJECT)
which declares the instance & class constructor functions
- Add an impl of the instance & class constructors
wiring up the finalize method to point to our dispose impl
In all files
- Replace VIR_AUTOUNREF(virIdentityPtr) with g_autoptr(virIdentity)
- Replace virObjectRef/Unref with g_object_ref/unref. Note
the latter functions do *NOT* accept a NULL object where as
libvirt's do. If you replace g_object_unref with g_clear_object
it is NULL safe, but also clears the pointer.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Consider having a nc binary in the path with a space in its name,
for example '/tmp/fo o/nc'
This results in libvirt running SSH with the following arg value
"'if ''/tmp/fo o/nc'' -q 2>&1 | grep \"requires
an argument\" >/dev/null 2>&1; then ARG=-q0;
else ARG=;fi;''/tmp/fo o/nc'' $ARG -U
/var/run/libvirt/libvirt-sock'"
The use of the single quote escaping was introduced by
commit 6ac6238de3
Author: Guido Günther <agx@sigxcpu.org>
Date: Thu Oct 13 21:49:01 2011 +0200
Use virBufferEscapeShell in virNetSocketNewConnectSSH
to escape the netcat command since it's passed to the shell. Adjust
expected test case output accordingly.
While the intention of this change was good, the result is broken as it
is still underquoted.
On the SSH server side, SSH itself runs the command via the shell.
Our command is then invoking the shell again. Thus we see
$ virsh -c qemu+ssh://root@domokun/system?netcat=%2Ftmp%2Ffo%20o%2Fnc list
error: failed to connect to the hypervisor
error: End of file while reading data: sh: /tmp/fo: No such file or directory: Input/output error
With the second level of escaping added we can now successfully use a nc
binary with a space in the path.
The original test case added was misleading as it illustrated using a
binary path of 'nc -4' which is not a path, it is a command with a
separate argument, which is getting interpreted as a path.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When opening a connection to a second driver inside the daemon, we must
ensure the identity of the current user is passed across. This allows
the second daemon to perform access control checks against the real end
users, instead of against the libvirt daemon that's proxying across the
API calls.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Remove the "UNIX" tag from the names for user name, group name,
process ID and process time, since these attributes are all usable
for non-UNIX platforms like Windows.
User ID and group ID are left with a "UNIX" tag, since there's no
equivalent on Windows. The closest equivalent concept on Windows,
SID, is a struct containing a number of integer fields, which is
commonly represented in string format instead. This would require
a separate attribute, and is left for a future exercise, since
the daemons are not currently built on Windows anyway.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The same way we check for limits when decoding typed parameters
(virTypedParamsDeserialize()) we should do the same check when
serializing them so that we don't put onto the wire more than our
limits allow. Surprisingly, we were doing so explicitly in some
places but not all of them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
As a side effect, this also silences the possible:
internal error: Unable to get DBus system bus connection:
Failed to connect to socket /run/dbus/system_bus_socket:
No such file or directory
error, since we check upfront whether dbus is available.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The driver dispatch methods access the priv->conn variables directly.
In future we want to dynamically open the connections for the secondary
driver. Thus we want the methods to call a method to get the connection
handle instead of assuming the private variable is non-NULL.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Now that 100% of libvirt code is forbidden in a SUID environment,
we no longer need to worry about whether env variables are
trustworthy or not. The virt-login-shell setuid program, which
does not link to any libvirt code, will purge all environment
variables, except $TERM, before invoking the virt-login-shell-helper
program which uses libvirt.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Now that 100% of libvirt code is forbidden in a SUID environment,
we no longer need to worry about whether env variables are
trustworthy or not. The virt-login-shell setuid program, which
does not link to any libvirt code, will purge all environment
variables, except $TERM, before invoking the virt-login-shell-helper
program which uses libvirt.
Thus we only need one API for env passthrough in virCommand.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The remote code generator had to be taught about the new
virDomainCheckpointPtr type, at which point the remote driver code for
checkpoints can be generated.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Shutting down the daemon after 30 seconds of being idle is a little bit
too aggressive. Especially when using 'virsh' in single-shot mode, as
opposed to interactive shell mode, it would not be unusual to have
more than 30 seconds between commands. This will lead to the daemon
shutting down and starting up between a series of commands.
Increasing the shutdown timer to 2 minutes will make it less likely that
the daemon will shutdown while the user is in the middle of a series of
commands.
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The use of the virNetServerAutoShutdownFunc typedef was removed in
commit 79b8a56995
Author: Daniel P. Berrange <berrange@redhat.com>
Date: Wed Oct 31 19:03:55 2012 +0000
Replace polling for active VMs with signalling by drivers
This unused typedef was then copied into the virNetDaemon object
when that was split off from virNetServer, resulting in a typedef
virNetDaemonAutoShutdownFunc that has never been needed.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The virNetServerServiceNewFDOrUNIX method cannot be correctly used when
dealing with systemd activation of a service which can receive more than
one socket FD as there is not guaranteed ordering of FDs.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The current libvirtd code for systemd socket activation assumes socket
FDs are passed in the order unix-rw, unix-ro, unix-admin. There is in
fact no ordering guarantee made by systemd. Applications are expected
to check the address or name associated with each FD to figure out its
identity.
This rewrites libvirtd to make use of the new systemd activation APIs
to make it robust wrt socket ordering changes.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently code has to first create the service and then separately
register it with the server. If the socket associated with a particular
service is not passed from systemd we want to skip creating the service
altogether. This means we can't put the systemd activation logic into
the constructors for virNetServerService.
This patch thus creates some helper methods against virNetServer which
combine systemd activation, service creation and service registration
into one single operation. This operation is automatically a no-op if
systemd activation is present and no sockets were passed in.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently the socket code will unlink any UNIX socket path which is
associated with a server socket. This is not fine grained enough, as we
need to avoid unlinking server sockets we were passed by systemd.
To deal with this we must explicitly track whether each socket needs to
be unlinked when closed, separately of the client vs server state.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The virNetServerServiceNewFD API only accepts a single FD, but it is
easily changed to allow for an array of FDs to be passed in.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Introduce a virNetServerServiceNewSocket API that allows the various
constructors to share more code.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When the service passed to getaddrinfo is NULL the kernel will choose a
free port to bind to. In a dual stack though we will get separate
sockets for IPv4 and IPv6 and we need them to bind to the same port
number. Thus once the kerel has auto-selected a port for the first
socket, we must disable auto-select for subsequent IP sockets and force
reuse of the first port.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Way back in the past, the "no_tty=1" option was added for the remote
driver to disable local password prompting by disabling use of the local
tty:
commit b32f429849
Author: Daniel P. Berrange <berrange@redhat.com>
Date: Fri Sep 21 20:17:09 2007 +0000
Added a no_tty param to remote URIs to stop SSH prompting for password
This was done by adding "-T -o BatchMode=yes -e none" args to ssh. This
achieved the desired results but is none the less semantically flawed
because it is mixing up config parameters for the local tty vs the
remote tty.
The "-T" arg stops allocation of a TTY on the remote host. This is good
for all libvirt SSH tunnels as we never require a TTY for our usage
model, so we should have just passed this unconditionally.
The "-e none" option disables the escape character for sessions with a
TTY. If we pass "-T" this is not required, but it also not harmful to
add it, so we should just pass it unconditionally too.
Only the "-o BatchMode=yes" option is related to disabling local
password prompts and thus needs control via the no_tty URI param.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Neither the sasl_client_init or sasl_server_init methods are even
remotely threadsafe. They do a bunch of one-time initialization and
merely use a simple integer counter to avoid repeated work, not even
using atomic increment/reads on the counter. This can easily race in a
threaded program. Protect the calls using a virOnce initializer function
which is guaranteed threadsafe at least from libvirt's POV.
If the application using libvirt also uses another library that makes
use of SASL then the race still exists. It is impossible to fix that
fully except in SASL code itself.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Commit 5a148ce84 altered the virNetServerNew to remove a parameter
but neglected to update the ATTRIBUTE_NONNULL's which causes a build
failure for when checking is enabled such as when lv_cv_static_analysis
is enabled.
Signed-off-by: John Ferlan <jferlan@redhat.com>
The files for libvirt-net-rpc-server.la refernce the sasl/sasl.h
system header but never used the $(SASL_CFLAGS) variable. This
was never noticed previously because the $(AVAHI_CLFAGS) were
set and these typically pulled in the same include directory.
When mDNS/Avahi support was removed this exposed the bug which
caused FreeBSD builds to break as /usr/local/include was no
longer searched for headers.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Libvirtd has long had integration with avahi for advertising libvirtd
using mDNS when TCP/TLS listening is enabled. For a long time the
virt-manager application had support for auto-detecting libvirtds
on the local network using mDNS, but this was removed last year
commit fc8f8d5d7e3ba80a0771df19cf20e84a05ed2422
Author: Cole Robinson <crobinso@redhat.com>
Date: Sat Oct 6 20:55:31 2018 -0400
connect: Drop avahi support
Libvirtd can advertise itself over avahi. The feature is disabled by
default though and in practice I hear of no one actually using it
and frankly I don't think it's all that useful
The 'Open Connection' wizard has a disproportionate amount of code
devoted to this feature, but I don't think it's useful or worth
maintaining, so let's drop it
I've never heard of any other applications having support for using
mDNS to detect libvirtd instances. Though it is theoretically possible
something exists out there, it is clearly going to be a niche use case
in the virt ecosystem as a whole.
By removing avahi integration we can cut down the dependency chain for
the basic libvirtd install and reduce our code maint burden.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
In libssh 0.9.0 functions ssh_is_server_known and ssh_write_knownhost
are marked as deprecated.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1722735
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Define the wire protocol for the virNetworkPort APIs and enable the
client/server RPC dispatch.
Reviewed-by: Laine Stump <laine@laine.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Due to the way that our virObjectUnref() is written it's not
possible that a NULL is passed into *Dispose() function. However,
some functions check for that regardless.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Failed new gnutls context allocations in virNetTLSContextNew function
results in double free and segfault. Occasional memory leaks may also
occur.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Adrian Brzezinski <redhat@adrb.pl>
Vim has trouble figuring out the filetype automatically because
the name doesn't follow existing conventions; annotations like
the ones we already have in Makefile.ci help it out.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
A bunch of files include src/rpc/virnetsaslcontext.h, which
in turn includes <sasl/sasl.h>, and without the corresponding
CFLAGS the compiler can't locate the latter if it happens to
be installed outside of the default include path as is the
case, for example, on FreeBSD.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Now that the memory disposal is handled automatically we can simplify
the cleanup paths. In this case it's not as simple as sometimes the
value of the called function is returned.
While at it fix the initialization value of the returned variable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Refactor code paths which clear strings on cleanup paths to use the
automatic helper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If 2 threads call abort for example then one of them
will hang because client will send 2 abort messages and
server will reply only on first of them, the second will be
ignored. And on server reply client changes the state only
one of abort message to complete, the second will hang forever.
There are other similar issues.
We should complete all messages waiting reply if we got
error or expected abort/finish reply from server. Also if one
thread send finish and another abort one of them will win
the race and server will either abort or finish stream. If
stream is aborted then thread requested finishing should report
error. In order to archive this let's keep stream closing reason
in @closed field. If we receive VIR_NET_OK message for stream
then stream is finished if oldest (closest to queue end) message
in stream queue is finish message and stream is aborted if oldest
message is abort message. Otherwise it is protocol error.
By the way we need to fix case of receiving VIR_NET_CONTINUE
message. Now we take oldest message in queue and check if
this is dummy message. If one thread first sends abort and
second thread then receives data then oldest message is abort
message and second thread won't be notified when data arrives.
Let's find oldest dummy message instead.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
If we call virStreamFinish and virStreamAbort from 2 distinct
threads for example we can have access to freed memory.
Because when virStreamFinish finishes for example virStreamAbort
yet to be finished and it access virNetClientStreamPtr object
in stream->privateData.
Also it does not make sense to clear @driver field. After
stream is finished/aborted it is better to have appropriate
error message instead of "unsupported error".
This commit reverts [1] or virNetClientStreamPtr and
virStreamPtr will never be unrefed due to cyclic dependency.
Before this patch we don't have leaks because all execution
paths we call virStreamFinish or virStreamAbort.
[1] 8b6ffe40 : virNetClientStreamNew: Track origin stream
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
This mixing errors and EOF condition in one flag is odd.
Instead let's check st->err.code where appropriate.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Checking virNetClientStreamRaiseError without client lock
is racy which is fixed in [1] for example. Thus let's remove such checks
when we are sending message to server. And in other cases
(like virNetClientStreamRecvHole for example) let's move the check
into client stream code.
virNetClientStreamRecvPacket already have stream lock so we could
introduce another error checking function like virNetClientStreamRaiseErrorLocked
but as error is set when both client and stream lock are hold we
can remove locking from virNetClientStreamRaiseError because all
callers hold either client or stream lock.
Also let's split virNetClientStreamRaiseErrorLocked into checking
state function and checking message send status function. They are
same yet.
[1] 1b6a29c21: rpc: fix race on stream abort/finish and server side abort
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Stream server error is not propagated if thread does not have the buck.
In case we have the buck we are ok due to the code added in [1].
Let's check for stream error on all paths. Now we don't need
to raise error in virNetClientCallDispatchStream.
Old code reported error only if the first message in wait
queue awaits reply. It is odd as depends on wait queue
situation. For example if we have only TX
message in queue and in one iteration loop both send the
message and receive error then thread sending TX message did
not receive the error. Next if we have RX message (first)
and TX message (second) in queue and in one iteration
loop both send the TX message and receive error then
thread sending TX message received error. In short
it was inconsistent. Let's report error whenever
we received it and for every type of message as it makes
sense to report errors as early as possible.
[1] 16c6e2b41: Fix propagation of RPC errors from streams
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
In next patches we'll add stream state checks to this
function that applicable to all call paths. This is handy
place because we hold client lock here.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Stream abort/finish can hang because we can receive abort message
from server and yet sent abort/finish message to server. The latter
will not be answered ever because after server sends abort message
it forgets the stream and messages for unknown stream are simply ignored.
We check for stream error at the very beginning of remoteStreamFinish/remoteStreamAbort
but stream error can be set after the check in another thread operating
on stream. Let's check for stream error under client lock similar
to what's done in [1].
[1] 833b901cb: stream: Check for stream EOF
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Missing semicolon at the end of macros can confuse some analyzers
(like cppcheck <filename>). VIR_ONCE_GLOBAL_INIT is almost
exclusively called without an ending semicolon, but let's
standardize on using one like the other macros.
Add a dummy struct definition at the end of the macro, so
the compiler will require callers to add a semicolon.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
if virNetClientNew finishes with error before sock is set
to client object then sock does not get unrefed. This is
unexpected by function clients like virNetClientNewUNIX.
Let's make sure sock gets unrefed on any error path.
Next some clients like virNetClientNewLibSSH2 try to unref
sock on virNetClientNew errors. This is not correct even
before this patch because in some cases virNetClientNew
unrefed sock on error path by itself. Let's give up
sock managment to virNetClientNew entirely.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Require that all headers are guarded by a symbol named
LIBVIRT_$FILENAME
where $FILENAME is the uppercased filename, with all characters
outside a-z changed into '_'.
Note we do not use a leading __ because that is technically a
namespace reserved for the toolchain.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This introduces a syntax-check script that validates header files use a
common layout:
/*
...copyright header...
*/
<one blank line>
#ifndef SYMBOL
# define SYMBOL
....content....
#endif /* SYMBOL */
For any file ending priv.h, before the #ifndef, we will require a
guard to prevent bogus imports:
#ifndef SYMBOL_ALLOW
# error ....
#endif /* SYMBOL_ALLOW */
<one blank line>
The many mistakes this script identifies are then fixed.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The make_nonnull_XXX methods can all fail due to OOM but this was being
silently ignored and thus also not checked by callers. Make the methods
propagate errors and use ATTRIBUTE_RETURN_CHECK to force callers to deal
with it.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
In many files there are header comments that contain an Author:
statement, supposedly reflecting who originally wrote the code.
In a large collaborative project like libvirt, any non-trivial
file will have been modified by a large number of different
contributors. IOW, the Author: comments are quickly out of date,
omitting people who have made significant contribitions.
In some places Author: lines have been added despite the person
merely being responsible for creating the file by moving existing
code out of another file. IOW, the Author: lines give an incorrect
record of authorship.
With this all in mind, the comments are useless as a means to identify
who to talk to about code in a particular file. Contributors will always
be better off using 'git log' and 'git blame' if they need to find the
author of a particular bit of code.
This commit thus deletes all Author: comments from the source and adds
a rule to prevent them reappearing.
The Copyright headers are similarly misleading and inaccurate, however,
we cannot delete these as they have legal meaning, despite being largely
inaccurate. In addition only the copyright holder is permitted to change
their respective copyright statement.
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This is a simple removal of a duplicated check of the return of the
filter function. There is a nested conditional checking exactly the same
thing since commit c9ede1cf removed the (ret > 0) check condition.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1631606
Changes made to manage and utilize a secondary connection
driver to APIs outside the scope of the primary connection
driver have resulted in some confusion processing polkit rules
since the simple "access denied" error message doesn't provide
enough of a clue when combined with the "authentication failed:
access denied by policy" as to which connection driver refused
or failed the ACL check.
In order to provide some context, let's modify the existing
"access denied" error returned from the various vir*EnsureACL
API's to provide the connection driver name that is causing
the failure. This should provide the context for writing the
polkit rules that would allow access via the driver, but yet
still adhere to the virAccessManagerSanitizeError commentary
regarding not telling the user why access was denied.
Signed-off-by: John Ferlan <jferlan@redhat.com>
This reverts commit ccc72d5cbd.
Based on upstream comment to a follow-up patch, this didn't take the
right approach and the right thing to do is revert and rework.
Signed-off-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1631606
Changes made to manage and utilize a secondary connection
driver to APIs outside the scope of the primary connection
driver have resulted in some confusion processing polkit rules
since the simple "access denied" error message doesn't provide
enough of a clue when combined with the "authentication failed:
access denied by policy" as to which connection driver refused
or failed the ACL check.
In order to provide some context, let's modify the existing
"access denied" error returne from the various vir*EnsureACL
API's to provide the connection driver name that is causing
the failure. This should provide the context for writing the
polkit rules that would allow access via the driver.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
The result of libssh2_userauth_password is being assigned to 'ret' in
one branch and 'rc' in the other branch. Checks are all done against the
'ret' variable, so one branch never does the correct check.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
If the same source gets built twice ('build same source on different
hosts at different times') the resulting files may differ.
Fix this by sorting the hash keys before usage.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
All of the ones being removed are pulled in by internal.h. The only
exception is sanlock which expects the application to include <stdint.h>
before sanlock's headers, because sanlock prototypes use fixed width
int, but they don't include stdint.h themselves, so we have to leave
that one in place.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
It doesn't really make sense for us to have stdlib.h and string.h but
not stdio.h in the internal.h header.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
Now that the virAuthGet*Path API's generate all the error messages
we can remove them from the callers.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
Semantically, there is no difference between an uninitialized worker
pool and an initialized worker pool with zero workers. Let's allow the
worker pool to be initialized for max_workers=0 as well then which
makes the API more symmetric and simplifies code. Validity of the
worker pool is delegated to virThreadPoolGetMaxWorkers instead.
This patch fixes segmentation faults in
virNetServerGetThreadPoolParameters and
virNetServerSetThreadPoolParameters for the case when no worker pool
is actually initialized (max_workers=0).
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Consider creating a listener socket from a hostname that resolves to
multiple addresses. It might be the case that the hostname resolves to
both an IPv4 and IPv6 address because it is reachable over both
protocols, but the IPv6 connectivity is provided off-host. In such a
case no local NIC will have IPv6 and so bind() would fail with the
EADDRNOTAVAIL errno. Thus it should be treated as non-fatal as long as
at least one socket was succesfully bound.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When reporting socket/bind failures we want to ensure any fatal error
reported is as accurate as possible. We'll prefer reporting a bind()
errno over a socket() errno, because if socket() works but bind() fails
that is a more significant event.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently, the functions return a pointer to the
destination buffer on success or NULL on failure.
Not only does this kind of error handling look quite
alien in the context of libvirt, where most functions
return zero on success and a negative int on failure,
but it's also somewhat pointless because unless there's
been a failure the returned pointer will be the same
one passed in by the user, thus offering no additional
value.
Change the functions so that they return an int
instead.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
The include guard should match the file name and comment.
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
@srv must be unlocked for the call virNetServerProcessMsg otherwise a
deadlock can occur.
Since the pointer 'srv->workers' will never be changed after
initialization and the thread pool has it's own locking we can release
the lock of 'srv' earlier. This also fixes the deadlock.
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Replace instances where we previously called virGetLastError just to
either get the code or to check if an error exists with
virGetLastErrorCode to avoid a validity pre-check.
Signed-off-by: Ramy Elkest <ramyelkest@gmail.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Now that GnuTLS is a requirement, we can drop a lot of
conditionally built code. However, not all ifdef-s can go because
we still want libvirt_setuid to build without gnutls.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Since GnuTLS is required there is no way to go with !WITH_GNUTLS
branch and just distribute these files.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
After version 0.7.5, libssh deprecated the function scope
ssh_get_publickey() and moved to ssh_get_server_publickey(). So, Libvirt
is failing to compile using this new function name.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Rather than have virJSONValueArraySize return a -1 when the input
is not an array and then splat an error message, let's check for
an array before calling and then change the return to be a size_t
instead of ssize_t.
That means using the helper virJSONValueIsArray as well as using a
more generic error message such as "Malformed <something> array".
In some cases we can remove stack variables and when we cannot,
those variables should be size_t not ssize_t. Alter a few references
of if (!value) to be if (value == 0) instead as well.
Some callers can already assume an array is being worked on based
on the previous call, so there's less to do.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
So far we are repeating the following lines over and over:
if (!(virSomeObjectClass = virClassNew(virClassForObject(),
"virSomeObject",
sizeof(virSomeObject),
virSomeObjectDispose)))
return -1;
While this works, it is impossible to do some checking. Firstly,
the class name (the 2nd argument) doesn't match the name in the
code in all cases (the 3rd argument). Secondly, the current style
is needlessly verbose. This commit turns example into following:
if (!(VIR_CLASS_NEW(virSomeObject,
virClassForObject)))
return -1;
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Whenever we declare a new object the first member of the struct
has to be virObject (or any other member of that family). Now, up
until now we did not care about the name of the struct member.
But lets unify it so that we can do some checks at compile time
later.
The unified name is 'parent'.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Future commits rely on the presence of this callback.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Calling a push_privconn method to directly push the connection object
name into the arg list is inconvenient. Refactor so that we acquire
the connection variable name upfront, and push it to the arg list
separately. This allows various hardcoded usage of "priv->conn" to
be parameterized.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When preparing for migration, the libxl driver creates a new TCP listen
socket for the incoming migration by calling virNetSocketNewListenTCP,
passing the destination host name. virNetSocketNewListenTCP calls
virSocketAddrParse to check if the host name is a wildcard address, in
which case it avoids adding the AI_ADDRCONFIG flag to the hints passed to
getaddrinfo. If the host name is not an IP address, virSocketAddrParse
reports an error
error : virSocketAddrParseInternal:121 : Cannot parse socket address
'myhost.example.com': Name or service not known
But virNetSocketNewListenTCP succeeds regardless and the overall migration
operation succeeds.
Introduce virSocketAddrParseAny and use it when simply testing if a host
name/addr is parsable.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
If max_workers is set to zero, then the worker thread pool won't be
created, so when serializing state for pre-exec we must set various
parameters to zero.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently if the virNetServer instance is created with max_workers==0 to
request a non-threaded dispatch process, we deadlock during dispatch
#0 0x00007fb845f6f42d in __lll_lock_wait () from /lib64/libpthread.so.0
#1 0x00007fb845f681d3 in pthread_mutex_lock () from /lib64/libpthread.so.0
#2 0x000055a6628bb305 in virMutexLock (m=<optimized out>) at util/virthread.c:89
#3 0x000055a6628a984b in virObjectLock (anyobj=<optimized out>) at util/virobject.c:435
#4 0x000055a66286fcde in virNetServerClientIsAuthenticated (client=client@entry=0x55a663a7b960)
at rpc/virnetserverclient.c:1565
#5 0x000055a66286cc17 in virNetServerProgramDispatchCall (msg=0x55a663a7bc50, client=0x55a663a7b960,
server=0x55a663a77550, prog=0x55a663a78020) at rpc/virnetserverprogram.c:407
#6 virNetServerProgramDispatch (prog=prog@entry=0x55a663a78020, server=server@entry=0x55a663a77550,
client=client@entry=0x55a663a7b960, msg=msg@entry=0x55a663a7bc50) at rpc/virnetserverprogram.c:307
#7 0x000055a662871d56 in virNetServerProcessMsg (msg=0x55a663a7bc50, prog=0x55a663a78020, client=0x55a663a7b960,
srv=0x55a663a77550) at rpc/virnetserver.c:148
#8 virNetServerDispatchNewMessage (client=0x55a663a7b960, msg=0x55a663a7bc50, opaque=0x55a663a77550)
at rpc/virnetserver.c:227
#9 0x000055a66286e4c0 in virNetServerClientDispatchRead (client=client@entry=0x55a663a7b960)
at rpc/virnetserverclient.c:1322
#10 0x000055a66286e813 in virNetServerClientDispatchEvent (sock=<optimized out>, events=1, opaque=0x55a663a7b960)
at rpc/virnetserverclient.c:1507
#11 0x000055a662899be0 in virEventPollDispatchHandles (fds=0x55a663a7bdc0, nfds=<optimized out>)
at util/vireventpoll.c:508
#12 virEventPollRunOnce () at util/vireventpoll.c:657
#13 0x000055a6628982f1 in virEventRunDefaultImpl () at util/virevent.c:327
#14 0x000055a6628716d5 in virNetDaemonRun (dmn=0x55a663a771b0) at rpc/virnetdaemon.c:858
#15 0x000055a662864c1d in main (argc=<optimized out>,
#argv=0x7ffd105b4838) at logging/log_daemon.c:1235
Reviewed-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently virNetServerClientDispatchFunc implementations are only
responsible for free'ing the "msg" parameter upon success. Simplify the
calling convention by making it their unconditional responsibility to
free the "msg", and close the client if desired.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
There's no reason why the virNetServerClientDispatchRead method needs to
acquire an extra reference on the "client" object. An extra reference is
only needed if the registered dispatch callback is going to keep hold of
the "client" for work in the background. Thus we can push reference
acquisition into virNetServerDispatchNewMessage.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Ensure all enum cases are listed in switch statements.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
No sense in calling ServiceToggle for all nservices during
ServiceDispose since ServerClose calls ServiceClose which
removes the IOCallback that's being toggled via ServiceToggle.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The position of various parameters changes depending on the WITH_GNUTLS
macro.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Since we annotate the APIs are having non-NULL parameters, we can remove
the checks for NULL in the code too.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
With the current code it is neccessary to call
virNetDaemonNewPostExecRestart()
and then for each server that needs restarting you are supposed
to call
virNetDaemonAddSeverPostExecRestart()
This is fine if there's only ever one server, but as soon as you
have two servers it is impossible to use this design. The code
has no idea which servers were recorded in the JSON state doc,
nor in which order the hash table serialized its keys.
So this patch changes things so that we only call
virNetDaemonNewPostExecRestart()
passing in a callback, which is invoked once for each server
found int he JSON state doc.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
It is not possible to blindly call virNetDaemonGetServer()
because in a post-exec restart scenario, some servers may
not exist and this method will pollute the error logs.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The server name and client data callbacks need to be non-NULL or the
system will crash at various times. This is particularly bad when some
of the crashes only occur post-exec restart.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virNetServer class is passing a pointer to itself to the
virNetServerClient as a 'void *' pointer. This is presumably due to fact
that the virnetserverclient.h file doesn't see the virNetServerPtr
typedef. The typedef is easily movable though, which lets us get
typesafe parameter passing, removing the confusion of passing two
distinct 'void *' pointers to one method.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virNetSocketWriteSASL method has to encode the buffer it is given and then
write it to the underlying socket. This write is not guaranteed to send the
full amount of data that was encoded by SASL. We cache the SASL encoded data so
that on the next invocation of virNetSocketWriteSASL we carry on sending it.
The subtle problem is that the 'len' value passed into virNetSocketWriteSASL on
the 2nd call may be larger than the original value. So when we've completed
sending the SASL encoded data we previously cached, we must return the original
length we encoded, not the new length.
This flaw means we could potentially have been discarded queued data without
sending it. This would have exhibited itself as a libvirt client never receiving
the reply to a method it invokes, async events silently going missing, or worse
stream data silently getting dropped.
For this to be a problem libvirtd would have to be queued data to send to the
client, while at the same time the TCP socket send buffer is full (due to a very
slow client). This is quite unlikely so if this bug was ever triggered by a real
world user it would be almost impossible to reproduce or diagnose, if indeed it
was ever noticed at all.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Replace virNetServerClientNeedAuth with
virNetServerClientIsAuthenticated because it makes it clearer what it
means.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>
'Squash' virNetServerClientNeedAuthLocked into
virNetServerClientNeedAuth and remove virNetServerClientNeedAuthLocked
as it's not longer needed.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>
There is a race between virNetServerProcessClients (main thread) and
remoteDispatchAuthList/remoteDispatchAuthPolkit/remoteSASLFinish (worker
thread) that can lead to decrementing srv->nclients_unauth when it's
zero. Since virNetServerCheckLimits relies on the value
srv->nclients_unauth the underrun causes libvirtd to stop accepting
new connections forever.
Example race scenario (assuming libvirtd is using policykit and the
client is privileged):
1. The client calls the RPC remoteDispatchAuthList =>
remoteDispatchAuthList is executed on a worker thread (Thread
T1). We're assuming now the execution stops for some time before
the line 'virNetServerClientSetAuth(client, 0)'
2. The client closes the connection irregularly. This causes the
event loop to wake up and virNetServerProcessClient to be
called (on the main thread T0). During the
virNetServerProcessClients the srv lock is hold. The condition
virNetServerClientNeedAuth(client) will be checked and as the
authentication is not finished right now
virNetServerTrackCompletedAuthLocked(srv) will be called =>
--srv->nclients_unauth => 0
3. The Thread T1 continues, marks the client as authenticated, and
calls virNetServerTrackCompletedAuthLocked(srv) =>
--srv->nclients_unauth => --0 => wrap around as nclient_unauth is
unsigned
4. virNetServerCheckLimits(srv) will disable the services forever
To fix it, add an auth_pending field to the client struct so that it
is now possible to determine if the authentication process has already
been handled for this client.
Setting the authentication method to none for the client in
virNetServerProcessClients is not a proper way to indicate that the
counter has been decremented, as this would imply that the client is
authenticated.
Additionally, adjust the existing test cases for this new field.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Combine virNetServerClientSetAuth(client,
VIR_NET_SERVER_SERVICE_AUTH_NONE) and virNetServerTrackCompletedAuth
into one new function named virNetServerSetClientAuthenticated.
After using this new function the function
virNetServerTrackCompletedAuth was superfluous and is therefore
removed. In addition, it is not very common that a
'{{function}}' (virNetServerTrackCompletedAuth) does more than just
the locking compared to
'{{function}}Locked' (virNetServerTrackCompletedAuthLocked).
virNetServerTrackPendingAuth was already superfluous and therefore
it's also removed.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The lock for @client must not only be held for the duration of
checking whether the client wants to close, but also for as long as
we're closing the client. The same applies to the tracking of
authentications.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Add virNetServerClientAuthMethodImpliesAuthenticated() for deciding
whether a authentication method implies that a client is automatically
authenticated or not. Use this new function in
virNetServerClientNeedAuthLocked().
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>
This makes the code more efficient.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
