server: fix NPE for the case where volume is not attached to a VM#3566
Conversation
Fixes NPE when trying to find suitable storage pools for a volume when the volume is not attached to a VM. Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
yadvr
requested review from
PaulAngus,
borisstoyanov,
nvazquez and
rafaelweingartner
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-263 |
|
@blueorangutan test centos7 vmware65 |
|
@borisstoyanov unsupported parameters provided. Supported mgmt server os are: |
|
@blueorangutan test centos7 vmware-65u2 |
|
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + vmware-65u2) has been kicked to run smoke tests |
borisstoyanov
left a comment
borisstoyanov
left a comment
There was a problem hiding this comment.
LGTM, run the failed test and it's passing now, also manually verified RC1 and this PR that it's not really a blocker, but def good to have
=== TestName: test_03_migrate_detached_volume | Status : SUCCESS ===
nvazquez
left a comment
nvazquez
left a comment
There was a problem hiding this comment.
Code LGTM. Just left a comment for code improvement but not mandatory
| allPools = getAllStoragePoolCompatileWithVolumeSourceStoragePool(srcVolumePool); | ||
| allPools.remove(srcVolumePool); | ||
| suitablePools = findAllSuitableStoragePoolsForVm(volume, vm, srcVolumePool); | ||
| if (vm != null) { |
There was a problem hiding this comment.
One liner improvement if you have some time: suitablePools = vm == null ? allPools : findAllSuitableStoragePools(....)
There was a problem hiding this comment.
I can consider it in the future, I generally like to avoid ternary operator which reduces code readability.
|
Trillian test result (tid-340)
|
6 participants
Fixes NPE when trying to find suitable storage pools for a volume
when the volume is not attached to a VM, when trying to call the findStoragePoolsForMigration API.
Types of changes