So, these are multiple separate issues described here.

The first that instantiates the password via '--object secret' will never work and it never worked before. This is the one thing that we try to fix with blockdev. In such case you must provide the backing image data with authentication explicitly in the XML. This will then ensure that the secret objects are created properly:

<disk type='file' device='disk'>
  <source file='/tmp/iscsi_auth_cipher'/>
  <backingStore type='network'>
    <source protocol='iscsi' name='iqn.2019-08.com.chap.target/0'>
      <host name='10.66.4.183'/>
      <auth username='redhat'>
        <secret type='iscsi' usage='FILL_ME_IN'/>
       </auth>
    </source>
    <backingStore/>
  </backingStore>
</disk>

This will properly instantiate a libvirt secret for the backing file.

The ISCSI URI case and json password passthrough scenarios which probably would work prior to enabling of blockdev, it's questionable whether it's a good idea to keep supporting them. Passing in the secrets in files is not very secure.

At any rate I can investigate whether we'll attempt to pass them through via secret objects.

> Libvirt also fails to parse iscsi url with auth
> # qemu-img create -b
> iscsi://redhat%redhat.4.183/iqn.2019-08.com.chap.target/0

Note that this is not a valid URI. The userinfo part must divide the username and password with a ":" rather than "%" ...

> /tmp/iscsi_auth_cleartxt -o backing_fmt=raw -f qcow2
> Formatting '/tmp/iscsi_auth_cleartxt', fmt=qcow2 size=4294967296
> backing_file=iscsi://redhat%redhat.4.183/iqn.2019-08.com.chap.target/0
> backing_fmt=raw cluster_size=65536 lazy_refcounts=off refcount_bits=16
> 
> # virsh attach-disk copy /tmp/iscsi_auth_cleartxt vdc --subdriver qcow2     
> 
> error: Failed to attach disk
> error: internal error: failed to parse backing file location
> 'iscsi://redhat%redhat.4.183/iqn.2019-08.com.chap.target/0'

... this error message points out that such uri isn't even valid.

Fixed upstream: 

commit 6ff9241058dba35565ed594c7931e52a46a82b13
Author: Peter Krempa <pkrempa>
Date:   Thu Aug 15 19:29:43 2019 +0200

    util: storagefile: Flag backing store strings with authentication
    
    Using inline authentication for storage volumes will not work properly
    as libvirt requires use of the secret driver for the auth data and
    thus would not be able to represent the passwords stored in the backing
    store string.
    
    Make sure that the backing store parsers return 1 which is a sign for
    the caller to not use the file in certain cases.
    
    The test data include iscsi via a json pseudo-protocol string and URIs
    with the userinfo part being present.
    
    Signed-off-by: Peter Krempa <pkrempa>
    Reviewed-by: Ján Tomko <jtomko>

commit b1c778d854a927c3b7d11f06add9761148b6d5d5
Author: Peter Krempa <pkrempa>
Date:   Fri Aug 16 11:34:27 2019 +0200

    util: storagefile: Don't traverse storage sources unusable by VM
    
    virStorageFileGetMetadataRecurse would include files in the backing
    chain which would not really be usable by libvirt directly e.g.
    when such file would be promoted to the top layer by an active block
    commit as for example inline authentication data can't be represented in
    the VM xml file. The idea is to use secrets for this.
    
    With the changes to the backing store string parsers we can report and
    propagate if such a thing is present in the configuration and thus start
    skipping those files in the backing chain traversal code. This approach
    still allows to report the appropriate backing store string in the
    storage driver which doesn't directly use the backing file.
    
    Signed-off-by: Peter Krempa <pkrempa>
    Reviewed-by: Ján Tomko <jtomko>

commit 46135dd40fbe112211c4ed7f6d9a6cc7b4001af1
Author: Peter Krempa <pkrempa>
Date:   Fri Aug 16 11:28:03 2019 +0200

    util: storagefile: Clarify docs for '@report_broken' of virStorageFileGetMetadata
    
    virStorageFileGetMetadata does not report error if we can't interrogate
    the file somehow. Clarify this in the description of the @report_broken
    flag as it implies we should report an error in that case. The problem
    is that we don't know whether there's a problem and unfortunately just
    offload it to qemu.
    
    Signed-off-by: Peter Krempa <pkrempa>
    Reviewed-by: Ján Tomko <jtomko>

commit 9467c37e963f9a68db0b6a32ccb331fbdc2664ae
Author: Peter Krempa <pkrempa>
Date:   Fri Aug 16 11:14:31 2019 +0200

    util: storagefile: Add handling of unusable storage sources
    
    Introduce new semantics to virStorageSourceNewFromBacking and some
    of the helpers used by it which propagate the return value from the
    callers.
    
    The new return value introduced by this patch allows to notify the
    calller that the parsed virStorageSource correctly describes the source
    but contains data such as inline authentication which libvirt does not
    want to support directly. This means that such file would e.g. unusable
    as a storage source (e.g. when actively commiting the overlay to it) or
    would not work with blockdev.
    
    The caller will then be able to decide whether to consider this backing
    file as viable or just fall back to qemu dealing with it.
    
    Signed-off-by: Peter Krempa <pkrempa>
    Reviewed-by: Ján Tomko <jtomko>

commit e1189ae5f320cfe8c87588ffd15ca02840391918
Author: Peter Krempa <pkrempa>
Date:   Thu Aug 15 19:27:43 2019 +0200

    tests: virstorage: Allow testing return value of virStorageSourceNewFromBackingAbsolute
    
    Modify testBackingParse to allow testing other return values of the
    backing store string parser.
    
    Signed-off-by: Peter Krempa <pkrempa>
    Reviewed-by: Ján Tomko <jtomko>

commit 5265743daad7c1ea0c501f5f017062d85042d1d8
Author: Peter Krempa <pkrempa>
Date:   Thu Aug 15 16:43:40 2019 +0200

    util: storagefile: Modify arguments of virStorageSourceNewFromBackingAbsolue
    
    Return the parsed storage source via an pointer in arguments and return
    an integer from the function. Describe the semantics with a comment for
    the function and adjust callers to the new semantics.
    
    Signed-off-by: Peter Krempa <pkrempa>
    Reviewed-by: Ján Tomko <jtomko>

commit dddc5524001310abf0246f9a27c268c351f78b13
Author: Peter Krempa <pkrempa>
Date:   Thu Aug 15 16:21:55 2019 +0200

    util: storagefile: Preserve return value in virStorageSourceParseBackingJSONUriStr
    
    virStorageSourceParseBackingURI will report special return values in
    some cases. Preserve it in virStorageSourceParseBackingJSONUriStr.
    
    Signed-off-by: Peter Krempa <pkrempa>
    Reviewed-by: Ján Tomko <jtomko>

commit 36cde667083d381eb247dcd5581e84b51d9e9fba
Author: Peter Krempa <pkrempa>
Date:   Thu Aug 15 14:49:49 2019 +0200

    util: storage: Modify return value of virStorageSourceNewFromBacking
    
    Return the storage source definition via a pointer in the arguments and
    document the returned values. This will simplify the possibility to
    ignore certain backing store types which are not representable by
    libvirt.
    
    Signed-off-by: Peter Krempa <pkrempa>
    Reviewed-by: Ján Tomko <jtomko>

commit ffabad75725f70eb35d9b7422f19122a6b5174b0
Author: Peter Krempa <pkrempa>
Date:   Thu Aug 15 19:16:21 2019 +0200

    tests: storage: Refactor cleanup in testBackingParse
    
    Automatically clean the temporary buffer and get rid of the cleanup
    label.
    
    Signed-off-by: Peter Krempa <pkrempa>
    Reviewed-by: Ján Tomko <jtomko>

