Getting address from domiffaddress and if adress is not found throws exception VMIPAddressMissingError in both ipv4 and ipv6 case #4251
Conversation
📝 WalkthroughWalkthroughAdds a VM._get_address override in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (1)virttest/libvirt_vm.py (2)
🪛 Ruff (0.14.0)virttest/libvirt_vm.py393-395: Within an (B904) 393-395: Avoid specifying long messages outside the exception class (TRY003) 397-399: Within an (B904) 397-399: Avoid specifying long messages outside the exception class (TRY003) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
TasmiyaNalatwad
force-pushed
the
fixing_regression_for_ipv6_address
branch
from
f06371d to
848dc9f
Compare
|
@smitterl @chloerh @luckyh I have tried fixing the regression due to these changes based on PR #4250 Please have a look. Now instead of returning None, i am raising exception VMIPAddressMissingError which allows the retry code to execute in both ipv4 and ipv6 cases. Please let me know your inputs for this fix. Thank you. |
TasmiyaNalatwad
force-pushed
the
fixing_regression_for_ipv6_address
branch
3 times, most recently
from
63c3920 to
d6279e5
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
virttest/libvirt_vm.py(2 hunks)virttest/utils_net.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
virttest/libvirt_vm.py (2)
virttest/virt_vm.py (4)
_get_address(814-872)_get_address(930-934)VMIPAddressMissingError(246-253)get_mac_address(767-782)virttest/utils_net.py (2)
get_mac_address(3413-3420)obtain_guest_ip_from_domifaddr(4899-4914)
virttest/utils_net.py (1)
virttest/virsh.py (1)
domifaddr(3533-3543)
🪛 Ruff (0.14.0)
virttest/libvirt_vm.py
393-395: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
393-395: Avoid specifying long messages outside the exception class
(TRY003)
397-399: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
397-399: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: avocado_devel AVOCADO_SRC:git+https://github.com/avocado-framework/avocado@92lts#egg=avocado_framework SETUP:-m pip install . VT_TYPE:qemu
| def _get_address(self, index=0, ip_version="ipv4", session=None, timeout=60.0): | ||
| try: | ||
| return super()._get_address(index, ip_version, session, timeout) | ||
| except virt_vm.VMIPAddressMissingError: | ||
| if ip_version == "ipv4": | ||
| mac = self.get_mac_address(index).lower() | ||
| ipaddr = utils_net.obtain_guest_ip_from_domifaddr(self.name, mac) | ||
| if ipaddr: | ||
| self.address_cache[mac] = ipaddr | ||
| return ipaddr | ||
| else: | ||
| raise virt_vm.VMIPAddressMissingError( | ||
| "IP address not found for MAC: %s", mac | ||
| ) | ||
| else: | ||
| raise virt_vm.VMIPAddressMissingError( | ||
| "No IP address found for IP version: %s", ip_version | ||
| ) | ||
|
|
There was a problem hiding this comment.
Construct VMIPAddressMissingError correctly and chain original exception
- VMIPAddressMissingError expects (mac, ip_version). Current code passes a formatted message and mis-assigns fields, breaking error semantics and retries.
- Also, chain the original exception per best practices (B904), and ensure MAC is available in both branches.
Apply this diff:
- def _get_address(self, index=0, ip_version="ipv4", session=None, timeout=60.0):
- try:
- return super()._get_address(index, ip_version, session, timeout)
- except virt_vm.VMIPAddressMissingError:
- if ip_version == "ipv4":
- mac = self.get_mac_address(index).lower()
- ipaddr = utils_net.obtain_guest_ip_from_domifaddr(self.name, mac)
- if ipaddr:
- self.address_cache[mac] = ipaddr
- return ipaddr
- else:
- raise virt_vm.VMIPAddressMissingError(
- "IP address not found for MAC: %s", mac
- )
- else:
- raise virt_vm.VMIPAddressMissingError(
- "No IP address found for IP version: %s", ip_version
- )
+ def _get_address(self, index=0, ip_version="ipv4", session=None, timeout=60.0):
+ try:
+ return super()._get_address(index, ip_version, session, timeout)
+ except virt_vm.VMIPAddressMissingError as err:
+ mac = self.get_mac_address(index).lower()
+ if ip_version == "ipv4":
+ ipaddr = utils_net.obtain_guest_ip_from_domifaddr(self.name, mac)
+ if ipaddr:
+ self.address_cache[mac] = ipaddr
+ return ipaddr
+ raise virt_vm.VMIPAddressMissingError(mac, ip_version) from err
+ raise virt_vm.VMIPAddressMissingError(mac, ip_version) from errThis corrects the constructor usage, ensures consistent error messaging, and preserves context. Based on static analysis hints.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _get_address(self, index=0, ip_version="ipv4", session=None, timeout=60.0): | |
| try: | |
| return super()._get_address(index, ip_version, session, timeout) | |
| except virt_vm.VMIPAddressMissingError: | |
| if ip_version == "ipv4": | |
| mac = self.get_mac_address(index).lower() | |
| ipaddr = utils_net.obtain_guest_ip_from_domifaddr(self.name, mac) | |
| if ipaddr: | |
| self.address_cache[mac] = ipaddr | |
| return ipaddr | |
| else: | |
| raise virt_vm.VMIPAddressMissingError( | |
| "IP address not found for MAC: %s", mac | |
| ) | |
| else: | |
| raise virt_vm.VMIPAddressMissingError( | |
| "No IP address found for IP version: %s", ip_version | |
| ) | |
| def _get_address(self, index=0, ip_version="ipv4", session=None, timeout=60.0): | |
| try: | |
| return super()._get_address(index, ip_version, session, timeout) | |
| except virt_vm.VMIPAddressMissingError as err: | |
| mac = self.get_mac_address(index).lower() | |
| if ip_version == "ipv4": | |
| ipaddr = utils_net.obtain_guest_ip_from_domifaddr(self.name, mac) | |
| if ipaddr: | |
| self.address_cache[mac] = ipaddr | |
| return ipaddr | |
| raise virt_vm.VMIPAddressMissingError(mac, ip_version) from err | |
| raise virt_vm.VMIPAddressMissingError(mac, ip_version) from err |
🧰 Tools
🪛 Ruff (0.14.0)
393-395: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
393-395: Avoid specifying long messages outside the exception class
(TRY003)
397-399: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
397-399: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In virttest/libvirt_vm.py around lines 382 to 400, fix the
VMIPAddressMissingError construction and exception chaining: capture the MAC
early (e.g., mac = self.get_mac_address(index).lower()) so it’s available in
both branches, and when re-raising use the VMIPAddressMissingError constructor
with the correct parameters (mac, ip_version) instead of a formatted message,
and chain the original exception (raise virt_vm.VMIPAddressMissingError(mac,
ip_version) from e) to preserve context.
| def obtain_guest_ip_from_domifaddr(vm_name, mac): | ||
| """ | ||
| Obtaining the guest ip address from virsh domifaddr command | ||
| :param: Mac address of the guest | ||
| :return: return ip-address if found for given mac in the | ||
| virsh domifaddr --full --source arp, else return None | ||
| """ | ||
| output = virsh.domifaddr(vm_name, "--full --source arp") | ||
| lines = output.stdout.splitlines() | ||
| for line in lines: | ||
| if mac in line: | ||
| parts = line.split() | ||
| for part in parts: | ||
| if "/" in part: | ||
| return part.split("/")[0] | ||
| return None |
There was a problem hiding this comment.
Fix domifaddr parsing: use stdout_text, case-insensitive MAC match, and prefer IPv4
- Use stdout_text (consistent with virsh CmdResult).
- Match MAC case-insensitively to avoid misses.
- Prefer IPv4 address when both IPv4/IPv6 are present.
Apply this diff:
-def obtain_guest_ip_from_domifaddr(vm_name, mac):
- """
- Obtaining the guest ip address from virsh domifaddr command
- :param: Mac address of the guest
- :return: return ip-address if found for given mac in the
- virsh domifaddr --full --source arp, else return None
- """
- output = virsh.domifaddr(vm_name, "--full --source arp")
- lines = output.stdout.splitlines()
- for line in lines:
- if mac in line:
- parts = line.split()
- for part in parts:
- if "/" in part:
- return part.split("/")[0]
- return None
+def obtain_guest_ip_from_domifaddr(vm_name, mac):
+ """
+ Obtain the guest IPv4 address from 'virsh domifaddr'.
+ :param vm_name: Domain name
+ :param mac: Guest MAC address
+ :return: IPv4 address string if found; otherwise None
+ """
+ result = virsh.domifaddr(vm_name, "--full --source arp")
+ lines = result.stdout_text.splitlines()
+ mac_l = mac.lower()
+ for line in lines:
+ if mac_l in line.lower():
+ parts = line.split()
+ for part in parts:
+ if "/" in part:
+ addr = part.split("/", 1)[0]
+ # Prefer IPv4 for callers expecting IPv4
+ if "." in addr:
+ return addr
+ return None📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def obtain_guest_ip_from_domifaddr(vm_name, mac): | |
| """ | |
| Obtaining the guest ip address from virsh domifaddr command | |
| :param: Mac address of the guest | |
| :return: return ip-address if found for given mac in the | |
| virsh domifaddr --full --source arp, else return None | |
| """ | |
| output = virsh.domifaddr(vm_name, "--full --source arp") | |
| lines = output.stdout.splitlines() | |
| for line in lines: | |
| if mac in line: | |
| parts = line.split() | |
| for part in parts: | |
| if "/" in part: | |
| return part.split("/")[0] | |
| return None | |
| def obtain_guest_ip_from_domifaddr(vm_name, mac): | |
| """ | |
| Obtain the guest IPv4 address from 'virsh domifaddr'. | |
| :param vm_name: Domain name | |
| :param mac: Guest MAC address | |
| :return: IPv4 address string if found; otherwise None | |
| """ | |
| result = virsh.domifaddr(vm_name, "--full --source arp") | |
| lines = result.stdout_text.splitlines() | |
| mac_l = mac.lower() | |
| for line in lines: | |
| if mac_l in line.lower(): | |
| parts = line.split() | |
| for part in parts: | |
| if "/" in part: | |
| addr = part.split("/", 1)[0] | |
| # Prefer IPv4 for callers expecting IPv4 | |
| if "." in addr: | |
| return addr | |
| return None |
🤖 Prompt for AI Agents
In virttest/utils_net.py around lines 4899 to 4914, the domifaddr parsing
currently reads output.stdout and does a case-sensitive MAC search and returns
the first CIDR part it finds; change it to read output.stdout_text, perform a
case-insensitive match for the MAC (lowercase both strings), and when a matching
line contains multiple address entries (IPv4 and IPv6), prefer and return the
IPv4 address (i.e., pick the CIDR part where the IP contains dots) falling back
to the first available address if no IPv4 is present; keep returning None if no
match is found.
Few of the times it is seen that the ip address is not being fetch and the avocado runs were failed with error "ERROR: Failures occurred while postprocess:\n\n: Guest virt-tests-vm1 dmesg verification failed: Login timeout expired (output: 'exceeded 240 s timeout, last failure: No ipv4 DHCP lease for MAC aa:bb:cc:dd:ee:ff') " To handle this error the patch has been sent. The patch helps in obtaining ip address of the guest using "virsh-net-dhcp-leases default" command. If the guest mac address is found in the command output, the mac ipv4 address is obatined and updated in the address.cache If the ip_version is ipv6, then it raises exception VMIPAddressMissingError, and going to retry logic to try getting the ip address. Even for ipv4 , if address is not found then it raises exception VMIPAddressMissingError and then retries will happen. Signed-off-by: Tasmiya Nalatwad <tasmiya@linux.vnet.ibm.com>
TasmiyaNalatwad
force-pushed
the
fixing_regression_for_ipv6_address
branch
from
d6279e5 to
57170c4
Compare
|
@smitterl Would you please run your test with these changes and check if the regression issue still occurs? This will help confirm whether the fix resolves the problem effectively. Thank You in Advance. |
2 participants
Few of the times it is seen that the ip address is not being fetch and the avocado runs were failed with error "ERROR: Failures occurred while postprocess:\n\n: Guest virt-tests-vm1 dmesg verification failed: Login timeout expired (output: 'exceeded 240 s timeout, last failure: No ipv4 DHCP lease for MAC aa:bb:cc:dd:ee:ff') "
To handle this error the patch has been sent. The patch helps in obtaining ip address of the guest using "virsh-net-dhcp-leases default" command. If the guest mac address is found in the command output, the mac ipv4 address is obatined and updated in the address.cache
If the ip_version is ipv6, then it raises exception VMIPAddressMissingError, and going to retry logic to try getting the ip address.
Even in case of ipv4, if the address is not found, instead of sending None, i am raising exception VMIPAddressMissingError which leads to retry menthod.
Summary by CodeRabbit
New Features
Bug Fixes