[libvirt] [PATCH v4 02/25] [ACKED but has additions] qemu: replace a lot of "def->controllers[i]" with equivalent "cont"

Laine Stump laine at laine.org
Fri Oct 21 23:50:40 UTC 2016


On 10/18/2016 07:22 AM, Andrea Bolognani wrote:
> On Fri, 2016-10-14 at 15:53 -0400, Laine Stump wrote:
>> There's no functional change here. This pointer was just used so many
>> times that the extra long lines became annoying.
>> ---
>>   
>> Change: added more instances of the same change.
>>   
>>    src/qemu/qemu_domain_address.c | 208 ++++++++++++++++++++++-------------------
>>    1 file changed, 111 insertions(+), 97 deletions(-)
>>   
>> diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
>> index dc67d51..e6abadf 100644
>> --- a/src/qemu/qemu_domain_address.c
>> +++ b/src/qemu/qemu_domain_address.c
>> @@ -220,18 +220,22 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def,
>>        }
>>    
>>        for (i = 0; i < def->ncontrollers; i++) {
>> -        model = def->controllers[i]->model;
>> -        if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
>> +        virDomainControllerDefPtr cont = def->controllers[i];
>> +
>> +        model = cont->model;
>> +        if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
>>                if (qemuDomainSetSCSIControllerModel(def, qemuCaps, &model) < 0)
> Definitely not something that should be touched by this patch,
> but shouldn't we pass &cont->model here?
>
> I mean, if the value stored in model will be different than
> the one that was already in cont->model, it means that the
> default controller model was not set properly earlier...
>
> On the other hand, the default model should really have been
> set in PostParse() or something like that.

The function qemuDomainSetSCSIControllerModel() seems to be a confused 
mess. If model is set (non-0) then it checks the qemuCaps to makes sure 
the model that's set is allowed. Otherwise, it sets a default model. 
*All* calls to this function are on a temporary variable rather than the 
item in the controller object, so any non-zero model set will never be 
stored in the config. Why it is that way, I have no idea.

I choose to not touch it, for fear that something offbeat would break. 
(Maybe it's done this way because some old version of libvirt 6 or 7 
years ago didn't support setting a scsi controller model or something? 
Who knows...)




More information about the libvir-list mailing list