On 12/20/2017 07:46 AM, Ján Tomko wrote: > On Wed, Dec 06, 2017 at 08:08:04AM -0500, John Ferlan wrote: >> When qemuDomainFindOrCreateSCSIDiskController adds a controller, >> let's use the same model as a currently found controller under the >> assumption that the reason to add the controller in hotplug is >> because virDomainHostdevAssignAddress determined that there were >> too many devices on the existing controller, but only assigned a >> new controller index and did not add a new controller and we >> desire to use the same controller model as any existing conroller > > s/conroller/controller/ > >> and not take a chance that qemuDomainSetSCSIControllerModel would >> use a default that may be incompatible. >> >> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> >> --- >> src/qemu/qemu_hotplug.c | 14 ++++++++++---- >> 1 file changed, 10 insertions(+), 4 deletions(-) >> > > ACK > > I like that this reduces the chances of model -1 appearing in XML, > (maybe a step closer to removing qemuDomainSetSCSIControllerModel?). > On the other hand, it does change the 'policy' of choosing the > controller model. We don't plug one on PCI hotplug. > True... OTOH for this path, we're processing *just* a <hostdev> or <disk> and will be creating the controller hopefully based on what is existing and already full. I would hope that wouldn't be considered bad policy! I found more problems when having a full virtio-scsi controller and creating an LSI controller because that's the default. Beyond that, Michal noted something on qemu-devel about having more than one LUN on an LSI controller being a problem: http://lists.nongnu.org/archive/html/qemu-devel/2017-12/msg01125.html There's also some patches on qemu-devel where iSCSI LUN's on an LSI controller were causing crashes (perhaps something I ran into, but didn't spend the time debugging): http://lists.nongnu.org/archive/html/qemu-devel/2017-12/msg03119.html >> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c >> index 9317e134a..90d50e7b1 100644 >> --- a/src/qemu/qemu_hotplug.c >> +++ b/src/qemu/qemu_hotplug.c > > [...] > >> @@ -596,6 +597,12 @@ >> qemuDomainFindOrCreateSCSIDiskController(virQEMUDriverPtr driver, >> >> if (cont->idx == controller) >> return cont; >> + >> + /* Save off the model - if we end up creating a controller it's > > What does 'save off' mean? > Just an expression... For the most recent text I used see: https://www.redhat.com/archives/libvir-list/2017-December/msg00483.html I can still remove the "off" part John > Jan > >> + * because the user didn't provide one and we need to >> automagically >> + * create one because the existing one is full - so let's be >> sure >> + * to keep the same model in that case. */ >> + model = cont->model; >> } >> >> /* No SCSI controller present, for backward compatibility we -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
