From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59991) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1akqhe-0005i4-US for qemu-devel@nongnu.org; Tue, 29 Mar 2016 06:16:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1akqhd-0004yt-V0 for qemu-devel@nongnu.org; Tue, 29 Mar 2016 06:16:50 -0400 Date: Tue, 29 Mar 2016 12:16:40 +0200 From: Kevin Wolf Message-ID: <20160329101640.GC4600@noname.redhat.com> References: <1458325289-17848-1-git-send-email-kwolf@redhat.com> <1458325289-17848-18-git-send-email-kwolf@redhat.com> <56F6F995.1040906@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="yNb1oOkm5a9FJOVX" Content-Disposition: inline In-Reply-To: <56F6F995.1040906@redhat.com> Subject: Re: [Qemu-devel] [PATCH 17/20] qemu-io: Use bdrv_parse_cache_mode() in reopen_f() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org --yNb1oOkm5a9FJOVX Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Am 26.03.2016 um 22:05 hat Max Reitz geschrieben: > On 18.03.2016 19:21, Kevin Wolf wrote: > > We must forbid changing the WCE flag in bdrv_reopen() in the same patch, > > as otherwise the behaviour would change so that the flag takes > > precedence over the explicitly specified option. > >=20 > > The correct value of the WCE flag depends on the BlockBackend user (e.g. > > guest device) and isn't a decision that the QMP client makes, so this > > change is what we want. > >=20 > > Signed-off-by: Kevin Wolf > > --- > > block.c | 18 ++++++------------ > > qemu-io-cmds.c | 14 +++++++++++++- > > tests/qemu-iotests/142 | 2 +- > > tests/qemu-iotests/142.out | 2 +- > > 4 files changed, 21 insertions(+), 15 deletions(-) > >=20 >=20 > [...] >=20 > > diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c > > index e929d24..7de3754 100644 > > --- a/qemu-io-cmds.c > > +++ b/qemu-io-cmds.c >=20 > [...] >=20 > > @@ -2136,14 +2137,25 @@ static int reopen_f(BlockBackend *blk, int argc= , char **argv) > > return qemuio_command_usage(&reopen_cmd); > > } > > =20 > > + if (writethrough !=3D blk_enable_write_cache(blk) && > > + blk_get_attached_dev(blk)) > > + { > > + error_report("Cannot change cache.writeback: Device attached"); > > + qemu_opts_reset(&reopen_opts); > > + return 0; > > + } > > + > > qopts =3D qemu_opts_find(&reopen_opts, NULL); > > opts =3D qopts ? qemu_opts_to_qdict(qopts, NULL) : NULL; > > qemu_opts_reset(&reopen_opts); > > =20 > > + flags |=3D blk_enable_write_cache(blk) ? BDRV_O_CACHE_WB : 0; >=20 > Shouldn't this be bdrv_enable_write_cache(bs)? >=20 > With blk_enable_write_cache(blk), reopening a non-WB BB should always > fail because bdrv_reopen_multiple() accuses us of trying to change the > WB mode (whereas we really don't want to change the BDS's mode). bdrv_enable_write_cache() is only a wrapper around blk_enable_write_cache(), so that wouldn't make any difference. It also means that bdrv_reopen() checks the cache mode of the BlockBackend rather than always returning true. This line exists precisely to avoid the error message you mention. And as you noticed, the line goes away anyway when BDRV_O_CACHE_WB is removed later in the series. Kevin --yNb1oOkm5a9FJOVX Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJW+lYIAAoJEH8JsnLIjy/W+VsP/RQrfFhSkWmck8iTOLftGLXa 2FnmgBemxfntlvRT071exGAKH6owuJY+pzDtp/1E7xkbb+tuF9EvGkMbiwY0OIxg 7Y6CoA6uJpVBMstlEEDnXHM8DFMVgNRd9bTdFs5TNuoLohDzVkYTyBUU3kRhHE/Z 1dN3yfXtSGLNP71+h/T1uk2ZDAO2Y6dVF0g6ZTQg+nAan9TQ/ZcMfu90sCb0FvB5 hkAsid09shc2uEiXvdWsqxOEFQ1+s+UlIjuq18hvNH0x54sZxf7wvZnMFEjGqyAW ysBA3xpVBSE0vJOLPedaHoAkhZUORnNjwK+kYDkrhyHi6NxaVazZaMlWFkZa1b6A EGwvEtmY9qLo7kuC2GiuLqeNmwgC/m5hpoAd89cZ1uVbOCoagAh+yaQDMNfquRRD v3Hq6ESwlFAQOf59Z9Fpu0JrQAZnBf9glpP9lCFHFmWK7aYcotnQj/KamvBaSPVB NBZaUj8+KQyFcgXLwygip/8HzuzMHuFkYrQBcdXmNAJKtqERcdInFNCoyck3QPRy rom8fI1pJd9XLLDNfrzCvosztm81r/6s5q/ulgliTqsF0O0KPDtBg/vMRGmeZKUr KNf4rNR8xKZNdO/afwlp06uaHr9no5VsRp7PTp0j3FFSyhOfoMuGiL5EK1yBHTGU WFSAVuWw6AKyd2ml4Mb6 =qYca -----END PGP SIGNATURE----- --yNb1oOkm5a9FJOVX--