From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:43527) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SAinH-0002yX-FF for qemu-devel@nongnu.org; Thu, 22 Mar 2012 10:15:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SAinB-00043s-5j for qemu-devel@nongnu.org; Thu, 22 Mar 2012 10:15:11 -0400 Received: from mail-lb0-f173.google.com ([209.85.217.173]:39274) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SAinA-00041Q-Re for qemu-devel@nongnu.org; Thu, 22 Mar 2012 10:15:05 -0400 Received: by lbon3 with SMTP id n3so1729474lbo.4 for ; Thu, 22 Mar 2012 07:15:01 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <1332345124-381-1-git-send-email-benoit.canet@gmail.com> <1332345124-381-5-git-send-email-benoit.canet@gmail.com> Date: Thu, 22 Mar 2012 15:15:01 +0100 Message-ID: From: =?UTF-8?Q?Beno=C3=AEt_Canet?= Content-Type: multipart/alternative; boundary=e0cb4efe2a34a5142104bbd58848 Subject: Re: [Qemu-devel] [PATCH V2 4/7] qed: add bdrv_invalidate_cache to be called after incoming live migration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: kwolf@redhat.com, pbonzini@redhat.com, Anthony Liguori , qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com --e0cb4efe2a34a5142104bbd58848 Content-Type: text/plain; charset=UTF-8 >It's not clear to me why we need to introduce this field to stash >flags values. bs->open_flags already has this information. >Originally this was introduced in 06d9260ffa9 ("qcow2: implement >bdrv_invalidate_cache (v2)") for qcow2. I wonder if that field is >necessary when we already have bs->open_flags. >What I don't like about s->flags is that it duplicates state *and* >it's done in each block driver that supports .bdrv_invalidate_cache(). > So I wonder if we can drop it? I added this flag after seeing the following code in in bdrv_open_common. /* * Clear flags that are internal to the block layer before opening the * image. */ open_flags = flags & ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING); This lead me to believe that bs->open_flags != open_flags. --e0cb4efe2a34a5142104bbd58848 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

>It's not clear to me w= hy we need to introduce this field to stash
>flags values. =C2=A0bs-&= gt;open_flags already has this information.

>Originally this was = introduced in 06d9260ffa9 ("qcow2: implement
>bdrv_invalidate_cache (v2)") for qcow2. =C2=A0I wonder if that fie= ld is
>necessary when we already have bs->open_flags.

>W= hat I don't like about s->flags is that it duplicates state *and* >it's done in each block driver that supports .bdrv_invalidate_cache= ().
> So I wonder if we can drop it?

I added this= flag after seeing the following code in=C2=A0in bdrv_open_common.

=
=C2=A0 =C2=A0 /*
=C2=A0 =C2=A0 =C2=A0* Clear flags that= are internal to the block layer before opening the
=C2=A0 =C2=A0= =C2=A0* image.
=C2=A0 =C2=A0 =C2=A0*/
=C2=A0 =C2=A0 op= en_flags =3D flags & ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);

This lead me to believe that =C2=A0bs->open_flags != =3D open_flags.
--e0cb4efe2a34a5142104bbd58848--