[libvirt] [PATCH v1 3/4] bhyve: fix SATA address allocation

[Roman Bogorodskiy]

  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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170130/d254f977/attachment-0001.sig>