diff mbox

[RFC,1/7] net/filter: Fix the output information for command 'info network'

Message ID 1453451811-11860-2-git-send-email-zhang.zhanghailiang@huawei.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zhanghailiang Jan. 22, 2016, 8:36 a.m. UTC
The properties of netfilter object could be changed by 'qom-set'
command, but the output of 'info network' command is not updated,
because it got the old information through nf->info_str, it will
not be updated while we change the value of netfilter's property.

Here we split a the helper function that could colletct the output
information for filter, and also remove the useless member
'info_str' from struct NetFilterState.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 include/net/filter.h |  3 ++-
 net/filter.c         | 47 ++++++++++++++++++++++++++---------------------
 net/net.c            |  5 ++++-
 3 files changed, 32 insertions(+), 23 deletions(-)

Comments

Jason Wang Jan. 25, 2016, 5:01 a.m. UTC | #1
On 01/22/2016 04:36 PM, zhanghailiang wrote:
> The properties of netfilter object could be changed by 'qom-set'
> command, but the output of 'info network' command is not updated,
> because it got the old information through nf->info_str, it will
> not be updated while we change the value of netfilter's property.
>
> Here we split a the helper function that could colletct the output
> information for filter, and also remove the useless member
> 'info_str' from struct NetFilterState.
>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> ---

Looks like a bug fix. Please send as a independent formal patch and
better cc Markus and Eric for better reviewing.

Thanks

>  include/net/filter.h |  3 ++-
>  net/filter.c         | 47 ++++++++++++++++++++++++++---------------------
>  net/net.c            |  5 ++++-
>  3 files changed, 32 insertions(+), 23 deletions(-)
>
> diff --git a/include/net/filter.h b/include/net/filter.h
> index 2deda36..8a20138 100644
> --- a/include/net/filter.h
> +++ b/include/net/filter.h
> @@ -55,7 +55,6 @@ struct NetFilterState {
>      char *netdev_id;
>      NetClientState *netdev;
>      NetFilterDirection direction;
> -    char info_str[256];
>      QTAILQ_ENTRY(NetFilterState) next;
>  };
>  
> @@ -74,4 +73,6 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
>                                      int iovcnt,
>                                      void *opaque);
>  
> +void netfilter_print_info(NetFilterState *nf, char *output_str, int size);
> +
>  #endif /* QEMU_NET_FILTER_H */
> diff --git a/net/filter.c b/net/filter.c
> index 5d90f83..40254bd 100644
> --- a/net/filter.c
> +++ b/net/filter.c
> @@ -128,6 +128,31 @@ static void netfilter_init(Object *obj)
>                               NULL);
>  }
>  
> +void netfilter_print_info(NetFilterState *nf, char *output_str, int size)
> +{
> +    char *str, *info;
> +    ObjectProperty *prop;
> +    ObjectPropertyIterator iter;
> +    StringOutputVisitor *ov;
> +
> +    /* generate info str */
> +    object_property_iter_init(&iter, OBJECT(nf));
> +    while ((prop = object_property_iter_next(&iter))) {
> +        if (!strcmp(prop->name, "type")) {
> +            continue;
> +        }
> +        ov = string_output_visitor_new(false);
> +        object_property_get(OBJECT(nf), string_output_get_visitor(ov),
> +                            prop->name, NULL);
> +        str = string_output_get_string(ov);
> +        string_output_visitor_cleanup(ov);
> +        info = g_strdup_printf(",%s=%s", prop->name, str);
> +        g_strlcat(output_str, info, size);
> +        g_free(str);
> +        g_free(info);
> +    }
> +}
> +
>  static void netfilter_complete(UserCreatable *uc, Error **errp)
>  {
>      NetFilterState *nf = NETFILTER(uc);
> @@ -135,10 +160,7 @@ static void netfilter_complete(UserCreatable *uc, Error **errp)
>      NetFilterClass *nfc = NETFILTER_GET_CLASS(uc);
>      int queues;
>      Error *local_err = NULL;
> -    char *str, *info;
> -    ObjectProperty *prop;
> -    ObjectPropertyIterator iter;
> -    StringOutputVisitor *ov;
> +
>  
>      if (!nf->netdev_id) {
>          error_setg(errp, "Parameter 'netdev' is required");
> @@ -172,23 +194,6 @@ static void netfilter_complete(UserCreatable *uc, Error **errp)
>          }
>      }
>      QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next);
> -
> -    /* generate info str */
> -    object_property_iter_init(&iter, OBJECT(nf));
> -    while ((prop = object_property_iter_next(&iter))) {
> -        if (!strcmp(prop->name, "type")) {
> -            continue;
> -        }
> -        ov = string_output_visitor_new(false);
> -        object_property_get(OBJECT(nf), string_output_get_visitor(ov),
> -                            prop->name, errp);
> -        str = string_output_get_string(ov);
> -        string_output_visitor_cleanup(ov);
> -        info = g_strdup_printf(",%s=%s", prop->name, str);
> -        g_strlcat(nf->info_str, info, sizeof(nf->info_str));
> -        g_free(str);
> -        g_free(info);
> -    }
>  }
>  
>  static void netfilter_finalize(Object *obj)
> diff --git a/net/net.c b/net/net.c
> index 87dd356..87de7c0 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1198,9 +1198,12 @@ void print_net_client(Monitor *mon, NetClientState *nc)
>      }
>      QTAILQ_FOREACH(nf, &nc->filters, next) {
>          char *path = object_get_canonical_path_component(OBJECT(nf));
> +        char info[256] = { 0 };
> +
> +        netfilter_print_info(nf, info, sizeof(info));
>          monitor_printf(mon, "  - %s: type=%s%s\n", path,
>                         object_get_typename(OBJECT(nf)),
> -                       nf->info_str);
> +                       info);
>          g_free(path);
>      }
>  }
Zhanghailiang Jan. 25, 2016, 5:58 a.m. UTC | #2
On 2016/1/25 13:01, Jason Wang wrote:
>
>
> On 01/22/2016 04:36 PM, zhanghailiang wrote:
>> The properties of netfilter object could be changed by 'qom-set'
>> command, but the output of 'info network' command is not updated,
>> because it got the old information through nf->info_str, it will
>> not be updated while we change the value of netfilter's property.
>>
>> Here we split a the helper function that could colletct the output
>> information for filter, and also remove the useless member
>> 'info_str' from struct NetFilterState.
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> ---
>
> Looks like a bug fix. Please send as a independent formal patch and
> better cc Markus and Eric for better reviewing.
>

OK, by the way, i didn't see there was a corresponding qmp command
for 'info network', is the hmp_info_network just an experimental command ?

> Thanks
>
>>   include/net/filter.h |  3 ++-
>>   net/filter.c         | 47 ++++++++++++++++++++++++++---------------------
>>   net/net.c            |  5 ++++-
>>   3 files changed, 32 insertions(+), 23 deletions(-)
>>
>> diff --git a/include/net/filter.h b/include/net/filter.h
>> index 2deda36..8a20138 100644
>> --- a/include/net/filter.h
>> +++ b/include/net/filter.h
>> @@ -55,7 +55,6 @@ struct NetFilterState {
>>       char *netdev_id;
>>       NetClientState *netdev;
>>       NetFilterDirection direction;
>> -    char info_str[256];
>>       QTAILQ_ENTRY(NetFilterState) next;
>>   };
>>
>> @@ -74,4 +73,6 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
>>                                       int iovcnt,
>>                                       void *opaque);
>>
>> +void netfilter_print_info(NetFilterState *nf, char *output_str, int size);
>> +
>>   #endif /* QEMU_NET_FILTER_H */
>> diff --git a/net/filter.c b/net/filter.c
>> index 5d90f83..40254bd 100644
>> --- a/net/filter.c
>> +++ b/net/filter.c
>> @@ -128,6 +128,31 @@ static void netfilter_init(Object *obj)
>>                                NULL);
>>   }
>>
>> +void netfilter_print_info(NetFilterState *nf, char *output_str, int size)
>> +{
>> +    char *str, *info;
>> +    ObjectProperty *prop;
>> +    ObjectPropertyIterator iter;
>> +    StringOutputVisitor *ov;
>> +
>> +    /* generate info str */
>> +    object_property_iter_init(&iter, OBJECT(nf));
>> +    while ((prop = object_property_iter_next(&iter))) {
>> +        if (!strcmp(prop->name, "type")) {
>> +            continue;
>> +        }
>> +        ov = string_output_visitor_new(false);
>> +        object_property_get(OBJECT(nf), string_output_get_visitor(ov),
>> +                            prop->name, NULL);
>> +        str = string_output_get_string(ov);
>> +        string_output_visitor_cleanup(ov);
>> +        info = g_strdup_printf(",%s=%s", prop->name, str);
>> +        g_strlcat(output_str, info, size);
>> +        g_free(str);
>> +        g_free(info);
>> +    }
>> +}
>> +
>>   static void netfilter_complete(UserCreatable *uc, Error **errp)
>>   {
>>       NetFilterState *nf = NETFILTER(uc);
>> @@ -135,10 +160,7 @@ static void netfilter_complete(UserCreatable *uc, Error **errp)
>>       NetFilterClass *nfc = NETFILTER_GET_CLASS(uc);
>>       int queues;
>>       Error *local_err = NULL;
>> -    char *str, *info;
>> -    ObjectProperty *prop;
>> -    ObjectPropertyIterator iter;
>> -    StringOutputVisitor *ov;
>> +
>>
>>       if (!nf->netdev_id) {
>>           error_setg(errp, "Parameter 'netdev' is required");
>> @@ -172,23 +194,6 @@ static void netfilter_complete(UserCreatable *uc, Error **errp)
>>           }
>>       }
>>       QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next);
>> -
>> -    /* generate info str */
>> -    object_property_iter_init(&iter, OBJECT(nf));
>> -    while ((prop = object_property_iter_next(&iter))) {
>> -        if (!strcmp(prop->name, "type")) {
>> -            continue;
>> -        }
>> -        ov = string_output_visitor_new(false);
>> -        object_property_get(OBJECT(nf), string_output_get_visitor(ov),
>> -                            prop->name, errp);
>> -        str = string_output_get_string(ov);
>> -        string_output_visitor_cleanup(ov);
>> -        info = g_strdup_printf(",%s=%s", prop->name, str);
>> -        g_strlcat(nf->info_str, info, sizeof(nf->info_str));
>> -        g_free(str);
>> -        g_free(info);
>> -    }
>>   }
>>
>>   static void netfilter_finalize(Object *obj)
>> diff --git a/net/net.c b/net/net.c
>> index 87dd356..87de7c0 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -1198,9 +1198,12 @@ void print_net_client(Monitor *mon, NetClientState *nc)
>>       }
>>       QTAILQ_FOREACH(nf, &nc->filters, next) {
>>           char *path = object_get_canonical_path_component(OBJECT(nf));
>> +        char info[256] = { 0 };
>> +
>> +        netfilter_print_info(nf, info, sizeof(info));
>>           monitor_printf(mon, "  - %s: type=%s%s\n", path,
>>                          object_get_typename(OBJECT(nf)),
>> -                       nf->info_str);
>> +                       info);
>>           g_free(path);
>>       }
>>   }
>
>
> .
>
Jason Wang Jan. 26, 2016, 3:05 a.m. UTC | #3
On 01/25/2016 01:58 PM, Hailiang Zhang wrote:
> On 2016/1/25 13:01, Jason Wang wrote:
>>
>>
>> On 01/22/2016 04:36 PM, zhanghailiang wrote:
>>> The properties of netfilter object could be changed by 'qom-set'
>>> command, but the output of 'info network' command is not updated,
>>> because it got the old information through nf->info_str, it will
>>> not be updated while we change the value of netfilter's property.
>>>
>>> Here we split a the helper function that could colletct the output
>>> information for filter, and also remove the useless member
>>> 'info_str' from struct NetFilterState.
>>>
>>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>>> ---
>>
>> Looks like a bug fix. Please send as a independent formal patch and
>> better cc Markus and Eric for better reviewing.
>>
>
> OK, by the way, i didn't see there was a corresponding qmp command
> for 'info network', is the hmp_info_network just an experimental
> command ? 

