Re: [Qemu-devel] [PATCH 01/12] net: add a new object netfilter

[Thomas Huth]

On Wednesday, July 29, 2015 12:51:45 PM,
"Yang Hongyang" <address@hidden> wrote:
> 
> Signed-off-by: Yang Hongyang <address@hidden>

Maybe add at least a very short patch description? You're doing
a little bit more than just "add a new object netfilter" as
mentioned in the title...

> ---
>  include/net/filter.h    | 15 +++++++++++++++
>  include/sysemu/sysemu.h |  1 +
>  net/Makefile.objs       |  1 +
>  net/filter.c            | 27 +++++++++++++++++++++++++++
>  qemu-options.hx         |  1 +
>  vl.c                    | 13 +++++++++++++
>  6 files changed, 58 insertions(+)
>  create mode 100644 include/net/filter.h
>  create mode 100644 net/filter.c
> 
> diff --git a/include/net/filter.h b/include/net/filter.h
> new file mode 100644
> index 0000000..4242ded
> --- /dev/null
> +++ b/include/net/filter.h
> @@ -0,0 +1,15 @@
> +/*
> + * Copyright (c) 2015 FUJITSU LIMITED
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later.  See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef QEMU_NET_FILTER_H
> +#define QEMU_NET_FILTER_H
> +
> +#include "qemu-common.h"
> +
> +int net_init_filters(void);
> +
> +#endif /* QEMU_NET_FILTER_H */
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 44570d1..15d6d00 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -212,6 +212,7 @@ extern QemuOptsList qemu_chardev_opts;
>  extern QemuOptsList qemu_device_opts;
>  extern QemuOptsList qemu_netdev_opts;
>  extern QemuOptsList qemu_net_opts;
> +extern QemuOptsList qemu_netfilter_opts;
>  extern QemuOptsList qemu_global_opts;
>  extern QemuOptsList qemu_mon_opts;
>  
> diff --git a/net/Makefile.objs b/net/Makefile.objs
> index ec19cb3..914aec0 100644
> --- a/net/Makefile.objs
> +++ b/net/Makefile.objs
> @@ -13,3 +13,4 @@ common-obj-$(CONFIG_HAIKU) += tap-haiku.o
>  common-obj-$(CONFIG_SLIRP) += slirp.o
>  common-obj-$(CONFIG_VDE) += vde.o
>  common-obj-$(CONFIG_NETMAP) += netmap.o
> +common-obj-y += filter.o
> diff --git a/net/filter.c b/net/filter.c
> new file mode 100644
> index 0000000..4e40f08
> --- /dev/null
> +++ b/net/filter.c
> @@ -0,0 +1,27 @@
> +/*
> + * Copyright (c) 2015 FUJITSU LIMITED
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later.  See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu-common.h"
> +#include "net/filter.h"
> +
> +int net_init_filters(void)
> +{
> +    return 0;
> +}
> +
> +QemuOptsList qemu_netfilter_opts = {
> +    .name = "netfilter",
> +    .implied_opt_name = "type",
> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_netfilter_opts.head),
> +    .desc = {
> +        /*
> +         * no elements => accept any params
> +         * validation will happen later
> +         */
> +        { /* end of list */ }
> +    },
> +};
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 8c9add9..8c1eb30 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1575,6 +1575,7 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
>      "socket][,vlan=n][,option][,option][,...]\n"
>      "                old way to initialize a host network interface\n"
>      "                (use the -netdev option if possible instead)\n",
>      QEMU_ARCH_ALL)
> +DEF("netfilter", HAS_ARG, QEMU_OPTION_netfilter, "", QEMU_ARCH_ALL)
>  STEXI
>  @item -net nic[,address@hidden,address@hidden,address@hidden
>  [,address@hidden,address@hidden,address@hidden
>  @findex -net
> diff --git a/vl.c b/vl.c
> index 5856396..1a0ebe1 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -75,6 +75,7 @@ int main(int argc, char **argv)
>  #include "monitor/qdev.h"
>  #include "sysemu/bt.h"
>  #include "net/net.h"
> +#include "net/filter.h"
>  #include "net/slirp.h"
>  #include "monitor/monitor.h"
>  #include "ui/console.h"
> @@ -2998,6 +2999,7 @@ int main(int argc, char **argv, char **envp)
>      qemu_add_opts(&qemu_device_opts);
>      qemu_add_opts(&qemu_netdev_opts);
>      qemu_add_opts(&qemu_net_opts);
> +    qemu_add_opts(&qemu_netfilter_opts);
>      qemu_add_opts(&qemu_rtc_opts);
>      qemu_add_opts(&qemu_global_opts);
>      qemu_add_opts(&qemu_mon_opts);
> @@ -3284,6 +3286,13 @@ int main(int argc, char **argv, char **envp)
>                      exit(1);
>                  }
>                  break;
> +            case QEMU_OPTION_netfilter:
> +                opts = qemu_opts_parse_noisily(qemu_find_opts("netfilter"),
> +                                               optarg, true);
> +                if (!opts) {
> +                    exit(1);
> +                }
> +                break;
>  #ifdef CONFIG_LIBISCSI
>              case QEMU_OPTION_iscsi:
>                  opts = qemu_opts_parse_noisily(qemu_find_opts("iscsi"),
> @@ -4413,6 +4422,10 @@ int main(int argc, char **argv, char **envp)
>          exit(1);
>      }
>  
> +    if (net_init_filters() < 0) {
> +        exit(1);
> +    }
> +
>  #ifdef CONFIG_TPM
>      if (tpm_init() < 0) {
>          exit(1);

Looks good to me.

Reviewed-by: Thomas Huth <address@hidden>