Skip to content

Commit

Permalink
code review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
lslezak committed Sep 21, 2014
1 parent aba61fa commit 529f794
Show file tree
Hide file tree
Showing 5 changed files with 147 additions and 88 deletions.
8 changes: 4 additions & 4 deletions package/yast2-packager.spec
Expand Up @@ -33,14 +33,14 @@ BuildRequires: rubygem(rspec)
# HwDetection
BuildRequires: yast2 >= 3.1.19

# Pkg::SetZConfig()
BuildRequires: yast2-pkg-bindings >= 2.21.8
# "growonly" in Pkg::SetTargetDU()
BuildRequires: yast2-pkg-bindings >= 3.1.19

# Newly added RPM
Requires: yast2-country-data >= 2.16.3

# Pkg::SetZConfig()
Requires: yast2-pkg-bindings >= 2.21.8
# "growonly" in Pkg::SetTargetDU()
Requires: yast2-pkg-bindings >= 3.1.19

# Fixed .proc.cmdline agent
Requires: yast2 >= 3.1.89
Expand Down
16 changes: 12 additions & 4 deletions src/clients/wrapper_storage.rb
Expand Up @@ -29,19 +29,20 @@ def main
Yast.import "Storage"

# call the required function
if @func == "GetTargetMap"
case @func
when "GetTargetMap"
@ret = Storage.GetTargetMap
elsif @func == "GetTargetChangeTime"
when "GetTargetChangeTime"
@ret = Storage.GetTargetChangeTime
elsif @func == "RemoveDmMapsTo"
when "RemoveDmMapsTo"
if Builtins.size(@param) == 0
Builtins.y2error("Missing argument for Storage::RemoveDmMapsTo()")
else
@param1 = Ops.get_string(@param, 0)

@ret = Storage.RemoveDmMapsTo(@param1)
end
elsif @func == "GetWinPrimPartitions"
when "GetWinPrimPartitions"
if Builtins.size(@param) == 0
Builtins.y2error(
"Missing argument for Storage::GetWinPrimPartitions()"
Expand All @@ -54,6 +55,13 @@ def main
)
@ret = Storage.GetWinPrimPartitions(@param1)
end
when "ClassicStringToByte"
if Builtins.size(@param) == 0
Builtins.y2error("Missing argument for Storage::ClassicStringToByte()")
else
param1 = Ops.get(@param, 0)

This comment has been minimized.

Copy link
@jreidinger

jreidinger Sep 22, 2014

Member

new code, so I think direct @param.first is good enough

@ret = Storage.ClassicStringToByte(param1)
end
else
# the required function is not known
Builtins.y2error("unknown function: %1", @func)
Expand Down
82 changes: 26 additions & 56 deletions src/modules/SpaceCalculation.rb
Expand Up @@ -130,7 +130,7 @@ def EvaluateFreeSpace(spare_percentage)
end
elsif target != "/"
if mountName.start_with?(target)
partName = mountName[target.size, -1]
partName = mountName[target.size..-1]
# nothing left, it was target root itself
part_info["name"] = partName.empty? ? "/" : partName
end # target is "/"
Expand All @@ -154,31 +154,31 @@ def EvaluateFreeSpace(spare_percentage)

if filesystem == "btrfs"
log.info "Detected btrfs at #{mountName}"
btrfs_used_kb = btrfs_used_size(mountName) / 1024
log.info "Difference to 'df': #{(part["used"].to_i - btrfs_used_kb) / 1024}MiB"
part_info["used"] = btrfs_used_kb
btrfs_used_kib = btrfs_used_size(mountName) / 1024
log.info "Difference to 'df': #{(part["used"].to_i - btrfs_used_kib) / 1024}MiB"
part_info["used"] = btrfs_used_kib
part_info["growonly"] = btrfs_snapshots?(mountName)
total_kb = part["whole"].to_i
free_size_kb = total_kb - btrfs_used_kb
free_size_kib = total_kb - btrfs_used_kib
else
part_info["used"] = part["used"].to_i
free_size_kb = part["free"].to_i
free_size_kib = part["free"].to_i
part_info["growonly"] = false
end

spare_size_kb = free_size_kb * spare_percentage / 100
spare_size_kb = free_size_kib * spare_percentage / 100

if spare_size_kb < MIN_SPARE_KIB
spare_size_kb = MIN_SPARE_KIB
elsif spare_size > MAX_SPARE_KIB
elsif spare_size_kb > MAX_SPARE_KIB
spare_size_kb = MAX_SPARE_KIB
end

free_size_kb -= spare_size_kb
free_size_kib -= spare_size_kb
# don't use a negative size
free_size_kb = 0 if free_size_kb < 0
free_size_kib = 0 if free_size_kib < 0

part_info["free"] = free_size_kb
part_info["free"] = free_size_kib

du_partitions << part_info
end
Expand Down Expand Up @@ -371,7 +371,7 @@ def EstimateTargetUsage(parts)

