Re: [Patch 0/2] Checkpoint and restart of file locks

[Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>]

On 01/11/2011 07:17 PM, Sukadev Bhattiprolu wrote:
> Oren Laadan [orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org] wrote:
> | 
> | The following two patches add c/r support for POSIX file locks. They
> | are based on the series posted by Suka some time ago:
> | https://lists.linux-foundation.org/pipermail/containers/2010-October/025855.html
> | 
> | Suka:
> | 
> | When I read your post I thought that boht posix and flock were   
> | supported, but looking at the code I saw only posix. Am I missing
> | anything ?
> 
> Hmm. I have been working with only the posix locks for starters since
> it checkpoints/restarts the more general, 'struct file_lock' object.
> 
> But I should make that explicit. If a patch description implies that
> it covers flocks can be checkpointed, please let me know and I will
> fix it.
> 

Ok, just checking - I already modified the patch description
and the inline comments.

> C/R of leases/file-owner info is blocked on C/R of 'struct pids'.

It's in the working ...

> 
> | 
> | Also, I'm not sure that all the input is well sanitized during
> | restart - for example, the possible values for the lock flags.
> | Any thoughts ?
> 
> I have this check in restore_file_locks():
> 
> +               ret = -EBADF;
> +               if (h->fl_flags & FL_POSIX)
> +                       ret = restore_one_posix_lock(ctx, file, fd, 

In this specific example, I fixed it to ensure that only one of
FL_POSIX, FL_FLOCK and FL_LEASE is is present.

(Actually, now I see that I used fl_type, and I should fix it to
be fl_flags, but the intent was good ...)

> 
> The other fields (lock type for instance) will be validated by flock64_set()
> as for a new fcntl() call - no ?

That's what I was asking :o

> 
> | 
> | Lastly, do you have test cases to run against this patch and test
> | it ?
> 
> Yes, I added some tests to cr-tests a while ago and then updated them.
> Will find the updates and post them.

Thanks,

Oren.