Skip to content

Commit

Permalink
Use GLib logging framework instead of custom code
Browse files Browse the repository at this point in the history
The idea here is to use a common logging framework that is available for us in a
library we already use in many other places -- GLib. This means:

* We can get rid of quite a big chunk of a custom logging code which was just
  blindly printing things to STDOUT and logging via the old syslog() API. Plus
  all the logic for setting colors, bold fonts, etc. All this happening even if
  debug logging was disabled with the '--no-debug' option in which case all
  would have been done in vain because the output was in the end written to
  '/dev/null'.

* We can use GLib's function(s) for structured logging (IOW, journal). This
  means that our (system) logs should be easier to work with as we can split
  what used to be a single string/message into fields with some semantic meaning
  (the major one still being the logged MESSAGE, of course).

* Our custom colors are gone. But worry no longer if you become addicted to
  them! GLib logging uses colors too when they detect that they have some usable
  STDOUT/STDERR. So it's still easy to spot the serious in the output.

* Messages that used to be ERROR are now CRITICAL because logging an ERROR also
  means calling abort() with GLib logging. That's quite unfortunate, I think,
  but at the same time not a big deal, really. Turns out we actually should exit
  early in some places where we were just logging ERROR messages, but for that
  we need to change our clean-up code to be registered as an atexit() handler so
  that we can just terminate and rely on the clean-up to be executed
  automatically.

* NOTICE messages are now MESSAGE because GLib logging doesn't know anything
  like a NOTICE log level (don't know why as it's a syslog level). Again, no big
  deal as we can keep our udisks_notice() macro and just use a different level
  for it. Plus a new udisks_message() macro for people used to GLib logging and
  it's MESSAGE level.

* DEBUG and INFO messages are discarded by default. But see the follow up commit
  that deals with this.
  • Loading branch information
vpodzime committed Nov 28, 2016
1 parent 3e422b7 commit ad2ce67
Show file tree
Hide file tree
Showing 17 changed files with 70 additions and 200 deletions.
4 changes: 2 additions & 2 deletions modules/bcache/udiskslinuxblockbcache.c
Expand Up @@ -160,7 +160,7 @@ udisks_linux_block_bcache_get_daemon (UDisksLinuxBlockBcache *block)
}
else
{
udisks_error ("%s", error->message);
udisks_critical ("%s", error->message);
g_clear_error (&error);
}

