Re: [Xen-devel] [PATCH] xenbus PM events support (was: [PATCH 2/2] Fix hangup after creating checkpoint on Xen.)

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2011-02-15 at 23:20 +0000, Shriram Rajagopalan wrote:
> The above patch is against upstream/for-linus tree.

In general it is better to pick either a mainline kernel (e.g. the
master branch from
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git),
one of the stable-* or next-* branches from xen.git or a specific topic
branch. The for-* branches are usually just throwaway branches which are
not necessarily kept up to date or relevant standalone. (in this case I
think it's ok since it is just 2.6.38-rc1 + a little bit)

> The xen-netfront driver does not have any suspend handler at all, in this 
> tree.
> I checked other trees (upstream/pvhvm, upstream/xen,
> upstream/netfront). Same issue.

This patch doesn't appear to touch xen-netfront though? 

It looks like the netfront driver only needs to do work on resume, not
suspend. This is consistent with the explanation of why freeze/suspend
and thaw/resume are the same under Xen which you gave in an earlier
mail, isn't it?

A quick "git grep xenbus_driver" doesn't show any of the other frontends
having a suspend callback either.

> So, I dont know which tree Kazuhiro's original patch applies to.
> 
> Shriram
> 
> On Tue, Feb 15, 2011 at 3:10 PM, Shriram Rajagopalan <rshriram@xxxxxxxxx> 
> wrote:
> > From: SUZUKI, Kazuhiro <kaz@xxxxxxxxxxxxxx>
> >
> > Hi,
> >
> > This is a Xen part patch.

Can you improve on this description please.

I meant to respond to this in the initial posting and forgot once the
thread got sidetracked into what the correct approach is.

My advice to "just pipe the mail into git am" may have been a bit
misleading, sorry.

In general it's fine to fixup the description and/or minor nits with the
patch as it passes through your hand. If the changes are significant
it's useful to acknowledge what you've done, many people add a
        [shriram--improved commit message]
style tag below their signed off.

In this case the actual patch itself is fine and:
        Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx> 

Ian.

> >
> > Thanks,
> > KAZ
> >
> > Signed-off-by: Kenji Wakamiya <wkenji@xxxxxxxxxxxxxx>
> > Signed-off-by: Kazuhiro Suzuki <kaz@xxxxxxxxxxxxxx>
> > Signed-off-by: Shriram Rajagopalan <rshriram@xxxxxxxxx>
> > ---
> >  drivers/xen/xenbus/xenbus_probe.c          |   12 ++++++++++--
> >  drivers/xen/xenbus/xenbus_probe.h          |    3 ++-
> >  drivers/xen/xenbus/xenbus_probe_frontend.c |    9 +++++++--
> >  include/xen/xenbus.h                       |    2 +-
> >  4 files changed, 20 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/xen/xenbus/xenbus_probe.c 
> > b/drivers/xen/xenbus/xenbus_probe.c
> > index baa65e7..7397695 100644
> > --- a/drivers/xen/xenbus/xenbus_probe.c
> > +++ b/drivers/xen/xenbus/xenbus_probe.c
> > @@ -577,7 +577,7 @@ void xenbus_dev_changed(const char *node, struct 
> > xen_bus_type *bus)
> >  }
> >  EXPORT_SYMBOL_GPL(xenbus_dev_changed);
> >
> > -int xenbus_dev_suspend(struct device *dev, pm_message_t state)
> > +int xenbus_dev_suspend(struct device *dev)
> >  {
> >        int err = 0;
> >        struct xenbus_driver *drv;
> > @@ -590,7 +590,7 @@ int xenbus_dev_suspend(struct device *dev, pm_message_t 
> > state)
> >                return 0;
> >        drv = to_xenbus_driver(dev->driver);
> >        if (drv->suspend)
> > -               err = drv->suspend(xdev, state);
> > +               err = drv->suspend(xdev);
> >        if (err)
> >                printk(KERN_WARNING
> >                       "xenbus: suspend %s failed: %i\n", dev_name(dev), 
> > err);
> > @@ -642,6 +642,14 @@ int xenbus_dev_resume(struct device *dev)
> >  }
> >  EXPORT_SYMBOL_GPL(xenbus_dev_resume);
> >
> > +int xenbus_dev_cancel(struct device *dev)
> > +{
> > +       /* Do nothing */
> > +       DPRINTK("cancel");
> > +       return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(xenbus_dev_cancel);
> > +
> >  /* A flag to determine if xenstored is 'ready' (i.e. has started) */
> >  int xenstored_ready = 0;
> >
> > diff --git a/drivers/xen/xenbus/xenbus_probe.h 
> > b/drivers/xen/xenbus/xenbus_probe.h
> > index 2466581..888b990 100644
> > --- a/drivers/xen/xenbus/xenbus_probe.h
> > +++ b/drivers/xen/xenbus/xenbus_probe.h
> > @@ -64,8 +64,9 @@ extern void xenbus_dev_changed(const char *node, struct 
> > xen_bus_type *bus);
> >
> >  extern void xenbus_dev_shutdown(struct device *_dev);
> >
> > -extern int xenbus_dev_suspend(struct device *dev, pm_message_t state);
> > +extern int xenbus_dev_suspend(struct device *dev);
> >  extern int xenbus_dev_resume(struct device *dev);
> > +extern int xenbus_dev_cancel(struct device *dev);
> >
> >  extern void xenbus_otherend_changed(struct xenbus_watch *watch,
> >                                    const char **vec, unsigned int len,
> > diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c 
> > b/drivers/xen/xenbus/xenbus_probe_frontend.c
> > index 5bcc2d6..9ad8868 100644
> > --- a/drivers/xen/xenbus/xenbus_probe_frontend.c
> > +++ b/drivers/xen/xenbus/xenbus_probe_frontend.c
> > @@ -85,6 +85,12 @@ static struct device_attribute 
> > xenbus_frontend_dev_attrs[] = {
> >        __ATTR_NULL
> >  };
> >
> > +static struct dev_pm_ops xenbus_pm_ops = {
> > +       .suspend = xenbus_dev_suspend,
> > +       .resume  = xenbus_dev_resume,
> > +       .thaw  = xenbus_dev_cancel,
> > +};
> > +
> >  static struct xen_bus_type xenbus_frontend = {
> >        .root = "device",
> >        .levels = 2,            /* device/type/<id> */
> > @@ -100,8 +106,7 @@ static struct xen_bus_type xenbus_frontend = {
> >                .shutdown       = xenbus_dev_shutdown,
> >                .dev_attrs      = xenbus_frontend_dev_attrs,
> >
> > -               .suspend        = xenbus_dev_suspend,
> > -               .resume         = xenbus_dev_resume,
> > +               .pm             = &xenbus_pm_ops,
> >        },
> >  };
> >
> > diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
> > index 7a1d15f..5467369 100644
> > --- a/include/xen/xenbus.h
> > +++ b/include/xen/xenbus.h
> > @@ -92,7 +92,7 @@ struct xenbus_driver {
> >        void (*otherend_changed)(struct xenbus_device *dev,
> >                                 enum xenbus_state backend_state);
> >        int (*remove)(struct xenbus_device *dev);
> > -       int (*suspend)(struct xenbus_device *dev, pm_message_t state);
> > +       int (*suspend)(struct xenbus_device *dev);
> >        int (*resume)(struct xenbus_device *dev);
> >        int (*uevent)(struct xenbus_device *, struct kobj_uevent_env *);
> >        struct device_driver driver;
> > --
> > 1.7.0.4
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxxxxxxxx
> > http://lists.xensource.com/xen-devel
> >
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel