Bug 1797879

Patch proposed upstream:

https://listman.redhat.com/archives/libvir-list/2021-June/msg00050.html

Merged upstream as:

Author:     Michal Prívozník <mprivozn>
AuthorDate: Wed Jun 2 10:34:46 2021 +0200
Commit:     Michal Prívozník <mprivozn>
CommitDate: Wed Jun 9 08:07:07 2021 +0200

    storage: Don't overwrite error in virISCSIDirectDisconnect()
    
    The iscsi-direct storage pool backend works merely like this: a
    connection is established to the target (usually done via
    virStorageBackendISCSIDirectSetConnection()), intended action is
    executed (e.g. reporting LUNs, volume wiping), and at the end the
    connection is closed via virISCSIDirectDisconnect().
    
    The problem is that virISCSIDirectDisconnect() reports its own
    errors which may overwrite error that occurred during LUN
    reporting, or volume wiping or whatever.
    
    To fix this, use virErrorPreserveLast() + virErrorRestore()
    combo, which either preserves previously reported error message,
    or is NOP if there's no error reported.
    
    Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1797879
    Signed-off-by: Michal Privoznik <mprivozn>
    Reviewed-by: Ján Tomko <jtomko>

v7.4.0-72-ga190906977

Verified:tested version:
libvirt-7.0.0-4.fc34.x86_64
qemu-kvm-5.2.0-8.fc34.x86_64

Get the expected result:
# virsh pool-create pool.xml 
error: Failed to create pool from pool.xml
error: internal error: Failed to login: Failed to log in to target. Status: Service unavailable(769)

Hi, Michal:

I can't get the expected result in libvirt-7.5.0-1.module+el8.5.0+11664+59f87560.x86_64 for this bug:
# iscsiadm -m discovery -t st -p 10.66.87.196
10.66.87.196:3260,1 iqn.2021-04.com.virttest:emulated-iscsi.target
# cat iscsi-direct.pool 
<pool type="iscsi-direct">
  <name>iscsi-direct</name>
  <source>
    <host name="10.66.87.196"/>
    <device path="iqn.2021-04.com.virttest:emulated-iscsi.target1"/>
    <initiator>
      <iqn name="iqn.2021-07.com.example:iscsi-initiator"/>
    </initiator>
  </source>
</pool>
# virsh pool-create iscsi-direct.pool
error: Failed to create pool from iscsi-direct.pool
error: internal error: Failed to reportluns: Failed to send ReportLuns command

But the error message in upstream is expected:
# virsh pool-create pool.xml 
error: Failed to create pool from pool.xml
error: internal error: Failed to login: Failed to log in to target. Status: Service unavailable(769)

Would you like to check this difference? Thanks.

(In reply to Meina Li from comment #8)
> 
> Would you like to check this difference? Thanks.

So the difference is that comment 5 tested libvirt on Fedora 34 and comment 8 on RHEL. I suspect there's a different version of libiscsi in RHEL and Fedora. In fact, RHEL has 1.18.0 which was released ~5 years ago. Therefore, it's very likely that libiscsi was fixed meanwhile to report error during login rather than when trying to list LUNs. I think it's okay, because this bug is originally about something slightly different: we were overwriting error when creating the pool and thus reported misleading error message. This is no longer the case. While I agree that error message is still weird a bit it is definitely better than it was.

BTW I think the following libiscsi commit fixed the libiscsi behaviour:

https://github.com/sahlberg/libiscsi/commit/e9c1f102588447255c75f0d57bbdc8341fc4ec96

Actually, I ran bisect and found that the following libiscsi commit fixes the problem:

https://github.com/sahlberg/libiscsi/commit/b5210a1e31afb9f0adf369cde11c612f3365432e

It's contained in 1.19.0 but not in 1.18.0 release. Hence the difference.

So according to comment 9, move this bug to be verified.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (virt:av bug fix and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2021:4684