On 4/20/20 10:32 AM, Kevin Wolf wrote:
@@ -445,6 +445,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags, bs->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED | ((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) & bs->file->bs->supported_zero_flags); + bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE;Shouldn't this be: bs->supported_truncate_flags = (bs->file->bs->supported_truncate_flags & BDRV_REQ_ZERO_WRITE); rather than unconditionally advertising something that the underlying layer may lack?Maybe that makes more sense, yes.
If nothing else, it is more consistent with what we are doing for supported_zero_flags. I also argue that having a reference to the passthrough is easier to grep for, if we ever add new flags in the future. That is, while keeping passthrough as opt-in rather than blind copying or blind assignment is slightly more code, it is easier to maintain.
I think in practice it wouldn't make a difference because the nested bdrv_co_truncate() would still fail rather than silently ignoring the flag. It would behave the same as filter drivers, which also recursively call bdrv_co_truncate() without checking the flag first (which is, of course, because I don't want to modify each filter driver).
Probably true, but consistency and ease of maintenance are better than proving action at a distance :)
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
