On Thu, Jan 25, 2024 at 10:20:05 -0800, Andrea Bolognani wrote: > On Thu, Jan 25, 2024 at 01:07:10PM +0100, Peter Krempa wrote: > > 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. > > That's why I've justified in the commit message as being something > that would pay off later on ;) > > I can roll it into the later patch. I think I had it there initially, > but it seemed that it would make reviewing the change harder. Since > you're the one reviewing, I'm happy to go with your preference. Please do so, this doesn't get an approval from me in this form. _______________________________________________ Devel mailing list -- devel@xxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
