On Fri, Feb 09, 2024 at 08:43:56AM +0100, Alice Frosi wrote: > On Wed, Feb 7, 2024 at 11:13 AM Daniel P. Berrangé <berrange@xxxxxxxxxx> > wrote: > > > 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. The issue is that this would be a change of API, and significant amount of work to do to audit each case, to solve a precision problem that doesn't appear to have any real world functional impact. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| _______________________________________________ Devel mailing list -- devel@xxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
