Re: [Qemu-devel] [PATCH v2 15/20] 9p: darwin: *xattr_nofollow implementations

[Greg Kurz <groug@kaod.org>]

On Thu, 31 May 2018 21:26:10 -0400
Keno Fischer <keno@juliacomputing.com> wrote:

> This implements the darwin equivalent of the functions that were
> moved to 9p-util(-linux) earlier in this series in the new
> 9p-util-darwin file.
> 
> Signed-off-by: Keno Fischer <keno@juliacomputing.com>
> ---
> 

The patch looks good but...

> Changes from v1:
>  * New 9p-util-darwin.c rather than ifdefs in 9p-util.c
>  * Drop incorrect AT_NOFOLLOW from the actual call
> 
>  hw/9pfs/9p-util-darwin.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/9pfs/Makefile.objs    |  1 +
>  2 files changed, 65 insertions(+)
>  create mode 100644 hw/9pfs/9p-util-darwin.c
> 
> diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
> new file mode 100644
> index 0000000..cdb4c9e
> --- /dev/null
> +++ b/hw/9pfs/9p-util-darwin.c
> @@ -0,0 +1,64 @@
> +/*
> + * 9p utilities (Darwin Implementation)
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/xattr.h"
> +#include "9p-util.h"
> +
> +ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const char *name,
> +                             void *value, size_t size)
> +{
> +    int ret;
> +    int fd = openat_file(dirfd, filename,
> +                         O_RDONLY | O_PATH_9P_UTIL | O_NOFOLLOW, 0);
> +    if (fd == -1) {
> +        return -1;
> +    }
> +    ret = fgetxattr(fd, name, value, size, 0, 0);
> +    close_preserve_errno(fd);
> +    return ret;
> +}
> +
> +ssize_t flistxattrat_nofollow(int dirfd, const char *filename,
> +                              char *list, size_t size)
> +{
> +    int ret;
> +    int fd = openat_file(dirfd, filename,
> +                         O_RDONLY | O_PATH_9P_UTIL | O_NOFOLLOW, 0);
> +    if (fd == -1) {
> +        return -1;
> +    }
> +    ret = flistxattr(fd, list, size, 0);
> +    close_preserve_errno(fd);
> +    return ret;
> +}
> +
> +ssize_t fremovexattrat_nofollow(int dirfd, const char *filename,
> +                                const char *name)
> +{
> +    int ret;
> +    int fd = openat_file(dirfd, filename, O_PATH_9P_UTIL | O_NOFOLLOW, 0);
> +    if (fd == -1) {
> +        return -1;
> +    }
> +    ret = fremovexattr(fd, name, 0);
> +    close_preserve_errno(fd);
> +    return ret;
> +}
> +
> +int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name,
> +                         void *value, size_t size, int flags)
> +{
> +    int ret;
> +    int fd = openat_file(dirfd, filename, O_PATH_9P_UTIL | O_NOFOLLOW, 0);
> +    if (fd == -1) {
> +        return -1;
> +    }
> +    ret = fsetxattr(fd, name, value, size, 0, flags);
> +    close_preserve_errno(fd);
> +    return ret;
> +}

... I now realize that flags may come from the client, ie, it should be
translated before being passed to the backends, pretty much like the other
patch with unlinkat.

The specification for 9p2000.L says it is derived from "Linux setxattr".

https://github.com/chaos/diod/blob/master/protocol.md#xattrcreate----prepare-to-set-extended-attribute

ie,

include/uapi/linux/xattr.h:#define XATTR_CREATE 0x1     /* set value, fail if attr already exists */
include/uapi/linux/xattr.h:#define XATTR_REPLACE        0x2     /* set value, fail if attr does not exist */

I guess this calls for some defines in 9p.h:

/* 9p2000.L says that the 'flags' argument of operation 'xattrcreate'
 * are derived from Linux setxattr.
 */
#define P9_XATTR_CREATE  1
#define P9_XATTR_REPLACE 2

Please do that in a preparatory patch.

I would also appreciate you look at other 9P operations and
check if we have other places where we need to translate
some flags.

> diff --git a/hw/9pfs/Makefile.objs b/hw/9pfs/Makefile.objs
> index 083508f..24a8695 100644
> --- a/hw/9pfs/Makefile.objs
> +++ b/hw/9pfs/Makefile.objs
> @@ -1,5 +1,6 @@
>  common-obj-y  = 9p.o
>  common-obj-$(CONFIG_LINUX) += 9p-util-linux.o
> +common-obj-$(CONFIG_DARWIN) += 9p-util-darwin.o
>  common-obj-y += 9p-local.o 9p-xattr.o
>  common-obj-y += 9p-xattr-user.o 9p-posix-acl.o
>  common-obj-y += coth.o cofs.o codir.o cofile.o