[libvirt] [PATCH 3/6] conf: Move <disk> authdef validation

[Peter Krempa]

On Thu, Sep 14, 2017 at 14:03:07 -0400, John Ferlan wrote:
> Rather than checking during XML processing, move the checks for correct
> and valid auth into virDomainDiskDefParseValidate. This will introduce
> virDomainDiskSourceDefParseAuthValidate to validate that the authdef
> stored for the virStorageSource is valid. This can then be expanded
> to service backingStore sources as well.
> 
> Alter the message text slightly as well to distinguish between an
> unknown name and an incorrectly used name.  Since type is not a
> mandatory field, add the NULLSTR() around the output of the unknown
> error. NB, a config using unknown formatting would fail virschematest
> since it only accepts 'iscsi' and 'ceph' as "valid" types.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/conf/domain_conf.c | 67 +++++++++++++++++++++++++-------------------------
>  1 file changed, 34 insertions(+), 33 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a43b25c31..07bda1a36 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -8500,6 +8500,39 @@ virDomainDiskDefGeometryParse(virDomainDiskDefPtr def,
>  
>  
>  static int
> +virDomainDiskSourceDefParseAuthValidate(const virStorageSource *src)
> +{
> +    virStorageAuthDefPtr authdef = src->auth;
> +    int actUsage;
> +
> +    /* Disk volume types won't have the secrettype filled in until
> +     * after virStorageTranslateDiskSourcePool is run
> +     */
> +    if (src->type == VIR_STORAGE_TYPE_VOLUME || !authdef)
> +        return 0;

Should this also include || src->type != VIR_STORAGE_TYPE_NETWORK?

> +
> +    if ((actUsage = virSecretUsageTypeFromString(authdef->secrettype)) < 0) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("unknown secret type '%s'"),
> +                       NULLSTR(authdef->secrettype));
> +        return -1;
> +    }
> +
> +    if ((src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI &&
> +         actUsage != VIR_SECRET_USAGE_TYPE_ISCSI) ||
> +        (src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD &&
> +         actUsage != VIR_SECRET_USAGE_TYPE_CEPH)) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("invalid secret type '%s'"),
> +                       virSecretUsageTypeToString(actUsage));
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +
> +static int
>  virDomainDiskDefParseValidate(const virDomainDiskDef *def)
>  {
>      if (def->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) {
> @@ -8572,7 +8605,7 @@ virDomainDiskDefParseValidate(const virDomainDiskDef *def)
>          }
>      }
>  
> -    return 0;
> +    return virDomainDiskSourceDefParseAuthValidate(def->src);

As common in similar functions, add a if block and leave the "return 0"
intact. Saving two lines is not worth breaking the structure and making
the code less extensible.

>  }
>  
>  

ACK with the above fixed.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170915/dd849d5e/attachment-0001.sig>