RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.
Bug 1159180 - Do NOT allow to define 'scsi_host' pool using the target already defined pool using 'fc_host'
Summary: Do NOT allow to define 'scsi_host' pool using the target already defined pool...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: libvirt
Version: 7.1
Hardware: x86_64
OS: Linux
medium
medium
Target Milestone: rc
: ---
Assignee: John Ferlan
QA Contact: Virtualization Bugs
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-10-31 07:03 UTC by Yang Yang
Modified: 2015-11-19 05:55 UTC (History)
9 users (show)

Fixed In Version: libvirt-1.2.13-1.el7
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-11-19 05:55:03 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2015:2202 0 normal SHIPPED_LIVE libvirt bug fix and enhancement update 2015-11-19 08:17:58 UTC

Description Yang Yang 2014-10-31 07:03:07 UTC
Description of problem:
Define a 'scsi_host' pool using the target already defined pool using 'fc_host'. In this case, they look for the same storage target. It
should be disallowed.

Version-Release number of selected component (if applicable):
libvirt-1.2.8-5.el7.x86_64

How reproducible:
100%

Steps to Reproduce:
1. discover the HBA
# virsh nodedev-list --cap vports
scsi_host4
scsi_host5

2. create persistent vHBA based on scsi_host5
# cat fc-pool.xml
<pool type='scsi'>
<name>fc-pool</name>
<source>
<adapter type='fc_host' parent='scsi_host5' wwnn='2101001b32a90002' wwpn='2101001b32a90003'/>
</source>
<target>
<path>/dev/disk/by-path</path>
<permissions>
<mode>0700</mode>
<owner>0</owner>
<group>0</group>
</permissions>
</target>
</pool>

# virsh pool-define fc-pool.xml
Pool fc-pool defined from fc-pool.xml
# virsh pool-start fc-pool
Pool fc-pool started
# virsh pool-refresh fc-pool
Pool fc-pool refreshed

# virsh nodedev-list scsi_host
scsi_host0
scsi_host1
scsi_host2
scsi_host3
scsi_host4
scsi_host5
scsi_host9      -------------> vHBA

# virsh vol-list fc-pool
 Name                 Path                                    
------------------------------------------------------------------------------
 unit:0:2:0           /dev/disk/by-path/pci-0000:04:00.1-fc-0x203600a0b85b5dd4-lun-0

3. define/start a scsi pool with parentaddr

# ll /sys/class/scsi_host/
lrwxrwxrwx. 1 root root 0 Oct 31 00:43 host9 -> ../../devices/pci0000:00/0000:00:0d.0/0000:04:00.1/host5/vport-5:0-3/host9/scsi_host/host9

#  cat /sys/class/scsi_host/host9/unique_id 
9

# cat scsi-pool-1.xml
<pool type='scsi'>
<name>scsi-pool-1</name>
<source>
<adapter type='scsi_host'>
<parentaddr unique_id='9'>  
<address domain='0x0' bus='0x04' slot='0x0' function='0x1'/>    -----> 0000:04:00.1
</parentaddr>
</adapter>
</source>
<target>
<path>/dev/disk/by-path</path>
<permissions>
<mode>0700</mode>
<owner>0</owner>
<group>0</group>
</permissions>
</target>
</pool>

# virsh pool-define scsi-pool-1.xml
Pool scsi-pool-1 defined from scsi-pool-1.xml
# virsh pool-start scsi-pool-1
Pool scsi-pool-1 started

# virsh pool-list --all
 Name                 State      Autostart
-------------------------------------------
 default              active     no        
 fc-pool              active     no        
 scsi-pool-1          active     no 

# virsh vol-list scsi-pool-1
 Name                 Path                                    
------------------------------------------------------------------------------
 unit:0:2:0           /dev/disk/by-path/pci-0000:04:00.1-fc-0x203600a0b85b5dd4-lun-0

4. destroy scsi-pool-1, define/start a scsi pool with scsi type and name

# virsh pool-destroy scsi-pool-1
Pool scsi-pool-1 destroyed
# virsh pool-undefine scsi-pool-1
Pool scsi-pool-1 has been undefined

# cat scsi-pool.xml 
<pool type='scsi'>
<name>scsi-pool</name>
<source>
<adapter type='scsi_host' name='scsi_host9'/>
</source>
<target>
<path>/dev/disk/by-path</path>
<permissions>
<mode>0700</mode>
<owner>0</owner>
<group>0</group>
</permissions>
</target>
</pool>

# virsh pool-define scsi-pool.xml 
Pool scsi-pool defined from scsi-pool.xml

[root@dell-pet105-04 yy]# virsh pool-start scsi-pool
Pool scsi-pool started

[root@dell-pet105-04 yy]# virsh pool-list --all
 Name                 State      Autostart 
-------------------------------------------
 default              active     no        
 fc-pool              active     no        
 scsi-pool            active     no        

[root@dell-pet105-04 yy]# virsh vol-list scsi-pool
 Name                 Path                                    
------------------------------------------------------------------------------
 unit:0:2:0           /dev/disk/by-path/pci-0000:04:00.1-fc-0x203600a0b85b5dd4-lun-0


Actual results:
In step 3, 'scsi_host' pool can be defined using 'parentaddr' with target already defined pool via 'fc_host'
In step 4, 'scsi_host' pool can be defined using scsi 'type' and 'name' with target already defined pool via 'fc_host'

Expected results:
Should NOT allow to define 'scsi_host' pool using the target already defined pool using 'fc_host'

Additional info:

Comment 2 John Ferlan 2014-11-03 13:21:02 UTC
It seems based on what was discovered during validation of https://bugzilla.redhat.com/show_bug.cgi?id=963817#c13, this bug becomes more generic to also not allow a 'scsi_host' adapter for what is a fc_host target. Thus for this configuation that includes scsi_host4 and scsi_host5, although that may only be handle-able at startup time.

