-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CLOUDSTACK-8605: KVM: Config Drive and getVmIp support #577
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
Conversation
kishankavala
commented
Jul 10, 2015
- CLOUDSTACK-8324 added support for External DHCP/DNS support along with ConfigDrive for XenServer
- FS link: https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=53740797
- This PR adds KVM support for config drive and getVmIpAddress assigned by external DHCP
- genisoimage package dependency is added for creating config drive ISO
- libguestfs is used to get IPAddress from guest VM. Linux: From dhcp leases file. Windows: From registry
|
cloudstack-pull-rats #33 ABORTED |
|
cloudstack-pull-requests #728 UNSTABLE |
|
/var/lib/libvirt/images is used for local storage also. I'll update the patch to use _localStoragePath instead of hard-coded /var/lib/libvirt/images |
packaging/centos63/cloud.spec
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you'll need to add this to other cloud.spec files in packages/{centos7,fedora20,fedora21}
|
cloudstack-pull-rats #73 SUCCESS |
|
cloudstack-pull-requests #771 UNSTABLE |
|
cloudstack-pull-analysis #6 UNSTABLE |
|
Code-wise I'm not to happy. There are all kinds of assumptions about paths. mkisofs for example always being there in /usr/bin. Using /tmp for temporary directories, who says that /tmp is big enough on every system? Always using /var/lib/libvirt to place the ISO? Why not fetch the local storage pool and figure out what the path is. It's not guaranteerd that it will be /var/lib/libvirt/images on all systems. Imho there are to many assumptions in the code which makes it fragile. |
|
@kishankavala Any update on this? |
This closes apache#577 This closes apache#566 This closes apache#562 This closes apache#561 This closes apache#556 This closes apache#555 This closes apache#554 This closes apache#548 This closes apache#544 This closes apache#540 This closes apache#508 This closes apache#384 This closes apache#372
This closes apache#577 This closes apache#566 This closes apache#562 This closes apache#561 This closes apache#556 This closes apache#555 This closes apache#554 This closes apache#548 This closes apache#544 This closes apache#540 This closes apache#384 This closes apache#372
This closes apache#577 This closes apache#566 This closes apache#562 This closes apache#561 This closes apache#556 This closes apache#555 This closes apache#554 This closes apache#548 This closes apache#544 This closes apache#540 This closes apache#384 This closes apache#372
|
Updated code to use local.storage.path config instead of hard-coded /var/lib/libvirt/images/. Local Storage pool is also created using same config. Removed /usr/bin path for mkisofs. |
|
@remibergsma I've update the PR. There is still one issue open regarding the usage of /tmp |
|
cloudstack-pull-rats #337 ABORTED |
|
cloudstack-pull-requests #1032 ABORTED |
|
cloudstack-pull-analysis #269 UNSTABLE |
|
@kishankavala Thanks for the update! When you're ready, also be sure to ping @wido so he can have another look. |
|
LGTM. |
|
maybe rebase and fix any issues, to get Travis green before merging this once @wido can review this. |
|
cloudstack-pull-rats #540 SUCCESS |
|
cloudstack-pull-analysis #474 SUCCESS |
|
@kishankavala Please remove the 4 merge commits, thanks. |
|
cloudstack-pull-rats #640 SUCCESS |
|
cloudstack-pull-analysis #579 ABORTED |
|
cloudstack-pull-analysis #585 SUCCESS |
|
@kishankavala There are 5 merge commits now... please remove them. |
Correção de NPE na classe VolumeOrchestrator Closes apache#577 See merge request scclouds/scclouds!229