| Message ID | 2d4de1dee301cd772fce97c90e08a390edbe2830.1572125022.git.berto@igalia.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add subcluster allocation to qcow2 | expand |
On 26.10.19 23:25, 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, these functions are no-ops > when used in traditional qcow2 images. Granted, I haven’t seen the following patches yet, but if these functions are indeed called for images that don’t have subclusters, shouldn’t they return 0x0*0f*f then? (i.e. everything allocated) If they aren’t, they should probably just abort(). Well, set_l2_bitmap() should probably always abort() if there aren’t any subclusters. Max > 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 29a253bfb9..741c41c80f 100644 > --- a/block/qcow2.h > +++ b/block/qcow2.h > @@ -507,15 +507,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 { > + 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) > +{ > + if (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; >
On Wed 30 Oct 2019 05:55:04 PM CET, Max Reitz wrote: >> This patch also adds the get/set_l2_bitmap() functions that are used >> to access the bitmaps. For convenience, these functions are no-ops >> when used in traditional qcow2 images. > > Granted, I haven’t seen the following patches yet, but if these > functions are indeed called for images that don’t have subclusters, > shouldn’t they return 0x0*0f*f then? (i.e. everything allocated) > > If they aren’t, they should probably just abort(). Well, > set_l2_bitmap() should probably always abort() if there aren’t any > subclusters. Yeah, set_l2_bitmap() should abort (I had this changed already). About get_l2_bitmap() ... I decided not to abort for convenience, for cases like this one: uint64_t l2_entry = get_l2_entry(s, l2_slice, l2_index); uint64_t l2_bitmap = get_l2_bitmap(s, l2_slice, l2_index); type = qcow2_get_subcluster_type(bs, l2_entry, l2_bitmap, sc); Here the value of l2_bitmap is going to be ignored anyway so it doesn't matter what we return, but perhaps for consistency we should return QCOW_OFLAG_SUB_ALLOC(0), which means that the first (and only, in this case) subcluster is allocated. Berto
diff --git a/block/qcow2.h b/block/qcow2.h index 29a253bfb9..741c41c80f 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -507,15 +507,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 { + 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) +{ + if (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, these functions are no-ops when used in traditional qcow2 images. Signed-off-by: Alberto Garcia <berto@igalia.com> --- block/qcow2.h | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)