On Wed, Feb 07, 2024 at 01:54:30AM -0800, Andrea Bolognani wrote:
> On Wed, Feb 07, 2024 at 10:06:46AM +0100, Alice Frosi wrote:
> > Hi everyone,
> >
> > I would like to bring up a possible issue with the libvirt-go-xml-module
> > and request your advice.
> >
> > We are considering using libvirtxml [2] as an alternative of the custom
> > types for the libvirt schema in KubeVirt [1].
> > Nonetheless, KubeVirt makes an effort to adhere to Kubernetes best
> > guidelines. One thing I've noticed, though, is that libvirtml consistently
> > uses uint or int rather than uint32 or int32. This is in opposition to the
> > primitive types that Kubernetes [3] recommends; in fact, KubeVirt uses them
> > in the schema.go [4].
>
> I sure spotted several uses of (u)int in there :)
>
> > Would it be possible to align the libvirtxml go library to the Kubernetes
> > conventions?
>
> Since this library is all about handling XML and doesn't talk at all
> with the underlying C library, I guess it would be possible to
> introduce explicit sizing for all struct members. I don't think we
> can do that without breaking API though.
It would also be a major effort to decide between int32 and int64
if we want to honour the other k8s rule that says to avoid int64
unless absolutely required.
> Another topic entirely would be whether the same is possible for the
> main binding. In that case, the underlying C library dictates the
> types, so I'm not sure we could change them at the Go level even if
> we wanted to. I know that you're not currently asking for this, I'm
> just trying to consider the full picture.
In the C API we have been pretty careful to use 'unsigned long long'
everywhere it was needed, which is mapped to int64 in the go bindings.
There were just 2 APIs where we mistakenly used 'unsigned long'
which varied in size, but we map them to int64 in Go too.
In any case, while k8s might care about 32-bit, in the virtualization
space almost no one cares anymore. i386 is basically dead and liable
to be removed from QEMU. Arm7 is limping along, but its use of virt
is larged in embedded / special scenarios - no one is going to be
offering public cloud with arm7.
IOW, I see no reason for kubevirt to care about 32-bit targets, and
thus for all practical purposes any use of 'int' can be assumed to
always be 64-bit [1].
But this isn't clear if we keep using (u)int types. What I'm suggesting here is if it would be possible in a next version of the library to use more explicit types.
My concern is that KubeVirt uses (u)int32 as much as possible and in this case, we might have a mismatch of the types.