On 12.11.18 11:26, Alberto Garcia wrote:
> On Sun 11 Nov 2018 10:01:05 PM CET, Max Reitz wrote:
>> On 07.11.18 13:59, Alberto Garcia wrote:
>>> This function takes three options (cache.direct, cache.no-flush and
>>> read-only) from a QemuOpts object and updates the flags accordingly.
>>
>> and auto-read-only now
> 
> Oops, will update.
> 
>> Hm, seems like one way to solve it and I can't really find issue with
>> it.  So, let's first give a
>>
>> Reviewed-by: Max Reitz <mreitz@redhat.com>
>>
>> However, I wonder why you dropped your patch from v1 for this.  It
>> seemed more reasonable to me.  You're basically trading half-updating
>> the flags for just not touching them at all (and the latter seems
>> better, even though it's all an error in the end anyway).
> 
> The main reason why I'm doing this is because if we keep the assertions
> then we're forced to have these four options always set, and I don't see
> any reason why they would need to be.
> 
> It's not a problem now but it will be later on. Have a look at this
> early implementation of qmp_x_blockdev_reopen():
> 
>    https://lists.gnu.org/archive/html/qemu-block/2018-06/msg00795.html
> 
> Here we need to explicitly set those options to false if they're
> unset. 'false' is already the default value of all of them, so this
> shouldn't be necessary, but if we don't do it we'd hit the assertions
> that I'm removing in this patch.

OK, that makes sense.

Max