[libvirt] [libvirt-python][PATCH] sanitytest: Sanitize VIR_TYPED_PARAM_*

[Peter Krempa]

On Fri, Apr 22, 2016 at 11:09:52 +0200, Michal Privoznik wrote:
> This test checks whether all enums that we produce in libvirt.py
> have some reasonable value. But because of some crazy backcompat
> we have the following in libvirt-domain.h:
> 
>   VIR_DOMAIN_SCHED_FIELD_UINT = VIR_TYPED_PARAM_UINT
>   VIR_DOMAIN_SCHED_FIELD_ULLONG = VIR_TYPED_PARAM_ULLONG

Yes that is true. We have this also for basically every API that takes
virDomainModificationImpact flags combined with something private.

> 
> with repetitions for other types. Now, when generating libvirt.py
> those values are special cased and thus assigned correct integer
> values. But sanitytest is missing the special treatment of
> VIR_TYPED_PARAM_* and therefore produces following error:
> 
>  /usr/bin/python sanitytest.py build/lib.linux-x86_64-2.7 /home/jenkins/build/libvirt/share/libvirt/api/libvirt-api.xml
>   Cannot get a value of enum VIR_TYPED_PARAM_UINT (originally VIR_DOMAIN_SCHED_FIELD_UINT)
>   Cannot get a value of enum VIR_TYPED_PARAM_ULLONG (originally VIR_DOMAIN_SCHED_FIELD_ULLONG)

This was caused by commit f5d9c5d00cfc0c in libvirt.git. The commit
moved the enum that declares VIR_TYPED_PARAM_* into libvirt-common.h
which was NOT handled by apibuild.py from libvirt.git.

sanitytest.py (from libvirt-python.git) then takes 'libvirt-api.xml'
wich was generated by apibuild.xml (from libvirt.git) and verifies that
all enum fields are represented in the python binding.

> While the test is technically correct, "VIR_TYPED_PARAM_UINT" is
> not an integer value, it's a name for an enum value. Yet again,
> special handling is missing here, as VIR_DOMAIN_SCHED_FIELD_* has
> correct integer value in libvirt.py.

And the test is able to correctly infer the type in a second pass if you
have VIR_TYPED_PARAM_* macros present in libvirt-api.xml.

> Same applies for VIR_DOMAIN_BLKIO_PARAM_* and
> VIR_DOMAIN_MEMORY_PARAM_* which are fixed by this too.
> 
> This has been exposed by previous commit of 3026a0593bd040 which
> started to generate values for symbols from libvirt-common.h too.
> The symbols I'm mentioning above live just in that very file.

No. This is not true. That commit is part of two fixes to libvirt.git
and libvirt-python.git that are set to fix the very issue 

> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  sanitytest.py | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>  mode change 100644 => 100755 sanitytest.py
> 
> diff --git a/sanitytest.py b/sanitytest.py
> old mode 100644
> new mode 100755
> index faabccb..23163f1
> --- a/sanitytest.py
> +++ b/sanitytest.py
> @@ -22,6 +22,21 @@ def get_libvirt_api_xml_path():
>          sys.exit(proc.returncode)
>      return stdout.splitlines()[0]
>  
> +def sanitize_enum_val(value):
> +    if value == 'VIR_TYPED_PARAM_INT':
> +        value = 1
> +    elif value == 'VIR_TYPED_PARAM_UINT':
> +        value = 2
> +    elif value == 'VIR_TYPED_PARAM_LLONG':
> +        value = 3
> +    elif value == 'VIR_TYPED_PARAM_ULLONG':
> +        value = 4
> +    elif value == 'VIR_TYPED_PARAM_DOUBLE':
> +        value = 5
> +    elif value == 'VIR_TYPED_PARAM_BOOLEAN':
> +        value = 6
> +    return value
> +
>  # Path to the libvirt API XML file
>  if len(sys.argv) >= 3:
>      xml = sys.argv[2]
> @@ -66,7 +81,7 @@ for n in set:
>  for n in second_pass:
>      typ = n.attrib['type']
>      name = n.attrib['name']
> -    val = n.attrib['value']
> +    val = sanitize_enum_val(n.attrib['value'])
>  
>      for v in enumvals.values():
>          if val in v:

NACK, this works the error around by doing a local lookup rather than
fixing the real issue which was already fixed by commit
99283874733b809a962df51c33ede4446f70bfe9 in libvirt.git.

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160422/14d75e6d/attachment-0001.sig>