Kevin Wolf <kw...@redhat.com> writes:
> This adds a --chardev option to the storage daemon that works the same
> as the -chardev option of the system emulator.
>
> Signed-off-by: Kevin Wolf <kw...@redhat.com>
> ---
> qemu-storage-daemon.c | 19 +++++++++++++++++++
> Makefile | 2 +-
> 2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/qemu-storage-daemon.c b/qemu-storage-daemon.c
> index 099388f645..46e0a6ea56 100644
> --- a/qemu-storage-daemon.c
> +++ b/qemu-storage-daemon.c
> @@ -26,6 +26,7 @@
>
> #include "block/block.h"
> #include "block/nbd.h"
> +#include "chardev/char.h"
> #include "crypto/init.h"
>
> #include "qapi/error.h"
> @@ -75,6 +76,9 @@ static void help(void)
> " [,driver specific parameters...]\n"
> " configure a block backend\n"
> "\n"
> +" --chardev <options> configure a character device backend\n"
> +" (see the qemu(1) man page for possible options)\n"
If pointing to the manual page was good enough for --help, we could save
ourselves a ton of trouble :)
> +"\n"
> " --export [type=]nbd,device=<node-name>[,name=<export-name>]\n"
> " [,writable=on|off][,bitmap=<name>]\n"
> " export the specified block node over NBD\n"
> @@ -96,10 +100,13 @@ QEMU_HELP_BOTTOM "\n",
> enum {
> OPTION_OBJECT = 256,
> OPTION_BLOCKDEV,
> + OPTION_CHARDEV,
> OPTION_NBD_SERVER,
> OPTION_EXPORT,
> };
>
> +extern QemuOptsList qemu_chardev_opts;
> +
> static QemuOptsList qemu_object_opts = {
> .name = "object",
> .implied_opt_name = "qom-type",
> @@ -130,6 +137,7 @@ static int process_options(int argc, char *argv[], Error
> **errp)
> {"help", no_argument, 0, 'h'},
> {"object", required_argument, 0, OPTION_OBJECT},
> {"blockdev", required_argument, 0, OPTION_BLOCKDEV},
> + {"chardev", required_argument, 0, OPTION_CHARDEV},
> {"nbd-server", required_argument, 0, OPTION_NBD_SERVER},
> {"export", required_argument, 0, OPTION_EXPORT},
> {"version", no_argument, 0, 'V'},
> @@ -189,6 +197,17 @@ static int process_options(int argc, char *argv[], Error
> **errp)
> qapi_free_BlockdevOptions(options);
> break;
> }
> + case OPTION_CHARDEV:
> + {
> + QemuOpts *opts = qemu_opts_parse(&qemu_chardev_opts,
> + optarg, true, &error_fatal);
> + if (!qemu_chr_new_from_opts(opts, NULL, &error_fatal)) {
> + /* No error, but NULL returned means help was printed */
> + exit(EXIT_SUCCESS);
> + }
> + qemu_opts_del(opts);
> + break;
> + }
Differs from vl.c similar to --object [PATCH 02]:
* Options are processed left to right. Good.
* You use &qemu_chardev_opts instead of qemu_opts_find(). Good.
* You use qemu_opts_parse() instead of qemu_opts_parse_noisily(). Only
here I can actually point to a loss of help.
For options where the argument is essentially a tagged union, an
option argument "help" usually lists the tags, an argument "T,help"
shows help on the parameters that go with tag T.
--chardev is such an option. Consider:
$ qemu-system-x86_64 --chardev help
Available chardev backend types:
ringbuf
[...]
tty
$ qemu-storage-daemon --chardev help
Available chardev backend types:
pty
[...]
tty
Works the same, only the available backend types differ. Intentional?
Next:
$ qemu-system-x86_64 --chardev tty,help
chardev options:
append=<bool (on/off)>
backend=<str>
chardev=<str>
cols=<num>
[...]
width=<num>
[Exit 1 ]
The second help isn't very helpful, and the exit code is wrong. Not
this patch's problem.
$ qemu-storage-daemon --chardev tty,help
qemu-storage-daemon: Invalid parameter 'help'
[Exit 1 ]
This patch's problem :)
Also like --object, --chardev is paired with a QMP command, namely
chardev-add, and the two differ for historical reasons. If we want to
give the storage daemon a QAPIfied command line from the start, we'll
have to decide how to address this issue.
> case OPTION_NBD_SERVER:
> {
> Visitor *v;
> diff --git a/Makefile b/Makefile
> index b913d4d736..0e3e98582d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -561,7 +561,7 @@ qemu-img.o: qemu-img-cmds.h
> qemu-img$(EXESUF): qemu-img.o $(authz-obj-y) $(block-obj-y) $(crypto-obj-y)
> $(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS)
> qemu-nbd$(EXESUF): qemu-nbd.o $(authz-obj-y) $(block-obj-y) $(crypto-obj-y)
> $(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS)
> qemu-io$(EXESUF): qemu-io.o $(authz-obj-y) $(block-obj-y) $(crypto-obj-y)
> $(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS)
> -qemu-storage-daemon$(EXESUF): qemu-storage-daemon.o $(authz-obj-y)
> $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y)
> $(storage-daemon-obj-y) $(COMMON_LDADDS)
> +qemu-storage-daemon$(EXESUF): qemu-storage-daemon.o $(authz-obj-y)
> $(block-obj-y) $(crypto-obj-y) $(chardev-obj-y) $(io-obj-y) $(qom-obj-y)
> $(storage-daemon-obj-y) $(COMMON_LDADDS)
>
> qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o $(COMMON_LDADDS)
