On Sat, Aug 01, 2020 at 10:16:57AM +0400, Roman Bogorodskiy wrote:
> Support modeling of the 'isa' controller for bhyve. User can manually
> define any PCI slot for the 'isa' controller, including PCI slot 1,
> but other devices are not allowed to use this address.
>
> When domain configuration requires the 'isa' controller to be present,
> automatically add it on domain post-parse stage.
>
> Now, as this controller is always available when needed, it's not
> necessary to implicitly add it to the bhyve command line, so remove
> bhyveBuildLPCArgStr().
>
> Also, make bhyveDomainDefNeedsISAController() static as it's no longer
> used outside of bhyve_domain.c.
>
> As more than one ISA controller is not supported by bhyve,
> and multiple controllers with the same index are forbidden,
> so forbid ISA controllers with non-zero index for bhyve.
>
> Signed-off-by: Roman Bogorodskiy <bogorods...@gmail.com>
> ---
> src/bhyve/bhyve_command.c | 27 +++++++-------
> src/bhyve/bhyve_device.c | 23 +++++++++---
> src/bhyve/bhyve_domain.c | 25 +++++++++++--
> src/bhyve/bhyve_domain.h | 2 --
> ...ml2argv-addr-isa-controller-on-slot-1.args | 10 ++++++
> ...2argv-addr-isa-controller-on-slot-1.ldargs | 3 ++
> ...xml2argv-addr-isa-controller-on-slot-1.xml | 26 ++++++++++++++
> ...l2argv-addr-isa-controller-on-slot-31.args | 10 ++++++
> ...argv-addr-isa-controller-on-slot-31.ldargs | 3 ++
> ...ml2argv-addr-isa-controller-on-slot-31.xml | 26 ++++++++++++++
> ...argv-addr-non-isa-controller-on-slot-1.xml | 23 ++++++++++++
> .../bhyvexml2argv-console.args | 2 +-
> .../bhyvexml2argv-isa-controller.args | 10 ++++++
> .../bhyvexml2argv-isa-controller.ldargs | 3 ++
> .../bhyvexml2argv-isa-controller.xml | 24 +++++++++++++
> ...bhyvexml2argv-isa-multiple-controllers.xml | 25 +++++++++++++
> .../bhyvexml2argv-serial-grub-nocons.args | 2 +-
> .../bhyvexml2argv-serial-grub.args | 2 +-
> .../bhyvexml2argv-serial.args | 2 +-
> .../bhyvexml2argvdata/bhyvexml2argv-uefi.args | 4 +--
> .../bhyvexml2argv-vnc-autoport.args | 4 +--
> .../bhyvexml2argv-vnc-vgaconf-io.args | 4 +--
> .../bhyvexml2argv-vnc-vgaconf-off.args | 4 +--
> .../bhyvexml2argv-vnc-vgaconf-on.args | 4 +--
> .../bhyvexml2argvdata/bhyvexml2argv-vnc.args | 4 +--
> tests/bhyvexml2argvtest.c | 5 +++
> ...l2xmlout-addr-isa-controller-on-slot-1.xml | 36 +++++++++++++++++++
> ...2xmlout-addr-isa-controller-on-slot-31.xml | 36 +++++++++++++++++++
> .../bhyvexml2xmlout-console.xml | 3 ++
> .../bhyvexml2xmlout-isa-controller.xml | 36 +++++++++++++++++++
> .../bhyvexml2xmlout-serial-grub-nocons.xml | 3 ++
> .../bhyvexml2xmlout-serial-grub.xml | 3 ++
> .../bhyvexml2xmlout-serial.xml | 3 ++
> .../bhyvexml2xmlout-vnc-autoport.xml | 3 ++
> .../bhyvexml2xmlout-vnc-vgaconf-io.xml | 3 ++
> .../bhyvexml2xmlout-vnc-vgaconf-off.xml | 3 ++
> .../bhyvexml2xmlout-vnc-vgaconf-on.xml | 3 ++
> .../bhyvexml2xmlout-vnc.xml | 3 ++
> tests/bhyvexml2xmltest.c | 3 ++
> 39 files changed, 378 insertions(+), 37 deletions(-)
> create mode 100644
> tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args
> create mode 100644
> tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs
> create mode 100644
> tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml
> create mode 100644
> tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args
> create mode 100644
> tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs
> create mode 100644
> tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml
> create mode 100644
> tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml
> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args
> create mode 100644
> tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs
> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml
> create mode 100644
> tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml
> create mode 100644
> tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml
> create mode 100644
> tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml
> create mode 100644
> tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml
>
> diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c
> index fc52280361..52a055f205 100644
> --- a/src/bhyve/bhyve_device.c
> +++ b/src/bhyve/bhyve_device.c
> @@ -46,10 +46,16 @@ bhyveCollectPCIAddress(virDomainDefPtr def G_GNUC_UNUSED,
> if (addr->slot == 0) {
> return 0;
> } else if (addr->slot == 1) {
> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("PCI bus 0 slot 1 is reserved for the implicit "
> - "LPC PCI-ISA bridge"));
> - return -1;
> + if (!(device->type == VIR_DOMAIN_DEVICE_CONTROLLER &&
> + device->data.controller->type ==
> VIR_DOMAIN_CONTROLLER_TYPE_ISA)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("PCI bus 0 slot 1 is reserved for the
> implicit "
> + "LPC PCI-ISA bridge"));
> + return -1;
> + } else {
> + /* We reserve slot 1 for LPC in bhyveAssignDevicePCISlots(),
> so exit early */
> + return 0;
> + }
I forgot to respond to your followup comments on v4
https://www.redhat.com/archives/libvir-list/2020-July/msg01761.html
> >
> > IIUC, this series makes it possible to put the TPC in a different
> > slot, so does it still make sense to forbid use of slot 1 as a
> > hardcoded rule ?
>
> IIRC, the idea behind that is to give some time window for users to
> allow moving guests from the new version to the old one. If we allow to
> use slot 1, it won't be possible to move the guest to the old libvirt as
> it will complain slot 1 should be used only for LPC.
If the user has decided to move their LPC to a slot != 1, then it is
already impossible to migrate the guest to an old libvirt.
If the user wants to explicitly specify another device in slot 1, then
we should not prevent that.
We just need to make sure that if no LPC is in the XML, and no other
device explicitly has slot 1, then make sure to auto-assign LPC in slot 1
not some other device.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
