Re: [PATCH v5 2/2] bhyve: support 'isa' controller for LPC

2020-09-17 Thread Daniel P . Berrangé
On Thu, Sep 17, 2020 at 06:55:36PM +0400, Roman Bogorodskiy wrote:
>   Daniel P. Berrangé wrote:
> 
> > On Thu, Sep 17, 2020 at 05:48:38PM +0400, Roman Bogorodskiy wrote:
> > >   Daniel P. Berrangé wrote:
> > > 
> > > > On Wed, Sep 16, 2020 at 07:14:39PM +0400, Roman Bogorodskiy wrote:
> > > > >   Daniel P. Berrangé wrote:
> > > > > 
> > > > > > On Sat, Aug 01, 2020 at 10:16:57AM +0400, Roman Bogorodskiy wrote:
> > > > > > > diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c
> > > > > > > index fc52280361..52a055f205 100644
> > > > > > > --- a/src/bhyve/bhyve_device.c
> > > > > > > +++ b/src/bhyve/bhyve_device.c
> > > > > > > @@ -46,10 +46,16 @@ bhyveCollectPCIAddress(virDomainDefPtr def 
> > > > > > > G_GNUC_UNUSED,
> > > > > > >  if (addr->slot == 0) {
> > > > > > >  return 0;
> > > > > > >  } else if (addr->slot == 1) {
> > > > > > > -virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > > > > > > -   _("PCI bus 0 slot 1 is reserved for 
> > > > > > > the implicit "
> > > > > > > - "LPC PCI-ISA bridge"));
> > > > > > > -return -1;
> > > > > > > +if (!(device->type == VIR_DOMAIN_DEVICE_CONTROLLER &&
> > > > > > > +  device->data.controller->type == 
> > > > > > > VIR_DOMAIN_CONTROLLER_TYPE_ISA)) {
> > > > > > > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > > > > > > +_("PCI bus 0 slot 1 is reserved 
> > > > > > > for the implicit "
> > > > > > > +  "LPC PCI-ISA bridge"));
> > > > > > > + return -1;
> > > > > > > +} else {
> > > > > > > +/* We reserve slot 1 for LPC in 
> > > > > > > bhyveAssignDevicePCISlots(), so exit early */
> > > > > > > +return 0;
> > > > > > > +}
> > > > > > 
> > > > > > I forgot to respond to your followup comments on v4
> > > > > > https://www.redhat.com/archives/libvir-list/2020-July/msg01761.html
> > > > > > 
> > > > > > > > 
> > > > > > > > IIUC, this series makes it possible to put the TPC in a 
> > > > > > > > different
> > > > > > > > slot, so does it still make sense to forbid use of slot 1 as a
> > > > > > > > hardcoded rule ?
> > > > > > >
> > > > > > > IIRC, the idea behind that is to give some time window for users 
> > > > > > > to
> > > > > > > allow moving guests from the new version to the old one. If we 
> > > > > > > allow to
> > > > > > > use slot 1, it won't be possible to move the guest to the old 
> > > > > > > libvirt as
> > > > > > > it will complain slot 1 should be used only for LPC.
> > > > > > 
> > > > > > If the user has decided to move their LPC to a slot != 1, then it is
> > > > > > already impossible to migrate the guest to an old libvirt.
> > > > > > 
> > > > > > If the user wants to explicitly specify another device in slot 1, 
> > > > > > then
> > > > > > we should not prevent that.
> > > > > > 
> > > > > > We just need to make sure that if no LPC is in the XML, and no other
> > > > > > device explicitly has slot 1, then make sure to auto-assign LPC in 
> > > > > > slot 1
> > > > > > not some other device.
> > > > > 
> > > > > I've started playing with that and remembered some more corner cases.
> > > > > 
> > > > > To elaborate on your example, i.e. "no explicit LPC in XML AND no 
> > > > > other
> > > > > device explicitly on slot 1": these conditions are not specific 
> > > > > enough to
> > > > > tell whether an LPC device will be added or not.
> > > > > 
> > > > > In case if the LPC device was not explicitly specified by a user,
> > > > > the bhyve driver tries to add it if it's needed (it's required
> > > > > for serial console, bootloader, and video devices;
> > > > > see bhyveDomainDefNeedsISAController()). Otherwise a domain will start
> > > > > without the LPC device.
> > > > > 
> > > > > This could lead to a case when a user starts a domain in configuration
> > > > > that does not require LPC device, but has e.g. a network device on
> > > > > a PCI controller that's auto-assigned to slot 1.
> > > > > 
> > > > > Later user decides to change the configuration and adds a video 
> > > > > device,
> > > > > which requires LPC. This will lead to addresses changes, as LPC will 
> > > > > go
> > > > > to slot 1 and a network device's controller will go from slot 1 to 
> > > > > slot
> > > > > 2, which could be troublesome in some guest OSes.
> > > > 
> > > > First of all, lets me clear that we're talking about persistent guests
> > > > here, not transient guests.
> > > > 
> > > > With a persistent guest, the PCI addresses are assigned at the time
> > > > the XML arrives in virDomainDefineXML. If nothing requires the LPC
> > > > at this time, then a NIC could get given slot 1. This is recorded in
> > > > the persistent XML.
> > > > 
> > > > If the user later uses 'virsh edit' to modify the XML and add a video
> > > > device, libvirt will see that the 

Re: [PATCH v5 2/2] bhyve: support 'isa' controller for LPC

2020-09-17 Thread Roman Bogorodskiy
  Daniel P. Berrangé wrote:

> On Thu, Sep 17, 2020 at 05:48:38PM +0400, Roman Bogorodskiy wrote:
> >   Daniel P. Berrangé wrote:
> > 
> > > On Wed, Sep 16, 2020 at 07:14:39PM +0400, Roman Bogorodskiy wrote:
> > > >   Daniel P. Berrangé wrote:
> > > > 
> > > > > On Sat, Aug 01, 2020 at 10:16:57AM +0400, Roman Bogorodskiy wrote:
> > > > > > diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c
> > > > > > index fc52280361..52a055f205 100644
> > > > > > --- a/src/bhyve/bhyve_device.c
> > > > > > +++ b/src/bhyve/bhyve_device.c
> > > > > > @@ -46,10 +46,16 @@ bhyveCollectPCIAddress(virDomainDefPtr def 
> > > > > > G_GNUC_UNUSED,
> > > > > >  if (addr->slot == 0) {
> > > > > >  return 0;
> > > > > >  } else if (addr->slot == 1) {
> > > > > > -virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > > > > > -   _("PCI bus 0 slot 1 is reserved for the 
> > > > > > implicit "
> > > > > > - "LPC PCI-ISA bridge"));
> > > > > > -return -1;
> > > > > > +if (!(device->type == VIR_DOMAIN_DEVICE_CONTROLLER &&
> > > > > > +  device->data.controller->type == 
> > > > > > VIR_DOMAIN_CONTROLLER_TYPE_ISA)) {
> > > > > > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > > > > > +_("PCI bus 0 slot 1 is reserved 
> > > > > > for the implicit "
> > > > > > +  "LPC PCI-ISA bridge"));
> > > > > > + return -1;
> > > > > > +} else {
> > > > > > +/* We reserve slot 1 for LPC in 
> > > > > > bhyveAssignDevicePCISlots(), so exit early */
> > > > > > +return 0;
> > > > > > +}
> > > > > 
> > > > > I forgot to respond to your followup comments on v4
> > > > > https://www.redhat.com/archives/libvir-list/2020-July/msg01761.html
> > > > > 
> > > > > > > 
> > > > > > > IIUC, this series makes it possible to put the TPC in a different
> > > > > > > slot, so does it still make sense to forbid use of slot 1 as a
> > > > > > > hardcoded rule ?
> > > > > >
> > > > > > IIRC, the idea behind that is to give some time window for users to
> > > > > > allow moving guests from the new version to the old one. If we 
> > > > > > allow to
> > > > > > use slot 1, it won't be possible to move the guest to the old 
> > > > > > libvirt as
> > > > > > it will complain slot 1 should be used only for LPC.
> > > > > 
> > > > > If the user has decided to move their LPC to a slot != 1, then it is
> > > > > already impossible to migrate the guest to an old libvirt.
> > > > > 
> > > > > If the user wants to explicitly specify another device in slot 1, then
> > > > > we should not prevent that.
> > > > > 
> > > > > We just need to make sure that if no LPC is in the XML, and no other
> > > > > device explicitly has slot 1, then make sure to auto-assign LPC in 
> > > > > slot 1
> > > > > not some other device.
> > > > 
> > > > I've started playing with that and remembered some more corner cases.
> > > > 
> > > > To elaborate on your example, i.e. "no explicit LPC in XML AND no other
> > > > device explicitly on slot 1": these conditions are not specific enough 
> > > > to
> > > > tell whether an LPC device will be added or not.
> > > > 
> > > > In case if the LPC device was not explicitly specified by a user,
> > > > the bhyve driver tries to add it if it's needed (it's required
> > > > for serial console, bootloader, and video devices;
> > > > see bhyveDomainDefNeedsISAController()). Otherwise a domain will start
> > > > without the LPC device.
> > > > 
> > > > This could lead to a case when a user starts a domain in configuration
> > > > that does not require LPC device, but has e.g. a network device on
> > > > a PCI controller that's auto-assigned to slot 1.
> > > > 
> > > > Later user decides to change the configuration and adds a video device,
> > > > which requires LPC. This will lead to addresses changes, as LPC will go
> > > > to slot 1 and a network device's controller will go from slot 1 to slot
> > > > 2, which could be troublesome in some guest OSes.
> > > 
> > > First of all, lets me clear that we're talking about persistent guests
> > > here, not transient guests.
> > > 
> > > With a persistent guest, the PCI addresses are assigned at the time
> > > the XML arrives in virDomainDefineXML. If nothing requires the LPC
> > > at this time, then a NIC could get given slot 1. This is recorded in
> > > the persistent XML.
> > > 
> > > If the user later uses 'virsh edit' to modify the XML and add a video
> > > device, libvirt will see that the NIC is already on slot 1. It will
> > > thus have to give the LPC slot 2 (or whatever is free). The NIC will
> > > not move from slot 1, as that slot is considered taken at this time.
> > > 
> > > The same is true if using virDomainAttachDevice to add a video
> > > card. Any modifications to the XML must never change addresses that
> > > are 

Re: [PATCH v5 2/2] bhyve: support 'isa' controller for LPC

