From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=40923 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PuR5u-0006A3-0m for qemu-devel@nongnu.org; Tue, 01 Mar 2011 10:02:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PuR5s-0005uV-Fa for qemu-devel@nongnu.org; Tue, 01 Mar 2011 10:02:33 -0500 Received: from e28smtp01.in.ibm.com ([122.248.162.1]:57092) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PuR5r-0005tE-IV for qemu-devel@nongnu.org; Tue, 01 Mar 2011 10:02:32 -0500 Received: from d28relay01.in.ibm.com (d28relay01.in.ibm.com [9.184.220.58]) by e28smtp01.in.ibm.com (8.14.4/8.13.1) with ESMTP id p21F2QkQ017027 for ; Tue, 1 Mar 2011 20:32:26 +0530 Received: from d28av05.in.ibm.com (d28av05.in.ibm.com [9.184.220.67]) by d28relay01.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p21F2PjX1630294 for ; Tue, 1 Mar 2011 20:32:26 +0530 Received: from d28av05.in.ibm.com (loopback [127.0.0.1]) by d28av05.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p21F2PT1026132 for ; Wed, 2 Mar 2011 02:02:25 +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> Date: Tue, 01 Mar 2011 20:32:09 +0530 Message-ID: <87k4gievf2.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 10:22:07 +0000, Stefan Hajnoczi wro= te: > 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 *pd= u) > > =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); > > +} >=20 > Please explain the semantics of P9_TSYNCFS. Won't returning success > 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 as a part of sync but 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 >=20 > It seems unnecessary to split v9fs_post_do_syncfs() into its own > function since there is no blocking point here and a callback will not > be needed. >=20 That is done as a place holder to add the per file system sync call once we get the support http://thread.gmane.org/gmane.linux.file-systems/44628 -aneesh