[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 07/11/2017 07:30 AM, Pavel Hrdina wrote:
> 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.
> 

OK - I made them a list, e.g.:

    testStoragePoolObjFindActiveByName
    testStoragePoolObjFindInactiveByName

>> 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>
> 

Done.

John


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