[Qemu-devel] [PATCH] virtio-9p: open should not return EBADF

[Sripathi Kodi]

When 9P server fails to create a file due to permission problems it should
return EPERM. However the current 9P2000.L code returns EBADF. EBADF is NOT
a valid return value from open() call.

The problem is because we do not preserve the errno variable properly. If the
file open had failed, the call to close() on the fd in v9fs_post_lcreate()
fails and sets errno to EBADF. We should preserve the errno that we got from
open() and we should call close() only if we had a valid fd.

Signed-off-by: Sripathi Kodi sripat...@in.ibm.com
---

 hw/virtio-9p.c |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 32fa3bc..fd2147e 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -1758,8 +1758,10 @@ static void v9fs_post_lcreate(V9fsState *s, 
V9fsLcreateState *vs, int err)
 err = vs-offset;
 } else {
 vs-fidp-fid_type = P9_FID_NONE;
-close(vs-fidp-fs.fd);
 err = -errno;
+if (vs-fidp-fs.fd  0) {
+close(vs-fidp-fs.fd);
+}
 }
 
 complete_pdu(s, vs-pdu, err);

[Qemu-devel] [PATCH] virtio-9p: Change handling of flags in open() path for 9P2000.L

[Sripathi Kodi]

This patch applies on top of 9P2000.L patches that we have on the list.
I took a look at how 9P server is handling open() flags in 9P2000.L path.
I think we can do away with the valid_flags() function and simplify the
code. The reasoning is as follows:

O_NOCTTY: (If the file is a terminal, don't make it the controlling
terminal of the process even though the process does not have a controlling
terminal) By the time the control reaches 9P client it is clear that what
we have is not a terminal device. Hence it does not matter what we do with
this flag. In any case 9P server can filter this flag out before making the
syscall.

O_NONBLOCK: (Don't block if i) Can't read/write to the file ii) Can't get
locks) This has an impact on FIFOs, but also on file locks. Hence we can
pass it down to the system call.

O_ASYNC: From the manpage:

   O_ASYNC
  Enable signal-driven I/O: generate a signal (SIGIO by default,  but
  this  can be changed via fcntl(2)) when input or output becomes pos-
  sible on this file descriptor.  This feature is only available  for
  terminals,  pseudo-terminals,  sockets,  and (since Linux 2.6) pipes
  and FIFOs.  See fcntl(2) for further details.

Again, this does not make any impact on regular files handled by 9P. Also,
we don't want 9P server to receive SIGIO. Hence I think 9P server can
filter this flag out before making the syscall.

O_CLOEXEC: This flag makes sense only on the client. If guest user space
sets this flag the guest VFS will take care of calling close() on the fd if
an exec() happens. Hence 9P client need not be bothered with this flag.
Also I think QEMU will not do an exec, but if it does, it makes sense to
close these fds. Hence we can pass this flag down to the syscall.

O_CREAT: Since we are in open() path it means we have confirmed that the file
exists. Hence there is no need to pass O_CREAT flag down to the system. In fact
on some versions of glibc this causes problems, because we pass O_CREAT flag,
but don't have permission bits. Hence we can just mask this flag out.

So in summary:

Mask out:
O_NOCTTY
O_ASYNC
O_CREAT

Pass-through:
O_NONBLOCK
O_CLOEXEC

Signed-off-by: Sripathi Kodi sripat...@in.ibm.com
---

 hw/virtio-9p.c |   14 +-
 1 files changed, 1 insertions(+), 13 deletions(-)

diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 29fc648..7f659b8 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -1672,15 +1672,6 @@ out:
 qemu_free(vs);
 }
 
