From d2b599dd5475d773a4235490eef93c35a1679a11 Mon Sep 17 00:00:00 2001 From: "yaohua.wu" Date: Fri, 3 Jul 2026 11:57:16 +0800 Subject: [PATCH] [externalStorage]: prevent split-brain on deactivate fail Backport external-storage split-brain guards to 5.4.10. 1. Why? Starting a VM on a new host may continue even when the old storage client still holds the volume. For xinfini Vhost volumes this can leave two QEMU processes using the same bdev. 2. How? Report xinfini deactivate failures instead of swallowing them, verify the bdev is really deleted, re-check active clients after deactivate success, and make blacklist failures propagate out of beforeStartVmOnKvm. 3. Side effects? Deactivate failure or a still-active old client now aborts VM start for that volume instead of silently continuing. Happy-path deactivate keeps the previous behavior. # Summary of changes (by module): - header: make PrimaryStorageNodeSvc.blacklist synchronous. - externalStorage: post-check active clients and propagate blacklist failures. - xinfini: surface deactivate/delete failures and abort unsupported isolation. - expon/zbs: adapt blacklist implementations to the synchronous signature. Related: ZSTAC-86444 Related: ZSTAC-85124 Change-Id: I59812c6d68eec6533e48a8c9cf05b7c55d6b85eb --- .../addon/primary/PrimaryStorageNodeSvc.java | 2 +- .../zstack/expon/ExponStorageController.java | 3 +- .../kvm/ExternalPrimaryStorageKvmFactory.java | 18 +++++-- .../org/zstack/xinfini/XInfiniApiHelper.java | 5 ++ .../xinfini/XInfiniStorageController.java | 53 +++++++++++++------ .../storage/zbs/ZbsStorageController.java | 3 +- 6 files changed, 61 insertions(+), 23 deletions(-) diff --git a/header/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageNodeSvc.java b/header/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageNodeSvc.java index f292e694183..2ab0bcb4653 100644 --- a/header/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageNodeSvc.java +++ b/header/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageNodeSvc.java @@ -13,7 +13,7 @@ public interface PrimaryStorageNodeSvc { void deactivate(String installPath, String protocol, ActiveVolumeClient client, Completion comp); - void blacklist(String installPath, String protocol, HostInventory h, Completion comp); + void blacklist(String installPath, String protocol, HostInventory h); String getActivePath(BaseVolumeInfo v, HostInventory h, boolean shareable); BaseVolumeInfo getActiveVolumeInfo(String activePath, HostInventory h, boolean shareable); diff --git a/plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java b/plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java index 2a3762f98ff..8cb331a91de 100644 --- a/plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java +++ b/plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java @@ -614,14 +614,13 @@ public void cleanActiveRecord(ImageCacheInventory cache) { } @Override - public void blacklist(String installPath, String protocol, HostInventory h, Completion comp) { + public void blacklist(String installPath, String protocol, HostInventory h) { logger.debug(String.format("blacklisting volume[path: %s, protocol:%s] on host[uuid:%s, ip:%s]", installPath, protocol, h.getUuid(), h.getManagementIp())); UssGatewayModule uss = getUssGateway(VolumeProtocol.valueOf(protocol), h.getManagementIp()); VolumeModule exponVol = apiHelper.getVolume(getVolIdFromPath(installPath)); apiHelper.addVolumePathToBlacklist(buildExponVolumeBoundPath(uss, exponVol.getVolumeName())); - comp.success(); } @Override diff --git a/plugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.java b/plugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.java index c6eb04a8557..90aca223f1e 100644 --- a/plugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.java +++ b/plugin/externalStorage/src/main/java/org/zstack/externalStorage/primary/kvm/ExternalPrimaryStorageKvmFactory.java @@ -334,6 +334,7 @@ public void done(ErrorCodeList errorCodeList) { @Override public void beforeStartVmOnKvm(KVMHostInventory host, VmInstanceSpec spec, KVMAgentCommands.StartVmCmd cmd) { List vols = getManagerExclusiveVolume(spec); + ErrorCode[] deactivateErr = new ErrorCode[1]; for (VolumeInventory vol : vols) { PrimaryStorageNodeSvc nodeSvc = extPsFactory.getNodeSvc(vol.getPrimaryStorageUuid()); @@ -359,20 +360,31 @@ public void beforeStartVmOnKvm(KVMHostInventory host, VmInstanceSpec spec, KVMAg clientHost.getUuid(), clientHost.getManagementIp(), host.getUuid(), host.getManagementIp())); + deactivateErr[0] = null; nodeSvc.deactivate(vol.getInstallPath(), vol.getProtocol(), client, new Completion(null) { @Override public void success() { + boolean stillActive = nodeSvc.getActiveClients(vol.getInstallPath(), vol.getProtocol()).stream() + .anyMatch(c -> client.getManagerIp().equals(c.getManagerIp())); + if (stillActive) { + deactivateErr[0] = operr("deactivate reported success but volume[uuid:%s, installPath:%s] is still active on host[uuid:%s, ip:%s]", + vol.getUuid(), vol.getInstallPath(), clientHost.getUuid(), clientHost.getManagementIp()); + return; + } logger.info(String.format("successfully deactivate volume[uuid:%s, installPath:%s] on host[uuid:%s, ip:%s]", vol.getUuid(), vol.getInstallPath(), clientHost.getUuid(), clientHost.getManagementIp())); } @Override public void fail(ErrorCode errorCode) { - logger.warn(String.format("failed to deactivate volume[uuid:%s, installPath:%s] on host[uuid:%s, ip:%s], add it to blacklist", - vol.getUuid(), vol.getInstallPath(), clientHost.getUuid(), clientHost.getManagementIp())); - nodeSvc.blacklist(vol.getInstallPath(), vol.getProtocol(), HostInventory.valueOf(clientHost), new NopeCompletion()); + deactivateErr[0] = errorCode; } }); + if (deactivateErr[0] != null) { + logger.warn(String.format("failed to deactivate volume[uuid:%s, installPath:%s] on host[uuid:%s, ip:%s]: %s, add it to blacklist", + vol.getUuid(), vol.getInstallPath(), clientHost.getUuid(), clientHost.getManagementIp(), deactivateErr[0].getDetails())); + nodeSvc.blacklist(vol.getInstallPath(), vol.getProtocol(), HostInventory.valueOf(clientHost)); + } } } }); diff --git a/plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniApiHelper.java b/plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniApiHelper.java index 3a90db3b043..2177283b48b 100644 --- a/plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniApiHelper.java +++ b/plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniApiHelper.java @@ -472,6 +472,11 @@ public void deleteBdcBdev(int bdevId, int bdcId) { } else { retryUtilResourceDeleted(gReq, GetBdcBdevResponse.class); } + + if (!call(gReq, GetBdcBdevResponse.class).resourceIsDeleted()) { + throw new OperationFailureException(operr("bdev[id:%s] still exists after deletion polling on bdc[id:%s], " + + "the old storage client may still hold the volume", bdevId, bdcId)); + } } public VolumeModule rollbackSnapshot(int volId, int snapId) { diff --git a/plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniStorageController.java b/plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniStorageController.java index 267d55f4039..53c8cca1751 100644 --- a/plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniStorageController.java +++ b/plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniStorageController.java @@ -202,18 +202,23 @@ public synchronized ActiveVolumeTO activeIscsiVolume(String clientIp, String cli public void deactivate(String installPath, String protocol, HostInventory h, Completion comp) { logger.debug(String.format("deactivating volume[path: %s, protocol:%s] on host[uuid:%s, ip:%s]", installPath, protocol, h.getUuid(), h.getManagementIp())); - if (VolumeProtocol.Vhost.toString().equals(protocol)) { - deactivateVhost(installPath, h); - comp.success(); - return; - } else if (VolumeProtocol.iSCSI.toString().equals(protocol)) { - // iscsi target is shared by all hosts, we cannot control one volume on one host for now. - deactivateIscsi(installPath, h); - comp.success(); + try { + if (VolumeProtocol.Vhost.toString().equals(protocol)) { + deactivateVhost(installPath, h); + } else if (VolumeProtocol.iSCSI.toString().equals(protocol)) { + // iscsi target is shared by all hosts, we cannot control one volume on one host for now. + deactivateIscsi(installPath, h); + } else { + comp.fail(operr("not supported protocol[%s] for deactivate", protocol)); + return; + } + } catch (Exception e) { + comp.fail(operr("failed to deactivate volume[path:%s, protocol:%s] on host[uuid:%s, ip:%s]: %s", + installPath, protocol, h.getUuid(), h.getManagementIp(), e.getMessage())); return; } - comp.fail(operr("not supported protocol[%s] for deactivate", protocol)); + comp.success(); } private void deactivateIscsi(String installPath, HostInventory h) { @@ -255,7 +260,7 @@ private void deactivateVhost(String installPath, HostInventory h) { return; } - retry(() -> apiHelper.deleteBdcBdev(bdev.getSpec().getId(), bdc.getSpec().getId())); + retryOrThrow(() -> apiHelper.deleteBdcBdev(bdev.getSpec().getId(), bdc.getSpec().getId())); } @Override @@ -271,9 +276,10 @@ public void deactivate(String installPath, String protocol, ActiveVolumeClient c } @Override - public void blacklist(String installPath, String protocol, HostInventory h, Completion comp) { - // todo - comp.success(); + public void blacklist(String installPath, String protocol, HostInventory h) { + throw new OperationFailureException(operr("xinfini does not support volume path isolation yet, " + + "abort starting VM on host[uuid:%s, ip:%s] to prevent split-brain on volume[path:%s, protocol:%s]", + h.getUuid(), h.getManagementIp(), installPath, protocol)); } @Override @@ -1038,16 +1044,33 @@ private void retry(Runnable r) { private void retry(Runnable r, int retry) { - while (retry-- > 0) { + try { + retryOrThrow(r, retry); + } catch (RuntimeException ignored) { + } + } + + private void retryOrThrow(Runnable r) { + retryOrThrow(r, 3); + } + + private void retryOrThrow(Runnable r, int retry) { + RuntimeException lastError = null; + for (int i = 0; i < retry; i++) { try { r.run(); return; - } catch (Exception e) { + } catch (RuntimeException e) { + lastError = e; logger.warn("runnable failed, try ", e); try { TimeUnit.SECONDS.sleep(3); } catch (InterruptedException ignore) {} } } + + if (lastError != null) { + throw lastError; + } } } diff --git a/plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java b/plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java index 4b725d7d952..faddfdbd5da 100644 --- a/plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java +++ b/plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java @@ -145,8 +145,7 @@ public void deactivate(String installPath, String protocol, ActiveVolumeClient c } @Override - public void blacklist(String installPath, String protocol, HostInventory h, Completion comp) { - comp.success(); + public void blacklist(String installPath, String protocol, HostInventory h) { } @Override