[dm-devel] [PATCH] multipathd: fix no checking persistent reservation registration

[lixiaokeng]

Hi Martin,

You are right!! This problem was found in 0.7.7. Now I know it
is fixed with commit 9b715bf9 in 0.8.1.
I will use the commit 9b715bf9 to fix it. Thanks for your
review and answer very much.

Regarts
Lixiaokeng

On 2021/6/24 18:19, Martin Wilck wrote:
> On Do, 2021-06-24 at 16:47 +0800, lixiaokeng wrote:
>> If one multipath device has two paths, the first is down (network
>> failure) and the second is up, then we register a prkey to the
>> device. The first path will register successfully but the second
>> won't. Then fix the network. The uev_update_path will race with
>> check_path. If uev_update_path -> pathinfo is called before
>> check_path, the state of the first path will be set PATH_UP
>> without checking persistent reservation registration.
>>
>> Here we add checking in uev_update_path.
>>
>> Signed-off-by: Lixiaokeng <lixiaokeng at huawei.com>
>> ---
>>  multipathd/main.c | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/multipathd/main.c b/multipathd/main.c
>> index 2251e02c..0948bf81 100644
>> --- a/multipathd/main.c
>> +++ b/multipathd/main.c
>> @@ -1316,6 +1316,7 @@ uev_update_path (struct uevent *uev, struct
>> vectors * vecs)
>>         struct path * pp;
>>         struct config *conf;
>>         int needs_reinit = 0;
>> +       int oldstate;
>>
>>         switch ((rc = change_foreign(uev->udev))) {
>>         case FOREIGN_OK:
>> @@ -1366,9 +1367,22 @@ uev_update_path (struct uevent *uev, struct
>> vectors * vecs)
>>                         pp->udev = udev_device_ref(uev->udev);
>>                         conf = get_multipath_config();
>>                         pthread_cleanup_push(put_multipath_config,
>> conf);
>> +                       oldstate = pp->state;
>> +
>>                         if (pathinfo(pp, conf, DI_SYSFS|DI_NOIO) !=
>> PATHINFO_OK)
>>                                 condlog(1, "%s: pathinfo failed after
>> change uevent",
>>                                         uev->kernel);
>> +
>> +                       if (pp->state != oldstate && (pp->state ==
>> PATH_UP || pp->state == PATH_GHOST)) {
> 
> 
> I don't understand. pathinfo(DI_SYSFS|DI_NOIO) doesn't modify 
> pp->state.  So your first condition should always be false.
> Am I overlooking something?
> 
> Regards
> Martin
> 
> 
> .
>