curtin-dev team mailing list archive

[Michael Hudson-Doyle <mwhudsonlp@xxxxxxxxxxx>]

> > But in groovy+, the udev rule from multipath-tools that has
> > always attempted to remove the devices nodes for the partitions
> > of a disk that is a multipath member actually succeeds, and
> 
> Won't this break curtin on older multipaths that don't have this rule?

No, if the device node for a partition of a disk that is a multipath member is still present, this branch ignores it (or at least that's the intent...)

> Or will this work with older multipath output since we're only referring to
> the mpath dev (rather than a path of the mpath device)?

Er, I think so. Not completely sure about the distinction you're making here.

> Some comments inline.

Replies inline.

Diff comments:

> diff --git a/curtin/storage_config.py b/curtin/storage_config.py
> index 494b142..658ebb3 100644
> --- a/curtin/storage_config.py
> +++ b/curtin/storage_config.py
> @@ -453,72 +453,17 @@ class ProbertParser(object):
>  
>          return None
>  
> -    def is_mpath(self, blockdev):
> -        if blockdev.get('DM_MULTIPATH_DEVICE_PATH') == "1":
> -            return True
> -
> -        return bool('mpath-' in blockdev.get('DM_UUID', ''))
> -
> -    def get_mpath_name(self, blockdev):
> -        mpath_data = self.probe_data.get('multipath')
> -        if not mpath_data:
> -            return None
> -
> -        bd_name = blockdev['DEVNAME']
> -        if blockdev['DEVTYPE'] == 'partition':
> -            bd_name = self.partition_parent_devname(blockdev)
> -        bd_name = os.path.basename(bd_name)
> -        for path in mpath_data.get('paths', []):
> -            if bd_name == path.get('device'):
> -                rv = path.get('multipath')
> -                return rv
> -
> -    def find_mpath_member(self, blockdev):
> -        if blockdev.get('DM_MULTIPATH_DEVICE_PATH') == "1":
> -            # find all other DM_MULTIPATH_DEVICE_PATH devs with same serial
> -            serial = blockdev.get('ID_SERIAL')
> -            members = sorted([os.path.basename(dev['DEVNAME'])
> -                              for dev in self.blockdev_data.values()
> -                              if dev.get("ID_SERIAL", "") == serial and
> -                              dev['DEVTYPE'] == blockdev['DEVTYPE']])
> -            # [/dev/sda, /dev/sdb]
> -            # [/dev/sda1, /dev/sda2, /dev/sdb1, /dev/sdb2]
> -
> -        else:
> -            dm_mpath = blockdev.get('DM_MPATH')
> -            dm_uuid = blockdev.get('DM_UUID')
> -            dm_part = blockdev.get('DM_PART')
> -            dm_name = blockdev.get('DM_NAME')
> -
> -            if dm_mpath:
> -                multipath = dm_mpath
> -            elif dm_name:
> -                multipath = dm_name
> -            else:
> -                # part1-mpath-30000000000000064
> -                # mpath-30000000000000064
> -                # mpath-36005076306ffd6b60000000000002406
> -                match = re.search(r'mpath\-([a-zA-Z]*|\d*)+$', dm_uuid)
> -                if not match:
> -                    LOG.debug('Failed to extract multipath ID pattern from '
> -                              'DM_UUID value: "%s"', dm_uuid)
> -                    return None
> -                # remove leading 'mpath-'
> -                multipath = match.group(0)[6:]
> -            mpath_data = self.probe_data.get('multipath')
> -            if not mpath_data:
> -                return None
> -            members = sorted([path['device'] for path in mpath_data['paths']
> -                              if path['multipath'] == multipath])
> -
> -            # append partition number if present
> -            if dm_part:
> -                members = [member + dm_part for member in members]
> -
> -        if len(members):
> -            return members[0]
> -
> -        return None
> +    def is_mpath_disk(self, blockdev):
> +        return blockdev.get('DM_UUID', '').startswith('mpath-')
> +
> +    def is_mpath_partition(self, blockdev):
> +        dm_uuid = blockdev.get('DM_UUID')

DM_PART can be set for things that are partitions of non-mulitpath device mapper things though:

$ sudo kpartx -sav autoinstall.img 
add map loop53p1 (253:2): 0 196608 linear 7:53 4096
$ udevadm info /dev/mapper/loop53p1 | grep DM_PART
E: DM_PART=1

I think the combination of DM_MPATH and DM_PART being present is a correct check though, let's do that instead.

> +        if dm_uuid is None:
> +            return False
> +        parts = dm_uuid.split('-', 2)
> +        if len(parts) < 2:
> +            return False
> +        return parts[0].startswith('part') and parts[1] == 'mpath'
>  
>      def blockdev_to_id(self, blockdev):
>          """ Examine a blockdev dictionary and return a tuple of curtin
> @@ -539,15 +484,13 @@ class ProbertParser(object):
>              if 'DM_LV_NAME' in blockdev:
>                  devtype = 'lvm-partition'
>                  name = blockdev['DM_LV_NAME']
> -            elif self.is_mpath(blockdev):
> -                # extract a multipath member device
> -                member = self.find_mpath_member(blockdev)
> -                if member:
> -                    name = member
> -                else:
> -                    name = blockdev['DM_UUID']
> -                if 'DM_PART' in blockdev:
> -                    devtype = 'partition'
> +            elif self.is_mpath_disk(blockdev):
> +                devtype = 'mpath-disk'
> +                name = blockdev['DM_NAME']
> +            elif self.is_mpath_partition(blockdev):
> +                devtype = 'mpath-partition'

This is for the id of the generated action, not the type.

> +                name = '{}-part{}'.format(
> +                    blockdev['DM_MPATH'], blockdev['DM_PART'])
>              elif is_dmcrypt(blockdev):
>                  devtype = 'dmcrypt'
>                  name = blockdev['DM_NAME']


-- 
https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/392353
Your team curtin developers is subscribed to branch curtin:master.