[libvirt-jenkins-ci PATCH 0/7] lcitool: Create and expose ccache wrappers
by Andrea Bolognani
This series makes ccache work out of the box for container-based
builds. Most of it is refactoring.
Applies on top of
https://www.redhat.com/archives/libvir-list/2020-March/msg01263.html
Andrea Bolognani (7):
lcitool: Improve ccache symlinks creation
lcitool: Configure ccache using environment variables
lcitool: Create ccache wrappers outside of $HOME
lcitool: Refactor cross_arch handling a bit
lcitool: Use commands[] for deb-based distros
lcitool: Use cross_commands[] for all distros
lcitool: Create and expose ccache wrappers
guests/host_vars/libvirt-centos-7/main.yml | 1 +
guests/host_vars/libvirt-centos-8/main.yml | 1 +
guests/host_vars/libvirt-debian-10/main.yml | 1 +
guests/host_vars/libvirt-debian-9/main.yml | 1 +
guests/host_vars/libvirt-debian-sid/main.yml | 1 +
guests/host_vars/libvirt-fedora-30/main.yml | 1 +
guests/host_vars/libvirt-fedora-31/main.yml | 1 +
.../host_vars/libvirt-fedora-rawhide/main.yml | 1 +
guests/host_vars/libvirt-freebsd-11/main.yml | 1 +
guests/host_vars/libvirt-freebsd-12/main.yml | 1 +
.../libvirt-freebsd-current/main.yml | 1 +
.../host_vars/libvirt-opensuse-151/main.yml | 1 +
guests/host_vars/libvirt-ubuntu-1604/main.yml | 1 +
guests/host_vars/libvirt-ubuntu-1804/main.yml | 1 +
guests/lcitool | 92 +++++++++++--------
guests/playbooks/update/main.yml | 1 +
guests/playbooks/update/tasks/global.yml | 14 +++
guests/playbooks/update/tasks/users.yml | 45 ---------
guests/playbooks/update/templates/bashrc.j2 | 3 +-
.../playbooks/update/templates/ccache.conf.j2 | 1 -
20 files changed, 87 insertions(+), 83 deletions(-)
create mode 100644 guests/playbooks/update/tasks/global.yml
delete mode 100644 guests/playbooks/update/templates/ccache.conf.j2
--
2.25.1
4 years, 5 months
[libvirt PATCH] qemu: fix missing error reports in capabilities probing
by Daniel P. Berrangé
The "virsh domcapabilities --arch ppc64" command will fail with no
error message set if qemu-system-ppc64 is not currently installed.
This is because virQEMUCapsCacheLookup() does not report any error
message if not capabilities can be obtained from the cache. Almost
all methods calling this expected an error to be set on failure.
Once that's fixed though, we see a further bug which is that
virQEMUCapsCacheLookupDefault() is passing a NULL binary path to
virQEMUCapsCacheLookup(), so we need to catch that too.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 11 +++++++++++
src/qemu/qemu_domain.c | 4 +++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index aa90eab229..448d6fa175 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5517,6 +5517,11 @@ virQEMUCapsCacheLookup(virFileCachePtr cache,
priv->microcodeVersion = virHostCPUGetMicrocodeVersion();
ret = virFileCacheLookup(cache, binary);
+ if (!ret) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("no capabilities available for %s"), binary);
+ return NULL;
+ }
VIR_DEBUG("Returning caps %p for %s", ret, binary);
return ret;
@@ -5664,6 +5669,12 @@ virQEMUCapsCacheLookupDefault(virFileCachePtr cache,
probedbinary = virQEMUCapsGetDefaultEmulator(hostarch, arch);
binary = probedbinary;
}
+ if (!binary) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unable to find any emulator to serve '%s' architecture"),
+ archStr);
+ goto cleanup;
+ }
if (!(qemuCaps = virQEMUCapsCacheLookup(cache, binary)))
goto cleanup;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 2dad823a86..97096073e6 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6068,8 +6068,10 @@ qemuDomainPostParseDataAlloc(const virDomainDef *def,
virQEMUDriverPtr driver = opaque;
if (!(*parseOpaque = virQEMUCapsCacheLookup(driver->qemuCapsCache,
- def->emulator)))
+ def->emulator))) {
+ virResetLastError();
return 1;
+ }
return 0;
}
--
2.26.2
4 years, 5 months
[PATCH] Substitute security_context_t with char *
by Michal Privoznik
Historically, we've used security_context_t for variables passed
to libselinux APIs. But almost 7 years ago, libselinux developers
admitted in their API that in fact, it's just a 'char *' type
[1]. Ever since then the APIs accept 'char *' instead, but they
kept the old alias just for API stability. Well, not anymore [2].
1: https://github.com/SELinuxProject/selinux/commit/9eb9c9327563014ad6a80781...
2: https://github.com/SELinuxProject/selinux/commit/7a124ca2758136f49cc38efc...
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/libvirt-lxc.c | 2 +-
src/rpc/virnetsocket.c | 2 +-
src/security/security_selinux.c | 26 +++++++++++++-------------
src/storage/storage_util.c | 2 +-
src/util/viridentity.c | 2 +-
tests/securityselinuxhelper.c | 16 ++++++++--------
tests/securityselinuxlabeltest.c | 4 ++--
tests/securityselinuxtest.c | 2 +-
tests/viridentitytest.c | 2 +-
9 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/src/libvirt-lxc.c b/src/libvirt-lxc.c
index 47a06a39f2..25f1cfc5f7 100644
--- a/src/libvirt-lxc.c
+++ b/src/libvirt-lxc.c
@@ -204,7 +204,7 @@ virDomainLxcEnterSecurityLabel(virSecurityModelPtr model,
if (STREQ(model->model, "selinux")) {
#ifdef WITH_SELINUX
if (oldlabel) {
- security_context_t ctx;
+ char *ctx;
if (getcon(&ctx) < 0) {
virReportSystemError(errno,
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index c62c2fb3fc..9aaabb4577 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -1612,7 +1612,7 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock G_GNUC_UNUSED,
int virNetSocketGetSELinuxContext(virNetSocketPtr sock,
char **context)
{
- security_context_t seccon = NULL;
+ char *seccon = NULL;
int ret = -1;
*context = NULL;
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 1d28430035..cc8fb1099c 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -198,7 +198,7 @@ virSecuritySELinuxTransactionAppend(const char *path,
static int
virSecuritySELinuxRememberLabel(const char *path,
- const security_context_t con)
+ const char *con)
{
return virSecuritySetRememberedLabel(SECURITY_SELINUX_NAME,
path, con);
@@ -207,7 +207,7 @@ virSecuritySELinuxRememberLabel(const char *path,
static int
virSecuritySELinuxRecallLabel(const char *path,
- security_context_t *con)
+ char **con)
{
int rv;
@@ -431,7 +431,7 @@ virSecuritySELinuxMCSGetProcessRange(char **sens,
int *catMin,
int *catMax)
{
- security_context_t ourSecContext = NULL;
+ char *ourSecContext = NULL;
context_t ourContext = NULL;
char *cat = NULL;
char *tmp;
@@ -530,8 +530,8 @@ virSecuritySELinuxMCSGetProcessRange(char **sens,
}
static char *
-virSecuritySELinuxContextAddRange(security_context_t src,
- security_context_t dst)
+virSecuritySELinuxContextAddRange(char *src,
+ char *dst)
{
char *str = NULL;
char *ret = NULL;
@@ -575,7 +575,7 @@ virSecuritySELinuxGenNewContext(const char *basecontext,
context_t context = NULL;
char *ret = NULL;
char *str;
- security_context_t ourSecContext = NULL;
+ char *ourSecContext = NULL;
context_t ourContext = NULL;
VIR_DEBUG("basecontext=%s mcs=%s isObjectContext=%d",
@@ -955,7 +955,7 @@ virSecuritySELinuxReserveLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
pid_t pid)
{
- security_context_t pctx;
+ char *pctx;
context_t ctx = NULL;
const char *mcs;
int rv;
@@ -1203,7 +1203,7 @@ virSecuritySELinuxGetProcessLabel(virSecurityManagerPtr mgr G_GNUC_UNUSED,
pid_t pid,
virSecurityLabelPtr sec)
{
- security_context_t ctx;
+ char *ctx;
if (getpidcon_raw(pid, &ctx) == -1) {
virReportSystemError(errno,
@@ -1316,7 +1316,7 @@ virSecuritySELinuxSetFilecon(virSecurityManagerPtr mgr,
bool remember)
{
bool privileged = virSecurityManagerGetPrivileged(mgr);
- security_context_t econ = NULL;
+ char *econ = NULL;
int refcount;
int rc;
bool rollback = false;
@@ -1426,7 +1426,7 @@ virSecuritySELinuxFSetFilecon(int fd, char *tcon)
/* Set fcon to the appropriate label for path and mode, or return -1. */
static int
getContext(virSecurityManagerPtr mgr G_GNUC_UNUSED,
- const char *newpath, mode_t mode, security_context_t *fcon)
+ const char *newpath, mode_t mode, char **fcon)
{
virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr);
@@ -1443,7 +1443,7 @@ virSecuritySELinuxRestoreFileLabel(virSecurityManagerPtr mgr,
{
bool privileged = virSecurityManagerGetPrivileged(mgr);
struct stat buf;
- security_context_t fcon = NULL;
+ char *fcon = NULL;
char *newpath = NULL;
int rc;
int ret = -1;
@@ -2974,7 +2974,7 @@ virSecuritySELinuxSetDaemonSocketLabel(virSecurityManagerPtr mgr G_GNUC_UNUSED,
{
/* TODO: verify DOI */
virSecurityLabelDefPtr secdef;
- security_context_t scon = NULL;
+ char *scon = NULL;
char *str = NULL;
int rc = -1;
@@ -3283,7 +3283,7 @@ virSecuritySELinuxSetTapFDLabel(virSecurityManagerPtr mgr,
int fd)
{
struct stat buf;
- security_context_t fcon = NULL;
+ char *fcon = NULL;
virSecurityLabelDefPtr secdef;
char *str = NULL, *proc = NULL, *fd_path = NULL;
int rc = -1;
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 8d92232a87..ee048f02fe 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -1814,7 +1814,7 @@ virStorageBackendUpdateVolTargetInfoFD(virStorageSourcePtr target,
struct stat *sb)
{
#if WITH_SELINUX
- security_context_t filecon = NULL;
+ char *filecon = NULL;
#endif
if (virStorageSourceUpdateBackingSizes(target, fd, sb) < 0)
diff --git a/src/util/viridentity.c b/src/util/viridentity.c
index 8cc2db2568..2cb9042a84 100644
--- a/src/util/viridentity.c
+++ b/src/util/viridentity.c
@@ -157,7 +157,7 @@ virIdentityPtr virIdentityGetSystem(void)
unsigned long long startTime;
g_autoptr(virIdentity) ret = NULL;
#if WITH_SELINUX
- security_context_t con;
+ char *con;
#endif
if (!(ret = virIdentityNew()))
diff --git a/tests/securityselinuxhelper.c b/tests/securityselinuxhelper.c
index c3d7f8c1ce..64d2b75740 100644
--- a/tests/securityselinuxhelper.c
+++ b/tests/securityselinuxhelper.c
@@ -55,7 +55,7 @@ static struct selabel_handle *(*real_selabel_open)(unsigned int backend,
unsigned nopts);
static void (*real_selabel_close)(struct selabel_handle *handle);
static int (*real_selabel_lookup_raw)(struct selabel_handle *handle,
- security_context_t *con,
+ char **con,
const char *key,
int type);
@@ -89,7 +89,7 @@ static void init_syms(void)
* the virt_use_nfs bool is set.
*/
-int getcon_raw(security_context_t *context)
+int getcon_raw(char **context)
{
if (!is_selinux_enabled()) {
errno = EINVAL;
@@ -104,12 +104,12 @@ int getcon_raw(security_context_t *context)
return 0;
}
-int getcon(security_context_t *context)
+int getcon(char **context)
{
return getcon_raw(context);
}
-int getpidcon_raw(pid_t pid, security_context_t *context)
+int getpidcon_raw(pid_t pid, char **context)
{
if (!is_selinux_enabled()) {
errno = EINVAL;
@@ -129,7 +129,7 @@ int getpidcon_raw(pid_t pid, security_context_t *context)
return 0;
}
-int getpidcon(pid_t pid, security_context_t *context)
+int getpidcon(pid_t pid, char **context)
{
return getpidcon_raw(pid, context);
}
@@ -165,7 +165,7 @@ int setfilecon(const char *path, const char *con)
return setfilecon_raw(path, con);
}
-int getfilecon_raw(const char *path, security_context_t *con)
+int getfilecon_raw(const char *path, char **con)
{
char *constr = NULL;
ssize_t len = getxattr(path, "user.libvirt.selinux",
@@ -189,7 +189,7 @@ int getfilecon_raw(const char *path, security_context_t *con)
}
-int getfilecon(const char *path, security_context_t *con)
+int getfilecon(const char *path, char **con)
{
return getfilecon_raw(path, con);
}
@@ -308,7 +308,7 @@ void selabel_close(struct selabel_handle *handle)
}
int selabel_lookup_raw(struct selabel_handle *handle,
- security_context_t *con,
+ char **con,
const char *key,
int type)
{
diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c
index 3040a36693..50b447c163 100644
--- a/tests/securityselinuxlabeltest.c
+++ b/tests/securityselinuxlabeltest.c
@@ -252,7 +252,7 @@ static int
testSELinuxCheckLabels(testSELinuxFile *files, size_t nfiles)
{
size_t i;
- security_context_t ctx;
+ char *ctx;
for (i = 0; i < nfiles; i++) {
ctx = NULL;
@@ -360,7 +360,7 @@ mymain(void)
if (virTestRun("Labelling " # name, testSELinuxLabeling, name) < 0) \
ret = -1;
- setcon((security_context_t)"system_r:system_u:libvirtd_t:s0:c0.c1023");
+ setcon("system_r:system_u:libvirtd_t:s0:c0.c1023");
DO_TEST_LABELING("disks");
DO_TEST_LABELING("kernel");
diff --git a/tests/securityselinuxtest.c b/tests/securityselinuxtest.c
index 6c8314de6b..3f069c2d6b 100644
--- a/tests/securityselinuxtest.c
+++ b/tests/securityselinuxtest.c
@@ -217,7 +217,7 @@ testSELinuxGenLabel(const void *opaque)
context_t con = NULL;
context_t imgcon = NULL;
- if (setcon_raw((security_context_t)data->pidcon) < 0) {
+ if (setcon_raw(data->pidcon) < 0) {
perror("Cannot set process security context");
return -1;
}
diff --git a/tests/viridentitytest.c b/tests/viridentitytest.c
index 3f87af1c3b..9a8c8914d3 100644
--- a/tests/viridentitytest.c
+++ b/tests/viridentitytest.c
@@ -120,7 +120,7 @@ static int testIdentityGetSystem(const void *data)
static int testSetFakeSELinuxContext(const void *data G_GNUC_UNUSED)
{
#if WITH_SELINUX
- return setcon_raw((security_context_t)data);
+ return setcon_raw(data);
#else
VIR_DEBUG("libvirt not compiled with SELinux, skipping this test");
return EXIT_AM_SKIP;
--
2.26.2
4 years, 5 months
[libvirt PATCH 0/2] tests: Don't assume IPv4 connectivity is available
by Andrea Bolognani
I started looking into this after seeing
FAIL: virnetsockettest
======================
TEST: virnetsockettest
1) Socket TCP/IPv4 Accept ... libvirt: XML-RPC error : Unable to resolve address '127.0.0.1' service '5672': Address family for hostname not supported
FAILED
2) Socket TCP/IPv6 Accept ... OK
3) Socket TCP/IPv4+IPv6 Accept ... OK
4) Socket TCP/IPv4+IPv6 Accept ... OK
5) Socket UNIX Accept ... OK
6) Socket UNIX Addrs ... OK
7) Socket External Command /dev/zero ... OK
8) Socket External Command /dev/does-not-exist ... libvirt: XML-RPC error : End of file while reading data: /bin/cat: /dev/does-not-exist: No such file or directory: Input/output error
OK
9) SSH test 1 ... OK
10) SSH test 2 ... OK
11) SSH test 3 ... OK
12) SSH test 4 ... libvirt: XML-RPC error : End of file while reading data: Cannot connect to host nosuchhost: Input/output error
OK
13) SSH test 5 ... libvirt: XML-RPC error : End of file while reading data: : Input/output error
OK
14) SSH test 6 ... OK
15) SSH test 7 ... OK
during a Debian package build.
Full log:
https://buildd.debian.org/status/fetch.php?pkg=libvirt&arch=armel&ver=6.4...
Just a few days ago, this issue was discussed in
https://lists.debian.org/debian-devel/2020/07/msg00070.html
and libvirt was mentioned explicitly as a package that could be
affected by it.
Reproducing the issue is quite simple: just remove all IPv4
addresses *except* the one assigned to 'lo', and you'll see it
immediately. The problem seems to be that, in this configuration,
virNetSocketCheckProtocols() reports that both IPv4 and IPv6 are
available, but XML-RPC is apparently not happy with that.
I'm pretty bad at networking, so I'm reporting this in the hope that
someone else will figure it out O:-)
Anyway, there's another scenario that I found while digging: if you
remove *all* IPv4 addresses, *including* the one assigned to 'lo',
you get a different failure. My patch addresses this last one.
Andrea Bolognani (2):
tests: Don't assume IPv4 connectivity is available
tests: Minimize variable scope
tests/virnetsockettest.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
--
2.25.4
4 years, 5 months
[RFC 00/21] RFC: Generate parsexml/formatbuf functions based on directives
by Shi Lei
Last RFC: [https://www.redhat.com/archives/libvir-list/2020-April/msg00970.html]
In last RFC, I suggested we can generate object-model code based on relax-ng
files and Daniel gave it some comments.
Follow the suggestion from Daniel, I make another try to generate parsexml/formatbuf
functions automatically.
============
Directives
============
Still, we need several directives that can direct a tool to generate functions.
These directives work on the declarations of structs. For example:
typedef struct _virNetworkDNSTxtDef virNetworkDNSTxtDef;
typedef virNetworkDNSTxtDef *virNetworkDNSTxtDefPtr;
struct _virNetworkDNSTxtDef { /* genparse:concisehook, genformat */
char *name; /* xmlattr, required */
char *value; /* xmlattr */
};
typedef struct _virNetworkDNSSrvDef virNetworkDNSSrvDef;
typedef virNetworkDNSSrvDef *virNetworkDNSSrvDefPtr;
struct _virNetworkDNSSrvDef { /* genparse:withhook, genformat */
char *service; /* xmlattr */
char *protocol; /* xmlattr */
char *domain; /* xmlattr */
char *target; /* xmlattr */
unsigned int port; /* xmlattr */
unsigned int priority; /* xmlattr */
unsigned int weight; /* xmlattr */
};
typedef struct _virNetworkDNSHostDef virNetworkDNSHostDef;
typedef virNetworkDNSHostDef *virNetworkDNSHostDefPtr;
struct _virNetworkDNSHostDef { /* genparse:withhook, genformat */
virSocketAddr ip; /* xmlattr */
size_t nnames;
char **names; /* xmlelem:hostname, array */
};
typedef struct _virNetworkDNSForwarder virNetworkDNSForwarder;
typedef virNetworkDNSForwarder *virNetworkDNSForwarderPtr;
struct _virNetworkDNSForwarder { /* genparse:withhook, genformat */
char *domain; /* xmlattr */
virSocketAddr addr; /* xmlattr */
};
typedef struct _virNetworkDNSDef virNetworkDNSDef;
typedef virNetworkDNSDef *virNetworkDNSDefPtr;
struct _virNetworkDNSDef { /* genparse:withhook, genformat */
virTristateBool enable; /* xmlattr */
virTristateBool forwardPlainNames; /* xmlattr */
size_t nforwarders;
virNetworkDNSForwarderPtr forwarders; /* xmlelem, array */
size_t ntxts;
virNetworkDNSTxtDefPtr txts; /* xmlelem, array */
size_t nsrvs;
virNetworkDNSSrvDefPtr srvs; /* xmlelem, array */
size_t nhosts;
virNetworkDNSHostDefPtr hosts; /* xmlelem, array */
};
Explanation for these directives:
- genparse[:withhook|concisehook]
Only work on a struct.
Generate parsexml function for this struct only if 'genparse' is specified.
The function name is based on the struct-name and suffixed with 'ParseXML'.
E.g., for struct virNetworkDNSDef, its parsexml function is
virNetworkDNSDefParseXML.
If a parsexml function includes some error-checking code, it needs a
post-process hook to hold them. Specify 'withhook' or 'concisehook' to
setup a hook point in the parsexml function.
Executing the tool manually can show the declaration of hook function.
E.g. check declaration of 'virNetworkDNSDefParseXMLHook'.
# ./build-aux/generator/go show virNetworkDNSDef -kp
int
virNetworkDNSDefParseXMLHook(xmlNodePtr node,
virNetworkDNSDefPtr def,
const char *instname,
void *opaque,
const char *enableStr,
const char *forwardPlainNamesStr,
int nForwarderNodes,
int nTxtNodes,
int nSrvNodes,
int nHostNodes);
Some helper arguments (such as 'enableStr', 'nTxtNodes', etc.) are
passed in to indicate the existence of fields.
If these arguments are useless, specify 'concisehook' to omit them.
When 'genparse' is specified, clear function for this struct is also
generated implicitly, it is because the generated parsexml function needs
to call the clear function.
- genformat
Only work on a struct.
Generate formatbuf function for this struct only if 'genformat' is specified.
The function name is based on struct-name and suffixed with 'FormatBuf'.
- xmlattr[:thename]
Parse/Format the field as an XML attribute.
If 'thename' is specified, use it as the XML attribute name;
or use the filed name.
- xmlelem[:thename]
Parse/Format the field as a child struct.
If 'thename' is specified, use it as the XML element name;
or use the filed name.
- array
Parse/Format the field as an array.
Its related field is a counter, which name should follow the pattern:
n + 'field_name'.
- required
The field must have a corresponding item defined in XML.
Note:
1. If a field isn't specified with 'xmlattr' or 'xmlelem', it will be
ignored in the parsexml/formatbuf functions.
2. For enum, use its name rather than int.
E.g., the type of the field 'enable' in virNetworkDef should be
'virTristateBool' rather than 'int'.
=======
Tool
=======
The Tool is based on libclang and its python-binding.
It has three subcommands: 'list', 'show' and 'generate'.
The 'list' and 'show' are used for previewing generated code;
the 'generate' is prepared to be invoked by Makefile whenever the c header files
under 'src/conf' and 'src/util' have changed.
===================
About this series
===================
To generate all parsexml/formatbuf functions for virNetworkDef and all its
subordinate structs, it needs around 95 patches.
In this RFC, I just post 21 patches for evaluation.
Thanks!
Shi Lei (21):
build-aux: Add a tool to generate xml parse/format functions
maint: Check libclang and its python3 binding
maint: Call generator automatically when c-head-files change
maint: Add helper macro VIR_USED
util: Add two helper functions virXMLChildNode and virXMLChildNodeSet
conf: Extract error-checking code from virNetworkDNSTxtDefParseXML
conf: Replace virNetworkDNSTxtDefParseXML(hardcoded) with
namesake(generated)
conf: Generate virNetworkDNSTxtDefFormatBuf
conf: Extract error-checking code from virNetworkDNSSrvDefParseXML
conf: Replace virNetworkDNSSrvDefParseXML(hardcoded) with
namesake(generated)
conf: Generate virNetworkDNSSrvDefFormatBuf
util: Add parsexml/formatbuf helper functions for virSocketAddr
conf: Extract error-checking code from virNetworkDNSHostDefParseXML
conf: Replace virNetworkDNSHostDefParseXML(hardcoded) with
namesake(generated)
conf: Generate virNetworkDNSHostDefFormatBuf
conf: Extract virNetworkDNSForwarderParseXML from
virNetworkDNSParseXML
conf: Replace virNetworkDNSForwarderParseXML(hardcoded) with
namesake(generated)
conf: Generate virNetworkDNSForwarderFormatBuf
conf: Extract error-checking code from virNetworkDNSDefParseXML
conf: Replace virNetworkDNSDefParseXML(hardcoded) with
namesake(generated)
conf: Generate virNetworkDNSDefFormatBuf
build-aux/generator/directive.py | 839 +++++++++++++++++++++++++++++++
build-aux/generator/go | 14 +
build-aux/generator/main.py | 416 +++++++++++++++
build-aux/generator/utils.py | 100 ++++
configure.ac | 12 +
po/POTFILES.in | 2 +
src/Makefile.am | 15 +
src/access/Makefile.inc.am | 2 +-
src/conf/Makefile.inc.am | 13 +-
src/conf/network_conf.c | 487 ++++--------------
src/conf/network_conf.h | 54 +-
src/esx/Makefile.inc.am | 2 +-
src/interface/Makefile.inc.am | 2 +-
src/internal.h | 2 +
src/lxc/Makefile.inc.am | 1 +
src/network/Makefile.inc.am | 2 +-
src/network/bridge_driver.c | 2 +-
src/node_device/Makefile.inc.am | 2 +-
src/nwfilter/Makefile.inc.am | 2 +-
src/qemu/Makefile.inc.am | 1 +
src/remote/Makefile.inc.am | 2 +-
src/secret/Makefile.inc.am | 2 +-
src/storage/Makefile.inc.am | 2 +-
src/test/Makefile.inc.am | 2 +-
src/util/Makefile.inc.am | 12 +-
src/util/virsocketaddr.c | 38 ++
src/util/virsocketaddr.h | 22 +-
src/util/virxml.c | 57 +++
src/util/virxml.h | 3 +
src/vbox/Makefile.inc.am | 2 +-
tests/Makefile.am | 2 +
tools/Makefile.am | 2 +
32 files changed, 1674 insertions(+), 442 deletions(-)
create mode 100644 build-aux/generator/directive.py
create mode 100755 build-aux/generator/go
create mode 100755 build-aux/generator/main.py
create mode 100644 build-aux/generator/utils.py
--
2.17.1
4 years, 5 months
[libvirt PATCH 0/2] ci: Drop Debian 9
by Andrea Bolognani
The errors caused by it going EOL last week are currently blocking
all CI jobs.
Andrea Bolognani (2):
ci: Drop Debian 9 jobs
ci: Drop Debian 9 containers
.gitlab-ci.yml | 74 ++--------
.../libvirt-debian-9-cross-aarch64.Dockerfile | 128 ------------------
.../libvirt-debian-9-cross-armv6l.Dockerfile | 126 -----------------
.../libvirt-debian-9-cross-armv7l.Dockerfile | 127 -----------------
.../libvirt-debian-9-cross-mips.Dockerfile | 127 -----------------
...libvirt-debian-9-cross-mips64el.Dockerfile | 127 -----------------
.../libvirt-debian-9-cross-mipsel.Dockerfile | 127 -----------------
.../libvirt-debian-9-cross-ppc64le.Dockerfile | 127 -----------------
.../libvirt-debian-9-cross-s390x.Dockerfile | 127 -----------------
ci/containers/libvirt-debian-9.Dockerfile | 118 ----------------
10 files changed, 12 insertions(+), 1196 deletions(-)
delete mode 100644 ci/containers/libvirt-debian-9-cross-aarch64.Dockerfile
delete mode 100644 ci/containers/libvirt-debian-9-cross-armv6l.Dockerfile
delete mode 100644 ci/containers/libvirt-debian-9-cross-armv7l.Dockerfile
delete mode 100644 ci/containers/libvirt-debian-9-cross-mips.Dockerfile
delete mode 100644 ci/containers/libvirt-debian-9-cross-mips64el.Dockerfile
delete mode 100644 ci/containers/libvirt-debian-9-cross-mipsel.Dockerfile
delete mode 100644 ci/containers/libvirt-debian-9-cross-ppc64le.Dockerfile
delete mode 100644 ci/containers/libvirt-debian-9-cross-s390x.Dockerfile
delete mode 100644 ci/containers/libvirt-debian-9.Dockerfile
--
2.25.4
4 years, 5 months
[PATCH v5 0/3] tpm: Fix default choices for CRB and SPAPR dev models
by Stefan Berger
From: Stefan Berger <stefanb(a)linux.ibm.com>
This series of patches adds an additional check for the SPAPR device model
that prevents the choice of a TPM 1.2 backend and chooses a TPM 2 as default.
Also CRB now chooses a TPM 2 as default since TPM 1.2 wouldn't work with it,
either.
Stefan
v4->v5:
- Added R-b's
Stefan Berger (3):
qemu: Move setting of TPM default to post parse function
qemu: Set SPAPR TPM default to 2.0 and prevent 1.2 choice
qemu: Choose TPM 2 for backend as default for CRB interface
src/qemu/qemu_domain.c | 12 +++++++++---
src/qemu/qemu_validate.c | 10 ++++++----
2 files changed, 15 insertions(+), 7 deletions(-)
--
2.17.1
4 years, 5 months
Re: device compatibility interface for live migration with assigned devices
by Sean Mooney
resending with full cc list since i had this typed up
i would blame my email provier but my email client does not seam to like long cc lists.
we probably want to continue on alex's thread to not split the disscusion.
but i have responed inline with some example of how openstack schdules and what i ment by different mdev_types
On Tue, 2020-07-14 at 20:29 +0100, Sean Mooney wrote:
> On Tue, 2020-07-14 at 11:01 -0600, Alex Williamson wrote:
> > On Tue, 14 Jul 2020 13:33:24 +0100
> > Sean Mooney <smooney(a)redhat.com> wrote:
> >
> > > On Tue, 2020-07-14 at 11:21 +0100, Daniel P. Berrangé wrote:
> > > > On Tue, Jul 14, 2020 at 07:29:57AM +0800, Yan Zhao wrote:
> > > > > hi folks,
> > > > > we are defining a device migration compatibility interface that helps upper
> > > > > layer stack like openstack/ovirt/libvirt to check if two devices are
> > > > > live migration compatible.
> > > > > The "devices" here could be MDEVs, physical devices, or hybrid of the two.
> > > > > e.g. we could use it to check whether
> > > > > - a src MDEV can migrate to a target MDEV,
> > >
> > > mdev live migration is completely possible to do but i agree with Dan barrange's comments
> > > from the point of view of openstack integration i dont see calling out to a vender sepecific
> > > tool to be an accpetable
> >
> > As I replied to Dan, I'm hoping Yan was referring more to vendor
> > specific knowledge rather than actual tools.
> >
> > > solutions for device compatiablity checking. the sys filesystem
> > > that describs the mdevs that can be created shoudl also
> > > contain the relevent infomation such
> > > taht nova could integrate it via libvirt xml representation or directly retrive the
> > > info from
> > > sysfs.
> > > > > - a src VF in SRIOV can migrate to a target VF in SRIOV,
> > >
> > > so vf to vf migration is not possible in the general case as there is no standarised
> > > way to transfer teh device state as part of the siorv specs produced by the pci-sig
> > > as such there is not vender neutral way to support sriov live migration.
> >
> > We're not talking about a general case, we're talking about physical
> > devices which have vfio wrappers or hooks with device specific
> > knowledge in order to support the vfio migration interface. The point
> > is that a discussion around vfio device migration cannot be limited to
> > mdev devices.
>
> ok upstream in openstack at least we do not plan to support generic livemigration
> for passthough devivces. we cheat with network interfaces since in generaly operating
> systems handel hotplug of a nic somewhat safely so wehre no abstraction layer like
> an mdev is present or a macvtap device we hot unplug the nic before the migration
> and attach a new one after. for gpus or crypto cards this likely would not be viable
> since you can bond generic hardware devices to hide the removal and readdtion of a generic
> pci device. we were hoping that there would be a convergenca around MDEVs as a way to provide
> that abstraction going forward for generic device or some other new mechanisum in the future.
> >
> > > > > - a src MDEV can migration to a target VF in SRIOV.
> > >
> > > that also makes this unviable
> > > > > (e.g. SIOV/SRIOV backward compatibility case)
> > > > >
> > > > > The upper layer stack could use this interface as the last step to check
> > > > > if one device is able to migrate to another device before triggering a real
> > > > > live migration procedure.
> > >
> > > well actully that is already too late really. ideally we would want to do this compaiablity
> > > check much sooneer to avoid the migration failing. in an openstack envionment at least
> > > by the time we invoke libvirt (assuming your using the libvirt driver) to do the migration we have alreaedy
> > > finished schduling the instance to the new host. if if we do the compatiablity check at this point
> > > and it fails then the live migration is aborted and will not be retired. These types of late check lead to a
> > > poor user experince as unless you check the migration detial it basically looks like the migration was ignored
> > > as it start to migrate and then continuge running on the orgininal host.
> > >
> > > when using generic pci passhotuhg with openstack, the pci alias is intended to reference a single vendor
> > > id/product
> > > id so you will have 1+ alias for each type of device. that allows openstack to schedule based on the availability
> > > of
> > > a
> > > compatibale device because we track inventories of pci devices and can query that when selecting a host.
> > >
> > > if we were to support mdev live migration in the future we would want to take the same declarative approch.
> > > 1 interospec the capability of the deivce we manage
> > > 2 create inventories of the allocatable devices and there capabilities
> > > 3 schdule the instance to a host based on the device-type/capabilities and claim it atomicly to prevent raceces
> > > 4 have the lower level hyperviors do addtional validation if need prelive migration.
> > >
> > > this proposal seams to be targeting extending step 4 where as ideally we should focuse on providing the info that
> > > would
> > > be relevant in set 1 preferably in a vendor neutral way vai a kernel interface like /sys.
> >
> > I think this is reading a whole lot into the phrase "last step". We
> > want to make the information available for a management engine to
> > consume as needed to make informed decisions regarding likely
> > compatible target devices.
>
> well openstack as a management engin has 3 stages for schdule and asignment,.
> in respocne to a live migration request the api does minimal valaidation then hand the task off to the conductor
> service
> ot orchestrate. the conductor invokes an rpc to the schduler service which makes a rest call to the plamcent service.
> the placment cervice generate a set of allocation candiate for host based on qunataive and qulaitivly
> queries agains an abstract resouce provider tree model of the hosts.
> currently device pasthough is not modeled in placment so plamcnet is basicaly returning a set of host that have enough
> cpu ram and disk for the instance. in the spacial of vGPU they technically are modelled in placement but not in a way
> that would gurarentee compatiablity for migration. a generic pci device request is haneled in the second phase of
> schduling called filtering and weighing. in this pahse the nova schuleer apply a series of filter to the list of host
> returned by plamcnet to assert things like anit afintiy, tenant isolation or in the case of this converation nuam
> affintiy and pci device avaiablity. when we have filtered the posible set of host down to X number we weigh the
> listing
> to select an optimal host and set of alternitive hosts. we then enter the code that this mail suggest modfiying which
> does an rpc call to the destiation host form teh conductor to have it assert compatiablity which internaly calls back
> to
> the sourc host.
>
> so my point is we have done a lot of work by the time we call check_can_live_migrate_destination and failing
> at this point is considerd quite a late failure but its still better then failing when qemu actully tries to migrate.
> in general we would prefer to move compatiablity check as early in that workflow as possible but to be fair we dont
> actully check cpu model compatiablity until check_can_live_migrate_destination.
>
https://github.com/openstack/nova/blob/8988316b8c132c9662dea6cf0345975e87...
>
> if we needed too we could read the version string on the source and write the version string on the dest at this
> point.
> doing so however would be considerd, inelegant, we have found this does not scale as the first copmpatabilty check.
> for cpu for example there are way to filter hosts by groups sets fo host with the same cpu or filtering on cpu feature
> flags that happen in the placment or filter stage both of which are very early and cheap to do at runtime.
>
> the "read for version, write for compatibility" workflow could be used as a final safe check if required but
> probing for compatibility via writes is basicaly considered an anti patteren in openstack. we try to always
> assert compatibility by reading avaiable info and asserting requirement over it not testing to see if it works.
>
> this has come up in the past in the context of virtio feature flag where the idea of spawning an instrance or trying
> to add a virtio port to ovs dpdk that reqested a specific feature flag was rejected as unacceptable from a performance
> and security point of view.
>
> >
> > > > > we are not sure if this interface is of value or help to you. please don't
> > > > > hesitate to drop your valuable comments.
> > > > >
> > > > >
> > > > > (1) interface definition
> > > > > The interface is defined in below way:
> > > > >
> > > > > __ userspace
> > > > > /\ \
> > > > > / \write
> > > > > / read \
> > > > > ________/__________ ___\|/_____________
> > > > > | migration_version | | migration_version |-->check migration
> > > > > --------------------- --------------------- compatibility
> > > > > device A device B
> > > > >
> > > > >
> > > > > a device attribute named migration_version is defined under each device's
> > > > > sysfs node. e.g. (/sys/bus/pci/devices/0000\:00\:02.0/$mdev_UUID/migration_version).
> > >
> > > this might be useful as we could tag the inventory with the migration version and only might to
> > > devices with the same version
> >
> > Is cross version compatibility something that you'd consider using?
>
> yes but it would depend on what cross version actully ment.
>
> the version of an mdev is not something we would want to be exposed to endusers.
> it would be a security risk to do so as the version sting would potentaily allow the untrused user
> to discover if a device has an unpatch vulnerablity. as a result in the context of live migration
> we can only support cross verion compatiabilyt if the device in the guest does not alter as
> part of the migration and the behavior does not change.
>
> going form version 1.0 with feature X to verions 1.1 with feature X and Y but only X enabled would
> be fine. going gorm 1.0 to 2.0 where thre is only feature Y would not be ok.
> being abstract makes it a little harder to readabout but i guess i would sumerisei if its
> transparent to the guest for the lifetime of the qemu process then its ok for the backing version to change.
> if a vm is rebooted its also ok fo the vm to pick up feature Y form the 1.1 device although at that point
> it could not be migrated back to the 1.0 host as it now has feature X and Y and 1.0 only has X so that woudl be
> an obserable change if it was drop as a reult of the live migration.
> >
> > > > > userspace tools read the migration_version as a string from the source device,
> > > > > and write it to the migration_version sysfs attribute in the target device.
> > >
> > > this would not be useful as the schduler cannot directlly connect to the compute host
> > > and even if it could it would be extreamly slow to do this for 1000s of hosts and potentally
> > > multiple devices per host.
> >
> > Seems similar to Dan's requirement, looks like the 'read for version,
> > write for compatibility' test idea isn't really viable.
>
> its ineffiecnt and we have reject adding such test in the case of virtio-feature flag compatiabilty
> in the past, so its more an option of last resourt if we have no other way to support compatiablity
> checking.
> >
> > > > >
> > > > > The userspace should treat ANY of below conditions as two devices not compatible:
> > > > > - any one of the two devices does not have a migration_version attribute
> > > > > - error when reading from migration_version attribute of one device
> > > > > - error when writing migration_version string of one device to
> > > > > migration_version attribute of the other device
> > > > >
> > > > > The string read from migration_version attribute is defined by device vendor
> > > > > driver and is completely opaque to the userspace.
> > >
> > > opaque vendor specific stings that higher level orchestros have to pass form host
> > > to host and cant reason about are evil, when allowed they prolifroate and
> > > makes any idea of a vendor nutral abstraction and interoperablity between systems
> > > impossible to reason about. that said there is a way to make it opaue but still useful
> > > to userspace. see below
> > > > > for a Intel vGPU, string format can be defined like
> > > > > "parent device PCI ID" + "version of gvt driver" + "mdev type" + "aggregator count".
> > > > >
> > > > > for an NVMe VF connecting to a remote storage. it could be
> > > > > "PCI ID" + "driver version" + "configured remote storage URL"
> > > > >
> > > > > for a QAT VF, it may be
> > > > > "PCI ID" + "driver version" + "supported encryption set".
> > > > >
> > > > > (to avoid namespace confliction from each vendor, we may prefix a driver name to
> > > > > each migration_version string. e.g. i915-v1-8086-591d-i915-GVTg_V5_8-1)
> > >
> > > honestly i would much prefer if the version string was just a semver string.
> > > e.g. {major}.{minor}.{bugfix}
> > >
> > > if you do a driver/frimware update and break compatiablity with an older version bump the
> > > major version.
> > >
> > > if you add optional a feature that does not break backwards compatiablity if you migrate
> > > an older instance to the new host then just bump the minor/feature number.
> > >
> > > if you have a fix for a bug that does not change the feature set or compatiblity backwards or
> > > forwards then bump the bugfix number
> > >
> > > then the check is as simple as
> > > 1.) is the mdev type the same
> > > 2.) is the major verion the same
> > > 3.) am i going form the same version to same version or same version to newer version
> > >
> > > if all 3 are true we can migrate.
> > > e.g.
> > > 2.0.1 -> 2.1.1 (ok same major version and migrating from older feature release to newer feature release)
> > > 2.1.1 -> 2.0.1 (not ok same major version and migrating from new feature release to old feature release may be
> > > incompatable)
> > > 2.0.0 -> 3.0.0 (not ok chaning major version)
> > > 2.0.1 -> 2.0.0 (ok same major and minor version, all bugfixs in the same minor release should be compatibly)
> >
> > What's the value of the bugfix field in this scheme?
>
> its not require but really its for a non visable chagne form a feature standpoint.
> a rather contrived example but if it was quadratic to inital a set of queues or device bufferes
> in 1.0.0 and you made it liniar in 1.0.1 that is a performace improvment in the device intialisation time
> which is great but it would not affect the feature set or compatiablity in any way. you could call it
> a feature but its really just an internal change but you might want to still bump the version number.
> >
> > The simplicity is good, but is it too simple. It's not immediately
> > clear to me whether all features can be hidden behind a minor version.
> > For instance, if we have an mdev device that supports this notion of
> > aggregation, which is proposed as a solution to the problem that
> > physical hardware might support lots and lots of assignable interfaces
> > which can be combined into arbitrary sets for mdev devices, making it
> > impractical to expose an mdev type for every possible enumeration of
> > assignable interfaces within a device.
>
> so this is a modeling problem and likely a limitation of the current way an mdev_type is exposed.
> stealing some linux doc eamples
>
>
> |- [parent physical device]
> |--- Vendor-specific-attributes [optional]
> |--- [mdev_supported_types]
> | |--- [<type-id>]
> | | |--- create
> | | |--- name
> | | |--- available_instances
> | | |--- device_api
> | | |--- description
>
> you could adress this in 1 of at least 3 ways.
> 1.) mdev type for each enmartion which is fine for 1-2 variabley othersize its a combinitroial explotions.
> 2.) report each of the consomable sub componetns as an mdev type and create mupltipel mdevs and assign them to the vm.
> 3.) provider an api to dynamically compose mdevs types which staticaly partion the reqouese and can then be consomed
> perferably embeding the resouce infomation in the description filed in a huma/machince readable form.
>
> 2 and 3 woudl work well with openstack however they both have there challanges
> 1 doesnt really work for anyone out side of a demo.
> > We therefore expose a base type
> > where the aggregation is built later. This essentially puts us in a
> > scenario where even within an mdev type running on the same driver,
> > there are devices that are not directly compatible with each other.
> >
> > > we dont need vendor to rencode the driver name or vendor id and product id in the string. that info is alreay
> > > available both to the device driver and to userspace via /sys already we just need to know if version of
> > > the same mdev are compatiable so a simple semver version string which is well know in the software world
> > > at least is a clean abstration we can reuse.
> >
> > This presumes there's no cross device migration.
>
> no but it does assume no cross mdev_type migration.
> it assuems that nvida_mdev_type_x on host 1 is the same as nvida_mdev_type_x on host 2.
> if the parent device differese but support the same mdev type we are asserting that they
> should be compatiable or a differnt mdev_type name should be used on each device.
>
> so we are presuming the mdev type cant change as part of a live migration and if the type
> was to change it would no longer be a live migration operation it would be something else.
> that is based on the premis that changing the mdev type would change the capabilities of the mdev
>
> > An mdev type can only
> > be migrated to the same mdev type, all of the devices within that type
> > have some based compatibility, a phsyical device can only be migrated to
> > the same physical device. In the latter case what defines the type?
>
> the type-id in /sysfs
>
> /sys/devices/virtual/mtty/mtty/
> |-- mdev_supported_types
> | |-- mtty-1 <---- this is an mdev type
> | | |-- available_instances
> | | |-- create
> | | |-- device_api
> | | |-- devices
> | | `-- name
> | `-- mtty-2 <---- as is this
> | |-- available_instances
> | |-- create
> | |-- device_api
> | |-- devices
> | `-- name
>
> |- [parent phy device]
> |--- [$MDEV_UUID]
> |--- remove
> |--- mdev_type {link to its type} <-- here
> |--- vendor-specific-attributes [optional]
>
> > If
> > it's a PCI device, is it only vendor:device IDs?
>
> no the mdev type is not defined by the vendor:device id of the parent device
> although the capablityes of that device will determin what mdev types if any it supprots.
> > What about revision?
> > What about subsystem IDs?
>
> at least for nvidia gpus i dont think if you by an evga branded v100 vs an pny branded one the capability
> would change but i do know that certenly the capablities of a dell branding intel nic and an intel branded
> one can. e.g. i have seen oem sku nics without sriov eventhoguh the same nic form intel supports it.
> sriov was deliberatly disabled in the dell firmware even though it share dhte same vendor and prodcut id but differnt
> subsystem id.
>
> if the odm made an incomatipable change like that which affect an mdev type in some way i guess i would expect them to
> change the name or the description filed content to signal that.
>
> > What about possibly an onboard ROM or
> > internal firmware?
>
> i would expect that updating the firmware/rom could result in changing a version string. that is how i was imagining
> it would change.
> > The information may be available, but which things
> > are relevant to migration?
>
> that i dont know an i really would not like to encode that knolage in the vendor specific way in higher level
> tools like openstack or even libvirt. declarative version sting comparisons or even simile feature flag
> check where an abstract huristic that can be applied across vendors would be fine. but yes i dont know
> what info would be needed in this case.
> > We already see desires to allow migration
> > between physical and mdev,
>
> migration between a phsical device and an mdev would not generally be considered a live migration in openstack.
> that would be a different operation as it would be user visible withing the guest vm.
> > but also to expose mdev types that might be
> > composable to be compatible with other types. Thanks,
>
> i think composable mdev types are really challanging without some kind of feature flag concept
> like cpu flags or ethtool nic capablities that are both human readable and easily parsable.
>
> we have the capability to schedule on cpu flags or gpu cuda level using a traits abstraction
> so instead of saying i want an vm on a host with an intel 2695v3 to ensure it has AVX
> you say i want an vm that is capable of using AVX
> https://github.com/openstack/os-traits/blob/master/os_traits/hw/cpu/x86/_...
>
> we also have trait for cuda level so instead of asking for a specifc mdev type or nvida
> gpu the idea was you woudl describe what feature cuda in this exmple you need
> https://github.com/openstack/os-traits/blob/master/os_traits/hw/gpu/cuda....
>
> That is what we call qualitative schudleing and is why we create teh placement service.
> with out going in to the weeds we try to decouple quantaitive request such as 4 cpus and 1G of ram
> form the qunative i need AVX supprot
>
> e.g. resouces:VCPU=4,resouces:MEMORY_MB=1024 triats:required=HW_CPU_X86_AVX
>
> declarative quantitive and capablites reporting of resouces fits easily into that model.
> dynamic quantities that change as other mdev are allocated from the parent device or as
> new mdevs types are composed on the fly are very challenging.
>
> >
> > Alex
> >
>
>
4 years, 5 months
[PATCH] docs: bhyve: document ignoring unknown MSRs
by Roman Bogorodskiy
Ignoring unknown MSRs using <features> element
<msrs unknown='ignore'/> was supported for quite some already,
so add documentation for it for completeness of flags coverage,
as some guests can be extra picky about flags passed to bhyve,
and it's useful to know how to control those.
Signed-off-by: Roman Bogorodskiy <bogorodskiy(a)gmail.com>
---
docs/drvbhyve.html.in | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/docs/drvbhyve.html.in b/docs/drvbhyve.html.in
index 2e9cf5551b..66a13be1f6 100644
--- a/docs/drvbhyve.html.in
+++ b/docs/drvbhyve.html.in
@@ -462,6 +462,24 @@ Example:</p>
</domain>
</pre>
+<h3><a id="msrs">Ignoring unknown MSRs reads and writes</a></h3>
+
+<p><span class="since">Since 5.1.0</span>, it's possible to make bhyve
+ignore accesses to unimplemented Model Specific Registers (MSRs).
+Example:</p>
+
+<pre>
+<domain type="bhyve">
+ ...
+ <features>
+ ...
+ <msrs unknown='ignore'/>
+ ...
+ </features>
+ ...
+</domain>
+</pre>
+
<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.27.0
4 years, 5 months
[PATCH v3] conf: add 'isa' controller type
by Roman Bogorodskiy
Decided to revive this patch to address
https://gitlab.com/libvirt/libvirt/-/issues/45.
Changes from v2:
- Rebased to master,
- Added bhyveDomainDeviceDefValidate() to disallow ISA
controllers idx other than 0 because we don't support
more than one ISA controller.
----
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.
bhyve: support 'isa' controller for LPC
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.
bhyve: automatically add 'isa' controller
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.
bhyve: validate that ISA controller always has index 0
More than one ISA controller is not supported by bhyve,
and multiple controllers with the same index are forbidden,
so forbid ISA controllers with non-zero index for bhyve.
Signed-off-by: Roman Bogorodskiy <bogorodskiy(a)gmail.com>
---
docs/schemas/domaincommon.rng | 6 ++++
src/bhyve/bhyve_command.c | 27 +++++++-------
src/bhyve/bhyve_device.c | 23 +++++++++---
src/bhyve/bhyve_domain.c | 25 +++++++++++--
src/bhyve/bhyve_domain.h | 2 --
src/conf/domain_conf.c | 9 +++++
src/conf/domain_conf.h | 8 +++++
src/qemu/qemu_command.c | 1 +
src/qemu/qemu_domain.c | 1 +
src/qemu/qemu_domain_address.c | 1 +
src/qemu/qemu_validate.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, 406 insertions(+), 37 deletions(-)
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml
create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml
create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml
create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4b4aa60c66..7bed99b161 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2416,6 +2416,12 @@
</attribute>
</optional>
</group>
+ <!-- isa -->
+ <group>
+ <attribute name="type">
+ <value>isa</value>
+ </attribute>
+ </group>
<!-- pci has an optional attribute "model" -->
<group>
<attribute name="type">
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index 22d0b24ec4..2a3e10d649 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -329,7 +329,8 @@ bhyveBuildControllerArgStr(const virDomainDef *def,
virDomainControllerDefPtr controller,
bhyveConnPtr driver,
virCommandPtr cmd,
- unsigned *nusbcontrollers)
+ unsigned *nusbcontrollers,
+ unsigned *nisacontrollers)
{
switch (controller->type) {
case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
@@ -354,18 +355,20 @@ bhyveBuildControllerArgStr(const virDomainDef *def,
if (bhyveBuildUSBControllerArgStr(def, controller, cmd) < 0)
return -1;
break;
+ case VIR_DOMAIN_CONTROLLER_TYPE_ISA:
+ if (++*nisacontrollers > 1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ "%s", _("only single ISA controller is supported"));
+ return -1;
+ }
+ virCommandAddArg(cmd, "-s");
+ virCommandAddArgFormat(cmd, "%d:0,lpc",
+ controller->info.addr.pci.slot);
+ break;
}
return 0;
}
-static int
-bhyveBuildLPCArgStr(const virDomainDef *def G_GNUC_UNUSED,
- virCommandPtr cmd)
-{
- virCommandAddArgList(cmd, "-s", "1,lpc", NULL);
- return 0;
-}
-
static int
bhyveBuildGraphicsArgStr(const virDomainDef *def,
virDomainGraphicsDefPtr graphics,
@@ -490,6 +493,7 @@ virBhyveProcessBuildBhyveCmd(bhyveConnPtr driver, virDomainDefPtr def,
virCommandPtr cmd = virCommandNew(BHYVE);
size_t i;
unsigned nusbcontrollers = 0;
+ unsigned nisacontrollers = 0;
unsigned nvcpus = virDomainDefGetVcpus(def);
/* CPUs */
@@ -595,7 +599,7 @@ virBhyveProcessBuildBhyveCmd(bhyveConnPtr driver, virDomainDefPtr def,
/* Devices */
for (i = 0; i < def->ncontrollers; i++) {
if (bhyveBuildControllerArgStr(def, def->controllers[i], driver, cmd,
- &nusbcontrollers) < 0)
+ &nusbcontrollers, &nisacontrollers) < 0)
goto error;
}
for (i = 0; i < def->nnets; i++) {
@@ -619,9 +623,6 @@ virBhyveProcessBuildBhyveCmd(bhyveConnPtr driver, virDomainDefPtr def,
}
}
- if (bhyveDomainDefNeedsISAController(def))
- bhyveBuildLPCArgStr(def, cmd);
-
if (bhyveBuildConsoleArgStr(def, cmd) < 0)
goto error;
diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c
index fc52280361..52a055f205 100644
--- a/src/bhyve/bhyve_device.c
+++ b/src/bhyve/bhyve_device.c
@@ -46,10 +46,16 @@ bhyveCollectPCIAddress(virDomainDefPtr def G_GNUC_UNUSED,
if (addr->slot == 0) {
return 0;
} else if (addr->slot == 1) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("PCI bus 0 slot 1 is reserved for the implicit "
- "LPC PCI-ISA bridge"));
- return -1;
+ if (!(device->type == VIR_DOMAIN_DEVICE_CONTROLLER &&
+ device->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("PCI bus 0 slot 1 is reserved for the implicit "
+ "LPC PCI-ISA bridge"));
+ return -1;
+ } else {
+ /* We reserve slot 1 for LPC in bhyveAssignDevicePCISlots(), so exit early */
+ return 0;
+ }
}
}
@@ -101,6 +107,15 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def,
return -1;
}
+ for (i = 0; i < def->ncontrollers; i++) {
+ if ((def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) &&
+ 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/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c
index 95d6ffd31c..91994c3da5 100644
--- a/src/bhyve/bhyve_domain.c
+++ b/src/bhyve/bhyve_domain.c
@@ -59,13 +59,13 @@ virDomainXMLPrivateDataCallbacks virBhyveDriverPrivateDataCallbacks = {
.free = bhyveDomainObjPrivateFree,
};
-bool
+static bool
bhyveDomainDefNeedsISAController(virDomainDefPtr def)
{
if (def->os.bootloader == NULL && def->os.loader)
return true;
- if (def->nserials)
+ if (def->nserials || def->nconsoles)
return true;
if (def->ngraphics && def->nvideos)
@@ -95,6 +95,11 @@ bhyveDomainDefPostParse(virDomainDefPtr def,
VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 0)
return -1;
+ if (bhyveDomainDefNeedsISAController(def))
+ if (virDomainDefMaybeAddController(def, VIR_DOMAIN_CONTROLLER_TYPE_ISA, 0,
+ VIR_DOMAIN_CONTROLLER_MODEL_ISA_DEFAULT) < 0)
+ return -1;
+
return 0;
}
@@ -191,10 +196,26 @@ virBhyveDriverCreateXMLConf(bhyveConnPtr driver)
NULL, NULL);
}
+
+static int
+bhyveDomainDeviceDefValidate(const virDomainDeviceDef *dev,
+ const virDomainDef *def G_GNUC_UNUSED,
+ void *opaque G_GNUC_UNUSED)
+{
+ if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER &&
+ dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA &&
+ dev->data.controller->idx != 0) {
+ return -1;
+ }
+
+ return 0;
+}
+
virDomainDefParserConfig virBhyveDriverDomainDefParserConfig = {
.devicesPostParseCallback = bhyveDomainDeviceDefPostParse,
.domainPostParseCallback = bhyveDomainDefPostParse,
.assignAddressesCallback = bhyveDomainDefAssignAddresses,
+ .deviceValidateCallback = bhyveDomainDeviceDefValidate,
};
static void
diff --git a/src/bhyve/bhyve_domain.h b/src/bhyve/bhyve_domain.h
index e985278041..4e2b11b8de 100644
--- a/src/bhyve/bhyve_domain.h
+++ b/src/bhyve/bhyve_domain.h
@@ -39,5 +39,3 @@ virDomainXMLOptionPtr virBhyveDriverCreateXMLConf(bhyveConnPtr);
extern virDomainXMLPrivateDataCallbacks virBhyveDriverPrivateDataCallbacks;
extern virDomainDefParserConfig virBhyveDriverDomainDefParserConfig;
extern virXMLNamespace virBhyveDriverDomainXMLNamespace;
-
-bool bhyveDomainDefNeedsISAController(virDomainDefPtr def);
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d14485f18d..6befe4bbd8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -399,6 +399,7 @@ VIR_ENUM_IMPL(virDomainController,
"usb",
"pci",
"xenbus",
+ "isa",
);
VIR_ENUM_IMPL(virDomainControllerModelPCI,
@@ -444,6 +445,9 @@ VIR_ENUM_IMPL(virDomainControllerModelSCSI,
"virtio-non-transitional",
);
+VIR_ENUM_IMPL(virDomainControllerModelISA, VIR_DOMAIN_CONTROLLER_MODEL_ISA_LAST,
+);
+
VIR_ENUM_IMPL(virDomainControllerModelUSB,
VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST,
"piix3-uhci",
@@ -2312,6 +2316,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;
}
@@ -10975,6 +10980,8 @@ virDomainControllerModelTypeFromString(const virDomainControllerDef *def,
return virDomainControllerModelIDETypeFromString(model);
else if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL)
return virDomainControllerModelVirtioSerialTypeFromString(model);
+ else if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA)
+ return virDomainControllerModelISATypeFromString(model);
return -1;
}
@@ -10994,6 +11001,8 @@ virDomainControllerModelTypeToString(virDomainControllerDefPtr def,
return virDomainControllerModelIDETypeToString(model);
else if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL)
return virDomainControllerModelVirtioSerialTypeToString(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 6a737591e2..188385cc6b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -595,6 +595,7 @@ typedef enum {
VIR_DOMAIN_CONTROLLER_TYPE_USB,
VIR_DOMAIN_CONTROLLER_TYPE_PCI,
VIR_DOMAIN_CONTROLLER_TYPE_XENBUS,
+ VIR_DOMAIN_CONTROLLER_TYPE_ISA,
VIR_DOMAIN_CONTROLLER_TYPE_LAST
} virDomainControllerType;
@@ -686,6 +687,12 @@ typedef enum {
VIR_DOMAIN_CONTROLLER_MODEL_VIRTIO_SERIAL_LAST
} virDomainControllerModelVirtioSerial;
+typedef enum {
+ VIR_DOMAIN_CONTROLLER_MODEL_ISA_DEFAULT = -1,
+
+ 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 || \
@@ -3547,6 +3554,7 @@ VIR_ENUM_DECL(virDomainControllerModelSCSI);
VIR_ENUM_DECL(virDomainControllerModelUSB);
VIR_ENUM_DECL(virDomainControllerModelIDE);
VIR_ENUM_DECL(virDomainControllerModelVirtioSerial);
+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 24e99e13b8..d4e190a1ae 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2699,6 +2699,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
case VIR_DOMAIN_CONTROLLER_TYPE_XENBUS:
+ 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 2d9d8226d6..f5dbadb323 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5054,6 +5054,7 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont,
case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
case VIR_DOMAIN_CONTROLLER_TYPE_XENBUS:
+ 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 058cbda2a2..4e548d2f43 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -677,6 +677,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
case VIR_DOMAIN_CONTROLLER_TYPE_CCID:
case VIR_DOMAIN_CONTROLLER_TYPE_XENBUS:
+ case VIR_DOMAIN_CONTROLLER_TYPE_ISA:
case VIR_DOMAIN_CONTROLLER_TYPE_LAST:
return 0;
}
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index bd7590a00a..b5f9ab7ed4 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -3220,6 +3220,7 @@ qemuValidateDomainDeviceDefController(const virDomainControllerDef *controller,
case VIR_DOMAIN_CONTROLLER_TYPE_CCID:
case VIR_DOMAIN_CONTROLLER_TYPE_USB:
case VIR_DOMAIN_CONTROLLER_TYPE_XENBUS:
+ case VIR_DOMAIN_CONTROLLER_TYPE_ISA:
case VIR_DOMAIN_CONTROLLER_TYPE_LAST:
break;
}
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 15f8eb074a..9b38963e2a 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -397,6 +397,7 @@ vboxSetStorageController(virDomainControllerDefPtr controller,
case VIR_DOMAIN_CONTROLLER_TYPE_USB:
case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
case VIR_DOMAIN_CONTROLLER_TYPE_XENBUS:
+ 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"),
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..4a72ca65a1
--- /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='0'>
+ <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..3b8e8a3664
--- /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='0'>
+ <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-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-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..279ac6669a
--- /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='0'/>
+ <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..d087d5fb4c
--- /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'/>
+ <controller type='isa' index='2'/>
+ <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-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/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c
index 9e7eb218b8..6e8b62dc9b 100644
--- a/tests/bhyvexml2argvtest.c
+++ b/tests/bhyvexml2argvtest.c
@@ -210,6 +210,8 @@ mymain(void)
DO_TEST_FAILURE("cputopology-nvcpu-mismatch");
DO_TEST("commandline");
DO_TEST("msrs");
+ DO_TEST("isa-controller");
+ DO_TEST_FAILURE("isa-multiple-controllers");
/* Address allocation tests */
DO_TEST("addr-single-sata-disk");
@@ -217,6 +219,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..c1424a65e6
--- /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='0'>
+ <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..f99c5b6431
--- /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='0'>
+ <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-console.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-console.xml
index 78d4d30016..5309bdc7cf 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='0'>
+ <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-isa-controller.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml
new file mode 100644
index 0000000000..c1424a65e6
--- /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='0'>
+ <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-serial-grub-nocons.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-serial-grub-nocons.xml
index 845cb09e9f..f89d678abd 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='0'>
+ <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..29a36499f5 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='0'>
+ <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..ec03da2bfe 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='0'>
+ <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..041ab4e319 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='0'>
+ <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..58e71282e7 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='0'>
+ <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..2c37762edb 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='0'>
+ <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..a9f37dca66 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='0'>
+ <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..58e71282e7 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='0'>
+ <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/bhyvexml2xmltest.c b/tests/bhyvexml2xmltest.c
index a0c20a14c1..cc04f8e3ce 100644
--- a/tests/bhyvexml2xmltest.c
+++ b/tests/bhyvexml2xmltest.c
@@ -110,6 +110,7 @@ mymain(void)
DO_TEST_DIFFERENT("vnc-autoport");
DO_TEST_DIFFERENT("commandline");
DO_TEST_DIFFERENT("msrs");
+ DO_TEST_DIFFERENT("isa-controller");
/* Address allocation tests */
DO_TEST_DIFFERENT("addr-single-sata-disk");
@@ -117,6 +118,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.27.0
4 years, 5 months