Re: [libvirt] [PATCH 0/1] bhyve: Make LPC slot number configurable

On 08/13/2018 11:00 AM, Daniel P. Berrangé wrote: > On Sun, Aug 12, 2018 at 08:22:08PM +0400, Roman Bogorodskiy wrote: >> Ivan Mishonov wrote: >> >>> Yes, that makes sense. I'll try to find some time next week to redo my >>> code and send another patch. Since my time for working on libvirt is >>> very limited can you confirm that the LPC configuration should look like >>> this: >>> >>>    <controller type="isa-bridge" index="0"> >>>        <address type="pci" domain="0" bus="0" slot="NNN" function="0"/> >>>    </controller> >> This looks reasonable to me. However, it adds some corner cases we need to >> handle: >> >> 1. I'm wondering if we should still default to 31 if this entry is not specified? >> We can generate this entry when post-processing XML, but I'm not sure >> what's the best way to handle upgrades for the existing domains... > It depends if the BHyve driver is at a point where you consider stable > upgrades important or not. It could be valid for you to just change the > default to 31 if you think its better and upgrade stability is not > required yet.

I decided to check what vm-bhyve does. https://github.com/churchers/vm-bhyve/blob/master/lib/vm-run#L367. They seem to always place LPC at slot 31 so I guess it's safe to move it > >> 2. According to bhyve(8) manual page, lpc is only supported on bus 0, so >> need to add 'isa-bridge' specific validation to check that. > If its only supported in 1 address, then arguably you don't need to add > this at all - just fix the historically mistaken use of 31 in the code > and leave it out of XML. Yes, we might not need that option at all

> Regards, > Daniel