curtin-dev team mailing list archive

[Chris Peterson <mp+494693@xxxxxxxxxxxxxxxxxx>]

Review: Approve

This generally looks good to me too. I seconded the easy commit-by-commit review for this, thanks for splitting those out nicely!


Diff comments:

> diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py
> index a31ef41..0334402 100644
> --- a/curtin/commands/curthooks.py
> +++ b/curtin/commands/curthooks.py
> @@ -1655,12 +1655,20 @@ def configure_mdadm(cfg, state_etcd, target, osfamily=DISTROS.debian):
>  
>  
>  def configure_nvme_over_tcp(cfg, target: pathlib.Path) -> None:
> -    """If any NVMe controller using the TCP transport is present in the storage
> -    configuration, create a nvme-stas configuration and configure the initramfs
> -    so that the remote drives can be made available at boot.
> -    Please note that the NVMe over TCP support in curtin is experimental and in
> -    active development. Currently, it only works with trivial network
> -    configurations ; supplied by Subiquity."""
> +    '''If any NVMe controller using the TCP transport is present in the storage

question: these three implementations are ordered in our attempt to make nvme-o-tcp work, but not necessarily configurable right?

wdyt about adding some text indicating this is also the order in which we will try to set things up?

> +    configuration, configure the target system in such a way that makes booting
> +    possible. We have three different implementations which come with different
> +    requirements and limitations:
> +    1. an implementation that leans on firmware support which makes it possible
> +    to boot with no local storage at all. This only works with dracut and leans
> +    on the nvmf module.
> +    2. one that does not require firmware support but can still be used if the
> +    kernel and initramfs are located on local storage. This works with dracut
> +    and initramfs-tools.
> +    3. a user-space implementation (using nvme-stas) that only works if all
> +    essential filesystems are located on remote storage.
> +    '''
> +
>      controllers = nvme_tcp.get_nvme_stas_controller_directives(cfg)
>  
>      if not controllers:


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