Add DHCP options and prefer prebuilt dnsmasq binary#11
Open
lateautumn233 wants to merge 3 commits into
Open
Conversation
Set DHCP router option based on the bridge IPv4 network containing the range start, advertise public DNS servers, and disable the DNS listener since dnsmasq is only used for DHCP here.
Resolve the dnsmasq executable to the prebuilt path when present and executable, falling back to PATH lookup otherwise.
There was a problem hiding this comment.
Pull request overview
Enhances the Dnsmasq networking backend to prefer the app's bundled prebuilt binary when present, build dnsmasq's argument list dynamically, advertise a sensible router address derived from the bridge's configured IPv4 addresses, and add DHCP-only DNS server hints while disabling dnsmasq's own resolver.
Changes:
- Add
resolveDnsmasqBinary()to prefer a prebuilt dnsmasq underDATA_DIR/usr/bin, falling back to the systemdnsmasq. - Refactor process startup to assemble args via
ArrayList, and add--dhcp-option=option:router,...,--dhcp-option=option:dns-server,8.8.8.8,1.1.1.1,--port=0, and--no-resolv. - Add
findRouterAddress()which matches the DHCP range start against the bridge's configured IPv4 CIDRs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+93
to
+103
| IPv4Network fallback = null; | ||
| for (var iter : inst.item.get("ipv4_addresses")) { | ||
| try { | ||
| var cidr = IPv4Network.parse(iter.getValue().asString()); | ||
| if (fallback == null) fallback = cidr; | ||
| if (dhcpStart != null && cidr.contains(dhcpStart)) | ||
| return cidr.address().toString(); | ||
| } catch (Exception ignored) { | ||
| } | ||
| } | ||
| return fallback != null ? fallback.address().toString() : null; |
| args.add(fmt("--dhcp-range=%s,%s,12h", rangeStart, rangeEnd)); | ||
| if (router != null) | ||
| args.add(fmt("--dhcp-option=option:router,%s", router)); | ||
| args.add("--dhcp-option=option:dns-server,8.8.8.8,1.1.1.1"); |
Add a DNS Servers section in NetworkEditActivity reusing the IPv4/IPv6 address row pattern. Defaults to 8.8.8.8 and 1.1.1.1, requires at least one entry. Dnsmasq now reads dns_servers from network config instead of the previously hardcoded list.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
app/src/main/java/cn/classfun/droidvm/ui/network/edit/NetworkEditActivity.java:607
- Same as in
onAddDns:IPv4Address.parseis@NonNulland never returns null (it throws on bad input, which the surrounding try/catch already handles). Theif (ip != null)guard is dead code and can be removed.
var ip = IPv4Address.parse(a.asString());
if (ip != null) out.add(ip);
| parseIPv6Addresses(config, ipv6Addresses); | ||
| dnsServers.clear(); | ||
| parseDnsServers(config, dnsServers); | ||
| if (dnsServers.isEmpty()) addDefaultDnsServers(); |
Comment on lines
+349
to
+350
| var ip = IPv4Address.parse(addr); | ||
| if (ip == null) { |
Comment on lines
366
to
371
| private void buildAddressList( | ||
| @NonNull LinearLayout container, | ||
| @NonNull TextView emptyView, | ||
| @NonNull List<? extends IPNetwork<?, ?, ?>> list, | ||
| boolean isIPv4 | ||
| @NonNull List<?> list, | ||
| @Nullable Runnable onRemoved | ||
| ) { |
| } | ||
| for (var existing : dnsServers) { | ||
| if (existing.value() == ip.value()) { | ||
| inputDns.setText(""); |
Comment on lines
+99
to
+109
| IPv4Network fallback = null; | ||
| for (var iter : inst.item.get("ipv4_addresses")) { | ||
| try { | ||
| var cidr = IPv4Network.parse(iter.getValue().asString()); | ||
| if (fallback == null) fallback = cidr; | ||
| if (dhcpStart != null && cidr.contains(dhcpStart)) | ||
| return cidr.address().toString(); | ||
| } catch (Exception ignored) { | ||
| } | ||
| } | ||
| return fallback != null ? fallback.address().toString() : null; |
Comment on lines
+74
to
+79
| args.add("--no-resolv"); | ||
| args.add("--no-daemon"); | ||
| args.add("--keep-in-foreground"); | ||
| args.add(fmt("--pid-file=%s", pidFile)); | ||
| args.add(fmt("--dhcp-leasefile=%s", leaseFile)); | ||
| args.add("--log-queries"); |
Comment on lines
+342
to
364
| private void onAddDns() { | ||
| var addr = inputDns.getText().trim(); | ||
| if (addr.isEmpty()) return; | ||
| if (!IPv4Address.isValid(addr)) { | ||
| inputDns.setError(getString(R.string.network_edit_error_dns_invalid)); | ||
| return; | ||
| } | ||
| var ip = IPv4Address.parse(addr); | ||
| if (ip == null) { | ||
| inputDns.setError(getString(R.string.network_edit_error_dns_invalid)); | ||
| return; | ||
| } | ||
| for (var existing : dnsServers) { | ||
| if (existing.value() == ip.value()) { | ||
| inputDns.setText(""); | ||
| return; | ||
| } | ||
| } | ||
| inputDns.setError(null); | ||
| dnsServers.add(ip); | ||
| inputDns.setText(""); | ||
| buildAddressList(layoutDns, tvDnsEmpty, dnsServers, null); | ||
| } |
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
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.
This pull request enhances the
Dnsmasqbackend in the networking daemon by improving how the dnsmasq binary is located and how its process is started. It introduces logic to dynamically resolve the dnsmasq binary path, adds more robust DHCP configuration options, and improves the selection of the router address for DHCP clients.Dnsmasq binary resolution and process startup improvements:
resolveDnsmasqBinary()method to locate a prebuilt dnsmasq binary if available, falling back to the system binary otherwise. This ensures the correct binary is used depending on the environment. [1] [2]ArrayList, allowing for more flexible and readable argument management.--dhcp-option=option:dns-server,8.8.8.8,1.1.1.1), disabled DNS port (--port=0), and disabled system resolver (--no-resolv) to improve network isolation and reliability.Router address determination:
findRouterAddress()method to select the appropriate router address for the DHCP option based on the DHCP range and available IPv4 addresses, improving network configuration accuracy for clients. [1] [2]Dependency and import updates:
IPv4Address,IPv4Network, and utility functions to support the new logic for binary resolution and network address parsing.