special handling for PREP partitions on PPC PowerNV machines #248
Conversation
| pb = {} | ||
| # PPCs do not need /boot but prep partition only. | ||
| if !Arch.ppc |
There was a problem hiding this comment.
well, what happen if board is powernv and cryptic first condition is true? then it create prep partition. so maybe use quick exit here?
like
# comment about powernv
return tc if Arch.ppc && Arch.board_powernv
if !@planHasBoot && root &&
....
|
|
||
| describe "#autopart" do | ||
|
|
||
| describe "ppc partitioning" do |
There was a problem hiding this comment.
this should be context and not describe
|
|
||
| context "AY configuration file has no /boot partition" do | ||
| before do | ||
| client.planHasBoot=false |
There was a problem hiding this comment.
this is wrong, as for ppc it is not about /boot but about prep partition. Only exception is powernv. So for ppc in general planHasBoot is true if there is prep or gpt_prep partition ( depending on disk label ).
There was a problem hiding this comment.
Yes, but unfortunately this flag wil be used for /boot and prep partitions. But at least I have changed the naming.
| context "checking device with root partition" do | ||
| let(:checked_device) { "/dev/sdc" } | ||
|
|
||
| it "do not add an additional prep disk if it is a pwoernv system" do |
| ret = client.try_add_boot( ay_device, target_map[checked_device]) | ||
| expect(ret["partitions"].size).to eq(ay_device["partitions"].size) | ||
| end | ||
| it "do add an additional prep disk if it is NOT a pwoernv system" do |
There was a problem hiding this comment.
and do add can be replaced by adds
| it "do add an additional prep disk if it is NOT a pwoernv system" do | ||
| allow(Yast::Arch).to receive(:board_powernv).and_return(false) | ||
| ret = client.try_add_boot( ay_device, target_map[checked_device]) | ||
| expect(ret["partitions"].size).to eq(ay_device["partitions"].size+1) |
There was a problem hiding this comment.
please space around + we should deploy rubocop here, to check it automatic.
| ret = client.try_add_boot( ay_device, target_map[checked_device]) | ||
| expect(ret["partitions"].size).to eq(ay_device["partitions"].size) | ||
| end | ||
| it "do add an additional prep disk if it is NOT a pwoernv system" do |
There was a problem hiding this comment.
and do add can be replaced by adds
| end | ||
| end | ||
|
|
||
| context "checking device with NO root partition" do |
There was a problem hiding this comment.
maybe without instead of with NO
| end | ||
| end | ||
|
|
||
| context "AY configuration file has a /boot partition" do |
| context "checking device with root partition" do | ||
| let(:checked_device) { "/dev/sdc" } | ||
|
|
||
| it "do not add an additional prep disk if it is NOT a pwoernv system" do |
There was a problem hiding this comment.
typo and I think here we should not mention powernv at all, as it is generic for all powerpcs.
| # bnc#989392 | ||
| allow(Yast::Arch).to receive(:board_powernv).and_return(true) | ||
| ret = client.try_add_boot( ay_device, target_map[checked_device]) | ||
| expect(ret["partitions"].size).to eq(ay_device["partitions"].size) | ||
| end | ||
| it "do add an additional prep disk if it is NOT a pwoernv system" do | ||
|
|
||
| it "adds an additional prep partition if it is NOT a powerpc system" do |
There was a problem hiding this comment.
powernv not powerpc! it change sense of this statement
| end | ||
|
|
||
| context "checking device with root partition" do | ||
| let(:checked_device) { "/dev/sdc" } | ||
|
|
||
| it "do not add an additional prep disk if it is a pwoernv system" do | ||
| it "do not add an additional prep partition if it is a powerpc system" do |
There was a problem hiding this comment.
same here, it is powernv! powerpc is whole architecture
| context "checking device with root partition" do | ||
| let(:checked_device) { "/dev/sdc" } | ||
|
|
||
| it "do not add an additional prep partition if it is NOT a powerpc system" do |
| @@ -271,6 +271,12 @@ def preprocess_partition_config(xmlflex) | |||
|
|
|||
| def try_add_boot(conf, disk) | |||
| conf = deep_copy(conf) | |||
There was a problem hiding this comment.
btw deep copy is not needed here, as eval do copy and conf is not used anywhere else.
|
LGTM |
No description provided.