Re: [libvirt] [PATCH v1 3/4] bhyve: fix SATA address allocation
Laine Stump wrote:
> On 01/05/2017 09:46 AM, Roman Bogorodskiy wrote:
> > As bhyve for a long time didn't have a notion of the explicit SATA
> > controller and created a controller for each drive, the bhyve driver
> > in libvirt acted in a similar way and didn't care about the SATA
> > controllers and assigned PCI addresses to drives directly, as
> > the generated command will look like this anyway:
> >
> > 2:0,ahci-hd,somedisk.img
> >
> > This no longer makes sense because:
> >
> > 1. After commit c07d1c1c4f it's not possible to assign
> > PCI addresses to disks
> > 2. Bhyve now supports multiple disk drives for a controller,
> > so it's going away from 1:1 controller:disk mapping, so
> > the controller object starts to make more sense now
> >
> > So, this patch does the following:
> >
> > - Assign PCI address to SATA controllers (previously we didn't do this)
> > - Assign disk addresses instead of PCI addresses for disks. Now, when
> > building a bhyve command, we take PCI address not from the disk
> > itself but from its controller
> > - Assign addresses at XML parsing time using the
> > assignAddressesCallback. This is done mainly for being able to
> > verify address allocation via xml2xml tests
> > - Adjust existing bhyvexml2{xml,argv} tests to chase the new
> > address allocation
> >
> > This patch is largely based on work of Fabian Freyer.
> > ---
> > po/POTFILES.in | 1 +
> > src/bhyve/bhyve_command.c | 143
> > -
> > src/bhyve/bhyve_device.c | 33 ++---
> > src/bhyve/bhyve_domain.c | 60 -
> > .../bhyvexml2argvdata/bhyvexml2argv-acpiapic.args | 4 +-
> > tests/bhyvexml2argvdata/bhyvexml2argv-base.args| 4 +-
> > .../bhyvexml2argv-bhyveload-bootorder.args | 5 +-
> > .../bhyvexml2argv-bhyveload-bootorder1.args| 5 +-
> > .../bhyvexml2argv-bhyveload-bootorder3.args| 5 +-
> > .../bhyvexml2argv-bhyveload-explicitargs.args | 4 +-
> > tests/bhyvexml2argvdata/bhyvexml2argv-console.args | 2 +-
> > .../bhyvexml2argv-custom-loader.args | 4 +-
> > .../bhyvexml2argv-disk-cdrom-grub.args | 4 +-
> > .../bhyvexml2argv-disk-cdrom.args | 4 +-
> > .../bhyvexml2argv-grub-bootorder.args | 6 +-
> > .../bhyvexml2argv-grub-bootorder2.args | 6 +-
> > .../bhyvexml2argv-grub-defaults.args | 4 +-
> > .../bhyvexml2argvdata/bhyvexml2argv-localtime.args | 4 +-
> > tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args | 4 +-
> > .../bhyvexml2argv-serial-grub-nocons.args | 2 +-
> > .../bhyvexml2argv-serial-grub.args | 2 +-
> > tests/bhyvexml2argvdata/bhyvexml2argv-serial.args | 2 +-
> > tests/bhyvexml2argvtest.c | 2 +-
> > .../bhyvexml2xmlout-acpiapic.xml | 4 +-
> > tests/bhyvexml2xmloutdata/bhyvexml2xmlout-base.xml | 4 +-
> > .../bhyvexml2xmlout-bhyveload-bootorder.xml| 4 +-
> > .../bhyvexml2xmlout-bhyveload-bootorder1.xml | 4 +-
> > .../bhyvexml2xmlout-bhyveload-bootorder2.xml | 4 +-
> > .../bhyvexml2xmlout-bhyveload-bootorder3.xml | 4 +-
> > .../bhyvexml2xmlout-bhyveload-bootorder4.xml | 4 +-
> > .../bhyvexml2xmlout-bhyveload-explicitargs.xml | 4 +-
> > .../bhyvexml2xmlout-console.xml| 4 +-
> > .../bhyvexml2xmlout-custom-loader.xml | 4 +-
> > .../bhyvexml2xmlout-disk-cdrom-grub.xml| 4 +-
> > .../bhyvexml2xmlout-disk-cdrom.xml | 4 +-
> > .../bhyvexml2xmlout-grub-bootorder.xml | 4 +-
> > .../bhyvexml2xmlout-grub-bootorder2.xml| 4 +-
> > .../bhyvexml2xmlout-grub-defaults.xml | 4 +-
> > .../bhyvexml2xmlout-localtime.xml | 4 +-
> > .../bhyvexml2xmlout-macaddr.xml| 4 +-
> > .../bhyvexml2xmlout-metadata.xml | 5 +-
> > .../bhyvexml2xmlout-serial-grub-nocons.xml | 4 +-
> > .../bhyvexml2xmlout-serial-grub.xml| 4 +-
> > .../bhyvexml2xmloutdata/bhyvexml2xmlout-serial.xml | 4 +-
> > tests/bhyvexml2xmltest.c | 2 +
> > 45 files changed, 279 insertions(+), 118 deletions(-)
> Aside from these minor nits, my only concern is that of compatibility
> when migrating forward and backward, but since I don't have a system
> with bhyve I can't test that, so I'll assume that you've done your due
> diligence in that regard. (Also, I'm assuming that you've successfully
> run make syntax-check in addition to make check). Based on those
> assumptions, ACK.
I pushed the series, thanks!
Roman Bogorodskiy
signature.asc
Description: PGP signature
--
libvir-list mailing list
Re: [libvirt] [PATCH v1 3/4] bhyve: fix SATA address allocation
Laine Stump wrote:
> On 01/05/2017 09:46 AM, Roman Bogorodskiy wrote:
> > As bhyve for a long time didn't have a notion of the explicit SATA
> > controller and created a controller for each drive, the bhyve driver
> > in libvirt acted in a similar way and didn't care about the SATA
> > controllers and assigned PCI addresses to drives directly, as
> > the generated command will look like this anyway:
> >
> > 2:0,ahci-hd,somedisk.img
> >
> > This no longer makes sense because:
> >
> > 1. After commit c07d1c1c4f it's not possible to assign
> > PCI addresses to disks
> > 2. Bhyve now supports multiple disk drives for a controller,
> > so it's going away from 1:1 controller:disk mapping, so
> > the controller object starts to make more sense now
> >
> > So, this patch does the following:
> >
> > - Assign PCI address to SATA controllers (previously we didn't do this)
> > - Assign disk addresses instead of PCI addresses for disks. Now, when
> > building a bhyve command, we take PCI address not from the disk
> > itself but from its controller
> > - Assign addresses at XML parsing time using the
> > assignAddressesCallback. This is done mainly for being able to
> > verify address allocation via xml2xml tests
> > - Adjust existing bhyvexml2{xml,argv} tests to chase the new
> > address allocation
> >
> > This patch is largely based on work of Fabian Freyer.
> > ---
> > po/POTFILES.in | 1 +
> > src/bhyve/bhyve_command.c | 143
> > -
> > src/bhyve/bhyve_device.c | 33 ++---
> > src/bhyve/bhyve_domain.c | 60 -
> > .../bhyvexml2argvdata/bhyvexml2argv-acpiapic.args | 4 +-
> > tests/bhyvexml2argvdata/bhyvexml2argv-base.args| 4 +-
> > .../bhyvexml2argv-bhyveload-bootorder.args | 5 +-
> > .../bhyvexml2argv-bhyveload-bootorder1.args| 5 +-
> > .../bhyvexml2argv-bhyveload-bootorder3.args| 5 +-
> > .../bhyvexml2argv-bhyveload-explicitargs.args | 4 +-
> > tests/bhyvexml2argvdata/bhyvexml2argv-console.args | 2 +-
> > .../bhyvexml2argv-custom-loader.args | 4 +-
> > .../bhyvexml2argv-disk-cdrom-grub.args | 4 +-
> > .../bhyvexml2argv-disk-cdrom.args | 4 +-
> > .../bhyvexml2argv-grub-bootorder.args | 6 +-
> > .../bhyvexml2argv-grub-bootorder2.args | 6 +-
> > .../bhyvexml2argv-grub-defaults.args | 4 +-
> > .../bhyvexml2argvdata/bhyvexml2argv-localtime.args | 4 +-
> > tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args | 4 +-
> > .../bhyvexml2argv-serial-grub-nocons.args | 2 +-
> > .../bhyvexml2argv-serial-grub.args | 2 +-
> > tests/bhyvexml2argvdata/bhyvexml2argv-serial.args | 2 +-
> > tests/bhyvexml2argvtest.c | 2 +-
> > .../bhyvexml2xmlout-acpiapic.xml | 4 +-
> > tests/bhyvexml2xmloutdata/bhyvexml2xmlout-base.xml | 4 +-
> > .../bhyvexml2xmlout-bhyveload-bootorder.xml| 4 +-
> > .../bhyvexml2xmlout-bhyveload-bootorder1.xml | 4 +-
> > .../bhyvexml2xmlout-bhyveload-bootorder2.xml | 4 +-
> > .../bhyvexml2xmlout-bhyveload-bootorder3.xml | 4 +-
> > .../bhyvexml2xmlout-bhyveload-bootorder4.xml | 4 +-
> > .../bhyvexml2xmlout-bhyveload-explicitargs.xml | 4 +-
> > .../bhyvexml2xmlout-console.xml| 4 +-
> > .../bhyvexml2xmlout-custom-loader.xml | 4 +-
> > .../bhyvexml2xmlout-disk-cdrom-grub.xml| 4 +-
> > .../bhyvexml2xmlout-disk-cdrom.xml | 4 +-
> > .../bhyvexml2xmlout-grub-bootorder.xml | 4 +-
> > .../bhyvexml2xmlout-grub-bootorder2.xml| 4 +-
> > .../bhyvexml2xmlout-grub-defaults.xml | 4 +-
> > .../bhyvexml2xmlout-localtime.xml | 4 +-
> > .../bhyvexml2xmlout-macaddr.xml| 4 +-
> > .../bhyvexml2xmlout-metadata.xml | 5 +-
> > .../bhyvexml2xmlout-serial-grub-nocons.xml | 4 +-
> > .../bhyvexml2xmlout-serial-grub.xml| 4 +-
> > .../bhyvexml2xmloutdata/bhyvexml2xmlout-serial.xml | 4 +-
> > tests/bhyvexml2xmltest.c | 2 +
> > 45 files changed, 279 insertions(+), 118 deletions(-)
> >
> > diff --git a/po/POTFILES.in b/po/POTFILES.in
> > index e66bb7a3a..632aa7736 100644
> > --- a/po/POTFILES.in
> > +++ b/po/POTFILES.in
> > @@ -14,6 +14,7 @@ src/access/viraccessdriverpolkit.c
> > src/access/viraccessmanager.c
> > src/bhyve/bhyve_command.c
> > src/bhyve/bhyve_device.c
> > +src/bhyve/bhyve_domain.c
> > src/bhyve/bhyve_driver.c
> > src/bhyve/bhyve_monitor.c
> > src/bhyve/bhyve_parse_command.c
> > diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
> >
Re: [libvirt] [PATCH v1 3/4] bhyve: fix SATA address allocation
On 01/05/2017 09:46 AM, Roman Bogorodskiy wrote:
As bhyve for a long time didn't have a notion of the explicit SATA
controller and created a controller for each drive, the bhyve driver
in libvirt acted in a similar way and didn't care about the SATA
controllers and assigned PCI addresses to drives directly, as
the generated command will look like this anyway:
2:0,ahci-hd,somedisk.img
This no longer makes sense because:
1. After commit c07d1c1c4f it's not possible to assign
PCI addresses to disks
2. Bhyve now supports multiple disk drives for a controller,
so it's going away from 1:1 controller:disk mapping, so
the controller object starts to make more sense now
So, this patch does the following:
- Assign PCI address to SATA controllers (previously we didn't do this)
- Assign disk addresses instead of PCI addresses for disks. Now, when
building a bhyve command, we take PCI address not from the disk
itself but from its controller
- Assign addresses at XML parsing time using the
assignAddressesCallback. This is done mainly for being able to
verify address allocation via xml2xml tests
- Adjust existing bhyvexml2{xml,argv} tests to chase the new
address allocation
This patch is largely based on work of Fabian Freyer.
---
po/POTFILES.in | 1 +
src/bhyve/bhyve_command.c | 143 -
src/bhyve/bhyve_device.c | 33 ++---
src/bhyve/bhyve_domain.c | 60 -
.../bhyvexml2argvdata/bhyvexml2argv-acpiapic.args | 4 +-
tests/bhyvexml2argvdata/bhyvexml2argv-base.args| 4 +-
.../bhyvexml2argv-bhyveload-bootorder.args | 5 +-
.../bhyvexml2argv-bhyveload-bootorder1.args| 5 +-
.../bhyvexml2argv-bhyveload-bootorder3.args| 5 +-
.../bhyvexml2argv-bhyveload-explicitargs.args | 4 +-
tests/bhyvexml2argvdata/bhyvexml2argv-console.args | 2 +-
.../bhyvexml2argv-custom-loader.args | 4 +-
.../bhyvexml2argv-disk-cdrom-grub.args | 4 +-
.../bhyvexml2argv-disk-cdrom.args | 4 +-
.../bhyvexml2argv-grub-bootorder.args | 6 +-
.../bhyvexml2argv-grub-bootorder2.args | 6 +-
.../bhyvexml2argv-grub-defaults.args | 4 +-
.../bhyvexml2argvdata/bhyvexml2argv-localtime.args | 4 +-
tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args | 4 +-
.../bhyvexml2argv-serial-grub-nocons.args | 2 +-
.../bhyvexml2argv-serial-grub.args | 2 +-
tests/bhyvexml2argvdata/bhyvexml2argv-serial.args | 2 +-
tests/bhyvexml2argvtest.c | 2 +-
.../bhyvexml2xmlout-acpiapic.xml | 4 +-
tests/bhyvexml2xmloutdata/bhyvexml2xmlout-base.xml | 4 +-
.../bhyvexml2xmlout-bhyveload-bootorder.xml| 4 +-
.../bhyvexml2xmlout-bhyveload-bootorder1.xml | 4 +-
.../bhyvexml2xmlout-bhyveload-bootorder2.xml | 4 +-
.../bhyvexml2xmlout-bhyveload-bootorder3.xml | 4 +-
.../bhyvexml2xmlout-bhyveload-bootorder4.xml | 4 +-
.../bhyvexml2xmlout-bhyveload-explicitargs.xml | 4 +-
.../bhyvexml2xmlout-console.xml| 4 +-
.../bhyvexml2xmlout-custom-loader.xml | 4 +-
.../bhyvexml2xmlout-disk-cdrom-grub.xml| 4 +-
.../bhyvexml2xmlout-disk-cdrom.xml | 4 +-
.../bhyvexml2xmlout-grub-bootorder.xml | 4 +-
.../bhyvexml2xmlout-grub-bootorder2.xml| 4 +-
.../bhyvexml2xmlout-grub-defaults.xml | 4 +-
.../bhyvexml2xmlout-localtime.xml | 4 +-
.../bhyvexml2xmlout-macaddr.xml| 4 +-
.../bhyvexml2xmlout-metadata.xml | 5 +-
.../bhyvexml2xmlout-serial-grub-nocons.xml | 4 +-
.../bhyvexml2xmlout-serial-grub.xml| 4 +-
.../bhyvexml2xmloutdata/bhyvexml2xmlout-serial.xml | 4 +-
tests/bhyvexml2xmltest.c | 2 +
45 files changed, 279 insertions(+), 118 deletions(-)
diff --git a/po/POTFILES.in b/po/POTFILES.in
index e66bb7a3a..632aa7736 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -14,6 +14,7 @@ src/access/viraccessdriverpolkit.c
src/access/viraccessmanager.c
src/bhyve/bhyve_command.c
src/bhyve/bhyve_device.c
+src/bhyve/bhyve_domain.c
src/bhyve/bhyve_driver.c
src/bhyve/bhyve_monitor.c
src/bhyve/bhyve_parse_command.c
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index 8a29977ff..a2fd40378 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -148,40 +148,97 @@ bhyveBuildConsoleArgStr(const virDomainDef *def,
virCommandPtr cmd)
}
static int
-bhyveBuildDiskArgStr(const virDomainDef *def ATTRIBUTE_UNUSED,
- virDomainDiskDefPtr disk,
- virCommandPtr cmd)
[libvirt] [PATCH v1 3/4] bhyve: fix SATA address allocation
As bhyve for a long time didn't have a notion of the explicit SATA
controller and created a controller for each drive, the bhyve driver
in libvirt acted in a similar way and didn't care about the SATA
controllers and assigned PCI addresses to drives directly, as
the generated command will look like this anyway:
2:0,ahci-hd,somedisk.img
This no longer makes sense because:
1. After commit c07d1c1c4f it's not possible to assign
PCI addresses to disks
2. Bhyve now supports multiple disk drives for a controller,
so it's going away from 1:1 controller:disk mapping, so
the controller object starts to make more sense now
So, this patch does the following:
- Assign PCI address to SATA controllers (previously we didn't do this)
- Assign disk addresses instead of PCI addresses for disks. Now, when
building a bhyve command, we take PCI address not from the disk
itself but from its controller
- Assign addresses at XML parsing time using the
assignAddressesCallback. This is done mainly for being able to
verify address allocation via xml2xml tests
- Adjust existing bhyvexml2{xml,argv} tests to chase the new
address allocation
This patch is largely based on work of Fabian Freyer.
---
po/POTFILES.in | 1 +
src/bhyve/bhyve_command.c | 143 -
src/bhyve/bhyve_device.c | 33 ++---
src/bhyve/bhyve_domain.c | 60 -
.../bhyvexml2argvdata/bhyvexml2argv-acpiapic.args | 4 +-
tests/bhyvexml2argvdata/bhyvexml2argv-base.args| 4 +-
.../bhyvexml2argv-bhyveload-bootorder.args | 5 +-
.../bhyvexml2argv-bhyveload-bootorder1.args| 5 +-
.../bhyvexml2argv-bhyveload-bootorder3.args| 5 +-
.../bhyvexml2argv-bhyveload-explicitargs.args | 4 +-
tests/bhyvexml2argvdata/bhyvexml2argv-console.args | 2 +-
.../bhyvexml2argv-custom-loader.args | 4 +-
.../bhyvexml2argv-disk-cdrom-grub.args | 4 +-
.../bhyvexml2argv-disk-cdrom.args | 4 +-
.../bhyvexml2argv-grub-bootorder.args | 6 +-
.../bhyvexml2argv-grub-bootorder2.args | 6 +-
.../bhyvexml2argv-grub-defaults.args | 4 +-
.../bhyvexml2argvdata/bhyvexml2argv-localtime.args | 4 +-
tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args | 4 +-
.../bhyvexml2argv-serial-grub-nocons.args | 2 +-
.../bhyvexml2argv-serial-grub.args | 2 +-
tests/bhyvexml2argvdata/bhyvexml2argv-serial.args | 2 +-
tests/bhyvexml2argvtest.c | 2 +-
.../bhyvexml2xmlout-acpiapic.xml | 4 +-
tests/bhyvexml2xmloutdata/bhyvexml2xmlout-base.xml | 4 +-
.../bhyvexml2xmlout-bhyveload-bootorder.xml| 4 +-
.../bhyvexml2xmlout-bhyveload-bootorder1.xml | 4 +-
.../bhyvexml2xmlout-bhyveload-bootorder2.xml | 4 +-
.../bhyvexml2xmlout-bhyveload-bootorder3.xml | 4 +-
.../bhyvexml2xmlout-bhyveload-bootorder4.xml | 4 +-
.../bhyvexml2xmlout-bhyveload-explicitargs.xml | 4 +-
.../bhyvexml2xmlout-console.xml| 4 +-
.../bhyvexml2xmlout-custom-loader.xml | 4 +-
.../bhyvexml2xmlout-disk-cdrom-grub.xml| 4 +-
.../bhyvexml2xmlout-disk-cdrom.xml | 4 +-
.../bhyvexml2xmlout-grub-bootorder.xml | 4 +-
.../bhyvexml2xmlout-grub-bootorder2.xml| 4 +-
.../bhyvexml2xmlout-grub-defaults.xml | 4 +-
.../bhyvexml2xmlout-localtime.xml | 4 +-
.../bhyvexml2xmlout-macaddr.xml| 4 +-
.../bhyvexml2xmlout-metadata.xml | 5 +-
.../bhyvexml2xmlout-serial-grub-nocons.xml | 4 +-
.../bhyvexml2xmlout-serial-grub.xml| 4 +-
.../bhyvexml2xmloutdata/bhyvexml2xmlout-serial.xml | 4 +-
tests/bhyvexml2xmltest.c | 2 +
45 files changed, 279 insertions(+), 118 deletions(-)
diff --git a/po/POTFILES.in b/po/POTFILES.in
index e66bb7a3a..632aa7736 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -14,6 +14,7 @@ src/access/viraccessdriverpolkit.c
src/access/viraccessmanager.c
src/bhyve/bhyve_command.c
src/bhyve/bhyve_device.c
+src/bhyve/bhyve_domain.c
src/bhyve/bhyve_driver.c
src/bhyve/bhyve_monitor.c
src/bhyve/bhyve_parse_command.c
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index 8a29977ff..a2fd40378 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -148,40 +148,97 @@ bhyveBuildConsoleArgStr(const virDomainDef *def,
virCommandPtr cmd)
}
static int
-bhyveBuildDiskArgStr(const virDomainDef *def ATTRIBUTE_UNUSED,
- virDomainDiskDefPtr disk,
- virCommandPtr cmd)
+bhyveBuildAHCIControllerArgStr(const virDomainDef *def,
+ virDomainControllerDefPtr controller,
+
