[ovs-dev] [PATCH v2 12/13] ovn-trace: Display friendlier port and datapath names.

[Andy Zhou]

On Thu, May 4, 2017 at 10:55 AM, Ben Pfaff <blp at ovn.org> wrote:
> On Thu, May 04, 2017 at 02:37:43AM -0700, Andy Zhou wrote:
>> On Wed, May 3, 2017 at 8:45 AM, Ben Pfaff <blp at ovn.org> wrote:
>> > This makes ovn-trace use short name instead of UUIDs (etc.) in its own
>> > output, by default.  Since it's possible that there's software out there
>> > parsing ovn-trace output, it also adds a --no-friendly-names option.
>> >
>> > Signed-off-by: Ben Pfaff <blp at ovn.org>
>> > @@ -533,6 +566,9 @@ read_datapaths(void)
>> >                      : xasprintf(UUID_FMT, UUID_ARGS(&dp->nb_uuid)));
>> >
>> >          dp->name2 = nullable_xstrdup(smap_get(ids, "name2"));
>> > +        dp->friendly_name = (!use_friendly_names
>> > +                             ? xasprintf(UUID_FMT, UUID_ARGS(&dp->nb_uuid))
>> > +                             : shorten_uuid(dp->name2 ? dp->name2 : dp->name));
>> In case both name and name2 are null and use_friendly_names is true, should
>> we try not to pass NULL to shorten_uuid, and may be provide a shorten the uuid
>> of nb_uuid?
>
> I think that's what will happen.  If name and name2 are null, then the
> following code will set dp->name2 to NULL and dp->name to the NB UUID
> string, and so shorten_uuid() will ultimately shorten the NB UUID.  Am I
> missing something?
Sorry, I miss read.

Acked-by: Andy Zhou <azhou at ovn.org>


>
>         const char *name = smap_get(ids, "name");
>         dp->name = (name
>                     ? xstrdup(name)
>                     : xasprintf(UUID_FMT, UUID_ARGS(&dp->nb_uuid)));
>
>         dp->name2 = nullable_xstrdup(smap_get(ids, "name2"));
>
>
>> > @@ -568,8 +604,11 @@ read_ports(void)
>> >          port->name = xstrdup(port_name);
>> >          port->type = xstrdup(sbpb->type);
>> >          port->tunnel_key = sbpb->tunnel_key;
>> > -        port->friendly_name = nullable_xstrdup(smap_get(&sbpb->external_ids,
>> > -                                                        "name"));
>> > +
>> > +        port->name2 = nullable_xstrdup(smap_get(&sbpb->external_ids, "name"));
>> > +        port->friendly_name = (!use_friendly_names ? xstrdup(port->name)
>> > +                               : shorten_uuid(port->name2
>> > +                                              ? port->name2 : port->name));
>> Here also about potentially passing NULL to shorten_uuid()
>
> I think that this situation is safe too, because port->name is always
> nonnull, because it is initialized as xstrdup(port_name).
You are right here too. Sorry.