Re: git: 68bff4a07e3f - main - Allow GEOM utilities to specify a -v option.

Fri, 05 Nov 2021 05:41:33 -0700 

Hi Kirk,

On 29 Oct 2021, at 22:12, Kristof Provost wrote:

On 29 Oct 2021, at 7:52, Kirk McKusick wrote:

The branch main has been updated by mckusick:
URL: https://cgit.FreeBSD.org/src/commit/?id=68bff4a07e3fa6c30a0c0ff6cf5f0bef95dcbd72 

commit 68bff4a07e3fa6c30a0c0ff6cf5f0bef95dcbd72
Author:     Kirk McKusick <[email protected]>
AuthorDate: 2021-10-29 05:49:48 +0000
Commit:     Kirk McKusick <[email protected]>
CommitDate: 2021-10-29 05:50:50 +0000

    Allow GEOM utilities to specify a -v option.
Geom utilities (geli(8), glabel(8), gmirror(8), gpart(8), gmirror(8), 
    gmountver(8), etc) all use the geom(8) utility as their back end
    to process their commands and pass them into the kernel. Creating
a new utility requires no more than filling out a template describing the commands and arguments that the utility supports. Consider the 
    specification for the very simple gmountver(8) utility:

    struct g_command class_commands[] = {
            { "create", G_FLAG_VERBOSE | G_FLAG_LOADKLD, NULL,
                {
                    G_OPT_SENTINEL
                },
                "[-v] prov ..."
            },
            { "destroy", G_FLAG_VERBOSE, NULL,
                {
                    { 'f', "force", NULL, G_TYPE_BOOL },
                    G_OPT_SENTINEL
                },
                "[-fv] name"
            },
            G_CMD_SENTINEL
    };

    It has just two commands of its own: "create" and "destroy" along
    with the four standard commands "list", "status", "load", and
    "unload" provided by the base geom(8) utility. The base geom(8)
utility allows each command to use the G_FLAG_VERBOSE flag to specify 
    that a command should accept the -v flag and when the -v flag is
given the utility prints "Done." if the command completes successfully. In the above example, both of the commands set the G_FLAG_VERBOSE, so have the -v option available. In addition the "destroy" command 
    accepts the -f boolean flag to force the destruction.
If the "destroy" command wanted to also print out verbose information, 
    it would need to explicitly declare its intent by adding a line:

                    { 'v', "verbose", NULL, G_TYPE_BOOL },
Before this change, the geom utility would silently ignore the above line in the configuration file, so it was impossible for the utility 
    to know that the -v flag had been set on the command. With this
    change a geom command can explicitly specify a -v option with a
line as given above and handle it as it would any other option. If 
    both a -v option and G_FLAG_VERBOSE are specified for a command
    then both types of verbose information will be output when that
    command is run with -v.

    MFC after:    1 week
    Sponsored by: Netflix
---
 sbin/geom/core/geom.c | 19 ++++++++++++-------
 sbin/geom/core/geom.h | 13 +++++++++++++
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/sbin/geom/core/geom.c b/sbin/geom/core/geom.c
index 58b33a067700..2e0d8683df49 100644
--- a/sbin/geom/core/geom.c
+++ b/sbin/geom/core/geom.c



@@ -440,7 +445,7 @@ set_flags(struct g_command *cmd)
 {
        unsigned flags = 0;

-       if ((cmd->gc_flags & G_FLAG_VERBOSE) != 0 && verbose)
+       if ((cmd->gc_flags & G_FLAG_VERBOSE) != 0)
                flags |= G_FLAG_VERBOSE;

        return (flags);
Given https://reviews.freebsd.org/D32736 I wonder if the removal of the verbose check here was correct.
If I'm reading this code right we now always set the verbose flag for any subcommand that supports it (i.e. has G_FLAG_VERBOSE set).
That leads to commands such as glabel always acting as if '-v' was specified.
The set_flags() output is only used if g_func is set, which isn't the case in any of the examples in the commit message, so I wonder if that case was overlooked.
Have you had a chance to look at this? It’s causing multiple failures in the geom regression tests. 

Thanks,
Kristof

Reply via email to