|
From: | Eric Blake |
Subject: | Re: [PATCH v4 5/9] raw-format: Support BDRV_REQ_ZERO_WRITE for truncate |
Date: | Mon, 20 Apr 2020 10:53:51 -0500 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 |
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
[Prev in Thread] | Current Thread | [Next in Thread] |