Re: [Qemu-devel] [PATCH] vl.c: Replace -virtfs string manipulation with QemuOpts

[Venkateswararao Jujjuri (JV)]

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 <address@hidden>

Looks good to me with one nitpick below.. otherwise
Reviewed-by: Venkateswararao Jujjuri <address@hidden>

> ---
>  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: