[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

>>> On 16.05.13 at 14:34, Aurelien Chartier <aurelien.chartier@xxxxxxxxxx> 
>>> wrote:
> @@ -440,6 +467,13 @@ static int __init xenbus_probe_frontend_init(void)
>       register_xenstore_notifier(&xenstore_notifier);
> +     xenbus_frontend_wq = create_workqueue("xenbus_frontend");
> +     if (!xenbus_frontend_wq) {
> +             printk(KERN_ERR "%s: create "
> +                    "xenbus_frontend_workqueue failed\n", __func__);

pr_err() should be the norm these days.

And personally I consider it quite bad a habit to put function names
in non-debugging log messages - this doesn't really help with
anything (as long as the rest of the message is meaningful), but
clutters the log.

And finally, you need to do proper error handling here - this code
can be built as a module, and hence leaving notifier and bus
registered upon failure sets up the kernel for crashing. Moving
the code ahead of register_xenstore_notifier() will take care of
one half of the problem, but you'll nevertheless will need to call
bus_unregister() if you really intend to make this a fatal error
condition (which by itself is questionable since in most scenarios
you won't need the work queue at all).


> +             return -EFAULT;
> +     }
> +
>       return 0;
>  }
>  subsys_initcall(xenbus_probe_frontend_init);

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.