Merged
Conversation
- Remove conflicting filtering conditions Log: Fix incomplete retrieval of disk hardware information
- Adjust conditions to bypass key checks for NVMe devices.
- Adjust for new version lshw.
lshw output example:
*-disk:0
description: ATA Disk
product: VBOX HARDDISK
vendor: VirtualBox
physical id: 0
bus info: scsi@2:0.0.0
logical name: /dev/sda
version: 1.0
serial: VB899a842b-39025490
size: 80GiB (85GB)
capabilities: partitioned partitioned:dos
configuration: ansiversion=5 logicalsectorsize=512 sectorsize=512 signature=ff40caad
*-disk:1
description: ATA Disk
product: VBOX HARDDISK
vendor: VirtualBox
physical id: 1
bus info: scsi@3:0.0.0
logical name: /dev/sdb
version: 1.0
serial: VB6950fcbf-358a6d1e
size: 1GiB (1073MB)
capabilities: partitioned partitioned:dos
configuration: ansiversion=5 logicalsectorsize=512 sectorsize=512 signature=84232800
*-namespace:0
description: NVMe disk
physical id: 0
logical name: hwmon0
*-namespace:1
description: NVMe disk
physical id: 2
logical name: /dev/ng0n1
*-namespace:2
description: NVMe disk
physical id: 1
bus info: nvme@0:1
logical name: /dev/nvme0n1
size: 238GiB (256GB)
capabilities: gpt-1.00 partitioned partitioned:gpt
configuration: guid=36245768-2893-4b0c-9c9a-cfd42e90e7f2 logicalsectorsize=512 sectorsize=512 wwid=eui.6479a73c71a03470
Log: Improve disk info retrieval logic
As title. Log: Update version to 6.0.11
|
TAG Bot TAG: 6.0.11 |
lzwind
approved these changes
Jul 31, 2025
Reviewer's GuideThis PR adds a special NVMe bypass in lshw parsing, refactors getDiskInfoFromLshw for nested parsing and clearer variable usage, and streamlines the regex condition in getMapInfoFromSmartctl, alongside a changelog update. Class diagram for DeviceStorage changesclassDiagram
class DeviceStorage {
+bool addInfoFromlshw(const QMap<QString, QString> &mapInfo)
+bool getDiskInfoFromLshw(const QString &devicePath)
+void getMapInfoFromSmartctl(QMap<QString, QString> &mapInfo, const QString &output)
-QString m_KeyToLshw
}
DeviceStorage : addInfoFromlshw() now bypasses m_KeyToLshw check for NVMe
DeviceStorage : getDiskInfoFromLshw() refactored for nested *-disk/*-namespace parsing
DeviceStorage : getMapInfoFromSmartctl() regex condition streamlined
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
[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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @wangrong1069 - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `service/diskoperation/DeviceStorage.cpp:455` </location>
<code_context>
- outPut = list.at(i);
- break;
+ QString diskInfo;
+ QStringList list = outPut.split("*-disk");
+ foreach (const QString &item, list) {
+ QStringList list2 = item.split("*-namespace");
+ foreach (const QString &item2, list2) {
</code_context>
<issue_to_address>
Splitting on '*-disk' without the newline may cause parsing issues.
Switching from splitting on '*-disk\n' to '*-disk' may introduce empty or malformed entries and affect disk section parsing. Please verify that the output structure remains correct.
</issue_to_address>
### Comment 2
<location> `service/diskoperation/DeviceStorage.cpp:457` </location>
<code_context>
+ QString diskInfo;
+ QStringList list = outPut.split("*-disk");
+ foreach (const QString &item, list) {
+ QStringList list2 = item.split("*-namespace");
+ foreach (const QString &item2, list2) {
+ if (item2.contains(devicePath)) {
+ qDebug() << "devicePath found in lshw output";
</code_context>
<issue_to_address>
Nested splitting on '*-namespace' may not be robust for all lshw outputs.
This approach relies on a fixed lshw output structure, which may not be reliable. Please add validation or fallback handling for unexpected formats.
</issue_to_address>
### Comment 3
<location> `service/diskoperation/DeviceStorage.cpp:804` </location>
<code_context>
int index = line.indexOf(ch);
#if QT_VERSION_MAJOR > 5
QRegularExpressionMatch match = reg.match(line);
- if (index > 0 && !match.hasMatch() && match.captured(0) == line && false == line.contains("Error") && false == line.contains("hh:mm:SS")) {
+ if (index > 0 && !match.hasMatch() && false == line.contains("Error") && false == line.contains("hh:mm:SS")) {
#else
if (index > 0 && reg.exactMatch(line) == false && false == line.contains("Error") && false == line.contains("hh:mm:SS")) {
</code_context>
<issue_to_address>
Removed redundant match.captured(0) == line check, which may affect filtering.
Please verify that removing this condition won't result in partial matches being included in mapInfo unintentionally.
</issue_to_address>
### Comment 4
<location> `service/diskoperation/DeviceStorage.cpp:145` </location>
<code_context>
- if (key != m_KeyToLshw) {
- qDebug() << "lshw key does not match, returning false";
- return false;
+ if (keys[0].trimmed() == "nvme") {
+ qDebug() << "nvme device found, not check KeyToLshw";
+ } else {
</code_context>
<issue_to_address>
Consider inverting conditionals and using regular expressions to flatten control flow and reduce string manipulation in the updated functions.
Here are two small focused ways to collapse the added nesting/branching without losing any behavior:
1) Simplify the NVMe‐vs‐key check in addInfoFromlshw by inverting and early‐returning, so you eliminate the else/indent:
```cpp
bool DeviceStorage::addInfoFromlshw(const QMap<QString, QString> &mapInfo) {
qDebug() << "Adding lshw info for storage device";
auto keys = mapInfo["bus info"].split("@");
if (keys.size() != 2) {
qWarning() << "Invalid bus info format:" << mapInfo["bus info"];
return false;
}
const QString busType = keys[0].trimmed();
// Only non‐NVMe needs to match m_KeyToLshw
if (busType != "nvme") {
QString key = keys[1].trimmed().replace(".", ":");
if (key != m_KeyToLshw) {
qDebug() << "lshw key does not match, returning false"
<< busType << key << m_KeyToLshw;
return false;
}
}
// now set m_KeyFromStorage as before...
auto words = mapInfo["bus info"].split(":");
if (words.size() == 2) {
qDebug() << "Setting KeyFromStorage";
m_KeyFromStorage = words[0].replace("@", "");
} else {
qDebug() << "Cannot set KeyFromStorage, invalid format for bus info";
}
return true;
}
```
2) Collapse the double‐split loop in getDiskInfoFromLshw into a single regex pass:
```cpp
bool DeviceStorage::getDiskInfoFromLshw(const QString &devicePath) {
QString out, err;
if (Utils::executCmd("lshw -C disk", out, err) != 0) {
qDebug() << "Failed to execute lshw command, error:" << err;
return false;
}
// grab each "*-disk…*-namespace" block and pick the one containing devicePath
QRegularExpression re(QStringLiteral(R"(\*\-disk[\s\S]*?\*\-namespace)"));
auto it = re.globalMatch(out);
QString diskInfo;
while (it.hasNext()) {
const auto block = it.next().captured(0);
if (block.contains(devicePath)) {
qDebug() << "devicePath found in lshw output";
diskInfo = block;
break;
}
}
if (diskInfo.isEmpty()) {
qDebug() << "devicePath not found in lshw output, return false";
return false;
}
QMap<QString, QString> mapInfo;
getMapInfoFromLshw(diskInfo, mapInfo);
addInfoFromlshw(mapInfo);
return true;
}
```
These keep the new NVMe behavior and full data extraction, but flatten out the nesting and string gymnastics.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
Author
|
/merge |
This was referenced Jul 31, 2025
Open
Open
Open
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
Improve storage device info retrieval by adding NVMe support in lshw parsing, enhancing lshw output handling to consider namespaces, and simplifying smartctl line filtering
Enhancements: