| Message ID | 1312207703-16947-1-git-send-email-levinsasha928@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 1 Aug 2011 17:08:22 +0300, Sasha Levin <levinsasha928@gmail.com> wrote: > This patch adds support for the UNIX extensions to 9p2000. > > Supporting thses extensions allow us to transperantly mount UNIX directories > without missing features such as symlinks. > > Signed-off-by: Sasha Levin <levinsasha928@gmail.com> > --- > tools/kvm/include/kvm/virtio-9p.h | 4 +- > tools/kvm/virtio/9p-pdu.c | 6 ++- > tools/kvm/virtio/9p.c | 75 +++++++++++++++++++++++++++++++------ > 3 files changed, 69 insertions(+), 16 deletions(-) > > diff --git a/tools/kvm/include/kvm/virtio-9p.h b/tools/kvm/include/kvm/virtio-9p.h > index 8584f49..0e55e5c 100644 > --- a/tools/kvm/include/kvm/virtio-9p.h > +++ b/tools/kvm/include/kvm/virtio-9p.h > @@ -11,8 +11,8 @@ > #define VIRTQUEUE_NUM 128 > #define VIRTIO_P9_DEFAULT_TAG "kvm_9p" > #define VIRTIO_P9_HDR_LEN (sizeof(u32)+sizeof(u8)+sizeof(u16)) > -#define VIRTIO_P9_MAX_FID 128 > -#define VIRTIO_P9_VERSION "9P2000" > +#define VIRTIO_P9_MAX_FID 256 > +#define VIRTIO_P9_VERSION "9P2000.u" > #define MAX_TAG_LEN 32 > > struct p9_msg { > diff --git a/tools/kvm/virtio/9p-pdu.c b/tools/kvm/virtio/9p-pdu.c > index 0c454db..8ed249f 100644 > --- a/tools/kvm/virtio/9p-pdu.c > +++ b/tools/kvm/virtio/9p-pdu.c > @@ -200,13 +200,15 @@ static int virtio_p9_pdu_encode(struct p9_pdu *pdu, const char *fmt, va_list ap) > case 'S': > { > struct p9_wstat *stbuf = va_arg(ap, struct p9_wstat *); > - retval = virtio_p9_pdu_writef(pdu, "wwdQdddqssss", > + retval = virtio_p9_pdu_writef(pdu, "wwdQdddqsssssddd", > stbuf->size, stbuf->type, > stbuf->dev, &stbuf->qid, > stbuf->mode, stbuf->atime, > stbuf->mtime, stbuf->length, > stbuf->name, stbuf->uid, > - stbuf->gid, stbuf->muid); > + stbuf->gid, stbuf->muid, > + stbuf->extension, stbuf->n_uid, > + stbuf->n_gid, stbuf->n_muid); > } > break; > default: > diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c > index 3b5555c..a6eafe0 100644 > --- a/tools/kvm/virtio/9p.c > +++ b/tools/kvm/virtio/9p.c > @@ -162,7 +162,7 @@ static void virtio_p9_error_reply(struct p9_dev *p9dev, > > err_str = strerror(err); > pdu->write_offset = VIRTIO_P9_HDR_LEN; > - virtio_p9_pdu_writef(pdu, "s", err_str); > + virtio_p9_pdu_writef(pdu, "sd", err_str, -err); Should that be -err ? I guess it should the errno value ? > *outlen = pdu->write_offset; > > pdu->read_offset = sizeof(u32) + sizeof(u8); > @@ -204,7 +204,6 @@ static void virtio_p9_open(struct p9_dev *p9dev, > struct p9_qid qid; > struct p9_fid *new_fid; > > - > virtio_p9_pdu_readf(pdu, "db", &fid, &mode); > new_fid = &p9dev->fids[fid]; > > @@ -242,13 +241,14 @@ static void virtio_p9_create(struct p9_dev *p9dev, > u8 mode; > u32 perm; > char *name; > + char *ext = NULL; > u32 fid_val; > struct stat st; > struct p9_qid qid; > struct p9_fid *fid; > char full_path[PATH_MAX]; > > - virtio_p9_pdu_readf(pdu, "dsdb", &fid_val, &name, &perm, &mode); > + virtio_p9_pdu_readf(pdu, "dsdbs", &fid_val, &name, &perm, &mode, > &ext); ext need to be freed. ? I guess we have other places where the memory alloc did for readf(.., "s", ..) is not freed. > fid = &p9dev->fids[fid_val]; > > sprintf(full_path, "%s/%s", fid->abs_path, name); > @@ -262,6 +262,30 @@ static void virtio_p9_create(struct p9_dev *p9dev, > close_fid(p9dev, fid_val); > fid->dir = dir; > fid->is_dir = 1; > + } else if (perm & P9_DMSYMLINK) { > + int r; > + > + r = symlink(ext, full_path); > + if (r < 0) > + goto err_out; > + fd = open(full_path, omode2uflags(mode)); > + if (fd < 0) > + goto err_out; > + close_fid(p9dev, fid_val); > + fid->fd = fd; that updates the fid ? Is that needed ? Also above we are just updating fid->fd where as fid->path still points to the old name. I guess symlink and link don't result in open > + } else if (perm & P9_DMLINK) { > + int r; > + int ext_fid = atoi(ext); > + > + r = link(p9dev->fids[ext_fid].abs_path, full_path); > + if (r < 0) > + goto err_out; > + > + fd = open(full_path, omode2uflags(mode)); > + if (fd < 0) > + goto err_out; > + close_fid(p9dev, fid_val); > + fid->fd = fd; > } else { > fd = open(full_path, omode2uflags(mode) | O_CREAT, 0777); > if (fd < 0) > @@ -294,7 +318,7 @@ static void virtio_p9_walk(struct p9_dev *p9dev, > u32 newfid_val; > struct p9_qid wqid; > struct p9_fid *new_fid; > - > + int ret; > > virtio_p9_pdu_readf(pdu, "ddw", &fid_val, &newfid_val, &nwname); > new_fid = &p9dev->fids[newfid_val]; > @@ -315,8 +339,10 @@ static void virtio_p9_walk(struct p9_dev *p9dev, > /* Format the new path we're 'walk'ing into */ > sprintf(tmp, "%s/%.*s", > fid->path, (int)strlen(str), str); > - if (lstat(rel_to_abs(p9dev, tmp, full_path), &st) < 0) > + if (lstat(rel_to_abs(p9dev, tmp, full_path), &st) < 0) { > + ret = ENOENT; > goto err_out; > + } Why ?. What error with lstat failed which we wanted to map to ENOENT ? > > st2qid(&st, &wqid); > new_fid->is_dir = S_ISDIR(st.st_mode); > @@ -340,7 +366,7 @@ static void virtio_p9_walk(struct p9_dev *p9dev, > virtio_p9_set_reply_header(pdu, *outlen); > return; > err_out: > - virtio_p9_error_reply(p9dev, pdu, errno, outlen); > + virtio_p9_error_reply(p9dev, pdu, ret, outlen); > return; > } > > @@ -381,6 +407,12 @@ err_out: > return; > } > > +static void virtio_p9_free_stat(struct p9_wstat *wstat) > +{ > + free(wstat->extension); > + free(wstat->name); > +} > + > static void virtio_p9_fill_stat(struct p9_dev *p9dev, const char *name, > struct stat *st, struct p9_wstat *wstat) > { > @@ -393,6 +425,17 @@ static void virtio_p9_fill_stat(struct p9_dev *p9dev, const char *name, > wstat->length = 0; > wstat->mode |= P9_DMDIR; > } > + if(S_ISLNK(st->st_mode)) { > + char tmp[PATH_MAX] = {0}, full_path[PATH_MAX] = {0}; > + > + rel_to_abs(p9dev, name, full_path); > + > + if (readlink(full_path, tmp, PATH_MAX) > 0) > + wstat->extension = strdup(tmp); > + wstat->mode |= P9_DMSYMLINK; > + } else { > + wstat->extension = NULL; > + } > > wstat->atime = st->st_atime; > wstat->mtime = st->st_mtime; > @@ -401,14 +444,20 @@ static void virtio_p9_fill_stat(struct p9_dev *p9dev, const char *name, > wstat->uid = NULL; > wstat->gid = NULL; > wstat->muid = NULL; > + wstat->n_uid = wstat->n_gid = wstat->n_muid = 0; > > - /* NOTE: size shouldn't include its own length */ > - /* size[2] type[2] dev[4] qid[13] */ > - /* mode[4] atime[4] mtime[4] length[8]*/ > - /* name[s] uid[s] gid[s] muid[s] */ > - wstat->size = 2+4+13+4+4+4+8+2+2+2+2; > + /* > + * NOTE: size shouldn't include its own length > + * size[2] type[2] dev[4] qid[13] > + * mode[4] atime[4] mtime[4] length[8] > + * name[s] uid[s] gid[s] muid[s] > + * ext[s] uid[4] gid[4] muid[4] > + */ > + wstat->size = 2+4+13+4+4+4+8+2+2+2+2+2+4+4+4; > if (wstat->name) > wstat->size += strlen(wstat->name); > + if (wstat->extension) > + wstat->size += strlen(wstat->extension); > } > > static void virtio_p9_read(struct p9_dev *p9dev, > @@ -440,6 +489,7 @@ static void virtio_p9_read(struct p9_dev *p9dev, > read = pdu->write_offset; > virtio_p9_pdu_writef(pdu, "S", &wstat); > rcount += pdu->write_offset - read; > + virtio_p9_free_stat(&wstat); > > cur = readdir(fid->dir); > } > @@ -486,6 +536,7 @@ static void virtio_p9_stat(struct p9_dev *p9dev, > > virtio_p9_pdu_writef(pdu, "wS", 0, &wstat); > *outlen = pdu->write_offset; > + virtio_p9_free_stat(&wstat); > virtio_p9_set_reply_header(pdu, *outlen); > return; > err_out: > -- > 1.7.6 > -aneesh -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2011-08-01 at 20:42 +0530, Aneesh Kumar K.V wrote: > On Mon, 1 Aug 2011 17:08:22 +0300, Sasha Levin <levinsasha928@gmail.com> wrote: > > This patch adds support for the UNIX extensions to 9p2000. > > > > Supporting thses extensions allow us to transperantly mount UNIX directories > > without missing features such as symlinks. > > > > Signed-off-by: Sasha Levin <levinsasha928@gmail.com> > > --- > > tools/kvm/include/kvm/virtio-9p.h | 4 +- > > tools/kvm/virtio/9p-pdu.c | 6 ++- > > tools/kvm/virtio/9p.c | 75 +++++++++++++++++++++++++++++++------ > > 3 files changed, 69 insertions(+), 16 deletions(-) > > > > diff --git a/tools/kvm/include/kvm/virtio-9p.h b/tools/kvm/include/kvm/virtio-9p.h > > index 8584f49..0e55e5c 100644 > > --- a/tools/kvm/include/kvm/virtio-9p.h > > +++ b/tools/kvm/include/kvm/virtio-9p.h > > @@ -11,8 +11,8 @@ > > #define VIRTQUEUE_NUM 128 > > #define VIRTIO_P9_DEFAULT_TAG "kvm_9p" > > #define VIRTIO_P9_HDR_LEN (sizeof(u32)+sizeof(u8)+sizeof(u16)) > > -#define VIRTIO_P9_MAX_FID 128 > > -#define VIRTIO_P9_VERSION "9P2000" > > +#define VIRTIO_P9_MAX_FID 256 > > +#define VIRTIO_P9_VERSION "9P2000.u" > > #define MAX_TAG_LEN 32 > > > > struct p9_msg { > > diff --git a/tools/kvm/virtio/9p-pdu.c b/tools/kvm/virtio/9p-pdu.c > > index 0c454db..8ed249f 100644 > > --- a/tools/kvm/virtio/9p-pdu.c > > +++ b/tools/kvm/virtio/9p-pdu.c > > @@ -200,13 +200,15 @@ static int virtio_p9_pdu_encode(struct p9_pdu *pdu, const char *fmt, va_list ap) > > case 'S': > > { > > struct p9_wstat *stbuf = va_arg(ap, struct p9_wstat *); > > - retval = virtio_p9_pdu_writef(pdu, "wwdQdddqssss", > > + retval = virtio_p9_pdu_writef(pdu, "wwdQdddqsssssddd", > > stbuf->size, stbuf->type, > > stbuf->dev, &stbuf->qid, > > stbuf->mode, stbuf->atime, > > stbuf->mtime, stbuf->length, > > stbuf->name, stbuf->uid, > > - stbuf->gid, stbuf->muid); > > + stbuf->gid, stbuf->muid, > > + stbuf->extension, stbuf->n_uid, > > + stbuf->n_gid, stbuf->n_muid); > > } > > break; > > default: > > diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c > > index 3b5555c..a6eafe0 100644 > > --- a/tools/kvm/virtio/9p.c > > +++ b/tools/kvm/virtio/9p.c > > @@ -162,7 +162,7 @@ static void virtio_p9_error_reply(struct p9_dev *p9dev, > > > > err_str = strerror(err); > > pdu->write_offset = VIRTIO_P9_HDR_LEN; > > - virtio_p9_pdu_writef(pdu, "s", err_str); > > + virtio_p9_pdu_writef(pdu, "sd", err_str, -err); > > Should that be -err ? I guess it should the errno value ? Yup, but usually we get -errno back from functions and pass it to virtio_p9_error_reply() while 9p expects it to be errno. > > > *outlen = pdu->write_offset; > > > > pdu->read_offset = sizeof(u32) + sizeof(u8); > > @@ -204,7 +204,6 @@ static void virtio_p9_open(struct p9_dev *p9dev, > > struct p9_qid qid; > > struct p9_fid *new_fid; > > > > - > > virtio_p9_pdu_readf(pdu, "db", &fid, &mode); > > new_fid = &p9dev->fids[fid]; > > > > @@ -242,13 +241,14 @@ static void virtio_p9_create(struct p9_dev *p9dev, > > u8 mode; > > u32 perm; > > char *name; > > + char *ext = NULL; > > u32 fid_val; > > struct stat st; > > struct p9_qid qid; > > struct p9_fid *fid; > > char full_path[PATH_MAX]; > > > > - virtio_p9_pdu_readf(pdu, "dsdb", &fid_val, &name, &perm, &mode); > > + virtio_p9_pdu_readf(pdu, "dsdbs", &fid_val, &name, &perm, &mode, > > &ext); > > ext need to be freed. ? I guess we have other places where the memory > alloc did for readf(.., "s", ..) is not freed. Oh heh, I took care of the part where we create wstat and write it, not for the part where we read it :) > > > fid = &p9dev->fids[fid_val]; > > > > sprintf(full_path, "%s/%s", fid->abs_path, name); > > @@ -262,6 +262,30 @@ static void virtio_p9_create(struct p9_dev *p9dev, > > close_fid(p9dev, fid_val); > > fid->dir = dir; > > fid->is_dir = 1; > > + } else if (perm & P9_DMSYMLINK) { > > + int r; > > + > > + r = symlink(ext, full_path); > > + if (r < 0) > > + goto err_out; > > + fd = open(full_path, omode2uflags(mode)); > > + if (fd < 0) > > + goto err_out; > > + close_fid(p9dev, fid_val); > > + fid->fd = fd; > > > that updates the fid ? Is that needed ? Also above we are just updating > fid->fd where as fid->path still points to the old name. I guess > symlink and link don't result in open I'm really not sure if it's needed, the RFC isn't specific as to whether we open 'special' files after creation or not, so I wanted to go with opening them just in case (the kernel will clunk them anyway later, so what the hell...). Regarding the open in the symlink case, it's same deal really - I wasn't sure whether create opens the file or not, and since theres no point in opening a symlink I went ahead and opened the target of the symlink. I didn't see a case where the kernel uses a linked/symlinked file straight away, but as the RFC wasn't specific it was implemented anyway. > > > + } else if (perm & P9_DMLINK) { > > + int r; > > + int ext_fid = atoi(ext); > > + > > + r = link(p9dev->fids[ext_fid].abs_path, full_path); > > + if (r < 0) > > + goto err_out; > > + > > + fd = open(full_path, omode2uflags(mode)); > > + if (fd < 0) > > + goto err_out; > > + close_fid(p9dev, fid_val); > > + fid->fd = fd; > > } else { > > fd = open(full_path, omode2uflags(mode) | O_CREAT, 0777); > > if (fd < 0) > > @@ -294,7 +318,7 @@ static void virtio_p9_walk(struct p9_dev *p9dev, > > u32 newfid_val; > > struct p9_qid wqid; > > struct p9_fid *new_fid; > > - > > + int ret; > > > > virtio_p9_pdu_readf(pdu, "ddw", &fid_val, &newfid_val, &nwname); > > new_fid = &p9dev->fids[newfid_val]; > > @@ -315,8 +339,10 @@ static void virtio_p9_walk(struct p9_dev *p9dev, > > /* Format the new path we're 'walk'ing into */ > > sprintf(tmp, "%s/%.*s", > > fid->path, (int)strlen(str), str); > > - if (lstat(rel_to_abs(p9dev, tmp, full_path), &st) < 0) > > + if (lstat(rel_to_abs(p9dev, tmp, full_path), &st) < 0) { > > + ret = ENOENT; > > goto err_out; > > + } > > Why ?. What error with lstat failed which we wanted to map to ENOENT ? stat of non-existent files. > > > > > st2qid(&st, &wqid); > > new_fid->is_dir = S_ISDIR(st.st_mode); > > @@ -340,7 +366,7 @@ static void virtio_p9_walk(struct p9_dev *p9dev, > > virtio_p9_set_reply_header(pdu, *outlen); > > return; > > err_out: > > - virtio_p9_error_reply(p9dev, pdu, errno, outlen); > > + virtio_p9_error_reply(p9dev, pdu, ret, outlen); > > return; > > } > > > > @@ -381,6 +407,12 @@ err_out: > > return; > > } > > > > +static void virtio_p9_free_stat(struct p9_wstat *wstat) > > +{ > > + free(wstat->extension); > > + free(wstat->name); > > +} > > + > > static void virtio_p9_fill_stat(struct p9_dev *p9dev, const char *name, > > struct stat *st, struct p9_wstat *wstat) > > { > > @@ -393,6 +425,17 @@ static void virtio_p9_fill_stat(struct p9_dev *p9dev, const char *name, > > wstat->length = 0; > > wstat->mode |= P9_DMDIR; > > } > > + if(S_ISLNK(st->st_mode)) { > > + char tmp[PATH_MAX] = {0}, full_path[PATH_MAX] = {0}; > > + > > + rel_to_abs(p9dev, name, full_path); > > + > > + if (readlink(full_path, tmp, PATH_MAX) > 0) > > + wstat->extension = strdup(tmp); > > + wstat->mode |= P9_DMSYMLINK; > > + } else { > > + wstat->extension = NULL; > > + } > > > > wstat->atime = st->st_atime; > > wstat->mtime = st->st_mtime; > > @@ -401,14 +444,20 @@ static void virtio_p9_fill_stat(struct p9_dev *p9dev, const char *name, > > wstat->uid = NULL; > > wstat->gid = NULL; > > wstat->muid = NULL; > > + wstat->n_uid = wstat->n_gid = wstat->n_muid = 0; > > > > - /* NOTE: size shouldn't include its own length */ > > - /* size[2] type[2] dev[4] qid[13] */ > > - /* mode[4] atime[4] mtime[4] length[8]*/ > > - /* name[s] uid[s] gid[s] muid[s] */ > > - wstat->size = 2+4+13+4+4+4+8+2+2+2+2; > > + /* > > + * NOTE: size shouldn't include its own length > > + * size[2] type[2] dev[4] qid[13] > > + * mode[4] atime[4] mtime[4] length[8] > > + * name[s] uid[s] gid[s] muid[s] > > + * ext[s] uid[4] gid[4] muid[4] > > + */ > > + wstat->size = 2+4+13+4+4+4+8+2+2+2+2+2+4+4+4; > > if (wstat->name) > > wstat->size += strlen(wstat->name); > > + if (wstat->extension) > > + wstat->size += strlen(wstat->extension); > > } > > > > static void virtio_p9_read(struct p9_dev *p9dev, > > @@ -440,6 +489,7 @@ static void virtio_p9_read(struct p9_dev *p9dev, > > read = pdu->write_offset; > > virtio_p9_pdu_writef(pdu, "S", &wstat); > > rcount += pdu->write_offset - read; > > + virtio_p9_free_stat(&wstat); > > > > cur = readdir(fid->dir); > > } > > @@ -486,6 +536,7 @@ static void virtio_p9_stat(struct p9_dev *p9dev, > > > > virtio_p9_pdu_writef(pdu, "wS", 0, &wstat); > > *outlen = pdu->write_offset; > > + virtio_p9_free_stat(&wstat); > > virtio_p9_set_reply_header(pdu, *outlen); > > return; > > err_out: > > -- > > 1.7.6 > > > > -aneesh
On Mon, 01 Aug 2011 18:26:23 +0300, Sasha Levin <levinsasha928@gmail.com> wrote: > On Mon, 2011-08-01 at 20:42 +0530, Aneesh Kumar K.V wrote: > > On Mon, 1 Aug 2011 17:08:22 +0300, Sasha Levin <levinsasha928@gmail.com> wrote: > > > This patch adds support for the UNIX extensions to 9p2000. > > > > > > Supporting thses extensions allow us to transperantly mount UNIX directories > > > without missing features such as symlinks. > > > > > > Signed-off-by: Sasha Levin <levinsasha928@gmail.com> > > > --- > > > tools/kvm/include/kvm/virtio-9p.h | 4 +- > > > tools/kvm/virtio/9p-pdu.c | 6 ++- > > > tools/kvm/virtio/9p.c | 75 +++++++++++++++++++++++++++++++------ > > > 3 files changed, 69 insertions(+), 16 deletions(-) > > > > > > diff --git a/tools/kvm/include/kvm/virtio-9p.h b/tools/kvm/include/kvm/virtio-9p.h > > > index 8584f49..0e55e5c 100644 > > > --- a/tools/kvm/include/kvm/virtio-9p.h > > > +++ b/tools/kvm/include/kvm/virtio-9p.h > > > @@ -11,8 +11,8 @@ > > > #define VIRTQUEUE_NUM 128 > > > #define VIRTIO_P9_DEFAULT_TAG "kvm_9p" > > > #define VIRTIO_P9_HDR_LEN (sizeof(u32)+sizeof(u8)+sizeof(u16)) > > > -#define VIRTIO_P9_MAX_FID 128 > > > -#define VIRTIO_P9_VERSION "9P2000" > > > +#define VIRTIO_P9_MAX_FID 256 > > > +#define VIRTIO_P9_VERSION "9P2000.u" > > > #define MAX_TAG_LEN 32 > > > > > > struct p9_msg { > > > diff --git a/tools/kvm/virtio/9p-pdu.c b/tools/kvm/virtio/9p-pdu.c > > > index 0c454db..8ed249f 100644 > > > --- a/tools/kvm/virtio/9p-pdu.c > > > +++ b/tools/kvm/virtio/9p-pdu.c > > > @@ -200,13 +200,15 @@ static int virtio_p9_pdu_encode(struct p9_pdu *pdu, const char *fmt, va_list ap) > > > case 'S': > > > { > > > struct p9_wstat *stbuf = va_arg(ap, struct p9_wstat *); > > > - retval = virtio_p9_pdu_writef(pdu, "wwdQdddqssss", > > > + retval = virtio_p9_pdu_writef(pdu, "wwdQdddqsssssddd", > > > stbuf->size, stbuf->type, > > > stbuf->dev, &stbuf->qid, > > > stbuf->mode, stbuf->atime, > > > stbuf->mtime, stbuf->length, > > > stbuf->name, stbuf->uid, > > > - stbuf->gid, stbuf->muid); > > > + stbuf->gid, stbuf->muid, > > > + stbuf->extension, stbuf->n_uid, > > > + stbuf->n_gid, stbuf->n_muid); > > > } > > > break; > > > default: > > > diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c > > > index 3b5555c..a6eafe0 100644 > > > --- a/tools/kvm/virtio/9p.c > > > +++ b/tools/kvm/virtio/9p.c > > > @@ -162,7 +162,7 @@ static void virtio_p9_error_reply(struct p9_dev *p9dev, > > > > > > err_str = strerror(err); > > > pdu->write_offset = VIRTIO_P9_HDR_LEN; > > > - virtio_p9_pdu_writef(pdu, "s", err_str); > > > + virtio_p9_pdu_writef(pdu, "sd", err_str, -err); > > > > Should that be -err ? I guess it should the errno value ? > > Yup, but usually we get -errno back from functions and pass it to > virtio_p9_error_reply() while 9p expects it to be errno. Most of the libc functions should return -1 on error and set errno right ? and i guess most of the 9p function caller virtio_p9_error_reply with errno as argument > > > > > > *outlen = pdu->write_offset; > > > > > > pdu->read_offset = sizeof(u32) + sizeof(u8); > > > @@ -204,7 +204,6 @@ static void virtio_p9_open(struct p9_dev *p9dev, > > > struct p9_qid qid; > > > struct p9_fid *new_fid; > > > > > > - > > > virtio_p9_pdu_readf(pdu, "db", &fid, &mode); > > > new_fid = &p9dev->fids[fid]; > > > > > > @@ -242,13 +241,14 @@ static void virtio_p9_create(struct p9_dev *p9dev, > > > u8 mode; > > > u32 perm; > > > char *name; > > > + char *ext = NULL; > > > u32 fid_val; > > > struct stat st; > > > struct p9_qid qid; > > > struct p9_fid *fid; > > > char full_path[PATH_MAX]; > > > > > > - virtio_p9_pdu_readf(pdu, "dsdb", &fid_val, &name, &perm, &mode); > > > + virtio_p9_pdu_readf(pdu, "dsdbs", &fid_val, &name, &perm, &mode, > > > &ext); > > > > ext need to be freed. ? I guess we have other places where the memory > > alloc did for readf(.., "s", ..) is not freed. > > Oh heh, I took care of the part where we create wstat and write it, not > for the part where we read it :) > > > > > > fid = &p9dev->fids[fid_val]; > > > > > > sprintf(full_path, "%s/%s", fid->abs_path, name); > > > @@ -262,6 +262,30 @@ static void virtio_p9_create(struct p9_dev *p9dev, > > > close_fid(p9dev, fid_val); > > > fid->dir = dir; > > > fid->is_dir = 1; > > > + } else if (perm & P9_DMSYMLINK) { > > > + int r; > > > + > > > + r = symlink(ext, full_path); > > > + if (r < 0) > > > + goto err_out; > > > + fd = open(full_path, omode2uflags(mode)); > > > + if (fd < 0) > > > + goto err_out; > > > + close_fid(p9dev, fid_val); > > > + fid->fd = fd; > > > > > > that updates the fid ? Is that needed ? Also above we are just updating > > fid->fd where as fid->path still points to the old name. I guess > > symlink and link don't result in open > > I'm really not sure if it's needed, the RFC isn't specific as to whether > we open 'special' files after creation or not, so I wanted to go with > opening them just in case (the kernel will clunk them anyway later, so > what the hell...). > > Regarding the open in the symlink case, it's same deal really - I wasn't > sure whether create opens the file or not, and since theres no point in > opening a symlink I went ahead and opened the target of the symlink. > > I didn't see a case where the kernel uses a linked/symlinked file > straight away, but as the RFC wasn't specific it was implemented anyway. > For the linux client we don't need the file to be open. There are few issues here. 1) You are only updating fid->fd, But fid->path is still the old path 2) You are following symlink on host. where as it should actually be followed on the guest. (for ex: if we do ln -s /etc/a k on guest, we actually want to open /etc/a on guest not the one on host.) > > > > > + } else if (perm & P9_DMLINK) { > > > + int r; > > > + int ext_fid = atoi(ext); > > > + > > > + r = link(p9dev->fids[ext_fid].abs_path, full_path); > > > + if (r < 0) > > > + goto err_out; > > > + > > > + fd = open(full_path, omode2uflags(mode)); > > > + if (fd < 0) > > > + goto err_out; > > > + close_fid(p9dev, fid_val); > > > + fid->fd = fd; > > > } else { > > > fd = open(full_path, omode2uflags(mode) | O_CREAT, 0777); > > > if (fd < 0) > > > @@ -294,7 +318,7 @@ static void virtio_p9_walk(struct p9_dev *p9dev, > > > u32 newfid_val; > > > struct p9_qid wqid; > > > struct p9_fid *new_fid; > > > - > > > + int ret; > > > > > > virtio_p9_pdu_readf(pdu, "ddw", &fid_val, &newfid_val, &nwname); > > > new_fid = &p9dev->fids[newfid_val]; > > > @@ -315,8 +339,10 @@ static void virtio_p9_walk(struct p9_dev *p9dev, > > > /* Format the new path we're 'walk'ing into */ > > > sprintf(tmp, "%s/%.*s", > > > fid->path, (int)strlen(str), str); > > > - if (lstat(rel_to_abs(p9dev, tmp, full_path), &st) < 0) > > > + if (lstat(rel_to_abs(p9dev, tmp, full_path), &st) < 0) { > > > + ret = ENOENT; > > > goto err_out; > > > + } > > > > Why ?. What error with lstat failed which we wanted to map to ENOENT ? > > stat of non-existent files. > Won't that return ENOENT ? lstat("k", 0x7fffd5d350c0) = -1 ENOENT (No such file or directory) > > > > > > > > st2qid(&st, &wqid); > > > new_fid->is_dir = S_ISDIR(st.st_mode); > > > @@ -340,7 +366,7 @@ static void virtio_p9_walk(struct p9_dev *p9dev, > > > virtio_p9_set_reply_header(pdu, *outlen); > > > return; > > > err_out: > > > - virtio_p9_error_reply(p9dev, pdu, errno, outlen); > > > + virtio_p9_error_reply(p9dev, pdu, ret, outlen); > > > return; > > > } -aneesh -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/tools/kvm/include/kvm/virtio-9p.h b/tools/kvm/include/kvm/virtio-9p.h index 8584f49..0e55e5c 100644 --- a/tools/kvm/include/kvm/virtio-9p.h +++ b/tools/kvm/include/kvm/virtio-9p.h @@ -11,8 +11,8 @@ #define VIRTQUEUE_NUM 128 #define VIRTIO_P9_DEFAULT_TAG "kvm_9p" #define VIRTIO_P9_HDR_LEN (sizeof(u32)+sizeof(u8)+sizeof(u16)) -#define VIRTIO_P9_MAX_FID 128 -#define VIRTIO_P9_VERSION "9P2000" +#define VIRTIO_P9_MAX_FID 256 +#define VIRTIO_P9_VERSION "9P2000.u" #define MAX_TAG_LEN 32 struct p9_msg { diff --git a/tools/kvm/virtio/9p-pdu.c b/tools/kvm/virtio/9p-pdu.c index 0c454db..8ed249f 100644 --- a/tools/kvm/virtio/9p-pdu.c +++ b/tools/kvm/virtio/9p-pdu.c @@ -200,13 +200,15 @@ static int virtio_p9_pdu_encode(struct p9_pdu *pdu, const char *fmt, va_list ap) case 'S': { struct p9_wstat *stbuf = va_arg(ap, struct p9_wstat *); - retval = virtio_p9_pdu_writef(pdu, "wwdQdddqssss", + retval = virtio_p9_pdu_writef(pdu, "wwdQdddqsssssddd", stbuf->size, stbuf->type, stbuf->dev, &stbuf->qid, stbuf->mode, stbuf->atime, stbuf->mtime, stbuf->length, stbuf->name, stbuf->uid, - stbuf->gid, stbuf->muid); + stbuf->gid, stbuf->muid, + stbuf->extension, stbuf->n_uid, + stbuf->n_gid, stbuf->n_muid); } break; default: diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c index 3b5555c..a6eafe0 100644 --- a/tools/kvm/virtio/9p.c +++ b/tools/kvm/virtio/9p.c @@ -162,7 +162,7 @@ static void virtio_p9_error_reply(struct p9_dev *p9dev, err_str = strerror(err); pdu->write_offset = VIRTIO_P9_HDR_LEN; - virtio_p9_pdu_writef(pdu, "s", err_str); + virtio_p9_pdu_writef(pdu, "sd", err_str, -err); *outlen = pdu->write_offset; pdu->read_offset = sizeof(u32) + sizeof(u8); @@ -204,7 +204,6 @@ static void virtio_p9_open(struct p9_dev *p9dev, struct p9_qid qid; struct p9_fid *new_fid; - virtio_p9_pdu_readf(pdu, "db", &fid, &mode); new_fid = &p9dev->fids[fid]; @@ -242,13 +241,14 @@ static void virtio_p9_create(struct p9_dev *p9dev, u8 mode; u32 perm; char *name; + char *ext = NULL; u32 fid_val; struct stat st; struct p9_qid qid; struct p9_fid *fid; char full_path[PATH_MAX]; - virtio_p9_pdu_readf(pdu, "dsdb", &fid_val, &name, &perm, &mode); + virtio_p9_pdu_readf(pdu, "dsdbs", &fid_val, &name, &perm, &mode, &ext); fid = &p9dev->fids[fid_val]; sprintf(full_path, "%s/%s", fid->abs_path, name); @@ -262,6 +262,30 @@ static void virtio_p9_create(struct p9_dev *p9dev, close_fid(p9dev, fid_val); fid->dir = dir; fid->is_dir = 1; + } else if (perm & P9_DMSYMLINK) { + int r; + + r = symlink(ext, full_path); + if (r < 0) + goto err_out; + fd = open(full_path, omode2uflags(mode)); + if (fd < 0) + goto err_out; + close_fid(p9dev, fid_val); + fid->fd = fd; + } else if (perm & P9_DMLINK) { + int r; + int ext_fid = atoi(ext); + + r = link(p9dev->fids[ext_fid].abs_path, full_path); + if (r < 0) + goto err_out; + + fd = open(full_path, omode2uflags(mode)); + if (fd < 0) + goto err_out; + close_fid(p9dev, fid_val); + fid->fd = fd; } else { fd = open(full_path, omode2uflags(mode) | O_CREAT, 0777); if (fd < 0) @@ -294,7 +318,7 @@ static void virtio_p9_walk(struct p9_dev *p9dev, u32 newfid_val; struct p9_qid wqid; struct p9_fid *new_fid; - + int ret; virtio_p9_pdu_readf(pdu, "ddw", &fid_val, &newfid_val, &nwname); new_fid = &p9dev->fids[newfid_val]; @@ -315,8 +339,10 @@ static void virtio_p9_walk(struct p9_dev *p9dev, /* Format the new path we're 'walk'ing into */ sprintf(tmp, "%s/%.*s", fid->path, (int)strlen(str), str); - if (lstat(rel_to_abs(p9dev, tmp, full_path), &st) < 0) + if (lstat(rel_to_abs(p9dev, tmp, full_path), &st) < 0) { + ret = ENOENT; goto err_out; + } st2qid(&st, &wqid); new_fid->is_dir = S_ISDIR(st.st_mode); @@ -340,7 +366,7 @@ static void virtio_p9_walk(struct p9_dev *p9dev, virtio_p9_set_reply_header(pdu, *outlen); return; err_out: - virtio_p9_error_reply(p9dev, pdu, errno, outlen); + virtio_p9_error_reply(p9dev, pdu, ret, outlen); return; } @@ -381,6 +407,12 @@ err_out: return; } +static void virtio_p9_free_stat(struct p9_wstat *wstat) +{ + free(wstat->extension); + free(wstat->name); +} + static void virtio_p9_fill_stat(struct p9_dev *p9dev, const char *name, struct stat *st, struct p9_wstat *wstat) { @@ -393,6 +425,17 @@ static void virtio_p9_fill_stat(struct p9_dev *p9dev, const char *name, wstat->length = 0; wstat->mode |= P9_DMDIR; } + if(S_ISLNK(st->st_mode)) { + char tmp[PATH_MAX] = {0}, full_path[PATH_MAX] = {0}; + + rel_to_abs(p9dev, name, full_path); + + if (readlink(full_path, tmp, PATH_MAX) > 0) + wstat->extension = strdup(tmp); + wstat->mode |= P9_DMSYMLINK; + } else { + wstat->extension = NULL; + } wstat->atime = st->st_atime; wstat->mtime = st->st_mtime; @@ -401,14 +444,20 @@ static void virtio_p9_fill_stat(struct p9_dev *p9dev, const char *name, wstat->uid = NULL; wstat->gid = NULL; wstat->muid = NULL; + wstat->n_uid = wstat->n_gid = wstat->n_muid = 0; - /* NOTE: size shouldn't include its own length */ - /* size[2] type[2] dev[4] qid[13] */ - /* mode[4] atime[4] mtime[4] length[8]*/ - /* name[s] uid[s] gid[s] muid[s] */ - wstat->size = 2+4+13+4+4+4+8+2+2+2+2; + /* + * NOTE: size shouldn't include its own length + * size[2] type[2] dev[4] qid[13] + * mode[4] atime[4] mtime[4] length[8] + * name[s] uid[s] gid[s] muid[s] + * ext[s] uid[4] gid[4] muid[4] + */ + wstat->size = 2+4+13+4+4+4+8+2+2+2+2+2+4+4+4; if (wstat->name) wstat->size += strlen(wstat->name); + if (wstat->extension) + wstat->size += strlen(wstat->extension); } static void virtio_p9_read(struct p9_dev *p9dev, @@ -440,6 +489,7 @@ static void virtio_p9_read(struct p9_dev *p9dev, read = pdu->write_offset; virtio_p9_pdu_writef(pdu, "S", &wstat); rcount += pdu->write_offset - read; + virtio_p9_free_stat(&wstat); cur = readdir(fid->dir); } @@ -486,6 +536,7 @@ static void virtio_p9_stat(struct p9_dev *p9dev, virtio_p9_pdu_writef(pdu, "wS", 0, &wstat); *outlen = pdu->write_offset; + virtio_p9_free_stat(&wstat); virtio_p9_set_reply_header(pdu, *outlen); return; err_out:
This patch adds support for the UNIX extensions to 9p2000. Supporting thses extensions allow us to transperantly mount UNIX directories without missing features such as symlinks. Signed-off-by: Sasha Levin <levinsasha928@gmail.com> --- tools/kvm/include/kvm/virtio-9p.h | 4 +- tools/kvm/virtio/9p-pdu.c | 6 ++- tools/kvm/virtio/9p.c | 75 +++++++++++++++++++++++++++++++------ 3 files changed, 69 insertions(+), 16 deletions(-)