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