Re: [libvirt] [PATCH] SpaprVio addresses are 32-bit, not 64-bit

On Tue, 2019-06-04 at 11:38 +1000, David Gibson wrote: > spapr-vio addresses are used on POWER platform qemu guests, which are based > on the PAPR specification. PAPR specifies a number of virtual devices (but > not virtio protocol) which are addressed in an abstract namespace. > > Currently, libvirt encodes these addresses as 64-bit values. This is not > correct: spapr-vio addresses are, and always have been 32-bit. That's true > both by the PAPR specification and the qemu implementation. > > Therefore, change this in libvirt. > > This looks like it would be a breaking change, but it actually isn't. > Because these have always been 32-bit at the lower levels, any attempt to > use a value here > 0xffffffff would always have failed in any case, this > will just make it fail earlier and more clearly. Thanks for providing this patch, and sorry for taking a while to get back to you about it. Unfortunately there's one major issue with your approach: even though it's true that a spapr-vio address that can't be represented as a 32-bit value would always have been rejected by QEMU and so the guest would never have been able to start, refusing to parse the value altogether would cause such a guest to disappear completely from libvirt. We don't consider this to be acceptable, because we want to give users a chance to fix their guests that doesn't involve poking at the filesystem behind libvirt's back. I have posted an alternative implementation: https://www.redhat.com/archives/libvir-list/2019-June/msg00393.html It addresses the issue mentioned above by validating the value after parsing it, so that users will be able to use 'virsh edit' or whatever other libvirt-mediated means to fix invalid configurations. In addition to that, it also updates the schema and documentation to match, and expands the test suite so that we can be sure we won't regress in the future. I even threw in a couple of cosmetic patches while at it :) I hope you don't mind. I'd appreciate any feedback you might want to provide; in the meantime, thanks for pushing me into finally looking into this long-ignored issue.