| Message ID | 30130e0f4662ea8ba705c5c54afc56bfb1ae70b6.1588699789.git.berto@igalia.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add subcluster allocation to qcow2 | expand |
On 5/5/20 12:38 PM, Alberto Garcia wrote: > Extended L2 entries are 128-bit wide: 64 bits for the entry itself and > 64 bits for the subcluster allocation bitmap. > > In order to support them correctly get/set_l2_entry() need to be > updated so they take the entry width into account in order to > calculate the correct offset. > > This patch also adds the get/set_l2_bitmap() functions that are > used to access the bitmaps. For convenience we allow calling > get_l2_bitmap() on images without subclusters. In this case the > returned value is always 0 and has no meaning. > > Signed-off-by: Alberto Garcia <berto@igalia.com> > --- > block/qcow2.h | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > +static inline void set_l2_bitmap(BDRVQcow2State *s, uint64_t *l2_slice, > + int idx, uint64_t bitmap) > +{ > + assert(has_subclusters(s)); > + idx *= l2_entry_size(s) / sizeof(uint64_t); > + l2_slice[idx + 1] = cpu_to_be64(bitmap); > +} Unrelated to this patch, but I just thought of it: What happens for an image whose size is not cluster-aligned? Must the bits corresponding to subclusters not present in the final cluster always be zero, or are they instead ignored regardless of value? But for this patch: Reviewed-by: Eric Blake <eblake@redhat.com>
On Tue 05 May 2020 10:04:32 PM CEST, Eric Blake wrote: > What happens for an image whose size is not cluster-aligned? Must the > bits corresponding to subclusters not present in the final cluster > always be zero, or are they instead ignored regardless of value? Attempting to read or write beyond the end of the image returns an error (see blk_check_byte_request()). But let's say we have a 32k image (only 1/2 of the first cluster is used) and we manipulate the bitmap to mark all subclusters as allocated. In this case if you try 'read 0 32k' then count_contiguous_subclusters() would indeed say that there are 64k of data available. However the caller knows that we only want 32k and if (bytes_available > bytes_needed) { bytes_available = bytes_needed; } so in the end it doesn't really matter. I think in practice it's the same as with traditional qcow2 images: once we reach qcow2_get_host_offset() the code does not really know or care about the size of the image or how much of the last cluster contains actual data. It only cares about how much data the caller needs. The limits have already been checked before. But we could document it: "if the image size is not a multiple of the cluster size then the bits corresponding to the subclusters beyond the end of the image are ignored and should be set to zero", or something like that. Berto
diff --git a/block/qcow2.h b/block/qcow2.h index 80ceb352c9..4ad93772b9 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -515,15 +515,36 @@ static inline size_t l2_entry_size(BDRVQcow2State *s) static inline uint64_t get_l2_entry(BDRVQcow2State *s, uint64_t *l2_slice, int idx) { + idx *= l2_entry_size(s) / sizeof(uint64_t); return be64_to_cpu(l2_slice[idx]); } +static inline uint64_t get_l2_bitmap(BDRVQcow2State *s, uint64_t *l2_slice, + int idx) +{ + if (has_subclusters(s)) { + idx *= l2_entry_size(s) / sizeof(uint64_t); + return be64_to_cpu(l2_slice[idx + 1]); + } else { + return 0; /* For convenience only; this value has no meaning. */ + } +} + static inline void set_l2_entry(BDRVQcow2State *s, uint64_t *l2_slice, int idx, uint64_t entry) { + idx *= l2_entry_size(s) / sizeof(uint64_t); l2_slice[idx] = cpu_to_be64(entry); } +static inline void set_l2_bitmap(BDRVQcow2State *s, uint64_t *l2_slice, + int idx, uint64_t bitmap) +{ + assert(has_subclusters(s)); + idx *= l2_entry_size(s) / sizeof(uint64_t); + l2_slice[idx + 1] = cpu_to_be64(bitmap); +} + static inline bool has_data_file(BlockDriverState *bs) { BDRVQcow2State *s = bs->opaque;
Extended L2 entries are 128-bit wide: 64 bits for the entry itself and 64 bits for the subcluster allocation bitmap. In order to support them correctly get/set_l2_entry() need to be updated so they take the entry width into account in order to calculate the correct offset. This patch also adds the get/set_l2_bitmap() functions that are used to access the bitmaps. For convenience we allow calling get_l2_bitmap() on images without subclusters. In this case the returned value is always 0 and has no meaning. Signed-off-by: Alberto Garcia <berto@igalia.com> --- block/qcow2.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)