curtin-dev team mailing list archive

[Michael Hudson-Doyle <mwhudsonlp@xxxxxxxxxxx>]

Thanks for the review.

Diff comments:

> diff --git a/curtin/block/__init__.py b/curtin/block/__init__.py
> index 0cf0866..eca69d8 100644
> --- a/curtin/block/__init__.py
> +++ b/curtin/block/__init__.py
> @@ -421,6 +418,14 @@ def get_blockdev_for_partition(devpath, strict=True):
>      if strict and not os.path.exists(syspath):
>          raise OSError("%s had no syspath (%s)" % (devpath, syspath))
>  
> +    dm_name_path = os.path.join(syspath, 'dm/name')
> +    if os.path.exists(dm_name_path):
> +        dm_name = util.load_file(dm_name_path).rstrip()
> +        if '-part' in dm_name:
> +            parent_name, ptnum = dm_name.rsplit('-part', 1)
> +            parent_path = os.path.realpath('/dev/mapper/' + parent_name)
> +            return (parent_path, ptnum)

I'm not sure what "really disks" means on a philosophical level <wink> but yes this is for multipath. I found that there was a currently-unused function in multipath.py that does most of what we want so I changed it to return the partition number as well and changed this code. (it used udev rather than sysfs pokery but well. no real difference).

On the subject of mocks, get_blockdev_for_partition is currently untested. Tests could be written but you'd have to mock so many filesystem accesses that it would be incredibly fragile and I'm not sure what value it would bring.

You must have thought about a more generic way to mock the filesystem by now, surely, like something where you supply a dictionary mapping paths to content and mocks for the various os.path.* functions get set up for you? Did you ever write anything for this I can steal? :)

> +
>      ptpath = os.path.join(syspath, "partition")
>      if not os.path.exists(ptpath):
>          return (rpath, None)


-- 
https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/396462
Your team curtin developers is requested to review the proposed merge of ~mwhudson/curtin:fix-multipath-partition-verification-2 into curtin:master.