Patchwork vl.c: Replace -virtfs string manipulation with QemuOpts

login
register
mail settings
Submitter Stefan Hajnoczi
Date March 13, 2011, 6:08 p.m.
Message ID <1300039734-15087-1-git-send-email-stefanha@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/86610/
State New
Headers show

Comments

Stefan Hajnoczi - March 13, 2011, 6:08 p.m.
The -virtfs option creates an fsdev representing the pass-through file
system and a guest-visible virtio-9p-pci device that can access this
file system.  This patch replaces the string manipulation used to build
and reparse option lists with direct QemuOpts calls.  Removing the
string manipulation code makes it easier to maintain and less error
prone.

An error message is also updated to use "mount_tag" instead of
"mnt_tag".

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 vl.c |   56 +++++++++++++++++++-------------------------------------
 1 files changed, 19 insertions(+), 37 deletions(-)

Aneesh: This approach should make a host page bypass option easier to add.
jvrao - March 15, 2011, 9:35 p.m.
On 3/13/2011 11:08 AM, Stefan Hajnoczi wrote:
> The -virtfs option creates an fsdev representing the pass-through file
> system and a guest-visible virtio-9p-pci device that can access this
> file system.  This patch replaces the string manipulation used to build
> and reparse option lists with direct QemuOpts calls.  Removing the
> string manipulation code makes it easier to maintain and less error
> prone.
> 
> An error message is also updated to use "mount_tag" instead of
> "mnt_tag".
> 


> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Looks good to me with one nitpick below.. otherwise
Reviewed-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>

> ---
>  vl.c |   56 +++++++++++++++++++-------------------------------------
>  1 files changed, 19 insertions(+), 37 deletions(-)
> 
> Aneesh: This approach should make a host page bypass option easier to add.
> 
> diff --git a/vl.c b/vl.c
> index 5e007a7..f500c4c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2411,9 +2411,8 @@ int main(int argc, char **argv, char **envp)
>                  }
>                  break;
>              case QEMU_OPTION_virtfs: {
> -                char *arg_fsdev = NULL;
> -                char *arg_9p = NULL;
> -                int len = 0;
> +                QemuOpts *fsdev;
> +                QemuOpts *device;
> 
>                  olist = qemu_find_opts("virtfs");
>                  if (!olist) {
> @@ -2432,45 +2431,28 @@ int main(int argc, char **argv, char **envp)
>                          qemu_opt_get(opts, "security_model") == NULL) {
>                      fprintf(stderr, "Usage: -virtfs fstype,path=/share_path/,"
>                              "security_model=[mapped|passthrough|none],"
> -                            "mnt_tag=tag.\n");
> +                            "mount_tag=tag.\n");
>                      exit(1);
>                  }
> 
> -                len = strlen(",id=,path=,security_model=");
> -                len += strlen(qemu_opt_get(opts, "fstype"));
> -                len += strlen(qemu_opt_get(opts, "mount_tag"));
> -                len += strlen(qemu_opt_get(opts, "path"));
> -                len += strlen(qemu_opt_get(opts, "security_model"));
> -                arg_fsdev = qemu_malloc((len + 1) * sizeof(*arg_fsdev));
> -
> -                snprintf(arg_fsdev, (len + 1) * sizeof(*arg_fsdev),
> -                         "%s,id=%s,path=%s,security_model=%s",
> -                         qemu_opt_get(opts, "fstype"),
> -                         qemu_opt_get(opts, "mount_tag"),
> -                         qemu_opt_get(opts, "path"),
> -                         qemu_opt_get(opts, "security_model"));
> -
> -                len = strlen("virtio-9p-pci,fsdev=,mount_tag=");
> -                len += 2*strlen(qemu_opt_get(opts, "mount_tag"));
> -                arg_9p = qemu_malloc((len + 1) * sizeof(*arg_9p));
> -
> -                snprintf(arg_9p, (len + 1) * sizeof(*arg_9p),
> -                         "virtio-9p-pci,fsdev=%s,mount_tag=%s",
> -                         qemu_opt_get(opts, "mount_tag"),
> -                         qemu_opt_get(opts, "mount_tag"));
> -
> -                if (!qemu_opts_parse(qemu_find_opts("fsdev"), arg_fsdev, 1)) {
> -                    fprintf(stderr, "parse error [fsdev]: %s\n", optarg);
> +                fsdev = qemu_opts_create(qemu_find_opts("fsdev"),
> +                                         qemu_opt_get(opts, "mount_tag"), 1);
> +                if (!fsdev) {
> +                    fprintf(stderr, "duplicate fsdev: %s\n",

"duplicate fsdev id: %s\n"

- JV

> +                            qemu_opt_get(opts, "mount_tag"));
>                      exit(1);
>                  }
> -
> -                if (!qemu_opts_parse(qemu_find_opts("device"), arg_9p, 1)) {
> -                    fprintf(stderr, "parse error [device]: %s\n", optarg);
> -                    exit(1);
> -                }
> -
> -                qemu_free(arg_fsdev);
> -                qemu_free(arg_9p);
> +                qemu_opt_set(fsdev, "fstype", qemu_opt_get(opts, "fstype"));
> +                qemu_opt_set(fsdev, "path", qemu_opt_get(opts, "path"));
> +                qemu_opt_set(fsdev, "security_model",
> +                             qemu_opt_get(opts, "security_model"));
> +
> +                device = qemu_opts_create(qemu_find_opts("device"), NULL, 0);
> +                qemu_opt_set(device, "driver", "virtio-9p-pci");
> +                qemu_opt_set(device, "fsdev",
> +                             qemu_opt_get(opts, "mount_tag"));
> +                qemu_opt_set(device, "mount_tag",
> +                             qemu_opt_get(opts, "mount_tag"));
>                  break;
>              }
>              case QEMU_OPTION_serial:

