[Qemu-devel] [PATCH] virtio-9p: open should not return EBADF
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
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().
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.
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.
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
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.
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
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
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.
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.
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
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
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
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
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
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
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.
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.
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
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
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.
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
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
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; -