commit aece36f7679c6aba91708d2a912543ea83b5cd5b
Author: Peter Krempa <pkrempa>
Date:   Thu Aug 15 16:16:59 2019 +0200

    tests: viruri: Add test for password in URI userinfo
    
    While it's a bad idea to use userinfo to pass credentials via a URI add
    a test that we at least do the correct thing.
    
    Signed-off-by: Peter Krempa <pkrempa>
    Reviewed-by: Ján Tomko <jtomko>

commit fe434a0ceb72e0689f6be6b8fef42b74ea2ac278
Author: Peter Krempa <pkrempa>
Date:   Thu Aug 15 16:25:47 2019 +0200

    util: storagefile: Simplify cleanup in virStorageSourceParseBackingJSON
    
    Automatically free the 'root' temporary variable to get rid of some
    complexity.
    
    Signed-off-by: Peter Krempa <pkrempa>
    Reviewed-by: Ján Tomko <jtomko>

commit e8578b245bd216f90711111d266c9583ca00fac4
Author: Peter Krempa <pkrempa>
Date:   Thu Aug 15 16:06:51 2019 +0200

    util: storagefile: Simplify cleanup handling in virStorageSourceParseBackingURI
    
    Automatically clean the 'uri' variable and get rid of the 'cleanup'
    label.
    
    Signed-off-by: Peter Krempa <pkrempa>
    Reviewed-by: Ján Tomko <jtomko>

commit 5a8de41e0fe9af77fdc08c9616290061ad958e27
Author: Peter Krempa <pkrempa>
Date:   Thu Aug 15 15:54:04 2019 +0200

    util: storagefile: Remove cleanup label from virStorageSourceParseBackingJSONiSCSI
    
    There is no cleanup code.
    
    Signed-off-by: Peter Krempa <pkrempa>
    Reviewed-by: Ján Tomko <jtomko>

commit ab1021e30479d3fcb0e0a58a09d154d0b2b9bc88
Author: Peter Krempa <pkrempa>
Date:   Thu Aug 15 15:41:58 2019 +0200

    util: storage: Simplify cleanup path handling in virStorageSourceParseBackingJSONInternal
    
    Automatically free the intermediate JSON data to get rid of the cleanup
    section.
    
    Signed-off-by: Peter Krempa <pkrempa>
    Reviewed-by: Ján Tomko <jtomko>

v5.6.0-353-g6ff9241058

Re-test the scenarios in this bug with libvirt secret and the test is passed.
Verified Version:
# rpm -q libvirt qemu-kvm
libvirt-6.0.0-15.virtcov.el8.x86_64
qemu-kvm-4.2.0-15.module+el8.2.0+6029+618ef2ec.x86_64

Verified Steps:
Pre-step: 
1. Setup a iscsi target with chap authorization redhat:redhat
2. Define an iscsi secret.

SC1: Refresh the default pool and attach the overlay image creating by iscsi json with authentication
1. Create the overlay image:
# qemu-img create --object secret,data='redhat',id=sec,format=raw -f qcow2 -b 'json:{"driver":"raw", "file":{"lun":"0", "portal":"10.66.144.87", "driver":"iscsi", "transport":"tcp", "target":"iqn.2020-04.com.chap.target", "user":"redhat", "password-secret":"sec"}}' -o backing_fmt='raw' /var/lib/libvirt/images/iscsi_auth_cipher
Formatting '/var/lib/libvirt/images/iscsi_auth_cipher', fmt=qcow2 size=10737418240 backing_file=json:{"driver":"raw",, "file":{"lun":"0",, "portal":"10.66.144.87",, "driver":"iscsi",, "transport":"tcp",, "target":"iqn.2020-04.com.chap.target",, "user":"redhat",, "password-secret":"sec"}} backing_fmt=raw cluster_size=65536 lazy_refcounts=off refcount_bits=16
2. Refresh the default pool and check the volume.
# virsh pool-refresh default
Pool default refreshed
 # virsh vol-list default | grep iscsi
 iscsi_auth_cipher                           /var/lib/libvirt/images/iscsi_auth_cipher

