Re: Re: [PATCH 15/33] qemu: Simplify qemuDomainFindOrCreateSCSIDiskController()

On Wed, Jan 24, 2024 at 20:37:35 +0100, Andrea Bolognani wrote: > @@ -876,10 +876,9 @@ qemuDomainFindOrCreateSCSIDiskController(virDomainObj *vm, > cont = g_new0(virDomainControllerDef, 1); > cont->type = VIR_DOMAIN_CONTROLLER_TYPE_SCSI; > cont->idx = controller; > - if (model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT) > - cont->model = qemuDomainGetSCSIControllerModel(vm->def, cont, priv->qemuCaps); > - else > - cont->model = model; > + cont->model = model; > + > + cont->model = qemuDomainGetSCSIControllerModel(vm->def, cont, priv->qemuCaps); This makes no sense at first look. Commit message doesn't explain it, nothing in the code explains it. You have to look into qemuDomainGetSCSIControllerModel to see why it can be done. I've looked ahead to see how you've refactored it, at which point it makes sense, but I'm not really persuaded that this can't be part of the commit that is actually fixing the function to not depend on the value in the 'model' struct. IMO this patch makes the code strictly worse, even when it's short lived.