VIR_CLOSE() sets errno on failure so it's better to use
virReportSystemError() than plain virReportError() as the former
reports errno value too.
Signed-off-by: Adam Julis <ajulis@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
After previous cleanups, virSocketSendFD() is but a thin wrapper
over virSocketSendMsgWithFDs(). Replace the body of the former
with a call to the latter.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Convert prototype of virFileLinkPointsTo to return bool.
Remove dead checks in virDomainObjListLoadConfig and
virNetworkLoadConfig.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Anastasia Belova <abelova@astralinux.ru>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
While glibc provides qsort(), which usually is just a mergesort,
until sorting arrays so huge that temporary array used by
mergesort would not fit into physical memory (which in our case
is never), we are not guaranteed it'll use mergesort. The
advantage of mergesort is clear - it's stable. IOW, if we have an
array of values parsed from XML, qsort() it and produce some
output based on those values, we can then compare the output with
some expected output, line by line.
But with newer glibc this is all history. After [1], qsort() is
no longer mergesort but introsort instead, which is not stable.
This is suboptimal, because in some cases we want to preserve
order of equal items. For instance, in ebiptablesApplyNewRules(),
nwfilter rules are sorted by their priority. But if two rules
have the same priority, we want to keep them in the order they
appear in the XML. Since it's hard/needless work to identify
places where stable or unstable sorting is needed, let's just
play it safe and use stable sorting everywhere.
Fortunately, glib provides g_qsort_with_data() which indeed
implement mergesort and it's a drop in replacement for qsort(),
almost. It accepts fifth argument (pointer to opaque data), that
is passed to comparator function, which then accepts three
arguments.
We have to keep one occurance of qsort() though - in NSS module
which deliberately does not link with glib.
1: https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=03bf8357e8291857a435afcc3048e0b697b6cc04
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
After previous cleanup the virBuildPathInternal() function is no
longer used. Drop it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Error messages are exempt from the 80 columns rule. Move them
onto one line.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
It is handy to close all FDs from given FD to infinity. On
FreeBSD the libc even has a function for that: closefrom(). It
was ported to glibc too, but not musl. At least glibc
implementation falls back to calling:
close_range(from, ~0U, 0);
Now that we have a wrapper for close_range() we implement
closefrom() trivially.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Linux gained new close_range() syscall (in v5.9) that allows
closing a range of FDs in a single syscall. Ideally, we would use
it to close FDs when spawning a process (e.g. via virCommand
module).
Glibc has close_range() wrapper over the syscall, which falls
back to iterative closing of all FDs inside the range if running
under older kernel. We don't wane that as in that case we might
just close opened FDs (see Linux version of
virCommandMassClose()). And musl doesn't have close_range() at
all. Therefore, call syscall directly.
Now, mass close of FDs happens in a fork()-ed off child. While it
could detect whether the kernel does support close_range(), it
has no way of passing this info back to the parent and thus each
child would need to query it again and again.
Since this can't change while we are running we can cache the
information - hence virCloseRangeInit().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
This is a more concise approach and guarantees there is
no time window where the struct is uninitialized.
Generated using the following semantic patch:
@@
type T;
identifier X;
@@
- T X;
+ T X = { 0 };
... when exists
(
- memset(&X, 0, sizeof(X));
|
- memset(&X, 0, sizeof(T));
)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Claudio Fontana <cfontana@suse.de>
There might be cases where we want to know whether given
directory is empty or not. Introduce a helper for that:
virDirIsEmpty().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now that a version of GLib that contains the fix has been
released, it's more useful to record that information. Adding
a TODO annotation makes the whole thing easily greppable.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The linux/magic.h header has existed since
commit e18fa700c9a31360bc8f193aa543b7ef7b39a06b
Author: Jeff Garzik <jeff@garzik.org>
Date: Sun Sep 24 11:13:19 2006 -0400
Move several *_SUPER_MAGIC symbols to include/linux/magic.h.
This is old enough that all our supported platforms can be assumed
to have this header.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The LO_FLAGS_AUTOCLEAR constant was introduced to Linux in
commit 96c5865559cee0f9cbc5173f3c949f6ce3525581
Author: David Woodhouse <dwmw2@infradead.org>
Date: Wed Feb 6 01:36:27 2008 -0800
Allow auto-destruction of loop devices
This is old enough that all our supported platforms can be assumed
to have this feature. For added fun this whole meson check was
semantically insane because EPOLL_CLOEXEC is not a valid arg
to unshare().
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The LOOP_CTL_GET_FREE constant was introduced to Linux in
commit 770fe30a46a12b6fb6b63fbe1737654d28e84844
Author: Kay Sievers <kay.sievers@vrfy.org>
Date: Sun Jul 31 22:08:04 2011 +0200
loop: add management interface for on-demand device allocation
This is old enough that all our supported platforms can be assumed
to have this feature. As a plus point, this meson check is going
to start failing with future GCC. It fails to set _GNU_SOURCE, thus
'unshare' is not defined by the header, and its relying on an
implicit function decl. For added fun this whole meson check was
semantically insane because LOOP_CTL_GET_FREE is not a valid arg
to unshare().
Fixes https://fedoraproject.org/wiki/Toolchain/PortingToModernC
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Use approach similar to virFileIsSharedFsFUSE to declaratively handle
the filesystem magic numbers mapping to libvirt's fstypes.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The filesystem type magic constant was added for the 'quobyte' shared
filesystem in commit 451094bd153 but is present neither in the kernel
sources nor in coreutils which we've historically used as source of
information.
Since the code dealing with FUSE-based filesystems doesn't need the
constant we can remove it and the now-dead check for it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
While the code works properly as no code path is specifically wanting to
check for glusterfs, we should properly declare glusterfs as a separate
from GFS2.
Fixes: 478da65fb46
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'virFileIsSharedFixFUSE' was used to update the 'f_type' field for
certain shared filesystem types.
This patch renames it to 'virFileIsSharedFsFUSE' and makes it directly
return whether the FUSE filesystem is shared or not and simplifies
additions to the list of shared FUSE filesystems.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Automatically free memory of 'canonPath' so that the failure of
'setmntent' doesn't have to go to 'cleanup'. This allows us to remove
the cleanup section and the 'ret' variable as the rest of the function
can't fail.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We populate the bits individually so unsigned is the proper type.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
where it can be reused by other helpers.
No changes other than the move.
Note that this makes iohelper now dependent on -lutil and -lacl.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Claudio Fontana <cfontana@suse.de>
When changing the size of pipe that virFileWrapperFdNew() creates
we start at 1MiB and if that fails because it's above the system
wide limit we get EPERM and continue with half of the size.
However, we might get another error in which case we should
report proper system error and return failure from
virFileWrapperFdNew().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
currently the only user of virFileWrapperFdNew is the qemu driver;
virsh save is very slow with a default pipe size.
This change improves throughput by ~400% on fast nvme or ramdisk.
Best value currently measured is 1MB, which happens to be also
the kernel default for the pipe-max-size.
Signed-off-by: Claudio Fontana <cfontana@suse.de>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Adding an exception for the whole file usually defeats the purpose of a
syntax check and is also likely to get forgotten once the file is
removed.
In case of the suggestion of using 'safewrite' instead of write even the
comment for safewrite states that the function needs to be used only in
certain cases.
Remove the blanket exceptions for files and use an exclude string
instead. The only instance where we keep the full file exception is for
src/libvirt-stream.c as there are multiple uses in example code in
comments where I couldn't find a nicer targetted wapproach.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When rewriting a file using virFileRewrite() and error occurs
while writing into a temporary file it's actually the callback
that can report the most accurate error. Move error reporting
into very few callback we have currently. Those callbacks are
trivial so the benefit of this change is not obvious, but this
will change shortly when slightly more complicated callback is
introduced.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Currently, due to the way virFileRewrite() works, the rewritten
file is owned by user and group that the daemon runs under. So
far, this is not a problem, because the function is used to write
XML files or secrets for persistent objects (domains, networks,
etc.) and we don't need other users to read/write those files.
But shortly, this function is going to be used for creating files
for QEMU domains. There we want the QEMU process (i.e. different
user) to read the file.
Therefore, introduce two new arguments: @uid and @gid that allow
setting desired owner of the file. Pass -1 to preserve current
behaviour (i.e. create the file owned by the user running the
daemon).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This change was generated using the following spatch:
@ rule1 @
expression a;
identifier f;
@@
<...
- f(*a);
... when != a;
- *a = NULL;
+ g_clear_pointer(a, f);
...>
@ rule2 @
expression a;
identifier f;
@@
<...
- f(a);
... when != a;
- a = NULL;
+ g_clear_pointer(&a, f);
...>
Then, I left some of the changes out, like tools/nss/ (which
doesn't link with glib) and put back a comment in
qemuBlockJobProcessEventCompletedActiveCommit() which coccinelle
decided to remove (I have no idea why).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are few places where the g_steal_pointer() is open coded.
Switch them to calling the g_steal_pointer() function instead.
Generated by the following spatch:
@ rule1 @
expression a, b;
@@
<...
- b = a;
... when != b
- a = NULL;
+ b = g_steal_pointer(&a);
...>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Use function virFileReadValueUllongQuiet to read unsigned long
long value without error report.
Signed-off-by: Yang Fei <yangfei85@huawei.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If the function is called with maxlen equal to `INT_MAX`, adding
one will trigger a signed integer overflow.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
We have an example in virDirRead() documentation on how to use
the function. In there, the directory structure is plain DIR, but
that won't work anymore. Switch over to g_autoptr(DIR) which is
what we use now.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
If given file is not found in $PATH then g_find_program_in_path()
returns NULL. However, g_canonicalize_filename() does not accept
NULL as input.
Fixes: 65c2901906b5e2d707f9184ff3719a5375bab427
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The g_build_filename() would decide which separator
to use instead of hardcoding in g_strdup_printf().
Related issue: https://gitlab.com/libvirt/libvirt/-/issues/12
Signed-off-by: Luke Yue <lukedyue@gmail.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
libacl is Linux-only, so we don't need to explicitly check for
either the target platform or header availability, and we can
simply rely on cc.find_library() instead. The corresponding
preprocessor define is renamed to more accurately reflect the
nature of the check.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Historically, we declared pointer type to our types:
typedef struct _virXXX virXXX;
typedef virXXX *virXXXPtr;
But usefulness of such declaration is questionable, at best.
Unfortunately, we can't drop every such declaration - we have to
carry some over, because they are part of public API (e.g.
virDomainPtr). But for internal types - we can do drop them and
use what every other C project uses 'virXXX *'.
This change was generated by a very ugly shell script that
generated sed script which was then called over each file in the
repository. For the shell script refer to the cover letter:
https://listman.redhat.com/archives/libvir-list/2021-March/msg00537.html
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The g_path_is_absolute() considers more situations
than just a simply "path[0] == '/'".
Related issue: https://gitlab.com/libvirt/libvirt/-/issues/12
Signed-off-by: Luke Yue <lukedyue@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
