| Message ID | 0229eca3c5199c5383b640f9a041a83ddfcf5b0c.1577014346.git.berto@igalia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add subcluster allocation to qcow2 | expand |
On 22.12.19 12:36, 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, although the caller > does not need and should ignore the returned value. > > Signed-off-by: Alberto Garcia <berto@igalia.com> > --- > block/qcow2.h | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/block/qcow2.h b/block/qcow2.h > index 8be020bb76..64b0a814f4 100644 > --- a/block/qcow2.h > +++ b/block/qcow2.h > @@ -518,15 +518,37 @@ static inline size_t l2_entry_size(BDRVQcow2State *s) [...] > +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 { > + /* For convenience only; the caller should ignore this value. */ > + return 0; Is there a reason you decided not to return the first subcluster as allocated? (As you had proposed in v2) Reviewed-by: Max Reitz <mreitz@redhat.com> > + } > +}
On Thu 20 Feb 2020 05:27:28 PM CET, Max Reitz wrote: >> +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 { >> + /* For convenience only; the caller should ignore this value. */ >> + return 0; > > Is there a reason you decided not to return the first subcluster as > allocated? (As you had proposed in v2) Yeah, I thought that it would not make much sense to return a meaningful value after a comment saying that the caller should ignore it. If there was a situation in which something depends on that value then it would be a bug in QEMU. Berto
diff --git a/block/qcow2.h b/block/qcow2.h index 8be020bb76..64b0a814f4 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -518,15 +518,37 @@ 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 { + /* For convenience only; the caller should ignore this value. */ + return 0; + } +} + 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, although the caller does not need and should ignore the returned value. Signed-off-by: Alberto Garcia <berto@igalia.com> --- block/qcow2.h | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)