curtin-dev team mailing list archive

[György Szombathelyi <gyurco@xxxxxxxxxxx>]

I have some minor explanations about some of your concerns, otherwise I'll try to implement as you suggested.

Diff comments:

> diff --git a/curtin/block/clear_holders.py b/curtin/block/clear_holders.py
> index 116ee81..ce1399e 100644
> --- a/curtin/block/clear_holders.py
> +++ b/curtin/block/clear_holders.py
> @@ -165,11 +165,17 @@ def shutdown_mdadm(device):
>      """
>  
>      blockdev = block.sysfs_to_devpath(device)
> +    query = mdadm.mdadm_query_detail(blockdev)
> +
> +    if query.get('MD_CONTAINER'):
> +        LOG.info('Array is in a container, skip discovering raid devices and spares for %s', device)
> +        md_devs = ()
> +    else:
> +        LOG.info('Discovering raid devices and spares for %s', device)
> +        md_devs = (
> +            mdadm.md_get_devices_list(blockdev) +
> +            mdadm.md_get_spares_list(blockdev))
>  
> -    LOG.info('Discovering raid devices and spares for %s', device)
> -    md_devs = (
> -        mdadm.md_get_devices_list(blockdev) +
> -        mdadm.md_get_spares_list(blockdev))
>      mdadm.set_sync_action(blockdev, action="idle")
>      mdadm.set_sync_action(blockdev, action="frozen")

I don't think so, in a multi-volume container, every sub-volume should skip discovering the underlying devices.

>  
> @@ -186,7 +192,7 @@ def shutdown_mdadm(device):
>          LOG.debug('Non-fatal error writing to array device %s, '
>                    'proceeding with shutdown: %s', blockdev, e)
>  
> -    LOG.info('Removing raid array members: %s', md_devs)
> +    LOG.info('Removing raid array members: ', md_devs)

When md_devs is empty (in a containerized array), the first form errors out.

>      for mddev in md_devs:
>          try:
>              mdadm.fail_device(blockdev, mddev)
> @@ -198,7 +204,7 @@ def shutdown_mdadm(device):
>      LOG.debug('using mdadm.mdadm_stop on dev: %s', blockdev)
>      mdadm.mdadm_stop(blockdev)
>  
> -    LOG.debug('Wiping mdadm member devices: %s' % md_devs)
> +    LOG.debug('Wiping mdadm member devices: ', md_devs)

Same as above

>      for mddev in md_devs:
>          mdadm.zero_device(mddev, force=True)
>  
> diff --git a/curtin/block/mdadm.py b/curtin/block/mdadm.py
> index 32b467c..97160e1 100644
> --- a/curtin/block/mdadm.py
> +++ b/curtin/block/mdadm.py
> @@ -603,6 +611,11 @@ def __mdadm_detail_to_dict(input):
>      # start after the first newline
>      remainder = input[input.find('\n')+1:]
>  
> +    # keep only the first section (imsm)
> +    arraysection = remainder.find('\n[');

hehe, hard to drop old habits :)

> +    if arraysection != -1:
> +        remainder = remainder[:arraysection]
> +
>      #  FIXME: probably could do a better regex to match the LHS which
>      #         has one, two or three words
>      rem = r'(\w+|\w+\ \w+|\w+\ \w+\ \w+)\ \:\ ([a-zA-Z0-9\-\.,: \(\)=\']+)'


-- 
https://code.launchpad.net/~gyurco/curtin/+git/curtin/+merge/390307
Your team curtin developers is requested to review the proposed merge of ~gyurco/curtin:imsm into curtin:master.