[libvirt] remote generator: Legacy support for hyper to long mappings

[Eric Blake]

On 05/30/2011 07:03 AM, Matthias Bolte wrote:

> From 726dae7b4c21d4c17ac19808c06d7fc978b36778 Mon Sep 17 00:00:00 2001
> From: Matthias Bolte <matthias.bolte at googlemail.com>
> 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.

> +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.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110531/e6fcd5a3/attachment-0001.sig>