Skip to content

Commit 1faa7ce

Browse files
Eugenio Grossoclaude
andcommitted
flasharray: always stamp the pool address type on returned volumes
FlashArrayVolume defaults addressType to FIBERWWN in its constructor, so a volume deserialised from the array reports an FC-style NAA WWN from getAddress() until the pool's real address type is stamped onto it. Most accessors do that via withAddressType(), but two paths returned an unstamped volume: * revert() returned the private getVolume() result directly, so reverting a snapshot on an NVMe-TCP pool produced a volume carrying an FC-style WWN. * copy() returned the newly created volume unstamped. The adaptive driver persists this one -- copyAsync() stores generatePathInfo(outVolume, ...) -- so the volume was recorded in the database with an address the host cannot resolve, since NVMe namespaces are located by EUI-128. Rather than add a third withAddressType() call at each site, stamp inside the private getVolume() accessor so every path is correct by construction. That matches the private getSnapshot() accessor, which already did this, and makes the wrap at the public getVolume() call site redundant. copy() now reuses the accessor for its source lookup instead of repeating the same GET inline, and stamps the volume it hands back. No behavioural change for Fibre Channel: FIBERWWN is what the constructor already defaulted to, so stamping it is a no-op there. Also drops a comment in getConnectionIdMap() describing connections as host-group scoped, which is no longer how NVMe-TCP volumes are connected after the preceding commit. The surrounding logic was already keyed on host name and is unchanged; only nsid vs lun differs between the transports. Found by auditing every NVMe-TCP divergence from the Fibre Channel path, on the principle that NVMe-TCP is only a different carrier: the array operations are identical and the host-side difference is an NQN instead of a WWN, which lives on the array and is never touched by this adapter (hosts are resolved by name). Signed-off-by: Eugenio Grosso <eugenio.grosso@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent dd58aac commit 1faa7ce

1 file changed

Lines changed: 14 additions & 10 deletions

File tree

  • plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray

‎plugins/storage/volume/flasharray/src/main/java/org/apache/cloudstack/storage/datastore/adapter/flasharray/FlashArrayAdapter.java‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ public ProviderVolume getVolume(ProviderAdapterContext context, ProviderAdapterD
318318
}
319319
FlashArrayVolume volume = null;
320320
try {
321-
volume = withAddressType(getVolume(externalName));
321+
volume = getVolume(externalName);
322322
// if we didn't get an address back its likely an empty object
323323
if (volume != null && volume.getAddress() == null) {
324324
return null;
@@ -501,10 +501,7 @@ public ProviderVolume copy(ProviderAdapterContext context, ProviderAdapterDataOb
501501
if (sourceDataObject.getType().equals(ProviderAdapterDataObject.Type.SNAPSHOT)) {
502502
currentVol = getSnapshot(sourceDataObject.getExternalName());
503503
} else {
504-
currentVol = (FlashArrayVolume) this
505-
.getFlashArrayItem(GET("/volumes?names=" + sourceDataObject.getExternalName(),
506-
new TypeReference<FlashArrayList<FlashArrayVolume>>() {
507-
}));
504+
currentVol = getVolume(sourceDataObject.getExternalName());
508505
}
509506

510507
if (currentVol == null) {
@@ -523,7 +520,7 @@ public ProviderVolume copy(ProviderAdapterContext context, ProviderAdapterDataOb
523520
"/volumes?names=" + payload.getExternalName() + "&overwrite=true", payload,
524521
new TypeReference<FlashArrayList<FlashArrayVolume>>() {
525522
});
526-
FlashArrayVolume outVolume = (FlashArrayVolume) getFlashArrayItem(list);
523+
FlashArrayVolume outVolume = withAddressType((FlashArrayVolume) getFlashArrayItem(list));
527524
pause(postCopyWait);
528525
return outVolume;
529526
}
@@ -990,11 +987,17 @@ private void removeVlunsAll(ProviderAdapterContext context, String volumeNamespa
990987
}
991988
}
992989

990+
/**
991+
* Look up a volume by name. The pool's address type is always stamped onto the
992+
* result, so no caller can emit an FC-style WWN for a volume that lives on an
993+
* NVMe-TCP pool (see {@link FlashArrayVolume#getAddress()}). The snapshot
994+
* accessor below holds the same invariant.
995+
*/
993996
private FlashArrayVolume getVolume(String volumeName) {
994997
FlashArrayList<FlashArrayVolume> list = GET("/volumes?names=" + volumeName,
995998
new TypeReference<FlashArrayList<FlashArrayVolume>>() {
996999
});
997-
return (FlashArrayVolume) getFlashArrayItem(list);
1000+
return withAddressType((FlashArrayVolume) getFlashArrayItem(list));
9981001
}
9991002

10001003
private FlashArrayPod getVolumeNamespace(String name) {
@@ -1325,9 +1328,10 @@ public Map<String, String> getConnectionIdMap(ProviderAdapterDataObject dataIn)
13251328
if (list != null && list.getItems() != null) {
13261329
for (FlashArrayConnection conn : list.getItems()) {
13271330
if (AddressType.NVMETCP.equals(volumeAddressType)) {
1328-
// Host-group-scoped NVMe connections come back as one
1329-
// entry per host in the group; key on the host name so
1330-
// connid.<hostname> is matched in parseAndValidatePath.
1331+
// Key on the host name so connid.<hostname> is matched by
1332+
// parseAndValidatePath. NVMe-TCP reports an nsid where Fibre
1333+
// Channel reports a lun; the connection itself is host-scoped
1334+
// on both transports.
13311335
if (conn.getHost() != null && conn.getHost().getName() != null) {
13321336
String id = conn.getNsid() != null ? "" + conn.getNsid() : "1";
13331337
map.put(conn.getHost().getName(), id);

0 commit comments

Comments
 (0)