Re: [PATCH 11/29] tools/xenlogd: add 9pfs create request support

[Jason Andryuk <jandryuk@xxxxxxxxx>]

On Wed, Nov 1, 2023 at 6:48 AM Juergen Gross <jgross@xxxxxxxx> wrote:
>
> Add the create request of the 9pfs protocol.
>
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
>  tools/xenlogd/io.c | 133 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 133 insertions(+)
>
> diff --git a/tools/xenlogd/io.c b/tools/xenlogd/io.c
> index 2607095e51..34f137be1b 100644
> --- a/tools/xenlogd/io.c
> +++ b/tools/xenlogd/io.c
> @@ -31,6 +31,7 @@
>  #define P9_CMD_ERROR      107
>  #define P9_CMD_WALK       110
>  #define P9_CMD_OPEN       112
> +#define P9_CMD_CREATE     114
>  #define P9_CMD_CLUNK      120
>
>  /* P9 protocol open flags. */
> @@ -41,6 +42,12 @@
>  #define P9_OTRUNC        0x10   /* or'ed in, truncate file first */
>  #define P9_OREMOVE       0x40   /* or'ed in, remove file after clunk */
>
> +/* P9 protocol create permission masks. */
> +#define P9_CREATE_PERM_DIR        0x80000000
> +#define P9_CREATE_PERM_NOTSUPP    0x03b00000   /* link, symlink, ... */
> +#define P9_CREATE_PERM_DIR_MASK   0777
> +#define P9_CREATE_PERM_FILE_MASK  0666
> +
>  #define P9_MIN_MSIZE      2048
>  #define P9_VERSION        "9P2000.u"
>  #define P9_WALK_MAXELEM   16
> @@ -861,6 +868,128 @@ static void p9_open(device *device, struct p9_header 
> *hdr)
>      fill_buffer(device, hdr->cmd + 1, hdr->tag, "QU", &qid, &iounit);
>  }
>
> +static void p9_create(device *device, struct p9_header *hdr)
> +{
> +    uint32_t fid;
> +    unsigned int name_off;
> +    uint32_t perm;
> +    uint8_t mode;
> +    unsigned int ext_off;
> +    struct p9_fid *fidp;
> +    struct p9_fid *new_fidp;
> +    char *path;
> +    struct stat st;
> +    struct p9_qid qid;
> +    uint32_t iounit;
> +    int flags;
> +    int ret;
> +
> +    ret = fill_data(device, "USUbS", &fid, &name_off, &perm, &mode, 
> &ext_off);
> +    if ( ret != 5 )
> +    {
> +        p9_error(device, hdr->tag, EINVAL);
> +        return;
> +    }
> +
> +    if ( !name_ok(device->str + name_off) )
> +    {
> +        p9_error(device, hdr->tag, ENOENT);
> +        return;
> +    }
> +
> +    if ( perm & P9_CREATE_PERM_NOTSUPP )
> +    {
> +        p9_error(device, hdr->tag, EINVAL);
> +        return;
> +    }
> +
> +    fidp = find_fid(device, fid);
> +    if ( !fidp || fidp->opened )
> +    {
> +        p9_error(device, hdr->tag, EINVAL);
> +        return;
> +    }
> +
> +    path = malloc(strlen(fidp->path) + strlen(device->str + name_off) + 2 -
> +                  strlen(device->host_path));
> +    if ( !path )
> +    {
> +        p9_error(device, hdr->tag, ENOMEM);
> +        return;
> +    }
> +    sprintf(path, "%s/%s", fidp->path + strlen(device->host_path),
> +            device->str + name_off);
> +    new_fidp = alloc_fid_mem(device, fid, path);
> +    free(path);
> +    if ( !new_fidp )
> +    {
> +        p9_error(device, hdr->tag, ENOMEM);
> +        return;
> +    }
> +
> +    if ( perm & P9_CREATE_PERM_DIR )
> +    {
> +        if ( mode != P9_OREAD )
> +        {
> +            p9_error(device, hdr->tag, EINVAL);
> +            free(new_fidp);
> +            return;
> +        }
> +        if ( mkdir(new_fidp->path, perm & P9_CREATE_PERM_DIR_MASK) < 0 )
> +        {
> +            p9_error(device, hdr->tag, errno);
> +            free(new_fidp);
> +            return;
> +        }
> +
> +        XEN_TAILQ_REMOVE(&device->fids, fidp, list);
> +        XEN_TAILQ_INSERT_HEAD(&device->fids, new_fidp, list);
> +        free(fidp);
> +        fidp = new_fidp;
> +
> +        fidp->data = opendir(fidp->path);
> +        if ( !fidp->data )
> +        {
> +            p9_error(device, hdr->tag, errno);
> +            return;
> +        }
> +        fidp->fd = dirfd(fidp->data);
> +    }
> +    else
> +    {
> +        flags = open_flags_from_mode(mode);
> +        if ( flags < 0 )
> +        {
> +            p9_error(device, hdr->tag, EINVAL);
> +            free(new_fidp);
> +            return;
> +        }
> +
> +        XEN_TAILQ_REMOVE(&device->fids, """fidp, list);
> +        XEN_TAILQ_INSERT_HEAD(&device->fids, new_fidp, list);
> +        free(fidp);
> +        fidp = new_fidp;
> +
> +        fidp->fd = creat(fidp->path, flags);

https://ericvh.github.io/9p-rfc/rfc9p2000.html#anchor29
"""
The create request asks the file server to create a new file with the
name supplied, in the directory (dir) represented by fid, and requires
write permission in the directory. The owner of the file is the
implied user id of the request, the group of the file is the same as
dir, and the permissions are the value of

perm & (~0666 | (dir.perm & 0666))

if a regular file is being created and

perm & (~0777 | (dir.perm & 0777))

if a directory is being created. This means, for example, that if the
create allows read permission to others, but the containing directory
does not, then the created file will not allow others to read the
file.
"""

So P9_CREATE_PERM_FILE_MASK (which is otherwise unused) should be
incorporated.  Also the mode and the permissions are handled
independently:
"""
Finally, the newly created file is opened according to mode, and fid
will represent the newly opened file. Mode is not checked against the
permissions in perm.
"""

I think fidp->mode should be set in here somewhere based on mode.  And
`flags` for creat should be derived from the incoming perms.

Regards,
Jason