Bug 1601377

Summary: pool-create-as pool-define-as failed when provide --adapter-parent-wwnn and --adapter-parent-wwpn
Product: Red Hat Enterprise Linux 7 Reporter: yisun
Component: libvirtAssignee: John Ferlan <jferlan>
Status: CLOSED ERRATA QA Contact: Meina Li <meili>
Severity: medium Docs Contact:
Priority: medium    
Version: 7.6CC: dyuan, jferlan, lmen, meili, xuzhang
Target Milestone: rc   
Target Release: ---   
Hardware: x86_64   
OS: Linux   
Whiteboard:
Fixed In Version: libvirt-4.5.0-4.el7 Doc Type: No Doc Update
Doc Text:
undefined
Story Points: ---
Clone Of: Environment:
Last Closed: 2018-10-30 09:58:24 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description yisun 2018-07-16 08:48:12 UTC
Description:
pool-create-as pool-define-as failed when provide --adapter-parent-wwnn and --adapter-parent-wwpn

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

How reproducible:
100%

Steps to Reproduce:
1.
## virsh pool-define-as name_of_pool scsi --adapter-wwnn 20000000c99e2b80 --adapter-wwpn 10000000c99e2b80 --adapter-parent-wwpn 10000000c99e2b81 --adapter-parent-wwnn 10000000c99e2b80 --target /dev/by-id/ 
error: Failed to define pool name_of_pool
error: (storage_pool_definition):4: Attribute parent_wwnn redefined
    <adapter type='fc_host' parent_wwnn='10000000c99e2b80' parent_wwnn='10000000c99e2b81' wwnn='20000000c99e2b80' wwpn='10000000c99e2b80'/>
-----------------------------------------------------------------------------------------------------------------------------------------^


Actual results:
failed to define/create pool

Additional info:
The "parent_wwnn" assigned twice when generate pool's xml in following commit.
# git show d45bee44
...
+            else if (adapterParentWwnn && adapterParentWwpn)
+                virBufferAsprintf(&buf, " parent_wwnn='%s' parent_wwnn='%s'",
+                                  adapterParentWwnn, adapterParentWwpn);

Comment 2 John Ferlan 2018-07-19 18:04:30 UTC
Patch posted upstream:

https://www.redhat.com/archives/libvir-list/2018-July/msg01304.html

Comment 3 John Ferlan 2018-07-20 11:50:05 UTC
This has been pushed upstream:

commit 313eaae3b5a4f9b1e94b153759b878614caecf34
Author: John Ferlan <jferlan>
Date:   Thu Jul 19 14:00:07 2018 -0400

    tools: Fix typo generating adapter_wwpn field
    
...
    
    Fix typo from commit id d45bee449 for the parent_wwpn field
    resulting in parent_wwnn being printed twice.
    
    Signed-off-by: John Ferlan <jferlan>
    Reviewed-by: Erik Skultety <eskultet>

diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index cc49a5b96d..6faff781b2 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -371,7 +371,7 @@ virshBuildPoolXML(vshControl *ctl,
             if (adapterParent)
                 virBufferAsprintf(&buf, " parent='%s'", adapterParent);
             else if (adapterParentWwnn && adapterParentWwpn)
-                virBufferAsprintf(&buf, " parent_wwnn='%s' parent_wwnn='%s'",
+                virBufferAsprintf(&buf, " parent_wwnn='%s' parent_wwpn='%s'",
                                   adapterParentWwnn, adapterParentWwpn);
             else if (adapterParentFabricWwn)
                 virBufferAsprintf(&buf, " parent_fabric_wwn='%s'",


$ git describe 313eaae3b5a4f9b1e94b153759b878614caecf34
v4.5.0-202-g313eaae3b5
$

Comment 6 Meina Li 2018-08-10 02:57:21 UTC
Verified on libvirt-4.5.0-6.el7.x86_64.

Test Steps:
1. Define a scsi pool with --adapter-parent-wwnn and --adapter-parent-wwpn.
# virsh pool-define-as npiv scsi --adapter-wwnn 20000024ff370144 --adapter-wwpn 2101001b32a90000 --adapter-parent-wwpn 21000024ff370145 --adapter-parent-wwnn 20000024ff370145 --target /dev/disk/by-path
Pool npiv defined

2. Start the pool, verify the pool state is active.

# virsh pool-start npiv
Pool npiv started

# virsh pool-list --all
Name                 State      Autostart 
-------------------------------------------
 default              active     yes       
 npiv                 active     no

3. Check pool-dumpxml, pool-info, pool-name, pool-uuid.
# virsh pool-dumpxml npiv
<pool type='scsi'>
  <name>npiv</name>
  <uuid>77adeef2-e803-49e3-8650-c73fc9f0d948</uuid>
  <capacity unit='bytes'>0</capacity>
  <allocation unit='bytes'>0</allocation>
  <available unit='bytes'>0</available>
  <source>
    <adapter type='fc_host' parent_wwnn='20000024ff370145' parent_wwpn='21000024ff370145' wwnn='20000024ff370144' wwpn='2101001b32a90000'/>
  </source>
  <target>
    <path>/dev/disk/by-path</path>
  </target>
</pool>

# virsh pool-info npiv
Name:           npiv
UUID:           77adeef2-e803-49e3-8650-c73fc9f0d948
State:          inactive
Persistent:     yes
Autostart:      no

# virsh pool-uuid npiv
77adeef2-e803-49e3-8650-c73fc9f0d948

# virsh pool-name 77adeef2-e803-49e3-8650-c73fc9f0d948
npiv

4. Check pool-autostart and pool-refresh.
# virsh pool-autostart npiv
Pool npiv marked as autostarted

# virsh pool-list --all
 Name                 State      Autostart 
-------------------------------------------
 default              active     yes       
 npiv                 active     yes

# virsh pool-refresh npiv
Pool npiv refreshed

5. Check the volume in pool.

# virsh vol-list npiv
 Name                 Path                                    
------------------------------------------------------------------------------
 unit:0:0:1           /dev/disk/by-path/pci-0000:18:00.1-vport-0x2101001b32a90000-fc-0x203500a0b85b0acc-lun-1
 unit:0:1:0           /dev/disk/by-path/pci-0000:18:00.1-vport-0x2101001b32a90000-fc-0x203400a0b85b0acc-lun-0

6. Destroy the pool and undefine the pool.
# virsh pool-destroy npiv
Pool npiv destroyed

# virsh pool-undefine npiv
Pool npiv has been undefined

7. Repeat the above steps using pool-create-as with --adapter-parent-wwnn and --adapter-parent-wwpn. --passed

Comment 7 Meina Li 2018-08-10 03:07:37 UTC
Hi John,

In `man virsh`, no relevant explanation for --adapter-parent-wwnn,--adapter-parent-wwpn and --adapter-parent-fabric-wwn in pool-define-as command. Please help check and modify it.

# man virsh
...
pool-define-as name type [--source-host hostname] [--source-path path] [--source-dev path] [--source-name name] [--target path] [--source-format format] [--auth-type authtype --auth-username username [--secret-usage usage | --secret-uuid uuid]] [[--adapter-name name] | [--adapter-wwnn --adapter-wwpn] [--adapter-parent parent]][--print-xml]
...

Comment 8 John Ferlan 2018-08-13 12:26:22 UTC
Hmmm, strange. Neglected to update the pool-create-as item list, but I did add the descriptions for each later in the description section:

See the series:
https://www.redhat.com/archives/libvir-list/2018-March/msg00458.html

and in particular patch 3:
https://www.redhat.com/archives/libvir-list/2018-March/msg00461.html

This was committed upstream here:

$ git show d45bee449c52c25a80dc61cb5af8fab7d77446e9
commit d45bee449c52c25a80dc61cb5af8fab7d77446e9
Author: John Ferlan <jferlan>
Date:   Thu Mar 8 18:17:10 2018 -0500

    tools: Add support for additional adapter parent options
    
    Add the ability to provide the adapter parent_wwnn and parent_wwpn
    or the parent_fabric_wwn on the virsh command line for the pool
    define/create as commands.  Update the virsh.pod description.
    
    Signed-off-by: John Ferlan <jferlan>
...

I just posted a patch upstream to add the strings to the virsh.pod item list:

https://www.redhat.com/archives/libvir-list/2018-August/msg00661.html

Comment 10 Meina Li 2018-08-14 03:30:01 UTC
To track the new problem, and verify this bug, file a new bug1615680.

According to comment 6, move this bug to be verified.

Comment 12 errata-xmlrpc 2018-10-30 09:58:24 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://access.redhat.com/errata/RHSA-2018:3113