2020-09-17 Thread Daniel P . Berrangé
On Thu, Sep 17, 2020 at 05:48:38PM +0400, Roman Bogorodskiy wrote:
>   Daniel P. Berrangé wrote:
> 
> > On Wed, Sep 16, 2020 at 07:14:39PM +0400, Roman Bogorodskiy wrote:
> > >   Daniel P. Berrangé wrote:
> > > 
> > > > On Sat, Aug 01, 2020 at 10:16:57AM +0400, Roman Bogorodskiy wrote:
> > > > > diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c
> > > > > index fc52280361..52a055f205 100644
> > > > > --- a/src/bhyve/bhyve_device.c
> > > > > +++ b/src/bhyve/bhyve_device.c
> > > > > @@ -46,10 +46,16 @@ bhyveCollectPCIAddress(virDomainDefPtr def 
> > > > > G_GNUC_UNUSED,
> > > > >  if (addr->slot == 0) {
> > > > >  return 0;
> > > > >  } else if (addr->slot == 1) {
> > > > > -virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > > > > -   _("PCI bus 0 slot 1 is reserved for the 
> > > > > implicit "
> > > > > - "LPC PCI-ISA bridge"));
> > > > > -return -1;
> > > > > +if (!(device->type == VIR_DOMAIN_DEVICE_CONTROLLER &&
> > > > > +  device->data.controller->type == 
> > > > > VIR_DOMAIN_CONTROLLER_TYPE_ISA)) {
> > > > > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > > > > +_("PCI bus 0 slot 1 is reserved for 
> > > > > the implicit "
> > > > > +  "LPC PCI-ISA bridge"));
> > > > > + return -1;
> > > > > +} else {
> > > > > +/* We reserve slot 1 for LPC in 
> > > > > bhyveAssignDevicePCISlots(), so exit early */
> > > > > +return 0;
> > > > > +}
> > > > 
> > > > I forgot to respond to your followup comments on v4
> > > > https://www.redhat.com/archives/libvir-list/2020-July/msg01761.html
> > > > 
> > > > > > 
> > > > > > IIUC, this series makes it possible to put the TPC in a different
> > > > > > slot, so does it still make sense to forbid use of slot 1 as a
> > > > > > hardcoded rule ?
> > > > >
> > > > > IIRC, the idea behind that is to give some time window for users to
> > > > > allow moving guests from the new version to the old one. If we allow 
> > > > > to
> > > > > use slot 1, it won't be possible to move the guest to the old libvirt 
> > > > > as
> > > > > it will complain slot 1 should be used only for LPC.
> > > > 
> > > > If the user has decided to move their LPC to a slot != 1, then it is
> > > > already impossible to migrate the guest to an old libvirt.
> > > > 
> > > > If the user wants to explicitly specify another device in slot 1, then
> > > > we should not prevent that.
> > > > 
> > > > We just need to make sure that if no LPC is in the XML, and no other
> > > > device explicitly has slot 1, then make sure to auto-assign LPC in slot 
> > > > 1
> > > > not some other device.
> > > 
> > > I've started playing with that and remembered some more corner cases.
> > > 
> > > To elaborate on your example, i.e. "no explicit LPC in XML AND no other
> > > device explicitly on slot 1": these conditions are not specific enough to
> > > tell whether an LPC device will be added or not.
> > > 
> > > In case if the LPC device was not explicitly specified by a user,
> > > the bhyve driver tries to add it if it's needed (it's required
> > > for serial console, bootloader, and video devices;
> > > see bhyveDomainDefNeedsISAController()). Otherwise a domain will start
> > > without the LPC device.
> > > 
> > > This could lead to a case when a user starts a domain in configuration
> > > that does not require LPC device, but has e.g. a network device on
> > > a PCI controller that's auto-assigned to slot 1.
> > > 
> > > Later user decides to change the configuration and adds a video device,
> > > which requires LPC. This will lead to addresses changes, as LPC will go
> > > to slot 1 and a network device's controller will go from slot 1 to slot
> > > 2, which could be troublesome in some guest OSes.
> > 
> > First of all, lets me clear that we're talking about persistent guests
> > here, not transient guests.
> > 
> > With a persistent guest, the PCI addresses are assigned at the time
> > the XML arrives in virDomainDefineXML. If nothing requires the LPC
> > at this time, then a NIC could get given slot 1. This is recorded in
> > the persistent XML.
> > 
> > If the user later uses 'virsh edit' to modify the XML and add a video
> > device, libvirt will see that the NIC is already on slot 1. It will
> > thus have to give the LPC slot 2 (or whatever is free). The NIC will
> > not move from slot 1, as that slot is considered taken at this time.
> > 
> > The same is true if using virDomainAttachDevice to add a video
> > card. Any modifications to the XML must never change addresses that
> > are currently recorded in the XML, only ever place devices in new
> > unused slots.
> 
> Sorry, I should have stated that I was assuming that LPC always
> gets slot 1 assigned if it has no address explicitly assigned by the user
> in the 

Re: [PATCH v5 2/2] bhyve: support 'isa' controller for LPC

2020-09-17 Thread Roman Bogorodskiy
  Daniel P. Berrangé wrote:

