feat: Merge from release/1071#185
Conversation
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
|
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.
| // 计算用户实际可用的空闲块数:可用 = (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我来对这段代码进行审查,从语法逻辑、代码质量、性能和安全几个方面进行分析:
建议改进:
总体来说,这些修改提高了代码的可靠性、可维护性和跨平台兼容性。特别是在文件系统空间计算方面的改进很有价值,修复了一些潜在的问题。 |
|
[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 |
|
/merge |
Summary by Sourcery
Improve filesystem usage calculations, broaden platform compatibility, and refine logging and device metadata handling.
Bug Fixes:
Enhancements:
Build: