[PATCH] virsh: Add source-initiator opt to build the initiator of pool XML

Han Han posted 1 patch 3 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200825035033.547105-1-hhan@redhat.com
Test syntax-check failed
docs/manpages/virsh.rst |  5 +++++
tools/virsh-pool.c      | 17 +++++++++++++++--
2 files changed, 20 insertions(+), 2 deletions(-)
[PATCH] virsh: Add source-initiator opt to build the initiator of pool XML
Posted by Han Han 3 years, 7 months ago
For iscsi-direct pool, the initiator is necessary for pool defining:
<pool type="iscsi-direct">
 ...
    <initiator>
      <iqn name="iqn.2013-06.com.example:iscsi-initiator"/>
    </initiator>
...
</pool>

Add --source-initiator to fill the initiator iqn for
pool-create-as/pool-define-as subcommands.

https://bugzilla.redhat.com/show_bug.cgi?id=1658082

Signed-off-by: Han Han <hhan@redhat.com>
---
 docs/manpages/virsh.rst |  5 +++++
 tools/virsh-pool.c      | 17 +++++++++++++++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 9187037a56..0482fe8b26 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -5732,6 +5732,7 @@ pool-create-as
    pool-create-as name type
       [--source-host hostname] [--source-path path] [--source-dev path]
       [--source-name name] [--target path] [--source-format format]
+      [--source-initiator initiator-iqn]
       [--auth-type authtype --auth-username username
       [--secret-usage usage | --secret-uuid uuid]]
       [--source-protocol-ver ver]
@@ -5769,6 +5770,9 @@ the host file system.
 [*--source-format format*] provides information about the format of the
 pool (pool types fs, netfs, disk, logical).
 
+[*--source-initiator initiator-iqn*] provides the initiator iqn for iscsi
+connection of the pool (pool type iscsi-direct).
+
 [*--auth-type authtype* *--auth-username username*
 [*--secret-usage usage* | *--secret-uuid uuid*]]
 provides the elements required to generate authentication credentials for
@@ -5831,6 +5835,7 @@ pool-define-as
    pool-define-as name type
       [--source-host hostname] [--source-path path] [--source-dev path]
       [*--source-name name*] [*--target path*] [*--source-format format*]
+      [--source-initiator initiator-iqn]
       [*--auth-type authtype* *--auth-username username*
       [*--secret-usage usage* | *--secret-uuid uuid*]]
       [*--source-protocol-ver ver*]
diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index fd43d3bb62..a7e0aa3010 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -141,6 +141,10 @@
     {.name = "source-protocol-ver", \
      .type = VSH_OT_STRING, \
      .help = N_("nfsvers value for NFS pool mount option") \
+    }, \
+    {.name = "source-initiator", \
+     .type = VSH_OT_STRING, \
+     .help = N_("initiator iqn for underlying storage") \
     }
 
 virStoragePoolPtr
