[{"id":1769934,"web_url":"http://patchwork.ozlabs.org/comment/1769934/","msgid":"<20170918075107.GK15551@lemon.lan>","list_archive_url":null,"date":"2017-09-18T07:51:07","subject":"Re: [Qemu-devel] [PATCH 0/6] block: Fix permissions after ro/rw\n\treopen","submitter":{"id":24872,"url":"http://patchwork.ozlabs.org/api/people/24872/","name":"Fam Zheng","email":"famz@redhat.com"},"content":"On Fri, 09/15 12:10, Kevin Wolf wrote:\n> bdrv_reopen() can switch nodes between read-only and read-write modes.\n> This has implications for the required permissions on their child nodes.\n> For example, a qcow2 node requests write permissions on bs->file only if\n> it is writable itself.\n> \n> This means that during bdrv_reopen(), the permissions need to be\n> recalculated in order to prevent failures where the bs->file\n> permissions don't match its actual read-only state (e.g. bs->file is a\n> read-write node, but the permission still enforces read-only access).\n\nPassing reopen_queue around makes the interface and implementations complicated.\nI wonder if any of the alternatives make sense:\n\n1) Don't pass reopen_queue as a parameter, just pass the one interesting\nBDRVReopenState pointer. So that callees don't need to call\nbdrv_reopen_get_flags().\n\n2) Don't change the prototypes at all, just change .bdrv_reopen_prepare contract\nso that after it returns, .bdrv_child_perm/.bdrv_check_perm should comply to the\nnew state that would be commited once .bdrv_reopen_commit() is called, or\nreverted if .bdrv_reopen_abort().\n\n3) Don't change the prototypes at all, track the reopen progress in block.c\ngenerically, (e.g. ignore conflicts and voilations) and update the permissions\nonly after bdrv_reopen_commit().\n\nFam\n\n> \n> Kevin Wolf (6):\n>   qemu-io: Reset qemuio_blk permissions before each command\n>   block: Add reopen_queue to bdrv_child_perm()\n>   block: Add reopen queue to bdrv_check_perm()\n>   block: Base permissions on rw state after reopen\n>   block: reopen: Queue children after their parents\n>   block: Fix permissions after bdrv_reopen()\n> \n>  include/block/block.h      |   2 +-\n>  include/block/block_int.h  |   7 ++\n>  block.c                    | 191 +++++++++++++++++++++++++++++++++------------\n>  block/commit.c             |   1 +\n>  block/mirror.c             |   1 +\n>  block/replication.c        |   1 +\n>  block/vvfat.c              |   1 +\n>  qemu-io.c                  |  13 +++\n>  tests/qemu-iotests/187.out |   2 +-\n>  9 files changed, 169 insertions(+), 50 deletions(-)\n> \n> -- \n> 2.13.5\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>)","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=famz@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 3xwdWB3Fv9z9s3w\n\tfor <incoming@patchwork.ozlabs.org>;\n\tMon, 18 Sep 2017 17:51:50 +1000 (AEST)","from localhost ([::1]:35118 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 1dtqqK-0004Hm-JT\n\tfor incoming@patchwork.ozlabs.org; Mon, 18 Sep 2017 03:51:48 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:60199)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <famz@redhat.com>) id 1dtqpo-0004FB-35\n\tfor qemu-devel@nongnu.org; Mon, 18 Sep 2017 03:51:17 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <famz@redhat.com>) id 1dtqpl-0002qS-20\n\tfor qemu-devel@nongnu.org; Mon, 18 Sep 2017 03:51:16 -0400","from mx1.redhat.com ([209.132.183.28]:43490)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <famz@redhat.com>)\n\tid 1dtqpi-0002oy-KI; Mon, 18 Sep 2017 03:51:10 -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 93D354ACB3;\n\tMon, 18 Sep 2017 07:51:09 +0000 (UTC)","from localhost (ovpn-12-141.pek2.redhat.com [10.72.12.141])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id E512E1880C;\n\tMon, 18 Sep 2017 07:51:08 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 93D354ACB3","Date":"Mon, 18 Sep 2017 15:51:07 +0800","From":"Fam Zheng <famz@redhat.com>","To":"Kevin Wolf <kwolf@redhat.com>","Message-ID":"<20170918075107.GK15551@lemon.lan>","References":"<20170915101008.16646-1-kwolf@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170915101008.16646-1-kwolf@redhat.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","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\tMon, 18 Sep 2017 07:51:09 +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 0/6] block: Fix permissions after ro/rw\n\treopen","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":"qemu-devel@nongnu.org, qemu-block@nongnu.org, mreitz@redhat.com","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":1769952,"web_url":"http://patchwork.ozlabs.org/comment/1769952/","msgid":"<20170918081103.GD31915@localhost.localdomain>","list_archive_url":null,"date":"2017-09-18T08:11:03","subject":"Re: [Qemu-devel] [PATCH 0/6] block: Fix permissions after ro/rw\n\treopen","submitter":{"id":2714,"url":"http://patchwork.ozlabs.org/api/people/2714/","name":"Kevin Wolf","email":"kwolf@redhat.com"},"content":"Am 18.09.2017 um 09:51 hat Fam Zheng geschrieben:\n> On Fri, 09/15 12:10, Kevin Wolf wrote:\n> > bdrv_reopen() can switch nodes between read-only and read-write modes.\n> > This has implications for the required permissions on their child nodes.\n> > For example, a qcow2 node requests write permissions on bs->file only if\n> > it is writable itself.\n> > \n> > This means that during bdrv_reopen(), the permissions need to be\n> > recalculated in order to prevent failures where the bs->file\n> > permissions don't match its actual read-only state (e.g. bs->file is a\n> > read-write node, but the permission still enforces read-only access).\n> \n> Passing reopen_queue around makes the interface and implementations\n> complicated.\n\nYes. I don't like it, but I couldn't find any easier way.\n\n> I wonder if any of the alternatives make sense:\n> \n> 1) Don't pass reopen_queue as a parameter, just pass the one\n> interesting BDRVReopenState pointer. So that callees don't need to\n> call bdrv_reopen_get_flags().\n\nThere isn't a single interesting BDRVReopenState. The subset that a\nsingle BDS needs to determine its new state is the the BDRVReopenState\nof all of its parents. This can be an arbitrary number.\n\n> 2) Don't change the prototypes at all, just change .bdrv_reopen_prepare contract\n> so that after it returns, .bdrv_child_perm/.bdrv_check_perm should comply to the\n> new state that would be commited once .bdrv_reopen_commit() is called, or\n> reverted if .bdrv_reopen_abort().\n\nHm, .bdrv_reopen_prepare already gets the whole queue passed, so I guess\nthis could technically work. I'm not sure if it is a good idea, though.\n\nSuch a change would still make .bdrv_child_perm depend on the reopen\nqueue, just without actually passing it as a parameter. I like such\nhidden data flows even less than adding an explicit one.\n\nIt would also mean that each block driver would have to save the queue\nin its local bs->opaque structure so that .bdrv_child_perm can access it\nlater. Alternatively, bdrv_reopen_prepare could already store the new\ncumulative parent permissions, but it would still involve two new fields\nin bs->opaque for storing something of a rather temporary nature.\n\nThough maybe I'm just missing another way to implement this that you had\nin mind?\n\n> 3) Don't change the prototypes at all, track the reopen progress in block.c\n> generically, (e.g. ignore conflicts and voilations) and update the permissions\n> only after bdrv_reopen_commit().\n\nBoth permission updates and reopen are transactional. You need to do\nboth prepare stages first before you can do commits. If you only start\ndoing the prepare stage of permissions during the commit stage of\nreopen, you break the error cases.\n\nKevin","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-mx07.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx07.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=kwolf@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 3xwdy640d4z9s3w\n\tfor <incoming@patchwork.ozlabs.org>;\n\tMon, 18 Sep 2017 18:11:42 +1000 (AEST)","from localhost ([::1]:35179 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 1dtr9Y-000237-NG\n\tfor incoming@patchwork.ozlabs.org; Mon, 18 Sep 2017 04:11:40 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:36125)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <kwolf@redhat.com>) id 1dtr9A-0001ze-OV\n\tfor qemu-devel@nongnu.org; Mon, 18 Sep 2017 04:11:17 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <kwolf@redhat.com>) id 1dtr99-00044g-Ob\n\tfor qemu-devel@nongnu.org; Mon, 18 Sep 2017 04:11:16 -0400","from mx1.redhat.com ([209.132.183.28]:36840)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <kwolf@redhat.com>)\n\tid 1dtr93-0003y2-5u; Mon, 18 Sep 2017 04:11:09 -0400","from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])\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 37A88C047B97;\n\tMon, 18 Sep 2017 08:11:08 +0000 (UTC)","from localhost.localdomain (ovpn-116-105.ams2.redhat.com\n\t[10.36.116.105])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 2A1D660606;\n\tMon, 18 Sep 2017 08:11:05 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 37A88C047B97","Date":"Mon, 18 Sep 2017 10:11:03 +0200","From":"Kevin Wolf <kwolf@redhat.com>","To":"Fam Zheng <famz@redhat.com>","Message-ID":"<20170918081103.GD31915@localhost.localdomain>","References":"<20170915101008.16646-1-kwolf@redhat.com>\n\t<20170918075107.GK15551@lemon.lan>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170918075107.GK15551@lemon.lan>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.31]);\n\tMon, 18 Sep 2017 08:11:08 +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 0/6] block: Fix permissions after ro/rw\n\treopen","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":"qemu-devel@nongnu.org, qemu-block@nongnu.org, mreitz@redhat.com","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":1770101,"web_url":"http://patchwork.ozlabs.org/comment/1770101/","msgid":"<20170918115303.GL15551@lemon.lan>","list_archive_url":null,"date":"2017-09-18T11:53:03","subject":"Re: [Qemu-devel] [PATCH 0/6] block: Fix permissions after ro/rw\n\treopen","submitter":{"id":24872,"url":"http://patchwork.ozlabs.org/api/people/24872/","name":"Fam Zheng","email":"famz@redhat.com"},"content":"On Mon, 09/18 10:11, Kevin Wolf wrote:\n> > 2) Don't change the prototypes at all, just change .bdrv_reopen_prepare contract\n> > so that after it returns, .bdrv_child_perm/.bdrv_check_perm should comply to the\n> > new state that would be commited once .bdrv_reopen_commit() is called, or\n> > reverted if .bdrv_reopen_abort().\n> \n> Hm, .bdrv_reopen_prepare already gets the whole queue passed, so I guess\n> this could technically work. I'm not sure if it is a good idea, though.\n> \n> Such a change would still make .bdrv_child_perm depend on the reopen\n> queue, just without actually passing it as a parameter. I like such\n> hidden data flows even less than adding an explicit one.\n> \n> It would also mean that each block driver would have to save the queue\n> in its local bs->opaque structure so that .bdrv_child_perm can access it\n> later. Alternatively, bdrv_reopen_prepare could already store the new\n> cumulative parent permissions, but it would still involve two new fields\n> in bs->opaque for storing something of a rather temporary nature.\n\nWhat about this?\n\n1) drv->bdrv_reopen_prepare() saves the desired new perms in BDRVReopenState.\n2) bdrv_reopen_prepare() checks the new perms after drv->bdrv_reopen_prepare()\n   returns.\n3) bdrv_reopen_commit() updates the bs to new perms.\n\nFam","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=famz@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 3xwktD6LSGz9s78\n\tfor <incoming@patchwork.ozlabs.org>;\n\tMon, 18 Sep 2017 21:53:39 +1000 (AEST)","from localhost ([::1]:35895 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 1dtucL-0007Cs-BI\n\tfor incoming@patchwork.ozlabs.org; Mon, 18 Sep 2017 07:53:37 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:45587)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <famz@redhat.com>) id 1dtuby-0007BX-4p\n\tfor qemu-devel@nongnu.org; Mon, 18 Sep 2017 07:53:15 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <famz@redhat.com>) id 1dtubx-0004KR-50\n\tfor qemu-devel@nongnu.org; Mon, 18 Sep 2017 07:53:14 -0400","from mx1.redhat.com ([209.132.183.28]:50014)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <famz@redhat.com>)\n\tid 1dtubq-0004HD-La; Mon, 18 Sep 2017 07:53: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 7C1D681DEC;\n\tMon, 18 Sep 2017 11:53:05 +0000 (UTC)","from localhost (ovpn-12-141.pek2.redhat.com [10.72.12.141])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id BAC2360BE2;\n\tMon, 18 Sep 2017 11:53:04 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 7C1D681DEC","Date":"Mon, 18 Sep 2017 19:53:03 +0800","From":"Fam Zheng <famz@redhat.com>","To":"Kevin Wolf <kwolf@redhat.com>","Message-ID":"<20170918115303.GL15551@lemon.lan>","References":"<20170915101008.16646-1-kwolf@redhat.com>\n\t<20170918075107.GK15551@lemon.lan>\n\t<20170918081103.GD31915@localhost.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170918081103.GD31915@localhost.localdomain>","User-Agent":"Mutt/1.8.3 (2017-05-23)","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.25]);\n\tMon, 18 Sep 2017 11:53: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","Subject":"Re: [Qemu-devel] [PATCH 0/6] block: Fix permissions after ro/rw\n\treopen","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":"qemu-devel@nongnu.org, qemu-block@nongnu.org, mreitz@redhat.com","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":1770115,"web_url":"http://patchwork.ozlabs.org/comment/1770115/","msgid":"<20170918121123.GH31915@localhost.localdomain>","list_archive_url":null,"date":"2017-09-18T12:11:23","subject":"Re: [Qemu-devel] [PATCH 0/6] block: Fix permissions after ro/rw\n\treopen","submitter":{"id":2714,"url":"http://patchwork.ozlabs.org/api/people/2714/","name":"Kevin Wolf","email":"kwolf@redhat.com"},"content":"Am 18.09.2017 um 13:53 hat Fam Zheng geschrieben:\n> On Mon, 09/18 10:11, Kevin Wolf wrote:\n> > > 2) Don't change the prototypes at all, just change .bdrv_reopen_prepare contract\n> > > so that after it returns, .bdrv_child_perm/.bdrv_check_perm should comply to the\n> > > new state that would be commited once .bdrv_reopen_commit() is called, or\n> > > reverted if .bdrv_reopen_abort().\n> > \n> > Hm, .bdrv_reopen_prepare already gets the whole queue passed, so I guess\n> > this could technically work. I'm not sure if it is a good idea, though.\n> > \n> > Such a change would still make .bdrv_child_perm depend on the reopen\n> > queue, just without actually passing it as a parameter. I like such\n> > hidden data flows even less than adding an explicit one.\n> > \n> > It would also mean that each block driver would have to save the queue\n> > in its local bs->opaque structure so that .bdrv_child_perm can access it\n> > later. Alternatively, bdrv_reopen_prepare could already store the new\n> > cumulative parent permissions, but it would still involve two new fields\n> > in bs->opaque for storing something of a rather temporary nature.\n> \n> What about this?\n> \n> 1) drv->bdrv_reopen_prepare() saves the desired new perms in\n>    BDRVReopenState.\n\nBut how does it determine the desired new perms? Either you duplicate\nthe logic of the .bdrv_child_perm implementation into a new set of\nfunctions that does the same thing, but based on the new state; or you\nextend the existing function with a BlockReopenQueue parameter. The\nlatter is basically this series, except with an additional unnecessary\ndetour through the driver code instead of doing it in common code.\n\nAlso note that storing it in BDRVReopenState would have to involve a new\nlist of an additional data structure because permissions are per\nBdrvChild, not per BlockDriverState.\n\n> 2) bdrv_reopen_prepare() checks the new perms after drv->bdrv_reopen_prepare()\n>    returns.\n> 3) bdrv_reopen_commit() updates the bs to new perms.\n\n2) and 3) are already implemented in this series like you suggest.\n\nKevin","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-mx06.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=kwolf@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 3xwlHb1Jb9z9s78\n\tfor <incoming@patchwork.ozlabs.org>;\n\tMon, 18 Sep 2017 22:12:11 +1000 (AEST)","from localhost ([::1]:36224 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 1dtuuH-00065Y-8h\n\tfor incoming@patchwork.ozlabs.org; Mon, 18 Sep 2017 08:12:09 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:52085)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <kwolf@redhat.com>) id 1dtutm-00063N-Uz\n\tfor qemu-devel@nongnu.org; Mon, 18 Sep 2017 08:11:41 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <kwolf@redhat.com>) id 1dtutl-0006TL-RU\n\tfor qemu-devel@nongnu.org; Mon, 18 Sep 2017 08:11:38 -0400","from mx1.redhat.com ([209.132.183.28]:40274)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <kwolf@redhat.com>)\n\tid 1dtutg-0006Pe-82; Mon, 18 Sep 2017 08:11:32 -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 47FA2356F0;\n\tMon, 18 Sep 2017 12:11:31 +0000 (UTC)","from localhost.localdomain (ovpn-116-105.ams2.redhat.com\n\t[10.36.116.105])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 2696F60466;\n\tMon, 18 Sep 2017 12:11:24 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 47FA2356F0","Date":"Mon, 18 Sep 2017 14:11:23 +0200","From":"Kevin Wolf <kwolf@redhat.com>","To":"Fam Zheng <famz@redhat.com>","Message-ID":"<20170918121123.GH31915@localhost.localdomain>","References":"<20170915101008.16646-1-kwolf@redhat.com>\n\t<20170918075107.GK15551@lemon.lan>\n\t<20170918081103.GD31915@localhost.localdomain>\n\t<20170918115303.GL15551@lemon.lan>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170918115303.GL15551@lemon.lan>","User-Agent":"Mutt/1.8.3 (2017-05-23)","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.30]);\n\tMon, 18 Sep 2017 12:11:31 +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 0/6] block: Fix permissions after ro/rw\n\treopen","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":"qemu-devel@nongnu.org, qemu-block@nongnu.org, mreitz@redhat.com","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":1770131,"web_url":"http://patchwork.ozlabs.org/comment/1770131/","msgid":"<20170918123212.GO15551@lemon.lan>","list_archive_url":null,"date":"2017-09-18T12:32:12","subject":"Re: [Qemu-devel] [PATCH 0/6] block: Fix permissions after ro/rw\n\treopen","submitter":{"id":24872,"url":"http://patchwork.ozlabs.org/api/people/24872/","name":"Fam Zheng","email":"famz@redhat.com"},"content":"On Mon, 09/18 14:11, Kevin Wolf wrote:\n> But how does it determine the desired new perms? Either you duplicate\n> the logic of the .bdrv_child_perm implementation into a new set of\n> functions that does the same thing, but based on the new state; or you\n> extend the existing function with a BlockReopenQueue parameter. The\n> latter is basically this series, except with an additional unnecessary\n> detour through the driver code instead of doing it in common code.\n> \n> Also note that storing it in BDRVReopenState would have to involve a new\n> list of an additional data structure because permissions are per\n> BdrvChild, not per BlockDriverState.\n\nIndeed, this is not going to remove any complexity. :(\n\nFam","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-mx04.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=famz@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 3xwllR0MKVz9s78\n\tfor <incoming@patchwork.ozlabs.org>;\n\tMon, 18 Sep 2017 22:32:51 +1000 (AEST)","from localhost ([::1]:36325 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 1dtvEH-00083z-7F\n\tfor incoming@patchwork.ozlabs.org; Mon, 18 Sep 2017 08:32:49 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:58584)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <famz@redhat.com>) id 1dtvDs-000839-8s\n\tfor qemu-devel@nongnu.org; Mon, 18 Sep 2017 08:32:28 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <famz@redhat.com>) id 1dtvDp-0007if-43\n\tfor qemu-devel@nongnu.org; Mon, 18 Sep 2017 08:32:24 -0400","from mx1.redhat.com ([209.132.183.28]:42150)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <famz@redhat.com>)\n\tid 1dtvDj-0007g5-1P; Mon, 18 Sep 2017 08:32:15 -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 1C8EC80476;\n\tMon, 18 Sep 2017 12:32:14 +0000 (UTC)","from localhost (ovpn-12-141.pek2.redhat.com [10.72.12.141])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 72B7D1851D;\n\tMon, 18 Sep 2017 12:32:13 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 1C8EC80476","Date":"Mon, 18 Sep 2017 20:32:12 +0800","From":"Fam Zheng <famz@redhat.com>","To":"Kevin Wolf <kwolf@redhat.com>","Message-ID":"<20170918123212.GO15551@lemon.lan>","References":"<20170915101008.16646-1-kwolf@redhat.com>\n\t<20170918075107.GK15551@lemon.lan>\n\t<20170918081103.GD31915@localhost.localdomain>\n\t<20170918115303.GL15551@lemon.lan>\n\t<20170918121123.GH31915@localhost.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170918121123.GH31915@localhost.localdomain>","User-Agent":"Mutt/1.8.3 (2017-05-23)","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.28]);\n\tMon, 18 Sep 2017 12:32:14 +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 0/6] block: Fix permissions after ro/rw\n\treopen","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":"qemu-devel@nongnu.org, qemu-block@nongnu.org, mreitz@redhat.com","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":1771969,"web_url":"http://patchwork.ozlabs.org/comment/1771969/","msgid":"<20170920103341.GC4730@localhost.localdomain>","list_archive_url":null,"date":"2017-09-20T10:33:41","subject":"Re: [Qemu-devel] [PATCH 0/6] block: Fix permissions after ro/rw\n\treopen","submitter":{"id":2714,"url":"http://patchwork.ozlabs.org/api/people/2714/","name":"Kevin Wolf","email":"kwolf@redhat.com"},"content":"Am 15.09.2017 um 12:10 hat Kevin Wolf geschrieben:\n> bdrv_reopen() can switch nodes between read-only and read-write modes.\n> This has implications for the required permissions on their child nodes.\n> For example, a qcow2 node requests write permissions on bs->file only if\n> it is writable itself.\n> \n> This means that during bdrv_reopen(), the permissions need to be\n> recalculated in order to prevent failures where the bs->file\n> permissions don't match its actual read-only state (e.g. bs->file is a\n> read-write node, but the permission still enforces read-only access).\n\nApplied to the block branch.\n\nKevin","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-mx07.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx07.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=kwolf@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 3xy3D472FTz9s4q\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 21 Sep 2017 01:13:40 +1000 (AEST)","from localhost ([::1]:48751 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 1dugh1-00018F-2U\n\tfor incoming@patchwork.ozlabs.org; Wed, 20 Sep 2017 11:13:39 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:55914)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <kwolf@redhat.com>) id 1dufl4-00029p-QQ\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 10:13:50 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <kwolf@redhat.com>) id 1dufky-0004et-So\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 10:13:46 -0400","from mx1.redhat.com ([209.132.183.28]:34502)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <kwolf@redhat.com>)\n\tid 1dufkq-0004aS-OF; Wed, 20 Sep 2017 10:13:32 -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 1B2E4C04B954;\n\tWed, 20 Sep 2017 10:33:46 +0000 (UTC)","from localhost.localdomain (ovpn-116-76.ams2.redhat.com\n\t[10.36.116.76])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 3D3375D97A;\n\tWed, 20 Sep 2017 10:33:43 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 1B2E4C04B954","Date":"Wed, 20 Sep 2017 12:33:41 +0200","From":"Kevin Wolf <kwolf@redhat.com>","To":"qemu-block@nongnu.org","Message-ID":"<20170920103341.GC4730@localhost.localdomain>","References":"<20170915101008.16646-1-kwolf@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170915101008.16646-1-kwolf@redhat.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","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.31]);\n\tWed, 20 Sep 2017 10:33:46 +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 0/6] block: Fix permissions after ro/rw\n\treopen","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":"famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com","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>"}}]