[libvirt] [PATCH] virsh: Add/allow secret-uuid for pool-{define|create}-as

https://bugzilla.redhat.com/show_bug.cgi?id=1476775 For the virsh pool-{define|create}-as command, let's allow using --secret-uuid on the command line as an alternative to --secret-usage, but ensure that they are mutually exclusive. Not sure why I neglected to add it for commit id '8932580' Signed-off-by: John Ferlan <jferlan@redhat.com> --- tools/virsh-pool.c | 16 +++++++++++++--- tools/virsh.pod | 9 ++++++--- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index ba9281f..03eae94 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -109,6 +109,10 @@ .type = VSH_OT_STRING, \ .help = N_("auth secret usage to be used for underlying storage") \ }, \ + {.name = "secret-uuid", \ + .type = VSH_OT_STRING, \ + .help = N_("auth secret UUID to be used for underlying storage") \ + }, \ {.name = "adapter-name", \ .type = VSH_OT_STRING, \ .help = N_("adapter name to be used for underlying storage") \ @@ -302,9 +306,11 @@ 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; + *adapterWwnn = NULL, *adapterWwpn = NULL, *secretUUID = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; + VSH_EXCLUSIVE_OPTIONS("secret-usage", "secret-uuid"); + if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0) goto cleanup; if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0) @@ -319,6 +325,7 @@ virshBuildPoolXML(vshControl *ctl, vshCommandOptStringReq(ctl, cmd, "auth-type", &authType) < 0 || vshCommandOptStringReq(ctl, cmd, "auth-username", &authUsername) < 0 || vshCommandOptStringReq(ctl, cmd, "secret-usage", &secretUsage) < 0 || + vshCommandOptStringReq(ctl, cmd, "secret-uuid", &secretUUID) < 0 || vshCommandOptStringReq(ctl, cmd, "adapter-name", &adapterName) < 0 || vshCommandOptStringReq(ctl, cmd, "adapter-wwnn", &adapterWwnn) < 0 || vshCommandOptStringReq(ctl, cmd, "adapter-wwpn", &adapterWwpn) < 0 || @@ -349,11 +356,14 @@ virshBuildPoolXML(vshControl *ctl, virBufferAsprintf(&buf, "<adapter type='scsi_host' name='%s'/>\n", adapterName); } - if (authType && authUsername && secretUsage) { + if (authType && authUsername && (secretUsage || secretUUID)) { virBufferAsprintf(&buf, "<auth type='%s' username='%s'>\n", authType, authUsername); virBufferAdjustIndent(&buf, 2); - virBufferAsprintf(&buf, "<secret usage='%s'/>\n", secretUsage); + if (secretUsage) + virBufferAsprintf(&buf, "<secret usage='%s'/>\n", secretUsage); + else + virBufferAsprintf(&buf, "<secret uuid='%s'/>\n", secretUUID); virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</auth>\n"); } diff --git a/tools/virsh.pod b/tools/virsh.pod index c13f96f..bf5a124 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3673,7 +3673,8 @@ just I<--build> is provided, then B<pool-build> is called with no flags. =item B<pool-create-as> I<name> I<type> [I<--source-host hostname>] [I<--source-path path>] [I<--source-dev path>] [I<--source-name name>] [I<--target path>] [I<--source-format format>] -[I<--auth-type authtype> I<--auth-username username> I<--secret-usage usage>] +[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-parent parent>]] [I<--build>] [[I<--overwrite>] | [I<--no-overwrite>]] [I<--print-xml>] @@ -3707,10 +3708,12 @@ the host file system. [I<--source-format format>] provides information about the format of the pool (pool types fs, netfs, disk, logical). -[I<--auth-type authtype> I<--auth-username username> I<--secret-usage usage>] +[I<--auth-type authtype> I<--auth-username username> +[I<--secret-usage usage> | I<--secret-uuid uuid>]] provides the elements required to generate authentication credentials for the storage pool. The I<authtype> is either chap for iscsi I<type> pools or -ceph for rbd I<type> pools. +ceph for rbd I<type> pools. Either the secret I<usage> or I<uuid> value may +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. -- 2.9.5