3. Prepare a disk xml which include backing image data with authentication
# cat disk.xml
<disk type='file' device='disk'>
  <driver name='qemu' type='qcow2' cache='none'/>
  <source file='/var/lib/libvirt/images/iscsi_auth_cipher'/>
  <backingStore type='network'>
    <format type='raw'/>
    <source protocol='iscsi' name='iqn.2020-04.com.chap.target/0'>
      <host name='10.66.144.87'/>
      <auth username='redhat'>
        <secret type='iscsi' usage='libvirtiscsi'/>
      </auth>
    </source>
  <backingStore/>
  <target dev='vdb' bus='virtio'/>
</disk>
 </backingStore>
4. Attach and detach the disk to a running guest.
# virsh attach-device lmn disk.xml 
Device attached successfully
# virsh domblklist lmn
 Target   Source
---------------------------------------------
 vda      /var/lib/libvirt/images/lmn.qcow2
 vdb      /var/lib/libvirt/images/iscsi_auth_cipher
# virsh detach-disk lmn vdb
Disk detached successfully
5. Cold plug this disk to the guest and unplug it.
# virsh attach-device lmn disk.xml --config 
Device attached successfully
# virsh destroy lmn;virsh start lmn
Domain lmn destroyed
Domain lmn started
# virsh detach-device lmn disk.xml --config 
Device detached successfully

SC2: Refresh the default pool and attach the overlay image creating by iscsi uri with authentication
1.  Create the overlay image:
# qemu-img create -f qcow2 -o backing_fmt=raw -b iscsi://redhat:redhat.144.87/iqn.2020-04.com.chap.target/0 /var/lib/libvirt/images/iscsi_auth_cipher
Formatting '/var/lib/libvirt/images/iscsi_auth_cipher', fmt=qcow2 size=10737418240 backing_file=iscsi://redhat:redhat.144.87/iqn.2020-04.com.chap.target/0 backing_fmt=raw cluster_size=65536 lazy_refcounts=off refcount_bits=16
2. Reproduced the step 2 ~ step 5 in SC1.

Hi Peter,

Please help to check the following issues, thank you in advance.

1) Can you help to check which scenarios I also need to cover? I only verified two scenarios in comment 5 which mentioned at the beginning of this bug because I found most of patches are related with return value or cleanup label.

2) The <auth>..</auth> is moved out of source element after doing blockcommit which I think it's unreasonable. Please review it.

SC3： Do blockcommit for the overlay image creating by iscsi json with authentication
1. Create the overlay image:
# qemu-img create --object secret,data='redhat',id=sec,format=raw -f qcow2 -b 'json:{"driver":"raw", "file":{"lun":"0", "portal":"10.66.144.87", "driver":"iscsi", "transport":"tcp", "target":"iqn.2020-04.com.chap.target", "user":"redhat", "password-secret":"sec"}}' -o backing_fmt='raw' /var/lib/libvirt/images/iscsi_auth_cipher
2. Start the guest with the following disk:
# virsh dumpxml lmn | grep /disk -B16
    <disk type='file' device='disk'>
      <driver name='qemu' type='qcow2' cache='none'/>
      <source file='/var/lib/libvirt/images/iscsi_auth_cipher' index='1'/>
      <backingStore type='network' index='2'>
        <format type='raw'/>
        <source protocol='iscsi' name='iqn.2020-04.com.chap.target/0'>
          <host name='10.66.144.87' port='3260'/>
          <auth username='redhat'>                                         ---->This auth element is in source element
            <secret type='iscsi' usage='libvirtiscsi'/>
          </auth>
        </source>
        <backingStore/>
      </backingStore>
      <target dev='vdb' bus='virtio'/>
      <alias name='virtio-disk1'/>
      <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/>
    </disk>
