GitHub PullRequestReviewCommentEvent at 2014-01-23 10:36:59
Repository: yast-kdump
Actor: mchf
Payload: {"comment":{"url":"https://api.github.com/repos/yast/yast-kdump/pulls/comments/9108466","id":9108466,"diff_hunk":"@@ -668,51 +668,33 @@ def ReadKdumpKernelParam\n       true\n     end\n \n-    # Propose reserved/allocated memory\n-    #\n-    #\n-    #  @return [Boolean] successfull\n+    TEMPORARY_CONFIG_FILE = \"/var/lib/YaST2/kdump.sysconfig\"\n+\n+    def write_temporary_config_file\n+      # FIXME parameterize Write instead of copying the old config\n+      # NOTE make sure we do not lose 600 mode (cp is ok)\n+      SCR.Execute(path(\".target.bash\"), \"cp #{@kdump_file} #{TEMPORARY_CONFIG_FILE}\")\n+    end\n+\n+    PROPOSE_ALLOCATED_MEMORY_MB_COMMAND = \"kdumptool --configfile #{TEMPORARY_CONFIG_FILE} calibrate\"\n+    # if the command fails\n+    PROPOSE_ALLOCATED_MEMORY_MB_DEFAULT = \"128\"\n \n+    # Propose reserved/allocated memory\n+    # Store the result as a string! to @allocated_memory\n+    # @return [Boolean] true, always successful\n     def ProposeAllocatedMemory\n-      if @allocated_memory == \"0\"\n-        if Ops.greater_or_equal(@total_memory, 512) &&\n-            Ops.less_than(Ops.divide(@total_memory, 1024), 2)\n-          @allocated_memory = \"64\"\n-        elsif Ops.greater_or_equal(Ops.divide(@total_memory, 1024), 2)\n-          @allocated_memory = \"128\"\n-        end\n+      # only propose once\n+      return true if @allocated_memory != \"0\"\n \n-        # bnc #431492 - UPT-LTE: /proc/vmcore is empty in kdump kerne\n-        if Arch.ppc64 && @allocated_memory != \"\"\n-          al_mem = Builtins.tointeger(@allocated_memory)\n-          al_mem = Ops.multiply(al_mem, 2)\n-          @allocated_memory = Builtins.tostring(al_mem)\n-        end\n-        # bnc #446480 - Fine-tune kdump memory proposal\n-        if Arch.ia64 && Ops.greater_or_equal(@total_memory, 1024)\n-          total_memory_gigabyte = Ops.divide(@total_memory, 1024)\n-          if Ops.greater_or_equal(total_memory_gigabyte, 1) &&\n-              Ops.less_than(total_memory_gigabyte, 8)\n-            @allocated_memory = \"256\"\n-          elsif Ops.greater_or_equal(total_memory_gigabyte, 8) &&\n-              Ops.less_than(total_memory_gigabyte, 128)\n-            @allocated_memory = \"512\"\n-          elsif Ops.greater_or_equal(total_memory_gigabyte, 128) &&\n-              Ops.less_than(total_memory_gigabyte, 256)\n-            @allocated_memory = \"768\"\n-          elsif Ops.greater_or_equal(total_memory_gigabyte, 256) &&\n-              Ops.less_than(total_memory_gigabyte, 378)\n-            @allocated_memory = \"1024\"\n-          elsif Ops.greater_or_equal(total_memory_gigabyte, 378) &&\n-              Ops.less_than(total_memory_gigabyte, 512)\n-            @allocated_memory = \"1536\"\n-          elsif Ops.greater_or_equal(total_memory_gigabyte, 512) &&\n-              Ops.less_than(total_memory_gigabyte, 768)\n-            @allocated_memory = \"2048\"\n-          elsif Ops.greater_or_equal(total_memory_gigabyte, 768)\n-            @allocated_memory = \"3072\"\n-          end\n-        end\n+      write_temporary_config_file\n+      out = SCR.Execute(path(\".target.bash_output\"), PROPOSE_ALLOCATED_MEMORY_MB_COMMAND)","path":"src/modules/Kdump.rb","position":66,"original_position":66,"commit_id":"d8228ac5e5fff3a915538af8811b3d41cbaef580","original_commit_id":"d8228ac5e5fff3a915538af8811b3d41cbaef580","user":{"login":"mchf","id":1579239,"avatar_url":"https://gravatar.com/avatar/5daca7b088109866a13c3a162a3cdfea?d=https%3A%2F%2Fidenticons.github.com%2Fe3eebd510ef00bea398d87775126da3e.png&r=x","gravatar_id":"5daca7b088109866a13c3a162a3cdfea","url":"https://api.github.com/users/mchf","html_url":"https://github.com/mchf","followers_url":"https://api.github.com/users/mchf/followers","following_url":"https://api.github.com/users/mchf/following{/other_user}","gists_url":"https://api.github.com/users/mchf/gists{/gist_id}","starred_url":"https://api.github.com/users/mchf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mchf/subscriptions","organizations_url":"https://api.github.com/users/mchf/orgs","repos_url":"https://api.github.com/users/mchf/repos","events_url":"https://api.github.com/users/mchf/events{/privacy}","received_events_url":"https://api.github.com/users/mchf/received_events","type":"User","site_admin":false},"body":"shouldn't it depend on successful config file write (line above)? ","created_at":"2014-01-23T10:36:59Z","updated_at":"2014-01-23T10:36:59Z","html_url":"https://github.com/yast/yast-kdump/pull/6#discussion_r9108466","pull_request_url":"https://api.github.com/repos/yast/yast-kdump/pulls/6","_links":{"self":{"href":"https://api.github.com/repos/yast/yast-kdump/pulls/comments/9108466"},"html":{"href":"https://github.com/yast/yast-kdump/pull/6#discussion_r9108466"},"pull_request":{"href":"https://api.github.com/repos/yast/yast-kdump/pulls/6"}}}}