From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55974) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1axaR7-00033g-Hn for qemu-devel@nongnu.org; Tue, 03 May 2016 09:32:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1axaQv-0005Az-Db for qemu-devel@nongnu.org; Tue, 03 May 2016 09:32:20 -0400 Date: Tue, 3 May 2016 15:31:36 +0200 From: Kevin Wolf Message-ID: <20160503133136.GF3917@noname.str.redhat.com> 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> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="1SQmhf2mF2YjsYvc" Content-Disposition: inline In-Reply-To: <57288929.7050804@redhat.com> 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: Max Reitz Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, Alberto Garcia , Eric Blake --1SQmhf2mF2YjsYvc Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 allo= ws > >> the caller to specify whether the runtime options allowed by the block > >> driver are actually significant for the guest-visible BDS content. > >> > >> Signed-off-by: Max Reitz > >=20 > > Is this actually good enough? I expect that many drivers will have some > > options that are significant and other options that aren't. We already > > have some (Quorum: children are significant, rewrite-corrupted isn't), > > but as we convert more things to proper options, we'll get more of them > > (raw-posix: filename is significant, aio=3Dnative isn't). > >=20 > > We might actually need to pass a list of significant fields instead that > > append_open_options() can use. >=20 > 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 of > keeping that implementation very simple for some drivers that only have > insignificant options. >=20 > 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. 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. Kevin > However, instead of extending this function, it may make more sense then > to introduce a new field to the BlockDriver struct which is a > NULL-terminated array of significant option names, or something like > that. If .bdrv_refresh_filename is NULL but that array pointer is not, > then the default implementation could behave accordingly. But this is > something I'd defer to the future, too, unless you can point out a > current block driver that would benefit from this functionality (I don't > think Quorum does, as I said above). >=20 > Max --1SQmhf2mF2YjsYvc Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJXKKg4AAoJEH8JsnLIjy/WmYEP/Ahsdp43mGE21QrSclR7aIfV FFA7YhTmPdTtzV28MVYIbpxug/S56FiEJkPa64dUQR4UuleuPlg6wpuQGgKT7TAl OAASHNAw4Y9Dbok6nWJzCmqULD55YcQsVIbhX1P42iMXyBB9hMpoUI/wjcmolK+f 8jy9wIG2g1Or45GIARAxjCrowE9RvbyZswX1/1LZePnDGtPG97hboq75i/eFqkkH uQCgtN7/AxH5z04njTsnRAU95V0MDC4gCgocz6CibjuuPiL9m6fO6kJRXteAx8/Z 5RrbAOY90atBF9LMIfX3hgqr4CJOlZE8zf9LsekVobE0SesQ/aaeprUa+ejgDhH6 fZNGHyERa7s6QqOAoFusA4VYcVcib7l+C+igO1L1A6zYjLM+BElt9YV6HINDGnP7 0S3M9DX+zSSzaryeP7TtlxK3/Y1Q12HtFKJuQ1Jk4zu71fq3Q8EByDXc70e4YRHf F5QZlAYX5oKHf+ii2rFvZSvVW8T35NIzY2J4X6c1sNcQdL3uFkkk6lO93/qJORSi U/I5gBPtwNmwcNAybKPZVI1u3XuLd3jHS7zwkAkIX2yDGYgCqpSjU9kWsJ62Fc8z LbKgf5W3i0gxn9HN9Cf9ynoZGynxYGrMFV+qC5HmFIRT1rXQAY/ooWE/2ow/eDb8 NcixnFN62IrKEmZpM0AD =yQWE -----END PGP SIGNATURE----- --1SQmhf2mF2YjsYvc--