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: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Always trim the full specified suffix.
All of the callers outside of tests were passing either
strlen or the actual length of the string.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Replace all the cases that only supply the length
and do not care about matching a suffix, as well
as that one test case that does.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The Windows platform does not have the signal handling
support we need, so it must be disabled in several parts
of the codebase.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
gmtime_r/localtime_r are mostly used in combination with
strftime to format timestamps in libvirt. This can all
be replaced with GDateTime resulting in simpler code
that is also more portable.
There is some boundary condition problem in parsing POSIX
timezone offsets in GLib which tickles our test suite.
The test suite is hacked to avoid the problem. The upsteam
GLib bug report is
https://gitlab.gnome.org/GNOME/glib/issues/1999
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The GNULIB termios module ensures termios.h exists (but
is none the less empty) when building for Windows. We
already exclude usage of the functions that would exist
in a real termios.h, so having an empty termios.h is
not especially useful.
It is simpler to just put all use of termios.h related
functions behind a "#ifndef WIN32" conditional.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
commandhelper.c is not converted since this is a standalone
program only run on UNIX, so can rely on getcwd().
Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
g_get_real_time() returns the time since epoch in microseconds.
It uses gettimeofday() internally while libvirt used clock_gettime
because it is declared async signal safe. In practice gettimeofday
is also async signal safe *provided* the timezone parameter is
NULL. This is indeed the case in g_get_real_time().
Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
virGetUserCacheDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Remove the usage where sanity of the length argument is verified
by other conditions not matching the previous patches.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We abort on allocation errors now so there is no need to
have a function for it.
Replace the only use by return -1, chosen by fair dice roll.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This flag is not implied by g_mkstemp_full, only by g_mkstemp.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reported-by: Bjoern Walk <bwalk@linux.ibm.com>
Fixes: 4ac4773040
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Glib implementation follows the ISO C99 standard so it's safe to replace
the gnulib implementation.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
With g_mkstemp_full, there is no need to distinguish between
mkostemp and mkostemps (no suffix vs. a suffix of a fixed length),
because the GLib function looks for the XXXXXX pattern everywhere
in the string.
Use S_IRUSR | S_IWUSR for the permissions and do not pass O_RDWR
in flags since it's implied.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The function now does not return an error so we can drop it fully.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function now does not return an error so we can drop it fully.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In few places we have the following code pattern:
int ret;
... /* @ret is not accessed here */
ret = f(...);
return ret;
This pattern can be written less verbose:
...
return f(...);
This patch was generated with following coccinelle spatch:
@@
type T;
constant C;
expression f;
identifier ret;
@@
-T ret = C;
... when != ret
-ret = f;
-return ret;
+return f;
Afterwards I needed to fix a few places, e.g. comment in
virDomainNetIPParseXML() was removed too because coccinelle
thinks it refers to @ret while in fact it doesn't. Also in few
places it replaced @ret declaration with a few spaces instead of
removing the line. But nothing terribly wrong.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel 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: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Now that we use g_strdup everywhere, delete vshStrdup.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Remove all the uses of vshStrdup in favor of GLib's g_strdup.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Split the parameters to make changes more readable.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Use G_GNUC_UNUSED from GLib instead of ATTRIBUTE_UNUSED.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We're using gnulib to get ffs, ffsl, rotl32, count_one_bits,
and count_leading_zeros. Except for rotl32 they can all be
replaced with gcc/clangs builtins. rotl32 is a one-line
trivial function.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel 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.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We don't need domain_conf or libvirt-{qemu,lxc} in these generic files.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Acked-by: Peter Krempa <pkrempa@redhat.com>
This code is needed to use readline older than 4.1, but all
our target platforms ship with at least 6.0 these days so we
can safely get rid of it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Since test:///default resets state on every connection, writing a test
that covers a sequence of commands must be done from a single
session. But if the test wants to exercise particular failure modes as
well as successes, it can be nice to leave witnesses in the stderr
stream immediately before and after the spot where the expected error
should be, to ensure the rest of the script is not causing errors.
Do this by adding an --err option.
Signed-off-by: Eric Blake <eblake@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
As the previous commit mentioned, argv mode (such as when you feed
virsh via stdin with <<\EOF instead of via a single shell argument)
didn't permit comments. Do this by treating any command name token
that starts with # as a comment which silently eats all remaining
arguments to the next newline or semicolon.
Note that batch mode recognizes unquoted # at the start of any word as
a command as part of the tokenizer, while this patch only treats # at
the start of the command word as a comment (any other # remaining by
the time vshCommandParse() is processing things was already quoted
during the tokenzier, and as such was probably intended as the actual
argument to the command word earlier in the line).
Now I can do something like:
$ virsh -c test:///default <<EOF
# setup
snapshot-create-as test s1
snapshot-create-as test s2
# check
snapshot-list test --name
EOF
Signed-off-by: Eric Blake <eblake@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
Continuing from what I did in commit 4817dec0, now I want to write a
sequence that is self-documenting. So I need comments :)
Now I can do something like:
$ virsh -c test:///default '
# setup
snapshot-create-as test s1
snapshot-create-as test s2
# check
snapshot-list test --name
'
Note that this does NOT accept comments in argv mode, another patch
will tackle that.
(If I'm not careful, I might turn virsh into a full-fledged 'sh'
replacement? Here's hoping I don't go that far...)
Signed-off-by: Eric Blake <eblake@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
It's meant for testing, not for production builds. Also we have a helper
for reporting OOM errors. Introduced by 23e0bf1c4e
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
In local testing, I accidentally introduced a self-test failure,
and spent way too much time debugging it. Make sure the testsuite
log includes some hint as to why command option validation failed.
Lone exception: allocation failure is unlikely during self-test,
and if it happens, we are better off asserting (vsh.c can do this,
even if libvirt.so cannot).
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Peter Krempa <pkrempa@redhat.com>
The previous patch made it possible to split multiple commands by
adding newline, but not to split a long single command. The sequence
backslash-newline was being used as if it were a quoted newline
character, rather than completely elided the way the shell does.
Again, add more tests, although this time it seems more like I am
suffering from a leaning-toothpick syndrome with all the \.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
I wanted to do a demonstration with virsh batch mode, which
takes multiple commands all packed into a single argument:
$ virsh -c test:///default 'echo a; echo b;'
a
b
but that produced a really long line, so I tried to make it
more legible:
$ virsh -c test:///default '
echo a;
echo b;
'
error: unknown command: '
'
Let's be more like the shell, and treat unquoted newline as a
command separator just as we do for semicolon. In fact, with
that, I can even now mix styles:
$ virsh -c test:///default '
echo a; echo b
echo c
'
a
b
c
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
In 600462834f we've tried to remove Author(s): lines
from comments at the beginning of our source files. Well, in some
files while we removed the "Author" line we did not remove the
actual list of authors.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Commit 4f4c3b13 (v3.3) fixed an issue where performing cleanup of
libvirt objects could sometimes lose error messages, by adding code
to copy the libvirt error into last_error prior to cleanup paths.
However, it caused a regression: on other paths, some errors are now
printed twice, if libvirt still remembers in its thread-local
storage that an error was set even after virsh cleared last_error.
For example:
$ virsh -c test:///default snapshot-delete test blah
error: Domain snapshot not found: no domain snapshot with matching name 'blah'
error: Domain snapshot not found: no domain snapshot with matching name 'blah'
Fix things by telling libvirt to discard any thread-local errors at
the same time virsh prints an error message (whether or not the libvirt
error is the same as what is stored in last_error).
Update the virsh-undefine testsuite (partially reverting portions of
commit b620bdee, by removing -q, to more easily pinpoint which commands
are causing which messages), now that there is only one error message
instead of two.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Michal 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: Erik Skultety <eskultet@redhat.com>
Acked-by: Michal 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: Erik Skultety <eskultet@redhat.com>
Acked-by: Michal 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: Ramy Elkest <ramyelkest@gmail.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Currently the VSH_OT_ARGV options don't support complete, But some of
VSH_OT_ARGV options are gonna support complete in upcoming patches.
Once applied the upcoming completion patches for VSH_OT_ARGV options, If
we don't ignore VSH_OT_ARGV here, The vshReadlineOptionsGenerator will
be called, Hence complete output will consist of the result by command
completer + the result by option completer, It's confusing.
e.g.
$ virsh domstats --domain <TAB><TAB>
--backing --interface --list-paused --perf --vcpu
--balloon leap42.3 --list-persistent --raw win10
--block --list-active --list-running sles12sp3
--cpu-total --list-inactive --list-shutoff sles15
--enforce --list-other --list-transient --state
After this patch and the upcoming completion patches:
$ virsh domstats --domain <TAB><TAB>
leap42.3 sles12sp3 sles15 win10
Signed-off-by: Lin Ma <lma@suse.com>
