Re: [Libguestfs] [PATCH libnbd 4/8] copy: Separate finishing a command from freeing it

On Sun, Feb 20, 2022 at 02:13:59PM +0200, Nir Soffer wrote: > free_command() was abused as a completion callback. Introduce > finish_command() completion callback, so code that want to free a > command does not have to add dummy errors. > > This will make it easier to manage worker state when a command > completes. > > Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt; > --- > copy/multi-thread-copying.c | 34 ++++++++++++++++++++-------------- > 1 file changed, 20 insertions(+), 14 deletions(-) In addition to Rich's review, > > static int > -free_command (void *vp, int *error) > +finished_command (void *vp, int *error) > { > struct command *command = vp; > - struct buffer *buffer = command->slice.buffer; > > if (*error) { > fprintf (stderr, "write at offset %" PRId64 " failed: %s\n", > command->offset, strerror (*error)); > exit (EXIT_FAILURE); > } > > + free_command (command); > + > + return 1; /* auto-retires the command */ > +} > + > +static void > +free_command (struct command *command) > +{ > + struct buffer *buffer = command->slice.buffer; Do we want to allow 'free_command (NULL)', in which case this should check if command is non-NULL before initializing buffer? Doing so may make some other cleanup paths easier to write.

But for now, all callers pass in non-NULL, so ACK either way. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org