[libvirt] [PATCH 2/3] qemu: Separate calls based on controller bus type
Eric Farman
farman at linux.vnet.ibm.com
Tue Nov 12 16:35:12 UTC 2013
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 at xxxxxxxx ~]# cat controller.xml
>> <controller type='scsi' model='virtio-scsi' index='0' />
>> [root at 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 at 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 at linux.vnet.ibm.com>
>> ---
>> 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
More information about the libvir-list
mailing list