Re: [Qemu-devel] [PATCH] monitor: print help for command errors

[Markus Armbruster]

Bandan Das <address@hidden> writes:

> Unlike machines, humans will be (mostly) appreciative on seeing
> help output when a command fails due to incorrect syntax or input.
> By default, print output of help_cmd() to the monitor in such cases.
> The only exceptions are if a command does not exist or parsing
> failed for some other reason.
>
> Before:
> (qemu) drive_add usb_flash_drive
> drive_add: string expected
> After:
> (qemu) drive_add usb_flash_drive
> drive_add: string expected
> Usage:
> drive_add [[<domain>:]<bus>:]<slot>
> [file=file][,if=type][,bus=n]
> [,unit=m][,media=d][,index=i]
> [,cyls=c,heads=h,secs=s[,trans=t]]
> [,snapshot=on|off][,cache=on|off]
> [,readonly=on|off][,copy-on-read=on|off] -- add drive to PCI storage 
> controller

I'm sure users will appreciate a little help on error.  What I don't
appreciate is having to search lengthy output for the error message :)

Our help consists of two parts:

1. .params, of the form: COMMAND-NAME PARAMETER-NAME...
2. .help, which is a free-form HELP-TEXT

Following the help command's example, you print them run together like

    COMMAND-NAME PARAMETER-NAME... -- HELP-TEXT

which can easily lead to output lines that are too long for easy
reading, e.g.

    hostfwd_add [vlan_id name] 
[tcp|udp]:[hostaddr]:hostport-[guestaddr]:guestport -- redirect TCP or UDP 
connections from host to guest (requires -net user)

Worse, a few commands have multi-line HELP-TEXT:

    block_job_cancel
    drive_backup
    drive_mirror
    dump-guest-memory
    migrate
    migrate_set_cache_size
    pcie_aer_inject_error
    snapshot_blkdev
    snapshot_blkdev_internal
    snapshot_delete_blkdev_internal

One even has multi-line PARAMETER-NAME..: drive_add.  That produces the
ugliest help of all; thanks for picking it as your example.

Two ways to avoid burying the error message in an avalanche of help:

A. Instead of possibly lengthy help, say how to get it

        (qemu) drive_add usb_flash_drive
        drive_add: string expected
        Try "help drive_add" for more information.

   For what it's worth, it's what many shell utilities do.

B. Ensure the help is brief, and easy to read

   Don't run together PARAMETER-NAME and HELP-TEXT, put them on separate
   lines.

   Print only the first line of HELP-TEXT.  Check the HELP-TEXTs have a
   sensible first line.

   When this doesn't print the full HELP-TEXT (because it has more than
   one line), add how to get complete help.

        (qemu) drive_backup 
        drive_backup: block device name expected
        Usage: drive_backup [-n] [-f] device target [format]
        initiates a point-in-time copy for a device
        Try "help drive_backup" for more information.

I prefer A.

Speaking of help, output of "help" is awful.  It's a flood of badly
formatted text, with long lines and weird indentation.  I'd rather see
one line per command, then a final line explaining how to get more help
on a specific command.  Something like

        (qemu) help
        List of commands:
        acl_add -- Add a match rule to the access control list
        [...]
        drive_backup -- Initiate a point-in-time copy for a device
        [...]
        xp -- Physical memory dump starting at 'addr'
        Try "help" followed by a command name for full command help.
        (qemu) help drive_backup
        drive_backup [-n] [-f] device target [format]
        Initiate a point-in-time copy for a device
        The device's contents are copied to the new image file,
        excluding data that is written after the command is started.
        The -n flag requests QEMU to reuse the image found in
        new-image-file, instead of recreating it from scratch.  The -f
        flag requests QEMU to copy the whole disk, so that the result
        does not need a backing file.
        (qemu)

Better, but "help" still floods the terminal with about 100 commands.
gdb avoids this by grouping commands:

        (gdb) help
        List of classes of commands:

        aliases -- Aliases of other commands
        breakpoints -- Making program stop at certain points
        data -- Examining data
        files -- Specifying and examining files
        internals -- Maintenance commands
        obscure -- Obscure features
        running -- Running the program
        stack -- Examining the stack
        status -- Status inquiries
        support -- Support facilities
        tracepoints -- Tracing of program execution without stopping the program
        user-defined -- User-defined commands

        Type "help" followed by a class name for a list of commands in that 
class.
        Type "help all" for the list of all commands.
        Type "help" followed by command name for full documentation.
        Type "apropos word" to search for commands related to "word".
        Command name abbreviations are allowed if unambiguous.
        (gdb) help stack
        Examining the stack.
        The stack is made up of stack frames.  Gdb assigns numbers to stack 
frames
        counting from zero for the innermost (currently executing) frame.

        At any time gdb identifies one frame as the "selected" frame.
        Variable lookups are done with respect to the selected frame.
        When the program being debugged stops, gdb selects the innermost frame.
        The commands below can be used to select other frames by number or 
address.

        List of commands:

        backtrace -- Print backtrace of all stack frames
        bt -- Print backtrace of all stack frames
        down -- Select and print stack frame called by this one
        frame -- Select and print a stack frame
        return -- Make selected stack frame return to its caller
        select-frame -- Select a stack frame without printing anything
        up -- Select and print stack frame that called this one

        Type "help" followed by command name for full documentation.
        Type "apropos word" to search for commands related to "word".
        Command name abbreviations are allowed if unambiguous.
        (gdb) help bt
        Print backtrace of all stack frames, or innermost COUNT frames.
        With a negative argument, print outermost -COUNT frames.
        Use of the 'full' qualifier also prints the values of the local 
variables.
        Use of the 'no-filters' qualifier prohibits frame filters from executing
        on this backtrace.

        (gdb)