Libguestfs - Mar 2021 - [PATCH libnbd 2/2] examples: copy-libev.c: Simple sparsifying

If a request includes only zeroes, use efficient zero method instead of
sending the zeroes over the wire.

Signed-off-by: Nir Soffer <nsoffer at redhat.com>
---
 examples/Makefile.am  |  1 +
 examples/copy-libev.c | 68 ++++++++++++++++++++++++++++++++++---------
 2 files changed, 55 insertions(+), 14 deletions(-)

diff --git a/examples/Makefile.am b/examples/Makefile.am
index a8286a3..27d2ead 100644
--- a/examples/Makefile.am
+++ b/examples/Makefile.am
@@ -224,6 +224,7 @@ copy_libev_SOURCES = \
 	copy-libev.c \
 	$(NULL)
 copy_libev_CPPFLAGS = \
+	-I$(top_srcdir)/common/include \
 	-I$(top_srcdir)/include \
 	$(NULL)
 copy_libev_CFLAGS = \
diff --git a/examples/copy-libev.c b/examples/copy-libev.c
index 70a9811..d9fb2c7 100644
--- a/examples/copy-libev.c
+++ b/examples/copy-libev.c
@@ -27,6 +27,8 @@
 
 #include <ev.h>
 
+#include "iszero.h"
+
 /* These values depend on the enviroment tested.
  *
  * For shared storage using direct I/O:
@@ -79,9 +81,12 @@ static int64_t offset;
 static int64_t written;
 static bool debug;
 
+static void start_request(struct request *r);
 static void start_read(struct request *r);
+static void start_write(struct request *r);
+static void start_zero(struct request *r);
 static int read_completed(void *user_data, int *error);
-static int write_completed(void *user_data, int *error);
+static int request_completed(void *user_data, int *error);
 
 static inline int
 get_fd(struct connection *c)
@@ -104,16 +109,25 @@ get_events(struct connection *c)
     return events;
 }
 
+/* Start async copy or zero request. */
 static void
-start_read(struct request *r)
+start_request(struct request *r)
 {
-    int64_t cookie;
-
     assert (offset < size);
 
     r->length = MIN (REQUEST_SIZE, size - offset);
     r->offset = offset;
 
+    start_read (r);
+
+    offset += r->length;
+}
+
+static void
+start_read(struct request *r)
+{
+    int64_t cookie;
+
     DEBUG ("start read offset=%ld len=%ld", r->offset,
r->length);
 
     cookie = nbd_aio_pread (
@@ -123,38 +137,64 @@ start_read(struct request *r)
         0);
     if (cookie == -1)
         FAIL ("Cannot start read: %s", nbd_get_error ());
-
-    offset += r->length;
 }
 
 static int
 read_completed (void *user_data, int *error)
 {
     struct request *r = (struct request *)user_data;
+
+    DEBUG ("read completed offset=%ld len=%ld", r->offset,
r->length);
+
+    if (is_zero (r->data, r->length))
+        start_zero (r);
+    else
+        start_write (r);
+
+    return 1;
+}
+
+static void
+start_write(struct request *r)
+{
     int64_t cookie;
 
-    DEBUG ("read completed, starting write offset=%ld len=%ld",
-           r->offset, r->length);
+    DEBUG ("start write offset=%ld len=%ld", r->offset,
r->length);
 
     cookie = nbd_aio_pwrite (
         dst.nbd, r->data, r->length, r->offset,
-        (nbd_completion_callback) { .callback=write_completed,
+        (nbd_completion_callback) { .callback=request_completed,
                                     .user_data=r },
         0);
     if (cookie == -1)
         FAIL ("Cannot start write: %s", nbd_get_error ());
+}
 
-    return 1;
+static void
+start_zero(struct request *r)
+{
+    int64_t cookie;
+
+    DEBUG ("start zero offset=%ld len=%ld", r->offset,
r->length);
+
+    cookie = nbd_aio_zero (
+        dst.nbd, r->length, r->offset,
+        (nbd_completion_callback) { .callback=request_completed,
+                                    .user_data=r },
+        0);
+    if (cookie == -1)
+        FAIL ("Cannot start zero: %s", nbd_get_error ());
 }
 
+/* Called when async copy or zero request completed. */
 static int
-write_completed (void *user_data, int *error)
+request_completed (void *user_data, int *error)
 {
     struct request *r = (struct request *)user_data;
 
     written += r->length;
 
-    DEBUG ("write completed offset=%ld len=%ld", r->offset,
r->length);
+    DEBUG ("request completed offset=%ld len=%ld", r->offset,
r->length);
 
     if (written == size) {
         /* The last write completed. Stop all watchers and break out
@@ -168,7 +208,7 @@ write_completed (void *user_data, int *error)
 
     /* If we have data to read, start a new read. */
     if (offset < size)
-        start_read(r);
+        start_request(r);
 
     return 1;
 }
@@ -249,7 +289,7 @@ main (int argc, char *argv[])
         if (r->data == NULL)
             FAIL ("Cannot allocate buffer: %s", strerror (errno));
 
-        start_read(r);
+        start_request(r);
     }
 
     /* Start watching events on src and dst handles. */
-- 
2.26.2

I would just say about the examples that often it's better if they are
completely stand-alone (since we want people to copy and adapt the
code).  That would imply not using the common/ header file, but either
inlining the function or using a less efficient but simpler loop.
This is also the reason why the examples don't #include <config.h>

Anyway the patch is fine, but IMO could be improved as above.

ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

On 3/5/21 11:16 AM, Nir Soffer wrote:
> If a request includes only zeroes, use efficient zero method instead of
> sending the zeroes over the wire.
> 
> Signed-off-by: Nir Soffer <nsoffer at redhat.com>
> ---
>  examples/Makefile.am  |  1 +
>  examples/copy-libev.c | 68 ++++++++++++++++++++++++++++++++++---------
>  2 files changed, 55 insertions(+), 14 deletions(-)
> 
> +static void
> +start_zero(struct request *r)
> +{
> +    int64_t cookie;
> +
> +    DEBUG ("start zero offset=%ld len=%ld", r->offset,
r->length);
> +
> +    cookie = nbd_aio_zero (
> +        dst.nbd, r->length, r->offset,
> +        (nbd_completion_callback) { .callback=request_completed,
> +                                    .user_data=r },
> +        0);
> +    if (cookie == -1)
> +        FAIL ("Cannot start zero: %s", nbd_get_error ());
You didn't check nbd_can_zero anywhere.  If the server does not
advertise support for NBD_CMD_ZERO (possible to do with nbdkit
--filter=nozero), then this will always fail.  It may be worth adding a
manual fallback to write in this case, instead of completely failing.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org