8:49 a.m.
On 3/19/19 11:34 AM, Richard W.M. Jones wrote:
This pair of calls allows plugins to describe which extents in the
virtual disk are allocated, holes or zeroes.
---
Resuming where I left off, if it helps:
+#ifdef IN_TEST_EXTENTS
+/* These functions are only compiled for and called from the unit tests. */
+
+void
+dump_extents_map (const struct nbdkit_extents_map *map)
+{
+ size_t i;
+
+ fprintf (stderr, "map %p:\n", map);
+ fprintf (stderr, "\tnr_extents=%zu allocated=%zu\n",
+ map->nr_extents, map->allocated);
+ for (i = 0; i < map->nr_extents; ++i) {
+ fprintf (stderr, "\textent %zu:", i);
+ fprintf (stderr,
+ "\toffset=%" PRIu64 " length=%" PRIu64 " "
+ "last=%" PRIu64 " type=%" PRIu32 "\n",
Worth using fixed-width formatting so that consecutive lines are
tabular, and/or using hex offsets (in addition to decimal) for easier
spotting of alignments?
+ map->extents[i].offset, map->extents[i].length,
+ map->extents[i].offset + map->extents[i].length - 1,
+ map->extents[i].type);
+ }
+}
+
+void
+validate_extents_map (const struct nbdkit_extents_map *map)
+{
+ size_t i;
+ bool fail = false;
+
+ if (map->nr_extents > map->allocated) {
+ fprintf (stderr, "validate_extents_map: nr_extents > allocated\n");
+ fail = true;
+ }
+
+ for (i = 0; i < map->nr_extents; ++i) {
Of course, if we failed the previous check, this now reads beyond the
end of the allocation. But it's debug code.
+ /* Extent length must be > 0. */
+ if (map->extents[i].length == 0) {
+ fprintf (stderr, "validate_extents_map: %zu: extent length must be >
0\n",
+ i);
+ fail = true;
+ }
+
+ /* Extent length must not wrap around, indicating that we have
+ * computed a negative length (as unsigned) somewhere.
+ */
+ if (map->extents[i].offset + map->extents[i].length <
+ map->extents[i].offset) {
+ fprintf (stderr, "validate_extents_map: %zu: negative length\n",
+ i);
+ fail = true;
+ }
+
No validation that extents don't exceed INT64_MAX?
+ if (i > 0) {
+ /* Extents are stored in strictly ascending order. */
+ if (map->extents[i-1].offset >= map->extents[i].offset) {
+ fprintf (stderr,
+ "validate_extents_map: %zu: "
+ "not strictly ascending order\n", i);
+ fail = true;
+ }
+
+ /* Adjacent extents must not overlap. */
+ if (map->extents[i-1].offset + map->extents[i-1].length >
+ map->extents[i].offset) {
+ fprintf (stderr,
+ "validate_extents_map: %zu: "
+ "overlapping extents\n", i);
+ fail = true;
+ }
+
+ /* Adjacent extents either have a different type or are
+ * separated by a gap.
This changes if you don't allow gaps (it does sound like the
implementation will be easier if it is gap-free...)
+ */
+ if (map->extents[i-1].type == map->extents[i].type &&
+ map->extents[i-1].offset + map->extents[i-1].length ==
+ map->extents[i].offset) {
+ fprintf (stderr,
+ "validate_extents_map: %zu: "
+ "adjacent extents with same type not coalesced\n", i);
+ fail = true;
+ }
+ }
+ }
+
+ if (fail) {
+ dump_extents_map (map);
+ abort ();
+ }
+}
@@ -511,6 +530,21 @@ filter_can_zero (struct backend *b, struct
connection *conn)
return f->backend.next->can_zero (f->backend.next, conn);
}
+static int
+filter_can_extents (struct backend *b, struct connection *conn)
+{
+ struct backend_filter *f = container_of (b, struct backend_filter, backend);
+ void *handle = connection_get_handle (conn, f->backend.i);
+ struct b_conn nxdata = { .b = f->backend.next, .conn = conn };
+
+ debug ("%s: can_extents", f->name);
+
+ if (f->filter.can_extents)
+ return f->filter.can_extents (&next_ops, &nxdata, handle);
+ else
+ return f->backend.next->can_extents (f->backend.next, conn);
For now, should we default filter.can_extents to false until we
completed the work for all filters?
+++ b/server/test-extents.c
+
+static int
+compare (uint64_t offset, uint64_t length, uint32_t type, void *mapv)
+{
+ const struct nbdkit_extents_map *map = mapv;
+ size_t j;
+
+ /* nbdkit_extents_foreach_range should ensure this is true even if
+ * there are extents which go below or above the range.
+ */
+ assert (offset >= 0);
Dead assertion (always true since offset is unsigned).
+
+ /* Read out the extents and compare them to the disk. */
+ assert (nbdkit_extents_foreach (map, compare, map,
+ NBDKIT_EXTENTS_FOREACH_FLAG_RANGE,
+ 0, DISK_SIZE-1) == 0);
+
The test is rendered less powerful when NDEBUG is defined. But it
doesn't affect production code (and maybe we could ensure that this file
#undef NDEBUG regardless of what the user decides via -D for assertions
in the overall project)
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org