From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=34751 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PuTua-0003Bk-9T for qemu-devel@nongnu.org; Tue, 01 Mar 2011 13:03:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PuTuU-0003qv-Sl for qemu-devel@nongnu.org; Tue, 01 Mar 2011 13:03:04 -0500 Received: from e23smtp07.au.ibm.com ([202.81.31.140]:39710) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PuTuU-0003pa-94 for qemu-devel@nongnu.org; Tue, 01 Mar 2011 13:02:58 -0500 Received: from d23relay05.au.ibm.com (d23relay05.au.ibm.com [202.81.31.247]) by e23smtp07.au.ibm.com (8.14.4/8.13.1) with ESMTP id p21I2muY025691 for ; Wed, 2 Mar 2011 05:02:48 +1100 Received: from d23av04.au.ibm.com (d23av04.au.ibm.com [9.190.235.139]) by d23relay05.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p21I2llK2416696 for ; Wed, 2 Mar 2011 05:02:48 +1100 Received: from d23av04.au.ibm.com (loopback [127.0.0.1]) by d23av04.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p21I2l4a030431 for ; Wed, 2 Mar 2011 05:02:47 +1100 From: "Aneesh Kumar K. V" Subject: Re: [Qemu-devel] [PATCH -V2 4/6] hw/9pfs: Implement syncfs In-Reply-To: References: <1298961534-8099-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com> <1298961534-8099-4-git-send-email-aneesh.kumar@linux.vnet.ibm.com> <87k4gievf2.fsf@linux.vnet.ibm.com> Date: Tue, 01 Mar 2011 23:32:29 +0530 Message-ID: <87r5aqhg7e.fsf@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org On Tue, 1 Mar 2011 15:59:19 +0000, Stefan Hajnoczi wro= te: > On Tue, Mar 1, 2011 at 3:02 PM, Aneesh Kumar K. V > wrote: > > On Tue, 1 Mar 2011 10:22:07 +0000, Stefan Hajnoczi = wrote: > >> On Tue, Mar 1, 2011 at 6:38 AM, Aneesh Kumar K.V > >> wrote: > >> > Signed-off-by: Aneesh Kumar K.V > >> > --- > >> > =C2=A0hw/9pfs/virtio-9p.c | =C2=A0 31 +++++++++++++++++++++++++++++++ > >> > =C2=A0hw/9pfs/virtio-9p.h | =C2=A0 =C2=A02 ++ > >> > =C2=A02 files changed, 33 insertions(+), 0 deletions(-) > >> > > >> > diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c > >> > index c4b0198..882f4f3 100644 > >> > --- a/hw/9pfs/virtio-9p.c > >> > +++ b/hw/9pfs/virtio-9p.c > >> > @@ -1978,6 +1978,36 @@ static void v9fs_fsync(V9fsState *s, V9fsPDU = *pdu) > >> > =C2=A0 =C2=A0 v9fs_post_do_fsync(s, pdu, err); > >> > =C2=A0} > >> > > >> > +static void v9fs_post_do_syncfs(V9fsState *s, V9fsPDU *pdu, int err) > >> > +{ > >> > + =C2=A0 =C2=A0if (err =3D=3D -1) { > >> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0err =3D -errno; > >> > + =C2=A0 =C2=A0} > >> > + =C2=A0 =C2=A0complete_pdu(s, pdu, err); > >> > +} > >> > + > >> > +static void v9fs_syncfs(V9fsState *s, V9fsPDU *pdu) > >> > +{ > >> > + =C2=A0 =C2=A0int err; > >> > + =C2=A0 =C2=A0int32_t fid; > >> > + =C2=A0 =C2=A0size_t offset =3D 7; > >> > + =C2=A0 =C2=A0V9fsFidState *fidp; > >> > + > >> > + =C2=A0 =C2=A0pdu_unmarshal(pdu, offset, "d", &fid); > >> > + =C2=A0 =C2=A0fidp =3D lookup_fid(s, fid); > >> > + =C2=A0 =C2=A0if (fidp =3D=3D NULL) { > >> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0err =3D -ENOENT; > >> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0v9fs_post_do_syncfs(s, pdu, err); > >> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0return; > >> > + =C2=A0 =C2=A0} > >> > + =C2=A0 =C2=A0/* > >> > + =C2=A0 =C2=A0 * We don't have per file system syncfs > >> > + =C2=A0 =C2=A0 * So just return success > >> > + =C2=A0 =C2=A0 */ > >> > + =C2=A0 =C2=A0err =3D 0; > >> > + =C2=A0 =C2=A0v9fs_post_do_syncfs(s, pdu, err); > >> > +} > >> > >> Please explain the semantics of P9_TSYNCFS. =C2=A0Won't returning succ= ess > >> without doing anything lead to data integrity issues? > > > > I should actually do the 9P Operation format as commit message. Will > > add in the next update. Whether returning here would cause a data > > integrity issue, it depends what sort of guarantee we want to > > provide. So calling sync on the guest will cause all the dirty pages in > > the guest to be flushed to host. Now all those changes are in the host > > page cache and it would be nice to flush them =C2=A0as a part of sync b= ut > > then since we don't have a per file system sync, the above would imply > > we flush all dirty pages on the host which can result in large > > performance impact. >=20 > You get the define the semantics of P9_TSYNCFS? I thought this is > part of a well-defined protocol :). If this is a .L extension then > it's probably a bad design and shouldn't be added to the protocol if > we can't implement it. It is a part of .L extension and we can definitely implement it. There is patch out there which is yet to be merged=20 http://thread.gmane.org/gmane.linux.file-systems/44628 >=20 > Is this operation supposed to flush the disk write cache too? I am not sure we need to specify that as a part of 9p operation. I guess we can only say maximum possible data integrity. Whether a sync will cause disk write cache flush depends on the file system. For ext* that can be controlled by mount option barrier.=20 >=20 > I think virtio-9p has a file descriptor cache. Would it be possible > to fsync() those file descriptors? >=20 Ideally we should. But that would involve a large number of fsync calls. -aneesh