Re: [PATCH] scanning and logging patches in 'cli-enhance' branch

[Dan Williams <dcbw redhat com>]

On Mon, 2012-12-17 at 17:58 +0100, Jirka Klimes wrote:
> Hello,
> 
> There are some patches in cli-enhance' branch ready to cherry-pick to master:
> 
> 1) libnm-glib nm_device_wifi_request_scan_simple()
> http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?h=cli-
> enhance&id=8eba31f02dab7ff0dae89853ede719ea93f7ea05

To be 100% correct, we should store a pointer to the active
RequestScanInfo and deny further requests while a scan is already in
progress.  Since the NMDevice might be destroyed while the scan request
is in-progress, we'll crash in request_scan_cb() if we don't handle that
case.  Basically if the device gets destroyed cancel the DBusPendingCall
and set a "canceled" boolean in RequestScanInfo perhaps?

Other than that looks fine.

> 2) 5 patches for logging
> 
> * core: add GetLogging() D-Bus call to org.freedesktop.NetworkManager
> http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?h=cli-
> enhance&id=f7e0b556321ed90dde94397cc8db1e5b1b47a5da
> 
> * libnm-glib: add nm_client_get_logging() function
> http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?h=cli-
> enhance&id=f2103a0b77f6f73b3300b4ac95e94f48cc9799a9

Code docs for @domains should say it's a list of log domains separated
by ",".

> * libnm-glib: add nm_client_set_logging()
> http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?h=cli-
> enhance&id=534fb8e5468792802f217dfb42df0f95208bec79

The code docs for @domains here should say that the string should be a
list of log domains separated by "," or something like that.

> * logging: add 'DEFAULT' logging domain
> http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?h=cli-
> enhance&id=46d4fc43e369fb4e2d2152f7b79fcf7f38ab0b5f

Formatting in nm_logging_setup() is odd; should be } else if (...).

> * update logging domains in introspection
> http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?h=cli-
> enhance&id=321ee9ad27c1e36f8da4ed6cbbf66da2f708bf78

This one is fine.

Thanks!
Dan