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

Allow volumes to be updated #43

Merged
merged 2 commits into from
Nov 25, 2014
Merged

Conversation

epienbroek
Copy link
Contributor

This allows volumes to be resized

@epienbroek
Copy link
Contributor Author

ping?

@epienbroek
Copy link
Contributor Author

could you please approve this change and prepare a new rbovirt release?

@lzap
Copy link
Collaborator

lzap commented Nov 14, 2014

Mind dropping some integration tests? The code looks good. @abonas ?

@abonas
Copy link
Collaborator

abonas commented Nov 16, 2014

@lzap, I have maintained this repo shortly in the past, but it's not the case anymore for some time, sorry. you can try checking with @ohadlevy who is the current maintainer.

@epienbroek
Copy link
Contributor Author

I just updated this PR. Compared to the original PR only the desired volume attributes are updated (instead of some additional default fallback attributes) and a testcase was added to test this functionality.

I can confirm that the tests succeed against a RHEV 3.4.1 environment (the commit from #44 is also needed to make the tests succeed).

The tests fail against an oVirt 3.5 CentOS 6 Live environment (the updated volume size doesn't get shown after the resize), but I guess this is due to an oVirt bug as the new size also can't be seen in the oVirt administration portal itself after the resize.

storage_domain_id = opts[:storage_domain] || storagedomains(:role => 'data', :search => search).first.id
http_post("/vms/%s/disks" % vm_id, OVIRT::Volume.to_xml(storage_domain_id, opts))
opts[:storage_domain_id] = opts[:storage_domain] || storagedomains(:role => 'data', :search => search).first.id
opts[:size] = 8589934592 unless opts[:size]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you leave a comment regarding what 8589934592 means?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment was added and a more readable form was applied in the updated commit

By supplying an opts hash containing a key named :size
the size of a volume can be changed
@abonas
Copy link
Collaborator

abonas commented Nov 24, 2014

@jhernand can you please review?

@jhernand
Copy link
Collaborator

I'm ok with merging these changes if we agree that the OVIRT::Whatever.to_xml methods are internal only and not intended for use outside rbovirt.

@jhernand
Copy link
Collaborator

By the way, I just tested this with the latest oVirt Engine master and the disk is resized correctly. @epienbroek, if you see this issue again please open an oVirt Engine bug.

@epienbroek
Copy link
Contributor Author

Okay, good to know that it works in oVirt master. I used the live iso from http://resources.ovirt.org/pub/ovirt-3.5/iso/ovirt-live-el6-3.5.0.iso which probably has a bug in this area

jhernand added a commit that referenced this pull request Nov 25, 2014
@jhernand jhernand merged commit 0fd757a into abenari:master Nov 25, 2014
@epienbroek epienbroek deleted the update_volume branch November 25, 2014 16:01
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.

None yet

4 participants