Re: [Linuxptp-devel] [PATCH RFC 1/3] Add options to prefix ptp4l and phc2sys log messages.
On Mon, Oct 17, 2016 at 04:33:30PM +0200, Miroslav Lichvar wrote:
> When running multiple instances of ptp4l or phc2sys, it's difficult to
> tell which log message belongs to which instance. Add new options to
> ptp4l and phc2sys which can specify a prefix for all messages printed to
> the standard output or system log, so messages from different instances
> can have different prefixes.
This is a useful feature, but ...
> +.BI \-o " print-prefix"
> +Specifies a prefix for messages printed to the standard output or system log.
> +The default is empty string.
Here we should add:
Omit this option from the configuration file in order to set the
tag to the empty string.
I said "tag" and not "prefix". See below...
> @@ -67,13 +73,17 @@ void print(int level, char const *format, ...)
>
> if (verbose) {
> f = level >= LOG_NOTICE ? stdout : stderr;
> - fprintf(f, "%s[%ld.%03ld]: %s\n",
> + fprintf(f, "%s[%ld.%03ld]: %s%s%s\n",
The string comes after the time stamp and after the program name. So
it really isn't a prefix at all. Maybe this should be called "tag"
instead?
> progname ? progname : "",
> - ts.tv_sec, ts.tv_nsec / 100, buf);
> + ts.tv_sec, ts.tv_nsec / 100,
> + prefix ? prefix : "", prefix ? " " : "",
> + buf);
> fflush(f);
> }
> if (use_syslog) {
> - syslog(level, "[%ld.%03ld] %s",
> -ts.tv_sec, ts.tv_nsec / 100, buf);
> + syslog(level, "[%ld.%03ld] %s%s%s",
> +ts.tv_sec, ts.tv_nsec / 100,
> +prefix ? prefix : "", prefix ? " " : "",
> +buf);
> }
> }
> diff --git a/ptp4l.c b/ptp4l.c
> index a87e7e6..c4813cb 100644
> --- a/ptp4l.c
> +++ b/ptp4l.c
> @@ -62,6 +62,7 @@ static void usage(char *progname)
> " (ignored for SOFTWARE/LEGACY HW time stamping)\n"
> " -sslave only mode (overrides configuration file)\n"
> " -l [num] set the logging level to 'num'\n"
> + " -o [str] set prefix for log messages\n"
Do we really have to have a new command line option?
I don't want another one of these, especially not for ptp4l, and not
even for phc2sys or pmc. It just gets unwieldy and confusing.
Instead we should add '-f config' for phc2sys and pmc.
> " -mprint messages to stdout\n"
> " -qdo not print messages to the syslog\n"
> " -vprints the software version and exits\n"
Thanks,
Richard
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH RFC 1/3] Add options to prefix ptp4l and phc2sys log messages.
On Wed, 2016-11-02 at 12:47 +0100, Richard Cochran wrote: > Do we really have to have a new command line option? > > I don't want another one of these, especially not for ptp4l, and not > even for phc2sys or pmc. It just gets unwieldy and confusing. > Instead we should add '-f config' for phc2sys and pmc. > I agree. I think we can do that a lot easier now thanks to the work you did to modernize the configuration settings. Thanks, Jake -- Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today. http://sdm.link/xeonphi ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH RFC 1/3] Add options to prefix ptp4l and phc2sys log messages.
On Wed, Nov 02, 2016 at 12:47:14PM +0100, Richard Cochran wrote:
> On Mon, Oct 17, 2016 at 04:33:30PM +0200, Miroslav Lichvar wrote:
> > @@ -67,13 +73,17 @@ void print(int level, char const *format, ...)
> >
> > if (verbose) {
> > f = level >= LOG_NOTICE ? stdout : stderr;
> > - fprintf(f, "%s[%ld.%03ld]: %s\n",
> > + fprintf(f, "%s[%ld.%03ld]: %s%s%s\n",
>
> The string comes after the time stamp and after the program name. So
> it really isn't a prefix at all. Maybe this should be called "tag"
> instead?
Ok, so the option should be called print-tag, log-tag, message-tag, or
something else?
> > @@ -62,6 +62,7 @@ static void usage(char *progname)
> > " (ignored for SOFTWARE/LEGACY HW time stamping)\n"
> > " -sslave only mode (overrides configuration file)\n"
> > " -l [num] set the logging level to 'num'\n"
> > + " -o [str] set prefix for log messages\n"
>
> Do we really have to have a new command line option?
>
> I don't want another one of these, especially not for ptp4l, and not
> even for phc2sys or pmc. It just gets unwieldy and confusing.
> Instead we should add '-f config' for phc2sys and pmc.
I wouldn't mind if phc2sys and pmc supported config files, but I think
this particular option might be one of those that make more sense to
be used on command line rather than config file. If I wanted to run
multiple ptp4l instances with different tags for their log messages,
I could still share the config file between them if I specified the
interface and tag on the command line.
Would it be less confusing if we introduced GNU-style long option
names?
--
Miroslav Lichvar
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH RFC 1/3] Add options to prefix ptp4l and phc2sys log messages.
On Fri, Nov 04, 2016 at 03:31:18PM +0100, Miroslav Lichvar wrote: > Ok, so the option should be called print-tag, log-tag, message-tag, or > something else? 'message_tag' seems best to me. > I wouldn't mind if phc2sys and pmc supported config files, but I think > this particular option might be one of those that make more sense to > be used on command line rather than config file. If I wanted to run > multiple ptp4l instances with different tags for their log messages, > I could still share the config file between them if I specified the > interface and tag on the command line. Ok, that is a reason. But please can we have '-t' instead of '-o'? -o makes me think of "output file". > Would it be less confusing if we introduced GNU-style long option > names? Not to me. PTP has tons of options, and every new profile forces more and more of these. I simply find it gross when programs have a gazillion command line options. It makes for poor usability. I really wanted ptp4l to have the "top ten" options available on the command line, in order to keep it simple for the most common use cases. I think we should consider the long options for v2 if people really want them... Thanks, Richard -- Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today. http://sdm.link/xeonphi ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH RFC 1/3] Add options to prefix ptp4l and phc2sys log messages.
On Fri, Nov 4, 2016 at 2:01 PM, Richard Cochran wrote: > On Fri, Nov 04, 2016 at 03:31:18PM +0100, Miroslav Lichvar wrote: > > Would it be less confusing if we introduced GNU-style long option > > names? > > Not to me. PTP has tons of options, and every new profile forces more > and more of these. I simply find it gross when programs have a > gazillion command line options. It makes for poor usability. I > really wanted ptp4l to have the "top ten" options available on the > command line, in order to keep it simple for the most common use > cases. > My $0.02 For an embeded linux system, I find command line args much easier to deal with. Just pass in some $VARIABLE or maybe a $(command) in the script that execs the command. But for a config file, I need to edit it somehow. Maybe saving it in a non-vol filesystem. Editing is either generating the whole file directly or having a template somewhere with some sed scripting (like autotools *.in) files. A combination would be good I think. So a config file for the basic setup, and command line overrides/additions for things that need to be configurable or host-specific. -Dale -- Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today. http://sdm.link/xeonphi___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH RFC 1/3] Add options to prefix ptp4l and phc2sys log messages.
> -Original Message- > From: Richard Cochran [mailto:richardcoch...@gmail.com] > Sent: Friday, November 04, 2016 11:02 AM > To: Miroslav Lichvar > Cc: linuxptp-devel@lists.sourceforge.net > Subject: Re: [Linuxptp-devel] [PATCH RFC 1/3] Add options to prefix ptp4l and > phc2sys log messages. > > On Fri, Nov 04, 2016 at 03:31:18PM +0100, Miroslav Lichvar wrote: > > Ok, so the option should be called print-tag, log-tag, message-tag, or > > something else? > > 'message_tag' seems best to me. > I think you could still consider it a (as in one of many) prefix, as both of the things you said that appear before it may also be considered prefix. I don't mind the name "message_tag", though. > > I wouldn't mind if phc2sys and pmc supported config files, but I think > > this particular option might be one of those that make more sense to > > be used on command line rather than config file. If I wanted to run > > multiple ptp4l instances with different tags for their log messages, > > I could still share the config file between them if I specified the > > interface and tag on the command line. > > Ok, that is a reason. But please can we have '-t' instead of '-o'? > > -o makes me think of "output file". > Agreed, -t is better. > > Would it be less confusing if we introduced GNU-style long option > > names? > > Not to me. PTP has tons of options, and every new profile forces more > and more of these. I simply find it gross when programs have a > gazillion command line options. It makes for poor usability. I > really wanted ptp4l to have the "top ten" options available on the > command line, in order to keep it simple for the most common use > cases. > I think in this case introducing a "long" option would make more sense than using more small options. I do agree that we shouldn't proliferate command line options for every single thing in the config, (as many of these are esoteric and we would have to add more and more options as time goes on). However, I think that using long options can provide clarity that a small 1letter option name doesn't. That being said, "-t" makes sense to me. > I think we should consider the long options for v2 if people really > want them... > > Thanks, > Richard > Thanks, Jake -- Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today. http://sdm.link/xeonphi ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH RFC 1/3] Add options to prefix ptp4l and phc2sys log messages.
On Tue, Nov 08, 2016 at 12:32:04AM +, Keller, Jacob E wrote:
> I think in this case introducing a "long" option would make more
> sense than using more small options. I do agree that we shouldn't
> proliferate command line options for every single thing in the
> config, (as many of these are esoteric and we would have to add more
> and more options as time goes on). However, I think that using long
> options can provide clarity that a small 1letter option name
> doesn't.
> > I think we should consider the long options for v2 if people really
> > want them...
So it sounds like people do want long options. I'll see if I can come
up with a way to automatically support the config_tab[] entries in
config.c as command line options. For example
PORT_ITEM_INT("announceReceiptTimeout", 3, 2, UINT8_MAX),
GLOB_ITEM_INT("assume_two_step", 0, 0, 1),
PORT_ITEM_INT("boundary_clock_jbod", 0, 0, 1),
becomes
--announceReceiptTimeout val
--assume_two_step
--boundary_clock_jbod
It should be possible, I would think.
Thanks,
Richard
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH RFC 1/3] Add options to prefix ptp4l and phc2sys log messages.
On Tue, 2016-11-08 at 10:25 +0100, Richard Cochran wrote:
> On Tue, Nov 08, 2016 at 12:32:04AM +, Keller, Jacob E wrote:
> >
> > I think in this case introducing a "long" option would make more
> > sense than using more small options. I do agree that we shouldn't
> > proliferate command line options for every single thing in the
> > config, (as many of these are esoteric and we would have to add
> > more
> > and more options as time goes on). However, I think that using long
> > options can provide clarity that a small 1letter option name
> > doesn't.
>
> >
> > >
> > > I think we should consider the long options for v2 if people
> > > really
> > > want them...
>
> So it sounds like people do want long options. I'll see if I can
> come
> up with a way to automatically support the config_tab[] entries in
> config.c as command line options. For example
>
> PORT_ITEM_INT("announceReceiptTimeout", 3, 2, UINT8_MAX),
> GLOB_ITEM_INT("assume_two_step", 0, 0, 1),
> PORT_ITEM_INT("boundary_clock_jbod", 0, 0, 1),
>
> becomes
>
> --announceReceiptTimeout val
> --assume_two_step
> --boundary_clock_jbod
>
> It should be possible, I would think.
>
> Thanks,
> Richard
>
>
>
>
That could work. I wouldn't necessarily include all options, you could
add a new field in the config block that indicates whether that option
should be considered? Or just supporting all long options also works
too I suppose if that's easier?
I do agree that we don't need to proliferate every long option, but I
think that some might be valuable and wouldn't want to waste our small
space of letters and numbers on them.
I'm personally ok with every config being available from its long name
as an option, so if that is easier then we can just go that route.
Thanks,
Jake
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH RFC 1/3] Add options to prefix ptp4l and phc2sys log messages.
On Tue, Nov 08, 2016 at 10:25:58AM +0100, Richard Cochran wrote:
> So it sounds like people do want long options. I'll see if I can come
> up with a way to automatically support the config_tab[] entries in
> config.c as command line options. For example
>
> PORT_ITEM_INT("announceReceiptTimeout", 3, 2, UINT8_MAX),
> GLOB_ITEM_INT("assume_two_step", 0, 0, 1),
> PORT_ITEM_INT("boundary_clock_jbod", 0, 0, 1),
>
> becomes
>
> --announceReceiptTimeout val
> --assume_two_step
> --boundary_clock_jbod
I think this would be nice. If it's implemented, it would probably
make sense to include all options, even the obscure ones, so there is
no confusion when someone tries to use them on command line and the
man page doesn't have to explain which can be used only in the config
file.
--
Miroslav Lichvar
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH RFC 1/3] Add options to prefix ptp4l and phc2sys log messages.
> -Original Message-
> From: Miroslav Lichvar [mailto:mlich...@redhat.com]
> Sent: Wednesday, November 09, 2016 3:38 AM
> To: Richard Cochran
> Cc: Keller, Jacob E ;
> linuxptp-devel@lists.sourceforge.net
> Subject: Re: [Linuxptp-devel] [PATCH RFC 1/3] Add options to prefix ptp4l and
> phc2sys log messages.
>
> On Tue, Nov 08, 2016 at 10:25:58AM +0100, Richard Cochran wrote:
> > So it sounds like people do want long options. I'll see if I can come
> > up with a way to automatically support the config_tab[] entries in
> > config.c as command line options. For example
> >
> > PORT_ITEM_INT("announceReceiptTimeout", 3, 2, UINT8_MAX),
> > GLOB_ITEM_INT("assume_two_step", 0, 0, 1),
> > PORT_ITEM_INT("boundary_clock_jbod", 0, 0, 1),
> >
> > becomes
> >
> > --announceReceiptTimeout val
> > --assume_two_step
> > --boundary_clock_jbod
>
> I think this would be nice. If it's implemented, it would probably
> make sense to include all options, even the obscure ones, so there is
> no confusion when someone tries to use them on command line and the
> man page doesn't have to explain which can be used only in the config
> file.
>
I think I changed my opinion and agree with you here. It makes sense since the
long names will be the same as the config file.
Thanks,
Jake
> --
> Miroslav Lichvar
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