Comment 3 John Ferlan 2014-11-13 01:40:14 UTC
As a way of a summary - between this case, some email, and the stable host bug 
mentioned in comment 2 - we have a plethora of situations where it's desired to have some sort of error.

First off, when trying to define a 'scsi_host' adapter storage pool we need to make sure that there's no conflict with some existing 'fc_host' adapter storage pool parent (if it exists) or the vHBA in use by the 'fc_host' adapter storage pool. If the parent doesn't exist, lookup of the vHBA followed by lookup of the parent of the vHBA is an involved process, which I'm not yet convinced is necessary. An additional issue raised is that it's possible to use the 'scsi_hostN' generated by iscsid in order to provide iSCSI storage pool LUNs.

The corollary to these is defining an 'fc_host' adapter storage pool using a 'scsi_hostN' that's already in use by some 'scsi_host' adapter storage pool. Again this is complicated when the incoming 'fc_host' adapter definition doesn't provide a parent. This is because the wwnn/wwpn could already be a vHBA as created by a 'virsh nodedev-create' and supplied to the 'fc_host' pool definition or it might be created by finding the 'best available' parent and having libvirt create the vHBA.

After doing some coding and thinking about it a bit, it may be better to document that using parent is highly suggested when both types of adapters are being used. 

I think we could spend a lot of cycles trying every single possible path at definition time, but that may be fruitless because something will get through. Additionally, there are some scenarios where it's best to disallow startup of the 'scsi_host' adapter storage pool even though the definition is allowed. In particular, a 'scsi_host' adapter storage pool that is using a 'scsi_hostN' of a vHBA or a iSCSI 'scsi_hostN'.  Now one could also argue that we shouldn't allow starting against a 'scsi_hostN' of a 'fc_host' capable 'scsi_hostN' (eg, a 'scsi_hostN' with a vport).  But I can also see there may be someone that wants to do that. I can be swayed on this, but only because we have the odd case where libvirt has created the vHBA using the 'best available' FC capable 'scsi_hostN' and it's already in use by some 'scsi_host' adapter storage pool. 

I've left a needsinfo just to be sure I haven't missed or forgotten something, whether you are OK with disallowing 'scsi_host' to use a 'scsi_hostN' of a vHBA & iSCSI, and of course to also get your opinion with respect to documenting the bizarre case where no parent is provided especially since it could be impossible to determine *until* startup is complete which is theoretically too late.

Comment 4 Yang Yang 2014-11-13 11:08:00 UTC
I'm so sorry. I am not quit sure whether it's reasonable to disable 'scsi_host' to use a 'scsi_hostN' of a vHBA & iSCSI. Leave a needinfo to timesu to double check. He ever researched and tested the feature. So I think he is the best man to give some suggestion.

Hi timesu,
Could you please help double check what mentioned in comment #3 ?

Thanks a lot
Yang

Comment 5 John Ferlan 2014-11-14 03:09:11 UTC
In https://bugzilla.redhat.com/show_bug.cgi?id=963817#c14 there's concern about defining a scsi_host adapter storage pool using a scsi_hostN of an iSCSI pool, so that's why I brought it in here - it went with the theme of not allowing certain types scsi_hostN's to be used. 

In this case it's using a vHBA as the scsi_host adapter with the desire for it to be determined at definition time if it's already in use.  That discovery is complicated by the 'parent=' attribute presence in the fc_host pool XML.

Whether disallowing either, both, or none is the choice - it's becoming increasingly difficult to keep track of all the various options and what should or shouldn't work. In the long run, there's so many different ways these scsi_host adapter pools could duplicate something that already exists or something to be created in the future, that I think the only reasonable solution is to document that scsi_host adapter storage pools must use "pci_*" parents - not sure how best to describe yet.

As a reminder this is for the <adapter type='scsi_host'... /> defs

At this point in the release cycle focusing on what doesn't work when things are defined correctly is becoming more important than chasing odd possible configurations that technically work, but might not technically be correct.  Having two pools looking at the same target is not desired, but determining that with the sundry ways the iSCSI, fc_host, and scsi_host can be used is mind boggling!

Comment 6 Luwen Su 2014-11-14 07:08:30 UTC
Hello John,

It's okay to disallow the behaviour from the point of view of mine, otherwise there would be a complicate matrix to cover (various pools need to match/mismatch kinds of device). Further more, the fc_pool is only aim to provide a method that able to use the vHBA conveniently and fit other component's requirement, from my memory which talked with osier, other purpose of using it was not mentioned.

But the thing what i'm concern is that if disallow it will have effect on other users and customers, since the feature was added from which Intel required for what it wants libvirt to provide the support for the device. And it has been added a long time.

So what do you think about it, john, need to ask someone to ensure if it's okay to prohibit for other users?

And if anything i'm wrong please just correct me since i have been while not track the feature...Thanks!

Comment 7 John Ferlan 2014-11-14 17:09:11 UTC
From a testing matrix viewpoint - I 110% agree.  It's also problematic in the code. The more checks/balances added - the more likely it is to inadvertently disallow something that works for someone (and as you point out "for years"). Today it's we don't want vHBA and iSCSI scsi_hostN's to be used, but in the near future it's some new 'thing' that uses scsi_hostN's for the transport and that has to be "added to the list".  It's like a never ending task.

In my opinion, focus should be placed on testing what is documented to work actually works. Coming up with options/configurations to test because we know the end result of an iSCSI/vHBA is to create a scsi_hostN is what I refer to as a rat hole.

