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

[Ján Tomko]

On Sun, Mar 03, 2019 at 06:00:32PM +0400, Roman Bogorodskiy wrote:
>  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 at gmail.com>
>> >---
>> > 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?

I meant the validation function common for all drivers, because I
thought a driver supporting more than one ISA controller is unlikely.

But since bhyve is the only one modeling it, bhyve's validate callback
works too. Hopefully we won't have to model it in other drivers anyway.

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190315/4205576e/attachment-0001.sig>