[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [V9fs-developer] [PATCH] virtio-9p: getattr server impl
From: |
Sripathi Kodi |
Subject: |
[Qemu-devel] Re: [V9fs-developer] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol. |
Date: |
Thu, 3 Jun 2010 18:29:02 +0530 |
On Wed, 02 Jun 2010 19:49:24 +0530
"Aneesh Kumar K. V" <address@hidden> wrote:
> On Fri, 28 May 2010 16:08:43 +0530, Sripathi Kodi <address@hidden> wrote:
> > From: M. Mohan Kumar <address@hidden>
> >
> > 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 <address@hidden>
> > Signed-off-by: Sripathi Kodi <address@hidden>
> > ---
> >
> > 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