On 11/12/2013 02:30 AM, Ján Tomko wrote:
On 10/08/2013 02:45 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 <farman@xxxxxxxxxxxxxxxxxx>
---
src/qemu/qemu_hotplug.c | 45 +++++++++++++++++++++++++++++++--------------
1 file changed, 31 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index bc0f57a..70a1b3d 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) {
+ if (STREQLEN(vm->def->os.machine, "s390-ccw", 8) &&
STRPREFIX(vm->def->os.machine, "s390-ccw") should do the same
Ok
+ 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;
+ }
+
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
- if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &controller->info) < 0)
- goto cleanup;
+ if (!controller->info.type ||
I'd prefer a comparison against VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE instead.
Ok
+ 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;
@@ -399,7 +413,8 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
qemuDomainObjExitMonitor(driver, vm);
if (ret == 0) {
- controller->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+ if (!controller->info.type)
Here too.
+ controller->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
virDomainControllerInsertPreAlloced(vm->def, controller);
}
@@ -3024,18 +3039,20 @@ 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;
+ }
Shouldn't we check if there is either a VIRTIO_S390, VIRTIO_CCW or a PCI address?
Ah, yes. This is true for VIRTIO_CCW, but not VIRTIO_S390. We don't
create one on the attach side, and besides the
virDomainDeviceAddressIsValid routine always returns true for that type.
I'll send a v2 patch shortly.
- 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;
+ }
}
if (qemuDomainControllerIsBusy(vm, detach)) {
Jan
Eric
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list