[libvirt] [PATCH v2 3/4] qemu: propagate virQEMUDriver object to qemuDomainDeviceCalculatePCIConnectFlags

Laine Stump laine at laine.org
Mon Nov 28 20:51:40 UTC 2016


On 11/24/2016 10:31 AM, Andrea Bolognani wrote:
> On Mon, 2016-11-21 at 00:01 -0500, Laine Stump wrote:
> [...]
>> @@ -2376,6 +2388,7 @@ qemuDomainAssignAddresses(virDomainDefPtr def,
>>     */
>>    int
>>    qemuDomainEnsurePCIAddress(virDomainObjPtr obj,
>> +                           virQEMUDriverPtr driver,
>>                               virDomainDeviceDefPtr dev)
> I'm pretty sure you want the virQEMUDriverPtr to be *after*
> the virDomainDeviceDefPtr, not before it.
>
>> @@ -1772,7 +1772,8 @@ qemuDomainChrRemove(virDomainDefPtr vmdef,
>>     *          0 otherwise
>>     */
>>    static int
>> -qemuDomainAttachChrDeviceAssignAddr(virDomainObjPtr vm,
>> +qemuDomainAttachChrDeviceAssignAddr(virQEMUDriverPtr driver,
>> +                                    virDomainObjPtr vm,
>>                                        virDomainChrDefPtr chr)
> Here the virQEMUDriverPtr should be last.
>
>> @@ -3373,8 +3373,10 @@ qemuProcessReconnect(void *opaque)
>>            goto cleanup;
>>        }
>>    
>> -    if ((qemuDomainAssignAddresses(obj->def, priv->qemuCaps, obj, false)) < 0)
>> +    if ((qemuDomainAssignAddresses(obj->def, priv->qemuCaps,
>> +                                   driver, obj, false)) < 0) {
>>            goto error;
>> +    }
> According to our style guidelines, the curly braces are
> optional here, as the second line of the condition is
> indented in a way that makes it obvious it's still part
> of the condition.
>
> I point this out just as an aside, I actually like it
> better with the curly braces :)

I agree. I never got the logic behind that exception, so I'm glad it's 
optional to omit the braces.

>
>
> ACK once you shuffle the arguments around.
>
> -- 
> Andrea Bolognani / Red Hat / Virtualization
>




More information about the libvir-list mailing list