Re: [Qemu-devel] [PATCH V4 2/2] hw/9pfs: Add readonly support for 9p export
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"
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
> > ---
> > 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;
Re: [Qemu-devel] [PATCH V4 2/2] hw/9pfs: Add readonly support for 9p export
On Wed, 12 Oct 2011 13:23:34 +0530, "M. Mohan Kumar" 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
> ---
> 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
[Qemu-devel] [PATCH V4 2/2] hw/9pfs: Add readonly support for 9p export
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
---
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))
