From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47417) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aBeS1-0007iw-6x for qemu-devel@nongnu.org; Wed, 23 Dec 2015 03:07:14 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aBeRx-0004dt-W6 for qemu-devel@nongnu.org; Wed, 23 Dec 2015 03:07:13 -0500 Received: from mail-pf0-x22e.google.com ([2607:f8b0:400e:c00::22e]:36610) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aBeRx-0004dp-QU for qemu-devel@nongnu.org; Wed, 23 Dec 2015 03:07:09 -0500 Received: by mail-pf0-x22e.google.com with SMTP id 65so4684589pff.3 for ; Wed, 23 Dec 2015 00:07:09 -0800 (PST) Date: Wed, 23 Dec 2015 16:07:01 +0800 From: Stefan Hajnoczi Message-ID: <20151223080701.GE5149@stefanha-x1.localdomain> References: <1450767586-28794-1-git-send-email-den@openvz.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="BZaMRJmqxGScZ8Mx" Content-Disposition: inline In-Reply-To: <1450767586-28794-1-git-send-email-den@openvz.org> Subject: Re: [Qemu-devel] [PATCH 1/1] block: fix inability to start VM with native AIO List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Denis V. Lunev" Cc: Kevin Wolf , qemu-devel@nongnu.org --BZaMRJmqxGScZ8Mx Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Dec 22, 2015 at 09:59:46AM +0300, Denis V. Lunev wrote: > error: Failed to start domain rhel7 > error: internal error: process exited while connecting to monitor: > 2015-12-22T06:55:18.812637Z qemu-system-x86_64: > -drive file=3D/var/lib/libvirt/images/rhel7.qcow2,if=3Dnone, > id=3Ddrive-scsi0-0-0-0,format=3Dqcow2,cache=3Dnone,aio=3Dnative: > aio=3Dnative was specified, but it requires cache.direct=3Don, > which was not specified. >=20 > cache=3Dnone option was specified as seen above while the VM is unable to > start. The patch properly passed BDRV_O_NOCACHE to underlying layer. >=20 > The problem is revealed with > commit d657c0c289e944fc22289f5c318f48da87d79dcb > Author: Kevin Wolf > Date: Tue Dec 15 11:35:36 2015 +0100 >=20 > raw-posix: Make aio=3Dnative option binding >=20 > Signed-off-by: Denis V. Lunev > CC: Kevin Wolf > --- > block.c | 1 + > 1 file changed, 1 insertion(+) >=20 > diff --git a/block.c b/block.c > index 411edbf..fe0fbbc 100644 > --- a/block.c > +++ b/block.c > @@ -990,6 +990,7 @@ static int bdrv_open_common(BlockDriverState *bs, Bdr= vChild *file, > bs->opaque =3D g_malloc0(drv->instance_size); > =20 > /* Apply cache mode options */ > + update_flags_from_options(&open_flags, opts); > update_flags_from_options(&bs->open_flags, opts); > bdrv_set_enable_write_cache(bs, bs->open_flags & BDRV_O_CACHE_WB); I tried to review this patch and failed to understand block.c flags handling. Perhaps there is a larger problem here because I see: 1. .bdrv_open() and friends take an int flags argument in addition to the already open bs node, which has bs->open_flags. Some of the code that manipulates the flags argument in block.c updates bs->open_flags to keep them in sync. Some code does not (e.g. BDRV_O_NO_BACKING). Is this a bug? Should int flags be removed in favor of just bs->open_flags? 2. The bdrv_open_common() open_flags local variable contains different values from bs->open_flags (i.e. BDRV_O_PROTOCOL). I'm not sure whether this is intentional or a bug. Your patch syncs them but I wonder if it would be cleaner to remove the open_flags local (avoiding similar problems in the future)? 3. block/qcow2.c stashes .bdrv_open(flags) in s->flags. I'm not sure if bs->open_flags can be used instead of s->flags. That would be simpler. Maybe someone can explain how this is all supposed to work. Stefan --BZaMRJmqxGScZ8Mx Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJWelYkAAoJEJykq7OBq3PIa+sH/2uj7bw/pGUGMuT4XPK+p5aK 4488SlWZ3ds0xE55pSOSwqFezjYYFkr+3geAv9+gRUwe4RLRkOTaanhfqXltemqS vNWDK6I+Cywx+lEE+tsHeFFVQjtkMwj1r8IYExvlN/556L0BUtyEDy0B5BJtoi86 Rw1FOFa2SPNGgwifLPmIKxDM+X0oLDL976ndyIPaXzx0Lhae8CZ99YTrxV4Zef9Z t08ivFbS5nq2U6S0za70qQVVJtXlLne7cQ1ttDQEU/mA+x9RLI0TdR1C9FDw2i6M 2DXOJ2qvfTOab4t4cD7kJdrDN8YIHePbM5ikudCBSf78iU2p1mV31VVg+kw6IxA= =BgZI -----END PGP SIGNATURE----- --BZaMRJmqxGScZ8Mx--