Right now, we display the message before actually attempting
to connect to the VM console. That operation, however, can
fail for a number of reasons: for example, is the VM doesn't
have a serial device, the output ends up looking like
$ virsh console cirros
Connected to domain 'cirros'
Escape character is ^] (Ctrl + ])
error: internal error: cannot find character device <null>
The initial message is misleading. Change things so that it's
only printed if we actually successfully connected to the VM
console.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The self-test command for both virsh and virt-admin is self contained
and directly reports success, thus we don't actually need to run a shell
wrapper around it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
For testing purposes it will come handy to change the directory from a
batch-mode script. Remove the check forbidding use of the 'cd' command
in batch mode.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Until now when '--name' was used the parent was not printed and the
option was ignored. One option would be to declare the options mutually
exclusive, but for testing it may come handy to print both the snapshot
name and parent. Adjust the code to print them tab-separated and adjust
the docs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@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.
Certain arguments where it makes sense are annotated as 'positional' too
in this patch.
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>
Annotate arguments which can be unintentionally parsed positionally.
(See previous commits for explanation.)
The pool name is optional but in all cases it can be promoted to an
optional positional argument so that it can be properly aligned with the
expectations of the parser.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Make certain optional arguments truly positional in cases when it makes
semantic sense.
Previously it wasn't possible to have optional positional arguments, but
the parser filled them regardless, thus this preserves functionality.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Mark the 'backupxml' as positional optional and the 'checkpointxml' as
'unwanted_positional' to preserve the positional parsing quirk.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The argument is optional thus couldn't be marked as positional until now,
despite being parsed positionally.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'snapshotname' argument is optional as by default "current" snapshot
is considered. Regardless of that we should treat it as positional as
it's the common usage. This is now possible as we can have one optional
positional argument.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The intended use of those commands is to use the argument directly
without the flag. Since the argument is optional in all cases we
couldn't declare them as positional until now.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We already allow a optional positional _ARGV argument but there's no
reason why any other argument type could not be allowed this way.
Add checks that there's just one such argument and it's placed after
required positional arguments.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The argument was being parsed positionally due to the command parser
quirk as we didn't opt out of it.
Since the code in virshLookupCheckpoint requires that the checkpointname
is present we can mark all the options as positional and required and
remove the redundant check from virshLookupCheckpoint.
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.)
Currently virsh accepts the arguments such as:
$ virsh attach-disk --print-xml 1 2 3 4 5 6 7 8 9 10
<disk type='file' device='10'>
<driver name='5' type='6' iothread='7' cache='8' io='9'/>
<source file='2'/>
<target dev='3' bus='4'/>
</disk>
While making virsh require the flags is technically a breaking change,
there were multiple instances where arguments were added to the argument
list thus changing the order the positional arguments would be
interpreted as. Examples are commits: 7e157858b4, bc5a8090af,
ca21d75d25. As of such there are multiple breaks of compatibility for
the positional arguments.
As of such, require the option flag for all optional arguments with
value for 'virsh attach-disk'.
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.)
Annotate '--migrateuri', '--graphicsuri', '--listen-address', '-dname',
'--timeout', '--xml', '--migrate-disks' and '--disks port' as
'unwanted_positional'. These were declared in chronological order per
git history.
All others are annotated with VSH_OFLAG_REQ_OPT which makes the parser
require the '--optionname'. This is due to the fact that '--disks-uri'
was introduced later and put in front of others declared earlier
breaking the order they would be accepted, thus changing the behaviour
between versions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Make all of the tunable parameter flags require the option name (don't
parse them positionally).
While techically this would be a breaking change if anyone were to
specify the tunable values positionally this is not the case as the
first two tunables are not compatible with each other:
$ virsh blkdeviotune cd vda 4 5
error: Unable to change block I/O throttle
error: invalid argument: total and read/write of bytes_sec cannot be set at the same time
The above is produced by all implementations of the API (qemu and test
drivers). It is true that the first tunable can be specified
positionally (--total-bytes-sec) but it is misleading and shoud not be
allowed either.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
While previous fixes kept the help output unchanged as base for the
refactors it turns out that the formatting of help for argv options is
wrong.
Specifically in SYNOPSIS the non-positional _ARGV would have the option
name in square brackets (which in other cases means that given thing is
optional) despite being required.
Similarly in the DESCRIPTION section positional versions would not show
the optional argument name and also didn't use the three dots to signal
that it can be used multiple times.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In most cases it's the usual/recommended way to use those commands:
$ virsh qemu-monitor-command VMNAME cmd args args args
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Our documentation in most places explicitly mentions --diskspec and it
was never meant to be positional, although we can't change the parser
any more. Annotate them as 'unwanted_positional'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Based on the rationale in previous commit, all commands which were
parsed as positional but not documented as such will be annotated with
this flag.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
While the virsh option definitions specify (either explicitly after
recent refactors, or implicitly before) whether an argument is
positional or not, the actual parser is way more lax and actually and
allows also arguments which were considered/documented as non-positional
to be filled positionally unless VSH_OFLAG_REQ_OPT is used in the flags.
This creates situations such as 'snapshot-create-as' which has the
following docs:
SYNOPSIS
snapshot-create-as <domain> [--name <string>] [--description <string>]
[--print-xml] [--no-metadata] [--halt] [--disk-only]
[--reuse-external] [--quiesce] [--atomic] [--live] [--validate]
[--memspec <string>] [[--diskspec] <string>]...
Thus showing as if '--name' and '--description' required the option, but
in fact the following happens when only positionals are passed:
$ virsh snapshot-create-as --print-xml 1 2 3 4 5
<domainsnapshot>
<name>2</name>
<description>3</description>
<disks>
<disk name='4'/>
<disk name='5'/>
</disks>
</domainsnapshot>
In the above example e.g. '--memspec' is not populated.
This disconnect makes it impossible to refactor the parser itself and
allows users to write buggy interactions with virsh.
In order to address this we'll be annotating every single of these
unwanted positional options as such so that this doesn't happen in the
future, while still preserving the quirk in the parser.
This patch introduces a tool which outputs list of options which are not
marked as positional but are lacking the VSH_OFLAG_REQ_OPT flag.
This tool will be removed once all the offenders found by it will be
addressed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The macro is used in just one place and the definition of the option is
going to be modified. Inline the macro.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The macro is used in one place only and the command definition will be
altered. Inline it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Upcoming patches will need to tweak some of the properties of the
command. Since the macro is used in just two places expand it inline.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
- move the check that completer_flags are 0 if no completer is set
into a common place and remove duplication
- add check that _BOOL arguments are not positional
- add missing checks to _ALIAS
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
All virsh commands in non-quiet mode append another separator line thus
having two is unnecessary and in quiet mode it still has a trailing
blank line. Remove it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As this command was introduced in this release add the flag requiring to
pass optionname.
This is needed to actually disallow positional parsing of the value
despite documenting that the flag name is required.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
vshAdmCatchDisconnect requires non-NULL structure vshControl for
getting connection name (stored at opaque), but
virAdmConnectRegisterCloseCallback at vshAdmConnect called it
with NULL.
Signed-off-by: Adam Julis <ajulis@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-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>
Use the new flags to do the decisions which will also fix the case when
an _INT option is required but non-positional.
This fixes the help for the 'timeout' argument of 'daemon-timeout'
virt-admin command:
SYNOPSIS
- daemon-timeout <timeout>
+ daemon-timeout --timeout <number>
[...]
OPTIONS
- [--timeout] <number> number of seconds the daemon will run without any active connection
+ --timeout <number> number of seconds the daemon will run without any active connection
Resolves: https://issues.redhat.com/browse/RHEL-25993
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now that the code was refactored and proved identical, remove the checks
so that they don't impede further refactors.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This is logically enforced by existing checks, thus we can formalize it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In at least one case we've wanted a mandatory argument which requires
the explicit flag. Fix the assumption before converting everything over
to the new flags.
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>
There's just one command taking a list of domains as argument, thus
declare it inline.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Declare one argument per line, separate disticnt conditions by newline,
move some checks earlier.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Extract flag check to a separate variable and replace ternary operators
by normal conditions and use allocated buffer instead of a static one
to improve readability.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Extract flag check to a separate variable and replace ternary operators
by normal conditions and directly output the text rather than using
extra variable to improve readability.
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>
The new option dumps the full help outputs for every command so that
it's possible to conveniently check that subsequent refactors will not
impact any of the external functionality.
No man page entry is needed as the command is internal/undocumented.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Some description of virsh commands referenced itself in a multi-line
example of usage, which is pointless as virsh help already shows how to
use the command:
.data = N_("Get or set the current memory parameters for a guest"
" domain.\n"
" To get the memory parameters use following command: \n\n"
" virsh # memtune <domain>")
Change it to just state what the command does and leave the example for
the help printer.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use a switch statement to cover all cases and check for missing
completers for arguments declared as VSH_OT_ARGV.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It's obvious that a command is an alias when the 'alias' property is
set, thus an extra flag is redundant. Remove it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a check that the default 0 assignment will not mean that an option
is considered to be VSH_OT_BOOL.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The command invoking the code is internal and meant for developers,
there's no point in translating the errors.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
None of the clients use the 'command set' approach and other pieces of
code such as the command validator already assume that command groups
are in use. Remove the unused 'command set' stuff.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Remove some code repetition between desc and net-desc commands.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When editing the title of a domain or network via the `desc` or
`net-desc` commands, we strip the final newline that is added by some
editors.
Do the same when editing the description as well.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Introduce the domdisplay-reload command to make the domain reload
its graphics certificates
#virsh domdisplay-reload <domain> --type <type>
Signed-off-by: Zheng Yan <yanzheng759@huawei.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Trying to print pages of a size larger than the UINT_MAX of the
given platform (for example, 4G on 64-bit ARM), results in a
system error even though this is a legitimate request.
The vshCommandOptScaledInt() used for parsing the pagesize is
given UINT_MAX as the upper limit. The parsed value is then
divided by 1024 and fed to virNodeGetFreePages() which expects an
unsigned int. We can't change the public API but the upper limit
can be raised by the factor of 1024.
Resolves: https://issues.redhat.com/browse/RHEL-23608
Signed-off-by: Adam Julis <ajulis@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Allow to modify a node device by using virNodeDeviceDefineXML() to align
its behavior with other drivers define methods.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Now that we can filter persistent and transient node devices in
virConnectListAllNodeDevices(), add these switches also to the
virsh nodedev-list command.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Allow to dump the XML of the persistent mdev when the mdev has been
started instead of the current state only.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@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>
The virshGetDBusDisplay() function is declared to return a
pointer and yet, in one error path false is returned. Switch the
statement to return NULL, which is what other error paths use to
indicate an error.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The admin connection defaults to the system-wide 'libvirtd' daemon to
manage (libvirtd:///system). As we've now switched to modular daemons
this will not work for most users out of the box:
$ virt-admin version
error: Failed to connect to the admin server
error: no valid connection
error: Failed to connect socket to '/run/user/1000/libvirt/libvirt-admin-sock': No such file or directory
As we don't want to assume which daemon the user wants to manage in the
modular topology there's no reasonable default to pick.
Give a hint to the users to use the '-c' if the connection to the
default URI fails:
$ virt-admin version
NOTE: Connecting to default daemon. Specify daemon using '-c' (e.g. virtqemud:///system)
error: Failed to connect to the admin server
error: no valid connection
error: Failed to connect socket to '/run/user/1000/libvirt/libvirt-admin-sock': No such file or directory
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
virsh already stores the connection URI in 'ctl->connname', use that
instead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Probe the current URI so that other places don't need to do that.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virsh already stores the connection URI in 'ctl->connname', use that
instead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Probe the current URI so that other places don't need to do that.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Currently this enum is defined in domain_conf.h and named
virDomainHostdevSubsysPCIDriverType. I want to use it in parts of the
network and networkport config, so am moving its definition to
device_conf.h which is / can be included by all interested parties,
and renaming it to match the name of the corresponding XML attribute
("driver name"). The name change (which includes enum values) does cause a
lot of churn, but it's all mechanical.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Allow users to easily resize 'raw' images on block devices to the full
capacity of the block device. Obviously this won't work on file-backed
storage (filling the remaining capacity is most likely wrong) or for
formats with metadata due to the overhead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
VSH_ALTERNATIVE_OPTIONS takes just the name of the options instead of
requiring also the getter functions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As the error message states we want to check that one of
'--copy-storage-all' or '--copy-storage-inc' is used, but the condition
mentioned VIR_MIGRATE_NON_SHARED_DISK twice.
Fixes: 1c2bd205ed
Resolves: https://issues.redhat.com/browse/RHEL-17596
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The API treats them as mutually exclusive and interlocks them at the
library handler. Provide better error in virsh.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
For the xpath "/domain/cpu/@mode", it will return a list type not a
string. Use string() method in the xpath for the string result.
Fixes: 6b95437c17
Signed-off-by: Han Han <hhan@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>
Now that we have virXMLParseWithIndent() and
virXMLParseStringCtxtWithIndent(), we can use them directly and
drop calls to xmlKeepBlanksDefault().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We recently unified all services and sockets, except a couple
were missed. Finish the job.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Xcode 15, which provides the compiler toolchain for building libvirt
on macOS has switched to a new linker that warns about duplicated
"-lblah" options on the ld commandline. In practice this is impossible
to prevent in a large project, and also harmless.
Fortunately the new ld command also has an option,
-no_warn_duplicate_libraries, that supresses this harmless/pointless
warning, meson has a simple way to check if that option is supported,
and libvirt's meson.build files already have examples of adding an
option to the ld commandline if it's available.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
When starting a guest via libvirt (`virsh create --console`), early
console output was missed because the guest was started first and then
the console was attached. This patch changes this to the following
sequence:
1. create a paused transient guest
2. attach the console
3. resume the guest
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When starting a guest via libvirt (`virsh start --console`), early
console output was missed because the guest was started first and then
the console was attached. This patch changes this to the following
sequence:
1. create a paused guest
2. attach the console
3. resume the guest
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This patch adds the command line flag `--resume` to the `virsh console`
command. This resumes a paused guest after connecting to the console.
This might be handy since it's a "common" pattern to start a guest
paused, connect to the console, and then resume it so as not to miss any
console messages.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The function returns how many array items were filled in, but virsh
never checked for anything other than errors. Just to make sure this
does not report invalid data, even though the only possibility would be
reporting 0 free pages, check the returned data so that possible errors
are detected.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
(cherry picked from commit c35ba64d18235bfe35617cb3d6d6cc778f6d166d)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When changing the metadata via virNetworkSetMetadata(), we can
now emit an event to notify the app of changes. This is useful
when co-ordinating different applications read/write of custom
metadata.
Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If a domain has no snapshots and 'virsh snapshot-list' is called,
this gets all the way down to virshSnapshotListCollect() which
then collects all snapshots (none), and passes them to qsort()
which doesn't like being called with NULL:
extern void qsort (void *__base, size_t __nmemb, size_t __size,
__compar_fn_t __compar) __nonnull ((1, 4));
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/533
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The --help output of virsh and virt-admin shows supported options
and commands and as such contains new lines. Both these strings
are marked for translation btw. But the way they are formatted
now ('\n' being at the start of new line instead at the end of
the previous) makes it hard to create a syntax-check rule for
'translation message on one line' (next commit).
Reformat both strings a bit (no user visible change though).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The 'buf', 'sa' and 'hints' stack allocated helper variables are never
used together. Decrease the stack memory usage by scoping them off into
do-while blocks.
In this instance we do not want to use dynamic allocation as this is the
NSS module.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
The 'port' buffer is passed to 'getnameinfo' which is supposed to fill
it but it's not actually later used. Drop the buffer as 'getnameinfo'
allows NULL arguments if they are not needed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Break up the argument and variable declarations to the preferred style.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Use virXMLNodeGetSubelement to find needed subelements.
In virshUpdateDiskXML this commit removes the code which keeps XML
formatting tidy, but that is not needed for the code to format proper
XMLs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@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>
Fixes the following bug:
Command: `net-desc --config [--title] my_network`
Expected Output: Title/Description of persistent config
Output: Title/Description of live config
This was caused due to the usage of a single `flags` variable in
`virshGetNetworkDescription()` which ended up in a wrong enum being
passed to `virNetworkGetMetadata()` (enum being that of LIVE instead of
CONFIG).
Although the domain object has the same code, this didn't cause a problem
there because the enum values of `VIR_DOMAIN_INACTIVE_XML` and
`VIR_DOMAIN_METADATA_CONFIG` turn out to be the same (1 << 1), whereas
they are not for network equivalent ones (1 << 0, 1 << 1).
Signed-off-by: K Shiva Kiran <shiva_kr@riseup.net>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
If the CPU family/model/stepping are provided on the command line, but
the firmware is being automatically extracted from the libvirt guest,
we try to build the VMSA too early. This leads to an exception trying
to parse the firmware that has not been loaded yet. We must delay
building the VMSA in that scenario.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The SEV-ES boot measurement includes the initial CPU register state
(VMSA) and one of the fields includes the CPU identification. When
building a VMSA blob we get the CPU family/model/stepping from the
host capabilities, however, the VMSA must reflect the guest CPU not
host CPU. Thus using host capabilities is only when whe the guest
has the 'host-passthrough' CPU mode active. With 'host-model' it is
cannot be assumed host and guest match, because QEMU may not (yet)
have a named CPU model for a given host CPU.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
