Re: [Libguestfs] [libnbd PATCH] api: Allow completion callbacks to auto-retire
On 7/23/19 3:30 PM, Eric Blake wrote:
When using the nbd_aio_FOO_callback commands, there is nothing
further
to be learned about the command by calling nbd_aio_command_completed()
compared to what the callback already had access to. There are still
scenarios where manually retiring the command after the fact is useful
(whether the return was 0 to keep the status unchanged, or -1 to alter
the retirement status to *error), but by allowing a return value of 1,
we can reduce the number of API calls required. We can also
auto-retire the lone NBD_CMD_DISC request, reducing the amount of
special casing in nbd_aio_[peek_]command_completed.
Note that although this is not an API change, it does change ABI, but
that's okay as we have not declared a stable interface yet.
Update a couple of the tests and examples to give this coverage:
examples/glib-main-loop no longer piles up unretired commands, and
tests/server-death shows that even a command stranded by server death
can still be auto-retired.
---
This probably will conflict with Rich's work to improve callbacks to
make it easier to free data on the last use of a callback.
Here's the counterpart patch for nbdkit to utilize this; however, my
timing runs of libnbd/examples/threaded-reads-and-writes did not show
any noticeable change, so this particular optimization is in the noise
compared to any real bottlenecks remaining. Still, the reduced lines of
code makes me like the idea:
diff --git i/plugins/nbd/nbd.c w/plugins/nbd/nbd.c
index 83a30583..ccd773ba 100644
--- i/plugins/nbd/nbd.c
+++ w/plugins/nbd/nbd.c
@@ -57,7 +57,6 @@
/* The per-transaction details */
struct transaction {
- int64_t cookie;
sem_t sem;
uint32_t early_err;
uint32_t err;
@@ -360,14 +359,12 @@ nbdplug_notify (void *opaque, int64_t cookie, int
*error)
nbdkit_debug ("cookie %" PRId64 " completed state machine, status
%d",
cookie, *error);
- assert (trans->cookie == 0 || trans->cookie == cookie);
- trans->cookie = cookie;
trans->err = *error;
if (sem_post (&trans->sem)) {
nbdkit_error ("failed to post semaphore: %m");
abort ();
}
- return 0;
+ return 1;
}
/* Register a cookie and kick the I/O thread. */
@@ -386,8 +383,6 @@ nbdplug_register (struct handle *h, struct
transaction *trans, int64_t cookie)
if (write (h->fds[1], &c, 1) != 1 && errno != EAGAIN)
nbdkit_debug ("failed to kick reader thread: %m");
- assert (trans->cookie == 0 || trans->cookie == cookie);
- trans->cookie = cookie;
}
/* Perform the reply half of a transaction. */
@@ -405,13 +400,8 @@ nbdplug_reply (struct handle *h, struct transaction
*trans)
nbdkit_debug ("failed to wait on semaphore: %m");
err = EIO;
}
- else {
- assert (trans->cookie > 0);
- err = nbd_aio_command_completed (h->nbd, trans->cookie);
- assert (err != 0);
- assert (err == 1 ? trans->err == 0 : trans->err == nbd_get_errno ());
+ else
err = trans->err;
- }
}
if (sem_destroy (&trans->sem))
abort ();
--
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)