It's for debugging I think , and if you want it to be used by
management, probably need a qmp version.

Thanks
Zhanghailiang Jan. 26, 2016, 3:34 a.m. UTC | #4
On 2016/1/26 11:05, Jason Wang wrote:
>
>
> On 01/25/2016 01:58 PM, Hailiang Zhang wrote:
>> On 2016/1/25 13:01, Jason Wang wrote:
>>>
>>>
>>> On 01/22/2016 04:36 PM, zhanghailiang wrote:
>>>> The properties of netfilter object could be changed by 'qom-set'
>>>> command, but the output of 'info network' command is not updated,
>>>> because it got the old information through nf->info_str, it will
>>>> not be updated while we change the value of netfilter's property.
>>>>
>>>> Here we split a the helper function that could colletct the output
>>>> information for filter, and also remove the useless member
>>>> 'info_str' from struct NetFilterState.
>>>>
>>>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>>>> ---
>>>
>>> Looks like a bug fix. Please send as a independent formal patch and
>>> better cc Markus and Eric for better reviewing.
>>>
>>
>> OK, by the way, i didn't see there was a corresponding qmp command
>> for 'info network', is the hmp_info_network just an experimental
>> command ?
>
> It's for debugging I think , and if you want it to be used by
> management, probably need a qmp version.

Got it, the management does not need it for now, thanks.

> .
>
diff mbox

Patch

diff --git a/include/net/filter.h b/include/net/filter.h
index 2deda36..8a20138 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -55,7 +55,6 @@  struct NetFilterState {
     char *netdev_id;
     NetClientState *netdev;
     NetFilterDirection direction;
-    char info_str[256];
     QTAILQ_ENTRY(NetFilterState) next;
 };
 
@@ -74,4 +73,6 @@  ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
                                     int iovcnt,
                                     void *opaque);
 
+void netfilter_print_info(NetFilterState *nf, char *output_str, int size);
+
 #endif /* QEMU_NET_FILTER_H */
diff --git a/net/filter.c b/net/filter.c
index 5d90f83..40254bd 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -128,6 +128,31 @@  static void netfilter_init(Object *obj)
                              NULL);
 }
 
+void netfilter_print_info(NetFilterState *nf, char *output_str, int size)
+{
+    char *str, *info;
+    ObjectProperty *prop;
+    ObjectPropertyIterator iter;
+    StringOutputVisitor *ov;
+
+    /* generate info str */
+    object_property_iter_init(&iter, OBJECT(nf));
+    while ((prop = object_property_iter_next(&iter))) {
+        if (!strcmp(prop->name, "type")) {
+            continue;
+        }
+        ov = string_output_visitor_new(false);
+        object_property_get(OBJECT(nf), string_output_get_visitor(ov),
+                            prop->name, NULL);
+        str = string_output_get_string(ov);
+        string_output_visitor_cleanup(ov);
+        info = g_strdup_printf(",%s=%s", prop->name, str);
+        g_strlcat(output_str, info, size);
+        g_free(str);
+        g_free(info);
+    }
+}
+
 static void netfilter_complete(UserCreatable *uc, Error **errp)
 {
     NetFilterState *nf = NETFILTER(uc);
@@ -135,10 +160,7 @@  static void netfilter_complete(UserCreatable *uc, Error **errp)
     NetFilterClass *nfc = NETFILTER_GET_CLASS(uc);
     int queues;
     Error *local_err = NULL;
-    char *str, *info;
-    ObjectProperty *prop;
-    ObjectPropertyIterator iter;
-    StringOutputVisitor *ov;
+
 
     if (!nf->netdev_id) {
         error_setg(errp, "Parameter 'netdev' is required");
@@ -172,23 +194,6 @@  static void netfilter_complete(UserCreatable *uc, Error **errp)
         }
     }
     QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next);
-
-    /* generate info str */
-    object_property_iter_init(&iter, OBJECT(nf));
-    while ((prop = object_property_iter_next(&iter))) {
-        if (!strcmp(prop->name, "type")) {
-            continue;
-        }
-        ov = string_output_visitor_new(false);
-        object_property_get(OBJECT(nf), string_output_get_visitor(ov),
-                            prop->name, errp);
-        str = string_output_get_string(ov);
-        string_output_visitor_cleanup(ov);
-        info = g_strdup_printf(",%s=%s", prop->name, str);
-        g_strlcat(nf->info_str, info, sizeof(nf->info_str));
-        g_free(str);
-        g_free(info);
-    }
 }
 
 static void netfilter_finalize(Object *obj)
diff --git a/net/net.c b/net/net.c
index 87dd356..87de7c0 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1198,9 +1198,12 @@  void print_net_client(Monitor *mon, NetClientState *nc)
     }
     QTAILQ_FOREACH(nf, &nc->filters, next) {
         char *path = object_get_canonical_path_component(OBJECT(nf));
+        char info[256] = { 0 };
+
+        netfilter_print_info(nf, info, sizeof(info));
         monitor_printf(mon, "  - %s: type=%s%s\n", path,
                        object_get_typename(OBJECT(nf)),
-                       nf->info_str);
+                       info);
         g_free(path);
     }
 }