Skip to content

<fix>[zbs]: sync cow snapshot size in scheduled volume resource sync #4599

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ZStack-Robot wants to merge 1 commit into
base: 5.5.30
Choose a base branch
from
Open

<fix>[zbs]: sync cow snapshot size in scheduled volume resource sync #4599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions ...rc/main/java/org/zstack/header/storage/addon/primary/ExternalPrimaryStorageInventory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ private static String desensitizeUrl(String url) {
return url;
}

@Override
public String getIdentity() {
return identity;
}
Expand Down
3 changes: 2 additions & 1 deletion ...er/src/main/java/org/zstack/header/storage/addon/primary/PrimaryStorageControllerSvc.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import org.zstack.header.core.ReturnValueCompletion;
import org.zstack.header.host.HostInventory;
import org.zstack.header.storage.addon.*;
import org.zstack.header.storage.primary.StorageResourceStats;
import org.zstack.header.storage.snapshot.VolumeSnapshotStats;
import org.zstack.header.volume.VolumeProtocol;
import org.zstack.header.volume.VolumeStats;
Expand Down Expand Up @@ -64,7 +65,7 @@ public interface PrimaryStorageControllerSvc {
// support uri or path
void stats(String installPath, ReturnValueCompletion<VolumeStats> comp);

void batchStats(Collection<String> installPath, ReturnValueCompletion<List<VolumeStats>> comp);
void batchStats(Collection<String> installPaths, ReturnValueCompletion<List<StorageResourceStats>> comp);

void expandVolume(String installPath, long size, ReturnValueCompletion<VolumeStats> comp);
void setVolumeQos(BaseVolumeInfo v, Completion comp);
Expand Down
9 changes: 9 additions & 0 deletions header/src/main/java/org/zstack/header/storage/primary/AskVolumeSnapshotCapabilityMsg.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
public class AskVolumeSnapshotCapabilityMsg extends NeedReplyMessage implements PrimaryStorageMessage {
private String primaryStorageUuid;
private VolumeInventory volume;
private String volumeType;

@Override
public String getPrimaryStorageUuid() {
Expand All @@ -26,4 +27,12 @@ public VolumeInventory getVolume() {
public void setVolume(VolumeInventory volume) {
this.volume = volume;
}

public String getVolumeType() {
return volumeType;
}

public void setVolumeType(String volumeType) {
this.volumeType = volumeType;
}
}
4 changes: 4 additions & 0 deletions header/src/main/java/org/zstack/header/storage/primary/PrimaryStorageInventory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,10 @@ public String getType() {
return type;
}

public String getIdentity() {
return type;
}

public void setType(String type) {
this.type = type;
}
Expand Down
31 changes: 31 additions & 0 deletions header/src/main/java/org/zstack/header/storage/primary/StorageResourceStats.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package org.zstack.header.storage.primary;

public class StorageResourceStats {
protected String installPath;
protected Long actualSize;
protected Long size;

public String getInstallPath() {
return installPath;
}

public void setInstallPath(String installPath) {
this.installPath = installPath;
}

public Long getActualSize() {
return actualSize;
}

public void setActualSize(Long actualSize) {
this.actualSize = actualSize;
}

public Long getSize() {
return size;
}

public void setSize(long size) {
this.size = size;
Comment on lines +28 to +29

@coderabbitai coderabbitai Bot Jul 17, 2026

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

setSize 参数类型与字段类型不一致，可能导致 NPE。

size 字段为 Long（包装类型），setActualSize 接受 Long，但 setSize 接受 long（基本类型）。在 ZbsStorageController.batchStats 中调用 s.setSize(v.getValue().get("length")) 时，若 Map 中不存在 "length" 键，get 返回 null，自动拆箱为 long 将抛出 NullPointerException。

🔧 建议修复：统一 setter 参数类型 
-    public void setSize(long size) {
+    public void setSize(Long size) {
         this.size = size;
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public void setSize(long size) {
this.size = size;
public void setSize(Long size) {
this.size = size;
🤖 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/StorageResourceStats.java`
around lines 28 - 29, 将 StorageResourceStats.setSize 的参数类型从基本类型 long 改为包装类型
Long，使其与 size 字段及 setActualSize 保持一致；同时保留 batchStats 调用中 length 缺失时传递 null
的行为，避免自动拆箱触发 NullPointerException。

}
}
15 changes: 15 additions & 0 deletions header/src/main/java/org/zstack/header/storage/primary/VolumeSnapshotCapability.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ public static enum VolumeSnapshotPlacementType {
EXTERNAL,
}

public static enum VolumeSnapshotMode {
REDIRECT_ON_WRITE,
COPY_ON_WRITE,
}

private boolean support;

/***
Expand All @@ -34,6 +39,8 @@ public static enum VolumeSnapshotPlacementType {
private VolumeSnapshotArrangementType arrangementType;

private VolumeSnapshotPlacementType placementType;

private VolumeSnapshotMode mode;

/***
* If volume snapshot is inner snapshot on volume, it must be set.
Expand Down Expand Up @@ -66,6 +73,14 @@ public void setPlacementType(VolumeSnapshotPlacementType placementType) {
this.placementType = placementType;
}

public VolumeSnapshotMode getMode() {
return mode;
}

public void setMode(VolumeSnapshotMode mode) {
this.mode = mode;
}

public boolean isSupportCreateOnHypervisor() {
return supportCreateOnHypervisor;
}
Expand Down
20 changes: 2 additions & 18 deletions header/src/main/java/org/zstack/header/storage/snapshot/VolumeSnapshotStats.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,6 @@
package org.zstack.header.storage.snapshot;

public class VolumeSnapshotStats {
private String installPath;
private long actualSize;
import org.zstack.header.storage.primary.StorageResourceStats;

public String getInstallPath() {
return installPath;
}

public void setInstallPath(String installPath) {
this.installPath = installPath;
}

public long getActualSize() {
return actualSize;
}

public void setActualSize(long actualSize) {
this.actualSize = actualSize;
}
public class VolumeSnapshotStats extends StorageResourceStats {
}
13 changes: 13 additions & 0 deletions header/src/main/java/org/zstack/header/volume/BatchSyncActiveVolumeSizeOnHostReply.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,16 @@

import org.zstack.header.message.MessageReply;

import java.util.HashSet;
import java.util.Set;

public class BatchSyncActiveVolumeSizeOnHostReply extends MessageReply {
private Integer successCount = 0;

private Integer failCount = 0;

private Set<String> snapshotSizeSyncRequiredCacheKeys = new HashSet<>();

public void setSuccessCount(Integer successCount) {
this.successCount = successCount;
}
Expand All @@ -30,4 +35,12 @@ public synchronized void addSuccessCount(Integer successCount) {
public synchronized void addFailCount(Integer failCount) {
this.failCount = this.failCount + failCount;
}

public Set<String> getSnapshotSizeSyncRequiredCacheKeys() {
return snapshotSizeSyncRequiredCacheKeys;
}

public void setSnapshotSizeSyncRequiredCacheKeys(Set<String> snapshotSizeSyncRequiredCacheKeys) {
this.snapshotSizeSyncRequiredCacheKeys = snapshotSizeSyncRequiredCacheKeys;
}
}
22 changes: 21 additions & 1 deletion ...tchSyncVolumeSizeOnPrimaryStorageMsg.java → ...olumeResourceSizeOnPrimaryStorageMsg.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,17 @@

import java.util.Map;

public class BatchSyncVolumeSizeOnPrimaryStorageMsg extends NeedReplyMessage implements PrimaryStorageMessage {
public class BatchSyncVolumeResourceSizeOnPrimaryStorageMsg extends NeedReplyMessage implements PrimaryStorageMessage {
private String hostUuid;

private String primaryStorageUuid;

private Map<String, String> volumeUuidInstallPaths;

private Map<String, String> snapshotUuidInstallPaths;

private boolean withSnapshot;

public void setHostUuid(String hostUuid) {
this.hostUuid = hostUuid;
}
Expand All @@ -35,4 +39,20 @@ public void setVolumeUuidInstallPaths(Map<String, String> volumeUuidInstallPaths
public Map<String, String> getVolumeUuidInstallPaths() {
return volumeUuidInstallPaths;
}

public Map<String, String> getSnapshotUuidInstallPaths() {
return snapshotUuidInstallPaths;
}

public void setSnapshotUuidInstallPaths(Map<String, String> snapshotUuidInstallPaths) {
this.snapshotUuidInstallPaths = snapshotUuidInstallPaths;
}

public boolean isWithSnapshot() {
return withSnapshot;
}

public void setWithSnapshot(boolean withSnapshot) {
this.withSnapshot = withSnapshot;
}
}
28 changes: 28 additions & 0 deletions .../main/java/org/zstack/header/volume/BatchSyncVolumeResourceSizeOnPrimaryStorageReply.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package org.zstack.header.volume;

import org.zstack.header.message.MessageReply;

import java.util.HashMap;
import java.util.Map;

public class BatchSyncVolumeResourceSizeOnPrimaryStorageReply extends MessageReply {
private Map<String, Long> volumeActualSizes = new HashMap<>();

private Map<String, Long> snapshotActualSizes = new HashMap<>();

public void setVolumeActualSizes(Map<String, Long> actualSizes) {
this.volumeActualSizes = actualSizes;
}

public Map<String, Long> getVolumeActualSizes() {
return volumeActualSizes;
}

public Map<String, Long> getSnapshotActualSizes() {
return snapshotActualSizes;
}

public void setSnapshotActualSizes(Map<String, Long> snapshotActualSizes) {
this.snapshotActualSizes = snapshotActualSizes;
}
}
18 changes: 0 additions & 18 deletions header/src/main/java/org/zstack/header/volume/BatchSyncVolumeSizeOnPrimaryStorageReply.java
View file
Open in desktop

This file was deleted.

31 changes: 3 additions & 28 deletions header/src/main/java/org/zstack/header/volume/VolumeStats.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
package org.zstack.header.volume;

public class VolumeStats {
protected String installPath;
import org.zstack.header.storage.primary.StorageResourceStats;

public class VolumeStats extends StorageResourceStats {
protected String format;
protected Long actualSize;
protected Long size;
/**
* The parent uri of the volume, vendor://pool/path@snapshot or snapshot://uuid
*/
Expand All @@ -29,30 +28,6 @@ public VolumeStats(String installPath, Long actualSize, Long size) {
public VolumeStats() {
}

public String getInstallPath() {
return installPath;
}

public void setInstallPath(String installPath) {
this.installPath = installPath;
}

public Long getActualSize() {
return actualSize;
}

public void setActualSize(Long actualSize) {
this.actualSize = actualSize;
}

public Long getSize() {
return size;
}

public void setSize(long size) {
this.size = size;
}

public void setFormat(String format) {
this.format = format;
}
Expand Down
17 changes: 11 additions & 6 deletions plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3325,11 +3325,16 @@ protected void handle(AskVolumeSnapshotCapabilityMsg msg) {
AskVolumeSnapshotCapabilityReply reply = new AskVolumeSnapshotCapabilityReply();
VolumeSnapshotCapability cap = new VolumeSnapshotCapability();

String volumeType = msg.getVolume().getType();
String volumeType = msg.getVolumeType();
if (volumeType == null) {
volumeType = msg.getVolume().getType();
}

if (VolumeType.Data.toString().equals(volumeType) || VolumeType.Root.toString().equals(volumeType)) {
cap.setSupport(true);
cap.setArrangementType(VolumeSnapshotArrangementType.INDIVIDUAL);
cap.setPlacementType(VolumeSnapshotCapability.VolumeSnapshotPlacementType.INTERNAL);
cap.setMode(VolumeSnapshotCapability.VolumeSnapshotMode.REDIRECT_ON_WRITE);
cap.setVolumePathFromInternalSnapshotRegex("^[^@]+");
} else if (VolumeType.Memory.toString().equals(volumeType)) {
cap.setSupport(false);
Expand Down Expand Up @@ -3397,10 +3402,10 @@ public void fail(ErrorCode errorCode) {
}

@Override
protected void handle(BatchSyncVolumeSizeOnPrimaryStorageMsg msg) {
protected void handle(BatchSyncVolumeResourceSizeOnPrimaryStorageMsg msg) {
inQueue().name(String.format("batch-sync-volume-size-on-primarystorage-%s", self.getUuid()))
.asyncBackup(msg)
.run(chain -> BatchSyncVolumeSizeOnPrimaryStorage(msg, new NoErrorCompletion(chain) {
.run(chain -> BatchSyncVolumeResourceSizeOnPrimaryStorage(msg, new NoErrorCompletion(chain) {
@Override
public void done() {
chain.next();
Expand All @@ -3419,8 +3424,8 @@ protected void syncVolumeSize(String volumeUuid, String installPath, final Retur
httpCall(GET_VOLUME_SIZE_PATH, cmd, GetVolumeSizeRsp.class, completion);
}

private void BatchSyncVolumeSizeOnPrimaryStorage(BatchSyncVolumeSizeOnPrimaryStorageMsg msg, NoErrorCompletion completion) {
BatchSyncVolumeSizeOnPrimaryStorageReply reply = new BatchSyncVolumeSizeOnPrimaryStorageReply();
private void BatchSyncVolumeResourceSizeOnPrimaryStorage(BatchSyncVolumeResourceSizeOnPrimaryStorageMsg msg, NoErrorCompletion completion) {
BatchSyncVolumeResourceSizeOnPrimaryStorageReply reply = new BatchSyncVolumeResourceSizeOnPrimaryStorageReply();

GetBatchVolumeSizeCmd cmd = new GetBatchVolumeSizeCmd();
cmd.volumeUuidInstallPaths = msg.getVolumeUuidInstallPaths();
Expand All @@ -3436,7 +3441,7 @@ public void success(GetBatchVolumeSizeRsp rsp) {
markVolumeActualSize(volumeUuid, actualSize);
}

reply.setActualSizes(rsp.actualSizes);
reply.setVolumeActualSizes(rsp.actualSizes);
bus.reply(msg, reply);
completion.done();
}
Expand Down
8 changes: 5 additions & 3 deletions plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.zstack.header.storage.addon.*;
import org.zstack.header.storage.addon.primary.*;
import org.zstack.header.storage.primary.ImageCacheInventory;
import org.zstack.header.storage.primary.StorageResourceStats;
import org.zstack.header.storage.primary.VolumeSnapshotCapability;
import org.zstack.header.storage.snapshot.VolumeSnapshotStats;
import org.zstack.header.volume.*;
Expand Down Expand Up @@ -93,6 +94,7 @@ public class ExponStorageController implements PrimaryStorageControllerSvc, Prim
scap.setSupport(true);
scap.setArrangementType(VolumeSnapshotCapability.VolumeSnapshotArrangementType.INDIVIDUAL);
scap.setPlacementType(VolumeSnapshotCapability.VolumeSnapshotPlacementType.INTERNAL);
scap.setMode(VolumeSnapshotCapability.VolumeSnapshotMode.REDIRECT_ON_WRITE);
scap.setSupportCreateOnHypervisor(false);
scap.setSupportLazyDelete(true);
scap.setVolumePathFromInternalSnapshotRegex("^[^@]+");
Expand Down Expand Up @@ -1158,15 +1160,15 @@ public void stats(String installPath, ReturnValueCompletion<VolumeStats> comp) {
}

@Override
public void batchStats(Collection<String> installPath, ReturnValueCompletion<List<VolumeStats>> comp) {
List<VolumeStats> stats = installPath.stream().map(it -> {
public void batchStats(Collection<String> installPaths, ReturnValueCompletion<List<StorageResourceStats>> comp) {
List<StorageResourceStats> stats = installPaths.stream().map(it -> {
VolumeModule vol = apiHelper.getVolume(getVolIdFromPath(it));
VolumeStats s = new VolumeStats();
s.setInstallPath(it);
s.setSize(vol.getVolumeSize());
s.setActualSize(vol.getDataSize());
s.setFormat(VolumeConstant.VOLUME_FORMAT_RAW);
return s;
return (StorageResourceStats) s;
}).collect(Collectors.toList());
comp.success(stats);
}
Expand Down
Loading