On Wed, Jan 27, 2016 at 04:57:41PM -0500, Russell Bryant wrote:
> On 12/08/2015 01:59 PM, Ben Pfaff wrote:
> > Requested-by: P R Dinesh
> > Requested-at: https://github.com/openvswitch/ovs/pull/94
> > Signed-off-by: Ben Pfaff <b...@ovn.org>
>
> Acked-by: Russell Bryant <russ...@ovn.org>
Thanks! I'm going to send a revision of this as part of a longer
series.
> > static void
> > +vlog_unixctl_close(struct unixctl_conn *conn, int argc OVS_UNUSED,
> > + const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
> > +{
> > + ovs_mutex_lock(&log_file_mutex);
> > + if (log_fd >= 0) {
> > + close(log_fd);
> > + log_fd = -1;
> > +
> > + async_append_destroy(log_writer);
> > + log_writer = NULL;
> > +
> > + struct vlog_module *mp;
> > + LIST_FOR_EACH (mp, list, &vlog_modules) {
> > + update_min_level(mp);
> > + }
> > + }
> > + ovs_mutex_unlock(&log_file_mutex);
>
> This got me looking around at where log_file_mutex is used. One thing I
> noticed was that it seems like it should be locked in
> vlog_insert_module(). In pratice it doesn't matter since it's only
> called in global constructors at startup. Maybe it makes sense in case
> someone decides to call vlog_insert_module() dynamically for some reason.
>
> If we do that, maybe the lock should be held in vlog_module_from_name(),
> as well.
>
> Again, not a real issue today, but I'm curious what you think ...
Fair enough. I'll send a separate patch.
> The two tests are *very* similar. It'd be nice to share most of this as
> is done in other parts of the test suit, but it's not that big of a
> deal. The minor differences might make it not worth the trouble.
At the moment I don't see much benefit, so I'm inclined to leave it.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
