| Message ID | cover.1707328606.git.mjt@tls.msk.ru (mailing list archive) |
|---|---|
| Headers | show |
| Series | Attempt to make qemu-img options consistent and --help working | expand |
On Wed, Feb 07, 2024 at 08:58:09PM +0300, Michael Tokarev wrote: > - create more or less consistent set of options between different > subcommands > - provide long options which can be used without figuring out which > -T/-t, -f|-F|-O etc to use for which of the two images given > - have qemu-img --help provide just a list of subcommands > - have qemu-img COMMAND --help to describe just this subcommand > - get rid of qemu-img-opts.hx, instead finish documentation in > qemu-img.rst based on the actual options implemented in > qemu-img.c. Yes, yes, yes, yes & more yes :-) I cry every time I have to read the qemu-img --help output, and I'm not much of a fan of the man page either to be fair, as I don't like the global list of options at the top which is divorced from which commands actually use them. These days I see many programs with subcommands switching to a separate man page per sub-command. Still, I'm not asking you todo that too, its just an idea for the gallery. > I started converting subcommands one by one, providing long options > and --help output. And immediately faced some questions which needs > wider discussion. > > o We have --image-opts and --target-image-opts. Do we really need both? > In my view, --image-opts should be sort of global, turning *all* > filenames on this command line into complete image specifications, > there's no need to have separate image-opts and --target-image-opts. > Don't know what to do wrt compatibility though. It shouldn't be made > this way from the beginning. As a possible solution, introduce a new > option and deprecate current set. This is basically a crutch for incomplete conversion of the block layer APIs, which meant we had a situation where we wanted to use image opts syntax for the source, but were unable todo so for the target: commit 305b4c60f200ee8e6267ac75f3f5b5d09fda1079 Author: Daniel P. Berrangé <berrange@redhat.com> Date: Mon May 15 17:47:11 2017 +0100 qemu-img: introduce --target-image-opts for 'convert' command The '--image-opts' flag indicates whether the source filename includes options. The target filename has to remain in the plain filename format though, since it needs to be passed to bdrv_create(). When using --skip-create though, it would be possible to use image-opts syntax. This adds --target-image-opts to indicate that the target filename includes options. Currently this mandates use of the --skip-create flag too. we do have internal support for creating block devices using the full QAPI schema, via BlockdevCreateOptions. I'm not sure if bdrv_create can be made to create using the image-opts syntax. If it can, there is still the additional problem that after creation it then needs to re-open the file, and the image-opts for open is defined by BlockdevOptions not BlockdevCreateOptions. So we would need a way to convert from the latter to the former. With regards, Daniel
Hello Michael, Such changes are long overdue. However given the complexity of commands and arguments, maybe it'd be a good idea to write a code generator for the command line interface, This way you could also generate --help outputs, manpages, shell completions just from the command line spec we'd use to generate the argv parsing routines. On Wed, 7 Feb 2024 at 19:58, Michael Tokarev <mjt@tls.msk.ru> wrote: > > This is an incomplete first attempt only, there's a lot left to do. > > All the options in qemu-img is a complete mess, - no, inconsistent or > incomplete syntax in documentation, many undocumented options, option > names are used inconsistently and differently for different commands, > no long options exists for many short options, --help output is a huge > mess by its own, and the way how it all is generated is another story. > docs/tools/qemu-img.rst with qemu-img-opts.hx is yet another. > > I hoped to fix just an option or two, but it ended up in a large task, > and I need some help and discussion, hence the RFC. > > The idea is: > > - create more or less consistent set of options between different > subcommands > - provide long options which can be used without figuring out which > -T/-t, -f|-F|-O etc to use for which of the two images given > - have qemu-img --help provide just a list of subcommands > - have qemu-img COMMAND --help to describe just this subcommand > - get rid of qemu-img-opts.hx, instead finish documentation in > qemu-img.rst based on the actual options implemented in > qemu-img.c. > > I started converting subcommands one by one, providing long options > and --help output. And immediately faced some questions which needs > wider discussion. > > o We have --image-opts and --target-image-opts. Do we really need both? > In my view, --image-opts should be sort of global, turning *all* > filenames on this command line into complete image specifications, > there's no need to have separate image-opts and --target-image-opts. > Don't know what to do wrt compatibility though. It shouldn't be made > this way from the beginning. As a possible solution, introduce a new > option and deprecate current set. > > o For conversion (convert, dd, etc), we've source and destination, > so it's easy to distinguish using long options, like --source-format > --target-cache etc (-t/-T -f/-F is a mess here already). What to > do with compare? --format1 --format2 is ugly, maybe --a-format and > --b-format? Maybe we can get off with --source (a) and --target (b) > instead of filename1 & filename2? > (--cache in this context is global for both). > > o qemu-img convert. It's the most messy one, and it is not really > documented (nor in qemu-img.rst , eg there's nothing in there about > FILENAME2, -B is difficult to understand, etc). > At the very least, I'd say the options should be > --source-format, --source-cache etc > --target-format, --target-options > --target-image-opts - this shold go IMHO > > o check and commit - inconsistent cache flags? > In convert, cache is backwards (source/target)? > > At first, I tried to have more or less common option descriptions, > using various parameters, variables or #defines, but in different > commands the same options has slightly different wording, and in > some option names are different, so it looks like it's best to > have complete text in each subcommand. > > > Michael Tokarev (8): > qemu-img: pass current cmdname into command handlers > qemu-img: refresh options/--help for "create" subcommand > qemu-img: factor out parse_output_format() and use it in the code > (this one has been sent in a separate patch, here it is just for completness) > qemu-img: refresh options/--help for "check" command > qemu-img: simplify --repair error message > qemu-img: refresh options/--help for "commit" command > qemu-img: refresh options/--help for "compare" command > qemu-img: refresh options/--help for "convert" command > > qemu-img.c | 352 ++++++++++++++++++++++++++++++++++------------------- > 1 file changed, 226 insertions(+), 126 deletions(-) > > -- > 2.39.2 > >
07.02.2024 22:01, Manos Pitsidianakis: > Hello Michael, > > Such changes are long overdue. However given the complexity of > commands and arguments, maybe it'd be a good idea to write a code > generator for the command line interface, This way you could also > generate --help outputs, manpages, shell completions just from the > command line spec we'd use to generate the argv parsing routines. I tried starting with just a set of common options in --help output, but quickly abandoned that idea. Even there, and I mentioned that in the email you're replying to, we should have slightly different text in different contexts. So it seems like it's better to just write it in two places. Two *different* places, - which is the --help output and qemu-img.rst documentation (from which the manpage is generated). The two places are really different, because --help is just a very brief usage/options summary, while the doc is a much more complete descriptive guide. There's one more context, - the option parsing code. There are ways to make it easier, like libpopt for example, but in my view, in an attempt to make life easier, it makes it even more complex. In my taste anyway, YMMV. In short, - while this stuff seems like a good candidate for some automation, it might be not, and the first step IMHO is to get the first task done, - to review and refresh all options, see what can be done with the messy difference of the meanings for subcommands, describe them. Maybe after that whole thing can be automated (if it's possible to do with this differently-named hell and with readable output). /mjt
Am 07.02.2024 um 18:58 hat Michael Tokarev geschrieben: > This is an incomplete first attempt only, there's a lot left to do. > > All the options in qemu-img is a complete mess, - no, inconsistent or > incomplete syntax in documentation, many undocumented options, option > names are used inconsistently and differently for different commands, > no long options exists for many short options, --help output is a huge > mess by its own, and the way how it all is generated is another story. > docs/tools/qemu-img.rst with qemu-img-opts.hx is yet another. > > I hoped to fix just an option or two, but it ended up in a large task, > and I need some help and discussion, hence the RFC. > > The idea is: > > - create more or less consistent set of options between different > subcommands > - provide long options which can be used without figuring out which > -T/-t, -f|-F|-O etc to use for which of the two images given > - have qemu-img --help provide just a list of subcommands > - have qemu-img COMMAND --help to describe just this subcommand The help desperately needs some cleanup like this, so thank you for doing something about it. > - get rid of qemu-img-opts.hx, instead finish documentation in > qemu-img.rst based on the actual options implemented in > qemu-img.c. You mean qemu-img-cmds.hx? The one advantage it has is that it makes it obvious if there is a mismatch in the options we show in the help output and in the documentation. But I'm not overly concerned either way. I would probably have left it alone just because leaving it is less work than changing it and the result isn't very different. > I started converting subcommands one by one, providing long options > and --help output. And immediately faced some questions which needs > wider discussion. > > o We have --image-opts and --target-image-opts. Do we really need both? > In my view, --image-opts should be sort of global, turning *all* > filenames on this command line into complete image specifications, > there's no need to have separate image-opts and --target-image-opts. > Don't know what to do wrt compatibility though. It shouldn't be made > this way from the beginning. As a possible solution, introduce a new > option and deprecate current set. I think it's better not to touch things like this. qemu-img is much more likely to be used directly by human users (and their small scripts) than QEMU itself, so we need to be even more careful with deprecating things. In fact, I'm not even sure if combining them would make it easier to use. Often, it's only source _or_ target that have a complicated setup that requires blockdev-type descriptions. As a human user, I prefer if I can still just use the file name for the other image instead of getting the full -blockdev verbosity there, too. > o For conversion (convert, dd, etc), we've source and destination, > so it's easy to distinguish using long options, like --source-format > --target-cache etc (-t/-T -f/-F is a mess here already). What to > do with compare? --format1 --format2 is ugly, maybe --a-format and > --b-format? Maybe we can get off with --source (a) and --target (b) > instead of filename1 & filename2? > (--cache in this context is global for both). For those commands where there is a source and a target, --source-format and --target-format have a clear advantage, they are easy to remember and hard to confuse. For compare, as you saw, there is no clear naming. 1/2 or a/b don't make the command any clearer than -f/-F. So maybe just don't add long versions there? > o qemu-img convert. It's the most messy one, and it is not really > documented (nor in qemu-img.rst , eg there's nothing in there about > FILENAME2, -B is difficult to understand, etc). > At the very least, I'd say the options should be > --source-format, --source-cache etc > --target-format, --target-options These seem obvious. (Actually, --target-options vs. just --options isn't completely obvious - after all, there are fundamentally no create options for source.) --source-* are also a bit weird because 'qemu-img convert' takes multiple source images and then it applies the same format to all of them. But that's more about how it works, so not a problem with the option _name_. > --target-image-opts - this shold go IMHO This one less so. Even generally speaking, changing interfaces incompatibly comes with a cost that is probably too big to justify it just for interfaces that look a bit nicer, but don't provide any new functionality. And as I said above, I don't agree that image-opts should be global. > o check and commit - inconsistent cache flags? > In convert, cache is backwards (source/target)? It's a bit messy because the cache mode options -T/-t work the opposite way of -f/-F. But I think the commands are consistent with each other? -T was added as a source cache parameter to all of the subcommands at the same time, in commit 40055951a. > At first, I tried to have more or less common option descriptions, > using various parameters, variables or #defines, but in different > commands the same options has slightly different wording, and in > some option names are different, so it looks like it's best to > have complete text in each subcommand. Yeah, let's not make this more complicated than it has to be. Kevin
This is an incomplete first attempt only, there's a lot left to do. All the options in qemu-img is a complete mess, - no, inconsistent or incomplete syntax in documentation, many undocumented options, option names are used inconsistently and differently for different commands, no long options exists for many short options, --help output is a huge mess by its own, and the way how it all is generated is another story. docs/tools/qemu-img.rst with qemu-img-opts.hx is yet another. I hoped to fix just an option or two, but it ended up in a large task, and I need some help and discussion, hence the RFC. The idea is: - create more or less consistent set of options between different subcommands - provide long options which can be used without figuring out which -T/-t, -f|-F|-O etc to use for which of the two images given - have qemu-img --help provide just a list of subcommands - have qemu-img COMMAND --help to describe just this subcommand - get rid of qemu-img-opts.hx, instead finish documentation in qemu-img.rst based on the actual options implemented in qemu-img.c. I started converting subcommands one by one, providing long options and --help output. And immediately faced some questions which needs wider discussion. o We have --image-opts and --target-image-opts. Do we really need both? In my view, --image-opts should be sort of global, turning *all* filenames on this command line into complete image specifications, there's no need to have separate image-opts and --target-image-opts. Don't know what to do wrt compatibility though. It shouldn't be made this way from the beginning. As a possible solution, introduce a new option and deprecate current set. o For conversion (convert, dd, etc), we've source and destination, so it's easy to distinguish using long options, like --source-format --target-cache etc (-t/-T -f/-F is a mess here already). What to do with compare? --format1 --format2 is ugly, maybe --a-format and --b-format? Maybe we can get off with --source (a) and --target (b) instead of filename1 & filename2? (--cache in this context is global for both). o qemu-img convert. It's the most messy one, and it is not really documented (nor in qemu-img.rst , eg there's nothing in there about FILENAME2, -B is difficult to understand, etc). At the very least, I'd say the options should be --source-format, --source-cache etc --target-format, --target-options --target-image-opts - this shold go IMHO o check and commit - inconsistent cache flags? In convert, cache is backwards (source/target)? At first, I tried to have more or less common option descriptions, using various parameters, variables or #defines, but in different commands the same options has slightly different wording, and in some option names are different, so it looks like it's best to have complete text in each subcommand. Michael Tokarev (8): qemu-img: pass current cmdname into command handlers qemu-img: refresh options/--help for "create" subcommand qemu-img: factor out parse_output_format() and use it in the code (this one has been sent in a separate patch, here it is just for completness) qemu-img: refresh options/--help for "check" command qemu-img: simplify --repair error message qemu-img: refresh options/--help for "commit" command qemu-img: refresh options/--help for "compare" command qemu-img: refresh options/--help for "convert" command qemu-img.c | 352 ++++++++++++++++++++++++++++++++++------------------- 1 file changed, 226 insertions(+), 126 deletions(-)