# invalid or empty input
if parts == nil || parts.empty?
log.error "Invalid input: #{parts}"
log.error "Invalid input: #{parts.inspect}"
return []
end

Expand All @@ -383,8 +383,8 @@ def EstimateTargetUsage(parts)
"/var/cache/zypp" => 38 * MIB, # zypp metadata cache after refresh (with OSS + update repos)
"/etc" => 2 * MIB, # various /etc config files not belonging to any package
"/usr/share" => 1 * MIB, # some files created by postinstall scripts
"/boot/initrd" => 11 * MIB
} # depends on HW but better than nothing
"/boot/initrd" => 11 * MIB # depends on HW but better than nothing
}

Builtins.y2milestone("Adding target size mapping: %1", used_mapping)

Expand Down Expand Up @@ -735,19 +735,19 @@ def get_partition_info
end

# convert into KiB for TargetInitDU
free_size_kb = free_size / 1024
used_kb = used / 1024
free_size_kib = free_size / 1024
used_kib = used / 1024
mount_name = part["mount"]
log.info "partition: mount: #{mount_name}, free: #{free_size_kb}KiB, used: #{used_kb}KiB"
log.info "partition: mount: #{mount_name}, free: #{free_size_kib}KiB, used: #{used_kib}KiB"

mount_name = mount_name[1..-1] if remove_slash && mount_name != "/"

target_partitions << {
"filesystem" => used_fs.to_s,
"growonly" => growonly,
"name" => mount_name,
"used" => used_kb,
"free" => free_size_kb
"used" => used_kib,
"free" => free_size_kib
}
end
end
Expand Down Expand Up @@ -1050,7 +1050,7 @@ def btrfs_snapshots?(directory)

if ret["exit"] != 0
log.error "btrfs call failed: #{ret}"
raise "Cannot detect Btrfs snapshots, subvolume listing failed"
raise "Cannot detect Btrfs snapshots, subvolume listing failed : #{ret["stderr"]}"
end

snapshots = ret["stdout"].split("\n")
Expand All @@ -1068,15 +1068,17 @@ def btrfs_used_size(directory)

if ret["exit"] != 0
log.error "btrfs call failed: #{ret}"
raise "Cannot detect Btrfs disk usage"
raise "Cannot detect Btrfs disk usage: #{ret["stderr"]}"
end

df_info = ret["stdout"].split("\n")
log.info "Usage reported by btrfs: #{df_info}"

# sum the "used" sizes
used = df_info.reduce(0) do |acc, line |
acc += size_from_string($1) if line.match(/used=(.*)$/)
size = line[/used=(\S+)/, 1]
size = size ? size_from_string(size) : 0
acc += size
end

log.info "Detected total used size: #{used} (#{used / 1024 / 1024}MiB)"
Expand All @@ -1087,43 +1089,11 @@ def btrfs_used_size(directory)
# @example
# size_from_string("2.45MiB") => 2569011
# @param size_str [String] input value in format "<number>[<space>][<unit>]"
# where unit can be one of: "" (none) or "B", "KiB", "MiB", "GiB", "TiB", "PiB" or "EiB"
# where unit can be one of: "" (none) or "B", "KiB", "MiB", "GiB", "TiB", "PiB"
# @return [Integer] size in bytes
def size_from_string(size_str)
if size_str.match(/^(\d+\.?\d*)\s*(\S*)/)
size = $1.to_f
unit = $2

case unit
when "", "B"
exp = 0
when "KiB"
exp = 10
when "MiB"
exp = 20
when "GiB"
exp = 30
when "TiB"
exp = 40
when "PiB"
exp = 50
when "EiB"
exp = 60
else
log.error "Unknown size unit: #{unit}"
raise "Unknown size unit: #{unit}"
end

ret = (size * (2**exp)).to_i

log.info "Size: #{size_str} => #{ret}"
ret
else
log.error "Cannot parse size string: #{size_str}"
raise "Cannot parse size string: #{size_str}"
end
WFM.call("wrapper_storage", ["ClassicStringToByte", [size_str]])
end

end

SpaceCalculation = SpaceCalculationClass.new
Expand Down
36 changes: 36 additions & 0 deletions test/data/run_df.yml
@@ -0,0 +1,36 @@
---
- free: '1470764'
name: /
prz: 12%
spec: tmpfs
type: tmpfs
used: '194072'
whole: '1664836'
- free: '0'
name: /parts/mp_0000
prz: 100%
spec: /dev/loop0
type: squashfs
used: '24320'
whole: '24320'
- free: '0'
name: /parts/mp_0001
prz: 100%
spec: /dev/loop1
type: squashfs
used: '13568'
whole: '13568'
- free: '807152'
name: /dev
prz: 1%
spec: devtmpfs
type: devtmpfs
used: '8'
whole: '807160'
- free: '2977336'
name: /mnt
prz: 53%
spec: /dev/sda2
type: ext4
used: '3259080'
whole: '6717440'

0 comments on commit 529f794

Please sign in to comment.