Patch

diff --git a/vl.c b/vl.c
index 5e007a7..f500c4c 100644
--- a/vl.c
+++ b/vl.c
@@ -2411,9 +2411,8 @@  int main(int argc, char **argv, char **envp)
                 }
                 break;
             case QEMU_OPTION_virtfs: {
-                char *arg_fsdev = NULL;
-                char *arg_9p = NULL;
-                int len = 0;
+                QemuOpts *fsdev;
+                QemuOpts *device;
 
                 olist = qemu_find_opts("virtfs");
                 if (!olist) {
@@ -2432,45 +2431,28 @@  int main(int argc, char **argv, char **envp)
                         qemu_opt_get(opts, "security_model") == NULL) {
                     fprintf(stderr, "Usage: -virtfs fstype,path=/share_path/,"
                             "security_model=[mapped|passthrough|none],"
-                            "mnt_tag=tag.\n");
+                            "mount_tag=tag.\n");
                     exit(1);
                 }
 
-                len = strlen(",id=,path=,security_model=");
-                len += strlen(qemu_opt_get(opts, "fstype"));
-                len += strlen(qemu_opt_get(opts, "mount_tag"));
-                len += strlen(qemu_opt_get(opts, "path"));
-                len += strlen(qemu_opt_get(opts, "security_model"));
-                arg_fsdev = qemu_malloc((len + 1) * sizeof(*arg_fsdev));
-
-                snprintf(arg_fsdev, (len + 1) * sizeof(*arg_fsdev),
-                         "%s,id=%s,path=%s,security_model=%s",
-                         qemu_opt_get(opts, "fstype"),
-                         qemu_opt_get(opts, "mount_tag"),
-                         qemu_opt_get(opts, "path"),
-                         qemu_opt_get(opts, "security_model"));
-
-                len = strlen("virtio-9p-pci,fsdev=,mount_tag=");
-                len += 2*strlen(qemu_opt_get(opts, "mount_tag"));
-                arg_9p = qemu_malloc((len + 1) * sizeof(*arg_9p));
-
-                snprintf(arg_9p, (len + 1) * sizeof(*arg_9p),
-                         "virtio-9p-pci,fsdev=%s,mount_tag=%s",
-                         qemu_opt_get(opts, "mount_tag"),
-                         qemu_opt_get(opts, "mount_tag"));
-
-                if (!qemu_opts_parse(qemu_find_opts("fsdev"), arg_fsdev, 1)) {
-                    fprintf(stderr, "parse error [fsdev]: %s\n", optarg);
+                fsdev = qemu_opts_create(qemu_find_opts("fsdev"),
+                                         qemu_opt_get(opts, "mount_tag"), 1);
+                if (!fsdev) {
+                    fprintf(stderr, "duplicate fsdev: %s\n",
+                            qemu_opt_get(opts, "mount_tag"));
                     exit(1);
                 }
-
-                if (!qemu_opts_parse(qemu_find_opts("device"), arg_9p, 1)) {
-                    fprintf(stderr, "parse error [device]: %s\n", optarg);
-                    exit(1);
-                }
-
-                qemu_free(arg_fsdev);
-                qemu_free(arg_9p);
+                qemu_opt_set(fsdev, "fstype", qemu_opt_get(opts, "fstype"));
+                qemu_opt_set(fsdev, "path", qemu_opt_get(opts, "path"));
+                qemu_opt_set(fsdev, "security_model",
+                             qemu_opt_get(opts, "security_model"));
+
+                device = qemu_opts_create(qemu_find_opts("device"), NULL, 0);
+                qemu_opt_set(device, "driver", "virtio-9p-pci");
+                qemu_opt_set(device, "fsdev",
+                             qemu_opt_get(opts, "mount_tag"));
+                qemu_opt_set(device, "mount_tag",
+                             qemu_opt_get(opts, "mount_tag"));
                 break;
             }
             case QEMU_OPTION_serial: