| Message ID | 20100528103843.4938.29265.stgit@localhost.localdomain |
|---|---|
| State | New |
| Headers | show |
On Fri, 28 May 2010 16:08:43 +0530, Sripathi Kodi <sripathik@in.ibm.com> wrote: > From: M. Mohan Kumar <mohan@in.ibm.com> > > SYNOPSIS > > size[4] Tgetattr tag[2] fid[4] > > size[4] Rgetattr tag[2] lstat[n] > > DESCRIPTION > > The getattr transaction inquires about the file identified by fid. > The reply will contain a machine-independent directory entry, > laid out as follows: > > qid.type[1] > the type of the file (directory, etc.), represented as a bit > vector corresponding to the high 8 bits of the file's mode > word. > > qid.vers[4] > version number for given path > > qid.path[8] > the file server's unique identification for the file > > st_mode[4] > Permission and flags > > st_nlink[8] > Number of hard links > > st_uid[4] > User id of owner > > st_gid[4] > Group ID of owner > > st_rdev[8] > Device ID (if special file) > > st_size[8] > Size, in bytes > > st_blksize[8] > Block size for file system IO So it should be scaled by iounit right ? If we say 9p block size is iounit. > > st_blocks[8] > Number of file system blocks allocated same here. > > st_atime_sec[8] > Time of last access, seconds > > st_atime_nsec[8] > Time of last access, nanoseconds > > st_mtime_sec[8] > Time of last modification, seconds > > st_mtime_nsec[8] > Time of last modification, nanoseconds > > st_ctime_sec[8] > Time of last status change, seconds > > st_ctime_nsec[8] > Time of last status change, nanoseconds > > > This patch implements the client side of getattr implementation for 9P2000.L. > It introduces a new structure p9_stat_dotl for getting Linux stat information > along with QID. The data layout is similar to stat structure in Linux user > space with the following major differences: > > inode (st_ino) is not part of data. Instead qid is. > > device (st_dev) is not part of data because this doesn't make sense on the > client. > > All time variables are 64 bit wide on the wire. The kernel seems to use > 32 bit variables for these variables. However, some of the architectures > have used 64 bit variables and glibc exposes 64 bit variables to user > space on some architectures. Hence to be on the safer side we have made > these 64 bit in the protocol. Refer to the comments in > include/asm-generic/stat.h > > > Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com> > Signed-off-by: Sripathi Kodi <sripathik@in.ibm.com> > --- > > hw/virtio-9p-debug.c | 32 ++++++++++++++++++++ > hw/virtio-9p.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++ > hw/virtio-9p.h | 28 +++++++++++++++++ > 3 files changed, 142 insertions(+), 0 deletions(-) > > diff --git a/hw/virtio-9p-debug.c b/hw/virtio-9p-debug.c > index a82b771..8bb817d 100644 > --- a/hw/virtio-9p-debug.c > +++ b/hw/virtio-9p-debug.c > @@ -178,6 +178,30 @@ static void pprint_stat(V9fsPDU *pdu, int rx, size_t *offsetp, const char *name) > fprintf(llogfile, "}"); > } > > +static void pprint_stat_dotl(V9fsPDU *pdu, int rx, size_t *offsetp, > + const char *name) > +{ > + fprintf(llogfile, "%s={", name); > + pprint_qid(pdu, rx, offsetp, "qid"); > + pprint_int32(pdu, rx, offsetp, ", st_mode"); > + pprint_int64(pdu, rx, offsetp, ", st_nlink"); > + pprint_int32(pdu, rx, offsetp, ", st_uid"); > + pprint_int32(pdu, rx, offsetp, ", st_gid"); > + pprint_int64(pdu, rx, offsetp, ", st_rdev"); > + pprint_int64(pdu, rx, offsetp, ", st_size"); > + pprint_int64(pdu, rx, offsetp, ", st_blksize"); > + pprint_int64(pdu, rx, offsetp, ", st_blocks"); > + pprint_int64(pdu, rx, offsetp, ", atime"); > + pprint_int64(pdu, rx, offsetp, ", atime_nsec"); > + pprint_int64(pdu, rx, offsetp, ", mtime"); > + pprint_int64(pdu, rx, offsetp, ", mtime_nsec"); > + pprint_int64(pdu, rx, offsetp, ", ctime"); > + pprint_int64(pdu, rx, offsetp, ", ctime_nsec"); > + fprintf(llogfile, "}"); > +} > + > + > + > static void pprint_strs(V9fsPDU *pdu, int rx, size_t *offsetp, const char *name) > { > int sg_count = get_sg_count(pdu, rx); > @@ -351,6 +375,14 @@ void pprint_pdu(V9fsPDU *pdu) > pprint_int32(pdu, 1, &offset, "msize"); > pprint_str(pdu, 1, &offset, ", version"); > break; > + case P9_TGETATTR: > + fprintf(llogfile, "TGETATTR: ("); > + pprint_int32(pdu, 0, &offset, "fid"); > + break; > + case P9_RGETATTR: > + fprintf(llogfile, "RGETATTR: ("); > + pprint_stat_dotl(pdu, 1, &offset, "getattr"); > + break; > case P9_TAUTH: > fprintf(llogfile, "TAUTH: ("); > pprint_int32(pdu, 0, &offset, "afid"); > diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c > index 097dce8..23ae8b8 100644 > --- a/hw/virtio-9p.c > +++ b/hw/virtio-9p.c > @@ -737,6 +737,17 @@ static size_t pdu_marshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...) > statp->n_gid, statp->n_muid); > break; > } > + case 'A': { > + V9fsStatDotl *statp = va_arg(ap, V9fsStatDotl *); > + offset += pdu_marshal(pdu, offset, "Qdqddqqqqqqqqqq", > + &statp->qid, statp->st_mode, statp->st_nlink, > + statp->st_uid, statp->st_gid, statp->st_rdev, > + statp->st_size, statp->st_blksize, statp->st_blocks, > + statp->st_atime_sec, statp->st_atime_nsec, > + statp->st_mtime_sec, statp->st_mtime_nsec, > + statp->st_ctime_sec, statp->st_ctime_nsec); > + break; > + } > default: > break; > } > @@ -964,6 +975,29 @@ static int stat_to_v9stat(V9fsState *s, V9fsString *name, > return 0; > } > > +static void stat_to_v9stat_dotl(V9fsState *s, const struct stat *stbuf, > + V9fsStatDotl *v9lstat) > +{ > + memset(v9lstat, 0, sizeof(*v9lstat)); > + > + v9lstat->st_mode = stbuf->st_mode; > + v9lstat->st_nlink = stbuf->st_nlink; > + v9lstat->st_uid = stbuf->st_uid; > + v9lstat->st_gid = stbuf->st_gid; > + v9lstat->st_rdev = stbuf->st_rdev; > + v9lstat->st_size = stbuf->st_size; > + v9lstat->st_blksize = stbuf->st_blksize; > + v9lstat->st_blocks = stbuf->st_blocks; > + v9lstat->st_atime_sec = stbuf->st_atime; > + v9lstat->st_atime_nsec = stbuf->st_atim.tv_nsec; > + v9lstat->st_mtime_sec = stbuf->st_mtime; > + v9lstat->st_mtime_nsec = stbuf->st_mtim.tv_nsec; > + v9lstat->st_ctime_sec = stbuf->st_ctime; > + v9lstat->st_ctime_nsec = stbuf->st_ctim.tv_nsec; > + > + stat_to_qid(stbuf, &v9lstat->qid); > +} > + > static struct iovec *adjust_sg(struct iovec *sg, int len, int *iovcnt) > { > while (len && *iovcnt) { > @@ -1131,6 +1165,53 @@ out: > qemu_free(vs); > } > > +static void v9fs_getattr_post_lstat(V9fsState *s, V9fsStatStateDotl *vs, > + int err) > +{ > + if (err == -1) { > + err = -errno; > + goto out; > + } > + > + stat_to_v9stat_dotl(s, &vs->stbuf, &vs->v9stat_dotl); > + vs->offset += pdu_marshal(vs->pdu, vs->offset, "A", &vs->v9stat_dotl); > + err = vs->offset; > + > +out: > + complete_pdu(s, vs->pdu, err); > + qemu_free(vs); > +} > + > +static void v9fs_getattr(V9fsState *s, V9fsPDU *pdu) > +{ > + int32_t fid; > + V9fsStatStateDotl *vs; > + ssize_t err = 0; > + V9fsFidState *fidp; > + > + vs = qemu_malloc(sizeof(*vs)); > + vs->pdu = pdu; > + vs->offset = 7; > + > + memset(&vs->v9stat_dotl, 0, sizeof(vs->v9stat_dotl)); > + > + pdu_unmarshal(vs->pdu, vs->offset, "d", &fid); > + > + fidp = lookup_fid(s, fid); > + if (fidp == NULL) { > + err = -ENOENT; > + goto out; > + } > + > + err = v9fs_do_lstat(s, &fidp->path, &vs->stbuf); > + v9fs_getattr_post_lstat(s, vs, err); > + return; > + > +out: > + complete_pdu(s, vs->pdu, err); > + qemu_free(vs); > +} > + > static void v9fs_walk_complete(V9fsState *s, V9fsWalkState *vs, int err) > { > complete_pdu(s, vs->pdu, err); > @@ -2343,6 +2424,7 @@ typedef void (pdu_handler_t)(V9fsState *s, V9fsPDU *pdu); > static pdu_handler_t *pdu_handlers[] = { > [P9_TREADDIR] = v9fs_readdir, > [P9_TSTATFS] = v9fs_statfs, > + [P9_TGETATTR] = v9fs_getattr, > [P9_TVERSION] = v9fs_version, > [P9_TATTACH] = v9fs_attach, > [P9_TSTAT] = v9fs_stat, > diff --git a/hw/virtio-9p.h b/hw/virtio-9p.h > index 9773659..7e5609e 100644 > --- a/hw/virtio-9p.h > +++ b/hw/virtio-9p.h > @@ -15,6 +15,8 @@ > enum { > P9_TSTATFS = 8, > P9_RSTATFS, > + P9_TGETATTR = 24, > + P9_RGETATTR, > P9_TREADDIR = 40, > P9_RREADDIR, > P9_TVERSION = 100, > @@ -177,6 +179,32 @@ typedef struct V9fsStatState { > struct stat stbuf; > } V9fsStatState; > > +typedef struct V9fsStatDotl { > + V9fsQID qid; > + uint32_t st_mode; > + uint64_t st_nlink; > + uint32_t st_uid; > + uint32_t st_gid; > + uint64_t st_rdev; > + uint64_t st_size; > + uint64_t st_blksize; > + uint64_t st_blocks; > + uint64_t st_atime_sec; > + uint64_t st_atime_nsec; > + uint64_t st_mtime_sec; > + uint64_t st_mtime_nsec; > + uint64_t st_ctime_sec; > + uint64_t st_ctime_nsec; > +} V9fsStatDotl; > + > +typedef struct V9fsStatStateDotl { > + V9fsPDU *pdu; > + size_t offset; > + V9fsStatDotl v9stat_dotl; > + struct stat stbuf; > +} V9fsStatStateDotl; > + > + > typedef struct V9fsWalkState { > V9fsPDU *pdu; > size_t offset; > -aneesh
On Wed, 02 Jun 2010 19:49:24 +0530 "Aneesh Kumar K. V" <aneesh.kumar@linux.vnet.ibm.com> wrote: > On Fri, 28 May 2010 16:08:43 +0530, Sripathi Kodi <sripathik@in.ibm.com> wrote: > > From: M. Mohan Kumar <mohan@in.ibm.com> > > > > SYNOPSIS > > > > size[4] Tgetattr tag[2] fid[4] > > > > size[4] Rgetattr tag[2] lstat[n] > > > > DESCRIPTION > > > > The getattr transaction inquires about the file identified by fid. > > The reply will contain a machine-independent directory entry, > > laid out as follows: > > > > qid.type[1] > > the type of the file (directory, etc.), represented as a bit > > vector corresponding to the high 8 bits of the file's mode > > word. > > > > qid.vers[4] > > version number for given path > > > > qid.path[8] > > the file server's unique identification for the file > > > > st_mode[4] > > Permission and flags > > > > st_nlink[8] > > Number of hard links > > > > st_uid[4] > > User id of owner > > > > st_gid[4] > > Group ID of owner > > > > st_rdev[8] > > Device ID (if special file) > > > > st_size[8] > > Size, in bytes > > > > st_blksize[8] > > Block size for file system IO > > > So it should be scaled by iounit right ? If we say 9p block size is iounit. Yes, I think it should be iounit. Currently st_blksize being returned in stat structure to the user space does not use this field that comes from the server. It is being calculated as follows in generic_fillattr(): stat->blksize = (1 << inode->i_blkbits); So there may not be a need to put st_blksize on the protocol. Further, inode->i_blkbits is copied from sb->s_blocksize_bits. For 9P this value is obtained as: sb->s_blocksize_bits = fls(v9ses->maxdata - 1); and v9ses->maxdata = v9ses->clnt->msize - P9_IOHDRSZ; Due to the above calculation sometimes stat() on a file can report incorrect values. For example, if I mount 9P file system with msize=5000 stat on a file shows me IO Block: 8192! However, we don't consider this when we do actual file data transfer. We use clnt->msize - P9_IOHDRSZ. Hence it looks to me like i_blkbits is only used to return stat data. > > > > > > st_blocks[8] > > Number of file system blocks allocated > > same here. Yes, this should be file size/iounit. Thanks, Sripathi. > > > > > st_atime_sec[8] > > Time of last access, seconds > > > > st_atime_nsec[8] > > Time of last access, nanoseconds > > > > st_mtime_sec[8] > > Time of last modification, seconds > > > > st_mtime_nsec[8] > > Time of last modification, nanoseconds > > > > st_ctime_sec[8] > > Time of last status change, seconds > > > > st_ctime_nsec[8] > > Time of last status change, nanoseconds > > > > > > This patch implements the client side of getattr implementation for 9P2000.L. > > It introduces a new structure p9_stat_dotl for getting Linux stat information > > along with QID. The data layout is similar to stat structure in Linux user > > space with the following major differences: > > > > inode (st_ino) is not part of data. Instead qid is. > > > > device (st_dev) is not part of data because this doesn't make sense on the > > client. > > > > All time variables are 64 bit wide on the wire. The kernel seems to use > > 32 bit variables for these variables. However, some of the architectures > > have used 64 bit variables and glibc exposes 64 bit variables to user > > space on some architectures. Hence to be on the safer side we have made > > these 64 bit in the protocol. Refer to the comments in > > include/asm-generic/stat.h > > > > > > Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com> > > Signed-off-by: Sripathi Kodi <sripathik@in.ibm.com> > > --- > > > > hw/virtio-9p-debug.c | 32 ++++++++++++++++++++ > > hw/virtio-9p.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > hw/virtio-9p.h | 28 +++++++++++++++++ > > 3 files changed, 142 insertions(+), 0 deletions(-) > > > > diff --git a/hw/virtio-9p-debug.c b/hw/virtio-9p-debug.c > > index a82b771..8bb817d 100644 > > --- a/hw/virtio-9p-debug.c > > +++ b/hw/virtio-9p-debug.c > > @@ -178,6 +178,30 @@ static void pprint_stat(V9fsPDU *pdu, int rx, size_t *offsetp, const char *name) > > fprintf(llogfile, "}"); > > } > > > > +static void pprint_stat_dotl(V9fsPDU *pdu, int rx, size_t *offsetp, > > + const char *name) > > +{ > > + fprintf(llogfile, "%s={", name); > > + pprint_qid(pdu, rx, offsetp, "qid"); > > + pprint_int32(pdu, rx, offsetp, ", st_mode"); > > + pprint_int64(pdu, rx, offsetp, ", st_nlink"); > > + pprint_int32(pdu, rx, offsetp, ", st_uid"); > > + pprint_int32(pdu, rx, offsetp, ", st_gid"); > > + pprint_int64(pdu, rx, offsetp, ", st_rdev"); > > + pprint_int64(pdu, rx, offsetp, ", st_size"); > > + pprint_int64(pdu, rx, offsetp, ", st_blksize"); > > + pprint_int64(pdu, rx, offsetp, ", st_blocks"); > > + pprint_int64(pdu, rx, offsetp, ", atime"); > > + pprint_int64(pdu, rx, offsetp, ", atime_nsec"); > > + pprint_int64(pdu, rx, offsetp, ", mtime"); > > + pprint_int64(pdu, rx, offsetp, ", mtime_nsec"); > > + pprint_int64(pdu, rx, offsetp, ", ctime"); > > + pprint_int64(pdu, rx, offsetp, ", ctime_nsec"); > > + fprintf(llogfile, "}"); > > +} > > + > > + > > + > > static void pprint_strs(V9fsPDU *pdu, int rx, size_t *offsetp, const char *name) > > { > > int sg_count = get_sg_count(pdu, rx); > > @@ -351,6 +375,14 @@ void pprint_pdu(V9fsPDU *pdu) > > pprint_int32(pdu, 1, &offset, "msize"); > > pprint_str(pdu, 1, &offset, ", version"); > > break; > > + case P9_TGETATTR: > > + fprintf(llogfile, "TGETATTR: ("); > > + pprint_int32(pdu, 0, &offset, "fid"); > > + break; > > + case P9_RGETATTR: > > + fprintf(llogfile, "RGETATTR: ("); > > + pprint_stat_dotl(pdu, 1, &offset, "getattr"); > > + break; > > case P9_TAUTH: > > fprintf(llogfile, "TAUTH: ("); > > pprint_int32(pdu, 0, &offset, "afid"); > > diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c > > index 097dce8..23ae8b8 100644 > > --- a/hw/virtio-9p.c > > +++ b/hw/virtio-9p.c > > @@ -737,6 +737,17 @@ static size_t pdu_marshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...) > > statp->n_gid, statp->n_muid); > > break; > > } > > + case 'A': { > > + V9fsStatDotl *statp = va_arg(ap, V9fsStatDotl *); > > + offset += pdu_marshal(pdu, offset, "Qdqddqqqqqqqqqq", > > + &statp->qid, statp->st_mode, statp->st_nlink, > > + statp->st_uid, statp->st_gid, statp->st_rdev, > > + statp->st_size, statp->st_blksize, statp->st_blocks, > > + statp->st_atime_sec, statp->st_atime_nsec, > > + statp->st_mtime_sec, statp->st_mtime_nsec, > > + statp->st_ctime_sec, statp->st_ctime_nsec); > > + break; > > + } > > default: > > break; > > } > > @@ -964,6 +975,29 @@ static int stat_to_v9stat(V9fsState *s, V9fsString *name, > > return 0; > > } > > > > +static void stat_to_v9stat_dotl(V9fsState *s, const struct stat *stbuf, > > + V9fsStatDotl *v9lstat) > > +{ > > + memset(v9lstat, 0, sizeof(*v9lstat)); > > + > > + v9lstat->st_mode = stbuf->st_mode; > > + v9lstat->st_nlink = stbuf->st_nlink; > > + v9lstat->st_uid = stbuf->st_uid; > > + v9lstat->st_gid = stbuf->st_gid; > > + v9lstat->st_rdev = stbuf->st_rdev; > > + v9lstat->st_size = stbuf->st_size; > > + v9lstat->st_blksize = stbuf->st_blksize; > > + v9lstat->st_blocks = stbuf->st_blocks; > > + v9lstat->st_atime_sec = stbuf->st_atime; > > + v9lstat->st_atime_nsec = stbuf->st_atim.tv_nsec; > > + v9lstat->st_mtime_sec = stbuf->st_mtime; > > + v9lstat->st_mtime_nsec = stbuf->st_mtim.tv_nsec; > > + v9lstat->st_ctime_sec = stbuf->st_ctime; > > + v9lstat->st_ctime_nsec = stbuf->st_ctim.tv_nsec; > > + > > + stat_to_qid(stbuf, &v9lstat->qid); > > +} > > + > > static struct iovec *adjust_sg(struct iovec *sg, int len, int *iovcnt) > > { > > while (len && *iovcnt) { > > @@ -1131,6 +1165,53 @@ out: > > qemu_free(vs); > > } > > > > +static void v9fs_getattr_post_lstat(V9fsState *s, V9fsStatStateDotl *vs, > > + int err) > > +{ > > + if (err == -1) { > > + err = -errno; > > + goto out; > > + } > > + > > + stat_to_v9stat_dotl(s, &vs->stbuf, &vs->v9stat_dotl); > > + vs->offset += pdu_marshal(vs->pdu, vs->offset, "A", &vs->v9stat_dotl); > > + err = vs->offset; > > + > > +out: > > + complete_pdu(s, vs->pdu, err); > > + qemu_free(vs); > > +} > > + > > +static void v9fs_getattr(V9fsState *s, V9fsPDU *pdu) > > +{ > > + int32_t fid; > > + V9fsStatStateDotl *vs; > > + ssize_t err = 0; > > + V9fsFidState *fidp; > > + > > + vs = qemu_malloc(sizeof(*vs)); > > + vs->pdu = pdu; > > + vs->offset = 7; > > + > > + memset(&vs->v9stat_dotl, 0, sizeof(vs->v9stat_dotl)); > > + > > + pdu_unmarshal(vs->pdu, vs->offset, "d", &fid); > > + > > + fidp = lookup_fid(s, fid); > > + if (fidp == NULL) { > > + err = -ENOENT; > > + goto out; > > + } > > + > > + err = v9fs_do_lstat(s, &fidp->path, &vs->stbuf); > > + v9fs_getattr_post_lstat(s, vs, err); > > + return; > > + > > +out: > > + complete_pdu(s, vs->pdu, err); > > + qemu_free(vs); > > +} > > + > > static void v9fs_walk_complete(V9fsState *s, V9fsWalkState *vs, int err) > > { > > complete_pdu(s, vs->pdu, err); > > @@ -2343,6 +2424,7 @@ typedef void (pdu_handler_t)(V9fsState *s, V9fsPDU *pdu); > > static pdu_handler_t *pdu_handlers[] = { > > [P9_TREADDIR] = v9fs_readdir, > > [P9_TSTATFS] = v9fs_statfs, > > + [P9_TGETATTR] = v9fs_getattr, > > [P9_TVERSION] = v9fs_version, > > [P9_TATTACH] = v9fs_attach, > > [P9_TSTAT] = v9fs_stat, > > diff --git a/hw/virtio-9p.h b/hw/virtio-9p.h > > index 9773659..7e5609e 100644 > > --- a/hw/virtio-9p.h > > +++ b/hw/virtio-9p.h > > @@ -15,6 +15,8 @@ > > enum { > > P9_TSTATFS = 8, > > P9_RSTATFS, > > + P9_TGETATTR = 24, > > + P9_RGETATTR, > > P9_TREADDIR = 40, > > P9_RREADDIR, > > P9_TVERSION = 100, > > @@ -177,6 +179,32 @@ typedef struct V9fsStatState { > > struct stat stbuf; > > } V9fsStatState; > > > > +typedef struct V9fsStatDotl { > > + V9fsQID qid; > > + uint32_t st_mode; > > + uint64_t st_nlink; > > + uint32_t st_uid; > > + uint32_t st_gid; > > + uint64_t st_rdev; > > + uint64_t st_size; > > + uint64_t st_blksize; > > + uint64_t st_blocks; > > + uint64_t st_atime_sec; > > + uint64_t st_atime_nsec; > > + uint64_t st_mtime_sec; > > + uint64_t st_mtime_nsec; > > + uint64_t st_ctime_sec; > > + uint64_t st_ctime_nsec; > > +} V9fsStatDotl; > > + > > +typedef struct V9fsStatStateDotl { > > + V9fsPDU *pdu; > > + size_t offset; > > + V9fsStatDotl v9stat_dotl; > > + struct stat stbuf; > > +} V9fsStatStateDotl; > > + > > + > > typedef struct V9fsWalkState { > > V9fsPDU *pdu; > > size_t offset; > > > > > -aneesh
On Thu, 3 Jun 2010 18:29:02 +0530, Sripathi Kodi <sripathik@in.ibm.com> wrote: > On Wed, 02 Jun 2010 19:49:24 +0530 > "Aneesh Kumar K. V" <aneesh.kumar@linux.vnet.ibm.com> wrote: > > > On Fri, 28 May 2010 16:08:43 +0530, Sripathi Kodi <sripathik@in.ibm.com> wrote: > > > From: M. Mohan Kumar <mohan@in.ibm.com> > > > > > > SYNOPSIS > > > > > > size[4] Tgetattr tag[2] fid[4] > > > > > > size[4] Rgetattr tag[2] lstat[n] > > > > > > DESCRIPTION > > > > > > The getattr transaction inquires about the file identified by fid. > > > The reply will contain a machine-independent directory entry, > > > laid out as follows: > > > > > > qid.type[1] > > > the type of the file (directory, etc.), represented as a bit > > > vector corresponding to the high 8 bits of the file's mode > > > word. > > > > > > qid.vers[4] > > > version number for given path > > > > > > qid.path[8] > > > the file server's unique identification for the file > > > > > > st_mode[4] > > > Permission and flags > > > > > > st_nlink[8] > > > Number of hard links > > > > > > st_uid[4] > > > User id of owner > > > > > > st_gid[4] > > > Group ID of owner > > > > > > st_rdev[8] > > > Device ID (if special file) > > > > > > st_size[8] > > > Size, in bytes > > > > > > st_blksize[8] > > > Block size for file system IO > > > > > > So it should be scaled by iounit right ? If we say 9p block size is iounit. > > Yes, I think it should be iounit. Currently st_blksize being returned > in stat structure to the user space does not use this field that comes > from the server. It is being calculated as follows in > generic_fillattr(): > > stat->blksize = (1 << inode->i_blkbits); > > So there may not be a need to put st_blksize on the protocol. Further, > inode->i_blkbits is copied from sb->s_blocksize_bits. For 9P this value > is obtained as: That is what linux kernel currently does. But from the protocol point of view and not looking at specific linux implementation i would suggest to put st_blksize on wire. -aneesh
Aneesh Kumar K. V wrote: > On Thu, 3 Jun 2010 18:29:02 +0530, Sripathi Kodi <sripathik@in.ibm.com> wrote: >> On Wed, 02 Jun 2010 19:49:24 +0530 >> "Aneesh Kumar K. V" <aneesh.kumar@linux.vnet.ibm.com> wrote: >> >>> On Fri, 28 May 2010 16:08:43 +0530, Sripathi Kodi <sripathik@in.ibm.com> wrote: >>>> From: M. Mohan Kumar <mohan@in.ibm.com> >>>> >>>> SYNOPSIS >>>> >>>> size[4] Tgetattr tag[2] fid[4] >>>> >>>> size[4] Rgetattr tag[2] lstat[n] >>>> >>>> DESCRIPTION >>>> >>>> The getattr transaction inquires about the file identified by fid. >>>> The reply will contain a machine-independent directory entry, >>>> laid out as follows: >>>> >>>> qid.type[1] >>>> the type of the file (directory, etc.), represented as a bit >>>> vector corresponding to the high 8 bits of the file's mode >>>> word. >>>> >>>> qid.vers[4] >>>> version number for given path >>>> >>>> qid.path[8] >>>> the file server's unique identification for the file >>>> >>>> st_mode[4] >>>> Permission and flags >>>> >>>> st_nlink[8] >>>> Number of hard links >>>> >>>> st_uid[4] >>>> User id of owner >>>> >>>> st_gid[4] >>>> Group ID of owner >>>> >>>> st_rdev[8] >>>> Device ID (if special file) >>>> >>>> st_size[8] >>>> Size, in bytes >>>> >>>> st_blksize[8] >>>> Block size for file system IO >>> >>> So it should be scaled by iounit right ? If we say 9p block size is iounit. >> Yes, I think it should be iounit. Currently st_blksize being returned >> in stat structure to the user space does not use this field that comes >> from the server. It is being calculated as follows in >> generic_fillattr(): >> >> stat->blksize = (1 << inode->i_blkbits); >> >> So there may not be a need to put st_blksize on the protocol. Further, >> inode->i_blkbits is copied from sb->s_blocksize_bits. For 9P this value >> is obtained as: > > That is what linux kernel currently does. But from the protocol point of > view and not looking at specific linux implementation i would suggest to > put st_blksize on wire. This is part of .L protocol. Specifically for Linux systems. So, if Linux is already doing it, I don't think we need to repeat it. Thanks, JV > > > -aneesh >
On Fri, 04 Jun 2010 07:59:42 -0700, "Venkateswararao Jujjuri (JV)" <jvrao@linux.vnet.ibm.com> wrote: > Aneesh Kumar K. V wrote: > > On Thu, 3 Jun 2010 18:29:02 +0530, Sripathi Kodi <sripathik@in.ibm.com> wrote: > >> On Wed, 02 Jun 2010 19:49:24 +0530 > >> "Aneesh Kumar K. V" <aneesh.kumar@linux.vnet.ibm.com> wrote: > >> > >>> On Fri, 28 May 2010 16:08:43 +0530, Sripathi Kodi <sripathik@in.ibm.com> wrote: > >>>> From: M. Mohan Kumar <mohan@in.ibm.com> > >>>> > >>>> SYNOPSIS > >>>> > >>>> size[4] Tgetattr tag[2] fid[4] > >>>> > >>>> size[4] Rgetattr tag[2] lstat[n] > >>>> > >>>> DESCRIPTION > >>>> > >>>> The getattr transaction inquires about the file identified by fid. > >>>> The reply will contain a machine-independent directory entry, > >>>> laid out as follows: > >>>> > >>>> qid.type[1] > >>>> the type of the file (directory, etc.), represented as a bit > >>>> vector corresponding to the high 8 bits of the file's mode > >>>> word. > >>>> > >>>> qid.vers[4] > >>>> version number for given path > >>>> > >>>> qid.path[8] > >>>> the file server's unique identification for the file > >>>> > >>>> st_mode[4] > >>>> Permission and flags > >>>> > >>>> st_nlink[8] > >>>> Number of hard links > >>>> > >>>> st_uid[4] > >>>> User id of owner > >>>> > >>>> st_gid[4] > >>>> Group ID of owner > >>>> > >>>> st_rdev[8] > >>>> Device ID (if special file) > >>>> > >>>> st_size[8] > >>>> Size, in bytes > >>>> > >>>> st_blksize[8] > >>>> Block size for file system IO > >>> > >>> So it should be scaled by iounit right ? If we say 9p block size is iounit. > >> Yes, I think it should be iounit. Currently st_blksize being returned > >> in stat structure to the user space does not use this field that comes > >> from the server. It is being calculated as follows in > >> generic_fillattr(): > >> > >> stat->blksize = (1 << inode->i_blkbits); > >> > >> So there may not be a need to put st_blksize on the protocol. Further, > >> inode->i_blkbits is copied from sb->s_blocksize_bits. For 9P this value > >> is obtained as: > > > > That is what linux kernel currently does. But from the protocol point of > > view and not looking at specific linux implementation i would suggest to > > put st_blksize on wire. > > This is part of .L protocol. Specifically for Linux systems. So, if Linux is already > doing it, I don't think we need to repeat it. > But nothing prevents from changing Linux internal implementation. So we can't depend on Linux kernel internal implementation. Later in 2.6.x we may not derive stat->blksize from inode->i_blkbits at all. So we cannot depend on Linux kernel internal implementation. -aneesh
On Fri, 28 May 2010 16:08:43 +0530, Sripathi Kodi <sripathik@in.ibm.com> wrote: > From: M. Mohan Kumar <mohan@in.ibm.com> > > SYNOPSIS > > size[4] Tgetattr tag[2] fid[4] > > size[4] Rgetattr tag[2] lstat[n] > > DESCRIPTION > > The getattr transaction inquires about the file identified by fid. > The reply will contain a machine-independent directory entry, > laid out as follows: > > qid.type[1] > the type of the file (directory, etc.), represented as a bit > vector corresponding to the high 8 bits of the file's mode > word. > > qid.vers[4] > version number for given path > > qid.path[8] > the file server's unique identification for the file > > st_mode[4] > Permission and flags > > st_nlink[8] > Number of hard links > > st_uid[4] > User id of owner > > st_gid[4] > Group ID of owner > > st_rdev[8] > Device ID (if special file) > > st_size[8] > Size, in bytes > > st_blksize[8] > Block size for file system IO > > st_blocks[8] > Number of file system blocks allocated > > st_atime_sec[8] > Time of last access, seconds > > st_atime_nsec[8] > Time of last access, nanoseconds > > st_mtime_sec[8] > Time of last modification, seconds > > st_mtime_nsec[8] > Time of last modification, nanoseconds > > st_ctime_sec[8] > Time of last status change, seconds > > st_ctime_nsec[8] > Time of last status change, nanoseconds > > > This patch implements the client side of getattr implementation for 9P2000.L. > It introduces a new structure p9_stat_dotl for getting Linux stat information > along with QID. The data layout is similar to stat structure in Linux user > space with the following major differences: > > inode (st_ino) is not part of data. Instead qid is. > > device (st_dev) is not part of data because this doesn't make sense on the > client. > > All time variables are 64 bit wide on the wire. The kernel seems to use > 32 bit variables for these variables. However, some of the architectures > have used 64 bit variables and glibc exposes 64 bit variables to user > space on some architectures. Hence to be on the safer side we have made > these 64 bit in the protocol. Refer to the comments in > include/asm-generic/stat.h > > Can we just hold on this patch. There is a discussion to add i_generation and inode create time to a variant of stat. So may be the protocol bits need those -aneesh
On Thu, 01 Jul 2010 11:01:15 +0530 "Aneesh Kumar K. V" <aneesh.kumar@linux.vnet.ibm.com> wrote: > On Fri, 28 May 2010 16:08:43 +0530, Sripathi Kodi <sripathik@in.ibm.com> wrote: > > From: M. Mohan Kumar <mohan@in.ibm.com> > > > > SYNOPSIS > > > > size[4] Tgetattr tag[2] fid[4] > > > > size[4] Rgetattr tag[2] lstat[n] > > > > DESCRIPTION > > > > The getattr transaction inquires about the file identified by fid. > > The reply will contain a machine-independent directory entry, > > laid out as follows: > > > > qid.type[1] > > the type of the file (directory, etc.), represented as a bit > > vector corresponding to the high 8 bits of the file's mode > > word. > > > > qid.vers[4] > > version number for given path > > > > qid.path[8] > > the file server's unique identification for the file > > > > st_mode[4] > > Permission and flags > > > > st_nlink[8] > > Number of hard links > > > > st_uid[4] > > User id of owner > > > > st_gid[4] > > Group ID of owner > > > > st_rdev[8] > > Device ID (if special file) > > > > st_size[8] > > Size, in bytes > > > > st_blksize[8] > > Block size for file system IO > > > > st_blocks[8] > > Number of file system blocks allocated > > > > st_atime_sec[8] > > Time of last access, seconds > > > > st_atime_nsec[8] > > Time of last access, nanoseconds > > > > st_mtime_sec[8] > > Time of last modification, seconds > > > > st_mtime_nsec[8] > > Time of last modification, nanoseconds > > > > st_ctime_sec[8] > > Time of last status change, seconds > > > > st_ctime_nsec[8] > > Time of last status change, nanoseconds > > > > > > This patch implements the client side of getattr implementation for 9P2000.L. > > It introduces a new structure p9_stat_dotl for getting Linux stat information > > along with QID. The data layout is similar to stat structure in Linux user > > space with the following major differences: > > > > inode (st_ino) is not part of data. Instead qid is. > > > > device (st_dev) is not part of data because this doesn't make sense on the > > client. > > > > All time variables are 64 bit wide on the wire. The kernel seems to use > > 32 bit variables for these variables. However, some of the architectures > > have used 64 bit variables and glibc exposes 64 bit variables to user > > space on some architectures. Hence to be on the safer side we have made > > these 64 bit in the protocol. Refer to the comments in > > include/asm-generic/stat.h > > > > > > Can we just hold on this patch. There is a discussion to add > i_generation and inode create time to a variant of stat. So may be the > protocol bits need those > IMHO, we can put this in now and change it later if needed based on how the discussion about VFS changes go because: a) 9P2000.L is still at experimental stage, so it allows us to change the protocol later. b) The kernel patch for this is already in linux-next. Without these patches in QEMU it won't be possible to use 9P2000.L. Thanks, Sripathi. > -aneesh >
On Thu, 1 Jul 2010 14:26:13 +0530, Sripathi Kodi <sripathik@in.ibm.com> wrote: > On Thu, 01 Jul 2010 11:01:15 +0530 > "Aneesh Kumar K. V" <aneesh.kumar@linux.vnet.ibm.com> wrote: > > > On Fri, 28 May 2010 16:08:43 +0530, Sripathi Kodi <sripathik@in.ibm.com> wrote: > > > From: M. Mohan Kumar <mohan@in.ibm.com> > > > > > > SYNOPSIS > > > > > > size[4] Tgetattr tag[2] fid[4] > > > > > > size[4] Rgetattr tag[2] lstat[n] > > > > > > DESCRIPTION > > > > > > The getattr transaction inquires about the file identified by fid. > > > The reply will contain a machine-independent directory entry, > > > laid out as follows: > > > > > > qid.type[1] > > > the type of the file (directory, etc.), represented as a bit > > > vector corresponding to the high 8 bits of the file's mode > > > word. > > > > > > qid.vers[4] > > > version number for given path > > > > > > qid.path[8] > > > the file server's unique identification for the file > > > > > > st_mode[4] > > > Permission and flags > > > > > > st_nlink[8] > > > Number of hard links > > > > > > st_uid[4] > > > User id of owner > > > > > > st_gid[4] > > > Group ID of owner > > > > > > st_rdev[8] > > > Device ID (if special file) > > > > > > st_size[8] > > > Size, in bytes > > > > > > st_blksize[8] > > > Block size for file system IO > > > > > > st_blocks[8] > > > Number of file system blocks allocated > > > > > > st_atime_sec[8] > > > Time of last access, seconds > > > > > > st_atime_nsec[8] > > > Time of last access, nanoseconds > > > > > > st_mtime_sec[8] > > > Time of last modification, seconds > > > > > > st_mtime_nsec[8] > > > Time of last modification, nanoseconds > > > > > > st_ctime_sec[8] > > > Time of last status change, seconds > > > > > > st_ctime_nsec[8] > > > Time of last status change, nanoseconds > > > > > > > > > This patch implements the client side of getattr implementation for 9P2000.L. > > > It introduces a new structure p9_stat_dotl for getting Linux stat information > > > along with QID. The data layout is similar to stat structure in Linux user > > > space with the following major differences: > > > > > > inode (st_ino) is not part of data. Instead qid is. > > > > > > device (st_dev) is not part of data because this doesn't make sense on the > > > client. > > > > > > All time variables are 64 bit wide on the wire. The kernel seems to use > > > 32 bit variables for these variables. However, some of the architectures > > > have used 64 bit variables and glibc exposes 64 bit variables to user > > > space on some architectures. Hence to be on the safer side we have made > > > these 64 bit in the protocol. Refer to the comments in > > > include/asm-generic/stat.h > > > > > > > > > > Can we just hold on this patch. There is a discussion to add > > i_generation and inode create time to a variant of stat. So may be the > > protocol bits need those > > > > IMHO, we can put this in now and change it later if needed based on how > the discussion about VFS changes go because: > a) 9P2000.L is still at experimental stage, so it allows us to change > the protocol later. > b) The kernel patch for this is already in linux-next. Without these > patches in QEMU it won't be possible to use 9P2000.L. > The comment was w.r.t kernel and qemu patches. I am not sure whether we would reach a conclusion about how the syscall interface soon. But i guess BSD already support birth time. So in any way having a protocol update to support i_generation and birth time is a good thing to do, because we already know that NFS and CIFS would need it from a file system. -aneesh
diff --git a/hw/virtio-9p-debug.c b/hw/virtio-9p-debug.c index a82b771..8bb817d 100644 --- a/hw/virtio-9p-debug.c +++ b/hw/virtio-9p-debug.c @@ -178,6 +178,30 @@ static void pprint_stat(V9fsPDU *pdu, int rx, size_t *offsetp, const char *name) fprintf(llogfile, "}"); } +static void pprint_stat_dotl(V9fsPDU *pdu, int rx, size_t *offsetp, + const char *name) +{ + fprintf(llogfile, "%s={", name); + pprint_qid(pdu, rx, offsetp, "qid"); + pprint_int32(pdu, rx, offsetp, ", st_mode"); + pprint_int64(pdu, rx, offsetp, ", st_nlink"); + pprint_int32(pdu, rx, offsetp, ", st_uid"); + pprint_int32(pdu, rx, offsetp, ", st_gid"); + pprint_int64(pdu, rx, offsetp, ", st_rdev"); + pprint_int64(pdu, rx, offsetp, ", st_size"); + pprint_int64(pdu, rx, offsetp, ", st_blksize"); + pprint_int64(pdu, rx, offsetp, ", st_blocks"); + pprint_int64(pdu, rx, offsetp, ", atime"); + pprint_int64(pdu, rx, offsetp, ", atime_nsec"); + pprint_int64(pdu, rx, offsetp, ", mtime"); + pprint_int64(pdu, rx, offsetp, ", mtime_nsec"); + pprint_int64(pdu, rx, offsetp, ", ctime"); + pprint_int64(pdu, rx, offsetp, ", ctime_nsec"); + fprintf(llogfile, "}"); +} + + + static void pprint_strs(V9fsPDU *pdu, int rx, size_t *offsetp, const char *name) { int sg_count = get_sg_count(pdu, rx); @@ -351,6 +375,14 @@ void pprint_pdu(V9fsPDU *pdu) pprint_int32(pdu, 1, &offset, "msize"); pprint_str(pdu, 1, &offset, ", version"); break; + case P9_TGETATTR: + fprintf(llogfile, "TGETATTR: ("); + pprint_int32(pdu, 0, &offset, "fid"); + break; + case P9_RGETATTR: + fprintf(llogfile, "RGETATTR: ("); + pprint_stat_dotl(pdu, 1, &offset, "getattr"); + break; case P9_TAUTH: fprintf(llogfile, "TAUTH: ("); pprint_int32(pdu, 0, &offset, "afid"); diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c index 097dce8..23ae8b8 100644 --- a/hw/virtio-9p.c +++ b/hw/virtio-9p.c @@ -737,6 +737,17 @@ static size_t pdu_marshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...) statp->n_gid, statp->n_muid); break; } + case 'A': { + V9fsStatDotl *statp = va_arg(ap, V9fsStatDotl *); + offset += pdu_marshal(pdu, offset, "Qdqddqqqqqqqqqq", + &statp->qid, statp->st_mode, statp->st_nlink, + statp->st_uid, statp->st_gid, statp->st_rdev, + statp->st_size, statp->st_blksize, statp->st_blocks, + statp->st_atime_sec, statp->st_atime_nsec, + statp->st_mtime_sec, statp->st_mtime_nsec, + statp->st_ctime_sec, statp->st_ctime_nsec); + break; + } default: break; } @@ -964,6 +975,29 @@ static int stat_to_v9stat(V9fsState *s, V9fsString *name, return 0; } +static void stat_to_v9stat_dotl(V9fsState *s, const struct stat *stbuf, + V9fsStatDotl *v9lstat) +{ + memset(v9lstat, 0, sizeof(*v9lstat)); + + v9lstat->st_mode = stbuf->st_mode; + v9lstat->st_nlink = stbuf->st_nlink; + v9lstat->st_uid = stbuf->st_uid; + v9lstat->st_gid = stbuf->st_gid; + v9lstat->st_rdev = stbuf->st_rdev; + v9lstat->st_size = stbuf->st_size; + v9lstat->st_blksize = stbuf->st_blksize; + v9lstat->st_blocks = stbuf->st_blocks; + v9lstat->st_atime_sec = stbuf->st_atime; + v9lstat->st_atime_nsec = stbuf->st_atim.tv_nsec; + v9lstat->st_mtime_sec = stbuf->st_mtime; + v9lstat->st_mtime_nsec = stbuf->st_mtim.tv_nsec; + v9lstat->st_ctime_sec = stbuf->st_ctime; + v9lstat->st_ctime_nsec = stbuf->st_ctim.tv_nsec; + + stat_to_qid(stbuf, &v9lstat->qid); +} + static struct iovec *adjust_sg(struct iovec *sg, int len, int *iovcnt) { while (len && *iovcnt) { @@ -1131,6 +1165,53 @@ out: qemu_free(vs); } +static void v9fs_getattr_post_lstat(V9fsState *s, V9fsStatStateDotl *vs, + int err) +{ + if (err == -1) { + err = -errno; + goto out; + } + + stat_to_v9stat_dotl(s, &vs->stbuf, &vs->v9stat_dotl); + vs->offset += pdu_marshal(vs->pdu, vs->offset, "A", &vs->v9stat_dotl); + err = vs->offset; + +out: + complete_pdu(s, vs->pdu, err); + qemu_free(vs); +} + +static void v9fs_getattr(V9fsState *s, V9fsPDU *pdu) +{ + int32_t fid; + V9fsStatStateDotl *vs; + ssize_t err = 0; + V9fsFidState *fidp; + + vs = qemu_malloc(sizeof(*vs)); + vs->pdu = pdu; + vs->offset = 7; + + memset(&vs->v9stat_dotl, 0, sizeof(vs->v9stat_dotl)); + + pdu_unmarshal(vs->pdu, vs->offset, "d", &fid); + + fidp = lookup_fid(s, fid); + if (fidp == NULL) { + err = -ENOENT; + goto out; + } + + err = v9fs_do_lstat(s, &fidp->path, &vs->stbuf); + v9fs_getattr_post_lstat(s, vs, err); + return; + +out: + complete_pdu(s, vs->pdu, err); + qemu_free(vs); +} + static void v9fs_walk_complete(V9fsState *s, V9fsWalkState *vs, int err) { complete_pdu(s, vs->pdu, err); @@ -2343,6 +2424,7 @@ typedef void (pdu_handler_t)(V9fsState *s, V9fsPDU *pdu); static pdu_handler_t *pdu_handlers[] = { [P9_TREADDIR] = v9fs_readdir, [P9_TSTATFS] = v9fs_statfs, + [P9_TGETATTR] = v9fs_getattr, [P9_TVERSION] = v9fs_version, [P9_TATTACH] = v9fs_attach, [P9_TSTAT] = v9fs_stat, diff --git a/hw/virtio-9p.h b/hw/virtio-9p.h index 9773659..7e5609e 100644 --- a/hw/virtio-9p.h +++ b/hw/virtio-9p.h @@ -15,6 +15,8 @@ enum { P9_TSTATFS = 8, P9_RSTATFS, + P9_TGETATTR = 24, + P9_RGETATTR, P9_TREADDIR = 40, P9_RREADDIR, P9_TVERSION = 100, @@ -177,6 +179,32 @@ typedef struct V9fsStatState { struct stat stbuf; } V9fsStatState; +typedef struct V9fsStatDotl { + V9fsQID qid; + uint32_t st_mode; + uint64_t st_nlink; + uint32_t st_uid; + uint32_t st_gid; + uint64_t st_rdev; + uint64_t st_size; + uint64_t st_blksize; + uint64_t st_blocks; + uint64_t st_atime_sec; + uint64_t st_atime_nsec; + uint64_t st_mtime_sec; + uint64_t st_mtime_nsec; + uint64_t st_ctime_sec; + uint64_t st_ctime_nsec; +} V9fsStatDotl; + +typedef struct V9fsStatStateDotl { + V9fsPDU *pdu; + size_t offset; + V9fsStatDotl v9stat_dotl; + struct stat stbuf; +} V9fsStatStateDotl; + + typedef struct V9fsWalkState { V9fsPDU *pdu; size_t offset;