Re: [libvirt] [PATCH v2 3/6] bhyve: support 'isa' controller for LPC

Ján Tomko wrote: > On Sun, Feb 17, 2019 at 05:04:02PM +0400, Roman Bogorodskiy wrote: > >Support modeling of the 'isa' controller for bhyve. When controller is > >not present in the domain XML, but domain requires it to be there (e.g. > >because bootrom is used), implicitly add addition of this controller to > >the command line arguments, and bind it to PCI slot 1. > > > >PCI slot 1 is always reserved still. 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. > > > > I thought one of the points was to allow the use of slot 1 for users > who request it. But this also works - the restriction can be relaxed > later if needed. The main goal for now is to allow to choose PCI slot for the LPC controller. My plan is to relax it later indeed. > >Signed-off-by: Roman Bogorodskiy <bogorodskiy(a)gmail.com&gt; > >--- > > src/bhyve/bhyve_command.c | 18 +++++++++- > > src/bhyve/bhyve_device.c | 25 ++++++++++--- > > ...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-isa-controller.args | 10 ++++++ > > .../bhyvexml2argv-isa-controller.ldargs | 3 ++ > > .../bhyvexml2argv-isa-controller.xml | 24 +++++++++++++ > > ...bhyvexml2argv-isa-multiple-controllers.xml | 25 +++++++++++++ > > tests/bhyvexml2argvtest.c | 5 +++ > > ...l2xmlout-addr-isa-controller-on-slot-1.xml | 36 +++++++++++++++++++ > > ...2xmlout-addr-isa-controller-on-slot-31.xml | 36 +++++++++++++++++++ > > .../bhyvexml2xmlout-isa-controller.xml | 36 +++++++++++++++++++ > > tests/bhyvexml2xmltest.c | 3 ++ > > 18 files changed, 317 insertions(+), 5 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_command.c b/src/bhyve/bhyve_command.c > >index f49dc77118..2c90265a93 100644 > >--- a/src/bhyve/bhyve_command.c > >+++ b/src/bhyve/bhyve_command.c > >@@ -461,6 +461,7 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, > > */ > > size_t i; > > int nusbcontrollers = 0; > >+ int nisacontrollers = 0; > > unsigned int nvcpus = virDomainDefGetVcpus(def); > > > > virCommandPtr cmd = virCommandNew(BHYVE); > >@@ -581,6 +582,21 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, > > if (bhyveBuildUSBControllerArgStr(def, controller, cmd) < 0) > > goto error; > > break; > >+ case VIR_DOMAIN_CONTROLLER_TYPE_ISA: > >+ if (controller->model != VIR_DOMAIN_CONTROLLER_MODEL_ISA_BRIDGE) { > >+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > >+ "%s", _("unsupported ISA controller model: only ISA bridge supported")); > >+ goto error; > >+ } > >+ if (++nisacontrollers > 1) { > >+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > >+ "%s", _("only single ISA controller is supported")); > >+ goto error; > >+ } > > This error message can be reported much sooner - since we already forbid > multiple controllers with the same index, all you need to do is forbid > non-zero index virDomainControllerDefValidate Do you mean implementing this check in virDomainDefParserConfig.deviceValidateCallback of the bhyve driver?