[Qemu-devel] [PATCH V5 01/14] hw/9pfs: Move opt validation to FsDriver callback

["M. Mohan Kumar" <mohan@in.ibm.com>]

From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>

    This remove all conditional code from common code path and
    make opt validation a FSDriver callback.

    Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 fsdev/file-op-9p.h         |   15 +++++++++++++--
 fsdev/qemu-fsdev.c         |   44 ++++++--------------------------------------
 fsdev/qemu-fsdev.h         |   10 ----------
 hw/9pfs/virtio-9p-device.c |   13 ++++++++-----
 hw/9pfs/virtio-9p-handle.c |   20 ++++++++++++++++++++
 hw/9pfs/virtio-9p-local.c  |   34 ++++++++++++++++++++++++++++++++++
 vl.c                       |    8 ++------
 7 files changed, 83 insertions(+), 61 deletions(-)

diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index a85ecd3..73de79f 100644
--- a/fsdev/file-op-9p.h
+++ b/fsdev/file-op-9p.h
@@ -61,6 +61,16 @@ typedef struct extended_ops {
 #define V9FS_SEC_MASK               0x0000001C
 
 
+typedef struct FileOperations FileOperations;
+/*
+ * Structure to store the various fsdev's passed through command line.
+ */
+typedef struct FsDriverEntry {
+    char *fsdev_id;
+    char *path;
+    int export_flags;
+    FileOperations *ops;
+} FsDriverEntry;
 
 typedef struct FsContext
 {
@@ -82,8 +92,9 @@ typedef union V9fsFidOpenState V9fsFidOpenState;
 
 void cred_init(FsCred *);
 
-typedef struct FileOperations
+struct FileOperations
 {
+    int (*parse_opts)(QemuOpts *, struct FsDriverEntry *);
     int (*init)(struct FsContext *);
     int (*lstat)(FsContext *, V9fsPath *, struct stat *);
     ssize_t (*readlink)(FsContext *, V9fsPath *, char *, size_t);
@@ -128,6 +139,6 @@ typedef struct FileOperations
                     V9fsPath *newdir, const char *new_name);
     int (*unlinkat)(FsContext *ctx, V9fsPath *dir, const char *name, int flags);
     void *opaque;
-} FileOperations;
+};
 
 #endif
diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
index 6684f7e..00f48ab 100644
--- a/fsdev/qemu-fsdev.c
+++ b/fsdev/qemu-fsdev.c
@@ -31,12 +31,10 @@ static FsDriverTable FsDrivers[] = {
 
 int qemu_fsdev_add(QemuOpts *opts)
 {
-    struct FsDriverListEntry *fsle;
     int i;
+    struct FsDriverListEntry *fsle;
     const char *fsdev_id = qemu_opts_id(opts);
     const char *fsdriver = qemu_opt_get(opts, "fsdriver");
-    const char *path = qemu_opt_get(opts, "path");
-    const char *sec_model = qemu_opt_get(opts, "security_model");
     const char *writeout = qemu_opt_get(opts, "writeout");
     bool ro = qemu_opt_get_bool(opts, "readonly", 0);
 
@@ -61,29 +59,9 @@ int qemu_fsdev_add(QemuOpts *opts)
         return -1;
     }
 
-    if (!strcmp(fsdriver, "local") && !sec_model) {
-        fprintf(stderr, "security model not specified, "
-                "local fs needs security model\nvalid options are:"
-                "\tsecurity_model=[passthrough|mapped|none]\n");
-        return -1;
-    }
-
-    if (strcmp(fsdriver, "local") && sec_model) {
-        fprintf(stderr, "only local fs driver needs security model\n");
-        return -1;
-    }
-
-    if (!path) {
-        fprintf(stderr, "fsdev: No path specified.\n");
-        return -1;
-    }
-
-    fsle = g_malloc(sizeof(*fsle));
-
+    fsle = g_malloc0(sizeof(*fsle));
     fsle->fse.fsdev_id = g_strdup(fsdev_id);
-    fsle->fse.path = g_strdup(path);
     fsle->fse.ops = FsDrivers[i].ops;
-    fsle->fse.export_flags = 0;
     if (writeout) {
         if (!strcmp(writeout, "immediate")) {
             fsle->fse.export_flags |= V9FS_IMMEDIATE_WRITEOUT;
@@ -95,22 +73,12 @@ int qemu_fsdev_add(QemuOpts *opts)
         fsle->fse.export_flags &= ~V9FS_RDONLY;
     }
 
-    if (strcmp(fsdriver, "local")) {
-        goto done;
+    if (fsle->fse.ops->parse_opts) {
+        if (fsle->fse.ops->parse_opts(opts, &fsle->fse)) {
+            return -1;
+        }
     }
 
-    if (!strcmp(sec_model, "passthrough")) {
-        fsle->fse.export_flags |= V9FS_SM_PASSTHROUGH;
-    } else if (!strcmp(sec_model, "mapped")) {
-        fsle->fse.export_flags |= V9FS_SM_MAPPED;
-    } else if (!strcmp(sec_model, "none")) {
-        fsle->fse.export_flags |= V9FS_SM_NONE;
-    } else {
-        fprintf(stderr, "Invalid security model %s specified, valid options are"
-                "\n\t [passthrough|mapped|none]\n", sec_model);
-        return -1;
-    }
-done:
     QTAILQ_INSERT_TAIL(&fsdriver_entries, fsle, next);
     return 0;
 }
diff --git a/fsdev/qemu-fsdev.h b/fsdev/qemu-fsdev.h
index 8ef8473..921452d 100644
--- a/fsdev/qemu-fsdev.h
+++ b/fsdev/qemu-fsdev.h
@@ -34,16 +34,6 @@ typedef struct FsDriverTable {
     FileOperations *ops;
 } FsDriverTable;
 
-/*
- * Structure to store the various fsdev's passed through command line.
- */
-typedef struct FsDriverEntry {
-    char *fsdev_id;
-    char *path;
-    int export_flags;
-    FileOperations *ops;
-} FsDriverEntry;
-
 typedef struct FsDriverListEntry {
     FsDriverEntry fse;
     QTAILQ_ENTRY(FsDriverListEntry) next;
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index cd343e1..642d5e2 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -77,16 +77,19 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
         exit(1);
     }
 
-    if (!fse->path || !conf->tag) {
-        /* we haven't specified a mount_tag or the path */
-        fprintf(stderr, "fsdev with id %s needs path "
-                "and Virtio-9p device needs mount_tag arguments\n",
+    if (!conf->tag) {
+        /* we haven't specified a mount_tag */
+        fprintf(stderr, "fsdev with id %s needs mount_tag arguments\n",
                 conf->fsdev_id);
         exit(1);
     }
 
     s->ctx.export_flags = fse->export_flags;
-    s->ctx.fs_root = g_strdup(fse->path);
+    if (fse->path) {
+        s->ctx.fs_root = g_strdup(fse->path);
+    } else {
+        s->ctx.fs_root = NULL;
+    }
     s->ctx.exops.get_st_gen = NULL;
 
     if (fse->export_flags & V9FS_SM_PASSTHROUGH) {
diff --git a/hw/9pfs/virtio-9p-handle.c b/hw/9pfs/virtio-9p-handle.c
index f97d898..dbecc4c 100644
--- a/hw/9pfs/virtio-9p-handle.c
+++ b/hw/9pfs/virtio-9p-handle.c
@@ -641,7 +641,27 @@ out:
     return ret;
 }
 
+static int handle_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
+{
+    const char *sec_model = qemu_opt_get(opts, "security_model");
+    const char *path = qemu_opt_get(opts, "path");
+
+    if (sec_model) {
+        fprintf(stderr, "Invalid argument security_model specified with handle fsdriver\n");
+        return -1;
+    }
+
+    if (!path) {
+        fprintf(stderr, "fsdev: No path specified.\n");
+        return -1;
+    }
+    fse->path = g_strdup(path);
+    return 0;
+
+}
+
 FileOperations handle_ops = {
+    .parse_opts   = handle_parse_opts,
     .init         = handle_init,
     .lstat        = handle_lstat,
     .readlink     = handle_readlink,
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 371a94d..3ae6ef2 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -756,7 +756,41 @@ static int local_init(FsContext *ctx)
     return err;
 }
 
+static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
+{
+    const char *sec_model = qemu_opt_get(opts, "security_model");
+    const char *path = qemu_opt_get(opts, "path");
+
+    if (!sec_model) {
+        fprintf(stderr, "security model not specified, "
+                "local fs needs security model\nvalid options are:"
+                "\tsecurity_model=[passthrough|mapped|none]\n");
+        return -1;
+    }
+
+    if (!strcmp(sec_model, "passthrough")) {
+        fse->export_flags |= V9FS_SM_PASSTHROUGH;
+    } else if (!strcmp(sec_model, "mapped")) {
+        fse->export_flags |= V9FS_SM_MAPPED;
+    } else if (!strcmp(sec_model, "none")) {
+        fse->export_flags |= V9FS_SM_NONE;
+    } else {
+        fprintf(stderr, "Invalid security model %s specified, valid options are"
+                "\n\t [passthrough|mapped|none]\n", sec_model);
+        return -1;
+    }
+
+    if (!path) {
+        fprintf(stderr, "fsdev: No path specified.\n");
+        return -1;
+    }
+    fse->path = g_strdup(path);
+
+    return 0;
+}
+
 FileOperations local_ops = {
+    .parse_opts = local_parse_opts,
     .init  = local_init,
     .lstat = local_lstat,
     .readlink = local_readlink,
diff --git a/vl.c b/vl.c
index 5372a96..0096865 100644
--- a/vl.c
+++ b/vl.c
@@ -2683,11 +2683,8 @@ int main(int argc, char **argv, char **envp)
                 }
 
                 if (qemu_opt_get(opts, "fsdriver") == NULL ||
-                        qemu_opt_get(opts, "mount_tag") == NULL ||
-                        qemu_opt_get(opts, "path") == NULL) {
-                    fprintf(stderr, "Usage: -virtfs fsdriver,path=/share_path/,"
-                            "[security_model={mapped|passthrough|none}],"
-                            "mount_tag=tag.\n");
+                    qemu_opt_get(opts, "mount_tag") == NULL) {
+                    fprintf(stderr, "Usage: -virtfs fsdriver,mount_tag=tag.\n");
                     exit(1);
                 }
                 fsdev = qemu_opts_create(qemu_find_opts("fsdev"),
@@ -2733,7 +2730,6 @@ int main(int argc, char **argv, char **envp)
                     exit(1);
                 }
                 qemu_opt_set(fsdev, "fsdriver", "synth");
-                qemu_opt_set(fsdev, "path", "/"); /* ignored */
 
                 device = qemu_opts_create(qemu_find_opts("device"), NULL, 0);
                 qemu_opt_set(device, "driver", "virtio-9p-pci");
-- 
1.7.6