When virDBusMessageRead() and virDBusMessageDecode were first added in
commit 834c9c94, they were identical except that virDBusMessageRead()
would unref the message after decoding it.
This difference was eliminated later in commit dc7f3ffc after it
became apparent that unref-ing the message so soon was never the right
thing to do. The two identical functions remained though, with the
tests and virDBus library itself calling the Decode variant, and all
other users calling the Read variant.
This patch eliminates the duplication, switching all users to
virDBusMessageDecode (and moving the nice API documentation comment
from the Read function up to the Decode function).
Signed-off-by: Laine Stump <laine@laine.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
We have this very handy macro called VIR_STEAL_PTR() which steals
one pointer into the other and sets the other to NULL. The
following coccinelle patch was used to create this commit:
@ rule1 @
identifier a, b;
@@
- b = a;
...
- a = NULL;
+ VIR_STEAL_PTR(b, a);
Some places were clean up afterwards to make syntax-check happy
(e.g. some curly braces were removed where the body become a one
liner).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@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>
Use the mnemonic macros of libdbus for 1 (TRUE) and 0 (FALSE).
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Report a debug message if dbus_watch_handle() returns FALSE.
dbus_watch_handle() returns FALSE if there wasn't enough memory for
reading or writing.
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
As documented at
https://dbus.freedesktop.org/doc/api/html/group__DBusConnection.html#ga2522ac5075dfe0a1535471f6e045e1ee
the creator of a non-shared D-Bus connection has to release the last
reference after closing for freeing.
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Grab a ref for info->bus (a DBus connection) as long as the while loop
is running. With the grabbed reference it is ensured that info->bus
isn't freed as long as the while loop is executed. This is necessary
as it's allowed to drop the last ref for the bus connection in a
handler.
There was already a bug of this kind in libdbus itself:
https://bugs.freedesktop.org/show_bug.cgi?id=15635.
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: John Ferlan <jferlan@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>
Apparently we are not the only ones with dumb free functions
because dbus_message_unref() does not accept NULL either. But if
I were to vote, this one is even more evil. Instead of returning
an error just like we do it immediately dereference any pointer
passed and thus crash you app. Well done DBus!
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7f878ebda700 (LWP 31264)]
0x00007f87be4016e5 in ?? () from /usr/lib64/libdbus-1.so.3
(gdb) bt
#0 0x00007f87be4016e5 in ?? () from /usr/lib64/libdbus-1.so.3
#1 0x00007f87be3f004e in dbus_message_unref () from /usr/lib64/libdbus-1.so.3
#2 0x00007f87bf6ecf95 in virSystemdGetMachineNameByPID (pid=9849) at util/virsystemd.c:228
#3 0x00007f879761bd4d in qemuConnectCgroup (driver=0x7f87600a32a0, vm=0x7f87600c7550) at qemu/qemu_cgroup.c:909
#4 0x00007f87976386b7 in qemuProcessReconnect (opaque=0x7f87600db840) at qemu/qemu_process.c:3386
#5 0x00007f87bf6edfff in virThreadHelper (data=0x7f87600d5580) at util/virthread.c:206
#6 0x00007f87bb602334 in start_thread (arg=0x7f878ebda700) at pthread_create.c:333
#7 0x00007f87bb3481bd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
(gdb) frame 2
#2 0x00007f87bf6ecf95 in virSystemdGetMachineNameByPID (pid=9849) at util/virsystemd.c:228
228 dbus_message_unref(reply);
(gdb) p reply
$1 = (DBusMessage *) 0x0
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Coverity claims it could be possible to call virDBusTypeStackFree with
*stack == NULL and although the two API's that call it don't appear to
allow that - I suppose it's better to be safe than sorry
The virDBusMethodCall method has a DBusError as one of its
parameters. If the caller wants to pass a non-NULL value
for this, it immediately makes the calling code require
DBus at build time. This has led to breakage of non-DBus
builds several times. It is desirable that only the virdbus.c
file should need WITH_DBUS conditionals, so we must ideally
remove the DBusError parameter from the method.
We can't simply raise a libvirt error, since the whole point
of this parameter is to give the callers a way to check if
the error is one they want to ignore, without having the logs
polluted with an error message. So, we add a virErrorPtr
parameter which the caller can then either ignore or raise
using the new virReportErrorObject method.
This new method is distinct from virSetError in that it
ensures the logging hooks are run.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When debugging libvirt it is helpful to set probes around RPC
calls. We already have probes for libvirt's native RPC layer,
so it makes sense to add them for the DBus RPC layer too.
Commit 2aa167ca tried to fix the DBus interaction code to allow
callers to use native types instead of 4-byte bools. But in
fixing the issue, I missed the case of an arrayref; Conrad Meyer
shows the following valid complaint issued by clang:
CC util/libvirt_util_la-virdbus.lo
util/virdbus.c:956:13: error: cast from 'bool *' to 'dbus_bool_t *' (aka 'unsigned int *') increases required alignment from 1 to 4 [-Werror,-Wcast-align]
GET_NEXT_VAL(dbus_bool_t, bool_val, bool, "%d");
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
util/virdbus.c:858:17: note: expanded from macro 'GET_NEXT_VAL'
x = (dbustype *)(*xptrptr + (*narrayptr - 1)); \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1 error generated.
But fixing that points out that we have NEVER supported arrayrefs
of sub-int types (byte, i16, u16, and now bool). Again, while raw
types promote, arrays do not; so the macros HAVE to deal with both
size possibilities rather than assuming that an arrayref uses the
same sizing as the promoted raw type.
Obviously, our testsuite wasn't covering as much as it should have.
* src/util/virdbus.c (GET_NEXT_VAL): Also fix array cases.
(SET_NEXT_VAL): Fix uses of sub-int arrays.
* tests/virdbustest.c (testMessageArray, testMessageArrayRef):
Test it.
Signed-off-by: Eric Blake <eblake@redhat.com>
Use of an 'int' to represent a 'bool' value is confusing. Just
because dbus made the mistake of cementing their 4-byte wire
format of dbus_bool_t into their API doesn't mean we have to
repeat the mistake. With a little bit of finesse, we can
guarantee that we provide a large-enough value to the DBus
code, while still copying only the relevant one-byte bool
to the client code, and isolate the rest of our code base from
the DBus stupidity.
* src/util/virdbus.c (GET_NEXT_VAL): Add parameter.
(virDBusMessageIterDecode): Adjust all clients.
* src/util/virpolkit.c (virPolkitCheckAuth): Use nicer type.
* tests/virdbustest.c (testMessageSimple, testMessageStruct):
Test new behavior.
Signed-off-by: Eric Blake <eblake@redhat.com>
If you trigger bug 1033369, we get the error message:
error from service: Invalid argument
Which is a bit too generic to pinpoint what is actually failing. This
changes it to:
error from service: CreateMachine: Invalid argument
Acked-by: Eric Blake <eblake@redhat.com>
This is the only callsite.
We drop use of localerror.name here, because it's not actually useful
to us: rather than the parameter name which received an invalid value
(which was assumed), it's actually the the dbus errno equivalent.
Just use the error string.
Acked-by: Eric Blake <eblake@redhat.com>
Commit 4897698 fixed the build without dbus by only building
the virSystemdPMSupportTarget with SYSTEMD_DAEMON.
Introduce a virDBusMessageUnref wrapper for dbus_message_unref
to let virsystemd.c build without dbus, while still allowing
virsystemdtest to run without SYSTEM_DAEMON.
The caller may not want all DBus error conditions to be turned
into libvirt errors, so provide a way for the caller to get
back the full DBusError object. They can then check the errors
and only report those that they consider to be fatal.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently the DBus helper APIs require the values for an array
to be passed inline in the variadic argument list. This change
introduces support for passing arrays using a pointer to a plain
C array of the basic type. This is of particular benefit for
decoding messages when you don't know how many array elements
are being received.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The dbus_connection_send_with_reply_and_block method will
automatically call dbus_set_error_from_message for us. We
mistakenly thought we had todo it because of a flaw in the
systemd unit test mock impl. The latter should have directly
set the error object, instead of creating an error message
object.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virDBusMessageRead method should not have side-effects on
the message parameter passed in, so unref'ing it is wrong.
The caller should unref only when they decided they are done
with it.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The test suites often have to create DBus method reply messages
with payloads. Create two helpers for simplifying the process
of creating replies with payloads.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Split the virDBusMethodCall method into a couple of new methods
virDBusCall, virDBusCreateMethod and virDBusCreateMethodV.
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>
If systemd is installed, but is not the init system,
systemd-machined fails with an unhelpful error message:
Launch helper exited with unknown return code 1
Currently we only check if the "machine1" service is
available (in ListActivatableNames).
Also check if "systemd1" service is registered with DBus
(ListNames).
This fixes https://bugs.gentoo.org/show_bug.cgi?id=493246#c22
Introduce virDBusIsServiceInList which can be used to call other
methods for listing services (ListNames), not just ListActivatableNames.
No functional change, fixed the 'Retruns' typo.
Currently the virDBusAddWatch does
virEventAddHandle(fd, flags,
virDBusWatchCallback,
watch, NULL);
dbus_watch_set_data(watch, info, virDBusWatchFree);
Unfortunately this is racy - since the event loop is in a
different thread, the virDBusWatchCallback method may be
run before we get to calling dbus_watch_set_data. We must
reverse the order of these calls
See https://bugzilla.redhat.com/show_bug.cgi?id=885445
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Previous commit
commit 7ada155cdf
Author: Gao feng <gaofeng@cn.fujitsu.com>
Date: Wed Sep 11 11:15:02 2013 +0800
DBus: introduce virDBusIsServiceEnabled
Made the cgroups code fallback to non-systemd based setup
when dbus is not running. It was too big a hammer though,
as it did not check what error code was received when the
dbus connection failed. Thus it silently ignored serious
errors from dbus such as "too many client connections",
which should always be treated as fatal.
We only want to ignore errors if the dbus unix socket does
not exist, or if nothing is listening on it.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The dbus_bus_get() function returns a shared bus connection that
all libraries in a process can use. You are forbidden from calling
close on this connection though, since you can never know if any
other code might be using it.
Add an option to use private dbus bus connections, if the app
wants to be able to close the connection.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
If virDBusMessageIterEncode hits an OOM condition it often
leaks the memory associated with the dbus iterator object
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This patch introduces virDBusIsServiceEnabled, we can use
this method to get if the service is supported.
In one case, if org.freedesktop.machine1 is unavailable on
host, we should skip creating machine through systemd.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
Some systems may not use DBus in their system. Add a method to check if
the system bus is available that doesn't print error messages so that
code can later check for this condition and use an alternative approach.
The way we were casting small (<32bit) integers was broken
on big endian hosts, causing stack smashing. This was detected
in the test suite either by test failures due to incorrect
results, or by libc/gcc abort'ing with its stack canary
triggered.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
dbus 1.2.24 (on RHEL 6) lacks DBUS_TYPE_UNIX_FD; but as we aren't
trying to pass one of those anyways, we can just drop support for
it in our wrapper. Solves this build error introduced in commit
834c9c94:
CC libvirt_util_la-virdbus.lo
util/virdbus.c:242: error: 'DBUS_TYPE_UNIX_FD' undeclared here (not in a function)
* src/util/virdbus.c (virDBusBasicTypes): Drop support for unix fds.
Signed-off-by: Eric Blake <eblake@redhat.com>
Current code for handling dbus errors only works for errors
received from the remote application itself. We must also
handle errors emitted by the bus itself, for example, when
it fails to spawn the target service.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
