Shorten the function name as there isn't any vshCommandOptString.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Drop the last enum member VSH_OFLAG_NONE and remove the 'flags' variable
from vshCmdOptDef.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Switch the command parser from using the VSH_OFLAG_REQ_OPT flag
opting out from positional parsing of arguments to a combination of the
'positional' flags for truly positional arguments and
'unwanted_positional' preserving semantics for the existing arguments
where the parser did it due to bad design.
This patch retires VSH_OFLAG_REQ_OPT along with the infrastructure that
was needed to refactor all uses properly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Historically the command parser in virsh parses/fills even optional
arguments with values as if they were positional unless opted out using
VSH_OFLAG_REQ_OPT. This creates unexpected situations when commands can
break in this unwanted semantics:
$ virsh snapshot-create-as --print-xml 1 2 3
<domainsnapshot>
<name>2</name>
<description>3</description>
</domainsnapshot>
To prevent any further addition annotate the rest of the arguments with
the 'unwanted_positional' flag, so that the parser can keep parsing them
as such but any further optional argument will not have this behaviour.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Annotate arguments which can be unintentionally parsed positionally.
(See previous commits for explanation.)
All of these options were added in order thus we must declare all of
them as 'unwanted_positional'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The flag was replaced by the 'required' field in the option definition.
Remove last few uses and all assignments.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the new 'positional' field to do decisions rather than have a
special type for positional strings.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add 'positional' and 'required' fields to vshCmdOptDef, which will
explicitly track the two properties of arguments.
To ensure that we have proper coverage, add checks to
vshCmddefCheckInternals validating the state of the above flags by
infering it from existing data.
This conversion will allow us:
- remove VSH_OT_DATA in favor of VSH_OT_STRING
- use VSH_OT_INT when required both as positional and non-positional
- properly annotate which VSH_OT_ARGV are positional and which are not
(currently inferred by whether an previous positional option exists)
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Store the pointers to 'help' and 'description' information in the struct
directly rather than in a key-value list.
The generic approach never got any extra use.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Adam Julis <ajulis@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
While glibc provides qsort(), which usually is just a mergesort,
until sorting arrays so huge that temporary array used by
mergesort would not fit into physical memory (which in our case
is never), we are not guaranteed it'll use mergesort. The
advantage of mergesort is clear - it's stable. IOW, if we have an
array of values parsed from XML, qsort() it and produce some
output based on those values, we can then compare the output with
some expected output, line by line.
But with newer glibc this is all history. After [1], qsort() is
no longer mergesort but introsort instead, which is not stable.
This is suboptimal, because in some cases we want to preserve
order of equal items. For instance, in ebiptablesApplyNewRules(),
nwfilter rules are sorted by their priority. But if two rules
have the same priority, we want to keep them in the order they
appear in the XML. Since it's hard/needless work to identify
places where stable or unstable sorting is needed, let's just
play it safe and use stable sorting everywhere.
Fortunately, glib provides g_qsort_with_data() which indeed
implement mergesort and it's a drop in replacement for qsort(),
almost. It accepts fifth argument (pointer to opaque data), that
is passed to comparator function, which then accepts three
arguments.
We have to keep one occurance of qsort() though - in NSS module
which deliberately does not link with glib.
1: https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=03bf8357e8291857a435afcc3048e0b697b6cc04
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Error messages are exempt from the 80 columns rule. Move them
onto one line.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Historically, the dumpxml command reject any unknown arguments,
for instance:
virsh dumpxml fedora xxx
However, after v8.5.0-rc1~31 the second argument ('xxx') is
treated as an XPath, but it's not that clearly visible.
Therefore, require the --xpath switch, like this:
virsh dumpxml fedora --xpath xxx
Yes, this breaks already released virsh, but I think we can argue
that the pool of users of this particular function is very small.
We also document the argument being mandatory:
dumpxml [--inactive] [--security-info] [--update-cpu] [--migratable]
[--xpath EXPRESSION] [--wrap] domain
The sooner we do this change, the better.
The same applies for other *dumpxml functions (net-dumpxml,
pool-dumpxml, vol-dumpxl to name a few).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2103524
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
While you can chain the virsh output up to a later 'xmllint' or 'xpath'
command, integrating it into virsh avoids needs for installing extra
binaries which we've often found to be missing on production installs
of libvirt. It also gives better response if the initial virsh command
hits an error, as you don't get an aborted pipeline.
$ virsh pool-dumpxml --xpath //permissions default
<permissions>
<mode>0711</mode>
<owner>1000</owner>
<group>1000</group>
<label>unconfined_u:object_r:svirt_home_t:s0</label>
</permissions>
If multiple nodes match, they are emitted individually:
$ virsh dumpxml --xpath '//devices/*/address[@type="pci"]' --wrap demo
<address type="pci" domain="0x0000" bus="0x05" slot="0x00" function="0x0"/>
<address type="pci" domain="0x0000" bus="0x03" slot="0x00" function="0x0"/>
...snip...
<address type="pci" domain="0x0000" bus="0x00" slot="0x02" function="0x0" multifunction="on"/>
<address type="pci" domain="0x0000" bus="0x07" slot="0x00" function="0x0"/>
but if intending to post-process the output further, the results
can be wrapped in a parent node
$ virsh dumpxml --xpath '//devices/*/address[@type="pci"]' --wrap demo
<nodes>
<address type="pci" domain="0x0000" bus="0x05" slot="0x00" function="0x0"/>
<address type="pci" domain="0x0000" bus="0x03" slot="0x00" function="0x0"/>
...snip...
<address type="pci" domain="0x0000" bus="0x00" slot="0x02" function="0x0" multifunction="on"/>
<address type="pci" domain="0x0000" bus="0x07" slot="0x00" function="0x0"/>
</nodes>
Fixes https://gitlab.com/libvirt/libvirt/-/issues/244
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This change was generated using the following spatch:
@ rule1 @
expression a;
identifier f;
@@
<...
- f(*a);
... when != a;
- *a = NULL;
+ g_clear_pointer(a, f);
...>
@ rule2 @
expression a;
identifier f;
@@
<...
- f(a);
... when != a;
- a = NULL;
+ g_clear_pointer(&a, f);
...>
Then, I left some of the changes out, like tools/nss/ (which
doesn't link with glib) and put back a comment in
qemuBlockJobProcessEventCompletedActiveCommit() which coccinelle
decided to remove (I have no idea why).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Similarly to virshDomainFree add a wrapper for the snapshot object
freeing function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
For now this serves just as an annotation because readline and also the
bash completion script insist on completing local paths when an empty
list is returned.
This will serve for future reference once we'll be able to properly
refuse to suggest anything.
The completer is used for fields such as names for new objects,
description strings, password strings etc, URIs and hostnames which we
can't feasibly autocomplete.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
'--pool' of the 'pool-event' command and '--inputpool' of
'vol-create-from' use the above mentioned completer.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
I think these functions look much more readable with just simple
if conditions.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Historically, we declared pointer type to our types:
typedef struct _virXXX virXXX;
typedef virXXX *virXXXPtr;
But usefulness of such declaration is questionable, at best.
Unfortunately, we can't drop every such declaration - we have to
carry some over, because they are part of public API (e.g.
virDomainPtr). But for internal types - we can do drop them and
use what every other C project uses 'virXXX *'.
This change was generated by a very ugly shell script that
generated sed script which was then called over each file in the
repository. For the shell script refer to the cover letter:
https://listman.redhat.com/archives/libvir-list/2021-March/msg00537.html
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
For iscsi-direct pool, the initiator is necessary for pool defining:
<pool type="iscsi-direct">
...
<initiator>
<iqn name="iqn.2013-06.com.example:iscsi-initiator"/>
</initiator>
...
</pool>
Add --source-initiator to fill the initiator iqn for
pool-create-as/pool-define-as subcommands.
https://bugzilla.redhat.com/show_bug.cgi?id=1658082
Signed-off-by: Han Han <hhan@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Both accept a NULL value gracefully and virStringFreeList
does not zero the pointer afterwards, so a straight replace
is safe.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
G_STATIC_ASSERT() is a drop-in functional equivalent of
the GNULIB verify() macro.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@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>
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>
Since commit 44e7f02915
util: rewrite auto cleanup macros to use glib's equivalent
VIR_AUTOFREE is just an alias for g_autofree. Use the GLib macros
directly instead of our custom aliases.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.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>
This completer can be used to complete pool types.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Add pool list type flag VIR_CONNECT_LIST_STORAGE_POOLS_ISCSI_DIRECT,
which was forgotten when introducing iscsi-direct pool at f0bf1be3.
https://bugzilla.redhat.com/show_bug.cgi?id=1726609
Signed-off-by: Han Han <hhan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virutil.(c|h) is a very gross collection of random code. Remove the enum
handlers from there so we can limit the scope where virtutil.h is used.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Only active pools can be refreshed. But our completer offers just
all pool, even inactive ones.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
