[libvirt] [PATCH] qemu: fix use a nonexist address in qemuDomainAttachDeviceConfig

[Luyao Huang]

On 12/16/2014 08:00 PM, Peter Krempa wrote:
> On 12/15/14 10:10, Luyao Huang wrote:
>> We free them before, then use it. This make we always do
> We clear the pointer, not free.
>
>> virDomainDefAddImplicitControllers when attach a disk.
>>
>> Signed-off-by: Luyao Huang <lhuang at redhat.com>
>> ---
>>   src/qemu/qemu_driver.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index df3ba6d..a9afa5d 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -7223,10 +7223,10 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
>>           if (virDomainDiskInsert(vmdef, disk))
>>               return -1;
>>           /* vmdef has the pointer. Generic codes for vmdef will do all jobs */
>> -        dev->data.disk = NULL;
> The comment even explains why the line is there. From that point on, the
> pointer is owned by 'vmdef' and thus the original needs to be cleared so
> that it isn't freed (and doulbe freed) later on.
Oh, i see. Thanks for your review and i found seems i don't check if gdb 
is real go to virDomainDefAddImplicitControllers, just check the 
pointer's value is optimized out.

(gdb)
7204            dev->data.disk = NULL;
(gdb)
7205            if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO)
(gdb) p disk
$3 = <optimized out>

Thanks a lot for your help :)
>
>>           if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO)
> Additionally, 'disk' contains still a valid copy of the pointer so the
> code should run just fine.
>
>>               if (virDomainDefAddImplicitControllers(vmdef) < 0)
>>                   return -1;
>> +        dev->data.disk = NULL;
> So moving the line here doesn't make sense.
>
>>           if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0)
>>               return -1;
>>           break;
>>
> NACK,
> Peter
>
Luyao