[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 08/10] nbd/server: introduce NBDExtentArray
From: |
Eric Blake |
Subject: |
Re: [PATCH v4 08/10] nbd/server: introduce NBDExtentArray |
Date: |
Wed, 26 Feb 2020 09:06:04 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 2/5/20 5:20 AM, Vladimir Sementsov-Ogievskiy wrote:
Introduce NBDExtentArray class, to handle extents list creation in more
controlled way and with fewer OUT parameters in functions.
Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
---
nbd/server.c | 210 +++++++++++++++++++++++++++++----------------------
1 file changed, 118 insertions(+), 92 deletions(-)
+
+/* Further modifications of the array after conversion are abandoned */
+static void nbd_extent_array_convert_to_be(NBDExtentArray *ea)
+{
+ int i;
+
+ assert(!ea->converted_to_be);
Comment is stale - further modifications after conversion are a bug that
aborts the program, not abandoned.
/*
- * Populate @extents from block status. Update @bytes to be the actual
- * length encoded (which may be smaller than the original), and update
- * @nb_extents to the number of extents used.
- *
- * Returns zero on success and -errno on bdrv_block_status_above failure.
+ * Add extent to NBDExtentArray. If extent can't be added (no available space),
+ * return -1.
+ * For safety, when returning -1 for the first time, .can_add is set to false,
+ * further call to nbd_extent_array_add() will crash.
s/further call/so further calls/
+ * (to avoid the situation, when after failing to add an extent (returned -1),
+ * user miss this failure and add another extent, which is successfully added
+ * (array is full, but new extent may be squashed into the last one), then we
+ * have invalid array with skipped extent)
Long comment with nested (). I'm not sure it adds much value, I think
it can safely be dropped. But if it is kept, I suggest:
(this ensures that after a failure, no further extents can accidentally
change the bounds of the last extent in the array)
*/
-static int blockstatus_to_extents(BlockDriverState *bs, uint64_t offset,
- uint64_t *bytes, NBDExtent *extents,
- unsigned int *nb_extents)
+static int nbd_extent_array_add(NBDExtentArray *ea,
+ uint32_t length, uint32_t flags)
{
- uint64_t remaining_bytes = *bytes;
- NBDExtent *extent = extents, *extents_end = extents + *nb_extents;
- bool first_extent = true;
+ assert(ea->can_add);
+
+ if (!length) {
+ return 0;
+ }
+
+ /* Extend previous extent if flags are the same */
+ if (ea->count > 0 && flags == ea->extents[ea->count - 1].flags) {
+ uint64_t sum = (uint64_t)length + ea->extents[ea->count - 1].length;
+
+ if (sum <= UINT32_MAX) {
+ ea->extents[ea->count - 1].length = sum;
+ ea->total_length += length;
+ return 0;
+ }
+ }
+
+ if (ea->count >= ea->nb_alloc) {
+ ea->can_add = false;
+ return -1;
+ }
+
+ ea->total_length += length;
+ ea->extents[ea->count] = (NBDExtent) {.length = length, .flags = flags};
+ ea->count++;
- assert(*nb_extents);
- while (remaining_bytes) {
+ return 0;
+}
Looks like you properly addressed my concerns from v3.
Comment changes are trivial, so you can add:
Reviewed-by: Eric Blake <address@hidden>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
- [PATCH v4 00/10] Further bitmaps improvements, Vladimir Sementsov-Ogievskiy, 2020/02/05
- [PATCH v4 03/10] hbitmap: unpublish hbitmap_iter_skip_words, Vladimir Sementsov-Ogievskiy, 2020/02/05
- [PATCH v4 09/10] nbd/server: use bdrv_dirty_bitmap_next_dirty_area, Vladimir Sementsov-Ogievskiy, 2020/02/05
- [PATCH v4 02/10] hbitmap: move hbitmap_iter_next_word to hbitmap.c, Vladimir Sementsov-Ogievskiy, 2020/02/05
- [PATCH v4 04/10] hbitmap: drop meta bitmaps as they are unused, Vladimir Sementsov-Ogievskiy, 2020/02/05
- [PATCH v4 05/10] block/dirty-bitmap: switch _next_dirty_area and _next_zero to int64_t, Vladimir Sementsov-Ogievskiy, 2020/02/05
- [PATCH v4 07/10] block/dirty-bitmap: improve _next_dirty_area API, Vladimir Sementsov-Ogievskiy, 2020/02/05
- [PATCH v4 06/10] block/dirty-bitmap: add _next_dirty API, Vladimir Sementsov-Ogievskiy, 2020/02/05