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

[Aurelien Chartier <aurelien.chartier@xxxxxxxxxx>]

On 29/05/13 16:44, Konrad Rzeszutek Wilk wrote:
> On Wed, May 29, 2013 at 08:03:13AM +0100, Jan Beulich wrote:
>>>>> 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
> Lets do that as a follow up patch. Aurlien, I've taken your patches in.

Thanks Konrad. I'll send the follow up patch later on.

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