Re: [Qemu-devel] [PATCH -V2 4/6] hw/9pfs: Implement syncfs

["Aneesh Kumar K. V" <aneesh.kumar@linux.vnet.ibm.com>]

On Tue, 1 Mar 2011 10:22:07 +0000, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Tue, Mar 1, 2011 at 6:38 AM, Aneesh Kumar K.V
> <aneesh.kumar@linux.vnet.ibm.com> wrote:
> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> > ---
> >  hw/9pfs/virtio-9p.c |   31 +++++++++++++++++++++++++++++++
> >  hw/9pfs/virtio-9p.h |    2 ++
> >  2 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)
> >     v9fs_post_do_fsync(s, pdu, err);
> >  }
> >
> > +static void v9fs_post_do_syncfs(V9fsState *s, V9fsPDU *pdu, int err)
> > +{
> > +    if (err == -1) {
> > +        err = -errno;
> > +    }
> > +    complete_pdu(s, pdu, err);
> > +}
> > +
> > +static void v9fs_syncfs(V9fsState *s, V9fsPDU *pdu)
> > +{
> > +    int err;
> > +    int32_t fid;
> > +    size_t offset = 7;
> > +    V9fsFidState *fidp;
> > +
> > +    pdu_unmarshal(pdu, offset, "d", &fid);
> > +    fidp = lookup_fid(s, fid);
> > +    if (fidp == NULL) {
> > +        err = -ENOENT;
> > +        v9fs_post_do_syncfs(s, pdu, err);
> > +        return;
> > +    }
> > +    /*
> > +     * We don't have per file system syncfs
> > +     * So just return success
> > +     */
> > +    err = 0;
> > +    v9fs_post_do_syncfs(s, pdu, err);
> > +}
> 
> 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. 

> 
> 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.
> 

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