fix: use NetworkManager D-Bus to enable/disable network interfaces#669
Conversation
There was a problem hiding this comment.
Sorry @GongHeng2017, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
9bc3cf2 to
3ef1ebb
Compare
max-lvs
left a comment
There was a problem hiding this comment.
-1, 注意代码逻辑及安全校验,参考Ai review comments
3ef1ebb to
b095a4a
Compare
Replace the previous iw/rfkill/ifconfig and ioctl approach with NetworkManager D-Bus API as the primary method for controlling network interface state. The D-Bus method sets the DeviceEnabled property via GetDeviceByIpIface. Fall back to the ioctl method when the D-Bus approach is unavailable or fails. - Add enableNetworkByDBus() as primary method (scheme 1) - Add enableNetworkByIoctl() as fallback method (scheme 2) - Remove wlan/wlp-specific iw/rfkill/ifconfig logic - Refactor ioctlOperateNetworkLogicalName() to try D-Bus first, then fall back to ioctl - Add QDBus headers and update SPDX copyright year Log: fix issue Bug: https://pms.uniontech.com/bug-view-362409.html
b095a4a to
7e642b6
Compare
deepin pr auto review这份代码变更将网卡启用/禁用的逻辑从原有的“无线网卡走 但在语法逻辑、代码质量、性能和安全方面,还有一些需要改进和注意的地方。以下是详细的审查意见: 一、 语法与逻辑问题
二、 代码质量与可读性
三、 代码性能
四、 代码安全
综合改进后的代码参考针对以上意见,以下是修改后的核心函数参考: bool EnableUtils::ioctlOperateNetworkLogicalName(const QString &logicalName, bool enable)
{
// 方案一:通过NetworkManager D-Bus设置DeviceEnabled属性
if (enableNetworkByDBus(logicalName, enable))
return true;
// 方案二:方案一失败,回退到ioctl方式
qWarning() << "Fallback to ioctl method for" << logicalName;
return enableNetworkByIoctl(logicalName, enable);
}
bool EnableUtils::enableNetworkByDBus(const QString &logicalName, bool enable)
{
QDBusInterface nmInterface("org.freedesktop.NetworkManager",
"/org/freedesktop/NetworkManager",
"org.freedesktop.NetworkManager",
QDBusConnection::systemBus());
if (!nmInterface.isValid()) {
qCritical() << "Failed to connect to NetworkManager:" << nmInterface.lastError().message();
return false;
}
QDBusReply<QDBusObjectPath> reply = nmInterface.call("GetDeviceByIpIface", logicalName);
if (!reply.isValid()) {
qCritical() << "Failed to GetDeviceByIpIface for" << logicalName << ":" << reply.error().message();
return false;
}
QString devicePath = reply.value().path();
if (devicePath.isEmpty()) {
qCritical() << "Got empty devicePath for interface:" << logicalName;
return false;
}
// 优化:直接通过 Device 接口设置属性,代码更简洁
QDBusInterface deviceInterface("org.freedesktop.NetworkManager",
devicePath,
"org.freedesktop.NetworkManager.Device",
QDBusConnection::systemBus());
if (!deviceInterface.isValid()) {
qCritical() << "Failed to connect to device interface:" << deviceInterface.lastError().message();
return false;
}
// 使用 setProperty 替代底层 Properties.Set 调用
if (!deviceInterface.setProperty("DeviceEnabled", QVariant::fromValue(enable))) {
qCritical() << "Failed to set DeviceEnabled for" << logicalName << ":" << deviceInterface.lastError().message();
return false;
}
return true;
}
bool EnableUtils::enableNetworkByIoctl(const QString &logicalName, bool enable)
{
if (logicalName.isEmpty() || logicalName.length() >= IFNAMSIZ) {
qCritical() << "Invalid logicalName length for ioctl";
return false;
}
int fd = socket(AF_INET, SOCK_STREAM, 0);
if (fd < 0)
return false;
struct ifreq ifr;
memset(&ifr, 0, sizeof(ifr));
std::string nameStr = logicalName.toStdString();
strncpy(ifr.ifr_name, nameStr.c_str(), IFNAMSIZ - 1);
ifr.ifr_name[IFNAMSIZ - 1] = '\0';
if (ioctl(fd, SIOCGIFFLAGS, &ifr) < 0) {
close(fd);
return false;
}
if (enable) {
ifr.ifr_flags |= IFF_UP;
} else {
ifr.ifr_flags &= ~IFF_UP;
}
if (ioctl(fd, SIOCSIFFLAGS, &ifr) < 0) {
close(fd);
return false;
}
// 修复:操作成功后必须关闭 fd
close(fd);
return true;
} |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: GongHeng2017, max-lvs 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 |
|
/forcemerge |
Replace the previous iw/rfkill/ifconfig and ioctl approach with NetworkManager D-Bus API as the primary method for controlling network interface state. The D-Bus method sets the DeviceEnabled property via GetDeviceByIpIface. Fall back to the ioctl method when the D-Bus approach is unavailable or fails.
Log: fix issue
Bug: https://pms.uniontech.com/bug-view-362409.html