Merged
Conversation
Just little code reorganization. No functionality change. Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Instead of hardcoding "docker-data" and "docker=meta" everywhere, use bash variables. No functionality change. Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Using VG/LV is more convenient and reliable instead of specifying full path. No functionality change. Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Move some code to do lvm to device mapper path conversion in a separate function. No functionality change. Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Use a variable DEFAULT_DATA_SIZE_PERCENT, to figure out default percentage which needs to be used while createing a data LV. No functionality change. Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Save restore IFS properly. Otherwise it breaks for loops later and one gets unexpected results. Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Currently we use 100% of free space while creating data volume. While setting up lvm thin pool, we need to leave some space free for internal spare meta volume. This volume is used if one needs to repair thin pool. (See "Spare metadata LV" in "man lvmthin"). So use 98% of free space instead of 100% and leave some to be used for spare metadata LV. Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
…ordingly If users passes "SETUP_LVM_THIN_POOL=yes" in docker-storage-setup config file, then setup an LVM thin pool and pass it to docker. That way docker does not have to setup a pool and it can just concern itself with creation and deletion of thin and snap devices. This can allow lvm to repair pool and autoamtically grow pool when less space is left in pool. Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
There are many lines which wrap around when reading man page making it difficult to read the man page. Fix these. Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Modify man page to put information about SETUP_LVM_THIN_POOL. Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Member
|
A next step here for Atomic/F22 is to default to this out of the box - it might be nice if we could auto-detect whether we had new enough versions of lvm2/docker in the script, but failing that, we can set the variable by default in the installer. |
Collaborator
Author
|
Agreed. I was hoping that let this get merged first and if there are no issues, then work on making this new option default. |
Collaborator
Author
|
cgwalters, can you please pull in this request. |
Collaborator
Author
|
Found a small bug in my patches. Will fix it and update the pull request soon. |
… exists lvm_pool_exists() checks whether lvm thin pool volume already exists or not. It comapres existing volume names with pool name "docker-pool". Problem is that name "docker-pool" is used for data volume too and if lvm_pool_exists() is called after data volume creation, then it thinks that pool already exists. But the fact is that docker-pool is data volume and not the pool volume. Fix this problem by also checking volume attributes. A pool volume will start with "t". A little background on why data volume name is same as pool volume name. lvconvert takes the data volume name and uses it as pool volume name. So data volume name was kept as dokcer-pool so that finally it ends up as pool name. This probably can be managed better by doing "lvrename" on pool after creation, but that's a todo improvement. Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Use lvm thin pool mode by default. There are two exceptions though. - Don't use pool mode if docker-storage-setup file is present and user has not set SETUP_LVM_THIN_POOL. - If docker-data or docker-meta volumes are already present, that means user is already using old mode and don't use pool mode. Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Collaborator
Author
|
Ok, I pushed two more commits. First commit is to fix a bug in previous patches. And second commit makes lvm pool mode as default mode and by default setups lvm pool to pass to docker. |
Collaborator
Author
|
Please have a look and pull the changes. |
Collaborator
Author
|
I ran the script from / and I don't see it expanding to /tmp. Things still work fine. Instead putting a double quote or single quote around it breaks the things. I want regular express to work and putting a quote around it might break it. |
Member
|
Ok, I understand |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request contains changes for support of setting up lvm thin pool for docker consumption.
I have opened following issue.
#3
This is not default right now and one will have to set SETUP_LVM_THIN_POOL=yes in /etc/sysconfig/docker-storage-setup for new code to kick in.
Please review and pull.