1:28 a.m.
Now teamd uses dot symbol to split path string into different nodes
in __teamd_json_path_lite_va() when dumping teamd state. But team
port ifname is a part of path string, if ifname has dot character
inside, it would be parsed as two nodes incorrectly.
Like, if users uses eth1.1 as a port ifname
teamdctl team0 state dump would be:
"ports": {
"eth1": {
"1": {
"ifinfo": {
...
and teamdctl team0 state view even crashs as:
...
ports:
Failed to parse JSON port dump.
command call failed (Invalid argument)
Actually it's common for users to use dot in ifname, especially for
vlan device.
This patch is to replace dot with space in ifname for then, after
being parsed, it would be changed back to dot.
Note that it's safe to change it with space, as space in ifname is
disallowed. Besides, no teamd state attributes/nodes include space
afaik, so no side effects could be caused by this fix.
Signed-off-by: Xin Long <lucien.xin(a)gmail.com>
---
teamd/teamd_json.c | 16 +++++++++++++++-
teamd/teamd_json.h | 1 +
teamd/teamd_state.c | 13 ++++++++++---
3 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/teamd/teamd_json.c b/teamd/teamd_json.c
index 5fd5e8e..7082484 100644
--- a/teamd/teamd_json.c
+++ b/teamd/teamd_json.c
@@ -42,6 +42,19 @@ static char *__strchrs(char *str, char *chars)
return NULL;
}
+char *strchrchg(char *str, char s, char d)
+{
+ char *tmp = str;
+
+ while (*str != '\0') {
+ if (*str == s)
+ *str = d;
+ str++;
+ }
+
+ return tmp;
+}
+
#define TEAMD_JSON_PATH_MAXLEN 128
typedef json_t *(*obj_constructor_t)(void);
@@ -82,7 +95,8 @@ static int __teamd_json_path_lite_va(json_t **p_json_obj, json_t
*json_root,
*end = '\0';
}
prev_json_obj = json_obj;
- json_obj = json_object_get(prev_json_obj, ptr);
+ json_obj = json_object_get(prev_json_obj,
+ strchrchg(ptr, ' ', '.'));
if (!json_obj && obj_constructor) {
/* In case new object is not supposed to be
* leaf, use json_object() as a constructor.
diff --git a/teamd/teamd_json.h b/teamd/teamd_json.h
index 30e9eaa..3dffdb5 100644
--- a/teamd/teamd_json.h
+++ b/teamd/teamd_json.h
@@ -24,6 +24,7 @@
#define TEAMD_JSON_DUMPS_FLAGS (JSON_INDENT(4) | JSON_ENSURE_ASCII | JSON_SORT_KEYS)
+char *strchrchg(char *str, char s, char d);
int teamd_json_path_lite_va(json_t **p_json_obj, json_t *json_root,
const char *fmt, va_list ap);
int teamd_json_path_lite(json_t **p_json_obj, json_t *json_root,
diff --git a/teamd/teamd_state.c b/teamd/teamd_state.c
index 51fac8d..d9931dc 100644
--- a/teamd/teamd_state.c
+++ b/teamd/teamd_state.c
@@ -194,11 +194,18 @@ static int teamd_state_build_val_json_subpath(json_t
**p_vg_json_obj,
int err;
char *dot;
- if (tdport)
+ if (tdport) {
+ char *tmp;
+
+ if (asprintf(&tmp, tdport->ifname) == -1)
+ return -ENOMEM;
+
ret = asprintf(&path, "$." TEAMD_STATE_PER_PORT_PREFIX "%s%s",
- tdport->ifname, subpath);
- else
+ strchrchg(tmp, '.', ' '), subpath);
+ free(tmp);
+ } else {
ret = asprintf(&path, "$%s", subpath);
+ }
if (ret == -1)
return -ENOMEM;
dot = strrchr(path, '.');
--
2.1.0
2:51 p.m.
New subject: [PATCH] teamd: replace dot with space in ifname when dumping
teamd state
Sun, Nov 27, 2016 at 08:28:16AM CET, lucien.xin(a)gmail.com wrote:
Now teamd uses dot symbol to split path string into different nodes
in __teamd_json_path_lite_va() when dumping teamd state. But team
port ifname is a part of path string, if ifname has dot character
inside, it would be parsed as two nodes incorrectly.
Like, if users uses eth1.1 as a port ifname
teamdctl team0 state dump would be:
"ports": {
"eth1": {
"1": {
"ifinfo": {
...
and teamdctl team0 state view even crashs as:
...
ports:
Failed to parse JSON port dump.
command call failed (Invalid argument)
Actually it's common for users to use dot in ifname, especially for
vlan device.
This patch is to replace dot with space in ifname for then, after
being parsed, it would be changed back to dot.
Hmm, how about perhaps rather to escape the dot somehow:
eth1.1 -> eth1\2e1
?
Seems a bit nicer
>
>Note that it's safe to change it with space, as space in ifname is
>disallowed. Besides, no teamd state attributes/nodes include space
>afaik, so no side effects could be caused by this fix.
>
>Signed-off-by: Xin Long <lucien.xin(a)gmail.com>
>---
> teamd/teamd_json.c | 16 +++++++++++++++-
> teamd/teamd_json.h | 1 +
> teamd/teamd_state.c | 13 ++++++++++---
> 3 files changed, 26 insertions(+), 4 deletions(-)
>
>diff --git a/teamd/teamd_json.c b/teamd/teamd_json.c
>index 5fd5e8e..7082484 100644
>--- a/teamd/teamd_json.c
>+++ b/teamd/teamd_json.c
>@@ -42,6 +42,19 @@ static char *__strchrs(char *str, char *chars)
> return NULL;
> }
>
>+char *strchrchg(char *str, char s, char d)
>+{
>+ char *tmp = str;
>+
>+ while (*str != '\0') {
>+ if (*str == s)
>+ *str = d;
>+ str++;
>+ }
>+
>+ return tmp;
>+}
>+
> #define TEAMD_JSON_PATH_MAXLEN 128
>
> typedef json_t *(*obj_constructor_t)(void);
>@@ -82,7 +95,8 @@ static int __teamd_json_path_lite_va(json_t **p_json_obj, json_t
*json_root,
> *end = '\0';
> }
> prev_json_obj = json_obj;
>- json_obj = json_object_get(prev_json_obj, ptr);
>+ json_obj = json_object_get(prev_json_obj,
>+ strchrchg(ptr, ' ', '.'));
> if (!json_obj && obj_constructor) {
> /* In case new object is not supposed to be
> * leaf, use json_object() as a constructor.
>diff --git a/teamd/teamd_json.h b/teamd/teamd_json.h
>index 30e9eaa..3dffdb5 100644
>--- a/teamd/teamd_json.h
>+++ b/teamd/teamd_json.h
>@@ -24,6 +24,7 @@
>
> #define TEAMD_JSON_DUMPS_FLAGS (JSON_INDENT(4) | JSON_ENSURE_ASCII | JSON_SORT_KEYS)
>
>+char *strchrchg(char *str, char s, char d);
> int teamd_json_path_lite_va(json_t **p_json_obj, json_t *json_root,
> const char *fmt, va_list ap);
> int teamd_json_path_lite(json_t **p_json_obj, json_t *json_root,
>diff --git a/teamd/teamd_state.c b/teamd/teamd_state.c
>index 51fac8d..d9931dc 100644
>--- a/teamd/teamd_state.c
>+++ b/teamd/teamd_state.c
>@@ -194,11 +194,18 @@ static int teamd_state_build_val_json_subpath(json_t
**p_vg_json_obj,
> int err;
> char *dot;
>
>- if (tdport)
>+ if (tdport) {
>+ char *tmp;
>+
>+ if (asprintf(&tmp, tdport->ifname) == -1)
>+ return -ENOMEM;
>+
> ret = asprintf(&path, "$." TEAMD_STATE_PER_PORT_PREFIX
"%s%s",
>- tdport->ifname, subpath);
>- else
>+ strchrchg(tmp, '.', ' '), subpath);
>+ free(tmp);
>+ } else {
> ret = asprintf(&path, "$%s", subpath);
>+ }
> if (ret == -1)
> return -ENOMEM;
> dot = strrchr(path, '.');
>--
>2.1.0
>
2:31 a.m.
New subject: [PATCH] teamd: replace dot with space in ifname when dumping
teamd state
On Tue, Nov 29, 2016 at 4:51 AM, Jiri Pirko <jiri(a)resnulli.us> wrote:
Sun, Nov 27, 2016 at 08:28:16AM CET, lucien.xin(a)gmail.com wrote:
>Now teamd uses dot symbol to split path string into different nodes
>in __teamd_json_path_lite_va() when dumping teamd state. But team
>port ifname is a part of path string, if ifname has dot character
>inside, it would be parsed as two nodes incorrectly.
>
>Like, if users uses eth1.1 as a port ifname
>teamdctl team0 state dump would be:
> "ports": {
> "eth1": {
> "1": {
> "ifinfo": {
> ...
>
>and teamdctl team0 state view even crashs as:
> ...
> ports:
> Failed to parse JSON port dump.
> command call failed (Invalid argument)
>
>Actually it's common for users to use dot in ifname, especially for
>vlan device.
>
>This patch is to replace dot with space in ifname for then, after
>being parsed, it would be changed back to dot.
Hmm, how about perhaps rather to escape the dot somehow:
eth1.1 -> eth1\2e1
?
you mean to change eth1.1 to eth1\2e1 before parsing, but will
not change it back to eth1.1 after parsing ? and dump/view, it show
ifname with "eth1\2e1" ?
if yes, I think for users it's looks not nice, as in some places it show
eth1.1, but some place it show eth1\2e1 when dumping or viewing it.
if we also change eth1\2e1 back to eth1.1 after parsing, coding will be
more complicate than with space.
beside, what if one ifname is "eth1\2e1", although it's not common.
do you know why we chose dot to split nodes?
I'm asking that, because Hangbin has another fix for this issue,
which changs to use slash "/" to split nodes, instead of dot. but
need to modify many places, he is not sure if "/" may cause other
problem ?
>
> Seems a bit nicer
>
>
>>
>>Note that it's safe to change it with space, as space in ifname is
>>disallowed. Besides, no teamd state attributes/nodes include space
>>afaik, so no side effects could be caused by this fix.
>>
>>Signed-off-by: Xin Long <lucien.xin(a)gmail.com>
>>---
>> teamd/teamd_json.c | 16 +++++++++++++++-
>> teamd/teamd_json.h | 1 +
>> teamd/teamd_state.c | 13 ++++++++++---
>> 3 files changed, 26 insertions(+), 4 deletions(-)
>>
>>diff --git a/teamd/teamd_json.c b/teamd/teamd_json.c
>>index 5fd5e8e..7082484 100644
>>--- a/teamd/teamd_json.c
>>+++ b/teamd/teamd_json.c
>>@@ -42,6 +42,19 @@ static char *__strchrs(char *str, char *chars)
>> return NULL;
>> }
>>
>>+char *strchrchg(char *str, char s, char d)
>>+{
>>+ char *tmp = str;
>>+
>>+ while (*str != '\0') {
>>+ if (*str == s)
>>+ *str = d;
>>+ str++;
>>+ }
>>+
>>+ return tmp;
>>+}
>>+
>> #define TEAMD_JSON_PATH_MAXLEN 128
>>
>> typedef json_t *(*obj_constructor_t)(void);
>>@@ -82,7 +95,8 @@ static int __teamd_json_path_lite_va(json_t **p_json_obj, json_t
*json_root,
>> *end = '\0';
>> }
>> prev_json_obj = json_obj;
>>- json_obj = json_object_get(prev_json_obj, ptr);
>>+ json_obj = json_object_get(prev_json_obj,
>>+ strchrchg(ptr, ' ',
'.'));
>> if (!json_obj && obj_constructor) {
>> /* In case new object is not supposed to be
>> * leaf, use json_object() as a constructor.
>>diff --git a/teamd/teamd_json.h b/teamd/teamd_json.h
>>index 30e9eaa..3dffdb5 100644
>>--- a/teamd/teamd_json.h
>>+++ b/teamd/teamd_json.h
>>@@ -24,6 +24,7 @@
>>
>> #define TEAMD_JSON_DUMPS_FLAGS (JSON_INDENT(4) | JSON_ENSURE_ASCII |
JSON_SORT_KEYS)
>>
>>+char *strchrchg(char *str, char s, char d);
>> int teamd_json_path_lite_va(json_t **p_json_obj, json_t *json_root,
>> const char *fmt, va_list ap);
>> int teamd_json_path_lite(json_t **p_json_obj, json_t *json_root,
>>diff --git a/teamd/teamd_state.c b/teamd/teamd_state.c
>>index 51fac8d..d9931dc 100644
>>--- a/teamd/teamd_state.c
>>+++ b/teamd/teamd_state.c
>>@@ -194,11 +194,18 @@ static int teamd_state_build_val_json_subpath(json_t
**p_vg_json_obj,
>> int err;
>> char *dot;
>>
>>- if (tdport)
>>+ if (tdport) {
>>+ char *tmp;
>>+
>>+ if (asprintf(&tmp, tdport->ifname) == -1)
>>+ return -ENOMEM;
>>+
>> ret = asprintf(&path, "$."
TEAMD_STATE_PER_PORT_PREFIX "%s%s",
>>- tdport->ifname, subpath);
>>- else
>>+ strchrchg(tmp, '.', ' '), subpath);
>>+ free(tmp);
>>+ } else {
>> ret = asprintf(&path, "$%s", subpath);
>>+ }
>> if (ret == -1)
>> return -ENOMEM;
>> dot = strrchr(path, '.');
>>--
>>2.1.0
>>
2:54 a.m.
New subject: [PATCH] teamd: replace dot with space in ifname when dumping
teamd state
Tue, Nov 29, 2016 at 09:31:21AM CET, lucien.xin(a)gmail.com wrote:
On Tue, Nov 29, 2016 at 4:51 AM, Jiri Pirko <jiri(a)resnulli.us>
wrote:
> Sun, Nov 27, 2016 at 08:28:16AM CET, lucien.xin(a)gmail.com wrote:
>>Now teamd uses dot symbol to split path string into different nodes
>>in __teamd_json_path_lite_va() when dumping teamd state. But team
>>port ifname is a part of path string, if ifname has dot character
>>inside, it would be parsed as two nodes incorrectly.
>>
>>Like, if users uses eth1.1 as a port ifname
>>teamdctl team0 state dump would be:
>> "ports": {
>> "eth1": {
>> "1": {
>> "ifinfo": {
>> ...
>>
>>and teamdctl team0 state view even crashs as:
>> ...
>> ports:
>> Failed to parse JSON port dump.
>> command call failed (Invalid argument)
>>
>>Actually it's common for users to use dot in ifname, especially for
>>vlan device.
>>
>>This patch is to replace dot with space in ifname for then, after
>>being parsed, it would be changed back to dot.
>
> Hmm, how about perhaps rather to escape the dot somehow:
> eth1.1 -> eth1\2e1
> ?
you mean to change eth1.1 to eth1\2e1 before parsing, but will
not change it back to eth1.1 after parsing ? and dump/view, it show
ifname with "eth1\2e1" ?
No, I suggest exactly the same you want to do with " ", only with
"\2e"
instead. Not sure why you think something else...
if yes, I think for users it's looks not nice, as in some places it show
eth1.1, but some place it show eth1\2e1 when dumping or viewing it.
if we also change eth1\2e1 back to eth1.1 after parsing, coding will be
more complicate than with space.
So what?
beside, what if one ifname is "eth1\2e1", although it's not common.
Okay. That was an example. The escape character should be either something
which is not allowed in the name or you have to escape the original "\".
I don't see any problem with that.
do you know why we chose dot to split nodes?
I'm asking that, because Hangbin has another fix for this issue,
which changs to use slash "/" to split nodes, instead of dot. but
need to modify many places, he is not sure if "/" may cause other
problem ?
Please leave the dot there. Just escape it.
It loosely follows jsonpath.
>
>
>>
>> Seems a bit nicer
>>
>>
>>>
>>>Note that it's safe to change it with space, as space in ifname is
>>>disallowed. Besides, no teamd state attributes/nodes include space
>>>afaik, so no side effects could be caused by this fix.
>>>
>>>Signed-off-by: Xin Long <lucien.xin(a)gmail.com>
>>>---
>>> teamd/teamd_json.c | 16 +++++++++++++++-
>>> teamd/teamd_json.h | 1 +
>>> teamd/teamd_state.c | 13 ++++++++++---
>>> 3 files changed, 26 insertions(+), 4 deletions(-)
>>>
>>>diff --git a/teamd/teamd_json.c b/teamd/teamd_json.c
>>>index 5fd5e8e..7082484 100644
>>>--- a/teamd/teamd_json.c
>>>+++ b/teamd/teamd_json.c
>>>@@ -42,6 +42,19 @@ static char *__strchrs(char *str, char *chars)
>>> return NULL;
>>> }
>>>
>>>+char *strchrchg(char *str, char s, char d)
>>>+{
>>>+ char *tmp = str;
>>>+
>>>+ while (*str != '\0') {
>>>+ if (*str == s)
>>>+ *str = d;
>>>+ str++;
>>>+ }
>>>+
>>>+ return tmp;
>>>+}
>>>+
>>> #define TEAMD_JSON_PATH_MAXLEN 128
>>>
>>> typedef json_t *(*obj_constructor_t)(void);
>>>@@ -82,7 +95,8 @@ static int __teamd_json_path_lite_va(json_t **p_json_obj,
json_t *json_root,
>>> *end = '\0';
>>> }
>>> prev_json_obj = json_obj;
>>>- json_obj = json_object_get(prev_json_obj, ptr);
>>>+ json_obj = json_object_get(prev_json_obj,
>>>+ strchrchg(ptr, ' ',
'.'));
>>> if (!json_obj && obj_constructor) {
>>> /* In case new object is not supposed to be
>>> * leaf, use json_object() as a constructor.
>>>diff --git a/teamd/teamd_json.h b/teamd/teamd_json.h
>>>index 30e9eaa..3dffdb5 100644
>>>--- a/teamd/teamd_json.h
>>>+++ b/teamd/teamd_json.h
>>>@@ -24,6 +24,7 @@
>>>
>>> #define TEAMD_JSON_DUMPS_FLAGS (JSON_INDENT(4) | JSON_ENSURE_ASCII |
JSON_SORT_KEYS)
>>>
>>>+char *strchrchg(char *str, char s, char d);
>>> int teamd_json_path_lite_va(json_t **p_json_obj, json_t *json_root,
>>> const char *fmt, va_list ap);
>>> int teamd_json_path_lite(json_t **p_json_obj, json_t *json_root,
>>>diff --git a/teamd/teamd_state.c b/teamd/teamd_state.c
>>>index 51fac8d..d9931dc 100644
>>>--- a/teamd/teamd_state.c
>>>+++ b/teamd/teamd_state.c
>>>@@ -194,11 +194,18 @@ static int teamd_state_build_val_json_subpath(json_t
**p_vg_json_obj,
>>> int err;
>>> char *dot;
>>>
>>>- if (tdport)
>>>+ if (tdport) {
>>>+ char *tmp;
>>>+
>>>+ if (asprintf(&tmp, tdport->ifname) == -1)
>>>+ return -ENOMEM;
>>>+
>>> ret = asprintf(&path, "$."
TEAMD_STATE_PER_PORT_PREFIX "%s%s",
>>>- tdport->ifname, subpath);
>>>- else
>>>+ strchrchg(tmp, '.', ' '),
subpath);
>>>+ free(tmp);
>>>+ } else {
>>> ret = asprintf(&path, "$%s", subpath);
>>>+ }
>>> if (ret == -1)
>>> return -ENOMEM;
>>> dot = strrchr(path, '.');
>>>--
>>>2.1.0
>>>
12:46 p.m.
New subject: [PATCH] teamd: replace dot with space in ifname when dumping
teamd state
On Thu, Dec 1, 2016 at 4:54 PM, Jiri Pirko <jiri(a)resnulli.us> wrote:
Tue, Nov 29, 2016 at 09:31:21AM CET, lucien.xin(a)gmail.com wrote:
>On Tue, Nov 29, 2016 at 4:51 AM, Jiri Pirko <jiri(a)resnulli.us> wrote:
>> Sun, Nov 27, 2016 at 08:28:16AM CET, lucien.xin(a)gmail.com wrote:
>>>Now teamd uses dot symbol to split path string into different nodes
>>>in __teamd_json_path_lite_va() when dumping teamd state. But team
>>>port ifname is a part of path string, if ifname has dot character
>>>inside, it would be parsed as two nodes incorrectly.
>>>
>>>Like, if users uses eth1.1 as a port ifname
>>>teamdctl team0 state dump would be:
>>> "ports": {
>>> "eth1": {
>>> "1": {
>>> "ifinfo": {
>>> ...
>>>
>>>and teamdctl team0 state view even crashs as:
>>> ...
>>> ports:
>>> Failed to parse JSON port dump.
>>> command call failed (Invalid argument)
>>>
>>>Actually it's common for users to use dot in ifname, especially for
>>>vlan device.
>>>
>>>This patch is to replace dot with space in ifname for then, after
>>>being parsed, it would be changed back to dot.
>>
>> Hmm, how about perhaps rather to escape the dot somehow:
>> eth1.1 -> eth1\2e1
>> ?
>you mean to change eth1.1 to eth1\2e1 before parsing, but will
>not change it back to eth1.1 after parsing ? and dump/view, it show
>ifname with "eth1\2e1" ?
No, I suggest exactly the same you want to do with " ", only with
"\2e"
instead. Not sure why you think something else...
ok
>
>if yes, I think for users it's looks not nice, as in some places it show
>eth1.1, but some place it show eth1\2e1 when dumping or viewing it.
>
>if we also change eth1\2e1 back to eth1.1 after parsing, coding will be
>more complicate than with space.
So what?
>
>beside, what if one ifname is "eth1\2e1", although it's not common.
Okay. That was an example. The escape character should be either something
which is not allowed in the name or you have to escape the original "\".
I don't see any problem with that.
That's why I'm using " "
which is not allowed in the name.
as the comment said:
"""
Note that it's safe to change it with space, as space in ifname is
disallowed. Besides, no teamd state attributes/nodes include space
afaik, so no side effects could be caused by this fix.
"""
I understand "\2e" is nicer, but we have to escape the original "\"
when ifname has "\2e", as you said above. It means we have to
escape dot and "\" to fix this problem completely.
sorry and pls correct me if I get you wrong. thanks
>
>
>>
>>
>>do you know why we chose dot to split nodes?
>>I'm asking that, because Hangbin has another fix for this issue,
>>which changs to use slash "/" to split nodes, instead of dot. but
>>need to modify many places, he is not sure if "/" may cause other
>>problem ?
>
> Please leave the dot there. Just escape it.
> It loosely follows jsonpath.
>
>
>>
>>
>>>
>>> Seems a bit nicer
>>>
>>>
>>>>
>>>>Note that it's safe to change it with space, as space in ifname is
>>>>disallowed. Besides, no teamd state attributes/nodes include space
>>>>afaik, so no side effects could be caused by this fix.
>>>>
>>>>Signed-off-by: Xin Long <lucien.xin(a)gmail.com>
>>>>---
>>>> teamd/teamd_json.c | 16 +++++++++++++++-
>>>> teamd/teamd_json.h | 1 +
>>>> teamd/teamd_state.c | 13 ++++++++++---
>>>> 3 files changed, 26 insertions(+), 4 deletions(-)
>>>>
>>>>diff --git a/teamd/teamd_json.c b/teamd/teamd_json.c
>>>>index 5fd5e8e..7082484 100644
>>>>--- a/teamd/teamd_json.c
>>>>+++ b/teamd/teamd_json.c
>>>>@@ -42,6 +42,19 @@ static char *__strchrs(char *str, char *chars)
>>>> return NULL;
>>>> }
>>>>
>>>>+char *strchrchg(char *str, char s, char d)
>>>>+{
>>>>+ char *tmp = str;
>>>>+
>>>>+ while (*str != '\0') {
>>>>+ if (*str == s)
>>>>+ *str = d;
>>>>+ str++;
>>>>+ }
>>>>+
>>>>+ return tmp;
>>>>+}
>>>>+
>>>> #define TEAMD_JSON_PATH_MAXLEN 128
>>>>
>>>> typedef json_t *(*obj_constructor_t)(void);
>>>>@@ -82,7 +95,8 @@ static int __teamd_json_path_lite_va(json_t
**p_json_obj, json_t *json_root,
>>>> *end = '\0';
>>>> }
>>>> prev_json_obj = json_obj;
>>>>- json_obj = json_object_get(prev_json_obj, ptr);
>>>>+ json_obj = json_object_get(prev_json_obj,
>>>>+ strchrchg(ptr, '
', '.'));
>>>> if (!json_obj && obj_constructor) {
>>>> /* In case new object is not supposed to
be
>>>> * leaf, use json_object() as a
constructor.
>>>>diff --git a/teamd/teamd_json.h b/teamd/teamd_json.h
>>>>index 30e9eaa..3dffdb5 100644
>>>>--- a/teamd/teamd_json.h
>>>>+++ b/teamd/teamd_json.h
>>>>@@ -24,6 +24,7 @@
>>>>
>>>> #define TEAMD_JSON_DUMPS_FLAGS (JSON_INDENT(4) | JSON_ENSURE_ASCII |
JSON_SORT_KEYS)
>>>>
>>>>+char *strchrchg(char *str, char s, char d);
>>>> int teamd_json_path_lite_va(json_t **p_json_obj, json_t *json_root,
>>>> const char *fmt, va_list ap);
>>>> int teamd_json_path_lite(json_t **p_json_obj, json_t *json_root,
>>>>diff --git a/teamd/teamd_state.c b/teamd/teamd_state.c
>>>>index 51fac8d..d9931dc 100644
>>>>--- a/teamd/teamd_state.c
>>>>+++ b/teamd/teamd_state.c
>>>>@@ -194,11 +194,18 @@ static int teamd_state_build_val_json_subpath(json_t
**p_vg_json_obj,
>>>> int err;
>>>> char *dot;
>>>>
>>>>- if (tdport)
>>>>+ if (tdport) {
>>>>+ char *tmp;
>>>>+
>>>>+ if (asprintf(&tmp, tdport->ifname) == -1)
>>>>+ return -ENOMEM;
>>>>+
>>>> ret = asprintf(&path, "$."
TEAMD_STATE_PER_PORT_PREFIX "%s%s",
>>>>- tdport->ifname, subpath);
>>>>- else
>>>>+ strchrchg(tmp, '.', ' '),
subpath);
>>>>+ free(tmp);
>>>>+ } else {
>>>> ret = asprintf(&path, "$%s", subpath);
>>>>+ }
>>>> if (ret == -1)
>>>> return -ENOMEM;
>>>> dot = strrchr(path, '.');
>>>>--
>>>>2.1.0
>>>>
3:06 a.m.
New subject: [PATCH] teamd: replace dot with space in ifname when dumping
teamd state
Sun, Dec 04, 2016 at 07:46:12PM CET, lucien.xin(a)gmail.com wrote:
On Thu, Dec 1, 2016 at 4:54 PM, Jiri Pirko <jiri(a)resnulli.us>
wrote:
> Tue, Nov 29, 2016 at 09:31:21AM CET, lucien.xin(a)gmail.com wrote:
>>On Tue, Nov 29, 2016 at 4:51 AM, Jiri Pirko <jiri(a)resnulli.us> wrote:
>>> Sun, Nov 27, 2016 at 08:28:16AM CET, lucien.xin(a)gmail.com wrote:
>>>>Now teamd uses dot symbol to split path string into different nodes
>>>>in __teamd_json_path_lite_va() when dumping teamd state. But team
>>>>port ifname is a part of path string, if ifname has dot character
>>>>inside, it would be parsed as two nodes incorrectly.
>>>>
>>>>Like, if users uses eth1.1 as a port ifname
>>>>teamdctl team0 state dump would be:
>>>> "ports": {
>>>> "eth1": {
>>>> "1": {
>>>> "ifinfo": {
>>>> ...
>>>>
>>>>and teamdctl team0 state view even crashs as:
>>>> ...
>>>> ports:
>>>> Failed to parse JSON port dump.
>>>> command call failed (Invalid argument)
>>>>
>>>>Actually it's common for users to use dot in ifname, especially for
>>>>vlan device.
>>>>
>>>>This patch is to replace dot with space in ifname for then, after
>>>>being parsed, it would be changed back to dot.
>>>
>>> Hmm, how about perhaps rather to escape the dot somehow:
>>> eth1.1 -> eth1\2e1
>>> ?
>>you mean to change eth1.1 to eth1\2e1 before parsing, but will
>>not change it back to eth1.1 after parsing ? and dump/view, it show
>>ifname with "eth1\2e1" ?
>
> No, I suggest exactly the same you want to do with " ", only with
"\2e"
> instead. Not sure why you think something else...
ok
>
>
>>
>>if yes, I think for users it's looks not nice, as in some places it show
>>eth1.1, but some place it show eth1\2e1 when dumping or viewing it.
>>
>>if we also change eth1\2e1 back to eth1.1 after parsing, coding will be
>>more complicate than with space.
>
> So what?
>
>
>>
>>beside, what if one ifname is "eth1\2e1", although it's not common.
>
> Okay. That was an example. The escape character should be either something
> which is not allowed in the name or you have to escape the original "\".
> I don't see any problem with that.
That's why I'm using " " which is not allowed in the name.
as the comment said:
"""
Note that it's safe to change it with space, as space in ifname is
disallowed. Besides, no teamd state attributes/nodes include space
afaik, so no side effects could be caused by this fix.
"""
I understand "\2e" is nicer, but we have to escape the original "\"
when ifname has "\2e", as you said above. It means we have to
escape dot and "\" to fix this problem completely.
Yes, please.
>>
>>
>>>
>>>
>>>do you know why we chose dot to split nodes?
>>>I'm asking that, because Hangbin has another fix for this issue,
>>>which changs to use slash "/" to split nodes, instead of dot. but
>>>need to modify many places, he is not sure if "/" may cause other
>>>problem ?
>>
>> Please leave the dot there. Just escape it.
>> It loosely follows jsonpath.
>>
>>
>>>
>>>
>>>>
>>>> Seems a bit nicer
>>>>
>>>>
>>>>>
>>>>>Note that it's safe to change it with space, as space in ifname
is
>>>>>disallowed. Besides, no teamd state attributes/nodes include space
>>>>>afaik, so no side effects could be caused by this fix.
>>>>>
>>>>>Signed-off-by: Xin Long <lucien.xin(a)gmail.com>
>>>>>---
>>>>> teamd/teamd_json.c | 16 +++++++++++++++-
>>>>> teamd/teamd_json.h | 1 +
>>>>> teamd/teamd_state.c | 13 ++++++++++---
>>>>> 3 files changed, 26 insertions(+), 4 deletions(-)
>>>>>
>>>>>diff --git a/teamd/teamd_json.c b/teamd/teamd_json.c
>>>>>index 5fd5e8e..7082484 100644
>>>>>--- a/teamd/teamd_json.c
>>>>>+++ b/teamd/teamd_json.c
>>>>>@@ -42,6 +42,19 @@ static char *__strchrs(char *str, char *chars)
>>>>> return NULL;
>>>>> }
>>>>>
>>>>>+char *strchrchg(char *str, char s, char d)
>>>>>+{
>>>>>+ char *tmp = str;
>>>>>+
>>>>>+ while (*str != '\0') {
>>>>>+ if (*str == s)
>>>>>+ *str = d;
>>>>>+ str++;
>>>>>+ }
>>>>>+
>>>>>+ return tmp;
>>>>>+}
>>>>>+
>>>>> #define TEAMD_JSON_PATH_MAXLEN 128
>>>>>
>>>>> typedef json_t *(*obj_constructor_t)(void);
>>>>>@@ -82,7 +95,8 @@ static int __teamd_json_path_lite_va(json_t
**p_json_obj, json_t *json_root,
>>>>> *end = '\0';
>>>>> }
>>>>> prev_json_obj = json_obj;
>>>>>- json_obj = json_object_get(prev_json_obj,
ptr);
>>>>>+ json_obj = json_object_get(prev_json_obj,
>>>>>+ strchrchg(ptr, '
', '.'));
>>>>> if (!json_obj && obj_constructor) {
>>>>> /* In case new object is not supposed
to be
>>>>> * leaf, use json_object() as a
constructor.
>>>>>diff --git a/teamd/teamd_json.h b/teamd/teamd_json.h
>>>>>index 30e9eaa..3dffdb5 100644
>>>>>--- a/teamd/teamd_json.h
>>>>>+++ b/teamd/teamd_json.h
>>>>>@@ -24,6 +24,7 @@
>>>>>
>>>>> #define TEAMD_JSON_DUMPS_FLAGS (JSON_INDENT(4) | JSON_ENSURE_ASCII |
JSON_SORT_KEYS)
>>>>>
>>>>>+char *strchrchg(char *str, char s, char d);
>>>>> int teamd_json_path_lite_va(json_t **p_json_obj, json_t *json_root,
>>>>> const char *fmt, va_list ap);
>>>>> int teamd_json_path_lite(json_t **p_json_obj, json_t *json_root,
>>>>>diff --git a/teamd/teamd_state.c b/teamd/teamd_state.c
>>>>>index 51fac8d..d9931dc 100644
>>>>>--- a/teamd/teamd_state.c
>>>>>+++ b/teamd/teamd_state.c
>>>>>@@ -194,11 +194,18 @@ static int
teamd_state_build_val_json_subpath(json_t **p_vg_json_obj,
>>>>> int err;
>>>>> char *dot;
>>>>>
>>>>>- if (tdport)
>>>>>+ if (tdport) {
>>>>>+ char *tmp;
>>>>>+
>>>>>+ if (asprintf(&tmp, tdport->ifname) == -1)
>>>>>+ return -ENOMEM;
>>>>>+
>>>>> ret = asprintf(&path, "$."
TEAMD_STATE_PER_PORT_PREFIX "%s%s",
>>>>>- tdport->ifname, subpath);
>>>>>- else
>>>>>+ strchrchg(tmp, '.', '
'), subpath);
>>>>>+ free(tmp);
>>>>>+ } else {
>>>>> ret = asprintf(&path, "$%s", subpath);
>>>>>+ }
>>>>> if (ret == -1)
>>>>> return -ENOMEM;
>>>>> dot = strrchr(path, '.');
>>>>>--
>>>>>2.1.0
>>>>>
2697
days inactive
2705
days old
libteam@lists.fedorahosted.org
5 comments
2 participants
participants (2)
-
Jiri Pirko -
Xin Long