@@ -324,7 +328,8 @@ virshBuildPoolXML(vshControl *ctl,
                *secretUsage = NULL, *adapterName = NULL, *adapterParent = NULL,
                *adapterWwnn = NULL, *adapterWwpn = NULL, *secretUUID = NULL,
                *adapterParentWwnn = NULL, *adapterParentWwpn = NULL,
-               *adapterParentFabricWwn = NULL, *protoVer = NULL;
+               *adapterParentFabricWwn = NULL, *protoVer = NULL,
+               *srcInitiator = NULL;
     g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
 
     VSH_EXCLUSIVE_OPTIONS("secret-usage", "secret-uuid");
@@ -352,7 +357,8 @@ virshBuildPoolXML(vshControl *ctl,
         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 ||
-        vshCommandOptStringReq(ctl, cmd, "source-protocol-ver", &protoVer) < 0) {
+        vshCommandOptStringReq(ctl, cmd, "source-protocol-ver", &protoVer) < 0 ||
+        vshCommandOptStringReq(ctl, cmd, "source-initiator", &srcInitiator) < 0) {
         return false;
     }
 
@@ -370,6 +376,13 @@ virshBuildPoolXML(vshControl *ctl,
             virBufferAsprintf(&buf, "<dir path='%s'/>\n", srcPath);
         if (srcDev)
             virBufferAsprintf(&buf, "<device path='%s'/>\n", srcDev);
+        if (srcInitiator) {
+            virBufferAddLit(&buf, "<initiator>\n");
+            virBufferAdjustIndent(&buf, 2);
+            virBufferAsprintf(&buf, "<iqn name='%s'/>\n", srcInitiator);
+            virBufferAdjustIndent(&buf, -2);
+            virBufferAddLit(&buf, "</initiator>\n");
+        }
         if (adapterWwnn && adapterWwpn) {
             virBufferAddLit(&buf, "<adapter type='fc_host'");
             if (adapterParent)
-- 
2.28.0

Re: [PATCH] virsh: Add source-initiator opt to build the initiator of pool XML
Posted by Michal Privoznik 3 years, 7 months ago
On 8/25/20 5:50 AM, Han Han wrote:
> For iscsi-direct pool, the initiator is necessary for pool defining:
> <pool type="iscsi-direct">
>   ...
>      <initiator>
>        <iqn name="iqn.2013-06.com.example:iscsi-initiator"/>
>      </initiator>
> ...
> </pool>
> 
> Add --source-initiator to fill the initiator iqn for
> pool-create-as/pool-define-as subcommands.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1658082
> 
> Signed-off-by: Han Han <hhan@redhat.com>
> ---
>   docs/manpages/virsh.rst |  5 +++++
>   tools/virsh-pool.c      | 17 +++++++++++++++--
>   2 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> index 9187037a56..0482fe8b26 100644
> --- a/docs/manpages/virsh.rst
> +++ b/docs/manpages/virsh.rst
> @@ -5732,6 +5732,7 @@ pool-create-as
>      pool-create-as name type
>         [--source-host hostname] [--source-path path] [--source-dev path]
>         [--source-name name] [--target path] [--source-format format]
> +      [--source-initiator initiator-iqn]
>         [--auth-type authtype --auth-username username
>         [--secret-usage usage | --secret-uuid uuid]]
>         [--source-protocol-ver ver]
> @@ -5769,6 +5770,9 @@ the host file system.
>   [*--source-format format*] provides information about the format of the
>   pool (pool types fs, netfs, disk, logical).
>   
> +[*--source-initiator initiator-iqn*] provides the initiator iqn for iscsi
> +connection of the pool (pool type iscsi-direct).
> +
>   [*--auth-type authtype* *--auth-username username*
>   [*--secret-usage usage* | *--secret-uuid uuid*]]
>   provides the elements required to generate authentication credentials for
> @@ -5831,6 +5835,7 @@ pool-define-as
>      pool-define-as name type
>         [--source-host hostname] [--source-path path] [--source-dev path]
>         [*--source-name name*] [*--target path*] [*--source-format format*]
> +      [--source-initiator initiator-iqn]
>         [*--auth-type authtype* *--auth-username username*
>         [*--secret-usage usage* | *--secret-uuid uuid*]]
>         [*--source-protocol-ver ver*]
> diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
> index fd43d3bb62..a7e0aa3010 100644
> --- a/tools/virsh-pool.c
> +++ b/tools/virsh-pool.c
> @@ -141,6 +141,10 @@
>       {.name = "source-protocol-ver", \
>        .type = VSH_OT_STRING, \
>        .help = N_("nfsvers value for NFS pool mount option") \
> +    }, \
> +    {.name = "source-initiator", \
> +     .type = VSH_OT_STRING, \
> +     .help = N_("initiator iqn for underlying storage") \
>       }
>   
>   virStoragePoolPtr
> @@ -324,7 +328,8 @@ virshBuildPoolXML(vshControl *ctl,
>                  *secretUsage = NULL, *adapterName = NULL, *adapterParent = NULL,
>                  *adapterWwnn = NULL, *adapterWwpn = NULL, *secretUUID = NULL,
>                  *adapterParentWwnn = NULL, *adapterParentWwpn = NULL,
> -               *adapterParentFabricWwn = NULL, *protoVer = NULL;
> +               *adapterParentFabricWwn = NULL, *protoVer = NULL,
> +               *srcInitiator = NULL;
>       g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
>   
>       VSH_EXCLUSIVE_OPTIONS("secret-usage", "secret-uuid");
> @@ -352,7 +357,8 @@ virshBuildPoolXML(vshControl *ctl,
>           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 ||
> -        vshCommandOptStringReq(ctl, cmd, "source-protocol-ver", &protoVer) < 0) {
> +        vshCommandOptStringReq(ctl, cmd, "source-protocol-ver", &protoVer) < 0 ||
> +        vshCommandOptStringReq(ctl, cmd, "source-initiator", &srcInitiator) < 0) {
>           return false;
>       }
>   
> @@ -370,6 +376,13 @@ virshBuildPoolXML(vshControl *ctl,

There is an if () at the beginning of this block which adds <source/> 
around these elements below. I guess we want to adjust the check to 
include srcInitiator check.

>               virBufferAsprintf(&buf, "<dir path='%s'/>\n", srcPath);
>           if (srcDev)
>               virBufferAsprintf(&buf, "<device path='%s'/>\n", srcDev);
> +        if (srcInitiator) {
> +            virBufferAddLit(&buf, "<initiator>\n");
> +            virBufferAdjustIndent(&buf, 2);
> +            virBufferAsprintf(&buf, "<iqn name='%s'/>\n", srcInitiator);
> +            virBufferAdjustIndent(&buf, -2);
> +            virBufferAddLit(&buf, "</initiator>\n");
> +        }
>           if (adapterWwnn && adapterWwpn) {
>               virBufferAddLit(&buf, "<adapter type='fc_host'");
>               if (adapterParent)
> 

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

and pushed.

Michal