From b6f2876cbbed40d99454da52b006d15f41d3fc71 Mon Sep 17 00:00:00 2001 From: Mo Morsi Date: Wed, 21 Aug 2013 12:43:14 -0400 Subject: [PATCH 1/5] introduce helper method to reduce code duplication --- lib/linux_admin/disk.rb | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/lib/linux_admin/disk.rb b/lib/linux_admin/disk.rb index df96f1a..b7a4f39 100644 --- a/lib/linux_admin/disk.rb +++ b/lib/linux_admin/disk.rb @@ -9,6 +9,21 @@ class LinuxAdmin class Disk < LinuxAdmin attr_accessor :path + private + + def str_to_bytes(val, unit) + case unit + when 'K' then + val.to_f.kilobytes + when 'M' then + val.to_f.megabytes + when 'G' then + val.to_f.gigabytes + end + end + + public + def self.local Dir.glob('/dev/[vhs]d[a-z]').collect do |d| Disk.new :path => d @@ -25,14 +40,7 @@ def size out = run!(cmd(:fdisk), :params => {"-l" => nil}).output out.each_line { |l| if l =~ /Disk #{path}: ([0-9\.]*) ([KMG])B.*/ - size = case $2 - when 'K' then - $1.to_f.kilobytes - when 'M' then - $1.to_f.megabytes - when 'G' then - $1.to_f.gigabytes - end + size = str_to_bytes($1, $2) break end } @@ -62,14 +70,7 @@ def partitions case fields[i] when :start_sector, :end_sector, :size if val =~ /([0-9\.]*)([KMG])B/ - val = case $2 - when 'K' then - $1.to_f.kilobytes - when 'M' then - $1.to_f.megabytes - when 'G' then - $1.to_f.gigabytes - end + val = str_to_bytes($1, $2) end when :id From f427399b2c459665d5d62302c9fd4545eade09e7 Mon Sep 17 00:00:00 2001 From: Mo Morsi Date: Wed, 21 Aug 2013 12:45:36 -0400 Subject: [PATCH 2/5] introduce helper method to reduce code duplication --- lib/linux_admin/service.rb | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/linux_admin/service.rb b/lib/linux_admin/service.rb index 9deadb3..3273019 100644 --- a/lib/linux_admin/service.rb +++ b/lib/linux_admin/service.rb @@ -7,6 +7,15 @@ class LinuxAdmin class Service < LinuxAdmin attr_accessor :id + private + + def systemctl(cmd) + run!(cmd(:systemctl), + :params => { nil => [cmd, "#{id}.service"] }) + end + + public + def initialize(id) @id = id end @@ -17,14 +26,12 @@ def running? end def enable - run!(cmd(:systemctl), - :params => { nil => ["enable", "#{id}.service"] }) + systemctl 'enable' self end def disable - run!(cmd(:systemctl), - :params => { nil => ["disable", "#{id}.service"] }) + systemctl 'disable' self end From 6862d72b31f22b355ad842434e25bf7a1c012ac6 Mon Sep 17 00:00:00 2001 From: Mo Morsi Date: Thu, 22 Aug 2013 13:09:43 -0400 Subject: [PATCH 3/5] introduce helper method to reduce code duplication --- lib/linux_admin.rb | 1 + lib/linux_admin/logical_volume.rb | 21 +++++-------------- lib/linux_admin/physical_volume.rb | 23 ++++++--------------- lib/linux_admin/volume.rb | 33 ++++++++++++++++++++++++++++++ 4 files changed, 45 insertions(+), 33 deletions(-) create mode 100644 lib/linux_admin/volume.rb diff --git a/lib/linux_admin.rb b/lib/linux_admin.rb index 26c471a..b221d86 100644 --- a/lib/linux_admin.rb +++ b/lib/linux_admin.rb @@ -16,6 +16,7 @@ require 'linux_admin/distro' require 'linux_admin/system' require 'linux_admin/fstab' +require 'linux_admin/volume' require 'linux_admin/logical_volume' require 'linux_admin/physical_volume' require 'linux_admin/volume_group' diff --git a/lib/linux_admin/logical_volume.rb b/lib/linux_admin/logical_volume.rb index 9379d0f..fb94e44 100644 --- a/lib/linux_admin/logical_volume.rb +++ b/lib/linux_admin/logical_volume.rb @@ -4,7 +4,7 @@ # Licensed under the MIT License class LinuxAdmin - class LogicalVolume < LinuxAdmin + class LogicalVolume < Volume # logical volume name attr_accessor :name @@ -51,22 +51,11 @@ def self.create(name, vg, size) def self.scan @lvs ||= begin - vgs = VolumeGroup.scan - lvs = [] - - out = run!(cmd(:lvdisplay), :params => { '-c' => nil}).output - - out.each_line do |line| - fields = line.split(':') - vgname = fields[1] - vg = vgs.find { |vg| vg.name == vgname } - - lvs << LogicalVolume.new(:name => fields[0], - :volume_group => vg, - :sectors => fields[6].to_i) + scan_volumes(cmd(:lvdisplay)) do |fields, vg| + LogicalVolume.new(:name => fields[0], + :volume_group => vg, + :sectors => fields[6].to_i) end - - lvs end end end diff --git a/lib/linux_admin/physical_volume.rb b/lib/linux_admin/physical_volume.rb index 863f203..9b27c02 100644 --- a/lib/linux_admin/physical_volume.rb +++ b/lib/linux_admin/physical_volume.rb @@ -4,7 +4,7 @@ # Licensed under the MIT License class LinuxAdmin - class PhysicalVolume < LinuxAdmin + class PhysicalVolume < Volume # physical volume device name attr_accessor :device_name @@ -51,22 +51,11 @@ def self.create(device) def self.scan @pvs ||= begin - vgs = VolumeGroup.scan - pvs = [] - - out = run!(cmd(:pvdisplay), :params => { '-c' => nil}).output - - out.each_line do |line| - fields = line.split(':') - vgname = fields[1] - vg = vgs.find { |vg| vg.name == vgname} - - pvs << PhysicalVolume.new(:device_name => fields[0], - :volume_group => vg, - :size => fields[2].to_i) - end - - pvs + scan_volumes(cmd(:pvdisplay)) do |fields, vg| + PhysicalVolume.new(:device_name => fields[0], + :volume_group => vg, + :size => fields[2].to_i) + end end end end diff --git a/lib/linux_admin/volume.rb b/lib/linux_admin/volume.rb new file mode 100644 index 0000000..2c1adcd --- /dev/null +++ b/lib/linux_admin/volume.rb @@ -0,0 +1,33 @@ +# LinuxAdmin Logical Volume Representation +# +# Copyright (C) 2013 Red Hat Inc. +# Licensed under the MIT License + +class LinuxAdmin + class Volume < LinuxAdmin + private + + def self.process_volume_display_line(line) + groups = VolumeGroup.scan + fields = line.split(':') + vgname = fields[1] + vg = groups.find { |vg| vg.name == vgname } + return fields, vg + end + + protected + + def self.scan_volumes(cmd) + volumes = [] + + out = run!(cmd, :params => { '-c' => nil}).output + + out.each_line do |line| + fields, vg = process_volume_display_line(line) + volumes << yield(fields, vg) + end + + volumes + end + end +end From ed1210837749dd6d05f7a1fb7a1e458fe8a53e29 Mon Sep 17 00:00:00 2001 From: Mo Morsi Date: Thu, 22 Aug 2013 13:17:30 -0400 Subject: [PATCH 4/5] code refactoring to simplify methods --- lib/linux_admin/fstab.rb | 51 ++++++++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 17 deletions(-) diff --git a/lib/linux_admin/fstab.rb b/lib/linux_admin/fstab.rb index b524ce1..41bb36d 100644 --- a/lib/linux_admin/fstab.rb +++ b/lib/linux_admin/fstab.rb @@ -13,6 +13,26 @@ class FSTabEntry < LinuxAdmin attr_accessor :mount_options attr_accessor :dumpable attr_accessor :fsck_order + + def initialize(args = {}) + @device = args[:device] + @mount_point = args[:mount_point] + @fs_type = args[:fs_type] + @mount_options = args[:mount_options] + @dumpable = args[:dumpable] + @fsck_order = args[:fsck_order] + end + + def self.from_line(fstab_line) + columns = fstab_line.chomp.split + FSTabEntry.new :device => columns[0], + :mount_point => columns[1], + :fs_type => columns[2], + :mount_options => columns[3], + :dumpable => columns[4].to_i, + :fsck_order => columns[5].to_i + + end end class FSTab < LinuxAdmin @@ -35,24 +55,21 @@ def write! private - def refresh - @entries = [] - f = File.read('/etc/fstab') - f.each_line { |line| - first_char = line.strip[0] - if first_char != '#' && first_char !~ /\s/ - columns = line.split - entry = FSTabEntry.new - entry.device = columns[0] - entry.mount_point = columns[1] - entry.fs_type = columns[2] - entry.mount_options = columns[3] - entry.dumpable = columns[4].to_i - entry.fsck_order = columns[5].to_i - @entries << entry - end + def read + contents = File.read('/etc/fstab') + contents = contents.lines.to_a + contents.reject! { |line| + first_char = line.strip[0] + first_char == '#' || first_char =~ /\s/ } - self + contents + end + + def refresh + @entries = + read.collect { |line| + FSTabEntry.from_line line + } end end end From 3bf7ec0326ebcd1b05ecf0aae378739e8b235e84 Mon Sep 17 00:00:00 2001 From: Mo Morsi Date: Thu, 22 Aug 2013 17:42:02 -0400 Subject: [PATCH 5/5] code refactoring to simplify methods --- lib/linux_admin/disk.rb | 78 +++++++++++++++++++++++------------------ 1 file changed, 44 insertions(+), 34 deletions(-) diff --git a/lib/linux_admin/disk.rb b/lib/linux_admin/disk.rb index b7a4f39..f4d03f3 100644 --- a/lib/linux_admin/disk.rb +++ b/lib/linux_admin/disk.rb @@ -7,6 +7,10 @@ class LinuxAdmin class Disk < LinuxAdmin + PARTED_FIELDS = + [:id, :start_sector, :end_sector, + :size, :partition_type, :fs_type] + attr_accessor :path private @@ -49,46 +53,52 @@ def size end def partitions - @partitions ||= begin - partitions = [] - - # TODO: Should this really catch non-zero RC, set output to the default "" and silently return [] ? - # If so, should other calls to parted also do the same? - # requires sudo - out = run(cmd(:parted), - :params => { nil => [@path, 'print'] }).output - - out.each_line do |l| - if l =~ /^ [0-9].*/ - p = l.split - args = {:disk => self} - fields = [:id, :start_sector, :end_sector, - :size, :partition_type, :fs_type] - - fields.each_index do |i| - val = p[i] - case fields[i] - when :start_sector, :end_sector, :size - if val =~ /([0-9\.]*)([KMG])B/ - val = str_to_bytes($1, $2) - end - - when :id - val = val.to_i - - end - - args[fields[i]] = val - end - partitions << Partition.new(args) + @partitions ||= + parted_output.collect { |disk| + partition_from_parted(disk) + } + end - end + private + + def parted_output + # TODO: Should this really catch non-zero RC, set output to the default "" and silently return [] ? + # If so, should other calls to parted also do the same? + # requires sudo + out = run(cmd(:parted), + :params => { nil => [@path, 'print'] }).output + split = [] + out.each_line do |l| + if l =~ /^ [0-9].*/ + split << l.split end + end + split + end - partitions + + def partition_from_parted(output_disk) + args = {:disk => self} + PARTED_FIELDS.each_index do |i| + val = output_disk[i] + case PARTED_FIELDS[i] + when :start_sector, :end_sector, :size + if val =~ /([0-9\.]*)([KMG])B/ + val = str_to_bytes($1, $2) + end + + when :id + val = val.to_i + + end + args[PARTED_FIELDS[i]] = val end + + Partition.new(args) end + public + def create_partition_table(type = "msdos") run!(cmd(:parted), :params => { nil => [path, "mklabel", type]}) end