Still I'm very happy to have the discussion - I wasn't here for the initial implementation, so there's a learning curve to understand the intent and who wanted what and why. Also, don't want to make a 'bad' decision in a vacuum (eg, by myself) only to be bitten later on when some customer comes back and says we *need* that support.

So, the end result is for this case at least I will add checks to the duplicate pool check to handle when a scsi_host adapter is being defined to check against the fc_host adapters to ensure the same parent attribute defined in the fc_host isn't being used and to check that the scsi_hostN of the fc_host adapter isn't being used in the attempted scsi_host adapter definition. Note, if the parent attribute isn't defined - I won't try to determine it based on the vHBA - something that I'll document. I'll also add the corollary check for a fc_host adapter definition with a parent attribute defined to compare against scsi_host adapter pool entries to ensure that none of the scsi_host adapter pool entries are using that parent.  Since the vHBA in this case isn't guaranteed to exist (it may *if* the nodedev-create was used to define it before calling), the check against the vHBA shouldn't be necessary.  Why would someone create their vHBA via nodedev-create, create some scsi_host adapter using that vHBA, and then create an fc_host adapter using that vHBA.

I will not make adjustments to the startup code to disallow using a vHBA or an iSCSI scsi_hostN node device as part of this change.  If that's desired, then a separate bug could be filed.  Because iSCSI usage/generation of the scsi_hostN is out of libvirt's control/knowledge - there's no "easy" mechanism to determine which scsi_hostN is being used by/for which iSCSI pool for which iSCSI target. My simple test shows that if I close/logout of a iSCSI host connect and open/login again a new scsi_hostN is created (and the old one removed).  The magic of scsi_hostN creation/deletion is handled by iscsid and is an "implementation detail" of their support much in the same way vHBA creation and deletion is for libvirt's NPIV support.

Comment 11 yisun 2015-03-27 10:13:39 UTC
Hi John, 
In the fix patch, there are following descriptions about duplicate:
=========quote begin==========
A 'fc_host' has two scsi_hostN values that need to be compared against
existing 'scsi_host' pools - first the fc_host "parent" scsi_hostN (if
it exists or was provided) and second the vHBA scsi_hostN as determined
from the wwnn/wwpn. If a 'fc_host' has a 'parent' attribute of a scsi_hostN
that's already in use by some 'scsi_host' pool or if it's vHBA (as created
via nodedev-create) was (for some unknown reason) used by someone for a
'scsi_host' pool, then we have a duplicate.

A 'scsi_host' conversely needs to check against using either the fc_host
'parent' or vHBA. If a 'scsi_host' is using a scsi_hostN of some existing
fc_host 'parent' attribute or vHBA, then we have a duplicate.
=========quote end=============

I have following concern:
In my test environment
--- There is a physical fc HBA which scsi name is SCSI_HOST4, and wwnn/wwpn is aaaa/bbbb. 
--- There is a virtual fc HBA which inherited form SCSI_HOST4, and its name is SCSI_HOST5, with wwnn/wwpn = cccc/dddd

so, obviously SCSI_HOST4 is SCSI_HOST5's parent, and they have different wwnn/wwpn which lead them to different SAN fabric storages. According to your description, creating pools for SCSI_HOST4 and SCSI_HOST5 is a "duplicate" and will be prevented, that means only one scsi host's connected volumes could be exposed by scsi type pool and the other (v)hba will be forbidden. It is a little unreasonable to me. Please correct me if I misunderstand it.

Comment 12 John Ferlan 2015-03-30 15:20:05 UTC
Not sure I follow your question/concern...  Maybe an example would help...  The patch commit test also states above the part you pulled:

"Currently libvirt only detects duplicate fc_host & scsi_host adapter sources
when the incoming source type definition is the same as the pool type. This
misses the even more oddball cases where a scsi_host/fc_host definition is
using a scsi_hostN where the type of the definition isn't the same as the
type of the pool.  It's a fairly twisted and perhaps edge kind of case."


First off, the 'fc_host' adapter and the 'scsi_host' adapter both use nodedev entries 'scsi_host#' (which gets really confusing). Internally, these are represented by an attribute for the SCSI_HOST pool type. 

In your setup scsi_host5 is the vHBA and scsi_host4 is the HBA.  We only want to allow creating 'fc_host' pools using the vHBA, e.g.

...
<pool type='scsi'>
<name>fc-pool</name>
<adapter type='fc_host' parent='scsi_host5' wwnn='2101001b32a90002' wwpn='2101001b32a90003'/>
...

What we don't want to allow is someone creating a 'scsi_host' type pool from a 'fc_host' pool... From your example, disallow the following

...

<pool type='scsi'>
<name>scsi-pool-1</name>
<source>
<adapter type='scsi_host'>
<parentaddr unique_id='5'>  
...

Note the "<adapter type='scsi_host'..."... while scsi_host5 is a valid scsi_host, if it's already in use by some active pool for a data source - using a second or duplicate pool for the same source is not legal (it's stopped for other such duplicate pool attempts).

What I believe you might be attempting to question is if you can have a 'scsi_host' adapter pool for SCSI_HOST4 and a 'fc_host' adapter pool for SCSI_HOST5 - to that question - I believe the answer is yes (although it's been a few months since I've had that code paged into my head.

The probably with attempting to quickly read the code again is I don't have the context of the ways a 'adapter type=scsi_host' <source> can be defined/used.  See http://libvirt.org/formatstorage.html - it shows either using the 'name' or the 'parentaddr'.  The former isn't necessarily persistent across reboots - what is scsi_host1 this time could be scsi_host2 the next time the host boots.  So the 'parentaddr' source was created which required using a much more persistent and detail source, but made sure whatever the pool was looking at between host reboots was the same.  Adding 'fc_host' adapters into the mix required having the fc_host adapter duplicate source check having more edge corner cases because some test matrix tried it and failed.  

It's about to get even more confusing because there's bz's in my queue which describe or question the "<host...>" element duplication possibilities for 'iscsi' pools. If the 'name' property uses 'iscsi.example.com' or perhaps '1.2.3.4' - where iscsi.example.com is defined as 1.2.3.4, then we allow creating two pools of the same source...

In any case, the first thing that should be done is to validate that the issues in .0 are rectified.  Then if you want and have the time, create examples of something you think should work that doesn't and let me know. The examples need to provide the exact XML used - please don't skimp on the details.

Comment 13 yisun 2015-04-01 11:18:16 UTC
Hi John, here is a sample about my previous concern

scsi_host5 is the physical HBA in my environment, and I'll use it to create vHBA. 

#cat child_pool.xml
<pool type='scsi'>
    <name>child_pool</name>
    <source>
        <adapter type='fc_host' parent='scsi_host5' wwnn='2101001b32a90002' wwpn='2101001b32a90003'/>
    </source>
    <target>
        <path>/dev/disk/by-id/</path>
        <permissions>
          <mode>0755</mode>
          <owner>-1</owner>
          <group>-1</group>
        </permissions>
    </target>
</pool>

# cat pool-host5.xml 
   <pool type="scsi">
     <name>host5</name>
     <source>
       <adapter name="scsi_host5"/>
     </source>
     <target>
       <path>/dev/disk/by-id</path>
     </target>
   </pool> 

#virsh pool-create child_pool.xml 
Pool child_pool created from child_pool.xml

# ls /sys/class/fc_host/
host3  host5  host9
<====== now, a vHBA scsi_host9 created.

# virsh pool-create pool-host5.xml 
error: Failed to create pool from pool-host5.xml
error: operation failed: Storage source conflict with pool: 'child_pool'

Well, my question is that why a child's pool will prevent its parent pool creation in this scenario?  This doesn't make sense to me. Because logically they are not the same scsi device and they have different san storage attached, as follow:
# lsscsi
....
[5:0:0:0]    disk    IBM      1726-4xx  FAStT  0617  -        
[5:0:1:0]    disk    IBM      1726-4xx  FAStT  0617  -        
[5:0:2:0]    disk    IBM      1726-4xx  FAStT  0617  /dev/sdd 
[5:0:3:0]    disk    IBM      1726-4xx  FAStT  0617  /dev/sde 
[9:0:0:0]    disk    IBM      1726-4xx  FAStT  0617  -        
[9:0:1:0]    disk    IBM      1726-4xx  FAStT  0617  /dev/sdf 
[9:0:2:0]    disk    IBM      1726-4xx  FAStT  0617  /dev/sdg 
[9:0:3:0]    disk    IBM      1726-4xx  FAStT  0617  -        

Please help me to understand this, thanks.

Comment 14 John Ferlan 2015-04-13 11:32:44 UTC
Obviously by the length in time it's taken me to respond - I've been in the middle of something else so I haven't had the cycles to think about this nor try it on the host that I can use that has this capability. But the bz has a needsinfo on it, so I get periodically dinged, and thus I guess means I have to give some sort of response.

Your vHBA (scsi_host9) is using scsi_host5 to present the FC/NPIV storage, so if you were allowed to create a separate scsi_host5 wouldn't that then mean you potentially have two storage pools viewing the same source? Why even create the FC pool if all you really want is the SCSI view? How do you then "stop" someone from using the SCSI view while someone else uses the FC view? The implementation details of FC/NPIV (which I don't have intimate knowledge of btw) have to create a /dev/sd* block and a /dev/sg* generic device - it's how it works - it's how it can present a "something" to the host that everyone can use just like any other device.

Just because the 'lsscsi' shows what appears to be two different LUNs doesn't mean they are *physically* different.

On my test system I have 'scsi_host3' as the parent and 'scsi_host9' as the child:

$ lsscsi -tg
...
[3:0:0:0]    enclosu fc:0x217800c0ffd79b2a0x1001ef   -          /dev/sg15
[3:0:0:13]   disk    fc:0x217800c0ffd79b2a0x1001ef   /dev/sdg   /dev/sg19
[3:0:1:0]    enclosu fc:0x217000c0ffd79b2a0x1002ef   -          /dev/sg20
[3:0:2:0]    enclosu fc:0x217000c0ffd5009b0x1003ef   -          /dev/sg16
[3:0:3:0]    enclosu fc:0x217800c0ffd5009b0x1004ef   -          /dev/sg17
[3:0:4:0]    disk    fc:0x50060168446021980x101f00   /dev/sdu   /dev/sg32
[3:0:5:0]    disk    fc:0x50060160446021980x102000   /dev/sdw   /dev/sg34
...
[9:0:0:0]    enclosu fc:0x217800c0ffd79b2a0x1001ef   -          /dev/sg11
[9:0:1:0]    enclosu fc:0x217000c0ffd79b2a0x1002ef   -          /dev/sg12
[9:0:2:0]    enclosu fc:0x217000c0ffd5009b0x1003ef   -          /dev/sg22
[9:0:3:0]    enclosu fc:0x217800c0ffd5009b0x1004ef   -          /dev/sg23
[9:0:4:0]    disk    fc:0x50060168446021980x101f00   /dev/sdd   /dev/sg24
[9:0:5:0]    disk    fc:0x50060160446021980x102000   /dev/sde   /dev/sg25
...

If you look really closely you will note the [*:0:4:0] and [*:0:5:0] disks *match* by that 0x5006* number - *that* is what makes this work. For a migration, on another host when a FC/NPIV pool is created it will create a 'scsi_hostN' which will point at the same physical storage.

Comment 15 yisun 2015-04-24 10:51:55 UTC
"What's really odd to me in what you showed below is the /dev/sdb &&
/dev/sdc and /dev/sdd && /dev/sde relationship... They both seem to have
the same 0x2036xxx values with the primary difference being the
"x:0:y:0" and "x:0:y:1" for the name."
=======> SAN admin assigned two LUN disks to my scsi_host4 (physical HBA), that's why /dev/sdb && /dev/sdc have only LUN numbers difference.
Summarily, I created HBA pool and used volumes inside the pool to create VMs, and when I need more SAN storage, I ask SAN admin to provide me a valid WWNN&&WWPN pair which configured in SAN switch, and used it to create a new FC pool which containing more SAN volumes.  
After this fix. This way is blocked (well, create SCSI pool is a work around but it requires to create vHBA device ahead, and this is not recommended because vHBA is transient)
I am just not sure if my usage is common, and how many users are working in this way. If it's a common usage, this fix will surely produce some "regression" issue to them. And if you think the risk is low, then I'll accept the fix and start verification according to your patch description. Thanks.  


----- Original Message -----
From: "John Ferlan" <jferlan>
To: "Yi Sun" <yisun>
Cc: "Yang Yang" <yanyang>
Sent: Wednesday, April 15, 2015 9:34:31 PM
Subject: Re: About bug 1159180 "Do NOT allow to define 'scsi_host' pool using the target already defined pool using 'fc_host'"



On 04/15/2015 09:00 AM, Yi Sun wrote:
> Hi John,
> About bug https://bugzilla.redhat.com/show_bug.cgi?id=1159180jferlan@redhat.com
> Following is your last comment.  I still have some concerns.

This type of correspondence needs to take place via bz - doing it
outside of bz only keeps history in email.

Obviously this is a complex issue with multiple ways to define things.
Perhaps even depending on the SAN provider/manufacturer there's multiple
ways to define things. If the existing check is too broad, then we can
adjust it, but understanding "the best way" to adjust it in a generic
enough manner requires understanding all the parameters.

What's really odd to me in what you showed below is the /dev/sdb &&
/dev/sdc and /dev/sdd && /dev/sde relationship... They both seem to have
the same 0x2036xxx values with the primary difference being the
"x:0:y:0" and "x:0:y:1" for the name.



