From: Sassan Panahinejad <sassan@sassan.me.uk>
To: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] Clean up virtio-9p error handling code
Date: Wed, 18 May 2011 08:37:57 +0100 [thread overview]
Message-ID: <BANLkTikhKnCeJamBUighZc8uDW=9V-V+6g@mail.gmail.com> (raw)
In-Reply-To: <4DD2D1E6.7030300@linux.vnet.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 38238 bytes --]
On 17 May 2011 20:52, Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>wrote:
> On 05/15/2011 09:43 AM, Sassan Panahinejad 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.
>>
>
> Hi, This portion of the code is going through heavy change as part of
> asynchronous threading work. You can get access to the new in-development
> repo here.
>
>
> http://repo.or.cz/w/qemu/aliguori/jvrao.git/shortlog/refs/heads/9p-coroutine-bh
>
> I would suggest you to redo your patch to the above repo to make the patch
> ready for the upstream.
>
>
Thanks, I will aim to do so over the weekend.
Sassan
> Thanks
> Jv
>
>
> Signed-off-by: Sassan Panahinejad<sassan@sassan.me.uk>
>> ---
>> fsdev/file-op-9p.h | 4 +-
>> hw/9pfs/virtio-9p-local.c | 123 +++++++++++++++++++++----------------
>> hw/9pfs/virtio-9p-xattr.c | 21 ++++++-
>> hw/9pfs/virtio-9p.c | 150
>> ++++++++++++---------------------------------
>> 4 files changed, 128 insertions(+), 170 deletions(-)
>>
>> diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
>> index 126e60e..81a822a 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/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
>> index 0a015de..de2597d 100644
>> --- a/hw/9pfs/virtio-9p-local.c
>> +++ b/hw/9pfs/virtio-9p-local.c
>> @@ -26,7 +26,7 @@ static int local_lstat(FsContext *fs_ctx, const char
>> *path, struct stat *stbuf)
>> int err;
>> err = lstat(rpath(fs_ctx, path), stbuf);
>> if (err) {
>> - return err;
>> + return -errno;
>> }
>> if (fs_ctx->fs_sm == SM_MAPPED) {
>> /* Actual credentials are part of extended attrs */
>> @@ -51,7 +51,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)
>> @@ -61,28 +61,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;
>> @@ -92,7 +92,7 @@ static int local_post_create_passthrough(FsContext
>> *fs_ctx, const char *path,
>> FsCred *credp)
>> {
>> if (chmod(rpath(fs_ctx, path), credp->fc_mode& 07777)< 0) {
>> - return -1;
>> + return -errno;
>> }
>> if (lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid)< 0) {
>> /*
>> @@ -100,7 +100,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;
>> @@ -114,38 +114,40 @@ static ssize_t local_readlink(FsContext *fs_ctx,
>> const char *path,
>> int fd;
>> fd = open(rpath(fs_ctx, path), 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), 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)
>> {
>> - return open(rpath(ctx, path), flags);
>> + int ret = open(rpath(ctx, path), 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)
>> {
>> - return opendir(rpath(ctx, path));
>> + *dir = opendir(rpath(ctx, path));
>> + return *dir ? 0 : -errno;
>> }
>>
>> static void local_rewinddir(FsContext *ctx, DIR *dir)
>> @@ -155,12 +157,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)
>> @@ -171,14 +176,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
>> }
>> @@ -186,27 +194,32 @@ 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
>> }
>>
>> static int local_chmod(FsContext *fs_ctx, const char *path, FsCred
>> *credp)
>> {
>> + int ret;
>> if (fs_ctx->fs_sm == SM_MAPPED) {
>> return local_set_xattr(rpath(fs_ctx, path), credp);
>> } else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
>> (fs_ctx->fs_sm == SM_NONE)) {
>> - return chmod(rpath(fs_ctx, path), credp->fc_mode);
>> + ret = chmod(rpath(fs_ctx, path), credp->fc_mode);
>> + return ret == -1 ? -errno : ret;
>> }
>> - return -1;
>> + return -ENOTSUP;
>> }
>>
>> static int local_mknod(FsContext *fs_ctx, const char *path, FsCred
>> *credp)
>> @@ -218,7 +231,7 @@ static int local_mknod(FsContext *fs_ctx, const char
>> *path, FsCred *credp)
>> if (fs_ctx->fs_sm == SM_MAPPED) {
>> err = mknod(rpath(fs_ctx, path), SM_LOCAL_MODE_BITS|S_IFREG, 0);
>> if (err == -1) {
>> - return err;
>> + return -errno;
>> }
>> local_set_xattr(rpath(fs_ctx, path), credp);
>> if (err == -1) {
>> @@ -229,7 +242,7 @@ static int local_mknod(FsContext *fs_ctx, const char
>> *path, FsCred *credp)
>> (fs_ctx->fs_sm == SM_NONE)) {
>> err = mknod(rpath(fs_ctx, path), 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) {
>> @@ -237,12 +250,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));
>> errno = serrno;
>> - return err;
>> + return -errno;
>> }
>>
>> static int local_mkdir(FsContext *fs_ctx, const char *path, FsCred
>> *credp)
>> @@ -254,7 +267,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), 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), credp);
>> @@ -266,7 +279,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), credp->fc_mode);
>> if (err == -1) {
>> - return err;
>> + return -errno;
>> }
>> err = local_post_create_passthrough(fs_ctx, path, credp);
>> if (err == -1) {
>> @@ -274,12 +287,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));
>> errno = serrno;
>> - return err;
>> + return -errno;
>> }
>>
>> static int local_fstat(FsContext *fs_ctx, int fd, struct stat *stbuf)
>> @@ -287,7 +300,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 */
>> @@ -309,7 +322,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,
>> @@ -323,7 +336,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), 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 */
>> @@ -336,7 +349,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), flags, credp->fc_mode);
>> if (fd == -1) {
>> - return fd;
>> + return -errno;
>> }
>> err = local_post_create_passthrough(fs_ctx, path, credp);
>> if (err == -1) {
>> @@ -350,7 +363,7 @@ err_end:
>> close(fd);
>> remove(rpath(fs_ctx, path));
>> errno = serrno;
>> - return err;
>> + return -errno;
>> }
>>
>>
>> @@ -367,7 +380,7 @@ static int local_symlink(FsContext *fs_ctx, const char
>> *oldpath,
>> fd = open(rpath(fs_ctx, newpath), 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);
>> @@ -393,7 +406,7 @@ static int local_symlink(FsContext *fs_ctx, const char
>> *oldpath,
>> (fs_ctx->fs_sm == SM_NONE)) {
>> err = symlink(oldpath, rpath(fs_ctx, newpath));
>> if (err) {
>> - return err;
>> + return -errno;
>> }
>> err = lchown(rpath(fs_ctx, newpath), credp->fc_uid,
>> credp->fc_gid);
>> if (err == -1) {
>> @@ -408,12 +421,12 @@ static int local_symlink(FsContext *fs_ctx, const
>> char *oldpath,
>> err = 0;
>> }
>> }
>> - return err;
>> + return 0;
>>
>> err_end:
>> remove(rpath(fs_ctx, newpath));
>> errno = serrno;
>> - return err;
>> + return -errno;
>> }
>>
>> static int local_link(FsContext *ctx, const char *oldpath, const char
>> *newpath)
>> @@ -436,12 +449,12 @@ static int local_link(FsContext *ctx, const char
>> *oldpath, const char *newpath)
>> errno = serrno;
>> }
>>
>> - return err;
>> + return err == -1 ? -errno : err;
>> }
>>
>> static int local_truncate(FsContext *ctx, const char *path, off_t size)
>> {
>> - return truncate(rpath(ctx, path), size);
>> + return truncate(rpath(ctx, path), size) == -1 ? -errno : 0;
>> }
>>
>> static int local_rename(FsContext *ctx, const char *oldpath,
>> @@ -461,7 +474,7 @@ static int local_rename(FsContext *ctx, const char
>> *oldpath,
>> qemu_free(tmp);
>> }
>>
>> - return err;
>> + return err == -1 ? -errno : err;
>>
>> }
>>
>> @@ -469,14 +482,16 @@ 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), credp->fc_uid, credp->fc_gid);
>> + return lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid)
>> == -1
>> + ? -errno : 0;
>> } else if (fs_ctx->fs_sm == SM_MAPPED) {
>> return local_set_xattr(rpath(fs_ctx, path), credp);
>> } else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
>> (fs_ctx->fs_sm == SM_NONE)) {
>> - return lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid);
>> + return lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid)
>> == -1
>> + ? -errno : 0;
>> }
>> - return -1;
>> + return -EINVAL;
>> }
>>
>> static int local_utimensat(FsContext *s, const char *path,
>> @@ -487,21 +502,21 @@ static int local_utimensat(FsContext *s, const char
>> *path,
>>
>> static int local_remove(FsContext *ctx, const char *path)
>> {
>> - return remove(rpath(ctx, path));
>> + return remove(rpath(ctx, path)) == -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)
>> {
>> - return statfs(rpath(s, path), stbuf);
>> + return statfs(rpath(s, path), 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 03c3d3f..b37ea44 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;
>> @@ -117,9 +122,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;
>> @@ -129,9 +139,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;
>> diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
>> index 9d72021..fb73f7d 100644
>> --- a/hw/9pfs/virtio-9p.c
>> +++ b/hw/9pfs/virtio-9p.c
>> @@ -110,9 +110,9 @@ static int v9fs_do_open(V9fsState *s, V9fsString
>> *path, int flags)
>> return s->ops->open(&s->ctx, path->data, flags);
>> }
>>
>> -static DIR *v9fs_do_opendir(V9fsState *s, V9fsString *path)
>> +static int v9fs_do_opendir(V9fsState *s, V9fsString *path, DIR **dir)
>> {
>> - return s->ops->opendir(&s->ctx, path->data);
>> + return s->ops->opendir(&s->ctx, path->data, dir);
>> }
>>
>> static void v9fs_do_rewinddir(V9fsState *s, DIR *dir)
>> @@ -125,9 +125,9 @@ static off_t v9fs_do_telldir(V9fsState *s, DIR *dir)
>> return s->ops->telldir(&s->ctx, dir);
>> }
>>
>> -static struct dirent *v9fs_do_readdir(V9fsState *s, DIR *dir)
>> +static int v9fs_do_readdir(V9fsState *s, DIR *dir, struct dirent
>> **dirent)
>> {
>> - return s->ops->readdir(&s->ctx, dir);
>> + return s->ops->readdir(&s->ctx, dir, dirent);
>> }
>>
>> static void v9fs_do_seekdir(V9fsState *s, DIR *dir, off_t off)
>> @@ -1038,8 +1038,7 @@ static int stat_to_v9stat(V9fsState *s, V9fsString
>> *name,
>>
>> if (v9stat->mode& P9_STAT_MODE_SYMLINK) {
>> err = v9fs_do_readlink(s, name,&v9stat->extension);
>> - if (err == -1) {
>> - err = -errno;
>> + if (err< 0) {
>> return err;
>> }
>> v9stat->extension.data[err] = 0;
>> @@ -1234,8 +1233,7 @@ out:
>>
>> static void v9fs_stat_post_lstat(V9fsState *s, V9fsStatState *vs, int
>> err)
>> {
>> - if (err == -1) {
>> - err = -errno;
>> + if (err< 0) {
>> goto out;
>> }
>>
>> @@ -1263,7 +1261,6 @@ static void v9fs_stat(V9fsState *s, V9fsPDU *pdu)
>> vs->offset = 7;
>>
>> memset(&vs->v9stat, 0, sizeof(vs->v9stat));
>> -
>> pdu_unmarshal(vs->pdu, vs->offset, "d",&fid);
>>
>> vs->fidp = lookup_fid(s, fid);
>> @@ -1285,8 +1282,7 @@ out:
>> static void v9fs_getattr_post_lstat(V9fsState *s, V9fsStatStateDotl *vs,
>> int err)
>> {
>> - if (err == -1) {
>> - err = -errno;
>> + if (err< 0) {
>> goto out;
>> }
>>
>> @@ -1348,8 +1344,7 @@ out:
>> static void v9fs_setattr_post_truncate(V9fsState *s, V9fsSetattrState
>> *vs,
>> int
>> err)
>> {
>> - if (err == -1) {
>> - err = -errno;
>> + if (err< 0) {
>> goto out;
>> }
>> err = vs->offset;
>> @@ -1361,8 +1356,7 @@ out:
>>
>> static void v9fs_setattr_post_chown(V9fsState *s, V9fsSetattrState *vs,
>> int err)
>> {
>> - if (err == -1) {
>> - err = -errno;
>> + if (err< 0) {
>> goto out;
>> }
>>
>> @@ -1380,8 +1374,7 @@ out:
>> static void v9fs_setattr_post_utimensat(V9fsState *s, V9fsSetattrState
>> *vs,
>> int
>> err)
>> {
>> - if (err == -1) {
>> - err = -errno;
>> + if (err< 0) {
>> goto out;
>> }
>>
>> @@ -1410,8 +1403,7 @@ out:
>>
>> static void v9fs_setattr_post_chmod(V9fsState *s, V9fsSetattrState *vs,
>> int err)
>> {
>> - if (err == -1) {
>> - err = -errno;
>> + if (err< 0) {
>> goto out;
>> }
>>
>> @@ -1506,7 +1498,7 @@ static void v9fs_walk_marshal(V9fsWalkState *vs)
>> static void v9fs_walk_post_newfid_lstat(V9fsState *s, V9fsWalkState *vs,
>> int err)
>> {
>> - if (err == -1) {
>> + if (err< 0) {
>> free_fid(s, vs->newfidp->fid);
>> v9fs_string_free(&vs->path);
>> err = -ENOENT;
>> @@ -1536,7 +1528,7 @@ out:
>> static void v9fs_walk_post_oldfid_lstat(V9fsState *s, V9fsWalkState *vs,
>> int err)
>> {
>> - if (err == -1) {
>> + if (err< 0) {
>> v9fs_string_free(&vs->path);
>> err = -ENOENT;
>> goto out;
>> @@ -1665,8 +1657,7 @@ static int32_t get_iounit(V9fsState *s, V9fsString
>> *name)
>>
>> static void v9fs_open_post_opendir(V9fsState *s, V9fsOpenState *vs, int
>> err)
>> {
>> - if (vs->fidp->fs.dir == NULL) {
>> - err = -errno;
>> + if (err< 0) {
>> goto out;
>> }
>> vs->fidp->fid_type = P9_FID_DIR;
>> @@ -1689,8 +1680,8 @@ static void v9fs_open_post_getiounit(V9fsState *s,
>> V9fsOpenState *vs)
>>
>> static void v9fs_open_post_open(V9fsState *s, V9fsOpenState *vs, int err)
>> {
>> - if (vs->fidp->fs.fd == -1) {
>> - err = -errno;
>> + if (vs->fidp->fs.fd< 0) {
>> + err = vs->fidp->fs.fd;
>> goto out;
>> }
>> vs->fidp->fid_type = P9_FID_FILE;
>> @@ -1707,14 +1698,13 @@ static void v9fs_open_post_lstat(V9fsState *s,
>> V9fsOpenState *vs, int err)
>> int flags;
>>
>> if (err) {
>> - err = -errno;
>> goto out;
>> }
>>
>> stat_to_qid(&vs->stbuf,&vs->qid);
>>
>> if (S_ISDIR(vs->stbuf.st_mode)) {
>> - vs->fidp->fs.dir = v9fs_do_opendir(s,&vs->fidp->path);
>> + err = v9fs_do_opendir(s,&vs->fidp->path,&vs->fidp->fs.dir);
>> v9fs_open_post_opendir(s, vs, err);
>> } else {
>> if (s->proto_version == V9FS_PROTO_2000L) {
>> @@ -1778,7 +1768,6 @@ static void v9fs_post_lcreate(V9fsState *s,
>> V9fsLcreateState *vs, int err)
>> err = vs->offset;
>> } else {
>> vs->fidp->fid_type = P9_FID_NONE;
>> - err = -errno;
>> if (vs->fidp->fs.fd> 0) {
>> close(vs->fidp->fs.fd);
>> }
>> @@ -1794,7 +1783,6 @@ static void v9fs_lcreate_post_get_iounit(V9fsState
>> *s, V9fsLcreateState *vs,
>> int err)
>> {
>> if (err) {
>> - err = -errno;
>> goto out;
>> }
>> err = v9fs_do_lstat(s,&vs->fullname,&vs->stbuf);
>> @@ -1806,8 +1794,8 @@ out:
>> static void v9fs_lcreate_post_do_open2(V9fsState *s, V9fsLcreateState
>> *vs,
>> int err)
>> {
>> - if (vs->fidp->fs.fd == -1) {
>> - err = -errno;
>> + if (vs->fidp->fs.fd< 0) {
>> + err = vs->fidp->fs.fd;
>> goto out;
>> }
>> vs->fidp->fid_type = P9_FID_FILE;
>> @@ -1860,9 +1848,6 @@ out:
>>
>> static void v9fs_post_do_fsync(V9fsState *s, V9fsPDU *pdu, int err)
>> {
>> - if (err == -1) {
>> - err = -errno;
>> - }
>> complete_pdu(s, pdu, err);
>> }
>>
>> @@ -1932,7 +1917,6 @@ static void v9fs_read_post_dir_lstat(V9fsState *s,
>> V9fsReadState *vs,
>> ssize_t err)
>> {
>> if (err) {
>> - err = -errno;
>> goto out;
>> }
>> err = stat_to_v9stat(s,&vs->name,&vs->stbuf,&vs->v9stat);
>> @@ -1952,7 +1936,7 @@ static void v9fs_read_post_dir_lstat(V9fsState *s,
>> V9fsReadState *vs,
>> v9fs_stat_free(&vs->v9stat);
>> v9fs_string_free(&vs->name);
>> vs->dir_pos = vs->dent->d_off;
>> - vs->dent = v9fs_do_readdir(s, vs->fidp->fs.dir);
>> + err = v9fs_do_readdir(s, vs->fidp->fs.dir,&vs->dent);
>> v9fs_read_post_readdir(s, vs, err);
>> return;
>> out:
>> @@ -1984,7 +1968,7 @@ static void v9fs_read_post_readdir(V9fsState *s,
>> V9fsReadState *vs, ssize_t err)
>>
>> static void v9fs_read_post_telldir(V9fsState *s, V9fsReadState *vs,
>> ssize_t err)
>> {
>> - vs->dent = v9fs_do_readdir(s, vs->fidp->fs.dir);
>> + v9fs_do_readdir(s, vs->fidp->fs.dir,&vs->dent);
>> v9fs_read_post_readdir(s, vs, err);
>> return;
>> }
>> @@ -2000,8 +1984,6 @@ static void v9fs_read_post_rewinddir(V9fsState *s,
>> V9fsReadState *vs,
>> static void v9fs_read_post_preadv(V9fsState *s, V9fsReadState *vs,
>> ssize_t err)
>> {
>> if (err< 0) {
>> - /* IO error return the error */
>> - err = -errno;
>> goto out;
>> }
>> vs->total += vs->len;
>> @@ -2017,9 +1999,6 @@ static void v9fs_read_post_preadv(V9fsState *s,
>> V9fsReadState *vs, ssize_t err)
>> vs->off += vs->len;
>> }
>> } while (vs->len == -1&& errno == EINTR);
>> - if (vs->len == -1) {
>> - err = -errno;
>> - }
>> v9fs_read_post_preadv(s, vs, err);
>> return;
>> }
>> @@ -2170,7 +2149,7 @@ static void v9fs_readdir_post_readdir(V9fsState *s,
>> V9fsReadDirState *vs)
>> vs->count += len;
>> v9fs_string_free(&vs->name);
>> vs->saved_dir_pos = vs->dent->d_off;
>> - vs->dent = v9fs_do_readdir(s, vs->fidp->fs.dir);
>> + v9fs_do_readdir(s, vs->fidp->fs.dir,&vs->dent);
>> v9fs_readdir_post_readdir(s, vs);
>> return;
>> }
>> @@ -2184,7 +2163,7 @@ static void v9fs_readdir_post_readdir(V9fsState *s,
>> V9fsReadDirState *vs)
>>
>> static void v9fs_readdir_post_telldir(V9fsState *s, V9fsReadDirState *vs)
>> {
>> - vs->dent = v9fs_do_readdir(s, vs->fidp->fs.dir);
>> + v9fs_do_readdir(s, vs->fidp->fs.dir,&vs->dent);
>> v9fs_readdir_post_readdir(s, vs);
>> return;
>> }
>> @@ -2236,8 +2215,6 @@ static void v9fs_write_post_pwritev(V9fsState *s,
>> V9fsWriteState *vs,
>> ssize_t err)
>> {
>> if (err< 0) {
>> - /* IO error return the error */
>> - err = -errno;
>> goto out;
>> }
>> vs->total += vs->len;
>> @@ -2253,9 +2230,6 @@ static void v9fs_write_post_pwritev(V9fsState *s,
>> V9fsWriteState *vs,
>> vs->off += vs->len;
>> }
>> } while (vs->len == -1&& errno == EINTR);
>> - if (vs->len == -1) {
>> - err = -errno;
>> - }
>> v9fs_write_post_pwritev(s, vs, err);
>> return;
>> }
>> @@ -2394,18 +2368,12 @@ static void v9fs_post_create(V9fsState *s,
>> V9fsCreateState *vs, int err)
>>
>> static void v9fs_create_post_perms(V9fsState *s, V9fsCreateState *vs, int
>> err)
>> {
>> - if (err) {
>> - err = -errno;
>> - }
>> v9fs_post_create(s, vs, err);
>> }
>>
>> static void v9fs_create_post_opendir(V9fsState *s, V9fsCreateState *vs,
>> int
>> err)
>> {
>> - if (!vs->fidp->fs.dir) {
>> - err = -errno;
>> - }
>> vs->fidp->fid_type = P9_FID_DIR;
>> v9fs_post_create(s, vs, err);
>> }
>> @@ -2414,11 +2382,10 @@ static void v9fs_create_post_dir_lstat(V9fsState
>> *s, V9fsCreateState *vs,
>> int
>> err)
>> {
>> if (err) {
>> - err = -errno;
>> goto out;
>> }
>>
>> - vs->fidp->fs.dir = v9fs_do_opendir(s,&vs->fullname);
>> + err = v9fs_do_opendir(s,&vs->fullname,&vs->fidp->fs.dir);
>> v9fs_create_post_opendir(s, vs, err);
>> return;
>>
>> @@ -2429,7 +2396,6 @@ out:
>> static void v9fs_create_post_mkdir(V9fsState *s, V9fsCreateState *vs, int
>> err)
>> {
>> if (err) {
>> - err = -errno;
>> goto out;
>> }
>>
>> @@ -2446,7 +2412,6 @@ static void v9fs_create_post_fstat(V9fsState *s,
>> V9fsCreateState *vs, int err)
>> if (err) {
>> vs->fidp->fid_type = P9_FID_NONE;
>> close(vs->fidp->fs.fd);
>> - err = -errno;
>> }
>> v9fs_post_create(s, vs, err);
>> return;
>> @@ -2455,7 +2420,6 @@ static void v9fs_create_post_fstat(V9fsState *s,
>> V9fsCreateState *vs, int err)
>> static void v9fs_create_post_open2(V9fsState *s, V9fsCreateState *vs, int
>> err)
>> {
>> if (vs->fidp->fs.fd == -1) {
>> - err = -errno;
>> goto out;
>> }
>> vs->fidp->fid_type = P9_FID_FILE;
>> @@ -2472,8 +2436,7 @@ out:
>> static void v9fs_create_post_lstat(V9fsState *s, V9fsCreateState *vs, int
>> err)
>> {
>>
>> - if (err == 0 || errno != ENOENT) {
>> - err = -errno;
>> + if (err == 0 || err != -ENOENT) {
>> goto out;
>> }
>>
>> @@ -2501,7 +2464,6 @@ static void v9fs_create_post_lstat(V9fsState *s,
>> V9fsCreateState *vs, int err)
>>
>> if (sscanf(vs->extension.data, "%c %u %u",&ctype,&major,
>> &minor) != 3) {
>> - err = -errno;
>> v9fs_post_create(s, vs, err);
>> }
>>
>> @@ -2583,8 +2545,6 @@ static void v9fs_post_symlink(V9fsState *s,
>> V9fsSymlinkState *vs, int err)
>> stat_to_qid(&vs->stbuf,&vs->qid);
>> vs->offset += pdu_marshal(vs->pdu, vs->offset, "Q",&vs->qid);
>> err = vs->offset;
>> - } else {
>> - err = -errno;
>> }
>> complete_pdu(s, vs->pdu, err);
>> v9fs_string_free(&vs->name);
>> @@ -2660,22 +2620,17 @@ static void v9fs_link(V9fsState *s, V9fsPDU *pdu)
>>
>> dfidp = lookup_fid(s, dfid);
>> if (dfidp == NULL) {
>> - err = -errno;
>> goto out;
>> }
>>
>> oldfidp = lookup_fid(s, oldfid);
>> if (oldfidp == NULL) {
>> - err = -errno;
>> goto out;
>> }
>>
>> v9fs_string_sprintf(&fullname, "%s/%s", dfidp->path.data, name.data);
>> err = offset;
>> err = v9fs_do_link(s,&oldfidp->path,&fullname);
>> - if (err) {
>> - err = -errno;
>> - }
>> v9fs_string_free(&fullname);
>>
>> out:
>> @@ -2686,9 +2641,7 @@ out:
>> static void v9fs_remove_post_remove(V9fsState *s, V9fsRemoveState *vs,
>> int err)
>> {
>> - if (err< 0) {
>> - err = -errno;
>> - } else {
>> + if (err == 0) {
>> err = vs->offset;
>> }
>>
>> @@ -2746,9 +2699,7 @@ static void v9fs_wstat_post_rename(V9fsState *s,
>> V9fsWstatState *vs, int err)
>> goto out;
>> }
>> if (vs->v9stat.length != -1) {
>> - if (v9fs_do_truncate(s,&vs->fidp->path, vs->v9stat.length)< 0) {
>> - err = -errno;
>> - }
>> + err = v9fs_do_truncate(s,&vs->fidp->path, vs->v9stat.length);
>> }
>> v9fs_wstat_post_truncate(s, vs, err);
>> return;
>> @@ -2800,9 +2751,8 @@ static int v9fs_complete_rename(V9fsState *s,
>> V9fsRenameState *vs)
>> vs->name.size = strlen(new_name);
>>
>> if (strcmp(new_name, vs->fidp->path.data) != 0) {
>> - if (v9fs_do_rename(s,&vs->fidp->path,&vs->name)) {
>> - err = -errno;
>> - } else {
>> + err = v9fs_do_rename(s,&vs->fidp->path,&vs->name);
>> + if (!err) {
>> V9fsFidState *fidp;
>> /*
>> * Fixup fid's pointing to the old name to
>> @@ -2908,10 +2858,8 @@ static void v9fs_wstat_post_utime(V9fsState *s,
>> V9fsWstatState *vs, int err)
>> }
>>
>> if (vs->v9stat.n_gid != -1 || vs->v9stat.n_uid != -1) {
>> - if (v9fs_do_chown(s,&vs->fidp->path, vs->v9stat.n_uid,
>> - vs->v9stat.n_gid)) {
>> - err = -errno;
>> - }
>> + err = v9fs_do_chown(s,&vs->fidp->path, vs->v9stat.n_uid,
>> + vs->v9stat.n_gid);
>> }
>> v9fs_wstat_post_chown(s, vs, err);
>> return;
>> @@ -2943,9 +2891,7 @@ static void v9fs_wstat_post_chmod(V9fsState *s,
>> V9fsWstatState *vs, int err)
>> times[1].tv_nsec = UTIME_OMIT;
>> }
>>
>> - if (v9fs_do_utimensat(s,&vs->fidp->path, times)) {
>> - err = -errno;
>> - }
>> + err = v9fs_do_utimensat(s,&vs->fidp->path, times);
>> }
>>
>> v9fs_wstat_post_utime(s, vs, err);
>> @@ -2959,9 +2905,6 @@ out:
>>
>> static void v9fs_wstat_post_fsync(V9fsState *s, V9fsWstatState *vs, int
>> err)
>> {
>> - if (err == -1) {
>> - err = -errno;
>> - }
>> v9fs_stat_free(&vs->v9stat);
>> complete_pdu(s, vs->pdu, err);
>> qemu_free(vs);
>> @@ -2971,8 +2914,7 @@ static void v9fs_wstat_post_lstat(V9fsState *s,
>> V9fsWstatState *vs, int err)
>> {
>> uint32_t v9_mode;
>>
>> - if (err == -1) {
>> - err = -errno;
>> + if (err< 0) {
>> goto out;
>> }
>>
>> @@ -2985,10 +2927,8 @@ static void v9fs_wstat_post_lstat(V9fsState *s,
>> V9fsWstatState *vs, int err)
>> goto out;
>> }
>>
>> - if (v9fs_do_chmod(s,&vs->fidp->path, v9mode_to_mode(vs->v9stat.mode,
>> -&vs->v9stat.extension))) {
>> - err = -errno;
>> - }
>> + err = v9fs_do_chmod(s,&vs->fidp->path,
>> v9mode_to_mode(vs->v9stat.mode,
>> +&vs->v9stat.extension));
>> v9fs_wstat_post_chmod(s, vs, err);
>> return;
>>
>> @@ -3049,7 +2989,6 @@ static void v9fs_statfs_post_statfs(V9fsState *s,
>> V9fsStatfsState *vs, int err)
>> int32_t bsize_factor;
>>
>> if (err) {
>> - err = -errno;
>> goto out;
>> }
>>
>> @@ -3119,8 +3058,7 @@ out:
>>
>> static void v9fs_mknod_post_lstat(V9fsState *s, V9fsMkState *vs, int err)
>> {
>> - if (err == -1) {
>> - err = -errno;
>> + if (err< 0) {
>> goto out;
>> }
>>
>> @@ -3136,8 +3074,7 @@ out:
>>
>> static void v9fs_mknod_post_mknod(V9fsState *s, V9fsMkState *vs, int err)
>> {
>> - if (err == -1) {
>> - err = -errno;
>> + if (err< 0) {
>> goto out;
>> }
>>
>> @@ -3234,7 +3171,6 @@ static void v9fs_lock(V9fsState *s, V9fsPDU *pdu)
>> goto out;
>> }
>> if (err< 0) {
>> - err = -errno;
>> goto out;
>> }
>> vs->status = P9_LOCK_SUCCESS;
>> @@ -3280,7 +3216,6 @@ static void v9fs_getlock(V9fsState *s, V9fsPDU *pdu)
>> goto out;
>> }
>> if (err< 0) {
>> - err = -errno;
>> goto out;
>> }
>> vs->glock->type = F_UNLCK;
>> @@ -3295,8 +3230,7 @@ out:
>>
>> static void v9fs_mkdir_post_lstat(V9fsState *s, V9fsMkState *vs, int err)
>> {
>> - if (err == -1) {
>> - err = -errno;
>> + if (err< 0) {
>> goto out;
>> }
>>
>> @@ -3312,8 +3246,7 @@ out:
>>
>> static void v9fs_mkdir_post_mkdir(V9fsState *s, V9fsMkState *vs, int err)
>> {
>> - if (err == -1) {
>> - err = -errno;
>> + if (err< 0) {
>> goto out;
>> }
>>
>> @@ -3366,7 +3299,6 @@ static void v9fs_post_xattr_getvalue(V9fsState *s,
>> V9fsXattrState *vs, int err)
>> {
>>
>> if (err< 0) {
>> - err = -errno;
>> free_fid(s, vs->xattr_fidp->fid);
>> goto out;
>> }
>> @@ -3382,7 +3314,6 @@ out:
>> static void v9fs_post_xattr_check(V9fsState *s, V9fsXattrState *vs,
>> ssize_t err)
>> {
>> if (err< 0) {
>> - err = -errno;
>> free_fid(s, vs->xattr_fidp->fid);
>> goto out;
>> }
>> @@ -3410,7 +3341,6 @@ static void v9fs_post_lxattr_getvalue(V9fsState *s,
>> V9fsXattrState *vs, int err)
>> {
>> if (err< 0) {
>> - err = -errno;
>> free_fid(s, vs->xattr_fidp->fid);
>> goto out;
>> }
>> @@ -3427,7 +3357,6 @@ static void v9fs_post_lxattr_check(V9fsState *s,
>> V9fsXattrState *vs, ssize_t err)
>> {
>> if (err< 0) {
>> - err = -errno;
>> free_fid(s, vs->xattr_fidp->fid);
>> goto out;
>> }
>> @@ -3548,7 +3477,6 @@ static void v9fs_readlink_post_readlink(V9fsState
>> *s, V9fsReadLinkState *vs,
>> int err)
>> {
>> if (err< 0) {
>> - err = -errno;
>> goto out;
>> }
>> vs->offset += pdu_marshal(vs->pdu, vs->offset, "s",&vs->target);
>>
>
>
>
[-- Attachment #2: Type: text/html, Size: 43002 bytes --]
next prev parent reply other threads:[~2011-05-18 7:38 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-08 12:36 [Qemu-devel] [PATCH] Clean up virtio-9p error handling code Sassan Panahinejad
2011-05-08 18:34 ` Venkateswararao Jujjuri
2011-05-09 9:21 ` Sassan Panahinejad
2011-05-13 23:59 ` Venkateswararao Jujjuri
2011-05-15 14:59 ` Sassan Panahinejad
2011-05-15 16:43 ` Sassan Panahinejad
2011-05-17 19:52 ` Venkateswararao Jujjuri
2011-05-18 7:37 ` Sassan Panahinejad [this message]
2011-06-08 16:21 ` Sassan Panahinejad
2011-06-28 12:25 ` Sassan Panahinejad
2011-06-28 14:22 ` Venkateswararao Jujjuri
2011-06-28 17:00 ` Sassan Panahinejad
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='BANLkTikhKnCeJamBUighZc8uDW=9V-V+6g@mail.gmail.com' \
--to=sassan@sassan.me.uk \
--cc=jvrao@linux.vnet.ibm.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).