修复无法使用未分区U盘制作启动盘的问题#98
Merged
deepin-bot[bot] merged 2 commits intolinuxdeepin:release/eaglefrom Nov 7, 2025
Merged
Conversation
|
TAG Bot TAG: 5.7.12 |
Reviewer's GuideThis PR implements support for creating bootable USB drives from unpartitioned USB disks by enhancing device enumeration and parsing routines to detect whole-disk devices, automatically create a partition when needed, and then format it. It also increases disk size precision and bumps the version to 5.7.12. Sequence diagram for creating a bootable USB from an unpartitioned disksequenceDiagram
participant Installer as QtBaseInstaller
participant DiskUtil
participant Device as DeviceInfo
Installer->Device: Enumerate USB devices
Device->Installer: Return device list (may include unpartitioned disks)
Installer->DiskUtil: Check if device is disk (no partition)
alt Device is disk (no partition)
Installer->DiskUtil: CreatePartition(targetDev)
DiskUtil-->>Installer: Partition created
Installer->DiskUtil: FormatPartion(targetDev + "1")
DiskUtil-->>Installer: Partition formatted
else Device is partition
Installer->DiskUtil: FormatPartion(targetDev)
DiskUtil-->>Installer: Partition formatted
end
Updated class diagram for DeviceInfo with increased precisionclassDiagram
class DeviceInfo {
QString path
quint64 used
quint64 total
QString label
QString uuid
QString fstype
// ... other members
DeviceInfo()
DeviceInfo(const QString &path, quint64 used, quint64 total, const QString &label)
}
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:
- The manual digit‐based check to distinguish disks from partitions can misclassify eMMC devices like
/dev/mmcblk0; consider using a regex (e.g. matchingp\d+$) or existing Qt device utilities to reliably detect partition names. - There are a large number of qDebug() calls added—consider abstracting these behind a logging level or macro to avoid verbose output in production builds.
- Instead of the deprecated QRegExp, switch to QRegularExpression for better performance, safety, and clearer syntax when parsing lsblk output.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The manual digit‐based check to distinguish disks from partitions can misclassify eMMC devices like `/dev/mmcblk0`; consider using a regex (e.g. matching `p\d+$`) or existing Qt device utilities to reliably detect partition names.
- There are a large number of qDebug() calls added—consider abstracting these behind a logging level or macro to avoid verbose output in production builds.
- Instead of the deprecated QRegExp, switch to QRegularExpression for better performance, safety, and clearer syntax when parsing lsblk output.
## Individual Comments
### Comment 1
<location> `src/libdbm/util/utils.cpp:327` </location>
<code_context>
+ deviceCount++;
info.path = reg.cap(1);
info.label = reg.cap(2);
+ info.total = reg.cap(3).toLongLong() * 1.0 / 1024 / 1024; // MB
info.uuid = reg.cap(4);
info.fstype = reg.cap(5);
</code_context>
<issue_to_address>
**suggestion:** Potential loss of precision when converting device size to MB.
Use integer division to avoid unnecessary floating-point operations and ensure type consistency with the field.
```suggestion
info.total = reg.cap(3).toLongLong() / 1024 / 1024; // MB
```
</issue_to_address>
### Comment 2
<location> `src/libdbm/util/utils.cpp:462-464` </location>
<code_context>
+ QString filePath = usbfileinfoL.at(i).canonicalFilePath();
+
+ // 过滤USB和MMC设备，但排除网络相关设备
+ bool isUsbOrMmc = fileName.contains(QRegExp("^usb-\\S{1,}$")) || fileName.contains(QRegExp("^mmc-\\S{1,}$"));
+ bool isNetwork = fileName.contains(QRegExp("nfs-|cifs-|smbfs-|sshfs-|ftpfs-|davfs-"));
+
+ if (isUsbOrMmc && !isNetwork) {
</code_context>
<issue_to_address>
**suggestion:** Regular expressions may not be optimal for device name filtering.
Prefer QString::startsWith over QRegExp for prefix checks to improve readability and performance.
```suggestion
// 过滤USB和MMC设备，但排除网络相关设备
bool isUsbOrMmc = fileName.startsWith("usb-") || fileName.startsWith("mmc-");
bool isNetwork = fileName.contains(QRegExp("nfs-|cifs-|smbfs-|sshfs-|ftpfs-|davfs-"));
```
</issue_to_address>
### Comment 3
<location> `src/libdbm/util/utils.cpp:573-581` </location>
<code_context>
outfile.close();
outfile.remove();
#endif
+ qDebug() << "Get device list completed: Found" << deviceList.size() << "available devices";
+ for (int i = 0; i < deviceList.size(); ++i) {
+ qDebug() << " Device" << (i+1) << ":" << deviceList.at(i).path
+ << "Label:" << deviceList.at(i).label
</code_context>
<issue_to_address>
**suggestion (performance):** Extensive debug logging may impact performance and readability.
Consider making debug logging configurable or reducing verbosity in production to minimize performance impact and log clutter.
```suggestion
qDebug() << "Get device list completed: Found" << deviceList.size() << "available devices";
static const bool verboseDebugLogging = false; // Set to true for detailed debug output
if (verboseDebugLogging) {
for (int i = 0; i < deviceList.size(); ++i) {
qDebug() << " Device" << (i+1) << ":" << deviceList.at(i).path
<< "Label:" << deviceList.at(i).label
<< "Filesystem:" << deviceList.at(i).fstype
<< "Size:" << deviceList.at(i).total << "MB"
<< "Used:" << deviceList.at(i).used << "MB"
<< "Need format:" << deviceList.at(i).needFormat;
}
}
```
</issue_to_address>
### Comment 4
<location> `src/libdbm/installer/qtbaseinstaller.cpp:380-388` </location>
<code_context>
+ // disk 节点如：/dev/sdb
+ // partition 节点如：/dev/sdb1
+ bool isDisk = true;
+ for (int i = targetDev.length() - 1; i >= 0; i--) {
+ if (targetDev[i].isDigit()) {
+ isDisk = false;
+ break;
+ }
+ if (targetDev[i] == '/') {
+ break;
+ }
</code_context>
<issue_to_address>
**suggestion:** Device type detection logic may not handle all device naming conventions.
Some device names, like /dev/nvme0n1, include digits but are still disks. Using device metadata instead of string patterns would improve reliability.
```suggestion
// Improved device type detection using device metadata
auto isDiskDevice = [](const QString& devPath) -> bool {
QFileInfo fi(devPath);
QString devName = fi.fileName(); // e.g. "sdb", "sdb1", "nvme0n1"
QString sysBlockPath = "/sys/class/block/" + devName;
QDir sysBlockDir(sysBlockPath);
// If 'device' symlink exists, it's a disk; if not, it's likely a partition
QFileInfo deviceSymlink(sysBlockPath + "/device");
return deviceSymlink.exists();
};
bool isDisk = isDiskDevice(targetDev);
```
</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
467
to
469
| removeDevice.insert(filePath, fileName); | ||
| } | ||
| } |
There was a problem hiding this comment.
suggestion: Regular expressions may not be optimal for device name filtering.
Prefer QString::startsWith over QRegExp for prefix checks to improve readability and performance.
Suggested change
| // 过滤USB和MMC设备，但排除网络相关设备 | |
| bool isUsbOrMmc = fileName.contains(QRegExp("^usb-\\S{1,}$")) || fileName.contains(QRegExp("^mmc-\\S{1,}$")); | |
| bool isNetwork = fileName.contains(QRegExp("nfs-|cifs-|smbfs-|sshfs-|ftpfs-|davfs-")); | |
| // 过滤USB和MMC设备，但排除网络相关设备 | |
| bool isUsbOrMmc = fileName.startsWith("usb-") || fileName.startsWith("mmc-"); | |
| bool isNetwork = fileName.contains(QRegExp("nfs-|cifs-|smbfs-|sshfs-|ftpfs-|davfs-")); |
src/libdbm/util/utils.cpp
Outdated
There was a problem hiding this comment.
suggestion (performance): Extensive debug logging may impact performance and readability.
Consider making debug logging configurable or reducing verbosity in production to minimize performance impact and log clutter.
Suggested change
| qDebug() << "Get device list completed: Found" << deviceList.size() << "available devices"; | |
| for (int i = 0; i < deviceList.size(); ++i) { | |
| qDebug() << " Device" << (i+1) << ":" << deviceList.at(i).path | |
| << "Label:" << deviceList.at(i).label | |
| << "Filesystem:" << deviceList.at(i).fstype | |
| << "Size:" << deviceList.at(i).total << "MB" | |
| << "Used:" << deviceList.at(i).used << "MB" | |
| << "Need format:" << deviceList.at(i).needFormat; | |
| } | |
| qDebug() << "Get device list completed: Found" << deviceList.size() << "available devices"; | |
| static const bool verboseDebugLogging = false; // Set to true for detailed debug output | |
| if (verboseDebugLogging) { | |
| for (int i = 0; i < deviceList.size(); ++i) { | |
| qDebug() << " Device" << (i+1) << ":" << deviceList.at(i).path | |
| << "Label:" << deviceList.at(i).label | |
| << "Filesystem:" << deviceList.at(i).fstype | |
| << "Size:" << deviceList.at(i).total << "MB" | |
| << "Used:" << deviceList.at(i).used << "MB" | |
| << "Need format:" << deviceList.at(i).needFormat; | |
| } | |
| } |
Comment on lines
+380
to
+388
| bool isDisk = true; | ||
| for (int i = targetDev.length() - 1; i >= 0; i--) { | ||
| if (targetDev[i].isDigit()) { | ||
| isDisk = false; | ||
| break; | ||
| } | ||
| if (targetDev[i] == '/') { | ||
| break; | ||
| } |
There was a problem hiding this comment.
suggestion: Device type detection logic may not handle all device naming conventions.
Some device names, like /dev/nvme0n1, include digits but are still disks. Using device metadata instead of string patterns would improve reliability.
Suggested change
| bool isDisk = true; | |
| for (int i = targetDev.length() - 1; i >= 0; i--) { | |
| if (targetDev[i].isDigit()) { | |
| isDisk = false; | |
| break; | |
| } | |
| if (targetDev[i] == '/') { | |
| break; | |
| } | |
| // Improved device type detection using device metadata | |
| auto isDiskDevice = [](const QString& devPath) -> bool { | |
| QFileInfo fi(devPath); | |
| QString devName = fi.fileName(); // e.g. "sdb", "sdb1", "nvme0n1" | |
| QString sysBlockPath = "/sys/class/block/" + devName; | |
| QDir sysBlockDir(sysBlockPath); | |
| // If 'device' symlink exists, it's a disk; if not, it's likely a partition | |
| QFileInfo deviceSymlink(sysBlockPath + "/device"); | |
| return deviceSymlink.exists(); | |
| }; | |
| bool isDisk = isDiskDevice(targetDev); |
- Add automatic partition creation for unpartitioned USB disks using fdisk - Optimize df command to exclude network filesystems (NFS, CIFS, SMBFS, SSHFS, FTPFS, DAVFS) - Improve device scanning logic to skip network-related devices Log: Enable creating bootable USB drives from unpartitioned USB disks and improve disk querying efficiency Bug: https://pms.uniontech.com/bug-view-338629.html
- Bump version number in debian/changelog - Add changelog entry for unpartitioned USB support feature Log: Version update to 5.7.12
lzwind
approved these changes
Nov 7, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: itsXuSt, lzwind 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 |
deepin pr auto review我来对这个代码变更进行详细审查：
a) qtbaseinstaller.cpp中的磁盘检测逻辑： bool isDisk = true;
for (int i = targetDev.length() - 1; i >= 0; i--) {
if (targetDev[i].isDigit()) {
isDisk = false;
break;
}
if (targetDev[i] == '/') {
break;
}
}建议改为使用正则表达式或QFileInfo来判断： QFileInfo devInfo(targetDev);
bool isDisk = !targetDev.endsWith(QRegularExpression("\\d+$"));b) CreatePartition函数中的分区创建： QString cmd = QString("echo -e 'o\\nn\\np\\n1\\n\\n\\nw' | fdisk ") + diskDev;建议使用更安全的QProcess方式： QProcess fdisk;
fdisk.start("fdisk", QStringList() << diskDev);
fdisk.write("o\nn\np\n1\n\n\nw\n");
fdisk.closeWriteChannel();
fdisk.waitForFinished();c) 错误处理优化： if (!QFileInfo::exists(diskDev)) {
qCritical() << "Disk device does not exist:" << diskDev;
return false;
}
a) 设备扫描过程：
b) 分区检查：
a) 命令执行：
b) 权限检查：
a) 日志输出：
b) 代码结构：
c) 用户反馈：
这些改进建议主要关注代码的健壮性、安全性和用户体验。总体来说，这个变更增加了有用的功能，但在实现细节上还有优化空间。 |
Contributor
Author
|
/forcemerge |
Contributor
|
This pr force merged! (status: unstable) |
deepin-bot
bot
merged commit fad5a43
into
linuxdeepin:release/eagle
15 of 18 checks passed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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
Enable bootable USB creation from unpartitioned USB drives by treating whole-disk devices as removable media, auto-partitioning them when needed, and enhancing disk enumeration and parsing logic.
New Features:
Bug Fixes:
Enhancements:
Build: