Re: [PATCH 0/21] SUSE multipath-tools resync

[Christophe Varoqui <christophe.varoqui@free.fr>]

Le lundi 21 mai 2007 à 11:22 +0200, Hannes Reinecke a écrit :
> Hi Christophe,
> 
> there have been quite some fixes accumulating here; and as I finally
> found some time I thought it a nice idea to have them sent upstream.
> There are fixes for various bits and pieces; mostly no big deal.
> Most important of these are the following:
> 
Thanks, I started reading and merging this work.

> - Added an option '-t' to dump the internal hardware table to multipath.
>   This is quite helpful when generating your own multipath-tools.conf
>   and you'd like to start with some sensible defaults.

Doesn't the daemon CLI "show config" command fill this need ?
If so, I'd rather drop this patch (and the associated manpage patch).

> - We now got a manpage multipath.conf.5 !

Nice

> - New priority checker 'hp_sw' which will allow us to group these
>   machines by priority, too.
Have you pushed the kernel hardware handler upstream too ? (the
START_STOP one ?)

> 
- directio checker async mode
Interesting stuff.
1) Do we need the libaio.h copy ?
2) Have you put some thought to SG_IO async mode feasability ?
3) Shouldn't we also timeout the execv() for path prios and ids to avoid
blocking the daemon.

> Some patches might be a bit controversial; they make sense to me, but
> others might think otherwise:
> 
> - I've removed libsysfs and implemented the sysfs handling internally.
>   With the current changes in sysfs the older libsysfs doesn't work
>   anymore and in either case it was just a big annoyance.

I mostly agree, but this one will be long to review :)

You seem to have a genuine fix in there, in cli_add_map(). It might
deserve a separate patch.

> - I've hacked up kpartx to handle extended partitions as well.
>   Currently it's quite peculiar with kpartx; we'd be generating
>   tables for all partitions _unless_ it's an extended one.
>   So we won't be getting any symlinks to the extended partition
>   when we're using kpartx, although the device itself is present.
> 
I'll read those later.

> Feel free to omit them if you don't agree.
> And of course, feedback etc is welcome.
> 
I add this one, that might interest you :

commit 9ff5a680b7803d8932059b2e0d6d295c60c95370
Author: Christophe Varoqui <cvaroqui@zezette.localdomain>
Date:   Tue May 22 00:24:22 2007 +0200

    [libmultipath] new devmapper log handler needs stdarg.h
    
    As it uses the va_list type.

> Christophe, please pull from
> 
> git://git.kernel.org/pub/scm/linux/kernel/hare/multipath-tools/.git
> #upstream-fixes.
> 
> Cheers,
> 
> Hannes