Re: [Libguestfs] [PATCH libnbd 5/8] copy: Introduce worker struct

2022-02-20 Thread Richard W.M. Jones


ACK patch 5

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [PATCH libnbd 5/8] copy: Introduce worker struct

2022-02-20 Thread Nir Soffer
I want to keep more info per worker, and using a worker struct is the
natural way to do this. This also allows cleaning up the ops-* interface
which accepted uintptr_t index while the index is never a pointer. I
think the pointer is a result of passing the index to the thread using
the void* pointer.

The worker struct is used only by the multi-threading-copy module, but
in future patch I want to keep the worker pointer in the command, to
allow commands to update worker state when they finish.

Signed-off-by: Nir Soffer 
---
 copy/file-ops.c |  4 +--
 copy/main.c |  6 ++---
 copy/multi-thread-copying.c | 49 +++--
 copy/nbd-ops.c  | 10 
 copy/nbdcopy.h  | 24 +++---
 copy/null-ops.c |  4 +--
 copy/pipe-ops.c |  2 +-
 7 files changed, 53 insertions(+), 46 deletions(-)

diff --git a/copy/file-ops.c b/copy/file-ops.c
index aaf04ade..ab378754 100644
--- a/copy/file-ops.c
+++ b/copy/file-ops.c
@@ -614,27 +614,27 @@ file_asynch_zero (struct rw *rw, struct command *command,
 {
   int dummy = 0;
 
   if (!file_synch_zero (rw, command->offset, command->slice.len, allocate))
 return false;
   cb.callback (cb.user_data, &dummy);
   return true;
 }
 
 static unsigned
-file_in_flight (struct rw *rw, uintptr_t index)
+file_in_flight (struct rw *rw, size_t index)
 {
   return 0;
 }
 
 static void
-file_get_extents (struct rw *rw, uintptr_t index,
+file_get_extents (struct rw *rw, size_t index,
   uint64_t offset, uint64_t count,
   extent_list *ret)
 {
   ret->len = 0;
 
 #ifdef SEEK_HOLE
   struct rw_file *rwf = (struct rw_file *)rw;
   static pthread_mutex_t lseek_lock = PTHREAD_MUTEX_INITIALIZER;
 
   if (rwf->seek_hole_supported) {
diff --git a/copy/main.c b/copy/main.c
index 67788b5d..390de1eb 100644
--- a/copy/main.c
+++ b/copy/main.c
@@ -513,44 +513,44 @@ print_rw (struct rw *rw, const char *prefix, FILE *fp)
 
   fprintf (fp, "%s: %s \"%s\"\n", prefix, rw->ops->ops_name, rw->name);
   fprintf (fp, "%s: size=%" PRIi64 " (%s)\n",
prefix, rw->size, human_size (buf, rw->size, NULL));
 }
 
 /* Default implementation of rw->ops->get_extents for backends which
  * don't/can't support extents.  Also used for the --no-extents case.
  */
 void
-default_get_extents (struct rw *rw, uintptr_t index,
+default_get_extents (struct rw *rw, size_t index,
  uint64_t offset, uint64_t count,
  extent_list *ret)
 {
   struct extent e;
 
   ret->len = 0;
 
   e.offset = offset;
   e.length = count;
   e.zero = false;
   if (extent_list_append (ret, e) == -1) {
 perror ("realloc");
 exit (EXIT_FAILURE);
   }
 }
 
 /* Implementations of get_polling_fd and asynch_notify_* for backends
  * which don't support polling.
  */
 void
-get_polling_fd_not_supported (struct rw *rw, uintptr_t index,
+get_polling_fd_not_supported (struct rw *rw, size_t index,
   int *fd_rtn, int *direction_rtn)
 {
   /* Not an error, this causes poll to ignore the fd. */
   *fd_rtn = -1;
   *direction_rtn = LIBNBD_AIO_DIRECTION_READ;
 }
 
 void
-asynch_notify_read_write_not_supported (struct rw *rw, uintptr_t index)
+asynch_notify_read_write_not_supported (struct rw *rw, size_t index)
 {
   /* nothing */
 }
diff --git a/copy/multi-thread-copying.c b/copy/multi-thread-copying.c
index aa6a9f41..a1a8d09c 100644
--- a/copy/multi-thread-copying.c
+++ b/copy/multi-thread-copying.c
@@ -70,184 +70,185 @@ get_next_offset (uint64_t *offset, uint64_t *count)
  * the commands.  We might move this into a callback, but those
  * are called from threads and not necessarily in monotonic order
  * so the progress bar would move erratically.
  */
 progress_bar (*offset, src->size);
   }
   pthread_mutex_unlock (&lock);
   return r;
 }
 
-static void *worker_thread (void *ip);
+static void *worker_thread (void *wp);
 
 void
 multi_thread_copying (void)
 {
-  pthread_t *workers;
+  struct worker *workers;
   size_t i;
   int err;
 
   /* Some invariants that should be true if the main program called us
* correctly.
*/
   assert (threads > 0);
   assert (threads == connections);
 /*
   if (src.ops == &nbd_ops)
 assert (src.u.nbd.handles.size == connections);
   if (dst.ops == &nbd_ops)
 assert (dst.u.nbd.handles.size == connections);
 */
   assert (src->size != -1);
 
-  workers = malloc (sizeof (pthread_t) * threads);
+  workers = calloc (threads, sizeof *workers);
   if (workers == NULL) {
-perror ("malloc");
+perror ("calloc");
 exit (EXIT_FAILURE);
   }
 
   /* Start the worker threads. */
   for (i = 0; i < threads; ++i) {
-err = pthread_create (&workers[i], NULL, worker_thread,
-  (void *)(uintptr_t)i);
+workers[i].index = i;
+err = pthread_create (&workers[i].thread, NULL, worker_thread,
+  &workers[i]);
 if (err != 0) {