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

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(a)redhat.com&gt; > --- > 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.