On Tue, May 14, 2024 at 12:45 PM Michal Prívozník <mprivozn@xxxxxxxxxx> 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@xxxxxxxxx> > > --- > > ... 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@xxxxxxxxxx> > > Sorry for the double mail, forgot to reply to all. -- Rayhan Faizel
