[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 04/19] test: Add helpers to fetch active/inactive storage pool by name



On Tue, May 09, 2017 at 11:30:11AM -0400, John Ferlan wrote:
> Rather than have repetitive code - create/use a couple of helpers:
> 
> testStoragePoolObjFindActiveByName and testStoragePoolObjFindInactiveByName

I would wrap this line, it's too long for no reason.

> This will also allow for the reduction of some cleanup path logic.
> 
> Signed-off-by: John Ferlan <jferlan redhat com>
> ---
>  src/test/test_driver.c | 256 +++++++++++++++++--------------------------------
>  1 file changed, 86 insertions(+), 170 deletions(-)
> 
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index efa54ff..9918df6 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -4030,6 +4030,46 @@ testStoragePoolObjFindByName(testDriverPtr privconn,
>  
>  
>  static virStoragePoolObjPtr
> +testStoragePoolObjFindActiveByName(testDriverPtr privconn,
> +                                   const char *name)
> +{
> +    virStoragePoolObjPtr obj;
> +
> +    if (!(obj = testStoragePoolObjFindByName(privconn, name)))
> +        return NULL;
> +
> +    if (!virStoragePoolObjIsActive(obj)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       _("storage pool '%s' is not active"), name);
> +        virStoragePoolObjUnlock(obj);
> +        return NULL;
> +    }
> +
> +    return obj;
> +}
> +
> +
> +static virStoragePoolObjPtr
> +testStoragePoolObjFindInactiveByName(testDriverPtr privconn,
> +                                     const char *name)
> +{
> +    virStoragePoolObjPtr obj;
> +
> +    if (!(obj = testStoragePoolObjFindByName(privconn, name)))
> +        return NULL;
> +
> +    if (virStoragePoolObjIsActive(obj)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       _("storage pool '%s' is already active"), name);

I would remove the "already" for the error message.  Since this is only
test driver I'll leave it up to you.  The reason is that for some APIs
like "Undefine" the error message doesn't make sense.

Reviewed-by: Pavel Hrdina <phrdina redhat com>

Attachment: signature.asc
Description: Digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]