This reverts commit e4b980c853.
When a binary links against a .a archive (as opposed to a shared library),
any symbols which are marked as 'weak' get silently dropped. As a result
when the binary later runs, those 'weak' functions have an address of
0x0 and thus crash when run.
This happened with virtlogd and virtlockd because they don't link to
libvirt.so, but instead just libvirt_util.a and libvirt_rpc.a. The
virRandomBits symbols was weak and so left out of the virtlogd &
virtlockd binaries, despite being required by virHashTable functions.
Various other binaries like libvirt_lxc, libvirt_iohelper, etc also
link directly to .a files instead of libvirt.so, so are potentially
at risk of dropping symbols leading to a later runtime crash.
This is normal linker behaviour because a weak symbol is not treated
as undefined, so nothing forces it to be pulled in from the .a You
have to force the linker to pull in weak symbols using -u$SYMNAME
which is not a practical approach.
This risk is silent bad linkage that affects runtime behaviour is
not acceptable for a fix that was merely trying to fix the test
suite. So stop using __weak__ again.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The Win32 platform will fail to link if you use weak symbols
because it is incompatible with exporting symbols in a DLL:
Cannot export virRandomGenerateWWN: symbol wrong type (2 vs 3)
We only need weak symbols for our test suite to do LD_PRELOAD
and this doesn't work on Win32, so we can just drop the hack
for Win32
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently all mockable functions are annotated with the 'noinline'
attribute. This is insufficient to guarantee that a function can
be reliably mocked with an LD_PRELOAD. The C language spec allows
the compiler to assume there is only a single implementation of
each function. It can thus do things like propagating constant
return values into the caller at compile time, or creating
multiple specialized copies of the function body each optimized
for a different caller. To prevent these optimizations we must
also set the 'noclone' and 'weak' attributes.
This fixes the test suite when libvirt.so is built with CLang
with optimization enabled.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The TODO macro expands to an fprintf() call and is used in several
places in the Xen driver. Anything that wishes to print such debug
messages should use the logging macros. In this case though, all the
places in the Xen driver should have been raising a formal libvirt
error instead. Add proper error handling and delete the TODO macro
to prevent future misuse.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The HOST_NAME_MAX, INET_ADDRSTRLEN and VIR_LOOPBACK_IPV4_ADDR
constants are only used by a handful of files, so are better
kept in virsocketaddr.h or the source file that uses them.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
We only ever test libvirt with GCC or CLang which provides a
GCC compatible compilation environment. Between them, these
compilers cover every important operating system platform,
even Windows.
Mandate their use to make it explicit that we don't care about
compilers like Microsoft VCC or other UNIX vendor C compilers.
GCC 4.4 was picked as the baseline, since RHEL-6 ships 4.4.7
and that lets us remove a large set of checks. There is a slight
issue that CLang reports itself as GCC 4.2, so we must also check
if __clang__ is defined. We could check a particular CLang version
too, but that would require someone to figure out a suitable min
version which is fun because OS-X reports totally different CLang
version numbers from CLang builds on Linux/BSD
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Back in this commit:
commit b436a8ae5c
Author: Fabian Freyer <fabian.freyer@physik.tu-berlin.de>
Date: Thu Jun 9 00:50:35 2016 +0000
gnulib: add getopt module
config-post.h was modified to define __GNUC_PREREQ, but the
original definition was never removed from internal.h, and
that is now dead code since config.h is always the first file
included.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently, we don't assign any meaning to that. Our current view
on virStream is that it's merely a pipe. And pipes don't support
seeking.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The debian etch distro was end-of-life a long time ago so we no
longer need the ULLONG_MAX hack. In any case gnulib now provides
an equivalent fix by default, and so our definition now triggers
syntax-check rule failure
src/internal.h:# define ULLONG_MAX ULONG_LONG_MAX
maint.mk: define the above via some gnulib .h file
maint.mk:843: recipe for target 'sc_prohibit_always-defined_macros' failed
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Apple have annotated all SASL functions as deprecated for
unknown reasons. Since they still work, lets just ignore
the warnings. If Apple finally delete the SASL functions
our configure check should already catch that
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
CLang's optimizer is more aggressive at inlining functions than
gcc and so will often inline functions that our tests want to
mock-override. This causes the test to fail in bizarre ways.
We don't want to disable inlining completely, but we must at
least prevent inlining of mocked functions. Fortunately there
is a 'noinline' attribute that lets us control this per function.
A syntax check rule is added that parses tests/*mock.c to extract
the list of functions that are mocked (restricted to names starting
with 'vir' prefix). It then checks that src/*.h header file to
ensure it has a 'ATTRIBUTE_NOINLINE' annotation. This should prevent
use from bit-rotting in future.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
In GCC 7 there is a new warning triggered when a switch
case has a conditional statement (eg if ... else...) and
some of the code paths fallthrough to the next switch
statement. e.g.
conf/domain_conf.c: In function 'virDomainChrEquals':
conf/domain_conf.c:14926:12: error: this statement may fall through [-Werror=implicit-fallthrough=]
if (src->targetTypeAttr != tgt->targetTypeAttr)
^
conf/domain_conf.c:14928:5: note: here
case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE:
^~~~
conf/domain_conf.c: In function 'virDomainChrDefFormat':
conf/domain_conf.c:22143:12: error: this statement may fall through [-Werror=implicit-fallthrough=]
if (def->targetTypeAttr) {
^
conf/domain_conf.c:22151:5: note: here
default:
^~~~~~~
GCC introduced a __attribute__((fallthrough)) to let you
indicate that this is intentionale behaviour rather than
a bug.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Our STREQ_NULLABLE and STRNEQ_NULLABLE macros are too
complicated. This was a result of some broken version of gcc.
However, that is long gone and therefore we can simplify the
macros.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
fdstream.c: In function 'virFDStreamWrite':
fdstream.c:390:29: error: logical 'or' of equal expressions [-Werror=logical-op]
if (errno == EAGAIN || errno == EWOULDBLOCK) {
^~
Fedora rawhide now uses gcc 6.0 and there is a bug with -Wlogical-op
producing false warnings.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69602
Use GCC pragma push/pop and ignore -Wlogical-op for GCC that supports
push/pop pragma and also has this bug.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
FreeBSD lacks ENODATA, and viruuid.c redefines it to EIO, but it's not
actually using it. On the other hand, we have virrandom.c that's using
ENODATA. So make this re-definition common by moving it to internal.h,
so all the current and possible future users don't need to care about
that.
Initial scratch of the admin library. It has its own virAdmConnectPtr
that inherits from virAbstractConnectPtr and thus trivially supports
error reporting.
There's pkg-config file added and spec-file adjusted as well.
Since the library should be "minimalistic" and not depend on any other
library, the list of files is especially crafted for it. Most of them
could've been put to it's own sub-libraries that would be LIBADD'd to
libvirt_util, libvirt_net_rpc and libvirt_setuid_rpc_client to minimize
the number of object files being built, but that's a refactoring that
isn't the orginal aim of this commit.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Inspired by commit 7e437ee7 that introduced similar macros for virsh
commands so we don't have to repeat the same code all over.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
There are two special cases, if the input number is 0 or the number is
larger then 2^31 (for 32bit unsigned int). For the special cases the
return value is 0 because they cannot be rounded.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
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>
Use virNetworkGetDHCPLeases and virNetworkGetDHCPLeasesForMAC in virsh.
The new feature supports the follwing methods:
1. Retrieve leases info for a given virtual network
2. Retrieve leases info for given network interface
tools/virsh-domain-monitor.c
* Introduce new command : net-dhcp-leases
Example Usage: net-dhcp-leases <network> [mac]
virsh # net-dhcp-leases --network default6
Expiry Time MAC address Protocol IP address Hostname Client ID or DUID
-------------------------------------------------------------------------------------------------------------------
2014-06-16 03:40:14 52:54:00:85:90:e2 ipv4 192.168.150.231/24 fedora20-test 01:52:54:00:85:90:e2
2014-06-16 03:40:17 52:54:00:85:90:e2 ipv6 2001:db8:ca2:2:1::c0/64 fedora20-test 00:04:b1:d8:86:42:e1:6a:aa:cf:d5:86:94:23:6f:94:04:cd
2014-06-16 03:34:42 52:54:00:e8:73:eb ipv4 192.168.150.181/24 ubuntu14-vm -
2014-06-16 03:34:46 52:54:00:e8:73:eb ipv6 2001:db8:ca2:2:1::5b/64 - 00:01:00:01:1b:30:c6:aa:52:54:00:e8:73:eb
tools/virsh.pod
* Document new command
src/internal.h
* Introduce new macro: EMPTYSTR
For future work we need two functions that fetches total number of
pages and number of free pages for given NUMA node and page size
(virNumaGetPageInfo()).
Then we need to learn pages of what sizes are supported on given node
(virNumaGetPages()).
Note that system page size is disabled at the moment as there's one
issue connected. If you have a NUMA node with huge pages allocated the
kernel would return the normal size of memory for that node. It
basically ignores the fact that huge pages steal size from the system
memory. Until we resolve this, it's safer to not confuse users and
hence not report any system pages yet.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The dtrace probe macros rely on the logging API. We can't make
the internal.h header include the virlog.h header though since
that'd be a circular include. Instead simply split the dtrace
probes into their own header file, since there's no compelling
reason for them to be in the main internal.h header.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When a child fails without exec'ing, we want a well-known status;
best is to match what env(1), nice(1), su(1), and other wrapper
programs do. This patch adds enum values that later patches will
use, and sets up virFork as the first client of EXIT_CANCELED
for errors detected prior to even attempting exec, as well as
virExec to distinguish between a missing executable vs. a binary
that cannot be executed.
This is a slight semantic change in the unlikely case of a child
process failing to restore its signal mask - we now kill the
child with a known status instead of relying on the caller to
notice and do an appropriate _exit(). A subsequent patch will
make further cleanups based on an audit of all callers.
* src/internal.h (EXIT_CANCELED, EXIT_CANNOT_INVOKE)
(EXIT_ENOENT): New enum.
* src/util/vircommand.c (virFork): Document specific exit value if
child aborts early.
(virExec): Distinguish between various exec failures.
* tests/commandtest.c (test1): Enhance test.
(test22): New test.
Signed-off-by: Eric Blake <eblake@redhat.com>
Eric Blake suggested to change this message to be different from the
glibc's NULL deref protection message in printf to be able to
differentiate errors.
The libvirt_internal.h header was included by the internal.h header.
This made it painful to add new stuff to the header file that would
require some more specific types. Remove inclusion by internal.h and add
it to appropriate places manually.
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>
qemu-img resize will fail with "The new size must be a multiple of 512"
if libvirt doesn't round it first.
This fixes rhbz#951495
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
The virt-compile-warnings.m4 file would do an explicit
check for whether the compile could use the 'diagnostic'
pragma push/pop feature. The src/internal.h file would
then only enable it for GCC >= 4.6
This breaks with clang which supports the pragma but
does not claim GCC 4.6 compat. Export a variable from
the m4 check to the header file so they are consistent.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
There are a number of places which generate cast alignment
warnings, which are difficult or impossible to address. Use
pragmas to disable the warnings in these few places
conf/nwfilter_conf.c: In function 'virNWFilterRuleDetailsParse':
conf/nwfilter_conf.c:1806:16: warning: cast increases required alignment of target type [-Wcast-align]
item = (nwItemDesc *)((char *)nwf + att[idx].dataIdx);
conf/nwfilter_conf.c: In function 'virNWFilterRuleDefDetailsFormat':
conf/nwfilter_conf.c:3238:16: warning: cast increases required alignment of target type [-Wcast-align]
item = (nwItemDesc *)((char *)def + att[i].dataIdx);
storage/storage_backend_mpath.c: In function 'virStorageBackendCreateVols':
storage/storage_backend_mpath.c:247:17: warning: cast increases required alignment of target type [-Wcast-align]
names = (struct dm_names *)(((char *)names) + next);
nwfilter/nwfilter_dhcpsnoop.c: In function 'virNWFilterSnoopDHCPDecode':
nwfilter/nwfilter_dhcpsnoop.c:994:15: warning: cast increases required alignment of target type [-Wcast-align]
pip = (struct iphdr *) pep->eh_data;
nwfilter/nwfilter_dhcpsnoop.c:1004:11: warning: cast increases required alignment of target type [-Wcast-align]
pup = (struct udphdr *) ((char *) pip + (pip->ihl << 2));
nwfilter/nwfilter_learnipaddr.c: In function 'procDHCPOpts':
nwfilter/nwfilter_learnipaddr.c:327:33: warning: cast increases required alignment of target type [-Wcast-align]
uint32_t *tmp = (uint32_t *)&dhcpopt->value;
nwfilter/nwfilter_learnipaddr.c: In function 'learnIPAddressThread':
nwfilter/nwfilter_learnipaddr.c:501:43: warning: cast increases required alignment of target type [-Wcast-align]
struct iphdr *iphdr = (struct iphdr*)(packet +
nwfilter/nwfilter_learnipaddr.c:538:43: warning: cast increases required alignment of target type [-Wcast-align]
struct iphdr *iphdr = (struct iphdr*)(packet +
nwfilter/nwfilter_learnipaddr.c:544:48: warning: cast increases required alignment of target type [-Wcast-align]
struct udphdr *udphdr= (struct udphdr *)
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Gcc lets you do:
int ATTRIBUTE_NONNULL(1) foo(void *param);
int foo(void *param) ATTRIBUTE_NONNULL(1);
int ATTRIBUTE_NONNULL(1) foo(void *param) { ... }
but chokes on:
int foo(void *param) ATTRIBUTE_NONNULL(1) { ... }
However, since commit eefb881, we have intentionally been disabling
ATTRIBUTE_NONNULL because of lame gcc handling of the attribute (that
is, gcc doesn't do decent warning reporting, then compiles code that
mysteriously fails if you break the contract of the attribute, which
is surprisingly easy to do), leaving it on only for Coverity (which
does a much better job of improved static analysis when the attribute
is present).
But completely eliding the macro makes it too easy to write code that
uses the fourth syntax option, if you aren't using Coverity. So this
patch forces us to avoid syntax errors, even when not using the
attribute under gcc. It also documents WHY we disable the warning
under gcc, rather than forcing you to find the commit log.
* src/internal.h (ATTRIBUTE_NONNULL): Expand to empty attribute,
rather than nothing, when on gcc.
This patch introduces support for LXC specific public APIs. In
common with what was done for QEMU, this creates a libvirt_lxc.so
library and libvirt/libvirt-lxc.h header file.
The actual APIs are
int virDomainLxcOpenNamespace(virDomainPtr domain,
int **fdlist,
unsigned int flags);
int virDomainLxcEnterNamespace(virDomainPtr domain,
unsigned int nfdlist,
int *fdlist,
unsigned int *noldfdlist,
int **oldfdlist,
unsigned int flags);
which provide a way to use the setns() system call to move the
calling process into the container's namespace. It is not
practical to write in a generically applicable manner. The
nearest that we could get to such an API would be an API which
allows to pass a command + argv to be executed inside a
container. Even if we had such a generic API, this LXC specific
API is still useful, because it allows the caller to maintain
the current process context, in particular any I/O streams they
have open.
NB the virDomainLxcEnterNamespace() API is special in that it
runs client side, so does not involve the internal driver API.
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>
The 'const char *category' parameter only has a few possible
values now that the filename has been separated. Turn this
parameter into an enum instead.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently the logging APIs have a 'const char *category' parameter
which indicates where the log message comes from. This is typically
a combination of the __FILE__ string and other prefix. Split the
__FILE__ off into a dedicated parameter so it can passed to the
log outputs
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
FreeBSD and OpenBSD have a <net/if.h> that is not self-contained;
and mingw lacks the header altogether. But gnulib has just taken
care of that for us, so we might as well simplify our code. In
the process, I got a syntax-check failure if we don't also take
the gnulib execinfo module.
* .gnulib: Update to latest, for execinfo and net_if.
* bootstrap.conf (gnulib_modules): Add execinfo and net_if modules.
* configure.ac: Let gnulib check for headers. Simplify check for
'struct ifreq', while also including enough prereq headers.
* src/internal.h (IF_NAMESIZE): Drop, now that gnulib guarantees it.
* src/nwfilter/nwfilter_learnipaddr.h: Use correct header for
IF_NAMESIZE.
* src/util/virnetdev.c (includes): Assume <net/if.h> exists.
* src/util/virnetdevbridge.c (includes): Likewise.
* src/util/virnetdevtap.c (includes): Likewise.
* src/util/logging.c (includes): Assume <execinfo.h> exists.
(virLogStackTraceToFd): Handle gnulib's fallback implementation.
OpenBSD ships with gcc 4.2.1, which annoyingly treats all format
strings as though they were also attribute((nonnull)). The two
concepts are orthogonal, though, as evidenced by the number of
spurious warnings it generates on uses where we know that
virReportError specifically handles NULL instead of a format
string; worse, since we now force -Werror on git builds, it
prevents development builds on OpenBSD.
I hate to do this, as it disables ALL format checking on older
gcc, and therefore misses out on some useful checks (code that
happened to compile on Linux may still have type mismatches
when compiled on other platforms, as evidenced by the number
of times I have fixed formatting mismatches for uid_t as found
by warnings on Cygwin), but I don't see any other way to keep
-Werror alive and still compile on OpenBSD.
A more invasive change would be to make virReportError() mark
its format attribute as nonnull, and fix (a lot of) fallout;
we may end up doing that anyways as part of danpb's error
refactoring improvements, but not today.
* src/internal.h (ATTRIBUTE_FMT_PRINTF): Use preferred spellings.
* m4/virt-compile-warnings.m4 (-Wformat): Disable on older gcc.
Our existing STRNEQ_NULLABLE() triggered a warning in gcc 4.7 when
used with a literal NULL argument:
qemumonitorjsontest.c: In function 'testQemuMonitorJSONGetMachines':
qemumonitorjsontest.c:289:5: error: null argument where non-null required (argument 1) [-Werror=nonnull]
even though the strcmp is provably dead when a null argument is
present. Squelch the warning by refactoring things so that gcc
never sees strcmp() called with NULL arguments (we still compare
NULL as not equal to "", this rewrite merely aids gcc).
Next, gcc has a valid warning about a literal NULLSTR(NULL):
qemumonitorjsontest.c:289:5: error: invalid application of 'sizeof' to a void type [-Werror=pointer-arith]
Of course, you'd never write NULLSTR(NULL) directly, but it is
handy to use through macros. But the entire part about verify_true()
is unnecessary - gcc already warns about type mismatch with ?:,
without needing to make it more complex.
* src/internal.h (STREQ_NULLABLE, STRNEQ_NULLABLE): Avoid gcc 4.7
stupidity.
(NULLSTR): Simplify, to allow passing compile-time constants.
Otherwise, in locations like virobject.c where PROBE is used,
for certain configure options, the compiler warns:
util/virobject.c:110:1: error: 'intptr_t' undeclared (first use in this function)
As long as we are making this header always available, we can
clean up several other files.
* src/internal.h (includes): Pull in <stdint.h>.
* src/conf/nwfilter_conf.h: Rely on internal.h.
* src/storage/storage_backend.c: Likewise.
* src/storage/storage_backend.h: Likewise.
* src/util/cgroup.c: Likewise.
* src/util/sexpr.h: Likewise.
* src/util/virhashcode.h: Likewise.
* src/util/virnetdevvportprofile.h: Likewise.
* src/util/virnetlink.h: Likewise.
* src/util/virrandom.h: Likewise.
* src/vbox/vbox_driver.c: Likewise.
* src/xenapi/xenapi_driver.c: Likewise.
* src/xenapi/xenapi_utils.c: Likewise.
* src/xenapi/xenapi_utils.h: Likewise.
* src/xenxs/xenxs_private.h: Likewise.
* tests/storagebackendsheepdogtest.c: Likewise.
Macro virCheckNullArgGoto is supposed to check for NULL argument but
checks non-NULL instead.
Macro virCheckNonNullArgReturn reports error as if the argument should
be NULL when it shouldn't.
