Fix bug with virtio-9p xattr functions return values

On 04/28/2011 06:48 AM, Sassan Panahinejad wrote:
> Several functions in virtio-9p-xattr.c are passing the return value of the associated host system call back to the client instead of errno.
> Since this value is -1 for any error (which, when received in the guest app as errno, indicates "operation not permitted") it is causing guest applications to fail in cases where the operation is not supported by the host.
> This causes a number of problems with dpkg and with install.
> This patch fixes the bug and returns the correct value, which means that guest applications are able to handle the error correctly.
>
> Signed-off-by: Sassan Panahinejad<sassan@sassan.me.uk>
The motivation and direction of this change is correct but unfortunately 
this is little bigger than
what is being identified here. This problem is not just limited to 
xattrs.  Unfortunately this is all over the place.

If you look at the post* functions of say, v9fs_do_lgetxattr( which is 
nothing but local_lgetxattr)
it tries to capture errno.
v9fs_post_xattr_getvalue()
v9fs_post_lxattr_check()
etc.

But this is not always the case say, in v9fs_xattr_fid_clunk()
retval = v9fs_do_lsetxattr(retval = v9fs_do_lsetxattr()).
So the plan is to change this from top to bottom. Starting from the 
virtio-9p.c to the virtio-9p-local.c
to other files and functions.
Just as you did, we need to push the errno to the source of the system 
call and from then on,
just use the function return values populated up. There should not be 
any errno references
other than immediately after the systemcall().  Something like below.
But I will be posting an initial patch which moves all the errno into 
v9fs_do*()
and then into local.c . It will be great if you want to take this all 
the way. :)








> ---
>   hw/virtio-9p-xattr.c |   21 ++++++++++++++++++---
>   1 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/hw/virtio-9p-xattr.c b/hw/virtio-9p-xattr.c
> index 1aab081..fd6d892 100644
> --- a/hw/virtio-9p-xattr.c
> +++ b/hw/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;