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

[David Gibson]

On Fri, Jun 14, 2019 at 01:45:24PM +0200, Andrea Bolognani wrote:
> 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.

Well, I think libvirt's obsession with maintaining backwards
compatibility in even ludicrous circumstances and at all costs
continues to make life hard for itself.  I seriously can't imagine
that anyone, anywhere has ever put a > 32-bit value in here.

But, libvirt can do libvirt, I guess, so I'm happy enough to cross it
off the list with your fix.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190701/44eeac1d/attachment-0001.sig>