fix(localstorage): check local capacity by disk requirements - #4669
fix(localstorage): check local capacity by disk requirements#4669MatheMatrix wants to merge 1 commit into
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 23 minutes Limit details: You’ve used all 1 included review currently available under your plan. You completed 89 included PR reviews in the past 7 days; at that activity level, included reviews refill at 1 review per hour. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
Warning
|
4c0b75f to
8e13bb2
Compare
| } | ||
|
|
||
| // Compatibility entry for callers that cannot determine primary storage yet. | ||
| public void setDiskSize(long diskSize) { |
There was a problem hiding this comment.
Comment from jin.ma:
set 删了
| private Set<String> requiredPrimaryStorageUuids = new HashSet<>(); | ||
| // for each set in the list, the primary storage inside is optional | ||
| private final List<Set<String>> optionalPrimaryStorageUuids = new ArrayList<>(); | ||
| private final List<RequiredDiskCapacity> requiredDiskCapacities = new ArrayList<>(); |
There was a problem hiding this comment.
Comment from jin.ma:
为啥不是 map
There was a problem hiding this comment.
Comment from 赵祥亨:
这里用List 是因为主存储uuid可能没有指定，存在多条记录的主存储uuid都没指定的场景
所以这里就没有用Map了
| } | ||
|
|
||
| // check primary storage capacity and host physical capacity | ||
| boolean capacityChecked = PrimaryStorageCapacityChecker.New(psUuid, |
There was a problem hiding this comment.
Comment from jin.ma:
不就只需要改这一行吗，你改其他那么多是干什么
8e13bb2 to
d391039
Compare
…rage Root Cause: LocalStorageAllocatorFactory used HostAllocatorSpec.getDiskSize() to check every local storage host ref. The value was the total size of root and data disks, so disks assigned to non-local primary storage were also counted against local storage and could incorrectly filter out a valid host. Solution: Carry root and data disk capacity requirements as primary-storage UUID and size tuples. Local storage sums only requirements assigned to the current local primary storage. When all primary storages are undetermined, retain the original total-size check for compatibility. When only some requirements are undetermined, check the determined local requirements and leave the undetermined requirements to primary storage allocation. Volume migration retains the total-size check because all migrated volumes share one destination primary storage. Test: Update CreateVmHostAllocateCase to cover insufficient all-local capacity, mixed local/NFS placement, and partially undetermined primary storage. Verified with: Not run as requested. Resolves: ZSTAC-84163 Change-Id: I3f9895c605819a570a580cedfeaf1d80db300c80
| private Set<String> requiredPrimaryStorageUuids = new HashSet<>(); | ||
| // for each set in the list, the primary storage inside is optional | ||
| private final List<Set<String>> optionalPrimaryStorageUuids = new ArrayList<>(); | ||
| private final List<RequiredDiskCapacity> requiredDiskCapacities = new ArrayList<>(); |
There was a problem hiding this comment.
Comment from 赵祥亨:
已经调整为List
| } | ||
|
|
||
| // Compatibility entry for callers that cannot determine primary storage yet. | ||
| public void setDiskSize(long diskSize) { |
There was a problem hiding this comment.
Comment from 赵祥亨:
已经删除
d391039 to
2542cf6
Compare
| String psUuid = ref.getPrimaryStorageUuid(); | ||
| long requiredSize; | ||
| if (VmOperation.MigrateVolume.toString().equals(spec.getVmOperation())) { | ||
| // Volume migration has one destination primary storage, so diskSize is the capacity required on the target local storage. |
There was a problem hiding this comment.
Comment from jin.ma:
- 如果有指定的 local ，按指定的检查
- 如果还有未指定的，且 host 关联都是 local ，再查一次 总大小
3 participants
Jira: ZSTAC-84163
Reason: LocalStorageAllocatorFactory used HostAllocatorSpec.getDiskSize() to pre-check each local storage host ref during VM creation. getDiskSize() represented the total size of all VM disks, so when root and data volumes could be placed on different primary storages, the local storage precheck could compare the full VM disk size against a single local primary storage and incorrectly filter out candidate hosts.
Solution: Replace the independent diskSize state with RequiredDiskCapacity entries in AllocateHostMsg and HostAllocatorSpec. VM creation records root and data disk capacity requirements with the determined primary storage UUID when available, or null when primary storage is not determined yet. LocalStorageAllocatorFactory now checks only the capacity requirements that explicitly belong to the current local primary storage, while migration keeps using getDiskSize() because it has one destination primary storage.
Change-Id: I3f9895c605819a570a580cedfeaf1d80db300c80
sync from gitlab !10722