Skip to content

Conversation

@kishankavala
Copy link
Contributor

  • 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

@asfbot
Copy link

asfbot commented Jul 10, 2015

cloudstack-pull-rats #33 ABORTED

@asfbot
Copy link

asfbot commented Jul 10, 2015

cloudstack-pull-requests #728 UNSTABLE
Looks like there's a problem with this pull request

@kishankavala
Copy link
Contributor Author

/var/lib/libvirt/images is used for local storage also.
com.cloud.hypervisor.kvm.resource.LibvirtComputingResource#configure
_localStoragePath = (String)params.get("local.storage.path");
if (_localStoragePath == null) {
_localStoragePath = "/var/lib/libvirt/images/";
}

I'll update the patch to use _localStoragePath instead of hard-coded /var/lib/libvirt/images

Copy link
Member

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}

@asfbot
Copy link

asfbot commented Jul 17, 2015

cloudstack-pull-rats #73 SUCCESS
This pull request looks good

@asfbot
Copy link

asfbot commented Jul 17, 2015

cloudstack-pull-requests #771 UNSTABLE
Looks like there's a problem with this pull request

@asfbot
Copy link

asfbot commented Jul 17, 2015

cloudstack-pull-analysis #6 UNSTABLE
Looks like there's a problem with this pull request

@kishankavala
Copy link
Contributor Author

@wido @bhaisaab Made the suggested changes

@wido
Copy link
Contributor

wido commented Jul 17, 2015

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.

@remibergsma
Copy link
Contributor

@kishankavala Any update on this?

remibergsma added a commit to remibergsma/cloudstack that referenced this pull request Aug 17, 2015
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
remibergsma added a commit to remibergsma/cloudstack that referenced this pull request Aug 17, 2015
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
remibergsma added a commit to remibergsma/cloudstack that referenced this pull request Aug 17, 2015
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
@kishankavala
Copy link
Contributor Author

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.
Other issue was regarding using /tmp for temporary directories. Should I change this also to use local storage path?

@kishankavala
Copy link
Contributor Author

@remibergsma I've update the PR. There is still one issue open regarding the usage of /tmp

@asfbot
Copy link

asfbot commented Aug 18, 2015

cloudstack-pull-rats #337 ABORTED

@asfbot
Copy link

asfbot commented Aug 18, 2015

cloudstack-pull-requests #1032 ABORTED

@asfbot
Copy link

asfbot commented Aug 18, 2015

cloudstack-pull-analysis #269 UNSTABLE
Looks like there's a problem with this pull request

@remibergsma
Copy link
Contributor

@kishankavala Thanks for the update! When you're ready, also be sure to ping @wido so he can have another look.

@rohityadavcloud
Copy link
Member

LGTM.

@rohityadavcloud
Copy link
Member

maybe rebase and fix any issues, to get Travis green before merging this once @wido can review this.

@asfbot
Copy link

asfbot commented Sep 8, 2015

cloudstack-pull-rats #540 SUCCESS
This pull request looks good

@asfbot
Copy link

asfbot commented Sep 8, 2015

cloudstack-pull-analysis #474 SUCCESS
This pull request looks good

@remibergsma
Copy link
Contributor

@kishankavala Please remove the 4 merge commits, thanks.

@asfbot
Copy link

asfbot commented Sep 17, 2015

cloudstack-pull-rats #640 SUCCESS
This pull request looks good

@asfbot
Copy link

asfbot commented Sep 17, 2015

cloudstack-pull-analysis #579 ABORTED

@asfbot
Copy link

asfbot commented Sep 17, 2015

cloudstack-pull-analysis #585 SUCCESS
This pull request looks good

@remibergsma
Copy link
Contributor

@kishankavala There are 5 merge commits now... please remove them.

RodrigoDLopez pushed a commit to RodrigoDLopez/cloudstack that referenced this pull request Aug 23, 2022
Correção de NPE na classe VolumeOrchestrator

Closes apache#577

See merge request scclouds/scclouds!229
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.

5 participants