On Thu, Mar 14, 2024 at 16:51:31 -0400, Sergio Durigan Junior wrote:
> On Wednesday, March 13 2024, I wrote:
>
> > On Tuesday, March 12 2024, Jiri Denemark wrote:
> >
> >> On Wed, Jan 24, 2024 at 22:19:56 -0500, Sergio Durigan Junior wrote:
> >>> On Monday, January 22 2024, I wrote:
> >>>
> >>> >
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
---
tools/virsh-domain.c | 1 +
1 file changed, 1 inserti
Part 2 was supposed to be the refactor of the parser, but it turns out
that our annotations of arguments and quirks of the parser itself
resulted with basically every optional argument with value being also
considered for positional parisng.
This series explains where this happens and annotates al
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
---
tools/virsh-domain-event.c | 7 +--
tools/virsh-domain.c | 14 +++---
tools/virsh.h | 13 ---
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
---
tools/vsh.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index f96071
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
---
tools/virsh-network.c | 17 +
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/tools/virsh-network.c b/tools/virsh-network
- 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
---
tools/vsh.c | 21 +++--
1 file changed, 19 insert
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
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
---
tools/vsh.c | 12 ++--
tools/vsh.h | 7 +++
2 files changed, 17 insertions(+), 2 deletions(-)
diff --
The macro is used in one place only and the command definition will be
altered. Inline it.
Signed-off-by: Peter Krempa
---
tools/virsh-host.c | 7 +--
tools/virsh.h | 7 ++-
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index
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
---
tools/virsh-checkpoint.c | 1 +
tools/virsh-snapshot.c | 1 +
2 files c
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
---
tools/virsh-domain.c | 8
tools/virsh-network.c | 1 +
2 files changed, 9 insertions(+)
diff --git a/tools/virsh-domain.c b/tool
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
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 chronolog
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
While making virsh require the flags is technically a breaking change,
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
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
---
tools/virsh.c | 1 +
tools/virt-admin.c | 1 +
tools/vsh.c| 2 ++
3 file
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 virshLookupCheckpoin
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
---
tools/vsh.c | 20 +++
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
---
tools/virsh-snapshot.c | 6 +
The argument is optional thus couldn't be marked as positional until now,
despite being parsed positionally.
Signed-off-by: Peter Krempa
---
tools/virsh-checkpoint.c | 1 +
tools/virsh-snapshot.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/tools/virsh-checkpoint.c b/tools/virsh-check
Mark the 'backupxml' as positional optional and the 'checkpointxml' as
'unwanted_positional' to preserve the positioanl parsing quirk.
Signed-off-by: Peter Krempa
---
tools/virsh-backup.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/virsh-backup.c b/tools/virsh-backup.c
index 7a78
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
---
tools/virsh-domain-monitor.c | 3
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: Pete
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
2
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
---
tools/virsh-pool.c | 19 +++
1 file ch
Replace the last bitwise flag with a separate member.
Signed-off-by: Peter Krempa
---
tools/virsh-domain-monitor.c | 2 +-
tools/virsh-domain.c | 6 +++---
tools/virsh.c| 2 +-
tools/virt-admin.c | 6 +++---
tools/vsh.c | 6 +++---
tools/vsh.h
Drop the last enum member VSH_OFLAG_NONE and remove the 'flags' variable
from vshCmdOptDef.
Signed-off-by: Peter Krempa
---
tools/virsh-domain-monitor.c | 3 ---
tools/virsh-domain.c | 4
tools/virsh-network.c| 1 -
tools/virsh-pool.c | 3 ---
tools/vsh.c
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 desi
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
2
when the thread pool is dynamically expanded, threads should
not be created from the existing workers; they should be created
from the newly expanded workers
Signed-off-by: Wei Gong
---
src/util/virthreadpool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virthrea
Please advise on what actions (if any) should be taken so that the changes in
the patch is merged into main.
Cheers,
Mostafa Mahmoud.
From: m kamal
Sent: Tuesday, March 12, 2024 5:58 PM
To: Peter Krempa
Cc: devel@lists.libvirt.org
Subject: Re: [PATCH v2] Remov
32 matches
Mail list logo
