'sameas:<device>' broken, fix included

Thomas Neumann blacky+fai at fluffbunny.de
Wed Jun 25 13:57:10 CEST 2014


On Wednesday 25 June 2014 12:12:15 Thomas Lange wrote:
>> [An alternative for 1) would be to change resolve_disk_shortname() to
>> recognize 'disk1' as a proper value. Now that I think about that
>> would be a better long-term-solution. Why does 'the user' / the caller
>> have to care whether the disk device is 'disk1' or 'sda'?]
 
> I would also put this function into resolve_disk_shortname.

It seems to be trivial to modify resolve_disk_shortname to always expect disk# 
instead of just #. There is only one other location which needs to be changed:

delete 

        | /^disk(\d+)/
        {
          # check, whether parted is available
          &FAI::in_path("parted") or die "parted not found in PATH\n";
          # initialise the entry of the hash corresponding to disk$1
          &FAI::init_disk_config($1);
        }
        option(s?)

and replace

        | /^\S+/
        {
          # check, whether parted is available
          &FAI::in_path("parted") or die "parted not found in PATH\n";
          # initialise the entry of the hash corresponding to $item[1]
          &FAI::init_disk_config($item[ 1 ]);
        }
        option(s?)

with

        | /^(\S+)/
        {
          # check, whether parted is available
          &FAI::in_path("parted") or die "parted not found in PATH\n";
          # initialise the entry of the hash corresponding to $1
          &FAI::init_disk_config($1);   # e.g. disk1, sda
        }
        option(s?)

This makes the parser treat
  disk_config disk1 <options>
and  
  disk_config sda <options>
the same way.

And while we're at it: The documentation for resolve_disk_shortname is wrong 
even without modifying the code in the above manner:

################################################################################
# @brief Determines a device's full path from a short name or number
#
# Resolves the device name (/dev/sda), short name (sda) or device number (0) 
to
# a full device name (/dev/sda) and tests whether the device is a valid block
# device.
#
# @param $disk Either an integer, occurring in the context of, e.g., disk2, or
# a device name. The latter may be fully qualified, such as /dev/hda, or a 
short
# name, such as sdb, in which case /dev/ is prepended.
################################################################################

* device number must be at least 1 in order to properly access the
  @FAI::disks array
* there is no test for <device> being a valid block device, however there
  is a test if <device> is present in the filesystem and resolves to a single
  object

The code is actually pretty dangerous. I haven't verified but it may be 
possible to use disk0 and provoke a subtle bug:

Single device config: ($disklist = sda):
  resolve_disk_shortname(0) = 'sda'
  resolve_disk_shortname(1) = 'sda'

Multi device config: ($disklist = sda sdb):
  resolve_disk_shortname(0) = 'sdb'    <--- Ouch.
  resolve_disk_shortname(1) = 'sda'
  resolve_disk_shortname(2) = 'sdb'

bye
thomas


More information about the linux-fai-devel mailing list