|
From: | Eric Blake |
Subject: | Re: [PATCH 09/17] block: Refactor bdrv_has_zero_init{,_truncate} |
Date: | Tue, 4 Feb 2020 09:49:46 -0600 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 |
On 2/4/20 9:35 AM, Vladimir Sementsov-Ogievskiy wrote:
31.01.2020 20:44, Eric Blake wrote:Having two slightly-different function names for related purposes is unwieldy, especially since I envision adding yet another notion of zero support in an upcoming patch. It doesn't help that bdrv_has_zero_init() is a misleading name (I originally thought that a driver could only return 1 when opening an already-existing image known to be all zeroes; but in reality many drivers always return 1 because it only applies to a just-created image). Refactor all uses to instead have a single function that returns multiple bits of information, with better naming and documentation.Sounds goodNo semantic change, although some of the changes (such as to qcow2.c) require a careful reading to see how it remains the same....diff --git a/include/block/block.h b/include/block/block.h index 6cd566324d95..a6a227f50678 100644 --- a/include/block/block.h +++ b/include/block/block.hHmm, header file in the middle of the patch, possibly you don't use [diff] orderFile = scripts/git.orderfile in git config.. Or it is broken.
I do have it set up, so I'm not sure why it didn't work as planned. I'll make sure v2 follows the order I intended.
@@ -85,6 +85,28 @@ typedef enum { BDRV_REQ_MASK = 0x3ff, } BdrvRequestFlags; +typedef enum { + /* + * bdrv_known_zeroes() should include this bit if the contents of + * a freshly-created image with no backing file reads as all + * zeroes without any additional effort. If .bdrv_co_truncate is + * set, then this must be clear if BDRV_ZERO_TRUNCATE is clear.I understand that this is preexisting logic, but could I ask: why? What's wrong if driver can guarantee that created file is all-zero, but is not sure about file resizing? I agree that it's normal for these flags to have the same value,but what is the reason for this restriction?..
For _this_ patch, my goal is to preserve pre-existing practice. Where we think pre-existing practice is wrong, we can then improve it in other patches (see patch 6, for example).
I _think_ the reason for this original limitation is as follows: If an image can be resized, we could choose to perform 'create(size=0), truncate(size=final)' instead of 'create(size=final)', and we want to guarantee the same behavior. If truncation can't guarantee a zero read, then why is creation doing so?
But as I did not write the original patch, I would welcome Max's input with regards to the thought behind commit ceaca56f.
So, the only possible combination of flags, when they differs, is create=0 andtruncate=1.. How is it possible?
qcow2 had that mode, at least before patch 5.
+ * Since this bit is only reliable at image creation, a driver may + * return this bit even for existing images that do not currently + * read as zero. + */ + BDRV_ZERO_CREATE = 0x1, + + /* + * bdrv_known_zeroes() should include this bit if growing an image + * with PREALLOC_MODE_OFF (either with no backing file, or beyond + * the size of the backing file) will read the new data as all + * zeroes without any additional effort. This bit only matters + * for drivers that set .bdrv_co_truncate. + */ + BDRV_ZERO_TRUNCATE = 0x2, +} BdrvZeroFlags; +...
-- 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] |