Skip to content

Commit 8326618

Browse files
committed
fix(kvm): NPE in PlugNic when libvirt domain XML is unavailable
LibvirtPlugNicCommandWrapper.findNextAvailablePciSlot calls vm.getXMLDesc(0) and pipes the result straight into Pattern.matcher, which NPEs if libvirt returned null (or, in the LibvirtComputingResourceTest.testPlugNicCommandNoMatchMack unit test, when the Domain mock isn't stubbed for getXMLDesc). Reported by @DaanHoogland after the SL packaging run on #12826. Defensive null check returns null from findNextAvailablePciSlot when the domain XML can't be parsed, which falls through to libvirt's auto-assignment of the PCI slot — same behaviour as before this PR when nextSlot is null. Also stubs Domain.getXMLDesc(0) in testPlugNicCommandNoMatchMack with a minimal <domain> XML that exercises the parser path (rather than just relying on the null-fallback), so the test continues to cover the happy path of the new logic.
1 parent c572b98 commit 8326618

2 files changed

Lines changed: 17 additions & 0 deletions

File tree

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPlugNicCommandWrapper.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,14 @@ private Integer findNextAvailablePciSlot(final Domain vm, final List<InterfaceDe
122122
try {
123123
String domXml = vm.getXMLDesc(0);
124124

125+
// Defensive: getXMLDesc can return null on certain libvirt error paths (and is
126+
// null in unit tests where the Domain mock isn't stubbed for this call). Fall
127+
// back to letting libvirt auto-assign the PCI slot.
128+
if (domXml == null) {
129+
logger.debug("Domain XML unavailable, letting libvirt auto-assign PCI slot");
130+
return null;
131+
}
132+
125133
// Parse all PCI slot numbers currently in use
126134
Set<Integer> usedSlots = new HashSet<>();
127135
Pattern slotPattern = Pattern.compile("slot='0x([0-9a-fA-F]+)'");

plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3548,6 +3548,15 @@ public void testPlugNicCommandNoMatchMack() {
35483548
when(vifDriver.plug(nic, "Other PV", "", null)).thenReturn(interfaceDef);
35493549
when(interfaceDef.toString()).thenReturn("Interface");
35503550

3551+
// Stub vm.getXMLDesc(0) so findNextAvailablePciSlot can scan the domain XML
3552+
// for in-use PCI slots. Returning a minimal <domain> with a single NIC at
3553+
// slot 0x03 exercises the production parser without forcing the production
3554+
// code into its null-fallback path.
3555+
when(vm.getXMLDesc(0)).thenReturn(
3556+
"<domain><devices><interface type='bridge'>" +
3557+
"<address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>" +
3558+
"</interface></devices></domain>");
3559+
35513560
final String interfaceDefStr = interfaceDef.toString();
35523561
doNothing().when(vm).attachDevice(interfaceDefStr);
35533562

0 commit comments

Comments
 (0)