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

[Nir Soffer]

On Mon, Feb 21, 2022 at 5:08 PM Eric Blake <eblake at redhat.com> wrote:
>
> 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 at redhat.com>
> > ---
> >  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.

I agree it is better to behave like free(). Will improve later.

>
> 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
>