Message ID | 1394605864-32237-6-git-send-email-famz@redhat.com |
---|---|
State | New |
Headers | show |
The Wednesday 12 Mar 2014 à 14:31:00 (+0800), Fam Zheng wrote : > This makes a deep copy of an HBitmap. > > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > include/qemu/hbitmap.h | 8 ++++++++ > util/hbitmap.c | 13 +++++++++++++ > 2 files changed, 21 insertions(+) > > diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h > index 550d7ce..b645cfc 100644 > --- a/include/qemu/hbitmap.h > +++ b/include/qemu/hbitmap.h > @@ -65,6 +65,14 @@ struct HBitmapIter { > HBitmap *hbitmap_alloc(uint64_t size, int granularity); > > /** > + * hbitmap_copy: > + * @bitmap: The original bitmap to copy. > + * > + * Copy a HBitmap. > + */ > +HBitmap *hbitmap_copy(const HBitmap *bitmap); > + > +/** > * hbitmap_empty: > * @hb: HBitmap to operate on. > * > diff --git a/util/hbitmap.c b/util/hbitmap.c > index d936831..cf670c7 100644 > --- a/util/hbitmap.c > +++ b/util/hbitmap.c > @@ -400,3 +400,16 @@ HBitmap *hbitmap_alloc(uint64_t size, int granularity) > hb->levels[0][0] |= 1UL << (BITS_PER_LONG - 1); > return hb; > } > + > +HBitmap *hbitmap_copy(const HBitmap *bitmap) > +{ > + int i; > + HBitmap *hb = g_memdup(bitmap, sizeof(struct HBitmap)); > + > + for (i = HBITMAP_LEVELS; i-- > 0; ) { > + hb->levels[i] = g_memdup(bitmap->levels[i], > + bitmap->size * sizeof(unsigned long)); This feel wrong: struct HBitmap { /* Number of total bits in the bottom level. */ uint64_t size; The comment about size imply that size apply only to the bottom level. Moreover the bitmaps are progressivelly less coarse so I think the size you use should goes down as you go up to the top. Best regards Benoît > + } > + > + return hb; > +} > -- > 1.9.0 > >
Il 13/03/2014 14:45, Benoît Canet ha scritto: > This feel wrong: > > struct HBitmap { > /* Number of total bits in the bottom level. */ > uint64_t size; > > The comment about size imply that size apply only to the bottom level. > > Moreover the bitmaps are progressivelly less coarse so I think the size you use > should goes down as you go up to the top. Correct. You can reuse the computation in hbitmap_alloc instead. Paolo
Il 12/03/2014 07:31, Fam Zheng ha scritto: > +HBitmap *hbitmap_copy(const HBitmap *bitmap) > +{ > + int i; > + HBitmap *hb = g_memdup(bitmap, sizeof(struct HBitmap)); > + > + for (i = HBITMAP_LEVELS; i-- > 0; ) { > + hb->levels[i] = g_memdup(bitmap->levels[i], > + bitmap->size * sizeof(unsigned long)); > + } > + > + return hb; Not quite... the bitmap->size is in bits, so you need to scale it down by size = MAX((size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1); before each memdup. Paolo
On Thu, 03/20 11:22, Paolo Bonzini wrote: > Il 12/03/2014 07:31, Fam Zheng ha scritto: > >+HBitmap *hbitmap_copy(const HBitmap *bitmap) > >+{ > >+ int i; > >+ HBitmap *hb = g_memdup(bitmap, sizeof(struct HBitmap)); > >+ > >+ for (i = HBITMAP_LEVELS; i-- > 0; ) { > >+ hb->levels[i] = g_memdup(bitmap->levels[i], > >+ bitmap->size * sizeof(unsigned long)); > >+ } > >+ > >+ return hb; > > Not quite... the bitmap->size is in bits, so you need to scale it down by > > size = MAX((size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1); > > before each memdup. > This is fixed in V3 which is posted on the list. Thanks, Fam
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h index 550d7ce..b645cfc 100644 --- a/include/qemu/hbitmap.h +++ b/include/qemu/hbitmap.h @@ -65,6 +65,14 @@ struct HBitmapIter { HBitmap *hbitmap_alloc(uint64_t size, int granularity); /** + * hbitmap_copy: + * @bitmap: The original bitmap to copy. + * + * Copy a HBitmap. + */ +HBitmap *hbitmap_copy(const HBitmap *bitmap); + +/** * hbitmap_empty: * @hb: HBitmap to operate on. * diff --git a/util/hbitmap.c b/util/hbitmap.c index d936831..cf670c7 100644 --- a/util/hbitmap.c +++ b/util/hbitmap.c @@ -400,3 +400,16 @@ HBitmap *hbitmap_alloc(uint64_t size, int granularity) hb->levels[0][0] |= 1UL << (BITS_PER_LONG - 1); return hb; } + +HBitmap *hbitmap_copy(const HBitmap *bitmap) +{ + int i; + HBitmap *hb = g_memdup(bitmap, sizeof(struct HBitmap)); + + for (i = HBITMAP_LEVELS; i-- > 0; ) { + hb->levels[i] = g_memdup(bitmap->levels[i], + bitmap->size * sizeof(unsigned long)); + } + + return hb; +}
This makes a deep copy of an HBitmap. Signed-off-by: Fam Zheng <famz@redhat.com> --- include/qemu/hbitmap.h | 8 ++++++++ util/hbitmap.c | 13 +++++++++++++ 2 files changed, 21 insertions(+)