[{"id":1764060,"web_url":"http://patchwork.ozlabs.org/comment/1764060/","msgid":"<20170906141846.0be413fb.cohuck@redhat.com>","list_archive_url":null,"date":"2017-09-06T12:18:46","subject":"Re: [Qemu-devel] [PATCH 1/5] s390x/css: introduce css data stream","submitter":{"id":71914,"url":"http://patchwork.ozlabs.org/api/people/71914/","name":"Cornelia Huck","email":"cohuck@redhat.com"},"content":"On Tue,  5 Sep 2017 13:16:41 +0200\nHalil Pasic <pasic@linux.vnet.ibm.com> wrote:\n\n> This is a preparation for introducing handling for indirect data\n> addressing and modified indirect data addressing (CCW). Here we introduce\n> an interface which should make the addressing scheme transparent for the\n> client code. Here we implement only basic scheme (no IDA or MIDA).\n\ns/basic scheme/the basic scheme/\n\n> \n> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>\n> ---\n>  hw/s390x/css.c         | 55 +++++++++++++++++++++++++++++++++++++++++\n>  include/hw/s390x/css.h | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++\n>  2 files changed, 122 insertions(+)\n> \n> diff --git a/hw/s390x/css.c b/hw/s390x/css.c\n> index 1880b1a0ff..87d913f81c 100644\n> --- a/hw/s390x/css.c\n> +++ b/hw/s390x/css.c\n> @@ -783,6 +783,61 @@ static CCW1 copy_ccw_from_guest(hwaddr addr, bool fmt1)\n>      }\n>      return ret;\n>  }\n> +/**\n> + * If out of bounds marks the stream broken. If broken returns -EINVAL,\n> + * otherwise the requested  length (may be zero)\n\ns/requested  length/requested length/\n\n> + */\n> +static inline int cds_check_len(CcwDataStream *cds, int len)\n> +{\n> +    if (cds->at_byte + len > cds->count) {\n> +        cds->flags |= CDS_F_STREAM_BROKEN;\n> +    }\n> +    return cds->flags & CDS_F_STREAM_BROKEN ? -EINVAL : len;\n> +}\n> +\n> +static int ccw_dstream_rw_noflags(CcwDataStream *cds, void *buff, int len,\n> +                                  CcwDataStreamOp op)\n> +{\n> +    int ret;\n> +\n> +    ret = cds_check_len(cds, len);\n> +    if (ret <= 0) {\n> +        return ret;\n> +    }\n> +    if (op == CDS_OP_A) {\n> +        goto incr;\n> +    }\n> +    ret = address_space_rw(&address_space_memory, cds->cda,\n> +                           MEMTXATTRS_UNSPECIFIED, buff, len, op);\n> +    if (ret != MEMTX_OK) {\n> +        cds->flags |= CDS_F_STREAM_BROKEN;\n\nDo we want to distinguish between different reasons for the stream\nbeing broken? I.e, is there a case where we want to signal different\nerrors back to the caller?\n\n> +        return -EINVAL;\n> +    }\n> +incr:\n> +    cds->at_byte += len;\n> +    cds->cda += len;\n> +    return 0;\n> +}\n> +\n> +void ccw_dstream_init(CcwDataStream *cds, CCW1 const *ccw, ORB const *orb)\n> +{\n> +    /*\n> +     * We don't support MIDA (an optional facility) yet and we\n> +     * catch this earlier. Just for expressing the precondition.\n> +     */\n> +    assert(!(orb->ctrl1 & ORB_CTRL1_MASK_MIDAW));\n\nI don't know, this is infrastructure, should it trust its callers? If\nyou keep the assert, please make it g_assert().\n\n> +    cds->flags = (orb->ctrl0 & ORB_CTRL0_MASK_I2K ? CDS_F_I2K : 0) |\n> +                 (orb->ctrl0 & ORB_CTRL0_MASK_C64 ? CDS_F_C64 : 0) |\n> +                 (ccw->flags & CCW_FLAG_IDA ? CDS_F_IDA : 0);\n> +    cds->count = ccw->count;\n> +    cds->cda_orig = ccw->cda;\n> +    ccw_dstream_rewind(cds);\n> +    if (!(cds->flags & CDS_F_IDA)) {\n> +        cds->op_handler = ccw_dstream_rw_noflags;\n> +    } else {\n> +        assert(false);\n\nSame here.\n\nOr should we make this return an error and have the callers deal with\nthat? (I still need to look at the users.)\n\n> +    }\n> +}\n>  \n>  static int css_interpret_ccw(SubchDev *sch, hwaddr ccw_addr,\n>                               bool suspend_allowed)","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx02.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 3xnN2v2Yy8z9sBZ\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed,  6 Sep 2017 22:20:39 +1000 (AEST)","from localhost ([::1]:35949 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dpZJt-0000RX-Fj\n\tfor incoming@patchwork.ozlabs.org; Wed, 06 Sep 2017 08:20:37 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:41564)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <cohuck@redhat.com>) id 1dpZIE-00081p-0u\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 08:18:58 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <cohuck@redhat.com>) id 1dpZIA-00058V-LB\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 08:18:53 -0400","from mx1.redhat.com ([209.132.183.28]:36508)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <cohuck@redhat.com>) id 1dpZIA-00057u-Cv\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 08:18:50 -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 3F2D5883AD;\n\tWed,  6 Sep 2017 12:18:49 +0000 (UTC)","from gondolin (dhcp-192-215.str.redhat.com [10.33.192.215])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 529C71880B;\n\tWed,  6 Sep 2017 12:18:48 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 3F2D5883AD","Date":"Wed, 6 Sep 2017 14:18:46 +0200","From":"Cornelia Huck <cohuck@redhat.com>","To":"Halil Pasic <pasic@linux.vnet.ibm.com>","Message-ID":"<20170906141846.0be413fb.cohuck@redhat.com>","In-Reply-To":"<20170905111645.18068-2-pasic@linux.vnet.ibm.com>","References":"<20170905111645.18068-1-pasic@linux.vnet.ibm.com>\n\t<20170905111645.18068-2-pasic@linux.vnet.ibm.com>","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.26]);\n\tWed, 06 Sep 2017 12:18:49 +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/5] s390x/css: introduce css data stream","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>,\n\tPierre Morel <pmorel@linux.vnet.ibm.com>, qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1764071,"web_url":"http://patchwork.ozlabs.org/comment/1764071/","msgid":"<bd30a18f-6d22-e5a7-6e8c-08f628466cc4@linux.vnet.ibm.com>","list_archive_url":null,"date":"2017-09-06T12:40:48","subject":"Re: [Qemu-devel] [PATCH 1/5] s390x/css: introduce css data stream","submitter":{"id":68297,"url":"http://patchwork.ozlabs.org/api/people/68297/","name":"Halil Pasic","email":"pasic@linux.vnet.ibm.com"},"content":"On 09/06/2017 02:18 PM, Cornelia Huck wrote:\n> On Tue,  5 Sep 2017 13:16:41 +0200\n> Halil Pasic <pasic@linux.vnet.ibm.com> wrote:\n> \n>> This is a preparation for introducing handling for indirect data\n>> addressing and modified indirect data addressing (CCW). Here we introduce\n>> an interface which should make the addressing scheme transparent for the\n>> client code. Here we implement only basic scheme (no IDA or MIDA).\n> \n> s/basic scheme/the basic scheme/\n> \n\nNod.\n\n>>\n>> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>\n>> ---\n>>  hw/s390x/css.c         | 55 +++++++++++++++++++++++++++++++++++++++++\n>>  include/hw/s390x/css.h | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++\n>>  2 files changed, 122 insertions(+)\n>>\n>> diff --git a/hw/s390x/css.c b/hw/s390x/css.c\n>> index 1880b1a0ff..87d913f81c 100644\n>> --- a/hw/s390x/css.c\n>> +++ b/hw/s390x/css.c\n>> @@ -783,6 +783,61 @@ static CCW1 copy_ccw_from_guest(hwaddr addr, bool fmt1)\n>>      }\n>>      return ret;\n>>  }\n>> +/**\n>> + * If out of bounds marks the stream broken. If broken returns -EINVAL,\n>> + * otherwise the requested  length (may be zero)\n> \n> s/requested  length/requested length/\n> \n\nNod.\n\n>> + */\n>> +static inline int cds_check_len(CcwDataStream *cds, int len)\n>> +{\n>> +    if (cds->at_byte + len > cds->count) {\n>> +        cds->flags |= CDS_F_STREAM_BROKEN;\n>> +    }\n>> +    return cds->flags & CDS_F_STREAM_BROKEN ? -EINVAL : len;\n>> +}\n>> +\n>> +static int ccw_dstream_rw_noflags(CcwDataStream *cds, void *buff, int len,\n>> +                                  CcwDataStreamOp op)\n>> +{\n>> +    int ret;\n>> +\n>> +    ret = cds_check_len(cds, len);\n>> +    if (ret <= 0) {\n>> +        return ret;\n>> +    }\n>> +    if (op == CDS_OP_A) {\n>> +        goto incr;\n>> +    }\n>> +    ret = address_space_rw(&address_space_memory, cds->cda,\n>> +                           MEMTXATTRS_UNSPECIFIED, buff, len, op);\n>> +    if (ret != MEMTX_OK) {\n>> +        cds->flags |= CDS_F_STREAM_BROKEN;\n> \n> Do we want to distinguish between different reasons for the stream\n> being broken? I.e, is there a case where we want to signal different\n> errors back to the caller?\n> \n\nHm, after I've done the error handling it seems that basically\neverything is to be handled with a program check. The stream\nrecords the place where the problem was encountered, so for debug\nwe would not have to search for long.\n\nThere seems to be no need to distinguish.\n\n>> +        return -EINVAL;\n>> +    }\n>> +incr:\n>> +    cds->at_byte += len;\n>> +    cds->cda += len;\n>> +    return 0;\n>> +}\n>> +\n>> +void ccw_dstream_init(CcwDataStream *cds, CCW1 const *ccw, ORB const *orb)\n>> +{\n>> +    /*\n>> +     * We don't support MIDA (an optional facility) yet and we\n>> +     * catch this earlier. Just for expressing the precondition.\n>> +     */\n>> +    assert(!(orb->ctrl1 & ORB_CTRL1_MASK_MIDAW));\n> \n> I don't know, this is infrastructure, should it trust its callers? If\n> you keep the assert, please make it g_assert().\n\n\nWhy g_assert? I think g_assert comes form a test framework, this is not\ntest code.\n\nI feel more comfortable having this assert around.\n\n> \n>> +    cds->flags = (orb->ctrl0 & ORB_CTRL0_MASK_I2K ? CDS_F_I2K : 0) |\n>> +                 (orb->ctrl0 & ORB_CTRL0_MASK_C64 ? CDS_F_C64 : 0) |\n>> +                 (ccw->flags & CCW_FLAG_IDA ? CDS_F_IDA : 0);\n>> +    cds->count = ccw->count;\n>> +    cds->cda_orig = ccw->cda;\n>> +    ccw_dstream_rewind(cds);\n>> +    if (!(cds->flags & CDS_F_IDA)) {\n>> +        cds->op_handler = ccw_dstream_rw_noflags;\n>> +    } else {\n>> +        assert(false);\n> \n> Same here.\n> \n> Or should we make this return an error and have the callers deal with\n> that? (I still need to look at the users.)\n> \n\nThis assert is going away soon (patch 4). I'm not sure doing much more here\nis justified.\n\n>> +    }\n>> +}\n>>  \n>>  static int css_interpret_ccw(SubchDev *sch, hwaddr ccw_addr,\n>>                               bool suspend_allowed)\n>","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","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 3xnNWB3qrYz9t2R\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed,  6 Sep 2017 22:41:40 +1000 (AEST)","from localhost ([::1]:36021 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dpZeD-0006rU-BN\n\tfor incoming@patchwork.ozlabs.org; Wed, 06 Sep 2017 08:41:37 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:48740)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <pasic@linux.vnet.ibm.com>) id 1dpZdi-0006nQ-Kr\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 08:41:12 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <pasic@linux.vnet.ibm.com>) id 1dpZdZ-0007PP-2y\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 08:41:06 -0400","from mx0a-001b2d01.pphosted.com ([148.163.156.1]:38847)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <pasic@linux.vnet.ibm.com>)\n\tid 1dpZdY-0007Ov-QO\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 08:40:57 -0400","from pps.filterd (m0098410.ppops.net [127.0.0.1])\n\tby mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tv86CdHFB112573\n\tfor <qemu-devel@nongnu.org>; Wed, 6 Sep 2017 08:40:55 -0400","from e06smtp14.uk.ibm.com (e06smtp14.uk.ibm.com [195.75.94.110])\n\tby mx0a-001b2d01.pphosted.com with ESMTP id 2ctbwfrt87-1\n\t(version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT)\n\tfor <qemu-devel@nongnu.org>; Wed, 06 Sep 2017 08:40:54 -0400","from localhost\n\tby e06smtp14.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use\n\tOnly! Violators will be prosecuted\n\tfor <qemu-devel@nongnu.org> from <pasic@linux.vnet.ibm.com>;\n\tWed, 6 Sep 2017 13:40:52 +0100","from b06cxnps4075.portsmouth.uk.ibm.com (9.149.109.197)\n\tby e06smtp14.uk.ibm.com (192.168.101.144) with IBM ESMTP SMTP\n\tGateway: Authorized Use Only! Violators will be prosecuted; \n\tWed, 6 Sep 2017 13:40:49 +0100","from d06av26.portsmouth.uk.ibm.com (d06av26.portsmouth.uk.ibm.com\n\t[9.149.105.62])\n\tby b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with\n\tESMTP id v86CenQZ21561480; Wed, 6 Sep 2017 12:40:49 GMT","from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id 6B68EAE053;\n\tWed,  6 Sep 2017 13:36:04 +0100 (BST)","from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id 29528AE045;\n\tWed,  6 Sep 2017 13:36:04 +0100 (BST)","from oc3836556865.ibm.com (unknown [9.152.224.207])\n\tby d06av26.portsmouth.uk.ibm.com (Postfix) with ESMTP;\n\tWed,  6 Sep 2017 13:36:04 +0100 (BST)"],"To":"Cornelia Huck <cohuck@redhat.com>","References":"<20170905111645.18068-1-pasic@linux.vnet.ibm.com>\n\t<20170905111645.18068-2-pasic@linux.vnet.ibm.com>\n\t<20170906141846.0be413fb.cohuck@redhat.com>","From":"Halil Pasic <pasic@linux.vnet.ibm.com>","Date":"Wed, 6 Sep 2017 14:40:48 +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":"<20170906141846.0be413fb.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":"17090612-0016-0000-0000-000004E91423","X-IBM-AV-DETECTION":"SAVI=unused REMOTE=unused XFE=unused","x-cbparentid":"17090612-0017-0000-0000-00002822A754","Message-Id":"<bd30a18f-6d22-e5a7-6e8c-08f628466cc4@linux.vnet.ibm.com>","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-09-06_05:, , 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-1709060175","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 3.x [generic] [fuzzy]","X-Received-From":"148.163.156.1","Subject":"Re: [Qemu-devel] [PATCH 1/5] s390x/css: introduce css data stream","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>,\n\tPierre Morel <pmorel@linux.vnet.ibm.com>, qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1764083,"web_url":"http://patchwork.ozlabs.org/comment/1764083/","msgid":"<20170906145158.252cf0cd.cohuck@redhat.com>","list_archive_url":null,"date":"2017-09-06T12:51:58","subject":"Re: [Qemu-devel] [PATCH 1/5] s390x/css: introduce css data stream","submitter":{"id":71914,"url":"http://patchwork.ozlabs.org/api/people/71914/","name":"Cornelia Huck","email":"cohuck@redhat.com"},"content":"On Wed, 6 Sep 2017 14:40:48 +0200\nHalil Pasic <pasic@linux.vnet.ibm.com> wrote:\n\n> On 09/06/2017 02:18 PM, Cornelia Huck wrote:\n> > On Tue,  5 Sep 2017 13:16:41 +0200\n> > Halil Pasic <pasic@linux.vnet.ibm.com> wrote:\n\n> >> +static int ccw_dstream_rw_noflags(CcwDataStream *cds, void *buff, int len,\n> >> +                                  CcwDataStreamOp op)\n> >> +{\n> >> +    int ret;\n> >> +\n> >> +    ret = cds_check_len(cds, len);\n> >> +    if (ret <= 0) {\n> >> +        return ret;\n> >> +    }\n> >> +    if (op == CDS_OP_A) {\n> >> +        goto incr;\n> >> +    }\n> >> +    ret = address_space_rw(&address_space_memory, cds->cda,\n> >> +                           MEMTXATTRS_UNSPECIFIED, buff, len, op);\n> >> +    if (ret != MEMTX_OK) {\n> >> +        cds->flags |= CDS_F_STREAM_BROKEN;  \n> > \n> > Do we want to distinguish between different reasons for the stream\n> > being broken? I.e, is there a case where we want to signal different\n> > errors back to the caller?\n> >   \n> \n> Hm, after I've done the error handling it seems that basically\n> everything is to be handled with a program check. The stream\n> records the place where the problem was encountered, so for debug\n> we would not have to search for long.\n> \n> There seems to be no need to distinguish.\n\nOK, makes sense. Let's keep it as it is.\n\n> \n> >> +        return -EINVAL;\n> >> +    }\n> >> +incr:\n> >> +    cds->at_byte += len;\n> >> +    cds->cda += len;\n> >> +    return 0;\n> >> +}\n> >> +\n> >> +void ccw_dstream_init(CcwDataStream *cds, CCW1 const *ccw, ORB const *orb)\n> >> +{\n> >> +    /*\n> >> +     * We don't support MIDA (an optional facility) yet and we\n> >> +     * catch this earlier. Just for expressing the precondition.\n> >> +     */\n> >> +    assert(!(orb->ctrl1 & ORB_CTRL1_MASK_MIDAW));  \n> > \n> > I don't know, this is infrastructure, should it trust its callers? If\n> > you keep the assert, please make it g_assert().  \n> \n> \n> Why g_assert? I think g_assert comes form a test framework, this is not\n> test code.\n\ng_assert() is glib, no?\n\n> \n> I feel more comfortable having this assert around.\n\nLet's revisit that when we add mida support :) I don't really object to\nit.\n\n> \n> >   \n> >> +    cds->flags = (orb->ctrl0 & ORB_CTRL0_MASK_I2K ? CDS_F_I2K : 0) |\n> >> +                 (orb->ctrl0 & ORB_CTRL0_MASK_C64 ? CDS_F_C64 : 0) |\n> >> +                 (ccw->flags & CCW_FLAG_IDA ? CDS_F_IDA : 0);\n> >> +    cds->count = ccw->count;\n> >> +    cds->cda_orig = ccw->cda;\n> >> +    ccw_dstream_rewind(cds);\n> >> +    if (!(cds->flags & CDS_F_IDA)) {\n> >> +        cds->op_handler = ccw_dstream_rw_noflags;\n> >> +    } else {\n> >> +        assert(false);  \n> > \n> > Same here.\n> > \n> > Or should we make this return an error and have the callers deal with\n> > that? (I still need to look at the users.)\n> >   \n> \n> This assert is going away soon (patch 4). I'm not sure doing much more here\n> is justified.\n\nOK, if it is transient anyway...","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","ext-mx08.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx08.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 3xnNnF34kqz9sBd\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed,  6 Sep 2017 22:53:53 +1000 (AEST)","from localhost ([::1]:36057 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dpZq3-00059t-K7\n\tfor incoming@patchwork.ozlabs.org; Wed, 06 Sep 2017 08:53:51 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:56450)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <cohuck@redhat.com>) id 1dpZoM-0004PW-ES\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 08:52:10 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <cohuck@redhat.com>) id 1dpZoI-0005bQ-I5\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 08:52:06 -0400","from mx1.redhat.com ([209.132.183.28]:37954)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <cohuck@redhat.com>) id 1dpZoI-0005ay-A7\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 08:52:02 -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 4FE90C0587DD;\n\tWed,  6 Sep 2017 12:52:01 +0000 (UTC)","from gondolin (dhcp-192-215.str.redhat.com [10.33.192.215])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 697AC5170D;\n\tWed,  6 Sep 2017 12:52:00 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 4FE90C0587DD","Date":"Wed, 6 Sep 2017 14:51:58 +0200","From":"Cornelia Huck <cohuck@redhat.com>","To":"Halil Pasic <pasic@linux.vnet.ibm.com>","Message-ID":"<20170906145158.252cf0cd.cohuck@redhat.com>","In-Reply-To":"<bd30a18f-6d22-e5a7-6e8c-08f628466cc4@linux.vnet.ibm.com>","References":"<20170905111645.18068-1-pasic@linux.vnet.ibm.com>\n\t<20170905111645.18068-2-pasic@linux.vnet.ibm.com>\n\t<20170906141846.0be413fb.cohuck@redhat.com>\n\t<bd30a18f-6d22-e5a7-6e8c-08f628466cc4@linux.vnet.ibm.com>","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.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.32]);\n\tWed, 06 Sep 2017 12:52:01 +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/5] s390x/css: introduce css data stream","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>,\n\tPierre Morel <pmorel@linux.vnet.ibm.com>, qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1766384,"web_url":"http://patchwork.ozlabs.org/comment/1766384/","msgid":"<27c027c4-58b4-9031-648e-9fd2fa1e5fa8@linux.vnet.ibm.com>","list_archive_url":null,"date":"2017-09-11T16:36:00","subject":"Re: [Qemu-devel] [PATCH 1/5] s390x/css: introduce css data stream","submitter":{"id":68297,"url":"http://patchwork.ozlabs.org/api/people/68297/","name":"Halil Pasic","email":"pasic@linux.vnet.ibm.com"},"content":"On 09/06/2017 02:51 PM, Cornelia Huck wrote:\n>>>> +void ccw_dstream_init(CcwDataStream *cds, CCW1 const *ccw, ORB const *orb)\n>>>> +{\n>>>> +    /*\n>>>> +     * We don't support MIDA (an optional facility) yet and we\n>>>> +     * catch this earlier. Just for expressing the precondition.\n>>>> +     */\n>>>> +    assert(!(orb->ctrl1 & ORB_CTRL1_MASK_MIDAW));  \n>>> I don't know, this is infrastructure, should it trust its callers? If\n>>> you keep the assert, please make it g_assert().  \n>>\n>> Why g_assert? I think g_assert comes form a test framework, this is not\n>> test code.\n> g_assert() is glib, no?\n> \n\nIt lives in GLib > GLib Utilities > Testing:\nhttps://developer.gnome.org/glib/stable/glib-Testing.html\n\nThe description of \"Testing\" starts like this: \"GLib provides a framework\nfor writing and maintaining unit tests in parallel to the code they are\ntesting. The API is designed according to established concepts found in\nthe other test frameworks (JUnit, NUnit, RUnit), which in turn is based\non smalltalk unit testing concepts.\"\n\nSo yes, it's both glib and testing framework. This is why I\nask why should one use g_assert in not-unit-test code.\n\nHalil","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","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 3xrYV25wyPz9s81\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 12 Sep 2017 02:36:41 +1000 (AEST)","from localhost ([::1]:58909 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1drRhN-0000ZC-EV\n\tfor incoming@patchwork.ozlabs.org; Mon, 11 Sep 2017 12:36:37 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:47280)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <pasic@linux.vnet.ibm.com>) id 1drRgx-0000Yz-SZ\n\tfor qemu-devel@nongnu.org; Mon, 11 Sep 2017 12:36:12 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <pasic@linux.vnet.ibm.com>) id 1drRgt-00038r-Rc\n\tfor qemu-devel@nongnu.org; Mon, 11 Sep 2017 12:36:11 -0400","from mx0b-001b2d01.pphosted.com ([148.163.158.5]:41656\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 <pasic@linux.vnet.ibm.com>)\n\tid 1drRgt-00038f-Me\n\tfor qemu-devel@nongnu.org; Mon, 11 Sep 2017 12:36:07 -0400","from pps.filterd (m0098417.ppops.net [127.0.0.1])\n\tby mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tv8BGZSXA042357\n\tfor <qemu-devel@nongnu.org>; Mon, 11 Sep 2017 12:36:06 -0400","from e06smtp14.uk.ibm.com (e06smtp14.uk.ibm.com [195.75.94.110])\n\tby mx0a-001b2d01.pphosted.com with ESMTP id 2cwsg0vrg8-1\n\t(version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT)\n\tfor <qemu-devel@nongnu.org>; Mon, 11 Sep 2017 12:36:05 -0400","from localhost\n\tby e06smtp14.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use\n\tOnly! Violators will be prosecuted\n\tfor <qemu-devel@nongnu.org> from <pasic@linux.vnet.ibm.com>;\n\tMon, 11 Sep 2017 17:36:03 +0100","from b06cxnps4075.portsmouth.uk.ibm.com (9.149.109.197)\n\tby e06smtp14.uk.ibm.com (192.168.101.144) with IBM ESMTP SMTP\n\tGateway: Authorized Use Only! Violators will be prosecuted; \n\tMon, 11 Sep 2017 17:36:01 +0100","from d06av26.portsmouth.uk.ibm.com (d06av26.portsmouth.uk.ibm.com\n\t[9.149.105.62])\n\tby b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with\n\tESMTP id v8BGa08s21168196; Mon, 11 Sep 2017 16:36:00 GMT","from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id 9789CAE051;\n\tMon, 11 Sep 2017 17:31:07 +0100 (BST)","from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id 58220AE045;\n\tMon, 11 Sep 2017 17:31:07 +0100 (BST)","from oc3836556865.ibm.com (unknown [9.152.224.207])\n\tby d06av26.portsmouth.uk.ibm.com (Postfix) with ESMTP;\n\tMon, 11 Sep 2017 17:31:07 +0100 (BST)"],"To":"Cornelia Huck <cohuck@redhat.com>","References":"<20170905111645.18068-1-pasic@linux.vnet.ibm.com>\n\t<20170905111645.18068-2-pasic@linux.vnet.ibm.com>\n\t<20170906141846.0be413fb.cohuck@redhat.com>\n\t<bd30a18f-6d22-e5a7-6e8c-08f628466cc4@linux.vnet.ibm.com>\n\t<20170906145158.252cf0cd.cohuck@redhat.com>","From":"Halil Pasic <pasic@linux.vnet.ibm.com>","Date":"Mon, 11 Sep 2017 18:36:00 +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":"<20170906145158.252cf0cd.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":"17091116-0016-0000-0000-000004EC1E96","X-IBM-AV-DETECTION":"SAVI=unused REMOTE=unused XFE=unused","x-cbparentid":"17091116-0017-0000-0000-000028262F21","Message-Id":"<27c027c4-58b4-9031-648e-9fd2fa1e5fa8@linux.vnet.ibm.com>","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-09-11_06:, , 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-1709110249","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/5] s390x/css: introduce css data stream","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>,\n\tPierre Morel <pmorel@linux.vnet.ibm.com>, qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1767740,"web_url":"http://patchwork.ozlabs.org/comment/1767740/","msgid":"<20170913115335.58995e36.cohuck@redhat.com>","list_archive_url":null,"date":"2017-09-13T09:53:35","subject":"Re: [Qemu-devel] [PATCH 1/5] s390x/css: introduce css data stream","submitter":{"id":71914,"url":"http://patchwork.ozlabs.org/api/people/71914/","name":"Cornelia Huck","email":"cohuck@redhat.com"},"content":"On Mon, 11 Sep 2017 18:36:00 +0200\nHalil Pasic <pasic@linux.vnet.ibm.com> wrote:\n\n> On 09/06/2017 02:51 PM, Cornelia Huck wrote:\n> >>>> +void ccw_dstream_init(CcwDataStream *cds, CCW1 const *ccw, ORB const *orb)\n> >>>> +{\n> >>>> +    /*\n> >>>> +     * We don't support MIDA (an optional facility) yet and we\n> >>>> +     * catch this earlier. Just for expressing the precondition.\n> >>>> +     */\n> >>>> +    assert(!(orb->ctrl1 & ORB_CTRL1_MASK_MIDAW));    \n> >>> I don't know, this is infrastructure, should it trust its callers? If\n> >>> you keep the assert, please make it g_assert().    \n> >>\n> >> Why g_assert? I think g_assert comes form a test framework, this is not\n> >> test code.  \n> > g_assert() is glib, no?\n> >   \n> \n> It lives in GLib > GLib Utilities > Testing:\n> https://developer.gnome.org/glib/stable/glib-Testing.html\n> \n> The description of \"Testing\" starts like this: \"GLib provides a framework\n> for writing and maintaining unit tests in parallel to the code they are\n> testing. The API is designed according to established concepts found in\n> the other test frameworks (JUnit, NUnit, RUnit), which in turn is based\n> on smalltalk unit testing concepts.\"\n> \n> So yes, it's both glib and testing framework. This is why I\n> ask why should one use g_assert in not-unit-test code.\n\nI have searched the archives, but unfortunately was not able to come to\na conclusion. Checkpatch advises against using anything but g_assert or\ng_assert_not_reached in anything but test code, but that is because\nthose other g_asserts can be made non-fatal. g_assert_not_reached does\nnot seem to have a non-glib equivalent.\n\nI have it somewhere in the back of my mind that g_assert should be\npreferred...","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx01.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 3xscSq73HNz9s76\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed, 13 Sep 2017 19:54:19 +1000 (AEST)","from localhost ([::1]:41273 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1ds4N8-0004ih-3B\n\tfor incoming@patchwork.ozlabs.org; Wed, 13 Sep 2017 05:54:18 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:59638)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <cohuck@redhat.com>) id 1ds4Mb-0004hc-2r\n\tfor qemu-devel@nongnu.org; Wed, 13 Sep 2017 05:53:46 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <cohuck@redhat.com>) id 1ds4MW-000284-6y\n\tfor qemu-devel@nongnu.org; Wed, 13 Sep 2017 05:53:45 -0400","from mx1.redhat.com ([209.132.183.28]:49218)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <cohuck@redhat.com>) id 1ds4MW-00027H-0Z\n\tfor qemu-devel@nongnu.org; Wed, 13 Sep 2017 05:53:40 -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 EBD2081DEB;\n\tWed, 13 Sep 2017 09:53:38 +0000 (UTC)","from gondolin (dhcp-192-215.str.redhat.com [10.33.192.215])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id D2FE55191C;\n\tWed, 13 Sep 2017 09:53:37 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com EBD2081DEB","Date":"Wed, 13 Sep 2017 11:53:35 +0200","From":"Cornelia Huck <cohuck@redhat.com>","To":"Halil Pasic <pasic@linux.vnet.ibm.com>","Message-ID":"<20170913115335.58995e36.cohuck@redhat.com>","In-Reply-To":"<27c027c4-58b4-9031-648e-9fd2fa1e5fa8@linux.vnet.ibm.com>","References":"<20170905111645.18068-1-pasic@linux.vnet.ibm.com>\n\t<20170905111645.18068-2-pasic@linux.vnet.ibm.com>\n\t<20170906141846.0be413fb.cohuck@redhat.com>\n\t<bd30a18f-6d22-e5a7-6e8c-08f628466cc4@linux.vnet.ibm.com>\n\t<20170906145158.252cf0cd.cohuck@redhat.com>\n\t<27c027c4-58b4-9031-648e-9fd2fa1e5fa8@linux.vnet.ibm.com>","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.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.25]);\n\tWed, 13 Sep 2017 09:53:39 +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/5] s390x/css: introduce css data stream","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>,\n\tPierre Morel <pmorel@linux.vnet.ibm.com>, qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1767804,"web_url":"http://patchwork.ozlabs.org/comment/1767804/","msgid":"<e356920f-f35f-2dc8-bdf2-6febce2c340d@linux.vnet.ibm.com>","list_archive_url":null,"date":"2017-09-13T11:35:36","subject":"Re: [Qemu-devel] [PATCH 1/5] s390x/css: introduce css data stream","submitter":{"id":68297,"url":"http://patchwork.ozlabs.org/api/people/68297/","name":"Halil Pasic","email":"pasic@linux.vnet.ibm.com"},"content":"On 09/13/2017 11:53 AM, Cornelia Huck wrote:\n> On Mon, 11 Sep 2017 18:36:00 +0200\n> Halil Pasic <pasic@linux.vnet.ibm.com> wrote:\n> \n>> On 09/06/2017 02:51 PM, Cornelia Huck wrote:\n>>>>>> +void ccw_dstream_init(CcwDataStream *cds, CCW1 const *ccw, ORB const *orb)\n>>>>>> +{\n>>>>>> +    /*\n>>>>>> +     * We don't support MIDA (an optional facility) yet and we\n>>>>>> +     * catch this earlier. Just for expressing the precondition.\n>>>>>> +     */\n>>>>>> +    assert(!(orb->ctrl1 & ORB_CTRL1_MASK_MIDAW));    \n>>>>> I don't know, this is infrastructure, should it trust its callers? If\n>>>>> you keep the assert, please make it g_assert().    \n>>>>\n>>>> Why g_assert? I think g_assert comes form a test framework, this is not\n>>>> test code.  \n>>> g_assert() is glib, no?\n>>>   \n>>\n>> It lives in GLib > GLib Utilities > Testing:\n>> https://developer.gnome.org/glib/stable/glib-Testing.html\n>>\n>> The description of \"Testing\" starts like this: \"GLib provides a framework\n>> for writing and maintaining unit tests in parallel to the code they are\n>> testing. The API is designed according to established concepts found in\n>> the other test frameworks (JUnit, NUnit, RUnit), which in turn is based\n>> on smalltalk unit testing concepts.\"\n>>\n>> So yes, it's both glib and testing framework. This is why I\n>> ask why should one use g_assert in not-unit-test code.\n> \n> I have searched the archives, but unfortunately was not able to come to\n> a conclusion. Checkpatch advises against using anything but g_assert or\n> g_assert_not_reached in anything but test code, but that is because\n> those other g_asserts can be made non-fatal. g_assert_not_reached does\n> not seem to have a non-glib equivalent.\n> \n\nI think the standard library equivalent of g_assert_not_reached is\nassert(false).\n\nYeah, checkpatch does not advise against using g_assert and\ng_assert_not_reached in non-test code, but it does not advise against\nusing standard lib assert.\n\n> I have it somewhere in the back of my mind that g_assert should be\n> preferred...\n> \n\nOK, I will use g_assert.","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","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=<UNKNOWN>)","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 3xsfmN2Vbfz9s9Y\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed, 13 Sep 2017 21:37:56 +1000 (AEST)","from localhost ([::1]:41708 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1ds5zO-0008He-HO\n\tfor incoming@patchwork.ozlabs.org; Wed, 13 Sep 2017 07:37:54 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:34261)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <pasic@linux.vnet.ibm.com>) id 1ds5xN-00072p-5Z\n\tfor qemu-devel@nongnu.org; Wed, 13 Sep 2017 07:35:50 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <pasic@linux.vnet.ibm.com>) id 1ds5xJ-0001AT-OT\n\tfor qemu-devel@nongnu.org; Wed, 13 Sep 2017 07:35:49 -0400","from mx0a-001b2d01.pphosted.com ([148.163.156.1]:36843)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <pasic@linux.vnet.ibm.com>)\n\tid 1ds5xJ-00019B-FE\n\tfor qemu-devel@nongnu.org; Wed, 13 Sep 2017 07:35:45 -0400","from pps.filterd (m0098409.ppops.net [127.0.0.1])\n\tby mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tv8DBZMPk120664\n\tfor <qemu-devel@nongnu.org>; Wed, 13 Sep 2017 07:35:44 -0400","from e06smtp13.uk.ibm.com (e06smtp13.uk.ibm.com [195.75.94.109])\n\tby mx0a-001b2d01.pphosted.com with ESMTP id 2cy388t92g-1\n\t(version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT)\n\tfor <qemu-devel@nongnu.org>; Wed, 13 Sep 2017 07:35:43 -0400","from localhost\n\tby e06smtp13.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use\n\tOnly! Violators will be prosecuted\n\tfor <qemu-devel@nongnu.org> from <pasic@linux.vnet.ibm.com>;\n\tWed, 13 Sep 2017 12:35:41 +0100","from b06cxnps4076.portsmouth.uk.ibm.com (9.149.109.198)\n\tby e06smtp13.uk.ibm.com (192.168.101.143) with IBM ESMTP SMTP\n\tGateway: Authorized Use Only! Violators will be prosecuted; \n\tWed, 13 Sep 2017 12:35:38 +0100","from d06av23.portsmouth.uk.ibm.com (d06av23.portsmouth.uk.ibm.com\n\t[9.149.105.59])\n\tby b06cxnps4076.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with\n\tESMTP id v8DBZbJ817432740; Wed, 13 Sep 2017 11:35:37 GMT","from d06av23.portsmouth.uk.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id AC701A4040;\n\tWed, 13 Sep 2017 12:31:47 +0100 (BST)","from d06av23.portsmouth.uk.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id 57451A4057;\n\tWed, 13 Sep 2017 12:31:47 +0100 (BST)","from oc3836556865.ibm.com (unknown [9.152.224.207])\n\tby d06av23.portsmouth.uk.ibm.com (Postfix) with ESMTP;\n\tWed, 13 Sep 2017 12:31:47 +0100 (BST)"],"To":"Cornelia Huck <cohuck@redhat.com>","References":"<20170905111645.18068-1-pasic@linux.vnet.ibm.com>\n\t<20170905111645.18068-2-pasic@linux.vnet.ibm.com>\n\t<20170906141846.0be413fb.cohuck@redhat.com>\n\t<bd30a18f-6d22-e5a7-6e8c-08f628466cc4@linux.vnet.ibm.com>\n\t<20170906145158.252cf0cd.cohuck@redhat.com>\n\t<27c027c4-58b4-9031-648e-9fd2fa1e5fa8@linux.vnet.ibm.com>\n\t<20170913115335.58995e36.cohuck@redhat.com>","From":"Halil Pasic <pasic@linux.vnet.ibm.com>","Date":"Wed, 13 Sep 2017 13:35:36 +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":"<20170913115335.58995e36.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":"17091311-0012-0000-0000-00000578AAA6","X-IBM-AV-DETECTION":"SAVI=unused REMOTE=unused XFE=unused","x-cbparentid":"17091311-0013-0000-0000-000018F1C69E","Message-Id":"<e356920f-f35f-2dc8-bdf2-6febce2c340d@linux.vnet.ibm.com>","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-09-13_04:, , 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-1709130183","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 3.x [generic] [fuzzy]","X-Received-From":"148.163.156.1","Subject":"Re: [Qemu-devel] [PATCH 1/5] s390x/css: introduce css data stream","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>,\n\tPierre Morel <pmorel@linux.vnet.ibm.com>, qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}}]