Re: [Xen-devel] [PATCH V5 2/2] xenbus: delay xenbus frontend resume if xenstored is not running

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.05.13 at 19:09, Aurelien Chartier <aurelien.chartier@xxxxxxxxxx> 
>>> wrote:
> If the xenbus frontend is located in a domain running xenstored, the device
> resume is hanging because it is happening before the process resume. This
> patch adds extra logic to the resume code to check if we are the domain
> running xenstored and delay the resume if needed.
> 
> Signed-off-by: Aurelien Chartier <aurelien.chartier@xxxxxxxxxx>
> 
> Changes in v2:
> - Instead of bypassing the resume, process it in a workqueue
> Changes in v3:
> - Add a struct work in xenbus_device to avoid dynamic allocation
> - Several small code fixes
> Changes in v4:
> - Use a dedicated workqueue
> Changes in v5:
> - Move create_workqueue error handling to xenbus_frontend_dev_resume

Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Yet nevertheless I'm still seeing room for improvement:

> +static int xenbus_frontend_dev_resume(struct device *dev)
> +{
> +     /* 
> +      * If xenstored is running in this domain, we cannot access the backend
> +      * state at the moment, so we need to defer xenbus_dev_resume
> +      */
> +     if (xen_store_domain_type == XS_LOCAL) {
> +             struct xenbus_device *xdev = to_xenbus_device(dev);
> +
> +             if (!xenbus_frontend_wq) {
> +                     pr_err("%s: no workqueue to process delayed resume\n",
> +                            xdev->nodename);
> +                     return -EFAULT;

Issuing a message here is fine, but I think you should also issue a
pr_warn() at initialization time.

> +             }
> +
> +             INIT_WORK(&xdev->work, xenbus_frontend_delayed_resume);

And I also think that this would better be done once at initialization
time too.

> +             queue_work(xenbus_frontend_wq, &xdev->work);
> +
> +             return 0;
> +     }

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel