Message ID | 1287774922-19033-1-git-send-email-jvrao@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
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().
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. While these are just wrappers for now, what is the future direction of this kind of calls like qemu_fdatasync() qemu_malloc() etc? Will they be OK to call in threadlets without any locks? If so, may be we can use them in this function. Thanks, JV
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.
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 >
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); + } else { + return fsync(fd); + } } static int local_statfs(FsContext *s, const char *path, struct statfs *stbuf) diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c index daade77..7c59988 100644 --- a/hw/virtio-9p.c +++ b/hw/virtio-9p.c @@ -248,9 +248,9 @@ static int v9fs_do_remove(V9fsState *s, V9fsString *path) return s->ops->remove(&s->ctx, path->data); } -static int v9fs_do_fsync(V9fsState *s, int fd) +static int v9fs_do_fsync(V9fsState *s, int fd, int datasync) { - return s->ops->fsync(&s->ctx, fd); + return s->ops->fsync(&s->ctx, fd, datasync); } static int v9fs_do_statfs(V9fsState *s, V9fsString *path, struct statfs *stbuf) @@ -1868,16 +1868,17 @@ static void v9fs_fsync(V9fsState *s, V9fsPDU *pdu) int32_t fid; size_t offset = 7; V9fsFidState *fidp; + int datasync; int err; - pdu_unmarshal(pdu, offset, "d", &fid); + pdu_unmarshal(pdu, offset, "dd", &fid, &datasync); fidp = lookup_fid(s, fid); if (fidp == NULL) { err = -ENOENT; v9fs_post_do_fsync(s, pdu, err); return; } - err = v9fs_do_fsync(s, fidp->fs.fd); + err = v9fs_do_fsync(s, fidp->fs.fd, datasync); v9fs_post_do_fsync(s, pdu, err); } @@ -3001,7 +3002,7 @@ static void v9fs_wstat(V9fsState *s, V9fsPDU *pdu) /* do we need to sync the file? */ if (donttouch_stat(&vs->v9stat)) { - err = v9fs_do_fsync(s, vs->fidp->fs.fd); + err = v9fs_do_fsync(s, vs->fidp->fs.fd, 0); v9fs_wstat_post_fsync(s, vs, err); return; }
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(-)