genegr commented on code in PR #13061: URL: https://github.com/apache/cloudstack/pull/13061#discussion_r3459549235
########## plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathNVMeOFAdapterBase.java: ########## @@ -0,0 +1,462 @@ +// 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(";"); Review Comment: Good catch @slavkap. The two public callers in this class (`getPhysicalDisk` and `connectPhysicalDisk`) already pre-guard via `StringUtils.isEmpty(volumePath)` before calling `parseAndValidatePath`, so the current code paths can't reach it with a null `inPath` — but as a public method it deserves defense-in-depth. Pushed `e503a1439d kvm: guard parseAndValidatePath against null inPath`: throws `CloudRuntimeException` with a clear message instead of letting it NPE on `inPath.split(...)`. Mirrors how the FC sibling `FiberChannelAdapter` throws on an invalid address type. Also rebased onto current `upstream/main` to clear the merge-conflict bot. The new conflict was `KVMStorageProcessor.java` — main added `CLVM` to the RAW-format list and we add `NVMeTCP`; resolved by keeping both. Compile verified via `mvn -pl plugins/hypervisors/kvm -am -DskipTests compile`. -- 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]
