Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rm: linux: add linux-nfs-boot information #646

Merged
merged 3 commits into from
Jan 9, 2024

Conversation

ldts
Copy link
Contributor

@ldts ldts commented Jan 5, 2024

Succintly document how to NFS mount a WIC image that uses OStree.

Readiness

  • Merge (pending reviews)
  • Merge after date or event
  • Draft

Overview

Why merge this PR? What does it solve?

Checklist

Optional. Add a 'x' to steps taken.
You can fill this out after opening the PR. "Did I..."

  • Run spelling and grammar check, preferably with linter.
  • Avoid changing any header associated with a link/reference.
  • Step through instructions (or ask someone to do so).
  • Review for wordiness
  • Match tone and style of page/section.
  • Run make linkcheck.
  • View HTML in a browser to check rendering.
  • Use semantic newlines.
  • follow best practices for commits.
    • Descriptive title written in the imperative.
    • Include brief overview of QA steps taken.
    • Mention any related issues numbers.
    • End message with sign off/DCO line (-s, --signoff).
    • Sign commit with your gpg key (-S, --gpg-sign).
    • Squash commits if needed.
  • Request PR review by a technical writer and at least one peer.

Comments

Any thing else that a maintainer/reviewer should know.
This could include potential issues, rational for approach, etc.

Succintly document how to NFS mount a WIC image that uses OStree.

Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>
@doanac
Copy link
Member

doanac commented Jan 5, 2024

Copy link
Contributor

@kprosise kprosise left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the suggestions are minor markdown formatting or related to style.

source/reference-manual/linux/linux-nfs-boot.rst Outdated Show resolved Hide resolved
source/reference-manual/linux/linux-nfs-boot.rst Outdated Show resolved Hide resolved
source/reference-manual/linux/linux-nfs-boot.rst Outdated Show resolved Hide resolved
source/reference-manual/linux/linux-nfs-boot.rst Outdated Show resolved Hide resolved
source/reference-manual/linux/linux-nfs-boot.rst Outdated Show resolved Hide resolved
source/reference-manual/linux/linux-nfs-boot.rst Outdated Show resolved Hide resolved
source/reference-manual/linux/linux-nfs-boot.rst Outdated Show resolved Hide resolved
source/reference-manual/linux/linux-nfs-boot.rst Outdated Show resolved Hide resolved
source/reference-manual/linux/linux-nfs-boot.rst Outdated Show resolved Hide resolved
source/reference-manual/linux/linux-nfs-boot.rst Outdated Show resolved Hide resolved
@doanac
Copy link
Member

doanac commented Jan 5, 2024

Copy link
Collaborator

@angolini angolini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines 18 to 19
Generally, NFS booting any LmP system requires configuring NFS support
in the ramdisk, along with a TFTP server and an NFS server.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be the first sentence in this intruduction.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right!

Comment on lines 13 to 16
Yocto enables modular initramfs configuration, allowing selective
support for udev, rootfs, ostree, or network filesystem in the
ramdisk. Adding NFS and OSTree ramdisk support permits switch root
scripts to prepare shared network filesystem for mounting on the target.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to understand the goal of this sentence.

Would this order be ok?

Suggested change
Yocto enables modular initramfs configuration, allowing selective
support for udev, rootfs, ostree, or network filesystem in the
ramdisk. Adding NFS and OSTree ramdisk support permits switch root
scripts to prepare shared network filesystem for mounting on the target.
Adding NFS and OSTree ramdisk support permits switch root
scripts to prepare shared network filesystem for mounting on the target
because there are modular initramfs configuration, allowing selective
support for udev, rootfs, ostree, or network filesystem in the
ramdisk.

Why is it important to mention that "Adding NFS and OSTree ramdisk support permits switch root
scripts to prepare shared network filesystem for mounting on the target."? It looks loose to me...

But if you want to mention this, use The Yocto Project instead of yocto

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it wss important to mention just because I didnt know about it :) but it could very well be that is common knowledge and it is just me that wasnt aware of it...but yeah, I'd rather keep it.

Enabling NFS Support on initramfs
---------------------------------

To configure the ramdisk for NFS boot using Yocto requires enabling the
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
To configure the ramdisk for NFS boot using Yocto requires enabling the
To configure the ramdisk for NFS boot using LmP requires enabling the

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lmp uses Yocto and Yocto uses the module. I'd rather point to the last element in the chain (ie Yocto)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in this case, use "the Yocto Project" instead of only yocto


.. prompt:: text

diff --git a/meta-lmp-base/recipes-core/images/initramfs-ostree-lmp-image.bb b/meta-lmp-base/recipes-core/images/initramfs-ostree-lmp-image.bb
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not supposed to be built on a factory?

Copy link
Contributor Author

@ldts ldts Jan 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yes, but it is not enabled by default so someone will have to take action in their override...should I provide a meta-subscriber-override patch instead? I always build locally on my box hence why I didnt bother with the override and just used the patch that @rsalveti shared with me

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main point here is what is needed to make the NFS work is only available via local build?

One argument to keep the local build only is to make sure people will not start using NFS for things other than testing/dev

Copy link
Contributor Author

@ldts ldts Jan 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should cover both cases.I'll add the subscribers patch as well. @angolini I think a more generic - less restrictive - doc is a better doc. What do you think?

@doanac
Copy link
Member

doanac commented Jan 8, 2024

@ldts
Copy link
Contributor Author

ldts commented Jan 8, 2024

@kprosise @rsalveti @angolini maybe we should have this for 92?

@kprosise
Copy link
Contributor

kprosise commented Jan 8, 2024

@ldts I can go ahead and merge whenever you're ready

@ldts
Copy link
Contributor Author

ldts commented Jan 8, 2024

@ldts I can go ahead and merge whenever you're ready

@angolini what do you think?

@ldts
Copy link
Contributor Author

ldts commented Jan 8, 2024

from my side it looks good - I tested on Zynqmp but being these a set of guidelines more than anything else, it should suffice.

@kprosise kprosise merged commit b6fcdf4 into foundriesio:main Jan 9, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants