[libvirt] [PREPOST 07/17] src/xenxs:Refactor disk config parsing code
Jim Fehlig
jfehlig at suse.com
Sat Jul 12 05:28:27 UTC 2014
Eric Blake wrote:
> On 07/11/2014 01:04 PM, Jim Fehlig wrote:
>
>> David Kiarie wrote:
>>
>>> From: Kiarie Kahurani <davidkiarie4 at gmail.com>
>>>
>>> Introduce the function
>>> xenParseXMDisk(..........);
>>>
>>> Parses disk config
>>>
>>> signed-off-by: David Kiarie<davidkiarie4 at gmail.com>
>>> ---
>>> src/xenxs/xen_xm.c | 204 ++++++++++++++++++++++++++++-------------------------
>>> 1 file changed, 108 insertions(+), 96 deletions(-)
>>>
>>>
>
>
>>> @@ -912,7 +839,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
>>>
>>> /* Maintain list in sorted order according to target device name */
>>> if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0)
>>> - goto cleanup;
>>> + return -1;
>>>
>>> skipdisk:
>>> list = list->next;
>>>
>>>
>> The next line here is
>>
>> virDomainDiskDefFree(disk);
>>
>> which is not right in the case VIR_APPEND_ELEMENT succeeds (pre-existing
>> bug). 'disk' should be set to NULL if VIR_APPEND_ELEMENT succeeds, so
>> we don't free the disk we've just added to the disk list.
>>
>
> VIR_APPEND_ELEMENT(, , disk) sets disk=NULL on success, precisely so you
> don't have to worry about double-freeing it (if you call the macro, you
> are guaranteed either transfer semantics and success, or no change on
> failure).
>
Ah, should have read the source :). Thanks Eric. That explains my
confusion as to why a crash was never reported in this code (or the vif
parsing code).
Regards,
Jim
More information about the libvir-list
mailing list