Commit Graph

41 Commits

Author SHA1 Message Date
Roman Bolshakov
c3d02040ac tests: commandhelper: Accept POLLNVAL on macOS
commandhelper hangs indefinitely in poll() on macOS on commandtest test2
and later because POLLNVAL is returned on revents for input file
descriptor opened from /dev/null, i.e this hangs:

  $ tests/commandhelper < /dev/null
  BEGIN STDOUT
  BEGIN STDERR
  ^C

But it works fine with regular stdin:

  $ tests/commandhelper <<< test
  BEGIN STDOUT
  BEGIN STDERR
  test
  test
  END STDOUT
  END STDERR

The issue is mentioned in poll(2):

  BUGS
    The poll() system call currently does not support devices.

With the change all 28 cases in commandtest pass.

Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
2020-10-08 17:04:08 +02:00
Daniel P. Berrangé
946c815024 tests: don't mix FILE* and UNIX FD I/O on same stream
There is currently a hang in test27 that exhibits itself on FreeBSD 11.4
only. The behaviour is that virCommandProcessIO gets POLLIN on the
FD for stdout, but read() blocks. Meanwhile commandtest also blocks
in write for stderr because the pipe buffers are full.

This fix in commandhelper likely does not really address the root cause
just hides it due to the buffering done by FILE *. Mixing UNIX FD I/O
and FILE * I/O is bad practice regardless.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2020-09-22 10:45:33 +01:00
Ján Tomko
a5152f23e7 Move declarations before statements
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-08-25 19:03:11 +02:00
Peter Krempa
6ae53a1509 tests: commandtest: Make 'test4' checking daemonization more reliable
The 'commandhelper' checks effectively whether the parent process is
still around to report whether it was daemonized or not.

This creates a unlikely race condition in cases when we do actually
daemonize the process as the intermediate process used for the
daemonization might not have terminated yet which would report wrong
result leading to test failure.

For now there's just 'test4' which actually daemonizes the process.

Add an argument '--check-daemonize' which asks for retries of the
daemonization check in cases where we expect that the commandhelper is
going to be daemonized and use it in 'test4' to make the test more
reliable.

I've observed the test failure sporadically when my box is under load
e.g. while building two trees at once.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2020-07-27 15:44:38 +02:00
Pavel Hrdina
966a0a75cc tests: commandhelper: change how we detect if running as daemon
The old code works correctly with make and running directly from shell
but it failed with Meson test suite where session ID and process group
are the same in both cases.

What changes in both cases is parent process ID so use that instead of
session ID.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2020-07-10 09:40:55 +02:00
Daniel P. Berrangé
4ab2120f3b src: remove virFilePrintf in favour of g_fprintf
The virFilePrintf function was a wrapper for fprintf() to provide
Windows portability, since gnulib's fprintf() replacement was
license restricted. This is no longer needed now we have the
g_fprintf function available.

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2020-02-04 14:00:45 +00:00
Daniel P. Berrangé
7aef7cdbb5 src: conditionalize / remove use of poll.h
Remove imports of poll.h which are redundant, and
conditionalize remaining usage that needs to compile
on Windows platforms.

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2020-02-04 14:00:44 +00:00
Daniel P. Berrangé
2df085bf24 tests: always declare environ
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: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2020-01-17 09:59:08 +00:00
Ján Tomko
67e72053c1 Use G_N_ELEMENTS instead of ARRAY_CARDINALITY
Prefer the GLib version of the macro.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2019-10-15 16:14:19 +02:00
Roman Bolshakov
c6b3bf9302 tests: Drop /private CWD prefix in commandhelper
/tmp is a symbolic link to /private/tmp on macOS. That causes failures
in commandtest, because getcwd returns /private/tmp and the expected
output doesn't match to "CWD: /tmp".

Rathern than making a copy of commanddata solely for macOS, the /private
prefix is stripped.

Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
2019-08-23 11:26:26 +01:00
Stefan Berger
52f115682f tests: Extend command test to transfer large data to process on multiple fds
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: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2019-07-26 10:30:57 +01:00
Erik Skultety
5165ff0971 src: More cleanup of some system headers already contained in internal.h
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: Erik Skultety <eskultet@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
2018-09-20 10:16:39 +02:00
Erik Skultety
9403b63102 internal: Move <stdio.h> include to internal.h
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: Erik Skultety <eskultet@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
2018-09-20 10:16:38 +02:00
John Ferlan
713342e953 tests: Return failure if log not fopen'd
If @log is not fopen'd then, going to cleanup and calling fclose
will make for an unhappy callee. So just fail immediately instead
since there's nothing to clean up.

Found by Coverity

Signed-off-by: John Ferlan <jferlan@redhat.com>
2018-04-19 14:24:12 -04:00
Daniel P. Berrange
4c71b0ee7c Fix commandhelper build on win32
For win32 we need EXIT_AM_SKIP which is in testutils.h. We must
define NO_LIBVIRT to prevent replacement of fprintf with
virFilePrintf as we can't link to libvirt_util.la

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2017-09-21 08:55:07 +01:00
Daniel P. Berrange
eae746b2d7 Stop linking tests/commandhelper to libvirt code
The commandhelper binary is a helper for commandtest that
validates what file handles were inherited. For this to
work reliably we must not have any libraries that leak
file descriptors into commandhelper. Unfortunately some
versions of gnutls will intentionally open file handles
at library load time via a constructor function.

We previously hacked around this in

  commit 4cbc15d037
  Author: Martin Kletzander <mkletzan@redhat.com>
  Date:   Fri May 2 09:55:52 2014 +0200

    tests: don't fail with newer gnutls

    gnutls-3.3.0 and newer leaves 2 FDs open in order to be backwards
    compatible when it comes to chrooted binaries [1].  Linking
    commandhelper with gnutls then leaves these two FDs open and
    commandtest fails thanks to that.  This patch does not link
    commandhelper with libvirt.la, but rather only the utilities making
    the test pass.

    Based on suggestion from Daniel [2].

    [1] http://lists.gnutls.org/pipermail/gnutls-help/2014-April/003429.html
    [2] https://www.redhat.com/archives/libvir-list/2014-April/msg01119.html

That fix relied on fact that while libvirt.so linked with
gnutls, libvirt_util.la did not link to it.  With the
introduction of the util/vircrypto.c file that assumption
is no longer valid. We must not link to libvirt_util.la
at all - only gnulib and libc can (hopefully) be relied
on not to open random file descriptors in constructors.

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2017-09-20 14:32:12 +01:00
Martin Kletzander
f5e65e4b71 Remove unnecessary curly brackets in tests/
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2014-11-14 17:13:36 +01:00
Wang Rui
cd2d7c6c3a tests: Resolve Coverity RESOURCE_LEAK in commandhelper
Coverity determined that 'log' and 'newenv' were not freed in
some cases. Free them in 'error' branch and normal branch.

Signed-off-by: Wang Rui <moon.wangrui@huawei.com>
2014-09-03 15:00:18 -04:00
Eric Blake
0c07d360f6 command: test umask support
Add testsuite coverage for the recent commit 0e1a1a8.

* 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: Eric Blake <eblake@redhat.com>
2014-09-03 09:36:13 -06:00
Ján Tomko
2dcdb7f654 Indent top-level labels by one space in tests/ 2014-03-25 14:58:41 +01:00
Martin Kletzander
f19b3a5f0c Use K&R style for curly braces in tests/
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2014-03-20 17:06:38 +01:00
John Ferlan
8283ef9ea2 testutils: Resolve Coverity issues
Recent changes uncovered a NEGATIVE_RETURNS in the return from sysconf()
when processing a for loop in virtTestCaptureProgramExecChild() in
testutils.c