ping - tks, John On 09/05/2017 02:42 PM, John Ferlan wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1476775 > > For the virsh pool-{define|create}-as command, let's allow using > --secret-uuid on the command line as an alternative to --secret-usage, > but ensure that they are mutually exclusive. > > Not sure why I neglected to add it for commit id '8932580' > > Signed-off-by: John Ferlan <jferlan@redhat.com> > --- > tools/virsh-pool.c | 16 +++++++++++++--- > tools/virsh.pod | 9 ++++++--- > 2 files changed, 19 insertions(+), 6 deletions(-) > > diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c > index ba9281f..03eae94 100644 > --- a/tools/virsh-pool.c > +++ b/tools/virsh-pool.c > @@ -109,6 +109,10 @@ > .type = VSH_OT_STRING, \ > .help = N_("auth secret usage to be used for underlying storage") \ > }, \ > + {.name = "secret-uuid", \ > + .type = VSH_OT_STRING, \ > + .help = N_("auth secret UUID to be used for underlying storage") \ > + }, \ > {.name = "adapter-name", \ > .type = VSH_OT_STRING, \ > .help = N_("adapter name to be used for underlying storage") \ > @@ -302,9 +306,11 @@ 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; > + *adapterWwnn = NULL, *adapterWwpn = NULL, *secretUUID = NULL; > virBuffer buf = VIR_BUFFER_INITIALIZER; > > + VSH_EXCLUSIVE_OPTIONS("secret-usage", "secret-uuid"); > + > if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0) > goto cleanup; > if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0) > @@ -319,6 +325,7 @@ virshBuildPoolXML(vshControl *ctl, > vshCommandOptStringReq(ctl, cmd, "auth-type", &authType) < 0 || > vshCommandOptStringReq(ctl, cmd, "auth-username", &authUsername) < 0 || > vshCommandOptStringReq(ctl, cmd, "secret-usage", &secretUsage) < 0 || > + vshCommandOptStringReq(ctl, cmd, "secret-uuid", &secretUUID) < 0 || > vshCommandOptStringReq(ctl, cmd, "adapter-name", &adapterName) < 0 || > vshCommandOptStringReq(ctl, cmd, "adapter-wwnn", &adapterWwnn) < 0 || > vshCommandOptStringReq(ctl, cmd, "adapter-wwpn", &adapterWwpn) < 0 || > @@ -349,11 +356,14 @@ virshBuildPoolXML(vshControl *ctl, > virBufferAsprintf(&buf, "<adapter type='scsi_host' name='%s'/>\n", > adapterName); > } > - if (authType && authUsername && secretUsage) { > + if (authType && authUsername && (secretUsage || secretUUID)) { > virBufferAsprintf(&buf, "<auth type='%s' username='%s'>\n", > authType, authUsername); > virBufferAdjustIndent(&buf, 2); > - virBufferAsprintf(&buf, "<secret usage='%s'/>\n", secretUsage); > + if (secretUsage) > + virBufferAsprintf(&buf, "<secret usage='%s'/>\n", secretUsage); > + else > + virBufferAsprintf(&buf, "<secret uuid='%s'/>\n", secretUUID); > virBufferAdjustIndent(&buf, -2); > virBufferAddLit(&buf, "</auth>\n"); > } > diff --git a/tools/virsh.pod b/tools/virsh.pod > index c13f96f..bf5a124 100644 > --- a/tools/virsh.pod > +++ b/tools/virsh.pod > @@ -3673,7 +3673,8 @@ just I<--build> is provided, then B<pool-build> is called with no flags. > =item B<pool-create-as> I<name> I<type> > [I<--source-host hostname>] [I<--source-path path>] [I<--source-dev path>] > [I<--source-name name>] [I<--target path>] [I<--source-format format>] > -[I<--auth-type authtype> I<--auth-username username> I<--secret-usage usage>] > +[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-parent parent>]] > [I<--build>] [[I<--overwrite>] | [I<--no-overwrite>]] [I<--print-xml>] > @@ -3707,10 +3708,12 @@ the host file system. > [I<--source-format format>] provides information about the format of the > pool (pool types fs, netfs, disk, logical). > > -[I<--auth-type authtype> I<--auth-username username> I<--secret-usage usage>] > +[I<--auth-type authtype> I<--auth-username username> > +[I<--secret-usage usage> | I<--secret-uuid uuid>]] > provides the elements required to generate authentication credentials for > the storage pool. The I<authtype> is either chap for iscsi I<type> pools or > -ceph for rbd I<type> pools. > +ceph for rbd I<type> pools. Either the secret I<usage> or I<uuid> value may > +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. >

On Tue, Sep 05, 2017 at 02:42:58PM -0400, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1476775
For the virsh pool-{define|create}-as command, let's allow using --secret-uuid on the command line as an alternative to --secret-usage, but ensure that they are mutually exclusive.
Not sure why I neglected to add it for commit id '8932580'
^This is IMHO more suitable under the --- :) [...]
diff --git a/tools/virsh.pod b/tools/virsh.pod index c13f96f..bf5a124 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3673,7 +3673,8 @@ just I<--build> is provided, then B<pool-build> is called with no flags. =item B<pool-create-as> I<name> I<type> [I<--source-host hostname>] [I<--source-path path>] [I<--source-dev path>] [I<--source-name name>] [I<--target path>] [I<--source-format format>] -[I<--auth-type authtype> I<--auth-username username> I<--secret-usage usage>] +[I<--auth-type authtype> I<--auth-username username>
pool-define-as should have the argument list updated the same way as pool-create-as and by that I of course mean just the enumeration, since pool-define-as states that the arguments mean the same thing as for pool-create-as. Reviewed-by: Erik Skultety <eskultet@redhat.com> (with the tiny adjustment)

On 09/27/2017 05:24 AM, Erik Skultety wrote:
On Tue, Sep 05, 2017 at 02:42:58PM -0400, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1476775
For the virsh pool-{define|create}-as command, let's allow using --secret-uuid on the command line as an alternative to --secret-usage, but ensure that they are mutually exclusive.
Not sure why I neglected to add it for commit id '8932580'
^This is IMHO more suitable under the --- :)
Kind of an after thought when I wrote it, but I also think working it into the commit message is good for the ease of chasing history...
[...]
diff --git a/tools/virsh.pod b/tools/virsh.pod index c13f96f..bf5a124 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3673,7 +3673,8 @@ just I<--build> is provided, then B<pool-build> is called with no flags. =item B<pool-create-as> I<name> I<type> [I<--source-host hostname>] [I<--source-path path>] [I<--source-dev path>] [I<--source-name name>] [I<--target path>] [I<--source-format format>] -[I<--auth-type authtype> I<--auth-username username> I<--secret-usage usage>] +[I<--auth-type authtype> I<--auth-username username>
pool-define-as should have the argument list updated the same way as pool-create-as and by that I of course mean just the enumeration, since pool-define-as states that the arguments mean the same thing as for pool-create-as.
Reviewed-by: Erik Skultety <eskultet@redhat.com> (with the tiny adjustment)
oh right - thanks... Fixed things up and pushed. John
participants (2)
-
Erik Skultety
-
John Ferlan