Page MenuHomeFreeBSD

bsdinstall: Clean up bsdinstall-esps file
AbandonedPublic

Authored by freqlabs on Dec 27 2019, 8:16 PM.

Details

Reviewers
bcran
Summary

Using bsdinstall for scripting VM image preparation leads to bizarre failures because the file used to keep track of ESPs created by the scriptedpart subprogram does not get cleaned up after use, causing new entries to be appended to the file by subsequent invocations of the partitioner. This would leave the contents of the file something like zvol/storage/bhyve/13.0-CURRENT/r356085/templatep1zvol/storage/bhyve/13.0-CURRENT/r356085/templatep1zvol/storage/bhyve/13.0-CURRENT/r356085/templatep1 for example, breaking the ESP preparation script.

To resolve this issue, the file can simply be removed before partitioning.

The lack of space also indicated another potential error. The intent suggested by surrounding code seems to be that multiple ESPs should be possible. Adding a newline after each entry in the bsdinstall-esps file should fix this.

Sponsored by: iXsystems, Inc.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

freqlabs edited the summary of this revision. (Show Details)

Add context to diff.

Looks like a good start but should it be even more robust to stop collision.

usr.sbin/bsdinstall/partedit/gpart_ops.c
714

Can we MKTEMP(3) and pass the file name that way no collision for multiple instances?

usr.sbin/bsdinstall/partedit/gpart_ops.c
714

Just using mktemp here won't avoid the issue because ultimately there has to be some way for the separate bsdinstall commands to know where to look for the files used to pass data between them. Currently there is none, the paths for temporary files are hard coded.

I think it would be a good idea for bsdinstall to more generally support specifying a temp dir using an environment variable. That way separate bsdinstall workflows can be running simultaneously without stomping on each other. It is outside of the scope of this change, but I will put the changes for that up as a separate review.

freqlabs marked an inline comment as done.

Incorporated in the TMPDIR revision: D22979