diff mbox

[V4,2/2] hw/9pfs: Add readonly support for 9p export

Message ID 1318406014-16911-2-git-send-email-mohan@in.ibm.com
State New
Headers show

Commit Message

Mohan Kumar M Oct. 12, 2011, 7:53 a.m. UTC 
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 <mohan@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(-)

Comments

Aneesh Kumar K.V Oct. 12, 2011, 2:08 p.m. UTC | #1 
On Wed, 12 Oct 2011 13:23:34 +0530, "M. Mohan Kumar" <mohan@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 <mohan@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_EXPORT    0x2
> 
>  /* 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
Mohan Kumar M Oct. 12, 2011, 3:31 p.m. UTC | #2 
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" <mohan@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 <mohan@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_EXPORT    0x2
> > 
> >  /* 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);
> >  
> >  }
> 
> -aneesh
diff mbox

Patch

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_EXPORT    0x2
 
 /* 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, pdu, -EROFS);
+        return;
+    }
+
     co = qemu_coroutine_create(handler);
     qemu_coroutine_enter(co, pdu);
 }
diff --git a/qemu-config.c b/qemu-config.c
index b2ab0b2..5cad718 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -180,7 +180,11 @@  QemuOptsList qemu_fsdev_opts = {
         }, {
             .name = "cache",
             .type = QEMU_OPT_STRING,
+        }, {
+            .name = "readonly",
+            .type = QEMU_OPT_BOOL,
         },
+
         { /*End of list */ }
     },
 };
@@ -205,6 +209,9 @@  QemuOptsList qemu_virtfs_opts = {
         }, {
             .name = "cache",
             .type = QEMU_OPT_STRING,
+        }, {
+            .name = "readonly",
+            .type = QEMU_OPT_BOOL,
         },
 
         { /*End of list */ }
diff --git a/vl.c b/vl.c
index cab4002..04ffb26 100644
--- a/vl.c
+++ b/vl.c
@@ -2848,6 +2848,8 @@  int main(int argc, char **argv, char **envp)
                 qemu_opt_set(fsdev, "security_model",
                              qemu_opt_get(opts, "security_model"));
 
+                qemu_opt_set_bool(fsdev, "readonly",
+                                qemu_opt_get_bool(opts, "readonly", 0));
                 device = qemu_opts_create(qemu_find_opts("device"), NULL, 0);
                 qemu_opt_set(device, "driver", "virtio-9p-pci");
                 qemu_opt_set(device, "fsdev",