[XenServer/XCP-ng] Pass the image store NFS version on storage commands#5886
Conversation
nvazquez
requested review from
DaanHoogland,
harikrishna-patnala,
sureshanaparti and
yadvr
|
@blueorangutan package |
|
@nvazquez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2271 |
|
@blueorangutan test centos7 xcpng82 |
|
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + xcpng82) has been kicked to run smoke tests |
|
Trillian test result (tid-2938)
|
|
@blueorangutan package |
|
@nvazquez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
@blueorangutan package |
|
@nvazquez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2276 |
|
@blueorangutan test centos7 xcpng82 |
|
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + xcpng82) has been kicked to run smoke tests |
|
Trillian test result (tid-2945)
|
| private ImageStoreDao imageStoreDao; | ||
|
|
||
| protected void setSecondaryStorageNfsVersionToParams(Long zoneId, Map<String, Object> params) { | ||
| ImageStoreVO imageStoreInZone = imageStoreDao.findOneByZoneAndProtocol(zoneId, "nfs"); |
There was a problem hiding this comment.
@nvazquez Are multiple image stores with different nfs versions supported in a zone?
There was a problem hiding this comment.
It could be possible since the 'secstorage.nfs.version' setting has scope = ImageStore
There was a problem hiding this comment.
ok @nvazquez, will there be any issues if the nfs version is set based on one image store when multiple image stores are added (with different versions).
There was a problem hiding this comment.
Potentially yes - suggesting we should pick the lower value set in this case? cc @DaanHoogland
There was a problem hiding this comment.
@nvazquez @sureshanaparti I do not see a problem here. Unless I am looking at the code in isolation too much and should be looking at a bigger picture, what happens here:
a store is picked
if a store is found
- the nfs version for that store is retrieved and used
if not
- the default nfs version is used
what could go wrong here is only that the operator has not set the right nfs versions to the system/stores.
|
@blueorangutan package |
|
@nvazquez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
nvazquez
changed the title
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2308 |
|
@blueorangutan test centos7 xcpng82 |
|
@sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + xcpng82) has been kicked to run smoke tests |
borisstoyanov
left a comment
borisstoyanov
left a comment
There was a problem hiding this comment.
LGTM, verified manually the latest changes
|
Trillian test result (tid-3012)
|
|
Trillian test result (tid-3015)
|
|
@blueorangutan test centos7 xcpng82 |
|
@sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + xcpng82) has been kicked to run smoke tests |
|
Trillian test result (tid-3029)
|
DaanHoogland
left a comment
DaanHoogland
left a comment
There was a problem hiding this comment.
cltgm, needs extensive testing though
|
@blueorangutan package |
|
@sureshanaparti a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2357 |
|
@blueorangutan test centos7 xcpng82 |
|
@sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + xcpng82) has been kicked to run smoke tests |
|
Trillian test result (tid-3047)
|
|
@sureshanaparti it seems that failing test was consistent across the previous failures, I’ll investigate it |
@nvazquez there is some issue connecting to hosts, seems to be environment issue. noticed this in other PR tests as well (not observed in the latest health check runs). |
5 participants
Description
This PR fixes issues on mounting secondary storage from XCP hosts, controlling the NFS version set.
secstorage.nfs.versionsetting of an image store, or if it is not set, the global value is usedmount -o vers=4....Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
Tested on a NFS 3 server as image store
Deploy the environment
Set image store 'secstorage.nfs.version' = 4
Restart the management server
Deploy VM --> failure as secondary storage could not be mounted on the host
Set image store 'secstorage.nfs.version' = 3
Restart the management server
Deploy VM --> Success
Set image store 'secstorage.nfs.version' = '' and global 'secstorage.nfs.version' = ''
Restart the management server
Take backups, snapshots, attach ISO -> Success