Re: [libvirt] [PATCH 2/4] qemu: Use same model when adding hostdev SCSI controller

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(a)redhat.com&gt; > --- > 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.

> 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?

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