This brings about a couple of benefits:
- use of VIR_AUTOUNREF() simplifies several callers
- Fixes a todo about virDomainMomentObjList not being polymorphic enough
Signed-off-by: Eric Blake <eblake@redhat.com>
Acked-by: Peter Krempa <pkrempa@redhat.com>
Drop the checking for _LAST optionally on the first line, previous
patch removed all those instances
Signed-off-by: Cole Robinson <crobinso@redhat.com>
The use of d_type is non-portable and leads to surprises when the OS
does not fill in any value except DT_UNKNOWN. Blacklist its usage
except in files which inherantly don't require portability (cgroups).
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Quite a few of the tests have a need to mock the stat() / lstat()
functions and they are taking somewhat different & inconsistent
approaches none of which are actually fully correct. This is shown
by fact that 'make check' fails on 32-bit hosts. Investigation
revealed that the code was calling into the native C library impl,
not getting intercepted by our mocks.
The POSIX stat() function might resolve to any number of different
symbols in the C library.
The may be an additional stat64() function exposed by the headers
too.
On 64-bit hosts the stat & stat64 functions are identical, always
refering to the 64-bit ABI.
On 32-bit hosts they refer to the 32-bit & 64-bit ABIs respectively.
Libvirt uses _FILE_OFFSET_BITS=64 on 32-bit hosts, which causes the
C library to transparently rewrite stat() calls to be stat64() calls.
Libvirt will never see the 32-bit ABI from the traditional stat()
call. We cannot assume this rewriting is done using a macro. It might
be, but on GLibC it is done with a magic __asm__ statement to apply
the rewrite at link time instead of at preprocessing.
In GLibC there may be two additional functions exposed by the headers,
__xstat() and __xstat64(). When these exist, stat() and stat64() are
transparently rewritten to call __xstat() and __xstat64() respectively.
The former symbols will not actally exist in the library at all, only
the header. The leading "__" indicates the symbols are a private impl
detail of the C library that applications should not care about.
Unfortunately, because we are trying to mock replace the C library,
we need to know about this internal impl detail.
With all this in mind the list of functions we have to mock will depend
on several factors
- If _FILE_OFFSET_BITS is set, then we are on a 32-bit host, and we
only need to mock stat64 and __xstat64. The other stat / __xstat
functions exist, but we'll never call them so they can be ignored
for mocking.
- If _FILE_OFFSET_BITS is not set, then we are on a 64-bit host and
we should mock stat, stat64, __xstat & __xstat64. Either may be
called by app code.
- If __xstat & __xstat64 exist, then stat & stat64 will not exist
as symbols in the library, so the latter should not be mocked.
The same all applies to lstat()
These rules are complex enough that we don't want to duplicate them
across every mock file, so this centralizes all the logic in a helper
file virmockstathelper.c that should be #included when needed. The
code merely need to provide a filename rewriting callback called
virMockStatRedirect(). Optionally VIR_MOCK_STAT_HOOK can be defined
as a macro if further processing is needed inline.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Our ChangeLog is generated by basically redirecting the output
of 'git log' into it so, as can be expected, it has only gotten
bigger as development has progressed. As of today, its size has
reached pretty much comical levels:
$ du -sk ChangeLog
11328 ChangeLog
All of that for information *literally nobody* cares about: end
users and distro maintainers have proper release notes lovingly
compiled for them, while developers peruse the history either by
calling 'git log' directly or through their favorite $EDITOR's
git integration.
Replacing the generated ChangeLog with a short message pointing
interested parties to the git repository does not only reduce
the size of the unpacked sources from 259904 KiB to 248576 KiB
(~4% saving): from a quick test on my laptop, doing so reduces
the size of the *compressed* release archive from 15140 KiB to
12364 KiB (~18% saving) and also takes the time needed to run
'make distcheck' down from 4:44 to 4:21 (~8% saving).
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Vim treats *.h files as cpp ones with respect to syntax highlighting.
Thus "class" in _virNodeDevCapPCIDev highlighted mistakenly.
This can be fixed by filetype detection code tunables but it
is more convinient to skip this tuning by every project member.
Let's just use "klass" as field name instead of _class or class
and add syntax rule.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Replace virDomainChrSourceDefFree with virObjectUnref.
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Move the majority of the packet-libvirt.h content into
packet-libvirt.c and expose only register functions which are the
only ones that are not static.
The rationale behind is that packet-libvirt.h will be included
from packet.c and therefore the header file needs to be as clean
as possible.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Security labeling of disks consists of labeling of the disk image
itself and it's backing chain. Modify
virSecurityManager[Set|Restore]ImageLabel to take a boolean flag that
will label the full chain rather than the top image itself.
This allows to delete/unify some parts of the code and will also
simplify callers in some cases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
mingw lacks localtime_r(); we were getting it from gnulib. But since
commit acf522e8 stopped linking examples against gnulib, we are
getting a build failure. Keep the examples standalone, and work
around mingw by using the non-reentrant localtime() (safe since our
examples are single-threaded), and add a necessary exemption to our
syntax check.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The check was concerning itself with whitespace where it
didn't need to, and used some confusing escaping for one
of its regular expressions - which GNU sed was fine with,
but FreeBSD's sed didn't like one bit.
Switch to extended regular expressions (which, incidentally,
were already in use in the same rule when calling grep) and
remove all whitespace handling.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similar to the gnulib changes we just incorporated into maint.mk,
it's time to use '$(VC_LIST) | xargs program' instead of
'program $$($(VC_LIST))', in order to bypass the problem of hitting
argv limits due to our large set of files.
Drop several uses of $$files as a temporary variable when we can
instead directly use xargs. While at it, fix a typo in the
prohibit_windows_special_chars error message.
Note that 'grep $pattern $(generate list)' has be be rewritten
as 'generate list | xargs grep $pattern /dev/null' - this is
because for a list that is just long enough, and without /dev/null,
xargs could make a worst-case split of 'grep $pattern all but one;
grep $pattern last' which has different output (grep includes the
filename when there was more than one file, but omits it for a
single file), while our conversion gives 'grep $pattern /dev/null
all but one; grep $pattern /dev/null last'. We are less concerned
about the empty list case (why would we run the syntax check if we
didn't have at least one file?), but grepping /dev/null happens to
produce no output and thus nicely also solves that problem without
relying on the GNU extension of 'xargs -r'.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We already used $(GREP) in some places, but might as well use it
everywhere during syntax check, in line with similar recent gnulib
changes.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This test checks if security label remembering works correctly.
It uses qemuSecurity* APIs to do that. And some mocking (even
though it's not real mocking as we are used to from other tests
like virpcitest). So far, only DAC driver is tested.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@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>
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>
Add zPCI definitions in preparation of extending the PCI address
with parameters uid (user-defined identifier) and fid (PCI function
identifier).
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1640465
Weirdly enough, there can be symlinks in the path we are trying
to fix. If it is the case our clever algorithm that finds matches
against mount table won't work. Canonicalize path at the
beginning then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Now that we know what metadata lock manager user wishes to use we
can load it when initializing security driver. This is achieved
by adding new argument to virSecurityManagerNewDriver() and
subsequently to all functions that end up calling it.
The cfg.mk change is needed in order to allow lock_manager.h
inclusion in security driver without 'syntax-check' complaining.
This is safe thing to do as locking APIs will always exist (it's
only backend implementation that changes). However, instead of
allowing the include for all other drivers (like cpu, network,
and so on) allow it only for security driver. This will still
trigger the error if including from other drivers.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
A variable, which is never assigned a value in the function, might get
passed into the cleanup function which may or may not raise any errors.
To maintain the correct usage, the variable must be initialized, either
with a value or with NULL. This syntax-check rule takes care of that.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
We want to make sure our wrapper is used instead in order
to keep the test suite working.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The script assumed to be run in the source directory.
Pass top_srcdir as the argument to fix VPATH builds.
My commit 81a7571 broke this.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Introduce a perl script that is able to regroup both
the QEMU_CAPS constants and the capability strings.
Check correct grouping as a part of syntax check.
For in-place regrouping after a rebase, just run:
tests/group-qemu-caps.pl
without any parameters.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Now that we have macro that does some checks lets forbid raw
usage of virClassNew() in favor of VIR_CLASS_NEW().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Remove the qmpOnly argument of virQEMUCapsNewForBinaryInternal
and instead always assume it's true.
This effectively sets the minimum QEMU version to 1.2.0.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
The functions like isalnum(), isalpha(), isdigit(), etc. are also
available in Python, however `make syntax-check` do not intend to
prohibit them.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
The diff files in gnulib/local may well contain trailing whitespace.
They are simply diff's of gnulib code which should not be modified to
follow libvirt style rules.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Having a daemon/ directory makes little sense from a code structure
point of view, as 90% of the code that is built into libvirtd already
lives in the src/ directory. The virtlockd and virlogd daemons also live
entirely in src/{locking,logging} directories. This moves the source
code for libvirtd into src/remote/, alongside the client code.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The THREADS.txt describes the way threading worked in libvirtd many many
years ago, long before even the RPC code was modularized. Things have
evolved significantly since then, so delete this potentially misleading doc.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The fix for CVE-2018-6764 introduced a potential deadlock scenario
that gets triggered by the NSS module when virGetHostname() calls
getaddrinfo to resolve the hostname:
#0 0x00007f6e714b57e7 in futex_wait
#1 futex_wait_simple
#2 __pthread_once_slow
#3 0x00007f6e71d16e7d in virOnce
#4 0x00007f6e71d0997c in virLogInitialize
#5 0x00007f6e71d0a09a in virLogVMessage
#6 0x00007f6e71d09ffd in virLogMessage
#7 0x00007f6e71d0db22 in virObjectNew
#8 0x00007f6e71d0dbf1 in virObjectLockableNew
#9 0x00007f6e71d0d3e5 in virMacMapNew
#10 0x00007f6e71cdc50a in findLease
#11 0x00007f6e71cdcc56 in _nss_libvirt_gethostbyname4_r
#12 0x00007f6e724631fc in gaih_inet
#13 0x00007f6e72464697 in __GI_getaddrinfo
#14 0x00007f6e71d19e81 in virGetHostnameImpl
#15 0x00007f6e71d1a057 in virGetHostnameQuiet
#16 0x00007f6e71d09936 in virLogOnceInit
#17 0x00007f6e71d09952 in virLogOnce
#18 0x00007f6e714b5829 in __pthread_once_slow
#19 0x00007f6e71d16e7d in virOnce
#20 0x00007f6e71d0997c in virLogInitialize
#21 0x00007f6e71d0a09a in virLogVMessage
#22 0x00007f6e71d09ffd in virLogMessage
#23 0x00007f6e71d0db22 in virObjectNew
#24 0x00007f6e71d0dbf1 in virObjectLockableNew
#25 0x00007f6e71d0d3e5 in virMacMapNew
#26 0x00007f6e71cdc50a in findLease
#27 0x00007f6e71cdc839 in _nss_libvirt_gethostbyname3_r
#28 0x00007f6e71cdc724 in _nss_libvirt_gethostbyname2_r
#29 0x00007f6e7248f72f in __gethostbyname2_r
#30 0x00007f6e7248f494 in gethostbyname2
#31 0x000056348c30c36d in hosts_keys
#32 0x000056348c30b7d2 in main
Fortunately the extra stuff virGetHostname does is totally irrelevant to
the needs of the logging code, so we can just inline a call to the
native hostname() syscall directly.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Loadable drivers must never depend on each other. Over time some usage
mistakenly crept in for the storage and network drivers, but now this is
eliminated the syntax-check rules can enforce this separation once more.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Add a virtlockd-admin-sock can serves the admin protocol for the virtlockd
daemon and define a virtlockd:///{system,session} URI scheme for
connecting to it.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The admin server functionality is a generic concept that should be wired
up into all libvirt daemons, but is currently integrated with the
libvirtd code. Move it all into the src/admin directory to prepare for
broader reuse.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The use of [[:blank:]] was intended to cover tabs as well, but
it couldn't possibly work in its current form, so the regex was
tweaked in d09429abe8.
With the original reason for using [[:blank:]] now gone, we can
replace its usage with plain spaces. A comment about the purpose
of the check is added as well.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
We have a check for backslash alignment that checks for two blanks preceding a
backslash. However there can be alignment done using a tabulator and in some
cases one might be enough. There are none currently, but I found out that was
the case before. at some point. so let's check for \t preceding the final
backslash as well.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
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>
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>
The commandhelper binary is a helper for commandtest that
validates what file handles were inherited. For this to
work reliably we must not have any libraries that leak
file descriptors into commandhelper. Unfortunately some
versions of gnutls will intentionally open file handles
at library load time via a constructor function.
We previously hacked around this in
commit 4cbc15d037
Author: Martin Kletzander <mkletzan@redhat.com>
Date: Fri May 2 09:55:52 2014 +0200
tests: don't fail with newer gnutls
gnutls-3.3.0 and newer leaves 2 FDs open in order to be backwards
compatible when it comes to chrooted binaries [1]. Linking
commandhelper with gnutls then leaves these two FDs open and
commandtest fails thanks to that. This patch does not link
commandhelper with libvirt.la, but rather only the utilities making
the test pass.
Based on suggestion from Daniel [2].
[1] http://lists.gnutls.org/pipermail/gnutls-help/2014-April/003429.html
[2] https://www.redhat.com/archives/libvir-list/2014-April/msg01119.html
That fix relied on fact that while libvirt.so linked with
gnutls, libvirt_util.la did not link to it. With the
introduction of the util/vircrypto.c file that assumption
is no longer valid. We must not link to libvirt_util.la
at all - only gnulib and libc can (hopefully) be relied
on not to open random file descriptors in constructors.
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Despite being a generated file, HACKING has been tracked in
the git repository along with actual source files. As far as
I'm aware, it's the only generated file for which that happens.
Times and times again, people[1] have committed changes to
the source file without refreshing the generated copy at the
same time.
The rationale for tracking the generated file is to help out
people who just cloned the git repository looking to contribue;
however, README-hacking already contains enough information to
get perspective contributors to a place where they can simply
look at docs/hacking.html instead.
[1] Mostly me, to be honest
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
This mock (which is actually not mock at all, see later) can redirect
all accesses to a path into another path. There is no need to
create mocks for particular directories, you just create a directory
with all the data a redirect the test there.
In the future, this should also be able to register callbacks for
calls/paths, e.g. when the test is going to write into anything under
"/sys/devices", call function fce(); Then in the open() call we would
add information about the fd into some structure and in write() we
would call fce() with parameters like @path to write to, @data to
be written and pointer to optional return value, so that fce() itself
could stop the call from happening or change its behaviour. But
that's an idea for a latter day.
This is not a mock because it will not be preloaded, but compiled in
the test itself. See future patches for usage.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Per xen-xl conversions from and to native under host-passthrough
mode we take care for Xen (nestedhvm = mode) applied and inherited
settings generating or processing correct feature policy:
[On Intel (VT-x) architectures]
<feature policy='disable' name='vmx'/>
or
[On AMD (AMD-V) architectures]
<feature policy='disable' name='svm'/>
It will then generate (or parse) for nestedhvm=1 in/from xl format.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Wim ten Have <wim.ten.have@oracle.com>
The goal is twofold: firstly, we want to extend the script so
that it can deal with more than a single git submodule, and
secondly we'd like to reduce the amount of duplicated code.
Moreover, since we're making heavy changes to the code anyway,
we might as well make sure it follows a somewhat consistent
coding style too.
To reduce code duplication, we introduce a new --dry-run
option, which can be used by third parties to figure out
whether calling autogen.sh is necessary or not: this allows
us to get rid of the reimplementation of part of the logic
in cfg.mk and guarantee they'll never get out of sync.
Other changes include: making dirty submodules checking and
cleaning entirely independent of other operations; removing
the use of 'set -e' and handling errors explicitly instead;
better parsing of command line arguments.