John
> ========================   Quote begin =================================
> Obviously by the length in time it's taken me to respond - I've been in the middle of something else so I haven't had the cycles to think about this nor try it on the host that I can use that has this capability. But the bz has a needsinfo on it, so I get periodically dinged, and thus I guess means I have to give some sort of response.
>
> Your vHBA (scsi_host9) is using scsi_host5 to present the FC/NPIV storage, so if you were allowed to create a separate scsi_host5 wouldn't that then mean you potentially have two storage pools viewing the same source? Why even create the FC pool if all you really want is the SCSI view? How do you then "stop" someone from using the SCSI view while someone else uses the FC view? The implementation details of FC/NPIV (which I don't have intimate knowledge of btw) have to create a /dev/sd* block and a /dev/sg* generic device - it's how it works - it's how it can present a "something" to the host that everyone can use just like any other device.
>
> Just because the 'lsscsi' shows what appears to be two different LUNs doesn't mean they are *physically* different.
>
> On my test system I have 'scsi_host3' as the parent and 'scsi_host9' as the child:
>
> $ lsscsi -tg
> ...
> [3:0:0:0]    enclosu fc:0x217800c0ffd79b2a0x1001ef   -          /dev/sg15
> [3:0:0:13]   disk    fc:0x217800c0ffd79b2a0x1001ef   /dev/sdg   /dev/sg19
> [3:0:1:0]    enclosu fc:0x217000c0ffd79b2a0x1002ef   -          /dev/sg20
> [3:0:2:0]    enclosu fc:0x217000c0ffd5009b0x1003ef   -          /dev/sg16
> [3:0:3:0]    enclosu fc:0x217800c0ffd5009b0x1004ef   -          /dev/sg17
> [3:0:4:0]    disk    fc:0x50060168446021980x101f00   /dev/sdu   /dev/sg32
> [3:0:5:0]    disk    fc:0x50060160446021980x102000   /dev/sdw   /dev/sg34
> ...
> [9:0:0:0]    enclosu fc:0x217800c0ffd79b2a0x1001ef   -          /dev/sg11
> [9:0:1:0]    enclosu fc:0x217000c0ffd79b2a0x1002ef   -          /dev/sg12
> [9:0:2:0]    enclosu fc:0x217000c0ffd5009b0x1003ef   -          /dev/sg22
> [9:0:3:0]    enclosu fc:0x217800c0ffd5009b0x1004ef   -          /dev/sg23
> [9:0:4:0]    disk    fc:0x50060168446021980x101f00   /dev/sdd   /dev/sg24
> [9:0:5:0]    disk    fc:0x50060160446021980x102000   /dev/sde   /dev/sg25
> ...
>
> If you look really closely you will note the [*:0:4:0] and [*:0:5:0] disks *match* by that 0x5006* number - *that* is what makes this work. For a migration, on another host when a FC/NPIV pool is created it will create a 'scsi_hostN' which will point at the same physical storage.
> =========================== Quote End====================================
> During my test
> 1. physical HBA and vHBA's pools won't have duplicate volumes. (scsi_host4 is my physical HBA)
> 1.1 before vHBA creation. lscssi shows following info.
> [root@ibm-x3850x5-05 xml]# lsscsi -tg
> [0:2:0:0]    disk                                    /dev/sda   -        
> [2:0:0:0]    cd/dvd  ata:                            /dev/sr0   -        
> [4:0:0:0]    disk    fc:0x203500a0b85b0acc0x2e0300   -          -        
> [4:0:1:0]    disk    fc:0x203400a0b85b0acc0x2e0400   -          -        
> [4:0:2:0]    disk    fc:0x203600a0b85b5dd40x2e3a00   /dev/sdb   -        
> [4:0:2:1]    disk    fc:0x203600a0b85b5dd40x2e3a00   /dev/sdc   -        
> [4:0:3:0]    disk    fc:0x203700a0b85b5dd40x2e3d00   /dev/sdd   -        
> [4:0:3:1]    disk    fc:0x203700a0b85b5dd40x2e3d00   /dev/sde   -  
>
> 1.2 create  fc pool for scsi_host4
> [root@ibm-x3850x5-05 xml]# cat scsi_pool1.xml
> <pool type='scsi'>
>     <name>scsi_pool1</name>
>     <source>
>         <adapter type='fc_host' name='host4' wwnn='20000024ff379999' wwpn='21000024ff370145'/>
>     </source>
>     <target>
>         <path>/dev/disk/by-id/</path>
>         <permissions>
>           <mode>0755</mode>
>           <owner>-1</owner>
>           <group>-1</group>
>         </permissions>
>     </target>
> </pool>
> [root@ibm-x3850x5-05 xml]# virsh pool-create scsi_pool1.xml
> Pool scsi_pool1 created from scsi_pool1.xml
>
> 1.3 Now the volume list of pool "scsi_pool1" as follow:
> [root@ibm-x3850x5-05 xml]# virsh vol-list scsi_pool1
>  Name                 Path                                    
> ------------------------------------------------------------------------------
>  unit:0:3:0           /dev/disk/by-id/wwn-0x600a0b80005ada7600006b16546bd5bc
>  unit:0:3:1           /dev/disk/by-id/wwn-0x600a0b80005ada7600008edb54efd406
>
> 1.4 The volumes in pool "scsi_pool1" have size 30G and 50G
> [root@ibm-x3850x5-05 xml]# qemu-img info /dev/disk/by-id/wwn-0x600a0b80005ada7600006b16546bd5bc
> image: /dev/disk/by-id/wwn-0x600a0b80005ada7600006b16546bd5bc
> file format: raw
> virtual size: 30G (32218546176 bytes)
> disk size: 0
>
> [root@ibm-x3850x5-05 xml]# qemu-img info /dev/disk/by-id/wwn-0x600a0b80005ada7600008edb54efd406
> image: /dev/disk/by-id/wwn-0x600a0b80005ada7600008edb54efd406
> file format: raw
> virtual size: 50G (53697576960 bytes)
> disk size: 0
>
> 1.5. Create vHBA using scsi_host4 as parent
> [root@ibm-x3850x5-05 xml]# cat vhba1.xml
> <device>
>   <parent>scsi_host4</parent>
>   <capability type='scsi_host'>
>     <capability type='fc_host'>
>          <wwnn>2101001b32a90002</wwnn>
>         <wwpn>2101001b32a90002</wwpn>
>      </capability>
>   </capability>
> </device>
>
> [root@ibm-x3850x5-05 xml]# virsh nodedev-create vhba1.xml
> Node device scsi_host20 created from vhba1.xml
>
> 1.6 Check scsi disks:
> [root@ibm-x3850x5-05 xml]# lsscsi -tg
> [0:2:0:0]    disk                                    /dev/sda   -        
> [2:0:0:0]    cd/dvd  ata:                            /dev/sr0   -        
> [4:0:0:0]    disk    fc:0x203500a0b85b0acc0x2e0300   -          -        
> [4:0:1:0]    disk    fc:0x203400a0b85b0acc0x2e0400   -          -        
> [4:0:2:0]    disk    fc:0x203600a0b85b5dd40x2e3a00   /dev/sdb   -        
> [4:0:2:1]    disk    fc:0x203600a0b85b5dd40x2e3a00   /dev/sdc   -        
> [4:0:3:0]    disk    fc:0x203700a0b85b5dd40x2e3d00   /dev/sdd   -        
> [4:0:3:1]    disk    fc:0x203700a0b85b5dd40x2e3d00   /dev/sde   -        
> [20:0:0:0]   disk    fc:0x203500a0b85b0acc0x2e0300   -          -        
> [20:0:1:0]   disk    fc:0x203400a0b85b0acc0x2e0400   -          -        
> [20:0:2:0]   disk    fc:0x203600a0b85b5dd40x2e3a00   /dev/sdf   -        
> [20:0:3:0]   disk    fc:0x203700a0b85b5dd40x2e3d00   /dev/sdg   -
>
> 1.7 scsi_host20 has attached to new LUN disk as follow(sdg is a ghost disk):
> [root@ibm-x3850x5-05 xml]# ll /dev/disk/by-id/ | grep sdf
> ...
> lrwxrwxrwx. 1 root root  9 Apr 15 17:37 wwn-0x600a0b80005ad1d700002ddc4fa32c87 -> ../../sdf
>
> 1.8 the new LUN disk's size is 10G, obviously not scsi_host4's LUN disks.
> [root@ibm-x3850x5-05 xml]# qemu-img info /dev/disk/by-id/wwn-0x600a0b80005ad1d700002ddc4fa32c87
> image: /dev/disk/by-id/wwn-0x600a0b80005ad1d700002ddc4fa32c87
> file format: raw
> virtual size: 10G (10737418240 bytes)
> disk size: 0
>
> 1.9 Refresh pool "scsi_pool1", and the volumes of scsi_host20 doesn't show up.
> [root@ibm-x3850x5-05 xml]# virsh pool-refresh scsi_pool1
> Pool scsi_pool1 refreshed
>
> [root@ibm-x3850x5-05 xml]# virsh vol-list scsi_pool1
>  Name                 Path                                    
> ------------------------------------------------------------------------------
>  unit:0:3:0           /dev/disk/by-id/wwn-0x600a0b80005ada7600006b16546bd5bc
>  unit:0:3:1           /dev/disk/by-id/wwn-0x600a0b80005ada7600008edb54efd406
> ==========================================================
> My concern is:
> - In libvirt doc, FC pool is recommended for vHBA storage. Because vHBA is transient during system reboot. FC pool can automatically create vHBA.  So that's why I think creating FC pool is more common. http://wiki.libvirt.org/page/NPIV_in_libvirt#Creation_of_vHBA_by_the_storage_pool
> - Physical HBA is often bound with LUN disks in SAN, and users may already created scsi pool for HBA.
> - So if user already created pool for HBA and used by VMs, that means vHBA FC pool can never be created?
> ===========================================================
> And in your comment, I think maybe following is your key point:
> Your vHBA (scsi_host9) is using scsi_host5 to present the FC/NPIV storage, so if you were allowed to create a separate scsi_host5 wouldn't that then mean you potentially have two storage pools viewing the same source?
> Could you please let me know how to reproduce the "have two storage pools (one parent HBA pool and one child vHBA pool) viewing the same source? "
>

