libvirt_vm: undefine stale domain before virt-install --import

[BulaYoungR]

VM.create() builds a "virt-install --import" command that runs with the default "--check path_in_use=on". If a persistent domain with the same name still owns the target image -- a common state when a previous test leaves the guest defined (env_cleanup=no) or a CI runner pre-installs the VM during bootstrap -- virt-install refuses with:

ERROR    Disk /path/to/image.qcow2 is already in use by other
         guests ['<vm_name>']. (Use --check path_in_use=off or
         --check all=off to override)

env_process preprocess then errors for every test that hits the re-install path, cascading across the whole suite. self.destroy() kills the qemu process for our name but does not drop the persistent libvirt definition, so the disk/name stays locked from virt-install's view.

Undefine the stale domain right after destroy(), gated on domain_exists. Detect nvram from the inactive domain XML and add --nvram when present so the undefine also succeeds on UEFI-only arches such as aarch64, where "virsh undefine" otherwise fails with "cannot delete inactive domain with nvram". Only the definition is removed; the qcow2 image is left intact and reused by --import.

Verified on aarch64 (RHEL-9.9, libvirt-11.10.0): the virtual_network.mtu and iface_options groups used to ERROR in env setup with the "Disk ... already in use" message on every test; with the patch they reach guest setup and pass (remaining failures are unrelated test image issues, e.g. "No dhcp client found on the system").

Author: Bolatbek Issakh bissakh@redhat.com

[gemini-code-assist]

Code Review

This pull request adds logic to undefine an existing persistent domain with the same name before creating a new VM, preventing "disk already in use" errors. The feedback identifies that the exception handling around virsh.dumpxml is ineffective because ignore_status defaults to True, which suppresses process.CmdError; passing ignore_status=False is recommended to fix this. Additionally, extracting this new logic into a private helper method is suggested to improve code readability and maintainability.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

The try...except process.CmdError block around this call is ineffective because virsh.dumpxml is called without ignore_status=False. The underlying virsh.command function defaults to ignore_status=True, which suppresses process.CmdError on failure, making the except block unreachable. To fix this, ignore_status=False should be passed to virsh.dumpxml.

                domxml = virsh.dumpxml(self.name, extra="--inactive", uri=self.connect_uri, ignore_status=False).stdout_text

[gemini-code-assist]

This block of code for undefining a stale domain is quite large and is placed directly within the create method. To improve readability and maintainability, consider extracting this logic into a new private helper method, such as _undefine_stale_domain(self).

The long comment explaining the logic can then be moved into the docstring of this new method, making the create method cleaner and the purpose of the extracted code more explicit.

For example:

    def _undefine_stale_domain(self):
        """
        Undefine a stale persistent domain to prevent virt-install failures.

        A persistent domain with our name -- left by a previous test
        (env_cleanup=no) or pre-installed during CI bootstrap -- makes the
        "virt-install --import" below fail under its default
        "--check path_in_use=on":
            Disk ... is already in use by other guests ['<name>']
        destroy() only kills the qemu process, it does not drop the
        persistent definition, so the disk/name stays locked. Undefine it
        here. Detect nvram from the domain XML so this also works on
        UEFI-only arches (e.g. aarch64), where "virsh undefine" fails
        without --nvram. Only the definition is removed; the qcow2 image is
        left intact for --import to reuse.
        """
        if not virsh.domain_exists(self.name, uri=self.connect_uri):
            return

        undefine_opts = "--managed-save"
        try:
            domxml = virsh.dumpxml(
                self.name, extra="--inactive", uri=self.connect_uri, ignore_status=False
            ).stdout_text
        except process.CmdError as detail:
            domxml = ""
            LOG.debug(
                "Could not dump XML of %s before undefine: %s",
                self.name,
                detail,
            )
        if "<nvram" in domxml:
            undefine_opts += " --nvram"
        virsh.undefine(
            self.name,
            options=undefine_opts,
            uri=self.connect_uri,
            ignore_status=True,
        )

    @error_context.context_aware
    def create(self, ...):
        # ...
        self.destroy(free_mac_addresses=False)
        self._undefine_stale_domain()
        # ...

[harihare]

May I request to fix the review comments from code assist. Sot the pre-commit checks are executed.

[harihare]

Please fix all auto review comments.