You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Anders Lannerbäck <an...@redbridge.se> on 2014/05/26 14:25:09 UTC

Review Request 21908: Fix for CLOUDSTACK-6464

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21908/
-----------------------------------------------------------

Review request for cloudstack.


Repository: cloudstack-git


Description
-------

Fix for CLOUDSTACK-6464.  This patch is against 4.3-branch.

The original code adds broadcastUri on the format "vlan://100", but later looks for HashMap keys without the "vlan://" bit.  This causes new interfaces be created with duplicate MACs and the routers become unusable.


Diffs
-----

  plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 36382e3 

Diff: https://reviews.apache.org/r/21908/diff/


Testing
-------

Used to repair our production Cloudstack instance.


Thanks,

Anders Lannerbäck


Re: Review Request 21908: Fix for CLOUDSTACK-6464

Posted by Marcus Sorensen <sh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21908/#review44634
-----------------------------------------------------------


Actually, this would be fixed by changing the upgraded db to have vlan:// in the vlan_id table, as fresh installs do. This bug exists because the IpAssocCommand crafted by the management server sends the ips broadcastUri in a different format from the network/nic setups. We're actually discussing how to fix that right now, and whether it makes sense to update the db during upgrade to match what 4.3+ does currently, or change 4.3+ back.

- Marcus Sorensen


On May 26, 2014, 12:25 p.m., Anders Lannerbäck wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21908/
> -----------------------------------------------------------
> 
> (Updated May 26, 2014, 12:25 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix for CLOUDSTACK-6464.  This patch is against 4.3-branch.
> 
> The original code adds broadcastUri on the format "vlan://100", but later looks for HashMap keys without the "vlan://" bit.  This causes new interfaces be created with duplicate MACs and the routers become unusable.
> 
> 
> Diffs
> -----
> 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 36382e3 
> 
> Diff: https://reviews.apache.org/r/21908/diff/
> 
> 
> Testing
> -------
> 
> Used to repair our production Cloudstack instance.
> 
> 
> Thanks,
> 
> Anders Lannerbäck
> 
>


Re: Review Request 21908: Fix for CLOUDSTACK-6464

Posted by ASF Subversion and Git Services <as...@urd.zones.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21908/#review44664
-----------------------------------------------------------


Commit 15385948dcdf4c69136e99bf3c602f95fd018f39 in cloudstack's branch refs/heads/4.3-forward from Edison Su
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=1538594 ]

CLOUDSTACK-6464: if guest network type is vlan://untagged, and traffic label is used, kvm agent needs to honor traffic label


- ASF Subversion and Git Services


On May 26, 2014, 12:25 p.m., Anders Lannerbäck wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21908/
> -----------------------------------------------------------
> 
> (Updated May 26, 2014, 12:25 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix for CLOUDSTACK-6464.  This patch is against 4.3-branch.
> 
> The original code adds broadcastUri on the format "vlan://100", but later looks for HashMap keys without the "vlan://" bit.  This causes new interfaces be created with duplicate MACs and the routers become unusable.
> 
> 
> Diffs
> -----
> 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 36382e3 
> 
> Diff: https://reviews.apache.org/r/21908/diff/
> 
> 
> Testing
> -------
> 
> Used to repair our production Cloudstack instance.
> 
> 
> Thanks,
> 
> Anders Lannerbäck
> 
>


Re: Review Request 21908: Fix for CLOUDSTACK-6464

Posted by Anders Lannerbäck <an...@redbridge.se>.

> On June 4, 2014, 2:29 p.m., Marcus Sorensen wrote:
> > let's be extra careful not to apply this patch now that the other fixes are in. This patch will break fresh 4.3 installs and is incompatible with the fixes for the upgrades.

Yes, this patch should be rejected now that a better fix is in.


- Anders


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21908/#review44711
-----------------------------------------------------------


On May 26, 2014, 12:25 p.m., Anders Lannerbäck wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21908/
> -----------------------------------------------------------
> 
> (Updated May 26, 2014, 12:25 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix for CLOUDSTACK-6464.  This patch is against 4.3-branch.
> 
> The original code adds broadcastUri on the format "vlan://100", but later looks for HashMap keys without the "vlan://" bit.  This causes new interfaces be created with duplicate MACs and the routers become unusable.
> 
> 
> Diffs
> -----
> 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 36382e3 
> 
> Diff: https://reviews.apache.org/r/21908/diff/
> 
> 
> Testing
> -------
> 
> Used to repair our production Cloudstack instance.
> 
> 
> Thanks,
> 
> Anders Lannerbäck
> 
>


Re: Review Request 21908: Fix for CLOUDSTACK-6464

Posted by Marcus Sorensen <sh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21908/#review44711
-----------------------------------------------------------


let's be extra careful not to apply this patch now that the other fixes are in. This patch will break fresh 4.3 installs and is incompatible with the fixes for the upgrades.

- Marcus Sorensen


On May 26, 2014, 12:25 p.m., Anders Lannerbäck wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21908/
> -----------------------------------------------------------
> 
> (Updated May 26, 2014, 12:25 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix for CLOUDSTACK-6464.  This patch is against 4.3-branch.
> 
> The original code adds broadcastUri on the format "vlan://100", but later looks for HashMap keys without the "vlan://" bit.  This causes new interfaces be created with duplicate MACs and the routers become unusable.
> 
> 
> Diffs
> -----
> 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 36382e3 
> 
> Diff: https://reviews.apache.org/r/21908/diff/
> 
> 
> Testing
> -------
> 
> Used to repair our production Cloudstack instance.
> 
> 
> Thanks,
> 
> Anders Lannerbäck
> 
>


Re: Review Request 21908: Fix for CLOUDSTACK-6464

Posted by daan Hoogland <da...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21908/#review43928
-----------------------------------------------------------


Anders, use the BroadcastDomainType from Networks.java for this.

- daan Hoogland


On May 26, 2014, 12:25 p.m., Anders Lannerbäck wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21908/
> -----------------------------------------------------------
> 
> (Updated May 26, 2014, 12:25 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix for CLOUDSTACK-6464.  This patch is against 4.3-branch.
> 
> The original code adds broadcastUri on the format "vlan://100", but later looks for HashMap keys without the "vlan://" bit.  This causes new interfaces be created with duplicate MACs and the routers become unusable.
> 
> 
> Diffs
> -----
> 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 36382e3 
> 
> Diff: https://reviews.apache.org/r/21908/diff/
> 
> 
> Testing
> -------
> 
> Used to repair our production Cloudstack instance.
> 
> 
> Thanks,
> 
> Anders Lannerbäck
> 
>


Re: Review Request 21908: Fix for CLOUDSTACK-6464

Posted by ASF Subversion and Git Services <as...@urd.zones.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21908/#review44665
-----------------------------------------------------------


Commit dfb59cd6cc0292a88cb619e53f34cdb713879ffd in cloudstack's branch refs/heads/4.4-forward from Edison Su
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=dfb59cd ]

CLOUDSTACK-6464: if guest network type is vlan://untagged, and traffic label is used, kvm agent needs to honor traffic label


- ASF Subversion and Git Services


On May 26, 2014, 12:25 p.m., Anders Lannerbäck wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21908/
> -----------------------------------------------------------
> 
> (Updated May 26, 2014, 12:25 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix for CLOUDSTACK-6464.  This patch is against 4.3-branch.
> 
> The original code adds broadcastUri on the format "vlan://100", but later looks for HashMap keys without the "vlan://" bit.  This causes new interfaces be created with duplicate MACs and the routers become unusable.
> 
> 
> Diffs
> -----
> 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 36382e3 
> 
> Diff: https://reviews.apache.org/r/21908/diff/
> 
> 
> Testing
> -------
> 
> Used to repair our production Cloudstack instance.
> 
> 
> Thanks,
> 
> Anders Lannerbäck
> 
>


Re: Review Request 21908: Fix for CLOUDSTACK-6464

Posted by Marcus Sorensen <sh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21908/#review44635
-----------------------------------------------------------


I would vote to reject this patch, by the way, on the terms that it just works around the inconsistency in the data being passed. I'm glad that you were able to find and use it as an interim solution, though.

- Marcus Sorensen


On May 26, 2014, 12:25 p.m., Anders Lannerbäck wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21908/
> -----------------------------------------------------------
> 
> (Updated May 26, 2014, 12:25 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix for CLOUDSTACK-6464.  This patch is against 4.3-branch.
> 
> The original code adds broadcastUri on the format "vlan://100", but later looks for HashMap keys without the "vlan://" bit.  This causes new interfaces be created with duplicate MACs and the routers become unusable.
> 
> 
> Diffs
> -----
> 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 36382e3 
> 
> Diff: https://reviews.apache.org/r/21908/diff/
> 
> 
> Testing
> -------
> 
> Used to repair our production Cloudstack instance.
> 
> 
> Thanks,
> 
> Anders Lannerbäck
> 
>


Re: Review Request 21908: Fix for CLOUDSTACK-6464

Posted by ASF Subversion and Git Services <as...@urd.zones.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21908/#review44713
-----------------------------------------------------------


Commit 91391e6779bd25b91a3ca4d014656e24592187eb in cloudstack's branch refs/heads/4.4 from Edison Su
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=91391e6 ]

CLOUDSTACK-6464: if guest network type is vlan://untagged, and traffic label is used, kvm agent needs to honor traffic label

(cherry picked from commit dfb59cd6cc0292a88cb619e53f34cdb713879ffd)


- ASF Subversion and Git Services


On May 26, 2014, 12:25 p.m., Anders Lannerbäck wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21908/
> -----------------------------------------------------------
> 
> (Updated May 26, 2014, 12:25 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix for CLOUDSTACK-6464.  This patch is against 4.3-branch.
> 
> The original code adds broadcastUri on the format "vlan://100", but later looks for HashMap keys without the "vlan://" bit.  This causes new interfaces be created with duplicate MACs and the routers become unusable.
> 
> 
> Diffs
> -----
> 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 36382e3 
> 
> Diff: https://reviews.apache.org/r/21908/diff/
> 
> 
> Testing
> -------
> 
> Used to repair our production Cloudstack instance.
> 
> 
> Thanks,
> 
> Anders Lannerbäck
> 
>


Re: Review Request 21908: Fix for CLOUDSTACK-6464

Posted by ASF Subversion and Git Services <as...@urd.zones.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21908/#review44778
-----------------------------------------------------------


Commit 15385948dcdf4c69136e99bf3c602f95fd018f39 in cloudstack's branch refs/heads/4.3 from Edison Su
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=1538594 ]

CLOUDSTACK-6464: if guest network type is vlan://untagged, and traffic label is used, kvm agent needs to honor traffic label


- ASF Subversion and Git Services


On May 26, 2014, 12:25 p.m., Anders Lannerbäck wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21908/
> -----------------------------------------------------------
> 
> (Updated May 26, 2014, 12:25 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Fix for CLOUDSTACK-6464.  This patch is against 4.3-branch.
> 
> The original code adds broadcastUri on the format "vlan://100", but later looks for HashMap keys without the "vlan://" bit.  This causes new interfaces be created with duplicate MACs and the routers become unusable.
> 
> 
> Diffs
> -----
> 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 36382e3 
> 
> Diff: https://reviews.apache.org/r/21908/diff/
> 
> 
> Testing
> -------
> 
> Used to repair our production Cloudstack instance.
> 
> 
> Thanks,
> 
> Anders Lannerbäck
> 
>