Skip to content
Open
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
Expand Up @@ -54,7 +54,6 @@ public static class VolumeEncryptionConversionItem implements Serializable {
private String resourceUuid;
private String resourceType;
private String sourceInstallPath;
private String sourceTrashInstallPath;
private String targetInstallPath;
private String targetBackingInstallPath;

Expand Down Expand Up @@ -82,14 +81,6 @@ public void setSourceInstallPath(String sourceInstallPath) {
this.sourceInstallPath = sourceInstallPath;
}

public String getSourceTrashInstallPath() {
return sourceTrashInstallPath;
}

public void setSourceTrashInstallPath(String sourceTrashInstallPath) {
this.sourceTrashInstallPath = sourceTrashInstallPath;
}

public String getTargetInstallPath() {
return targetInstallPath;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package org.zstack.header.storage.primary;

import org.zstack.header.message.NeedReplyMessage;
import org.zstack.header.volume.VolumeInventory;

import java.util.ArrayList;
import java.util.List;

public class RollbackVolumeEncryptionOnPrimaryStorageMsg extends NeedReplyMessage implements PrimaryStorageMessage {
private String hostUuid;
private VolumeInventory volume;
private List<ConvertVolumeEncryptionOnPrimaryStorageMsg.VolumeEncryptionConversionItem> items = new ArrayList<>();

@Override
public String getPrimaryStorageUuid() {
return volume.getPrimaryStorageUuid();
}
Comment on lines +14 to +17

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

getPrimaryStorageUuid() 未对 volume 做空值校验,可能在消息路由阶段抛出 NPE

若调用方在未设置 volume 的情况下发送该消息,消息总线路由时调用 getPrimaryStorageUuid() 会直接抛出 NullPointerException,此时 handle() 方法中已有的空值校验根本不会被执行到(路由发生在 handle() 之前)。建议在此处补充空值保护。

🤖 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
`@header/src/main/java/org/zstack/header/storage/primary/RollbackVolumeEncryptionOnPrimaryStorageMsg.java`
around lines 14 - 17, 在 RollbackVolumeEncryptionOnPrimaryStorageMsg 的
getPrimaryStorageUuid() 中增加 volume 为空时的保护,避免路由阶段直接抛出 NullPointerException;未设置
volume 时返回可表示缺失的结果,并保留已设置 volume 时通过 getPrimaryStorageUuid() 获取主存储 UUID 的行为。


public String getHostUuid() {
return hostUuid;
}

public void setHostUuid(String hostUuid) {
this.hostUuid = hostUuid;
}

public VolumeInventory getVolume() {
return volume;
}

public void setVolume(VolumeInventory volume) {
this.volume = volume;
}

public List<ConvertVolumeEncryptionOnPrimaryStorageMsg.VolumeEncryptionConversionItem> getItems() {
return items;
}

public void setItems(List<ConvertVolumeEncryptionOnPrimaryStorageMsg.VolumeEncryptionConversionItem> items) {
this.items = items;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package org.zstack.header.storage.primary;

import org.zstack.header.message.MessageReply;

public class RollbackVolumeEncryptionOnPrimaryStorageReply extends MessageReply {
}
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,6 @@ public static class KVMHostLuksConvertCmd implements Serializable {
public String encryptedDek;
public String installPath;
public String targetInstallPath;
public String sourceTrashInstallPath;
public boolean targetEncrypted;
public Long virtualSize;
}
Expand Down Expand Up @@ -490,17 +489,8 @@ public static class KVMHostLuksRsp extends KVMAgentCommands.AgentResponse {
public Long actualSize;
}

public static class ReplaceLuksRbdCmd extends AgentCommand {
public String installPath;
public static class RollbackLuksRbdCmd extends AgentCommand {
public String targetInstallPath;
public String temporaryInstallPath;
public String sourceTrashInstallPath;
public boolean deleteSourceTrash;
}

public static class ReplaceLuksRbdRsp extends AgentResponse {
public Long size;
public Long actualSize;
}

public static class SwapInPlaceLuksRbdCmd extends AgentCommand {
Expand Down Expand Up @@ -1454,7 +1444,7 @@ public int compareTo(SnapInfo snapInfo) {
public static final String KVM_HOST_LUKS_ENCRYPT_IN_PLACE_PATH = "/ceph/primarystorage/kvmhost/encryptinplace";
public static final String KVM_HOST_LUKS_RESIZE_PATH = "/ceph/primarystorage/kvmhost/luksresize";
public static final String KVM_HOST_LUKS_CONVERT_PATH = "/ceph/primarystorage/kvmhost/luksconvert";
public static final String LUKS_REPLACE_VOLUME_PATH = "/ceph/primarystorage/volume/luksreplace";
public static final String LUKS_ROLLBACK_CONVERT_PATH = "/ceph/primarystorage/volume/luksconvert/rollback";
public static final String LUKS_SWAP_IN_PLACE_PATH = "/ceph/primarystorage/volume/luksswapinplace";
public static final String KVM_HOST_IMAGESTORE_ENCRYPTED_DOWNLOAD_PATH = "/ceph/primarystorage/kvmhost/imagestore/encrypteddownload";
public static final String FLATTEN_PATH = "/ceph/primarystorage/volume/flatten";
Expand Down Expand Up @@ -3629,12 +3619,83 @@ protected void handle(ConvertVolumeEncryptionOnPrimaryStorageMsg msg) {
KVMHostLuksConvertCmd kcmd = new KVMHostLuksConvertCmd();
kcmd.psUuid = self.getUuid();
kcmd.installPath = item.getSourceInstallPath();
kcmd.targetInstallPath = makeTemporaryLuksRbdInstallPath(item.getTargetInstallPath(), "converting");
kcmd.sourceTrashInstallPath = item.getSourceTrashInstallPath();
kcmd.targetInstallPath = item.getTargetInstallPath();
kcmd.targetEncrypted = msg.isTargetEncrypted();
kcmd.virtualSize = msg.getVolume().getSize();
kcmd.encryptedDek = volumeEncryptedSecretHelper.materializeAndSealVolumeDekForHost(hostUuid, msg.getVolume().getUuid());

CheckIsBitsExistingCmd checkTargetCmd = new CheckIsBitsExistingCmd();
checkTargetCmd.setInstallPath(item.getTargetInstallPath());
httpCall(CHECK_BITS_PATH, checkTargetCmd, CheckIsBitsExistingRsp.class,
new ReturnValueCompletion<CheckIsBitsExistingRsp>(msg) {
@Override
public void success(CheckIsBitsExistingRsp returnValue) {
if (returnValue.isExisting()) {
reply.setError(operr("RBD LUKS conversion target already exists: %s", item.getTargetInstallPath()));
bus.reply(msg, reply);
return;
}
convertVolumeEncryptionOnKvmHost(msg, reply, kcmd, hostUuid);
}

@Override
public void fail(ErrorCode errorCode) {
reply.setError(errorCode);
bus.reply(msg, reply);
}
});
}

@Override
protected void handle(RollbackVolumeEncryptionOnPrimaryStorageMsg msg) {
RollbackVolumeEncryptionOnPrimaryStorageReply reply = new RollbackVolumeEncryptionOnPrimaryStorageReply();
if (msg.getItems() == null || msg.getItems().size() != 1) {
reply.setError(operr(
"ceph rollback volume encryption conversion requires volume snapshots to be merged; volume[uuid:%s] has %s rollback items",
msg.getVolume().getUuid(), msg.getItems() == null ? 0 : msg.getItems().size()));
bus.reply(msg, reply);
return;
}

ConvertVolumeEncryptionOnPrimaryStorageMsg.VolumeEncryptionConversionItem item = msg.getItems().get(0);
if (!VolumeVO.class.getSimpleName().equals(item.getResourceType())) {
reply.setError(operr(
"ceph rollback volume encryption conversion only supports the active volume, but got resource[type:%s, uuid:%s]",
item.getResourceType(), item.getResourceUuid()));
bus.reply(msg, reply);
return;
}

if (StringUtils.isBlank(item.getTargetInstallPath()) || item.getTargetInstallPath().contains("@")) {
reply.setError(operr(
"ceph rollback volume encryption conversion requires an active RBD target path, but got target path[%s]",
item.getTargetInstallPath()));
bus.reply(msg, reply);
return;
}

RollbackLuksRbdCmd cmd = new RollbackLuksRbdCmd();
cmd.targetInstallPath = item.getTargetInstallPath();
httpCall(LUKS_ROLLBACK_CONVERT_PATH, cmd, AgentResponse.class,
new ReturnValueCompletion<AgentResponse>(msg) {
@Override
public void success(AgentResponse returnValue) {
bus.reply(msg, reply);
}

@Override
public void fail(ErrorCode errorCode) {
reply.setError(errorCode);
bus.reply(msg, reply);
}
});
}

private void convertVolumeEncryptionOnKvmHost(ConvertVolumeEncryptionOnPrimaryStorageMsg msg,
ConvertVolumeEncryptionOnPrimaryStorageReply reply,
KVMHostLuksConvertCmd kcmd,
String hostUuid) {
ConvertVolumeEncryptionOnPrimaryStorageMsg.VolumeEncryptionConversionItem item = msg.getItems().get(0);
httpCallToKvmHost(hostUuid, KVM_HOST_LUKS_CONVERT_PATH, kcmd, KVMHostLuksRsp.class,
new ReturnValueCompletion<KVMHostLuksRsp>(msg) {
@Override
Expand All @@ -3646,16 +3707,16 @@ public void fail(ErrorCode err) {

@Override
public void success(KVMHostLuksRsp ret) {
ReplaceLuksRbdCmd rcmd = new ReplaceLuksRbdCmd();
rcmd.installPath = item.getSourceInstallPath();
rcmd.targetInstallPath = item.getTargetInstallPath();
rcmd.temporaryInstallPath = kcmd.targetInstallPath;
rcmd.sourceTrashInstallPath = item.getSourceTrashInstallPath();
rcmd.deleteSourceTrash = false;
httpCall(LUKS_REPLACE_VOLUME_PATH, rcmd, ReplaceLuksRbdRsp.class,
new ReturnValueCompletion<ReplaceLuksRbdRsp>(msg) {
if (ret.actualSize != null) {
reply.setActualSizes(Collections.singletonMap(msg.getVolume().getUuid(), ret.actualSize));
bus.reply(msg, reply);
return;
}

syncVolumeSize(msg.getVolume().getUuid(), item.getTargetInstallPath(),
new ReturnValueCompletion<GetVolumeSizeRsp>(msg) {
@Override
public void success(ReplaceLuksRbdRsp rsp) {
public void success(GetVolumeSizeRsp rsp) {
if (rsp.actualSize != null) {
reply.setActualSizes(Collections.singletonMap(msg.getVolume().getUuid(), rsp.actualSize));
}
Expand All @@ -3664,8 +3725,8 @@ public void success(ReplaceLuksRbdRsp rsp) {

@Override
public void fail(ErrorCode errorCode) {
deleteRbdBitsBestEffort(kcmd.targetInstallPath);
reply.setError(errorCode);
logger.warn(String.format("failed to sync converted RBD volume[uuid:%s, installPath:%s] size: %s",
msg.getVolume().getUuid(), item.getTargetInstallPath(), errorCode.getReadableDetails()));
bus.reply(msg, reply);
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -912,6 +912,8 @@ public void handleLocalMessage(Message msg) {
handle((EncryptVolumeBitsOnPrimaryStorageMsg) msg);
} else if (msg instanceof ConvertVolumeEncryptionOnPrimaryStorageMsg) {
handle((ConvertVolumeEncryptionOnPrimaryStorageMsg) msg);
} else if (msg instanceof RollbackVolumeEncryptionOnPrimaryStorageMsg) {
handle((RollbackVolumeEncryptionOnPrimaryStorageMsg) msg);
} else {
super.handleLocalMessage(msg);
}
Expand Down Expand Up @@ -959,6 +961,48 @@ public void fail(ErrorCode errorCode) {
});
}

protected void handle(RollbackVolumeEncryptionOnPrimaryStorageMsg msg) {
RollbackVolumeEncryptionOnPrimaryStorageReply reply = new RollbackVolumeEncryptionOnPrimaryStorageReply();
if (msg.getVolume() == null || StringUtils.isBlank(msg.getVolume().getUuid())) {
reply.setError(operr("rollback volume encryption on local primary storage[uuid:%s] requires volume with non-blank uuid",
self.getUuid()));
bus.reply(msg, reply);
return;
}

String hostUuid = StringUtils.isNotBlank(msg.getHostUuid()) ?
msg.getHostUuid() : null;
if (StringUtils.isBlank(hostUuid)) {
try {
hostUuid = getHostUuidByResourceUuid(msg.getVolume().getUuid());
} catch (OperationFailureException e) {
reply.setError(e.getErrorCode());
bus.reply(msg, reply);
return;
}
}
if (StringUtils.isBlank(hostUuid)) {
reply.setError(operr("cannot determine host for rolling back volume[uuid:%s] encryption conversion on local primary storage[uuid:%s]",
msg.getVolume().getUuid(), self.getUuid()));
bus.reply(msg, reply);
return;
}

LocalStorageHypervisorBackend bkd = getHypervisorBackendFactoryByHostUuid(hostUuid).getHypervisorBackend(self);
bkd.handle(msg, hostUuid, new ReturnValueCompletion<RollbackVolumeEncryptionOnPrimaryStorageReply>(msg) {
@Override
public void success(RollbackVolumeEncryptionOnPrimaryStorageReply returnValue) {
bus.reply(msg, returnValue);
}

@Override
public void fail(ErrorCode errorCode) {
reply.setError(errorCode);
bus.reply(msg, reply);
}
});
}

private void handle(EncryptVolumeBitsOnPrimaryStorageMsg msg) {
if (StringUtils.isBlank(msg.getHostUuid()) || StringUtils.isBlank(msg.getVolumeUuid())) {
EncryptVolumeBitsOnPrimaryStorageReply reply = new EncryptVolumeBitsOnPrimaryStorageReply();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,4 +139,6 @@ abstract void createEmptyVolumeWithBackingFile(VolumeInventory volume, String ho
abstract void handle(EncryptVolumeBitsOnPrimaryStorageMsg msg, ReturnValueCompletion<EncryptVolumeBitsOnPrimaryStorageReply> completion);

abstract void handle(ConvertVolumeEncryptionOnPrimaryStorageMsg msg, String hostUuid, ReturnValueCompletion<ConvertVolumeEncryptionOnPrimaryStorageReply> completion);

abstract void handle(RollbackVolumeEncryptionOnPrimaryStorageMsg msg, String hostUuid, ReturnValueCompletion<RollbackVolumeEncryptionOnPrimaryStorageReply> completion);
}
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,14 @@ public static class ConvertVolumeEncryptionRsp extends AgentResponse {
public Map<String, Long> actualSizes;
}

public static class RollbackVolumeEncryptionCmd extends AgentCommand implements Serializable {
public String volumeUuid;
public List<ConvertVolumeEncryptionOnPrimaryStorageMsg.VolumeEncryptionConversionItem> items;
}

public static class RollbackVolumeEncryptionRsp extends AgentResponse {
}

public static class GetPhysicalCapacityCmd extends AgentCommand {
private String hostUuid;

Expand Down Expand Up @@ -1097,6 +1105,7 @@ public static class PrefixRebaseBackingFilesRsp extends LocalStorageKvmBackend.A
public static final String PREFIX_REBASE_BACKING_FILES_PATH = "/localstorage/snapshot/prefixrebasebackingfiles";
public static final String ENCRYPT_VOLUME_BITS_PATH = "/localstorage/volume/encryptinplace";
public static final String CONVERT_VOLUME_ENCRYPTION_PATH = "/localstorage/volume/convertencryption";
public static final String ROLLBACK_VOLUME_ENCRYPTION_PATH = "/localstorage/volume/convertencryption/rollback";

public LocalStorageKvmBackend() {
}
Expand Down Expand Up @@ -4183,6 +4192,28 @@ public void fail(ErrorCode errorCode) {
});
}

@Override
void handle(RollbackVolumeEncryptionOnPrimaryStorageMsg msg, String hostUuid,
ReturnValueCompletion<RollbackVolumeEncryptionOnPrimaryStorageReply> completion) {
RollbackVolumeEncryptionCmd cmd = new RollbackVolumeEncryptionCmd();
cmd.volumeUuid = msg.getVolume().getUuid();
cmd.items = msg.getItems();

httpCall(ROLLBACK_VOLUME_ENCRYPTION_PATH, hostUuid, cmd, RollbackVolumeEncryptionRsp.class,
new ReturnValueCompletion<RollbackVolumeEncryptionRsp>(completion) {
@Override
public void success(RollbackVolumeEncryptionRsp rsp) {
completion.success(new RollbackVolumeEncryptionOnPrimaryStorageReply());
}

@Override
public void fail(ErrorCode errorCode) {
completion.fail(operr("failed to rollback volume[uuid:%s] encryption conversion on local storage host[uuid:%s]: %s",
msg.getVolume().getUuid(), hostUuid, errorCode));
}
});
}

private String prepareVolumeEncryptedDek(String hostUuid, VolumeInventory volume, boolean required) {
if (!required) {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ protected void handleLocalMessage(Message msg) {
handle((EncryptVolumeBitsOnPrimaryStorageMsg) msg);
} else if (msg instanceof ConvertVolumeEncryptionOnPrimaryStorageMsg) {
handle((ConvertVolumeEncryptionOnPrimaryStorageMsg) msg);
} else if (msg instanceof RollbackVolumeEncryptionOnPrimaryStorageMsg) {
handle((RollbackVolumeEncryptionOnPrimaryStorageMsg) msg);
} else {
super.handleLocalMessage(msg);
}
Expand Down Expand Up @@ -169,6 +171,30 @@ public void fail(ErrorCode errorCode) {
});
}

protected void handle(RollbackVolumeEncryptionOnPrimaryStorageMsg msg) {
RollbackVolumeEncryptionOnPrimaryStorageReply reply = new RollbackVolumeEncryptionOnPrimaryStorageReply();
NfsPrimaryStorageBackend backend = getUsableBackend();
if (backend == null) {
reply.setError(operr("the NFS primary storage[uuid:%s, name:%s] cannot find any usable host to rollback volume[uuid:%s] encryption conversion",
self.getUuid(), self.getName(), msg.getVolume().getUuid()));
bus.reply(msg, reply);
return;
}

backend.handle(getSelfInventory(), msg, new ReturnValueCompletion<RollbackVolumeEncryptionOnPrimaryStorageReply>(msg) {
@Override
public void success(RollbackVolumeEncryptionOnPrimaryStorageReply returnValue) {
bus.reply(msg, returnValue);
}

@Override
public void fail(ErrorCode errorCode) {
reply.setError(errorCode);
bus.reply(msg, reply);
}
});
}

private void handle(EncryptVolumeBitsOnPrimaryStorageMsg msg) {
NfsPrimaryStorageBackend backend = getUsableBackend();
if (backend == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ public interface NfsPrimaryStorageBackend {

void handle(PrimaryStorageInventory inv, ConvertVolumeEncryptionOnPrimaryStorageMsg msg, ReturnValueCompletion<ConvertVolumeEncryptionOnPrimaryStorageReply> completion);

void handle(PrimaryStorageInventory inv, RollbackVolumeEncryptionOnPrimaryStorageMsg msg, ReturnValueCompletion<RollbackVolumeEncryptionOnPrimaryStorageReply> completion);

void getPhysicalCapacity(PrimaryStorageInventory inv, ReturnValueCompletion<PhysicalCapacityUsage> completion);

void checkIsBitsExisting(PrimaryStorageInventory inv, String installPath, ReturnValueCompletion<Boolean> completion);
Expand Down
Loading