[libvirt] [PATCH 0/3] Update the virsh pool define/create for vHBA/NPIV

Looks like when adding support in commit 78be2e8b (and the 3 just before it), I neglected to update the virsh command allow command line usage of the fields. This series updates virsh in order to allow adding the fields (as ugly as they are) to make it easier to define/create the pool with the fields. John Ferlan (3): tools: Add the wwnn/wwpn to the man page for storage pool fields tools: Update the help description of the adapter-parent field tools: Add support for additional adapter parent options tools/virsh-pool.c | 29 ++++++++++++++++++++++++++--- tools/virsh.pod | 17 ++++++++++++----- 2 files changed, 38 insertions(+), 8 deletions(-) -- 2.13.6

The description was missing the wwnn and wwpn names for the --adapter-wwnn and --adapter-wwpn switches. Just add it to be clear that the fields cannot be empty (IOW they are not boolean). Signed-off-by: John Ferlan <jferlan@redhat.com> --- tools/virsh.pod | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index 8f0e8d74b..9fa483da3 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3725,7 +3725,7 @@ just I<--build> is provided, then B<pool-build> is called with no flags. [I<--source-name name>] [I<--target path>] [I<--source-format format>] [I<--auth-type authtype> I<--auth-username username> [I<--secret-usage usage> | I<--secret-uuid uuid>]] -[[I<--adapter-name name>] | [I<--adapter-wwnn> I<--adapter-wwpn>] +[[I<--adapter-name name>] | [I<--adapter-wwnn> wwnn I<--adapter-wwpn> wwpn] [I<--adapter-parent parent>]] [I<--build>] [[I<--overwrite>] | [I<--no-overwrite>]] [I<--print-xml>] @@ -3768,10 +3768,10 @@ be provided, but not both. [I<--adapter-name name>] defines the scsi_hostN adapter name to be used for the scsi_host adapter type pool. -[I<--adapter-wwnn> I<--adapter-wwpn> [I<--adapter-parent parent>]] defines -the wwnn and wwpn to be used for the fc_host adapter type pool. The parent -optionally provides the name of the scsi_hostN node device to be used for -the vHBA. +[I<--adapter-wwnn wwnn> I<--adapter-wwpn wwpn> [I<--adapter-parent parent>]] +defines the wwnn and wwpn to be used for the fc_host adapter type pool. +The parent optionally provides the name of the scsi_hostN node device to +be used for the vHBA. [I<--build>] [[I<--overwrite>] | [I<--no-overwrite>]] perform a B<pool-build> after creation in order to remove the need for a -- 2.13.6

One short sentence won't do it justice, but it could help by listing scsi_hostN and vHBA to point one in the right direction. Signed-off-by: John Ferlan <jferlan@redhat.com> --- tools/virsh-pool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 9c7e972ba..668e8087d 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -127,7 +127,7 @@ }, \ {.name = "adapter-parent", \ .type = VSH_OT_STRING, \ - .help = N_("adapter parent to be used for underlying storage") \ + .help = N_("adapter parent scsi_hostN to be used for underlying vHBA storage") \ } virStoragePoolPtr -- 2.13.6

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@redhat.com> --- tools/virsh-pool.c | 27 +++++++++++++++++++++++++-- tools/virsh.pod | 13 ++++++++++--- 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 668e8087d..36bf8d9fa 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -128,6 +128,18 @@ {.name = "adapter-parent", \ .type = VSH_OT_STRING, \ .help = N_("adapter parent scsi_hostN to be used for underlying vHBA storage") \ + }, \ + {.name = "adapter-parent-wwnn", \ + .type = VSH_OT_STRING, \ + .help = N_("adapter parent scsi_hostN wwnn to be used for underlying vHBA storage") \ + }, \ + {.name = "adapter-parent-wwpn", \ + .type = VSH_OT_STRING, \ + .help = N_("adapter parent scsi_hostN wwpn to be used for underlying vHBA storage") \ + }, \ + {.name = "adapter-parent-fabric-wwn", \ + .type = VSH_OT_STRING, \ + .help = N_("adapter parent scsi_hostN fabric_wwn to be used for underlying vHBA storage") \ } virStoragePoolPtr @@ -309,7 +321,9 @@ virshBuildPoolXML(vshControl *ctl, *srcDev = NULL, *srcName = NULL, *srcFormat = NULL, *target = NULL, *authType = NULL, *authUsername = NULL, *secretUsage = NULL, *adapterName = NULL, *adapterParent = NULL, - *adapterWwnn = NULL, *adapterWwpn = NULL, *secretUUID = NULL; + *adapterWwnn = NULL, *adapterWwpn = NULL, *secretUUID = NULL, + *adapterParentWwnn = NULL, *adapterParentWwpn = NULL, + *adapterParentFabricWwn = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; VSH_EXCLUSIVE_OPTIONS("secret-usage", "secret-uuid"); @@ -332,7 +346,10 @@ virshBuildPoolXML(vshControl *ctl, vshCommandOptStringReq(ctl, cmd, "adapter-name", &adapterName) < 0 || vshCommandOptStringReq(ctl, cmd, "adapter-wwnn", &adapterWwnn) < 0 || vshCommandOptStringReq(ctl, cmd, "adapter-wwpn", &adapterWwpn) < 0 || - vshCommandOptStringReq(ctl, cmd, "adapter-parent", &adapterParent) < 0) + vshCommandOptStringReq(ctl, cmd, "adapter-parent", &adapterParent) < 0 || + vshCommandOptStringReq(ctl, cmd, "adapter-parent-wwnn", &adapterParentWwnn) < 0 || + vshCommandOptStringReq(ctl, cmd, "adapter-parent-wwpn", &adapterParentWwpn) < 0 || + vshCommandOptStringReq(ctl, cmd, "adapter-parent-fabric-wwn", &adapterParentFabricWwn) < 0) goto cleanup; virBufferAsprintf(&buf, "<pool type='%s'>\n", type); @@ -353,6 +370,12 @@ virshBuildPoolXML(vshControl *ctl, virBufferAddLit(&buf, "<adapter type='fc_host'"); if (adapterParent) virBufferAsprintf(&buf, " parent='%s'", adapterParent); + else if (adapterParentWwnn && adapterParentWwpn) + virBufferAsprintf(&buf, " parent_wwnn='%s' parent_wwnn='%s'", + adapterParentWwnn, adapterParentWwpn); + else if (adapterParentFabricWwn) + virBufferAsprintf(&buf, " parent_fabric_wwn='%s'", + adapterParentFabricWwn); virBufferAsprintf(&buf, " wwnn='%s' wwpn='%s'/>\n", adapterWwnn, adapterWwpn); } else if (adapterName) { diff --git a/tools/virsh.pod b/tools/virsh.pod index 9fa483da3..20236885e 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3768,10 +3768,17 @@ be provided, but not both. [I<--adapter-name name>] defines the scsi_hostN adapter name to be used for the scsi_host adapter type pool. -[I<--adapter-wwnn wwnn> I<--adapter-wwpn wwpn> [I<--adapter-parent parent>]] +[I<--adapter-wwnn wwnn> I<--adapter-wwpn wwpn> [I<--adapter-parent parent> | +I<--adapter-parent-wwnn parent_wwnn> I<adapter-parent-wwpn parent_wwpn> | +I<--adapter-parent-fabric-wwn parent_fabric_wwn>]] defines the wwnn and wwpn to be used for the fc_host adapter type pool. -The parent optionally provides the name of the scsi_hostN node device to -be used for the vHBA. +Optionally provide the parent scsi_hostN node device to be used for the +vHBA either by parent name, parent_wwnn and parent_wwpn, or parent_fabric_wwn. +The parent name could change between reboots if the hardware environment +changes, so providing the parent_wwnn and parent_wwpn ensure usage of the +same physical HBA even if the scsi_hostN node device changes. Usage of the +parent_fabric_wwn allows a bit more flexibility to choose an HBA on the +same storage fabric in order to define the pool. [I<--build>] [[I<--overwrite>] | [I<--no-overwrite>]] perform a B<pool-build> after creation in order to remove the need for a -- 2.13.6

On 03/09/2018 12:31 AM, John Ferlan wrote:
Looks like when adding support in commit 78be2e8b (and the 3 just before it), I neglected to update the virsh command allow command line usage of the fields. This series updates virsh in order to allow adding the fields (as ugly as they are) to make it easier to define/create the pool with the fields.
John Ferlan (3): tools: Add the wwnn/wwpn to the man page for storage pool fields tools: Update the help description of the adapter-parent field tools: Add support for additional adapter parent options
tools/virsh-pool.c | 29 ++++++++++++++++++++++++++--- tools/virsh.pod | 17 ++++++++++++----- 2 files changed, 38 insertions(+), 8 deletions(-)
ACK Michal
participants (2)
-
John Ferlan
-
Michal Privoznik