search for: bitmap_resize

This is exactly the same as v1: https://www.redhat.com/archives/libguestfs/2018-December/msg00004.html except that it now frees the bitmap on unload (which the old code did not - there was always a memory leak). Rich.

I have some patches I'm working on to fix the cache filter. However this is a prelude. It should be simply pure refactoring. All tests pass still. Rich.

...w --filter=truncate memory size=512 round-up=4096 --- common/bitmap/bitmap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/bitmap/bitmap.c b/common/bitmap/bitmap.c index caac916..6bf04dc 100644 --- a/common/bitmap/bitmap.c +++ b/common/bitmap/bitmap.c @@ -60,7 +60,7 @@ bitmap_resize (struct bitmap *bm, uint64_t new_size) new_bm_size = (size_t) new_bm_size_u64; new_bitmap = realloc (bm->bitmap, new_bm_size); - if (new_bitmap == NULL) { + if (new_bm_size && new_bitmap == NULL) { nbdkit_error ("realloc: %m"); return -1; } -- 2.23.0

...r commit 3166d2bcbfd2 is to round down the size of the underlying disk to the cow/cache filter block size. => Size of the underlying disk is 512 or 0 bytes, block size is 4096 bytes. => Size of the disk is rounded down to 0. => The cow/cache filter requests a bitmap of size 0. => bitmap_resize calls realloc (ptr, 0). => The glibc implementation of realloc returns NULL + errno == 0. (Other realloc implementations can behave differently.) => This is not an error, but the existing code thinks it is because of the NULL return. This commit changes the code so it doesn't bot...

...BDKIT_NEXTNONZERO_H */ diff --git a/common/bitmap/bitmap.c b/common/bitmap/bitmap.c index fb5dbe7..9690a2e 100644 --- a/common/bitmap/bitmap.c +++ b/common/bitmap/bitmap.c @@ -42,6 +42,7 @@ #include "bitmap.h" #include "rounding.h" +#include "nextnonzero.h" int bitmap_resize (struct bitmap *bm, uint64_t new_size) @@ -73,3 +74,37 @@ bitmap_resize (struct bitmap *bm, uint64_t new_size) return 0; } + +int64_t +bitmap_next (const struct bitmap *bm, uint64_t blk) +{ + uint64_t limit = bm->size * bm->ibpb; + const uint8_t *p; + + /* Align to the next byte boun...

...m appear before all .h before all .c, to make patches a little easier to follow logically (what gets built, what interfaces does it need, and how is it implemented). 'git config diff.orderFile /path/to/file'; see qemu's scripts/git.orderfile for an example. > + > +int > +bitmap_resize (struct bitmap *bm, uint64_t new_size) > +{ > + uint8_t *new_bitmap; > + const size_t old_bm_size = bm->size; > + uint64_t new_bm_size_u64; > + size_t new_bm_size; > + > + new_bm_size_u64 = DIV_ROUND_UP (new_size, bm->blksize * 8 / bm->bpb); Can the computation o...

...ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <stdint.h> + +#include <nbdkit-plugin.h> + +#include "bitmap.h" +#include "rounding.h" + +int +bitmap_resize (struct bitmap *bm, uint64_t new_size) +{ + uint8_t *new_bitmap; + const size_t old_bm_size = bm->size; + uint64_t new_bm_size_u64; + size_t new_bm_size; + + new_bm_size_u64 = DIV_ROUND_UP (new_size, bm->blksize * 8 / bm->bpb); + if (new_bm_size_u64 > SIZE_MAX) { + nbdkit_error...

...ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <stdint.h> + +#include <nbdkit-plugin.h> + +#include "bitmap.h" +#include "rounding.h" + +int +bitmap_resize (struct bitmap *bm, uint64_t new_size) +{ + uint8_t *new_bitmap; + const size_t old_bm_size = bm->size; + uint64_t new_bm_size_u64; + size_t new_bm_size; + + new_bm_size_u64 = DIV_ROUND_UP (new_size, bm->blksize * 8 / bm->bpb); + if (new_bm_size_u64 > SIZE_MAX) { + nbdkit_error...

...f, since it is assumed that the struct + * bitmap is statically allocated. + */ +static inline void +bitmap_free (struct bitmap *bm) +{ + if (bm) + free (bm->bitmap); +} + +/* Resize the bitmap to the virtual disk size in bytes. + * Returns -1 on error, setting nbdkit_error. + */ +extern int bitmap_resize (struct bitmap *bm, uint64_t new_size); + +/* This macro calculates the byte offset in the bitmap and which + * bit/mask we are addressing within that byte. + * + * bpb blk_offset blk_bit mask + * 1 blk >> 3 0,1,2,...,7 any single bit + * 2 blk...

v1 was here: https://www.redhat.com/archives/libguestfs/2019-September/msg00100.html In v2 I've changed the patch so it avoids calling realloc at all in this case. The patch is a bit longer this way. But I don't see any other alternative if we are to avoid having a "realloc wrapper" of some kind that we use everywhere, which I guess we should avoid because it makes plugins

...100; + +void +lru_init (void) +{ + bitmap_init (&bm[0], BLKSIZE, 1 /* bits per block */); + bitmap_init (&bm[1], BLKSIZE, 1 /* bits per block */); +} + +void +lru_free (void) +{ + bitmap_free (&bm[0]); + bitmap_free (&bm[1]); +} + +int +lru_set_size (uint64_t new_size) +{ + if (bitmap_resize (&bm[0], new_size) == -1) + return -1; + if (bitmap_resize (&bm[1], new_size) == -1) + return -1; + + /* XXX Choose this better. */ + N = MAX (new_size / BLKSIZE / 4, 100); + + return 0; +} + +void +lru_set_recently_accessed (uint64_t blknum) +{ + /* If the block is already set i...

...100; + +void +lru_init (void) +{ + bitmap_init (&bm[0], BLKSIZE, 1 /* bits per block */); + bitmap_init (&bm[1], BLKSIZE, 1 /* bits per block */); +} + +void +lru_free (void) +{ + bitmap_free (&bm[0]); + bitmap_free (&bm[1]); +} + +int +lru_set_size (uint64_t new_size) +{ + if (bitmap_resize (&bm[0], new_size) == -1) + return -1; + if (bitmap_resize (&bm[1], new_size) == -1) + return -1; + + /* XXX Choose this better. */ + N = MAX (new_size / BLKSIZE / 4, 100); + + return 0; +} + +void +lru_set_recently_accessed (uint64_t blknum) +{ + /* If the block is already set i...

v2: https://www.redhat.com/archives/libguestfs/2018-December/msg00039.html v2 -> v3: - Fix all the issues raised in Eric's review. - Precompute some numbers to make the calculations easier. - Calculations now use bitshifts and masks in preference to division and modulo. - Clear existing bits before setting (which fixes a bug in the cache filter). Rich.

These are essentially identical to what was previously posted as patches 6/9 through 9/9 here: https://www.redhat.com/archives/libguestfs/2018-December/msg00145.html except that it has been rebased onto the current git master and retested thoroughly. Rich.

This patch series enhances the cache filter in a few ways, primarily adding a "cache-on-read" feature (similar to qemu's copyonread); and adding the ability to limit the cache size and the antecedent of that which is having a method to reclaim cache blocks. As the cache is stored as a sparse temporary file, reclaiming cache blocks simply means punching holes in the temporary file.

Should we use attribute((nonnull)) at all? There's a very interesting history of this in libvirt -- try looking at commit eefb881 plus the commits referencing eefb881 -- but it does seem to work for me using recent GCC and Clang. I only did a few functions because annotating them gets old quickly... Rich.

Patch 1 is the same as last time, except for a minor comment fix. Patch 2 should address everything that Eric mentioned in his review, and has been retested. Rich.

...__((__nonnull__ (1))) bitmap_init (struct bitmap *bm, unsigned blksize, unsigned bpb) { assert (is_power_of_2 (blksize)); @@ -99,10 +99,11 @@ bitmap_free (struct bitmap *bm) /* Resize the bitmap to the virtual disk size in bytes. * Returns -1 on error, setting nbdkit_error. */ -extern int bitmap_resize (struct bitmap *bm, uint64_t new_size); +extern int bitmap_resize (struct bitmap *bm, uint64_t new_size) + __attribute__((__nonnull__ (1))); /* Clear the bitmap (set everything to zero). */ -static inline void +static inline void __attribute__((__nonnull__ (1))) bitmap_clear (struct bitmap *bm...

v3 was broken by a bad rebase, so let's forget about that one. Compared to v2: - Patch 1 is the same except for a minor comment change. - Patch 2 splits the reclaim code into a separate file (filters/cache/reclaim.c) - Addressed Eric's comments from his review of v2. - Retested on Linux and FreeBSD.

v1 was here: https://www.redhat.com/archives/libguestfs/2019-January/msg00008.html In v2 I have provided two patches: The first patch extends attribute((nonnull)) to most internal functions, but not to the external API. The second patch uses a macro so that attribute((format)) is only used in the public API on GCC or Clang. At least in theory these headers could be used by a C compiler which