CLOUDSTACK-9196: Fixing null pointer exception when vm meta data is synced on upgraded setup#1274
Conversation
…ynced on upgraded setup.
|
LGTM but please describe (preferable in a unit test) what situation is prevented from happening here. |
|
@DaanHoogland Updated the descrition |
|
Code changes LGTM |
|
Shouldn't we log this information? Doesn't this mean that some resources could be freed up on the hypervisor? |
|
@pdube That part is handled elsewhere. Here we are concerned about syncing of VM metadata. So here if the VM is not in cloud DB then we are ignoring it. |
|
Null check LGTM |
|
LGTM, merging based on comments |
CLOUDSTACK-9196: Fixing null pointer exception when vm meta data is synced on upgraded setuphttps://issues.apache.org/jira/browse/CLOUDSTACK-9196 NullPointerException can occur if XenServer reports non-existing VM in cloud DB. * pr/1274: CLOUDSTACK-9196: Fixing null pointer exception when vm meta data is synced on upgraded setup. Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
|
@bhaisaab @pdube @DaanHoogland @koushik-das lots of LGTM here , so great. but we don't know why you LGTM'd and as far as I know no tests were run... |
|
@Runseb checked the diff, travis/jenkins passed, diff looked good to me. IMO, some changes cannot be tested by integration tests - if they are text related, in non-Java files such as READMEs, reset scripts (not even used by systemvm/systemvm.iso). Then, there are some changes such as this where we can cheat and avoid running integration tests given you know the code, and build/travis passes. |
|
@bhaisaab actually I am not even seeing the travis runs here anymore. Reason I am asking is genuine (again), because in the last few months Travis has shown to be flaky and was Green when actually tests did not run ... |
https://issues.apache.org/jira/browse/CLOUDSTACK-9196
NullPointerException can occur if XenServer reports non-existing VM in cloud DB.