Re: [Xen-devel] [PATCH] xen: drop writing error messages to xenstore

[Joao Martins <joao.m.martins@oracle.com>]

On 10/09/2018 05:09 PM, Juergen Gross wrote:
> xenbus_va_dev_error() will try to write error messages to Xenstore
> under the error/<dev-name>/error node (with <dev-name> something like
> "device/vbd/51872"). This will fail normally and another message
> about this failure is added to dmesg.
> 
> I believe this is a remnant from very ancient times, as it was added
> in the first pvops rush of commits in 2007.
> 
> So remove the additional message when writing to Xenstore failed as
> a minimum step.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> I am considering removing the Xenstore write altogether, but I'm
> not sure it isn't needed e.g. by xend based installations. So please
> speak up in case you know why this write is there.

So this:

"This will fail normally and another message about this failure is added to dmesg."

Brings me to the question: What about {stub,driver}domains? Ideally you
shouldn't be looking at domU's dmesg as a control domain no? I can't remember
any other error node, but if something fails e.g. netfront fails to allocate an
unbound event channel - how do you know the cause from the control domain
perspective?

Irrespective of xend or not: isn't this 'error' node the only one that
propagates error causes per device from domU?

	Joao

> ---
>  drivers/xen/xenbus/xenbus_client.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c
> index a1c17000129b..e17ca8156171 100644
> --- a/drivers/xen/xenbus/xenbus_client.c
> +++ b/drivers/xen/xenbus/xenbus_client.c
> @@ -278,10 +278,8 @@ static void xenbus_va_dev_error(struct xenbus_device *dev, int err,
>  	dev_err(&dev->dev, "%s\n", printf_buffer);
>  
>  	path_buffer = kasprintf(GFP_KERNEL, "error/%s", dev->nodename);
> -	if (!path_buffer ||
> -	    xenbus_write(XBT_NIL, path_buffer, "error", printf_buffer))
> -		dev_err(&dev->dev, "failed to write error node for %s (%s)\n",
> -			dev->nodename, printf_buffer);
> +	if (path_buffer)
> +		xenbus_write(XBT_NIL, path_buffer, "error", printf_buffer);
>  
>  	kfree(printf_buffer);
>  	kfree(path_buffer);
>