Discussion:
[linux-lvm] pvscan: bugs in manpage and implementation
Tom Hale
2017-09-18 07:52:04 UTC
Permalink
Hi,

MAN PAGE

In http://man7.org/linux/man-pages/man8/pvscan.8.html I see the
following issues:

* The string "-a--activate" appears several times. Should be:
"-a|--activate"

* "-a|--activate y|n|ay" is mentioned, but later on:
"Only ay is applicable." Please remove "y|n|".

PROGRAM

# pvscan --activate ay
Command does not accept option: --activate ay.

The message is confusing. It would be better to say "--activate requires
--cache"

In fact, why not drop the "ay" argument all together and just allow
"--cache --activate"?

Cheers,
Tom
Zdenek Kabelac
2017-09-18 11:49:20 UTC
Permalink
Post by Tom Hale
Hi,
MAN PAGE
In http://man7.org/linux/man-pages/man8/pvscan.8.html I see the
"-a|--activate"
"Only ay is applicable." Please remove "y|n|".
PROGRAM
# pvscan --activate ay
Command does not accept option: --activate ay.
The message is confusing. It would be better to say "--activate requires
--cache"
In fact, why not drop the "ay" argument all together and just allow
"--cache --activate"?
Hi

In general - we use internal logic for parsing options and its parameters and
in this case the options 'takes' more parameters then 'pvscan' can accept.

We could possibly add 'extra' special option e.i. :
--pvscanautoactivation

but we've considered at given time it's not worth since normally no user is
ever supposed to use it in regular user space (since noone likes to see more
and more individal options doing nearly the same thing)

This option is mostly targeted for execution inside udev rules - and since
there was some 'evolution' how to make this working - it's now not really
worth to make it more complicated - since we would have to keep old option
present anyway for backward compatibility.

Regards

Zdenek
David Teigland
2017-09-18 15:56:13 UTC
Permalink
Post by Tom Hale
Hi,
MAN PAGE
In http://man7.org/linux/man-pages/man8/pvscan.8.html I see the
"-a|--activate"
Yes, that's odd, the | exists in the source, but isn't being printed.
I'll just change to --activate for now.
Post by Tom Hale
"Only ay is applicable." Please remove "y|n|".
Unfortunately --activate is one of those options that was given different
acceptable values depending on the command, and we haven't added a special
case to the code that generates man pages to display it differently.
Post by Tom Hale
PROGRAM
# pvscan --activate ay
Command does not accept option: --activate ay.
The message is confusing. It would be better to say "--activate requires
--cache"
Yes, this is a limitation in the new code that matches what you have typed
to a specific command. All possible commands are now defined here:
https://sourceware.org/git/?p=lvm2.git;a=blob_plain;f=tools/command-lines.in;hb=HEAD

It matches 'pvscan --activate' to the 'pvscan' command (which doesn't
accept -a, thus the error), rather than to the 'pvscan --cache' command
(which does accept -a).

Loading...