2011/4/27 Daniel Veillard <veillard@xxxxxxxxxx>:
> On Mon, Apr 25, 2011 at 01:32:43PM +0200, Matthias Bolte wrote:
>> 2011/4/25 Matthias Bolte <matthias.bolte@xxxxxxxxxxxxxx>:
>> > This was broken by the refactoring in ac1e6586ec75. It resulted in a
>> > segfault for 'virsh vol-dumpxml' and related volume functions.
>> >
>>
>> Actually that patch doesn't fix the problem correctly. It just turns
>> the segfault into an error message.
>>
>> Here's v2 that really fixes the problem. Note the important difference
>> between anyType->_type and anyType->type :)
>>
>> Matthias
>
>> From a17245fb92c88439ffbb84e34bebf1afb6903885 Mon Sep 17 00:00:00 2001
>> From: Matthias Bolte <matthias.bolte@xxxxxxxxxxxxxx>
>> Date: Mon, 25 Apr 2011 12:38:17 +0200
>> Subject: [PATCH] esx: Fix dynamic dispatch for CastFromAnyType functions
>>
>> This was broken by the refactoring in ac1e6586ec75. It resulted in a
>> segfault for 'virsh vol-dumpxml' and related volume functions.
>>
>> Before the refactoring all users of the ESX_VI__TEMPLATE__DISPATCH
>> macro dispatched on the item type, as the item is the input to all those
>> functions.
>>
>> Commit ac1e6586ec75 made the dynamically dispatched CastFromAnyType
>> functions use this macro too, but this functions dispatched on the
>> actual type of the AnyType object. The item is the output of the
>> CastFromAnyType functions.
>>
>> This difference was missed in the refactoring, making CastFromAnyType
>> functions dereferencing the item pointer that is NULL at the time of
>> the dispatch.
>> ---
>> Âsrc/esx/esx_vi_types.c | Â 14 ++++++++------
>> Â1 files changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c
>> index dd83954..3c81021 100644
>> --- a/src/esx/esx_vi_types.c
>> +++ b/src/esx/esx_vi_types.c
>> @@ -533,8 +533,9 @@
>> Â * Macros to implement dynamic dispatched functions
>> Â */
>>
>> -#define ESX_VI__TEMPLATE__DISPATCH(__type, _dispatch, _error_return) Â Â Â Â Â\
>> - Â Âswitch (item->_type) { Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
>> +#define ESX_VI__TEMPLATE__DISPATCH(_actual_type, __type, _dispatch, Â Â Â Â Â \
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â _error_return) Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
>> + Â Âswitch (_actual_type) { Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
>>    Â_dispatch                                \
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
>> Â Â Â Âcase esxVI_Type_##__type: Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
>> @@ -543,7 +544,7 @@
>> Â Â Â Âdefault: Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
>> Â Â Â Â ÂESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
>> Â Â Â Â Â Â Â Â Â Â Â _("Call to %s for unexpected type '%s'"), __FUNCTION__, Â\
>> - Â Â Â Â Â Â Â Â Â Â esxVI_Type_ToString(item->_type)); Â Â Â Â Â Â Â Â Â Â Â \
>> + Â Â Â Â Â Â Â Â Â Â esxVI_Type_ToString(_actual_type)); Â Â Â Â Â Â Â Â Â Â Â\
>> Â Â Â Â Âreturn _error_return; Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
>> Â Â Â}
>>
>> @@ -585,7 +586,8 @@
>>
>> Â#define ESX_VI__TEMPLATE__DYNAMIC_FREE(__type, _dispatch, _body) Â Â Â Â Â Â Â\
>> Â Â ÂESX_VI__TEMPLATE__FREE(__type, Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
>> - Â Â ÂESX_VI__TEMPLATE__DISPATCH(__type, _dispatch, /* nothing */) Â Â Â Â Â Â\
>> + Â Â ÂESX_VI__TEMPLATE__DISPATCH(item->_type, __type, _dispatch, Â Â Â Â Â Â Â\
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â /* nothing */) Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
>> Â Â Â Â_body)
>>
>>
>> @@ -618,14 +620,14 @@
>>
>> Â#define ESX_VI__TEMPLATE__DYNAMIC_CAST_FROM_ANY_TYPE(__type, _dispatch) Â Â Â \
>> Â Â ÂESX_VI__TEMPLATE__CAST_FROM_ANY_TYPE_EXTRA(__type, esxVI_##__type, Â Â Â Â\
>> - Â Â ÂESX_VI__TEMPLATE__DISPATCH(__type, _dispatch, -1), Â Â Â Â Â Â Â Â Â Â Â\
>> + Â Â ÂESX_VI__TEMPLATE__DISPATCH(anyType->type, __type, _dispatch, -1), Â Â Â \
>> Â Â Â Â/* nothing */)
>>
>>
>>
>> Â#define ESX_VI__TEMPLATE__DYNAMIC_SERIALIZE(__type, _dispatch, _serialize) Â Â\
>> Â Â ÂESX_VI__TEMPLATE__SERIALIZE_EXTRA(__type, Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
>> - Â Â ÂESX_VI__TEMPLATE__DISPATCH(__type, _dispatch, -1), Â Â Â Â Â Â Â Â Â Â Â\
>> + Â Â ÂESX_VI__TEMPLATE__DISPATCH(item->_type, __type, _dispatch, -1), Â Â Â Â \
>> Â Â Â Â_serialize)
>>
>
> ÂNot sure I fully understand but looks regular and okay, ACK :-)
>
> Daniel
>
Actually, this shows that even I'm not always aware of all the details
in the ESX driver, otherwise this regression fix wouldn't have be
necessary and I wouldn't have needed two attempts to get it right. So
don't feel too bad about not fully understanding it :)
This is probably a sign that I should try to reduce the complexity and
levels of indirection in the generated code and the code generator.
Thanks, finally pushed v2.
Matthias
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list