From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S941290AbcLVPuS (ORCPT ); Thu, 22 Dec 2016 10:50:18 -0500 Received: from aserp1040.oracle.com ([141.146.126.69]:16385 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752929AbcLVPuR (ORCPT ); Thu, 22 Dec 2016 10:50:17 -0500 Subject: Re: [PATCH 2/3] xen: return xenstore command failures via response instead of rc To: Juergen Gross , linux-kernel@vger.kernel.org, xen-devel@lists.xenproject.org References: <20161222071948.23862-1-jgross@suse.com> <20161222071948.23862-3-jgross@suse.com> From: Boris Ostrovsky Message-ID: <9dc05fb2-cb84-d8e7-848b-37fb9c86b782@oracle.com> Date: Thu, 22 Dec 2016 10:49:21 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20161222071948.23862-3-jgross@suse.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Source-IP: userv0022.oracle.com [156.151.31.74] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 12/22/2016 02:19 AM, Juergen Gross wrote: > When the xenbus driver does some special handling for a Xenstore > command any error condition related to the command should be returned > via an error response instead of letting the related write operation > fail. Otherwise the user land handler might take wrong decisions > assuming the connection to Xenstore is broken. Do we expect the user to always read the reply if no error code was returned? -boris > > While at it try to return the same error values xenstored would > return for those cases. > > Signed-off-by: Juergen Gross > --- > drivers/xen/xenbus/xenbus_dev_frontend.c | 47 ++++++++++++++++++-------------- > 1 file changed, 27 insertions(+), 20 deletions(-) > > diff --git a/drivers/xen/xenbus/xenbus_dev_frontend.c b/drivers/xen/xenbus/xenbus_dev_frontend.c > index a068281..79130b3 100644 > --- a/drivers/xen/xenbus/xenbus_dev_frontend.c > +++ b/drivers/xen/xenbus/xenbus_dev_frontend.c > @@ -302,6 +302,29 @@ static void watch_fired(struct xenbus_watch *watch, > mutex_unlock(&adap->dev_data->reply_mutex); > } > > +static int xenbus_command_reply(struct xenbus_file_priv *u, > + unsigned int msg_type, const char *reply) > +{ > + struct { > + struct xsd_sockmsg hdr; > + const char body[16]; > + } msg; > + int rc; > + > + msg.hdr = u->u.msg; > + msg.hdr.type = msg_type; > + msg.hdr.len = strlen(reply) + 1; > + if (msg.hdr.len > sizeof(msg.body)) > + return -E2BIG; > + > + mutex_lock(&u->reply_mutex); > + rc = queue_reply(&u->read_buffers, &msg, sizeof(msg.hdr) + msg.hdr.len); > + wake_up(&u->read_waitq); > + mutex_unlock(&u->reply_mutex); > + > + return rc; > +} > + > static int xenbus_write_transaction(unsigned msg_type, > struct xenbus_file_priv *u) > { > @@ -321,7 +344,7 @@ static int xenbus_write_transaction(unsigned msg_type, > if (trans->handle.id == u->u.msg.tx_id) > break; > if (&trans->list == &u->transactions) > - return -ESRCH; > + return xenbus_command_reply(u, XS_ERROR, "ENOENT"); > } > > reply = xenbus_dev_request_and_reply(&u->u.msg); > @@ -372,12 +395,12 @@ static int xenbus_write_watch(unsigned msg_type, struct xenbus_file_priv *u) > path = u->u.buffer + sizeof(u->u.msg); > token = memchr(path, 0, u->u.msg.len); > if (token == NULL) { > - rc = -EILSEQ; > + rc = xenbus_command_reply(u, XS_ERROR, "EINVAL"); > goto out; > } > token++; > if (memchr(token, 0, u->u.msg.len - (token - path)) == NULL) { > - rc = -EILSEQ; > + rc = xenbus_command_reply(u, XS_ERROR, "EINVAL"); > goto out; > } > > @@ -411,23 +434,7 @@ static int xenbus_write_watch(unsigned msg_type, struct xenbus_file_priv *u) > } > > /* Success. Synthesize a reply to say all is OK. */ > - { > - struct { > - struct xsd_sockmsg hdr; > - char body[3]; > - } __packed reply = { > - { > - .type = msg_type, > - .len = sizeof(reply.body) > - }, > - "OK" > - }; > - > - mutex_lock(&u->reply_mutex); > - rc = queue_reply(&u->read_buffers, &reply, sizeof(reply)); > - wake_up(&u->read_waitq); > - mutex_unlock(&u->reply_mutex); > - } > + rc = xenbus_command_reply(u, msg_type, "OK"); > > out: > return rc;