Re: [Qemu-devel] Re: [V9fs-developer] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.
On Thu, 01 Jul 2010 11:01:15 +0530 Aneesh Kumar K. V aneesh.ku...@linux.vnet.ibm.com wrote: On Fri, 28 May 2010 16:08:43 +0530, Sripathi Kodi sripat...@in.ibm.com wrote: From: M. Mohan Kumar mo...@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
Re: [Qemu-devel] Re: [V9fs-developer] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.
On Thu, 1 Jul 2010 14:26:13 +0530, Sripathi Kodi sripat...@in.ibm.com wrote: On Thu, 01 Jul 2010 11:01:15 +0530 Aneesh Kumar K. V aneesh.ku...@linux.vnet.ibm.com wrote: On Fri, 28 May 2010 16:08:43 +0530, Sripathi Kodi sripat...@in.ibm.com wrote: From: M. Mohan Kumar mo...@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
[Qemu-devel] Re: [V9fs-developer] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.
On Fri, 28 May 2010 16:08:43 +0530, Sripathi Kodi sripat...@in.ibm.com wrote: From: M. Mohan Kumar mo...@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
Re: [Qemu-devel] Re: [V9fs-developer] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.
On Sat, 05 Jun 2010 19:11:53 +0530
Aneesh Kumar K. V aneesh.ku...@linux.vnet.ibm.com wrote:
On Fri, 04 Jun 2010 07:59:42 -0700, Venkateswararao Jujjuri (JV)
jv...@linux.vnet.ibm.com wrote:
Aneesh Kumar K. V wrote:
On Thu, 3 Jun 2010 18:29:02 +0530, Sripathi Kodi sripat...@in.ibm.com
wrote:
On Wed, 02 Jun 2010 19:49:24 +0530
Aneesh Kumar K. V aneesh.ku...@linux.vnet.ibm.com wrote:
On Fri, 28 May 2010 16:08:43 +0530, Sripathi Kodi
sripat...@in.ibm.com wrote:
From: M. Mohan Kumar mo...@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 sripat...@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 sripat...@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,
Re: [Qemu-devel] Re: [V9fs-developer] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.
On Fri, 04 Jun 2010 07:59:42 -0700, Venkateswararao Jujjuri (JV) jv...@linux.vnet.ibm.com wrote: Aneesh Kumar K. V wrote: On Thu, 3 Jun 2010 18:29:02 +0530, Sripathi Kodi sripat...@in.ibm.com wrote: On Wed, 02 Jun 2010 19:49:24 +0530 Aneesh Kumar K. V aneesh.ku...@linux.vnet.ibm.com wrote: On Fri, 28 May 2010 16:08:43 +0530, Sripathi Kodi sripat...@in.ibm.com wrote: From: M. Mohan Kumar mo...@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
[Qemu-devel] Re: [V9fs-developer] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.
On Thu, 3 Jun 2010 18:29:02 +0530, Sripathi Kodi sripat...@in.ibm.com wrote: On Wed, 02 Jun 2010 19:49:24 +0530 Aneesh Kumar K. V aneesh.ku...@linux.vnet.ibm.com wrote: On Fri, 28 May 2010 16:08:43 +0530, Sripathi Kodi sripat...@in.ibm.com wrote: From: M. Mohan Kumar mo...@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
Re: [Qemu-devel] Re: [V9fs-developer] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.
Aneesh Kumar K. V wrote: On Thu, 3 Jun 2010 18:29:02 +0530, Sripathi Kodi sripat...@in.ibm.com wrote: On Wed, 02 Jun 2010 19:49:24 +0530 Aneesh Kumar K. V aneesh.ku...@linux.vnet.ibm.com wrote: On Fri, 28 May 2010 16:08:43 +0530, Sripathi Kodi sripat...@in.ibm.com wrote: From: M. Mohan Kumar mo...@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
[Qemu-devel] Re: [V9fs-developer] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.
On Fri, 28 May 2010 16:08:43 +0530, Sripathi Kodi sripat...@in.ibm.com wrote:
From: M. Mohan Kumar mo...@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 mo...@in.ibm.com
Signed-off-by: Sripathi Kodi sripat...@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
