[Libvirt-cim] [PATCH V4] DevicePool, reimplement get_diskpool_config with libvirt
Wenchao Xia
xiawenc at linux.vnet.ibm.com
Tue Mar 19 07:06:31 UTC 2013
于 2013-3-12 13:57, Wenchao Xia 写道:
> 于 2013-3-12 4:24, John Ferlan 写道:
>>
>> Since I don't have an email to reply-to, here is the link:
>>
>> https://www.redhat.com/archives/libvirt-cim/2012-December/msg00039.html
>>
>>
>>
>> In get_diskpool_config():
>>
>> * Rather than use the "racy" NumOfStoragePools and ListStoragePools, why
>> not use virConnectListAllStoragePools() passing the "Active" flag for
>> all active pools only?
>>
> Let me check if the version of minium libvirt requirement in configure,
> to see if the API exist.
>
>> * You may even want to consider keeping the returned virStoragePoolPtr
>> structures around..
>>
>>
>>
>> I also imagine the other objects (networks and domains) could use the
>> similar calls. I guess the answer somewhat depends on what is the
>> minimum version of libvirt that needs to be supported.
>>
> Yep, that is the problem. To limit the work, I guess other change
> for networks and domains should be separate patches, if we decide
> they are worthy.
>
>> But if you "have" to stay with the current model...
Checked with the new API, it is too new. I hope to stick to
API requirement around libvirt 0.9.0 to avoid trouble when
user want to just upgrade libvirt-cim as fix, so if this function
can ensure no more risk with old libvirt API, I prefer the old
way.
>>
>> * The return 'names[i]' is something you'd have to free() anyway, so
>> rather than strdup(names[i]), just take it when setting pools[i].tag and
>> set names[i] = NULL; That avoids an error path.
>>
> let me check.
>
This seems workable, thanks.
>>
>>
>> John
>>
>>
>>
>> _______________________________________________
>> Libvirt-cim mailing list
>> Libvirt-cim at redhat.com
>> https://www.redhat.com/mailman/listinfo/libvirt-cim
>>
>
>
--
Best Regards
Wenchao Xia
More information about the Libvirt-cim
mailing list