Comment 17 John Ferlan 2015-05-20 16:05:23 UTC
Again - getting bz email regarding needing a response so here it goes.

I really don't know what the "actual" usage people have will be. Personally using the scsi_host{N} for FC scsi_host{N+M} seems to mean the FC scsi_host{N+M} is "using" scsi_host{N} and thus should restrict someone from using it. Using both for separate pools only seems to provide the possibility that something could go wrong. And again, the configuration of these things is very confusing and complicated, so I could be wrong (and that's fine).

Comment 18 yisun 2015-05-25 09:33:27 UTC
Verified on:
libvirt-1.2.15-2.el7.x86_64
qemu-kvm-rhev-2.3.0-1.el7.x86_64
kernel-3.10.0-253.el7.x86_64

scenario 1:
fc_host's parent vs. scsi_host
1. fc_host pool exists with parent option, check if scsi_host pool using the same scsi device can be created.
1.1 # cat fc_pool.xml
<pool type='scsi'>
    <name>fc_pool</name>
    <source>
        <adapter type='fc_host' parent='scsi_host5' wwnn='2101001b32a90002' wwpn='2101001b32a90003'/>
    </source>
    <target>
        <path>/dev/disk/by-path/</path>
        <permissions>
          <mode>0755</mode>
          <owner>-1</owner>
          <group>-1</group>
        </permissions>
    </target>
</pool>

1.2 # cat scsi_pool.xml
<pool type="scsi">
    <name>scsi_pool</name>
    <source>
        <adapter name="host5"/>
    </source>
    <target>
        <path>/dev/disk/by-id</path>  <!-- by-path -->
    </target>
</pool>

1.3 # virsh pool-create fc_pool.xml
Pool fc_pool created from fc_pool.xml

1.4 # virsh pool-create scsi_pool.xml
error: Failed to create pool from scsi_pool.xml
error: operation failed: Storage source conflict with pool: 'fc_pool'
1.5 # virsh pool-list --all
 Name                 State      Autostart
-------------------------------------------
 boot-scratch         inactive   no        
 default              active     yes       
 fc_pool              active     no  
1.6 # virsh pool-destroy fc_pool
Pool fc_pool destroyed

2. scsi_host pool exists check if fc_host pool using the same scsi device as parent can be created. 
2.1 # virsh pool-create scsi_pool.xml
Pool scsi_pool created from scsi_pool.xml

2.2 # virsh pool-create fc_pool.xml
error: Failed to create pool from fc_pool.xml
error: operation failed: Storage source conflict with pool: 'scsi_pool'

2.3 # virsh pool-list --all
 Name                 State      Autostart
-------------------------------------------
 boot-scratch         inactive   no        
 default              active     yes       
 scsi_pool            active     no   
2.4 # virsh pool-destroy scsi_pool
Pool scsi_pool destroyed




========================
scenario 2: fc_host pool exists and vhba generated automatically, then use the vhba to create a scsi_host pool.
1 # cat fc_pool.xml
<pool type='scsi'>
    <name>fc_pool</name>
    <source>
        <adapter type='fc_host' parent='scsi_host5' wwnn='2101001b32a90002' wwpn='2101001b32a90003'/>
    </source>
    <target>
        <path>/dev/disk/by-path/</path>
        <permissions>
          <mode>0755</mode>
          <owner>-1</owner>
          <group>-1</group>
        </permissions>
    </target>
</pool>

2 # virsh pool-create fc_pool.xml
Pool fc_pool created from fc_pool.xml
3 # lsscsi       //scsi_host27 is generated during my test
...
[27:0:0:0]   disk    IBM      1726-4xx  FAStT  0617  -        
[27:0:1:0]   disk    IBM      1726-4xx  FAStT  0617  -        
[27:0:2:0]   disk    IBM      1726-4xx  FAStT  0617  /dev/sdf
[27:0:3:0]   disk    IBM      1726-4xx  FAStT  0617  /dev/sdg 
4 # cat scsi_pool.xml
<pool type="scsi">
    <name>scsi_pool</name>
    <source>
        <adapter name="host27"/>
    </source>
    <target>
        <path>/dev/disk/by-id</path>  <!-- by-path -->
    </target>
</pool>
5 # virsh pool-create scsi_pool.xml
error: Failed to create pool from scsi_pool.xml
error: operation failed: Storage source conflict with pool: 'fc_pool'

6 # virsh pool-list --all
 Name                 State      Autostart
-------------------------------------------
 boot-scratch         inactive   no        
 default              active     yes       
 fc_pool              active     no
7 # virsh pool-destroy fc_pool
Pool fc_pool destroyed





========================
scenario 3: occupy all hba by creating scsi_host pool, then try to create fc_host pool
1. # cat scsi4_pool.xml
<pool type="scsi">
    <name>scsi4_pool</name>
    <source>
        <adapter name="host4"/>
    </source>
    <target>
        <path>/dev/disk/by-id</path>  <!-- by-path -->
    </target>
</pool>
2. # cat scsi5_pool.xml
<pool type="scsi">
    <name>scsi5_pool</name>
    <source>
        <adapter name="host5"/>
    </source>
    <target>
        <path>/dev/disk/by-id</path>  <!-- by-path -->
    </target>
</pool>
3. # virsh pool-create scsi4_pool.xml
Pool scsi4_pool created from scsi4_pool.xml
# virsh pool-create scsi5_pool.xml
Pool scsi_pool5 created from scsi5_pool.xml

4. # cat fc_pool.xml     //parent is not indicated
<pool type='scsi'>
    <name>fc_pool</name>
    <source>
        <adapter type='fc_host'  wwnn='2101001b32a90002' wwpn='2101001b32a90003'/>
    </source>
    <target>
        <path>/dev/disk/by-path/</path>
        <permissions>
          <mode>0755</mode>
          <owner>-1</owner>
          <group>-1</group>
        </permissions>
    </target>
</pool>

5. # virsh pool-create fc_pool.xml
Pool fc_pool created from fc_pool.xml    <==== the duplicate check fails

The failure is documented in patch description of http://post-office.corp.redhat.com/archives/rhvirt-patches/2014-December/msg00012.html , as follow:

To make matters more complex, the 'fc_host' 'parent' attribute doesn't have
to be provided in the fc_host XML. If not provided, then it can be determined
if the vHBA already exists (either via nodedev-create or a running fc_host
pool).  This means moving the code that was created for fc_host startup to
find the vHBA parent into the storage_backend_scsi code in order for it to
be used there instead. I did try moving into virutil with no success since
that code wasn't very happy to be calling the virNodeDevice* API's.

The only oddball case that cannot be tested for in this scenario is if
the incoming fc_host definition isn't using a nodedev-create'd vHBA and
it doesn't provide a 'parent' attribute, then it is possible that the
vportCreate startup code will find "an available" scsi_hostN that was
(again for some strange reason) already being used by some 'scsi_host' pool.






========================
scenario 4: do not indicate parent option in fc_host pool creation, let the code automatically create a vhba, then try to create scsi_host pool for that vhba, and make sure duplicate could be checked.
1. # cat fc_pool
cat: fc_pool: No such file or directory
[root@ibm-x3850x5-05 ~]# cat fc_pool.xml
<pool type='scsi'>
    <name>fc_pool</name>
    <source>
        <adapter type='fc_host' wwnn='2101001b32a90002' wwpn='2101001b32a90003'/>
    </source>
    <target>
        <path>/dev/disk/by-path/</path>
        <permissions>
          <mode>0755</mode>
          <owner>-1</owner>
          <group>-1</group>
        </permissions>
    </target>
</pool>
2. # virsh pool-destroy fc_pool
Pool fc_pool destroyed
3. # lsscsi    //scsi_host7 is newly created
[0:2:0:0]    disk    IBM      ServeRAID M5015  2.13  /dev/sda
[2:0:0:0]    cd/dvd  IBM SATA  DEVICE 81Y3661  SA80  /dev/sr0
[4:0:0:0]    disk    IBM      1726-4xx  FAStT  0617  -        
[4:0:1:0]    disk    IBM      1726-4xx  FAStT  0617  -        
[4:0:2:0]    disk    IBM      1726-4xx  FAStT  0617  /dev/sdb
[4:0:2:1]    disk    IBM      1726-4xx  FAStT  0617  /dev/sdc
[4:0:3:0]    disk    IBM      1726-4xx  FAStT  0617  /dev/sdd
[4:0:3:1]    disk    IBM      1726-4xx  FAStT  0617  /dev/sde
[7:0:0:0]    disk    IBM      1726-4xx  FAStT  0617  /dev/sdf
[7:0:1:0]    disk    IBM      1726-4xx  FAStT  0617 
 
4. # cat scsi7_pool.xml
<pool type="scsi">
    <name>scsi7_pool</name>
    <source>
        <adapter name="host4"/>
    </source>
    <target>
        <path>/dev/disk/by-id</path>  <!-- by-path -->
    </target>
5. # virsh pool-create scsi7_pool.xml
error: Failed to create pool from scsi7_pool.xml
error: operation failed: Storage source conflict with pool: 'fc_pool'
6. # cat scsi7_1_pool.xml
<pool type='scsi'>
<name>scsi7_1_pool</name>
<source>
<adapter type='scsi_host'>
<parentaddr unique_id='7'>  
<address domain='0x0' bus='0x18' slot='0x0' function='0x1'/>
</parentaddr>
</adapter>
</source>
<target>
<path>/dev/disk/by-path</path>
<permissions>
<mode>0700</mode>
<owner>0</owner>
<group>0</group>
</permissions>
</target>
</pool>




7. # virsh pool-create scsi7_1_pool.xml
error: Failed to create pool from scsi7_1_pool.xml
error: operation failed: Storage source conflict with pool: 'fc_pool'



NB:
scenario 3 is failed but it's documented in patch description as a failure situation, so this bug is verified.

Comment 20 errata-xmlrpc 2015-11-19 05:55:03 UTC
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://rhn.redhat.com/errata/RHBA-2015-2202.html


Note You need to log in before you can comment on or make changes to this bug.