On 28 June 2011 15:22, Venkateswararao Jujjuri <jv...@linux.vnet.ibm.com>wrote:
> **
> On 06/28/2011 05:25 AM, Sassan Panahinejad wrote:
>
> Hi JV,
>
> Any progress regarding merging this patch (and the fsync patch I
> submitted)?
> Is there anything I can do to assist/speed the process?
>
> Sussan, Thanks for the ping. :)
>
> Everything is on hold waiting for 0.15 tag; Including threading/coroutine
> patches.
> As soon as we go in with the coroutines, you can just rebase your patch and
> we should be
> ready to go. I am waiting too. :-D
>
Thanks for the info :)
>
> Thanks,
> JV
>
>
>
> Thanks
> Sassan
>
>
> On 8 June 2011 17:21, Sassan Panahinejad <sas...@sassan.me.uk> wrote:
>
>> In a lot of cases, the handling of errors was quite ugly.
>> This patch moves reading of errno to immediately after the system calls
>> and passes it up through the system more cleanly.
>> Also, in the case of the xattr functions (and possibly others), completely
>> the wrong error was being returned.
>>
>>
>> This patch is created against your 9p-coroutine-bh branch, as requested.
>> Sorry for the delay, I was unexpectedly required to work abroad for 2 weeks.
>>
>> Signed-off-by: Sassan Panahinejad <sas...@sassan.me.uk>
>>
>> ---
>> fsdev/file-op-9p.h | 4 +-
>> hw/9pfs/codir.c | 14 +----
>> hw/9pfs/virtio-9p-local.c | 123
>> +++++++++++++++++++++++++--------------------
>> hw/9pfs/virtio-9p-xattr.c | 21 +++++++-
>> 4 files changed, 90 insertions(+), 72 deletions(-)
>>
>> diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
>> index af9daf7..3d9575b 100644
>> --- a/fsdev/file-op-9p.h
>> +++ b/fsdev/file-op-9p.h
>> @@ -73,12 +73,12 @@ typedef struct FileOperations
>> int (*setuid)(FsContext *, uid_t);
>> int (*close)(FsContext *, int);
>> int (*closedir)(FsContext *, DIR *);
>> - DIR *(*opendir)(FsContext *, const char *);
>> + int (*opendir)(FsContext *, const char *, DIR **);
>> int (*open)(FsContext *, const char *, int);
>> int (*open2)(FsContext *, const char *, int, FsCred *);
>> void (*rewinddir)(FsContext *, DIR *);
>> off_t (*telldir)(FsContext *, DIR *);
>> - struct dirent *(*readdir)(FsContext *, DIR *);
>> + int (*readdir)(FsContext *, DIR *, struct dirent **);
>> void (*seekdir)(FsContext *, DIR *, off_t);
>> ssize_t (*preadv)(FsContext *, int, const struct iovec *, int, off_t);
>> ssize_t (*pwritev)(FsContext *, int, const struct iovec *, int,
>> off_t);
>> diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c
>> index 110289f..acbbb39 100644
>> --- a/hw/9pfs/codir.c
>> +++ b/hw/9pfs/codir.c
>> @@ -25,12 +25,7 @@ int v9fs_co_readdir(V9fsState *s, V9fsFidState *fidp,
>> struct dirent **dent)
>> {
>> errno = 0;
>> /*FIXME!! need to switch to readdir_r */
>> - *dent = s->ops->readdir(&s->ctx, fidp->fs.dir);
>> - if (!*dent && errno) {
>> - err = -errno;
>> - } else {
>> - err = 0;
>> - }
>> + err = s->ops->readdir(&s->ctx, fidp->fs.dir, dent);
>> });
>> return err;
>> }
>> @@ -93,12 +88,7 @@ int v9fs_co_opendir(V9fsState *s, V9fsFidState *fidp)
>>
>> v9fs_co_run_in_worker(
>> {
>> - dir = s->ops->opendir(&s->ctx, fidp->path.data);
>> - if (!dir) {
>> - err = -errno;
>> - } else {
>> - err = 0;
>> - }
>> + err = s->ops->opendir(&s->ctx, fidp->path.data, &dir);
>> });
>> fidp->fs.dir = dir;
>> return err;
>> diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
>> index 77904c3..65f35eb 100644
>> --- a/hw/9pfs/virtio-9p-local.c
>> +++ b/hw/9pfs/virtio-9p-local.c
>> @@ -28,7 +28,7 @@ static int local_lstat(FsContext *fs_ctx, const char
>> *path, struct stat *stbuf)
>> char buffer[PATH_MAX];
>> err = lstat(rpath(fs_ctx, path, buffer), stbuf);
>> if (err) {
>> - return err;
>> + return -errno;
>> }
>> if (fs_ctx->fs_sm == SM_MAPPED) {
>> /* Actual credentials are part of extended attrs */
>> @@ -53,7 +53,7 @@ static int local_lstat(FsContext *fs_ctx, const char
>> *path, struct stat *stbuf)
>> stbuf->st_rdev = tmp_dev;
>> }
>> }
>> - return err;
>> + return 0;
>> }
>>
>> static int local_set_xattr(const char *path, FsCred *credp)
>> @@ -63,28 +63,28 @@ static int local_set_xattr(const char *path, FsCred
>> *credp)
>> err = setxattr(path, "user.virtfs.uid", &credp->fc_uid,
>> sizeof(uid_t),
>> 0);
>> if (err) {
>> - return err;
>> + return -errno;
>> }
>> }
>> if (credp->fc_gid != -1) {
>> err = setxattr(path, "user.virtfs.gid", &credp->fc_gid,
>> sizeof(gid_t),
>> 0);
>> if (err) {
>> - return err;
>> + return -errno;
>> }
>> }
>> if (credp->fc_mode != -1) {
>> err = setxattr(path, "user.virtfs.mode", &credp->fc_mode,
>> sizeof(mode_t), 0);
>> if (err) {
>> - return err;
>> + return -errno;
>> }
>> }
>> if (credp->fc_rdev != -1) {
>> err = setxattr(path, "user.virtfs.rdev", &credp->fc_rdev,
>> sizeof(dev_t), 0);
>> if (err) {
>> - return err;
>> + return -errno;
>> }
>> }
>> return 0;
>> @@ -95,7 +95,7 @@ static int local_post_create_passthrough(FsContext
>> *fs_ctx, const char *path,
>> {
>> char buffer[PATH_MAX];
>> if (chmod(rpath(fs_ctx, path, buffer), credp->fc_mode & 07777) < 0) {
>> - return -1;
>> + return -errno;
>> }
>> if (lchown(rpath(fs_ctx, path, buffer), credp->fc_uid,
>> credp->fc_gid) < 0) {
>> @@ -104,7 +104,7 @@ static int local_post_create_passthrough(FsContext
>> *fs_ctx, const char *path,
>> * using security model none. Ignore the error
>> */
>> if (fs_ctx->fs_sm != SM_NONE) {
>> - return -1;
>> + return -errno;
>> }
>> }
>> return 0;
>> @@ -119,40 +119,42 @@ static ssize_t local_readlink(FsContext *fs_ctx,
>> const char *path,
>> int fd;
>> fd = open(rpath(fs_ctx, path, buffer), O_RDONLY);
>> if (fd == -1) {
>> - return -1;
>> + return -errno;
>> }
>> do {
>> tsize = read(fd, (void *)buf, bufsz);
>> } while (tsize == -1 && errno == EINTR);
>> close(fd);
>> - return tsize;
>> + return tsize == -1 ? -errno : tsize;
>> } else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
>> (fs_ctx->fs_sm == SM_NONE)) {
>> tsize = readlink(rpath(fs_ctx, path, buffer), buf, bufsz);
>> }
>> - return tsize;
>> + return tsize == -1 ? -errno : tsize;
>> }
>>
>> static int local_close(FsContext *ctx, int fd)
>> {
>> - return close(fd);
>> + return close(fd) == -1 ? -errno : 0;
>> }
>>
>> static int local_closedir(FsContext *ctx, DIR *dir)
>> {
>> - return closedir(dir);
>> + return closedir(dir) == -1 ? -errno : 0;
>> }
>>
>> static int local_open(FsContext *ctx, const char *path, int flags)
>> {
>> char buffer[PATH_MAX];
>> - return open(rpath(ctx, path, buffer), flags);
>> + int ret = open(rpath(ctx, path, buffer), flags);
>> + return ret == -1 ? -errno : ret;
>> }
>>
>> -static DIR *local_opendir(FsContext *ctx, const char *path)
>> +static int local_opendir(FsContext *ctx, const char *path, DIR **dir)
>> {
>> char buffer[PATH_MAX];
>> - return opendir(rpath(ctx, path, buffer));
>> + *dir = opendir(rpath(ctx, path, buffer));
>> + return *dir ? 0 : -errno;
>> }
>>
>> static void local_rewinddir(FsContext *ctx, DIR *dir)
>> @@ -162,12 +164,15 @@ static void local_rewinddir(FsContext *ctx, DIR
>> *dir)
>>
>> static off_t local_telldir(FsContext *ctx, DIR *dir)
>> {
>> - return telldir(dir);
>> + int ret = telldir(dir);
>> + return ret == -1 ? -errno : ret;
>> }
>>
>> -static struct dirent *local_readdir(FsContext *ctx, DIR *dir)
>> +static int local_readdir(FsContext *ctx, DIR *dir, struct dirent
>> **dirent)
>> {
>> - return readdir(dir);
>> + *dirent = readdir(dir);
>> + return *dirent ? 0 : -errno;
>> +
>> }
>>
>> static void local_seekdir(FsContext *ctx, DIR *dir, off_t off)
>> @@ -178,14 +183,17 @@ static void local_seekdir(FsContext *ctx, DIR *dir,
>> off_t off)
>> static ssize_t local_preadv(FsContext *ctx, int fd, const struct iovec
>> *iov,
>> int iovcnt, off_t offset)
>> {
>> + int err;
>> #ifdef CONFIG_PREADV
>> - return preadv(fd, iov, iovcnt, offset);
>> + err = preadv(fd, iov, iovcnt, offset);
>> + return err == -1 ? -errno : err;
>> #else
>> int err = lseek(fd, offset, SEEK_SET);
>> if (err == -1) {
>> - return err;
>> + return -errno;
>> } else {
>> - return readv(fd, iov, iovcnt);
>> + err = readv(fd, iov, iovcnt);
>> + return err == -1 ? -errno : err;
>> }
>> #endif
>> }
>> @@ -193,14 +201,17 @@ static ssize_t local_preadv(FsContext *ctx, int fd,
>> const struct iovec *iov,
>> static ssize_t local_pwritev(FsContext *ctx, int fd, const struct iovec
>> *iov,
>> int iovcnt, off_t offset)
>> {
>> + int err;
>> #ifdef CONFIG_PREADV
>> - return pwritev(fd, iov, iovcnt, offset);
>> + err = pwritev(fd, iov, iovcnt, offset);
>> + return err == -1 ? -errno : err;
>> #else
>> int err = lseek(fd, offset, SEEK_SET);
>> if (err == -1) {
>> - return err;
>> + return -errno;
>> } else {
>> - return writev(fd, iov, iovcnt);
>> + err = writev(fd, iov, iovcnt);
>> + return err == -1 ? -errno : err;
>> }
>> #endif
>> }
>> @@ -208,13 +219,16 @@ static ssize_t local_pwritev(FsContext *ctx, int
>> fd, const struct iovec *iov,
>> static int local_chmod(FsContext *fs_ctx, const char *path, FsCred
>> *credp)
>> {
>> char buffer[PATH_MAX];
>> + int ret;
>> +
>> if (fs_ctx->fs_sm == SM_MAPPED) {
>> return local_set_xattr(rpath(fs_ctx, path, buffer), credp);
>> } else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
>> (fs_ctx->fs_sm == SM_NONE)) {
>> - return chmod(rpath(fs_ctx, path, buffer), credp->fc_mode);
>> + ret = chmod(rpath(fs_ctx, path, buffer), credp->fc_mode);
>> + return ret == -1 ? -errno : ret;
>> }
>> - return -1;
>> + return -ENOTSUP;
>> }
>>
>> static int local_mknod(FsContext *fs_ctx, const char *path, FsCred
>> *credp)
>> @@ -228,7 +242,7 @@ static int local_mknod(FsContext *fs_ctx, const char
>> *path, FsCred *credp)
>> err = mknod(rpath(fs_ctx, path, buffer),
>> SM_LOCAL_MODE_BITS|S_IFREG, 0);
>> if (err == -1) {
>> - return err;
>> + return -errno;
>> }
>> local_set_xattr(rpath(fs_ctx, path, buffer), credp);
>> if (err == -1) {
>> @@ -240,7 +254,7 @@ static int local_mknod(FsContext *fs_ctx, const char
>> *path, FsCred *credp)
>> err = mknod(rpath(fs_ctx, path, buffer), credp->fc_mode,
>> credp->fc_rdev);
>> if (err == -1) {
>> - return err;
>> + return -errno;
>> }
>> err = local_post_create_passthrough(fs_ctx, path, credp);
>> if (err == -1) {
>> @@ -248,12 +262,12 @@ static int local_mknod(FsContext *fs_ctx, const
>> char *path, FsCred *credp)
>> goto err_end;
>> }
>> }
>> - return err;
>> + return 0;
>>
>> err_end:
>> remove(rpath(fs_ctx, path, buffer));
>> errno = serrno;
>> - return err;
>> + return -errno;
>> }
>>
>> static int local_mkdir(FsContext *fs_ctx, const char *path, FsCred
>> *credp)
>> @@ -266,7 +280,7 @@ static int local_mkdir(FsContext *fs_ctx, const char
>> *path, FsCred *credp)
>> if (fs_ctx->fs_sm == SM_MAPPED) {
>> err = mkdir(rpath(fs_ctx, path, buffer), SM_LOCAL_DIR_MODE_BITS);
>> if (err == -1) {
>> - return err;
>> + return -errno;
>> }
>> credp->fc_mode = credp->fc_mode|S_IFDIR;
>> err = local_set_xattr(rpath(fs_ctx, path, buffer), credp);
>> @@ -278,7 +292,7 @@ static int local_mkdir(FsContext *fs_ctx, const char
>> *path, FsCred *credp)
>> (fs_ctx->fs_sm == SM_NONE)) {
>> err = mkdir(rpath(fs_ctx, path, buffer), credp->fc_mode);
>> if (err == -1) {
>> - return err;
>> + return -errno;
>> }
>> err = local_post_create_passthrough(fs_ctx, path, credp);
>> if (err == -1) {
>> @@ -286,12 +300,12 @@ static int local_mkdir(FsContext *fs_ctx, const
>> char *path, FsCred *credp)
>> goto err_end;
>> }
>> }
>> - return err;
>> + return 0;
>>
>> err_end:
>> remove(rpath(fs_ctx, path, buffer));
>> errno = serrno;
>> - return err;
>> + return -errno;
>> }
>>
>> static int local_fstat(FsContext *fs_ctx, int fd, struct stat *stbuf)
>> @@ -299,7 +313,7 @@ static int local_fstat(FsContext *fs_ctx, int fd,
>> struct stat *stbuf)
>> int err;
>> err = fstat(fd, stbuf);
>> if (err) {
>> - return err;
>> + return -errno;
>> }
>> if (fs_ctx->fs_sm == SM_MAPPED) {
>> /* Actual credentials are part of extended attrs */
>> @@ -321,7 +335,7 @@ static int local_fstat(FsContext *fs_ctx, int fd,
>> struct stat *stbuf)
>> stbuf->st_rdev = tmp_dev;
>> }
>> }
>> - return err;
>> + return 0;
>> }
>>
>> static int local_open2(FsContext *fs_ctx, const char *path, int flags,
>> @@ -336,7 +350,7 @@ static int local_open2(FsContext *fs_ctx, const char
>> *path, int flags,
>> if (fs_ctx->fs_sm == SM_MAPPED) {
>> fd = open(rpath(fs_ctx, path, buffer), flags,
>> SM_LOCAL_MODE_BITS);
>> if (fd == -1) {
>> - return fd;
>> + return -errno;
>> }
>> credp->fc_mode = credp->fc_mode|S_IFREG;
>> /* Set cleint credentials in xattr */
>> @@ -349,7 +363,7 @@ static int local_open2(FsContext *fs_ctx, const char
>> *path, int flags,
>> (fs_ctx->fs_sm == SM_NONE)) {
>> fd = open(rpath(fs_ctx, path, buffer), flags, credp->fc_mode);
>> if (fd == -1) {
>> - return fd;
>> + return -errno;
>> }
>> err = local_post_create_passthrough(fs_ctx, path, credp);
>> if (err == -1) {
>> @@ -363,7 +377,7 @@ err_end:
>> close(fd);
>> remove(rpath(fs_ctx, path, buffer));
>> errno = serrno;
>> - return err;
>> + return -errno;
>> }
>>
>>
>> @@ -381,7 +395,7 @@ static int local_symlink(FsContext *fs_ctx, const
>> char *oldpath,
>> fd = open(rpath(fs_ctx, newpath, buffer), O_CREAT|O_EXCL|O_RDWR,
>> SM_LOCAL_MODE_BITS);
>> if (fd == -1) {
>> - return fd;
>> + return -errno;
>> }
>> /* Write the oldpath (target) to the file. */
>> oldpath_size = strlen(oldpath);
>> @@ -407,7 +421,7 @@ static int local_symlink(FsContext *fs_ctx, const
>> char *oldpath,
>> (fs_ctx->fs_sm == SM_NONE)) {
>> err = symlink(oldpath, rpath(fs_ctx, newpath, buffer));
>> if (err) {
>> - return err;
>> + return -errno;
>> }
>> err = lchown(rpath(fs_ctx, newpath, buffer), credp->fc_uid,
>> credp->fc_gid);
>> @@ -423,25 +437,24 @@ static int local_symlink(FsContext *fs_ctx, const
>> char *oldpath,
>> err = 0;
>> }
>> }
>> - return err;
>> + return 0;
>>
>> err_end:
>> remove(rpath(fs_ctx, newpath, buffer));
>> errno = serrno;
>> - return err;
>> + return -errno;
>> }
>>
>> static int local_link(FsContext *ctx, const char *oldpath, const char
>> *newpath)
>> {
>> char buffer[PATH_MAX], buffer1[PATH_MAX];
>> -
>> - return link(rpath(ctx, oldpath, buffer), rpath(ctx, newpath,
>> buffer1));
>> + return link(rpath(ctx, oldpath, buffer), rpath(ctx, newpath,
>> buffer1)) == -1 ? -errno : 0;
>> }
>>
>> static int local_truncate(FsContext *ctx, const char *path, off_t size)
>> {
>> char buffer[PATH_MAX];
>> - return truncate(rpath(ctx, path, buffer), size);
>> + return truncate(rpath(ctx, path, buffer), size) == -1 ? -errno : 0;
>> }
>>
>> static int local_rename(FsContext *ctx, const char *oldpath,
>> @@ -449,7 +462,7 @@ static int local_rename(FsContext *ctx, const char
>> *oldpath,
>> {
>> char buffer[PATH_MAX], buffer1[PATH_MAX];
>>
>> - return rename(rpath(ctx, oldpath, buffer), rpath(ctx, newpath,
>> buffer1));
>> + return rename(rpath(ctx, oldpath, buffer), rpath(ctx, newpath,
>> buffer1)) == -1 ? -errno : 0;
>> }
>>
>> static int local_chown(FsContext *fs_ctx, const char *path, FsCred
>> *credp)
>> @@ -458,15 +471,15 @@ static int local_chown(FsContext *fs_ctx, const
>> char *path, FsCred *credp)
>> if ((credp->fc_uid == -1 && credp->fc_gid == -1) ||
>> (fs_ctx->fs_sm == SM_PASSTHROUGH)) {
>> return lchown(rpath(fs_ctx, path, buffer), credp->fc_uid,
>> - credp->fc_gid);
>> + credp->fc_gid) == -1 ? -errno : 0;
>> } else if (fs_ctx->fs_sm == SM_MAPPED) {
>> return local_set_xattr(rpath(fs_ctx, path, buffer), credp);
>> } else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
>> (fs_ctx->fs_sm == SM_NONE)) {
>> return lchown(rpath(fs_ctx, path, buffer), credp->fc_uid,
>> - credp->fc_gid);
>> + credp->fc_gid) == -1 ? -errno : 0;
>> }
>> - return -1;
>> + return -EINVAL;
>> }
>>
>> static int local_utimensat(FsContext *s, const char *path,
>> @@ -480,22 +493,22 @@ static int local_utimensat(FsContext *s, const char
>> *path,
>> static int local_remove(FsContext *ctx, const char *path)
>> {
>> char buffer[PATH_MAX];
>> - return remove(rpath(ctx, path, buffer));
>> + return remove(rpath(ctx, path, buffer)) == -1 ? -errno : 0;
>> }
>>
>> static int local_fsync(FsContext *ctx, int fd, int datasync)
>> {
>> if (datasync) {
>> - return qemu_fdatasync(fd);
>> + return qemu_fdatasync(fd) == -1 ? -errno : 0;
>> } else {
>> - return fsync(fd);
>> + return fsync(fd) == -1 ? -errno : 0;
>> }
>> }
>>
>> static int local_statfs(FsContext *s, const char *path, struct statfs
>> *stbuf)
>> {
>> char buffer[PATH_MAX];
>> - return statfs(rpath(s, path, buffer), stbuf);
>> + return statfs(rpath(s, path, buffer), stbuf) == -1 ? -errno : 0;
>> }
>>
>> static ssize_t local_lgetxattr(FsContext *ctx, const char *path,
>> diff --git a/hw/9pfs/virtio-9p-xattr.c b/hw/9pfs/virtio-9p-xattr.c
>> index bde0b7f..a5a6134 100644
>> --- a/hw/9pfs/virtio-9p-xattr.c
>> +++ b/hw/9pfs/virtio-9p-xattr.c
>> @@ -32,9 +32,14 @@ static XattrOperations
>> *get_xattr_operations(XattrOperations **h,
>> ssize_t v9fs_get_xattr(FsContext *ctx, const char *path,
>> const char *name, void *value, size_t size)
>> {
>> + int ret;
>> XattrOperations *xops = get_xattr_operations(ctx->xops, name);
>> if (xops) {
>> - return xops->getxattr(ctx, path, name, value, size);
>> + ret = xops->getxattr(ctx, path, name, value, size);
>> + if (ret < 0) {
>> + return -errno;
>> + }
>> + return ret;
>> }
>> errno = -EOPNOTSUPP;
>> return -1;
>> @@ -118,9 +123,14 @@ err_out:
>> int v9fs_set_xattr(FsContext *ctx, const char *path, const char *name,
>> void *value, size_t size, int flags)
>> {
>> + int ret;
>> XattrOperations *xops = get_xattr_operations(ctx->xops, name);
>> if (xops) {
>> - return xops->setxattr(ctx, path, name, value, size, flags);
>> + ret = xops->setxattr(ctx, path, name, value, size, flags);
>> + if (ret < 0) {
>> + return -errno;
>> + }
>> + return ret;
>> }
>> errno = -EOPNOTSUPP;
>> return -1;
>> @@ -130,9 +140,14 @@ int v9fs_set_xattr(FsContext *ctx, const char *path,
>> const char *name,
>> int v9fs_remove_xattr(FsContext *ctx,
>> const char *path, const char *name)
>> {
>> + int ret;
>> XattrOperations *xops = get_xattr_operations(ctx->xops, name);
>> if (xops) {
>> - return xops->removexattr(ctx, path, name);
>> + ret = xops->removexattr(ctx, path, name);
>> + if (ret < 0) {
>> + return -errno;
>> + }
>> + return ret;
>> }
>> errno = -EOPNOTSUPP;
>> return -1;
>> --
>> 1.7.4.1
>>
>>
>
>
