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

[Juergen Gross <jgross@xxxxxxxx>]

On 06.11.23 20:53, Jason Andryuk wrote:
> 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.
Yes, I agree.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature