Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [Xen-devel] [PATCH] xen: drop writing error messages to xenstore
To:     Juergen Gross <jgross@suse.com>
Cc:     linux-kernel@vger.kernel.org, xen-devel@lists.xenproject.org,
        boris.ostrovsky@oracle.com
References: <20181009160959.31076-1-jgross@suse.com>
From:   Joao Martins <joao.m.martins@oracle.com>
Message-ID: <5126873e-ade5-86b0-4ebf-58cb47c9cbe7@oracle.com>
Date:   Wed, 10 Oct 2018 16:09:02 +0100
MIME-Version: 1.0
In-Reply-To: <20181009160959.31076-1-jgross@suse.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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);
>