| Message ID | 20231101093325.30302-8-jgross@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tools: enable xenstore-stubdom to use 9pfs | expand |
On Wed, Nov 1, 2023 at 5:54 AM Juergen Gross <jgross@suse.com> wrote: > > Add the attach request of the 9pfs protocol. This introduces the "fid" > scheme of the 9pfs protocol. > > As this will be needed later, use a dedicated memory allocation > function in alloc_fid(). > > For filling the qid data take the approach from the qemu 9pfs backend > implementation. > > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > tools/xenlogd/io.c | 128 ++++++++++++++++++++++++++++++++++++++++ > tools/xenlogd/xenlogd.c | 1 + > tools/xenlogd/xenlogd.h | 11 ++++ > 3 files changed, 140 insertions(+) > > diff --git a/tools/xenlogd/io.c b/tools/xenlogd/io.c > index f35520018f..fa825c9f39 100644 > --- a/tools/xenlogd/io.c > +++ b/tools/xenlogd/io.c > +static struct p9_fid *alloc_fid_mem(device *device, unsigned int fid, > + const char *path) > +{ > + struct p9_fid *fidp; > + size_t pathlen; > + > + pathlen = strlen(device->host_path) + strlen(path) + 1; > + fidp = calloc(sizeof(*fidp) + pathlen, 1); > + if ( !fidp ) > + return NULL; > + > + fidp->fid = fid; > + snprintf(fidp->path, pathlen, "%s%s", device->host_path, path); check_host_path() should be enhanced to ensure host_path has a trailing '/', or switch this to "%s/%s" to ensure it's always present? > + > + return fidp; > +} > + > +static void free_fid(device *device, struct p9_fid *fidp) > +{ > + if ( !fidp ) > + return; > + > + device->n_fids--; > + XEN_TAILQ_REMOVE(&device->fids, fidp, list); > + free(fidp); > +} > + > +static int fill_qid(const char *path, struct p9_qid *qid, struct stat *stbuf) Nit: ordering is input, output, optional input, so you might want to re-order? stbuf can be const? > +{ > + struct stat st; > + > + if ( !stbuf ) > + { > + if ( stat(path, &st) ) > + return errno; > + > + stbuf = &st; > + } > + > + qid->type = S_ISDIR(stbuf->st_mode) ? QID_TYPE_DIR : 0; > + qid->version = stbuf->st_mtime ^ (stbuf->st_size << 8); > + qid->path = stbuf->st_ino; > + > + return 0; > +} > + > static void p9_error(device *device, uint16_t tag, uint32_t err) > { > unsigned int erroff; > @@ -476,6 +565,41 @@ static void p9_version(device *device, struct p9_header *hdr) > version); > } > > +static void p9_attach(device *device, struct p9_header *hdr) > +{ > + uint32_t fid; > + uint32_t dummy_u32; > + unsigned int dummy_uint; > + struct p9_qid qid; > + int ret; > + > + ret = fill_data(device, "UUSSU", &fid, &dummy_u32, &dummy_uint, &dummy_uint, > + &dummy_u32); > + if ( ret != 5 ) > + { > + p9_error(device, hdr->tag, errno); > + return; > + } We might want to check the first dummy_u32 (afid) to ensure it's NOFID? """ If the client does not wish to authenticate the connection, or knows that authentication is not required, the afid field in the attach mes- sage should be set to NOFID, defined as (u32int)~0 in fcall.h. If the client does wish to authenticate, it must acquire and validate an afid using an auth message before doing the attach. """ Since auth isn't implemented, it's probably not necessary to check afid? I've been looking at these as reference: https://ericvh.github.io/9p-rfc/rfc9p2000.html https://ericvh.github.io/9p-rfc/rfc9p2000.u.html > + > + device->root_fid = alloc_fid(device, fid, ""); > + if ( !device->root_fid ) > + { > + p9_error(device, hdr->tag, errno); > + return; > + } > + > + ret = fill_qid(device->host_path, &qid, NULL); > + if ( ret ) > + { > + free_fid(device, device->root_fid); root_fid is only freed in this error path. Maybe free_device() should free all the fids? Regards, Jason
On 03.11.23 16:13, Jason Andryuk wrote: > On Wed, Nov 1, 2023 at 5:54 AM Juergen Gross <jgross@suse.com> wrote: >> >> Add the attach request of the 9pfs protocol. This introduces the "fid" >> scheme of the 9pfs protocol. >> >> As this will be needed later, use a dedicated memory allocation >> function in alloc_fid(). >> >> For filling the qid data take the approach from the qemu 9pfs backend >> implementation. >> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> --- >> tools/xenlogd/io.c | 128 ++++++++++++++++++++++++++++++++++++++++ >> tools/xenlogd/xenlogd.c | 1 + >> tools/xenlogd/xenlogd.h | 11 ++++ >> 3 files changed, 140 insertions(+) >> >> diff --git a/tools/xenlogd/io.c b/tools/xenlogd/io.c >> index f35520018f..fa825c9f39 100644 >> --- a/tools/xenlogd/io.c >> +++ b/tools/xenlogd/io.c > >> +static struct p9_fid *alloc_fid_mem(device *device, unsigned int fid, >> + const char *path) >> +{ >> + struct p9_fid *fidp; >> + size_t pathlen; >> + >> + pathlen = strlen(device->host_path) + strlen(path) + 1; >> + fidp = calloc(sizeof(*fidp) + pathlen, 1); >> + if ( !fidp ) >> + return NULL; >> + >> + fidp->fid = fid; >> + snprintf(fidp->path, pathlen, "%s%s", device->host_path, path); > > check_host_path() should be enhanced to ensure host_path has a > trailing '/', or switch this to "%s/%s" to ensure it's always present? No, "path" is always starting with a "/" if it is not empty. > >> + >> + return fidp; >> +} >> + > > >> +static void free_fid(device *device, struct p9_fid *fidp) >> +{ >> + if ( !fidp ) >> + return; >> + >> + device->n_fids--; >> + XEN_TAILQ_REMOVE(&device->fids, fidp, list); >> + free(fidp); >> +} >> + >> +static int fill_qid(const char *path, struct p9_qid *qid, struct stat *stbuf) > > Nit: ordering is input, output, optional input, so you might want to re-order? Hmm, I did it on purpose, as the last parameter is optional (as you said). > > stbuf can be const? Yes. > >> +{ >> + struct stat st; >> + >> + if ( !stbuf ) >> + { >> + if ( stat(path, &st) ) >> + return errno; >> + >> + stbuf = &st; >> + } >> + >> + qid->type = S_ISDIR(stbuf->st_mode) ? QID_TYPE_DIR : 0; >> + qid->version = stbuf->st_mtime ^ (stbuf->st_size << 8); >> + qid->path = stbuf->st_ino; >> + >> + return 0; >> +} >> + >> static void p9_error(device *device, uint16_t tag, uint32_t err) >> { >> unsigned int erroff; >> @@ -476,6 +565,41 @@ static void p9_version(device *device, struct p9_header *hdr) >> version); >> } >> >> +static void p9_attach(device *device, struct p9_header *hdr) >> +{ >> + uint32_t fid; >> + uint32_t dummy_u32; >> + unsigned int dummy_uint; >> + struct p9_qid qid; >> + int ret; >> + >> + ret = fill_data(device, "UUSSU", &fid, &dummy_u32, &dummy_uint, &dummy_uint, >> + &dummy_u32); >> + if ( ret != 5 ) >> + { >> + p9_error(device, hdr->tag, errno); >> + return; >> + } > > We might want to check the first dummy_u32 (afid) to ensure it's NOFID? > """ > If the client does not wish to authenticate the connection, or knows > that authentication is not required, the afid field in the attach mes- > sage should be set to NOFID, defined as (u32int)~0 in fcall.h. If the > client does wish to authenticate, it must acquire and validate an afid > using an auth message before doing the attach. > """ > > Since auth isn't implemented, it's probably not necessary to check afid? That was the idea, yes. > > I've been looking at these as reference: > https://ericvh.github.io/9p-rfc/rfc9p2000.html > https://ericvh.github.io/9p-rfc/rfc9p2000.u.html > >> + >> + device->root_fid = alloc_fid(device, fid, ""); >> + if ( !device->root_fid ) >> + { >> + p9_error(device, hdr->tag, errno); >> + return; >> + } >> + >> + ret = fill_qid(device->host_path, &qid, NULL); >> + if ( ret ) >> + { >> + free_fid(device, device->root_fid); > > root_fid is only freed in this error path. Maybe free_device() should > free all the fids? No, but io_thread() should do so at the end. Juergen
diff --git a/tools/xenlogd/io.c b/tools/xenlogd/io.c index f35520018f..fa825c9f39 100644 --- a/tools/xenlogd/io.c +++ b/tools/xenlogd/io.c @@ -16,6 +16,8 @@ #include <stdlib.h> #include <string.h> #include <syslog.h> +#include <sys/types.h> +#include <sys/stat.h> #include <xenctrl.h> /* For cpu barriers. */ #include <xen-tools/common-macros.h> @@ -23,6 +25,7 @@ /* P9 protocol commands (response is either cmd+1 or P9_CMD_ERROR). */ #define P9_CMD_VERSION 100 +#define P9_CMD_ATTACH 104 #define P9_CMD_ERROR 107 #define P9_MIN_MSIZE 2048 @@ -434,6 +437,92 @@ static int fill_data(device *device, const char *fmt, ...) return pars; } +static struct p9_fid *find_fid(device *device, unsigned int fid) +{ + struct p9_fid *fidp; + + XEN_TAILQ_FOREACH(fidp, &device->fids, list) + { + if ( fidp->fid == fid ) + return fidp; + } + + return NULL; +} + +static struct p9_fid *alloc_fid_mem(device *device, unsigned int fid, + const char *path) +{ + struct p9_fid *fidp; + size_t pathlen; + + pathlen = strlen(device->host_path) + strlen(path) + 1; + fidp = calloc(sizeof(*fidp) + pathlen, 1); + if ( !fidp ) + return NULL; + + fidp->fid = fid; + snprintf(fidp->path, pathlen, "%s%s", device->host_path, path); + + return fidp; +} + +static struct p9_fid *alloc_fid(device *device, unsigned int fid, + const char *path) +{ + struct p9_fid *fidp; + + if ( find_fid(device, fid) ) + { + errno = EBADFD; + return NULL; + } + + if ( device->n_fids >= device->max_open_files ) + { + errno = EMFILE; + return NULL; + } + + fidp = alloc_fid_mem(device, fid, path); + if ( !fidp ) + return NULL; + + XEN_TAILQ_INSERT_HEAD(&device->fids, fidp, list); + device->n_fids++; + + return fidp; +} + +static void free_fid(device *device, struct p9_fid *fidp) +{ + if ( !fidp ) + return; + + device->n_fids--; + XEN_TAILQ_REMOVE(&device->fids, fidp, list); + free(fidp); +} + +static int fill_qid(const char *path, struct p9_qid *qid, struct stat *stbuf) +{ + struct stat st; + + if ( !stbuf ) + { + if ( stat(path, &st) ) + return errno; + + stbuf = &st; + } + + qid->type = S_ISDIR(stbuf->st_mode) ? QID_TYPE_DIR : 0; + qid->version = stbuf->st_mtime ^ (stbuf->st_size << 8); + qid->path = stbuf->st_ino; + + return 0; +} + static void p9_error(device *device, uint16_t tag, uint32_t err) { unsigned int erroff; @@ -476,6 +565,41 @@ static void p9_version(device *device, struct p9_header *hdr) version); } +static void p9_attach(device *device, struct p9_header *hdr) +{ + uint32_t fid; + uint32_t dummy_u32; + unsigned int dummy_uint; + struct p9_qid qid; + int ret; + + ret = fill_data(device, "UUSSU", &fid, &dummy_u32, &dummy_uint, &dummy_uint, + &dummy_u32); + if ( ret != 5 ) + { + p9_error(device, hdr->tag, errno); + return; + } + + device->root_fid = alloc_fid(device, fid, ""); + if ( !device->root_fid ) + { + p9_error(device, hdr->tag, errno); + return; + } + + ret = fill_qid(device->host_path, &qid, NULL); + if ( ret ) + { + free_fid(device, device->root_fid); + device->root_fid = NULL; + p9_error(device, hdr->tag, ret); + return; + } + + fill_buffer(device, hdr->cmd + 1, hdr->tag, "Q", &qid); +} + void *io_thread(void *arg) { device *device = arg; @@ -535,6 +659,10 @@ void *io_thread(void *arg) p9_version(device, &hdr); break; + case P9_CMD_ATTACH: + p9_attach(device, &hdr); + break; + default: syslog(LOG_DEBUG, "%u.%u sent unhandled command %u\n", device->domid, device->devid, hdr.cmd); diff --git a/tools/xenlogd/xenlogd.c b/tools/xenlogd/xenlogd.c index da0a09a122..d2de7bfbf2 100644 --- a/tools/xenlogd/xenlogd.c +++ b/tools/xenlogd/xenlogd.c @@ -274,6 +274,7 @@ static device *new_device(unsigned int domid, unsigned int devid) pthread_cond_init(&device->cond, NULL); pthread_mutex_init(&device->mutex, NULL); + XEN_TAILQ_INIT(&device->fids); val = read_backend_node(device, "security_model"); if ( !val || strcmp(val, "none") ) diff --git a/tools/xenlogd/xenlogd.h b/tools/xenlogd/xenlogd.h index c10c6aa9e5..bd2a283ccb 100644 --- a/tools/xenlogd/xenlogd.h +++ b/tools/xenlogd/xenlogd.h @@ -19,6 +19,12 @@ struct p9_header { uint16_t tag; } __attribute__((packed)); +struct p9_fid { + XEN_TAILQ_ENTRY(struct p9_fid) list; + unsigned int fid; + char path[]; +}; + typedef struct device device; struct device { /* Admin data. */ @@ -60,6 +66,11 @@ struct device { char *str; /* String work space. */ unsigned int str_size; /* Size of *str. */ unsigned int str_used; /* Currently used size of *str. */ + + /* File system handling. */ + XEN_TAILQ_HEAD(fidhead, struct p9_fid) fids; + struct p9_fid *root_fid; + unsigned int n_fids; }; extern xenevtchn_handle *xe;
Add the attach request of the 9pfs protocol. This introduces the "fid" scheme of the 9pfs protocol. As this will be needed later, use a dedicated memory allocation function in alloc_fid(). For filling the qid data take the approach from the qemu 9pfs backend implementation. Signed-off-by: Juergen Gross <jgross@suse.com> --- tools/xenlogd/io.c | 128 ++++++++++++++++++++++++++++++++++++++++ tools/xenlogd/xenlogd.c | 1 + tools/xenlogd/xenlogd.h | 11 ++++ 3 files changed, 140 insertions(+)