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

Add initial support for splitting var as a separated partition #1422

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

ricardosalveti
Copy link
Member

No description provided.

Variable used to split the /var content in a separated partition instead
of making it part of the rootfs.

Signed-off-by: Ricardo Salveti <ricardo@foundries.io>
wic file used to deploy /var as a separated partition.

Signed-off-by: Ricardo Salveti <ricardo@foundries.io>
Replace license with the respective SPDX entry and include a copyright
line for Foundries.IO based on the modifications done over the past few
years.

Signed-off-by: Ricardo Salveti <ricardo@foundries.io>
To cover scenarios where the rootfs or var (or any other extra
partition) is used as the last partition instead.

Signed-off-by: Ricardo Salveti <ricardo@foundries.io>
… set

Signed-off-by: Ricardo Salveti <ricardo@foundries.io>
Allow automatically setting a different WKS_FILE when OSTREE_SPLIT_VAR
is set, which requires var to be defined as a separated partition.

Signed-off-by: Ricardo Salveti <ricardo@foundries.io>
@ricardosalveti ricardosalveti requested a review from a team February 20, 2024 02:43
Copy link
Contributor

@MrCry0 MrCry0 left a comment

Choose a reason for hiding this comment

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

LGTM with a minor note.

}
OTA_BOOT = "${WORKDIR}/ota-boot"
do_image_ota[dirs] += "${OTA_BOOT}"
do_image_ota[cleandirs] += "${OTA_BOOT}"
OTA_VAR = "${WORKDIR}/ota-var"
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be more logical and readable to place setting vars above using them?

Copy link
Member Author

Choose a reason for hiding this comment

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

Funny enough most tasks end up defining the variables and cleaning actions after the task itself is defined (if you see the other tasks in this class). We can probably have a separated commit to move things around if needed (here I would prefer to just add the new variable to avoid a larger diff).

if [ "${OSTREE_SPLIT_VAR}" = "1" ]; then
rm -rf ${OTA_VAR}
mv ${OTA_SYSROOT}/ostree/deploy/${OSTREE_OSNAME}/var ${OTA_VAR}
mkdir -p ${OTA_SYSROOT}/ostree/deploy/${OSTREE_OSNAME}/var
Copy link
Member

Choose a reason for hiding this comment

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

Using mv will fail if we run this task more than one time because on the second one the ${OSTREE_OSNAME}/var is clean. It should be better to copy all the files with cp -a and we can drop the mkdir.

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need to have the content in ${OTA_SYSROOT}/ostree/deploy/${OSTREE_OSNAME}/var after this code.

But I agree we may need to move the content only once like

if [ "${OSTREE_SPLIT_VAR}" = "1" ] && ! -d ${OTA_VAR}; then
		mv ${OTA_SYSROOT}/ostree/deploy/${OSTREE_OSNAME}/var ${OTA_VAR}
		mkdir -p ${OTA_SYSROOT}/ostree/deploy/${OSTREE_OSNAME}/var
fi

Copy link
Member Author

Choose a reason for hiding this comment

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

OTA_SYSROOT is already defined for dir and cleandirs:

do_image_ota[dirs] = "${OTA_SYSROOT}"
do_image_ota[cleandirs] = "${OTA_SYSROOT}"

So a re-run wouldn't be an issue here.

mkdir might not necessarily be needed indeed, will check.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, best to let var in deploy as that is expected by ostree when creating .ostree-selabeled during deploy.

Copy link
Member

Choose a reason for hiding this comment

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

I hadn't even taken into account possible dependencies that have more impact than running more than once

@ricardosalveti
Copy link
Member Author

@Tim-Anderson mind reviewing the resize-helper related changes?

Copy link
Contributor

@Tim-Anderson Tim-Anderson left a comment

Choose a reason for hiding this comment

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

this removes any validation of partition and simply expands the last partition of the device. If that is what you want with no validation that it is the right partition then that should work.

@ricardosalveti
Copy link
Member Author

this removes any validation of partition and simply expands the last partition of the device. If that is what you want with no validation that it is the right partition then that should work.

Yeah, the idea is to expand the latest, which in our standard setup ends up being basically the rootfs. Once we add another patition (e.g. var), it expands that one instead.

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