Expand Down Expand Up @@ -197,7 +197,7 @@ udisks_linux_block_bcache_update (UDisksLinuxBlockBcache *block,
mode = bd_kbd_bcache_get_mode_str(bd_kbd_bcache_get_mode(dev_file, &error), &error);
if (! stats || ! mode)
{
udisks_error ("Can't get Bcache block device info for %s", dev_file);
udisks_critical ("Can't get Bcache block device info for %s", dev_file);
rval = FALSE;
goto out;
}
Expand Down
4 changes: 2 additions & 2 deletions modules/btrfs/udiskslinuxfilesystembtrfs.c
Expand Up @@ -203,7 +203,7 @@ udisks_linux_filesystem_btrfs_get_daemon (UDisksLinuxFilesystemBTRFS *l_fs_btrfs
}
else
{
udisks_error ("%s", error->message);
udisks_critical ("%s", error->message);
g_clear_error (&error);
}

Expand Down Expand Up @@ -243,7 +243,7 @@ udisks_linux_filesystem_btrfs_update (UDisksLinuxFilesystemBTRFS *l_fs_btrfs,

if (! btrfs_info)
{
udisks_error ("Can't get BTRFS filesystem info for %s", dev_file);
udisks_critical ("Can't get BTRFS filesystem info for %s", dev_file);
rval = FALSE;
goto out;
}
Expand Down
2 changes: 1 addition & 1 deletion modules/iscsi/udiskslinuxiscsisessionobject.c
Expand Up @@ -369,7 +369,7 @@ udisks_linux_iscsi_session_object_update_iface (UDisksLinuxISCSISessionObject *s
/* Get session info */
if (libiscsi_get_session_info_by_id (ctx, &session_info, session_object->session_id) != 0)
{
udisks_error ("Can not retrieve session information for %s", session_object->session_id);
udisks_critical ("Can not retrieve session information for %s", session_object->session_id);
return;
}

Expand Down
4 changes: 2 additions & 2 deletions modules/zram/udiskslinuxblockzram.c
Expand Up @@ -161,7 +161,7 @@ udisks_linux_block_zram_get_daemon (UDisksLinuxBlockZRAM *zramblock)
}
else
{
udisks_error ("%s", error->message);
udisks_critical ("%s", error->message);
g_clear_error (&error);
}

Expand Down Expand Up @@ -197,7 +197,7 @@ udisks_linux_block_zram_update (UDisksLinuxBlockZRAM *zramblock,

if (! zram_info)
{
udisks_error ("Can't get ZRAM block device info for %s", dev_file);
udisks_critical ("Can't get ZRAM block device info for %s", dev_file);
rval = FALSE;
goto out;
}
Expand Down
2 changes: 1 addition & 1 deletion src/main.c
Expand Up @@ -69,7 +69,7 @@ on_name_lost (GDBusConnection *connection,
{
if (the_daemon == NULL)
{
udisks_error ("Failed to connect to the system message bus");
udisks_critical ("Failed to connect to the system message bus");
}
else
{
Expand Down
2 changes: 1 addition & 1 deletion src/udiskscrypttabmonitor.c
Expand Up @@ -281,7 +281,7 @@ udisks_crypttab_monitor_constructed (GObject *object)
&error);
if (monitor->file_monitor == NULL)
{
udisks_error ("Error monitoring /etc/crypttab: %s (%s, %d)",
udisks_critical ("Error monitoring /etc/crypttab: %s (%s, %d)",
error->message, g_quark_to_string (error->domain), error->code);
g_clear_error (&error);
}
Expand Down
6 changes: 3 additions & 3 deletions src/udisksdaemon.c
Expand Up @@ -248,7 +248,7 @@ udisks_daemon_constructed (GObject *object)
daemon->authority = polkit_authority_get_sync (NULL, &error);
if (daemon->authority == NULL)
{
udisks_error ("Error initializing polkit authority: %s (%s, %d)",
udisks_critical ("Error initializing polkit authority: %s (%s, %d)",
error->message, g_quark_to_string (error->domain), error->code);
g_clear_error (&error);
}
Expand All @@ -259,15 +259,15 @@ udisks_daemon_constructed (GObject *object)
{
if (g_mkdir_with_parents ("/run/udisks2", 0700) != 0)
{
udisks_error ("Error creating directory %s: %m", "/run/udisks2");
udisks_critical ("Error creating directory %s: %m", "/run/udisks2");
}
}

if (!g_file_test (PACKAGE_LOCALSTATE_DIR "/lib/udisks2", G_FILE_TEST_IS_DIR))
{
if (g_mkdir_with_parents (PACKAGE_LOCALSTATE_DIR "/lib/udisks2", 0700) != 0)
{
udisks_error ("Error creating directory %s: %m", PACKAGE_LOCALSTATE_DIR "/lib/udisks2");
udisks_critical ("Error creating directory %s: %m", PACKAGE_LOCALSTATE_DIR "/lib/udisks2");
}
}

Expand Down
11 changes: 6 additions & 5 deletions src/udisksdaemontypes.h
Expand Up @@ -167,11 +167,12 @@ typedef enum
*/
typedef enum
{
UDISKS_LOG_LEVEL_DEBUG,
UDISKS_LOG_LEVEL_INFO,
UDISKS_LOG_LEVEL_NOTICE,
UDISKS_LOG_LEVEL_WARNING,
UDISKS_LOG_LEVEL_ERROR
UDISKS_LOG_LEVEL_DEBUG = G_LOG_LEVEL_DEBUG,
UDISKS_LOG_LEVEL_INFO = G_LOG_LEVEL_INFO,
UDISKS_LOG_LEVEL_MESSAGE = G_LOG_LEVEL_MESSAGE,
UDISKS_LOG_LEVEL_WARNING = G_LOG_LEVEL_WARNING,
UDISKS_LOG_LEVEL_CRITICAL = G_LOG_LEVEL_CRITICAL,
UDISKS_LOG_LEVEL_ERROR = G_LOG_LEVEL_ERROR
} UDisksLogLevel;

struct _UDisksAtaCommandOutput;
Expand Down
8 changes: 4 additions & 4 deletions src/udisksdaemonutil.c
Expand Up @@ -1360,7 +1360,7 @@ udisks_daemon_util_inhibit_system_sync (const gchar *reason)
connection = g_bus_get_sync (G_BUS_TYPE_SYSTEM, NULL, &error);
if (connection == NULL)
{
udisks_error ("Error getting system bus: %s (%s, %d)",
udisks_critical ("Error getting system bus: %s (%s, %d)",
error->message, g_quark_to_string (error->domain), error->code);
g_clear_error (&error);
goto out;
Expand All @@ -1385,7 +1385,7 @@ udisks_daemon_util_inhibit_system_sync (const gchar *reason)
&error);
if (value == NULL)
{
udisks_error ("Error inhibiting: %s (%s, %d)",
udisks_critical ("Error inhibiting: %s (%s, %d)",
error->message, g_quark_to_string (error->domain), error->code);
g_clear_error (&error);
goto out;
Expand All @@ -1399,7 +1399,7 @@ udisks_daemon_util_inhibit_system_sync (const gchar *reason)
ret->fd = g_unix_fd_list_get (fd_list, index, &error);
if (ret->fd == -1)
{
udisks_error ("Error getting fd: %s (%s, %d)",
udisks_critical ("Error getting fd: %s (%s, %d)",
error->message, g_quark_to_string (error->domain), error->code);
g_clear_error (&error);
g_free (ret);
Expand Down Expand Up @@ -1435,7 +1435,7 @@ udisks_daemon_util_uninhibit_system_sync (UDisksInhibitCookie *cookie)
g_assert (cookie->magic == 0xdeadbeef);
if (close (cookie->fd) != 0)
{
udisks_error ("Error closing inhbit-fd: %m");
udisks_critical ("Error closing inhbit-fd: %m");
}
g_free (cookie);
}
Expand Down
2 changes: 1 addition & 1 deletion src/udisksfstabmonitor.c
Expand Up @@ -282,7 +282,7 @@ udisks_fstab_monitor_constructed (GObject *object)
&error);
if (monitor->file_monitor == NULL)
{
udisks_error ("Error monitoring /etc/fstab: %s (%s, %d)",
udisks_critical ("Error monitoring /etc/fstab: %s (%s, %d)",
error->message, g_quark_to_string (error->domain), error->code);
g_clear_error (&error);
}
Expand Down
2 changes: 1 addition & 1 deletion src/udiskslinuxdevice.c
Expand Up @@ -127,7 +127,7 @@ udisks_linux_device_new_sync (GUdevDevice *udev_device)
out:
if (error != NULL)
{
udisks_error ("Error probing device: %s (%s, %d)",
udisks_critical ("Error probing device: %s (%s, %d)",
error->message, g_quark_to_string (error->domain), error->code);
g_clear_error (&error);
}
Expand Down
6 changes: 3 additions & 3 deletions src/udiskslinuxdrive.c
Expand Up @@ -268,7 +268,7 @@ update_configuration (UDisksLinuxDrive *drive,
{
if (!g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_NOENT))
{
udisks_error ("Error loading drive config file: %s (%s, %d)",
udisks_critical ("Error loading drive config file: %s (%s, %d)",
error->message, g_quark_to_string (error->domain), error->code);
}
g_clear_error (&error);
Expand All @@ -288,7 +288,7 @@ update_configuration (UDisksLinuxDrive *drive,
gint32 int_value = g_key_file_get_integer (key_file, mapping->group, mapping->key, &error);
if (error != NULL)
{
udisks_error ("Error parsing int32 key %s in group %s in drive config file %s: %s (%s, %d)",
udisks_critical ("Error parsing int32 key %s in group %s in drive config file %s: %s (%s, %d)",
mapping->key, mapping->group, path,
error->message, g_quark_to_string (error->domain), error->code);
g_clear_error (&error);
Expand All @@ -303,7 +303,7 @@ update_configuration (UDisksLinuxDrive *drive,
gboolean bool_value = g_key_file_get_boolean (key_file, mapping->group, mapping->key, &error);
if (error != NULL)
{
udisks_error ("Error parsing boolean key %s in group %s in drive config file %s: %s (%s, %d)",
udisks_critical ("Error parsing boolean key %s in group %s in drive config file %s: %s (%s, %d)",
mapping->key, mapping->group, path,
error->message, g_quark_to_string (error->domain), error->code);
g_clear_error (&error);
Expand Down
14 changes: 7 additions & 7 deletions src/udiskslinuxdriveata.c
Expand Up @@ -1677,7 +1677,7 @@ apply_configuration_thread_func (gpointer user_data)
fd = open (device_file, O_RDWR|O_NONBLOCK);
if (fd == -1)
{
udisks_error ("Error opening device file %s: %m", device_file);
udisks_critical ("Error opening device file %s: %m", device_file);
goto out;
}

Expand All @@ -1693,7 +1693,7 @@ apply_configuration_thread_func (gpointer user_data)
&output,
&error))
{
udisks_error ("Error sending ATA command IDLE (timeout=%d) to %s: %s (%s, %d)",
udisks_critical ("Error sending ATA command IDLE (timeout=%d) to %s: %s (%s, %d)",
data->ata_pm_standby, device_file,
error->message, g_quark_to_string (error->domain), error->code);
g_clear_error (&error);
Expand Down Expand Up @@ -1727,7 +1727,7 @@ apply_configuration_thread_func (gpointer user_data)
&output,
&error))
{
udisks_error ("Error sending ATA command SET FEATURES, sub-command 0x%02x (ata_apm_level=%d) to %s: %s (%s, %d)",
udisks_critical ("Error sending ATA command SET FEATURES, sub-command 0x%02x (ata_apm_level=%d) to %s: %s (%s, %d)",
(guint) input.feature, data->ata_apm_level, device_file,
error->message, g_quark_to_string (error->domain), error->code);
g_clear_error (&error);
Expand Down Expand Up @@ -1758,7 +1758,7 @@ apply_configuration_thread_func (gpointer user_data)
&output,
&error))
{
udisks_error ("Error sending ATA command SET FEATURES, sub-command 0x%02x (ata_aam_level=%d) to %s: %s (%s, %d)",
udisks_critical ("Error sending ATA command SET FEATURES, sub-command 0x%02x (ata_aam_level=%d) to %s: %s (%s, %d)",
(guint) input.feature, data->ata_aam_level, device_file,
error->message, g_quark_to_string (error->domain), error->code);
g_clear_error (&error);
Expand Down Expand Up @@ -1786,7 +1786,7 @@ apply_configuration_thread_func (gpointer user_data)
&output,
&error))
{
udisks_error ("Error sending ATA command SET FEATURES, sub-command 0x%02x to %s: %s (%s, %d)",
udisks_critical ("Error sending ATA command SET FEATURES, sub-command 0x%02x to %s: %s (%s, %d)",
(guint) input.feature, device_file,
error->message, g_quark_to_string (error->domain), error->code);
g_clear_error (&error);
Expand Down Expand Up @@ -1815,7 +1815,7 @@ apply_configuration_thread_func (gpointer user_data)
&output,
&error))
{
udisks_error ("Error sending ATA command SET FEATURES, sub-command 0x%02x to %s: %s (%s, %d)",
udisks_critical ("Error sending ATA command SET FEATURES, sub-command 0x%02x to %s: %s (%s, %d)",
(guint) input.feature, device_file,
error->message, g_quark_to_string (error->domain), error->code);
g_clear_error (&error);
Expand Down Expand Up @@ -2209,7 +2209,7 @@ udisks_linux_drive_ata_secure_erase_sync (UDisksLinuxDriveAta *drive,
&output,
&cleanup_error))
{
udisks_error ("Failed to clear user password '%s' on %s (%s) while attemping clean-up after a failed secure erase operation. You may need to manually unlock the drive. The error was: %s (%s, %d)",
udisks_critical ("Failed to clear user password '%s' on %s (%s) while attemping clean-up after a failed secure erase operation. You may need to manually unlock the drive. The error was: %s (%s, %d)",
pass,
device_file,
udisks_drive_get_id (_drive),
Expand Down

0 comments on commit ad2ce67

Please sign in to comment.