2011/5/31 Eric Blake <eblake@xxxxxxxxxx>:
> On 05/30/2011 07:03 AM, Matthias Bolte wrote:
>
>> From 726dae7b4c21d4c17ac19808c06d7fc978b36778 Mon Sep 17 00:00:00 2001
>> From: Matthias Bolte <matthias.bolte@xxxxxxxxxxxxxx>
>> Date: Mon, 30 May 2011 12:58:57 +0200
>> Subject: [PATCH] remote generator: Legacy support for hyper to long mappings
>>
>> Remove some special case code that took care of mapping hyper to the
>> correct C types.
>
> I like patch 1B better than 1A, so that's what I'm reviewing here.
>
>> +++ b/configure.ac
>> @@ -117,6 +117,7 @@ if test "x$have_cpuid" = xyes; then
>> Âfi
>> ÂAC_MSG_RESULT([$have_cpuid])
>>
>> +AC_CHECK_SIZEOF(long)
>
> AC_CHECK_SIZEOF([long])
>
> so that we follow the autoconf recommended quoting rules.
>
>> +++ b/daemon/remote_generator.pl
>> @@ -174,6 +174,58 @@ while (<PROTOCOL>) {
>>
>> Âclose(PROTOCOL);
>>
>> +# this dict contains the procedures that are allowed to map [unsigend] hyper
>
> s/unsigend/unsigned/
>
>> +# to [unsigend] long for legacy reasons in their signature and return type.
>> +# this list is fixed. new procedures and public APIs have to map [unsigend]
>> +# hyper to [unsigend] long long
>
> 3 more instances.
Nice copy-n-paste here, isn't it :)
>> +my $long_legacy = {
>> +  ÂDomainGetMaxMemory     Â=> { ret => { memory => 1 } },
>> +  ÂDomainGetInfo        => { ret => { maxMem => 1, memory => 1 } },
>> +  ÂDomainMigrate        => { arg => { flags => 1, resource => 1 } },
>> + Â ÂDomainMigrate2 Â Â Â Â Â Â Â=> { arg => { flags => 1, resource => 1 } },
>> + Â ÂDomainMigrateBegin3 Â Â Â Â => { arg => { flags => 1, resource => 1 } },
>
> Dan, DomainMigrate2 and DomainMigrateBegin3 are new APIs as of this
> release (similarly for other Migration v3 commands). ÂShould these use
> 'long long' rather than 'long' for resource, as well as 'unsigned int'
> for flags, as part of your edict that all new APIs should avoid 'long'?
> ÂRight _now_ is the time to make this change, before 0.9.2 goes gold.
>
>> +++ b/src/remote/remote_driver.c
>> @@ -87,6 +87,24 @@
>>
>> Â#define VIR_FROM_THIS VIR_FROM_REMOTE
>>
>> +#define HYPER_TO_TYPE(_type, _to, _from) Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
>
> I'm thinking we should move the definition of HYPER_TO_TYPE inside...
>
>> + Â Âdo { Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
>> + Â Â Â Âif ((_from) != (_type)(_from)) { Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
>> + Â Â Â Â Â ÂremoteError(VIR_ERR_INTERNAL_ERROR, Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
>> + Â Â Â Â Â Â Â Â Â Â Â Â_("conversion from hyper to %s overflowed"), #_type); \
>> + Â Â Â Â Â Âgoto done; Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
>> + Â Â Â Â} Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
>> + Â Â Â Â(_to) = (_from); Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
>> + Â Â} while (0)
>> +
>> +#if SIZEOF_LONG < 8
>
> ...this #if conditional, so that gcc won't complain about the macro
> HYPER_TO_TYPE being unused on 64 bit platforms. ÂThat's a one-liner
> change, but copied into two files.
>
>> +# define HYPER_TO_LONG(_to, _from) HYPER_TO_TYPE(long, _to, _from)
>> +# define HYPER_TO_ULONG(_to, _from) HYPER_TO_TYPE(unsigned long, _to, _from)
>> +#else
>> +# define HYPER_TO_LONG(_to, _from) (_to) = (_from)
>> +# define HYPER_TO_ULONG(_to, _from) (_to) = (_from)
>> +#endif
>> +
>
> ACK with the nits fixed. ÂI think we can push this in now whether or not
> we have Dan's answer on whether migration v3 calls should avoid 'long',
> because that would be an independent cleanup.
As Dan explained we will stick to long in migration v3 calls.
I addressed your comments and pushed 1B.
Matthias
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list