[{"id":1760662,"web_url":"http://patchwork.ozlabs.org/comment/1760662/","msgid":"","list_archive_url":null,"date":"2017-08-31T05:51:17","subject":"Re: [Qemu-devel] [PATCH 1/9] s390x/css: fix cc handling for XSCH","submitter":{"id":66152,"url":"http://patchwork.ozlabs.org/api/people/66152/","name":"Thomas Huth","email":"thuth@redhat.com"},"content":"On 30.08.2017 18:36, Halil Pasic wrote:\n> The function ioinst_handle_xsch is presenting cc 2 when it's supposed to\n> present cc 1 and the other way around, because css_do_xsch has the error\n> codes mixed up. Fixing the error codes also fixes the priority.\n> \n> Let us fix this.\n\n(Nit: In case you respin, I'd suggest to remove the last sentence. You\nalready used \"fix\" two times in the previous one)\n\n> Signed-off-by: Halil Pasic \n> Reported-by: Pierre Morel\n\nSpace missing -------------^\n\n> ---\n> hw/s390x/css.c | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/hw/s390x/css.c b/hw/s390x/css.c\n> index 1880b1a0ff..a50fb0727e 100644\n> --- a/hw/s390x/css.c\n> +++ b/hw/s390x/css.c\n> @@ -1281,12 +1281,12 @@ int css_do_xsch(SubchDev *sch)\n> (!(s->ctrl &\n> (SCSW_ACTL_RESUME_PEND | SCSW_ACTL_START_PEND | SCSW_ACTL_SUSP))) ||\n> (s->ctrl & SCSW_ACTL_SUBCH_ACTIVE)) {\n> - ret = -EINPROGRESS;\n> + ret = -EBUSY;\n> goto out;\n> }\n> \n> if (s->ctrl & SCSW_CTRL_MASK_STCTL) {\n> - ret = -EBUSY;\n> + ret = -EINPROGRESS;\n> goto out;\n> }\n\nUsing both, EBUSY and EINPROGRESS as error codes sounds very confusing\nto me here ... what's the difference between busy and in-progress? So\nwhile you're at it, maybe you could replace the code for CC 2 (\"CANCEL\nSUBCHANNEL not applicable\") with a different error code?\n\n Thomas","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx09.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx09.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=thuth@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xjWj571KNz9s83\n\tfor ;\n\tThu, 31 Aug 2017 15:51:50 +1000 (AEST)","from localhost ([::1]:54042 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dnIOH-0001g6-3s\n\tfor incoming@patchwork.ozlabs.org; Thu, 31 Aug 2017 01:51:45 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:44664)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dnINw-0001fC-Of\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 01:51:25 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dnINt-0007Wd-LC\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 01:51:24 -0400","from mx1.redhat.com ([209.132.183.28]:49270)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dnINt-0007WT-Ei\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 01:51:21 -0400","from smtp.corp.redhat.com\n\t(int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 07B5B4A6E4;\n\tThu, 31 Aug 2017 05:51:20 +0000 (UTC)","from [10.36.116.27] (ovpn-116-27.ams2.redhat.com [10.36.116.27])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id BCDA2757D1;\n\tThu, 31 Aug 2017 05:51:18 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 07B5B4A6E4","To":"Halil Pasic , Cornelia Huck ","References":"<20170830163609.50260-1-pasic@linux.vnet.ibm.com>\n\t<20170830163609.50260-2-pasic@linux.vnet.ibm.com>","From":"Thomas Huth ","Message-ID":"","Date":"Thu, 31 Aug 2017 07:51:17 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170830163609.50260-2-pasic@linux.vnet.ibm.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.14","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.38]);\n\tThu, 31 Aug 2017 05:51:20 +0000 (UTC)","Content-Transfer-Encoding":"quoted-printable","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 1/9] s390x/css: fix cc handling for XSCH","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Dong Jia Shi ,\n\tPierre Morel , qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1760678,"web_url":"http://patchwork.ozlabs.org/comment/1760678/","msgid":"<20170831083859.11694707.cohuck@redhat.com>","list_archive_url":null,"date":"2017-08-31T06:38:59","subject":"Re: [Qemu-devel] [PATCH 1/9] s390x/css: fix cc handling for XSCH","submitter":{"id":71914,"url":"http://patchwork.ozlabs.org/api/people/71914/","name":"Cornelia Huck","email":"cohuck@redhat.com"},"content":"On Thu, 31 Aug 2017 07:51:17 +0200\nThomas Huth wrote:\n\n> On 30.08.2017 18:36, Halil Pasic wrote:\n> > The function ioinst_handle_xsch is presenting cc 2 when it's supposed to\n> > present cc 1 and the other way around, because css_do_xsch has the error\n> > codes mixed up. Fixing the error codes also fixes the priority.\n> > \n> > Let us fix this. \n> \n> (Nit: In case you respin, I'd suggest to remove the last sentence. You\n> already used \"fix\" two times in the previous one)\n\nI can also remove it on applying, if Halil agrees (I have not yet\nreviewed it, though).\n\n> \n> > Signed-off-by: Halil Pasic \n> > Reported-by: Pierre Morel \n> \n> Space missing -------------^\n\nAnd I can also add that space on applying :)\n\n> \n> > ---\n> > hw/s390x/css.c | 4 ++--\n> > 1 file changed, 2 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/hw/s390x/css.c b/hw/s390x/css.c\n> > index 1880b1a0ff..a50fb0727e 100644\n> > --- a/hw/s390x/css.c\n> > +++ b/hw/s390x/css.c\n> > @@ -1281,12 +1281,12 @@ int css_do_xsch(SubchDev *sch)\n> > (!(s->ctrl &\n> > (SCSW_ACTL_RESUME_PEND | SCSW_ACTL_START_PEND | SCSW_ACTL_SUSP))) ||\n> > (s->ctrl & SCSW_ACTL_SUBCH_ACTIVE)) {\n> > - ret = -EINPROGRESS;\n> > + ret = -EBUSY;\n> > goto out;\n> > }\n> > \n> > if (s->ctrl & SCSW_CTRL_MASK_STCTL) {\n> > - ret = -EBUSY;\n> > + ret = -EINPROGRESS;\n> > goto out;\n> > } \n> \n> Using both, EBUSY and EINPROGRESS as error codes sounds very confusing\n> to me here ... what's the difference between busy and in-progress? So\n> while you're at it, maybe you could replace the code for CC 2 (\"CANCEL\n> SUBCHANNEL not applicable\") with a different error code?\n\nIIRC, I used these two as they matched my idea of what happens best\n(there's a difference between \"subchannel is busy\" and \"the I/O is\nalready in progress, too late to cancel\"). \"xsch not applicable\" is\nvery hard to translate to an Unix error code :/\n\nI'll double-check with the PoP.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx05.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx05.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=cohuck@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xjXm15jS0z9s7F\n\tfor ;\n\tThu, 31 Aug 2017 16:39:29 +1000 (AEST)","from localhost ([::1]:54140 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dnJ8R-0004Kj-VB\n\tfor incoming@patchwork.ozlabs.org; Thu, 31 Aug 2017 02:39:27 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:50945)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dnJ88-0004Ke-QE\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 02:39:09 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dnJ85-00048x-Jf\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 02:39:08 -0400","from mx1.redhat.com ([209.132.183.28]:56284)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dnJ85-00048Y-Cd\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 02:39:05 -0400","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 5092913A94;\n\tThu, 31 Aug 2017 06:39:04 +0000 (UTC)","from gondolin (ovpn-116-164.ams2.redhat.com [10.36.116.164])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id D487DA2A7E;\n\tThu, 31 Aug 2017 06:39:02 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 5092913A94","Date":"Thu, 31 Aug 2017 08:38:59 +0200","From":"Cornelia Huck ","To":"Thomas Huth ","Message-ID":"<20170831083859.11694707.cohuck@redhat.com>","In-Reply-To":"","References":"<20170830163609.50260-1-pasic@linux.vnet.ibm.com>\n\t<20170830163609.50260-2-pasic@linux.vnet.ibm.com>\n\t","Organization":"Red Hat GmbH","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.12","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.29]);\n\tThu, 31 Aug 2017 06:39:04 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 1/9] s390x/css: fix cc handling for XSCH","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Halil Pasic ,\n\tPierre Morel ,\n\tDong Jia Shi , qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1760710,"web_url":"http://patchwork.ozlabs.org/comment/1760710/","msgid":"","list_archive_url":null,"date":"2017-08-31T07:32:49","subject":"Re: [Qemu-devel] [PATCH 1/9] s390x/css: fix cc handling for XSCH","submitter":{"id":66152,"url":"http://patchwork.ozlabs.org/api/people/66152/","name":"Thomas Huth","email":"thuth@redhat.com"},"content":"On 31.08.2017 08:38, Cornelia Huck wrote:\n> On Thu, 31 Aug 2017 07:51:17 +0200\n> Thomas Huth wrote:\n> \n>> On 30.08.2017 18:36, Halil Pasic wrote:\n>>> The function ioinst_handle_xsch is presenting cc 2 when it's supposed to\n>>> present cc 1 and the other way around, because css_do_xsch has the error\n>>> codes mixed up. Fixing the error codes also fixes the priority.\n>>>\n>>> Let us fix this. \n>>\n>> (Nit: In case you respin, I'd suggest to remove the last sentence. You\n>> already used \"fix\" two times in the previous one)\n> \n> I can also remove it on applying, if Halil agrees (I have not yet\n> reviewed it, though).\n> \n>>\n>>> Signed-off-by: Halil Pasic \n>>> Reported-by: Pierre Morel \n>>\n>> Space missing -------------^\n> \n> And I can also add that space on applying :)\n> \n>>\n>>> ---\n>>> hw/s390x/css.c | 4 ++--\n>>> 1 file changed, 2 insertions(+), 2 deletions(-)\n>>>\n>>> diff --git a/hw/s390x/css.c b/hw/s390x/css.c\n>>> index 1880b1a0ff..a50fb0727e 100644\n>>> --- a/hw/s390x/css.c\n>>> +++ b/hw/s390x/css.c\n>>> @@ -1281,12 +1281,12 @@ int css_do_xsch(SubchDev *sch)\n>>> (!(s->ctrl &\n>>> (SCSW_ACTL_RESUME_PEND | SCSW_ACTL_START_PEND | SCSW_ACTL_SUSP))) ||\n>>> (s->ctrl & SCSW_ACTL_SUBCH_ACTIVE)) {\n>>> - ret = -EINPROGRESS;\n>>> + ret = -EBUSY;\n>>> goto out;\n>>> }\n>>> \n>>> if (s->ctrl & SCSW_CTRL_MASK_STCTL) {\n>>> - ret = -EBUSY;\n>>> + ret = -EINPROGRESS;\n>>> goto out;\n>>> } \n>>\n>> Using both, EBUSY and EINPROGRESS as error codes sounds very confusing\n>> to me here ... what's the difference between busy and in-progress? So\n>> while you're at it, maybe you could replace the code for CC 2 (\"CANCEL\n>> SUBCHANNEL not applicable\") with a different error code?\n> \n> IIRC, I used these two as they matched my idea of what happens best\n> (there's a difference between \"subchannel is busy\" and \"the I/O is\n> already in progress, too late to cancel\"). \"xsch not applicable\" is\n> very hard to translate to an Unix error code :/\n\nOK, the codes make more sense with your description ==> Maybe simply add\na proper comment for each of the return codes?\n\n Thomas","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx09.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx09.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=thuth@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xjYyQ4sW6z9sPt\n\tfor ;\n\tThu, 31 Aug 2017 17:33:30 +1000 (AEST)","from localhost ([::1]:54315 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dnJyh-0004lW-J0\n\tfor incoming@patchwork.ozlabs.org; Thu, 31 Aug 2017 03:33:27 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:33019)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dnJyD-0004k5-Ix\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 03:32:58 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dnJyA-0000bv-Bz\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 03:32:57 -0400","from mx1.redhat.com ([209.132.183.28]:39548)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dnJyA-0000bh-2q\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 03:32:54 -0400","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id A8D8B4E4C8;\n\tThu, 31 Aug 2017 07:32:52 +0000 (UTC)","from [10.36.116.27] (ovpn-116-27.ams2.redhat.com [10.36.116.27])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 65D51A8440;\n\tThu, 31 Aug 2017 07:32:51 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com A8D8B4E4C8","To":"Cornelia Huck ","References":"<20170830163609.50260-1-pasic@linux.vnet.ibm.com>\n\t<20170830163609.50260-2-pasic@linux.vnet.ibm.com>\n\t\n\t<20170831083859.11694707.cohuck@redhat.com>","From":"Thomas Huth ","Message-ID":"","Date":"Thu, 31 Aug 2017 09:32:49 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170831083859.11694707.cohuck@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.12","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.38]);\n\tThu, 31 Aug 2017 07:32:52 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 1/9] s390x/css: fix cc handling for XSCH","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Halil Pasic ,\n\tPierre Morel ,\n\tDong Jia Shi , qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1760748,"web_url":"http://patchwork.ozlabs.org/comment/1760748/","msgid":"<20170831104243.43cd8992.cohuck@redhat.com>","list_archive_url":null,"date":"2017-08-31T08:42:43","subject":"Re: [Qemu-devel] [PATCH 1/9] s390x/css: fix cc handling for XSCH","submitter":{"id":71914,"url":"http://patchwork.ozlabs.org/api/people/71914/","name":"Cornelia Huck","email":"cohuck@redhat.com"},"content":"On Thu, 31 Aug 2017 09:32:49 +0200\nThomas Huth wrote:\n\n> On 31.08.2017 08:38, Cornelia Huck wrote:\n> > On Thu, 31 Aug 2017 07:51:17 +0200\n> > Thomas Huth wrote:\n> > \n> >> On 30.08.2017 18:36, Halil Pasic wrote: \n> >>> The function ioinst_handle_xsch is presenting cc 2 when it's supposed to\n> >>> present cc 1 and the other way around, because css_do_xsch has the error\n> >>> codes mixed up. Fixing the error codes also fixes the priority.\n> >>>\n> >>> Let us fix this. \n> >>\n> >> (Nit: In case you respin, I'd suggest to remove the last sentence. You\n> >> already used \"fix\" two times in the previous one) \n> > \n> > I can also remove it on applying, if Halil agrees (I have not yet\n> > reviewed it, though).\n> > \n> >> \n> >>> Signed-off-by: Halil Pasic \n> >>> Reported-by: Pierre Morel \n> >>\n> >> Space missing -------------^ \n> > \n> > And I can also add that space on applying :)\n> > \n> >> \n> >>> ---\n> >>> hw/s390x/css.c | 4 ++--\n> >>> 1 file changed, 2 insertions(+), 2 deletions(-)\n> >>>\n> >>> diff --git a/hw/s390x/css.c b/hw/s390x/css.c\n> >>> index 1880b1a0ff..a50fb0727e 100644\n> >>> --- a/hw/s390x/css.c\n> >>> +++ b/hw/s390x/css.c\n> >>> @@ -1281,12 +1281,12 @@ int css_do_xsch(SubchDev *sch)\n> >>> (!(s->ctrl &\n> >>> (SCSW_ACTL_RESUME_PEND | SCSW_ACTL_START_PEND | SCSW_ACTL_SUSP))) ||\n> >>> (s->ctrl & SCSW_ACTL_SUBCH_ACTIVE)) {\n> >>> - ret = -EINPROGRESS;\n> >>> + ret = -EBUSY;\n> >>> goto out;\n> >>> }\n> >>> \n> >>> if (s->ctrl & SCSW_CTRL_MASK_STCTL) {\n> >>> - ret = -EBUSY;\n> >>> + ret = -EINPROGRESS;\n> >>> goto out;\n> >>> } \n> >>\n> >> Using both, EBUSY and EINPROGRESS as error codes sounds very confusing\n> >> to me here ... what's the difference between busy and in-progress? So\n> >> while you're at it, maybe you could replace the code for CC 2 (\"CANCEL\n> >> SUBCHANNEL not applicable\") with a different error code? \n> > \n> > IIRC, I used these two as they matched my idea of what happens best\n> > (there's a difference between \"subchannel is busy\" and \"the I/O is\n> > already in progress, too late to cancel\"). \"xsch not applicable\" is\n> > very hard to translate to an Unix error code :/ \n> \n> OK, the codes make more sense with your description ==> Maybe simply add\n> a proper comment for each of the return codes?\n\nTaking a step back and looking at the other I/O instructions and their\nimplementation in qemu:\n\n- For those instructions that can set it, cc 1 is set when the\n subchannel is status pending. That's usually the \"default\" branch in\n ioinst.c.\n- cc 2 is set when the subchannel is \"busy\" (or, in the case of xsch,\n \"not applicable for xsch\"... sigh) This is supposed to be handled via\n -EBUSY.\n\nSo, there are actually two problems with the current implementation of\nxsch:\n\n- The return codes are switched around, which this patch fixes.\n- But \"status pending\" should also take precedence over \"not\n applicable\", if I read the PoP correctly, so the second if needs to\n be moved up.\n\nAnd yes, it makes sense do add some comments...","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=cohuck@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xjbW60qGgz9t2Q\n\tfor ;\n\tThu, 31 Aug 2017 18:43:28 +1000 (AEST)","from localhost ([::1]:54544 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dnL4O-0001oU-Oj\n\tfor incoming@patchwork.ozlabs.org; Thu, 31 Aug 2017 04:43:24 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:47180)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dnL3u-0001nj-2l\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 04:42:55 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dnL3p-0004I2-3T\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 04:42:54 -0400","from mx1.redhat.com ([209.132.183.28]:49720)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dnL3o-0004Hw-Qq\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 04:42:49 -0400","from smtp.corp.redhat.com\n\t(int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id CAE525F7B7;\n\tThu, 31 Aug 2017 08:42:46 +0000 (UTC)","from gondolin (dhcp-192-215.str.redhat.com [10.33.192.215])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id A14EC91B38;\n\tThu, 31 Aug 2017 08:42:45 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com CAE525F7B7","Date":"Thu, 31 Aug 2017 10:42:43 +0200","From":"Cornelia Huck ","To":"Thomas Huth ","Message-ID":"<20170831104243.43cd8992.cohuck@redhat.com>","In-Reply-To":"","References":"<20170830163609.50260-1-pasic@linux.vnet.ibm.com>\n\t<20170830163609.50260-2-pasic@linux.vnet.ibm.com>\n\t\n\t<20170831083859.11694707.cohuck@redhat.com>\n\t","Organization":"Red Hat GmbH","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.11","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.39]);\n\tThu, 31 Aug 2017 08:42:47 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 1/9] s390x/css: fix cc handling for XSCH","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Halil Pasic ,\n\tPierre Morel ,\n\tDong Jia Shi , qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1760761,"web_url":"http://patchwork.ozlabs.org/comment/1760761/","msgid":"<4e253d22-8b62-aaf8-f0ff-609853caf22c@linux.vnet.ibm.com>","list_archive_url":null,"date":"2017-08-31T09:09:28","subject":"Re: [Qemu-devel] [PATCH 1/9] s390x/css: fix cc handling for XSCH","submitter":{"id":68297,"url":"http://patchwork.ozlabs.org/api/people/68297/","name":"Halil Pasic","email":"pasic@linux.vnet.ibm.com"},"content":"On 08/31/2017 07:51 AM, Thomas Huth wrote:\n> On 30.08.2017 18:36, Halil Pasic wrote:\n>> The function ioinst_handle_xsch is presenting cc 2 when it's supposed to\n>> present cc 1 and the other way around, because css_do_xsch has the error\n>> codes mixed up. Fixing the error codes also fixes the priority.\n>>\n>> Let us fix this.\n> \n> (Nit: In case you respin, I'd suggest to remove the last sentence. You\n> already used \"fix\" two times in the previous one)\n> \n>> Signed-off-by: Halil Pasic \n>> Reported-by: Pierre Morel\n> \n> Space missing -------------^\n> \n\ncopy-paste :(\n\n>> ---\n>> hw/s390x/css.c | 4 ++--\n>> 1 file changed, 2 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/hw/s390x/css.c b/hw/s390x/css.c\n>> index 1880b1a0ff..a50fb0727e 100644\n>> --- a/hw/s390x/css.c\n>> +++ b/hw/s390x/css.c\n>> @@ -1281,12 +1281,12 @@ int css_do_xsch(SubchDev *sch)\n>> (!(s->ctrl &\n>> (SCSW_ACTL_RESUME_PEND | SCSW_ACTL_START_PEND | SCSW_ACTL_SUSP))) ||\n>> (s->ctrl & SCSW_ACTL_SUBCH_ACTIVE)) {\n>> - ret = -EINPROGRESS;\n>> + ret = -EBUSY;\n>> goto out;\n>> }\n>> \n>> if (s->ctrl & SCSW_CTRL_MASK_STCTL) {\n>> - ret = -EBUSY;\n>> + ret = -EINPROGRESS;\n>> goto out;\n>> }\n> \n> Using both, EBUSY and EINPROGRESS as error codes sounds very confusing\n> to me here ... what's the difference between busy and in-progress? So\n> while you're at it, maybe you could replace the code for CC 2 (\"CANCEL\n> SUBCHANNEL not applicable\") with a different error code?\n> \n> Thomas\n> \n\nWell, the idea of the series is to get rid of these artificial error codes,\nso your concern of using EBUSY and EINPROGRESS will be dealt with in patch\n5.\n\nThe idea was to first do the fixes and then do the transformation without\nchanging behavior.\n\nThanks for having a look!\n\nRegards,\n\nHalil","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xjc5l5CGZz9sRW\n\tfor ;\n\tThu, 31 Aug 2017 19:10:02 +1000 (AEST)","from localhost ([::1]:54591 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dnLU6-0007Kv-B3\n\tfor incoming@patchwork.ozlabs.org; Thu, 31 Aug 2017 05:09:58 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:52020)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dnLTl-0007Kc-Gw\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 05:09:38 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dnLTh-0004hI-DM\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 05:09:37 -0400","from mx0b-001b2d01.pphosted.com ([148.163.158.5]:52354\n\thelo=mx0a-001b2d01.pphosted.com)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dnLTh-0004gs-7G\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 05:09:33 -0400","from pps.filterd (m0098416.ppops.net [127.0.0.1])\n\tby mx0b-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tv7V94Mh0059589\n\tfor ; Thu, 31 Aug 2017 05:09:32 -0400","from e06smtp10.uk.ibm.com (e06smtp10.uk.ibm.com [195.75.94.106])\n\tby mx0b-001b2d01.pphosted.com with ESMTP id 2cpca8jrxm-1\n\t(version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT)\n\tfor ; Thu, 31 Aug 2017 05:09:32 -0400","from localhost\n\tby e06smtp10.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use\n\tOnly! Violators will be prosecuted\n\tfor from ;\n\tThu, 31 Aug 2017 10:09:30 +0100","from b06cxnps4075.portsmouth.uk.ibm.com (9.149.109.197)\n\tby e06smtp10.uk.ibm.com (192.168.101.140) with IBM ESMTP SMTP\n\tGateway: Authorized Use Only! Violators will be prosecuted; \n\tThu, 31 Aug 2017 10:09:28 +0100","from d06av25.portsmouth.uk.ibm.com (d06av25.portsmouth.uk.ibm.com\n\t[9.149.105.61])\n\tby b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with\n\tESMTP id v7V99SMJ16449672; Thu, 31 Aug 2017 09:09:28 GMT","from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id B2E3D11C04C;\n\tThu, 31 Aug 2017 10:05:57 +0100 (BST)","from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id 5CA0611C04A;\n\tThu, 31 Aug 2017 10:05:57 +0100 (BST)","from oc3836556865.ibm.com (unknown [9.152.224.207])\n\tby d06av25.portsmouth.uk.ibm.com (Postfix) with ESMTP;\n\tThu, 31 Aug 2017 10:05:57 +0100 (BST)"],"To":"Thomas Huth , Cornelia Huck ","References":"<20170830163609.50260-1-pasic@linux.vnet.ibm.com>\n\t<20170830163609.50260-2-pasic@linux.vnet.ibm.com>\n\t","From":"Halil Pasic ","Date":"Thu, 31 Aug 2017 11:09:28 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.0","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-TM-AS-GCONF":"00","x-cbid":"17083109-0040-0000-0000-000003D49D92","X-IBM-AV-DETECTION":"SAVI=unused REMOTE=unused XFE=unused","x-cbparentid":"17083109-0041-0000-0000-000025D511A8","Message-Id":"<4e253d22-8b62-aaf8-f0ff-609853caf22c@linux.vnet.ibm.com>","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-08-31_03:, , signatures=0","X-Proofpoint-Spam-Details":"rule=outbound_notspam policy=outbound score=0\n\tspamscore=0 suspectscore=0\n\tmalwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam\n\tadjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000\n\tdefinitions=main-1708310138","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 3.x [generic] [fuzzy]","X-Received-From":"148.163.158.5","Subject":"Re: [Qemu-devel] [PATCH 1/9] s390x/css: fix cc handling for XSCH","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Dong Jia Shi ,\n\tPierre Morel , qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1760767,"web_url":"http://patchwork.ozlabs.org/comment/1760767/","msgid":"<4ee35bcf-9acd-facb-1d06-d585ca8f3ec9@redhat.com>","list_archive_url":null,"date":"2017-08-31T09:16:46","subject":"Re: [Qemu-devel] [PATCH 1/9] s390x/css: fix cc handling for XSCH","submitter":{"id":66152,"url":"http://patchwork.ozlabs.org/api/people/66152/","name":"Thomas Huth","email":"thuth@redhat.com"},"content":"On 31.08.2017 11:09, Halil Pasic wrote:\n> \n> \n> On 08/31/2017 07:51 AM, Thomas Huth wrote:\n>> On 30.08.2017 18:36, Halil Pasic wrote:\n>>> The function ioinst_handle_xsch is presenting cc 2 when it's supposed to\n>>> present cc 1 and the other way around, because css_do_xsch has the error\n>>> codes mixed up. Fixing the error codes also fixes the priority.\n>>>\n>>> Let us fix this.\n>>\n>> (Nit: In case you respin, I'd suggest to remove the last sentence. You\n>> already used \"fix\" two times in the previous one)\n>>\n>>> Signed-off-by: Halil Pasic \n>>> Reported-by: Pierre Morel\n>>\n>> Space missing -------------^\n>>\n> \n> copy-paste :(\n> \n>>> ---\n>>> hw/s390x/css.c | 4 ++--\n>>> 1 file changed, 2 insertions(+), 2 deletions(-)\n>>>\n>>> diff --git a/hw/s390x/css.c b/hw/s390x/css.c\n>>> index 1880b1a0ff..a50fb0727e 100644\n>>> --- a/hw/s390x/css.c\n>>> +++ b/hw/s390x/css.c\n>>> @@ -1281,12 +1281,12 @@ int css_do_xsch(SubchDev *sch)\n>>> (!(s->ctrl &\n>>> (SCSW_ACTL_RESUME_PEND | SCSW_ACTL_START_PEND | SCSW_ACTL_SUSP))) ||\n>>> (s->ctrl & SCSW_ACTL_SUBCH_ACTIVE)) {\n>>> - ret = -EINPROGRESS;\n>>> + ret = -EBUSY;\n>>> goto out;\n>>> }\n>>> \n>>> if (s->ctrl & SCSW_CTRL_MASK_STCTL) {\n>>> - ret = -EBUSY;\n>>> + ret = -EINPROGRESS;\n>>> goto out;\n>>> }\n>>\n>> Using both, EBUSY and EINPROGRESS as error codes sounds very confusing\n>> to me here ... what's the difference between busy and in-progress? So\n>> while you're at it, maybe you could replace the code for CC 2 (\"CANCEL\n>> SUBCHANNEL not applicable\") with a different error code?\n>>\n>> Thomas\n>>\n> \n> Well, the idea of the series is to get rid of these artificial error codes,\n> so your concern of using EBUSY and EINPROGRESS will be dealt with in patch\n> 5.\n> \n> The idea was to first do the fixes and then do the transformation without\n> changing behavior.\n\nYeah, I realized that when I started to look at the later patches ... so\nplease ignore my comment, it should be OK the way you're doing it.\n\n Thomas","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=thuth@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xjcGL5jy8z9sRW\n\tfor ;\n\tThu, 31 Aug 2017 19:17:30 +1000 (AEST)","from localhost ([::1]:54613 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dnLbM-0002JH-Td\n\tfor incoming@patchwork.ozlabs.org; Thu, 31 Aug 2017 05:17:28 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:53388)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dnLap-0002B7-GV\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 05:16:56 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dnLam-0007Wo-7y\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 05:16:55 -0400","from mx1.redhat.com ([209.132.183.28]:59194)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dnLam-0007WX-0n\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 05:16:52 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 0676E5F7AD;\n\tThu, 31 Aug 2017 09:16:51 +0000 (UTC)","from [10.36.116.27] (ovpn-116-27.ams2.redhat.com [10.36.116.27])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id BA47A9C134;\n\tThu, 31 Aug 2017 09:16:49 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 0676E5F7AD","To":"Halil Pasic , Cornelia Huck ","References":"<20170830163609.50260-1-pasic@linux.vnet.ibm.com>\n\t<20170830163609.50260-2-pasic@linux.vnet.ibm.com>\n\t\n\t<4e253d22-8b62-aaf8-f0ff-609853caf22c@linux.vnet.ibm.com>","From":"Thomas Huth ","Message-ID":"<4ee35bcf-9acd-facb-1d06-d585ca8f3ec9@redhat.com>","Date":"Thu, 31 Aug 2017 11:16:46 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<4e253d22-8b62-aaf8-f0ff-609853caf22c@linux.vnet.ibm.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.39]);\n\tThu, 31 Aug 2017 09:16:51 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 1/9] s390x/css: fix cc handling for XSCH","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Dong Jia Shi ,\n\tPierre Morel , qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1760809,"web_url":"http://patchwork.ozlabs.org/comment/1760809/","msgid":"","list_archive_url":null,"date":"2017-08-31T10:19:04","subject":"Re: [Qemu-devel] [PATCH 1/9] s390x/css: fix cc handling for XSCH","submitter":{"id":68297,"url":"http://patchwork.ozlabs.org/api/people/68297/","name":"Halil Pasic","email":"pasic@linux.vnet.ibm.com"},"content":"On 08/31/2017 10:42 AM, Cornelia Huck wrote:\n> On Thu, 31 Aug 2017 09:32:49 +0200\n> Thomas Huth wrote:\n> \n>> On 31.08.2017 08:38, Cornelia Huck wrote:\n>>> On Thu, 31 Aug 2017 07:51:17 +0200\n>>> Thomas Huth wrote:\n>>> \n>>>> On 30.08.2017 18:36, Halil Pasic wrote: \n>>>>> The function ioinst_handle_xsch is presenting cc 2 when it's supposed to\n>>>>> present cc 1 and the other way around, because css_do_xsch has the error\n>>>>> codes mixed up. Fixing the error codes also fixes the priority.\n>>>>>\n>>>>> Let us fix this. \n>>>>\n>>>> (Nit: In case you respin, I'd suggest to remove the last sentence. You\n>>>> already used \"fix\" two times in the previous one) \n>>>\n>>> I can also remove it on applying, if Halil agrees (I have not yet\n>>> reviewed it, though).\n>>> \n>>>> \n>>>>> Signed-off-by: Halil Pasic \n>>>>> Reported-by: Pierre Morel \n>>>>\n>>>> Space missing -------------^ \n>>>\n>>> And I can also add that space on applying :)\n>>> \n>>>> \n>>>>> ---\n>>>>> hw/s390x/css.c | 4 ++--\n>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)\n>>>>>\n>>>>> diff --git a/hw/s390x/css.c b/hw/s390x/css.c\n>>>>> index 1880b1a0ff..a50fb0727e 100644\n>>>>> --- a/hw/s390x/css.c\n>>>>> +++ b/hw/s390x/css.c\n>>>>> @@ -1281,12 +1281,12 @@ int css_do_xsch(SubchDev *sch)\n>>>>> (!(s->ctrl &\n>>>>> (SCSW_ACTL_RESUME_PEND | SCSW_ACTL_START_PEND | SCSW_ACTL_SUSP))) ||\n>>>>> (s->ctrl & SCSW_ACTL_SUBCH_ACTIVE)) {\n>>>>> - ret = -EINPROGRESS;\n>>>>> + ret = -EBUSY;\n>>>>> goto out;\n>>>>> }\n>>>>> \n>>>>> if (s->ctrl & SCSW_CTRL_MASK_STCTL) {\n>>>>> - ret = -EBUSY;\n>>>>> + ret = -EINPROGRESS;\n>>>>> goto out;\n>>>>> } \n>>>>\n>>>> Using both, EBUSY and EINPROGRESS as error codes sounds very confusing\n>>>> to me here ... what's the difference between busy and in-progress? So\n>>>> while you're at it, maybe you could replace the code for CC 2 (\"CANCEL\n>>>> SUBCHANNEL not applicable\") with a different error code? \n>>>\n>>> IIRC, I used these two as they matched my idea of what happens best\n>>> (there's a difference between \"subchannel is busy\" and \"the I/O is\n>>> already in progress, too late to cancel\"). \"xsch not applicable\" is\n>>> very hard to translate to an Unix error code :/ \n>>\n>> OK, the codes make more sense with your description ==> Maybe simply add\n>> a proper comment for each of the return codes?\n> \n> Taking a step back and looking at the other I/O instructions and their\n> implementation in qemu:\n> \n> - For those instructions that can set it, cc 1 is set when the\n> subchannel is status pending. That's usually the \"default\" branch in\n> ioinst.c.\n> - cc 2 is set when the subchannel is \"busy\" (or, in the case of xsch,\n> \"not applicable for xsch\"... sigh) This is supposed to be handled via\n> -EBUSY.\n> \n> So, there are actually two problems with the current implementation of\n> xsch:\n> \n> - The return codes are switched around, which this patch fixes.\n> - But \"status pending\" should also take precedence over \"not\n> applicable\", if I read the PoP correctly, so the second if needs to\n> be moved up.\n\nYou are right and I was wrong. \"Condition code 1 has precedence over\ncondition code 2.\" So it's 3 > 1 > 2 (and I remembered 3 > 2 > 1).\n\nI will fix this for v2.\n\n> \n> And yes, it makes sense do add some comments...\n> \n\nIf we apply the series as a whole adding comments would an overkill\nIMHO. We will switch this to iret.cc = ? so it should become obvious.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xjdgT2xZzz9sPm\n\tfor ;\n\tThu, 31 Aug 2017 20:20:53 +1000 (AEST)","from localhost ([::1]:54790 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dnMah-0005Zk-IV\n\tfor incoming@patchwork.ozlabs.org; Thu, 31 Aug 2017 06:20:51 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:36381)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dnMZ9-0004cA-GR\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 06:19:18 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dnMZ4-0006uX-Ia\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 06:19:15 -0400","from mx0b-001b2d01.pphosted.com ([148.163.158.5]:33963\n\thelo=mx0a-001b2d01.pphosted.com)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dnMZ4-0006uI-9E\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 06:19:10 -0400","from pps.filterd (m0098413.ppops.net [127.0.0.1])\n\tby mx0b-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tv7VAIsng048290\n\tfor ; Thu, 31 Aug 2017 06:19:09 -0400","from e06smtp14.uk.ibm.com (e06smtp14.uk.ibm.com [195.75.94.110])\n\tby mx0b-001b2d01.pphosted.com with ESMTP id 2cpbh8hmfq-1\n\t(version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT)\n\tfor ; Thu, 31 Aug 2017 06:19:09 -0400","from localhost\n\tby e06smtp14.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use\n\tOnly! Violators will be prosecuted\n\tfor from ;\n\tThu, 31 Aug 2017 11:19:07 +0100","from b06cxnps3075.portsmouth.uk.ibm.com (9.149.109.195)\n\tby e06smtp14.uk.ibm.com (192.168.101.144) with IBM ESMTP SMTP\n\tGateway: Authorized Use Only! Violators will be prosecuted; \n\tThu, 31 Aug 2017 11:19:05 +0100","from d06av25.portsmouth.uk.ibm.com (d06av25.portsmouth.uk.ibm.com\n\t[9.149.105.61])\n\tby b06cxnps3075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with\n\tESMTP id v7VAJ4tP11141272; Thu, 31 Aug 2017 10:19:04 GMT","from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id 3A47711C04C;\n\tThu, 31 Aug 2017 11:15:34 +0100 (BST)","from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id 00F0211C04A;\n\tThu, 31 Aug 2017 11:15:34 +0100 (BST)","from oc3836556865.ibm.com (unknown [9.152.224.207])\n\tby d06av25.portsmouth.uk.ibm.com (Postfix) with ESMTP;\n\tThu, 31 Aug 2017 11:15:33 +0100 (BST)"],"To":"Cornelia Huck , Thomas Huth ","References":"<20170830163609.50260-1-pasic@linux.vnet.ibm.com>\n\t<20170830163609.50260-2-pasic@linux.vnet.ibm.com>\n\t\n\t<20170831083859.11694707.cohuck@redhat.com>\n\t\n\t<20170831104243.43cd8992.cohuck@redhat.com>","From":"Halil Pasic ","Date":"Thu, 31 Aug 2017 12:19:04 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.0","MIME-Version":"1.0","In-Reply-To":"<20170831104243.43cd8992.cohuck@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"8bit","X-TM-AS-GCONF":"00","x-cbid":"17083110-0016-0000-0000-000004E7AEC5","X-IBM-AV-DETECTION":"SAVI=unused REMOTE=unused XFE=unused","x-cbparentid":"17083110-0017-0000-0000-000028212D36","Message-Id":"","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-08-31_03:, , signatures=0","X-Proofpoint-Spam-Details":"rule=outbound_notspam policy=outbound score=0\n\tspamscore=0 suspectscore=0\n\tmalwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam\n\tadjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000\n\tdefinitions=main-1708310157","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 3.x [generic] [fuzzy]","X-Received-From":"148.163.158.5","Subject":"Re: [Qemu-devel] [PATCH 1/9] s390x/css: fix cc handling for XSCH","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Dong Jia Shi ,\n\tPierre Morel , qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]