[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/2] qcow2: Force preallocation with data-file-raw
From: |
Max Reitz |
Subject: |
Re: [PATCH 1/2] qcow2: Force preallocation with data-file-raw |
Date: |
Mon, 22 Jun 2020 11:48:46 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.9.0 |
On 22.06.20 11:35, Max Reitz wrote:
> On 19.06.20 18:47, Alberto Garcia wrote:
>> On Fri 19 Jun 2020 12:40:11 PM CEST, Max Reitz wrote:
>>> + if (qcow2_opts->data_file_raw &&
>>> + qcow2_opts->preallocation == PREALLOC_MODE_OFF)
>>> + {
>>> + /*
>>> + * data-file-raw means that "the external data file can be
>>> + * read as a consistent standalone raw image without looking
>>> + * at the qcow2 metadata." It does not say that the metadata
>>> + * must be ignored, though (and the qcow2 driver in fact does
>>> + * not ignore it), so the L1/L2 tables must be present and
>>> + * give a 1:1 mapping, so you get the same result regardless
>>> + * of whether you look at the metadata or whether you ignore
>>> + * it.
>>> + */
>>> + qcow2_opts->preallocation = PREALLOC_MODE_METADATA;
>>
>> I'm not convinced by this,
>
> Why not?
>
> This is how I read the spec. Furthermore, I see two problems that we
> have right now that are fixed by this patch (namely (1) using a device
> file as the external data file, which may have non-zero data at
> creation; and (2) assigning a backing file at runtime must not show the
> data).
>
>> but your comment made me think of another
>> possible alternative: in qcow2_get_cluster_offset(), if the cluster is
>> unallocated and we are using a raw data file then we return _ZERO_PLAIN:
>>
>> --- a/block/qcow2-cluster.c
>> +++ b/block/qcow2-cluster.c
>> @@ -654,6 +654,10 @@ out:
>> assert(bytes_available - offset_in_cluster <= UINT_MAX);
>> *bytes = bytes_available - offset_in_cluster;
>>
>> + if (type == QCOW2_CLUSTER_UNALLOCATED && data_file_is_raw(bs)) {
>> + type = QCOW2_CLUSTER_ZERO_PLAIN;
>> + }
>> +
>> return type;
>>
>> You could even add a '&& bs->backing' to the condition and emit a
>> warning to make it more explicit.
>
> No, this is wrong. This still wouldn’t fix the problem of having a
> device file as the external data file, when it already has non-zero data
> during creation. (Reading the qcow2 file would return zeroes, but
> reading the device would not.)
>
> So it would need to be QCOW2_CLUSTER_NORMAL. Which is kind of the
> point, when you think about it – with data-file-raw, all clusters must
> always effectively be QCOW2_CLUSTER_NORMAL and be mapped 1:1.
>
> Well, and that’s in turn the point of this patch.
>
> I interpret the spec in that the metadata can be ignored, but it does
> not need to be ignored. So the L1/L2 tables must be 1:1 mapping of
> QCOW2_CLUSTER_NORMAL entries.
>
> We could also choose to interpret it as “With data-file-raw, the L1/L2
> tables must be ignored”. In that case, our qcow2 driver would need to
> be modified to indeed fully ignore the L1/L2 tables with data-file-raw.
> (I certainly don’t interpret the spec this way, but I suppose we could
> call it a bug fix and amend it.)
I just realized that this is not possible. data-file-raw is an
autoclear flag, so the image must appear the same to qemu versions that
do not support it.
If we want to fully ignore the L1/L2 tables or interpret them some
non-default way (like you’re proposing), we would have to add a new
incompatible flag.
Max
signature.asc
Description: OpenPGP digital signature
- [PATCH 0/2] qcow2: Force preallocation with data-file-raw, Max Reitz, 2020/06/19
- [PATCH 1/2] qcow2: Force preallocation with data-file-raw, Max Reitz, 2020/06/19
- Re: [PATCH 1/2] qcow2: Force preallocation with data-file-raw, Alberto Garcia, 2020/06/19
- Re: [PATCH 1/2] qcow2: Force preallocation with data-file-raw, Alberto Garcia, 2020/06/22
- Re: [PATCH 1/2] qcow2: Force preallocation with data-file-raw, Max Reitz, 2020/06/22
- Re: [PATCH 1/2] qcow2: Force preallocation with data-file-raw, Nir Soffer, 2020/06/22
- Re: [PATCH 1/2] qcow2: Force preallocation with data-file-raw, Max Reitz, 2020/06/22
- Re: [PATCH 1/2] qcow2: Force preallocation with data-file-raw, Eric Blake, 2020/06/22
- Re: [PATCH 1/2] qcow2: Force preallocation with data-file-raw, Alberto Garcia, 2020/06/22
- Re: [PATCH 1/2] qcow2: Force preallocation with data-file-raw, Max Reitz, 2020/06/23
[PATCH 2/2] iotests/244: Test preallocation for data-file-raw, Max Reitz, 2020/06/19
Re: [PATCH 0/2] qcow2: Force preallocation with data-file-raw, no-reply, 2020/06/19