When running a function in a forked child, so far the only thing
we could report is exit status of the child and the error
message. However, it may be beneficial to the caller to know the
actual error that happened in the child.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NPavel Mores <pmores@redhat.com>

Include virutil.h in all files that use it,
instead of relying on it being pulled in somehow.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NDaniel Henrique Barboza <danielhb413@gmail.com>

This addreses portability to Windows and standardizes
error reporting. This fixes a number of places which
failed to set O_CLOEXEC or failed to report errors.
Reviewed-by: NPavel Hrdina <phrdina@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Most code now uses the virProcess / virCommand APIs, so
the need for sys/wait.h is quite limited. Removing this
include removes the dependency on GNULIB providing a
dummy sys/wait.h for Windows.
Reviewed-by: NPavel Hrdina <phrdina@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

This effectively reverts

  commit 39c77fe5
  Author: Michal Prívozník <mprivozn@redhat.com>
  Date:   Wed Jan 16 11:58:00 2013 +0100

    Introduce event loop to commandtest

because nothing in the current test suite needs this
event loop.
Reviewed-by: NPavel Hrdina <phrdina@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Some UNIX platforms don't declare 'environ' in their
header files. We can unconditionally declare it ourselves
to avoid this problem.

There is no need to do this in the aa-helper code
since that is Linux only code.
Reviewed-by: NPavel Hrdina <phrdina@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel Henrique Barboza <danielhb413@gmail.com>

Replace all occurrences of
  if (VIR_STRDUP(a, b) < 0)
     /* effectively dead code */
with:
  a = g_strdup(b);
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Use G_GNUC_UNUSED from GLib instead of ATTRIBUTE_UNUSED.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

The usleep function was missing on older mingw versions, but we can rely
on it existing everywhere these days. It may only support times upto 1
second in duration though, so we'll prefer to use g_usleep instead.

The commandhelper program is not changed since that can't link to glib.
Fortunately it doesn't need to build on Windows platforms either.
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

g_strerror is offers the safety/correctness benefits of strerror_r, with
the API design convenience of strerror.

Use of virStrerror should be eliminated through the codebase in favour
of g_strerror.

commandhelper.c is a special case as its a tiny single threaded test
program, not linked to glib, so it just uses traditional strerror().
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Neither virThreadInitialize or virThreadOnExit do anything since we
dropped the Win32 threads impl, in favour of win-pthreads with:

  commit 0240d94c
  Author: Daniel P. Berrange <berrange@redhat.com>
  Date:   Wed Jan 22 16:17:10 2014 +0000

      Remove windows thread implementation in favour of pthreads
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Now that 100% of libvirt code is forbidden in a SUID environment,
we no longer need to worry about whether env variables are
trustworthy or not. The virt-login-shell setuid program, which
does not link to any libvirt code, will purge all environment
variables, except $TERM, before invoking the virt-login-shell-helper
program which uses libvirt.

Thus we only need one API for env passthrough in virCommand.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

The recently added test27 spawns commandhelper. This is fine,
except, one of the things that commandhelper does is it records
arguments it was spawn with into commandhelper.log. Other test
cases then use checkoutput() to compare the arguments against the
expected ones and also unlink() the log file. However, test27()
is not doing that and thus it leaves the file behind. This
breaks distcheck.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Fix a potential memory leak by calling virCommandFree() in the cleanup
section.
Signed-off-by: NStefan Berger <stefanb@linux.ibm.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
Message-Id: <20190726205633.2041912-3-stefanb@linux.vnet.ibm.com>

Add a test case to commandtest.c to test the transfer of data to a
process who received the read-end of pipes' file descriptors. Transfer
large (128 kb) byte streams.

Extend the commandhelper.c with support for --readfd <fd> command line
parameter and convert the data receive loop to use poll and receive data
on multiple file descriptors (up to 3) and read data into distinct buffers
that we grow while adding more (string) data.
Signed-off-by: NStefan Berger <stefanb@linux.ibm.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

The only use of this code was removed by:

  commit be78814a
  Author: Michal Privoznik <mprivozn@redhat.com>
  Date:   Thu Apr 2 14:41:17 2015 +0200

    virNetSocketNewConnectUNIX: Use flocks when spawning a daemon

less than a year after it was first introduced in

  commit 1b807f92
  Author: Martin Kletzander <mkletzan@redhat.com>
  Date:   Wed Jul 16 08:00:19 2014 +0200

    rpc: pass listen FD to the daemon being started
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

If an FD is passed into a child using:

  virCommandPassFD(cmd, fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);

then the parent should refrain from touching @fd thereafter. This
is even documented in virCommandPassFD() comment. The reason is
that either at virCommandRun()/virCommandRunAsync() or
virCommandFree() time the @fd will be closed. Closing it earlier,
e.g. right after virCommandPassFD() call might result in
undesired results. Another thread might open a file and receive
the same FD which is then unexpectedly closed by virCommandFree()
or virCommandRun().
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The QEMU command line arguments are very long and currently all written
on a single line to /var/log/libvirt/qemu/$GUEST.log. This introduces
logic to add line breaks after every env variable and "-" optional
argument, and every positional argument. This will create a clearer log
file, which will in turn present better in bug reports when people cut +
paste from the log into a bug comment.