3. Do blockcommit and check the disk xml in guest.
# virsh blockcommit lmn vdb --verbose --pivot
Block commit: [100 %]
Successfully pivoted
# virsh dumpxml lmn | grep /disk -B12
    <disk type='network' device='disk'>
      <driver name='qemu' type='raw' cache='none'/>
      <auth username='redhat'>                                          ---->This auth element is out of source element
        <secret type='iscsi' usage='libvirtiscsi'/>
      </auth>
      <source protocol='iscsi' name='iqn.2020-04.com.chap.target/0' index='2'>
        <host name='10.66.144.87' port='3260'/>
      </source>
      <backingStore/>
      <target dev='vdb' bus='virtio'/>
      <alias name='virtio-disk1'/>
      <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/>
    </disk>

(In reply to Meina Li from comment #6)
> Hi Peter,
> 
> Please help to check the following issues, thank you in advance.
> 
> 1) Can you help to check which scenarios I also need to cover? I only
> verified two scenarios in comment 5 which mentioned at the beginning of this
> bug because I found most of patches are related with return value or cleanup
> label.

That should be okay I don't really have any other ideas

> 2) The <auth>..</auth> is moved out of source element after doing
> blockcommit which I think it's unreasonable. Please review it.

Hmm, yeah the block jobs will change it. Please file a bug, we should prefer when auth is under <source>.

Hi Peter,

About the auth element, I had another issue to confirm: 
Does the <auth> or <encryption> outside <source> are ignored now?

I found some bugs need to be verified with <auth> under in <source>. And I also found that creating rbd pool with auth outside of source element failed because it can't get related authorization. 
#cat rbd.pool
<pool type='rbd'>
<name>rbdpool</name>
<source>
<name>libvirt-pool</name>
<host name='10.66.146.31'/>
</source>
<auth username='admin'>
<secret type='ceph' usage='libvirtceph'/>
</auth>
</pool>

#virsh pool-create rbd.pool
error: Failed to create pool from rbd.pool
error: failed to connect to the RADOS monitor on: 10.66.146.31: Success

#cat /var/log/libvirt/libvirtd.conf

...

... debug : virStorageBackendRBDOpenRADOSConn:229 : Not using cephx authorization
... debug : virStorageBackendRBDRADOSConfSet:172 : Setting RADOS option 'auth_supported' to 'none'
... debug: virStorageBackendRBDOpenRADOSConn:241 : Found 1 RADOS cluster monitors in the pool configuration
... debug : virStorageBackendRBDRADOSConfSet:172 : Setting RADOS option 'mon_host' to '10.66.146.31:3300,'
... debug : virStorageBackendRBDRADOSConfSet:172 : Setting RADOS option 'client_mount_timeout' to '30'
... debug : virStorageBackendRBDRADOSConfSet:172 : Setting RADOS option 'rados_mon_op_timeout' to '30'
... debug : virStorageBackendRBDRADOSConfSet:172 : Setting RADOS option 'rados_osd_op_timeout' to '30'
... debug : virStorageBackendRBDRADOSConfSet:172 : Setting RADOS option 'rbd_default_format' to '2'
... error : virStorageBackendRBDOpenRADOSConn:323 : failed to connect to the RADOS monitor on: 10.66.146.31:3300,: Success
... debug : virStorageBackendRBDCloseRADOSConn:360 : Closing RADOS connection
... virStorageBackendRBDCloseRADOSConn:366 : RADOS connection existed for 0 seconds

(In reply to Meina Li from comment #8)
> Hi Peter,
> 
> About the auth element, I had another issue to confirm: 
> Does the <auth> or <encryption> outside <source> are ignored now?

No, but only for <disk>. Any other place doesn't specify it in places other than <source>.

> I found some bugs need to be verified with <auth> under in <source>. And I
> also found that creating rbd pool with auth outside of source element failed
> because it can't get related authorization. 
> #cat rbd.pool
> <pool type='rbd'>
> <name>rbdpool</name>

Storage pool issues are not related to the original bug.

According to comment 5, 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, 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-2020:2017