Skip to content

Commit 8d09790

Browse files
vishesh92vishesh92
committed
Address comments
1 parent 6fa2a88 commit 8d09790

3 files changed

Lines changed: 67 additions & 32 deletions

File tree

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtGpuDef.java‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -89,27 +89,27 @@ private void generatePciXml(StringBuilder gpuBuilder) {
8989

9090
if (busAddress != null && !busAddress.isEmpty()) {
9191
String[] parts = busAddress.split(":");
92-
String slotFunction = null;
9392
try {
93+
String slotFunction;
9494
if (parts.length == 3) {
9595
domainVal = Integer.parseInt(parts[0], 16);
9696
busVal = Integer.parseInt(parts[1], 16);
9797
slotFunction = parts[2];
9898
} else if (parts.length == 2) {
9999
busVal = Integer.parseInt(parts[0], 16);
100100
slotFunction = parts[1];
101+
} else {
102+
throw new IllegalArgumentException("Invalid PCI bus address format: '" + busAddress + "'");
101103
}
102-
if (slotFunction != null) {
103-
String[] slotFunctionParts = slotFunction.split("\\.");
104-
if (slotFunctionParts.length > 0) {
105-
slotVal = Integer.parseInt(slotFunctionParts[0], 16);
106-
if (slotFunctionParts.length > 1) {
107-
funcVal = Integer.parseInt(slotFunctionParts[1].trim(), 16);
108-
}
109-
}
104+
String[] sf = slotFunction.split("\\.");
105+
if (sf.length == 2) {
106+
slotVal = Integer.parseInt(sf[0], 16);
107+
funcVal = Integer.parseInt(sf[1].trim(), 16);
108+
} else {
109+
throw new IllegalArgumentException("Invalid PCI bus address format: '" + busAddress + "'");
110110
}
111111
} catch (NumberFormatException e) {
112-
// leave all values at 0 (safe defaults)
112+
throw new IllegalArgumentException("Invalid PCI bus address format: '" + busAddress + "'", e);
113113
}
114114
}
115115

@@ -119,7 +119,7 @@ private void generatePciXml(StringBuilder gpuBuilder) {
119119
String function = String.format("0x%x", funcVal);
120120

121121
gpuBuilder.append(" <address domain='").append(domain).append("' bus='").append(bus).append("' slot='")
122-
.append(slot).append("' function='").append(function.trim()).append("'/>\n");
122+
.append(slot).append("' function='").append(function).append("'/>\n");
123123
gpuBuilder.append(" </source>\n");
124124
gpuBuilder.append("</hostdev>\n");
125125
}

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,39 @@ public void testGpuDef_withLegacyShortBdfDefaultsDomainToZero() {
221221
assertTrue(gpuXml.contains("<address domain='0x0000' bus='0xaf' slot='0x00' function='0x0'/>"));
222222
}
223223

224+
@Test
225+
public void testGpuDef_withInvalidBusAddressThrows() {
226+
String[] invalidAddresses = {
227+
"notahex:00.0", // non-hex bus
228+
"gg:00:02.0", // non-hex domain
229+
"00:02:03:04", // too many colon-separated parts
230+
"00", // missing slot/function
231+
"00:02", // missing function (no dot)
232+
"00:02.0.1", // extra dot in ss.f
233+
};
234+
for (String addr : invalidAddresses) {
235+
LibvirtGpuDef gpuDef = new LibvirtGpuDef();
236+
VgpuTypesInfo info = new VgpuTypesInfo(
237+
GpuDevice.DeviceType.PCI,
238+
"passthrough",
239+
"passthrough",
240+
addr,
241+
"10de",
242+
"NVIDIA Corporation",
243+
"1b38",
244+
"Tesla T4"
245+
);
246+
gpuDef.defGpu(info);
247+
try {
248+
String ignored = gpuDef.toString();
249+
fail("Expected IllegalArgumentException for address: " + addr + " but got: " + ignored);
250+
} catch (IllegalArgumentException e) {
251+
assertTrue("Exception message should contain the bad address",
252+
e.getMessage().contains(addr));
253+
}
254+
}
255+
}
256+
224257
@Test
225258
public void testGpuDef_withNullDeviceType() {
226259
LibvirtGpuDef gpuDef = new LibvirtGpuDef();

‎scripts/vm/hypervisor/kvm/gpudiscovery.sh‎

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -448,16 +448,12 @@ parse_nvidia_vgpu_profiles() {
448448
# normalize_pci_address output used at lookup time. Short
449449
# addresses ("AF:00.0") are widened by prepending domain 0.
450450
if [[ $gpu_address =~ ^([0-9A-Fa-f]+):([0-9A-Fa-f]+):([0-9A-Fa-f]+)\.([0-9A-Fa-f]+)$ ]]; then
451-
gpu_address=$(printf "%04x:%s:%s.%s" \
452-
"0x${BASH_REMATCH[1]}" \
453-
"${BASH_REMATCH[2],,}" \
454-
"${BASH_REMATCH[3],,}" \
455-
"${BASH_REMATCH[4],,}")
451+
gpu_address=$(printf "%04x:%02x:%02x.%x" \
452+
"0x${BASH_REMATCH[1]}" "0x${BASH_REMATCH[2]}" \
453+
"0x${BASH_REMATCH[3]}" "0x${BASH_REMATCH[4]}")
456454
elif [[ $gpu_address =~ ^([0-9A-Fa-f]+):([0-9A-Fa-f]+)\.([0-9A-Fa-f]+)$ ]]; then
457-
gpu_address=$(printf "0000:%s:%s.%s" \
458-
"${BASH_REMATCH[1],,}" \
459-
"${BASH_REMATCH[2],,}" \
460-
"${BASH_REMATCH[3],,}")
455+
gpu_address=$(printf "0000:%02x:%02x.%x" \
456+
"0x${BASH_REMATCH[1]}" "0x${BASH_REMATCH[2]}" "0x${BASH_REMATCH[3]}")
461457
else
462458
gpu_address="${gpu_address,,}"
463459
fi
@@ -557,15 +553,15 @@ get_nvidia_profile_info() {
557553
parse_pci_address() {
558554
local addr="$1"
559555
if [[ $addr =~ ^([0-9A-Fa-f]+):([0-9A-Fa-f]+):([0-9A-Fa-f]+)\.([0-9A-Fa-f]+)$ ]]; then
560-
DOMAIN="0x${BASH_REMATCH[1]}"
561-
BUS="0x${BASH_REMATCH[2]}"
562-
SLOT="0x${BASH_REMATCH[3]}"
563-
FUNC="0x${BASH_REMATCH[4]}"
556+
DOMAIN=$(printf "0x%04x" "0x${BASH_REMATCH[1]}")
557+
BUS=$(printf "0x%02x" "0x${BASH_REMATCH[2]}")
558+
SLOT=$(printf "0x%02x" "0x${BASH_REMATCH[3]}")
559+
FUNC=$(printf "0x%x" "0x${BASH_REMATCH[4]}")
564560
elif [[ $addr =~ ^([0-9A-Fa-f]+):([0-9A-Fa-f]+)\.([0-9A-Fa-f]+)$ ]]; then
565561
DOMAIN="0x0000"
566-
BUS="0x${BASH_REMATCH[1]}"
567-
SLOT="0x${BASH_REMATCH[2]}"
568-
FUNC="0x${BASH_REMATCH[3]}"
562+
BUS=$(printf "0x%02x" "0x${BASH_REMATCH[1]}")
563+
SLOT=$(printf "0x%02x" "0x${BASH_REMATCH[2]}")
564+
FUNC=$(printf "0x%x" "0x${BASH_REMATCH[3]}")
569565
else
570566
DOMAIN="0x0000"
571567
BUS="0x00"
@@ -574,14 +570,20 @@ parse_pci_address() {
574570
fi
575571
}
576572

577-
# Normalize a PCI address to its full domain-qualified form ("dddd:bb:ss.f").
578-
# Short addresses are widened by prepending domain "0000".
573+
# Normalize a PCI address to its canonical full domain-qualified form
574+
# ("dddd:bb:ss.f", lowercase, zero-padded). Both "dddd:bb:ss.f" (full) and
575+
# "bb:ss.f" (short, domain 0) inputs are accepted.
579576
normalize_pci_address() {
580577
local addr="$1"
581-
if [[ $addr =~ ^[0-9A-Fa-f]+:[0-9A-Fa-f]+:[0-9A-Fa-f]+\.[0-9A-Fa-f]+$ ]]; then
582-
echo "$addr"
578+
if [[ $addr =~ ^([0-9A-Fa-f]+):([0-9A-Fa-f]+):([0-9A-Fa-f]+)\.([0-9A-Fa-f]+)$ ]]; then
579+
printf "%04x:%02x:%02x.%x\n" \
580+
"0x${BASH_REMATCH[1]}" "0x${BASH_REMATCH[2]}" \
581+
"0x${BASH_REMATCH[3]}" "0x${BASH_REMATCH[4]}"
582+
elif [[ $addr =~ ^([0-9A-Fa-f]+):([0-9A-Fa-f]+)\.([0-9A-Fa-f]+)$ ]]; then
583+
printf "0000:%02x:%02x.%x\n" \
584+
"0x${BASH_REMATCH[1]}" "0x${BASH_REMATCH[2]}" "0x${BASH_REMATCH[3]}"
583585
else
584-
echo "0000:$addr"
586+
echo "$addr"
585587
fi
586588
}
587589

0 commit comments

Comments
 (0)