* [Qemu-devel] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.
@ 2010-05-28 10:38 Sripathi Kodi
2010-06-02 14:19 ` [Qemu-devel] Re: [V9fs-developer] " Aneesh Kumar K. V
2010-07-01 5:31 ` [Qemu-devel] Re: [V9fs-developer] " Aneesh Kumar K. V
0 siblings, 2 replies; 12+ messages in thread
From: Sripathi Kodi @ 2010-05-28 10:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Sripathi Kodi, v9fs-developer, M. Mohan Kumar
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
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;
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] Re: [V9fs-developer] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.
2010-05-28 10:38 [Qemu-devel] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol Sripathi Kodi
@ 2010-06-02 14:19 ` Aneesh Kumar K. V
2010-06-03 12:59 ` Sripathi Kodi
2010-07-01 5:31 ` [Qemu-devel] Re: [V9fs-developer] " Aneesh Kumar K. V
1 sibling, 1 reply; 12+ messages in thread
From: Aneesh Kumar K. V @ 2010-06-02 14:19 UTC (permalink / raw)
To: Sripathi Kodi, qemu-devel; +Cc: v9fs-developer
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
^ permalink raw reply [flat|nested] 12+ messages in thread* [Qemu-devel] Re: [V9fs-developer] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.
2010-06-02 14:19 ` [Qemu-devel] Re: [V9fs-developer] " Aneesh Kumar K. V
@ 2010-06-03 12:59 ` Sripathi Kodi
2010-06-04 8:28 ` Aneesh Kumar K. V
0 siblings, 1 reply; 12+ messages in thread
From: Sripathi Kodi @ 2010-06-03 12:59 UTC (permalink / raw)
To: Aneesh Kumar K. V; +Cc: v9fs-developer, qemu-devel
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
^ permalink raw reply [flat|nested] 12+ messages in thread* [Qemu-devel] Re: [V9fs-developer] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.
2010-06-03 12:59 ` Sripathi Kodi
@ 2010-06-04 8:28 ` Aneesh Kumar K. V
2010-06-04 14:59 ` Venkateswararao Jujjuri (JV)
0 siblings, 1 reply; 12+ messages in thread
From: Aneesh Kumar K. V @ 2010-06-04 8:28 UTC (permalink / raw)
To: Sripathi Kodi; +Cc: v9fs-developer, qemu-devel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] Re: [V9fs-developer] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.
2010-06-04 8:28 ` Aneesh Kumar K. V
@ 2010-06-04 14:59 ` Venkateswararao Jujjuri (JV)
2010-06-05 13:41 ` Aneesh Kumar K. V
0 siblings, 1 reply; 12+ messages in thread
From: Venkateswararao Jujjuri (JV) @ 2010-06-04 14:59 UTC (permalink / raw)
To: Aneesh Kumar K. V; +Cc: v9fs-developer, Sripathi Kodi, qemu-devel
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
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] Re: [V9fs-developer] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.
2010-06-04 14:59 ` Venkateswararao Jujjuri (JV)
@ 2010-06-05 13:41 ` Aneesh Kumar K. V
2010-06-07 10:34 ` Sripathi Kodi
0 siblings, 1 reply; 12+ messages in thread
From: Aneesh Kumar K. V @ 2010-06-05 13:41 UTC (permalink / raw)
To: Venkateswararao Jujjuri (JV); +Cc: v9fs-developer, Sripathi Kodi, qemu-devel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] Re: [V9fs-developer] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.
2010-06-05 13:41 ` Aneesh Kumar K. V
@ 2010-06-07 10:34 ` Sripathi Kodi
2010-06-07 12:28 ` [V9fs-developer] [Qemu-devel] " Sripathi Kodi
0 siblings, 1 reply; 12+ messages in thread
From: Sripathi Kodi @ 2010-06-07 10:34 UTC (permalink / raw)
To: Aneesh Kumar K. V
Cc: v9fs-developer, M. Mohan Kumar, Venkateswararao Jujjuri (JV),
qemu-devel
On Sat, 05 Jun 2010 19:11:53 +0530
"Aneesh Kumar K. V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
> 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.
Okay, agreed.
I changed my patch to implement the change you have suggested.
Basically I return 'iounit' as 'st_blksize' and in 'st_blocks' I return
the number of iounit blocks required to fit st_size of the file. The
following patches are diffs from my old patch. They require the iounit
patches that Mohan has sent to this list on 4th.
Comments welcome. Specifically please look at the changes in
v9fs_getattr_post_lstat() below.
Thanks,
Sripathi.
Kernel patch:
=============
Fix block size of getattr call. Depends on Mohan's iounit patch.
Signed-off-by: Sripathi Kodi <sripathik@in.ibm.com>
---
fs/9p/vfs_inode.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 19067de..c01d33b 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -955,6 +955,8 @@ v9fs_vfs_getattr_dotl(struct vfsmount *mnt, struct dentry *dentry,
v9fs_stat2inode_dotl(st, dentry->d_inode);
generic_fillattr(dentry->d_inode, stat);
+ /* Change block size to what the server returned */
+ stat->blksize = st->st_blksize;
kfree(st);
return 0;
QEMU patch:
===========
Fix block size of getattr call. Depends on Mohan's iounit patch.
Signed-off-by: Sripathi Kodi <sripathik@in.ibm.com>
---
hw/virtio-9p.c | 55 +++++++++++++++++++++++++++++++------------------------
hw/virtio-9p.h | 1 +
2 files changed, 32 insertions(+), 24 deletions(-)
diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 4843820..d164ad3 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -1180,6 +1180,26 @@ out:
qemu_free(vs);
}
+static int32_t get_iounit(V9fsState *s, V9fsString *name)
+{
+ struct statfs stbuf;
+ int32_t iounit = 0;
+
+ /*
+ * iounit should be multiples of f_bsize (host filesystem block size
+ * and as well as less than (client msize - P9_IOHDRSZ))
+ */
+ if (!v9fs_do_statfs(s, name, &stbuf)) {
+ iounit = stbuf.f_bsize;
+ iounit *= (s->msize - P9_IOHDRSZ)/stbuf.f_bsize;
+ }
+
+ if (!iounit) {
+ iounit = s->msize - P9_IOHDRSZ;
+ }
+ return iounit;
+}
+
static void v9fs_getattr_post_lstat(V9fsState *s, V9fsStatStateDotl *vs,
int err)
{
@@ -1188,7 +1208,15 @@ static void v9fs_getattr_post_lstat(V9fsState *s, V9fsStatStateDotl *vs,
goto out;
}
+ /* Recalculate block size and number of blocks based on iounit */
stat_to_v9stat_dotl(s, &vs->stbuf, &vs->v9stat_dotl);
+ vs->v9stat_dotl.st_blksize = get_iounit(s, &vs->fidp->path);
+ vs->v9stat_dotl.st_blocks = vs->v9stat_dotl.st_size /
+ vs->v9stat_dotl.st_blksize;
+ if (vs->v9stat_dotl.st_size % vs->v9stat_dotl.st_blksize) {
+ vs->v9stat_dotl.st_blocks++;
+ }
+
vs->offset += pdu_marshal(vs->pdu, vs->offset, "A", &vs->v9stat_dotl);
err = vs->offset;
@@ -1202,7 +1230,6 @@ 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;
@@ -1212,13 +1239,13 @@ static void v9fs_getattr(V9fsState *s, V9fsPDU *pdu)
pdu_unmarshal(vs->pdu, vs->offset, "d", &fid);
- fidp = lookup_fid(s, fid);
- if (fidp == NULL) {
+ vs->fidp = lookup_fid(s, fid);
+ if (vs->fidp == NULL) {
err = -ENOENT;
goto out;
}
- err = v9fs_do_lstat(s, &fidp->path, &vs->stbuf);
+ err = v9fs_do_lstat(s, &vs->fidp->path, &vs->stbuf);
v9fs_getattr_post_lstat(s, vs, err);
return;
@@ -1390,26 +1417,6 @@ out:
v9fs_walk_complete(s, vs, err);
}
-static int32_t get_iounit(V9fsState *s, V9fsString *name)
-{
- struct statfs stbuf;
- int32_t iounit = 0;
-
- /*
- * iounit should be multiples of f_bsize (host filesystem block size
- * and as well as less than (client msize - P9_IOHDRSZ))
- */
- if (!v9fs_do_statfs(s, name, &stbuf)) {
- iounit = stbuf.f_bsize;
- iounit *= (s->msize - P9_IOHDRSZ)/stbuf.f_bsize;
- }
-
- if (!iounit) {
- iounit = s->msize - P9_IOHDRSZ;
- }
- return iounit;
-}
-
static void v9fs_open_post_opendir(V9fsState *s, V9fsOpenState *vs, int err)
{
if (vs->fidp->dir == NULL) {
diff --git a/hw/virtio-9p.h b/hw/virtio-9p.h
index 700666a..6b09b4b 100644
--- a/hw/virtio-9p.h
+++ b/hw/virtio-9p.h
@@ -211,6 +211,7 @@ typedef struct V9fsStatDotl {
typedef struct V9fsStatStateDotl {
V9fsPDU *pdu;
size_t offset;
+ V9fsFidState *fidp;
V9fsStatDotl v9stat_dotl;
struct stat stbuf;
} V9fsStatStateDotl;
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [V9fs-developer] [Qemu-devel] Re: [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.
2010-06-07 10:34 ` Sripathi Kodi
@ 2010-06-07 12:28 ` Sripathi Kodi
0 siblings, 0 replies; 12+ messages in thread
From: Sripathi Kodi @ 2010-06-07 12:28 UTC (permalink / raw)
To: Sripathi Kodi; +Cc: v9fs-developer, Aneesh Kumar K. V, qemu-devel
On Mon, 7 Jun 2010 16:04:17 +0530
Sripathi Kodi <sripathik@in.ibm.com> wrote:
There was one mistake in my patch. See below.
> On Sat, 05 Jun 2010 19:11:53 +0530
> "Aneesh Kumar K. V" <aneesh.kumar@linux.vnet.ibm.com> wrote:
>
> > 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.
>
> Okay, agreed.
>
> I changed my patch to implement the change you have suggested.
> Basically I return 'iounit' as 'st_blksize' and in 'st_blocks' I return
> the number of iounit blocks required to fit st_size of the file. The
> following patches are diffs from my old patch. They require the iounit
> patches that Mohan has sent to this list on 4th.
>
> Comments welcome. Specifically please look at the changes in
> v9fs_getattr_post_lstat() below.
>
> Thanks,
> Sripathi.
>
>
> Kernel patch:
> =============
>
> Fix block size of getattr call. Depends on Mohan's iounit patch.
>
> Signed-off-by: Sripathi Kodi <sripathik@in.ibm.com>
>
>
> ---
>
> fs/9p/vfs_inode.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
>
> diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
> index 19067de..c01d33b 100644
> --- a/fs/9p/vfs_inode.c
> +++ b/fs/9p/vfs_inode.c
> @@ -955,6 +955,8 @@ v9fs_vfs_getattr_dotl(struct vfsmount *mnt, struct dentry *dentry,
>
> v9fs_stat2inode_dotl(st, dentry->d_inode);
> generic_fillattr(dentry->d_inode, stat);
> + /* Change block size to what the server returned */
> + stat->blksize = st->st_blksize;
>
> kfree(st);
> return 0;
>
>
>
> QEMU patch:
> ===========
>
> Fix block size of getattr call. Depends on Mohan's iounit patch.
>
> Signed-off-by: Sripathi Kodi <sripathik@in.ibm.com>
>
>
> ---
>
> hw/virtio-9p.c | 55 +++++++++++++++++++++++++++++++------------------------
> hw/virtio-9p.h | 1 +
> 2 files changed, 32 insertions(+), 24 deletions(-)
>
>
> diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
> index 4843820..d164ad3 100644
> --- a/hw/virtio-9p.c
> +++ b/hw/virtio-9p.c
> @@ -1180,6 +1180,26 @@ out:
> qemu_free(vs);
> }
>
> +static int32_t get_iounit(V9fsState *s, V9fsString *name)
> +{
> + struct statfs stbuf;
> + int32_t iounit = 0;
> +
> + /*
> + * iounit should be multiples of f_bsize (host filesystem block size
> + * and as well as less than (client msize - P9_IOHDRSZ))
> + */
> + if (!v9fs_do_statfs(s, name, &stbuf)) {
> + iounit = stbuf.f_bsize;
> + iounit *= (s->msize - P9_IOHDRSZ)/stbuf.f_bsize;
> + }
> +
> + if (!iounit) {
> + iounit = s->msize - P9_IOHDRSZ;
> + }
> + return iounit;
> +}
> +
> static void v9fs_getattr_post_lstat(V9fsState *s, V9fsStatStateDotl *vs,
> int err)
> {
> @@ -1188,7 +1208,15 @@ static void v9fs_getattr_post_lstat(V9fsState *s, V9fsStatStateDotl *vs,
> goto out;
> }
>
> + /* Recalculate block size and number of blocks based on iounit */
> stat_to_v9stat_dotl(s, &vs->stbuf, &vs->v9stat_dotl);
> + vs->v9stat_dotl.st_blksize = get_iounit(s, &vs->fidp->path);
> + vs->v9stat_dotl.st_blocks = vs->v9stat_dotl.st_size /
> + vs->v9stat_dotl.st_blksize;
> + if (vs->v9stat_dotl.st_size % vs->v9stat_dotl.st_blksize) {
> + vs->v9stat_dotl.st_blocks++;
> + }
> +
I should not update st_blocks when I update st_blksize. This is
because from the manpage, st_blocks is always number of 512 byte blocks
needed for this file.
blkcnt_t st_blocks; /* number of 512B blocks allocated */
Hence by changing st_blocks user space tools like 'du' go wrong.
Thanks,
Sripathi.
> vs->offset += pdu_marshal(vs->pdu, vs->offset, "A", &vs->v9stat_dotl);
> err = vs->offset;
>
> @@ -1202,7 +1230,6 @@ 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;
> @@ -1212,13 +1239,13 @@ static void v9fs_getattr(V9fsState *s, V9fsPDU *pdu)
>
> pdu_unmarshal(vs->pdu, vs->offset, "d", &fid);
>
> - fidp = lookup_fid(s, fid);
> - if (fidp == NULL) {
> + vs->fidp = lookup_fid(s, fid);
> + if (vs->fidp == NULL) {
> err = -ENOENT;
> goto out;
> }
>
> - err = v9fs_do_lstat(s, &fidp->path, &vs->stbuf);
> + err = v9fs_do_lstat(s, &vs->fidp->path, &vs->stbuf);
> v9fs_getattr_post_lstat(s, vs, err);
> return;
>
> @@ -1390,26 +1417,6 @@ out:
> v9fs_walk_complete(s, vs, err);
> }
>
> -static int32_t get_iounit(V9fsState *s, V9fsString *name)
> -{
> - struct statfs stbuf;
> - int32_t iounit = 0;
> -
> - /*
> - * iounit should be multiples of f_bsize (host filesystem block size
> - * and as well as less than (client msize - P9_IOHDRSZ))
> - */
> - if (!v9fs_do_statfs(s, name, &stbuf)) {
> - iounit = stbuf.f_bsize;
> - iounit *= (s->msize - P9_IOHDRSZ)/stbuf.f_bsize;
> - }
> -
> - if (!iounit) {
> - iounit = s->msize - P9_IOHDRSZ;
> - }
> - return iounit;
> -}
> -
> static void v9fs_open_post_opendir(V9fsState *s, V9fsOpenState *vs, int err)
> {
> if (vs->fidp->dir == NULL) {
> diff --git a/hw/virtio-9p.h b/hw/virtio-9p.h
> index 700666a..6b09b4b 100644
> --- a/hw/virtio-9p.h
> +++ b/hw/virtio-9p.h
> @@ -211,6 +211,7 @@ typedef struct V9fsStatDotl {
> typedef struct V9fsStatStateDotl {
> V9fsPDU *pdu;
> size_t offset;
> + V9fsFidState *fidp;
> V9fsStatDotl v9stat_dotl;
> struct stat stbuf;
> } V9fsStatStateDotl;
>
> ------------------------------------------------------------------------------
> ThinkGeek and WIRED's GeekDad team up for the Ultimate
> GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the
> lucky parental unit. See the prize list and enter to win:
> http://p.sf.net/sfu/thinkgeek-promo
> _______________________________________________
> V9fs-developer mailing list
> V9fs-developer@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/v9fs-developer
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] Re: [V9fs-developer] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.
2010-05-28 10:38 [Qemu-devel] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol Sripathi Kodi
2010-06-02 14:19 ` [Qemu-devel] Re: [V9fs-developer] " Aneesh Kumar K. V
@ 2010-07-01 5:31 ` Aneesh Kumar K. V
2010-07-01 8:56 ` Sripathi Kodi
1 sibling, 1 reply; 12+ messages in thread
From: Aneesh Kumar K. V @ 2010-07-01 5:31 UTC (permalink / raw)
To: Sripathi Kodi, qemu-devel; +Cc: v9fs-developer
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] Re: [V9fs-developer] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.
2010-07-01 5:31 ` [Qemu-devel] Re: [V9fs-developer] " Aneesh Kumar K. V
@ 2010-07-01 8:56 ` Sripathi Kodi
2010-07-01 12:41 ` Aneesh Kumar K. V
0 siblings, 1 reply; 12+ messages in thread
From: Sripathi Kodi @ 2010-07-01 8:56 UTC (permalink / raw)
To: Aneesh Kumar K. V; +Cc: v9fs-developer, qemu-devel
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
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] Re: [V9fs-developer] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.
2010-07-01 8:56 ` Sripathi Kodi
@ 2010-07-01 12:41 ` Aneesh Kumar K. V
0 siblings, 0 replies; 12+ messages in thread
From: Aneesh Kumar K. V @ 2010-07-01 12:41 UTC (permalink / raw)
To: Sripathi Kodi; +Cc: v9fs-developer, qemu-devel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.
@ 2010-07-12 15:23 Sripathi Kodi
0 siblings, 0 replies; 12+ messages in thread
From: Sripathi Kodi @ 2010-07-12 15:23 UTC (permalink / raw)
To: qemu-devel
From: M. Mohan Kumar <mohan@in.ibm.com>
This is a new version of getattr patch for 9P2000.L that I posted some
time ago. This version adds 3 fields to the on-the-wire data:
file creation time
generation number
data version
It also adds a bit field to indicate which fields of the structure have
valid data. Usually the server fills in at least all the basic fields, but
it may fill in the additional fields if possible. The server will use
the bit field to tell the client which fields it has populated. Currently
there is no clean way for the server to obtain these additional fields, so
it sends back just the basic fields.
The client (Linux kernel) patch corresponding to this is at
http://sourceforge.net/mailarchive/forum.php?thread_name=20100712143815.6665.35892.stgit%40localhost.localdomain&forum_name=v9fs-developer
Comments welcome.
SYNOPSIS
size[4] Tgetattr tag[2] fid[4] request_mask[8]
size[4] Rgetattr tag[2] lstat[n]
DESCRIPTION
The getattr transaction inquires about the file identified by fid.
request_mask is a bit mask that specifies which fields of the
stat structure is the client interested in.
The reply will contain a machine-independent directory entry,
laid out as follows:
st_result_mask[8]
Bit mask that indicates which fields in the stat structure
have been populated by the server
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_uid[4]
User id of owner
st_gid[4]
Group ID of owner
st_nlink[8]
Number of hard links
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
st_btime_sec[8]
Time of creation (birth) of file, seconds
st_btime_nsec[8]
Time of creation (birth) of file, nanoseconds
st_gen[8]
Inode generation
st_data_version[8]
Data version number
request_mask and result_mask bit masks contain the following bits
#define P9_STATS_MODE 0x00000001ULL
#define P9_STATS_NLINK 0x00000002ULL
#define P9_STATS_UID 0x00000004ULL
#define P9_STATS_GID 0x00000008ULL
#define P9_STATS_RDEV 0x00000010ULL
#define P9_STATS_ATIME 0x00000020ULL
#define P9_STATS_MTIME 0x00000040ULL
#define P9_STATS_CTIME 0x00000080ULL
#define P9_STATS_INO 0x00000100ULL
#define P9_STATS_SIZE 0x00000200ULL
#define P9_STATS_BLOCKS 0x00000400ULL
#define P9_STATS_BTIME 0x00000800ULL
#define P9_STATS_GEN 0x00001000ULL
#define P9_STATS_DATA_VERSION 0x00002000ULL
#define P9_STATS_BASIC 0x000007ffULL
#define P9_STATS_ALL 0x00003fffULL
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
There are some additional fields: st_btime_sec, st_btime_nsec, st_gen,
st_data_version apart from the bitmask, st_result_mask.
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 | 112 ++++++++++++++++++++++++++++++++++++++++++++++++++
hw/virtio-9p.h | 33 +++++++++++++++
3 files changed, 177 insertions(+), 0 deletions(-)
diff --git a/hw/virtio-9p-debug.c b/hw/virtio-9p-debug.c
index 030b3e6..6072491 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 00527c4..10cc003 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -736,6 +736,21 @@ 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, "qQdddqqqqqqqqqqqqqqq",
+ statp->st_result_mask,
+ &statp->qid, statp->st_mode,
+ statp->st_uid, statp->st_gid,
+ statp->st_nlink, 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,
+ statp->st_btime_sec, statp->st_btime_nsec,
+ statp->st_gen, statp->st_data_version);
+ break;
+ }
default:
break;
}
@@ -963,6 +978,51 @@ static int stat_to_v9stat(V9fsState *s, V9fsString *name,
return 0;
}
+#define P9_STATS_MODE 0x00000001ULL
+#define P9_STATS_NLINK 0x00000002ULL
+#define P9_STATS_UID 0x00000004ULL
+#define P9_STATS_GID 0x00000008ULL
+#define P9_STATS_RDEV 0x00000010ULL
+#define P9_STATS_ATIME 0x00000020ULL
+#define P9_STATS_MTIME 0x00000040ULL
+#define P9_STATS_CTIME 0x00000080ULL
+#define P9_STATS_INO 0x00000100ULL
+#define P9_STATS_SIZE 0x00000200ULL
+#define P9_STATS_BLOCKS 0x00000400ULL
+
+#define P9_STATS_BTIME 0x00000800ULL
+#define P9_STATS_GEN 0x00001000ULL
+#define P9_STATS_DATA_VERSION 0x00002000ULL
+
+#define P9_STATS_BASIC 0x000007ffULL /* Mask for fields up to BLOCKS */
+#define P9_STATS_ALL 0x00003fffULL /* Mask for All fields above */
+
+
+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;
+ /* Currently we only support BASIC fields in stat */
+ v9lstat->st_result_mask = P9_STATS_BASIC;
+
+ stat_to_qid(stbuf, &v9lstat->qid);
+}
+
static struct iovec *adjust_sg(struct iovec *sg, int len, int *iovcnt)
{
while (len && *iovcnt) {
@@ -1129,6 +1189,57 @@ 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;
+ uint64_t request_mask;
+
+ 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, "dq", &fid, &request_mask);
+
+ fidp = lookup_fid(s, fid);
+ if (fidp == NULL) {
+ err = -ENOENT;
+ goto out;
+ }
+
+ /* Currently we only support BASIC fields in stat, so there is no
+ * need to look at request_mask.
+ */
+ 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);
@@ -2392,6 +2503,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 4ee3830..1feed82 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,
@@ -185,6 +187,37 @@ typedef struct V9fsStatState {
struct stat stbuf;
} V9fsStatState;
+typedef struct V9fsStatDotl {
+ uint64_t st_result_mask;
+ V9fsQID qid;
+ uint32_t st_mode;
+ uint32_t st_uid;
+ uint32_t st_gid;
+ uint64_t st_nlink;
+ 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;
+ uint64_t st_btime_sec;
+ uint64_t st_btime_nsec;
+ uint64_t st_gen;
+ uint64_t st_data_version;
+} V9fsStatDotl;
+
+typedef struct V9fsStatStateDotl {
+ V9fsPDU *pdu;
+ size_t offset;
+ V9fsStatDotl v9stat_dotl;
+ struct stat stbuf;
+} V9fsStatStateDotl;
+
+
typedef struct V9fsWalkState {
V9fsPDU *pdu;
size_t offset;
^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2010-07-12 15:23 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-28 10:38 [Qemu-devel] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol Sripathi Kodi
2010-06-02 14:19 ` [Qemu-devel] Re: [V9fs-developer] " Aneesh Kumar K. V
2010-06-03 12:59 ` Sripathi Kodi
2010-06-04 8:28 ` Aneesh Kumar K. V
2010-06-04 14:59 ` Venkateswararao Jujjuri (JV)
2010-06-05 13:41 ` Aneesh Kumar K. V
2010-06-07 10:34 ` Sripathi Kodi
2010-06-07 12:28 ` [V9fs-developer] [Qemu-devel] " Sripathi Kodi
2010-07-01 5:31 ` [Qemu-devel] Re: [V9fs-developer] " Aneesh Kumar K. V
2010-07-01 8:56 ` Sripathi Kodi
2010-07-01 12:41 ` Aneesh Kumar K. V
-- strict thread matches above, loose matches on Subject: below --
2010-07-12 15:23 [Qemu-devel] " Sripathi Kodi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).