Patchwork [07/19] Convert netdev client types to use an enumeration

login
register
mail settings
Submitter Daniel P. Berrange
Date June 7, 2010, 2:42 p.m.
Message ID <1275921752-29420-8-git-send-email-berrange@redhat.com>
Download mbox | patch
Permalink /patch/54870/
State New
Headers show

Comments

Daniel P. Berrange - June 7, 2010, 2:42 p.m.
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 <berrange@redhat.com>
---
 net.c |  124 ++++++++++++++++++++++++++++++++++++++--------------------------
 1 files changed, 74 insertions(+), 50 deletions(-)
Anthony Liguori - June 7, 2010, 3:09 p.m.
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<berrange@redhat.com>
> ---
>   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)
>
Daniel P. Berrange - June 7, 2010, 3:13 p.m.
On Mon, Jun 07, 2010 at 10:09:02AM -0500, Anthony Liguori wrote:
> 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<berrange@redhat.com>
> >---
> >  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.

Ah, I didn't know you could do that for array initializers. That
should result in even stronger compile time validation which is always
nice.

Daniel

Patch

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[] = {
-    {
-        .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)