-static inline int valid_flags(int flag)
-{
-if (flag  O_NOCTTY || flag  O_NONBLOCK || flag  O_ASYNC ||
-flag  O_CLOEXEC)
-return 0;
-else
-return 1;
-}
-
 static void v9fs_open_post_lstat(V9fsState *s, V9fsOpenState *vs, int err)
 {
 int flags;
@@ -1697,11 +1688,8 @@ static void v9fs_open_post_lstat(V9fsState *s, 
V9fsOpenState *vs, int err)
 v9fs_open_post_opendir(s, vs, err);
 } else {
 if (s-proto_version == V9FS_PROTO_2000L) {
-if (!valid_flags(vs-mode)) {
-err = -EINVAL;
-goto out;
-}
 flags = vs-mode;
+flags = ~(O_NOCTTY | O_ASYNC | O_CREAT);
 } else {
 flags = omode_to_uflags(vs-mode);
 }

[Qemu-devel] [PATCH] virtio-9p: Allow O_NOCTTY flag in open().

[Sripathi Kodi]

Currently virtio-9p server doesn't allow O_NOCTTY flag in open().
However, gcc passes this flag while opening files. As a result it
is now impossible to compile any C code on the 9P mount!

I don't see any reason for dis-allowing O_NOCTTY flag in QEMU 9P server.
AFAIK QEMU starts off with a controlling terminal, so calling open with
O_NOCTTY flag is not going to make any difference to QEMU.

The following patch makes 9P server allow O_NOCTTY flag in open().

Signed-off-by: Sripathi Kodi sripat...@in.ibm.com
---

 hw/virtio-9p.c |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 24115ef..f3b8bce 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -1676,11 +1676,11 @@ out:
 
 static inline int valid_flags(int flag)
 {
-if (flag  O_NOCTTY || flag  O_NONBLOCK || flag  O_ASYNC ||
-flag  O_CLOEXEC)
+if (flag  O_NONBLOCK || flag  O_ASYNC || flag  O_CLOEXEC) {
 return 0;
-else
+} else {
 return 1;
+}
 }
 
 static void v9fs_open_post_lstat(V9fsState *s, V9fsOpenState *vs, int err)

Re: [Qemu-devel] [PATCH 1/2] [virtio-9p] Cleanup legacy 'dotu' variable.

[Sripathi Kodi]

On Tue, 13 Jul 2010 16:24:41 +0530
Arun R Bharadwaj a...@linux.vnet.ibm.com wrote:

 Hi,
 
 This patch cleans up the legacy 'dotu' variable which is always set to
 1 by default, since qemu doesnt support legacy 9p clients.
 
 Signed-off-by: Arun R Bharadwaj a...@linux.vnet.ibm.com

Acked-by: Sripathi Kodi sripat...@in.ibm.com

 ---
  hw/virtio-9p-debug.c |   26 +++
  hw/virtio-9p-debug.h |1 
  hw/virtio-9p.c   |  119 
 +++
  3 files changed, 63 insertions(+), 83 deletions(-)
 
 Index: qemu/hw/virtio-9p-debug.h
 ===
 --- qemu.orig/hw/virtio-9p-debug.h
 +++ qemu/hw/virtio-9p-debug.h
 @@ -1,7 +1,6 @@
  #ifndef _QEMU_VIRTIO_9P_DEBUG_H
  #define _QEMU_VIRTIO_9P_DEBUG_H
 
 -extern int dotu;
  void pprint_pdu(V9fsPDU *pdu);
 
  #endif
 Index: qemu/hw/virtio-9p-debug.c
 ===
 --- qemu.orig/hw/virtio-9p-debug.c
 +++ qemu/hw/virtio-9p-debug.c
 @@ -169,12 +169,10 @@ static void pprint_stat(V9fsPDU *pdu, in
  pprint_str(pdu, rx, offsetp, , uid);
  pprint_str(pdu, rx, offsetp, , gid);
  pprint_str(pdu, rx, offsetp, , muid);
 -if (dotu) {
 -pprint_str(pdu, rx, offsetp, , extension);
 -pprint_int32(pdu, rx, offsetp, , uid);
 -pprint_int32(pdu, rx, offsetp, , gid);
 -pprint_int32(pdu, rx, offsetp, , muid);
 -}
 +pprint_str(pdu, rx, offsetp, , extension);
 +pprint_int32(pdu, rx, offsetp, , uid);
 +pprint_int32(pdu, rx, offsetp, , gid);
 +pprint_int32(pdu, rx, offsetp, , muid);
  fprintf(llogfile, });
  }
 
 @@ -343,9 +341,7 @@ void pprint_pdu(V9fsPDU *pdu)
  pprint_int32(pdu, 0, offset, afid);
  pprint_str(pdu, 0, offset, , uname);
  pprint_str(pdu, 0, offset, , aname);
 -if (dotu) {
 -pprint_int32(pdu, 0, offset, , n_uname);
 -}
 +pprint_int32(pdu, 0, offset, , n_uname);
  break;
  case P9_RAUTH:
  fprintf(llogfile, RAUTH: ();
 @@ -357,9 +353,7 @@ void pprint_pdu(V9fsPDU *pdu)
  pprint_int32(pdu, 0, offset, , afid);
  pprint_str(pdu, 0, offset, , uname);
  pprint_str(pdu, 0, offset, , aname);
 -if (dotu) {
 -pprint_int32(pdu, 0, offset, , n_uname);
 -}
 +pprint_int32(pdu, 0, offset, , n_uname);
  break;
  case P9_RATTACH:
  fprintf(llogfile, RATTACH: ();
 @@ -371,9 +365,7 @@ void pprint_pdu(V9fsPDU *pdu)
  case P9_RERROR:
  fprintf(llogfile, RERROR: ();
  pprint_str(pdu, 1, offset, ename);
 -if (dotu) {
 -pprint_int32(pdu, 1, offset, , ecode);
 -}
 +pprint_int32(pdu, 1, offset, , ecode);
  break;
  case P9_TFLUSH:
  fprintf(llogfile, TFLUSH: ();
 @@ -408,9 +400,7 @@ void pprint_pdu(V9fsPDU *pdu)
  pprint_str(pdu, 0, offset, , name);
  pprint_int32(pdu, 0, offset, , perm);
  pprint_int8(pdu, 0, offset, , mode);
 -if (dotu) {
 -pprint_str(pdu, 0, offset, , extension);
 -}
 +pprint_str(pdu, 0, offset, , extension);
  break;
  case P9_RCREATE:
  fprintf(llogfile, RCREATE: ();
 Index: qemu/hw/virtio-9p.c
 ===
 --- qemu.orig/hw/virtio-9p.c
 +++ qemu/hw/virtio-9p.c
 @@ -18,7 +18,6 @@
  #include fsdev/qemu-fsdev.h
  #include virtio-9p-debug.h
 
 -int dotu = 1;
  int debug_9p_pdu;
 
  enum {
 @@ -753,9 +752,7 @@ static void complete_pdu(V9fsState *s, V
 
  len = 7;
  len += pdu_marshal(pdu, len, s, str);
 -if (dotu) {
 -len += pdu_marshal(pdu, len, d, err);
 -}
 +len += pdu_marshal(pdu, len, d, err);
 
  id = P9_RERROR;
  }
 @@ -785,22 +782,20 @@ static mode_t v9mode_to_mode(uint32_t mo
  ret |= S_IFDIR;
  }
 
 -if (dotu) {
 -if (mode  P9_STAT_MODE_SYMLINK) {
 -ret |= S_IFLNK;
 -}
 -if (mode  P9_STAT_MODE_SOCKET) {
 -ret |= S_IFSOCK;
 -}
 -if (mode  P9_STAT_MODE_NAMED_PIPE) {
 -ret |= S_IFIFO;
 -}
 -if (mode  P9_STAT_MODE_DEVICE) {
 -if (extension  extension-data[0] == 'c') {
 -ret |= S_IFCHR;
 -} else {
 -ret |= S_IFBLK;
 -}
 +if (mode  P9_STAT_MODE_SYMLINK) {
 +ret |= S_IFLNK;
 +}
 +if (mode  P9_STAT_MODE_SOCKET) {
 +ret |= S_IFSOCK;
 +}
 +if (mode  P9_STAT_MODE_NAMED_PIPE) {
 +ret |= S_IFIFO;
 +}
 +if (mode  P9_STAT_MODE_DEVICE) {
 +if (extension  extension-data[0] == 'c') {
 +ret |= S_IFCHR;
 +} else {
 +ret |= S_IFBLK;
  }
  }
 
 @@ -863,34 +858,32 @@ static uint32_t stat_to_v9mode(const str
  mode |= P9_STAT_MODE_DIR;
  }
 
 -if (dotu

[Qemu-devel] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.

[Sripathi Kodi]

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 mo...@in.ibm.com
Signed-off-by: Sripathi Kodi sripat...@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, qQdddqqq,
+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  0x0001ULL
+#define P9_STATS_NLINK 0x0002ULL
+#define P9_STATS_UID   0x0004ULL
+#define P9_STATS_GID   0x0008ULL
+#define P9_STATS_RDEV  0x0010ULL
+#define P9_STATS_ATIME 0x0020ULL
+#define P9_STATS_MTIME 0x0040ULL
+#define P9_STATS_CTIME 0x0080ULL
+#define P9_STATS_INO   0x0100ULL
+#define P9_STATS_SIZE  0x0200ULL
+#define P9_STATS_BLOCKS0x0400ULL
+
+#define P9_STATS_BTIME 0x0800ULL
+#define P9_STATS_GEN   0x1000ULL
+#define P9_STATS_DATA_VERSION  0x2000ULL
+
+#define P9_STATS_BASIC 0x07ffULL /* Mask for fields up to BLOCKS */
+#define P9_STATS_ALL   0x3fffULL /* 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

[Qemu-devel] Reg. trace infrastructure

[Sripathi Kodi]

Hi Stefan, Prerna,

I pulled down QEMU tracing code from git://repo.or.cz/qemu/stefanha.git
and tried to use it to trace something in virtio-9p code. I don't have
any knowledge of how traces are implemented and I just went with the
documentation. With help from Prerna, I was able to insert a trace
point and observe it getting hit from the QEMU monitor. However, I have
a few questions about this. Could you please help me out?

*) My understanding is the traces are dumped to a file only when the
trace buffer gets filled. Is there a way to: 
  a) Forcibly dump the contents of the buffer when needed.
  b) When QEMU exits it should dump the buffer. Doesn't seem to be
  doing this now.
  c) A way to specify the file into which it writes the traces.

*) Is QEMU monitor the only way to enable/disable trace points? Is it
possible to programmatically enable/disable trace points from within
QEMU code? If that is not possible, is it possible to specify all trace
points to be enabled in a text file and ask QEMU to read the file when
it starts up?

*) Is it possible to enable/disable a bunch of trace points at one
shot? Can trace points be grouped in some way? Can I say enable all
trace points in a particular .c file or enable all trace points in
these 2 functions?

Thanks a lot for your help.

-Sripathi.

Re: [Qemu-devel] Re: [V9fs-developer] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.

[Sripathi Kodi]

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
 

[Qemu-devel] [PATCH] virtio-9p: Avoid SEGV when log file couldn't be opened

[Sripathi Kodi]

While running in debug mode if 9P server is unable to open the log file
it results in a SEGV deep down in glibc:

Program received signal SIGSEGV, Segmentation fault.
0x008fca8c in fwrite () from /lib/libc.so.6
(gdb) bt
#0  0x008fca8c in fwrite () from /lib/libc.so.6
#1  0x081eb87e in pprint_pdu (pdu=0x89a52e1c)
at /data/sripathi/code/qemu/new/qemu-next-upstream/hw/virtio-9p-debug.c:380
#2  0x0806dad8 in submit_pdu (s=0x897dc008, pdu=0x89a52e1c)
at /data/sripathi/code/qemu/new/qemu-next-upstream/hw/virtio-9p.c:3092
#3  0x0806dc63 in handle_9p_output (vdev=0x897dc008, vq=0x86d8218)
at /data/sripathi/code/qemu/new/qemu-next-upstream/hw/virtio-9p.c:3122
#4  0x081ac728 in virtio_queue_notify (vdev=0x897dc008, n=0)
at /data/sripathi/code/qemu/new/qemu-next-upstream/hw/virtio.c:563
#5  0x08063876 in virtio_ioport_write (opaque=0x86d7b98, addr=16, val=0)
at /data/sripathi/code/qemu/new/qemu-next-upstream/hw/virtio-pci.c:222
#6  0x08063e26 in virtio_pci_config_writew (opaque=0x86d7b98, addr=16, val=0)
at /data/sripathi/code/qemu/new/qemu-next-upstream/hw/virtio-pci.c:357
#7  0x080c881a in ioport_write (index=1, address=49296, data=0) at ioport.c:80
#8  0x080c8d4c in cpu_outw (addr=49296, val=0) at ioport.c:204
#9  0x08073010 in kvm_handle_io (port=49296, data=0xab393000, direction=1, 
size=2, count=1)
at /data/sripathi/code/qemu/new/qemu-next-upstream/kvm-all.c:735
...
...

This is ugly and misleading. The following patch adds a BUG_ON to catch this
error. With this patch we get an abort message like the following, which makes
it easier to analyze:

f12-kvm login: qemu: 
/data/sripathi/code/qemu/new/qemu-next-upstream/hw/virtio-9p-debug.c:353: 
pprint_pdu: Assertion `!(!llogfile)' failed.

Signed-off-by: Sripathi Kodi sripat...@in.ibm.com
---

 hw/virtio-9p-debug.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-9p-debug.c b/hw/virtio-9p-debug.c
index e4ab4bc..c1b0e6f 100644
--- a/hw/virtio-9p-debug.c
+++ b/hw/virtio-9p-debug.c
@@ -327,6 +327,8 @@ void pprint_pdu(V9fsPDU *pdu)
 llogfile = fopen(/tmp/pdu.log, w);
 }
 
+BUG_ON(!llogfile);
+
 switch (pdu-id) {
 case P9_TVERSION:
 fprintf(llogfile, TVERSION: ();

Re: [Qemu-devel] [PATCH] virtio-9p: Fix the memset usage

[Sripathi Kodi]

On Wed, 30 Jun 2010 10:15:29 +0530
Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com wrote:

 The arguments are wrong. Use qemu_mallocz directly
 
 Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com

Reviewed-by: Sripathi Kodi sripat...@in.ibm.com

 ---
  hw/virtio-9p.c |3 +--
  1 files changed, 1 insertions(+), 2 deletions(-)
 
 diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
 index 5c5a34b..fba6619 100644
 --- a/hw/virtio-9p.c
 +++ b/hw/virtio-9p.c
 @@ -2782,8 +2782,7 @@ static void v9fs_wstat_post_chown(V9fsState *s, 
 V9fsWstatState *vs, int err)
  if (vs-v9stat.name.size != 0) {
   V9fsRenameState *vr;
 
 - vr = qemu_malloc(sizeof(V9fsRenameState));
 - memset(vr, sizeof(*vr), 0);
 + vr = qemu_mallocz(sizeof(V9fsRenameState));
   vr-newdirfid= -1;
   vr-pdu = vs-pdu;
   vr-fidp = vs-fidp;
 -- 
 1.7.2.rc0
 
 

Re: [Qemu-devel] Re: [V9fs-developer] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.

[Sripathi Kodi]

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: [V9fs-developer] [Qemu-devel] Re: [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.

[Sripathi Kodi]

On Mon, 7 Jun 2010 16:04:17 +0530
Sripathi Kodi sripat...@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.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

Re: [Qemu-devel] qemu:virtio-9p: [RFC] [PATCH 01/02] Send iounit to client for read/write operations

[Sripathi Kodi]

On Tue,  1 Jun 2010 19:47:14 +0530
M. Mohan Kumar mo...@in.ibm.com wrote:

 Compute iounit based on the host filesystem block size and pass it to
 client with open/create response. Also return iounit as statfs's f_bsize
 for optimal block size transfers.
 
 Signed-off-by: M. Mohan Kumar mo...@in.ibm.com
 ---
  hw/virtio-9p.c |   56 
 ++--
  hw/virtio-9p.h |3 +++
  2 files changed, 45 insertions(+), 14 deletions(-)
 
 diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
 index f087122..4357f1f 100644
 --- a/hw/virtio-9p.c
 +++ b/hw/virtio-9p.c
 @@ -1,4 +1,4 @@
 -/*
 + /*
   * Virtio 9p backend
   *
   * Copyright IBM, Corp. 2010
 @@ -269,6 +269,11 @@ static int v9fs_do_fsync(V9fsState *s, int fd)
  return s-ops-fsync(s-ctx, fd);
  }
 
 +static int v9fs_do_statfs(V9fsState *s, V9fsString *path, struct statfs 
 *stbuf)
 +{
 +return s-ops-statfs(s-ctx, path-data, stbuf);
 +}
 +
  static void v9fs_string_init(V9fsString *str)
  {
  str-data = NULL;
 @@ -1035,11 +1040,10 @@ static void v9fs_fix_path(V9fsString *dst, V9fsString 
 *src, int len)
 
  static void v9fs_version(V9fsState *s, V9fsPDU *pdu)
  {
 -int32_t msize;
  V9fsString version;
  size_t offset = 7;
 
 -pdu_unmarshal(pdu, offset, ds, msize, version);
 +pdu_unmarshal(pdu, offset, ds, s-msize, version);
 
  if (!strcmp(version.data, 9P2000.u)) {
  s-proto_version = V9FS_PROTO_2000U;
 @@ -1049,7 +1053,7 @@ static void v9fs_version(V9fsState *s, V9fsPDU *pdu)
  v9fs_string_sprintf(version, unknown);
  }
 
 -offset += pdu_marshal(pdu, offset, ds, msize, version);
 +offset += pdu_marshal(pdu, offset, ds, s-msize, version);
  complete_pdu(s, pdu, offset);
 
  v9fs_string_free(version);
 @@ -1304,6 +1308,20 @@ out:
  v9fs_walk_complete(s, vs, err);
  }
 
 +static int32_t get_iounit(V9fsState *s, V9fsString *name)
 +{
 +struct statfs stbuf;
 +int32_t iounit = 0;
 +
 +
 +if (!v9fs_do_statfs(s, name, stbuf)) {
 +iounit = stbuf.f_bsize;
 +iounit *= (s-msize - P9_IOHDRSZ)/stbuf.f_bsize;

If (s-msize - P9_IOHDRSZ) is less than stbuf.f_bsize iounit becomes
zero. See below.

 +}
 +
 +return iounit;
 +}
 +
  static void v9fs_open_post_opendir(V9fsState *s, V9fsOpenState *vs, int err)
  {
  if (vs-fidp-dir == NULL) {
 @@ -1321,12 +1339,15 @@ out:
 
  static void v9fs_open_post_open(V9fsState *s, V9fsOpenState *vs, int err)
  {
 +int32_t iounit;
 +
  if (vs-fidp-fd == -1) {
  err = -errno;
  goto out;
  }
 
 -vs-offset += pdu_marshal(vs-pdu, vs-offset, Qd, vs-qid, 0);
 +iounit = get_iounit(s, vs-fidp-path);
 +vs-offset += pdu_marshal(vs-pdu, vs-offset, Qd, vs-qid, iounit);
  err = vs-offset;
  out:
  complete_pdu(s, vs-pdu, err);
 @@ -1800,11 +1821,16 @@ out:
 
  static void v9fs_post_create(V9fsState *s, V9fsCreateState *vs, int err)
  {
 +int32_t iounit;
 +
 +iounit = get_iounit(s, vs-fidp-path);
 +
  if (err == 0) {
  v9fs_string_copy(vs-fidp-path, vs-fullname);
  stat_to_qid(vs-stbuf, vs-qid);
 
 -vs-offset += pdu_marshal(vs-pdu, vs-offset, Qd, vs-qid, 0);
 +vs-offset += pdu_marshal(vs-pdu, vs-offset, Qd, vs-qid,
 +iounit);
 
  err = vs-offset;
  }
 @@ -2295,23 +2321,25 @@ out:
  qemu_free(vs);
  }
 
 -static int v9fs_do_statfs(V9fsState *s, V9fsString *path, struct statfs 
 *stbuf)
 -{
 -return s-ops-statfs(s-ctx, path-data, stbuf);
 -}
 -
  static void v9fs_statfs_post_statfs(V9fsState *s, V9fsStatfsState *vs, int 
 err)
  {
 +int32_t bsize_factor;
 +
  if (err) {
  err = -errno;
  goto out;
  }
 
 +bsize_factor = (s-msize - P9_IOHDRSZ)/vs-stbuf.f_bsize;
 +if (!bsize_factor) {
 +bsize_factor = 1;
 +}

Again, if (s-msize - P9_IOHDRSZ) is less than stbuf.f_bsize
bsize_factor becomes zero. The following divisions become divide by
zero!

Thanks,
Sripathi.

  vs-v9statfs.f_type = vs-stbuf.f_type;
  vs-v9statfs.f_bsize = vs-stbuf.f_bsize;
 -vs-v9statfs.f_blocks = vs-stbuf.f_blocks;
 -vs-v9statfs.f_bfree = vs-stbuf.f_bfree;
 -vs-v9statfs.f_bavail = vs-stbuf.f_bavail;
 +vs-v9statfs.f_bsize *= bsize_factor;
 +vs-v9statfs.f_blocks = vs-stbuf.f_blocks/bsize_factor;
 +vs-v9statfs.f_bfree = vs-stbuf.f_bfree/bsize_factor;
 +vs-v9statfs.f_bavail = vs-stbuf.f_bavail/bsize_factor;
  vs-v9statfs.f_files = vs-stbuf.f_files;
  vs-v9statfs.f_ffree = vs-stbuf.f_ffree;
  vs-v9statfs.fsid_val = (unsigned int) vs-stbuf.f_fsid.__val[0] |
 diff --git a/hw/virtio-9p.h b/hw/virtio-9p.h
 index 6b3d4a4..9264163 100644
 --- a/hw/virtio-9p.h
 +++ b/hw/virtio-9p.h
 @@ -72,6 +72,8 @@ enum p9_proto_version {
  #define P9_NOFID(u32)(~0)
  #define P9_MAXWELEM 16
 
 +#define P9_IOHDRSZ 24
 +
  typedef struct V9fsPDU V9fsPDU;
 
  struct V9fsPDU
 @@ -156,6 +158,7 @@ typedef struct V9fsState
  uint8_t 

Re: [Qemu-devel] 9p: [RFC] [PATCH 02/02] Make use of iounit for read/write

[Sripathi Kodi]

On Tue,  1 Jun 2010 19:47:49 +0530
M. Mohan Kumar mo...@in.ibm.com wrote:

 Change the v9fs_file_readn function to limit the maximum transfer size
 based on the iounit instead of msize.
 
 Also remove the redundant check for limiting the transfer size in
 v9fs_file_write. This check is done by p9_client_write.
 
 Signed-off-by: M. Mohan Kumar mo...@in.ibm.com
 ---
  fs/9p/vfs_file.c |   10 ++
  1 files changed, 2 insertions(+), 8 deletions(-)
 
 diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
 index 25b300e..b8c0891 100644
 --- a/fs/9p/vfs_file.c
 +++ b/fs/9p/vfs_file.c
 @@ -160,7 +160,7 @@ v9fs_file_readn(struct file *filp, char *data, char 
 __user *udata, u32 count,
   offset += n;
   count -= n;
   total += n;
 - } while (count  0  n == (fid-clnt-msize - P9_IOHDRSZ));
 + } while (count  0  n == fid-iounit);

If fid-iounit is zero this will go wrong. With the current version of
your server side patch, fid-iounit can be zero, right?

 
   if (n  0)
   total = n;
 @@ -187,11 +187,7 @@ v9fs_file_read(struct file *filp, char __user *udata, 
 size_t count,
   P9_DPRINTK(P9_DEBUG_VFS, count %zu offset %lld\n, count, *offset);
   fid = filp-private_data;
 
 - if (count  (fid-clnt-msize - P9_IOHDRSZ))
 - ret = v9fs_file_readn(filp, NULL, udata, count, *offset);
 - else
 - ret = p9_client_read(fid, NULL, udata, *offset, count);
 -
 + ret = v9fs_file_readn(filp, NULL, udata, count, *offset);
   if (ret  0)
   *offset += ret;
 
 @@ -225,8 +221,6 @@ v9fs_file_write(struct file *filp, const char __user * 
 data,
   clnt = fid-clnt;
 
   rsize = fid-iounit;
 - if (!rsize || rsize  clnt-msize-P9_IOHDRSZ)
 - rsize = clnt-msize - P9_IOHDRSZ;

This will be needed if fid-iounit = 0

Thanks,
Sripathi.
 
   do {
   if (count  rsize)
 -- 
 1.6.6.1
 
 

[Qemu-devel] [PATCH] [V4] 9p: readdir implementation for 9p2000.L

[Sripathi Kodi]

This patch implements the kernel part of readdir() implementation for 9p2000.L

Change from V3: Instead of inode, server now sends qids for each dirent

SYNOPSIS

size[4] Treaddir tag[2] fid[4] offset[8] count[4]
size[4] Rreaddir tag[2] count[4] data[count]

DESCRIPTION

The readdir request asks the server to read the directory specified by 'fid'
at an offset specified by 'offset' and return as many dirent structures as
possible that fit into count bytes. Each dirent structure is 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

offset[8]
  offset into the next dirent.

type[1]
  type of this directory entry.

name[256]
  name of this directory entry.

This patch adds v9fs_dir_readdir_dotl() as the readdir() call for 9p2000.L.
This function sends P9_TREADDIR command to the server. In response the 
server
sends a buffer filled with dirent structures. This is different from the
existing v9fs_dir_readdir() call which receives stat structures from the 
server.
This results in significant speedup of readdir() on large directories.
For example, doing 'ls /dev/null' on a directory with 1 files on my
laptop takes 1.088 seconds with the existing code, but only takes 0.339 
seconds
with the new readdir.

Signed-off-by: Sripathi Kodi sripat...@in.ibm.com
Reviewed-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
---

 fs/9p/vfs_dir.c |  134 +--
 include/net/9p/9p.h |   17 ++
 include/net/9p/client.h |   18 ++
 net/9p/client.c |   47 
 net/9p/protocol.c   |   27 +
 5 files changed, 227 insertions(+), 16 deletions(-)

diff --git a/fs/9p/vfs_dir.c b/fs/9p/vfs_dir.c
index d61e3b2..aa1852d 100644
--- a/fs/9p/vfs_dir.c
+++ b/fs/9p/vfs_dir.c
@@ -87,29 +87,19 @@ static void p9stat_init(struct p9_wstat *stbuf)
 }
 
 /**
- * v9fs_dir_readdir - read a directory
+ * v9fs_alloc_rdir_buf - Allocate buffer used for read and readdir
  * @filp: opened file structure
- * @dirent: directory structure ???
- * @filldir: function to populate directory structure ???
+ * @buflen: Length in bytes of buffer to allocate
  *
  */
 
-static int v9fs_dir_readdir(struct file *filp, void *dirent, filldir_t filldir)
+static int v9fs_alloc_rdir_buf(struct file *filp, int buflen)
 {
-   int over;
-   struct p9_wstat st;
-   int err = 0;
-   struct p9_fid *fid;
-   int buflen;
-   int reclen = 0;
struct p9_rdir *rdir;
+   struct p9_fid *fid;
+   int err = 0;
 
-   P9_DPRINTK(P9_DEBUG_VFS, name %s\n, filp-f_path.dentry-d_name.name);
fid = filp-private_data;
-
-   buflen = fid-clnt-msize - P9_IOHDRSZ;
-
-   /* allocate rdir on demand */
if (!fid-rdir) {
rdir = kmalloc(sizeof(struct p9_rdir) + buflen, GFP_KERNEL);
 
@@ -128,6 +118,36 @@ static int v9fs_dir_readdir(struct file *filp, void 
*dirent, filldir_t filldir)
spin_unlock(filp-f_dentry-d_lock);
kfree(rdir);
}
+exit:
+   return err;
+}
+
+/**
+ * v9fs_dir_readdir - read a directory
+ * @filp: opened file structure
+ * @dirent: directory structure ???
+ * @filldir: function to populate directory structure ???
+ *
+ */
+
+static int v9fs_dir_readdir(struct file *filp, void *dirent, filldir_t filldir)
+{
+   int over;
+   struct p9_wstat st;
+   int err = 0;
+   struct p9_fid *fid;
+   int buflen;
+   int reclen = 0;
+   struct p9_rdir *rdir;
+
+   P9_DPRINTK(P9_DEBUG_VFS, name %s\n, filp-f_path.dentry-d_name.name);
+   fid = filp-private_data;
+
+   buflen = fid-clnt-msize - P9_IOHDRSZ;
+
+   err = v9fs_alloc_rdir_buf(filp, buflen);
+   if (err)
+   goto exit;
rdir = (struct p9_rdir *) fid-rdir;
 
err = mutex_lock_interruptible(rdir-mutex);
@@ -176,6 +196,88 @@ exit:
return err;
 }
 
+/**
+ * v9fs_dir_readdir_dotl - read a directory
+ * @filp: opened file structure
+ * @dirent: buffer to fill dirent structures
+ * @filldir: function to populate dirent structures
+ *
+ */
+static int v9fs_dir_readdir_dotl(struct file *filp, void *dirent,
+   filldir_t filldir)
+{
+   int over;
+   int err = 0;
+   struct p9_fid *fid;
+   int buflen;
+   struct p9_rdir *rdir;
+   struct p9_dirent curdirent;
+   u64 oldoffset = 0;
+
+   P9_DPRINTK(P9_DEBUG_VFS, name %s\n, filp-f_path.dentry-d_name.name);
+   fid = filp-private_data;
+
+   buflen = fid-clnt

[Qemu-devel] [PATCH] [V4] virtio-9p: readdir implementation for 9p2000.L

[Sripathi Kodi]

This patch implements the server part of readdir() implementation for
9p2000.L

Change from V3: Instead of inode, server now sends qids for each dirent

SYNOPSIS

size[4] Treaddir tag[2] fid[4] offset[8] count[4]
size[4] Rreaddir tag[2] count[4] data[count]

DESCRIPTION

The readdir request asks the server to read the directory specified by 'fid'
at an offset specified by 'offset' and return as many dirent structures as
possible that fit into count bytes. Each dirent structure is 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

offset[8]
  offset into the next dirent.

type[1]
  type of this directory entry.

name[256]
  name of this directory entry.

Signed-off-by: Sripathi Kodi sripat...@in.ibm.com
Reviewed-by: M. Mohan Kumar mo...@in.ibm.com
Reviewed-by: Venkateswararao Jujjuri jv...@linux.vnet.ibm.com
---

 hw/virtio-9p-debug.c |   13 +
 hw/virtio-9p.c   |  119 ++
 hw/virtio-9p.h   |2 +
 3 files changed, 134 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-9p-debug.c b/hw/virtio-9p-debug.c
index 2fb2673..a82b771 100644
--- a/hw/virtio-9p-debug.c
+++ b/hw/virtio-9p-debug.c
@@ -328,6 +328,19 @@ void pprint_pdu(V9fsPDU *pdu)
 }
 
 switch (pdu-id) {
+case P9_TREADDIR:
+fprintf(llogfile, TREADDIR: ();
+pprint_int32(pdu, 0, offset, fid);
+pprint_int64(pdu, 0, offset, , initial offset);
+pprint_int32(pdu, 0, offset, , max count);
+break;
+case P9_RREADDIR:
+fprintf(llogfile, RREADDIR: ();
+pprint_int32(pdu, 1, offset, count);
+#ifdef DEBUG_DATA
+pprint_data(pdu, 1, offset, , data);
+#endif
+break;
 case P9_TVERSION:
 fprintf(llogfile, TVERSION: ();
 pprint_int32(pdu, 0, offset, msize);
diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 2d1cbd5..9c7e256 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -1583,6 +1583,124 @@ out:
 qemu_free(vs);
 }
 
+typedef struct V9fsReadDirState {
+V9fsPDU *pdu;
+V9fsFidState *fidp;
+V9fsQID qid;
+off_t saved_dir_pos;
+struct dirent *dent;
+int32_t count;
+int32_t max_count;
+size_t offset;
+int64_t initial_offset;
+V9fsString name;
+} V9fsReadDirState;
+
+static void v9fs_readdir_post_seekdir(V9fsState *s, V9fsReadDirState *vs)
+{
+vs-offset += pdu_marshal(vs-pdu, vs-offset, d, vs-count);
+vs-offset += vs-count;
+complete_pdu(s, vs-pdu, vs-offset);
+qemu_free(vs);
+return;
+}
+
+/* Size of each dirent on the wire: size of qid (13) + size of offset (8)
+ * size of type (1) + size of name.size (2) + strlen(name.data)
+ */
+#define V9_READDIR_DATA_SZ (24 + strlen(vs-name.data))
+
+static void v9fs_readdir_post_readdir(V9fsState *s, V9fsReadDirState *vs)
+{
+int len;
+size_t size;
+
+if (vs-dent) {
+v9fs_string_init(vs-name);
+v9fs_string_sprintf(vs-name, %s, vs-dent-d_name);
+
+if ((vs-count + V9_READDIR_DATA_SZ)  vs-max_count) {
+/* Ran out of buffer. Set dir back to old position and return */
+v9fs_do_seekdir(s, vs-fidp-dir, vs-saved_dir_pos);
+v9fs_readdir_post_seekdir(s, vs);
+return;
+}
+
+/* Fill up just the path field of qid because the client uses
+ * only that. To fill the entire qid structure we will have
+ * to stat each dirent found, which is expensive
+ */
+size = MIN(sizeof(vs-dent-d_ino), sizeof(vs-qid.path));
+memcpy(vs-qid.path, vs-dent-d_ino, size);
+
+len = pdu_marshal(vs-pdu, vs-offset+4+vs-count, Qqbs,
+  vs-qid, vs-dent-d_off,
+  vs-dent-d_type, vs-name);
+vs-count += len;
+v9fs_string_free(vs-name);
+vs-saved_dir_pos = vs-dent-d_off;
+vs-dent = v9fs_do_readdir(s, vs-fidp-dir);
+v9fs_readdir_post_readdir(s, vs);
+return;
+}
+
+vs-offset += pdu_marshal(vs-pdu, vs-offset, d, vs-count);
+vs-offset += vs-count;
+complete_pdu(s, vs-pdu, vs-offset);
+qemu_free(vs);
+return;
+}
+
+static void v9fs_readdir_post_telldir(V9fsState *s, V9fsReadDirState *vs)
+{
+vs-dent = v9fs_do_readdir(s, vs-fidp-dir);
+v9fs_readdir_post_readdir(s, vs);
+return;
+}
+
+static void v9fs_readdir_post_setdir(V9fsState *s, V9fsReadDirState *vs)
+{
+vs-saved_dir_pos = v9fs_do_telldir(s, vs-fidp-dir);
+v9fs_readdir_post_telldir(s, vs);
+return;
+}
+
+static void v9fs_readdir(V9fsState *s, V9fsPDU *pdu

[Qemu-devel] [PATCH] [V4] 9p: readdir implementation for 9p2000.L

[Sripathi Kodi]

This patch implements the kernel part of readdir() implementation for 9p2000.L

Change from V3: Instead of inode, server now sends qids for each dirent

SYNOPSIS

size[4] Treaddir tag[2] fid[4] offset[8] count[4]
size[4] Rreaddir tag[2] count[4] data[count]

DESCRIPTION

The readdir request asks the server to read the directory specified by 'fid'
at an offset specified by 'offset' and return as many dirent structures as
possible that fit into count bytes. Each dirent structure is 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

offset[8]
  offset into the next dirent.

type[1]
  type of this directory entry.

name[256]
  name of this directory entry.

This patch adds v9fs_dir_readdir_dotl() as the readdir() call for 9p2000.L.
This function sends P9_TREADDIR command to the server. In response the 
server
sends a buffer filled with dirent structures. This is different from the
existing v9fs_dir_readdir() call which receives stat structures from the 
server.
This results in significant speedup of readdir() on large directories.
For example, doing 'ls /dev/null' on a directory with 1 files on my
laptop takes 1.088 seconds with the existing code, but only takes 0.339 
seconds
with the new readdir.

Signed-off-by: Sripathi Kodi sripat...@in.ibm.com
Reviewed-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
---

 fs/9p/vfs_dir.c |  134 +--
 include/net/9p/9p.h |   17 ++
 include/net/9p/client.h |   18 ++
 net/9p/client.c |   47 
 net/9p/protocol.c   |   27 +
 5 files changed, 227 insertions(+), 16 deletions(-)

diff --git a/fs/9p/vfs_dir.c b/fs/9p/vfs_dir.c
index d61e3b2..aa1852d 100644
--- a/fs/9p/vfs_dir.c
+++ b/fs/9p/vfs_dir.c
@@ -87,29 +87,19 @@ static void p9stat_init(struct p9_wstat *stbuf)
 }
 
 /**
- * v9fs_dir_readdir - read a directory
+ * v9fs_alloc_rdir_buf - Allocate buffer used for read and readdir
  * @filp: opened file structure
- * @dirent: directory structure ???
- * @filldir: function to populate directory structure ???
+ * @buflen: Length in bytes of buffer to allocate
  *
  */
 
-static int v9fs_dir_readdir(struct file *filp, void *dirent, filldir_t filldir)
+static int v9fs_alloc_rdir_buf(struct file *filp, int buflen)
 {
-   int over;
-   struct p9_wstat st;
-   int err = 0;
-   struct p9_fid *fid;
-   int buflen;
-   int reclen = 0;
struct p9_rdir *rdir;
+   struct p9_fid *fid;
+   int err = 0;
 
-   P9_DPRINTK(P9_DEBUG_VFS, name %s\n, filp-f_path.dentry-d_name.name);
fid = filp-private_data;
-
-   buflen = fid-clnt-msize - P9_IOHDRSZ;
-
-   /* allocate rdir on demand */
if (!fid-rdir) {
rdir = kmalloc(sizeof(struct p9_rdir) + buflen, GFP_KERNEL);
 
@@ -128,6 +118,36 @@ static int v9fs_dir_readdir(struct file *filp, void 
*dirent, filldir_t filldir)
spin_unlock(filp-f_dentry-d_lock);
kfree(rdir);
}
+exit:
+   return err;
+}
+
+/**
+ * v9fs_dir_readdir - read a directory
+ * @filp: opened file structure
+ * @dirent: directory structure ???
+ * @filldir: function to populate directory structure ???
+ *
+ */
+
+static int v9fs_dir_readdir(struct file *filp, void *dirent, filldir_t filldir)
+{
+   int over;
+   struct p9_wstat st;
+   int err = 0;
+   struct p9_fid *fid;
+   int buflen;
+   int reclen = 0;
+   struct p9_rdir *rdir;
+
+   P9_DPRINTK(P9_DEBUG_VFS, name %s\n, filp-f_path.dentry-d_name.name);
+   fid = filp-private_data;
+
+   buflen = fid-clnt-msize - P9_IOHDRSZ;
+
+   err = v9fs_alloc_rdir_buf(filp, buflen);
+   if (err)
+   goto exit;
rdir = (struct p9_rdir *) fid-rdir;
 
err = mutex_lock_interruptible(rdir-mutex);
@@ -176,6 +196,88 @@ exit:
return err;
 }
 
+/**
+ * v9fs_dir_readdir_dotl - read a directory
+ * @filp: opened file structure
+ * @dirent: buffer to fill dirent structures
+ * @filldir: function to populate dirent structures
+ *
+ */
+static int v9fs_dir_readdir_dotl(struct file *filp, void *dirent,
+   filldir_t filldir)
+{
+   int over;
+   int err = 0;
+   struct p9_fid *fid;
+   int buflen;
+   struct p9_rdir *rdir;
+   struct p9_dirent curdirent;
+   u64 oldoffset = 0;
+
+   P9_DPRINTK(P9_DEBUG_VFS, name %s\n, filp-f_path.dentry-d_name.name);
+   fid = filp-private_data;
+
+   buflen = fid-clnt

[Qemu-devel] [PATCH] [V4] 9p: readdir implementation for 9p2000.L

[Sripathi Kodi]

This patch implements the kernel part of readdir() implementation for 9p2000.L

Change from V3: Instead of inode, server now sends qids for each dirent

SYNOPSIS

size[4] Treaddir tag[2] fid[4] offset[8] count[4]
size[4] Rreaddir tag[2] count[4] data[count]

DESCRIPTION

The readdir request asks the server to read the directory specified by 'fid'
at an offset specified by 'offset' and return as many dirent structures as
possible that fit into count bytes. Each dirent structure is 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

offset[8]
  offset into the next dirent.

type[1]
  type of this directory entry.

name[256]
  name of this directory entry.

This patch adds v9fs_dir_readdir_dotl() as the readdir() call for 9p2000.L.
This function sends P9_TREADDIR command to the server. In response the 
server
sends a buffer filled with dirent structures. This is different from the
existing v9fs_dir_readdir() call which receives stat structures from the 
server.
This results in significant speedup of readdir() on large directories.
For example, doing 'ls /dev/null' on a directory with 1 files on my
laptop takes 1.088 seconds with the existing code, but only takes 0.339 
seconds
with the new readdir.

Signed-off-by: Sripathi Kodi sripat...@in.ibm.com
Reviewed-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
---

 fs/9p/vfs_dir.c |  134 +--
 include/net/9p/9p.h |   17 ++
 include/net/9p/client.h |   18 ++
 net/9p/client.c |   47 
 net/9p/protocol.c   |   27 +
 5 files changed, 227 insertions(+), 16 deletions(-)

diff --git a/fs/9p/vfs_dir.c b/fs/9p/vfs_dir.c
index d61e3b2..aa1852d 100644
--- a/fs/9p/vfs_dir.c
+++ b/fs/9p/vfs_dir.c
@@ -87,29 +87,19 @@ static void p9stat_init(struct p9_wstat *stbuf)
 }
 
 /**
- * v9fs_dir_readdir - read a directory
+ * v9fs_alloc_rdir_buf - Allocate buffer used for read and readdir
  * @filp: opened file structure
- * @dirent: directory structure ???
- * @filldir: function to populate directory structure ???
+ * @buflen: Length in bytes of buffer to allocate
  *
  */
 
-static int v9fs_dir_readdir(struct file *filp, void *dirent, filldir_t filldir)
+static int v9fs_alloc_rdir_buf(struct file *filp, int buflen)
 {
-   int over;
-   struct p9_wstat st;
-   int err = 0;
-   struct p9_fid *fid;
-   int buflen;
-   int reclen = 0;
struct p9_rdir *rdir;
+   struct p9_fid *fid;
+   int err = 0;
 
-   P9_DPRINTK(P9_DEBUG_VFS, name %s\n, filp-f_path.dentry-d_name.name);
fid = filp-private_data;
-
-   buflen = fid-clnt-msize - P9_IOHDRSZ;
-
-   /* allocate rdir on demand */
if (!fid-rdir) {
rdir = kmalloc(sizeof(struct p9_rdir) + buflen, GFP_KERNEL);
 
@@ -128,6 +118,36 @@ static int v9fs_dir_readdir(struct file *filp, void 
*dirent, filldir_t filldir)
spin_unlock(filp-f_dentry-d_lock);
kfree(rdir);
}
+exit:
+   return err;
+}
+
+/**
+ * v9fs_dir_readdir - read a directory
+ * @filp: opened file structure
+ * @dirent: directory structure ???
+ * @filldir: function to populate directory structure ???
+ *
+ */
+
+static int v9fs_dir_readdir(struct file *filp, void *dirent, filldir_t filldir)
+{
+   int over;
+   struct p9_wstat st;
+   int err = 0;
+   struct p9_fid *fid;
+   int buflen;
+   int reclen = 0;
+   struct p9_rdir *rdir;
+
+   P9_DPRINTK(P9_DEBUG_VFS, name %s\n, filp-f_path.dentry-d_name.name);
+   fid = filp-private_data;
+
+   buflen = fid-clnt-msize - P9_IOHDRSZ;
+
+   err = v9fs_alloc_rdir_buf(filp, buflen);
+   if (err)
+   goto exit;
rdir = (struct p9_rdir *) fid-rdir;
 
err = mutex_lock_interruptible(rdir-mutex);
@@ -176,6 +196,88 @@ exit:
return err;
 }
 
+/**
+ * v9fs_dir_readdir_dotl - read a directory
+ * @filp: opened file structure
+ * @dirent: buffer to fill dirent structures
+ * @filldir: function to populate dirent structures
+ *
+ */
+static int v9fs_dir_readdir_dotl(struct file *filp, void *dirent,
+   filldir_t filldir)
+{
+   int over;
+   int err = 0;
+   struct p9_fid *fid;
+   int buflen;
+   struct p9_rdir *rdir;
+   struct p9_dirent curdirent;
+   u64 oldoffset = 0;
+
+   P9_DPRINTK(P9_DEBUG_VFS, name %s\n, filp-f_path.dentry-d_name.name);
+   fid = filp-private_data;
+
+   buflen = fid-clnt

[Qemu-devel] [PATCH 1/2] Make v9fs_do_utimensat accept timespec structures instead of v9stat.

[Sripathi Kodi]

One of Mohan's recent patches (Message-Id:
1275286613-16757-1-git-send-email-mo...@in.ibm.com) implements
v9fs_do_utimensat function. Currently v9fs_do_utimensat takes a V9fsStat
argument and builds timespec structures. It sets tv_nsec values to 0 by
default. Instead of this it should take struct timespec[2] and pass it
down to the system directly. This will make it more generic and useful
elsewhere.

Signed-off-by: Sripathi Kodi sripat...@in.ibm.com
---

 hw/virtio-9p.c |   37 ++---
 1 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 1c7a428..8c1cdfb 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -239,25 +239,10 @@ static int v9fs_do_chown(V9fsState *s, V9fsString *path, 
uid_t uid, gid_t gid)
 return s-ops-chown(s-ctx, path-data, cred);
 }
 
-static int v9fs_do_utimensat(V9fsState *s, V9fsString *path, V9fsStat v9stat)
+static int v9fs_do_utimensat(V9fsState *s, V9fsString *path,
+   const struct timespec times[2])
 {
-struct timespec ts[2];
-
-if (v9stat.atime != -1) {
-ts[0].tv_sec = v9stat.atime;
-ts[0].tv_nsec = 0;
-} else {
-ts[0].tv_nsec = UTIME_OMIT;
-}
-
-if (v9stat.mtime != -1) {
-ts[1].tv_sec = v9stat.mtime;
-ts[1].tv_nsec = 0;
-} else {
-ts[1].tv_nsec = UTIME_OMIT;
-}
-
-return s-ops-utimensat(s-ctx, path-data, ts);
+return s-ops-utimensat(s-ctx, path-data, times);
 }
 
 static int v9fs_do_remove(V9fsState *s, V9fsString *path)
@@ -2345,7 +2330,21 @@ static void v9fs_wstat_post_chmod(V9fsState *s, 
V9fsWstatState *vs, int err)
 }
 
 if (vs-v9stat.mtime != -1 || vs-v9stat.atime != -1) {
-if (v9fs_do_utimensat(s, vs-fidp-path, vs-v9stat)) {
+struct timespec times[2];
+if (vs-v9stat.atime != -1) {
+times[0].tv_sec = vs-v9stat.atime;
+times[0].tv_nsec = 0;
+} else {
+times[0].tv_nsec = UTIME_OMIT;
+}
+if (vs-v9stat.mtime != -1) {
+times[1].tv_sec = vs-v9stat.mtime;
+times[1].tv_nsec = 0;
+} else {
+times[1].tv_nsec = UTIME_OMIT;
+}
+
+if (v9fs_do_utimensat(s, vs-fidp-path, times)) {
 err = -errno;
 }
 }

[Qemu-devel] [PATCH 2/2] virtio-9p: Implement server side of setattr for 9P2000.L protocol.

[Sripathi Kodi]

SYNOPSIS

  size[4] Tsetattr tag[2] attr[n]

  size[4] Rsetattr tag[2]

   DESCRIPTION

  The setattr command changes some of the file status information.
  attr resembles the iattr structure used in Linux kernel. It
  specifies which status parameter is to be changed and to what
  value. It is laid out as follows:

 valid[4]
specifies which status information is to be changed. Possible
values are:
ATTR_MODE(1  0)
ATTR_UID (1  1)
ATTR_GID (1  2)
ATTR_SIZE(1  3)
ATTR_ATIME   (1  4)
ATTR_MTIME   (1  5)

 mode[4]
File permission bits

 uid[4]
Owner id of file

 gid[4]
Group id of the file

 size[8]
File size

 atime_sec[8]
Time of last file access, seconds

 atime_nsec[8]
Time of last file access, nanoseconds

 mtime_sec[8]
Time of last file modification, seconds

 mtime_nsec[8]
Time of last file modification, nanoseconds



Explanation of the patches:
--

*) The kernel just copies relevent contents of iattr structure to p9_iattr_dotl
   structure and passes it down to the client. The only check it has is calling
   inode_change_ok()
*) The p9_iattr_dotl structure does not have ctime and ia_file parameters 
because
   I don't think these are needed in our case.
*) The server currently supports changing mode, time, ownership and size of the
   file.
*) 9P RFC says Either all the changes in wstat request happen, or none of them
   does: if the request succeeds, all changes were made; if it fails, none 
were.
   I have not implemented this as I think this is not needed.

Signed-off-by: Sripathi Kodi sripat...@in.ibm.com
---

 hw/virtio-9p.c |  137 
 hw/virtio-9p.h |   23 +
 2 files changed, 160 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 8c1cdfb..a51f5ac 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -662,6 +662,15 @@ static size_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, 
const char *fmt, ...)
 statp-n_muid);
 break;
 }
+case 'I': {
+V9fsIattr *iattr = va_arg(ap, V9fsIattr *);
+offset += pdu_unmarshal(pdu, offset, q,
+iattr-valid, iattr-mode,
+iattr-uid, iattr-gid, iattr-size,
+iattr-atime_sec, iattr-atime_nsec,
+iattr-mtime_sec, iattr-mtime_nsec);
+break;
+}
 default:
 break;
 }
@@ -1208,6 +1217,133 @@ out:
 qemu_free(vs);
 }
 
+/* From Linux kernel code */
+#define ATTR_MODE(1  0)
+#define ATTR_UID (1  1)
+#define ATTR_GID (1  2)
+#define ATTR_SIZE(1  3)
+#define ATTR_ATIME   (1  4)
+#define ATTR_MTIME   (1  5)
+
+static void v9fs_setattr_post_truncate(V9fsState *s, V9fsSetattrState *vs,
+  int err)
+{
+if (err == -1) {
+err = -errno;
+goto out;
+}
+err = vs-offset;
+
+out:
+complete_pdu(s, vs-pdu, err);
+qemu_free(vs);
+}
+
+static void v9fs_setattr_post_chown(V9fsState *s, V9fsSetattrState *vs, int 
err)
+{
+if (err == -1) {
+err = -errno;
+goto out;
+}
+
+if (vs-v9iattr.valid  (ATTR_SIZE)) {
+err = v9fs_do_truncate(s, vs-fidp-path, vs-v9iattr.size);
+}
+v9fs_setattr_post_truncate(s, vs, err);
+return;
+
+out:
+complete_pdu(s, vs-pdu, err);
+qemu_free(vs);
+}
+
+static void v9fs_setattr_post_utimensat(V9fsState *s, V9fsSetattrState *vs,
+   int err)
+{
+if (err == -1) {
+err = -errno;
+goto out;
+}
+
+if (vs-v9iattr.valid  (ATTR_UID | ATTR_GID)) {
+if (! (vs-v9iattr.valid  ATTR_UID)) {
+vs-v9iattr.uid = -1;
+}
+if (! (vs-v9iattr.valid  ATTR_GID)) {
+vs-v9iattr.gid = -1;
+}
+err = v9fs_do_chown(s, vs-fidp-path, vs-v9iattr.uid,
+vs-v9iattr.gid);
+}
+v9fs_setattr_post_chown(s, vs, err);
+return;
+
+out:
+complete_pdu(s, vs-pdu, err);
+qemu_free(vs);
+}
+
+static void v9fs_setattr_post_chmod(V9fsState *s, V9fsSetattrState *vs, int 
err)
+{
+if (err == -1) {
+err = -errno;
+goto out;
+}
+
+if (vs-v9iattr.valid  (ATTR_ATIME | ATTR_MTIME)) {
+struct timespec times[2];
+if (vs-v9iattr.valid  ATTR_ATIME) {
+times[0].tv_sec = vs-v9iattr.atime_sec;
+times[0].tv_nsec = vs-v9iattr.atime_nsec;
+} else {
+times[0].tv_nsec = UTIME_OMIT;
+}
+if (vs-v9iattr.valid  ATTR_MTIME

[Qemu-devel] [PATCH] In v9fs_remove_post_remove() we currently ignore the error returned by

[Sripathi Kodi]

the previous call to remove() and return an error only if freeing the
fid fails. However, the client expects to see the error from remove().
Currently the client falsely thinks that the remove call has always
succeeded. For example, doing rmdir on a non-empty directory does
not return ENOTEMPTY.

With this patch we ignore the error from free_fid(). The client cannot
use this error value anyway.

Signed-off-by: Sripathi Kodi sripat...@in.ibm.com
---

 hw/virtio-9p.c |   11 ++-
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index e5d0112..999c0d5 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -1943,14 +1943,15 @@ typedef struct V9fsRemoveState {
 static void v9fs_remove_post_remove(V9fsState *s, V9fsRemoveState *vs,
 int err)
 {
-/* For TREMOVE we need to clunk the fid even on failed remove */
-err = free_fid(s, vs-fidp-fid);
 if (err  0) {
-goto out;
+err = -errno;
+} else {
+err = vs-offset;
 }
 
-err = vs-offset;
-out:
+/* For TREMOVE we need to clunk the fid even on failed remove */
+free_fid(s, vs-fidp-fid);
+
 complete_pdu(s, vs-pdu, err);
 qemu_free(vs);
 }

[Qemu-devel] [PATCH] virtio-9p: Return correct error from v9fs_remove

[Sripathi Kodi]

This patch got mangled last time. Resending.

virtio-9p: Return correct error from v9fs_remove

In v9fs_remove_post_remove() we currently ignore the error returned by
the previous call to remove() and return an error only if freeing the
fid fails. However, the client expects to see the error from remove().
Currently the client falsely thinks that the remove call has always
succeeded. For example, doing rmdir on a non-empty directory does
not return ENOTEMPTY.

With this patch we ignore the error from free_fid(). The client cannot
use this error value anyway.

Signed-off-by: Sripathi Kodi sripat...@in.ibm.com
---

 hw/virtio-9p.c |   11 ++-
 1 files changed, 6 insertions(+), 5 deletions(-)


diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index e5d0112..999c0d5 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -1943,14 +1943,15 @@ typedef struct V9fsRemoveState {
 static void v9fs_remove_post_remove(V9fsState *s, V9fsRemoveState *vs,
 int err)
 {
-/* For TREMOVE we need to clunk the fid even on failed remove */
-err = free_fid(s, vs-fidp-fid);
 if (err  0) {
-goto out;
+err = -errno;
+} else {
+err = vs-offset;
 }
 
-err = vs-offset;
-out:
+/* For TREMOVE we need to clunk the fid even on failed remove */
+free_fid(s, vs-fidp-fid);
+
 complete_pdu(s, vs-pdu, err);
 qemu_free(vs);
 }

[Qemu-devel] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.

[Sripathi Kodi]

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


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 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, Qdqddqq

[Qemu-devel] [PATCH] virtio-9p: readdir implementation for 9p2000.L

[Sripathi Kodi]

This patch implements the server part of readdir() implementation for
9p2000.L

SYNOPSIS

size[4] Treaddir tag[2] fid[4] offset[8] count[4]
size[4] Rreaddir tag[2] count[4] data[count]

DESCRIPTION

The readdir request asks the server to read the directory specified by 'fid'
at an offset specified by 'offset' and return as many dirent structures as
possible that fit into count bytes. Each dirent structure is laid out as
follows.

inode[8]
  inode number of the file.

offset[8]
  offset into the next dirent.

type[1]
  type of this directory entry.

name[256]
  name of this directory entry.

Signed-off-by: Sripathi Kodi sripat...@in.ibm.com
Reviewed-by: M. Mohan Kumar mo...@in.ibm.com
Reviewed-by: Venkateswararao Jujjuri jv...@linux.vnet.ibm.com
---

 hw/virtio-9p-debug.c |   13 ++
 hw/virtio-9p.c   |  110 ++
 hw/virtio-9p.h   |2 +
 3 files changed, 125 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-9p-debug.c b/hw/virtio-9p-debug.c
index 2fb2673..a82b771 100644
--- a/hw/virtio-9p-debug.c
+++ b/hw/virtio-9p-debug.c
@@ -328,6 +328,19 @@ void pprint_pdu(V9fsPDU *pdu)
 }
 
 switch (pdu-id) {
+case P9_TREADDIR:
+fprintf(llogfile, TREADDIR: ();
+pprint_int32(pdu, 0, offset, fid);
+pprint_int64(pdu, 0, offset, , initial offset);
+pprint_int32(pdu, 0, offset, , max count);
+break;
+case P9_RREADDIR:
+fprintf(llogfile, RREADDIR: ();
+pprint_int32(pdu, 1, offset, count);
+#ifdef DEBUG_DATA
+pprint_data(pdu, 1, offset, , data);
+#endif
+break;
 case P9_TVERSION:
 fprintf(llogfile, TVERSION: ();
 pprint_int32(pdu, 0, offset, msize);
diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index e5d0112..39a535a 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -1606,6 +1606,115 @@ typedef struct V9fsWriteState {
 int cnt;
 } V9fsWriteState;
 
+typedef struct V9fsReadDirState {
+V9fsPDU *pdu;
+V9fsFidState *fidp;
+off_t saved_dir_pos;
+struct dirent *dent;
+int32_t count;
+int32_t max_count;
+size_t offset;
+int64_t initial_offset;
+V9fsString name;
+} V9fsReadDirState;
+
+static void v9fs_readdir_post_seekdir(V9fsState *s, V9fsReadDirState *vs)
+{
+vs-offset += pdu_marshal(vs-pdu, vs-offset, d, vs-count);
+vs-offset += vs-count;
+complete_pdu(s, vs-pdu, vs-offset);
+qemu_free(vs);
+return;
+}
+
+/* Size of each dirent on the wire: size of inode (8) + size of offset (8)
+ * size of type (1) + size of name.size (2) + strlen(name.data)
+ */
+#define V9_READDIR_DATA_SZ (19+strlen(vs-name.data))
+
+static void v9fs_readdir_post_readdir(V9fsState *s, V9fsReadDirState *vs)
+{
+int len;
+
+if (vs-dent) {
+v9fs_string_init(vs-name);
+v9fs_string_sprintf(vs-name, %s, vs-dent-d_name);
+
+if ((vs-count + V9_READDIR_DATA_SZ)  vs-max_count) {
+/* Ran out of buffer. Set dir back to old position and return */
+v9fs_do_seekdir(s, vs-fidp-dir, vs-saved_dir_pos);
+v9fs_readdir_post_seekdir(s, vs);
+return;
+}
+
+len = pdu_marshal(vs-pdu, vs-offset+4+vs-count, qqbs,
+  vs-dent-d_ino, vs-dent-d_off,
+  vs-dent-d_type, vs-name);
+vs-count += len;
+v9fs_string_free(vs-name);
+vs-saved_dir_pos = vs-dent-d_off;
+vs-dent = v9fs_do_readdir(s, vs-fidp-dir);
+v9fs_readdir_post_readdir(s, vs);
+return;
+}
+
+vs-offset += pdu_marshal(vs-pdu, vs-offset, d, vs-count);
+vs-offset += vs-count;
+complete_pdu(s, vs-pdu, vs-offset);
+qemu_free(vs);
+return;
+}
+
+static void v9fs_readdir_post_telldir(V9fsState *s, V9fsReadDirState *vs)
+{
+vs-dent = v9fs_do_readdir(s, vs-fidp-dir);
+v9fs_readdir_post_readdir(s, vs);
+return;
+}
+
+static void v9fs_readdir_post_setdir(V9fsState *s, V9fsReadDirState *vs)
+{
+vs-saved_dir_pos = v9fs_do_telldir(s, vs-fidp-dir);
+v9fs_readdir_post_telldir(s, vs);
+return;
+}
+
+static void v9fs_readdir(V9fsState *s, V9fsPDU *pdu)
+{
+int32_t fid;
+V9fsReadDirState *vs;
+ssize_t err = 0;
+size_t offset = 7;
+
+vs = qemu_malloc(sizeof(*vs));
+vs-pdu = pdu;
+vs-offset = 7;
+vs-count = 0;
+
+pdu_unmarshal(vs-pdu, offset, dqd, fid, vs-initial_offset,
+vs-max_count);
+
+vs-fidp = lookup_fid(s, fid);
+if (vs-fidp == NULL || !(vs-fidp-dir)) {
+err = -EINVAL;
+goto out;
+}
+
+if (vs-initial_offset == 0) {
+v9fs_do_rewinddir(s, vs-fidp-dir);
+} else {
+v9fs_do_seekdir(s, vs-fidp-dir, vs-initial_offset);
+}
+
+v9fs_readdir_post_setdir(s, vs);
+return;
+
+out:
+complete_pdu(s, pdu, err);
+qemu_free(vs);
+return;
+}
+
 static void

Re: [Qemu-devel] [PATCH -V3 2/7] virtio-9p: Rearrange fileop structures

[Sripathi Kodi]

On Fri, 21 May 2010 14:26:05 -0700
Venkateswararao Jujjuri (JV) jv...@linux.vnet.ibm.com wrote:

Hi JV,

While I agree that this patch is nice to have, why is this part of the
security model patchset? Is it required to implement the models?

Thanks,
Sripathi.

 Signed-off-by: Venkateswararao Jujjuri jv...@linux.vnet.ibm.com
 ---
  hw/virtio-9p.c |  185 
 ++--
  hw/virtio-9p.h |   92 
  2 files changed, 138 insertions(+), 139 deletions(-)
 
 diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
 index 8ecd39c..fda3c4a 100644
 --- a/hw/virtio-9p.c
 +++ b/hw/virtio-9p.c
 @@ -21,6 +21,52 @@
  int dotu = 1;
  int debug_9p_pdu;
 
 +enum {
 +Oread   = 0x00,
 +Owrite  = 0x01,
 +Ordwr   = 0x02,
 +Oexec   = 0x03,
 +Oexcl   = 0x04,
 +Otrunc  = 0x10,
 +Orexec  = 0x20,
 +Orclose = 0x40,
 +Oappend = 0x80,
 +};
 +
 +static int omode_to_uflags(int8_t mode)
 +{
 +int ret = 0;
 +
 +switch (mode  3) {
 +case Oread:
 +ret = O_RDONLY;
 +break;
 +case Ordwr:
 +ret = O_RDWR;
 +break;
 +case Owrite:
 +ret = O_WRONLY;
 +break;
 +case Oexec:
 +ret = O_RDONLY;
 +break;
 +}
 +
 +if (mode  Otrunc) {
 +ret |= O_TRUNC;
 +}
 +
 +if (mode  Oappend) {
 +ret |= O_APPEND;
 +}
 +
 +if (mode  Oexcl) {
 +ret |= O_EXCL;
 +}
 +
 +return ret;
 +}
 +
  static int v9fs_do_lstat(V9fsState *s, V9fsString *path, struct stat *stbuf)
  {
  return s-ops-lstat(s-ctx, path-data, stbuf);
 @@ -999,14 +1045,6 @@ out:
  v9fs_string_free(aname);
  }
 
 -typedef struct V9fsStatState {
 -V9fsPDU *pdu;
 -size_t offset;
 -V9fsStat v9stat;
 -V9fsFidState *fidp;
 -struct stat stbuf;
 -} V9fsStatState;
 -
  static void v9fs_stat_post_lstat(V9fsState *s, V9fsStatState *vs, int err)
  {
  if (err == -1) {
 @@ -1057,19 +1095,6 @@ out:
  qemu_free(vs);
  }
 
 -typedef struct V9fsWalkState {
 -V9fsPDU *pdu;
 -size_t offset;
 -int16_t nwnames;
 -int name_idx;
 -V9fsQID *qids;
 -V9fsFidState *fidp;
 -V9fsFidState *newfidp;
 -V9fsString path;
 -V9fsString *wnames;
 -struct stat stbuf;
 -} V9fsWalkState;
 -
  static void v9fs_walk_complete(V9fsState *s, V9fsWalkState *vs, int err)
  {
  complete_pdu(s, vs-pdu, err);
 @@ -1233,62 +1258,6 @@ out:
  v9fs_walk_complete(s, vs, err);
  }
 
 -typedef struct V9fsOpenState {
 -V9fsPDU *pdu;
 -size_t offset;
 -int8_t mode;
 -V9fsFidState *fidp;
 -V9fsQID qid;
 -struct stat stbuf;
 -
 -} V9fsOpenState;
 -
 -enum {
 -Oread   = 0x00,
 -Owrite  = 0x01,
 -Ordwr   = 0x02,
 -Oexec   = 0x03,
 -Oexcl   = 0x04,
 -Otrunc  = 0x10,
 -Orexec  = 0x20,
 -Orclose = 0x40,
 -Oappend = 0x80,
 -};
 -
 -static int omode_to_uflags(int8_t mode)
 -{
 -int ret = 0;
 -
 -switch (mode  3) {
 -case Oread:
 -ret = O_RDONLY;
 -break;
 -case Ordwr:
 -ret = O_RDWR;
 -break;
 -case Owrite:
 -ret = O_WRONLY;
 -break;
 -case Oexec:
 -ret = O_RDONLY;
 -break;
 -}
 -
 -if (mode  Otrunc) {
 -ret |= O_TRUNC;
 -}
 -
 -if (mode  Oappend) {
 -ret |= O_APPEND;
 -}
 -
 -if (mode  Oexcl) {
 -ret |= O_EXCL;
 -}
 -
 -return ret;
 -}
 -
  static void v9fs_open_post_opendir(V9fsState *s, V9fsOpenState *vs, int err)
  {
  if (vs-fidp-dir == NULL) {
 @@ -1391,25 +1360,6 @@ out:
  complete_pdu(s, pdu, err);
  }
 
 -typedef struct V9fsReadState {
 -V9fsPDU *pdu;
 -size_t offset;
 -int32_t count;
 -int32_t total;
 -int64_t off;
 -V9fsFidState *fidp;
 -struct iovec iov[128]; /* FIXME: bad, bad, bad */
 -struct iovec *sg;
 -off_t dir_pos;
 -struct dirent *dent;
 -struct stat stbuf;
 -V9fsString name;
 -V9fsStat v9stat;
 -int32_t len;
 -int32_t cnt;
 -int32_t max_count;
 -} V9fsReadState;
 -
  static void v9fs_read_post_readdir(V9fsState *, V9fsReadState *, ssize_t);
 
  static void v9fs_read_post_seekdir(V9fsState *s, V9fsReadState *vs, ssize_t 
 err)
 @@ -1597,19 +1547,6 @@ out:
  qemu_free(vs);
  }
 
 -typedef struct V9fsWriteState {
 -V9fsPDU *pdu;
 -size_t offset;
 -int32_t len;
 -int32_t count;
 -int32_t total;
 -int64_t off;
 -V9fsFidState *fidp;
 -struct iovec iov[128]; /* FIXME: bad, bad, bad */
 -struct iovec *sg;
 -int cnt;
 -} V9fsWriteState;
 -
  static void v9fs_write_post_writev(V9fsState *s, V9fsWriteState *vs,
 ssize_t err)
  {
 @@ -1706,19 +1643,6 @@ out:
  qemu_free(vs);
  }
 
 -typedef struct V9fsCreateState {
 -V9fsPDU *pdu;
 -size_t offset;
 -V9fsFidState *fidp;
 -V9fsQID qid;
 -int32_t perm;
 -int8_t mode;
 -struct stat stbuf;
 -V9fsString name;
 -