Re: [Qemu-devel] [PATCH] [virtio-9p] Add datasync to server side TFSYNC/RFSYNC for dotl

["Venkateswararao Jujjuri (JV)" <jvrao@linux.vnet.ibm.com>]

On 10/23/2010 11:27 AM, Blue Swirl wrote:
> On Sat, Oct 23, 2010 at 3:39 PM, Venkateswararao Jujjuri (JV)
> <jvrao@linux.vnet.ibm.com> wrote:
>> On 10/23/2010 5:44 AM, Blue Swirl wrote:
>>> On Fri, Oct 22, 2010 at 7:15 PM, Venkateswararao Jujjuri (JV)
>>> <jvrao@linux.vnet.ibm.com> wrote:
>>>> SYNOPSIS
>>>>    size[4] Tfsync tag[2] fid[4] datasync[4]
>>>>
>>>>    size[4] Rfsync tag[2]
>>>>
>>>> DESCRIPTION
>>>>
>>>>    The Tfsync transaction transfers ("flushes") all modified in-core data of
>>>>    file identified by fid to the disk device (or other  permanent  storage
>>>>    device)  where that  file  resides.
>>>>
>>>>    If datasync flag is specified data will be fleshed but does not flush
>>>>    modified metadata unless  that  metadata  is  needed  in order to allow a
>>>>    subsequent data retrieval to be correctly handled.
>>>>
>>>> Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
>>>> ---
>>>>  hw/file-op-9p.h      |    2 +-
>>>>  hw/virtio-9p-local.c |    8 ++++++--
>>>>  hw/virtio-9p.c       |   11 ++++++-----
>>>>  3 files changed, 13 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/hw/file-op-9p.h b/hw/file-op-9p.h
>>>> index 21d60b5..c7731c2 100644
>>>> --- a/hw/file-op-9p.h
>>>> +++ b/hw/file-op-9p.h
>>>> @@ -86,7 +86,7 @@ typedef struct FileOperations
>>>>     int (*fstat)(FsContext *, int, struct stat *);
>>>>     int (*rename)(FsContext *, const char *, const char *);
>>>>     int (*truncate)(FsContext *, const char *, off_t);
>>>> -    int (*fsync)(FsContext *, int);
>>>> +    int (*fsync)(FsContext *, int, int);
>>>>     int (*statfs)(FsContext *s, const char *path, struct statfs *stbuf);
>>>>     ssize_t (*lgetxattr)(FsContext *, const char *,
>>>>                          const char *, void *, size_t);
>>>> diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
>>>> index 0d52020..d0c79e4 100644
>>>> --- a/hw/virtio-9p-local.c
>>>> +++ b/hw/virtio-9p-local.c
>>>> @@ -490,9 +490,13 @@ static int local_remove(FsContext *ctx, const char *path)
>>>>     return remove(rpath(ctx, path));
>>>>  }
>>>>
>>>> -static int local_fsync(FsContext *ctx, int fd)
>>>> +static int local_fsync(FsContext *ctx, int fd, int datasync)
>>>>  {
>>>> -    return fsync(fd);
>>>> +    if (datasync) {
>>>> +        return fdatasync(fd);
>>>
>>> Please use qemu_fdatasync().
>>
>> This file and functions in it are intended as wrappers of syscalls
>> which can be used in threadlets. Per Anthony's statement about avoiding
>> any QEMU specific code in threadlets, l I am planning to avoid any QEMU specific
>> calls
>> in the virtio-9p-local.c library.
> 
> That way would lead to code duplication, reduced functionality and
> portability. If the problem is threading and that the current
> functions are not thread safe, then we should add new functions to
> QEMU instead.

Thanks, Sent another patch with qemu_fdatasync.
The function in its current form has no issues with threading.

Thanks,
JV
>