Laine Stump wrote:
> Rather than only assigning a PCI address when no address is given at
> all, also do it when the config says that the address type is 'pci',
> but it gives no address.
> ---
> src/bhyve/bhyve_device.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c
> index 3eb2956..8373a5f 100644
> --- a/src/bhyve/bhyve_device.c
> +++ b/src/bhyve/bhyve_device.c
> @@ -98,7 +98,7 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def,
> goto error;
>
> for (i = 0; i < def->nnets; i++) {
> - if (def->nets[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
> + if (!virDeviceInfoPCIAddressWanted(&def->nets[i]->info))
> continue;
> if (virDomainPCIAddressReserveNextSlot(addrs,
> &def->nets[i]->info,
> @@ -107,8 +107,7 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def,
> }
>
> for (i = 0; i < def->ndisks; i++) {
> - if (def->disks[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
> - def->disks[i]->info.addr.pci.slot != 0)
> + if (!virDeviceInfoPCIAddressWanted(&def->disks[i]->info))
> continue;
I just noticed that this change breaks address allocation for disks in
some cases.
E.g. for the following piece virDeviceInfoPCIAddressWanted() returns false
because it expects info.type to be either
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE or
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI, but we have
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE in this case.
<disk type='file' device='cdrom'>
<driver name='file' type='raw'/>
<source file='/home/test/foobar.iso'/>
<target dev='hdc' bus='sata'/>
<readonly/>
</disk>
Therefore address is not allocated, it stays 0:0 that's reserved for the
hostbridge and therefore VM fails to start.
Adding <address type='pci'/> fixes that, but that's not very obvious
thing for users.
Generally, it makes sense, but not in the bhyve driver currently,
because it's been using a scheme where each disk resides on its own
controller, so we didn't have to bother with disk addressing.
Not so long ago a possibility to have multiple disk on a single
controller was introduced to bhyve:
https://svnweb.freebsd.org/base?view=revision&revision=302459 (thanks
to Fabian for the link!) and we'd need to implement proper disk
addressing for it.
However, for the upcoming release I'm wondering if we should go with a
simple solution/workaround similar to this one:
diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c
index 8373a5f..f662012 100644
--- a/src/bhyve/bhyve_device.c
+++ b/src/bhyve/bhyve_device.c
@@ -107,7 +107,8 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def,
}
for (i = 0; i < def->ndisks; i++) {
- if (!virDeviceInfoPCIAddressWanted(&def->disks[i]->info))
+ if (def->disks[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+ !virPCIDeviceAddressIsEmpty(&def->disks[i]->info.addr.pci))
continue;
if (virDomainPCIAddressReserveNextSlot(addrs,
&def->disks[i]->info,
Thoughts?
> if (virDomainPCIAddressReserveNextSlot(addrs,
> &def->disks[i]->info,
> @@ -118,9 +117,8 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def,
>
> for (i = 0; i < def->ncontrollers; i++) {
> if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
> - if (def->controllers[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
> - continue;
> - if (def->controllers[i]->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)
> + if (def->controllers[i]->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT ||
> + !virDeviceInfoPCIAddressWanted(&def->controllers[i]->info))
> continue;
>
> if (virDomainPCIAddressReserveNextSlot(addrs,
> --
> 2.5.5
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Roman Bogorodskiy
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list