From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=40857 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OLdws-0005bv-2r for qemu-devel@nongnu.org; Mon, 07 Jun 2010 11:09:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OLdwq-0001Sb-Km for qemu-devel@nongnu.org; Mon, 07 Jun 2010 11:09:09 -0400 Received: from mail-iw0-f173.google.com ([209.85.214.173]:59152) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OLdwq-0001SO-Ft for qemu-devel@nongnu.org; Mon, 07 Jun 2010 11:09:08 -0400 Received: by iwn41 with SMTP id 41so3592966iwn.4 for ; Mon, 07 Jun 2010 08:09:07 -0700 (PDT) Message-ID: <4C0D0B8E.30501@codemonkey.ws> Date: Mon, 07 Jun 2010 10:09:02 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 07/19] Convert netdev client types to use an enumeration References: <1275921752-29420-1-git-send-email-berrange@redhat.com> <1275921752-29420-8-git-send-email-berrange@redhat.com> In-Reply-To: <1275921752-29420-8-git-send-email-berrange@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" Cc: qemu-devel@nongnu.org On 06/07/2010 09:42 AM, Daniel P. Berrange wrote: > Declare an enumeration for all netdev client types, values > matching indexes in the net_client_types array. Use the > enum helpers for the string<-> int conversion of client types. > > Before: > > $ qemu -net type=foo,sfs > qemu: -net type=foo,sfs: Parameter 'type' expects a network client type > > After: > > $ qemu -net type=foo,sfs > qemu: -net type=foo,sfs: Parameter 'type' expects none, nic, user, tap, socket, dump > > Signed-off-by: Daniel P. Berrange > --- > net.c | 124 ++++++++++++++++++++++++++++++++++++++-------------------------- > 1 files changed, 74 insertions(+), 50 deletions(-) > > diff --git a/net.c b/net.c > index efa8b3d..5349001 100644 > --- a/net.c > +++ b/net.c > @@ -42,6 +42,36 @@ static QTAILQ_HEAD(, VLANClientState) non_vlan_clients; > > int default_net = 1; > > +enum { > + NET_CLIENT_NONE, > + NET_CLIENT_NIC, > +#ifdef CONFIG_SLIRP > + NET_CLIENT_USER, > +#endif > + NET_CLIENT_TAP, > + NET_CLIENT_SOCKET, > +#ifdef CONFIG_VDE > + NET_CLIENT_VDE, > +#endif > + NET_CLIENT_DUMP, > + > + NET_CLIENT_LAST > +}; > + > +QEMU_ENUM_DECL(qemu_net_type); > +QEMU_ENUM_IMPL(qemu_net_type, NET_CLIENT_LAST, > + "none", > + "nic", > +#ifdef CONFIG_SLIRP > + "user", > +#endif > + "tap", > + "socket", > +#ifdef CONFIG_VDE > + "vde", > +#endif > + "dump"); > + > /***********************************************************/ > /* network device redirectors */ > > @@ -844,18 +874,15 @@ typedef int (*net_client_init_func)(QemuOpts *opts, > #define NET_MAX_DESC 20 > > static const struct { > - const char *type; > net_client_init_func init; > QemuOptDesc desc[NET_MAX_DESC]; > } net_client_types[] = { > I think: [NET_CLIENT_NONE] = { .desc = {...} }, Would be a bit more robust than relying on explicit ordering. Regards, Anthony Liguori > - { > - .type = "none", > + { /* NET_CLIENT_NONE */ > .desc = { > NET_COMMON_PARAMS_DESC, > { /* end of list */ } > }, > - }, { > - .type = "nic", > + }, { /* NET_CLIENT_NIC */ > .init = net_init_nic, > .desc = { > NET_COMMON_PARAMS_DESC, > @@ -884,8 +911,7 @@ static const struct { > { /* end of list */ } > }, > #ifdef CONFIG_SLIRP > - }, { > - .type = "user", > + }, { /* NET_CLIENT_USER */ > .init = net_init_slirp, > .desc = { > NET_COMMON_PARAMS_DESC, > @@ -945,8 +971,7 @@ static const struct { > { /* end of list */ } > }, > #endif > - }, { > - .type = "tap", > + }, { /* NET_CLIENT_TAP */ > .init = net_init_tap, > .desc = { > NET_COMMON_PARAMS_DESC, > @@ -988,8 +1013,7 @@ static const struct { > #endif /* _WIN32 */ > { /* end of list */ } > }, > - }, { > - .type = "socket", > + }, { /* NET_CLIENT_SOCKET */ > .init = net_init_socket, > .desc = { > NET_COMMON_PARAMS_DESC, > @@ -1013,8 +1037,7 @@ static const struct { > { /* end of list */ } > }, > #ifdef CONFIG_VDE > - }, { > - .type = "vde", > + }, { /* NET_CLIENT_VDE */ > .init = net_init_vde, > .desc = { > NET_COMMON_PARAMS_DESC, > @@ -1038,8 +1061,7 @@ static const struct { > { /* end of list */ } > }, > #endif > - }, { > - .type = "dump", > + }, { /* NET_CLIENT_DUMP */ > .init = net_init_dump, > .desc = { > NET_COMMON_PARAMS_DESC, > @@ -1055,30 +1077,41 @@ static const struct { > { /* end of list */ } > }, > }, > - { /* end of list */ } > }; > +verify(ARRAY_SIZE(net_client_types) == NET_CLIENT_LAST); > > int net_client_init(Monitor *mon, QemuOpts *opts, int is_netdev) > { > const char *name; > - const char *type; > - int i; > + const char *typestr; > + VLANState *vlan = NULL; > + int type; > > - type = qemu_opt_get(opts, "type"); > - if (!type) { > + typestr = qemu_opt_get(opts, "type"); > + if (!typestr) { > qerror_report(QERR_MISSING_PARAMETER, "type"); > return -1; > } > + type = qemu_net_type_from_string(typestr); > + > + if (type< 0) { > + char *valid = qemu_net_type_to_string_list(); > + qerror_report(QERR_INVALID_PARAMETER_VALUE, "type", > + valid); > + qemu_free(valid); > + return -1; > + } > > if (is_netdev) { > - if (strcmp(type, "tap") != 0&& > + if (type< 0 || > + (type != NET_CLIENT_TAP&& > #ifdef CONFIG_SLIRP > - strcmp(type, "user") != 0&& > + type != NET_CLIENT_USER&& > #endif > #ifdef CONFIG_VDE > - strcmp(type, "vde") != 0&& > + type != NET_CLIENT_VDE&& > #endif > - strcmp(type, "socket") != 0) { > + type != NET_CLIENT_SOCKET)) { > qerror_report(QERR_INVALID_PARAMETER_VALUE, "type", > "a netdev backend type"); > return -1; > @@ -1103,35 +1136,26 @@ int net_client_init(Monitor *mon, QemuOpts *opts, int is_netdev) > name = qemu_opt_get(opts, "name"); > } > > - for (i = 0; net_client_types[i].type != NULL; i++) { > - if (!strcmp(net_client_types[i].type, type)) { > - VLANState *vlan = NULL; > - > - if (qemu_opts_validate(opts,&net_client_types[i].desc[0]) == -1) { > - return -1; > - } > - > - /* Do not add to a vlan if it's a -netdev or a nic with a > - * netdev= parameter. */ > - if (!(is_netdev || > - (strcmp(type, "nic") == 0&& qemu_opt_get(opts, "netdev")))) { > - vlan = qemu_find_vlan(qemu_opt_get_number(opts, "vlan", 0), 1); > - } > + if (qemu_opts_validate(opts,&net_client_types[type].desc[0]) == -1) { > + return -1; > + } > > - if (net_client_types[i].init) { > - if (net_client_types[i].init(opts, mon, name, vlan)< 0) { > - /* TODO push error reporting into init() methods */ > - qerror_report(QERR_DEVICE_INIT_FAILED, type); > - return -1; > - } > - } > - return 0; > - } > + /* Do not add to a vlan if it's a -netdev or a nic with a > + * netdev= parameter. */ > + if (!(is_netdev || > + (type == NET_CLIENT_NIC&& qemu_opt_get(opts, "netdev")))) { > + vlan = qemu_find_vlan(qemu_opt_get_number(opts, "vlan", 0), 1); > } > > - qerror_report(QERR_INVALID_PARAMETER_VALUE, "type", > - "a network client type"); > - return -1; > + if (net_client_types[type].init) { > + if (net_client_types[type].init(opts, mon, name, vlan)< 0) { > + /* TODO push error reporting into init() methods */ > + qerror_report(QERR_DEVICE_INIT_FAILED, > + qemu_net_type_to_string(type)); > + return -1; > + } > + } > + return 0; > } > > static int net_host_check_device(const char *device) >