On Wed, 12 Oct 2011 13:24:16 +0530, "M. Mohan Kumar" <mo...@in.ibm.com> wrote:
> Security model is needed only for 'local' fs driver.
Can you also cleanup that fstype -> fsdriver rename ? fsdriver seems
more appropriate.
>
> Signed-off-by: M. Mohan Kumar <mo...@in.ibm.com>
> ---
> fsdev/qemu-fsdev.c | 6 +----
> fsdev/qemu-fsdev.h | 1 +
> hw/9pfs/virtio-9p-device.c | 47 ++++++++++++++++++++++---------------------
> vl.c | 20 +++++++++++++++--
> 4 files changed, 43 insertions(+), 31 deletions(-)
>
> diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
> index 36db127..d08ba9c 100644
> --- a/fsdev/qemu-fsdev.c
> +++ b/fsdev/qemu-fsdev.c
> @@ -58,11 +58,6 @@ int qemu_fsdev_add(QemuOpts *opts)
> return -1;
> }
>
> - if (!sec_model) {
> - fprintf(stderr, "fsdev: No security_model specified.\n");
> - return -1;
> - }
> -
> if (!path) {
> fprintf(stderr, "fsdev: No path specified.\n");
> return -1;
> @@ -72,6 +67,7 @@ int qemu_fsdev_add(QemuOpts *opts)
>
> fsle->fse.fsdev_id = g_strdup(fsdev_id);
> fsle->fse.path = g_strdup(path);
> + fsle->fse.fsdriver = g_strdup(fstype);
Why use it as a string ? Why can't this again be an export_flag. That
would help us to avoid that strdup
> fsle->fse.security_model = g_strdup(sec_model);
> fsle->fse.ops = FsTypes[i].ops;
> fsle->fse.cache_flags = 0;
> diff --git a/fsdev/qemu-fsdev.h b/fsdev/qemu-fsdev.h
> index 9c440f2..0f67880 100644
> --- a/fsdev/qemu-fsdev.h
> +++ b/fsdev/qemu-fsdev.h
> @@ -40,6 +40,7 @@ typedef struct FsTypeTable {
> typedef struct FsTypeEntry {
> char *fsdev_id;
> char *path;
> + char *fsdriver;
> char *security_model;
> int cache_flags;
> FileOperations *ops;
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index aac58ad..1846e36 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -83,29 +83,30 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf
> *conf)
> exit(1);
> }
>
> - if (!strcmp(fse->security_model, "passthrough")) {
> - /* Files on the Fileserver set to client user credentials */
> - s->ctx.fs_sm = SM_PASSTHROUGH;
> - s->ctx.xops = passthrough_xattr_ops;
> - } else if (!strcmp(fse->security_model, "mapped")) {
> - /* Files on the fileserver are set to QEMU credentials.
> - * Client user credentials are saved in extended attributes.
> - */
> - s->ctx.fs_sm = SM_MAPPED;
> - s->ctx.xops = mapped_xattr_ops;
> - } else if (!strcmp(fse->security_model, "none")) {
> - /*
> - * Files on the fileserver are set to QEMU credentials.
> - */
> - s->ctx.fs_sm = SM_NONE;
> - s->ctx.xops = none_xattr_ops;
> - } else {
> - fprintf(stderr, "Default to security_model=none. You may want"
> - " enable advanced security model using "
> - "security option:\n\t security_model=passthrough\n\t "
> - "security_model=mapped\n");
> - s->ctx.fs_sm = SM_NONE;
> - s->ctx.xops = none_xattr_ops;
> + /* security models is needed only for local fs driver */
> + if (!strcmp(fse->fsdriver, "local")) {
> + if (!strcmp(fse->security_model, "passthrough")) {
> + /* Files on the Fileserver set to client user credentials */
> + s->ctx.fs_sm = SM_PASSTHROUGH;
> + s->ctx.xops = passthrough_xattr_ops;
> + } else if (!strcmp(fse->security_model, "mapped")) {
> + /* Files on the fileserver are set to QEMU credentials.
> + * Client user credentials are saved in extended attributes.
> + */
> + s->ctx.fs_sm = SM_MAPPED;
> + s->ctx.xops = mapped_xattr_ops;
> + } else if (!strcmp(fse->security_model, "none")) {
> + /*
> + * Files on the fileserver are set to QEMU credentials.
> + */
> + s->ctx.fs_sm = SM_NONE;
> + s->ctx.xops = none_xattr_ops;
> + } else {
> + fprintf(stderr, "Invalid security_model %s specified.\n"
> + "Available security models are:\t "
> + "passthrough,mapped or none\n", fse->security_model);
> + exit(1);
> + }
> }
>
> s->ctx.cache_flags = fse->cache_flags;
> diff --git a/vl.c b/vl.c
> index 6760e39..a961fa3 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2795,6 +2795,7 @@ int main(int argc, char **argv, char **envp)
> QemuOpts *fsdev;
> QemuOpts *device;
> const char *cache;
> + const char *fsdriver;
>
> olist = qemu_find_opts("virtfs");
> if (!olist) {
> @@ -2809,13 +2810,26 @@ int main(int argc, char **argv, char **envp)
>
> if (qemu_opt_get(opts, "fstype") == NULL ||
> qemu_opt_get(opts, "mount_tag") == NULL ||
> - qemu_opt_get(opts, "path") == NULL ||
> - qemu_opt_get(opts, "security_model") == NULL) {
> + qemu_opt_get(opts, "path") == NULL) {
> fprintf(stderr, "Usage: -virtfs
> fstype,path=/share_path/,"
> - "security_model=[mapped|passthrough|none],"
> +
> "{security_model=[mapped|passthrough|none]},"
That should be
[security_model=....]
> "mount_tag=tag.\n");
> exit(1);
> }
> + fsdriver = qemu_opt_get(opts, "fstype");
> + /* security model is mandatory for local fs driver */
> + if (!strcmp(fsdriver,"local") &&
> + !qemu_opt_get(opts,"security_model")) {
> + fprintf(stderr, "security model not specified for local"
> + " fs driver\n");
> + exit(1);
> + }
> + if (strcmp(fsdriver,"local") &&
> + qemu_opt_get(opts,"security_model")) {
> + fprintf(stderr, "security model is not needed for %s"
> + " fs driver\n", fsdriver);
> + exit(1);
> + }
>
> fsdev = qemu_opts_create(qemu_find_opts("fsdev"),
> qemu_opt_get(opts, "mount_tag"), 1);
Also needs a documentation update.
-aneesh
