Uh oh!
There was an error while loading. Please reload this page.
CLOUDSTACK-8940: Wrong value is inserted into nics table netmask fiel… - #916
Conversation
ke4qqq
commented
Oct 9, 2015
So several questions:
|
yadvr
commented
Oct 10, 2015
@kansal LGTM. Looking at the JIRA issue, this looks like should apply to 4.6/master but can you update the ticket with the version details, more details on the bug and also note if this also needs to be backported to 4.5/4.4 or older branches. Thanks. |
kansal
commented
Oct 12, 2015
@bhaisaab Sure will check about the back-porting of the issue. Apart from that, the previous values stored incorrectly need to be corrected in the database. I think a simple bash script will do. Your views and will it be part of this PR only? |
yadvr
commented
Oct 12, 2015
Use db migration from either past 4.5.x to 4.6.0 version, or this PR gets merged post 4.6.0; add the data migration code in 4.6.0 to 4.6.1 |
remibergsma
commented
Oct 25, 2015
Hi @kansal Can you please rebase this with current master? I tried running some tests but face old problems. Also, what about the db migration script to fix old problems? Thanks for the update! |
remibergsma
commented
Oct 27, 2015
LGTM, based on a set of tests that I run on this branch (which I rebased myself first): Result: The 3 errors at the bottom are due to CLOUDSTACK-8991 and unrelated to this PR. Please try running Trying again the other failed test: Result: So, must have been a local issue. Next test run: Result: Before we consider merging, please respond to the comment about the upgrade script. Thanks! |
kansal
commented
Oct 27, 2015
@remibergsma Thank you very much. I was busy somewhere. Will update this PR with "update script" and rebase it. Sorry for the delayed response. |
kansal
commented
Oct 29, 2015
@remibergsma@bhaisaab The above changes were made as the part of DHCP/DNS offload feature. The related ticket is https://issues.apache.org/jira/browse/CLOUDSTACK-8324 |
…d when creating a VM - Fixed
wilderrodrigues
commented
Oct 29, 2015
Hi @kansal Should I deploy a basic zone with shared network in order to see if that fix is fine? I will also run the other set of tests, same as @remibergsma did. Cheers, |
DaanHoogland
commented
Oct 29, 2015
code lgtm @remibergsma can you retest? A newer commit is in since your lgtm |
kansal
commented
Oct 30, 2015
Hi @wilderrodrigues, To check the bug is solved, try deploying the VM in shared network with no service. As you can see, this function is called only when is isSharedNetworkWithoutService() is true. |
wilderrodrigues
commented
Oct 30, 2015
Ping @remibergsma@karuturi@kansal I will run the hardware required tests as well and try to deployed a VM with a shared network. Partial test results:
Cheers, |
remibergsma
commented
Oct 30, 2015
LGTM, based on a set of tests that I run on this branch (which I rebased myself first): Result: And: Result: These test do not cover your change, all they do is show you didn't break them. Someone else needs to review the code. |
remibergsma
commented
Oct 30, 2015
@wilderrodrigues any update on your review? |
wilderrodrigues
commented
Nov 2, 2015
LGTM 👍 Please proceed with merge. |
CLOUDSTACK-8940: Wrong value is inserted into nics table netmask field when creating a VM - Fixed Problem: When creating a VM in shared network with no service, the value of netmask is added in the table in the CIDR format unlike other cases where it is added as normal string in the format xxx.xxx.xxx.xxx. The netmask column in the nics table has a length of 15 chars which gets violated if the CIDR exceeds it(Max CIDR length can be 18). Fix: Before storing the netmask convert from CIDR to native format. * pr/916: CLOUDSTACK-8940: Wrong value is inserted into nics table netmask field when creating a VM - Fixed Signed-off-by: Remi Bergsma <github@remi.nl>
By switching to this directory we can use relative paths regardless of how people execute the script. cd tools ./docker.sh OR ./tools/docker.sh OR /path/to/git-repo/tools/docker.sh Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

…d when creating a VM - Fixed
Problem: When creating a VM in shared network with no service, the value of netmask is added in the table in the CIDR format unlike other cases where it is added as normal string in the format xxx.xxx.xxx.xxx. The netmask column in the nics table has a length of 15 chars which gets violated if the CIDR exceeds it(Max CIDR length can be 18).
Fix: Before storing the netmask convert from CIDR to native format.