From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:46317) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQh41-0006sw-I1 for qemu-devel@nongnu.org; Tue, 23 Oct 2012 12:10:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TQh3q-0004O9-Vm for qemu-devel@nongnu.org; Tue, 23 Oct 2012 12:10:45 -0400 Received: from e9.ny.us.ibm.com ([32.97.182.139]:33751) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQh3q-000478-Rd for qemu-devel@nongnu.org; Tue, 23 Oct 2012 12:10:34 -0400 Received: from /spool/local by e9.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 23 Oct 2012 12:00:04 -0400 Received: from d01relay01.pok.ibm.com (d01relay01.pok.ibm.com [9.56.227.233]) by d01dlp02.pok.ibm.com (Postfix) with ESMTP id 6A12C6E8066 for ; Tue, 23 Oct 2012 11:59:55 -0400 (EDT) Received: from d01av01.pok.ibm.com (d01av01.pok.ibm.com [9.56.224.215]) by d01relay01.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q9NFxt6X282924 for ; Tue, 23 Oct 2012 11:59:55 -0400 Received: from d01av01.pok.ibm.com (loopback [127.0.0.1]) by d01av01.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q9NFxsoa019710 for ; Tue, 23 Oct 2012 11:59:54 -0400 Message-ID: <5086BEF4.7090101@linux.vnet.ibm.com> Date: Tue, 23 Oct 2012 11:59:48 -0400 From: Corey Bryant MIME-Version: 1.0 References: <1350971732-16621-1-git-send-email-otubo@linux.vnet.ibm.com> <1350971732-16621-4-git-send-email-otubo@linux.vnet.ibm.com> In-Reply-To: <1350971732-16621-4-git-send-email-otubo@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCHv2 4/4] Warning messages on net devices hotplug List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Otubo Cc: pmoore@redhat.com, aliguori@us.ibm.com, qemu-devel@nongnu.org On 10/23/2012 01:55 AM, Eduardo Otubo wrote: > With the inclusion of the new "double whitelist" seccomp filter, Qemu > won't be able to execve() in runtime, thus, no hotplug net devices > allowed. > > v2: * Error messages moved to the backend function, net_init_tap(), recommended > by Paolo Bonzini > * Documentation added to QMP and HMP commands, and also to the Qemu options. > > Signed-off-by: Eduardo Otubo > --- > hmp-commands.hx | 4 ++-- > net.c | 1 + > net/tap.c | 5 +++++ > qemu-options.hx | 3 ++- > qmp-commands.hx | 3 ++- > 5 files changed, 12 insertions(+), 4 deletions(-) > > diff --git a/hmp-commands.hx b/hmp-commands.hx > index e0b537d..3e28501 100644 > --- a/hmp-commands.hx > +++ b/hmp-commands.hx > @@ -1068,7 +1068,7 @@ ETEXI > .name = "host_net_add", > .args_type = "device:s,opts:s?", > .params = "tap|user|socket|vde|dump [options]", > - .help = "add host VLAN client", > + .help = "add host VLAN client, feature disabled when -sandbox is in use", Maybe the "feature disabled.." part should be in parenthesis? There's also another section in hmp-commands.hx a few lines below this code where you can add the same note. > .mhandler.cmd = net_host_device_add, > }, > > @@ -1096,7 +1096,7 @@ ETEXI > .name = "netdev_add", > .args_type = "netdev:O", > .params = "[user|tap|socket],id=str[,prop=value][,...]", > - .help = "add host network device", > + .help = "add host network device, feature disabled when -sandbox is in use", Same comments as above. > .mhandler.cmd = hmp_netdev_add, > }, > > diff --git a/net.c b/net.c > index ae4bc0d..02188f0 100644 > --- a/net.c > +++ b/net.c > @@ -765,6 +765,7 @@ void net_host_device_add(Monitor *mon, const QDict *qdict) > qemu_opt_set(opts, "type", device); > > net_client_init(opts, 0, &local_err); > + You can get rid of this change. > if (error_is_set(&local_err)) { > qerror_report_err(local_err); > error_free(local_err); > diff --git a/net/tap.c b/net/tap.c > index df89caa..dd8c79b 100644 > --- a/net/tap.c > +++ b/net/tap.c > @@ -590,6 +590,11 @@ static int net_tap_init(const NetdevTapOptions *tap, int *vnet_hdr, > int net_init_tap(const NetClientOptions *opts, const char *name, > NetClientState *peer) > { > +#ifdef CONFIG_SECCOMP > + error_report("Cannot hotplug TAP device when -sandbox is in effect"); > + return -1; > +#endif > + It seems like it would make more sense to put this code after the local variables are defined. But more importantly, does this prevent adding a tap device on the command line? If so, that's not good. Also I don't think you are preventing a "netdev_add bridge" any more in v2, which also calls execve(). > const NetdevTapOptions *tap; > > int fd, vnet_hdr = 0; > diff --git a/qemu-options.hx b/qemu-options.hx > index 7d97f96..02afba3 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -2767,7 +2767,8 @@ STEXI > @item -sandbox > @findex -sandbox > Enable Seccomp mode 2 system call filter. 'on' will enable syscall filtering and 'off' will > -disable it. The default is 'off'. > +disable it. The default is 'on'. Note that when using the '-sandbox on' option the hot plug > +of new devices will be disabled. Only network devices are prevented, right? Also, as I mentioned before, can you limit this to the subset of options that cause execve() to be issued? For example, can we allow libvirt to pass an fd for hotplugging a network device (e.g. netdev_add tap,fd=23)? I don't know for sure but I'm guessing libvirt does that. Also I think it would be worth-while to update the qemu-kvm --help output with a similar note too. > ETEXI > > DEF("readconfig", HAS_ARG, QEMU_OPTION_readconfig, > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 2f8477e..cccb8f1 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -757,7 +757,8 @@ Example: > > Note: The supported device options are the same ones supported by the '-net' > command-line argument, which are listed in the '-help' output or QEMU's > - manual > + manual. Also note that the hot plug is disabled when -sandbox is in > + effect Not all hotplug abilities are disabled. Just network devices. This is missing a period too. > > EQMP > -- Regards, Corey Bryant