[ovs-dev] vlog: deprecate --syslog-target argument
diff mbox

Message ID 1442456970-15309-1-git-send-email-aatteka@nicira.com
State Accepted
Headers show

Commit Message

Ansis Atteka Sept. 17, 2015, 2:29 a.m. UTC
Commit fe089c0d1e18 ("vlog: abstract out interface to syslog daemon")
introduced --syslog-method flag that supersedes --syslog-target flag by:
1. making logging format configurable
2. letting daemon to also talk over UNIX domain socket (this is handy
   when local rsyslog daemon is running in different network namespace
   on the same host)

Signed-off-by: Ansis Atteka <aatteka@nicira.com>
---
 NEWS       |  1 +
 lib/vlog.c | 10 ++++++++++
 2 files changed, 11 insertions(+)

Comments

Ben Pfaff Sept. 18, 2015, 10:35 p.m. UTC | #1
On Wed, Sep 16, 2015 at 07:29:30PM -0700, Ansis Atteka wrote:
> Commit fe089c0d1e18 ("vlog: abstract out interface to syslog daemon")
> introduced --syslog-method flag that supersedes --syslog-target flag by:
> 1. making logging format configurable
> 2. letting daemon to also talk over UNIX domain socket (this is handy
>    when local rsyslog daemon is running in different network namespace
>    on the same host)
> 
> Signed-off-by: Ansis Atteka <aatteka@nicira.com>
> ---
>  NEWS       |  1 +
>  lib/vlog.c | 10 ++++++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/NEWS b/NEWS
> index ca22c8e..8bdaf3e 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -21,6 +21,7 @@ Post-v2.4.0
>       targets to run a new system testsuite.  These tests can be run inside
>       a Vagrant box.  See INSTALL.md for details
>     - Dropped support for GRE64 tunnel.
> +   - Mark --syslog-target argument as deprecated.

In the past when we've deprecated features we've also given a date after
which we might remove the feature.  It's usually 6 to 12 months out.  Do
you want to do that here?

Acked-by: Ben Pfaff <blp@nicira.com>
Ansis Atteka Sept. 18, 2015, 11:09 p.m. UTC | #2
On 18 September 2015 at 15:35, Ben Pfaff <blp@nicira.com> wrote:

> On Wed, Sep 16, 2015 at 07:29:30PM -0700, Ansis Atteka wrote:
> > Commit fe089c0d1e18 ("vlog: abstract out interface to syslog daemon")
> > introduced --syslog-method flag that supersedes --syslog-target flag by:
> > 1. making logging format configurable
> > 2. letting daemon to also talk over UNIX domain socket (this is handy
> >    when local rsyslog daemon is running in different network namespace
> >    on the same host)
> >
> > Signed-off-by: Ansis Atteka <aatteka@nicira.com>
> > ---
> >  NEWS       |  1 +
> >  lib/vlog.c | 10 ++++++++++
> >  2 files changed, 11 insertions(+)
> >
> > diff --git a/NEWS b/NEWS
> > index ca22c8e..8bdaf3e 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -21,6 +21,7 @@ Post-v2.4.0
> >       targets to run a new system testsuite.  These tests can be run
> inside
> >       a Vagrant box.  See INSTALL.md for details
> >     - Dropped support for GRE64 tunnel.
> > +   - Mark --syslog-target argument as deprecated.
>
> In the past when we've deprecated features we've also given a date after
> which we might remove the feature.  It's usually 6 to 12 months out.  Do
> you want to do that here?
>

> Acked-by: Ben Pfaff <blp@nicira.com>
>
I will mark this as "to be removed in 6 months" and push.

Do you think that we should have a sentence or two in CONTRIBUTING.md file
about "feature deprecation guidelines"? If so I can try to write this up.




> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
Ben Pfaff Sept. 19, 2015, 4:37 p.m. UTC | #3
On Fri, Sep 18, 2015 at 04:09:48PM -0700, Ansis Atteka wrote:
> On 18 September 2015 at 15:35, Ben Pfaff <blp@nicira.com> wrote:
> 
> > On Wed, Sep 16, 2015 at 07:29:30PM -0700, Ansis Atteka wrote:
> > > Commit fe089c0d1e18 ("vlog: abstract out interface to syslog daemon")
> > > introduced --syslog-method flag that supersedes --syslog-target flag by:
> > > 1. making logging format configurable
> > > 2. letting daemon to also talk over UNIX domain socket (this is handy
> > >    when local rsyslog daemon is running in different network namespace
> > >    on the same host)
> > >
> > > Signed-off-by: Ansis Atteka <aatteka@nicira.com>
> > > ---
> > >  NEWS       |  1 +
> > >  lib/vlog.c | 10 ++++++++++
> > >  2 files changed, 11 insertions(+)
> > >
> > > diff --git a/NEWS b/NEWS
> > > index ca22c8e..8bdaf3e 100644
> > > --- a/NEWS
> > > +++ b/NEWS
> > > @@ -21,6 +21,7 @@ Post-v2.4.0
> > >       targets to run a new system testsuite.  These tests can be run
> > inside
> > >       a Vagrant box.  See INSTALL.md for details
> > >     - Dropped support for GRE64 tunnel.
> > > +   - Mark --syslog-target argument as deprecated.
> >
> > In the past when we've deprecated features we've also given a date after
> > which we might remove the feature.  It's usually 6 to 12 months out.  Do
> > you want to do that here?
> >
> 
> > Acked-by: Ben Pfaff <blp@nicira.com>
> >
> I will mark this as "to be removed in 6 months" and push.

Can you give a specific date (e.g. "March 2016") so that users don't
have to guess what the start date is?

> Do you think that we should have a sentence or two in CONTRIBUTING.md file
> about "feature deprecation guidelines"? If so I can try to write this up.

That sounds like a good idea to me.
Ansis Atteka Sept. 19, 2015, 9:14 p.m. UTC | #4
On 19 September 2015 at 09:37, Ben Pfaff <blp@nicira.com> wrote:

> On Fri, Sep 18, 2015 at 04:09:48PM -0700, Ansis Atteka wrote:
> > On 18 September 2015 at 15:35, Ben Pfaff <blp@nicira.com> wrote:
> >
> > > On Wed, Sep 16, 2015 at 07:29:30PM -0700, Ansis Atteka wrote:
> > > > Commit fe089c0d1e18 ("vlog: abstract out interface to syslog daemon")
> > > > introduced --syslog-method flag that supersedes --syslog-target flag
> by:
> > > > 1. making logging format configurable
> > > > 2. letting daemon to also talk over UNIX domain socket (this is handy
> > > >    when local rsyslog daemon is running in different network
> namespace
> > > >    on the same host)
> > > >
> > > > Signed-off-by: Ansis Atteka <aatteka@nicira.com>
> > > > ---
> > > >  NEWS       |  1 +
> > > >  lib/vlog.c | 10 ++++++++++
> > > >  2 files changed, 11 insertions(+)
> > > >
> > > > diff --git a/NEWS b/NEWS
> > > > index ca22c8e..8bdaf3e 100644
> > > > --- a/NEWS
> > > > +++ b/NEWS
> > > > @@ -21,6 +21,7 @@ Post-v2.4.0
> > > >       targets to run a new system testsuite.  These tests can be run
> > > inside
> > > >       a Vagrant box.  See INSTALL.md for details
> > > >     - Dropped support for GRE64 tunnel.
> > > > +   - Mark --syslog-target argument as deprecated.
> > >
> > > In the past when we've deprecated features we've also given a date
> after
> > > which we might remove the feature.  It's usually 6 to 12 months out.
> Do
> > > you want to do that here?
> > >
> >
> > > Acked-by: Ben Pfaff <blp@nicira.com>
> > >
> > I will mark this as "to be removed in 6 months" and push.
>
> Can you give a specific date (e.g. "March 2016") so that users don't
> have to guess what the start date is?
>
Instead I put "will be removed in the next release" since my personal
opinion is that this feature is really insignificant and can be removed
sooner rather than later.



> > Do you think that we should have a sentence or two in CONTRIBUTING.md
> file
> > about "feature deprecation guidelines"? If so I can try to write this up.
>
> That sounds like a good idea to me.
>
I sent out proposed changes to CONTRIBUTING.md.

