On Tue, 1 Mar 2011 15:59:19 +0000, Stefan Hajnoczi <stefa...@gmail.com> wrote:
> On Tue, Mar 1, 2011 at 3:02 PM, Aneesh Kumar K. V
> <aneesh.ku...@linux.vnet.ibm.com> wrote:
> > On Tue, 1 Mar 2011 10:22:07 +0000, Stefan Hajnoczi <stefa...@gmail.com>
> > wrote:
> >> On Tue, Mar 1, 2011 at 6:38 AM, Aneesh Kumar K.V
> >> <aneesh.ku...@linux.vnet.ibm.com> wrote:
> >> > Signed-off-by: Aneesh Kumar K.V <aneesh.ku...@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.
>
> 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
http://thread.gmane.org/gmane.linux.file-systems/44628
>
> 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.
>
> I think virtio-9p has a file descriptor cache. Would it be possible
> to fsync() those file descriptors?
>
Ideally we should. But that would involve a large number of fsync calls.
-aneesh
