Commit id '252610f7d' used a hash table to store the @srv, but
didn't handle the virObjectUnref if virNetDaemonNew failed nor
did it use virObjectUnref once successfully placed into the table
which will now be managing it's lifetime (and would cause the
virObjectRef if successfully inserted into the table).
Right-aligning backslashes when defining macros or using complex
commands in Makefiles looks cute, but as soon as any changes is
required to the code you end up with either distractingly broken
alignment or unnecessarily big diffs where most of the changes
are just pushing all backslashes a few characters to one side.
Generated using
$ git grep -El '[[:blank:]][[:blank:]]\\$' | \
grep -E '*\.([chx]|am|mk)$$' | \
while read f; do \
sed -Ei 's/[[:blank:]]*[[:blank:]]\\$/ \\/g' "$f"; \
done
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
The assumption so far was an average of 4 disks per guest.
But some architectures, like s390x, still often use plenty of smaller disks.
To include those in the considerations an assumption of an average of 10
disks is more reasonable.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Seeing a log message saying 'flags=93' is ambiguous & confusing unless
you happen to know that libvirt always prints flags as hex. Change our
debug messages so that they always add a '0x' prefix when printing flags,
and '0' prefix when printing mode. A few other misc places gain a '0x'
prefix in error messages too.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
There were a few places in our code where the following pattern in 'if'
condition occurred:
if ((foo = bar() < 0))
do something;
This patch adjusts the conditions to the expected format:
if ((foo = bar()) < 0)
do something;
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1488192
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Commit 94c465d0 refactored the logging setup phase but introduced an
issue, where the daemon ignores verbose mode when there are no outputs
defined and the default must be used. The problem is that the default
output was determined too early, thus ignoring the potential '--verbose'
option taking effect. This patch postpones the creation of the default
output to the very last moment when nothing else can change. Since the
default output is only created during the init phase, it's safe to leave
the pointer as NULL for a while, but it will be set eventually, thus not
affecting runtime.
Patch also adjusts both the other daemons.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1442947
Signed-off-by: Erik Skultety <eskultet@redhat.com>
The default conf files, for example libvirtd.conf, virtlockd.conf, and
virtlogd.conf, should be located under the directory "/etc/libvirt" when
root as root, rather than "/etc". When run as non-root, the configuration
files should be located under "$XDG_CONFIG_HOME/libvirt/", rather than
"XDG_CONFIG_HOME".
Signed-off-by: Lily Zhu <lizhu@redhat.com>
Signed-off-by: Erik Skultety <eskultet@redhat.com>
The recently added sanlock_strerror function can be used to translate
sanlock's numeric errors into human readable strings.
https://bugzilla.redhat.com/show_bug.cgi?id=1409511
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
The log and lock protocol don't have an extra handshake to close the
connection. Instead they just close the socket. Unfortunately that
resulted into a lot of spurious garbage logged to the system log files:
2017-03-17 14:00:09.730+0000: 4714: error : virNetSocketReadWire:1800 : End of file while reading data: Input/output error
or in the journal as:
Mar 13 16:19:33 xxxx virtlogd[32360]: End of file while reading data: Input/output error
Use the new facility in the netserverclient to suppress the IO error
report from the virNetSocket layer.
Linux still defaults to a 1024 open file handle limit. This causes
scalability problems for libvirtd / virtlockd / virtlogd on large
hosts which might want > 1024 guest to be running. In fact if each
guest needs > 1 FD, we can't even get to 500 guests. This is not
good enough when we see machines with 100's of physical cores and
TBs of RAM.
In comparison to other memory requirements of libvirtd & related
daemons, the resource usage associated with open file handles
is essentially line noise. It is thus reasonable to increase the
limits unconditionally for all installs.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
After deploying virtlogd by default we identified a number of
mistakes in the systemd unit file. virtlockd's relationship
to libvirtd is the same as virtlogd, so we must apply the
same unit file fixes to virtlockd
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Now that virLog{Get,Set}DefaultOutput routines are introduced we can wire them
up to the daemon's logging initialization code. Also, change the order of
operations a bit so that we still strictly honor our precedence of settings:
cmdline > env > config now that outputs and filters are not appended anymore.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Along with an empty string, it should also be possible for users to pass
NULL to the public APIs which in turn would trigger a routine(future
work) responsible for defining an appropriate default logging output
given the current circumstances.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
This initially started as a fix of some debug printing in
virCgroupDetect. However it turned out that other places suffer
from the similar problem. While dealing with pids, esp. in cases
where we cannot use pid_t for ABI stability reasons, we often
chose an unsigned integer type. This makes no sense as pid_t is
signed.
Also, new syntax-check rule is introduced so we won't repeat this
mistake.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Similar to outputs, parser should do parsing only, thus the 'define' logic
is going to be stripped from virLogParseAndDefineFilters by replacing calls to
this method to virLogSetFilters instead.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Since virLogParseAndDefineOutputs is going to be stripped from 'output defining'
logic, replace all relevant occurrences with virLogSetOutputs call to make the
change transparent to all original callers (daemons mostly).
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Right now virLogParse* functions are doing both parsing and defining of filters
and outputs which should be two separate operations. Since the naming is
apparently a bit poor this patch renames these functions to
virLogParseAndDefine* which eventually will be replaced by virLogSet*.
Additionally, virLogParse{Filter,Output} will be later (after the split) reused,
so that these functions do exactly what the their name suggests.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1292984
Hold on to your hats, because this is gonna be wild.
In bd3e16a3 I've tried to expose sanlock io_timeout. What I had
not realized (because there is like no documentation for sanlock
at all) was very unusual way their APIs work. Basically, what we
do currently is:
sanlock_add_lockspace_timeout(&ls, io_timeout);
which adds a lockspace to sanlock daemon. One would expect that
io_timeout sets the io_timeout for it. Nah! That's where you are
completely off the tracks. It sets timeout for next lockspace you
will probably add later. Therefore:
sanlock_add_lockspace_timeout(&ls, io_timeout = 10);
/* adds new lockspace with default io_timeout */
sanlock_add_lockspace_timeout(&ls, io_timeout = 20);
/* adds new lockspace with io_timeout = 10 */
sanlock_add_lockspace_timeout(&ls, io_timeout = 40);
/* adds new lockspace with io_timeout = 20 */
And so on. You get the picture.
Fortunately, we don't allow setting io_timeout per domain or per
domain disk. So we just need to set the default used in the very
first step and hope for the best (as all the io_timeout-s used
later will have the same value).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Currently, we are checking for sanlock_add_lockspace_timeout
which is good for now. But in a subsequent patch we are going to
use sanlock_write_lockspace (which sets an initial value for io
timeout for sanlock). Now, there is no reason to check for both
functions in sanlock library as the sanlock_write_lockspace was
introduced in 2.7 release and the one we are currently checking
for in the 2.5 release. Therefore it is safe to assume presence
of sanlock_add_lockspace_timeout when sanlock_write_lockspace
is detected.
Moreover, the macro for conditional compilation is renamed to
HAVE_SANLOCK_IO_TIMEOUT (as it now encapsulates two functions).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Admin API needs a way of addressing specific clients. Unlike servers, which we
are happy to address by names both because its name reflects its purpose (to
some extent) and we only have two of them (so far), naming clients doesn't make
any sense, since a) each client is an anonymous, i.e. not recognized after a
disconnect followed by a reconnect, b) we can't predict what kind of requests
it's going to send to daemon, and c) the are loads of them comming and going,
so the only viable option is to use an ID which is of a reasonably wide data
type.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
After this commit, all man pages are generated using the same two
steps:
1. Process a source $command.pod file with pod2man(1) to obtain
a valid man page in $command.$section.in
2. Process $command.$section.in with sed(1) to obtain the final
man page in $command.$section
Since servers know their name, there is no need to supply such
information twice. Also defeats inconsistencies.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
At first I did not want to do this, but after trying to implement some
newer feaures in the admin API I realized we need that to make our lives
easier. On the other hand they are not saved redundantly and the
virNetServer objects are still kept in a hash table.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Since the daemon can manage and add (at fresh start) multiple servers,
we also should be able to add them from a JSON state file in case of a
daemon restart, so post exec restart support for multiple servers is also
provided. Patch also updates virnetdaemontest accordingly.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Our existing virHashForEach method iterates through all items disregarding the
fact, that some of the iterators might have actually failed. Errors are usually
dispatched through an error element in opaque data which then causes the
original caller of virHashForEach to return -1. In that case, virHashForEach
could return as soon as one of the iterators fail. This patch changes the
iterator return type and adjusts all of its instances accordingly, so the
actual refactor of virHashForEach method can be dealt with later.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
When building with gcc-5 (particularly gcc-5.3.0 now) and having pdwtags
installed (package dwarves) make check fails with the following error:
$ make lock_protocol-struct
GEN lock_protocol-struct
--- lock_protocol-structs 2016-01-13 15:04:59.318809607 +0100
+++ lock_protocol-struct-t3 2016-01-13 15:05:17.703501234 +0100
@@ -26,10 +26,6 @@
virLockSpaceProtocolNonNullString name;
u_int flags;
};
-enum virLockSpaceProtocolAcquireResourceFlags {
- VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED = 1,
- VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE = 2,
-};
struct virLockSpaceProtocolAcquireResourceArgs {
virLockSpaceProtocolNonNullString path;
virLockSpaceProtocolNonNullString name;
Makefile:10415: recipe for target 'lock_protocol-struct' failed
make: *** [lock_protocol-struct] Error 1
That happens because without any specific options gcc doesn't keep enum
information in the resulting binary object. I managed to isolate the
parameters of gcc that caused this issue to disappear, however I
remember that they influenced the resulting binaries quite a bit and
were definitely not something we would want to add as mandatory to the
build process.
So to deal with this cleanly, let's take that enum and separate it out
to its own header file. Since it is only used in the lockd driver and
the protocol, lock_driver_lockd.h feels like a suitable name.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Commit b22344f328 mistakenly reordered
Default-* lines. Thanks to that I noticed that we are very inconsistent
with our init scripts, so I took the liberty of synchronizing them,
updating them and making them all look shiny and new. So apart from
fixing the LSB requirements, I also fixed the ordering, specified
runlevels and fix the link to the reference specification.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1251190
So, if domain loses access to storage, sanlock tries to kill it
after some timeout. So far, the default is 80 seconds. But for
some scenarios this might not be enough. We should allow users to
adjust the timeout according to their needs.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Lets use wrapper functions virLockDaemonLock and
virLockDaemonUnlock instead of virMutexLock and virMutexUnlock.
This has no functional impact, but it's easier to read (at least
for me).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
So we have this mechanism that on SIGUSR1 the virtlockd dumps its
internal state into a JSON file, reexec itself and the reloads
the internal state back. However, there's a bug in our
implementation:
(gdb) signal SIGUSR1
Continuing with signal SIGUSR1.
[Thread 0x7fd094f7b700 (LWP 10602) exited]
process 10600 is executing new program: /home/zippy/work/libvirt/libvirt.git/src/virtlockd
warning: Could not load shared library symbols for linux-vdso.so.1.
Do you need "set solib-search-path" or "set sysroot"?
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
[New Thread 0x7fb28bc3c700 (LWP 14501)]
Program received signal SIGSEGV, Segmentation fault.
0x00007fb29133d530 in virExpandN (ptrptr=0x70, size=8, countptr=0x68, add=1, report=true, domcode=7, filename=0x7fb29138aeab "rpc/virnetserver.c", funcname=0x7fb29138b680 <__FUNCTION__.15821> "virNetServerAddProgram", linenr=661) at util/viralloc.c:288
288 if (*countptr + add < *countptr) {
(gdb) bt
#0 0x00007fb29133d530 in virExpandN (ptrptr=0x70, size=8, countptr=0x68, add=1, report=true, domcode=7, filename=0x7fb29138aeab "rpc/virnetserver.c", funcname=0x7fb29138b680 <__FUNCTION__.15821> "virNetServerAddProgram", linenr=661) at util/viralloc.c:288
#1 0x00007fb29132a267 in virNetServerAddProgram (srv=0x0, prog=0x7fb2915d08b0) at rpc/virnetserver.c:661
#2 0x00007fb29131f27f in main (argc=1, argv=0x7fff8f771298) at locking/lock_daemon.c:1445
Notice the NULL @srv passed to frame 2? Usually, the @srv
variable is initialized on fresh start. However, in case of
daemon reload, the code path that is responsible for initializing
the value was not triggered and therefore we crashed immediately.
Fix this by always setting the variable.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The internal representation of a JSON array counts the items in
size_t. However, for some reason, when asking for the count it's
reported as int. Firstly, we need the function to return a signed
type as it's returning -1 on an error. But, not every system has
integer the same size as size_t. Therefore, lets return ssize_t.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Now that we have virNetDaemon object holding all the data and being
capable of referencing multiple servers, having a duplicate reference to
a single server stored in virLockDaemon isn't necessary anymore. This
patch removes the above described element.
Since its introduction in 2011 (particularly in commit f4324e3292),
the option doesn't work. It just effectively disables all incoming
connections. That's because the client private data that contain the
'keepalive_supported' boolean, are initialized to zeroes so the bool is
false and the only other place where the bool is used is when checking
whether the client supports keepalive. Thus, according to the server,
no client supports keepalive.
Removing this instead of fixing it is better because a) apparently
nobody ever tried it since 2011 (4 years without one month) and b) we
cannot know whether the client supports keepalive until we get a ping or
pong keepalive packet. And that won't happen until after we dispatched
the ConnectOpen call.
Another two reasons would be c) the keepalive_required was tracked on
the server level, but keepalive_supported was in private data of the
client as well as the check that was made in the remote layer, thus
making all other instances of virNetServer miss this feature unless they
all implemented it for themselves and d) we can always add it back in
case there is a request and a use-case for it.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Some hypervisors like Xen do not have PIDs associated with domains.
Relax the requirement for PID != 0 in the locking code so it can
be used by hypervisors that do not represent domains as a process
running on the host.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
When acquiring resource via sanlock fails, we would report it as
VIR_ERR_INTERNAL_ERROR, which is not very friendly to applications using
libvirt. Moreover, the lockd driver would report the same failure as
VIR_ERR_RESOURCE_BUSY, which looks better.
Unfortunately, in sanlock driver we don't really know if acquiring the
resource failed because it was already locked or there was another
reason behind. But the end result is the same and I think using
VIR_ERR_RESOURCE_BUSY reason for all acquire failures is still better
than what we have now.
https://bugzilla.redhat.com/show_bug.cgi?id=1165119
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
In the order of appearance:
* MAX_LISTEN - never used
added by 23ad665c (qemud) and addec57 (lock daemon)
* NEXT_FREE_CLASS_ID - never used, added by 07d1b6b
* virLockError - never used, added by eb8268a4
* OPENVZ_MAX_ARG, CMDBUF_LEN, CMDOP_LEN
unused since the removal of ADD_ARG_LIT in d8b31306
* QEMU_NB_PER_CPU_STAT_PARAM - unused since 897808e
* QEMU_CMD_PROMPT, QEMU_PASSWD_PROMPT - unused since 1dc10a7
* TEST_MODEL_WORDSIZE - unused since c25c18f7
* TEMPDIR - never used, added by 714bef5
* NSIG - workaround around old headers
added by commit 60ed1d2
unused since virExec was moved by commit 02e8691
* DO_TEST_PARSE - never used, added by 9afa006
* DIFF_MSEC, GETTIMEOFDAY - unused since eee6eb6
Commit v1.2.4-52-gda879e5 fixed issues with domains started before
sanlock driver was enabled by checking whether a running domain is
registered with sanlock and if it's not, sanlock driver is basically
ignored for the domain.
However, it was checking this even for domain which has just been
started and no sanlock_* API was called for them yet. This results in
cmd 9 target pid 2135544 not found
error messages to appear in sanlock.log whenever we start a new domain.
This patch avoids this useless check for freshly started domains.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
virLockManager*New APIs are never called with
VIR_LOCK_MANAGER_USES_STATE. Moreover, lockd driver does not maintain
any state that would need to be transferred during migration and thus it
should not mention VIR_LOCK_MANAGER_USES_STATE at all.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Not all files we want to find using virFileFindResource{,Full} are
generated when libvirt is built, some of them (such as RNG schemas) are
distributed with sources. The current API was not able to find source
files if libvirt was built in VPATH.
Both RNG schemas and cpu_map.xml are distributed in source tarball.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Remove the resize flag and use the same code path for all callers.
This flag was added by commit 18f0316 to allow virStorageFileResize
use 'safezero' while preserving the behavior.
Explicitly return -2 when a fallback to a different method should
be done, to make the code path more obvious.
Fail immediately when ftruncate fails in the mmap method,
as we did before commit 18f0316.
Currently virStorageFileResize() function uses build conditionals to
choose either the posix_fallocate() or syscall(SYS_fallocate) with no
fallback in order to preallocate the space in the newly resized file.
Since the safezero code has a similar set of conditionals modify the
resize and safezero code in order to allow the resize logic to make use
of safezero to unify the look/feel of the code paths.
Add a new boolean (resize) to safezero() to make the optional decision
whether to try syscall(SYS_fallocate) if the posix_fallocate fails because
HAVE_POSIX_FALLOCATE is not defined (eg, return -1 and errno == 0).
Create a local safezero_sys_fallocate in order to handle the resize
code paths that support that. If not present, the set errno = ENOSYS
in order to allow the caller to handle the failure scenarios.
Signed-off-by: John Ferlan <jferlan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1160995
In our config files users are expected to pass several integer values
for different configuration knobs. However, majority of them expect a
nonnegative number and only a few of them accept a negative number too
(notably keepalive_interval in libvirtd.conf).
Therefore, a new type to config value is introduced: VIR_CONF_ULONG
that is set whenever an integer is positive or zero. With this
approach knobs accepting VIR_CONF_LONG should accept VIR_CONF_ULONG
too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Since virDomainFree will call virObjectUnref anyway, let's just use that
directly so as to avoid the possibility that we inadvertently clear out
a pending error message when using the public API.
On some places in the libvirt code we have:
f(a,z)
instead of
f(a, z)
This trivial patch fixes couple of such occurrences.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Since not only systemd can do this (we'll be doing it as well few
patches later), change 'systemd' to 'caller' and fix LISTEN_FDS to
LISTEN_PID where applicable.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
In many places we define a variable as a 'const char *' when in fact
we modify it just a few lines below. Or even free it. We should not do
that.
There's one exception though, in xenSessionFree() xenapi_utils.c. We
are freeing the xen_session structure which is defined in
xen/api/xen_common.h public header. The structure contains session_id
which is type of 'const char *' when in fact it should have been just
'char *'. So I'm leaving this unmodified, just noticing the fact in
comment.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In the future we might need to track state of individual images. Move
the readonly and shared flags to the virStorageSource struct so that we
can keep them in a per-image basis.
Replace:
if (virBufferError(&buf)) {
virBufferFreeAndReset(&buf);
virReportOOMError();
...
}
with:
if (virBufferCheckError(&buf) < 0)
...
This should not be a functional change (unless some callers
misused the virBuffer APIs - a different error would be reported
then)
When a domain was started without registration in sanlock, but libvirt
was restarted after that, most of the operations failed due to
contacting sanlock about that process. E.g. migration could not be
performed because the locks couldn't be released (or inquired before a
release).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1088034
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Just move some code around for future patches to ease the review.
With this patch there is no need for drastic cleanup path later.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
The virnetsocket.c API is hardcoded to pass --timeout=30 to
any daemon it auto-starts. For inexplicable reasons the virtlockd
daemon did not implement the --timeout option, so it would
immediately exit on autostart with an error.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Make the lock plugin use virFileFindResource to find the
virtlockd daemon path, so that it executes the in-builddir
daemon if run from source tree.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=905282https://bugzilla.redhat.com/show_bug.cgi?id=967494
When lock failure is detected by sanlock, our sanlock_helper kill script
will try to restart (shutdown followed by start) the affected domain
when RESTART action is configured for it. While shutting down kills QEMU
and removes all its leases (which is what sanlock wants to happen),
trying to start it again just hangs because libvirt tries reacquire the
locks in the failed lock space. Hence, this action cannot be supported
by sanlock driver.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=905280https://bugzilla.redhat.com/show_bug.cgi?id=967493
Sanlock expects that the configured kill script either kills the PID on
lock failure or removes all locks the PID owns. If none of the two
options happen, sanlock will reboot the host. Although IGNORE action is
supposed to ignore the request to kill the PID or remove all leases,
it's certainly not designed to cause the host to be rebooted. That said,
IGNORE action is incompatible with sanlock and should be forbidden by
libvirt.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Part of a series of cleanups to use new accessor methods.
* src/locking/domain_lock.c (virDomainLockManagerAddDisk): Use
accessors.
Signed-off-by: Eric Blake <eblake@redhat.com>
A earlier commit changed the global log buffer so that it only
records messages that are explicitly requested via the log
filters setting. This removes the performance burden, and
improves the signal/noise ratio for messages in the global
buffer. At the same time though, it is somewhat pointless, since
all the recorded log messages are already going to be sent to an
explicit log output like syslog, stderr or the journal. The
global log buffer is thus just duplicating this data on stderr
upon crash.
The log_buffer_size config parameter is left in the augeas
lens to prevent breakage for users on upgrade. It is however
completely ignored hereafter.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Any source file which calls the logging APIs now needs
to have a VIR_LOG_INIT("source.name") declaration at
the start of the file. This provides a static variable
of the virLogSource type.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Commit a1cbe4b5 added a check for spaces around assignments and this
patch extends it to checks for spaces around '=='. One exception is
virAssertCmpInt where comma after '==' is acceptable (since it is a
macro and '==' is its argument).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=992980
This config tunable allows users to determine the maximum number of
accepted but yet not authenticated users.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Our current pidfile acquire APis (virPidFileAcquire) simply return -1 upon
failure to acquire a lock. This patch adds a parameter 'bool waitForLock'
which instructs the APIs if we want to make it block and wait for the lock
or not.
Convert the sanlock and lockd lock driver plugins over to use
the new virCryptoHashString APIs instead of having their own
duplicated code.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Libvirt uses a domain name to fill in owner_name in sanlock_options in
virLockManagerSanlockAcquire. Unfortunately, owner_name is limited to
SANLK_NAME_LEN characters (including trailing '\0'), which means domains
with longer names fail to start when sanlock is enabled. However, we can
truncate the name when setting owner_name as explained by sanlock's
author:
Setting sanlk_options or the owner_name is unnecessary, and has very
little to no benefit. If you do provide something in owner_name, it can
be anything, sanlock doesn't care or use it.
If you run the command "sanlock status", the output will display a list
of clients connected to the sanlock daemon. This client list is
displayed as "pid owner_name" if the client has provided an owner_name
via sanlk_options. This debugging output is the only usage of
owner_name, so its only benefit is to potentially provide a more human
friendly output for debugging purposes.
- Use $XDG_RUNTIME_DIR for re-exec state file when running unprivileged.
- argv[0] may not contain a full path to the binary, however it should
contain something that can be looked up in the PATH. Use execvp() to
do path lookup on re-exec.
- As per list discussion [1], ignore --daemon on re-exec.
[1] https://www.redhat.com/archives/libvir-list/2013-December/msg00514.html
Signed-off-by: Michael Chapman <mike@very.puzzling.org>
Some of our operation denied messages are outright stupid; for
example, if virIdentitySetAttr fails:
error: operation Identity attribute is already set forbidden for read only access
This patch fixes things to a saner:
error: operation forbidden: Identity attribute is already set
It also consolidates the most common usage pattern for operation
denied errors: read-only connections preventing a public API. In
this case, 'virsh -r -c test:///default destroy test' changes from:
error: operation virDomainDestroy forbidden for read only access
to:
error: operation forbidden: read only access prevents virDomainDestroy
Note that we were previously inconsistent on which APIs used
VIR_FROM_DOM (such as virDomainDestroy) vs. VIR_FROM_NONE (such as
virDomainPMSuspendForDuration). After this patch, all uses
consistently use VIR_FROM_NONE, on the grounds that it is unlikely
that a caller learning that a call is denied can do anything in
particular with extra knowledge which error domain the call belongs
to (similar to what we did in commit baa7244).
* src/util/virerror.c (virErrorMsg): Rework OPERATION_DENIED error
message.
* src/internal.h (virCheckReadOnlyGoto): New macro.
* src/util/virerror.h (virReportRestrictedError): New macro.
* src/libvirt-lxc.c: Use new macros.
* src/libvirt-qemu.c: Likewise.
* src/libvirt.c: Likewise.
* src/locking/lock_daemon.c (virLockDaemonClientNew): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
SIGHUP is commonly used to instruct a daemon to reload its config. For
now we should handle it in virtlockd just like SIGUSR1, rather than
having it kill the process.
Signed-off-by: Michael Chapman <mike@very.puzzling.org>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
- Use SIGUSR1, not SIGHUP, on reload. At present, virtlockd only
responds to the former.
- Fix PID file for virtlockd.
- Do not start virtlockd in any runlevels by default. It needs to be
explicitly selected in libvirt's qemu.conf anyway, so there is no
need to have it running on all systems regardless.
- Fix chkconfig priorities to ensure virtlockd is started before
libvirtd is started, and stopped after libvirtd is stopped.
- Add "Should-Start: virtlockd" to the libvirtd initscript's LSB header,
for the same reason.
- Add "Default-Stop" to both libvirtd and virtlockd initscripts. LSB
does not guarantee that this defaults to the inverse of
"Default-Start".
Signed-off-by: Michael Chapman <mike@very.puzzling.org>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
- Pass VIRTLOCKD_ARGS through to virtlockd.
- Use SIGUSR1, not SIGHUP, in ExecReload. At present, virtlockd only
responds to the former.
- Have "systemctl enable virtlockd.service" enable virtlockd.socket,
rather than throw an error.
- Make virtlockd.socket wanted by sockets.target, rather than
multi-user.target. This is consistent with other socket units in
Fedora, and it ensures that the socket is available before libvirtd is
started.
Signed-off-by: Michael Chapman <mike@very.puzzling.org>
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>
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>
Each new VM requires a new connection from libvirtd to virtlockd.
The default max clients limit in virtlockd of 20 is thus woefully
insufficient. virtlockd sockets are only accessible to matching
users, so there is no security need for such a tight limit. Make
it configurable and default to 1024.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virtlockd/libvirtd daemons had listed '?' as the short option
for --help. getopt_long uses '?' for any unknown option. We want
to be able to distinguish unknown options (which use EXIT_FAILURE)
from correct usage of help (which should use EXIT_SUCCESS). Thus
we should use 'h' as a short option for --help. Also add this to
the man page docs
The virtlockd/libvirtd daemons did not list any short option
for the --version arg. Add -V as a valid short option, since
-v is already used for --verbose.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virtlockd daemon supports an /etc/libvirt/virtlockd.conf
config file, but we never installed a default config, nor
created any augeas scripts. This change addresses that omission.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This configuration knob lets user to set the length of queue of
connection requests waiting to be accept()-ed by the daemon. IOW, it
just controls the @backlog passed to listen:
int listen(int sockfd, int backlog);
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>
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>
Since PIDs can be reused, polkit prefers to be given
a (PID,start time) pair. If given a PID on its own,
it will attempt to lookup the start time in /proc/pid/stat,
though this is subject to races.
It is safer if the client app resolves the PID start
time itself, because as long as the app has the client
socket open, the client PID won't be reused.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
A 'uri' parameter was added for the benefit of sanlock. This
causes a warning in the lockd driver though
2013-05-03 13:20:35.347+0000: 28403: error : virLockManagerLockDaemonNew:482 : internal error Unexpected parameter uri for object
Ignore this parameter, since lockd does not require it and it
is harmless if not used.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The lockd plugin for the lock manager was not correctly
handling the release of resource locks. This meant that
during migration, or when pausing a VM, the locks would
not get released. This in turn made it impossible to
resume the domain, or finish migration
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.
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>
In virLockSpaceProtocolDispatchNew() the returned value of lockspace from
virLockDaemonFindLockSpace() is overwritten by the virLockSpaceNew() return.
Coverity complains that it's unused.
In virLockSpaceProtocolDispatchCreateLockSpace() lockspace is also overwritten
in a similar manner resulting in the same Coverity message.
POSIX does not guarantee whether uid_t and gid_t are signed or
unsigned, nor does it guarantee whether they are smaller, same
size, or larger than int (or even the same size as one another).
Therefore, it is possible to have platforms where '(uid_t)-1==-1'
is false or where 'uid = gid = -1' sets uid to the wrong value,
thanks to integer promotion rules. The only portable way to use
the placeholder value of these two types is to always use a cast.
Thankfully, the issue is mostly theoretical - sanlock only
compiles on Linux for now, and on Linux, these types do not
suffer from strange promotion problems.
* src/locking/lock_driver_sanlock.c
(virLockManagerSanlockSetupLockspace, virLockManagerSanlockInit)
(virLockManagerSanlockCreateLease): Cast -1 to proper type before
comparing with uid_t or gid_t.
virtlockd.service could be installed to a configurable root,
but virtlockd.socket was hardcoded to installation into a
distro.
* src/Makefile.am (virtlockd.service, virtlockd.socket): Drop
unused substitutions.
* src/locking/virtlockd.socket.in (ListenStream): Don't hard-code
/var.
We had several different styles of .in conversion in our Makefiles:
ALLCAPS, @ALLCAPS@, @lower@, ::lower::
Canonicalize on one form, to make it easier to copy and paste
between .in files.
Also, we were using some non-portable sed constructs: \@ is an
undefined escape sequence (it happens to be @ itself in GNU sed,
but POSIX allows it to mean something else), as well as risky
behavior (failure to consistently quote things means a space
in $(sysconfdir) could throw things off; also, Autoconf recommends
using | rather than , or ! in the s||| operator, because | has to
be quoted in shell and is therefore less likely to appear in file
names than , or !).
Fix all of these uses to follow the same syntax.
* daemon/libvirtd.8.in: Switch to @var@.
* tools/virt-xml-validate.in: Likewise.
* tools/virt-pki-validate.in: Likewise.
* src/locking/virtlockd.init.in: Likewise.
* daemon/Makefile.am: Prefer | over ! in sed.
(libvirtd.8): Prefer consistent substitution.
(libvirtd.init, libvirtd.service): Avoid non-portable sed.
* tools/Makefile.am (libvirt-guests.sh, libvirt-guests.init)
(libvirt-guests.service): Likewise.
(virt-xml-validate, virt-pki-validate, virt-sanlock-cleanup):
Prefer consistent capitalization.
* src/Makefile.am (virtlockd.init, virtlockd.service)
(virtlockd.socket): Prefer consistent substitution.
Since sanlock doesn't run under root:root, we have chown()'ed the
__LIBVIRT__DISKS__ lease file to the user:group defined in the
sanlock config. However, when writing the patch I've forgot about
lease files for each disk (this is the
/var/lib/libvirt/sanlock/<md5>) file.
Currently, if sanlock is already registering a lockspace other
libvirtd instances (from other hosts) obtain -EINPROGRESS. On
sufficiently new sanlock, sanlock_inq_lockspace() is called,
which suspend execution until lockspace state is changed. With
current libvirt implementation, we fail to retry adding the
lockspace again but continue in error path. Therefore we produce
meaningless error message:
virLockManagerSanlockSetupLockspace:363 : Unable to add lockspace
/var/lib/libvirt/sanlock/__LIBVIRT__DISKS__: Success
qemudLoadDriverConfig:558 : Failed to load lock manager sanlock
We should try to re-add the lockspace after its state change to
be sure it was added successfully. In fact, with sufficiently new
sanlock we can just avoid dummy usleep() which is used if there's
no inquire API.
The default lockd driver behavour is to acquire leases
directly on the disk files. This introduces an alternative
mode, where leases are acquire indirectly on a file that
is based on a SHA256 hash of the disk filename.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This adds a 'lockd' lock driver which is just a client which
talks to the lockd daemon to perform all locking. This will
be the default lock driver for any hypervisor which needs one.
* src/Makefile.am: Add lockd.so plugin
* src/locking/lock_driver_lockd.c: Lockd driver impl
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virtlockd daemon maintains file locks on behalf of libvirtd
and any VMs it is running. These file locks must be held for as
long as any VM is running. If virtlockd itself ever quits, then
it is expected that a node would be fenced/rebooted. Thus to
allow for software upgrads on live systemd, virtlockd needs the
ability to re-exec() itself.
Upon receipt of SIGUSR1, virtlockd will save its current live
state out to a file /var/run/virtlockd-restart-exec.json
It then re-exec()'s itself with exactly the same argv as it
originally had, and loads the state file, reconstructing any
objects as appropriate.
The state file contains information about all locks held and
all network services and clients currently active. An example
state document is
{
"server": {
"min_workers": 1,
"max_workers": 20,
"priority_workers": 0,
"max_clients": 20,
"keepaliveInterval": 4294967295,
"keepaliveCount": 0,
"keepaliveRequired": false,
"services": [
{
"auth": 0,
"readonly": false,
"nrequests_client_max": 1,
"socks": [
{
"fd": 6,
"errfd": -1,
"pid": 0,
"isClient": false
}
]
}
],
"clients": [
{
"auth": 0,
"readonly": false,
"nrequests_max": 1,
"sock": {
"fd": 9,
"errfd": -1,
"pid": 0,
"isClient": true
},
"privateData": {
"restricted": true,
"ownerPid": 1722,
"ownerId": 6,
"ownerName": "f18x86_64",
"ownerUUID": "97586ba9-df27-9459-c806-f016c8bbd224"
}
},
{
"auth": 0,
"readonly": false,
"nrequests_max": 1,
"sock": {
"fd": 10,
"errfd": -1,
"pid": 0,
"isClient": true
},
"privateData": {
"restricted": true,
"ownerPid": 1784,
"ownerId": 7,
"ownerName": "f16x86_64",
"ownerUUID": "7b8e5e42-b875-61e9-b981-91ad8fa46979"
}
}
]
},
"defaultLockspace": {
"resources": [
{
"name": "/var/lib/libvirt/images/f16x86_64.raw",
"path": "/var/lib/libvirt/images/f16x86_64.raw",
"fd": 14,
"lockHeld": true,
"flags": 0,
"owners": [
1784
]
},
{
"name": "/var/lib/libvirt/images/shared.img",
"path": "/var/lib/libvirt/images/shared.img",
"fd": 12,
"lockHeld": true,
"flags": 1,
"owners": [
1722,
1784
]
},
{
"name": "/var/lib/libvirt/images/f18x86_64.img",
"path": "/var/lib/libvirt/images/f18x86_64.img",
"fd": 11,
"lockHeld": true,
"flags": 0,
"owners": [
1722
]
}
]
},
"lockspaces": [
],
"magic": "30199"
}
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This enhancement virtlockd so that it can receive a pre-opened
UNIX domain socket from systemd at launch time, and adds the
systemd service/socket unit files
* daemon/libvirtd.service.in: Require virtlockd to be running
* libvirt.spec.in: Add virtlockd systemd files
* src/Makefile.am: Install systemd files
* src/locking/lock_daemon.c: Support socket activation
* src/locking/virtlockd.service.in, src/locking/virtlockd.socket.in:
systemd unit files
* src/rpc/virnetserverservice.c, src/rpc/virnetserverservice.h:
Add virNetServerServiceNewFD() method
* src/rpc/virnetsocket.c, src/rpc/virnetsocket.h: Add virNetSocketNewListenFD
method
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Introduce a lock_daemon_dispatch.c file which implements the
server side dispatcher the RPC APIs previously defined in the
lock protocol.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