Code review uncovered 3 other code paths with the same condition that
weren't found by Covirity, so fixed those as well.
2013-07-11 14:18:11 -04:00
Daniel P. Berrange
7a1e691711 Convert 'int i' to 'size_t i' in tests/ files
Convert the type of loop iterators named 'i', 'j', k',
'ii', 'jj', 'kk', to be 'size_t' instead of 'int' or
'unsigned int', also santizing 'ii', 'jj', 'kk' to use
the normal 'i', 'j', 'k' naming

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2013-07-10 17:40:13 +01:00
Michal Privoznik
3ea84b9548 Adapt to VIR_ALLOC and virAsprintf in tests/* 2013-07-10 11:07:33 +02:00
Michal Privoznik
8290cbbc38 viralloc: Report OOM error on failure
Similarly to VIR_STRDUP, we want the OOM error to be reported in
VIR_ALLOC and friends.
2013-07-10 11:07:31 +02:00
Osier Yang
bb3ea8416b tests/: Remove the whitespace before ";" 2013-05-21 23:41:45 +08:00
Michal Privoznik
e60b9783e1 Adapt to VIR_STRDUP and VIR_STRNDUP in tests/* 2013-05-10 11:54:29 +02:00
Eric Blake
dce95297e3 tests: uniformly report test failures
testutils.c likes to print summaries after a test completes,
including if it failed.  But if the test outright exit()s,
this summary is skipped.  Enforce that we return instead of exit.

* cfg.mk (sc_prohibit_exit_in_tests): New syntax check.
* tests/commandhelper.c (main): Fix offenders.
* tests/qemumonitorjsontest.c (mymain): Likewise.
* tests/seclabeltest.c (main): Likewise.
* tests/securityselinuxlabeltest.c (mymain): Likewise.
* tests/securityselinuxtest.c (mymain): Likewise.
* tests/testutils.h (VIRT_TEST_MAIN_PRELOAD): Likewise.
* tests/testutils.c (virtTestMain): Likewise.
(virtTestCaptureProgramOutput): Use symbolic name.
2013-02-25 17:38:11 -07:00
Daniel P. Berrange
44f6ae27fe Rename util.{c,h} to virutil.{c,h} 2012-12-21 11:19:49 +00:00
Daniel P. Berrange
ab9b7ec2f6 Rename memory.{c,h} to viralloc.{c,h} 2012-12-21 11:17:14 +00:00
Eric Blake
4ecb723b9e maint: fix up copyright notice inconsistencies
https://www.gnu.org/licenses/gpl-howto.html recommends that
the 'If not, see <url>.' phrase be a separate sentence.

* tests/securityselinuxhelper.c: Remove doubled line.
* tests/securityselinuxtest.c: Likewise.
* globally: s/;  If/.  If/
2012-09-20 16:30:55 -06:00
Osier Yang
f9ce7dad60 Desert the FSF address in copyright
Per the FSF address could be changed from time to time, and GNU
recommends the following now: (http://www.gnu.org/licenses/gpl-howto.html)

  You should have received a copy of the GNU General Public License
  along with Foobar.  If not, see <http://www.gnu.org/licenses/>.

This patch removes the explicit FSF address, and uses above instead
(of course, with inserting 'Lesser' before 'General').

Except a bunch of files for security driver, all others are changed
automatically, the copyright for securify files are not complete,
that's why to do it manually:

  src/security/security_selinux.h
  src/security/security_driver.h
  src/security/security_selinux.c
  src/security/security_apparmor.h
  src/security/security_apparmor.c
  src/security/security_driver.c
2012-07-23 10:50:50 +08:00
Eric Blake
858c2476d9 command: avoid deadlock on EPIPE situation
It is possible to deadlock libvirt by having a domain with XML
longer than PIPE_BUF, and by writing a hook script that closes
stdin early.  This is because libvirt was keeping a copy of the
child's stdin read fd open, which means the write fd in the
parent will never see EPIPE (remember, libvirt should always be
run with SIGPIPE ignored, so we should never get a SIGPIPE signal).
Since there is no error, libvirt blocks waiting for a write to
complete, even though the only reader is also libvirt.  The
solution is to ensure that only the child can act as a reader
before the parent does any writes; and then dealing with the
fallout of dealing with EPIPE.

Thankfully, this is not a security hole - since the only way to
trigger the deadlock is to install a custom hook script, anyone
that already has privileges to install a hook script already has
privileges to do any number of other equally disruptive things
to libvirt; it would only be a security hole if an unprivileged
user could install a hook script to DoS a privileged user.

* src/util/command.c (virCommandRun): Close parent's copy of child
read fd earlier.
(virCommandProcessIO): Don't let EPIPE be fatal; the child may
be done parsing input.
* tests/commandhelper.c (main): Set up a SIGPIPE situation.
* tests/commandtest.c (test20): Trigger it.
* tests/commanddata/test20.log: New file.
2012-06-04 13:06:07 -06:00
Daniel P. Berrange
57351139a7 Disable build of commandhelper & ssh on Win32
The commandhelper.c & ssh.c programs rely on various APIs not present
on Win32. Disable them, since the tests that uses these helpers are
already disabled

* tests/commandhelper.c, tests/ssh.c: Disable on WIN32

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2012-03-30 11:47:14 +01:00
Eric Blake
8fe454ce90 build: expand rule to cover testsuite
The bulk of this patch was done with:

sed -i 's/\(\bfree *(/VIR_FREE(/g' tests/*.c

followed by fixing the few compile errors that resulted.

* cfg.mk (exclude_file_name_regexp--sc_prohibit_raw_allocation):
Remove tests from exemption.
* tests/testutils.h: Add common header.
* tests/commandhelper.c: Fix offenders.
* tests/cputest.c: Likewise.
* tests/domainsnapshotxml2xmltest.c: Likewise.
* tests/interfacexml2xmltest.c: Likewise.
* tests/networkxml2argvtest.c: Likewise.
* tests/networkxml2xmltest.c: Likewise.
* tests/nodedevxml2xmltest.c: Likewise.
* tests/nodeinfotest.c: Likewise.
* tests/nwfilterxml2xmltest.c: Likewise.
* tests/qemuargv2xmltest.c: Likewise.
* tests/qemuxml2argvtest.c: Likewise.
* tests/qemuxml2xmltest.c: Likewise.
* tests/qemuxmlnstest.c: Likewise.
* tests/qparamtest.c: Likewise.
* tests/sexpr2xmltest.c: Likewise.
* tests/storagepoolxml2xmltest.c: Likewise.
* tests/storagevolxml2xmltest.c: Likewise.
* tests/testutils.c: Likewise.
* tests/virshtest.c: Likewise.
* tests/xencapstest.c: Likewise.
* tests/xmconfigtest.c: Likewise.
* tests/xml2sexprtest.c: Likewise.
2012-02-03 10:41:46 -07:00
Eric Blake
8e22e08935 build: rename files.h to virfile.h
In preparation for a future patch adding new virFile APIs.

* src/util/files.h, src/util/files.c: Move...
* src/util/virfile.h, src/util/virfile.c: ...here, and rename
functions to virFile prefix.  Macro names are intentionally
left alone.
* *.c: All '#include "files.h"' uses changed.
* src/Makefile.am (UTIL_SOURCES): Reflect rename.
* cfg.mk (exclude_file_name_regexp--sc_prohibit_close): Likewise.
* src/libvirt_private.syms: Likewise.
* docs/hacking.html.in: Likewise.
* HACKING: Regenerate.
2011-07-21 10:34:51 -06:00
Eric Blake
e39c46a5fd build: fix getcwd portability problems
* bootstrap.conf (gnulib_modules): Add getcwd-lgpl.
* tests/commandtest.c (checkoutput): Drop unused cwd.
* tests/commandhelper.c (main): Let getcwd malloc.
* tests/testutils.c (virTestMain): Likewise.
* tools/virsh.c (cmdPwd): Likewise.
(virshCmds): Expose cmdPwd and cmdCd on mingw.
2011-04-29 12:08:26 -06:00
Diego Elio Pettenò
d86ce05797 commandtest: avoid printing loader-control variables from commandhelper
This avoids throwing the tests off if LD_LIBRARY_PATH or LD_PRELOAD or
other variables are set.

Signed-off-by: Diego Elio Pettenò <flameeyes@gmail.com>
2011-01-06 14:21:17 -07:00
Eric Blake
630fe36f06 tests: avoid data race
I got some spurious failures when commandhelper won the race and
ran to the point of parent detection prior to the intermediate
daemonizing process getting a chance to exit.  This fixes it.

* tests/commandhelper.c (main): Checking for re-parenting to
init(1) is racy; instead check that we belong to a new session.
2010-12-20 10:29:50 -07:00
Jiri Denemark
58d0f4d737 tests: Don't ignore return value of getcwd() 2010-12-06 10:12:01 +01:00
Daniel P. Berrange
f16ad06fb2 Introduce new APIs for spawning processes
This introduces a new set of APIs in src/util/command.h
to use for invoking commands. This is intended to replace
all current usage of virRun and virExec variants, with a
more flexible and less error prone API.

* src/util/command.c: New file.
* src/util/command.h: New header.
* src/Makefile.am (UTIL_SOURCES): Build it.
* src/libvirt_private.syms: Export symbols internally.
* tests/commandtest.c: New test.
* tests/Makefile.am (check_PROGRAMS): Run it.
* tests/commandhelper.c: Auxiliary program.
* tests/commanddata/test2.log - test15.log: New expected outputs.
* cfg.mk (useless_free_options): Add virCommandFree.
(msg_gen_function): Add virCommandError.
* po/POTFILES.in: New translation.
* .x-sc_avoid_write: Add exemption.
* tests/.gitignore: Ignore new built file.
2010-12-02 16:00:44 -07:00