Re: [Qemu-devel] [PATCH v5 2/2] qemu-img: Check for backing image if specified during create

[Eric Blake <eblake@redhat.com>]

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

On 07/18/2017 10:33 AM, John Snow wrote:
> 
> 
> On 07/18/2017 08:51 AM, Eric Blake wrote:
>> On 07/17/2017 07:34 PM, John Snow wrote:
>>> Or, rather, force the open of a backing image if one was specified
>>> for creation. Using a similar -unsafe option as rebase, allow qemu-img
>>> to ignore the backing file validation if possible.

>>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>
>> Really? It seems like you've changed since v4.
>>
> 
> Duh. I missed this because the patchset grew to two patches, same with
> revising the message. I'm sorry about that.

Kevin's got it on his block branch, with that fixed already. No problem.

>>>      size = qemu_opt_get_size(opts, BLOCK_OPT_SIZE, 0);
>>
>> On v4, we talked about making this use qemu_opt_get_size(, -1) to make
>> it less confusing about how qemu_opt_get_size() refers back to a
>> caller-provided default embedded in QemuOpt (rather than the parameter).>
> 
> I actually got scared away from this because of the get_size signature,
> is it safe to pass -1 here?

I'm posting a separate patch for that now (yours is fine left alone,
because it is pre-existing).

> 
>>> +        if (!bs && size != -1) {
>>> +            /* Couldn't open BS, but we have a size, so it's nonfatal */
>>> +            error_reportf_err(local_err,
>>> +                              "Warning: could not verify backing image. "
>>> +                              "This may become an error in future versions.\n");
>>
>> Patchew rightly complained here about the trailing newline. Also, we
>> have the new warning* functions merged in, this should probably be using
>> those (see commit 3dc6f869, for example)
>>
> 
> I tried omitting it, but the printing looked wrong, and the test would
> mash input against the tail of the sentence.

Kevin adjusted it slightly on the block branch. If patchew still
complains, we may need to fix checkpatch.pl (the semantics of
error_reportf_err() are slightly different than error_setg(), after all).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


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