Add --driver-block and --action-block arguments

These will set the environment variables accordingly to block drivers
or actions from loading.

README.md

@@ -179,8 +179,8 @@ in one of two ways:
 
 1. Adding `amdgpu.abmlevel=0` to the kernel command line.  This will disable abm
    value changes entirely.
-2. By using `POWER_PROFILE_DAEMON_ACTION_BLOCK=amdgpu_panel_power` in the
-   `power-profiles-daemon` environment as described below. This will allow you to
+2. By using `--block-action=amdgpu_panel_power` in the
+   `power-profiles-daemon` `ExecStart` command. This will allow you to
    still change values manually in sysfs but `power-profiles-daemon` will not
    change anything.
 
@@ -188,8 +188,7 @@ in one of two ways:
 
 Power-profiles daemon will load all supported drivers and actions by default.
 If you have a problem with a given driver or action, you can disable it by
-populating the `POWER_PROFILE_DAEMON_DRIVER_BLOCK` or `POWER_PROFILE_DAEMON_ACTION_BLOCK`
-environment variables with the name of the driver or action you want to disable
+using `--block-action` or `--block-driver` with the name of the driver or action you want to disable
 in the environment that launches the daemon (such as the systemd unit file).
 
 For example to edit the unit:
@@ -198,12 +197,11 @@ For example to edit the unit:
 sudo systemctl edit power-profiles-daemon.service
 ```
 
-Then add to the drop-in file:
+Then modify the Execstart line to the drop-in file:
 
 ```text
 [Service]
-Environment=POWER_PROFILE_DAEMON_DRIVER_BLOCK=xxx
-Environment=POWER_PROFILE_DAEMON_ACTION_BLOCK=yyy
+ExecStart=/usr/libexec/power-profiles-daemon --block-action=FOO
 ```
 
 Then restart the service:

src/power-profiles-daemon.c

@@ -54,6 +54,8 @@ typedef struct {
   GOptionGroup *group;
   GLogLevelFlags log_level;
   gboolean replace;
+  GStrv blocked_drivers;
+  GStrv blocked_actions;
 } DebugOptions;
 
 typedef struct {
@@ -84,6 +86,9 @@ typedef struct {
   PpdPowerChangedReason power_changed_reason;
 
   guint logind_sleep_signal_id;
+
+  GStrv blocked_drivers;
+  GStrv blocked_actions;
 } PpdApp;
 
 typedef struct {
@@ -97,9 +102,11 @@ typedef struct {
 static void
 debug_options_free(DebugOptions *options)
 {
-  g_option_group_unref (options->group);
+  g_strfreev (options->blocked_drivers);
+  g_strfreev (options->blocked_actions);
   g_free (options);
 }
+G_DEFINE_AUTOPTR_CLEANUP_FUNC (DebugOptions, debug_options_free)
 
 static void
 profile_hold_free (ProfileHold *hold)
@@ -1250,32 +1257,34 @@ stop_profile_drivers (PpdApp *data)
 }
 
 static gboolean
-action_blocked (PpdAction *action)
+action_blocked (PpdApp *app, PpdAction *action)
 {
   const gchar *action_name = ppd_action_get_action_name (action);
-  const gchar *env = g_getenv ("POWER_PROFILE_DAEMON_ACTION_BLOCK");
+  gboolean blocked;
 
-  if (env == NULL)
+  if (app->blocked_actions == NULL || g_strv_length (app->blocked_actions) == 0)
     return FALSE;
 
-  g_auto(GStrv) actions = NULL;
+  blocked = g_strv_contains ((const gchar *const *) app->blocked_actions, action_name);
 
-  actions = g_strsplit (env, ",", -1);
-  return g_strv_contains ((const gchar *const *)actions, action_name);
+  if (blocked)
+    g_debug ("Action '%s' is blocked", action_name);
+  return blocked;
 }
 
 static gboolean
-driver_blocked (PpdDriver *driver)
+driver_blocked (PpdApp *app, PpdDriver *driver)
 {
   const gchar *driver_name = ppd_driver_get_driver_name (driver);
-  const gchar *env = g_getenv ("POWER_PROFILE_DAEMON_DRIVER_BLOCK");
+  gboolean blocked;
 
-  if (env == NULL)
+  if (app->blocked_drivers == NULL || g_strv_length (app->blocked_drivers) == 0)
     return FALSE;
 
-  g_auto(GStrv) drivers = NULL;
-  drivers = g_strsplit (env, ",", -1);
-  return g_strv_contains ((const char **) drivers, driver_name);
+  blocked = g_strv_contains ((const gchar *const *) app->blocked_drivers, driver_name);
+  if (blocked)
+    g_debug ("Driver '%s' is blocked", driver_name);
+  return blocked;
 }
 
 static void
@@ -1299,7 +1308,7 @@ start_profile_drivers (PpdApp *data)
       PpdProbeResult result;
 
       g_debug ("Handling driver '%s'", ppd_driver_get_driver_name (driver));
-      if (driver_blocked (driver)) {
+      if (driver_blocked (data, driver)) {
         g_debug ("Driver '%s' is blocked, skipping", ppd_driver_get_driver_name (driver));
         continue;
       }
@@ -1366,7 +1375,7 @@ start_profile_drivers (PpdApp *data)
 
       g_debug ("Handling action '%s'", ppd_action_get_action_name (action));
 
-      if (action_blocked (action)) {
+      if (action_blocked (data, action)) {
         g_debug ("Action '%s' is blocked, skipping", ppd_action_get_action_name (action));
         continue;
       }
@@ -1523,6 +1532,8 @@ free_app_data (PpdApp *data)
   g_clear_handle_id (&data->name_id, g_bus_unown_name);
   g_clear_handle_id (&data->legacy_name_id, g_bus_unown_name);
 
+  g_strfreev (data->blocked_drivers);
+  g_strfreev (data->blocked_actions);
   g_clear_pointer (&data->config_path, g_free);
   g_clear_pointer (&data->config, g_key_file_unref);
   g_clear_pointer (&data->probed_drivers, g_ptr_array_unref);
@@ -1653,6 +1664,24 @@ debug_pre_parse_hook (GOptionContext *context,
       "Replace the running instance of power-profiles-daemon",
       NULL,
     },
+    {
+      "block-driver",
+      0,
+      G_OPTION_FLAG_NONE,
+      G_OPTION_ARG_STRING_ARRAY,
+      &data->blocked_drivers,
+      "Block driver(s) from loading",
+      NULL,
+    },
+    {
+      "block-action",
+      0,
+      G_OPTION_FLAG_NONE,
+      G_OPTION_ARG_STRING_ARRAY,
+      &data->blocked_actions,
+      "Block action(s) from loading",
+      NULL,
+    },
     { NULL }
   };
   g_option_group_add_entries (group, options);
@@ -1675,7 +1704,7 @@ debug_post_parse_hook (GOptionContext *context,
 
 int main (int argc, char **argv)
 {
-  DebugOptions *debug_options = g_new0 (DebugOptions, 1);
+  g_autoptr(DebugOptions) debug_options = g_new0 (DebugOptions, 1);
   g_autoptr(PpdApp) data = NULL;
   g_autoptr(GOptionContext) option_context = NULL;
   g_autoptr(GError) error = NULL;
@@ -1685,7 +1714,7 @@ int main (int argc, char **argv)
                                              "Debugging Options",
                                              "Show debugging options",
                                              debug_options,
-                                             (GDestroyNotify)debug_options_free);
+                                             NULL);
   g_option_group_set_parse_hooks (debug_options->group,
                                   debug_pre_parse_hook,
                                   debug_post_parse_hook);
@@ -1707,6 +1736,8 @@ int main (int argc, char **argv)
   data->profile_holds = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, (GDestroyNotify) profile_hold_free);
   data->active_profile = PPD_PROFILE_BALANCED;
   data->selected_profile = PPD_PROFILE_BALANCED;
+  data->blocked_drivers = g_steal_pointer (&debug_options->blocked_drivers);
+  data->blocked_actions = g_steal_pointer (&debug_options->blocked_actions);
 
   g_unix_signal_add (SIGTERM, quit_signal_callback, data);
   g_unix_signal_add (SIGINT, quit_signal_callback, data);

tests/integration_test.py

@@ -164,7 +164,7 @@ class Tests(dbusmock.DBusTestCase):
     # Daemon control and D-BUS I/O
     #
 
-    def start_daemon(self):
+    def start_daemon(self, args=None):
         """Start daemon and create DBus proxy.
 
         When done, this sets self.proxy as the Gio.DBusProxy for power-profiles-daemon.
@@ -178,6 +178,8 @@ class Tests(dbusmock.DBusTestCase):
         env["LD_PRELOAD"] = os.getenv("PPD_LD_PRELOAD") + " " + os.getenv("LD_PRELOAD")
         self.log = tempfile.NamedTemporaryFile()  # pylint: disable=consider-using-with
         daemon_path = [self.daemon_path, "-vv"]
+        if args:
+            daemon_path += args
         if os.getenv("PPD_TEST_WRAPPER"):
             daemon_path = os.getenv("PPD_TEST_WRAPPER").split(" ") + daemon_path
         elif os.getenv("VALGRIND"):
@@ -908,8 +910,7 @@ class Tests(dbusmock.DBusTestCase):
         )
 
         # Block panel_power action
-        os.environ["POWER_PROFILE_DAEMON_ACTION_BLOCK"] = "amdgpu_panel_power"
-        self.start_daemon()
+        self.start_daemon(["--block-action", "amdgpu_panel_power"])
         self.assertNotIn("amdgpu_panel_power", self.get_dbus_property("Actions"))
 
     def test_driver_blocklist(self):
@@ -957,10 +958,8 @@ class Tests(dbusmock.DBusTestCase):
         self.write_file_contents(os.path.join(dir3, "pm_profile"), "1\n")
 
         # block platform profile
-        os.environ["POWER_PROFILE_DAEMON_DRIVER_BLOCK"] = "platform_profile"
-
+        self.start_daemon(["--block-driver", "platform_profile"])
         # Verify that only amd-pstate is loaded
-        self.start_daemon()
         profiles = self.get_dbus_property("Profiles")
         self.assertEqual(len(profiles), 3)
         self.assertEqual(profiles[0]["Driver"], "multiple")
@@ -970,10 +969,10 @@ class Tests(dbusmock.DBusTestCase):
         self.stop_daemon()
 
         # block both drivers
-        os.environ["POWER_PROFILE_DAEMON_DRIVER_BLOCK"] = "amd_pstate,platform_profile"
-
+        self.start_daemon(
+            ["--block-driver", "amd_pstate", "--block-driver", "platform_profile"]
+        )
         # Verify that only placeholder is loaded
-        self.start_daemon()
         profiles = self.get_dbus_property("Profiles")
         self.assertEqual(len(profiles), 2)
         self.assertEqual(profiles[0]["PlatformDriver"], "placeholder")