On 04/24/2017 02:02 PM, Eric Farman wrote:
> 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'
>
> Seems that the virReportError issued for an internal error
> overwrites one that might have already existed. Rather than
> remove them altogether (there may be error paths that don't
> spit out a more helpful message), I thought maybe it'd be
> best to check if one exists, and exit if one does. If not,
> the existing internal error messages are probably helpful.
Yep - this is more or less what happens you get the last error reported.
Although I
>
> Make this adjustment for both virtio-scsi and vhost-scsi
> devices.
>
> Signed-off-by: Eric Farman <farman@xxxxxxxxxxxxxxxxxx>
> Reviewed-by: Bjoern Walk <bwalk@xxxxxxxxxxxxxxxxxx>
> Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxxxxxxx>
> ---
> src/qemu/qemu_hotplug.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 120bcdc..d421a77 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -2470,6 +2470,10 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
> if (qemuHostdevPrepareSCSIDevices(driver, vm->def->name,
> &hostdev, 1)) {
This should have been a "< 0" check, sigh.... Looks like commit id
'8f76ad99' never added that... This should be adjusted as well.
> virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi;
> +
> + if (virGetLastError())
> + return -1;
> +
So the question becomes can qemuHostdevPrepareSCSIDevices return -1
without setting an error message...
For all the paths I checked I couldn't find one that did, so I feel
comfortable in just saying nix the messages below.
> if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) {
> virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi;
> virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -2595,9 +2599,12 @@ qemuDomainAttachSCSIVHostDevice(virQEMUDriverPtr driver,
>
> 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 (!virGetLastError()) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Unable to prepare scsi_host hostdev: %s"),
> + hostsrc->wwpn);
> + }
Same here... Although it's related to the previous one - let's create a
separate patch for it.
I think you end up with 3 patches #1 to check -1, #2 to get rid of the
first set of messages, and #3 to get rid of this message set.
John
> return -1;
> }
>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
