Re: [Libguestfs] [PATCH nbdkit v5 FINAL 01/19] server: Implement extents/can_extents calls for plugins and filters.
On 3/28/19 11:18 AM, Richard W.M. Jones wrote:
This new pair of callbacks allows plugins to describe which extents
in
the virtual disk are allocated, holes or zeroes.
---
+++ b/server/extents.c
@@ -0,0 +1,210 @@
+/* nbdkit
+ * Copyright (C) 2019 Red Hat Inc.
+ * All rights reserved.
We started work on this phrase but it got waylaid by extents work; maybe
now is a good time to revisit that?
+struct nbdkit_extents *
+nbdkit_extents_new (uint64_t start, uint64_t end)
+{
+ struct nbdkit_extents *r;
+
+ if (start >= INT64_MAX || end >= INT64_MAX) {
Is this an off-by-one? INT64_MAX is a valid offset in our memory plugin,
while INT64_MAX + 1 is not.
+ nbdkit_error ("nbdkit_extents_new: "
+ "start (%" PRIu64 ") or end (%" PRIu64 ")
>= INT64_MAX",
+ start, end);
+ errno = ERANGE;
+ return NULL;
+ }
+
+ /* 0-length ranges are possible, so start == end is not an error. */
+ if (start > end) {
The protocol declares them to be unspecified behavior (so a portable
client shouldn't be doing it). Doesn't valid_range() in protocol.c
already filter out 0-length requests from the client? Thus, I'm
wondering if this should be >= instead of >
+int
+nbdkit_add_extent (struct nbdkit_extents *exts,
+ uint64_t offset, uint64_t length, uint32_t type)
+{
+ uint64_t overlap;
+
+ if (length == 0)
+ return 0;
+
+ /* Ignore extents beyond the end of the range. Unfortunately we
+ * lost the information about whether this is contiguous with a
+ * previously added extent so we can't check for API usage errors.
Nice comment...
+ */
+ if (offset >= exts->end)
+ return 0;
Is returning 0 always right, or should we also check whether the user
tried to pass in offset > end as their very first nbdkit_extents_add
(where we know that because nr_extents is 0 that it is an API usage error).
+
+ /* Shorten extents that overlap the end of the range. */
+ if (offset + length >= exts->end) {
+ overlap = offset + length - exts->end;
+ length -= overlap;
+ }
+
+ /* If there are existing extents, the new extent must be contiguous. */
+ if (exts->nr_extents > 0) {
+ const struct nbdkit_extent *ep;
+
+ ep = &exts->extents[exts->nr_extents-1];
+ if (offset != ep->offset + ep->length) {
+ nbdkit_error ("nbdkit_add_extent: "
+ "extents must be added in ascending order and "
+ "must be contiguous");
+ return -1;
+ }
+ }
+ else {
+ /* If there are no existing extents, and the new extent is
+ * entirely before start, ignore it.
...should this one also mention that we can't check for API usage errors
for extents prior to start?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 488 bytes)