Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package org.zstack.header.storage.ceph;

import org.zstack.header.core.ReturnValueCompletion;

/**
* SPI for Ceph HA sibling-fence (ZSTAC-83890).
* Implementation lives in premium {@code storage-ha-plugin}.
* Called from {@code CephPrimaryStorageFactory.preInstantiateVmResource} when
* Ceph watcher list is empty — SSH-kills stale QEMU on the failed host before
* HA starts the VM to prevent split-brain.
*/
public interface CephSiblingFenceExtensionPoint {
void fenceVmOnFailedHost(String failedHostUuid,
String vmUuid,
String clusterUuid,
String haTargetHostUuid,
ReturnValueCompletion<SiblingFenceVmOnHostReply> completion);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package org.zstack.header.storage.ceph;

import org.zstack.header.message.NeedReplyMessage;

public class SiblingFenceVmOnHostMsg extends NeedReplyMessage {
private String failedHostUuid;
private String vmUuid;
private String clusterUuid;
private String haTargetHostUuid;

public String getFailedHostUuid() {
return failedHostUuid;
}

public void setFailedHostUuid(String failedHostUuid) {
this.failedHostUuid = failedHostUuid;
}

public String getVmUuid() {
return vmUuid;
}

public void setVmUuid(String vmUuid) {
this.vmUuid = vmUuid;
}

public String getClusterUuid() {
return clusterUuid;
}

public void setClusterUuid(String clusterUuid) {
this.clusterUuid = clusterUuid;
}

public String getHaTargetHostUuid() {
return haTargetHostUuid;
}

public void setHaTargetHostUuid(String haTargetHostUuid) {
this.haTargetHostUuid = haTargetHostUuid;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package org.zstack.header.storage.ceph;

import org.zstack.header.message.MessageReply;

public class SiblingFenceVmOnHostReply extends MessageReply {
private boolean alive;
private boolean killed;
private boolean sshReachable;
private boolean qemuFound;
private String executorHostUuid;
private String executorRole;
private String reason;

public boolean isAlive() {
return alive;
}

public void setAlive(boolean alive) {
this.alive = alive;
}

public boolean isKilled() {
return killed;
}

public void setKilled(boolean killed) {
this.killed = killed;
}

public boolean isSshReachable() {
return sshReachable;
}

public void setSshReachable(boolean sshReachable) {
this.sshReachable = sshReachable;
}

public boolean isQemuFound() {
return qemuFound;
}

public void setQemuFound(boolean qemuFound) {
this.qemuFound = qemuFound;
}

public String getExecutorHostUuid() {
return executorHostUuid;
}

public void setExecutorHostUuid(String executorHostUuid) {
this.executorHostUuid = executorHostUuid;
}

public String getExecutorRole() {
return executorRole;
}

public void setExecutorRole(String executorRole) {
this.executorRole = executorRole;
}

public String getReason() {
return reason;
}

public void setReason(String reason) {
this.reason = reason;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.zstack.header.configuration.userconfig.InstanceOfferingUserConfig;
import org.zstack.header.configuration.userconfig.InstanceOfferingUserConfigValidator;
import org.zstack.header.core.Completion;
import org.zstack.header.core.ReturnValueCompletion;
import org.zstack.header.core.WhileDoneCompletion;
import org.zstack.header.core.progress.TaskProgressRange;
import org.zstack.header.core.workflow.*;
Expand All @@ -47,6 +48,8 @@
import org.zstack.header.host.HostVO_;
import org.zstack.header.message.MessageReply;
import org.zstack.header.storage.backup.*;
import org.zstack.header.storage.ceph.CephSiblingFenceExtensionPoint;
import org.zstack.header.storage.ceph.SiblingFenceVmOnHostReply;
import org.zstack.header.storage.primary.*;
import org.zstack.header.storage.snapshot.*;
import org.zstack.header.vm.*;
Expand Down Expand Up @@ -1229,7 +1232,50 @@ public void run(MessageReply reply) {
GetVolumeWatchersReply rly = (GetVolumeWatchersReply)reply;
List watchers = rly.getWatchers();
if (watchers == null || watchers.isEmpty()) {
completion.success();
List<CephSiblingFenceExtensionPoint> exts =
pluginRgty.getExtensionList(CephSiblingFenceExtensionPoint.class);
if (exts.isEmpty()) {
// Open-source / no premium installed -> preserve original behaviour
completion.success();
return;
}

final String vmUuid = spec.getVmInventory().getUuid();
final String haTargetHostUuid = spec.getDestHost().getUuid();
final String clusterUuid = spec.getDestHost().getClusterUuid();
final String failedHostUuid = spec.getVmInventory().getLastHostUuid() != null
? spec.getVmInventory().getLastHostUuid()
: spec.getVmInventory().getHostUuid();
if (failedHostUuid == null) {
// No previous host -> fresh instantiation, no fence needed
completion.success();
return;
}

CephSiblingFenceExtensionPoint ext = exts.get(0);
logger.debug(String.format("dispatching ceph sibling-fence for vm[%s] on failedHost[%s], target[%s]",
vmUuid, failedHostUuid, haTargetHostUuid));
ext.fenceVmOnFailedHost(failedHostUuid, vmUuid, clusterUuid, haTargetHostUuid,
new ReturnValueCompletion<SiblingFenceVmOnHostReply>(completion) {
@Override
public void success(SiblingFenceVmOnHostReply r) {
if (!r.isAlive() || r.isKilled()) {
logger.debug(String.format("ceph sibling-fence succeeded for vm[%s] on failedHost[%s]: killed=%s sshReachable=%s",
vmUuid, failedHostUuid, r.isKilled(), r.isSshReachable()));
completion.success();
} else {
logger.warn(String.format("ceph sibling-fence failed for vm[%s] on failedHost[%s]: %s — blocking VM start to prevent split-brain",
vmUuid, failedHostUuid, r.getReason()));
completion.fail(operr("sibling fence failed: %s", r.getReason()));
Comment on lines +1261 to +1269
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

未显式放行 SSH 不可达分支,可能误阻断 HA 拉起

Line [1262] 仅判断 !r.isAlive() || r.isKilled()。按本 PR 目标存在 success-ssh-unreachable 成功态;若扩展实现返回 sshReachable=falsealive=true(或后续实现调整),这里会错误进入 completion.fail(),导致可恢复场景被阻断。建议把 !r.isSshReachable() 作为明确成功条件,避免依赖实现细节。

🔧 建议修复
-                            if (!r.isAlive() || r.isKilled()) {
+                            if (!r.isSshReachable() || !r.isAlive() || r.isKilled()) {
                                 logger.debug(String.format("ceph sibling-fence succeeded for vm[%s] on failedHost[%s]: killed=%s sshReachable=%s",
                                         vmUuid, failedHostUuid, r.isKilled(), r.isSshReachable()));
                                 completion.success();
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageFactory.java`
around lines 1261 - 1269, The success handler for SiblingFenceVmOnHostReply
currently treats only !r.isAlive() || r.isKilled() as success and may
mis-classify ssh-unreachable cases; update the success(SiblingFenceVmOnHostReply
r) branch in CephPrimaryStorageFactory so that r.isSshReachable() is checked
explicitly as part of the decision: treat ssh-unreachable (i.e.,
!r.isSshReachable()) as a success case that calls completion.success(), while
preserving the existing checks for r.isAlive() and r.isKilled() and using
completion.fail(...) only when the reply indicates an actual fence failure
(e.g., r.isAlive() && r.isSshReachable() && !r.isKilled()), referencing the
success(...) method, SiblingFenceVmOnHostReply, r.isSshReachable(), r.isAlive(),
r.isKilled(), completion.success(), and completion.fail().

}
}
@Override
public void fail(ErrorCode err) {
logger.warn(String.format("ceph sibling-fence error for vm[%s] on failedHost[%s]: %s",
vmUuid, failedHostUuid, err));
completion.fail(err);
}
});
return;
}

Expand Down