On Tue, Mar 07, 2023 at 12:24:55PM +0800, Wan Junjie wrote:
> Hi Simon Horman,
>
> Thanks for your review.
>
>
> On Mon, Mar 6, 2023 at 11:23 PM Simon Horman <simon.hor...@corigine.com>
> wrote:
> > On Sat, Mar 04, 2023 at 11:23:50AM +0800, Wan Junjie wrote:
...
> > > On Fri, Mar 3, 2023 at 11:06 PM Simon Horman <simon.hor...@corigine.com>
> > > wrote:
> > > > On Wed, Mar 01, 2023 at 04:05:17PM +0800, Wan Junjie wrote:
...
> > > > > + struct ofputil_meter_config *mcs;
> > > > > + size_t n_mtrs;
> > > > > + run(vconn_dump_meters(vconn, request, &mcs, &n_mtrs),
> > > > > + "dump meters");
> > > > > +
> > > > > + struct ds s = DS_EMPTY_INITIALIZER;
> > > > > + for (size_t i = 0; i < n_mtrs; i ++) {
> > > > > + ds_clear(&s);
> > > > > + ofputil_format_meter_config(&s, &mcs[i], 1);
> > > > > + printf("%s\n", ds_cstr(&s));
> > > > > + }
> > > > > + ds_destroy(&s);
> > > > > +
> > > > > + for (size_t i = 0; i < n_mtrs; i ++) {
> > > > > + free(CONST_CAST(struct ofputil_meter_band *,
> > > > > mcs[i].bands));
> > > > > + }
> > > > > + free(mcs);
> > > >
> > > > The above, ofputil_format_meter_config(), and recv_meter_stats_reply()
> > > > seems to be a lot of code to customise what was otherwise a call to
> > > > dump_transaction().
> > > >
> > > > Perhaps it would be cleaner to parameterise ofp_print() over 'oneline'.
> > > >
> > > That was my first thought, then I realize that I would have to add
> > > this parameter to several
> > > functions recursively, like dump_transaction(), ofp_print(),
> > > ofp_to_string__(), ofp_to_string().
> > > I don't think these functions really need that parameter since only
> > > meter has compatibility issues.
> >
> > I understand your point. But I think it would be better to teach the
> > core/common code how to handle this and avoid open coding the special case.
> >
> > Perhaps a 'oneline' parameter isn't the best approach.
> > But really one bit of information is needed in order
> > for core/common code to implement a (slightly) different behaviour.
> >
> > And it is conceivable that other dump functions will need
> > alternate behaviour in future.
> >
> OK, I will see if I can merge it to 'verbosity'.
Thanks. I agree that overloading/reusing the verbosity parameter
is worth a shot.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
