| Message ID | 20200415074927.19897-10-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qemu-option: Fix corner cases and clean up | expand |
On 4/15/20 2:49 AM, Markus Armbruster wrote: > qemu-img create, convert, amend, and measure use accumulate_options() > to merge multiple -o options. This is broken for -o "": > > $ qemu-img create -f qcow2 -o backing_file=a -o "" -o backing_fmt=raw,size=1M new.qcow2 > qemu-img: warning: Could not verify backing image. This may become an error in future versions. > Could not open 'a,backing_fmt=raw': No such file or directory > Formatting 'new.qcow2', fmt=qcow2 size=1048576 backing_file=a,,backing_fmt=raw cluster_size=65536 lazy_refcounts=off refcount_bits=16 > $ qemu-img info new.qcow2 > image: new.qcow2 > file format: qcow2 > virtual size: 1 MiB (1048576 bytes) > disk size: 196 KiB > cluster_size: 65536 > --> backing file: a,backing_fmt=raw > Format specific information: > compat: 1.1 > lazy refcounts: false > refcount bits: 16 > corrupt: false > > Merging these three -o the obvious way is wrong, because it results in > an unwanted ',' escape: > > backing_file=a,,backing_fmt=raw,size=1M > ~~ > > We could silently drop -o "", but Kevin asked me to reject it instead. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > qemu-img.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) This results in the error message: qemu-img: Invalid option list: with a trailing space and no indication that it was an empty string we were trying to warn about. But that's tolerable. Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/qemu-img.c b/qemu-img.c index cc51db7ed4..a2369766f0 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -229,14 +229,16 @@ static bool qemu_img_object_print_help(const char *type, QemuOpts *opts) * To make that work, @optarg must not start with ',' (or else a * separating ',' preceding it gets escaped), and it must not end with * an odd number of ',' (or else a separating ',' following it gets - * escaped). + * escaped), or be empty (or else a separating ',' preceding it can + * escape a separating ',' following it). + * */ static bool is_valid_option_list(const char *optarg) { size_t len = strlen(optarg); size_t i; - if (optarg[0] == ',') { + if (!optarg[0] || optarg[0] == ',') { return false; }
qemu-img create, convert, amend, and measure use accumulate_options() to merge multiple -o options. This is broken for -o "": $ qemu-img create -f qcow2 -o backing_file=a -o "" -o backing_fmt=raw,size=1M new.qcow2 qemu-img: warning: Could not verify backing image. This may become an error in future versions. Could not open 'a,backing_fmt=raw': No such file or directory Formatting 'new.qcow2', fmt=qcow2 size=1048576 backing_file=a,,backing_fmt=raw cluster_size=65536 lazy_refcounts=off refcount_bits=16 $ qemu-img info new.qcow2 image: new.qcow2 file format: qcow2 virtual size: 1 MiB (1048576 bytes) disk size: 196 KiB cluster_size: 65536 --> backing file: a,backing_fmt=raw Format specific information: compat: 1.1 lazy refcounts: false refcount bits: 16 corrupt: false Merging these three -o the obvious way is wrong, because it results in an unwanted ',' escape: backing_file=a,,backing_fmt=raw,size=1M ~~ We could silently drop -o "", but Kevin asked me to reject it instead. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- qemu-img.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)