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

Ship it!


Thanks Bill, thank you for your patch. 
Applied on master; (pl. test and let me know if this works, I cleaned up some 
code, see the commit after yours)

commit 1ae2d720a3ca3bde2baacb0d06610e7466f95bfe
Author: Bill Rich <bill.r...@gmail.com>
Date:   Fri Dec 7 08:39:13 2012 -0800

    CLOUDSTACK-591: Changed bridge name parsing in security_group.py to support 
bridges named with dashes

I removed the list pop-ing which makes no sense and instead of getting and 
splitting on newline and take the first element from the list, I fixed the 
execute statement to take the first from the list, using head -1:

commit 6f29317a8492008d37c7eb0770f0cee650c14c40
Author: Rohit Yadav <bhais...@apache.org>
Date:   Thu Dec 13 15:26:05 2012 -0800

    CLOUDSTACK-591: Fix execute and string processing logic for reboot_vm in 
security_group
    
    - Since we're always getting the first from the list, use head -1 to get 
the first
      of the results instead of processing again
    - Remove unecessay pop (why was it even there)
    
    Signed-off-by: Rohit Yadav <bhais...@apache.org>

- Rohit Yadav


On Dec. 7, 2012, 5:21 p.m., Bill Rich wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8406/
> -----------------------------------------------------------
> 
> (Updated Dec. 7, 2012, 5:21 p.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Description
> -------
> 
> When checking for rebooted VMs, security_group.py assumed bridge names would 
> not include dashed. The original code split the name found in iptables and 
> used the 2nd member of the returned array. In a case where the bridge had a 
> hyphen in the name, only a partial name was returned. For example, with a 
> bridge named br-public, the line 'iptables-save  |grep physdev-is-bridged 
> |grep FORWARD |grep BF |grep '\-o' |awk '{print $9}'' returns BF-br-public. 
> The name is parsed by splitting the name by hyphens and taking the 2nd member 
> of the array. This returns br resulting in the script attempting to modify 
> chains 'BF-br-IN' and 'BF-br-OUT' which don't exist.
> 
> I changed the code to use regular expressions to remove "^BF-" from the chain 
> name. This will consistently remove just the unwanted part of the chain name 
> to get the standard brname used throughout the rest of the script.
> 
> 
> This addresses bug CLOUDSTACK-591.
> 
> 
> Diffs
> -----
> 
>   scripts/vm/network/security_group.py b079890 
> 
> Diff: https://reviews.apache.org/r/8406/diff/
> 
> 
> Testing
> -------
> 
> -Modified /var/run/cloud/<VM>.log on hv to include a different ID. 
> -Waited for security_group.py get_rule_logs_for_vms to be run.
> -Checked /var/log/cloud/security_group.log for errors and that the correct 
> information was being parsed
> -Confirmed network connectivity for VM
> 
> 
> Thanks,
> 
> Bill Rich
> 
>

Reply via email to