Re: [Qemu-devel] [PATCH RFC 1/7] net/filter: Fix the output information for command 'info network'

[Hailiang Zhang]

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 <address@hidden>
> > ---
>
> 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);
> >       }
> >   }
>
>
> .