[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

  Ivan Mishonov wrote:

> 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.

My general view on upgrades is that it's best-effort based at this point. In this
specific case, I hope it won't affect many users as we can't run guests
that a picky about LPC, and other guests should be fine with the changed
LPC slot... Though I'll test if my existing guests will handle that

I guess when bhyve supports migrations (there are some WIP patches for
that), we'll have less flexibility in moving devices/slots around...

> 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

Good, so let's stick to the simpler approach for now, and we can always
make things configurable if there's a demand for that.

> > Regards,
> > Daniel

Roman Bogorodskiy

Attachment: signature.asc
Description: PGP signature

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]