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 an option for disk_size in case kickstart parsing fails #75

Closed
wants to merge 1 commit into from
Closed

Add an option for disk_size in case kickstart parsing fails #75

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Dec 3, 2015

In cases where the passed kickstart does not have any partition sizes specified (autopart or similar), livemedia-creator defaults to a 2MiB disk, which is too small for many use cases.

Add an option to specify the disk size in GiB in case users want to override the default logic.

In cases where the passed kickstart does not have any partition
sizes specified (autopart or similar), livemedia-creator defaults
to a 2MiB disk, which is too small for many use cases.

Add an option to specify the disk size in GiB in case users want
to override the default logic.

Forgot to make the type int
@bcl
Copy link
Contributor

bcl commented Dec 3, 2015

I'm more inclined to add better checking for the / part size than to override it. I'd like to keep as much build information off the cmdline and in the kickstart as possible. autopart shouldn't be used with lmc.

@ghost
Copy link
Author

ghost commented Dec 4, 2015

Not an unexpected reply, though I wonder if lmc also has the same problem with --size=1 --grow, which is common as well (maybe not often with lmc).

We can probably change our kickstart, though it would be preferable if lmc didn't require tweaking of kickstarts.

As a suggestion, if this won't be merged, lmc should probably warn and die if there's no size specified in the kickstart, unless there's a use case for that I'm missing. The behavior (Anaconda fails in storage) is non-obvious.

@vpodzime
Copy link
Contributor

vpodzime commented Dec 4, 2015

I see this as a nice way to override/force the size of the image no matter what the size of the / patition is. That could be handy in many cases, I think.

@bcl
Copy link
Contributor

bcl commented Dec 9, 2015

Yes, it won't work with grow either, the reason is that it needs to create the disk image before running virt-install so you need to provide a kickstart with the correct part / size already setup. The problem with overriding this outside the kickstart is that you then have a kickstart that possibly won't work unless you remember what you ran lmc with.

The goal is to make the kickstart the keeper of the information, not to be able to handle every possible kickstart layout.

@ghost
Copy link
Author

ghost commented Dec 9, 2015

While true, I would suggest that a kickstart which won't work with lmc at all is just as frustrating from a user perspective.

Would you be happier if --disk-size was orthogonal to detecting disk size from the kickstart? That is -- if "part --size..." is specified (and the kickstart parser finds it) and --disk-size is simultaneously specified, to exit lmc? That way lmc is not overriding any part of the kickstart, just providing supplementary information in the case of --grow and autopart.

@bcl
Copy link
Contributor

bcl commented Dec 11, 2015

Not really. I just don't see a strong need for something like this.

@linux-modder
Copy link

Ryan,

If I understand your point right you want it to have a de-dup like
feature. The issue I see is how to parse it properly with raid & lvm
where pv{foo} --grow and the like would exist.

Corey W Sheldon
Freelance IT Consultant, Multi-Discipline Tutor
Ameridea LLC, Founder, CTO
https://github.com/ameridea
Fedora Ambassador, North America
http://getfedora.org
Server Intern Staff, Citygate
http://citygate.org
(p) +1 (310) 909-7672
Find Me on any of the sites I teach /frequent:

https://gist.github.com/linux-modder/ac5dc6fa211315c633c9

"Have no way as way, no limitation as limitation. One must never

underestimate the power of boredom...from which creativity and laziness are

borne, which can spark great works of chaos and genius."

PGP: 0xe958c5d6718bf597 FP = 2930 99EB 083D D332 0752 88C4 E958 C5D6 718B
F597 [email protected]
Tox: Corey84 || Linux-modder
9357BC6A5944A08AFC7D1EFFD61F6A73B9EABF8B2FB84ACF1DAC9A1A4D0A4705FFCCD0E5499B
Linphone: sip:linuxmodder
BitAddress:15cn1BvAFEREHk8UekJ6i9Dxi9Wbw6vzDD

On Fri, Dec 11, 2015 at 1:58 PM, Brian C. Lane [email protected]
wrote:

Not really. I just don't see a strong need for something like this.


Reply to this email directly or view it on GitHub
#75 (comment).

@ghost
Copy link
Author

ghost commented Dec 12, 2015

Hey Corey -

It's primarily that we're rebuilding oVirt Node, and in doing so, we're moving to a new, clean codebase. The old oVirt Node is a relatively old project (as upstream goes), and the core design hasn't changed since its inception.

Since oVirt Node is/was essentially a custom distro built on top of packages from RPM-based distros (EL, Fedora, whatever), years of development meant that there's a significant amount of logic to add functionality and fix bugs which landed in the kickstart. This is a design methodology that we don't want to repeat, and we're a small development team, so keeping up with platform changes can be quite frustrating.

We'd like to keep the kickstart as simplistic as possible. It's less about dedupe, which autopart wouldn't cover anyway, and more about having a very minimal kickstart.

In particular, since oVirt supports PPC (and may support aarch64 at some point in the future), using autopart would allow us to leverage all the work the anaconda team is doing/has done for supporting installs on different platforms. We know from experience that it's a ton of work to even keep up with legacy boot, EFI, and iSCSI roots by yourself, and that's just a fraction of what Anaconda supports.

From a practical sense, we'd only be using this kickstart to build images in koji and in Jenkins, and everything Node does is x86_64 (for now), so it's not the end of the world for us to specify actual partition sizes, but being able to add a --disk-size flag and use autopart provides us with a huge amount of future-proofing, since a future RFE for PPC/aarch64/etc support on Node won't involve chasing down what changes are necessary, opening bugs/filing RFEs there, etc.

We're trying to get our ducks in a row.

In a more immediate sense, if this pull request isn't merged, as a consumer of livemedia-creator, it's ok for lorax to be opinionated. "We don't support autopart or --grow" is absolutely an acceptable design decision (though one that we'd like to influence, if possible). But it's off-putting for an end-user to throw a kickstart at lmc, watch it start, then fail 10 minutes later. It's visible from the output that the disk size is much too small, and the anaconda logs are clear enough (if you're used to reading anaconda logs), but neither of these are obvious or intuitive enough.

If autopart and --grow are not supported, my opinion is that lmc should throw a very obvious warning. This is already done for autopart if --make-fsimage is specified, but not --make-disk (or any other variant).

@bcl
Copy link
Contributor

bcl commented Dec 23, 2015

I think adding more checks on the kickstart are the right way to go with this. Also, I think it makes sense to keep the disk size in the ks, since the required space will only change when the %package selection changes. I've created Issue #79 for this.

@bcl bcl closed this Dec 23, 2015
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.

3 participants