feat: Merge from release/1071 #185
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As title.
Remove exfat-fuse dependency. Log: Remove exfat-fuse dependency
Remove support for FAT32 format. Log: Fix fail to format disk to FAT32 Bug: https://pms.uniontech.com/bug-view-299499.html
cannot obtain model info from either smartctl nor lshw commands, try with udevadm command to obtain ID_MODEL field. Log: fix block model missing. Bug: https://pms.uniontech.com/bug-view-322579.html
Changes: 1. Reserved space handling - Now counts reserved blocks as used space for ext2/3/4 filesystems - Matches behavior of standard tools like df 2. Mounted device detection - Uses stat-based calculation for accurate free space on mounted devices - Falls back to previous method for unmounted devices Technical Impact: - Fixes discrepancy between displayed and actual available space - Provides more consistent behavior with system utilities Log: Improved ext* filesystem space calculation accuracy Bug: https://pms.uniontech.com/bug-view-323359.html
-- In the special platform, the stroage interface show error.
Reviewer's GuideRefactors ext* filesystem usage calculation to rely purely on dumpe2fs output and statvfs, adjusts device model/interface detection, introduces Qt5/Qt6‑aware build and translation setup plus an optional DBus auth bypass, and makes small logging/formatting and include tweaks. Sequence diagram for EXT2 setUsedSectors usage calculationsequenceDiagram
participant Caller
participant EXT2
participant Utils
participant Partition
Caller->>EXT2: setUsedSectors(partition)
EXT2->>Partition: getPath()
Partition-->>EXT2: path
EXT2->>Utils: executCmd("dumpe2fs -h path", output, error)
Utils-->>EXT2: success_or_failure
alt dumpe2fs_success
EXT2->>EXT2: parse Block count, Block size, Free blocks, Reserved block count
EXT2->>EXT2: validate blockCount, blockSize, freeBlocks, reservedBlocks
alt partition_mounted
EXT2->>Partition: getMountPoints()
Partition-->>EXT2: mountPoints
EXT2->>Partition: getMountPoint()
Partition-->>EXT2: mountpoint
EXT2->>Utils: getMountedFileSystemUsage(mountpoint, statTotalSize, statAvailableSize)
Utils-->>EXT2: rc
alt stat_success
EXT2->>EXT2: convert sizes to totalSectors, statAvailableSectors
EXT2->>Partition: setSectorUsage(totalSectors, statAvailableSectors)
else stat_failed
EXT2->>EXT2: compute availableSectors from dumpe2fs
EXT2->>Partition: setSectorUsage(totalSectors, availableSectors)
end
else partition_not_mounted
EXT2->>EXT2: compute userAvailableBlocks = max(freeBlocks - reservedBlocks, 0)
EXT2->>EXT2: compute totalSectors, availableSectors
EXT2->>Partition: setSectorUsage(totalSectors, availableSectors)
end
EXT2->>Partition: m_fsBlockSize = blockSize
else dumpe2fs_failed
EXT2->>EXT2: log error and skip sector update
end
EXT2-->>Caller: return
Sequence diagram for DiskManagerService authorization with optional bypasssequenceDiagram
participant Client
participant DBus
participant DiskManagerService
participant AuthAgent
Client->>DBus: call disk operation
DBus->>DiskManagerService: dispatch message
DiskManagerService->>DiskManagerService: checkAuthorization()
alt NO_DBUS_CALLER_AUTH_CHECK defined
DiskManagerService->>DiskManagerService: log "Authorization check was skipped"
DiskManagerService-->>DBus: authorized
else NO_DBUS_CALLER_AUTH_CHECK not defined
DiskManagerService->>DiskManagerService: read message().service()
DiskManagerService->>AuthAgent: check action com.deepin.pkexec.deepin-diskmanager
AuthAgent-->>DiskManagerService: allow_or_deny
alt allow
DiskManagerService-->>DBus: authorized
else deny
DiskManagerService->>DBus: sendErrorReply(AccessDenied)
end
end
DBus-->>Client: reply or error
Class diagram for updated filesystem and utility interactionsclassDiagram
class Partition {
+QString m_uuid
+bool m_busy
+qulonglong m_sectorSize
+qulonglong m_fsBlockSize
+QString getPath()
+QString getMountPoint()
+QList~QString~ getMountPoints()
+void setSectorUsage(Sector totalSectors, Sector freeSectors)
}
class EXT2 {
-long long m_blocksSize
-long long m_numOfFreeOrUsedBlocks
-long long m_totalNumOfBlock
+FS getFilesystemSupport()
+void setUsedSectors(Partition partition)
}
class Utils {
+bool executCmd(QString command, QString output, QString error)
+QString regexpLabel(QString text, QString pattern)
+int getMountedFileSystemUsage(QString mountpoint, Byte_Value fileSystemSize, Byte_Value fileSystemFree)
+QString readContent(QString path)
}
class DeviceStorage {
+void getDiskInfoModel(QString devicePath, QString model)
+void getDiskInfoInterface(QString devicePath, QString interface)
}
class DiskManagerService {
+int test()
+bool checkAuthorization()
}
class PartedCore {
+bool hidePartition()
+void onRefreshDeviceInfo(int type, bool arg1, QString arg2)
}
EXT2 --> Partition : updates_sector_usage
EXT2 --> Utils : uses_dumpe2fs_and_statvfs
Utils --> Partition : fills_usage_via_mountpoint
DeviceStorage --> Utils : reads_sysfs_and_udevadm
DiskManagerService ..> Utils : affected_by_NO_DBUS_CALLER_AUTH_CHECK
PartedCore --> Partition : manages_visibility
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In getDiskInfoInterface(), the sysfs path is hard-coded to
/sys/block/sdd/device/spec_version; consider deriving the block device name fromdevicePathso the UFS interface detection works for arbitrary devices instead of onlysdd. - The new udevadm model fallback in getDiskInfoModel() builds a shell command with an unquoted
devicePathand greps the output; usingudevadm info --query=property --name=<device>and parsing key/value lines directly (with proper quoting) would be more robust and safer. - Replacing
Qt::endlwith"\n"in logging changes flushing behavior; if immediate flush after each log line is still desired, consider callingts.flush()explicitly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In getDiskInfoInterface(), the sysfs path is hard-coded to `/sys/block/sdd/device/spec_version`; consider deriving the block device name from `devicePath` so the UFS interface detection works for arbitrary devices instead of only `sdd`.
- The new udevadm model fallback in getDiskInfoModel() builds a shell command with an unquoted `devicePath` and greps the output; using `udevadm info --query=property --name=<device>` and parsing key/value lines directly (with proper quoting) would be more robust and safer.
- Replacing `Qt::endl` with `"\n"` in logging changes flushing behavior; if immediate flush after each log line is still desired, consider calling `ts.flush()` explicitly.
## Individual Comments
### Comment 1
<location> `service/diskoperation/filesystems/ext2.cpp:186-187` </location>
<code_context>
+ if (blockCount > 0 && blockSize > 0 && freeBlocks >= 0 && reservedBlocks >= 0) {
+ qDebug() << "Filesystem information validation successful, calculating sector usage";
+
+ // 计算用户实际可用的空闲块数：可用 = (freeBlocks - reservedBlocks)
+ long long userAvailableBlocks = (freeBlocks > reservedBlocks) ? (freeBlocks - reservedBlocks) : 0;
+
+ // 转换为扇区数
</code_context>
<issue_to_address>
**issue (bug_risk):** Subtracting `reservedBlocks` from `freeBlocks` likely underestimates user-available space for ext*.
On ext2/3/4, `dumpe2fs` typically reports `Free blocks:` with reserved space already excluded, and also exposes `Reserved block count:` separately. Assuming `freeBlocks` includes reserved blocks and subtracting `reservedBlocks` again can double‑count the reservation and under‑report available space. Please confirm the semantics against real `dumpe2fs -h` output and adjust the computation accordingly (e.g., use `freeBlocks` as‑is and keep `reservedBlocks` only as metadata, or derive used space from `blockCount - freeBlocks` while tracking reserved separately).
</issue_to_address>
### Comment 2
<location> `basestruct/utils.cpp:512-515` </location>
<code_context>
if (ret == 0) {
- fileSystemSize = static_cast<Byte_Value>(sfs.f_blocks) * sfs.f_frsize;
- fileSystemFree = static_cast<Byte_Value>(sfs.f_bfree) * sfs.f_bsize;
+ fileSystemSize = static_cast<Byte_Value>(sfs.f_blocks) * sfs.f_bsize;
+ fileSystemFree = static_cast<Byte_Value>(sfs.f_bavail) * sfs.f_bsize;
} else {
qWarning() << "Utils::getMountedFileSystemUsage - Failed for:" << mountpoint << "Error:" << errno;
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Using `f_bsize` for filesystem size may be less correct than `f_frsize`.
`statvfs` typically defines total size as `f_blocks * f_frsize`, while `f_bsize` is only the optimal transfer block size and may differ. The previous use of `f_frsize` for total size followed that convention. I’d keep `fileSystemSize = f_blocks * f_frsize` and only switch free space to `f_bavail * f_bsize` to avoid incorrect total size on filesystems where these fields differ.
```suggestion
if (ret == 0) {
fileSystemSize = static_cast<Byte_Value>(sfs.f_blocks) * sfs.f_frsize;
fileSystemFree = static_cast<Byte_Value>(sfs.f_bavail) * sfs.f_bsize;
} else {
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+186
to
+187
| // 计算用户实际可用的空闲块数：可用 = (freeBlocks - reservedBlocks) | ||
| long long userAvailableBlocks = (freeBlocks > reservedBlocks) ? (freeBlocks - reservedBlocks) : 0; |
There was a problem hiding this comment.
issue (bug_risk): Subtracting reservedBlocks from freeBlocks likely underestimates user-available space for ext*.
On ext2/3/4, dumpe2fs typically reports Free blocks: with reserved space already excluded, and also exposes Reserved block count: separately. Assuming freeBlocks includes reserved blocks and subtracting reservedBlocks again can double‑count the reservation and under‑report available space. Please confirm the semantics against real dumpe2fs -h output and adjust the computation accordingly (e.g., use freeBlocks as‑is and keep reservedBlocks only as metadata, or derive used space from blockCount - freeBlocks while tracking reserved separately).
Resolving security issues and compilation errors.
deepin pr auto review我来对这段代码进行审查，从语法逻辑、代码质量、性能和安全几个方面进行分析：
建议改进：
总体来说，这些修改提高了代码的可靠性、可维护性和跨平台兼容性。特别是在文件系统空间计算方面的改进很有价值，修复了一些潜在的问题。 |
lzwind
approved these changes
Dec 19, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lzwind, wangrong1069 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
Author
|
/merge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Improve filesystem usage calculations, broaden platform compatibility, and refine logging and device metadata handling.
Bug Fixes:
Enhancements:
Build: