From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:42409) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R9Hzb-0004Iy-MI for qemu-devel@nongnu.org; Thu, 29 Sep 2011 10:53:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R9Hza-0002OD-9f for qemu-devel@nongnu.org; Thu, 29 Sep 2011 10:53:43 -0400 Received: from mail-vx0-f173.google.com ([209.85.220.173]:47433) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R9Hza-0002O7-6M for qemu-devel@nongnu.org; Thu, 29 Sep 2011 10:53:42 -0400 Received: by vcbfl10 with SMTP id fl10so609662vcb.4 for ; Thu, 29 Sep 2011 07:53:41 -0700 (PDT) Message-ID: <4E848672.9040204@codemonkey.ws> Date: Thu, 29 Sep 2011 09:53:38 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1317304660-18072-1-git-send-email-levinsasha928@gmail.com> In-Reply-To: <1317304660-18072-1-git-send-email-levinsasha928@gmail.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] tap: Add optional parameters to up/down script List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sasha Levin Cc: swoop3r@gmx.de, Anthony Liguori , qemu-devel@nongnu.org On 09/29/2011 08:57 AM, Sasha Levin wrote: > This allows the user to add custom parameters to the up or down > scripts. > > Cc: Anthony Liguori > Signed-off-by: Sasha Levin > --- > net.c | 8 ++++++++ > net/tap.c | 37 ++++++++++++++++++++++++++++++------- > 2 files changed, 38 insertions(+), 7 deletions(-) > > diff --git a/net.c b/net.c > index d05930c..bb27598 100644 > --- a/net.c > +++ b/net.c > @@ -952,10 +952,18 @@ static const struct { > .type = QEMU_OPT_STRING, > .help = "script to initialize the interface", > }, { > + .name = "scriptparams", > + .type = QEMU_OPT_STRING, > + .help = "parameters for the initialization script", > + }, { > .name = "downscript", > .type = QEMU_OPT_STRING, > .help = "script to shut down the interface", > }, { > + .name = "downscriptparams", > + .type = QEMU_OPT_STRING, > + .help = "parameters for the deinitialization script", > + }, { > .name = "sndbuf", > .type = QEMU_OPT_SIZE, > .help = "send buffer limit" > diff --git a/net/tap.c b/net/tap.c > index 1f26dc9..5a9141e 100644 > --- a/net/tap.c > +++ b/net/tap.c > @@ -52,7 +52,7 @@ typedef struct TAPState { > VLANClientState nc; > int fd; > char down_script[1024]; > - char down_script_arg[128]; > + char down_script_arg[1024]; > uint8_t buf[TAP_BUFSIZE]; > unsigned int read_poll : 1; > unsigned int write_poll : 1; > @@ -392,7 +392,8 @@ static int net_tap_init(QemuOpts *opts, int *vnet_hdr) > { > int fd, vnet_hdr_required; > char ifname[128] = {0,}; > - const char *setup_script; > + const char *setup_script, *setup_script_params; > + char setup_script_formatted[1024]; > > if (qemu_opt_get(opts, "ifname")) { > pstrcpy(ifname, sizeof(ifname), qemu_opt_get(opts, "ifname")); > @@ -411,10 +412,16 @@ static int net_tap_init(QemuOpts *opts, int *vnet_hdr) > } > > setup_script = qemu_opt_get(opts, "script"); > + setup_script_params = qemu_opt_get(opts, "scriptparams"); > + if (setup_script_params == NULL) > + setup_script_params = ""; > + > + snprintf(setup_script_formatted, sizeof(setup_script_formatted), > + "%s %s", ifname, setup_script_params); > if (setup_script&& > setup_script[0] != '\0'&& > strcmp(setup_script, "no") != 0&& > - launch_script(setup_script, ifname, fd)) { > + launch_script(setup_script, setup_script_formatted, fd)) { This is a little awkward. launch_script uses execv() so you'll effectively do: /etc/qemu-ifup "tap0 myargument" When what you'd expect to happen is: /etc/qemu-ifup "tap0" "myargument" I'd suggest adding another parameter to launch script so that you can preserve the argument ordering. As an added bonus, you won't have to truncate at 1024 bytes anymore. Regards, Anthony Liguori > close(fd); > return -1; > } > @@ -432,9 +439,12 @@ int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan > if (qemu_opt_get(opts, "fd")) { > if (qemu_opt_get(opts, "ifname") || > qemu_opt_get(opts, "script") || > + qemu_opt_get(opts, "scriptparams") || > qemu_opt_get(opts, "downscript") || > + qemu_opt_get(opts, "downscriptparams") || > qemu_opt_get(opts, "vnet_hdr")) { > - error_report("ifname=, script=, downscript= and vnet_hdr= is invalid with fd="); > + error_report("ifname=, script=, downscript=, scriptparams=, " > + "downscriptparams= and vnet_hdr= is invalid with fd="); > return -1; > } > > @@ -455,6 +465,14 @@ int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan > qemu_opt_set(opts, "downscript", DEFAULT_NETWORK_DOWN_SCRIPT); > } > > + if (!qemu_opt_get(opts, "scriptparams")) { > + qemu_opt_set(opts, "scriptparams", ""); > + } > + > + if (!qemu_opt_get(opts, "downscriptparams")) { > + qemu_opt_set(opts, "downscriptparams", ""); > + } > + > fd = net_tap_init(opts,&vnet_hdr); > if (fd == -1) { > return -1; > @@ -475,18 +493,23 @@ int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan > snprintf(s->nc.info_str, sizeof(s->nc.info_str), "fd=%d", fd); > } else { > const char *ifname, *script, *downscript; > + const char *scriptparams, *downscriptparams; > > ifname = qemu_opt_get(opts, "ifname"); > script = qemu_opt_get(opts, "script"); > downscript = qemu_opt_get(opts, "downscript"); > + scriptparams = qemu_opt_get(opts, "scriptparams"); > + downscriptparams = qemu_opt_get(opts, "downscriptparams"); > > snprintf(s->nc.info_str, sizeof(s->nc.info_str), > - "ifname=%s,script=%s,downscript=%s", > - ifname, script, downscript); > + "ifname=%s,script=%s,scriptparams=%s,downscript=%s," > + "downscriptparams=%s", ifname, script, scriptparams, > + downscript, downscriptparams); > > if (strcmp(downscript, "no") != 0) { > snprintf(s->down_script, sizeof(s->down_script), "%s", downscript); > - snprintf(s->down_script_arg, sizeof(s->down_script_arg), "%s", ifname); > + snprintf(s->down_script_arg, sizeof(s->down_script_arg), "%s %s", ifname, > + downscriptparams); > } > } >