You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by bharat kumar <bh...@citrix.com> on 2013/07/22 14:50:58 UTC
Review Request 12810: DnsMasqConfigurator need to be rewrite in bash script
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12810/
-----------------------------------------------------------
Review request for cloudstack, Alena Prokharchyk and Sheng Yang.
Bugs: CLOUDSTACK-3694
Repository: cloudstack-git
Description
-------
https://issues.apache.org/jira/browse/CLOUDSTACK-3694
wrote the dnsmasq config in bash instead of creating the config file in java and overwriting.
Diffs
-----
core/src/com/cloud/agent/api/routing/DnsMasqConfigCommand.java 521ad70
core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java 0b26220
core/src/com/cloud/network/DnsMasqConfigurator.java 3fc61df
patches/systemvm/debian/config/root/createIpAlias.sh 5498195
patches/systemvm/debian/config/root/dnsmasq.sh b70e2d3
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java c7f487e
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java f80d4b6
scripts/vm/hypervisor/xenserver/vmops f8c0253
server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 6c0f7a1
Diff: https://reviews.apache.org/r/12810/diff/
Testing
-------
Tested on old master using xenserver.
Could not test on the latest one as it is broken.
Thanks,
bharat kumar
Re: Review Request 12810: DnsMasqConfigurator need to be rewrite in bash
script
Posted by "Jenkins Cloudstack.org" <hu...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12810/#review23629
-----------------------------------------------------------
Review 12810 PASSED the build test
The url of build cloudstack-master-with-patch #11 is : http://jenkins.cloudstack.org/job/cloudstack-master-with-patch/11/
- Jenkins Cloudstack.org
On July 22, 2013, 12:54 p.m., bharat kumar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12810/
> -----------------------------------------------------------
>
> (Updated July 22, 2013, 12:54 p.m.)
>
>
> Review request for cloudstack, Alena Prokharchyk and Sheng Yang.
>
>
> Bugs: CLOUDSTACK-3694
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/CLOUDSTACK-3694
>
> wrote the dnsmasq config in bash instead of creating the config file in java and overwriting.
>
>
> Diffs
> -----
>
> core/src/com/cloud/agent/api/routing/DnsMasqConfigCommand.java 521ad70
> core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java 0b26220
> core/src/com/cloud/network/DnsMasqConfigurator.java 3fc61df
> patches/systemvm/debian/config/root/createIpAlias.sh 5498195
> patches/systemvm/debian/config/root/dnsmasq.sh b70e2d3
> plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java c7f487e
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java f80d4b6
> scripts/vm/hypervisor/xenserver/vmops f8c0253
> server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 6c0f7a1
>
> Diff: https://reviews.apache.org/r/12810/diff/
>
>
> Testing
> -------
>
> Tested on old master using xenserver.
> Could not test on the latest one as it is broken.
>
>
> Thanks,
>
> bharat kumar
>
>
Re: Review Request 12810: DnsMasqConfigurator need to be rewrite in bash
script
Posted by Sheng Yang <sh...@yasker.org>.
> On July 22, 2013, 10:23 p.m., Sheng Yang wrote:
> > patches/systemvm/debian/config/root/dnsmasq.sh, line 54
> > <https://reviews.apache.org/r/12810/diff/2/?file=324528#file324528line54>
> >
> > You don't need so complicate way to find dns is enabled or not. Just check /var/cache/cloud/cmdline for entry " dns1", " dns2"(remember add leading space to avoid ip6dns)
> >
> > And if cmdline contained "useextdns=true", then dns1/dns2 should be used directly in such case. You don't need to check eth0 ip is dns or not.
> >
> > In fact I don't think you need to add dhcp-option=tag:xxx,6,xxx for dns server of every range you added. You should able to use the original dhcp-option=6,xxx to cover all the cases. It should by default cover everything.
>
> bharat kumar wrote:
> Hi sheng,
>
> in case if useextndns is not true we have to add the routerip corresponding to each subnet so we need to add the option like this dhcp-option=tag:xxx,6,xxx.
>
>
Yes, you're right. dhcp-option=6,xxx only apply to useextdns.
- Sheng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12810/#review23649
-----------------------------------------------------------
On July 23, 2013, 1:07 p.m., bharat kumar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12810/
> -----------------------------------------------------------
>
> (Updated July 23, 2013, 1:07 p.m.)
>
>
> Review request for cloudstack, Alena Prokharchyk and Sheng Yang.
>
>
> Bugs: CLOUDSTACK-3694
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/CLOUDSTACK-3694
>
> wrote the dnsmasq config in bash instead of creating the config file in java and overwriting.
>
>
> Diffs
> -----
>
> core/src/com/cloud/agent/api/routing/DnsMasqConfigCommand.java 521ad70
> core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java 0b26220
> core/src/com/cloud/network/DnsMasqConfigurator.java 3fc61df
> patches/systemvm/debian/config/etc/init.d/cloud-early-config c04ff90
> patches/systemvm/debian/config/root/dnsmasq.sh b70e2d3
> plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java c7f487e
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java f80d4b6
> scripts/vm/hypervisor/xenserver/vmops f8c0253
> server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 6c0f7a1
>
> Diff: https://reviews.apache.org/r/12810/diff/
>
>
> Testing
> -------
>
> Tested on old master using xenserver.
> Could not test on the latest one as it is broken.
>
>
> Thanks,
>
> bharat kumar
>
>
Re: Review Request 12810: DnsMasqConfigurator need to be rewrite in bash
script
Posted by bharat kumar <bh...@citrix.com>.
> On July 22, 2013, 10:23 p.m., Sheng Yang wrote:
> > patches/systemvm/debian/config/root/dnsmasq.sh, line 54
> > <https://reviews.apache.org/r/12810/diff/2/?file=324528#file324528line54>
> >
> > You don't need so complicate way to find dns is enabled or not. Just check /var/cache/cloud/cmdline for entry " dns1", " dns2"(remember add leading space to avoid ip6dns)
> >
> > And if cmdline contained "useextdns=true", then dns1/dns2 should be used directly in such case. You don't need to check eth0 ip is dns or not.
> >
> > In fact I don't think you need to add dhcp-option=tag:xxx,6,xxx for dns server of every range you added. You should able to use the original dhcp-option=6,xxx to cover all the cases. It should by default cover everything.
Hi sheng,
in case if useextndns is not true we have to add the routerip corresponding to each subnet so we need to add the option like this dhcp-option=tag:xxx,6,xxx.
- bharat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12810/#review23649
-----------------------------------------------------------
On July 22, 2013, 12:54 p.m., bharat kumar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12810/
> -----------------------------------------------------------
>
> (Updated July 22, 2013, 12:54 p.m.)
>
>
> Review request for cloudstack, Alena Prokharchyk and Sheng Yang.
>
>
> Bugs: CLOUDSTACK-3694
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/CLOUDSTACK-3694
>
> wrote the dnsmasq config in bash instead of creating the config file in java and overwriting.
>
>
> Diffs
> -----
>
> core/src/com/cloud/agent/api/routing/DnsMasqConfigCommand.java 521ad70
> core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java 0b26220
> core/src/com/cloud/network/DnsMasqConfigurator.java 3fc61df
> patches/systemvm/debian/config/root/createIpAlias.sh 5498195
> patches/systemvm/debian/config/root/dnsmasq.sh b70e2d3
> plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java c7f487e
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java f80d4b6
> scripts/vm/hypervisor/xenserver/vmops f8c0253
> server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 6c0f7a1
>
> Diff: https://reviews.apache.org/r/12810/diff/
>
>
> Testing
> -------
>
> Tested on old master using xenserver.
> Could not test on the latest one as it is broken.
>
>
> Thanks,
>
> bharat kumar
>
>
Re: Review Request 12810: DnsMasqConfigurator need to be rewrite in bash
script
Posted by Sheng Yang <sh...@yasker.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12810/#review23649
-----------------------------------------------------------
patches/systemvm/debian/config/root/dnsmasq.sh
<https://reviews.apache.org/r/12810/#comment47548>
You don't need so complicate way to find dns is enabled or not. Just check /var/cache/cloud/cmdline for entry " dns1", " dns2"(remember add leading space to avoid ip6dns)
And if cmdline contained "useextdns=true", then dns1/dns2 should be used directly in such case. You don't need to check eth0 ip is dns or not.
In fact I don't think you need to add dhcp-option=tag:xxx,6,xxx for dns server of every range you added. You should able to use the original dhcp-option=6,xxx to cover all the cases. It should by default cover everything.
patches/systemvm/debian/config/root/dnsmasq.sh
<https://reviews.apache.org/r/12810/#comment47547>
You should use "logger -t cloud xxxx" if you want to have any log for script. See other scripts for example.
patches/systemvm/debian/config/root/dnsmasq.sh
<https://reviews.apache.org/r/12810/#comment47552>
You'd better add log here, using "logger -t cloud", say you're clearing the old entries.
patches/systemvm/debian/config/root/dnsmasq.sh
<https://reviews.apache.org/r/12810/#comment47551>
I don't know dhcp-option has <set> keyword. I thought you meant dhcp-range but it's already cleared. So what here for?
patches/systemvm/debian/config/root/dnsmasq.sh
<https://reviews.apache.org/r/12810/#comment47556>
I think we can do it better by create an separate configuration file in e.g. /etc/dnsmasq.d/multiple_ranges.conf for your dhcp-range and related dhcp-option changed. The first range created by cloud-early-config can remain in dnsmasq.conf until DnsMasqConfig command clear it and setup separate ranges in the separate configuration file. You can check vpc_guestnw.sh for reference.
Do remember to clean the multiple_ranges.conf in cloud-early-config so it would be cleaned every time when VR reboot.
patches/systemvm/debian/config/root/dnsmasq.sh
<https://reviews.apache.org/r/12810/#comment47553>
For every change you added to dnsmasq.conf, please add log. It would be easier to tell what's wrong in product.
patches/systemvm/debian/config/root/dnsmasq.sh
<https://reviews.apache.org/r/12810/#comment47559>
Need logger -t rather than echo.
patches/systemvm/debian/config/root/dnsmasq.sh
<https://reviews.apache.org/r/12810/#comment47557>
This is wrong. It cannot fail silently. The return result should be $result, rather than $?.
should be:
unlock_exit $result $lock $locked
patches/systemvm/debian/config/root/dnsmasq.sh
<https://reviews.apache.org/r/12810/#comment47558>
Should be $result too.
- Sheng Yang
On July 22, 2013, 12:54 p.m., bharat kumar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12810/
> -----------------------------------------------------------
>
> (Updated July 22, 2013, 12:54 p.m.)
>
>
> Review request for cloudstack, Alena Prokharchyk and Sheng Yang.
>
>
> Bugs: CLOUDSTACK-3694
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/CLOUDSTACK-3694
>
> wrote the dnsmasq config in bash instead of creating the config file in java and overwriting.
>
>
> Diffs
> -----
>
> core/src/com/cloud/agent/api/routing/DnsMasqConfigCommand.java 521ad70
> core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java 0b26220
> core/src/com/cloud/network/DnsMasqConfigurator.java 3fc61df
> patches/systemvm/debian/config/root/createIpAlias.sh 5498195
> patches/systemvm/debian/config/root/dnsmasq.sh b70e2d3
> plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java c7f487e
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java f80d4b6
> scripts/vm/hypervisor/xenserver/vmops f8c0253
> server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 6c0f7a1
>
> Diff: https://reviews.apache.org/r/12810/diff/
>
>
> Testing
> -------
>
> Tested on old master using xenserver.
> Could not test on the latest one as it is broken.
>
>
> Thanks,
>
> bharat kumar
>
>
Re: Review Request 12810: DnsMasqConfigurator need to be rewrite in bash
script
Posted by Sheng Yang <sh...@yasker.org>.
> On July 24, 2013, 6:50 p.m., Sheng Yang wrote:
> > Nice work!
Fixed version pushed to 4.2 and MASTER.
- Sheng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12810/#review23779
-----------------------------------------------------------
On July 24, 2013, 10:28 a.m., bharat kumar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12810/
> -----------------------------------------------------------
>
> (Updated July 24, 2013, 10:28 a.m.)
>
>
> Review request for cloudstack, Alena Prokharchyk and Sheng Yang.
>
>
> Bugs: CLOUDSTACK-3694
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/CLOUDSTACK-3694
>
> wrote the dnsmasq config in bash instead of creating the config file in java and overwriting.
>
>
> Diffs
> -----
>
> core/src/com/cloud/agent/api/routing/DnsMasqConfigCommand.java 521ad70
> core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java 0b26220
> core/src/com/cloud/network/DnsMasqConfigurator.java 3fc61df
> patches/systemvm/debian/config/etc/init.d/cloud-early-config c04ff90
> patches/systemvm/debian/config/root/dnsmasq.sh b70e2d3
> plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java c7f487e
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java f80d4b6
> scripts/vm/hypervisor/xenserver/vmops f8c0253
> server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 6c0f7a1
> server/src/com/cloud/vm/VirtualMachineManagerImpl.java 7a4bf50
>
> Diff: https://reviews.apache.org/r/12810/diff/
>
>
> Testing
> -------
>
> Tested on old master using xenserver.
> Could not test on the latest one as it is broken.
>
>
> Thanks,
>
> bharat kumar
>
>
Re: Review Request 12810: DnsMasqConfigurator need to be rewrite in
bash script
Posted by Bharat Kumar <bh...@citrix.com>.
Thank you Sheng.
Bharat.
On Jul 25, 2013, at 12:20 AM, Sheng Yang <sh...@yasker.org>> wrote:
This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12810/
Ship it!
Nice work!
patches/systemvm/debian/config/root/dnsmasq.sh<https://reviews.apache.org/r/12810/diff/4-5/?file=325449#file325449line59> (Diff revisions 4 - 5)
56
dns2=$(grep -o " dns2=.* " "$CMDLINE" | sed -e 's/dns2=//' | awk '{print $1}')
59
dns2=$(echo "CMDLINE" | grep -o " dns2=.* " | sed -e 's/dns2=//' | awk '{print $1}')
Typo here. I would fix it when check in the patch.
- Sheng Yang
On July 24th, 2013, 10:28 a.m. UTC, bharat kumar wrote:
Review request for cloudstack, Alena Prokharchyk and Sheng Yang.
By bharat kumar.
Updated July 24, 2013, 10:28 a.m.
Bugs: CLOUDSTACK-3694
Repository: cloudstack-git
Description
https://issues.apache.org/jira/browse/CLOUDSTACK-3694
wrote the dnsmasq config in bash instead of creating the config file in java and overwriting.
Testing
Tested on old master using xenserver.
Could not test on the latest one as it is broken.
Diffs
* core/src/com/cloud/agent/api/routing/DnsMasqConfigCommand.java (521ad70)
* core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java (0b26220)
* core/src/com/cloud/network/DnsMasqConfigurator.java (3fc61df)
* patches/systemvm/debian/config/etc/init.d/cloud-early-config (c04ff90)
* patches/systemvm/debian/config/root/dnsmasq.sh (b70e2d3)
* plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java (c7f487e)
* plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java (f80d4b6)
* scripts/vm/hypervisor/xenserver/vmops (f8c0253)
* server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java (6c0f7a1)
* server/src/com/cloud/vm/VirtualMachineManagerImpl.java (7a4bf50)
View Diff<https://reviews.apache.org/r/12810/diff/>
Re: Review Request 12810: DnsMasqConfigurator need to be rewrite in bash
script
Posted by Sheng Yang <sh...@yasker.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12810/#review23779
-----------------------------------------------------------
Ship it!
Nice work!
patches/systemvm/debian/config/root/dnsmasq.sh
<https://reviews.apache.org/r/12810/#comment47626>
Typo here. I would fix it when check in the patch.
- Sheng Yang
On July 24, 2013, 10:28 a.m., bharat kumar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12810/
> -----------------------------------------------------------
>
> (Updated July 24, 2013, 10:28 a.m.)
>
>
> Review request for cloudstack, Alena Prokharchyk and Sheng Yang.
>
>
> Bugs: CLOUDSTACK-3694
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/CLOUDSTACK-3694
>
> wrote the dnsmasq config in bash instead of creating the config file in java and overwriting.
>
>
> Diffs
> -----
>
> core/src/com/cloud/agent/api/routing/DnsMasqConfigCommand.java 521ad70
> core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java 0b26220
> core/src/com/cloud/network/DnsMasqConfigurator.java 3fc61df
> patches/systemvm/debian/config/etc/init.d/cloud-early-config c04ff90
> patches/systemvm/debian/config/root/dnsmasq.sh b70e2d3
> plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java c7f487e
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java f80d4b6
> scripts/vm/hypervisor/xenserver/vmops f8c0253
> server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 6c0f7a1
> server/src/com/cloud/vm/VirtualMachineManagerImpl.java 7a4bf50
>
> Diff: https://reviews.apache.org/r/12810/diff/
>
>
> Testing
> -------
>
> Tested on old master using xenserver.
> Could not test on the latest one as it is broken.
>
>
> Thanks,
>
> bharat kumar
>
>
Re: Review Request 12810: DnsMasqConfigurator need to be rewrite in bash
script
Posted by bharat kumar <bh...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12810/
-----------------------------------------------------------
(Updated July 24, 2013, 10:28 a.m.)
Review request for cloudstack, Alena Prokharchyk and Sheng Yang.
Changes
-------
Fixed the issues pointed out in the review. Also added a fix to handle vms with multiple nics.
Bugs: CLOUDSTACK-3694
Repository: cloudstack-git
Description
-------
https://issues.apache.org/jira/browse/CLOUDSTACK-3694
wrote the dnsmasq config in bash instead of creating the config file in java and overwriting.
Diffs (updated)
-----
core/src/com/cloud/agent/api/routing/DnsMasqConfigCommand.java 521ad70
core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java 0b26220
core/src/com/cloud/network/DnsMasqConfigurator.java 3fc61df
patches/systemvm/debian/config/etc/init.d/cloud-early-config c04ff90
patches/systemvm/debian/config/root/dnsmasq.sh b70e2d3
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java c7f487e
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java f80d4b6
scripts/vm/hypervisor/xenserver/vmops f8c0253
server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 6c0f7a1
server/src/com/cloud/vm/VirtualMachineManagerImpl.java 7a4bf50
Diff: https://reviews.apache.org/r/12810/diff/
Testing
-------
Tested on old master using xenserver.
Could not test on the latest one as it is broken.
Thanks,
bharat kumar
Re: Review Request 12810: DnsMasqConfigurator need to be rewrite in bash
script
Posted by bharat kumar <bh...@citrix.com>.
> On July 23, 2013, 8:51 p.m., Sheng Yang wrote:
> > patches/systemvm/debian/config/root/dnsmasq.sh, line 100
> > <https://reviews.apache.org/r/12810/diff/4/?file=325449#file325449line100>
> >
> > dhcp-option=tag.* is not in the log
the dhcp-option=tag* will be there in the new config. I am not logging it as it is not a part of the config which i am removing from /etc/dnsmasq.conf.
- bharat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12810/#review23713
-----------------------------------------------------------
On July 23, 2013, 1:07 p.m., bharat kumar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12810/
> -----------------------------------------------------------
>
> (Updated July 23, 2013, 1:07 p.m.)
>
>
> Review request for cloudstack, Alena Prokharchyk and Sheng Yang.
>
>
> Bugs: CLOUDSTACK-3694
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/CLOUDSTACK-3694
>
> wrote the dnsmasq config in bash instead of creating the config file in java and overwriting.
>
>
> Diffs
> -----
>
> core/src/com/cloud/agent/api/routing/DnsMasqConfigCommand.java 521ad70
> core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java 0b26220
> core/src/com/cloud/network/DnsMasqConfigurator.java 3fc61df
> patches/systemvm/debian/config/etc/init.d/cloud-early-config c04ff90
> patches/systemvm/debian/config/root/dnsmasq.sh b70e2d3
> plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java c7f487e
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java f80d4b6
> scripts/vm/hypervisor/xenserver/vmops f8c0253
> server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 6c0f7a1
>
> Diff: https://reviews.apache.org/r/12810/diff/
>
>
> Testing
> -------
>
> Tested on old master using xenserver.
> Could not test on the latest one as it is broken.
>
>
> Thanks,
>
> bharat kumar
>
>
Re: Review Request 12810: DnsMasqConfigurator need to be rewrite in bash
script
Posted by bharat kumar <bh...@citrix.com>.
> On July 23, 2013, 8:51 p.m., Sheng Yang wrote:
> > patches/systemvm/debian/config/root/dnsmasq.sh, line 100
> > <https://reviews.apache.org/r/12810/diff/4/?file=325449#file325449line100>
> >
> > dhcp-option=tag.* is not in the log
>
> bharat kumar wrote:
> the dhcp-option=tag* will be there in the new config. I am not logging it as it is not a part of the config which i am removing from /etc/dnsmasq.conf.
>
> Sheng Yang wrote:
> Not quite understand... All your new config should be in /etc/dnsmasq.d/multiple_ranges.conf rather than /etc/dnsmasq.conf. Why there are any dhcp-options=tag* in dnsmasq.conf?
Sheng you are right. This setting is not there in the dnsmasq.conf. so this line is not needed. I will remove it.
- bharat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12810/#review23713
-----------------------------------------------------------
On July 23, 2013, 1:07 p.m., bharat kumar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12810/
> -----------------------------------------------------------
>
> (Updated July 23, 2013, 1:07 p.m.)
>
>
> Review request for cloudstack, Alena Prokharchyk and Sheng Yang.
>
>
> Bugs: CLOUDSTACK-3694
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/CLOUDSTACK-3694
>
> wrote the dnsmasq config in bash instead of creating the config file in java and overwriting.
>
>
> Diffs
> -----
>
> core/src/com/cloud/agent/api/routing/DnsMasqConfigCommand.java 521ad70
> core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java 0b26220
> core/src/com/cloud/network/DnsMasqConfigurator.java 3fc61df
> patches/systemvm/debian/config/etc/init.d/cloud-early-config c04ff90
> patches/systemvm/debian/config/root/dnsmasq.sh b70e2d3
> plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java c7f487e
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java f80d4b6
> scripts/vm/hypervisor/xenserver/vmops f8c0253
> server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 6c0f7a1
>
> Diff: https://reviews.apache.org/r/12810/diff/
>
>
> Testing
> -------
>
> Tested on old master using xenserver.
> Could not test on the latest one as it is broken.
>
>
> Thanks,
>
> bharat kumar
>
>
Re: Review Request 12810: DnsMasqConfigurator need to be rewrite in bash
script
Posted by Sheng Yang <sh...@yasker.org>.
> On July 23, 2013, 8:51 p.m., Sheng Yang wrote:
> > patches/systemvm/debian/config/root/dnsmasq.sh, line 100
> > <https://reviews.apache.org/r/12810/diff/4/?file=325449#file325449line100>
> >
> > dhcp-option=tag.* is not in the log
>
> bharat kumar wrote:
> the dhcp-option=tag* will be there in the new config. I am not logging it as it is not a part of the config which i am removing from /etc/dnsmasq.conf.
Not quite understand... All your new config should be in /etc/dnsmasq.d/multiple_ranges.conf rather than /etc/dnsmasq.conf. Why there are any dhcp-options=tag* in dnsmasq.conf?
- Sheng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12810/#review23713
-----------------------------------------------------------
On July 23, 2013, 1:07 p.m., bharat kumar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12810/
> -----------------------------------------------------------
>
> (Updated July 23, 2013, 1:07 p.m.)
>
>
> Review request for cloudstack, Alena Prokharchyk and Sheng Yang.
>
>
> Bugs: CLOUDSTACK-3694
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/CLOUDSTACK-3694
>
> wrote the dnsmasq config in bash instead of creating the config file in java and overwriting.
>
>
> Diffs
> -----
>
> core/src/com/cloud/agent/api/routing/DnsMasqConfigCommand.java 521ad70
> core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java 0b26220
> core/src/com/cloud/network/DnsMasqConfigurator.java 3fc61df
> patches/systemvm/debian/config/etc/init.d/cloud-early-config c04ff90
> patches/systemvm/debian/config/root/dnsmasq.sh b70e2d3
> plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java c7f487e
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java f80d4b6
> scripts/vm/hypervisor/xenserver/vmops f8c0253
> server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 6c0f7a1
>
> Diff: https://reviews.apache.org/r/12810/diff/
>
>
> Testing
> -------
>
> Tested on old master using xenserver.
> Could not test on the latest one as it is broken.
>
>
> Thanks,
>
> bharat kumar
>
>
Re: Review Request 12810: DnsMasqConfigurator need to be rewrite in bash
script
Posted by Sheng Yang <sh...@yasker.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12810/#review23713
-----------------------------------------------------------
patches/systemvm/debian/config/root/dnsmasq.sh
<https://reviews.apache.org/r/12810/#comment47591>
dns2 can be null.
patches/systemvm/debian/config/root/dnsmasq.sh
<https://reviews.apache.org/r/12810/#comment47592>
dhcp-option=tag.* is not in the log
patches/systemvm/debian/config/root/dnsmasq.sh
<https://reviews.apache.org/r/12810/#comment47594>
Backup the failure configures would also help to investigate in the future.
patches/systemvm/debian/config/root/dnsmasq.sh
<https://reviews.apache.org/r/12810/#comment47593>
No, don't override the original result. It failed, then we need to fail loudly. The overriding here would cover the fact that it's failed to apply the new config.
It's in much better shape than previous edition. Please do more testing, e.g. VPC, multiple guest network for user VM. It's likely we can get it in tomorrow.
Thanks!
- Sheng Yang
On July 23, 2013, 1:07 p.m., bharat kumar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12810/
> -----------------------------------------------------------
>
> (Updated July 23, 2013, 1:07 p.m.)
>
>
> Review request for cloudstack, Alena Prokharchyk and Sheng Yang.
>
>
> Bugs: CLOUDSTACK-3694
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/CLOUDSTACK-3694
>
> wrote the dnsmasq config in bash instead of creating the config file in java and overwriting.
>
>
> Diffs
> -----
>
> core/src/com/cloud/agent/api/routing/DnsMasqConfigCommand.java 521ad70
> core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java 0b26220
> core/src/com/cloud/network/DnsMasqConfigurator.java 3fc61df
> patches/systemvm/debian/config/etc/init.d/cloud-early-config c04ff90
> patches/systemvm/debian/config/root/dnsmasq.sh b70e2d3
> plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java c7f487e
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java f80d4b6
> scripts/vm/hypervisor/xenserver/vmops f8c0253
> server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 6c0f7a1
>
> Diff: https://reviews.apache.org/r/12810/diff/
>
>
> Testing
> -------
>
> Tested on old master using xenserver.
> Could not test on the latest one as it is broken.
>
>
> Thanks,
>
> bharat kumar
>
>
Re: Review Request 12810: DnsMasqConfigurator need to be rewrite in bash
script
Posted by bharat kumar <bh...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12810/
-----------------------------------------------------------
(Updated July 23, 2013, 1:07 p.m.)
Review request for cloudstack, Alena Prokharchyk and Sheng Yang.
Changes
-------
added the remove multiple_ranges.conf statement in cloud-early-config.
Bugs: CLOUDSTACK-3694
Repository: cloudstack-git
Description
-------
https://issues.apache.org/jira/browse/CLOUDSTACK-3694
wrote the dnsmasq config in bash instead of creating the config file in java and overwriting.
Diffs (updated)
-----
core/src/com/cloud/agent/api/routing/DnsMasqConfigCommand.java 521ad70
core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java 0b26220
core/src/com/cloud/network/DnsMasqConfigurator.java 3fc61df
patches/systemvm/debian/config/etc/init.d/cloud-early-config c04ff90
patches/systemvm/debian/config/root/dnsmasq.sh b70e2d3
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java c7f487e
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java f80d4b6
scripts/vm/hypervisor/xenserver/vmops f8c0253
server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 6c0f7a1
Diff: https://reviews.apache.org/r/12810/diff/
Testing
-------
Tested on old master using xenserver.
Could not test on the latest one as it is broken.
Thanks,
bharat kumar
Re: Review Request 12810: DnsMasqConfigurator need to be rewrite in bash
script
Posted by bharat kumar <bh...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12810/
-----------------------------------------------------------
(Updated July 23, 2013, 12:42 p.m.)
Review request for cloudstack, Alena Prokharchyk and Sheng Yang.
Changes
-------
Incorporated the review comments.
Bugs: CLOUDSTACK-3694
Repository: cloudstack-git
Description
-------
https://issues.apache.org/jira/browse/CLOUDSTACK-3694
wrote the dnsmasq config in bash instead of creating the config file in java and overwriting.
Diffs (updated)
-----
build/replace.properties 265f335
core/src/com/cloud/agent/api/routing/DnsMasqConfigCommand.java 521ad70
core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java 0b26220
core/src/com/cloud/network/DnsMasqConfigurator.java 3fc61df
patches/systemvm/debian/config/root/createIpAlias.sh 5498195
patches/systemvm/debian/config/root/dnsmasq.sh b70e2d3
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java c7f487e
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java f80d4b6
scripts/vm/hypervisor/xenserver/vmops f8c0253
server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 6c0f7a1
Diff: https://reviews.apache.org/r/12810/diff/
Testing
-------
Tested on old master using xenserver.
Could not test on the latest one as it is broken.
Thanks,
bharat kumar
Re: Review Request 12810: DnsMasqConfigurator need to be rewrite in bash
script
Posted by bharat kumar <bh...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12810/
-----------------------------------------------------------
(Updated July 22, 2013, 12:54 p.m.)
Review request for cloudstack, Alena Prokharchyk and Sheng Yang.
Changes
-------
Removed set -x from dnsmasq.sh
Bugs: CLOUDSTACK-3694
Repository: cloudstack-git
Description
-------
https://issues.apache.org/jira/browse/CLOUDSTACK-3694
wrote the dnsmasq config in bash instead of creating the config file in java and overwriting.
Diffs (updated)
-----
core/src/com/cloud/agent/api/routing/DnsMasqConfigCommand.java 521ad70
core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java 0b26220
core/src/com/cloud/network/DnsMasqConfigurator.java 3fc61df
patches/systemvm/debian/config/root/createIpAlias.sh 5498195
patches/systemvm/debian/config/root/dnsmasq.sh b70e2d3
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java c7f487e
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java f80d4b6
scripts/vm/hypervisor/xenserver/vmops f8c0253
server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 6c0f7a1
Diff: https://reviews.apache.org/r/12810/diff/
Testing
-------
Tested on old master using xenserver.
Could not test on the latest one as it is broken.
Thanks,
bharat kumar