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

Support disk type conversion #102

Merged
merged 6 commits into from
Dec 8, 2013

Conversation

miurahr
Copy link
Collaborator

@miurahr miurahr commented Dec 8, 2013

7c399e1 by @adrahon breaks disk image type conversion.

This pull request fix it with libvirt API.

TBD: progress information when conversion.

- support backword compatibility about box disk format.
 use create_vol_xml_from() to convert disk format automatically
 instead of create_vol_xml() and upload

Signed-off-by: Hiroshi Miura <miurahr@linux.com>
@miurahr miurahr mentioned this pull request Dec 8, 2013
@miurahr
Copy link
Collaborator Author

miurahr commented Dec 8, 2013

FYI:

http://libvirt.org/html/libvirt-libvirt.html#virStorageVolCreateXMLFrom

Since libvirt 1.0.1, we can get higher performance with qcow2, with specifying flag, but need Fedora19, and Debian Jessie. That's because I don't add the flag here.

@miurahr
Copy link
Collaborator Author

miurahr commented Dec 8, 2013

This modification cause box directory such as ~/.vagrant.d/boxes/lucid/kvm become root.root owner.
need more considerations.

- add permission block to XML

Signed-off-by: Hiroshi Miura <miurahr@linux.com>
Signed-off-by: Hiroshi Miura <miurahr@linux.com>
@miurahr
Copy link
Collaborator Author

miurahr commented Dec 8, 2013

Fix the permission issue.

Signed-off-by: Hiroshi Miura <miurahr@linux.com>
@adrahon
Copy link
Owner

adrahon commented Dec 8, 2013

I'm not sure I understand the change. Are you creating a storage pool for each VM? What's the point.

This also blocks a change I'm working on to let the storage pool be defined by a parameter.

Signed-off-by: Hiroshi Miura <miurahr@linux.com>
@miurahr
Copy link
Collaborator Author

miurahr commented Dec 8, 2013

Upload code in vagrant-libvit don't support qcow2/raw conversion.

virStorageVolCreateXMLFrom in libvirt support qcow2/raw conversion.

To use virStorageVolCreateXMLFrom, I add box directory as storage pool in temporary.

@adrahon
Copy link
Owner

adrahon commented Dec 8, 2013

OK, I understand better now, makes sense.

adrahon added a commit that referenced this pull request Dec 8, 2013
@adrahon adrahon merged commit 8619c89 into adrahon:master Dec 8, 2013
@adrahon
Copy link
Owner

adrahon commented Dec 8, 2013

I have a regression with your code from commit 5bd991d#diff-8fbdf238b2fce9e29b19e7ba2100083c where you give ownership of the pool to the user. This doesn't work for me, it sets ownership to UID/GUID -1/-1, also I think the owner is supposed to be root with libvirt giving ownership to qemu when needed.

Does it work for you? Is it a Debian/Fedora difference issue?

We need to work on adding test to prevent issues like this in the future.

@miurahr miurahr deleted the support_disk_type_conversion branch December 12, 2013 06:24
@miurahr
Copy link
Collaborator Author

miurahr commented Jan 6, 2014

in ubuntu/debian, gid/uid returns user's one. It seems be a different behavior.

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.

2 participants