Break these checks out into their own function, and clearly document
each one. This shouldn't change behavior

VIR_DEBUG and VIR_WARN will automatically add a new line to the message,
having "\n" at the end or at the beginning of the message results in
empty lines.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

We have macros for both positive and negative string matching.
Therefore there is no need to use !STREQ or !STRNEQ. At the same
time as we are dropping this, new syntax-check rule is
introduced to make sure we won't introduce it again.
Signed-off-by: NIshmanpreet Kaur Khera <khera.ishman@gmail.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

After a successful creation of a directory, if some other call results
in returning a failure, let's remove the directory we created to
prevent another round trip or confusion in the caller. In particular, this
function can be called during a storage backend buildVol, so in order
to ensure that caller doesn't need to distinguish between failed create
or some other failure after create, just remove the directory we created.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

After a successful creation of a file, if some other call results
in returning a failure, let's unlink the file we created to prevent
another round trip or confusion in the caller. In particular, this
function can be called during a storage backend buildVol, so in order
to ensure that caller doesn't need to distinguish between failed create
or some other failure after create, just remove the volume we created.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

As it turns out the caller in this case expects a return < 0 for failure
and to get/use "errno" rather than using the negative of returned status.
Again different than the create path.

If someone "deleted" a file from the pool without using virsh vol-delete,
then the unlink/rmdir would return an error (-1) and set errno to ENOENT.
The caller checks errno for ENOENT when determining whether to throw an
error message indicating the failure.  Without the change, the error
message is:

error: Failed to delete vol $vol
error: cannot unlink file '/$pathto/$vol': Success

This patch thus allows the fork path to follow the non-fork path
where unlink/rmdir return -1 and errno.

Unlike create options, if the file to be removed is already in the
pool, then the uid/gid will come from the pool. If it's the same as the
currently running process, then just do the unlink/rmdir directly
rather than going through the fork processing unnecessarily

Rather than inlining the code, use the common API.

Rather than inlining the code, use the common API.

Rather than inlining the code, use the common API

Similar to commit id '35847860', it's possible to attempt to create
a 'netfs' directory in an NFS root-squash environment which will cause
the 'vol-delete' command to fail.  It's also possible error paths from
the 'vol-create' would result in an error to remove a created directory
if the permissions were incorrect (and disallowed root access).

Thus rename the virFileUnlink to be virFileRemove to match the C API
functionality, adjust the code to following using rmdir or unlink
depending on the path type, and then use/call it for the VIR_STORAGE_VOL_DIR

Commit id 'f1f68ca3' added code to remove the directory paths for
auto-generated sockets, but that code could be called before the
paths were created resulting in generating error messages from
virFileDeleteTree indicating that the file doesn't exist.

Rather than "enforce" all callers to make the non-NULL and existence
checks, modify the virFileDeleteTree API to silently ignore NULL on
input and non-existent directory trees.

Commit 35847860 Added the virFileUnlink function, but failed to add
a version for mingw build, causing the following error:

  Cannot export virFileUnlink: symbol not defined
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

In virFileNBDDeviceFindUnused if virFileNBDDeviceIsBusy returns 0,
then both branches jumped to cleanup, so just use ignore_value
since the function returns NULL or some memory and the caller
handles the error.

In an NFS root-squashed environment the 'vol-delete' command will fail to
'unlink' the target volume since it was created under a different uid:gid.

This code continues the concepts introduced in virFileOpenForked and
virDirCreate[NoFork] with respect to running the unlink command under
the uid/gid of the child. Unlike the other two, don't retry on EACCES
(that's why we're here doing this now).

This will only be seen when debugging, but in order to help determine
whether a virFileOpenForceOwnerMode failed during an NFS root-squash
volume/file creation, add an error message from the child.

So far qemu-nbd is run even if the nbd kernel module isn't loaded. This
leads to errors when the user starts his lxc container while libvirt
could easily load the nbd module automatically.

This happens if user requires creation of a directory with specified
UID/GID permissions. To accomplish this, we use fork approach and
set particular UID/GID permissions in child process. However, child
process doesn't have a valid descriptor to a logfile (this is prohibited
explicitly) and since parent process doesn't handle negative exit codes from
child in any way, 'uknown cause' error is returned to the user.

Commit 92d9114e tweaked the way we handle child errors when using fork
approach to set specific permissions (features originally introduced
by 98f6f381). The same logic should be used to create directories with
specified permissions as well.

https://bugzilla.redhat.com/show_bug.cgi?id=1230137

Previous patch of this series proposed a fix to virDirCreate, so that parent
process reports an error if child process failed its task.
However our logic still permits the child to exit with negative errno followed
by a check of the status on the parent side using WEXITSTATUS which, being
POSIX compliant, takes the lower 8 bits of the exit code and returns is to
the caller. However, by taking 8 bits from a negative exit code
(two's complement) the status value we read and append to stream is
'2^8 - abs(original exit code)' which doesn't quite reflect the real cause when
compared to the meaning of errno values.

Only set directory permissions at pool build time, if:

- User explicitly requested a mode via the XML
- The directory needs to be created
- We need to do the crazy NFS root-squash workaround

This allows qemu:///session to call build on an existing directory
like /tmp.

The only two virDirCreate callers already use it

I screwed this up in the previous (post 1.2.16) commits

Currently we try to chown any directory passed to virDirCreate,
even if the user didn't request any explicit owner/group via the
pool/vol XML.

This causes issues with qemu:///session: try to build a pool of
a root owned directory like /tmp, and it fails trying to chown the
directory to the session user. Instead it should just leave things
as they are, unless the user requests changing permissions via
the pool XML.

Similarly this is annoying if creating a storage pool via system
libvirtd of an existing directory in user $HOME, it's now owned
by root.

The virDirCreate function is pretty convoluted, since it needs to
fork off in certain specific cases. Try to document that, to make
it clear where exactly we are changing behavior.

The current code attempts to handle this, but it only catches mkdir
failing with EEXIST. However if say trying to build /tmp for an
unprivileged qemu:///session, mkdir will fail with EPERM.

Rather than catch any errors, just don't attempt mkdir if the directory
already exists.

rfc3986 states that the separator in URI path is a single slash.
Multiple slashes may potentially lead to different resources and thus we
should not remove them.

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: NJiri Denemark <jdenemar@redhat.com>

Rather than have a dummy waitpid loop and return of the failure status
from recvfd, adjust the logic to save the recvfd error & fd and then
in priority order:

- if waitpid failed, use that errno value
- waitpid succeeded, but if the child exited abnormally, report failure
(use EACCES to report as return failure, since either EACCES or EPERM is
what caused us to fall into the fork+setuid path)
- waitpid succeeded, but if the child reported non-zero status, report
failure (use the errno value that the child encoded into exit status)
- waitpid succeeded, but if recvfd failed, report recvfd_errno
- waitpid and recvfd succeeded, use the fd

NOTE: Original logic to retry the open and force owner mode was
"documented" as only being attempted if we had already tried opening
with the fork+setuid, but checked flags vs. VIR_FILE_OPEN_NOFORK which
is counter to how we would get to that point. So that code was removed.

Some code paths have special logic depending on the page size
reported by sysconf, which in turn affects the test results.
We must mock this so tests always have a consistent page size.

A gnulib change (commit id 'beae0bdc') causes ENOTCONN to be returned
from recvfd which causes us to fall into the throwaway waitpid() call
and return ENOTCONN to the caller, this then gets displayed during
a 'virsh save' when using a root squashed NFS environment that's trying
to save the file as something other than root:root.

This patch will add the additional check for ENOTCONN to force the code
into the waitpid loop looking for the actual status from the _exit()'d
child fork.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Remove the resize flag and use the same code path for all callers.
This flag was added by commit 18f03166 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 18f03166.

* src/util/virfile.c (safezero_mmap): Fix missing semicolon.
Signed-off-by: NEric Blake <eblake@redhat.com>

When any of the functions modified in commit 214c687b took false branch,
the function itself used none of its parameters resulting in "unused
parameter" error.  Rewriting these functions to the stubs we use
elsewhere should fix the problem.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

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: NJohn Ferlan <jferlan@redhat.com>

Currently build conditionals decide which of two safezero() functions
should be built - either the posix_fallocate() or mmap() with a fallback
to a slower safewrite() algorithm in order to preallocate space in a raw file.

This patch will refactor safezero to utilize static functions for either
posix_fallocate or mmap/safewrite. The build conditional still exist, but
are only for shorter sections of code.

The posix_fallocate path will make use of the ret/errno setting to contain
the logic for safezero to decide whether it needs to fallback to other
algorithms. A return of -1 with errno not changed will indicate the conditional
is not present; otherwise, a return of -1 with errno change indicates the
call was made and it failed (no functional difference to current algorithm).

The mmap/safewrite option changes only slightly to handle the ftruncate
failure for mmap. That is, previously if the ftruncate failed, there was
no fallback to the slow safewrite option.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

C guarantees that static variables are zero-initialized.  Some older
compilers (and also gcc -fno-zero-initialized-in-bss) create larger
binaries if you explicitly zero-initialize a static variable.

* src/libvirt.c: Fix initialization.
* src/util/viralloc.c: Likewise.
* src/util/virdbus.c: Likewise.
* src/util/virevent.c: Likewise.
* src/util/virfile.c (safezero): Likewise.
* src/util/virlog.c: Likewise.
* src/util/virnetlink.c: Likewise.
* src/util/virthread.h (VIR_ONCE_GLOBAL_INIT): Likewise.
* src/util/virprocess.c (virProcessGetStartTime): Likewise.
Signed-off-by: NEric Blake <eblake@redhat.com>

and tweak the code to avoid using it.

With the virGetGroupList() change in place - Coverity further complains
that if we fail to virFork(), the groups will be leaked - which aha seems
to be the case. Adjust the logic to save off the -errno, free the groups,
and then return the value we saved
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>