An example log file entry now looks like this:

  2018-12-14 12:57:03.677+0000: starting up libvirt version: 5.0.0, qemu version: 3.0.0qemu-3.0.0-1.fc29, kernel: 4.19.5-300.fc29.x86_64, hostname: localhost.localdomain
  LC_ALL=C \
  PATH=/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin \
  HOME=/home/berrange \
  USER=berrange \
  LOGNAME=berrange \
  QEMU_AUDIO_DRV=none \
  /usr/bin/qemu-system-ppc64 \
  -name guest=guest,debug-threads=on \
  -S \
  -object secret,id=masterKey0,format=raw,file=/home/berrange/.config/libvirt/qemu/lib/domain-33-guest/master-key.aes \
  -machine pseries-2.10,accel=tcg,usb=off,dump-guest-core=off \
  -m 1024 \
  -realtime mlock=off \
  -smp 1,sockets=1,cores=1,threads=1 \
  -uuid c8a74977-ab18-41d0-ae3b-4041c7fffbcd \
  -display none \
  -no-user-config \
  -nodefaults \
  -chardev socket,id=charmonitor,fd=23,server,nowait \
  -mon chardev=charmonitor,id=monitor,mode=control \
  -rtc base=utc \
  -no-shutdown \
  -boot strict=on \
  -device qemu-xhci,id=usb,bus=pci.0,addr=0x1 \
  -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \
  -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
  -msg timestamp=on
  2018-12-14 12:57:03.730+0000: shutting down, reason=failed
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

It's possible that the @outbuf and/or @errbuf could be NULL
and thus we need to use the right comparison macro.

Found by Coverity
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
ACKed-by: NMichal Privoznik <mprivozn@redhat.com>

All of the ones being removed are pulled in by internal.h. The only
exception is sanlock which expects the application to include <stdint.h>
before sanlock's headers, because sanlock prototypes use fixed width
int, but they don't include stdint.h themselves, so we have to leave
that one in place.
Signed-off-by: NErik Skultety <eskultet@redhat.com>
Acked-by: NMichal Privoznik <mprivozn@redhat.com>

It doesn't really make sense for us to have stdlib.h and string.h but
not stdio.h in the internal.h header.
Signed-off-by: NErik Skultety <eskultet@redhat.com>
Acked-by: NMichal Privoznik <mprivozn@redhat.com>

Replace instances where we previously called virGetLastError just to
either get the code or to check if an error exists with
virGetLastErrorCode to avoid a validity pre-check.
Signed-off-by: NRamy Elkest <ramyelkest@gmail.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrange <berrange@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: NAndrea Bolognani <abologna@redhat.com>

This commit fixes the deadlock introduced by commit
0980764d. The call getgrouplist() of
the glibc library isn't safe to be called in between fork and
exec (see commit 75c12564).
Signed-off-by: NMarc Hartmayer <mhartmay@linux.vnet.ibm.com>
Fixes: 0980764d ("util: share code between virExec and virCommandExec")
Reviewed-by: NBjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: NBoris Fiuczynski <fiuczy@linux.vnet.ibm.com>

Found by running valgrind for these tests.
Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

We use the "vir" prefix pretty consistently in our
APIs, both external and internal, which made these
macros stood out.

There is no "node driver" as there was before, drivers have to do
their own ACL checking anyway, so they all specify their functions and
nodeinfo is basically just extending conf/capablities.  Hence moving
the code to src/conf/ is the right way to go.

Also that way we can de-duplicate some code that is in virsysfs and/or
virhostcpu that got duplicated during the virhostcpu.c split.  And
Some cleanup is done throughout the changes, like adding the vir*
prefix etc.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Commit ca10bb04 introduced a new test that fails to build
on at least some architectures:

  commandtest.c: In function 'test25':
  commandtest.c:1121:5: error: comparison is always true due to
                        limited range of data type [-Werror=type-limits]
    if (rv >= 0) {
    ^

Change the type of 'rv' from char to int, which is the proper
return type for virCommandExec() anyway.

In an unlikely event of execve() failing, the virCommandExec()
function does not report any error, even though checks that are
at the beginning of the function are verbose when failing.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

This function doesn't follow our convention of naming functions.

This function doesn't follow our convention of naming functions.

Use virGetLastErrorMessage() instead of virGetLastError() in tests

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>

Signed-off-by: NZhang Bo <oscar.zhangbo@huawei.com>

Add testsuite coverage for the recent commit 0e1a1a8c.

* tests/commandhelper.c (main): Output umask.
* tests/commandtest.c (test15): Also test umask.
(mymain): Add test.
* tests/commanddata/*.log: Update expected output.
Signed-off-by: NEric Blake <eblake@redhat.com>

Since '62f263a7' - Coverity complains if the !pidfile path is taken,
then newfd1 would be leaked.

That sets a new flag, but that flag does mean the child will get
LISTEN_FDS and LISTEN_PID environment variables properly set and
passed FDs reordered so that it corresponds with LISTEN_FDS (they must
start right after STDERR_FILENO).
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>