Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add two more functions to lvm, lvcreate and lvremove. #3075

Merged
merged 1 commit into from Dec 30, 2012

Conversation

Projects
None yet
3 participants
Contributor

torhve commented Dec 30, 2012

Lvremove assums lv is deactivated and ready to be removed. This might not be the correct
way to go about this.

@torhve torhve Add two more functions to lvm, lvcreate and lvremove. Lvremove assums lv
is deactivated and ready to be removed. This might not be the correct
way to go about this.
adfda26
Owner

techhat commented Dec 30, 2012

Wow, I didn't expect any interest in this module so soon! One comment I have is on the -L switch. There is also a -L which can be used to specify extents instead. How do you feel about adding that?

Contributor

torhve commented Dec 30, 2012

I have need of lvcreate for my salt virt web project, since my VMs are LV backed. That's why :)
As for lvcreate, the switch you are talking about is -l not -L. And that's not very useful for humans, or is it?
I never use it.
But I'm sure all the different options could be added as people have need of them.

Owner

techhat commented Dec 30, 2012

You are correct; my phone must have done that on me. I agree that humans are more likely to use sizes than extents. But as my primary purpose for this module involves it being used as part of an external program, extents are important to me. I'm more than happy to merge this and make the change myself if you prefer, but either way the interfaces you set are are likely to change.

Contributor

torhve commented Dec 30, 2012

Yeah, I kind of pushed this in an early state since I was worried you might do it yourself and work would be duplicate. I'll leave you to have ownership of this module. So just merge my changes and generate your own pull request I think.

Owner

techhat commented Dec 30, 2012

Sounds good.

@techhat techhat added a commit that referenced this pull request Dec 30, 2012

@techhat techhat Merge pull request #3075 from torhve/lvm
Add two more functions to lvm, lvcreate and lvremove.
ce1eaa0

@techhat techhat merged commit ce1eaa0 into saltstack:develop Dec 30, 2012

Owner

thatch45 commented Dec 31, 2012

Yes, I like these, but they should return more detailed information and be a bit more encompasing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment