Re: [PATCH v4 1/4] Introduce yank feature

[Lukas Straub]

On Tue, 16 Jun 2020 15:39:57 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Mon, May 25, 2020 at 05:44:23PM +0200, Lukas Straub wrote:
> > The yank feature allows to recover from hanging qemu by "yanking"
> > at various parts. Other qemu systems can register themselves and
> > multiple yank functions. Then all yank functions for selected
> > instances can be called by the 'yank' out-of-band qmp command.
> > Available instances can be queried by a 'query-yank' oob command.
> > 
> > Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> > ---
> >  qapi/misc.json |  45 +++++++++++++
> >  yank.c         | 174 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  yank.h         |  67 +++++++++++++++++++
> >  3 files changed, 286 insertions(+)
> >  create mode 100644 yank.c
> >  create mode 100644 yank.h  
> 
> > +void yank_register_function(char *instance_name, YankFn *func, void 
> > *opaque)
> > +{
> > +    struct YankInstance *instance;
> > +    struct YankFuncAndParam *entry;
> > +
> > +    qemu_mutex_lock(&lock);
> > +    instance = yank_find_instance(instance_name);
> > +    assert(instance);
> > +
> > +    entry = g_slice_new(struct YankFuncAndParam);
> > +    entry->func = func;
> > +    entry->opaque = opaque;
> > +
> > +    QLIST_INSERT_HEAD(&instance->yankfns, entry, next);
> > +    qemu_mutex_unlock(&lock);
> > +}
> > +
> > +void yank_unregister_function(char *instance_name, YankFn *func, void 
> > *opaque)
> > +{
> > +    struct YankInstance *instance;
> > +    struct YankFuncAndParam *entry;
> > +
> > +    qemu_mutex_lock(&lock);
> > +    instance = yank_find_instance(instance_name);
> > +    assert(instance);
> > +
> > +    QLIST_FOREACH(entry, &instance->yankfns, next) {
> > +        if (entry->func == func && entry->opaque == opaque) {
> > +            QLIST_REMOVE(entry, next);
> > +            g_slice_free(struct YankFuncAndParam, entry);
> > +            qemu_mutex_unlock(&lock);
> > +            return;
> > +        }
> > +    }
> > +
> > +    abort();
> > +}  
> 
> Since the NBD impl no longer needs to register multiple different functions
> on the same insance_nane, these methods could be be simplified, to only
> accept a single function, instead of keeping a whole list. This would avoid
> need to pass a function into the unregister() method at all.

Multiple yank functions are still needed for multifd migration.

> I don't consider this a blocker though, so you pick whether to do this
> or not.
> 
> 
> > diff --git a/yank.h b/yank.h
> > new file mode 100644
> > index 0000000000..f1c8743b72
> > --- /dev/null
> > +++ b/yank.h
> > @@ -0,0 +1,67 @@
> > +  
> 
> Missing license header

I will fix that in the next version.

Thanks,
Lukas Straub

> 
> > +#ifndef YANK_H
> > +#define YANK_H
> > +
> > +typedef void (YankFn) (void *opaque);
> > +
> > +/**
> > + * yank_register_instance: Register a new instance.
> > + *
> > + * This registers a new instance for yanking. Must be called before any 
> > yank
> > + * function is registered for this instance.
> > + *
> > + * This function is thread-safe.
> > + *
> > + * @instance_name: The globally unique name of the instance.
> > + */
> > +void yank_register_instance(char *instance_name);
> > +
> > +/**
> > + * yank_unregister_instance: Unregister a instance.
> > + *
> > + * This unregisters a instance. Must be called only after every yank 
> > function
> > + * of the instance has been unregistered.
> > + *
> > + * This function is thread-safe.
> > + *
> > + * @instance_name: The name of the instance.
> > + */
> > +void yank_unregister_instance(char *instance_name);
> > +
> > +/**
> > + * yank_register_function: Register a yank function
> > + *
> > + * This registers a yank function. All limitations of qmp oob commands 
> > apply
> > + * to the yank function as well.
> > + *
> > + * This function is thread-safe.
> > + *
> > + * @instance_name: The name of the instance
> > + * @func: The yank function
> > + * @opaque: Will be passed to the yank function
> > + */
> > +void yank_register_function(char *instance_name, YankFn *func, void 
> > *opaque);
> > +
> > +/**
> > + * yank_unregister_function: Unregister a yank function
> > + *
> > + * This unregisters a yank function.
> > + *
> > + * This function is thread-safe.
> > + *
> > + * @instance_name: The name of the instance
> > + * @func: func that was passed to yank_register_function
> > + * @opaque: opaque that was passed to yank_register_function
> > + */
> > +void yank_unregister_function(char *instance_name, YankFn *func, void 
> > *opaque);
> > +
> > +/**
> > + * yank_unregister_function: Generic yank function for iochannel
> > + *
> > + * This is a generic yank function which will call qio_channel_shutdown on 
> > the
> > + * provided QIOChannel.
> > + *
> > + * @opaque: QIOChannel to shutdown
> > + */
> > +void yank_generic_iochannel(void *opaque);
> > +#endif  
> 
> 
> 
> Regards,
> Daniel

pgpPDYjH49JJR.pgp
Description: OpenPGP digital signature