[libvirt] [PATCH v2 0/3] Cleanup messages after hotplug of scsi hostdev

v1 was here: https://www.redhat.com/archives/libvir-list/2017-April/msg01066.html Per review, removed the "internal error" messages and split the resulting patch into patches 2 (virtio-scsi) and 3 (vhost-scsi). Patch 1 fixes a return code check that is a little off. Eric Farman (3): qemu: Check return code from qemuHostdevPrepareSCSIDevices qemu: Remove extra messages from virtio-scsi hotplug qemu: Remove extra messages for vhost-scsi hotplug src/qemu/qemu_hotplug.c | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-) -- 1.9.1

Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> --- src/qemu/qemu_hotplug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index eec99af..6d568d6 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2468,7 +2468,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, } if (qemuHostdevPrepareSCSIDevices(driver, vm->def->name, - &hostdev, 1)) { + &hostdev, 1) < 0) { virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi; -- 1.9.1

I tried to attach a SCSI LUN to two different guests, and forgot to specify "shareable" in the hostdev XML. Attaching the device to the second guest failed, but the message was not helpful in telling me what I was doing wrong: $ cat scsi_scratch_disk.xml <hostdev mode='subsystem' type='scsi'> <source> <adapter name='scsi_host3'/> <address bus='0' target='15' unit='1074151456'/> </source> </hostdev> $ virsh attach-device dasd_sles_d99c scsi_scratch_disk.xml Device attached successfully $ virsh attach-device dasd_fedora_0e1e scsi_scratch_disk.xml error: Failed to attach device from scsi_scratch_disk.xml error: internal error: Unable to prepare scsi hostdev: scsi_host3:0:15:1074151456 I eventually discovered my error, but thought it was weird that Libvirt doesn't provide something more helpful in this case. Looking over the code we had just gone through, I commented out the "internal error" message, and got something more useful: $ virsh attach-device dasd_fedora_0e1e scsi_scratch_disk.xml error: Failed to attach device from scsi_scratch_disk.xml error: Requested operation is not valid: SCSI device 3:0:15:1074151456 is already in use by other domain(s) as 'non-shareable' Looking over the error paths here, we seem to issue better messages deeper in the callchain so these "internal error" messages overwrite any of them. Remove them, so that the more detailed errors are seen. Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> --- src/qemu/qemu_hotplug.c | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 6d568d6..f9e8b0a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2467,23 +2467,8 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, return -1; } - if (qemuHostdevPrepareSCSIDevices(driver, vm->def->name, - &hostdev, 1) < 0) { - virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; - if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { - virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi; - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to prepare scsi hostdev for iSCSI: %s"), - iscsisrc->path); - } else { - virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to prepare scsi hostdev: %s:%u:%u:%llu"), - scsihostsrc->adapter, scsihostsrc->bus, - scsihostsrc->target, scsihostsrc->unit); - } + if (qemuHostdevPrepareSCSIDevices(driver, vm->def->name, &hostdev, 1) < 0) return -1; - } if (qemuDomainNamespaceSetupHostdev(driver, vm, hostdev) < 0) goto cleanup; -- 1.9.1

As with virtio-scsi, the "internal error" messages after preparing a vhost-scsi hostdev overwrites more meaningful error messages deeper in the callchain. Remove it too. Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com> --- src/qemu/qemu_hotplug.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f9e8b0a..4ca1086 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2578,13 +2578,8 @@ qemuDomainAttachSCSIVHostDevice(virQEMUDriverPtr driver, return -1; } - if (qemuHostdevPrepareSCSIVHostDevices(driver, vm->def->name, &hostdev, 1) < 0) { - virDomainHostdevSubsysSCSIVHostPtr hostsrc = &hostdev->source.subsys.u.scsi_host; - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to prepare scsi_host hostdev: %s"), - hostsrc->wwpn); + if (qemuHostdevPrepareSCSIVHostDevices(driver, vm->def->name, &hostdev, 1) < 0) return -1; - } if (qemuDomainNamespaceSetupHostdev(driver, vm, hostdev) < 0) goto cleanup; -- 1.9.1

On 04/26/2017 05:09 PM, Eric Farman wrote:
v1 was here: https://www.redhat.com/archives/libvir-list/2017-April/msg01066.html
Per review, removed the "internal error" messages and split the resulting patch into patches 2 (virtio-scsi) and 3 (vhost-scsi). Patch 1 fixes a return code check that is a little off.
Eric Farman (3): qemu: Check return code from qemuHostdevPrepareSCSIDevices qemu: Remove extra messages from virtio-scsi hotplug qemu: Remove extra messages for vhost-scsi hotplug
src/qemu/qemu_hotplug.c | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-)
ACK and pushed... Tks, John
participants (2)
-
Eric Farman
-
John Ferlan