Skip to content

Commit 4eb4365

Browse files
abh1sarabh1sar
authored
Ability to specify NFS mount options while adding a primary storage and modify them on a pre-existing primary storage (#8947)
* Ability to specify NFS mount options while adding a primary storage and modify it later * Pull 8947: Rename all occurrence of nfsopt to nfsMountOpt and added nfsMountOpts to ApiConstants * Pull 8947: Refactor code - move into separate methods * Pull 8947: CollectionsUtils.isNotEmpty and switch statement in LibvirtStoragePoolDef.java * Pull 8947: UI - cancel maintainenace will remount the storage pool and apply the options * Pull 8947: UI - moved edit NFS mount options to edit Primary Storage form * Pull 8947: UI - moved 'NFS Mount Options' to below 'Type' in dataview * Pull 8947: Fixed message in AddPrimaryStorage.vue * Pull 8947: Convert _nfsmountOpts to Set in libvirtStoragePoolDef * Pull 8947: Throw exception and log error if mount fails due to incorrect mount option * Pull 8947: Added UT and moved integration test to component/maint * Pull 8947: Review comments * Pull 8947: Removed password from integration test * Pull 8947: move details allocation to inside the if loop in getStoragePoolNFSMountOpts * Pull 8947: Fixed a bug in AddPrimaryStorage.vue * Pull 8947: Pool should remain in maintenance mode if mount fails * Pull 8947: Removed password from integration test * Pull 8947: Added UT * Pull 8875: Fixed a bug in CloudStackPrimaryDataStoreLifeCycleImplTest * Pull 8875: Fixed a bug in LibvirtStoragePoolDefTest * Pull 8947: minor code restructuring * Pull 8947 : added some ut for coverage * Fix LibvirtStorageAdapterTest UT
1 parent 620ed16 commit 4eb4365

File tree

24 files changed

+1037
-66
lines changed

24 files changed

+1037
-66
lines changed

‎api/src/main/java/org/apache/cloudstack/api/ApiConstants.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,6 +1099,8 @@ public class ApiConstants {
10991099

11001100
public static final String PARAMETER_DESCRIPTION_IS_TAG_A_RULE = "Whether the informed tag is a JS interpretable rule or not.";
11011101

1102+
public static final String NFS_MOUNT_OPTIONS = "nfsmountopts";
1103+
11021104
/**
11031105
* This enum specifies IO Drivers, each option controls specific policies on I/O.
11041106
* Qemu guests support "threads" and "native" options Since 0.8.8 ; "io_uring" is supported Since 6.3.0 (QEMU 5.0).

‎api/src/main/java/org/apache/cloudstack/api/response/StoragePoolResponse.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,10 @@ public class StoragePoolResponse extends BaseResponseWithAnnotations {
101101
@Param(description = "the tags for the storage pool")
102102
private String tags;
103103

104+
@SerializedName(ApiConstants.NFS_MOUNT_OPTIONS)
105+
@Param(description = "the nfs mount options for the storage pool", since = "4.19.1")
106+
private String nfsMountOpts;
107+
104108
@SerializedName(ApiConstants.IS_TAG_A_RULE)
105109
@Param(description = ApiConstants.PARAMETER_DESCRIPTION_IS_TAG_A_RULE)
106110
private Boolean isTagARule;
@@ -347,4 +351,12 @@ public String getProvider() {
347351
public void setProvider(String provider) {
348352
this.provider = provider;
349353
}
354+
355+
public String getNfsMountOpts() {
356+
return nfsMountOpts;
357+
}
358+
359+
public void setNfsMountOpts(String nfsMountOpts) {
360+
this.nfsMountOpts = nfsMountOpts;
361+
}
350362
}

‎core/src/main/java/com/cloud/agent/api/ModifyStoragePoolCommand.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ public ModifyStoragePoolCommand(boolean add, StoragePool pool, String localPath,
4646
this.details = details;
4747
}
4848

49+
public ModifyStoragePoolCommand(boolean add, StoragePool pool, Map<String, String> details) {
50+
this(add, pool, LOCAL_PATH_PREFIX + File.separator + UUID.nameUUIDFromBytes((pool.getHostAddress() + pool.getPath()).getBytes()), details);
51+
}
52+
4953
public ModifyStoragePoolCommand(boolean add, StoragePool pool) {
5054
this(add, pool, LOCAL_PATH_PREFIX + File.separator + UUID.nameUUIDFromBytes((pool.getHostAddress() + pool.getPath()).getBytes()));
5155
}

‎engine/components-api/src/main/java/com/cloud/storage/StorageManager.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import java.math.BigDecimal;
2020
import java.util.List;
21+
import java.util.Map;
2122

2223
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
2324
import org.apache.cloudstack.engine.subsystem.api.storage.HypervisorHostListener;
@@ -341,6 +342,10 @@ static Boolean getFullCloneConfiguration(Long storeId) {
341342

342343
boolean registerHostListener(String providerUuid, HypervisorHostListener listener);
343344

345+
Pair<Map<String, String>, Boolean> getStoragePoolNFSMountOpts(StoragePool pool, Map<String, String> details);
346+
347+
String getStoragePoolMountFailureReason(String error);
348+
344349
boolean connectHostToSharedPool(long hostId, long poolId) throws StorageUnavailableException, StorageConflictException;
345350

346351
void disconnectHostFromSharedPool(long hostId, long poolId) throws StorageUnavailableException, StorageConflictException;

‎engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,9 @@
4242
import com.cloud.storage.StoragePoolHostVO;
4343
import com.cloud.storage.StorageService;
4444
import com.cloud.storage.dao.StoragePoolHostDao;
45+
import com.cloud.utils.Pair;
4546
import com.cloud.utils.exception.CloudRuntimeException;
47+
4648
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
4749
import org.apache.cloudstack.engine.subsystem.api.storage.HypervisorHostListener;
4850
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
@@ -53,7 +55,9 @@
5355
import org.apache.log4j.Logger;
5456

5557
import javax.inject.Inject;
58+
5659
import java.util.List;
60+
import java.util.Map;
5761

5862
public class DefaultHostListener implements HypervisorHostListener {
5963
private static final Logger s_logger = Logger.getLogger(DefaultHostListener.class);
@@ -125,7 +129,9 @@ private NicTO createNicTOFromNetworkAndOffering(NetworkVO networkVO, NetworkOffe
125129
@Override
126130
public boolean hostConnect(long hostId, long poolId) throws StorageConflictException {
127131
StoragePool pool = (StoragePool) this.dataStoreMgr.getDataStore(poolId, DataStoreRole.Primary);
128-
ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(true, pool);
132+
Pair<Map<String, String>, Boolean> nfsMountOpts = storageManager.getStoragePoolNFSMountOpts(pool, null);
133+
134+
ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(true, pool, nfsMountOpts.first());
129135
cmd.setWait(modifyStoragePoolCommandWait);
130136
s_logger.debug(String.format("Sending modify storage pool command to agent: %d for storage pool: %d with timeout %d seconds",
131137
hostId, poolId, cmd.getWait()));
@@ -138,7 +144,7 @@ public boolean hostConnect(long hostId, long poolId) throws StorageConflictExcep
138144
if (!answer.getResult()) {
139145
String msg = "Unable to attach storage pool" + poolId + " to the host" + hostId;
140146
alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, pool.getDataCenterId(), pool.getPodId(), msg, msg);
141-
throw new CloudRuntimeException("Unable establish connection from storage head to storage pool " + pool.getId() + " due to " + answer.getDetails() +
147+
throw new CloudRuntimeException("Unable to establish connection from storage head to storage pool " + pool.getId() + " due to " + answer.getDetails() +
142148
pool.getId());
143149
}
144150

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java‎

Lines changed: 88 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@
1616
// under the License.
1717
package com.cloud.hypervisor.kvm.resource;
1818

19+
import java.util.HashSet;
20+
import java.util.List;
21+
import java.util.Set;
22+
23+
import org.apache.commons.collections.CollectionUtils;
24+
1925
public class LibvirtStoragePoolDef {
2026
public enum PoolType {
2127
ISCSI("iscsi"), NETFS("netfs"), LOGICAL("logical"), DIR("dir"), RBD("rbd"), GLUSTERFS("glusterfs"), POWERFLEX("powerflex");
@@ -55,6 +61,7 @@ public String toString() {
5561
private String _authUsername;
5662
private AuthenticationType _authType;
5763
private String _secretUuid;
64+
private Set<String> _nfsMountOpts = new HashSet<>();
5865

5966
public LibvirtStoragePoolDef(PoolType type, String poolName, String uuid, String host, int port, String dir, String targetPath) {
6067
_poolType = type;
@@ -75,6 +82,15 @@ public LibvirtStoragePoolDef(PoolType type, String poolName, String uuid, String
7582
_targetPath = targetPath;
7683
}
7784

85+
public LibvirtStoragePoolDef(PoolType type, String poolName, String uuid, String host, String dir, String targetPath, List<String> nfsMountOpts) {
86+
this(type, poolName, uuid, host, dir, targetPath);
87+
if (CollectionUtils.isNotEmpty(nfsMountOpts)) {
88+
for (String nfsMountOpt : nfsMountOpts) {
89+
this._nfsMountOpts.add(nfsMountOpt);
90+
}
91+
}
92+
}
93+
7894
public LibvirtStoragePoolDef(PoolType type, String poolName, String uuid, String sourceHost, int sourcePort, String dir, String authUsername, AuthenticationType authType,
7995
String secretUuid) {
8096
_poolType = type;
@@ -124,69 +140,98 @@ public AuthenticationType getAuthType() {
124140
return _authType;
125141
}
126142

143+
public Set<String> getNfsMountOpts() {
144+
return _nfsMountOpts;
145+
}
146+
127147
@Override
128148
public String toString() {
129149
StringBuilder storagePoolBuilder = new StringBuilder();
130-
if (_poolType == PoolType.GLUSTERFS) {
131-
/* libvirt mounts a Gluster volume, similar to NFS */
132-
storagePoolBuilder.append("<pool type='netfs'>\n");
133-
} else {
134-
storagePoolBuilder.append("<pool type='");
135-
storagePoolBuilder.append(_poolType);
136-
storagePoolBuilder.append("'>\n");
150+
String poolTypeXML;
151+
switch (_poolType) {
152+
case NETFS:
153+
if (_nfsMountOpts != null) {
154+
poolTypeXML = "netfs' xmlns:fs='http://libvirt.org/schemas/storagepool/fs/1.0";
155+
} else {
156+
poolTypeXML = _poolType.toString();
157+
}
158+
break;
159+
case GLUSTERFS:
160+
/* libvirt mounts a Gluster volume, similar to NFS */
161+
poolTypeXML = "netfs";
162+
break;
163+
default:
164+
poolTypeXML = _poolType.toString();
137165
}
138166

167+
storagePoolBuilder.append("<pool type='");
168+
storagePoolBuilder.append(poolTypeXML);
169+
storagePoolBuilder.append("'>\n");
170+
139171
storagePoolBuilder.append("<name>" + _poolName + "</name>\n");
140172
if (_uuid != null)
141173
storagePoolBuilder.append("<uuid>" + _uuid + "</uuid>\n");
142-
if (_poolType == PoolType.NETFS) {
143-
storagePoolBuilder.append("<source>\n");
144-
storagePoolBuilder.append("<host name='" + _sourceHost + "'/>\n");
145-
storagePoolBuilder.append("<dir path='" + _sourceDir + "'/>\n");
146-
storagePoolBuilder.append("</source>\n");
147-
}
148-
if (_poolType == PoolType.RBD) {
149-
storagePoolBuilder.append("<source>\n");
150-
for (String sourceHost : _sourceHost.split(",")) {
174+
175+
switch (_poolType) {
176+
case NETFS:
177+
storagePoolBuilder.append("<source>\n");
178+
storagePoolBuilder.append("<host name='" + _sourceHost + "'/>\n");
179+
storagePoolBuilder.append("<dir path='" + _sourceDir + "'/>\n");
180+
storagePoolBuilder.append("</source>\n");
181+
break;
182+
183+
case RBD:
184+
storagePoolBuilder.append("<source>\n");
185+
for (String sourceHost : _sourceHost.split(",")) {
186+
storagePoolBuilder.append("<host name='");
187+
storagePoolBuilder.append(sourceHost);
188+
if (_sourcePort != 0) {
189+
storagePoolBuilder.append("' port='");
190+
storagePoolBuilder.append(_sourcePort);
191+
}
192+
storagePoolBuilder.append("'/>\n");
193+
}
194+
195+
storagePoolBuilder.append("<name>" + _sourceDir + "</name>\n");
196+
if (_authUsername != null) {
197+
storagePoolBuilder.append("<auth username='" + _authUsername + "' type='" + _authType + "'>\n");
198+
storagePoolBuilder.append("<secret uuid='" + _secretUuid + "'/>\n");
199+
storagePoolBuilder.append("</auth>\n");
200+
}
201+
storagePoolBuilder.append("</source>\n");
202+
break;
203+
204+
case GLUSTERFS:
205+
storagePoolBuilder.append("<source>\n");
151206
storagePoolBuilder.append("<host name='");
152-
storagePoolBuilder.append(sourceHost);
207+
storagePoolBuilder.append(_sourceHost);
153208
if (_sourcePort != 0) {
154209
storagePoolBuilder.append("' port='");
155210
storagePoolBuilder.append(_sourcePort);
156211
}
157212
storagePoolBuilder.append("'/>\n");
158-
}
159-
160-
storagePoolBuilder.append("<name>" + _sourceDir + "</name>\n");
161-
if (_authUsername != null) {
162-
storagePoolBuilder.append("<auth username='" + _authUsername + "' type='" + _authType + "'>\n");
163-
storagePoolBuilder.append("<secret uuid='" + _secretUuid + "'/>\n");
164-
storagePoolBuilder.append("</auth>\n");
165-
}
166-
storagePoolBuilder.append("</source>\n");
167-
}
168-
if (_poolType == PoolType.GLUSTERFS) {
169-
storagePoolBuilder.append("<source>\n");
170-
storagePoolBuilder.append("<host name='");
171-
storagePoolBuilder.append(_sourceHost);
172-
if (_sourcePort != 0) {
173-
storagePoolBuilder.append("' port='");
174-
storagePoolBuilder.append(_sourcePort);
175-
}
176-
storagePoolBuilder.append("'/>\n");
177-
storagePoolBuilder.append("<dir path='");
178-
storagePoolBuilder.append(_sourceDir);
179-
storagePoolBuilder.append("'/>\n");
180-
storagePoolBuilder.append("<format type='");
181-
storagePoolBuilder.append(_poolType);
182-
storagePoolBuilder.append("'/>\n");
183-
storagePoolBuilder.append("</source>\n");
213+
storagePoolBuilder.append("<dir path='");
214+
storagePoolBuilder.append(_sourceDir);
215+
storagePoolBuilder.append("'/>\n");
216+
storagePoolBuilder.append("<format type='");
217+
storagePoolBuilder.append(_poolType);
218+
storagePoolBuilder.append("'/>\n");
219+
storagePoolBuilder.append("</source>\n");
220+
break;
184221
}
222+
185223
if (_poolType != PoolType.RBD && _poolType != PoolType.POWERFLEX) {
186224
storagePoolBuilder.append("<target>\n");
187225
storagePoolBuilder.append("<path>" + _targetPath + "</path>\n");
188226
storagePoolBuilder.append("</target>\n");
189227
}
228+
if (_poolType == PoolType.NETFS && _nfsMountOpts != null) {
229+
storagePoolBuilder.append("<fs:mount_opts>\n");
230+
for (String options : _nfsMountOpts) {
231+
storagePoolBuilder.append("<fs:option name='" + options + "'/>\n");
232+
}
233+
storagePoolBuilder.append("</fs:mount_opts>\n");
234+
}
190235
storagePoolBuilder.append("</pool>\n");
191236
return storagePoolBuilder.toString();
192237
}

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolXMLParser.java‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,19 @@
3737
public class LibvirtStoragePoolXMLParser {
3838
private static final Logger s_logger = Logger.getLogger(LibvirtStoragePoolXMLParser.class);
3939

40+
private List<String> getNFSMountOptsFromRootElement(Element rootElement) {
41+
List<String> nfsMountOpts = new ArrayList<>();
42+
Element mountOpts = (Element) rootElement.getElementsByTagName("fs:mount_opts").item(0);
43+
if (mountOpts != null) {
44+
NodeList options = mountOpts.getElementsByTagName("fs:option");
45+
for (int i = 0; i < options.getLength(); i++) {
46+
Element option = (Element) options.item(i);
47+
nfsMountOpts.add(option.getAttribute("name"));
48+
}
49+
}
50+
return nfsMountOpts;
51+
}
52+
4053
public LibvirtStoragePoolDef parseStoragePoolXML(String poolXML) {
4154
DocumentBuilder builder;
4255
try {
@@ -94,11 +107,15 @@ public LibvirtStoragePoolDef parseStoragePoolXML(String poolXML) {
94107
poolName, uuid, host, port, path, targetPath);
95108
} else {
96109
String path = getAttrValue("dir", "path", source);
97-
98110
Element target = (Element)rootElement.getElementsByTagName("target").item(0);
99111
String targetPath = getTagValue("path", target);
100112

101-
return new LibvirtStoragePoolDef(LibvirtStoragePoolDef.PoolType.valueOf(type.toUpperCase()), poolName, uuid, host, path, targetPath);
113+
if (type.equalsIgnoreCase("netfs")) {
114+
List<String> nfsMountOpts = getNFSMountOptsFromRootElement(rootElement);
115+
return new LibvirtStoragePoolDef(LibvirtStoragePoolDef.PoolType.valueOf(type.toUpperCase()), poolName, uuid, host, path, targetPath, nfsMountOpts);
116+
} else {
117+
return new LibvirtStoragePoolDef(LibvirtStoragePoolDef.PoolType.valueOf(type.toUpperCase()), poolName, uuid, host, path, targetPath);
118+
}
102119
}
103120
} catch (ParserConfigurationException e) {
104121
s_logger.debug(e.toString());

0 commit comments

Comments
 (0)