Re: [PATCH libvirt v1 0/6] Fix ZPCI address auto-generation on s390

On Mon, 2020-04-20 at 21:55 +0200, Boris Fiuczynski wrote: > On 4/10/20 2:06 PM, Andrea Bolognani wrote: >> On Thu, 2020-04-09 at 12:30 +0200, Shalini Chellathurai Saroja wrote: >>> The ZPCI address validation or autogeneration does not work as >>> expected in the following scenarios >>> 1. uid = 0 and fid = 0 >>> 2. uid = 0 and fid > 0 >>> 3. uid = 0 and fid not specified >>> 4. uid not specified and fid > 0 >>> 5. 2 ZPCI devices with uid > 0 and fid not specified. >>> >>> This is because of the following reasons >>> 1. If uid = 0 or fid = 0 the code assumes that user has not specified >>> the corresponding address >>> 2. If either uid or fid is provided, the code assumes that both uid >>> and fid addresses are specified by the user. >> >> I'd have to dig up the old threads, but based on what I remember the >> behaviors you describe are entirely intentional. >> >> For PCI addresses, setting all parts of the address to zero or not >> setting it at all is equivalent, and we wanted to be consistent with >> that behavior for ZPCI; additionally, zero is not a valid value for >> uid so of course neither is the address uid=0 fid=0, which means that >> we're not preventing the user from specifying a valid address by >> conflating the all-zero address with the unspecified address. >> >> For partially-specified addresses, the behavior is also the same as >> PCI: any part you don't specify is considered to be zero, which >> results in >> >> uid=0 fid=0 -> uid=0 fid=0 -> address gets autogenerated >> uid=0 fid=x -> uid=0 fid=x -> address is rejected as invalid >> uid=0 -> uid=0 fid=0 -> address gets autogenerated >> fid=x -> uid=0 fid=x -> address is rejected as invalid >> uid=x -> uid=x fid=0 -> address is accepted >> >> So, just like for PCI addresses, you have basically two reasonable >> options: either don't specify any zPCI address and leave allocation >> entirely up to libvirt, or specify all of the addresses completely: >> anything in between will likely not work as you'd expect or want. >> >> Again, this is based purely on my recollection of design discussions >> that happened one and a half years ago, so I might have gotten some >> of the details wrong - in which case by all means call me out on >> that O:-) >> > Hi Andrea, > sorry for the delayed answer. I (and some others as well) lost some > emails on my IMAP account and I just found your answer today. No apologies needed: I also took a long time to reply to your message, and in my case there's no mail server malfunction that I can assign the blame to O:-) > I can remember that you had a discussion with the original author of the > zpci code. There are a few issues with the currently implemented "rules" > which partially are not even working as you outlined above in more > complex scenarios. I disagree with this assessment - they work exactly as designed and as described above. Whether we *want* them to behave that way... Now that's a different topic :) I think the disconnect lies in what the user's expectations are and what libvirt actually implements. Basically the user expects that * if either one of uid and fid is explictly assigned a value by the user, then the guest will use that value - unless such value is invalid, in which case libvirt will report an error; * if either one of uid and fid is absent from the user-provided configuration, then libvirt will automatically pick a valid value for the attribute. This is not how the current zPCI implementation works, or how PCI address assignment works in libvirt; on the other hand, I think these expectations are in fact completely reasonable, as the examples you provide illustrate quite well. I think you successfully convinced me that the current approach is not good for users and we should fix it; my only doubt at this point is whether can we safely do that without breaking libvirt's backwards compatibility guarantees. Dan, Laine, what's your take?