[{"id":1763183,"web_url":"http://patchwork.ozlabs.org/comment/1763183/","msgid":"<2ad3ad60-1f03-4585-45e1-d12ab593008f@redhat.com>","list_archive_url":null,"date":"2017-09-05T10:26:05","subject":"Re: [Qemu-devel] [PATCH v6 16/29] libqos: Use explicit QTestState\n\tfor virtio operations","submitter":{"id":66152,"url":"http://patchwork.ozlabs.org/api/people/66152/","name":"Thomas Huth","email":"thuth@redhat.com"},"content":"On 01.09.2017 20:03, Eric Blake wrote:\n> Drop one more client of global_qtest by teaching all virtio test\n> functionality to pass in an explicit QTestState in constructors,\n> where it is then reused for later access. Adjust all callers.\n> This gets us one step closer to eliminating implicit use of\n> global_qtest.\n> \n> Signed-off-by: Eric Blake \n> ---\n> tests/libqos/virtio-mmio.h | 3 +-\n> tests/libqos/virtio.h | 5 ++-\n> tests/libqos/virtio-mmio.c | 54 +++++++++++++++++---------------\n> tests/libqos/virtio-pci.c | 3 ++\n> tests/libqos/virtio.c | 77 ++++++++++++++++++++++++++--------------------\n> tests/virtio-blk-test.c | 3 +-\n> 6 files changed, 84 insertions(+), 61 deletions(-)\n> \n> diff --git a/tests/libqos/virtio-mmio.h b/tests/libqos/virtio-mmio.h\n> index e3e52b9ce1..bd01386054 100644\n> --- a/tests/libqos/virtio-mmio.h\n> +++ b/tests/libqos/virtio-mmio.h\n> @@ -41,6 +41,7 @@ typedef struct QVirtioMMIODevice {\n> \n> extern const QVirtioBus qvirtio_mmio;\n> \n> -QVirtioMMIODevice *qvirtio_mmio_init_device(uint64_t addr, uint32_t page_size);\n> +QVirtioMMIODevice *qvirtio_mmio_init_device(QTestState *qts, uint64_t addr,\n> + uint32_t page_size);\n> \n> #endif\n> diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h\n> index 8fbcd1869c..d180d54fc4 100644\n> --- a/tests/libqos/virtio.h\n> +++ b/tests/libqos/virtio.h\n> @@ -19,6 +19,7 @@ typedef struct QVirtioBus QVirtioBus;\n> \n> typedef struct QVirtioDevice {\n> const QVirtioBus *bus;\n> + QTestState *qts;\n> /* Device type */\n> uint16_t device_type;\n> } QVirtioDevice;\n> @@ -35,12 +36,14 @@ typedef struct QVirtQueue {\n> uint16_t last_used_idx;\n> bool indirect;\n> bool event;\n> + QTestState *qts;\n> } QVirtQueue;\n> \n> typedef struct QVRingIndirectDesc {\n> uint64_t desc; /* This points to an array fo struct vring_desc */\n> uint16_t index;\n> uint16_t elem;\n> + QTestState *qts;\n> } QVRingIndirectDesc;\n\nAdding QTestState to QVRingIndirectDesc and QVirtQueue sounds somewhat\nugly to me. I think they should either rather have a pointer to the\nassociated QVirtioDevice, or the functions where this is needed\n(qvring_init() for example) should get a \"QTestState *\" parameter instead.\n\nJust my 0.02 €.\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-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=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 3xmjZ50RhYz9s3w\n\tfor ;\n\tTue, 5 Sep 2017 20:26:53 +1000 (AEST)","from localhost ([::1]:57973 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dpB4F-0003tQ-7G\n\tfor incoming@patchwork.ozlabs.org; Tue, 05 Sep 2017 06:26:51 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:50423)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dpB3q-0003sx-G2\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 06:26:31 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dpB3l-0005K6-N0\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 06:26:26 -0400","from mx1.redhat.com ([209.132.183.28]:37054)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dpB3b-00055x-KX; Tue, 05 Sep 2017 06:26:11 -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 A74E4C057FAF;\n\tTue, 5 Sep 2017 10:26:10 +0000 (UTC)","from [10.36.116.114] (ovpn-116-114.ams2.redhat.com [10.36.116.114])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id E8D027DEE0;\n\tTue, 5 Sep 2017 10:26:07 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com A74E4C057FAF","To":"Eric Blake , qemu-devel@nongnu.org","References":"<20170901180340.30009-1-eblake@redhat.com>\n\t<20170901180340.30009-17-eblake@redhat.com>","From":"Thomas Huth ","Message-ID":"<2ad3ad60-1f03-4585-45e1-d12ab593008f@redhat.com>","Date":"Tue, 5 Sep 2017 12:26:05 +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":"<20170901180340.30009-17-eblake@redhat.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.32]);\n\tTue, 05 Sep 2017 10:26:10 +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 v6 16/29] libqos: Use explicit QTestState\n\tfor virtio operations","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":"pbonzini@redhat.com, Stefan Hajnoczi ,\n\tarmbru@redhat.com, \"open list:virtio-blk\" ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1763307,"web_url":"http://patchwork.ozlabs.org/comment/1763307/","msgid":"","list_archive_url":null,"date":"2017-09-05T12:43:41","subject":"Re: [Qemu-devel] [Qemu-block] [PATCH v6 16/29] libqos: Use explicit\n\tQTestState for virtio operations","submitter":{"id":2701,"url":"http://patchwork.ozlabs.org/api/people/2701/","name":"Paolo Bonzini","email":"pbonzini@redhat.com"},"content":"> typedef struct QVirtioDevice {\n> const QVirtioBus *bus;\n> + QTestState *qts;\n> /* Device type */\n> uint16_t device_type;\n> } QVirtioDevice;\n> @@ -35,12 +36,14 @@ typedef struct QVirtQueue {\n> uint16_t last_used_idx;\n> bool indirect;\n> bool event;\n> + QTestState *qts;\n> } QVirtQueue;\n>\n> typedef struct QVRingIndirectDesc {\n> uint64_t desc; /* This points to an array fo struct vring_desc */\n> uint16_t index;\n> uint16_t elem;\n> + QTestState *qts;\n> } QVRingIndirectDesc;\n\nAdding QTestState to QVRingIndirectDesc and QVirtQueue sounds somewhat\nugly to me. I think they should either rather have a pointer to the\nassociated QVirtioDevice, or the functions where this is needed\n(qvring_init() for example) should get a \"QTestState *\" parameter instead.\n\n\nI agree with Thomas and even extend it to QVirtioDevice. If there is a\nhas-a hierarchy between libqos objects, only the topmost one (such as the\nvirtio mmio bus device, and the PCI host bridge---or possibly even the\nmachine object exclusively) should have a QTestState. Everyone else can\naccess it implicitly through operations on its bus.\n\nTest code on the other hand can use global_qtest implicitly when they\nprepare/read buffers that the devices do DMA from/to.\n\nPaolo\n\n\n\nJust my 0.02 €.\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=)","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 3xmmcy6wGZz9sRm\n\tfor ;\n\tTue, 5 Sep 2017 22:44:34 +1000 (AEST)","from localhost ([::1]:58730 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dpDDV-00082f-2e\n\tfor incoming@patchwork.ozlabs.org; Tue, 05 Sep 2017 08:44:33 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:60383)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dpDCn-0007u0-3Q\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 08:43:57 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dpDCi-00035G-90\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 08:43:49 -0400","from mail-oi0-f41.google.com ([209.85.218.41]:36654)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)\n\t(Exim 4.71) (envelope-from ) id 1dpDCi-00034T-3D\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 08:43:44 -0400","by mail-oi0-f41.google.com with SMTP id x190so8032784oix.3\n\tfor ; Tue, 05 Sep 2017 05:43:43 -0700 (PDT)","by 10.157.56.185 with HTTP; Tue, 5 Sep 2017 05:43:41 -0700 (PDT)","by 10.157.56.185 with HTTP; Tue, 5 Sep 2017 05:43:41 -0700 (PDT)"],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=Oh0ivolMN7SjZg5OcHQ3DwaBJsPdUdwSMLYaSwLT6GA=;\n\tb=SXdce9o8K8J5A5RwkvudsTuFlYF88z5V6dbgWOh/lgolk3IsQWtDCf8cWFMNlEsf6c\n\t/Ds/SWAsOphDLuJKCi8+hdX/nxGQF4UBRtBhCoBRREYQOIjig1dNXomP+RUukROFpeeJ\n\t6wH/9LmqnI52+PFWQazbCgig69FTW7YiL24jz8tLbk7AQyEGjM7TdTNv3JsLoEnbO/X5\n\tbhqPbPy/y0FDZU15R0cp7XjAraD3VyMigxjNBm5L91aeGDi7qdBi1HPb+AVmmmfH+xM2\n\tdAsKVgLDKmyMfj2mgPzrMH6uqFRsRHmt3NJ02Tv1PSLUs0WQ/A+IYe32N5S9W72HlGbg\n\t4PSQ==","X-Gm-Message-State":"AHPjjUgRxmTbJYiCipBorOKh8EVTNiA6PXlheS4AtYRKVfIjl6yFqLAM\n\ttL1gxkgA3rPnoAv/Su3niL5BWYN7/jaa","X-Google-Smtp-Source":"ADKCNb7SxfuErFHkfK56pIgfDg2p/9qskxUq7q+lPkoG6tKTFf3gvzmm7S8PTrwyrwWldyu7QvlnDH6527hHT5zxo9E=","X-Received":"by 10.202.81.194 with SMTP id f185mr1017206oib.31.1504615422440; \n\tTue, 05 Sep 2017 05:43:42 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<2ad3ad60-1f03-4585-45e1-d12ab593008f@redhat.com>","References":"<20170901180340.30009-1-eblake@redhat.com>\n\t<20170901180340.30009-17-eblake@redhat.com>\n\t<2ad3ad60-1f03-4585-45e1-d12ab593008f@redhat.com>","From":"Paolo Bonzini ","Date":"Tue, 5 Sep 2017 14:43:41 +0200","Message-ID":"","To":"Thomas Huth ","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.85.218.41","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","X-Content-Filtered-By":"Mailman/MimeDel 2.1.21","Subject":"Re: [Qemu-devel] [Qemu-block] [PATCH v6 16/29] libqos: Use explicit\n\tQTestState for virtio operations","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":"Stefan Hajnoczi , qemu-devel ,\n\t\"open list:Block layer core\" , armbru@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1764397,"web_url":"http://patchwork.ozlabs.org/comment/1764397/","msgid":"<44ecdf8c-dc4f-7318-414e-24c7c4cbf6aa@redhat.com>","list_archive_url":null,"date":"2017-09-06T21:42:58","subject":"Re: [Qemu-devel] [Qemu-block] [PATCH v6 16/29] libqos: Use explicit\n\tQTestState for virtio operations","submitter":{"id":6591,"url":"http://patchwork.ozlabs.org/api/people/6591/","name":"Eric Blake","email":"eblake@redhat.com"},"content":"On 09/05/2017 07:43 AM, Paolo Bonzini wrote:\n>> typedef struct QVirtioDevice {\n>> const QVirtioBus *bus;\n>> + QTestState *qts;\n>> /* Device type */\n>> uint16_t device_type;\n>> } QVirtioDevice;\n>> @@ -35,12 +36,14 @@ typedef struct QVirtQueue {\n>> uint16_t last_used_idx;\n>> bool indirect;\n>> bool event;\n>> + QTestState *qts;\n>> } QVirtQueue;\n>>\n>> typedef struct QVRingIndirectDesc {\n>> uint64_t desc; /* This points to an array fo struct vring_desc */\n>> uint16_t index;\n>> uint16_t elem;\n>> + QTestState *qts;\n>> } QVRingIndirectDesc;\n> \n> Adding QTestState to QVRingIndirectDesc and QVirtQueue sounds somewhat\n> ugly to me. I think they should either rather have a pointer to the\n> associated QVirtioDevice, or the functions where this is needed\n> (qvring_init() for example) should get a \"QTestState *\" parameter instead.\n> \n> \n> I agree with Thomas and even extend it to QVirtioDevice. If there is a\n> has-a hierarchy between libqos objects, only the topmost one (such as the\n> virtio mmio bus device, and the PCI host bridge---or possibly even the\n> machine object exclusively) should have a QTestState. Everyone else can\n> access it implicitly through operations on its bus.\n\n[Paolo, gmail is really messing up with your quoting style ;( ]\n\nOkay, so if I understand correctly, at most a QVRingIndirectDesc can\nhave a pointer back to the QVirtioDevice that owns it, and in turn the\nQVirtioDevice has a pointer back to the QPCIBus that the device is on,\nand anywhere that needs a QTestState follows the chain back to the\nQPCIBus to get it. I'll play with the idea, as it may still allow for\nmore compact code than passing an explicit QTestState parameter through\nevery call.\n\n> \n> Test code on the other hand can use global_qtest implicitly when they\n> prepare/read buffers that the devices do DMA from/to.\n\nNot when I'm done - my next series gets rid of implicit global_qtest\neverywhere.","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-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=eblake@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 3xncXd4G32z9t2R\n\tfor ;\n\tThu, 7 Sep 2017 07:43:44 +1000 (AEST)","from localhost ([::1]:37889 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dpi6n-000357-UP\n\tfor incoming@patchwork.ozlabs.org; Wed, 06 Sep 2017 17:43:41 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:41198)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dpi6Q-00034Q-2W\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 17:43:22 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dpi6L-0003GX-8A\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 17:43:18 -0400","from mx1.redhat.com ([209.132.183.28]:49262)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dpi6E-0003Cw-N7; Wed, 06 Sep 2017 17:43:06 -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 76F32C0587C6;\n\tWed, 6 Sep 2017 21:43:05 +0000 (UTC)","from [10.10.120.228] (ovpn-120-228.rdu2.redhat.com [10.10.120.228])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id B1C0D5FCA4;\n\tWed, 6 Sep 2017 21:43:00 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 76F32C0587C6","To":"Paolo Bonzini , Thomas Huth ","References":"<20170901180340.30009-1-eblake@redhat.com>\n\t<20170901180340.30009-17-eblake@redhat.com>\n\t<2ad3ad60-1f03-4585-45e1-d12ab593008f@redhat.com>\n\t","From":"Eric Blake ","Openpgp":"url=http://people.redhat.com/eblake/eblake.gpg","Organization":"Red Hat, Inc.","Message-ID":"<44ecdf8c-dc4f-7318-414e-24c7c4cbf6aa@redhat.com>","Date":"Wed, 6 Sep 2017 16:42:58 -0500","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":"","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"neN1beGBIt9biPBTMnRM88gx1KJf8comH\"","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.32]);\n\tWed, 06 Sep 2017 21:43:05 +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","X-Content-Filtered-By":"Mailman/MimeDel 2.1.21","Subject":"Re: [Qemu-devel] [Qemu-block] [PATCH v6 16/29] libqos: Use explicit\n\tQTestState for virtio operations","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":"Stefan Hajnoczi , qemu-devel ,\n\t\"open list:Block layer core\" , armbru@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]