> On Wed, Sep 16, 2020 at 07:14:39PM +0400, Roman Bogorodskiy wrote:
> >   Daniel P. Berrangé wrote:
> > 
> > > On Sat, Aug 01, 2020 at 10:16:57AM +0400, Roman Bogorodskiy wrote:
> > > > Support modeling of the 'isa' controller for bhyve. User can manually
> > > > define any PCI slot for the 'isa' controller, including PCI slot 1,
> > > > but other devices are not allowed to use this address.
> > > > 
> > > > When domain configuration requires the 'isa' controller to be present,
> > > > automatically add it on domain post-parse stage.
> > > > 
> > > > Now, as this controller is always available when needed, it's not
> > > > necessary to implicitly add it to the bhyve command line, so remove
> > > > bhyveBuildLPCArgStr().
> > > > 
> > > > Also, make bhyveDomainDefNeedsISAController() static as it's no longer
> > > > used outside of bhyve_domain.c.
> > > > 
> > > > As more than one ISA controller is not supported by bhyve,
> > > > and multiple controllers with the same index are forbidden,
> > > > so forbid ISA controllers with non-zero index for bhyve.
> > > > 
> > > > Signed-off-by: Roman Bogorodskiy 
> > > > ---
> > > >  src/bhyve/bhyve_command.c | 27 +++---
> > > >  src/bhyve/bhyve_device.c  | 23 +---
> > > >  src/bhyve/bhyve_domain.c  | 25 +++--
> > > >  src/bhyve/bhyve_domain.h  |  2 --
> > > >  ...ml2argv-addr-isa-controller-on-slot-1.args | 10 ++
> > > >  ...2argv-addr-isa-controller-on-slot-1.ldargs |  3 ++
> > > >  ...xml2argv-addr-isa-controller-on-slot-1.xml | 26 ++
> > > >  ...l2argv-addr-isa-controller-on-slot-31.args | 10 ++
> > > >  ...argv-addr-isa-controller-on-slot-31.ldargs |  3 ++
> > > >  ...ml2argv-addr-isa-controller-on-slot-31.xml | 26 ++
> > > >  ...argv-addr-non-isa-controller-on-slot-1.xml | 23 
> > > >  .../bhyvexml2argv-console.args|  2 +-
> > > >  .../bhyvexml2argv-isa-controller.args | 10 ++
> > > >  .../bhyvexml2argv-isa-controller.ldargs   |  3 ++
> > > >  .../bhyvexml2argv-isa-controller.xml  | 24 +
> > > >  ...bhyvexml2argv-isa-multiple-controllers.xml | 25 +
> > > >  .../bhyvexml2argv-serial-grub-nocons.args |  2 +-
> > > >  .../bhyvexml2argv-serial-grub.args|  2 +-
> > > >  .../bhyvexml2argv-serial.args |  2 +-
> > > >  .../bhyvexml2argvdata/bhyvexml2argv-uefi.args |  4 +--
> > > >  .../bhyvexml2argv-vnc-autoport.args   |  4 +--
> > > >  .../bhyvexml2argv-vnc-vgaconf-io.args |  4 +--
> > > >  .../bhyvexml2argv-vnc-vgaconf-off.args|  4 +--
> > > >  .../bhyvexml2argv-vnc-vgaconf-on.args |  4 +--
> > > >  .../bhyvexml2argvdata/bhyvexml2argv-vnc.args  |  4 +--
> > > >  tests/bhyvexml2argvtest.c |  5 +++
> > > >  ...l2xmlout-addr-isa-controller-on-slot-1.xml | 36 +++
> > > >  ...2xmlout-addr-isa-controller-on-slot-31.xml | 36 +++
> > > >  .../bhyvexml2xmlout-console.xml   |  3 ++
> > > >  .../bhyvexml2xmlout-isa-controller.xml| 36 +++
> > > >  .../bhyvexml2xmlout-serial-grub-nocons.xml|  3 ++
> > > >  .../bhyvexml2xmlout-serial-grub.xml   |  3 ++
> > > >  .../bhyvexml2xmlout-serial.xml|  3 ++
> > > >  .../bhyvexml2xmlout-vnc-autoport.xml  |  3 ++
> > > >  .../bhyvexml2xmlout-vnc-vgaconf-io.xml|  3 ++
> > > >  .../bhyvexml2xmlout-vnc-vgaconf-off.xml   |  3 ++
> > > >  .../bhyvexml2xmlout-vnc-vgaconf-on.xml|  3 ++
> > > >  .../bhyvexml2xmlout-vnc.xml   |  3 ++
> > > >  tests/bhyvexml2xmltest.c  |  3 ++
> > > >  39 files changed, 378 insertions(+), 37 deletions(-)
> > > >  create mode 100644 
> > > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args
> > > >  create mode 100644 
> > > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs
> > > >  create mode 100644 
> > > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml
> > > >  create mode 100644 
> > > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args
> > > >  create mode 100644 
> > > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs
> > > >  create mode 100644 
> > > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml
> > > >  create mode 100644 
> > > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml
> > > >  create mode 100644 
> > > > tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args
> > > >  create mode 100644 
> > > > tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs
> > > >  create mode 100644 
> > > > tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml
> > > >  create mode 100644 
> > > > 

Re: [PATCH v5 2/2] bhyve: support 'isa' controller for LPC

