Re: [Xen-devel] [PATCH v3 18/28] tools/libxl: Infrastructure to convert a legacy stream

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 13/07/15 16:44, Ian Jackson wrote:
> Andrew Cooper writes ("Re: [PATCH v3 18/28] tools/libxl: Infrastructure to 
> convert a legacy stream"):
>> On 13/07/15 15:45, Ian Jackson wrote:
> ...
>>> I'm afraid I don't understand why this script needs to be told the
>>> toolstack build bit width.  (If indeed the toolstack build bit width
>>> is the right thing, can't the script tell what bitness it is running
>>> on, some other way?)
>> The script needs to know the bitness of the toolstack which saved the
>> legacy stream, not the bitness of the one which is currently running.
> But an ifdef tells you the bitness of the toolstack which is currently
> running.
>
>> The ifdefary above is an assumption for the common case.  By
>> observation, noone outside of XenServer has tried migrating a VM from a
>> 32bit toolstack to a 64bit, or they have and not reported a bug. 
>> Therefore, the common case is that the bitness of toolstack is the same.
> I see.
>
>> If in the future someone genuinely needs to convert legacy -> v2 and 32
>> -> 64bit at the same time, they can use the conversion script manually
>> to achieve this (even in a migration), which will bypass the conversion
>> logic in libxl, as the incoming stream is already v2.
> How ... exciting.
>
>>> I appreciate that this seems very pernickety for compatibility code
>>> which will only be executed on i386 or amd64, but #ifdefs like this
>>> make the code hard to read IMO.
>> Would it be acceptable to format the above description into a comment?
> That would help.
>
> But also I want rid of the #ifdef for stylistic reasons.
>
> How about you make the --width option optional, and default it to
> native bitness in the conversion script ?

I could do, but that would make the further assumption that the width of
libxl is the same as the width of python, which is by no means
guaranteed to be the case.

I could see about doing something based on BITS_PER_LONG which doesn't
have a visible ifdef, but is still ifdefary.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel