[logger] Start using handler callback changing_config/3 in built-in handlers

The new parameter to this function, SetOrUpdate, indicates how
unspecified configuration data fields shall be set. The rule is that
if SetOrUpdate equals set, then default values shall be used, and if
SetOrUpdate equals update, then existing configuration values shall be
used. Consequently, these examples now apply to logger_std_h and
logger_disk_log_h:

set_handler_config(default, config, #{sync_mode_qlen => 20}) sets the
value of sync_mode_qlen to 20, and resets all other (writable) fields
in the config map to their default values.

update_handler_config(default, config, #{sync_mode_qlen => 20}) sets
the value of sync_mode_qlen to 20, and leaves all other fields in the
config map unchanged.

3 files changed, 133 insertions, 78 deletions

diff --git a/lib/kernel/src/logger_disk_log_h.erl b/lib/kernel/src/logger_disk_log_h.erl
index a8f141f135..bb8b9f08a4 100644
--- a/lib/kernel/src/logger_disk_log_h.erl
+++ b/lib/kernel/src/logger_disk_log_h.erl
@@ -33,7 +33,7 @@
          terminate/2, code_change/3]).
 
 %% logger callbacks
--export([log/2, adding_handler/1, removing_handler/1, changing_config/2]).
+-export([log/2, adding_handler/1, removing_handler/1, changing_config/3]).
 
 %% handler internal
 -export([log_handler_info/4]).
@@ -114,9 +114,8 @@ reset(Name) ->
 %%% Handler being added
 adding_handler(#{id:=Name}=Config) ->
     case check_config(adding, Config) of
-        {ok, Config1} ->
+        {ok, #{config:=HConfig}=Config1} ->
             %% create initial handler state by merging defaults with config
-            HConfig = maps:get(config, Config1, #{}),
             HState = maps:merge(get_init_state(), HConfig),
             case logger_h_common:overload_levels_ok(HState) of
                 true ->
@@ -133,32 +132,40 @@ adding_handler(#{id:=Name}=Config) ->
 
 %%%-----------------------------------------------------------------
 %%% Updating handler config
-changing_config(OldConfig = #{id:=Name, config:=OldHConfig},
-                NewConfig = #{id:=Name, config:=NewHConfig}) ->
-    #{type:=Type, file:=File, max_no_files:=MaxFs,
-      max_no_bytes:=MaxBytes} = OldHConfig,
-    case NewHConfig of
-        #{type:=Type, file:=File, max_no_files:=MaxFs,
-          max_no_bytes:=MaxBytes} ->
-            changing_config1(OldConfig, NewConfig);
-        _ ->
-            {error,{illegal_config_change,OldConfig,NewConfig}}
-    end;
-changing_config(OldConfig, NewConfig) ->
-    {error,{illegal_config_change,OldConfig,NewConfig}}.
+changing_config(SetOrUpdate,OldConfig=#{config:=OldHConfig},NewConfig) ->
+    WriteOnce = maps:with([type,file,max_no_files,max_no_bytes],OldHConfig),
+    ReadOnly = maps:with([handler_pid,mode_tab],OldHConfig),
+    NewHConfig0 = maps:get(config, NewConfig, #{}),
+    Default =
+        case SetOrUpdate of
+            set ->
+                %% Do not reset write-once fields to defaults
+                maps:merge(get_default_config(),WriteOnce);
+            update ->
+                OldHConfig
+        end,
+
+    %% Allow (accidentially) included read-only fields - just overwrite them
+    NewHConfig = maps:merge(maps:merge(Default,NewHConfig0),ReadOnly),
+
+    %% But fail if write-once fields are changed
+    case maps:with([type,file,max_no_files,max_no_bytes],NewHConfig) of
+        WriteOnce ->
+            changing_config1(maps:get(handler_pid,OldHConfig),
+                             OldConfig,
+                             NewConfig#{config=>NewHConfig});
+        Other ->
+            {Old,New} = logger_server:diff_maps(WriteOnce,Other),
+            {error,{illegal_config_change,#{config=>Old},#{config=>New}}}
+    end.
 
-changing_config1(OldConfig=#{config:=OldHConfig}, NewConfig) ->
+changing_config1(HPid, OldConfig, NewConfig) ->
     case check_config(changing, NewConfig) of
-        {ok,NewConfig1 = #{config:=NewHConfig}} ->
-            #{handler_pid:=HPid,
-              mode_tab:=ModeTab} = OldHConfig,
-            NewHConfig1 = NewHConfig#{handler_pid=>HPid,
-                                      mode_tab=>ModeTab},
-            NewConfig2 = NewConfig1#{config=>NewHConfig1},
-            try gen_server:call(HPid, {change_config,OldConfig,NewConfig2},
+        Result = {ok,NewConfig1} ->
+            try gen_server:call(HPid, {change_config,OldConfig,NewConfig1},
                                 ?DEFAULT_CALL_TIMEOUT) of
-                ok      -> {ok,NewConfig2};
-                HError  -> HError
+                ok      -> Result;
+                Error  -> Error
             catch
                 _:{timeout,_} -> {error,handler_busy}
             end;
@@ -168,10 +175,12 @@ changing_config1(OldConfig=#{config:=OldHConfig}, NewConfig) ->
 
 check_config(adding, #{id:=Name}=Config) ->
     %% merge handler specific config data
-    HConfig = merge_default_logopts(Name, maps:get(config, Config, #{})),
-    case check_h_config(maps:to_list(HConfig)) of
+    HConfig1 = maps:get(config, Config, #{}),
+    HConfig2 = maps:merge(get_default_config(), HConfig1),
+    HConfig3 = merge_default_logopts(Name, HConfig2),
+    case check_h_config(maps:to_list(HConfig3)) of
         ok ->
-            {ok,Config#{config=>HConfig}};
+            {ok,Config#{config=>HConfig3}};
         Error ->
             Error
     end;
@@ -438,7 +447,7 @@ code_change(_OldVsn, State, _Extra) ->
 
 %%%-----------------------------------------------------------------
 %%%
-get_init_state() ->
+get_default_config() ->
      #{sync_mode_qlen              => ?SYNC_MODE_QLEN,
        drop_mode_qlen              => ?DROP_MODE_QLEN,
        flush_qlen                  => ?FLUSH_QLEN,
@@ -449,10 +458,12 @@ get_init_state() ->
        overload_kill_qlen          => ?OVERLOAD_KILL_QLEN,
        overload_kill_mem_size      => ?OVERLOAD_KILL_MEM_SIZE,
        overload_kill_restart_after => ?OVERLOAD_KILL_RESTART_AFTER,
-       dl_sync_int                 => ?CONTROLLER_SYNC_INTERVAL,
-       filesync_ok_qlen            => ?FILESYNC_OK_QLEN,
        filesync_repeat_interval    => ?FILESYNC_REPEAT_INTERVAL}.
 
+get_init_state() ->
+     #{dl_sync_int                 => ?CONTROLLER_SYNC_INTERVAL,
+       filesync_ok_qlen            => ?FILESYNC_OK_QLEN}.
+
 %%%-----------------------------------------------------------------
 %%% Add a disk_log handler to the logger.
 %%% This starts a dedicated handler process which should always
diff --git a/lib/kernel/src/logger_server.erl b/lib/kernel/src/logger_server.erl
index 9e043f9362..b7735dbcf7 100644
--- a/lib/kernel/src/logger_server.erl
+++ b/lib/kernel/src/logger_server.erl
@@ -31,6 +31,9 @@
          update_config/2, update_config/3,
          update_formatter_config/2]).
 
+%% Helper
+-export([diff_maps/2]).
+
 %% gen_server callbacks
 -export([init/1, handle_call/3, handle_cast/2, handle_info/2,
          terminate/2]).
@@ -244,16 +247,22 @@ handle_call({change_config,SetOrUpd,HandlerId,Config0}, From,
                     update -> OldConfig
                 end,
             Config = maps:merge(Default,Config0),
-            call_h_async(
-              fun() ->
-                      call_h(Module,changing_config,[SetOrUpd,OldConfig,Config],
-                             {ok,Config})
-              end,
-              fun({ok,Config1}) ->
-                      logger_config:set(Tid,HandlerId,Config1);
-                 (Error) ->
-                      Error
-              end,From,State);
+            case check_config_change(OldConfig,Config) of
+                ok ->
+                    call_h_async(
+                      fun() ->
+                              call_h(Module,changing_config,
+                                     [SetOrUpd,OldConfig,Config],
+                                     {ok,Config})
+                      end,
+                      fun({ok,Config1}) ->
+                              logger_config:set(Tid,HandlerId,Config1);
+                         (Error) ->
+                              Error
+                      end,From,State);
+                Error ->
+                    {reply,Error,State}
+            end;
         _ ->
             {reply,{error,{not_found,HandlerId}},State}
     end;
@@ -262,16 +271,22 @@ handle_call({change_config,SetOrUpd,HandlerId,Key,Value}, From,
     case logger_config:get(Tid,HandlerId) of
         {ok,#{module:=Module}=OldConfig} ->
             Config = OldConfig#{Key=>Value},
-            call_h_async(
-              fun() ->
-                      call_h(Module,changing_config,[SetOrUpd,OldConfig,Config],
-                             {ok,Config})
-              end,
-              fun({ok,Config1}) ->
-                      logger_config:set(Tid,HandlerId,Config1);
-                 (Error) ->
-                      Error
-              end,From,State);
+            case check_config_change(OldConfig,Config) of
+                ok ->
+                    call_h_async(
+                      fun() ->
+                              call_h(Module,changing_config,
+                                     [SetOrUpd,OldConfig,Config],
+                                     {ok,Config})
+                      end,
+                      fun({ok,Config1}) ->
+                              logger_config:set(Tid,HandlerId,Config1);
+                         (Error) ->
+                              Error
+                      end,From,State);
+                Error ->
+                    {reply,Error,State}
+            end;
         _ ->
             {reply,{error,{not_found,HandlerId}},State}
     end;
@@ -486,6 +501,15 @@ check_formatter({Mod,Config}) ->
 check_formatter(Formatter) ->
     throw({invalid_formatter,Formatter}).
 
+%% When changing configuration for a handler, the id and module fields
+%% can not be changed.
+check_config_change(#{id:=Id,module:=Module},#{id:=Id,module:=Module}) ->
+    ok;
+check_config_change(OldConfig,NewConfig) ->
+    {Old,New} = logger_server:diff_maps(maps:with([id,module],OldConfig),
+                                        maps:with([id,module],NewConfig)),
+    {error,{illegal_config_change,Old,New}}.
+
 call_h(Module, Function, Args, DefRet) ->
     %% Not calling code:ensure_loaded + erlang:function_exported here,
     %% since in some rare terminal cases, the code_server might not
@@ -558,3 +582,14 @@ call_h_reply(Unexpected,State) ->
                    {process,?SERVER},
                    {message,Unexpected}]),
     {noreply,State}.
+
+%% Return two maps containing only the fields that differ.
+diff_maps(M1,M2) ->
+    diffs(lists:sort(maps:to_list(M1)),lists:sort(maps:to_list(M2)),#{},#{}).
+
+diffs([H|T1],[H|T2],D1,D2) ->
+    diffs(T1,T2,D1,D2);
+diffs([{K,V1}|T1],[{K,V2}|T2],D1,D2) ->
+    diffs(T1,T2,D1#{K=>V1},D2#{K=>V2});
+diffs([],[],D1,D2) ->
+    {D1,D2}.
diff --git a/lib/kernel/src/logger_std_h.erl b/lib/kernel/src/logger_std_h.erl
index 66fa6b6ab6..6bbf75e0ab 100644
--- a/lib/kernel/src/logger_std_h.erl
+++ b/lib/kernel/src/logger_std_h.erl
@@ -35,7 +35,7 @@
          terminate/2, code_change/3]).
 
 %% logger callbacks
--export([log/2, adding_handler/1, removing_handler/1, changing_config/2]).
+-export([log/2, adding_handler/1, removing_handler/1, changing_config/3]).
 
 %% handler internal
 -export([log_handler_info/4]).
@@ -116,9 +116,8 @@ reset(Name) ->
 %%% Handler being added
 adding_handler(#{id:=Name}=Config) ->
     case check_config(adding, Config) of
-        {ok, Config1} ->
+        {ok, #{config:=HConfig}=Config1} ->
             %% create initial handler state by merging defaults with config
-            HConfig = maps:get(config, Config1, #{}),
             HState = maps:merge(get_init_state(), HConfig),
             case logger_h_common:overload_levels_ok(HState) of
                 true ->
@@ -135,22 +134,31 @@ adding_handler(#{id:=Name}=Config) ->
 
 %%%-----------------------------------------------------------------
 %%% Updating handler config
-changing_config(OldConfig=#{id:=Name, config:=OldHConfig},
-                NewConfig=#{id:=Name}) ->
-    #{type:=Type, handler_pid:=HPid, mode_tab:=ModeTab} = OldHConfig,
-    NewHConfig = maps:get(config, NewConfig, #{}),
-    case maps:get(type, NewHConfig, Type) of
-        Type ->
-            NewHConfig1 = NewHConfig#{type=>Type,
-                                      handler_pid=>HPid,
-                                      mode_tab=>ModeTab},
-            changing_config1(HPid, OldConfig,
-                             NewConfig#{config=>NewHConfig1});
-        _ ->
-            {error,{illegal_config_change,OldConfig,NewConfig}}
-    end;
-changing_config(OldConfig, NewConfig) ->
-    {error,{illegal_config_change,OldConfig,NewConfig}}.
+changing_config(SetOrUpdate,OldConfig=#{config:=OldHConfig},NewConfig) ->
+    WriteOnce = maps:with([type],OldHConfig),
+    ReadOnly = maps:with([handler_pid,mode_tab],OldHConfig),
+    NewHConfig0 = maps:get(config, NewConfig, #{}),
+    Default =
+        case SetOrUpdate of
+            set ->
+                %% Do not reset write-once fields to defaults
+                maps:merge(get_default_config(),WriteOnce);
+            update ->
+                OldHConfig
+        end,
+
+    %% Allow (accidentially) included read-only fields - just overwrite them
+    NewHConfig = maps:merge(maps:merge(Default, NewHConfig0),ReadOnly),
+
+    %% But fail if write-once fields are changed
+    case maps:with([type],NewHConfig) of
+        WriteOnce ->
+            changing_config1(maps:get(handler_pid,OldHConfig),
+                             OldConfig,
+                             NewConfig#{config=>NewHConfig});
+        Other ->
+            {error,{illegal_config_change,#{config=>WriteOnce},#{config=>Other}}}
+    end.
 
 changing_config1(HPid, OldConfig, NewConfig) ->
     case check_config(changing, NewConfig) of
@@ -169,8 +177,7 @@ changing_config1(HPid, OldConfig, NewConfig) ->
 check_config(adding, Config) ->
     %% Merge in defaults on handler level
     HConfig0 = maps:get(config, Config, #{}),
-    HConfig = maps:merge(#{type => standard_io},
-                         HConfig0),
+    HConfig = maps:merge(get_default_config(),HConfig0),
     case check_h_config(maps:to_list(HConfig)) of
         ok ->
             {ok,Config#{config=>HConfig}};
@@ -428,8 +435,9 @@ code_change(_OldVsn, State, _Extra) ->
 
 %%%-----------------------------------------------------------------
 %%%
-get_init_state() ->
-     #{sync_mode_qlen              => ?SYNC_MODE_QLEN,
+get_default_config() ->
+     #{type                        => standard_io,
+       sync_mode_qlen              => ?SYNC_MODE_QLEN,
        drop_mode_qlen              => ?DROP_MODE_QLEN,
        flush_qlen                  => ?FLUSH_QLEN,
        burst_limit_enable          => ?BURST_LIMIT_ENABLE,
@@ -439,10 +447,12 @@ get_init_state() ->
        overload_kill_qlen          => ?OVERLOAD_KILL_QLEN,
        overload_kill_mem_size      => ?OVERLOAD_KILL_MEM_SIZE,
        overload_kill_restart_after => ?OVERLOAD_KILL_RESTART_AFTER,
-       file_ctrl_sync_int          => ?CONTROLLER_SYNC_INTERVAL,
-       filesync_ok_qlen            => ?FILESYNC_OK_QLEN,
        filesync_repeat_interval    => ?FILESYNC_REPEAT_INTERVAL}.
 
+get_init_state() ->
+    #{file_ctrl_sync_int          => ?CONTROLLER_SYNC_INTERVAL,
+      filesync_ok_qlen            => ?FILESYNC_OK_QLEN}.
+
 %%%-----------------------------------------------------------------
 %%% Add a standard handler to the logger.
 %%% This starts a dedicated handler process which should always
@@ -825,4 +835,3 @@ sync_dev(Fd, DevName, PrevSyncResult, HandlerName) ->
             logger_h_common:error_notify({HandlerName,filesync,DevName,Error}),
             Error
     end.
-