Re: [libvirt] [PATCH 2/4] qemu: Use same model when adding hostdev SCSI controller
On 12/06/2017 08:08 AM, 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>
---
src/qemu/qemu_hotplug.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
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
@@ -587,6 +587,7 @@ qemuDomainFindOrCreateSCSIDiskController(virQEMUDriverPtr driver,
{
size_t i;
virDomainControllerDefPtr cont;
+ virDomainControllerModelSCSI model = -1;
for (i = 0; i < vm->def->ncontrollers; i++) {
cont = vm->def->controllers[i];
@@ -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
+ * 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;
Hrm... I'm missing something...
This is confusing, because nothing in this loop tells us the controller
we find is full, just that we have a TYPE_SCSI controller and the index
isn't found. And if the index WAS found, we would've exited before this
line anyway so we don't know if it was full or not.
Nothing in a <hostdev> tag says what type of controller we want, so what
happens when controller[1] is virtio-scsi, and controller[2] is LSI?
This will create a second LSI controller which wouldn't help if we're
hotplugging another virtio-scsi device and controller[1] was full. (Is
mixing SCSI controller types common? I don't know, I stay in a little
virtio-scsi playpen.)
And I'm still trying to figure out how qemuDomainSetSCSIControllerModel
plays into this and other codepaths.
Sorry, just sending my questions because this code is more fresh in your
mind than my own.
}
/* No SCSI controller present, for backward compatibility we
@@ -604,11 +611,10 @@ qemuDomainFindOrCreateSCSIDiskController(virQEMUDriverPtr driver,
return NULL;
cont->type = VIR_DOMAIN_CONTROLLER_TYPE_SCSI;
cont->idx = controller;
- cont->model = -1;
+ cont->model = model;
- VIR_INFO("No SCSI controller present, hotplugging one");
- if (qemuDomainAttachControllerDevice(driver,
- vm, cont) < 0) {
+ VIR_INFO("No SCSI controller present, hotplugging one model=%d", model);
Seems like a good use for
virDomainControllerModelSCSITypeToString(model) perhaps?
+ if (qemuDomainAttachControllerDevice(driver, vm, cont) < 0)
{
VIR_FREE(cont);
return NULL;
}