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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Roman Bogorodskiy