[ovs-dev] [PATCH] ovs-vsctl: add caller information by appending comment

[Ansis Atteka]

On Wed, Dec 23, 2015 at 9:41 AM, Ben Pfaff <blp at ovn.org> wrote:
> I think maybe you misinterpreted what I meant when I said "every
> caller".  When I said that, I meant "every program that invokes
> ovs-vsctl", not the entire call stack.

William came to me to discuss this approach, If we both understood it
correctly, then it would be impossible to get the LINE number with it,
right?

Nevertheless, it would make comment generation simpler at expense of
printing only the parent process.

>
> Anyhow, either way this is going to be helpful for debugging, thanks for
> writing it up.

Ben, Thanks for reviewing this.

William, can you send V2 patch? I think you were trying to figure out
if it would be possible to source the common ovs_vsctl function to
avoid repetition?

>
> On Wed, Dec 23, 2015 at 07:41:56AM -0800, William Tu wrote:
>> Hi Ben,
>>
>> Thanks for the feedback.
>> I will work around the "caller" by changing #!/bin/sh to #!/bin/bash
>>
>> As for the parent process, I think it depends on how verbose people
>> consider the information helpful. The current patch only adds the immediate
>> caller, but we could definitely add the entire call stack within the same
>> process, or like you suggest, add the parent process.
>>
>> Regards,
>> William
>>
>> On Tue, Dec 22, 2015 at 11:18 PM, Ben Pfaff <blp at ovn.org> wrote:
>>
>> > On Tue, Dec 22, 2015 at 01:56:55PM -0800, William Tu wrote:
>> > > The patch adds the caller's information of ovs_vsctl() in order
>> > > to assist debugging. The caller's information is formatted as
>> > > "(filename, line number)".
>> > >
>> > > An example:
>> > > > ovsdb-tool show-log
>> > > record 183: 2015-12-22 21:12:26.050 "ovs-vsctl: ovs-vsctl --no-wait
>> > > add-br br0 -- comment (FILE:./ifupdown.sh,LINE:81)"
>> >
>> > This looks useful but as-is the ifupdown.sh code appears bash-specific,
>> > because I don't think "caller" is POSIX.  You can probably work around
>> > that by checking for bash, or by changing #!/bin/sh to #!/bin/bash since
>> > this is Debian-specific anyway.
>> >
>> > You could get most of the benefit of this change by modifying ovs-vsctl
>> > to get the name of its parent process.  That requires OS-specific code
>> > but at least on Linux it shouldn't be difficult.  It would have the
>> > advantage that it would work for every caller not just for the ones that
>> > the patch specifically updates.  Did you consider that idea?
>> >
>> > Thanks,
>> >
>> > Ben.
>> >
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev