[Qemu-devel] [PATCH V4 2/2] hw/9pfs: Add readonly support for 9p export

2011-10-12 Thread M. Mohan Kumar 
A new fsdev parameter readonly is introduced to control accessing 9p export.
readonly=on|off can be used to specify the access type. By default rw
access is given.

Signed-off-by: M. Mohan Kumar mo...@in.ibm.com
---
Changes from previous version V3:
* Use opt_set_bool function to set readonly option
* Change the flag from MS_READONLY to 9p specific

Change from previous version V2:
* QEMU_OPT_BOOL is used for readdonly parameter

Changes from previous version:
* Use readonly option instead of access
* Change function return type to boolean where its needed

 fsdev/file-op-9p.h |3 +-
 fsdev/qemu-fsdev.c |   12 +-
 fsdev/qemu-fsdev.h |1 +
 hw/9pfs/virtio-9p-device.c |3 ++
 hw/9pfs/virtio-9p.c|   46 
 qemu-config.c  |7 ++
 vl.c   |2 +
 7 files changed, 71 insertions(+), 3 deletions(-)

diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index 33fb07f..b75290d 100644
--- a/fsdev/file-op-9p.h
+++ b/fsdev/file-op-9p.h
@@ -58,7 +58,8 @@ typedef struct extended_ops {
 } extended_ops;
 
 /* FsContext flag values */
-#define PATHNAME_FSCONTEXT 0x1
+#define PATHNAME_FSCONTEXT  0x1
+#define P9_RDONLY_EXPORT0x2
 
 /* cache flags */
 #define V9FS_WRITETHROUGH_CACHE 0x1
diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
index d08ba9c..f8a8227 100644
--- a/fsdev/qemu-fsdev.c
+++ b/fsdev/qemu-fsdev.c
@@ -29,13 +29,13 @@ static FsTypeTable FsTypes[] = {
 int qemu_fsdev_add(QemuOpts *opts)
 {
 struct FsTypeListEntry *fsle;
-int i;
+int i, flags = 0;
 const char *fsdev_id = qemu_opts_id(opts);
 const char *fstype = qemu_opt_get(opts, fstype);
 const char *path = qemu_opt_get(opts, path);
 const char *sec_model = qemu_opt_get(opts, security_model);
 const char *cache = qemu_opt_get(opts, cache);
-
+int rdonly = qemu_opt_get_bool(opts, readonly, 0);
 
 if (!fsdev_id) {
 fprintf(stderr, fsdev: No id specified\n);
@@ -76,6 +76,14 @@ int qemu_fsdev_add(QemuOpts *opts)
 fsle-fse.cache_flags = V9FS_WRITETHROUGH_CACHE;
 }
 }
+if (rdonly) {
+flags |= P9_RDONLY_EXPORT;
+} else {
+flags = ~P9_RDONLY_EXPORT;
+}
+
+fsle-fse.flags = flags;
+
 QTAILQ_INSERT_TAIL(fstype_entries, fsle, next);
 return 0;
 }
diff --git a/fsdev/qemu-fsdev.h b/fsdev/qemu-fsdev.h
index 0f67880..2938eaf 100644
--- a/fsdev/qemu-fsdev.h
+++ b/fsdev/qemu-fsdev.h
@@ -44,6 +44,7 @@ typedef struct FsTypeEntry {
 char *security_model;
 int cache_flags;
 FileOperations *ops;
+int flags;
 } FsTypeEntry;
 
 typedef struct FsTypeListEntry {
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 1846e36..336292c 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -125,6 +125,9 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf 
*conf)
 s-tag_len = len;
 s-ctx.uid = -1;
 s-ctx.flags = 0;
+if (fse-flags  P9_RDONLY_EXPORT) {
+s-ctx.flags |= P9_RDONLY_EXPORT;
+}
 
 s-ops = fse-ops;
 s-vdev.get_features = virtio_9p_get_features;
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 47ed2f1..9f15787 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -1271,6 +1271,11 @@ static void v9fs_fix_path(V9fsPath *dst, V9fsPath *src, 
int len)
 dst-size++;
 }
 
+static inline bool is_ro_export(FsContext *fs_ctx)
+{
+return fs_ctx-flags  P9_RDONLY_EXPORT;
+}
+
 static void v9fs_version(void *opaque)
 {
 V9fsPDU *pdu = opaque;
@@ -1690,6 +1695,14 @@ static void v9fs_open(void *opaque)
 } else {
 flags = omode_to_uflags(mode);
 }
+if (is_ro_export(s-ctx)) {
+if (mode  O_WRONLY || mode  O_RDWR || mode  O_APPEND) {
+err = -EROFS;
+goto out;
+} else {
+flags |= O_NOATIME;
+}
+}
 err = v9fs_co_open(pdu, fidp, flags);
 if (err  0) {
 goto out;
@@ -3301,6 +3314,33 @@ static void v9fs_op_not_supp(void *opaque)
 complete_pdu(pdu-s, pdu, -EOPNOTSUPP);
 }
 
+static inline bool is_read_only_op(int id)
+{
+switch (id) {
+case P9_TREADDIR:
+case P9_TSTATFS:
+case P9_TGETATTR:
+case P9_TXATTRWALK:
+case P9_TLOCK:
+case P9_TGETLOCK:
+case P9_TREADLINK:
+case P9_TVERSION:
+case P9_TLOPEN:
+case P9_TATTACH:
+case P9_TSTAT:
+case P9_TWALK:
+case P9_TCLUNK:
+case P9_TFSYNC:
+case P9_TOPEN:
+case P9_TREAD:
+case P9_TAUTH:
+case P9_TFLUSH:
+return 1;
+default:
+return 0;
+}
+}
+
 static void submit_pdu(V9fsState *s, V9fsPDU *pdu)
 {
 Coroutine *co;
@@ -3312,6 +3352,12 @@ static void submit_pdu(V9fsState *s, V9fsPDU *pdu)
 } else {
 handler = pdu_co_handlers[pdu-id];
 }
+
+if (is_ro_export(s-ctx)  !is_read_only_op(pdu-id)) {
+complete_pdu(s,

Re: [Qemu-devel] [PATCH V4 2/2] hw/9pfs: Add readonly support for 9p export

2011-10-12 Thread Aneesh Kumar K.V 
On Wed, 12 Oct 2011 13:23:34 +0530, M. Mohan Kumar mo...@in.ibm.com wrote:
 A new fsdev parameter readonly is introduced to control accessing 9p export.
 readonly=on|off can be used to specify the access type. By default rw
 access is given.
 
 Signed-off-by: M. Mohan Kumar mo...@in.ibm.com
 ---
 Changes from previous version V3:
 * Use opt_set_bool function to set readonly option
 * Change the flag from MS_READONLY to 9p specific
 
 Change from previous version V2:
 * QEMU_OPT_BOOL is used for readdonly parameter
 
 Changes from previous version:
 * Use readonly option instead of access
 * Change function return type to boolean where its needed
 
  fsdev/file-op-9p.h |3 +-
  fsdev/qemu-fsdev.c |   12 +-
  fsdev/qemu-fsdev.h |1 +
  hw/9pfs/virtio-9p-device.c |3 ++
  hw/9pfs/virtio-9p.c|   46 
 
  qemu-config.c  |7 ++
  vl.c   |2 +
  7 files changed, 71 insertions(+), 3 deletions(-)
 
 diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
 index 33fb07f..b75290d 100644
 --- a/fsdev/file-op-9p.h
 +++ b/fsdev/file-op-9p.h
 @@ -58,7 +58,8 @@ typedef struct extended_ops {
  } extended_ops;
 
  /* FsContext flag values */
 -#define PATHNAME_FSCONTEXT 0x1
 +#define PATHNAME_FSCONTEXT  0x1

why ?

 +#define P9_RDONLY_EXPORT0x2
 
  /* cache flags */
  #define V9FS_WRITETHROUGH_CACHE 0x1
 diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
 index d08ba9c..f8a8227 100644
 --- a/fsdev/qemu-fsdev.c
 +++ b/fsdev/qemu-fsdev.c
 @@ -29,13 +29,13 @@ static FsTypeTable FsTypes[] = {
  int qemu_fsdev_add(QemuOpts *opts)
  {
  struct FsTypeListEntry *fsle;
 -int i;
 +int i, flags = 0;
  const char *fsdev_id = qemu_opts_id(opts);
  const char *fstype = qemu_opt_get(opts, fstype);
  const char *path = qemu_opt_get(opts, path);
  const char *sec_model = qemu_opt_get(opts, security_model);
  const char *cache = qemu_opt_get(opts, cache);
 -
 +int rdonly = qemu_opt_get_bool(opts, readonly, 0);
 
  if (!fsdev_id) {
  fprintf(stderr, fsdev: No id specified\n);
 @@ -76,6 +76,14 @@ int qemu_fsdev_add(QemuOpts *opts)
  fsle-fse.cache_flags = V9FS_WRITETHROUGH_CACHE;
  }
  }
 +if (rdonly) {
 +flags |= P9_RDONLY_EXPORT;
 +} else {
 +flags = ~P9_RDONLY_EXPORT;
 +}
 +
 +fsle-fse.flags = flags;
 +
  QTAILQ_INSERT_TAIL(fstype_entries, fsle, next);
  return 0;
  }
 diff --git a/fsdev/qemu-fsdev.h b/fsdev/qemu-fsdev.h
 index 0f67880..2938eaf 100644
 --- a/fsdev/qemu-fsdev.h
 +++ b/fsdev/qemu-fsdev.h
 @@ -44,6 +44,7 @@ typedef struct FsTypeEntry {
  char *security_model;
  int cache_flags;
  FileOperations *ops;
 +int flags;

do we need extra flags ? Why not use cache_flags ? renaming that to
export_flags ?

  } FsTypeEntry;
 
  typedef struct FsTypeListEntry {
 diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
 index 1846e36..336292c 100644
 --- a/hw/9pfs/virtio-9p-device.c
 +++ b/hw/9pfs/virtio-9p-device.c
 @@ -125,6 +125,9 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf 
 *conf)
  s-tag_len = len;
  s-ctx.uid = -1;
  s-ctx.flags = 0;
 +if (fse-flags  P9_RDONLY_EXPORT) {
 +s-ctx.flags |= P9_RDONLY_EXPORT;
 +}
 
  s-ops = fse-ops;
  s-vdev.get_features = virtio_9p_get_features;
 diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
 index 47ed2f1..9f15787 100644
 --- a/hw/9pfs/virtio-9p.c
 +++ b/hw/9pfs/virtio-9p.c
 @@ -1271,6 +1271,11 @@ static void v9fs_fix_path(V9fsPath *dst, V9fsPath 
 *src, int len)
  dst-size++;
  }
 
 +static inline bool is_ro_export(FsContext *fs_ctx)
 +{
 +return fs_ctx-flags  P9_RDONLY_EXPORT;
 +}
 +
  static void v9fs_version(void *opaque)
  {
  V9fsPDU *pdu = opaque;
 @@ -1690,6 +1695,14 @@ static void v9fs_open(void *opaque)
  } else {
  flags = omode_to_uflags(mode);
  }
 +if (is_ro_export(s-ctx)) {
 +if (mode  O_WRONLY || mode  O_RDWR || mode  O_APPEND) {
 +err = -EROFS;
 +goto out;
 +} else {
 +flags |= O_NOATIME;
 +}
 +}

What about O_TRUNC ?

  err = v9fs_co_open(pdu, fidp, flags);
  if (err  0) {
  goto out;
 @@ -3301,6 +3314,33 @@ static void v9fs_op_not_supp(void *opaque)
  complete_pdu(pdu-s, pdu, -EOPNOTSUPP);
  }
 

-aneesh

Re: [Qemu-devel] [PATCH V4 2/2] hw/9pfs: Add readonly support for 9p export

2011-10-12 Thread M. Mohan Kumar 
On Wednesday, October 12, 2011 07:38:25 PM Aneesh Kumar K.V wrote:
 On Wed, 12 Oct 2011 13:23:34 +0530, M. Mohan Kumar mo...@in.ibm.com 
wrote:
  A new fsdev parameter readonly is introduced to control accessing 9p
  export. readonly=on|off can be used to specify the access type. By
  default rw access is given.
  
  Signed-off-by: M. Mohan Kumar mo...@in.ibm.com
  ---
  Changes from previous version V3:
  * Use opt_set_bool function to set readonly option
  * Change the flag from MS_READONLY to 9p specific
  
  Change from previous version V2:
  * QEMU_OPT_BOOL is used for readdonly parameter
  
  Changes from previous version:
  * Use readonly option instead of access
  * Change function return type to boolean where its needed
  
   fsdev/file-op-9p.h |3 +-
   fsdev/qemu-fsdev.c |   12 +-
   fsdev/qemu-fsdev.h |1 +
   hw/9pfs/virtio-9p-device.c |3 ++
   hw/9pfs/virtio-9p.c|   46
    qemu-config.c 
   |7 ++
   vl.c   |2 +
   7 files changed, 71 insertions(+), 3 deletions(-)
  
  diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
  index 33fb07f..b75290d 100644
  --- a/fsdev/file-op-9p.h
  +++ b/fsdev/file-op-9p.h
  @@ -58,7 +58,8 @@ typedef struct extended_ops {
  
   } extended_ops;
   
   /* FsContext flag values */
  
  -#define PATHNAME_FSCONTEXT 0x1
  +#define PATHNAME_FSCONTEXT  0x1
 
 why ?
It was part of alignment change for above line

 
  +#define P9_RDONLY_EXPORT0x2
  
   /* cache flags */
   #define V9FS_WRITETHROUGH_CACHE 0x1
  
  diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
  index d08ba9c..f8a8227 100644
  --- a/fsdev/qemu-fsdev.c
  +++ b/fsdev/qemu-fsdev.c
  @@ -29,13 +29,13 @@ static FsTypeTable FsTypes[] = {
  
   int qemu_fsdev_add(QemuOpts *opts)
   {
   
   struct FsTypeListEntry *fsle;
  
  -int i;
  +int i, flags = 0;
  
   const char *fsdev_id = qemu_opts_id(opts);
   const char *fstype = qemu_opt_get(opts, fstype);
   const char *path = qemu_opt_get(opts, path);
   const char *sec_model = qemu_opt_get(opts, security_model);
   const char *cache = qemu_opt_get(opts, cache);
  
  -
  +int rdonly = qemu_opt_get_bool(opts, readonly, 0);
  
   if (!fsdev_id) {
   
   fprintf(stderr, fsdev: No id specified\n);
  
  @@ -76,6 +76,14 @@ int qemu_fsdev_add(QemuOpts *opts)
  
   fsle-fse.cache_flags = V9FS_WRITETHROUGH_CACHE;
   
   }
   
   }
  
  +if (rdonly) {
  +flags |= P9_RDONLY_EXPORT;
  +} else {
  +flags = ~P9_RDONLY_EXPORT;
  +}
  +
  +fsle-fse.flags = flags;
  +
  
   QTAILQ_INSERT_TAIL(fstype_entries, fsle, next);
   return 0;
   
   }
  
  diff --git a/fsdev/qemu-fsdev.h b/fsdev/qemu-fsdev.h
  index 0f67880..2938eaf 100644
  --- a/fsdev/qemu-fsdev.h
  +++ b/fsdev/qemu-fsdev.h
  @@ -44,6 +44,7 @@ typedef struct FsTypeEntry {
  
   char *security_model;
   int cache_flags;
   FileOperations *ops;
  
  +int flags;
 
 do we need extra flags ? Why not use cache_flags ? renaming that to
 export_flags ?
Yes, we can use same variable.
 
   } FsTypeEntry;
   
   typedef struct FsTypeListEntry {
  
  diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
  index 1846e36..336292c 100644
  --- a/hw/9pfs/virtio-9p-device.c
  +++ b/hw/9pfs/virtio-9p-device.c
  @@ -125,6 +125,9 @@ VirtIODevice *virtio_9p_init(DeviceState *dev,
  V9fsConf *conf)
  
   s-tag_len = len;
   s-ctx.uid = -1;
   s-ctx.flags = 0;
  
  +if (fse-flags  P9_RDONLY_EXPORT) {
  +s-ctx.flags |= P9_RDONLY_EXPORT;
  +}
  
   s-ops = fse-ops;
   s-vdev.get_features = virtio_9p_get_features;
  
  diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
  index 47ed2f1..9f15787 100644
  --- a/hw/9pfs/virtio-9p.c
  +++ b/hw/9pfs/virtio-9p.c
  @@ -1271,6 +1271,11 @@ static void v9fs_fix_path(V9fsPath *dst, V9fsPath
  *src, int len)
  
   dst-size++;
   
   }
  
  +static inline bool is_ro_export(FsContext *fs_ctx)
  +{
  +return fs_ctx-flags  P9_RDONLY_EXPORT;
  +}
  +
  
   static void v9fs_version(void *opaque)
   {
   
   V9fsPDU *pdu = opaque;
  
  @@ -1690,6 +1695,14 @@ static void v9fs_open(void *opaque)
  
   } else {
   
   flags = omode_to_uflags(mode);
   
   }
  
  +if (is_ro_export(s-ctx)) {
  +if (mode  O_WRONLY || mode  O_RDWR || mode  O_APPEND) {
  +err = -EROFS;
  +goto out;
  +} else {
  +flags |= O_NOATIME;
  +}
  +}
 
 What about O_TRUNC ?

Thanks, I will include the check for O_TRUNC
 
   err = v9fs_co_open(pdu, fidp, flags);
   if (err  0) {
   
   goto out;
  
  @@ -3301,6 +3314,33 @@ static void v9fs_op_not_supp(void *opaque)
  
   complete_pdu(pdu-s, pdu, -EOPNOTSUPP);