2020-09-16 Thread Daniel P . Berrangé
On Wed, Sep 16, 2020 at 07:14:39PM +0400, Roman Bogorodskiy wrote:
>   Daniel P. Berrangé wrote:
> 
> > On Sat, Aug 01, 2020 at 10:16:57AM +0400, Roman Bogorodskiy wrote:
> > > Support modeling of the 'isa' controller for bhyve. User can manually
> > > define any PCI slot for the 'isa' controller, including PCI slot 1,
> > > but other devices are not allowed to use this address.
> > > 
> > > When domain configuration requires the 'isa' controller to be present,
> > > automatically add it on domain post-parse stage.
> > > 
> > > Now, as this controller is always available when needed, it's not
> > > necessary to implicitly add it to the bhyve command line, so remove
> > > bhyveBuildLPCArgStr().
> > > 
> > > Also, make bhyveDomainDefNeedsISAController() static as it's no longer
> > > used outside of bhyve_domain.c.
> > > 
> > > As more than one ISA controller is not supported by bhyve,
> > > and multiple controllers with the same index are forbidden,
> > > so forbid ISA controllers with non-zero index for bhyve.
> > > 
> > > Signed-off-by: Roman Bogorodskiy 
> > > ---
> > >  src/bhyve/bhyve_command.c | 27 +++---
> > >  src/bhyve/bhyve_device.c  | 23 +---
> > >  src/bhyve/bhyve_domain.c  | 25 +++--
> > >  src/bhyve/bhyve_domain.h  |  2 --
> > >  ...ml2argv-addr-isa-controller-on-slot-1.args | 10 ++
> > >  ...2argv-addr-isa-controller-on-slot-1.ldargs |  3 ++
> > >  ...xml2argv-addr-isa-controller-on-slot-1.xml | 26 ++
> > >  ...l2argv-addr-isa-controller-on-slot-31.args | 10 ++
> > >  ...argv-addr-isa-controller-on-slot-31.ldargs |  3 ++
> > >  ...ml2argv-addr-isa-controller-on-slot-31.xml | 26 ++
> > >  ...argv-addr-non-isa-controller-on-slot-1.xml | 23 
> > >  .../bhyvexml2argv-console.args|  2 +-
> > >  .../bhyvexml2argv-isa-controller.args | 10 ++
> > >  .../bhyvexml2argv-isa-controller.ldargs   |  3 ++
> > >  .../bhyvexml2argv-isa-controller.xml  | 24 +
> > >  ...bhyvexml2argv-isa-multiple-controllers.xml | 25 +
> > >  .../bhyvexml2argv-serial-grub-nocons.args |  2 +-
> > >  .../bhyvexml2argv-serial-grub.args|  2 +-
> > >  .../bhyvexml2argv-serial.args |  2 +-
> > >  .../bhyvexml2argvdata/bhyvexml2argv-uefi.args |  4 +--
> > >  .../bhyvexml2argv-vnc-autoport.args   |  4 +--
> > >  .../bhyvexml2argv-vnc-vgaconf-io.args |  4 +--
> > >  .../bhyvexml2argv-vnc-vgaconf-off.args|  4 +--
> > >  .../bhyvexml2argv-vnc-vgaconf-on.args |  4 +--
> > >  .../bhyvexml2argvdata/bhyvexml2argv-vnc.args  |  4 +--
> > >  tests/bhyvexml2argvtest.c |  5 +++
> > >  ...l2xmlout-addr-isa-controller-on-slot-1.xml | 36 +++
> > >  ...2xmlout-addr-isa-controller-on-slot-31.xml | 36 +++
> > >  .../bhyvexml2xmlout-console.xml   |  3 ++
> > >  .../bhyvexml2xmlout-isa-controller.xml| 36 +++
> > >  .../bhyvexml2xmlout-serial-grub-nocons.xml|  3 ++
> > >  .../bhyvexml2xmlout-serial-grub.xml   |  3 ++
> > >  .../bhyvexml2xmlout-serial.xml|  3 ++
> > >  .../bhyvexml2xmlout-vnc-autoport.xml  |  3 ++
> > >  .../bhyvexml2xmlout-vnc-vgaconf-io.xml|  3 ++
> > >  .../bhyvexml2xmlout-vnc-vgaconf-off.xml   |  3 ++
> > >  .../bhyvexml2xmlout-vnc-vgaconf-on.xml|  3 ++
> > >  .../bhyvexml2xmlout-vnc.xml   |  3 ++
> > >  tests/bhyvexml2xmltest.c  |  3 ++
> > >  39 files changed, 378 insertions(+), 37 deletions(-)
> > >  create mode 100644 
> > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args
> > >  create mode 100644 
> > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs
> > >  create mode 100644 
> > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml
> > >  create mode 100644 
> > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args
> > >  create mode 100644 
> > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs
> > >  create mode 100644 
> > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml
> > >  create mode 100644 
> > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml
> > >  create mode 100644 
> > > tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args
> > >  create mode 100644 
> > > tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs
> > >  create mode 100644 
> > > tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml
> > >  create mode 100644 
> > > tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml
> > >  create mode 100644 
> > > tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml
> > >  create mode 100644 
> > > 

Re: [PATCH v5 2/2] bhyve: support 'isa' controller for LPC

2020-09-16 Thread Roman Bogorodskiy
  Daniel P. Berrangé wrote:

> On Sat, Aug 01, 2020 at 10:16:57AM +0400, Roman Bogorodskiy wrote:
> > Support modeling of the 'isa' controller for bhyve. User can manually
> > define any PCI slot for the 'isa' controller, including PCI slot 1,
> > but other devices are not allowed to use this address.
> > 
> > When domain configuration requires the 'isa' controller to be present,
> > automatically add it on domain post-parse stage.
> > 
> > Now, as this controller is always available when needed, it's not
> > necessary to implicitly add it to the bhyve command line, so remove
> > bhyveBuildLPCArgStr().
> > 
> > Also, make bhyveDomainDefNeedsISAController() static as it's no longer
> > used outside of bhyve_domain.c.
> > 
> > As more than one ISA controller is not supported by bhyve,
> > and multiple controllers with the same index are forbidden,
> > so forbid ISA controllers with non-zero index for bhyve.
> > 
> > Signed-off-by: Roman Bogorodskiy 
> > ---
> >  src/bhyve/bhyve_command.c | 27 +++---
> >  src/bhyve/bhyve_device.c  | 23 +---
> >  src/bhyve/bhyve_domain.c  | 25 +++--
> >  src/bhyve/bhyve_domain.h  |  2 --
> >  ...ml2argv-addr-isa-controller-on-slot-1.args | 10 ++
> >  ...2argv-addr-isa-controller-on-slot-1.ldargs |  3 ++
> >  ...xml2argv-addr-isa-controller-on-slot-1.xml | 26 ++
> >  ...l2argv-addr-isa-controller-on-slot-31.args | 10 ++
> >  ...argv-addr-isa-controller-on-slot-31.ldargs |  3 ++
> >  ...ml2argv-addr-isa-controller-on-slot-31.xml | 26 ++
> >  ...argv-addr-non-isa-controller-on-slot-1.xml | 23 
> >  .../bhyvexml2argv-console.args|  2 +-
> >  .../bhyvexml2argv-isa-controller.args | 10 ++
> >  .../bhyvexml2argv-isa-controller.ldargs   |  3 ++
> >  .../bhyvexml2argv-isa-controller.xml  | 24 +
> >  ...bhyvexml2argv-isa-multiple-controllers.xml | 25 +
> >  .../bhyvexml2argv-serial-grub-nocons.args |  2 +-
> >  .../bhyvexml2argv-serial-grub.args|  2 +-
> >  .../bhyvexml2argv-serial.args |  2 +-
> >  .../bhyvexml2argvdata/bhyvexml2argv-uefi.args |  4 +--
> >  .../bhyvexml2argv-vnc-autoport.args   |  4 +--
> >  .../bhyvexml2argv-vnc-vgaconf-io.args |  4 +--
> >  .../bhyvexml2argv-vnc-vgaconf-off.args|  4 +--
> >  .../bhyvexml2argv-vnc-vgaconf-on.args |  4 +--
> >  .../bhyvexml2argvdata/bhyvexml2argv-vnc.args  |  4 +--
> >  tests/bhyvexml2argvtest.c |  5 +++
> >  ...l2xmlout-addr-isa-controller-on-slot-1.xml | 36 +++
> >  ...2xmlout-addr-isa-controller-on-slot-31.xml | 36 +++
> >  .../bhyvexml2xmlout-console.xml   |  3 ++
> >  .../bhyvexml2xmlout-isa-controller.xml| 36 +++
> >  .../bhyvexml2xmlout-serial-grub-nocons.xml|  3 ++
> >  .../bhyvexml2xmlout-serial-grub.xml   |  3 ++
> >  .../bhyvexml2xmlout-serial.xml|  3 ++
> >  .../bhyvexml2xmlout-vnc-autoport.xml  |  3 ++
> >  .../bhyvexml2xmlout-vnc-vgaconf-io.xml|  3 ++
> >  .../bhyvexml2xmlout-vnc-vgaconf-off.xml   |  3 ++
> >  .../bhyvexml2xmlout-vnc-vgaconf-on.xml|  3 ++
> >  .../bhyvexml2xmlout-vnc.xml   |  3 ++
> >  tests/bhyvexml2xmltest.c  |  3 ++
> >  39 files changed, 378 insertions(+), 37 deletions(-)
> >  create mode 100644 
> > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args
> >  create mode 100644 
> > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs
> >  create mode 100644 
> > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml
> >  create mode 100644 
> > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args
> >  create mode 100644 
> > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs
> >  create mode 100644 
> > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml
> >  create mode 100644 
> > tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml
> >  create mode 100644 
> > tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args
> >  create mode 100644 
> > tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs
> >  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml
> >  create mode 100644 
> > tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml
> >  create mode 100644 
> > tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml
> >  create mode 100644 
> > tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml
> >  create mode 100644 
> > tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml
> > 
> 
> 
> > diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c
> > index fc52280361..52a055f205 100644
> > --- 

Re: [PATCH v5 2/2] bhyve: support 'isa' controller for LPC

2020-09-10 Thread Ján Tomko

On a Saturday in 2020, Roman Bogorodskiy wrote:

Support modeling of the 'isa' controller for bhyve. User can manually
define any PCI slot for the 'isa' controller, including PCI slot 1,
but other devices are not allowed to use this address.

When domain configuration requires the 'isa' controller to be present,
automatically add it on domain post-parse stage.

Now, as this controller is always available when needed, it's not
necessary to implicitly add it to the bhyve command line, so remove
bhyveBuildLPCArgStr().

Also, make bhyveDomainDefNeedsISAController() static as it's no longer
used outside of bhyve_domain.c.

As more than one ISA controller is not supported by bhyve,
and multiple controllers with the same index are forbidden,
so forbid ISA controllers with non-zero index for bhyve.

Signed-off-by: Roman Bogorodskiy 
---
src/bhyve/bhyve_command.c | 27 +++---
src/bhyve/bhyve_device.c  | 23 +---
src/bhyve/bhyve_domain.c  | 25 +++--
src/bhyve/bhyve_domain.h  |  2 --
...ml2argv-addr-isa-controller-on-slot-1.args | 10 ++
...2argv-addr-isa-controller-on-slot-1.ldargs |  3 ++
...xml2argv-addr-isa-controller-on-slot-1.xml | 26 ++
...l2argv-addr-isa-controller-on-slot-31.args | 10 ++
...argv-addr-isa-controller-on-slot-31.ldargs |  3 ++
...ml2argv-addr-isa-controller-on-slot-31.xml | 26 ++
...argv-addr-non-isa-controller-on-slot-1.xml | 23 
.../bhyvexml2argv-console.args|  2 +-
.../bhyvexml2argv-isa-controller.args | 10 ++
.../bhyvexml2argv-isa-controller.ldargs   |  3 ++
.../bhyvexml2argv-isa-controller.xml  | 24 +
...bhyvexml2argv-isa-multiple-controllers.xml | 25 +
.../bhyvexml2argv-serial-grub-nocons.args |  2 +-
.../bhyvexml2argv-serial-grub.args|  2 +-
.../bhyvexml2argv-serial.args |  2 +-
.../bhyvexml2argvdata/bhyvexml2argv-uefi.args |  4 +--
.../bhyvexml2argv-vnc-autoport.args   |  4 +--
.../bhyvexml2argv-vnc-vgaconf-io.args |  4 +--
.../bhyvexml2argv-vnc-vgaconf-off.args|  4 +--
.../bhyvexml2argv-vnc-vgaconf-on.args |  4 +--
.../bhyvexml2argvdata/bhyvexml2argv-vnc.args  |  4 +--
tests/bhyvexml2argvtest.c |  5 +++
...l2xmlout-addr-isa-controller-on-slot-1.xml | 36 +++
...2xmlout-addr-isa-controller-on-slot-31.xml | 36 +++
.../bhyvexml2xmlout-console.xml   |  3 ++
.../bhyvexml2xmlout-isa-controller.xml| 36 +++
.../bhyvexml2xmlout-serial-grub-nocons.xml|  3 ++
.../bhyvexml2xmlout-serial-grub.xml   |  3 ++
.../bhyvexml2xmlout-serial.xml|  3 ++
.../bhyvexml2xmlout-vnc-autoport.xml  |  3 ++
.../bhyvexml2xmlout-vnc-vgaconf-io.xml|  3 ++
.../bhyvexml2xmlout-vnc-vgaconf-off.xml   |  3 ++
.../bhyvexml2xmlout-vnc-vgaconf-on.xml|  3 ++
.../bhyvexml2xmlout-vnc.xml   |  3 ++
tests/bhyvexml2xmltest.c  |  3 ++
39 files changed, 378 insertions(+), 37 deletions(-)
create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args
create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs
create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml
create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args
create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs
create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml
create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml
create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml
create mode 100644 
tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml
create mode 100644 
tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml
create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature


Re: [PATCH v5 2/2] bhyve: support 'isa' controller for LPC

2020-09-10 Thread Daniel P . Berrangé
On Sat, Aug 01, 2020 at 10:16:57AM +0400, Roman Bogorodskiy wrote:
> Support modeling of the 'isa' controller for bhyve. User can manually
> define any PCI slot for the 'isa' controller, including PCI slot 1,
> but other devices are not allowed to use this address.
> 
> When domain configuration requires the 'isa' controller to be present,
> automatically add it on domain post-parse stage.
> 
> Now, as this controller is always available when needed, it's not
> necessary to implicitly add it to the bhyve command line, so remove
> bhyveBuildLPCArgStr().
> 
> Also, make bhyveDomainDefNeedsISAController() static as it's no longer
> used outside of bhyve_domain.c.
> 
> As more than one ISA controller is not supported by bhyve,
> and multiple controllers with the same index are forbidden,
> so forbid ISA controllers with non-zero index for bhyve.
> 
> Signed-off-by: Roman Bogorodskiy 
> ---
>  src/bhyve/bhyve_command.c | 27 +++---
>  src/bhyve/bhyve_device.c  | 23 +---
>  src/bhyve/bhyve_domain.c  | 25 +++--
>  src/bhyve/bhyve_domain.h  |  2 --
>  ...ml2argv-addr-isa-controller-on-slot-1.args | 10 ++
>  ...2argv-addr-isa-controller-on-slot-1.ldargs |  3 ++
>  ...xml2argv-addr-isa-controller-on-slot-1.xml | 26 ++
>  ...l2argv-addr-isa-controller-on-slot-31.args | 10 ++
>  ...argv-addr-isa-controller-on-slot-31.ldargs |  3 ++
>  ...ml2argv-addr-isa-controller-on-slot-31.xml | 26 ++
>  ...argv-addr-non-isa-controller-on-slot-1.xml | 23 
>  .../bhyvexml2argv-console.args|  2 +-
>  .../bhyvexml2argv-isa-controller.args | 10 ++
>  .../bhyvexml2argv-isa-controller.ldargs   |  3 ++
>  .../bhyvexml2argv-isa-controller.xml  | 24 +
>  ...bhyvexml2argv-isa-multiple-controllers.xml | 25 +
>  .../bhyvexml2argv-serial-grub-nocons.args |  2 +-
>  .../bhyvexml2argv-serial-grub.args|  2 +-
>  .../bhyvexml2argv-serial.args |  2 +-
>  .../bhyvexml2argvdata/bhyvexml2argv-uefi.args |  4 +--
>  .../bhyvexml2argv-vnc-autoport.args   |  4 +--
>  .../bhyvexml2argv-vnc-vgaconf-io.args |  4 +--
>  .../bhyvexml2argv-vnc-vgaconf-off.args|  4 +--
>  .../bhyvexml2argv-vnc-vgaconf-on.args |  4 +--
>  .../bhyvexml2argvdata/bhyvexml2argv-vnc.args  |  4 +--
>  tests/bhyvexml2argvtest.c |  5 +++
>  ...l2xmlout-addr-isa-controller-on-slot-1.xml | 36 +++
>  ...2xmlout-addr-isa-controller-on-slot-31.xml | 36 +++
>  .../bhyvexml2xmlout-console.xml   |  3 ++
>  .../bhyvexml2xmlout-isa-controller.xml| 36 +++
>  .../bhyvexml2xmlout-serial-grub-nocons.xml|  3 ++
>  .../bhyvexml2xmlout-serial-grub.xml   |  3 ++
>  .../bhyvexml2xmlout-serial.xml|  3 ++
>  .../bhyvexml2xmlout-vnc-autoport.xml  |  3 ++
>  .../bhyvexml2xmlout-vnc-vgaconf-io.xml|  3 ++
>  .../bhyvexml2xmlout-vnc-vgaconf-off.xml   |  3 ++
>  .../bhyvexml2xmlout-vnc-vgaconf-on.xml|  3 ++
>  .../bhyvexml2xmlout-vnc.xml   |  3 ++
>  tests/bhyvexml2xmltest.c  |  3 ++
>  39 files changed, 378 insertions(+), 37 deletions(-)
>  create mode 100644 
> tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args
>  create mode 100644 
> tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs
>  create mode 100644 
> tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml
>  create mode 100644 
> tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args
>  create mode 100644 
> tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs
>  create mode 100644 
> tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml
>  create mode 100644 
> tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml
>  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args
>  create mode 100644 
> tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs
>  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml
>  create mode 100644 
> tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml
>  create mode 100644 
> tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml
>  create mode 100644 
> tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml
>  create mode 100644 
> tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml
> 


> diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c
> index fc52280361..52a055f205 100644
> --- a/src/bhyve/bhyve_device.c
> +++ b/src/bhyve/bhyve_device.c
> @@ -46,10 +46,16 @@ bhyveCollectPCIAddress(virDomainDefPtr def G_GNUC_UNUSED,
>  if (addr->slot == 0) {
>  return 0;
>  } else if 

[PATCH v5 2/2] bhyve: support 'isa' controller for LPC

2020-08-01 Thread Roman Bogorodskiy
Support modeling of the 'isa' controller for bhyve. User can manually
define any PCI slot for the 'isa' controller, including PCI slot 1,
but other devices are not allowed to use this address.

When domain configuration requires the 'isa' controller to be present,
automatically add it on domain post-parse stage.

Now, as this controller is always available when needed, it's not
necessary to implicitly add it to the bhyve command line, so remove
bhyveBuildLPCArgStr().

Also, make bhyveDomainDefNeedsISAController() static as it's no longer
used outside of bhyve_domain.c.

As more than one ISA controller is not supported by bhyve,
and multiple controllers with the same index are forbidden,
so forbid ISA controllers with non-zero index for bhyve.

Signed-off-by: Roman Bogorodskiy 
---
 src/bhyve/bhyve_command.c | 27 +++---
 src/bhyve/bhyve_device.c  | 23 +---
 src/bhyve/bhyve_domain.c  | 25 +++--
 src/bhyve/bhyve_domain.h  |  2 --
 ...ml2argv-addr-isa-controller-on-slot-1.args | 10 ++
 ...2argv-addr-isa-controller-on-slot-1.ldargs |  3 ++
 ...xml2argv-addr-isa-controller-on-slot-1.xml | 26 ++
 ...l2argv-addr-isa-controller-on-slot-31.args | 10 ++
 ...argv-addr-isa-controller-on-slot-31.ldargs |  3 ++
 ...ml2argv-addr-isa-controller-on-slot-31.xml | 26 ++
 ...argv-addr-non-isa-controller-on-slot-1.xml | 23 
 .../bhyvexml2argv-console.args|  2 +-
 .../bhyvexml2argv-isa-controller.args | 10 ++
 .../bhyvexml2argv-isa-controller.ldargs   |  3 ++
 .../bhyvexml2argv-isa-controller.xml  | 24 +
 ...bhyvexml2argv-isa-multiple-controllers.xml | 25 +
 .../bhyvexml2argv-serial-grub-nocons.args |  2 +-
 .../bhyvexml2argv-serial-grub.args|  2 +-
 .../bhyvexml2argv-serial.args |  2 +-
 .../bhyvexml2argvdata/bhyvexml2argv-uefi.args |  4 +--
 .../bhyvexml2argv-vnc-autoport.args   |  4 +--
 .../bhyvexml2argv-vnc-vgaconf-io.args |  4 +--
 .../bhyvexml2argv-vnc-vgaconf-off.args|  4 +--
 .../bhyvexml2argv-vnc-vgaconf-on.args |  4 +--
 .../bhyvexml2argvdata/bhyvexml2argv-vnc.args  |  4 +--
 tests/bhyvexml2argvtest.c |  5 +++
 ...l2xmlout-addr-isa-controller-on-slot-1.xml | 36 +++
 ...2xmlout-addr-isa-controller-on-slot-31.xml | 36 +++
 .../bhyvexml2xmlout-console.xml   |  3 ++
 .../bhyvexml2xmlout-isa-controller.xml| 36 +++
 .../bhyvexml2xmlout-serial-grub-nocons.xml|  3 ++
 .../bhyvexml2xmlout-serial-grub.xml   |  3 ++
 .../bhyvexml2xmlout-serial.xml|  3 ++
 .../bhyvexml2xmlout-vnc-autoport.xml  |  3 ++
 .../bhyvexml2xmlout-vnc-vgaconf-io.xml|  3 ++
 .../bhyvexml2xmlout-vnc-vgaconf-off.xml   |  3 ++
 .../bhyvexml2xmlout-vnc-vgaconf-on.xml|  3 ++
 .../bhyvexml2xmlout-vnc.xml   |  3 ++
 tests/bhyvexml2xmltest.c  |  3 ++
 39 files changed, 378 insertions(+), 37 deletions(-)
 create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args
 create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs
 create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml
 create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args
 create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs
 create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml
 create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml
 create mode 100644 
tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml
 create mode 100644 
tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml
 create mode 100644 
tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml
 create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml

diff --git a/src/bhyve/bhyve_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