[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-24 Thread wido
Github user wido commented on the pull request: https://github.com/apache/cloudstack/pull/1545#issuecomment-221221816 I understand @swill. I'll leave this up to you. The PR has two LGTMs, so we can merge when we want. Same goes for #1531 --- If your project is set up for it,

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-23 Thread swill
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1545#issuecomment-221166274 I missed that this one was going to need a new systemvmtemplate. Given that I am already over a week late on this release and I have not done any testing or work

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-23 Thread rhtyd
Github user rhtyd commented on the pull request: https://github.com/apache/cloudstack/pull/1545#issuecomment-221165022 LGTM, to use this feature. KVM users will need to use new systemvmtemplates, can we get #1531 merged as well? /cc @swill --- If your project is set up for it, you

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-23 Thread swill
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1545#issuecomment-221158910 This is coming back clean. Can I get a second LGTM? Maybe @rhtyd? --- If your project is set up for it, you can reply to this email and have your reply appear on

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-23 Thread swill
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1545#issuecomment-221158751 ### CI RESULTS ``` Tests Run: 85 Skipped: 0 Failed: 0 Errors: 0 Duration: 4h 25m 18s ```

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-20 Thread swill
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1545#issuecomment-220621430 @DaanHoogland yes, I will retest now. Thanks guys... --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well.

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-20 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1545#issuecomment-220599638 LGTM, do we retest @swill? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-20 Thread wido
Github user wido commented on the pull request: https://github.com/apache/cloudstack/pull/1545#issuecomment-220579267 @swill I have pushed the new version of the commits @rhtyd @DaanHoogland Thanks for your review and feedback. Should be good now I think. --- If your

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-19 Thread swill
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1545#issuecomment-220512533 These failures are not related to this PR, but if you have ideas why they are happening I would be interested. I will wait till @wido has a chance to work on this

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-19 Thread swill
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1545#issuecomment-220512458 ### CI RESULTS ``` Tests Run: 78 Skipped: 0 Failed: 0 Errors: 4 Duration: 7h 47m 49s ``` **Summary of the

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-19 Thread wido
Github user wido commented on the pull request: https://github.com/apache/cloudstack/pull/1545#issuecomment-220440726 @swill I will work on the changes tomorrow morning (my time, CET). Shouldn't be very hard to do. --- If your project is set up for it, you can reply to this email

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-19 Thread swill
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1545#issuecomment-220365886 Sorry, I need to queue this one up... --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-19 Thread wido
Github user wido commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1545#discussion_r63889814 --- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java --- @@ -1978,11 +1978,16 @@ So if getMinSpeed()

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-19 Thread wido
Github user wido commented on the pull request: https://github.com/apache/cloudstack/pull/1545#issuecomment-220342553 Thanks for the feedback, I will work on that tomorrow. @rhtyd I tested this on both Ubuntu and CentOS systems. The main issue with Ubuntu is AppArmor

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-19 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1545#issuecomment-220281463 @swill, did this pass the (not so C)I? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-19 Thread rhtyd
Github user rhtyd commented on the pull request: https://github.com/apache/cloudstack/pull/1545#issuecomment-220278587 In general, a good idea and would be useful for flushing out disk IO and doing clean shutdown of VRs and vms. There are some outstanding issues, but the feature is

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-19 Thread rhtyd
Github user rhtyd commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1545#discussion_r63850379 --- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java --- @@ -1978,11 +1978,16 @@ So if getMinSpeed()

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-19 Thread rhtyd
Github user rhtyd commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1545#discussion_r63850026 --- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java --- @@ -1209,25 +1209,95 @@ public String toString() {

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-19 Thread rhtyd
Github user rhtyd commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1545#discussion_r63849902 --- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java --- @@ -171,6 +173,25 @@ public boolean

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-19 Thread rhtyd
Github user rhtyd commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1545#discussion_r63849851 --- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java --- @@ -1978,11 +1978,16 @@ So if getMinSpeed()

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-19 Thread DaanHoogland
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1545#issuecomment-220275302 apart from some code style and convention things LGTM --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-19 Thread DaanHoogland
Github user DaanHoogland commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1545#discussion_r63848658 --- Diff: plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParserTest.java --- @@ -144,7 +152,7 @@ public void

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-19 Thread DaanHoogland
Github user DaanHoogland commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1545#discussion_r63848326 --- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java --- @@ -1209,25 +1209,95 @@ public String toString() {

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-19 Thread DaanHoogland
Github user DaanHoogland commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1545#discussion_r63848237 --- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java --- @@ -1209,25 +1209,95 @@ public String toString() {

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-19 Thread DaanHoogland
Github user DaanHoogland commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1545#discussion_r63848121 --- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java --- @@ -1209,25 +1209,95 @@ public String toString() {

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-19 Thread DaanHoogland
Github user DaanHoogland commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1545#discussion_r63847694 --- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java --- @@ -1978,11 +1978,16 @@ So if

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-19 Thread wido
Github user wido commented on the pull request: https://github.com/apache/cloudstack/pull/1545#issuecomment-220263263 @rhtyd and @DaanHoogland Could you maybe take a quick look at this one? We could finally get the first part of this in. --- If your project is set up for it, you can

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-13 Thread swill
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1545#issuecomment-219168754 ok, thank you sir. 👍 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-13 Thread wido
Github user wido commented on the pull request: https://github.com/apache/cloudstack/pull/1545#issuecomment-219168529 @swill I added a small fix for inside the SSVM. During some additional manual testing I found that the SSVM is picky in the order in the XML definition. My

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-13 Thread wido
Github user wido commented on the pull request: https://github.com/apache/cloudstack/pull/1545#issuecomment-219152627 @swill Yes, running this in production on our cloud I just started an Instance and the Agent (backport to 4.8, clean merge) produced this XML: ```

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-13 Thread swill
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1545#issuecomment-219104928 Ya, I like this. I think this is a nice advancement. If we are going to try to get this into 4.9, I will need some help getting this moving forward cause we are

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-13 Thread wido
Github user wido commented on the pull request: https://github.com/apache/cloudstack/pull/1545#issuecomment-219034798 @swill and @rhtyd could you take a look at this one? Once we fork libvirt-java (which I want to do when we switch to new repos) we can implement the actual

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-13 Thread wido
Github user wido commented on the pull request: https://github.com/apache/cloudstack/pull/1545#issuecomment-218998413 This PR follows up on #985, but this one does not have a libvirt-java dependency. It only adds the channel in the XML for all Instances and it installs the

[GitHub] cloudstack pull request: CLOUDSTACK-8715: Add channel to Instances...

2016-05-13 Thread wido
GitHub user wido opened a pull request: https://github.com/apache/cloudstack/pull/1545 CLOUDSTACK-8715: Add channel to Instances for Qemu Guest Agent This commit adds a additional VirtIO channel with the name 'org.qemu.guest_agent.0' to all Instances. With the Qemu