[PATCH v2] qemu_hotplug: Properly assign USB address to hotplugged usb-net device
Previously, the network device hotplug logic would try to ensure only CCW or PCI addresses. With recent support for the usb-net model, this patch will ensure USB addresses for usb-net network devices. [Changes in v2] - Additional information in commit message - Relevant gitlab issue link (mostly already resolved but hotplugging is mentioned in one of the comments) Resolves: https://gitlab.com/libvirt/libvirt/-/issues/14 Signed-off-by: Rayhan Faizel <rayhan.faizel@gmail.com> --- src/qemu/qemu_hotplug.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 774962b0df..3b39941780 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1159,8 +1159,11 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, goto cleanup; } - if (qemuDomainIsS390CCW(vm->def) && - net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + if (net->model == VIR_DOMAIN_NET_MODEL_USB_NET) { + if (virDomainUSBAddressEnsure(priv->usbaddrs, &net->info) < 0) + goto cleanup; + } else if (qemuDomainIsS390CCW(vm->def) && + net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; if (!(ccwaddrs = virDomainCCWAddressSetCreateFromDomain(vm->def))) goto cleanup; -- 2.34.1
On 5/13/24 15:55, Rayhan Faizel wrote:
Previously, the network device hotplug logic would try to ensure only CCW or PCI addresses. With recent support for the usb-net model, this patch will ensure USB addresses for usb-net network devices.
[Changes in v2] - Additional information in commit message - Relevant gitlab issue link (mostly already resolved but hotplugging is mentioned in one of the comments)
This changelog belongs ...
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/14 Signed-off-by: Rayhan Faizel <rayhan.faizel@gmail.com> ---
... here. Right after these three dashes. The changelong isn't necessary helpful when reading git log, it's needed only for review.
src/qemu/qemu_hotplug.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 774962b0df..3b39941780 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1159,8 +1159,11 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, goto cleanup; }
- if (qemuDomainIsS390CCW(vm->def) && - net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + if (net->model == VIR_DOMAIN_NET_MODEL_USB_NET) { + if (virDomainUSBAddressEnsure(priv->usbaddrs, &net->info) < 0) + goto cleanup; + } else if (qemuDomainIsS390CCW(vm->def) && + net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; if (!(ccwaddrs = virDomainCCWAddressSetCreateFromDomain(vm->def))) goto cleanup;
Right. I was wondering what about the case when a new domain is defined, whether we do fill in address in that case and we do. It's just that our test case doesn't cover that case. I mean, there's tests/qemuxmlconfdata/net-usb.xml and also tests/qemuxmlconfdata/net-usb.x86_64-latest.xml but because the former doesn't have any USB address, it causes qemuDomainAssignUSBAddresses() to return early without assigning any addresses. At any rate, your patch is correct. Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
On Tue, May 14, 2024 at 12:45 PM Michal Prívozník <mprivozn@redhat.com> wrote:
On 5/13/24 15:55, Rayhan Faizel wrote:
Previously, the network device hotplug logic would try to ensure only CCW or PCI addresses. With recent support for the usb-net model, this patch will ensure USB addresses for usb-net network devices.
[Changes in v2] - Additional information in commit message - Relevant gitlab issue link (mostly already resolved but hotplugging is mentioned in one of the comments)
This changelog belongs ...
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/14 Signed-off-by: Rayhan Faizel <rayhan.faizel@gmail.com> ---
... here. Right after these three dashes. The changelong isn't necessary helpful when reading git log, it's needed only for review.
src/qemu/qemu_hotplug.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 774962b0df..3b39941780 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1159,8 +1159,11 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, goto cleanup; }
- if (qemuDomainIsS390CCW(vm->def) && - net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + if (net->model == VIR_DOMAIN_NET_MODEL_USB_NET) { + if (virDomainUSBAddressEnsure(priv->usbaddrs, &net->info) < 0) + goto cleanup; + } else if (qemuDomainIsS390CCW(vm->def) && + net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; if (!(ccwaddrs = virDomainCCWAddressSetCreateFromDomain(vm->def))) goto cleanup;
Right. I was wondering what about the case when a new domain is defined, whether we do fill in address in that case and we do. It's just that our test case doesn't cover that case. I mean, there's tests/qemuxmlconfdata/net-usb.xml and also tests/qemuxmlconfdata/net-usb.x86_64-latest.xml but because the former doesn't have any USB address, it causes qemuDomainAssignUSBAddresses() to return early without assigning any addresses.
This is something I've also noticed. The auto-assigned USB address does not show up in the x86-64-latest.xml file but appears later in the args file. Some other examples like usb-audio, usb-mtp, usb disk, etc. in the test cases seem to exhibit the same behaviour so I didn't think too much of it. Though from virt-manager's POV, the USB address does still appear correctly.
At any rate, your patch is correct.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Sorry for the double mail, forgot to reply to all. -- Rayhan Faizel
participants (2)
-
Michal Prívozník -
Rayhan Faizel