I agree that "in 6 months" is not clearly interpretable. However, even with
absolute times there could be some confusion if it takes for us very long
time to deliver release (just like 2.4.). In this case if current release
spans over March 2016 then it would not be clear to users when they should
be prepared for the feature to be removed.
Ben Pfaff Sept. 29, 2015, 11:49 p.m. UTC | #5
On Sat, Sep 19, 2015 at 02:14:45PM -0700, Ansis Atteka wrote:
> On 19 September 2015 at 09:37, Ben Pfaff <blp@nicira.com> wrote:
> 
> > On Fri, Sep 18, 2015 at 04:09:48PM -0700, Ansis Atteka wrote:
> > > On 18 September 2015 at 15:35, Ben Pfaff <blp@nicira.com> wrote:
> > >
> > > > On Wed, Sep 16, 2015 at 07:29:30PM -0700, Ansis Atteka wrote:
> > > > > Commit fe089c0d1e18 ("vlog: abstract out interface to syslog daemon")
> > > > > introduced --syslog-method flag that supersedes --syslog-target flag
> > by:
> > > > > 1. making logging format configurable
> > > > > 2. letting daemon to also talk over UNIX domain socket (this is handy
> > > > >    when local rsyslog daemon is running in different network
> > namespace
> > > > >    on the same host)
> > > > >
> > > > > Signed-off-by: Ansis Atteka <aatteka@nicira.com>
> > > > > ---
> > > > >  NEWS       |  1 +
> > > > >  lib/vlog.c | 10 ++++++++++
> > > > >  2 files changed, 11 insertions(+)
> > > > >
> > > > > diff --git a/NEWS b/NEWS
> > > > > index ca22c8e..8bdaf3e 100644
> > > > > --- a/NEWS
> > > > > +++ b/NEWS
> > > > > @@ -21,6 +21,7 @@ Post-v2.4.0
> > > > >       targets to run a new system testsuite.  These tests can be run
> > > > inside
> > > > >       a Vagrant box.  See INSTALL.md for details
> > > > >     - Dropped support for GRE64 tunnel.
> > > > > +   - Mark --syslog-target argument as deprecated.
> > > >
> > > > In the past when we've deprecated features we've also given a date
> > after
> > > > which we might remove the feature.  It's usually 6 to 12 months out.
> > Do
> > > > you want to do that here?
> > > >
> > >
> > > > Acked-by: Ben Pfaff <blp@nicira.com>
> > > >
> > > I will mark this as "to be removed in 6 months" and push.
> >
> > Can you give a specific date (e.g. "March 2016") so that users don't
> > have to guess what the start date is?
> >
> Instead I put "will be removed in the next release" since my personal
> opinion is that this feature is really insignificant and can be removed
> sooner rather than later.
> 
> 
> 
> > > Do you think that we should have a sentence or two in CONTRIBUTING.md
> > file
> > > about "feature deprecation guidelines"? If so I can try to write this up.
> >
> > That sounds like a good idea to me.
> >
> I sent out proposed changes to CONTRIBUTING.md.
> 
> I agree that "in 6 months" is not clearly interpretable. However, even with
> absolute times there could be some confusion if it takes for us very long
> time to deliver release (just like 2.4.). In this case if current release
> spans over March 2016 then it would not be clear to users when they should
> be prepared for the feature to be removed.

It sounds like you've thought this through.  I'm happy with it.
Ansis Atteka Sept. 30, 2015, 12:33 a.m. UTC | #6
Thanks for review, I pushed this.

