On 11/12/2013 05:59 PM, Eric Farman wrote: > For systems without a PCI bus, attaching a SCSI controller fails: > > [root@xxxxxxxx ~]# cat controller.xml > <controller type='scsi' model='virtio-scsi' index='0' /> > [root@xxxxxxxx ~]# virsh attach-device guest01 controller.xml > error: Failed to attach device from controller.xml > error: XML error: No PCI buses available > > A similar problem occurs with the detach of a controller: > > [root@xxxxxxxx ~]# virsh detach-device guest01 controller.xml > error: Failed to detach device from controller.xml > error: operation failed: controller scsi:0 not found > > The qemuDomainXXtachPciControllerDevice routines made assumptions > that any caller had a PCI bus. These routines now selectively calls > PCI functions where necessary, and assigns the device information > type to one appropriate for the bus in use. > > Signed-off-by: Eric Farman <[email protected]> > --- > src/qemu/qemu_hotplug.c | 52 > ++++++++++++++++++++++++++++++++++------------- > 1 file changed, 38 insertions(+), 14 deletions(-) > > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c > index 29c6aea..f18e233 100644 > --- a/src/qemu/qemu_hotplug.c > +++ b/src/qemu/qemu_hotplug.c > @@ -365,9 +365,23 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr > driver, > return -1; > } > > + if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { > + if (STRPREFIX(vm->def->os.machine, "s390-ccw") && > + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) > + controller->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; > + else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) > + controller->info.type = > VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390; > + } > +
These would be better inside the if below (if QEMU_CAPS_DEVICE is not set, we
don't probe for the VIRTIO_CCW and VIRTIO_S390 caps anyway).
> if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
> - if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs,
> &controller->info) < 0)
> - goto cleanup;
> + if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
> + controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
> + if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs,
> &controller->info) < 0)
> + goto cleanup;
> + } else if (controller->info.type ==
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
> + if (qemuDomainCCWAddressAssign(&controller->info,
> priv->ccwaddrs, true) < 0)
> + goto cleanup;
> + }
> releaseaddr = true;
> if (qemuAssignDeviceControllerAlias(controller) < 0)
> goto cleanup;
> @@ -3031,18 +3046,27 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr
> driver,
>
> detach = vm->def->controllers[idx];
>
> - if (!virDomainDeviceAddressIsValid(&detach->info,
> - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) {
> - virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> - _("device cannot be detached without a PCI address"));
> - goto cleanup;
> - }
> + if (detach->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
> + if (!virDomainDeviceAddressIsValid(&detach->info,
> +
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) {
> + virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> + _("device cannot be detached without a PCI
> address"));
> + goto cleanup;
> + }
>
> - if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) {
> - virReportError(VIR_ERR_OPERATION_FAILED,
> - _("cannot hot unplug multifunction PCI device: %s"),
> - dev->data.disk->dst);
> - goto cleanup;
> + if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) {
> + virReportError(VIR_ERR_OPERATION_FAILED,
> + _("cannot hot unplug multifunction PCI device:
> %s"),
> + dev->data.disk->dst);
> + goto cleanup;
> + }
> + } else if (detach->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
> + if (!virDomainDeviceAddressIsValid(&detach->info,
> +
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) {
> + virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> + _("device cannot be detached without a CCW
> address"));
> + goto cleanup;
> + }
> }
>
> if (qemuDomainControllerIsBusy(vm, detach)) {
>
This doesn't catch the case of ADDRESS_TYPE_NONE.
How about:
if (detach->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
detach->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW &&
detach->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) {
virReportError(VIR_ERR_OPERATION_FAILED,
_("device with '%s' address cannot be detached"),
virDomainDeviceAddressTypeToString(detach->info.type));
goto cleanup;
}
if (!virDomainDeviceAddressIsValid(&detach->info, detach->info.type)) {
virReportError(VIR_ERR_OPERATION_FAILED,
_("device with invalid '%s' address cannot be detached"),
virDomainDeviceAddressTypeToString(detach->info.type));
goto cleanup;
}
if (detach->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
qemuIsMultiFunctionDevice(vm->def, &detach->info)) {
virReportError(VIR_ERR_OPERATION_FAILED,
_("cannot hot unplug multifunction PCI device: %s"),
dev->data.disk->dst);
goto cleanup;
}
Jan
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
