jmsperu commented on PR #12826: URL: https://github.com/apache/cloudstack/pull/12826#issuecomment-5598865186
@shwstppr thanks for testing it properly. What you saw is what this change predicts for that layout, which tells me the PR description oversold it, so let me be precise about what it does and does not do. **What the change does.** On NIC hot-plug it picks the lowest free PCI slot *above the highest slot already used by a NIC*, instead of letting libvirt pick the lowest free slot anywhere on the bus. **Why you saw no difference.** After deploy plus a data disk, the slots right after the first NIC (typically 0x04 to 0x08: virtio-serial, balloon, root disk, rng, the added disk) are all taken by non-NIC devices. So "lowest free above 0x03" and "lowest free anywhere" are the same slot, 0x09, and the guest names it ens9 either way. **Where it does differ.** Whenever a slot *below* the highest NIC is free, libvirt fills that gap and this patch does not. To reproduce: 1. VM with two NICs, on 0x03 and 0x04 (ens3, ens4). 2. Unplug the second NIC, so 0x04 is free. 3. Hot-plug a NIC. Before this change libvirt reuses 0x04 and the new NIC comes up as ens4, taking the name of the NIC the tenant just removed, and it enumerates *before* NICs that were plugged earlier. After the change it goes above the highest existing NIC, so interface names only ever move up and never collide with a removed one. **What it cannot do on its own** is make the hot-plugged NIC ens5 in your scenario. That needs slots 0x04 upward kept free for NICs, which means placing the non-NIC devices at high slots when the VM is defined. I left that out deliberately: pinning device addresses changes the PCI layout of existing VMs on their next start, which renames their interfaces, so it has to apply only to VMs created after the change (a detail set at deploy), most likely behind a setting. That is a separate PR and I am happy to write it. Two ways forward for this one: (a) keep it as the monotonic-order guarantee it actually is, retitled accordingly (done), or (b) hold it until the creation-time reservation exists and land both together. I would take (a) because it is self-contained and fixes the removed-NIC name reuse above, but it is your call. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
