[libvirt] [PATCH v2 0/6] bhyve: model PCI ISA bridge
Changes from v1: * Changed LPC controller type from PCI to ISA * Split bhyve changes into smaller chunks Also, I decided to keep PCI slot 1 reserved for LPC only, even if users choose other slot for LPC. If things work fine, it could be easily made available for other devices in future. Roman Bogorodskiy (6): conf: add 'isa' controller type bhyve: add bhyveDomainDefNeedsISAController helper bhyve: support 'isa' controller for LPC bhyve: automatically add 'isa' controller docs: bhyve: document isa-bridge addressing news: document bhyve isa-bridge changes docs/drvbhyve.html.in | 22 ++++++++++++ docs/news.xml | 10 ++++++ docs/schemas/domaincommon.rng | 13 +++++++ src/bhyve/bhyve_command.c | 30 ++++++++-------- src/bhyve/bhyve_device.c | 25 ++++++++++--- src/bhyve/bhyve_domain.c | 15 ++++++++ src/conf/domain_conf.c | 10 ++++++ src/conf/domain_conf.h | 9 +++++ src/qemu/qemu_command.c | 1 + src/qemu/qemu_domain.c | 2 ++ src/qemu/qemu_domain_address.c | 1 + src/vbox/vbox_common.c | 1 + ...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 ++ 47 files changed, 443 insertions(+), 34 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 -- 2.20.1
Introduce 'isa' controller type. The only supported model now is 'isa-bridge'. In domain XML it looks this way: ... <controller type='isa' index='1' model='isa-bridge'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> </controller> ... Currently, this is needed for the bhyve driver to allow choosing a specific PCI address for that. In bhyve, this controller is used to attach serial ports and a boot ROM. Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- docs/schemas/domaincommon.rng | 13 +++++++++++++ src/conf/domain_conf.c | 10 ++++++++++ src/conf/domain_conf.h | 9 +++++++++ src/qemu/qemu_command.c | 1 + src/qemu/qemu_domain.c | 2 ++ src/qemu/qemu_domain_address.c | 1 + src/vbox/vbox_common.c | 1 + 7 files changed, 37 insertions(+) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index ba80440c72..2bf67eaa5a 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2201,6 +2201,19 @@ </attribute> </optional> </group> + <!-- isa has an optional attribute "model" --> + <group> + <attribute name="type"> + <value>isa</value> + </attribute> + <optional> + <attribute name="model"> + <choice> + <value>isa-bridge</value> + </choice> + </attribute> + </optional> + </group> <!-- pci has an optional attribute "model" --> <group> <attribute name="type"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cd69323f28..4d5fbed6f4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -341,6 +341,7 @@ VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST, "ccid", "usb", "pci", + "isa", ); VIR_ENUM_IMPL(virDomainControllerModelPCI, VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST, @@ -404,6 +405,10 @@ VIR_ENUM_IMPL(virDomainControllerModelIDE, VIR_DOMAIN_CONTROLLER_MODEL_IDE_LAST, "ich6", ); +VIR_ENUM_IMPL(virDomainControllerModelISA, VIR_DOMAIN_CONTROLLER_MODEL_ISA_LAST, + "isa-bridge", +); + VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST, "mount", "block", @@ -2041,6 +2046,7 @@ virDomainControllerDefNew(virDomainControllerType type) case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: case VIR_DOMAIN_CONTROLLER_TYPE_SATA: case VIR_DOMAIN_CONTROLLER_TYPE_CCID: + case VIR_DOMAIN_CONTROLLER_TYPE_ISA: case VIR_DOMAIN_CONTROLLER_TYPE_LAST: break; } @@ -10445,6 +10451,8 @@ virDomainControllerModelTypeFromString(const virDomainControllerDef *def, return virDomainControllerModelPCITypeFromString(model); else if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE) return virDomainControllerModelIDETypeFromString(model); + else if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) + return virDomainControllerModelISATypeFromString(model); return -1; } @@ -10462,6 +10470,8 @@ virDomainControllerModelTypeToString(virDomainControllerDefPtr def, return virDomainControllerModelPCITypeToString(model); else if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE) return virDomainControllerModelIDETypeToString(model); + else if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) + return virDomainControllerModelISATypeToString(model); return NULL; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2bc3f879f7..3c595aab62 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -687,6 +687,7 @@ typedef enum { VIR_DOMAIN_CONTROLLER_TYPE_CCID, VIR_DOMAIN_CONTROLLER_TYPE_USB, VIR_DOMAIN_CONTROLLER_TYPE_PCI, + VIR_DOMAIN_CONTROLLER_TYPE_ISA, VIR_DOMAIN_CONTROLLER_TYPE_LAST } virDomainControllerType; @@ -767,6 +768,13 @@ typedef enum { VIR_DOMAIN_CONTROLLER_MODEL_IDE_LAST } virDomainControllerModelIDE; +typedef enum { + VIR_DOMAIN_CONTROLLER_MODEL_ISA_DEFAULT = -1, + VIR_DOMAIN_CONTROLLER_MODEL_ISA_BRIDGE, + + VIR_DOMAIN_CONTROLLER_MODEL_ISA_LAST +} virDomainControllerModelISA; + # define IS_USB2_CONTROLLER(ctrl) \ (((ctrl)->type == VIR_DOMAIN_CONTROLLER_TYPE_USB) && \ ((ctrl)->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1 || \ @@ -3423,6 +3431,7 @@ VIR_ENUM_DECL(virDomainControllerPCIModelName); VIR_ENUM_DECL(virDomainControllerModelSCSI); VIR_ENUM_DECL(virDomainControllerModelUSB); VIR_ENUM_DECL(virDomainControllerModelIDE); +VIR_ENUM_DECL(virDomainControllerModelISA); VIR_ENUM_DECL(virDomainFS); VIR_ENUM_DECL(virDomainFSDriver); VIR_ENUM_DECL(virDomainFSAccessMode); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f511193fca..7203a2762c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3024,6 +3024,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, case VIR_DOMAIN_CONTROLLER_TYPE_IDE: case VIR_DOMAIN_CONTROLLER_TYPE_FDC: + case VIR_DOMAIN_CONTROLLER_TYPE_ISA: case VIR_DOMAIN_CONTROLLER_TYPE_LAST: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported controller type: %s"), diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ac01e861f7..306f957eb7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5851,6 +5851,7 @@ qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller, case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: case VIR_DOMAIN_CONTROLLER_TYPE_CCID: case VIR_DOMAIN_CONTROLLER_TYPE_USB: + case VIR_DOMAIN_CONTROLLER_TYPE_ISA: case VIR_DOMAIN_CONTROLLER_TYPE_LAST: break; } @@ -6469,6 +6470,7 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont, case VIR_DOMAIN_CONTROLLER_TYPE_CCID: case VIR_DOMAIN_CONTROLLER_TYPE_IDE: case VIR_DOMAIN_CONTROLLER_TYPE_FDC: + case VIR_DOMAIN_CONTROLLER_TYPE_ISA: case VIR_DOMAIN_CONTROLLER_TYPE_LAST: break; } diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 32fdd59566..3e072ee8fd 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -669,6 +669,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, case VIR_DOMAIN_CONTROLLER_TYPE_FDC: case VIR_DOMAIN_CONTROLLER_TYPE_CCID: + case VIR_DOMAIN_CONTROLLER_TYPE_ISA: case VIR_DOMAIN_CONTROLLER_TYPE_LAST: /* should be 0 */ return pciFlags; diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 664650f217..6b75a127f8 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -377,6 +377,7 @@ vboxSetStorageController(virDomainControllerDefPtr controller, case VIR_DOMAIN_CONTROLLER_TYPE_CCID: case VIR_DOMAIN_CONTROLLER_TYPE_USB: case VIR_DOMAIN_CONTROLLER_TYPE_PCI: + case VIR_DOMAIN_CONTROLLER_TYPE_ISA: case VIR_DOMAIN_CONTROLLER_TYPE_LAST: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("The vbox driver does not support %s controller type"), -- 2.20.1
On Sun, Feb 17, 2019 at 05:04:00PM +0400, Roman Bogorodskiy wrote:
Introduce 'isa' controller type. The only supported model now is 'isa-bridge'. In domain XML it looks this way:
... <controller type='isa' index='1' model='isa-bridge'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> </controller> ...
Currently, this is needed for the bhyve driver to allow choosing a specific PCI address for that. In bhyve, this controller is used to attach serial ports and a boot ROM.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- docs/schemas/domaincommon.rng | 13 +++++++++++++ src/conf/domain_conf.c | 10 ++++++++++ src/conf/domain_conf.h | 9 +++++++++
This esentially implements the parser and formatter for the controller, so the XML to XML test addition should be a part of it.
src/qemu/qemu_command.c | 1 + src/qemu/qemu_domain.c | 2 ++ src/qemu/qemu_domain_address.c | 1 + src/vbox/vbox_common.c | 1 + 7 files changed, 37 insertions(+)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index ba80440c72..2bf67eaa5a 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2201,6 +2201,19 @@ </attribute> </optional> </group> + <!-- isa has an optional attribute "model" -->
I sincerely hope nobody will ever need to specify the model for an ISA controller.
+ <group> + <attribute name="type"> + <value>isa</value> + </attribute>
+ <optional> + <attribute name="model"> + <choice> + <value>isa-bridge</value> + </choice> + </attribute> + </optional>
This would not be needed then.
+ </group> <!-- pci has an optional attribute "model" --> <group> <attribute name="type">
Ján Tomko wrote:
On Sun, Feb 17, 2019 at 05:04:00PM +0400, Roman Bogorodskiy wrote:
Introduce 'isa' controller type. The only supported model now is 'isa-bridge'. In domain XML it looks this way:
... <controller type='isa' index='1' model='isa-bridge'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> </controller> ...
Currently, this is needed for the bhyve driver to allow choosing a specific PCI address for that. In bhyve, this controller is used to attach serial ports and a boot ROM.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- docs/schemas/domaincommon.rng | 13 +++++++++++++ src/conf/domain_conf.c | 10 ++++++++++ src/conf/domain_conf.h | 9 +++++++++
This esentially implements the parser and formatter for the controller, so the XML to XML test addition should be a part of it.
In this case I'm inclined to squash three patches together: this one is fairly small (will be even smaller with the model attr dropped), and the automatic addition of the isa controller is also fairly small.
src/qemu/qemu_command.c | 1 + src/qemu/qemu_domain.c | 2 ++ src/qemu/qemu_domain_address.c | 1 + src/vbox/vbox_common.c | 1 + 7 files changed, 37 insertions(+)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index ba80440c72..2bf67eaa5a 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2201,6 +2201,19 @@ </attribute> </optional> </group> + <!-- isa has an optional attribute "model" -->
I sincerely hope nobody will ever need to specify the model for an ISA controller.
+ <group> + <attribute name="type"> + <value>isa</value> + </attribute>
+ <optional> + <attribute name="model"> + <choice> + <value>isa-bridge</value> + </choice> + </attribute> + </optional>
This would not be needed then.
+ </group> <!-- pci has an optional attribute "model" --> <group> <attribute name="type">
Roman Bogorodskiy
Add a bhyveDomainDefNeedsISAController() helper function which by domain configuration determines whether LPC controller is required or not. Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- src/bhyve/bhyve_command.c | 5 +---- src/bhyve/bhyve_domain.c | 10 ++++++++++ src/bhyve/bhyve_domain.h | 2 ++ 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 1f215dac08..f49dc77118 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -460,7 +460,6 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, * vm0 */ size_t i; - bool add_lpc = false; int nusbcontrollers = 0; unsigned int nvcpus = virDomainDefGetVcpus(def); @@ -549,7 +548,6 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_LPC_BOOTROM)) { virCommandAddArg(cmd, "-l"); virCommandAddArgFormat(cmd, "bootrom,%s", def->os.loader->path); - add_lpc = true; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Installed bhyve binary does not support " @@ -613,7 +611,6 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, if (bhyveBuildGraphicsArgStr(def, def->graphics[0], def->videos[0], conn, cmd, dryRun) < 0) goto error; - add_lpc = true; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Multiple graphics devices are not supported")); @@ -621,7 +618,7 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, } } - if (add_lpc || def->nserials) + if (bhyveDomainDefNeedsISAController(def)) bhyveBuildLPCArgStr(def, cmd); if (bhyveBuildConsoleArgStr(def, cmd) < 0) diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c index 79cf103d28..e476ef7e7d 100644 --- a/src/bhyve/bhyve_domain.c +++ b/src/bhyve/bhyve_domain.c @@ -61,6 +61,16 @@ virDomainXMLPrivateDataCallbacks virBhyveDriverPrivateDataCallbacks = { .free = bhyveDomainObjPrivateFree, }; +bool +bhyveDomainDefNeedsISAController(virDomainDefPtr def) +{ + if ((def->os.bootloader == NULL && def->os.loader) || + (def->nconsoles || def->nserials) || (def->ngraphics && def->nvideos)) + return true; + else + return false; +} + static int bhyveDomainDefPostParse(virDomainDefPtr def, virCapsPtr caps ATTRIBUTE_UNUSED, diff --git a/src/bhyve/bhyve_domain.h b/src/bhyve/bhyve_domain.h index 5f94038e89..03a2b369d9 100644 --- a/src/bhyve/bhyve_domain.h +++ b/src/bhyve/bhyve_domain.h @@ -41,4 +41,6 @@ extern virDomainXMLPrivateDataCallbacks virBhyveDriverPrivateDataCallbacks; extern virDomainDefParserConfig virBhyveDriverDomainDefParserConfig; extern virDomainXMLNamespace virBhyveDriverDomainXMLNamespace; +bool bhyveDomainDefNeedsISAController(virDomainDefPtr def); + #endif /* LIBVIRT_BHYVE_DOMAIN_H */ -- 2.20.1
On Sun, Feb 17, 2019 at 05:04:01PM +0400, Roman Bogorodskiy wrote:
Add a bhyveDomainDefNeedsISAController() helper function which by domain configuration determines whether LPC controller is required or not.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- src/bhyve/bhyve_command.c | 5 +---- src/bhyve/bhyve_domain.c | 10 ++++++++++ src/bhyve/bhyve_domain.h | 2 ++ 3 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 1f215dac08..f49dc77118 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -460,7 +460,6 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, * vm0 */ size_t i; - bool add_lpc = false; int nusbcontrollers = 0; unsigned int nvcpus = virDomainDefGetVcpus(def);
@@ -549,7 +548,6 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_LPC_BOOTROM)) { virCommandAddArg(cmd, "-l"); virCommandAddArgFormat(cmd, "bootrom,%s", def->os.loader->path); - add_lpc = true; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Installed bhyve binary does not support " @@ -613,7 +611,6 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, if (bhyveBuildGraphicsArgStr(def, def->graphics[0], def->videos[0], conn, cmd, dryRun) < 0) goto error; - add_lpc = true; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Multiple graphics devices are not supported")); @@ -621,7 +618,7 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, } }
- if (add_lpc || def->nserials) + if (bhyveDomainDefNeedsISAController(def)) bhyveBuildLPCArgStr(def, cmd);
if (bhyveBuildConsoleArgStr(def, cmd) < 0) diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c index 79cf103d28..e476ef7e7d 100644 --- a/src/bhyve/bhyve_domain.c +++ b/src/bhyve/bhyve_domain.c @@ -61,6 +61,16 @@ virDomainXMLPrivateDataCallbacks virBhyveDriverPrivateDataCallbacks = { .free = bhyveDomainObjPrivateFree, };
+bool +bhyveDomainDefNeedsISAController(virDomainDefPtr def) +{ + if ((def->os.bootloader == NULL && def->os.loader) || + (def->nconsoles || def->nserials) || (def->ngraphics && def->nvideos)) + return true;
Where did nconsoles come from? Also, since it's a separate function, you can use return more: if (def->os.bootloader == NULL && def->os.loader) return true; if (def->nserials) return true;
+ else + return false; +} + static int bhyveDomainDefPostParse(virDomainDefPtr def, virCapsPtr caps ATTRIBUTE_UNUSED,
With nconsoles removed: Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Ján Tomko wrote:
On Sun, Feb 17, 2019 at 05:04:01PM +0400, Roman Bogorodskiy wrote:
Add a bhyveDomainDefNeedsISAController() helper function which by domain configuration determines whether LPC controller is required or not.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- src/bhyve/bhyve_command.c | 5 +---- src/bhyve/bhyve_domain.c | 10 ++++++++++ src/bhyve/bhyve_domain.h | 2 ++ 3 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 1f215dac08..f49dc77118 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -460,7 +460,6 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, * vm0 */ size_t i; - bool add_lpc = false; int nusbcontrollers = 0; unsigned int nvcpus = virDomainDefGetVcpus(def);
@@ -549,7 +548,6 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_LPC_BOOTROM)) { virCommandAddArg(cmd, "-l"); virCommandAddArgFormat(cmd, "bootrom,%s", def->os.loader->path); - add_lpc = true; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Installed bhyve binary does not support " @@ -613,7 +611,6 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, if (bhyveBuildGraphicsArgStr(def, def->graphics[0], def->videos[0], conn, cmd, dryRun) < 0) goto error; - add_lpc = true; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Multiple graphics devices are not supported")); @@ -621,7 +618,7 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, } }
- if (add_lpc || def->nserials) + if (bhyveDomainDefNeedsISAController(def)) bhyveBuildLPCArgStr(def, cmd);
if (bhyveBuildConsoleArgStr(def, cmd) < 0) diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c index 79cf103d28..e476ef7e7d 100644 --- a/src/bhyve/bhyve_domain.c +++ b/src/bhyve/bhyve_domain.c @@ -61,6 +61,16 @@ virDomainXMLPrivateDataCallbacks virBhyveDriverPrivateDataCallbacks = { .free = bhyveDomainObjPrivateFree, };
+bool +bhyveDomainDefNeedsISAController(virDomainDefPtr def) +{ + if ((def->os.bootloader == NULL && def->os.loader) || + (def->nconsoles || def->nserials) || (def->ngraphics && def->nvideos)) + return true;
Where did nconsoles come from?
Also, since it's a separate function, you can use return more: if (def->os.bootloader == NULL && def->os.loader) return true; if (def->nserials) return true;
+ else + return false; +} + static int bhyveDomainDefPostParse(virDomainDefPtr def, virCapsPtr caps ATTRIBUTE_UNUSED,
With nconsoles removed: Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
Thanks, merged this one with suggestions applied, the rest will go into v3. Roman Bogorodskiy
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. Signed-off-by: Roman Bogorodskiy <bogorodskiy@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; + } + virCommandAddArg(cmd, "-s"); + virCommandAddArgFormat(cmd, "%d:0,lpc", + controller->info.addr.pci.slot); + break; } } for (i = 0; i < def->nnets; i++) { @@ -618,7 +634,7 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, } } - if (bhyveDomainDefNeedsISAController(def)) + if (nisacontrollers == 0 && bhyveDomainDefNeedsISAController(def)) bhyveBuildLPCArgStr(def, cmd); if (bhyveBuildConsoleArgStr(def, cmd) < 0) diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c index 201044d9e6..42093afb7b 100644 --- a/src/bhyve/bhyve_device.c +++ b/src/bhyve/bhyve_device.c @@ -47,10 +47,17 @@ bhyveCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_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) && + (device->data.controller->model == VIR_DOMAIN_CONTROLLER_MODEL_ISA_BRIDGE))) { + 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; + } } } @@ -103,6 +110,16 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def, goto error; } + for (i = 0; i < def->ncontrollers; i++) { + if ((def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) && + (def->controllers[i]->model == VIR_DOMAIN_CONTROLLER_MODEL_ISA_BRIDGE) && + virDeviceInfoPCIAddressIsWanted(&def->controllers[i]->info)) { + def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + def->controllers[i]->info.addr.pci = lpc_addr; + break; + } + } + for (i = 0; i < def->ncontrollers; i++) { if ((def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) || (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) || diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args new file mode 100644 index 0000000000..910d1bbcfa --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args @@ -0,0 +1,10 @@ +/usr/sbin/bhyve \ +-c 1 \ +-m 214 \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ +-s 1:0,lpc \ +-s 2:0,ahci,hd:/tmp/freebsd.img \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs new file mode 100644 index 0000000000..32538b558e --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs @@ -0,0 +1,3 @@ +/usr/sbin/bhyveload \ +-m 214 \ +-d /tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml new file mode 100644 index 0000000000..796903a9fa --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml @@ -0,0 +1,26 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + </os> + <devices> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args new file mode 100644 index 0000000000..ee833eb460 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args @@ -0,0 +1,10 @@ +/usr/sbin/bhyve \ +-c 1 \ +-m 214 \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ +-s 31:0,lpc \ +-s 2:0,ahci,hd:/tmp/freebsd.img \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs new file mode 100644 index 0000000000..32538b558e --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs @@ -0,0 +1,3 @@ +/usr/sbin/bhyveload \ +-m 214 \ +-d /tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml new file mode 100644 index 0000000000..65d7480db6 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml @@ -0,0 +1,26 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + </os> + <devices> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x0'/> + </controller> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml new file mode 100644 index 0000000000..88ad9aebb7 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml @@ -0,0 +1,23 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + </os> + <devices> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args new file mode 100644 index 0000000000..910d1bbcfa --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args @@ -0,0 +1,10 @@ +/usr/sbin/bhyve \ +-c 1 \ +-m 214 \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ +-s 1:0,lpc \ +-s 2:0,ahci,hd:/tmp/freebsd.img \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs new file mode 100644 index 0000000000..32538b558e --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs @@ -0,0 +1,3 @@ +/usr/sbin/bhyveload \ +-m 214 \ +-d /tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml new file mode 100644 index 0000000000..71265ea32c --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml @@ -0,0 +1,24 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + </os> + <devices> + <controller type='isa' index='1' model='isa-bridge'/> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml new file mode 100644 index 0000000000..25622ce78a --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml @@ -0,0 +1,25 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + </os> + <devices> + <controller type='isa' index='1' model='isa-bridge'/> + <controller type='isa' index='2' model='isa-bridge'/> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c index 4a7f65a8e2..d120bd43ca 100644 --- a/tests/bhyvexml2argvtest.c +++ b/tests/bhyvexml2argvtest.c @@ -211,6 +211,8 @@ mymain(void) DO_TEST("cputopology"); DO_TEST_FAILURE("cputopology-nvcpu-mismatch"); DO_TEST("commandline"); + DO_TEST("isa-controller"); + DO_TEST_FAILURE("isa-multiple-controllers"); /* Address allocation tests */ DO_TEST("addr-single-sata-disk"); @@ -218,6 +220,9 @@ mymain(void) DO_TEST("addr-more-than-32-sata-disks"); DO_TEST("addr-single-virtio-disk"); DO_TEST("addr-multiple-virtio-disks"); + DO_TEST("addr-isa-controller-on-slot-1"); + DO_TEST("addr-isa-controller-on-slot-31"); + DO_TEST_FAILURE("addr-non-isa-controller-on-slot-1"); /* The same without 32 devs per controller support */ driver.bhyvecaps ^= BHYVE_CAP_AHCI32SLOT; diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml new file mode 100644 index 0000000000..db84185fe6 --- /dev/null +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml @@ -0,0 +1,36 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <disk type='file' device='disk'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <source bridge='virbr0'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml new file mode 100644 index 0000000000..6ab5e6ad1a --- /dev/null +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml @@ -0,0 +1,36 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <disk type='file' device='disk'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x0'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <source bridge='virbr0'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml new file mode 100644 index 0000000000..db84185fe6 --- /dev/null +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml @@ -0,0 +1,36 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <disk type='file' device='disk'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <source bridge='virbr0'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2xmltest.c b/tests/bhyvexml2xmltest.c index ed421b8839..4b7f59ea29 100644 --- a/tests/bhyvexml2xmltest.c +++ b/tests/bhyvexml2xmltest.c @@ -111,6 +111,7 @@ mymain(void) DO_TEST_DIFFERENT("vnc-vgaconf-io"); DO_TEST_DIFFERENT("vnc-autoport"); DO_TEST_DIFFERENT("commandline"); + DO_TEST_DIFFERENT("isa-controller"); /* Address allocation tests */ DO_TEST_DIFFERENT("addr-single-sata-disk"); @@ -118,6 +119,8 @@ mymain(void) DO_TEST_DIFFERENT("addr-more-than-32-sata-disks"); DO_TEST_DIFFERENT("addr-single-virtio-disk"); DO_TEST_DIFFERENT("addr-multiple-virtio-disks"); + DO_TEST_DIFFERENT("addr-isa-controller-on-slot-1"); + DO_TEST_DIFFERENT("addr-isa-controller-on-slot-31"); /* The same without 32 devs per controller support */ driver.bhyvecaps ^= BHYVE_CAP_AHCI32SLOT; -- 2.20.1
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.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@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 Also, this whole switch is indented by 8 instead of 4 spaces.
+ virCommandAddArg(cmd, "-s"); + virCommandAddArgFormat(cmd, "%d:0,lpc", + controller->info.addr.pci.slot); + break; } } for (i = 0; i < def->nnets; i++) { @@ -618,7 +634,7 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, } }
- if (bhyveDomainDefNeedsISAController(def)) + if (nisacontrollers == 0 && bhyveDomainDefNeedsISAController(def)) bhyveBuildLPCArgStr(def, cmd);
if (bhyveBuildConsoleArgStr(def, cmd) < 0) diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c index 201044d9e6..42093afb7b 100644 --- a/src/bhyve/bhyve_device.c +++ b/src/bhyve/bhyve_device.c @@ -47,10 +47,17 @@ bhyveCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_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) &&
== has higher precedence than &&, the inner parentheses can be dropped
+ (device->data.controller->model == VIR_DOMAIN_CONTROLLER_MODEL_ISA_BRIDGE))) { + 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; + } } }
@@ -103,6 +110,16 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def, goto error; }
+ for (i = 0; i < def->ncontrollers; i++) { + if ((def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) && + (def->controllers[i]->model == VIR_DOMAIN_CONTROLLER_MODEL_ISA_BRIDGE) && + virDeviceInfoPCIAddressIsWanted(&def->controllers[i]->info)) { + def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + def->controllers[i]->info.addr.pci = lpc_addr; + break; + } + } + for (i = 0; i < def->ncontrollers; i++) { if ((def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) || (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) || diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args new file mode 100644 index 0000000000..910d1bbcfa --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args @@ -0,0 +1,10 @@ +/usr/sbin/bhyve \ +-c 1 \ +-m 214 \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ +-s 1:0,lpc \ +-s 2:0,ahci,hd:/tmp/freebsd.img \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs new file mode 100644 index 0000000000..32538b558e --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs @@ -0,0 +1,3 @@ +/usr/sbin/bhyveload \ +-m 214 \ +-d /tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml new file mode 100644 index 0000000000..796903a9fa --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml @@ -0,0 +1,26 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + </os> + <devices> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args new file mode 100644 index 0000000000..ee833eb460 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args @@ -0,0 +1,10 @@ +/usr/sbin/bhyve \ +-c 1 \ +-m 214 \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ +-s 31:0,lpc \ +-s 2:0,ahci,hd:/tmp/freebsd.img \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs new file mode 100644 index 0000000000..32538b558e --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs @@ -0,0 +1,3 @@ +/usr/sbin/bhyveload \ +-m 214 \ +-d /tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml new file mode 100644 index 0000000000..65d7480db6 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml @@ -0,0 +1,26 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + </os> + <devices> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x0'/> + </controller> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml new file mode 100644 index 0000000000..88ad9aebb7 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml @@ -0,0 +1,23 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + </os> + <devices> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args new file mode 100644 index 0000000000..910d1bbcfa --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args @@ -0,0 +1,10 @@ +/usr/sbin/bhyve \ +-c 1 \ +-m 214 \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ +-s 1:0,lpc \ +-s 2:0,ahci,hd:/tmp/freebsd.img \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs new file mode 100644 index 0000000000..32538b558e --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs @@ -0,0 +1,3 @@ +/usr/sbin/bhyveload \ +-m 214 \ +-d /tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml new file mode 100644 index 0000000000..71265ea32c --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml @@ -0,0 +1,24 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + </os> + <devices> + <controller type='isa' index='1' model='isa-bridge'/> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml new file mode 100644 index 0000000000..25622ce78a --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml @@ -0,0 +1,25 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + </os> + <devices> + <controller type='isa' index='1' model='isa-bridge'/> + <controller type='isa' index='2' model='isa-bridge'/> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c index 4a7f65a8e2..d120bd43ca 100644 --- a/tests/bhyvexml2argvtest.c +++ b/tests/bhyvexml2argvtest.c @@ -211,6 +211,8 @@ mymain(void) DO_TEST("cputopology"); DO_TEST_FAILURE("cputopology-nvcpu-mismatch"); DO_TEST("commandline"); + DO_TEST("isa-controller"); + DO_TEST_FAILURE("isa-multiple-controllers");
/* Address allocation tests */ DO_TEST("addr-single-sata-disk"); @@ -218,6 +220,9 @@ mymain(void) DO_TEST("addr-more-than-32-sata-disks"); DO_TEST("addr-single-virtio-disk"); DO_TEST("addr-multiple-virtio-disks"); + DO_TEST("addr-isa-controller-on-slot-1"); + DO_TEST("addr-isa-controller-on-slot-31"); + DO_TEST_FAILURE("addr-non-isa-controller-on-slot-1");
/* The same without 32 devs per controller support */ driver.bhyvecaps ^= BHYVE_CAP_AHCI32SLOT; diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml new file mode 100644 index 0000000000..db84185fe6 --- /dev/null +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml @@ -0,0 +1,36 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <disk type='file' device='disk'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <source bridge='virbr0'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml new file mode 100644 index 0000000000..6ab5e6ad1a --- /dev/null +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml @@ -0,0 +1,36 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <disk type='file' device='disk'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x0'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <source bridge='virbr0'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml new file mode 100644 index 0000000000..db84185fe6 --- /dev/null +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml @@ -0,0 +1,36 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <disk type='file' device='disk'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <source bridge='virbr0'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2xmltest.c b/tests/bhyvexml2xmltest.c index ed421b8839..4b7f59ea29 100644 --- a/tests/bhyvexml2xmltest.c +++ b/tests/bhyvexml2xmltest.c @@ -111,6 +111,7 @@ mymain(void) DO_TEST_DIFFERENT("vnc-vgaconf-io"); DO_TEST_DIFFERENT("vnc-autoport"); DO_TEST_DIFFERENT("commandline"); + DO_TEST_DIFFERENT("isa-controller");
/* Address allocation tests */ DO_TEST_DIFFERENT("addr-single-sata-disk"); @@ -118,6 +119,8 @@ mymain(void) DO_TEST_DIFFERENT("addr-more-than-32-sata-disks"); DO_TEST_DIFFERENT("addr-single-virtio-disk"); DO_TEST_DIFFERENT("addr-multiple-virtio-disks"); + DO_TEST_DIFFERENT("addr-isa-controller-on-slot-1"); + DO_TEST_DIFFERENT("addr-isa-controller-on-slot-31");
/* The same without 32 devs per controller support */ driver.bhyvecaps ^= BHYVE_CAP_AHCI32SLOT; -- 2.20.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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@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?
Also, this whole switch is indented by 8 instead of 4 spaces.
+ virCommandAddArg(cmd, "-s"); + virCommandAddArgFormat(cmd, "%d:0,lpc", + controller->info.addr.pci.slot); + break; } } for (i = 0; i < def->nnets; i++) { @@ -618,7 +634,7 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, } }
- if (bhyveDomainDefNeedsISAController(def)) + if (nisacontrollers == 0 && bhyveDomainDefNeedsISAController(def)) bhyveBuildLPCArgStr(def, cmd);
if (bhyveBuildConsoleArgStr(def, cmd) < 0) diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c index 201044d9e6..42093afb7b 100644 --- a/src/bhyve/bhyve_device.c +++ b/src/bhyve/bhyve_device.c @@ -47,10 +47,17 @@ bhyveCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_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) &&
== has higher precedence than &&, the inner parentheses can be dropped
+ (device->data.controller->model == VIR_DOMAIN_CONTROLLER_MODEL_ISA_BRIDGE))) { + 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; + } } }
@@ -103,6 +110,16 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def, goto error; }
+ for (i = 0; i < def->ncontrollers; i++) { + if ((def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) && + (def->controllers[i]->model == VIR_DOMAIN_CONTROLLER_MODEL_ISA_BRIDGE) && + virDeviceInfoPCIAddressIsWanted(&def->controllers[i]->info)) { + def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + def->controllers[i]->info.addr.pci = lpc_addr; + break; + } + } + for (i = 0; i < def->ncontrollers; i++) { if ((def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) || (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) || diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args new file mode 100644 index 0000000000..910d1bbcfa --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args @@ -0,0 +1,10 @@ +/usr/sbin/bhyve \ +-c 1 \ +-m 214 \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ +-s 1:0,lpc \ +-s 2:0,ahci,hd:/tmp/freebsd.img \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs new file mode 100644 index 0000000000..32538b558e --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs @@ -0,0 +1,3 @@ +/usr/sbin/bhyveload \ +-m 214 \ +-d /tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml new file mode 100644 index 0000000000..796903a9fa --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml @@ -0,0 +1,26 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + </os> + <devices> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args new file mode 100644 index 0000000000..ee833eb460 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args @@ -0,0 +1,10 @@ +/usr/sbin/bhyve \ +-c 1 \ +-m 214 \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ +-s 31:0,lpc \ +-s 2:0,ahci,hd:/tmp/freebsd.img \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs new file mode 100644 index 0000000000..32538b558e --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs @@ -0,0 +1,3 @@ +/usr/sbin/bhyveload \ +-m 214 \ +-d /tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml new file mode 100644 index 0000000000..65d7480db6 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml @@ -0,0 +1,26 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + </os> + <devices> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x0'/> + </controller> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml new file mode 100644 index 0000000000..88ad9aebb7 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml @@ -0,0 +1,23 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + </os> + <devices> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args new file mode 100644 index 0000000000..910d1bbcfa --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args @@ -0,0 +1,10 @@ +/usr/sbin/bhyve \ +-c 1 \ +-m 214 \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ +-s 1:0,lpc \ +-s 2:0,ahci,hd:/tmp/freebsd.img \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs new file mode 100644 index 0000000000..32538b558e --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs @@ -0,0 +1,3 @@ +/usr/sbin/bhyveload \ +-m 214 \ +-d /tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml new file mode 100644 index 0000000000..71265ea32c --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml @@ -0,0 +1,24 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + </os> + <devices> + <controller type='isa' index='1' model='isa-bridge'/> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml new file mode 100644 index 0000000000..25622ce78a --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml @@ -0,0 +1,25 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + </os> + <devices> + <controller type='isa' index='1' model='isa-bridge'/> + <controller type='isa' index='2' model='isa-bridge'/> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c index 4a7f65a8e2..d120bd43ca 100644 --- a/tests/bhyvexml2argvtest.c +++ b/tests/bhyvexml2argvtest.c @@ -211,6 +211,8 @@ mymain(void) DO_TEST("cputopology"); DO_TEST_FAILURE("cputopology-nvcpu-mismatch"); DO_TEST("commandline"); + DO_TEST("isa-controller"); + DO_TEST_FAILURE("isa-multiple-controllers");
/* Address allocation tests */ DO_TEST("addr-single-sata-disk"); @@ -218,6 +220,9 @@ mymain(void) DO_TEST("addr-more-than-32-sata-disks"); DO_TEST("addr-single-virtio-disk"); DO_TEST("addr-multiple-virtio-disks"); + DO_TEST("addr-isa-controller-on-slot-1"); + DO_TEST("addr-isa-controller-on-slot-31"); + DO_TEST_FAILURE("addr-non-isa-controller-on-slot-1");
/* The same without 32 devs per controller support */ driver.bhyvecaps ^= BHYVE_CAP_AHCI32SLOT; diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml new file mode 100644 index 0000000000..db84185fe6 --- /dev/null +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml @@ -0,0 +1,36 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <disk type='file' device='disk'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <source bridge='virbr0'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml new file mode 100644 index 0000000000..6ab5e6ad1a --- /dev/null +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml @@ -0,0 +1,36 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <disk type='file' device='disk'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x0'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <source bridge='virbr0'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml new file mode 100644 index 0000000000..db84185fe6 --- /dev/null +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml @@ -0,0 +1,36 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <disk type='file' device='disk'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <source bridge='virbr0'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2xmltest.c b/tests/bhyvexml2xmltest.c index ed421b8839..4b7f59ea29 100644 --- a/tests/bhyvexml2xmltest.c +++ b/tests/bhyvexml2xmltest.c @@ -111,6 +111,7 @@ mymain(void) DO_TEST_DIFFERENT("vnc-vgaconf-io"); DO_TEST_DIFFERENT("vnc-autoport"); DO_TEST_DIFFERENT("commandline"); + DO_TEST_DIFFERENT("isa-controller");
/* Address allocation tests */ DO_TEST_DIFFERENT("addr-single-sata-disk"); @@ -118,6 +119,8 @@ mymain(void) DO_TEST_DIFFERENT("addr-more-than-32-sata-disks"); DO_TEST_DIFFERENT("addr-single-virtio-disk"); DO_TEST_DIFFERENT("addr-multiple-virtio-disks"); + DO_TEST_DIFFERENT("addr-isa-controller-on-slot-1"); + DO_TEST_DIFFERENT("addr-isa-controller-on-slot-31");
/* The same without 32 devs per controller support */ driver.bhyvecaps ^= BHYVE_CAP_AHCI32SLOT; -- 2.20.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Roman Bogorodskiy
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@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
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. Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- src/bhyve/bhyve_command.c | 11 ----------- src/bhyve/bhyve_domain.c | 7 ++++++- src/bhyve/bhyve_domain.h | 2 -- tests/bhyvexml2argvdata/bhyvexml2argv-console.args | 2 +- .../bhyvexml2argv-serial-grub-nocons.args | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-serial-grub.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-serial.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args | 4 ++-- .../bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.args | 4 ++-- .../bhyvexml2argv-vnc-vgaconf-io.args | 4 ++-- .../bhyvexml2argv-vnc-vgaconf-off.args | 4 ++-- .../bhyvexml2argv-vnc-vgaconf-on.args | 4 ++-- tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args | 4 ++-- tests/bhyvexml2xmloutdata/bhyvexml2xmlout-console.xml | 3 +++ .../bhyvexml2xmlout-serial-grub-nocons.xml | 3 +++ .../bhyvexml2xmlout-serial-grub.xml | 3 +++ tests/bhyvexml2xmloutdata/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 +++ tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc.xml | 3 +++ 22 files changed, 49 insertions(+), 30 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 2c90265a93..6e0af3892e 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -325,14 +325,6 @@ bhyveBuildVirtIODiskArgStr(const virDomainDef *def ATTRIBUTE_UNUSED, return 0; } -static int -bhyveBuildLPCArgStr(const virDomainDef *def ATTRIBUTE_UNUSED, - virCommandPtr cmd) -{ - virCommandAddArgList(cmd, "-s", "1,lpc", NULL); - return 0; -} - static int bhyveBuildGraphicsArgStr(const virDomainDef *def, virDomainGraphicsDefPtr graphics, @@ -634,9 +626,6 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, } } - if (nisacontrollers == 0 && bhyveDomainDefNeedsISAController(def)) - bhyveBuildLPCArgStr(def, cmd); - if (bhyveBuildConsoleArgStr(def, cmd) < 0) goto error; diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c index e476ef7e7d..68f66904a3 100644 --- a/src/bhyve/bhyve_domain.c +++ b/src/bhyve/bhyve_domain.c @@ -61,7 +61,7 @@ virDomainXMLPrivateDataCallbacks virBhyveDriverPrivateDataCallbacks = { .free = bhyveDomainObjPrivateFree, }; -bool +static bool bhyveDomainDefNeedsISAController(virDomainDefPtr def) { if ((def->os.bootloader == NULL && def->os.loader) || @@ -83,6 +83,11 @@ bhyveDomainDefPostParse(virDomainDefPtr def, VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 0) return -1; + if (bhyveDomainDefNeedsISAController(def)) + if (virDomainDefMaybeAddController(def, VIR_DOMAIN_CONTROLLER_TYPE_ISA, 1, + VIR_DOMAIN_CONTROLLER_MODEL_ISA_BRIDGE) < 0) + return -1; + return 0; } diff --git a/src/bhyve/bhyve_domain.h b/src/bhyve/bhyve_domain.h index 03a2b369d9..5f94038e89 100644 --- a/src/bhyve/bhyve_domain.h +++ b/src/bhyve/bhyve_domain.h @@ -41,6 +41,4 @@ extern virDomainXMLPrivateDataCallbacks virBhyveDriverPrivateDataCallbacks; extern virDomainDefParserConfig virBhyveDriverDomainDefParserConfig; extern virDomainXMLNamespace virBhyveDriverDomainXMLNamespace; -bool bhyveDomainDefNeedsISAController(virDomainDefPtr def); - #endif /* LIBVIRT_BHYVE_DOMAIN_H */ diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-console.args b/tests/bhyvexml2argvdata/bhyvexml2argv-console.args index 6ab91ae7e4..25fbd4727e 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-console.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-console.args @@ -5,7 +5,7 @@ -H \ -P \ -s 0:0,hostbridge \ +-s 1:0,lpc \ -s 2:0,ahci,hd:/tmp/freebsd.img \ -s 3:0,virtio-net,faketapdev,mac=52:54:00:b1:42:eb \ --s 1,lpc \ -l com1,/dev/nmdm0A bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.args b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.args index 42a278208d..02846cb893 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.args @@ -5,7 +5,7 @@ -H \ -P \ -s 0:0,hostbridge \ +-s 1:0,lpc \ -s 2:0,ahci-hd,/tmp/freebsd.img \ -s 3:0,virtio-net,faketapdev,mac=52:54:00:a7:cd:5b \ --s 1,lpc \ -l com1,/dev/nmdm0A bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.args b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.args index 313724dc90..e4712b448c 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.args @@ -5,7 +5,7 @@ -H \ -P \ -s 0:0,hostbridge \ +-s 1:0,lpc \ -s 2:0,ahci,hd:/tmp/freebsd.img \ -s 3:0,virtio-net,faketapdev,mac=52:54:00:f0:72:11 \ --s 1,lpc \ -l com1,/dev/nmdm0A bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial.args b/tests/bhyvexml2argvdata/bhyvexml2argv-serial.args index 059e457072..f45a190137 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-serial.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial.args @@ -5,7 +5,7 @@ -H \ -P \ -s 0:0,hostbridge \ +-s 1:0,lpc \ -s 2:0,ahci,hd:/tmp/freebsd.img \ -s 3:0,virtio-net,faketapdev,mac=52:54:00:4f:f3:5b \ --s 1,lpc \ -l com1,/dev/nmdm0A bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args b/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args index 8ff8673ed4..937b066e8c 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args @@ -6,6 +6,6 @@ -P \ -s 0:0,hostbridge \ -l bootrom,/path/to/test.fd \ +-s 1:0,lpc \ -s 2:0,ahci,hd:/tmp/freebsd.img \ --s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ --s 1,lpc bhyve +-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.args b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.args index 039526ff35..551469dabe 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.args @@ -6,7 +6,7 @@ -P \ -s 0:0,hostbridge \ -l bootrom,/path/to/test.fd \ +-s 1:0,lpc \ -s 2:0,ahci,hd:/tmp/freebsd.img \ -s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ --s 4:0,fbuf,tcp=127.0.0.1:5900 \ --s 1,lpc bhyve +-s 4:0,fbuf,tcp=127.0.0.1:5900 bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-io.args b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-io.args index da37971009..47022e84cf 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-io.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-io.args @@ -6,7 +6,7 @@ -P \ -s 0:0,hostbridge \ -l bootrom,/path/to/test.fd \ +-s 1:0,lpc \ -s 2:0,ahci,hd:/tmp/freebsd.img \ -s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ --s 4:0,fbuf,tcp=127.0.0.1:5904,vga=io \ --s 1,lpc bhyve +-s 4:0,fbuf,tcp=127.0.0.1:5904,vga=io bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-off.args b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-off.args index 70347ee0b6..923098f3db 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-off.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-off.args @@ -6,7 +6,7 @@ -P \ -s 0:0,hostbridge \ -l bootrom,/path/to/test.fd \ +-s 1:0,lpc \ -s 2:0,ahci,hd:/tmp/freebsd.img \ -s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ --s 4:0,fbuf,tcp=127.0.0.1:5904,vga=off \ --s 1,lpc bhyve +-s 4:0,fbuf,tcp=127.0.0.1:5904,vga=off bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-on.args b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-on.args index d0e1d81e2a..9225f5d133 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-on.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-vgaconf-on.args @@ -6,7 +6,7 @@ -P \ -s 0:0,hostbridge \ -l bootrom,/path/to/test.fd \ +-s 1:0,lpc \ -s 2:0,ahci,hd:/tmp/freebsd.img \ -s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ --s 4:0,fbuf,tcp=127.0.0.1:5904,vga=on \ --s 1,lpc bhyve +-s 4:0,fbuf,tcp=127.0.0.1:5904,vga=on bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args index 90889b8f39..cd7a543265 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args @@ -6,7 +6,7 @@ -P \ -s 0:0,hostbridge \ -l bootrom,/path/to/test.fd \ +-s 1:0,lpc \ -s 2:0,ahci,hd:/tmp/freebsd.img \ -s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ --s 4:0,fbuf,tcp=127.0.0.1:5904 \ --s 1,lpc bhyve +-s 4:0,fbuf,tcp=127.0.0.1:5904 bhyve diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-console.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-console.xml index 78d4d30016..2b453ee640 100644 --- a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-console.xml +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-console.xml @@ -20,6 +20,9 @@ <address type='drive' controller='0' bus='0' target='2' unit='0'/> </disk> <controller type='pci' index='0' model='pci-root'/> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> <controller type='sata' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </controller> diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-serial-grub-nocons.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-serial-grub-nocons.xml index 845cb09e9f..e012d586c8 100644 --- a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-serial-grub-nocons.xml +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-serial-grub-nocons.xml @@ -20,6 +20,9 @@ <address type='drive' controller='0' bus='0' target='2' unit='0'/> </disk> <controller type='pci' index='0' model='pci-root'/> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> <controller type='sata' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </controller> diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-serial-grub.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-serial-grub.xml index 6c8fda32af..ecb13907df 100644 --- a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-serial-grub.xml +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-serial-grub.xml @@ -20,6 +20,9 @@ <address type='drive' controller='0' bus='0' target='2' unit='0'/> </disk> <controller type='pci' index='0' model='pci-root'/> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> <controller type='sata' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </controller> diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-serial.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-serial.xml index eb50cc05ad..6dd2b94be7 100644 --- a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-serial.xml +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-serial.xml @@ -20,6 +20,9 @@ <address type='drive' controller='0' bus='0' target='2' unit='0'/> </disk> <controller type='pci' index='0' model='pci-root'/> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> <controller type='sata' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </controller> diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-autoport.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-autoport.xml index d6cfe76b70..2c99dc6233 100644 --- a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-autoport.xml +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-autoport.xml @@ -21,6 +21,9 @@ <address type='drive' controller='0' bus='0' target='2' unit='0'/> </disk> <controller type='pci' index='0' model='pci-root'/> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> <controller type='sata' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </controller> diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-vgaconf-io.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-vgaconf-io.xml index 9e470e432e..9d152e4079 100644 --- a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-vgaconf-io.xml +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-vgaconf-io.xml @@ -21,6 +21,9 @@ <address type='drive' controller='0' bus='0' target='2' unit='0'/> </disk> <controller type='pci' index='0' model='pci-root'/> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> <controller type='sata' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </controller> diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-vgaconf-off.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-vgaconf-off.xml index 89c4ceac57..ece20ddbab 100644 --- a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-vgaconf-off.xml +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-vgaconf-off.xml @@ -21,6 +21,9 @@ <address type='drive' controller='0' bus='0' target='2' unit='0'/> </disk> <controller type='pci' index='0' model='pci-root'/> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> <controller type='sata' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </controller> diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-vgaconf-on.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-vgaconf-on.xml index 86d8939364..f2942c91aa 100644 --- a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-vgaconf-on.xml +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-vgaconf-on.xml @@ -21,6 +21,9 @@ <address type='drive' controller='0' bus='0' target='2' unit='0'/> </disk> <controller type='pci' index='0' model='pci-root'/> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> <controller type='sata' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </controller> diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc.xml index 9e470e432e..9d152e4079 100644 --- a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc.xml +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc.xml @@ -21,6 +21,9 @@ <address type='drive' controller='0' bus='0' target='2' unit='0'/> </disk> <controller type='pci' index='0' model='pci-root'/> + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> <controller type='sata' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </controller> -- 2.20.1
On Sun, Feb 17, 2019 at 05:04:03PM +0400, Roman Bogorodskiy wrote:
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().
Technically, you can move this patch before the previous one, since only the implicit controller is added here and that one is already formatted,
Also, make bhyveDomainDefNeedsISAController() static as it's no longer used outside of bhyve_domain.c.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- src/bhyve/bhyve_command.c | 11 ----------- src/bhyve/bhyve_domain.c | 7 ++++++- src/bhyve/bhyve_domain.h | 2 -- tests/bhyvexml2argvdata/bhyvexml2argv-console.args | 2 +- .../bhyvexml2argv-serial-grub-nocons.args | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-serial-grub.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-serial.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args | 4 ++-- .../bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.args | 4 ++-- .../bhyvexml2argv-vnc-vgaconf-io.args | 4 ++-- .../bhyvexml2argv-vnc-vgaconf-off.args | 4 ++-- .../bhyvexml2argv-vnc-vgaconf-on.args | 4 ++-- tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args | 4 ++-- tests/bhyvexml2xmloutdata/bhyvexml2xmlout-console.xml | 3 +++ .../bhyvexml2xmlout-serial-grub-nocons.xml | 3 +++ .../bhyvexml2xmlout-serial-grub.xml | 3 +++ tests/bhyvexml2xmloutdata/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 +++ tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc.xml | 3 +++ 22 files changed, 49 insertions(+), 30 deletions(-)
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 2c90265a93..6e0af3892e 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -325,14 +325,6 @@ bhyveBuildVirtIODiskArgStr(const virDomainDef *def ATTRIBUTE_UNUSED, return 0; }
-static int -bhyveBuildLPCArgStr(const virDomainDef *def ATTRIBUTE_UNUSED, - virCommandPtr cmd) -{ - virCommandAddArgList(cmd, "-s", "1,lpc", NULL); - return 0; -} - static int bhyveBuildGraphicsArgStr(const virDomainDef *def, virDomainGraphicsDefPtr graphics, @@ -634,9 +626,6 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, } }
- if (nisacontrollers == 0 && bhyveDomainDefNeedsISAController(def))
that would remove the need for the nisacontrollers variable and the previous patch would delete this. Jano
Document ability to specify LCP PCI-ISA bridge PCI address. Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- docs/drvbhyve.html.in | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/docs/drvbhyve.html.in b/docs/drvbhyve.html.in index 2e9cf5551b..d31ce781cf 100644 --- a/docs/drvbhyve.html.in +++ b/docs/drvbhyve.html.in @@ -462,6 +462,28 @@ Example:</p> </domain> </pre> +<h3><a id="lpc">LPC PCI-ISA bridge address</a></h3> + +<p>LPC PCI-ISA bridge is used in bhyve to attach serial ports and a boot ROM. +By default, the bhyve driver assigns PCI slot 1 for it. +However, sometimes it may be necessary to use other slot for it. +For example, placing it on slot 31 instead of slot 1 will look like this:</p> + +<pre> +<domain type="bhyve"> + ... + <devices> + ... + <controller type='isa' index='1' model='isa-bridge'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x0'/> + </controller> + ... + </devices> +</domain> +</pre> + +<p>This is supported <span class="since">since 5.1.0</span>.</p> + <h3><a id="bhyvecommand">Pass-through of arbitrary bhyve commands</a></h3> <p><span class="since">Since 5.1.0</span>, it's possible to pass additional command-line -- 2.20.1
On Sun, Feb 17, 2019 at 05:04:04PM +0400, Roman Bogorodskiy wrote:
Document ability to specify LCP PCI-ISA bridge PCI address.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- docs/drvbhyve.html.in | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
diff --git a/docs/drvbhyve.html.in b/docs/drvbhyve.html.in index 2e9cf5551b..d31ce781cf 100644 --- a/docs/drvbhyve.html.in +++ b/docs/drvbhyve.html.in @@ -462,6 +462,28 @@ Example:</p> </domain> </pre>
+<h3><a id="lpc">LPC PCI-ISA bridge address</a></h3> + +<p>LPC PCI-ISA bridge is used in bhyve to attach serial ports and a boot ROM. +By default, the bhyve driver assigns PCI slot 1 for it. +However, sometimes it may be necessary to use other slot for it. +For example, placing it on slot 31 instead of slot 1 will look like this:</p> + +<pre> +<domain type="bhyve"> + ... + <devices> + ... + <controller type='isa' index='1' model='isa-bridge'>
Numbering from 1 seems unnatural. Also, with the model dropped, this will need an update.
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x0'/> + </controller> + ... + </devices> +</domain> +</pre> + +<p>This is supported <span class="since">since 5.1.0</span>.</p> +
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- docs/news.xml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 8d6d58ae6a..e0c9c3590f 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -59,6 +59,16 @@ of the network's <code>bridge</code> element. </description> </change> + <change> + <summary> + bhyve: allow to configure LPC PCI address + </summary> + <description> + The bhyve driver now explicty models LPC PCI-ISA bridge, thus + allowing to specify PCI address for it instead of using + the default PCI slot 1. + </description> + </change> </section> <section title="Improvements"> </section> -- 2.20.1
On Sun, Feb 17, 2019 at 05:04:05PM +0400, Roman Bogorodskiy wrote:
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- docs/news.xml | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml index 8d6d58ae6a..e0c9c3590f 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -59,6 +59,16 @@ of the network's <code>bridge</code> element. </description> </change> + <change> + <summary> + bhyve: allow to configure LPC PCI address + </summary> + <description> + The bhyve driver now explicty models LPC PCI-ISA bridge, thus
explicitly
+ allowing to specify PCI address for it instead of using + the default PCI slot 1. + </description> + </change>
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Roman Bogorodskiy