-
Notifications
You must be signed in to change notification settings - Fork 30
further parameterize disk#create_partition, accept params for % of free ... #52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,30 @@ def str_to_bytes(val, unit) | |
val.to_f.gigabytes | ||
end | ||
end | ||
|
||
def overlapping_ranges?(ranges) | ||
ranges.find do |range1| | ||
ranges.any? do |range2| | ||
range1 != range2 && | ||
range1.overlaps?(range2) | ||
end | ||
end | ||
end | ||
|
||
def check_if_partitions_overlap(partitions) | ||
ranges = | ||
partitions.collect do |partition| | ||
start = partition[:start] | ||
finish = partition[:end] | ||
start.delete('%') | ||
finish.delete('%') | ||
start.to_f..finish.to_f | ||
end | ||
|
||
if overlapping_ranges?(ranges) | ||
raise ArgumentError, "overlapping partitions" | ||
end | ||
end | ||
|
||
public | ||
|
||
|
@@ -109,27 +133,46 @@ def has_partition_table? | |
result_indicates_partition_table?(result) | ||
end | ||
|
||
def create_partition(partition_type, size) | ||
def create_partition(partition_type, *args) | ||
create_partition_table unless has_partition_table? | ||
|
||
id, start = | ||
partitions.empty? ? [1, 0] : | ||
[(partitions.last.id + 1), | ||
partitions.last.end_sector] | ||
start = finish = size = nil | ||
case args.length | ||
when 1 then | ||
start = partitions.empty? ? 0 : partitions.last.end_sector | ||
size = args.first | ||
finish = start + size | ||
|
||
when 2 then | ||
start = args[0] | ||
finish = args[1] | ||
|
||
else | ||
raise ArgumentError, "must specify start/finish or size" | ||
end | ||
|
||
options = parted_options_array('mkpart', partition_type, start, start + size) | ||
id = partitions.empty? ? 1 : (partitions.last.id + 1) | ||
options = parted_options_array('mkpart', '-a opt', partition_type, start, finish) | ||
run!(cmd(:parted), :params => { nil => options}) | ||
|
||
partition = Partition.new(:disk => self, | ||
:id => id, | ||
:start_sector => start, | ||
:end_sector => start+size, | ||
:end_sector => finish, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will this have adverse results with start_sector and end_sector as percents? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jrafanie size start / end sectors aren't used in linux_admin so they should be fine being percentages and size will be nil if the percentages are given. If there is a need for these we can probably figure out a way to extract these post-creation. |
||
:size => size, | ||
:partition_type => partition_type) | ||
partitions << partition | ||
partition | ||
end | ||
|
||
def create_partitions(partition_type, *args) | ||
check_if_partitions_overlap(args) | ||
|
||
args.each { |arg| | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason this can't be one-lined? |
||
self.create_partition(partition_type, arg[:start], arg[:end]) | ||
} | ||
end | ||
|
||
def clear! | ||
@partitions = [] | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -99,6 +99,35 @@ | |
end | ||
end | ||
|
||
describe "#create_partitions" do | ||
before(:each) do | ||
@disk = LinuxAdmin::Disk.new(:path => '/dev/hda') | ||
end | ||
|
||
it "dispatches to create_partition" do | ||
@disk.should_receive(:create_partition).with("primary", "0%", "50%") | ||
@disk.create_partitions "primary", :start => "0%", :end => "50%" | ||
end | ||
|
||
context "multiple partitions specified" do | ||
it "calls create_partition for each partition" do | ||
@disk.should_receive(:create_partition).with("primary", "0%", "49%") | ||
@disk.should_receive(:create_partition).with("primary", "50%", "100%") | ||
@disk.create_partitions("primary", {:start => "0%", :end => "49%"}, | ||
{:start => "50%", :end => "100%"}) | ||
end | ||
|
||
context "partitions overlap" do | ||
it "raises argument error" do | ||
expect{ | ||
@disk.create_partitions("primary", {:start => "0%", :end => "50%"}, | ||
{:start => "49%", :end => "100%"}) | ||
}.to raise_error(ArgumentError) | ||
end | ||
end | ||
end | ||
end | ||
|
||
describe "#create_partition" do | ||
before(:each) do | ||
# test disk w/ existing partition | ||
|
@@ -110,12 +139,29 @@ | |
end | ||
|
||
it "uses parted" do | ||
@disk.should_receive(:run!). | ||
with(@disk.cmd(:parted), | ||
:params => { nil => ['--script', '/dev/hda', 'mkpart', 'primary', 1024, 2048] }) | ||
params = ['--script', '/dev/hda', 'mkpart', '-a opt', 'primary', 1024, 2048] | ||
@disk.should_receive(:run!).with(@disk.cmd(:parted), :params => { nil => params }) | ||
@disk.create_partition 'primary', 1024 | ||
end | ||
|
||
it "accepts start/end params" do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would you be opposed to this? |
||
params = ['--script', '/dev/hda', 'mkpart', '-a opt', 'primary', "0%", "50%"] | ||
@disk.should_receive(:run!).with(@disk.cmd(:parted), :params => { nil => params }) | ||
@disk.create_partition 'primary', "0%", "50%" | ||
end | ||
|
||
context "missing params" do | ||
it "raises ArgumentError" do | ||
expect{ | ||
@disk.create_partition 'primary' | ||
}.to raise_error(ArgumentError) | ||
|
||
expect{ | ||
@disk.create_partition 'primary', '0%', '50%', 100 | ||
}.to raise_error(ArgumentError) | ||
end | ||
end | ||
|
||
it "returns partition" do | ||
@disk.should_receive(:run!) # stub out call to parted | ||
partition = @disk.create_partition 'primary', 1024 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just checked and Range#overlaps? and Range#include? are already provided by active_support and should be preferred over rolling our own.