Re: [Qemu-devel] [PATCH 6/7] qemu-nbd: allow specifying image as a set of options args

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3590 bytes --]

On 12/22/2015 04:06 AM, Daniel P. Berrange wrote:
> Currently qemu-nbd allows an image filename to be passed on the
> command line, but does not have a way to set any options except
> the format eg
> 
>    qemu-nbd https://127.0.0.1/images/centos7.iso
>    qemu-nbd /home/berrange/demo.qcow2

Well, technically we have:
qemu-nbd json:{...}

but I'm fine with adding the nicer syntax.

> 
> This adds a --source arg (that is mutually exclusive with a
> positional filename arg and -f arg) that accepts a full option
> string, as well as the original syntax eg
> 
>    qemu-nbd --source driver=http,url=https://127.0.0.1/images,sslverify=off
>    qemu-nbd --source https://127.0.0.1/images/centos7.iso
>    qemu-nbd --source file=/home/berrange/demo.qcow2
>    qemu-nbd --source /home/berrange/demo.qcow2
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  qemu-nbd.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 50 insertions(+), 7 deletions(-)
> 
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index b4b6681..0b9f99e 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -49,6 +49,7 @@
>  #define QEMU_NBD_OPT_DISCARD       3
>  #define QEMU_NBD_OPT_DETECT_ZEROES 4
>  #define QEMU_NBD_OPT_OBJECT        5
> +#define QEMU_NBD_OPT_SOURCE        6

Same comment as earlier about this being safer if it is > 255.

> @@ -486,6 +497,7 @@ int main(int argc, char **argv)
>          { "persistent", 0, NULL, 't' },
>          { "verbose", 0, NULL, 'v' },
>          { "object", 1, NULL, QEMU_NBD_OPT_OBJECT },
> +        { "source", 1, NULL, QEMU_NBD_OPT_SOURCE },

Same comment as earlier on spelling this as required_argument.

>          { NULL, 0, NULL, 0 }
>      };
>      int ch;
> @@ -657,13 +669,23 @@ int main(int argc, char **argv)
>                  exit(1);
>              }
>              break;
> +        case QEMU_NBD_OPT_SOURCE:
> +            if (srcpath) {
> +                errx(EXIT_FAILURE, "--source can only be used once");

Markus has a series getting rid of errx() usage; you'll want to follow suit:
https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg03492.html

> +            }
> +            if (!qemu_opts_parse_noisily(&file_opts, optarg, true)) {
> +                qemu_opts_reset(&file_opts);
> +                exit(EXIT_FAILURE);
> +            }
> +            srcpath = optarg;
> +            break;
>          case '?':
>              errx(EXIT_FAILURE, "Try `%s --help' for more information.",
>                   argv[0]);
>          }
>      }
>  
> -    if ((argc - optind) != 1) {
> +    if ((argc - optind) > 1) {

Inner () are redundant.

>          errx(EXIT_FAILURE, "Invalid number of argument.\n"
>               "Try `%s --help' for more information.",
>               argv[0]);
> @@ -757,15 +779,36 @@ int main(int argc, char **argv)
>      bdrv_init();
>      atexit(bdrv_close_all);
>  
> -    if (fmt) {
> -        options = qdict_new();
> -        qdict_put(options, "driver", qstring_from_str(fmt));
> +    if (srcpath) {
> +        char *file = NULL;
> +        opts = qemu_opts_find(&file_opts, NULL);
> +        if (fmt) {
> +            errx(EXIT_FAILURE, "--source and -f are mutually exclusive");
> +        }
> +        if ((argc - optind) > 1) {

and again.

> +            errx(EXIT_FAILURE, "--source and filename are mutually exclusive");

More errx() to avoid.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]