Copilot commented on code in PR #13061:
URL: https://github.com/apache/cloudstack/pull/13061#discussion_r3128714419
##########
plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayAdapter.java:
##########
@@ -167,13 +185,18 @@ public String attach(ProviderAdapterContext context,
ProviderAdapterDataObject d
});
if (list != null && list.getItems() != null) {
for (FlashArrayConnection conn : list.getItems()) {
- if (conn.getHost() != null && conn.getHost().getName()
!= null &&
+ if (AddressType.NVMETCP.equals(volumeAddressType)) {
+ if (conn.getHostGroup() != null &&
conn.getHostGroup().getName() != null
+ &&
conn.getHostGroup().getName().equals(hostgroup)) {
+ return conn.getNsid() != null ? "" +
conn.getNsid() : "1";
+ }
+ } else if (conn.getHost() != null &&
conn.getHost().getName() != null &&
(conn.getHost().getName().equals(hostname) ||
conn.getHost().getName().equals(hostname.substring(0, hostname.indexOf('.'))))
&&
conn.getLun() != null) {
Review Comment:
In NVMe-TCP mode, the "Connection already exists" retry path only matches
host-group connections (`conn.getHostGroup().getName().equals(hostgroup)`). If
`transport=nvme-tcp` is used without `hostgroup`, the initial attach falls back
to host-scoped connections but this retry lookup will never match them, causing
attach to fail. Either require `hostgroup` for NVMe-TCP upfront, or also match
host-scoped NVMe connections here when `hostgroup` is null.
##########
plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayAdapter.java:
##########
@@ -140,22 +144,36 @@ public String attach(ProviderAdapterContext context,
ProviderAdapterDataObject d
String volumeName = normalizeName(pod, dataObject.getExternalName());
try {
FlashArrayList<FlashArrayConnection> list = null;
- FlashArrayHost host = getHost(hostname);
- if (host != null) {
- list = POST("/connections?host_names=" + host.getName() +
"&volume_names=" + volumeName, null,
+ if (AddressType.NVMETCP.equals(volumeAddressType) && hostgroup !=
null) {
+ // NVMe-TCP pod volumes are connected at the host-group level
so the
+ // array assigns a consistent NSID visible to every member
host.
+ list = POST("/connections?host_group_names=" + hostgroup +
"&volume_names=" + volumeName, null,
new TypeReference<FlashArrayList<FlashArrayConnection>>() {
});
+ } else {
+ FlashArrayHost host = getHost(hostname);
+ if (host != null) {
+ list = POST("/connections?host_names=" + host.getName() +
"&volume_names=" + volumeName, null,
+ new
TypeReference<FlashArrayList<FlashArrayConnection>>() {
+ });
+ }
}
if (list == null || list.getItems() == null ||
list.getItems().size() == 0) {
throw new RuntimeException("Volume attach did not return lun
information");
}
Review Comment:
The exception message here is NVMe-unfriendly: in NVMe-TCP mode the code
returns an NSID (or a default) rather than a LUN. Consider changing the message
to something transport-agnostic (e.g., "did not return connection information")
or conditional on `volumeAddressType` so debugging NVMe-TCP attach failures
isn’t misleading.
##########
plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayAdapter.java:
##########
@@ -781,6 +816,13 @@ private FlashArrayVolume getSnapshot(String snapshotName) {
return (FlashArrayVolume) getFlashArrayItem(list);
Review Comment:
`getSnapshot()` returns a `FlashArrayVolume` without applying
`withAddressType(...)`, so the returned object keeps the constructor default
`AddressType.FIBERWWN`. On an NVMe-TCP pool this makes
`ProviderSnapshot.getAddress()` emit an FC-style WWN instead of the NVMe
EUI-128, and adaptive’s `takeSnapshot` persists that address as the snapshot
path. Wrap this return (and the public `snapshot()` path) with
`withAddressType(...)` so snapshot addressing matches the pool transport.
##########
plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayAdapter.java:
##########
@@ -260,14 +283,19 @@ public ProviderVolume
getVolumeByAddress(ProviderAdapterContext context, Address
throw new RuntimeException("Invalid search criteria provided for
getVolumeByAddress");
}
- // only support WWN type addresses at this time.
- if (!ProviderVolume.AddressType.FIBERWWN.equals(addressType)) {
+ String serial;
+ if (ProviderVolume.AddressType.FIBERWWN.equals(addressType)) {
+ // Strip the NAA prefix (1 char) + Pure OUI to recover the volume
serial.
+ serial = address.substring(FlashArrayVolume.PURE_OUI.length() +
1).toUpperCase();
+ } else if (ProviderVolume.AddressType.NVMETCP.equals(addressType)) {
+ // Reverse the EUI-128 layout: serial = eui[2:16] + eui[22:32],
after
+ // stripping the optional "eui." prefix that appears in udev paths.
+ String eui = address.startsWith("eui.") ? address.substring(4) :
address;
+ serial = (eui.substring(2, 16) + eui.substring(22)).toUpperCase();
+ } else {
Review Comment:
The NVMe-TCP branch assumes `address` (after optional `eui.` stripping) is a
full 32-hex EUI-128 and immediately slices fixed offsets (`substring(2,16)` /
`substring(22)`). If a malformed/short address reaches here, this throws
`StringIndexOutOfBoundsException` and is caught as "not found", potentially
masking a real input problem. Add explicit length/format validation (and
ideally verify the embedded Pure OUI) and fail with a clear error when the
address doesn’t match the expected EUI-128 layout.
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathNVMeOFAdapterBase.java:
##########
@@ -0,0 +1,448 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package com.cloud.hypervisor.kvm.storage;
+
+import java.io.File;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.cloudstack.utils.qemu.QemuImg;
+import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat;
+import org.apache.cloudstack.utils.qemu.QemuImgException;
+import org.apache.cloudstack.utils.qemu.QemuImgFile;
+import org.libvirt.LibvirtException;
+
+import com.cloud.storage.Storage;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.script.OutputInterpreter;
+import com.cloud.utils.script.Script;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+/**
+ * Base class for KVM storage adapters that surface remote block volumes over
+ * NVMe-over-Fabrics (NVMe-oF). It is the NVMe-oF counterpart of
+ * {@link MultipathSCSIAdapterBase}: it does not drive device-mapper multipath
+ * and does not rescan the SCSI bus, because NVMe-oF has its own multipath
+ * (the kernel's native NVMe multipath) and namespaces show up via
+ * asynchronous event notifications as soon as the target grants access.
+ *
+ * Volumes are identified on the host by their EUI-128 NGUID, which udev
+ * exposes as {@code /dev/disk/by-id/nvme-eui.<eui>}.
+ */
+public abstract class MultipathNVMeOFAdapterBase implements StorageAdaptor {
+ protected static Logger LOGGER =
LogManager.getLogger(MultipathNVMeOFAdapterBase.class);
+ static final Map<String, KVMStoragePool> MapStorageUuidToStoragePool = new
HashMap<>();
+
+ static final int DEFAULT_DISK_WAIT_SECS = 240;
+ static final long NS_RESCAN_TIMEOUT_SECS = 5;
+ private static final long POLL_INTERVAL_MS = 2000;
+
+ @Override
+ public KVMStoragePool getStoragePool(String uuid) {
+ KVMStoragePool pool = MapStorageUuidToStoragePool.get(uuid);
+ if (pool == null) {
+ // Dummy pool - adapters that dispatch per-volume don't need
+ // connectivity information on the pool itself.
+ pool = new MultipathNVMeOFPool(uuid, this);
+ MapStorageUuidToStoragePool.put(uuid, pool);
+ }
+ return pool;
+ }
+
+ @Override
+ public KVMStoragePool getStoragePool(String uuid, boolean refreshInfo) {
+ return getStoragePool(uuid);
+ }
+
+ public abstract String getName();
+
+ @Override
+ public abstract Storage.StoragePoolType getStoragePoolType();
+
+ public abstract boolean isStoragePoolTypeSupported(Storage.StoragePoolType
type);
+
+ /**
+ * Parse a {@code type=NVMETCP; address=<eui>; connid.<host>=<nsid>; ...}
+ * volume path and produce an {@link AddressInfo} with the host-side device
+ * path set to {@code /dev/disk/by-id/nvme-eui.<eui>}.
+ */
+ public AddressInfo parseAndValidatePath(String inPath) {
+ String type = null;
+ String address = null;
+ String connectionId = null;
+ String path = null;
+ String hostname = resolveHostnameShort();
+ String hostnameFq = resolveHostnameFq();
+ String[] parts = inPath.split(";");
+ for (String part : parts) {
+ String[] pair = part.split("=");
+ if (pair.length != 2) {
+ continue;
+ }
+ String key = pair[0].trim();
+ String value = pair[1].trim();
+ if (key.equals("type")) {
+ type = value.toUpperCase();
+ } else if (key.equals("address")) {
+ address = value;
+ } else if (key.equals("connid")) {
+ connectionId = value;
+ } else if (key.startsWith("connid.")) {
+ String inHostname = key.substring("connid.".length());
+ if (inHostname.equals(hostname) ||
inHostname.equals(hostnameFq)) {
+ connectionId = value;
+ }
+ }
+ }
+
+ if (!"NVMETCP".equals(type)) {
+ throw new CloudRuntimeException("Invalid address type provided for
NVMe-oF target disk: " + type);
+ }
+ if (address == null) {
+ throw new CloudRuntimeException("NVMe-oF volume path is missing
the required address field");
+ }
+ path = "/dev/disk/by-id/nvme-eui." + address.toLowerCase();
+ return new AddressInfo(type, address, connectionId, path);
+ }
+
+ @Override
+ public KVMPhysicalDisk getPhysicalDisk(String volumePath, KVMStoragePool
pool) {
+ if (StringUtils.isEmpty(volumePath) || pool == null) {
+ LOGGER.error("Unable to get physical disk, volume path or pool not
specified");
+ return null;
+ }
+ return getPhysicalDisk(parseAndValidatePath(volumePath), pool);
+ }
+
+ private KVMPhysicalDisk getPhysicalDisk(AddressInfo address,
KVMStoragePool pool) {
+ KVMPhysicalDisk disk = new KVMPhysicalDisk(address.getPath(),
address.toString(), pool);
+ disk.setFormat(QemuImg.PhysicalDiskFormat.RAW);
+
+ if (!isConnected(address.getPath())) {
+ if (!connectPhysicalDisk(address, pool, null)) {
+ throw new CloudRuntimeException("Unable to connect to NVMe
namespace at " + address.getPath());
+ }
+ }
+ long diskSize = getPhysicalDiskSize(address.getPath());
+ disk.setSize(diskSize);
+ disk.setVirtualSize(diskSize);
+ return disk;
+ }
+
+ @Override
+ public KVMStoragePool createStoragePool(String uuid, String host, int
port, String path, String userInfo, Storage.StoragePoolType type, Map<String,
String> details, boolean isPrimaryStorage) {
+ LOGGER.info(String.format("createStoragePool(uuid,host,port,path,type)
called with args (%s, %s, %d, %s, %s)", uuid, host, port, path, type));
+ MultipathNVMeOFPool pool = new MultipathNVMeOFPool(uuid, host, port,
path, type, details, this);
+ MapStorageUuidToStoragePool.put(uuid, pool);
+ return pool;
+ }
+
+ @Override
+ public boolean deleteStoragePool(String uuid) {
+ MapStorageUuidToStoragePool.remove(uuid);
+ return true;
+ }
+
+ @Override
+ public boolean deleteStoragePool(KVMStoragePool pool) {
+ return deleteStoragePool(pool.getUuid());
+ }
+
+ @Override
+ public boolean connectPhysicalDisk(String volumePath, KVMStoragePool pool,
Map<String, String> details, boolean isVMMigrate) {
+ if (StringUtils.isEmpty(volumePath) || pool == null) {
+ LOGGER.error("Unable to connect NVMe-oF physical disk:
insufficient arguments");
+ return false;
+ }
+ return connectPhysicalDisk(parseAndValidatePath(volumePath), pool,
details);
+ }
+
+ private boolean connectPhysicalDisk(AddressInfo address, KVMStoragePool
pool, Map<String, String> details) {
+ if (address.getConnectionId() == null) {
+ LOGGER.error("NVMe-oF volume " + address.getPath() + " on pool " +
pool.getUuid() + " is missing a connid.<host> token in its path");
+ return false;
+ }
+ long waitSecs = DEFAULT_DISK_WAIT_SECS;
+ if (details != null &&
details.containsKey(com.cloud.storage.StorageManager.STORAGE_POOL_DISK_WAIT.toString()))
{
+ String waitTime =
details.get(com.cloud.storage.StorageManager.STORAGE_POOL_DISK_WAIT.toString());
+ if (StringUtils.isNotEmpty(waitTime)) {
+ waitSecs = Integer.parseInt(waitTime);
+ }
+ }
+ return waitForNamespace(address, pool, waitSecs);
+ }
+
+ /**
+ * Poll for the EUI-keyed udev symlink to show up. On every iteration also
+ * nudge the kernel with {@code nvme ns-rescan} on every local NVMe
+ * controller, to cover arrays / firmware combinations that do not emit a
+ * reliable asynchronous event notification when a new namespace is
+ * mapped.
+ */
+ private boolean waitForNamespace(AddressInfo address, KVMStoragePool pool,
long waitSecs) {
+ if (waitSecs < 60) {
+ waitSecs = 60;
+ }
+ long deadline = System.currentTimeMillis() + (waitSecs * 1000);
+ File dev = new File(address.getPath());
+ while (System.currentTimeMillis() < deadline) {
+ if (dev.exists() && isConnected(address.getPath())) {
+ long size = getPhysicalDiskSize(address.getPath());
+ if (size > 0) {
+ LOGGER.debug("Found NVMe namespace at " +
address.getPath());
+ return true;
+ }
+ }
+ rescanAllControllers();
+ try {
+ Thread.sleep(POLL_INTERVAL_MS);
+ } catch (InterruptedException ie) {
+ Thread.currentThread().interrupt();
+ return false;
+ }
+ }
+ LOGGER.debug("NVMe namespace did not appear at " + address.getPath() +
" within " + waitSecs + "s");
+ return false;
+ }
+
+ private void rescanAllControllers() {
+ try {
+ File sysClass = new File("/sys/class/nvme");
+ File[] ctrls = sysClass.listFiles();
+ if (ctrls == null) {
+ return;
+ }
+ for (File ctrl : ctrls) {
+ Process p = new ProcessBuilder("nvme", "ns-rescan", "/dev/" +
ctrl.getName())
+ .redirectErrorStream(true).start();
+ p.waitFor(NS_RESCAN_TIMEOUT_SECS, TimeUnit.SECONDS);
+ }
Review Comment:
`rescanAllControllers()` starts an `nvme ns-rescan` process per controller
and waits up to `NS_RESCAN_TIMEOUT_SECS`, but ignores the boolean result of
`waitFor(...)` and never terminates the process if it times out. This can leave
hung `nvme` processes accumulating under load. Consider checking the `waitFor`
return value and calling `destroyForcibly()` (and optionally logging) when the
timeout is hit.
##########
plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayVolume.java:
##########
@@ -107,6 +111,13 @@ public AddressType getAddressType() {
@JsonIgnore
public String getAddress() {
if (serial == null) return null;
+ if (AddressType.NVMETCP.equals(addressType)) {
+ // EUI-128 layout for FlashArray NVMe namespaces:
+ // 00 + serial[0:14] + <Pure OUI (24a937)> + serial[14:24]
+ // This is the value the Linux kernel exposes as
+ // /dev/disk/by-id/nvme-eui.<result>
Review Comment:
NVMe-TCP address generation assumes `serial` is long enough for
`substring(0, 14)` / `substring(14)` (and per comment effectively expects a
24-hex serial). If FlashArray ever returns an unexpected/short serial, this
will throw at runtime. Please validate `serial` length/format before slicing
and fail with a clear exception (or return null) when it doesn’t match the
expected layout.
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathNVMeOFAdapterBase.java:
##########
@@ -0,0 +1,448 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package com.cloud.hypervisor.kvm.storage;
+
+import java.io.File;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.cloudstack.utils.qemu.QemuImg;
+import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat;
+import org.apache.cloudstack.utils.qemu.QemuImgException;
+import org.apache.cloudstack.utils.qemu.QemuImgFile;
+import org.libvirt.LibvirtException;
+
+import com.cloud.storage.Storage;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.script.OutputInterpreter;
+import com.cloud.utils.script.Script;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+/**
+ * Base class for KVM storage adapters that surface remote block volumes over
+ * NVMe-over-Fabrics (NVMe-oF). It is the NVMe-oF counterpart of
+ * {@link MultipathSCSIAdapterBase}: it does not drive device-mapper multipath
+ * and does not rescan the SCSI bus, because NVMe-oF has its own multipath
+ * (the kernel's native NVMe multipath) and namespaces show up via
+ * asynchronous event notifications as soon as the target grants access.
+ *
+ * Volumes are identified on the host by their EUI-128 NGUID, which udev
+ * exposes as {@code /dev/disk/by-id/nvme-eui.<eui>}.
+ */
+public abstract class MultipathNVMeOFAdapterBase implements StorageAdaptor {
+ protected static Logger LOGGER =
LogManager.getLogger(MultipathNVMeOFAdapterBase.class);
+ static final Map<String, KVMStoragePool> MapStorageUuidToStoragePool = new
HashMap<>();
+
+ static final int DEFAULT_DISK_WAIT_SECS = 240;
+ static final long NS_RESCAN_TIMEOUT_SECS = 5;
+ private static final long POLL_INTERVAL_MS = 2000;
+
+ @Override
+ public KVMStoragePool getStoragePool(String uuid) {
+ KVMStoragePool pool = MapStorageUuidToStoragePool.get(uuid);
+ if (pool == null) {
+ // Dummy pool - adapters that dispatch per-volume don't need
+ // connectivity information on the pool itself.
+ pool = new MultipathNVMeOFPool(uuid, this);
+ MapStorageUuidToStoragePool.put(uuid, pool);
+ }
+ return pool;
+ }
+
+ @Override
+ public KVMStoragePool getStoragePool(String uuid, boolean refreshInfo) {
+ return getStoragePool(uuid);
+ }
+
+ public abstract String getName();
+
+ @Override
+ public abstract Storage.StoragePoolType getStoragePoolType();
+
+ public abstract boolean isStoragePoolTypeSupported(Storage.StoragePoolType
type);
+
+ /**
+ * Parse a {@code type=NVMETCP; address=<eui>; connid.<host>=<nsid>; ...}
+ * volume path and produce an {@link AddressInfo} with the host-side device
+ * path set to {@code /dev/disk/by-id/nvme-eui.<eui>}.
+ */
+ public AddressInfo parseAndValidatePath(String inPath) {
+ String type = null;
+ String address = null;
+ String connectionId = null;
+ String path = null;
+ String hostname = resolveHostnameShort();
+ String hostnameFq = resolveHostnameFq();
+ String[] parts = inPath.split(";");
+ for (String part : parts) {
+ String[] pair = part.split("=");
+ if (pair.length != 2) {
+ continue;
+ }
+ String key = pair[0].trim();
+ String value = pair[1].trim();
+ if (key.equals("type")) {
+ type = value.toUpperCase();
+ } else if (key.equals("address")) {
+ address = value;
+ } else if (key.equals("connid")) {
+ connectionId = value;
+ } else if (key.startsWith("connid.")) {
+ String inHostname = key.substring("connid.".length());
+ if (inHostname.equals(hostname) ||
inHostname.equals(hostnameFq)) {
+ connectionId = value;
+ }
+ }
+ }
+
+ if (!"NVMETCP".equals(type)) {
+ throw new CloudRuntimeException("Invalid address type provided for
NVMe-oF target disk: " + type);
+ }
+ if (address == null) {
+ throw new CloudRuntimeException("NVMe-oF volume path is missing
the required address field");
+ }
+ path = "/dev/disk/by-id/nvme-eui." + address.toLowerCase();
+ return new AddressInfo(type, address, connectionId, path);
+ }
+
+ @Override
+ public KVMPhysicalDisk getPhysicalDisk(String volumePath, KVMStoragePool
pool) {
+ if (StringUtils.isEmpty(volumePath) || pool == null) {
+ LOGGER.error("Unable to get physical disk, volume path or pool not
specified");
+ return null;
+ }
+ return getPhysicalDisk(parseAndValidatePath(volumePath), pool);
+ }
+
+ private KVMPhysicalDisk getPhysicalDisk(AddressInfo address,
KVMStoragePool pool) {
+ KVMPhysicalDisk disk = new KVMPhysicalDisk(address.getPath(),
address.toString(), pool);
+ disk.setFormat(QemuImg.PhysicalDiskFormat.RAW);
+
+ if (!isConnected(address.getPath())) {
+ if (!connectPhysicalDisk(address, pool, null)) {
+ throw new CloudRuntimeException("Unable to connect to NVMe
namespace at " + address.getPath());
+ }
+ }
+ long diskSize = getPhysicalDiskSize(address.getPath());
+ disk.setSize(diskSize);
+ disk.setVirtualSize(diskSize);
+ return disk;
+ }
+
+ @Override
+ public KVMStoragePool createStoragePool(String uuid, String host, int
port, String path, String userInfo, Storage.StoragePoolType type, Map<String,
String> details, boolean isPrimaryStorage) {
+ LOGGER.info(String.format("createStoragePool(uuid,host,port,path,type)
called with args (%s, %s, %d, %s, %s)", uuid, host, port, path, type));
+ MultipathNVMeOFPool pool = new MultipathNVMeOFPool(uuid, host, port,
path, type, details, this);
+ MapStorageUuidToStoragePool.put(uuid, pool);
+ return pool;
+ }
+
+ @Override
+ public boolean deleteStoragePool(String uuid) {
+ MapStorageUuidToStoragePool.remove(uuid);
+ return true;
+ }
+
+ @Override
+ public boolean deleteStoragePool(KVMStoragePool pool) {
+ return deleteStoragePool(pool.getUuid());
+ }
+
+ @Override
+ public boolean connectPhysicalDisk(String volumePath, KVMStoragePool pool,
Map<String, String> details, boolean isVMMigrate) {
+ if (StringUtils.isEmpty(volumePath) || pool == null) {
+ LOGGER.error("Unable to connect NVMe-oF physical disk:
insufficient arguments");
+ return false;
+ }
+ return connectPhysicalDisk(parseAndValidatePath(volumePath), pool,
details);
+ }
+
+ private boolean connectPhysicalDisk(AddressInfo address, KVMStoragePool
pool, Map<String, String> details) {
+ if (address.getConnectionId() == null) {
+ LOGGER.error("NVMe-oF volume " + address.getPath() + " on pool " +
pool.getUuid() + " is missing a connid.<host> token in its path");
+ return false;
+ }
+ long waitSecs = DEFAULT_DISK_WAIT_SECS;
+ if (details != null &&
details.containsKey(com.cloud.storage.StorageManager.STORAGE_POOL_DISK_WAIT.toString()))
{
+ String waitTime =
details.get(com.cloud.storage.StorageManager.STORAGE_POOL_DISK_WAIT.toString());
+ if (StringUtils.isNotEmpty(waitTime)) {
+ waitSecs = Integer.parseInt(waitTime);
Review Comment:
`waitSecs = Integer.parseInt(waitTime)` can throw `NumberFormatException` if
the STORAGE_POOL_DISK_WAIT detail is set to a non-integer value, which would
abort connect rather than falling back to the default. Consider guarding this
parse with a try/catch (logging a warning and keeping `DEFAULT_DISK_WAIT_SECS`)
to match the more defensive parsing used elsewhere in the codebase for adapter
parameters.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
