On 04.11.19 14:03, Alberto Garcia wrote: > On Mon 04 Nov 2019 01:57:42 PM CET, Max Reitz wrote: >> On 26.10.19 23:25, Alberto Garcia wrote: >>> In the previous patch we added a new QCow2ClusterType named >>> QCOW2_CLUSTER_UNALLOCATED_SUBCLUSTER. There is a couple of places >>> where this new value needs to be handled, and that is what this patch >>> does. >>> >>> Signed-off-by: Alberto Garcia <be...@igalia.com> >>> --- >>> block/qcow2.c | 13 +++++++++---- >>> 1 file changed, 9 insertions(+), 4 deletions(-) >> This patch deals with everything in qcow2.c. There are more places that >> reference QCOW2_CLUSTER_* constants elsewhere, and I suppose most of >> them are handled by the following patches. >> >> But I wonder what the criterion is on where it needs to be handled and >> where it’s OK not to. Right now it looks to me like it’s a bit >> arbitrary maybe? But I suppose I’ll just have to wait until after the >> next patches. > > This is the part of the series that I'm the least happy about, because > the existing qcow2_get_cluster_type() can never return this new value, I > only updated the cases where this can actually happen. > > I'm still considering a different approach for this. I still don’t know what you’re doing in the later patches, but to me it looks a bit like you don’t dare breaking up the existing structure that just deals with clusters.
If that is so, I think it will help to make a clear cut between what concerns subclusters and what concerns clusters at a whole. QCOW2_CLUSTER_UNALLOCATED_SUBCLUSTER shouldn’t be in QCow2ClusterType; there should be a separate QCow2SubclusterType. OTOH, that would require more modifications, but (naïvely) I believe that would make for the cleaner interface in the end. Max
signature.asc
Description: OpenPGP digital signature
