search

projectatomic / container-storage-setup

Service to set up storage for Docker and other container systems
Apache License 2.0
153 stars 77 forks source link

Allow for extents based size for ROOT_SIZE #258

Closed dustymabe closed 6 years ago

dustymabe commented 6 years ago

There are two ways to specify a size to LVM. One is -l (extents) and one is -L (size). In absolute terms -L is nice, but in relative terms (i.e. use a percentage of something) -l is nice. Allow for users to provide a % based option with this commit.

cgwalters commented 6 years ago

LGTM, but will let @rhvgoyal do approval.

cgwalters commented 6 years ago

Eh, if @rhvgoyal doesn't like it we can follow up after.

@rh-atomic-bot r+ 20af8d8

rh-atomic-bot commented 6 years ago

:zap: Test exempted: pull fully rebased and already tested.

rhvgoyal commented 6 years ago

@cgwalters Was this urgent enough that this could not wait for my review. Just now @dustymabe pinged me on IRC for this and I told him that I will look at it in the afternoon.

A quick look says, It needs man page update, it needs update to description in config file and "SIZE_ARG" does not need to be a global variable. Just use a local variable.

rhvgoyal commented 6 years ago

@dustymabe can you please open another PR to take care of above.

dustymabe commented 6 years ago

@dustymabe can you please open another PR to take care of above.

will do. thanks @rhvgoyal

cgwalters commented 6 years ago

@rhvgoyal Some urgency yes, we realized it's critical for https://pagure.io/atomic-wg/issue/281

dustymabe commented 6 years ago

@rhvgoyal docs PR here: https://github.com/projectatomic/container-storage-setup/pull/259