From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59863) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1axahg-0007yF-MN for qemu-devel@nongnu.org; Tue, 03 May 2016 09:49:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1axahU-0002zv-Rm for qemu-devel@nongnu.org; Tue, 03 May 2016 09:49:27 -0400 References: <1461706338-20219-1-git-send-email-mreitz@redhat.com> <1461706338-20219-7-git-send-email-mreitz@redhat.com> <20160502153611.GJ4882@noname.redhat.com> <57288929.7050804@redhat.com> <20160503133136.GF3917@noname.str.redhat.com> From: Max Reitz Message-ID: <5728AC3A.8080002@redhat.com> Date: Tue, 3 May 2016 15:48:42 +0200 MIME-Version: 1.0 In-Reply-To: <20160503133136.GF3917@noname.str.redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="565Ggl6svt4Ma7w8CcGKcb5G5bLD5Jkv2" Subject: Re: [Qemu-devel] [PATCH 06/19] block: Make bdrv_default_refresh_format_filename public List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, Alberto Garcia , Eric Blake This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --565Ggl6svt4Ma7w8CcGKcb5G5bLD5Jkv2 Content-Type: multipart/mixed; boundary="wWKdFfsFUkEatNlvUMXhwGTGrDsCIDtwp" From: Max Reitz To: Kevin Wolf Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, Alberto Garcia , Eric Blake Message-ID: <5728AC3A.8080002@redhat.com> Subject: Re: [PATCH 06/19] block: Make bdrv_default_refresh_format_filename public References: <1461706338-20219-1-git-send-email-mreitz@redhat.com> <1461706338-20219-7-git-send-email-mreitz@redhat.com> <20160502153611.GJ4882@noname.redhat.com> <57288929.7050804@redhat.com> <20160503133136.GF3917@noname.str.redhat.com> In-Reply-To: <20160503133136.GF3917@noname.str.redhat.com> --wWKdFfsFUkEatNlvUMXhwGTGrDsCIDtwp Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 03.05.2016 15:31, Kevin Wolf wrote: > Am 03.05.2016 um 13:19 hat Max Reitz geschrieben: >> On 02.05.2016 17:36, Kevin Wolf wrote: >>> Am 26.04.2016 um 23:32 hat Max Reitz geschrieben: >>>> In order to allow block drivers to use that function, it needs to be= >>>> public. In order to be useful, it needs to take a parameter which al= lows >>>> the caller to specify whether the runtime options allowed by the blo= ck >>>> driver are actually significant for the guest-visible BDS content. >>>> >>>> Signed-off-by: Max Reitz >>> >>> Is this actually good enough? I expect that many drivers will have so= me >>> options that are significant and other options that aren't. We alread= y >>> have some (Quorum: children are significant, rewrite-corrupted isn't)= , >>> but as we convert more things to proper options, we'll get more of th= em >>> (raw-posix: filename is significant, aio=3Dnative isn't). >>> >>> We might actually need to pass a list of significant fields instead t= hat >>> append_open_options() can use. >> >> Well, in theory, every driver with insignificant options would just >> implement .bdrv_refresh_filename() however it's needed. Making >> bdrv_default_refresh_format_filename() function public is just a way o= f >> keeping that implementation very simple for some drivers that only hav= e >> insignificant options. >> >> I'm not opposed to extending this function in the future when it >> actually makes sense. Right now I don't think it does. The only thing >> that changes if a significant option is detected is that no plain >> filename is generated; however, for Quorum we can never generate such = a >> filename. Therefore, we cannot use this function for Quorum anyway. >=20 > If you integrate it into append_open_options(), I suppose it would also= > mean that insignificant options are dropped from the json: description,= > i.e. Quorum would return a json: object with all children, but not the > rewrite-corrupted setting. Which I think would be a good thing. I'm not sure I do. :-) At least right now the JSON version is supposed to contain all options, be they significant or not. Let me try to remember my reasoning: Ideally, we want to get a filename which *exactly* results in the same BDS that we have. This should always be possible if instead of a plain filename one specifies options, e.g. using a JSON filename. However, such a JSON filename (or giving options using the dot syntax or as JSON with blockdev-add) is cumbersome. In many simple cases, we can (re-)construct a plain filename which yields exactly the same BDS, though. That's nice so that's what we try to do first. In some cases, it is impossible to construct a plain filename which yields a BDS that will return the same data when accessed. Then, we just cannot give such a filename and have to stick to a JSON filename, there's no way around this. However, sometimes we are in a gray area. We can construct a plain filename which yields a slightly different BDS than the one we have; but it will return the same data when accessed and thus it is "close enough". We then have to make a tradeoff between getting exactly the same BDS and having a nice and simple filename. I opted for the latter. However, if we do have to emit a JSON filename at some point in the tree I think we've basically "lost" already. If we get to that point, we may as well just emit all the options that have been used to construct the BDS, even if they don't change the data it yields. Max --wWKdFfsFUkEatNlvUMXhwGTGrDsCIDtwp-- --565Ggl6svt4Ma7w8CcGKcb5G5bLD5Jkv2 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJXKKw6AAoJEDuxQgLoOKytHQ0IAJscY4kGf/7AZWPxoPyCexkS 1j8KypRjwar8AKGtpnkPaYrN8JivgsCDcuJk71ZuC4zglsjfvRyq4KK7r6Bv5UBj MSSmX6lk6yinTxBYpn+LtCzS4FlnNMqP98JQlh48E1mZ85lUYvBgWj0RF1bZX+VG jtEDFrP1XPeswUFZLHw3WlVBTjjli+MysvE0VYKb+WES0Stg/V1OzJ9NTjHeCJaY J46IndP1t7iyM3rwcmaNmGVySdutw/wSSxaQcCbyIjlcnzgasC/Fx32lRxvHS+Zr gEuy+7gfOKyzThGd3ZpIRB6K3Lx+T9fwIO+/ecGOSHlkJXsco6wazdGdR/C6Cq8= =3BWV -----END PGP SIGNATURE----- --565Ggl6svt4Ma7w8CcGKcb5G5bLD5Jkv2--