* [OPW PATCH V3] tools: xl: refactor code to parse network device options
@ 2014-10-21 14:19 Alexandra Sandulescu
2014-10-21 14:22 ` Alexandra Sandulescu
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Alexandra Sandulescu @ 2014-10-21 14:19 UTC (permalink / raw)
To: xen-devel
Cc: Alexandra Sandulescu, wei.liu2, ian.campbell, Alexandra Sandulescu
From: Alexandra Sandulescu <alexandra.sandulescu1@1and1.ro>
This patch removes duplicate code in /tools/libxl/xl_cmdimpl.c by
adding parse_nic_config function. This function parses configuration
data and adds the information into libxl_device_nic struct. It is
called in both main_networkattach and parse_config_data functions
to replace duplicate code.
Signed-off-by: Alexandra Sandulescu <alecsandra.sandulescu@gmail.com>
---
tools/libxl/xl_cmdimpl.c | 188 ++++++++++++++++++-----------------------------
1 file changed, 71 insertions(+), 117 deletions(-)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index ea43761..31a0dcb 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -903,6 +903,74 @@ static void replace_string(char **str, const char *val)
*str = xstrdup(val);
}
+/* for now used only by main_networkattach, but can be reused elsewhere */
+static int match_option_size(const char *prefix, size_t len,
+ char *arg, char **argopt)
+{
+ int rc = strncmp(prefix, arg, len);
+ if (!rc) *argopt = arg+len;
+ return !rc;
+}
+#define MATCH_OPTION(prefix, arg, oparg) \
+ match_option_size((prefix "="), sizeof((prefix)), (arg), &(oparg))
+
+/* Parses network data and adds info into nic
+ * Returns 1 if the input token does not match one of the keys
+ * or parsed values are not correct. Successful parse returns 0 */
+static int parse_nic_config(libxl_device_nic *nic, XLU_Config **config, char *token)
+{
+ char *endptr, *oparg;
+ int i;
+ unsigned int val;
+
+ if (MATCH_OPTION("type", token, oparg)) {
+ if (!strcmp("vif", oparg)) {
+ nic->nictype = LIBXL_NIC_TYPE_VIF;
+ } else if (!strcmp("ioemu", oparg)) {
+ nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
+ } else {
+ fprintf(stderr, "Invalid parameter `type'.\n");
+ return 1;
+ }
+ } else if (MATCH_OPTION("mac", token, oparg)) {
+ for (i = 0; i < 6; i++){
+ val = strtoul(oparg, &endptr, 16);
+ if ((oparg == endptr) || (val > 255)) {
+ fprintf(stderr, "Invalid parameter `mac'.\n");
+ return 1;
+ }
+ nic->mac[i] = val;
+ oparg = endptr + 1;
+ }
+ } else if (MATCH_OPTION("bridge", token, oparg)) {
+ replace_string(&nic->bridge, oparg);
+ } else if (MATCH_OPTION("netdev", token, oparg)) {
+ fprintf(stderr, "the netdev parameter is deprecated, "
+ "please use gatewaydev instead\n");
+ replace_string(&nic->gatewaydev, oparg);
+ } else if (MATCH_OPTION("gatewaydev", token, oparg)) {
+ replace_string(&nic->gatewaydev, oparg);
+ } else if (MATCH_OPTION("ip", token, oparg)) {
+ replace_string(&nic->ip, oparg);
+ } else if (MATCH_OPTION("script", token, oparg)) {
+ replace_string(&nic->script, oparg);
+ } else if (MATCH_OPTION("backend", token, oparg)) {
+ replace_string(&nic->backend_domname, oparg);
+ } else if (MATCH_OPTION("vifname", token, oparg)) {
+ replace_string(&nic->ifname, oparg);
+ } else if (MATCH_OPTION("model", token, oparg)) {
+ replace_string(&nic->model, oparg);
+ } else if (MATCH_OPTION("rate", token, oparg)) {
+ parse_vif_rate(config, oparg, nic);
+ } else if (MATCH_OPTION("accel", token, oparg)) {
+ fprintf(stderr, "the accel parameter for vifs is currently not supported\n");
+ } else {
+ fprintf(stderr, "unrecognized argument `%s'\n", token);
+ return 1;
+ }
+ return 0;
+}
+
static void parse_config_data(const char *config_source,
const char *config_data,
int config_len,
@@ -1530,7 +1598,7 @@ static void parse_config_data(const char *config_source,
while ((buf = xlu_cfg_get_listitem (nics, d_config->num_nics)) != NULL) {
libxl_device_nic *nic;
char *buf2 = strdup(buf);
- char *p, *p2;
+ char *p;
d_config->nics = (libxl_device_nic *) realloc(d_config->nics, sizeof (libxl_device_nic) * (d_config->num_nics+1));
nic = d_config->nics + d_config->num_nics;
@@ -1544,64 +1612,7 @@ static void parse_config_data(const char *config_source,
do {
while (*p == ' ')
p++;
- if ((p2 = strchr(p, '=')) == NULL)
- break;
- *p2 = '\0';
- if (!strcmp(p, "model")) {
- free(nic->model);
- nic->model = strdup(p2 + 1);
- } else if (!strcmp(p, "mac")) {
- char *p3 = p2 + 1;
- *(p3 + 2) = '\0';
- nic->mac[0] = strtol(p3, NULL, 16);
- p3 = p3 + 3;
- *(p3 + 2) = '\0';
- nic->mac[1] = strtol(p3, NULL, 16);
- p3 = p3 + 3;
- *(p3 + 2) = '\0';
- nic->mac[2] = strtol(p3, NULL, 16);
- p3 = p3 + 3;
- *(p3 + 2) = '\0';
- nic->mac[3] = strtol(p3, NULL, 16);
- p3 = p3 + 3;
- *(p3 + 2) = '\0';
- nic->mac[4] = strtol(p3, NULL, 16);
- p3 = p3 + 3;
- *(p3 + 2) = '\0';
- nic->mac[5] = strtol(p3, NULL, 16);
- } else if (!strcmp(p, "bridge")) {
- free(nic->bridge);
- nic->bridge = strdup(p2 + 1);
- } else if (!strcmp(p, "type")) {
- if (!strcmp(p2 + 1, "ioemu"))
- nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
- else
- nic->nictype = LIBXL_NIC_TYPE_VIF;
- } else if (!strcmp(p, "ip")) {
- free(nic->ip);
- nic->ip = strdup(p2 + 1);
- } else if (!strcmp(p, "script")) {
- free(nic->script);
- nic->script = strdup(p2 + 1);
- } else if (!strcmp(p, "vifname")) {
- free(nic->ifname);
- nic->ifname = strdup(p2 + 1);
- } else if (!strcmp(p, "backend")) {
- free(nic->backend_domname);
- nic->backend_domname = strdup(p2 + 1);
- } else if (!strcmp(p, "rate")) {
- parse_vif_rate(&config, (p2 + 1), nic);
- } else if (!strcmp(p, "accel")) {
- fprintf(stderr, "the accel parameter for vifs is currently not supported\n");
- } else if (!strcmp(p, "netdev")) {
- fprintf(stderr, "the netdev parameter is deprecated, "
- "please use gatewaydev instead\n");
- free(nic->gatewaydev);
- nic->gatewaydev = strdup(p2 + 1);
- } else if (!strcmp(p, "gatewaydev")) {
- free(nic->gatewaydev);
- nic->gatewaydev = strdup(p2 + 1);
- }
+ parse_nic_config(nic, &config, p);
} while ((p = strtok(NULL, ",")) != NULL);
skip_nic:
free(buf2);
@@ -2115,17 +2126,6 @@ static int handle_domain_death(uint32_t *r_domid,
return restart;
}
-/* for now used only by main_networkattach, but can be reused elsewhere */
-static int match_option_size(const char *prefix, size_t len,
- char *arg, char **argopt)
-{
- int rc = strncmp(prefix, arg, len);
- if (!rc) *argopt = arg+len;
- return !rc;
-}
-#define MATCH_OPTION(prefix, arg, oparg) \
- match_option_size((prefix "="), sizeof((prefix)), (arg), &(oparg))
-
/* Preserve a copy of a domain under a new name. Updates *r_domid */
static int preserve_domain(uint32_t *r_domid, libxl_event *event,
libxl_domain_config *d_config)
@@ -6138,10 +6138,6 @@ int main_networkattach(int argc, char **argv)
int opt;
libxl_device_nic nic;
XLU_Config *config = 0;
- char *endptr, *oparg;
- const char *tok;
- int i;
- unsigned int val;
SWITCH_FOREACH_OPT(opt, "", NULL, "network-attach", 1) {
/* No options */
@@ -6164,50 +6160,8 @@ int main_networkattach(int argc, char **argv)
set_default_nic_values(&nic);
for (argv += optind+1, argc -= optind+1; argc > 0; ++argv, --argc) {
- if (MATCH_OPTION("type", *argv, oparg)) {
- if (!strcmp("vif", oparg)) {
- nic.nictype = LIBXL_NIC_TYPE_VIF;
- } else if (!strcmp("ioemu", oparg)) {
- nic.nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
- } else {
- fprintf(stderr, "Invalid parameter `type'.\n");
- return 1;
- }
- } else if (MATCH_OPTION("mac", *argv, oparg)) {
- tok = strtok(oparg, ":");
- for (i = 0; tok && i < 6; tok = strtok(NULL, ":"), ++i) {
- val = strtoul(tok, &endptr, 16);
- if ((tok == endptr) || (val > 255)) {
- fprintf(stderr, "Invalid parameter `mac'.\n");
- return 1;
- }
- nic.mac[i] = val;
- }
- } else if (MATCH_OPTION("bridge", *argv, oparg)) {
- replace_string(&nic.bridge, oparg);
- } else if (MATCH_OPTION("netdev", *argv, oparg)) {
- fprintf(stderr, "the netdev parameter is deprecated, "
- "please use gatewaydev instead\n");
- replace_string(&nic.gatewaydev, oparg);
- } else if (MATCH_OPTION("gatewaydev", *argv, oparg)) {
- replace_string(&nic.gatewaydev, oparg);
- } else if (MATCH_OPTION("ip", *argv, oparg)) {
- replace_string(&nic.ip, oparg);
- } else if (MATCH_OPTION("script", *argv, oparg)) {
- replace_string(&nic.script, oparg);
- } else if (MATCH_OPTION("backend", *argv, oparg)) {
- replace_string(&nic.backend_domname, oparg);
- } else if (MATCH_OPTION("vifname", *argv, oparg)) {
- replace_string(&nic.ifname, oparg);
- } else if (MATCH_OPTION("model", *argv, oparg)) {
- replace_string(&nic.model, oparg);
- } else if (MATCH_OPTION("rate", *argv, oparg)) {
- parse_vif_rate(&config, oparg, &nic);
- } else if (MATCH_OPTION("accel", *argv, oparg)) {
- } else {
- fprintf(stderr, "unrecognized argument `%s'\n", *argv);
+ if (parse_nic_config(&nic, &config, *argv))
return 1;
- }
}
if (dryrun_only) {
--
1.9.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [OPW PATCH V3] tools: xl: refactor code to parse network device options
2014-10-21 14:19 [OPW PATCH V3] tools: xl: refactor code to parse network device options Alexandra Sandulescu
@ 2014-10-21 14:22 ` Alexandra Sandulescu
2014-10-21 15:24 ` Wei Liu
2014-10-21 16:27 ` Konrad Rzeszutek Wilk
2 siblings, 0 replies; 5+ messages in thread
From: Alexandra Sandulescu @ 2014-10-21 14:22 UTC (permalink / raw)
To: xen-devel, wei.liu2, Ian Campbell
[-- Attachment #1.1: Type: text/plain, Size: 11594 bytes --]
Hello Wei,
I did rebase the commit against staging.
I forgot to add yesterday that I moved one function and a define
(match_option) to avoid forward declaration for parse_nic_config for
parse_config_data usage.
I am waiting for your next review.
Thank you,
Alexandra Sandulescu
On Tue, Oct 21, 2014 at 5:19 PM, Alexandra Sandulescu <
alecsandra.sandulescu@gmail.com> wrote:
> From: Alexandra Sandulescu <alexandra.sandulescu1@1and1.ro>
>
> This patch removes duplicate code in /tools/libxl/xl_cmdimpl.c by
> adding parse_nic_config function. This function parses configuration
> data and adds the information into libxl_device_nic struct. It is
> called in both main_networkattach and parse_config_data functions
> to replace duplicate code.
>
> Signed-off-by: Alexandra Sandulescu <alecsandra.sandulescu@gmail.com>
> ---
> tools/libxl/xl_cmdimpl.c | 188
> ++++++++++++++++++-----------------------------
> 1 file changed, 71 insertions(+), 117 deletions(-)
>
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index ea43761..31a0dcb 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -903,6 +903,74 @@ static void replace_string(char **str, const char
> *val)
> *str = xstrdup(val);
> }
>
> +/* for now used only by main_networkattach, but can be reused elsewhere */
> +static int match_option_size(const char *prefix, size_t len,
> + char *arg, char **argopt)
> +{
> + int rc = strncmp(prefix, arg, len);
> + if (!rc) *argopt = arg+len;
> + return !rc;
> +}
> +#define MATCH_OPTION(prefix, arg, oparg) \
> + match_option_size((prefix "="), sizeof((prefix)), (arg), &(oparg))
> +
> +/* Parses network data and adds info into nic
> + * Returns 1 if the input token does not match one of the keys
> + * or parsed values are not correct. Successful parse returns 0 */
> +static int parse_nic_config(libxl_device_nic *nic, XLU_Config **config,
> char *token)
> +{
> + char *endptr, *oparg;
> + int i;
> + unsigned int val;
> +
> + if (MATCH_OPTION("type", token, oparg)) {
> + if (!strcmp("vif", oparg)) {
> + nic->nictype = LIBXL_NIC_TYPE_VIF;
> + } else if (!strcmp("ioemu", oparg)) {
> + nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
> + } else {
> + fprintf(stderr, "Invalid parameter `type'.\n");
> + return 1;
> + }
> + } else if (MATCH_OPTION("mac", token, oparg)) {
> + for (i = 0; i < 6; i++){
> + val = strtoul(oparg, &endptr, 16);
> + if ((oparg == endptr) || (val > 255)) {
> + fprintf(stderr, "Invalid parameter `mac'.\n");
> + return 1;
> + }
> + nic->mac[i] = val;
> + oparg = endptr + 1;
> + }
> + } else if (MATCH_OPTION("bridge", token, oparg)) {
> + replace_string(&nic->bridge, oparg);
> + } else if (MATCH_OPTION("netdev", token, oparg)) {
> + fprintf(stderr, "the netdev parameter is deprecated, "
> + "please use gatewaydev instead\n");
> + replace_string(&nic->gatewaydev, oparg);
> + } else if (MATCH_OPTION("gatewaydev", token, oparg)) {
> + replace_string(&nic->gatewaydev, oparg);
> + } else if (MATCH_OPTION("ip", token, oparg)) {
> + replace_string(&nic->ip, oparg);
> + } else if (MATCH_OPTION("script", token, oparg)) {
> + replace_string(&nic->script, oparg);
> + } else if (MATCH_OPTION("backend", token, oparg)) {
> + replace_string(&nic->backend_domname, oparg);
> + } else if (MATCH_OPTION("vifname", token, oparg)) {
> + replace_string(&nic->ifname, oparg);
> + } else if (MATCH_OPTION("model", token, oparg)) {
> + replace_string(&nic->model, oparg);
> + } else if (MATCH_OPTION("rate", token, oparg)) {
> + parse_vif_rate(config, oparg, nic);
> + } else if (MATCH_OPTION("accel", token, oparg)) {
> + fprintf(stderr, "the accel parameter for vifs is currently not
> supported\n");
> + } else {
> + fprintf(stderr, "unrecognized argument `%s'\n", token);
> + return 1;
> + }
> + return 0;
> +}
> +
> static void parse_config_data(const char *config_source,
> const char *config_data,
> int config_len,
> @@ -1530,7 +1598,7 @@ static void parse_config_data(const char
> *config_source,
> while ((buf = xlu_cfg_get_listitem (nics, d_config->num_nics)) !=
> NULL) {
> libxl_device_nic *nic;
> char *buf2 = strdup(buf);
> - char *p, *p2;
> + char *p;
>
> d_config->nics = (libxl_device_nic *) realloc(d_config->nics,
> sizeof (libxl_device_nic) * (d_config->num_nics+1));
> nic = d_config->nics + d_config->num_nics;
> @@ -1544,64 +1612,7 @@ static void parse_config_data(const char
> *config_source,
> do {
> while (*p == ' ')
> p++;
> - if ((p2 = strchr(p, '=')) == NULL)
> - break;
> - *p2 = '\0';
> - if (!strcmp(p, "model")) {
> - free(nic->model);
> - nic->model = strdup(p2 + 1);
> - } else if (!strcmp(p, "mac")) {
> - char *p3 = p2 + 1;
> - *(p3 + 2) = '\0';
> - nic->mac[0] = strtol(p3, NULL, 16);
> - p3 = p3 + 3;
> - *(p3 + 2) = '\0';
> - nic->mac[1] = strtol(p3, NULL, 16);
> - p3 = p3 + 3;
> - *(p3 + 2) = '\0';
> - nic->mac[2] = strtol(p3, NULL, 16);
> - p3 = p3 + 3;
> - *(p3 + 2) = '\0';
> - nic->mac[3] = strtol(p3, NULL, 16);
> - p3 = p3 + 3;
> - *(p3 + 2) = '\0';
> - nic->mac[4] = strtol(p3, NULL, 16);
> - p3 = p3 + 3;
> - *(p3 + 2) = '\0';
> - nic->mac[5] = strtol(p3, NULL, 16);
> - } else if (!strcmp(p, "bridge")) {
> - free(nic->bridge);
> - nic->bridge = strdup(p2 + 1);
> - } else if (!strcmp(p, "type")) {
> - if (!strcmp(p2 + 1, "ioemu"))
> - nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
> - else
> - nic->nictype = LIBXL_NIC_TYPE_VIF;
> - } else if (!strcmp(p, "ip")) {
> - free(nic->ip);
> - nic->ip = strdup(p2 + 1);
> - } else if (!strcmp(p, "script")) {
> - free(nic->script);
> - nic->script = strdup(p2 + 1);
> - } else if (!strcmp(p, "vifname")) {
> - free(nic->ifname);
> - nic->ifname = strdup(p2 + 1);
> - } else if (!strcmp(p, "backend")) {
> - free(nic->backend_domname);
> - nic->backend_domname = strdup(p2 + 1);
> - } else if (!strcmp(p, "rate")) {
> - parse_vif_rate(&config, (p2 + 1), nic);
> - } else if (!strcmp(p, "accel")) {
> - fprintf(stderr, "the accel parameter for vifs is
> currently not supported\n");
> - } else if (!strcmp(p, "netdev")) {
> - fprintf(stderr, "the netdev parameter is deprecated, "
> - "please use gatewaydev instead\n");
> - free(nic->gatewaydev);
> - nic->gatewaydev = strdup(p2 + 1);
> - } else if (!strcmp(p, "gatewaydev")) {
> - free(nic->gatewaydev);
> - nic->gatewaydev = strdup(p2 + 1);
> - }
> + parse_nic_config(nic, &config, p);
> } while ((p = strtok(NULL, ",")) != NULL);
> skip_nic:
> free(buf2);
> @@ -2115,17 +2126,6 @@ static int handle_domain_death(uint32_t *r_domid,
> return restart;
> }
>
> -/* for now used only by main_networkattach, but can be reused elsewhere */
> -static int match_option_size(const char *prefix, size_t len,
> - char *arg, char **argopt)
> -{
> - int rc = strncmp(prefix, arg, len);
> - if (!rc) *argopt = arg+len;
> - return !rc;
> -}
> -#define MATCH_OPTION(prefix, arg, oparg) \
> - match_option_size((prefix "="), sizeof((prefix)), (arg), &(oparg))
> -
> /* Preserve a copy of a domain under a new name. Updates *r_domid */
> static int preserve_domain(uint32_t *r_domid, libxl_event *event,
> libxl_domain_config *d_config)
> @@ -6138,10 +6138,6 @@ int main_networkattach(int argc, char **argv)
> int opt;
> libxl_device_nic nic;
> XLU_Config *config = 0;
> - char *endptr, *oparg;
> - const char *tok;
> - int i;
> - unsigned int val;
>
> SWITCH_FOREACH_OPT(opt, "", NULL, "network-attach", 1) {
> /* No options */
> @@ -6164,50 +6160,8 @@ int main_networkattach(int argc, char **argv)
> set_default_nic_values(&nic);
>
> for (argv += optind+1, argc -= optind+1; argc > 0; ++argv, --argc) {
> - if (MATCH_OPTION("type", *argv, oparg)) {
> - if (!strcmp("vif", oparg)) {
> - nic.nictype = LIBXL_NIC_TYPE_VIF;
> - } else if (!strcmp("ioemu", oparg)) {
> - nic.nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
> - } else {
> - fprintf(stderr, "Invalid parameter `type'.\n");
> - return 1;
> - }
> - } else if (MATCH_OPTION("mac", *argv, oparg)) {
> - tok = strtok(oparg, ":");
> - for (i = 0; tok && i < 6; tok = strtok(NULL, ":"), ++i) {
> - val = strtoul(tok, &endptr, 16);
> - if ((tok == endptr) || (val > 255)) {
> - fprintf(stderr, "Invalid parameter `mac'.\n");
> - return 1;
> - }
> - nic.mac[i] = val;
> - }
> - } else if (MATCH_OPTION("bridge", *argv, oparg)) {
> - replace_string(&nic.bridge, oparg);
> - } else if (MATCH_OPTION("netdev", *argv, oparg)) {
> - fprintf(stderr, "the netdev parameter is deprecated, "
> - "please use gatewaydev instead\n");
> - replace_string(&nic.gatewaydev, oparg);
> - } else if (MATCH_OPTION("gatewaydev", *argv, oparg)) {
> - replace_string(&nic.gatewaydev, oparg);
> - } else if (MATCH_OPTION("ip", *argv, oparg)) {
> - replace_string(&nic.ip, oparg);
> - } else if (MATCH_OPTION("script", *argv, oparg)) {
> - replace_string(&nic.script, oparg);
> - } else if (MATCH_OPTION("backend", *argv, oparg)) {
> - replace_string(&nic.backend_domname, oparg);
> - } else if (MATCH_OPTION("vifname", *argv, oparg)) {
> - replace_string(&nic.ifname, oparg);
> - } else if (MATCH_OPTION("model", *argv, oparg)) {
> - replace_string(&nic.model, oparg);
> - } else if (MATCH_OPTION("rate", *argv, oparg)) {
> - parse_vif_rate(&config, oparg, &nic);
> - } else if (MATCH_OPTION("accel", *argv, oparg)) {
> - } else {
> - fprintf(stderr, "unrecognized argument `%s'\n", *argv);
> + if (parse_nic_config(&nic, &config, *argv))
> return 1;
> - }
> }
>
> if (dryrun_only) {
> --
> 1.9.1
>
>
[-- Attachment #1.2: Type: text/html, Size: 15102 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [OPW PATCH V3] tools: xl: refactor code to parse network device options
2014-10-21 14:19 [OPW PATCH V3] tools: xl: refactor code to parse network device options Alexandra Sandulescu
2014-10-21 14:22 ` Alexandra Sandulescu
@ 2014-10-21 15:24 ` Wei Liu
2014-10-21 15:29 ` Ian Campbell
2014-10-21 16:27 ` Konrad Rzeszutek Wilk
2 siblings, 1 reply; 5+ messages in thread
From: Wei Liu @ 2014-10-21 15:24 UTC (permalink / raw)
To: Alexandra Sandulescu
Cc: xen-devel, Alexandra Sandulescu, wei.liu2, ian.campbell
On Tue, Oct 21, 2014 at 05:19:35PM +0300, Alexandra Sandulescu wrote:
> From: Alexandra Sandulescu <alexandra.sandulescu1@1and1.ro>
>
> This patch removes duplicate code in /tools/libxl/xl_cmdimpl.c by
> adding parse_nic_config function. This function parses configuration
> data and adds the information into libxl_device_nic struct. It is
> called in both main_networkattach and parse_config_data functions
> to replace duplicate code.
>
> Signed-off-by: Alexandra Sandulescu <alecsandra.sandulescu@gmail.com>
> ---
> tools/libxl/xl_cmdimpl.c | 188 ++++++++++++++++++-----------------------------
> 1 file changed, 71 insertions(+), 117 deletions(-)
>
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index ea43761..31a0dcb 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -903,6 +903,74 @@ static void replace_string(char **str, const char *val)
> *str = xstrdup(val);
> }
>
> +/* for now used only by main_networkattach, but can be reused elsewhere */
A minor nit is that you should remove this comment because it is not
true anymore.
Other than that:
Acked-by: Wei Liu <wei.liu2@citrix.com>
(CC'ing Konrad for his input as release manager)
Now we are in freeze, not sure how applicable is it for this patch to go
in.
To Konrad:
-ve: this is only code refactoring, not bug fix
+ve: the risk of introducing bug is low as 1) this refactoring is
straight-forward, 2) the code path is tested in OSSTest.
Wei.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [OPW PATCH V3] tools: xl: refactor code to parse network device options
2014-10-21 15:24 ` Wei Liu
@ 2014-10-21 15:29 ` Ian Campbell
0 siblings, 0 replies; 5+ messages in thread
From: Ian Campbell @ 2014-10-21 15:29 UTC (permalink / raw)
To: Wei Liu; +Cc: xen-devel, Alexandra Sandulescu, Alexandra Sandulescu
On Tue, 2014-10-21 at 16:24 +0100, Wei Liu wrote:
> Now we are in freeze, not sure how applicable is it for this patch to go
> in.
>
> To Konrad:
>
> -ve: this is only code refactoring, not bug fix
> +ve: the risk of introducing bug is low as 1) this refactoring is
> straight-forward, 2) the code path is tested in OSSTest.
Note that this was a task I suggested as part of the OPW application
process. I said to Alexandra at the time:
I think OPW wants you to get a patch accepted as part of the application
process, however Xen is currently in code freeze for the 4.5 release so
I think we will have to aim for a patch in a state where it could/would
be accepted if there was no code freeze. The patch can then be held
until the 4.6 development cycle opens and be applied then.
Of course if the patch was allowed in now that would be awesome too ;-)
Ian.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [OPW PATCH V3] tools: xl: refactor code to parse network device options
2014-10-21 14:19 [OPW PATCH V3] tools: xl: refactor code to parse network device options Alexandra Sandulescu
2014-10-21 14:22 ` Alexandra Sandulescu
2014-10-21 15:24 ` Wei Liu
@ 2014-10-21 16:27 ` Konrad Rzeszutek Wilk
2 siblings, 0 replies; 5+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-10-21 16:27 UTC (permalink / raw)
To: Alexandra Sandulescu
Cc: xen-devel, Alexandra Sandulescu, wei.liu2, ian.campbell
On Tue, Oct 21, 2014 at 05:19:35PM +0300, Alexandra Sandulescu wrote:
> From: Alexandra Sandulescu <alexandra.sandulescu1@1and1.ro>
>
> This patch removes duplicate code in /tools/libxl/xl_cmdimpl.c by
> adding parse_nic_config function. This function parses configuration
> data and adds the information into libxl_device_nic struct. It is
> called in both main_networkattach and parse_config_data functions
> to replace duplicate code.
>
> Signed-off-by: Alexandra Sandulescu <alecsandra.sandulescu@gmail.com>
> ---
> tools/libxl/xl_cmdimpl.c | 188 ++++++++++++++++++-----------------------------
> 1 file changed, 71 insertions(+), 117 deletions(-)
>
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index ea43761..31a0dcb 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -903,6 +903,74 @@ static void replace_string(char **str, const char *val)
> *str = xstrdup(val);
> }
>
> +/* for now used only by main_networkattach, but can be reused elsewhere */
> +static int match_option_size(const char *prefix, size_t len,
> + char *arg, char **argopt)
> +{
> + int rc = strncmp(prefix, arg, len);
> + if (!rc) *argopt = arg+len;
> + return !rc;
> +}
> +#define MATCH_OPTION(prefix, arg, oparg) \
> + match_option_size((prefix "="), sizeof((prefix)), (arg), &(oparg))
> +
> +/* Parses network data and adds info into nic
> + * Returns 1 if the input token does not match one of the keys
> + * or parsed values are not correct. Successful parse returns 0 */
> +static int parse_nic_config(libxl_device_nic *nic, XLU_Config **config, char *token)
> +{
> + char *endptr, *oparg;
> + int i;
> + unsigned int val;
> +
> + if (MATCH_OPTION("type", token, oparg)) {
> + if (!strcmp("vif", oparg)) {
> + nic->nictype = LIBXL_NIC_TYPE_VIF;
> + } else if (!strcmp("ioemu", oparg)) {
> + nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
> + } else {
> + fprintf(stderr, "Invalid parameter `type'.\n");
> + return 1;
> + }
> + } else if (MATCH_OPTION("mac", token, oparg)) {
> + for (i = 0; i < 6; i++){
Your '{' needs a space beforehand.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-10-21 16:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-21 14:19 [OPW PATCH V3] tools: xl: refactor code to parse network device options Alexandra Sandulescu
2014-10-21 14:22 ` Alexandra Sandulescu
2014-10-21 15:24 ` Wei Liu
2014-10-21 15:29 ` Ian Campbell
2014-10-21 16:27 ` Konrad Rzeszutek Wilk
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.