Skip to content

Lspci: only refresh the pciids database once per node - #4656

Open
mcgov (mcgov) wants to merge 3 commits into
mcgov/stack-2-nic-bind-uuidfrom
mcgov/stack-3-lspci-pciids
Open

Lspci: only refresh the pciids database once per node#4656
mcgov (mcgov) wants to merge 3 commits into
mcgov/stack-2-nic-bind-uuidfrom
mcgov/stack-3-lspci-pciids

Conversation

@mcgov

Copy link
Copy Markdown
Collaborator

Part 3 of 9 of a stacked series that reworks the DPDK SRIOV hot plug tests. Stacked on #4655, review only the last commit.

get_devices ran update-pciids on every forced refresh, which adds a network download and a noticeable delay to hot plug tests that rescan the pci bus repeatedly. The database is now refreshed once per tool instance.

Key Test Cases:
verify_dpdk_sriov_rescind_failover_send_only|verify_device_reports_correct_pci_slot

Impacted LISA Features:
Sriov, NetworkInterface, Nvme, Gpu

Tested Azure Marketplace Images:

  • canonical 0001-com-ubuntu-server-jammy 22_04-lts latest
  • microsoftcblmariner azure-linux-3 azure-linux-3 latest

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR optimizes Lspci.get_devices() by avoiding repeated update-pciids network downloads during forced PCI refreshes, which is especially beneficial for hot-plug test loops that rescan PCI state frequently.

Changes:

  • Add a per-tool-instance _pciids_initialized flag to ensure update-pciids runs at most once.
  • Move the update-pciids call behind the initialization guard while keeping device list refresh behavior unchanged.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lisa/tools/lspci.py Outdated
Copilot AI review requested due to automatic review settings August 17, 2026 23:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings August 18, 2026 06:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

lisa/tools/lspci.py:303

  • _pciids_initialized is only set to True when update-pciids returns 0. If update-pciids is slow and fails (e.g., transient network/DNS issues), then each subsequent force_run will re-run update-pciids, reintroducing the repeated delay this PR is trying to remove. Consider marking the refresh as "attempted" regardless of exit code (best-effort update) and log stderr for troubleshooting.
            if not self._pciids_initialized:
                # Ensure pci device ids and name mappings are updated.
                exit_code = self.node.execute("update-pciids", sudo=True, shell=True).exit_code
                if exit_code == 0:
                    self._pciids_initialized = True
                else:
                    self.node.log.debug(f"update-pciids failed, returned code {exit_code}")

Copilot AI review requested due to automatic review settings August 18, 2026 10:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

lisa/tools/lspci.py:307

  • _pciids_initialized is only set to True when update-pciids succeeds. If update-pciids is slow and fails (e.g., transient network issues), every force_run=True refresh will re-run it, reintroducing the repeated download/delay this PR is trying to remove. Consider marking it initialized after the first attempt (success or failure) and just logging failures.
            if not self._pciids_initialized:
                # Ensure pci device ids and name mappings are updated.
                exit_code = self.node.execute(
                    "update-pciids", sudo=True, shell=True
                ).exit_code
                if exit_code == 0:
                    self._pciids_initialized = True
                else:
                    self.node.log.debug(
                        f"update-pciids failed, returned code {exit_code}"
                    )

lisa/tools/lspci.py:296

  • The PR description lists verify_dpdk_sriov_rescind_failover_send_only and verify_device_reports_correct_pci_slot as executed test cases, but those method names don’t exist in this branch (no matches in lisa/microsoft/testsuites/). Please update the PR description to reference the actual test methods that were run so reviewers can reproduce validation.
        if (not self._pci_devices) or force_run:

mcgov (mcgov) and others added 3 commits August 19, 2026 11:47
get_devices ran update-pciids on every forced refresh, which adds a
network download and a noticeable delay to hot plug tests that rescan
the pci bus repeatedly. Track whether the database was already updated
for this tool instance and skip the refresh afterwards.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5d5f58ad-b9df-4420-ad37-22caee78e925
@mcgov
mcgov (mcgov) force-pushed the mcgov/stack-3-lspci-pciids branch from ad62757 to ed17764 Compare August 19, 2026 18:47
Copilot AI review requested due to automatic review settings August 19, 2026 18:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

lisa/tools/lspci.py:307

  • _pciids_initialized is only set to True when update-pciids succeeds. If the command fails once (e.g., no outbound network access), every subsequent force_run will retry update-pciids, reintroducing the repeated download/delay this PR is trying to eliminate. Consider marking the pciids refresh as "attempted" after the first run regardless of exit code, while still logging failures.
                if exit_code == 0:
                    self._pciids_initialized = True
                else:
                    self.node.log.debug(
                        f"update-pciids failed, returned code {exit_code}"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants