Re: [Qemu-devel] [RFC v2 07/32] postcopy: Add notifier chain

[Dr. David Alan Gilbert]

* Peter Xu (address@hidden) wrote:
> On Thu, Aug 24, 2017 at 08:27:05PM +0100, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <address@hidden>
> > 
> > Add a notifier chain for postcopy with a 'reason' flag
> > and an opportunity for a notifier member to return an error.
> > 
> > Call it when enabling postcopy.
> > 
> > This will initially used to enable devices to declare they're unable
> > to postcopy and later to notify of devices of stages within postcopy.
> > 
> > Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> > ---
> >  migration/postcopy-ram.c | 41 +++++++++++++++++++++++++++++++++++++++++
> >  migration/postcopy-ram.h | 26 ++++++++++++++++++++++++++
> >  vl.c                     |  2 ++
> >  3 files changed, 69 insertions(+)
> > 
> > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > index 640b72d86d..95007c00ef 100644
> > --- a/migration/postcopy-ram.c
> > +++ b/migration/postcopy-ram.c
> > @@ -23,6 +23,8 @@
> >  #include "savevm.h"
> >  #include "postcopy-ram.h"
> >  #include "ram.h"
> > +#include "qapi/error.h"
> > +#include "qemu/notify.h"
> >  #include "sysemu/sysemu.h"
> >  #include "sysemu/balloon.h"
> >  #include "qemu/error-report.h"
> > @@ -45,6 +47,38 @@ struct PostcopyDiscardState {
> >      unsigned int nsentcmds;
> >  };
> >  
> > +/* A notifier chain for postcopy
> > + * The notifier should return 0 if it's OK, or a
> > + * -errno on error.
> > + * The notifier should expect an Error ** as it's data
> 
> "PostcopyNotifyData *" but not "Error **"?

Ah well spotted.

> Maybe we can just remove this block of comment since there is a
> similar one in the header below.

Yes, that's what I've done.

> Besides:
> 
> Reviewed-by: Peter Xu <address@hidden>

Thanks.

Dave

> 
> > + */
> > +static NotifierWithReturnList postcopy_notifier_list;
> > +
> > +void postcopy_infrastructure_init(void)
> > +{
> > +    notifier_with_return_list_init(&postcopy_notifier_list);
> > +}
> > +
> > +void postcopy_add_notifier(NotifierWithReturn *nn)
> > +{
> > +    notifier_with_return_list_add(&postcopy_notifier_list, nn);
> > +}
> > +
> > +void postcopy_remove_notifier(NotifierWithReturn *n)
> > +{
> > +    notifier_with_return_remove(n);
> > +}
> > +
> > +int postcopy_notify(enum PostcopyNotifyReason reason, Error **errp)
> > +{
> > +    struct PostcopyNotifyData pnd;
> > +    pnd.reason = reason;
> > +    pnd.errp = errp;
> > +
> > +    return notifier_with_return_list_notify(&postcopy_notifier_list,
> > +                                            &pnd);
> > +}
> > +
> >  /* Postcopy needs to detect accesses to pages that haven't yet been copied
> >   * across, and efficiently map new pages in, the techniques for doing this
> >   * are target OS specific.
> > @@ -133,6 +167,7 @@ bool postcopy_ram_supported_by_host(void)
> >      struct uffdio_register reg_struct;
> >      struct uffdio_range range_struct;
> >      uint64_t feature_mask;
> > +    Error *local_err = NULL;
> >  
> >      if (qemu_target_page_size() > pagesize) {
> >          error_report("Target page size bigger than host page size");
> > @@ -146,6 +181,12 @@ bool postcopy_ram_supported_by_host(void)
> >          goto out;
> >      }
> >  
> > +    /* Give devices a chance to object */
> > +    if (postcopy_notify(POSTCOPY_NOTIFY_PROBE, &local_err)) {
> > +        error_report_err(local_err);
> > +        goto out;
> > +    }
> > +
> >      /* Version and features check */
> >      if (!ufd_version_check(ufd)) {
> >          goto out;
> > diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
> > index 78a3591322..d688411674 100644
> > --- a/migration/postcopy-ram.h
> > +++ b/migration/postcopy-ram.h
> > @@ -114,4 +114,30 @@ PostcopyState postcopy_state_get(void);
> >  /* Set the state and return the old state */
> >  PostcopyState postcopy_state_set(PostcopyState new_state);
> >  
> > +/*
> > + * To be called once at the start before any device initialisation
> > + */
> > +void postcopy_infrastructure_init(void);
> > +
> > +/* Add a notifier to a list to be called when checking whether the devices
> > + * can support postcopy.
> > + * It's data is a *PostcopyNotifyData
> > + * It should return 0 if OK, or a negative value on failure.
> > + * On failure it must set the data->errp to an error.
> > + *
> > + */
> > +enum PostcopyNotifyReason {
> > +    POSTCOPY_NOTIFY_PROBE = 0,
> > +};
> > +
> > +struct PostcopyNotifyData {
> > +    enum PostcopyNotifyReason reason;
> > +    Error **errp;
> > +};
> > +
> > +void postcopy_add_notifier(NotifierWithReturn *nn);
> > +void postcopy_remove_notifier(NotifierWithReturn *n);
> > +/* Call the notifier list set by postcopy_add_start_notifier */
> > +int postcopy_notify(enum PostcopyNotifyReason reason, Error **errp);
> > +
> >  #endif
> > diff --git a/vl.c b/vl.c
> > index 8e247cc2a2..65dd9dc324 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -95,6 +95,7 @@ int main(int argc, char **argv)
> >  #include "audio/audio.h"
> >  #include "sysemu/cpus.h"
> >  #include "migration/colo.h"
> > +#include "migration/postcopy-ram.h"
> >  #include "sysemu/kvm.h"
> >  #include "sysemu/hax.h"
> >  #include "qapi/qobject-input-visitor.h"
> > @@ -3082,6 +3083,7 @@ int main(int argc, char **argv, char **envp)
> >      module_call_init(MODULE_INIT_OPTS);
> >  
> >      runstate_init();
> > +    postcopy_infrastructure_init();
> >  
> >      if (qcrypto_init(&err) < 0) {
> >          error_reportf_err(err, "cannot initialize crypto: ");
> > -- 
> > 2.13.5
> > 
> 
> -- 
> Peter Xu
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK