After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 756129 - [review] refactor scheduling activation stages in NMDevice [th/device-schedule-activation-bgo756129]
[review] refactor scheduling activation stages in NMDevice [th/device-schedul...
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2015-10-06 15:38 UTC by Thomas Haller
Modified: 2015-10-07 16:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Description Thomas Haller 2015-10-06 15:38:43 UTC 
please review
Comment 1 Beniamino Galvani 2015-10-07 07:17:27 UTC 
> device/logging: refactor logging for activation-stage

I find the old messages easier to recognize and understand while
analyzing logs; but either way is fine.

LGTM.
Comment 2 Thomas Haller 2015-10-07 09:23:15 UTC 
(In reply to Beniamino Galvani from comment #1)
> > device/logging: refactor logging for activation-stage
> 
> I find the old messages easier to recognize and understand while
> analyzing logs; but either way is fine.

if we want that, then the logging line can be adjusted to look  more like it used to be...

(I like the new format better, as I prefer having this idea of a lowercase "tag:", contrary to english text. Anyway... opinions?)
Comment 3 Thomas Haller 2015-10-07 13:05:35 UTC 
(In reply to Beniamino Galvani from comment #1)
> > device/logging: refactor logging for activation-stage
> 
> I find the old messages easier to recognize and understand while
> analyzing logs; but either way is fine.
> 
> LGTM.

I improved logging ~somewhat~ (yes??).

Now all "activation-stage:" lines will log the function name instead of the function pointer:

   _LOGD (LOGD_DEVICE, "activation-stage: clear %s,%d (id %u)",
          _activation_func_to_string (act_data->func), family, act_data->id);

also, I renamed the functions, so that they look nicer in the log.

  - nm_device_activate_stage1_device_prepare
  + activate_stage1_device_prepare

  - nm_device_activate_ip4_config_timeout
  + activate_stage4_ip4_config_timeout



So, the previous:

  "Activation: Stage 1 of 5 (Device Prepare) started..."

becomes:

  "activation-stage: invoke activate_stage1_device_prepare,2 (id x)"




Repushed
Comment 4 Lubomir Rintel 2015-10-07 15:03:38 UTC 
(In reply to Thomas Haller from comment #2)
> (In reply to Beniamino Galvani from comment #1)
> > > device/logging: refactor logging for activation-stage
> > 
> > I find the old messages easier to recognize and understand while
> > analyzing logs; but either way is fine.
> 
> if we want that, then the logging line can be adjusted to look  more like it
> used to be...
> 
> (I like the new format better, as I prefer having this idea of a lowercase
> "tag:", contrary to english text. Anyway... opinions?)

I tend to agree with Thomas that "tag:" looks a bit more convenient to read; but either way works for me.

Please rebase the branch; I guess there will be some conflict with changes that need to be reflected into activation_source_schedule().
Comment 5 Thomas Haller 2015-10-07 15:48:01 UTC 
Rebased