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 <mre...@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
signature.asc
Description: OpenPGP digital signature
