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>
It's helpful for users while they type certain kind of VSH_OT_ARGV options.
e.g.
$ virsh domstats --domain sles12sp3 --d<TAB>
Signed-off-by: Lin Ma <lma@suse.com>
The first entry in the returned array is the substitution for TEXT. It
causes unnecessary output if other commands or options share the same
prefix, e.g.
$ virsh des<TAB><TAB>
des desc destroy
or
$ virsh domblklist --d<TAB><TAB>
--d --details --domain
This patch fixes the above issue.
Signed-off-by: Lin Ma <lma@suse.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Currently if cmd->skipChecks is set (done only from completers)
some basic checks are skipped because we're working over
partially parsed command. See a26ff63ae4 for more detailed
explanation. Anyway, the referenced commit was too aggressive in
disabling checks and effectively returned success even in clear
case of failure. For instance:
# domif-getlink --interface <TAB><TAB>
causes virshDomainInterfaceCompleter() to be called, which calls
virshDomainGetXML() which eventually calls
vshCommandOptStringReq(.., name = "domain"); The --domain
argument is required for the command and if not present -1 should
be returned to tell the caller the argument was not found. Well,
zero is returned meaning the argument was not found but it's not
required either.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Unfortunately, we have a number of aliases in virsh and even though
these are not visible any more, we have to support them. The problem is
that when trying to print help for the alias, we get SIGSEGV because
there isn't any @def structure anymore and we need to query the command
being aliased instead.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1538570
Signed-off-by: Erik Skultety <eskultet@redhat.com>
These helpers are called from a single place only - cmdHelp wrapper and
just before the wrapper invokes the helpers, it performs the search,
either for command group or for the command itself, except the result is
discarded and the helper therefore needs to do it again. Drop this
inefficient handling and pass the @def structure rather than a name,
thus preventing the helper from needing to perform the search again.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
When building without readline, this function does nothing but
return false. Without touching any of its arguments which
triggers a build error. Therefore, provide a stub that has
arguments marked as unused.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The current state of art is as follows:
1) vshReadlineOptionsGenerator() generate all possible --options
for given command, and then
2) vshReadlineOptionsPrune() clears out already provided ones
from the list.
Not only this brings needless memory complexity it is also not
trivial to get right. We can switch to easier approach: just
don't add already specified --options in the first step.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
This command is going to be called from bash completion script in
the following form:
virsh complete -- start --domain
Its only purpose is to return list of possible strings for
completion. Note that this is a 'hidden', unlisted command and
therefore there's no documentation to it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Similarly to the previous commit, once we've presented an
--option for a command to the user it makes no sense to offer it
again. Therefore, we can prune all already specified options. For
instance, after this patch:
virsh # migrate --verbose <TAB><TAB>
will no longer offer --verbose option.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Instead of having completers prune returned string list based on
user's input we can do that right after the callback is called.
Only strings matching the prefix will be presented to the user
then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Now that we have everything prepared we can call options'
completer again. At the same time, pass partially parsed input to
the completer callback - it will help the callbacks to narrow
down the list of returned options based on user's input. For
instance, if the completer is supposed to return list of
interfaces depending on user input it may return just those
interfaces defined for already specified domain. Of course,
completers might ignore this parameter.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In the future, completer callbacks will receive partially parsed
command (and thus possibly incomplete). However, we still want
them to use command options fetching APIs we already have (e.g.
vshCommandOpt*()) and at the same time don't report any errors
(nor call any asserts).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
It's better to fetch list of either commands or options just once
and then iterate over it. Moreover, it makes future completers
way simpler as they will return string lists too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When returning a string that needs escaping there are two
scenarios that can happen. Firstly, user already started the
string with a quote (or double quote) in which case we don't need
to do anything - readline takes care of that. However, if they
haven't typed anything yet, we need to escape the string
ourselves.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Now that we have a way of retrieving partly parsed command we
don't need duplicate code that parses the user's input.
Yes, this code removes call of opt's completer, but:
a) current implementation is broken anyway, and
b) it will be added back shortly
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In the future, this function is going to be called from
vshReadlineParse() to provide parsed input for completer
callbacks. The idea is to allow the callbacks to provide more
specific data. For instance, for the following input:
virsh # domifaddr --domain fedora --interface <TAB><TAB>
the --interface completer callback is going to be called. Now, it
is more user friendly if the completer offers only those
interfaces found in 'fedora' domain. But in order to do that it
needs to be able to retrieve partially parsed result.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When parsing cmd line which has "--" on it, this is leaked.
Problem is, parser->getNextArg() allocates new string and stores
it into tkdata. But as soon as "--" is detected 'continue' is
issued without any free of the allocated memory.
==5304== 3 bytes in 1 blocks are definitely lost in loss record 1 of 782
==5304== at 0x4C2AF50: malloc (vg_replace_malloc.c:299)
==5304== by 0x8BB5AA9: strdup (strdup.c:42)
==5304== by 0x55842CA: virStrdup (virstring.c:941)
==5304== by 0x172B21: _vshStrdup (vsh.c:162)
==5304== by 0x175E8E: vshCommandArgvGetArg (vsh.c:1622)
==5304== by 0x17551D: vshCommandParse (vsh.c:1418)
==5304== by 0x175F25: vshCommandArgvParse (vsh.c:1638)
==5304== by 0x130940: virshParseArgv (virsh.c:820)
==5304== by 0x130C49: main (virsh.c:922)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
My latest commit of a785186446 uncovered a problem we fixed
in 9eb23fe2 but then reverted in 834c5720e4. Turns out, some
systems (I'm looking at you OS X) have ancient readline with
broken header file.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
There are couple of limitations when it comes to option types and
flags for the options. For instance, VSH_OT_STRING cannot have
VSH_OFLAG_REQ set (commit c7543a728). For some reason this is
checked in vshCmddefHelp() but not in vshCmddefCheckInternals().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Avoid the annoying issue where the public object freeing APIs overwrite
the error set by helper functions, since they don't invoke the callback.
The new helper remembers the error only if no previous error was set.
Some arguments in vshErrorHandler, vshReadlineCompletion and
cmdSelfTest functions are not used. Mark them as such.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
We have couple of functions that operate over NULL terminated
lits of strings. However, our naming sucks:
virStringJoin
virStringFreeList
virStringFreeListCount
virStringArrayHasString
virStringGetFirstWithPrefix
We can do better:
virStringListJoin
virStringListFree
virStringListFreeCount
virStringListHasString
virStringListGetFirstWithPrefix
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
There were a few places in our virsh* code where instead of calling vshError
on failure we called vshPrint.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
First, since commit 834c5720 the error reporting within the vshErrorHandler
doesn't work because there was a lot of renaming going on (dull mechanical
renaming without much thinking about it, yep - shame on me) and so the original
env variable VIRSH_DEBUG got renamed to VSH_DEBUG which we don't support nor
document anywhere. Second, by specifying this env variable, the last libvirt
error gets reported twice despite the fact we say the error reporting should be
deferred until the command finishes, and last but not least the vintage code's
logic is a bit 'odd', since the error would get reported iff the env variable
is set, even if the value should be equal to our DEFAULT value in which case it
doesn't make sense that we behave differently when an env variable is set to
some value and when there's no env variable at all but we use the same value
automatically as default.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Unlike the other error messages in vshInitDebug, this one relied on a hardcoded
name of a variable instead of using the prefix of the tool calling the init
routine.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1393854
Signed-off-by: Erik Skultety <eskultet@redhat.com>
After 06a7b1ff4 the @&opts_need_arg is not used anywhere. Well,
it is set but never read:
vsh.c: In function 'vshReadlineParse':
vsh.c:2658:14: warning: variable 'opts_need_arg' set but not used [-Wunused-but-set-variable]
uint64_t opts_need_arg, opts_seen;
^~~~~~~~~~~~~
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Commit id 'dcfdf341' passes 'opts_need_arg' and 'opts_seen' to
vshCmddefGetData, but that seems to be incorrect as those values
are not initialized properly (something at least one compiler found).
Instead the static 'const_opts_need_arg' and 'const_opts_seen' values
should be passed.
By passing unitialized values leads to not finding possible options
for simpler commands (domfsfreeze for example), where if you're in
a virsh shell using command line completion - you'll get a list of
files in your current directory instead of two options --domain and
--mountpoint (as would happen with this patch applied.
1. Move the declaration of const vshCmdDef *help - it should be at the
top of the "if" rather than in the middle.
2. Change a comparison from && to || - without doing so we could crash
on commands like 'virsh list' which would allow completion of some
non -- option based on whatever was found in the current working
directory and then as soon as that was completed, the next <tab>
would crash since "opt" would be returned as NULL, but the check
was dereferencing "&& opt->type"
3. Before dereferencing opt->completer, be sure opt isn't NULL.
GCC complained that
vsh.c: In function 'vshReadlineOptionsGenerator':
vsh.c:2622:29: warning: unused variable 'opt' [-Wunused-variable]
const vshCmdOptDef *opt = &cmd->opts[list_index];
^
vsh.c: In function 'vshReadlineParse':
vsh.c:2830:44: warning: 'opt' may be used uninitialized in this function
[-Wmaybe-uninitialized]
completed_list = opt->completer(autoCompleteOpaque,
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1379895
Introduced by commit id '834c5720'.
During the code motion and creation of vsh.c, the function 'vshDeinit()'
in the (new) vsh.c was altered from whence it came in virsh.c such that
calling 'vshReadlineDeinit(ctl)' was conditional on "ctl->imode".
This causes a problem for the interactive running if the "quit" and "exit"
commands are used because 'cmdQuit' will clear ctl->imode, thus when the
interactive loop in main() of virsh.c exits because ctl->mode is clear and
virshDeinit is called which calls vshDeinit, the history file is now not
written. Conversely, if one had exited the interactive loop via pressing
<ctrl>D the file would be created because loop control is broken on EOF
and ctl->imode is not set to false.
This patch will remove the conditional call to vshReadlineDeinit and
restore the former behaviour.
Change the logic in a way, so that VSH_CMD_FLAG_ALIAS behaves similarly to
how VSH_OT_ALIAS for command options, i.e. there is no need for code duplication
for the alias and the aliased command structures. Along with that change,
switch any existing VSH_CMD_FLAG_ALIAS occurrences to this new format. Also,
since this patch introduces a new command structure element, adjust the
virsh-self-test test to make sure we won't ever miss to specify the '.alias'
member for an aliased command because doing that would lead to an internal
error.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Recent changes extracted the command internals validation routine from
vshCmddefOptParse method which now just calls vshCmddefOptFill. Therefore, make
vshCmddefOptFill the new vshCmddefOptParse and drop the unnecessary name.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Originally introduced by commit 2432521e which correctly split
vshCmddefOptParse into command's options validation and options parsing.
However, command's 'internals' are not tied solely to .options, rather it
should be about the overall structure, therefore the validation should be
extracted from vshCmddefOptParse and performed only within our test suite, i.e.
in vshSelfTest.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
If the initial check is true the function immediately returns so there's no
need to enclose the code following the check within an 'else' block.
Also, by removing the 'else' block, the declarations need to be moved to
beginning of the function block to conform with our guidelines.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Since it's used on a single place only, it can easily be replaced by the right
side of the original assignment.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
The intention is to move vshCmddefCheckInternals out of vshCmddefOptParse to
our test suite. First step to do that is to enforce checking for an existing
help string (that also means it's non-empty) in a command because a command
without a help is not much of a use.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
This command should be exposed to other shells of ours.
They are gonna need it as soon as we want to test them too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This patch changes the signature of vshCompleters, allowing to pass along
some data that we might want to along with the completers; for example,
we might want to pass the autocomplete vshControl along with the
completer, in case the completer requires a connection to libvirtd.
Signed-off-by: Nishith Shah <nishithshah.2211@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Call option completers if argument completion is requested using the
corresponding option completer, if it is defined.
Signed-off-by: Nishith Shah <nishithshah.2211@gmail.com>
Since commit 834c5720 which extracted the generic functionality out of virsh
and made it available for other clients like virt-admin to make use of it, it
also introduced a bug when it renamed the original VIRSH_ environment variables
to VSH_ variables. Virt-admin of course suffers from the same bug, so this
patch modifies the generic module vsh.c to construct the correct name for
environment variables of each client from information it has.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1357363
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Well, the reason behind this change is that if the function is extended in some
way that e.g. would involve allocation we do not have a way of telling it to
the caller. More specifically, vshInitDebug only relies on some hardcoded
environment variables (by a mistake) that aren't documented anywhere so neither
virsh's nor virt-admin's documented environment variables take effect. One
possible solution would be duplicate the code for each CLI client or leave the
method be generic and provide means that it could figure out, which client
called it, thus initializing the proper environment variables but that could
involve operations that might as well fail in certain circumstances and the
caller should know that an error occurred.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
My compiler identified some variables that were set, but never
actually used. For instance, opts_required, and data_acomplete.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This function tries to look up desired option for a given parsed
command. Upon successful return it also stores option position
into passed *opt_index. Now, this variable is type of int, even
though it is never ever used to store negative value. Moreover,
the variable is set from a local variable which is type of
size_t.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The 'res' variable was only being initialized to NULL in the
if (!state) path; however, that path never used res and evenutally
res is assigned one of two results based on a pair of if then else if
conditions. If for some reason neither of those paths was taken and
the (!state) path wasn't taken, then 'res' would be indeterminate.
Found by Coverity, probably a false positive based on code paths, but
better safe than sorry for the future.
Signed-off-by: John Ferlan <jferlan@redhat.com>
The new function works as expected, and matches the current level of
autocomplete offered, along with several other improvements like quotes
handling, multiple command completion and space handling. Now, it is easy
to introduce options completer here.
Signed-off-by: Nishith Shah <nishithshah.2211@gmail.com>
A bool 'report' has been introduced in various functions, which when set
to true will produce the error it is suppposed to produce, and when
false, will suppress the error. These functions are used in the next
patch for auto-completion.
Signed-off-by: Nishith Shah <nishithshah.2211@gmail.com>
Decompose vshCmddefOptParse into two helper functions, vshCmddefOptFill
and vshCmddefCheckInternals.
vshCmddefCheckInternals checks if the internal command definitions are
correct or not.
vshCmddefOptFill keeps track of the required options and mandatory
arguments through opts_required and opts_need_arg.
Signed-off-by: Nishith Shah <nishithshah.2211@gmail.com>
Add ULL suffix to all related operands of << or shift will give
all zeros instead of correct mask.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Upcoming compression options for migration command patch
series hits current limit of 32 possible options for a command.
Lets take one step further and support 64 possible options.
And all it takes is moving from 32 bit integers to 64 bit ones.
The only less then trivial change i found is moving from
'ffs' to 'ffsl'.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Historically we've used 'unsigned long' and allowed wrapping of negative
numbers for bandwidth values. Add a helper that will simplify adding
support for scaled integers and support for byte granularity while
keeping the compatibility with the older approach.
It was too similar to the non-scaled alternative.
before:
error: Numeric value 'abc' for <size> option is malformed or out of range
after:
error: Scaled numeric value 'abc' for <size> option is malformed or out of range
The macro would eat the first parameter. In some cases the format string
for vshPrint was eaten. In other cases the calls referenced variables
which did not exist in the given context. Avoid errors by doing compile
time checking.
When we hit OOM it doesn't really make sense to format the error message
by attempting to allocate it. Introduce a simple helper that prints a
static message and terminates the execution.
There's no reason why debug initialization could not be made completely
hidden, just like readline initialization is. The point of the global
initializer vshInit is to make initialization of smaller features transparent
to the user/caller.
Commit a0b6a36f separated vshInitDebug from the original vshInit
(before virsh got split and vshInit became virshInit - commit 834c5720)
in order to be able to debug command line parsing.
After the parsing is finished, debugging is reinitialized to work properly.
There might as well be other features that require re-initialization as
the command line could specify parameters that override our defaults which
had been set prior to calling vshArgvParse.
As part of the effort to stay consistent, change the vshInit signature
from returning int to returning bool. Moreover, remove the
unnecessary error label as there is no cleanup that would make use of
it.
Caller is responsible for freeing the result of virStringJoin()
when no longer needed:
==10701== 1 bytes in 1 blocks are definitely lost in loss record 1 of 806
==10701== at 0x4C29F80: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==10701== by 0xAADB679: strdup (in /lib64/libc-2.20.so)
==10701== by 0x4F18655: virStrdup (virstring.c:726)
==10701== by 0x4F175AF: virStringJoin (virstring.c:165)
==10701== by 0x131D4D: vshReadlineInit (vsh.c:2572)
==10701== by 0x1322DF: vshInit (vsh.c:2736)
==10701== by 0x1347C1: main (virsh.c:907)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This patch removes the static keyword from the vshReadline which was
introduced in commit 834c5720e4. With
readline the vshReadline function is not static but when compiling
without readline it was defined as static which caused compilation
error.
In order to share as much virsh' logic as possible with upcomming
virt-admin client we need to split virsh logic into virsh specific and
client generic features.
Since majority of virsh methods should be generic enough to be used by
other clients, it's much easier to rename virsh specific data to virshX
than doing this vice versa. It moved generic virsh commands (including info
and opts structures) to generic module vsh.c.
Besides renaming methods and structures, this patch also involves introduction
of a client specific control structure being referenced as private data in the
original control structure, introduction of a new global vsh Initializer,
which currently doesn't do much, but there is a potential for added
functionality in the future.
Lastly it introduced client hooks which are especially necessary during
client connecting phase.
