From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=47797 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Pzbu7-0001rR-Pq for qemu-devel@nongnu.org; Tue, 15 Mar 2011 17:35:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Pzbu5-0000p5-JN for qemu-devel@nongnu.org; Tue, 15 Mar 2011 17:35:46 -0400 Received: from e32.co.us.ibm.com ([32.97.110.150]:42198) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Pzbu5-0000or-Dq for qemu-devel@nongnu.org; Tue, 15 Mar 2011 17:35:45 -0400 Received: from d03relay02.boulder.ibm.com (d03relay02.boulder.ibm.com [9.17.195.227]) by e32.co.us.ibm.com (8.14.4/8.13.1) with ESMTP id p2FLP34O012160 for ; Tue, 15 Mar 2011 15:25:03 -0600 Received: from d03av02.boulder.ibm.com (d03av02.boulder.ibm.com [9.17.195.168]) by d03relay02.boulder.ibm.com (8.13.8/8.13.8/NCO v9.1) with ESMTP id p2FLZfuP109922 for ; Tue, 15 Mar 2011 15:35:43 -0600 Received: from d03av02.boulder.ibm.com (loopback [127.0.0.1]) by d03av02.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p2FLZd90020646 for ; Tue, 15 Mar 2011 15:35:40 -0600 Message-ID: <4D7FDBA8.4030900@linux.vnet.ibm.com> Date: Tue, 15 Mar 2011 14:35:36 -0700 From: "Venkateswararao Jujjuri (JV)" MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] vl.c: Replace -virtfs string manipulation with QemuOpts References: <1300039734-15087-1-git-send-email-stefanha@linux.vnet.ibm.com> In-Reply-To: <1300039734-15087-1-git-send-email-stefanha@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Anthony Liguori , qemu-devel@nongnu.org, "Aneesh Kumar K.V" 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 Looks good to me with one nitpick below.. otherwise Reviewed-by: Venkateswararao Jujjuri > --- > 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: