On Thu, Nov 16, 2017 at 03:44:57PM +0100, Andrea Bolognani wrote: > On Thu, 2017-11-16 at 14:22 +0100, Pavel Hrdina wrote: > > On Wed, Nov 15, 2017 at 12:50:10PM +0100, Andrea Bolognani wrote: > > > We can finally introduce a specific target type for the spapr-vty > > > device used by pSeries guests, which means isa-serial will no longer > > > show up to confuse users. > > > > > > We make sure migration works in both directions by interpreting the > > > isa-serial target type, or the lack of target type, appropriately > > > when parsing the guest XML, and skipping the newly-introduced type > > > when formatting if for migration. We also verify that spapr-vty is > > > not used for non-pSeries guests and add a bunch of test cases. > > > > > > This commit is best viewed with 'git diff -w'. > > > > It would be probably good idea to split it into two patches, one > > that changes all the if-else conditions to switch and second where > > the actual new code is introduced. > > I'm not changing any if-else to switch, I'm just folding a special > case that was outside of the existing all-encompassing switch back > into it and getting rid of the if-else that only existed to support > that special case at the same time. Right, you are folding the if-else into an existing switch. > I can't really think of a good way to split the change right now, > plus I think what's happening is pretty clear if you use '-w'. Well, it wasn't that clear to me, obviously, even if I used '-w'. Now I can see that the folding isn't possible without introducing the new spapr type. So ignore this comment :). > > > @@ -3585,6 +3585,7 @@ > > > <value>isa-serial</value> > > > <value>usb-serial</value> > > > <value>pci-serial</value> > > > + <value>spapr-vty</value> > > > > This name doesn't feel right. Previous names are based on the BUS that > > the serial device is connected with "-serial" appended to the BUS name. > > Since sPAPR is specification that defines a set of para-virtualized > > devices, there is no actual BUS, but I think that in this case we can > > consider spapr as a BUS name and use "spapr-serial". It would be better > > than just copying the device name from QEMU. > > I'm not sure that's how it went: it looks to me like all the > *-serial names have been adopted merely because that's what the > QEMU folks had chosen for the corresponding device. I believe that it was like you are describing :). > We could abstract this further but that would mean adding another > layer of translation, since at the moment we're basically passing > it through to QEMU and that would no longer fly. That's not an issue that we would not pass it directly to QEMU, and sometimes it's even better to abstract it a little bit. > I'm not opposed to doing that on principle, but both for pSeries > and for other non-x86 architecture, as you noted in response to > other commits, obvious candidates for the name don't necessarily > exist in the same way they do for ISA, USB and PCI. So I wonder > whether it would be worth adding more machinery when the proposed > names, while maybe not pretty, do not cause any ambiguity and can > be matched to the corresponding platform just as easily. I would like to make it right and not to use names that will backfire later. Pavel
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