On Tue, Sep 29, 2015 at 4:49 PM, Ben Pfaff <blp@nicira.com> wrote:
> On Sat, Sep 19, 2015 at 02:14:45PM -0700, Ansis Atteka wrote:
>> On 19 September 2015 at 09:37, Ben Pfaff <blp@nicira.com> wrote:
>>
>> > On Fri, Sep 18, 2015 at 04:09:48PM -0700, Ansis Atteka wrote:
>> > > On 18 September 2015 at 15:35, Ben Pfaff <blp@nicira.com> wrote:
>> > >
>> > > > On Wed, Sep 16, 2015 at 07:29:30PM -0700, Ansis Atteka wrote:
>> > > > > Commit fe089c0d1e18 ("vlog: abstract out interface to syslog daemon")
>> > > > > introduced --syslog-method flag that supersedes --syslog-target flag
>> > by:
>> > > > > 1. making logging format configurable
>> > > > > 2. letting daemon to also talk over UNIX domain socket (this is handy
>> > > > >    when local rsyslog daemon is running in different network
>> > namespace
>> > > > >    on the same host)
>> > > > >
>> > > > > Signed-off-by: Ansis Atteka <aatteka@nicira.com>
>> > > > > ---
>> > > > >  NEWS       |  1 +
>> > > > >  lib/vlog.c | 10 ++++++++++
>> > > > >  2 files changed, 11 insertions(+)
>> > > > >
>> > > > > diff --git a/NEWS b/NEWS
>> > > > > index ca22c8e..8bdaf3e 100644
>> > > > > --- a/NEWS
>> > > > > +++ b/NEWS
>> > > > > @@ -21,6 +21,7 @@ Post-v2.4.0
>> > > > >       targets to run a new system testsuite.  These tests can be run
>> > > > inside
>> > > > >       a Vagrant box.  See INSTALL.md for details
>> > > > >     - Dropped support for GRE64 tunnel.
>> > > > > +   - Mark --syslog-target argument as deprecated.
>> > > >
>> > > > In the past when we've deprecated features we've also given a date
>> > after
>> > > > which we might remove the feature.  It's usually 6 to 12 months out.
>> > Do
>> > > > you want to do that here?
>> > > >
>> > >
>> > > > Acked-by: Ben Pfaff <blp@nicira.com>
>> > > >
>> > > I will mark this as "to be removed in 6 months" and push.
>> >
>> > Can you give a specific date (e.g. "March 2016") so that users don't
>> > have to guess what the start date is?
>> >
>> Instead I put "will be removed in the next release" since my personal
>> opinion is that this feature is really insignificant and can be removed
>> sooner rather than later.
>>
>>
>>
>> > > Do you think that we should have a sentence or two in CONTRIBUTING.md
>> > file
>> > > about "feature deprecation guidelines"? If so I can try to write this up.
>> >
>> > That sounds like a good idea to me.
>> >
>> I sent out proposed changes to CONTRIBUTING.md.
>>
>> I agree that "in 6 months" is not clearly interpretable. However, even with
>> absolute times there could be some confusion if it takes for us very long
>> time to deliver release (just like 2.4.). In this case if current release
>> spans over March 2016 then it would not be clear to users when they should
>> be prepared for the feature to be removed.
>
> It sounds like you've thought this through.  I'm happy with it.

Patch
diff mbox

diff --git a/NEWS b/NEWS
index ca22c8e..8bdaf3e 100644
--- a/NEWS
+++ b/NEWS
@@ -21,6 +21,7 @@  Post-v2.4.0
      targets to run a new system testsuite.  These tests can be run inside
      a Vagrant box.  See INSTALL.md for details
    - Dropped support for GRE64 tunnel.
+   - Mark --syslog-target argument as deprecated.
 
 
 v2.4.0 - 20 Aug 2015
diff --git a/lib/vlog.c b/lib/vlog.c
index 29ba620..da31e6f 100644
--- a/lib/vlog.c
+++ b/lib/vlog.c
@@ -706,6 +706,7 @@  vlog_init(void)
     if (ovsthread_once_start(&once)) {
         long long int now;
         int facility;
+        bool print_syslog_target_deprecation;
 
         /* Do initialization work that needs to be done before any logging
          * occurs.  We want to keep this really minimal because any attempt to
@@ -740,6 +741,15 @@  vlog_init(void)
                                  0, INT_MAX, vlog_disable_rate_limit, NULL);
         unixctl_command_register("vlog/reopen", "", 0, 0,
                                  vlog_unixctl_reopen, NULL);
+
+        ovs_rwlock_rdlock(&pattern_rwlock);
+        print_syslog_target_deprecation = syslog_fd >= 0;
+        ovs_rwlock_unlock(&pattern_rwlock);
+
+        if (print_syslog_target_deprecation) {
+            VLOG_WARN("--syslog-target flag is deprecated, use "
+                      "--syslog-method instead");
+        }
     }
 }