Re: multipath -f/-F exit status

Re: multipath -f/-F exit status

[Christophe Varoqui]

> Hi Folks,
> 
> Older versions of multipath-tools would always exit with status 0
> (success) when flushing multipath device maps (-f/-F). This was never
> guaranteed but happened because the "r" local in multipath/main.c was
> used uninitialised in these code paths (by chance, it always lands on
> a freshly zeroed stack page, so although undefined the behaviour was
> pretty reliable).
> 
> This was changed by commit 8497928514aa3df6d46f24d8d9b70b086e9fcfbd:
>
Then by a9a6c71592d4e2271a38c3d334bee7cbce963cff.

Does the upstream version works as you expect in this regard ?
 
> multipath -F
> ------------
> 
> -F     flush all unused multipath device maps
> 
> Exit status
>        0 At least one unused multipath device map was flushed
>        1 No unused maps were found or no maps could be flushed
> 
> The alternative for -F seems to be to return 0 if all unused maps were
> flushed and 1 if there were unused maps that could not be flushed.
> This doesn't seem like a very likely occurrence, but maybe it should
> be handled separately?
> 
I guess the current implementation return r as the number failures.

Regards,
cvaroqui

Re: multipath -f/-F exit status

[Bryn M. Reeves]

Christophe Varoqui wrote:
>> Hi Folks,
>>
>> Older versions of multipath-tools would always exit with status 0
>> (success) when flushing multipath device maps (-f/-F). This was never
>> guaranteed but happened because the "r" local in multipath/main.c was
>> used uninitialised in these code paths (by chance, it always lands on
>> a freshly zeroed stack page, so although undefined the behaviour was
>> pretty reliable).
>>
>> This was changed by commit 8497928514aa3df6d46f24d8d9b70b086e9fcfbd:
>>
> Then by a9a6c71592d4e2271a38c3d334bee7cbce963cff.
> 
> Does the upstream version works as you expect in this regard ?

Sorry - not sure how I missed that last week.

>> The alternative for -F seems to be to return 0 if all unused maps were
>> flushed and 1 if there were unused maps that could not be flushed.
>> This doesn't seem like a very likely occurrence, but maybe it should
>> be handled separately?
>>
> I guess the current implementation return r as the number failures.

It does, which seems a slightly strange thing to return - I don't see 
how it's useful as it's actually the number of device-mapper maps that 
we couldn't flush (sum of returns from dm_flush_map), i.e. the total 
number of maps that either weren't multipath, weren't unused etc :)

This will cause problems on systems that have 256 such maps since the 
exit status is truncated to 8-bits and we would falsely reported the 
status as zero.

It seems more useful to return an overall success/failure indication for 
these commands and would be more in-keeping with other similar tools and 
POSIX.

Regards,
Bryn.

Re: multipath -f/-F exit status

[Christophe Varoqui]

The following patch will appear soon at k.org
It should do what you expect.

Thanks for the comment,
cvaroqui

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index bcbeb28..6b289cf 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -568,7 +568,7 @@ dm_flush_maps (void)
                goto out;
 
        do {
-               r += dm_flush_map(names->name);
+               r |= dm_flush_map(names->name);
                next = names->next;
                names = (void *) names + next;
        } while (next);

Re: multipath -f/-F exit status

[Bryn M. Reeves]

Christophe Varoqui wrote:
> The following patch will appear soon at k.org
> It should do what you expect.

Great - thanks! I've had the user who reported this test with an 
identical change and they are happy with this behaviour.

Cheers,
Bryn.

multipath -f/-F exit status

[Bryn M. Reeves]

Hi Folks,

Older versions of multipath-tools would always exit with status 0
(success) when flushing multipath device maps (-f/-F). This was never
guaranteed but happened because the "r" local in multipath/main.c was
used uninitialised in these code paths (by chance, it always lands on a
freshly zeroed stack page, so although undefined the behaviour was
pretty reliable).

This was changed by commit 8497928514aa3df6d46f24d8d9b70b086e9fcfbd:

commit 8497928514aa3df6d46f24d8d9b70b086e9fcfbd
Author: Christophe Varoqui <root@xa-s05.(none)>
Date:   Mon Mar 13 14:55:16 2006 +0100

     [build] minor compilation issues

     SuSE checker found some minor issues:

     - refwwid in libmultipath/configure.c:get_refwwid() might be used
     uninitialized. As dev_type is a simple 'int' there is nothing which
     prevents the unsuspecting user to call it with an unhandled value.
     We shoud rather use an enum and set refwwid to NULL to be on the
     safe side.

     - attr in libmultipath/discovery.c:devt2devname() might be used
     uninitialized. Quite unlikely, but might happen in principle.
     So better have it initialized.

     - r in multipath/main.c:main() might be used uninitialized.
     True. So have it initialized.

This causes -f/-F in more recent versions of the tools to always exit
with status 1 (failure) which does not seem correct.

I think the following would be an improvement on the current behaviour:

multipath -f <map>
------------------

-f     flush a multipath device map specified as parameter, if unused

Exit Status
        0 The map existed and was successfully flushed
        1 The map did not exist or could not be flushed

multipath -F
------------

-F     flush all unused multipath device maps

Exit status
        0 At least one unused multipath device map was flushed
        1 No unused maps were found or no maps could be flushed

The alternative for -F seems to be to return 0 if all unused maps were
flushed and 1 if there were unused maps that could not be flushed. This
doesn't seem like a very likely occurrence, but maybe it should be
handled separately?

Regards,
Bryn.