[libvirt] [PATCH] Make ZFS storage pool XML tests optional

[Gary R Hook]

On 1/15/15 4:37 PM, Eric Blake wrote:
> On 01/15/2015 03:32 PM, Gary R Hook wrote:
>> From: Gary R Hook <grhookatwork at gmail.com>
>>
>> Do not run ZFS tests when ZFS is unsupported in the environment.
>>
>> The recent patch b4af40226d09adeaf9e33a1d6594c4e8ce7f771d adds tests
>> to storagepoolxml2xmltest for the optional ZFS feature. When ZFS is
>> not included in the configuration these tests should not / cannot be
>> run. Modify Makefile.am and the test source file to check for use of
>> the feature and compile in the tests accordingly.
>>
>> Signed-off-by: Gary R Hook <gary.hook at nimboxx.com>
>>
>> ---
>>   tests/Makefile.am              | 4 ++++
>>   tests/storagepoolxml2xmltest.c | 2 ++
>>   2 files changed, 6 insertions(+)
>>
>> diff --git a/tests/Makefile.am b/tests/Makefile.am
>> index e9418ea..8f8df27 100644
>> --- a/tests/Makefile.am
>> +++ b/tests/Makefile.am
>> @@ -780,6 +780,10 @@ storagepoolxml2xmltest_SOURCES = \
>>   	storagepoolxml2xmltest.c \
>>   	testutils.c testutils.h
>>   storagepoolxml2xmltest_LDADD = $(LDADDS)
>> +storagepoolxml2xmltest_CFLAGS = $(AM_CFLAGS)
>> +if WITH_STORAGE_ZFS
>> +storagepoolxml2xmltest_CFLAGS += -DWITH_STORAGE_ZFS
>> +endif
>
> This shouldn't be needed.  Our config.h should already be defining all
> the appropriate WITH_* feature macros.  If it is not, that's a bug in
> configure.ac.

I'm not adding a #define. I'm just passing one along to the compiler. 
Turning a configure parameter in to a compiler parameter.

If you have a suggestion as to how to #ifdef the source code (below) in 
a way that keeps it from being compiled when ZFS isn't in use, I'm all 
ears. I thought this was pretty clean.

>>
>>   nodedevxml2xmltest_SOURCES = \
>>   	nodedevxml2xmltest.c \
>> diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c
>> index 52e2193..270f75d 100644
>> --- a/tests/storagepoolxml2xmltest.c
>> +++ b/tests/storagepoolxml2xmltest.c
>> @@ -106,8 +106,10 @@ mymain(void)
>>       DO_TEST("pool-gluster");
>>       DO_TEST("pool-gluster-sub");
>>       DO_TEST("pool-scsi-type-scsi-host-stable");
>> +#ifdef WITH_STORAGE_ZFS
>>       DO_TEST("pool-zfs");
>>       DO_TEST("pool-zfs-sourcedev");
>> +#endif
>
> Why is only this test conditional?  Shouldn't other things like gluster
> also be conditional, based on whether those features were built into
> libvirt?

You have a very good point. I wasn't trying to fix any/all problems, 
just the one that bit me, and that is the most recent of this type of 
mistake. I might conclude that any older conditional tests haven't been 
exposed as problematic, although clearly every test should be properly 
vetted against the actual environment.

Really appreciate the quick notice and response, however. Thank you.

-- 
Gary R Hook
Senior Kernel Engineer
NIMBOXX, Inc