Hi Julien, > Michael Tautschnig <m...@debian.org> wrote: > > Hi, > > > absolutely fine if the fix goes in via this patch). Just one stylistic note: > > Unless I'm missing something, the following if ... elsif ... > > > >> - if (&FAI::set_partition_type_on_phys_dev($d, "raid")) { > >> + if ($vol->{preserve}) { > >> + $pre_req .= ",pt_complete_" . (&FAI::phys_dev($d))[1]; > >> + } elsif (&FAI::set_partition_type_on_phys_dev($d, "raid")) { > >> $pre_req .= ",pt_complete_" . (&FAI::phys_dev($d))[1]; > >> } else { > >> $pre_req .= ",exist_$d"; > >> } > >> } > > > > ... could be rewritten as > > > > if ($vol->{preserve} || &FAI::set_partition_type_on_phys_dev($d, "raid")) > > That was actually written this way on purpose, for several reasons: > - I don't know how Perl evaluates the if condition (this is usually > unspecified in most languages), and set_partition_... should not be > executed at all for preserved partitions, it just helps making this > clear > - it makes it clear that preserved partitions need the > pt_complete_... prereq for reasons that are different from the > non-preserved case. > > Though pt_complete is only for physical devices, so all things > considered I should have a second look at this. I'll do this on monday. >
Ouch, yes, you're absolutely right. I think the following code should do it: if ($vol->{preserve}) { $pre_req .= &FAI::phys_dev($d)[0] ? ",pt_complete_" . (&FAI::phys_dev($d))[1] : ",exist_$d"; } elsif (&FAI::set_partition_type_on_phys_dev($d, "raid")) { $pre_req .= ",pt_complete_" . (&FAI::phys_dev($d))[1]; } else { $pre_req .= ",exist_$d"; } Hope this helps, Michael
pgpJA0LwX8eEv.pgp
Description: PGP signature