[CRIU] [PATCH v7 01/15] sysctl: add CTL_FLAGS_HAS to mark successful sysctl_op request

[Pavel Tikhomirov]

On 05/06/2016 02:47 PM, Pavel Emelyanov wrote:
> On 04/28/2016 07:38 PM, Pavel Tikhomirov wrote:
>> v4: replace separate has pointer to CTL_FLAGS_HAS flag, second part in
>> patch "net/ipv4: add net_conf_op to reuse for ipv6"
>> v6: define CTL_FLAGS_HAS
>> v7: also allow EIO on do_sysctl_op for optional sysctls like
>> stable_secret and fix sysctl file to close in error path
>
> Wait a second. If we've managed to open a sysctl file, but failed to write into
> it, why should we pretend as if there was not this sysctl at all?

Now we only set CTL_FLAGS_OPTIONAL for op == CTL_READ. So it meant to 
be: if we opened sysctl but failed to read it, it is in 
not-yet-initialized state and we can skip it safely. Will adding an 
implicit op-check here be satisfactory?

>
>>
>> https://jira.sw.ru/browse/PSBM-30942
>>
>> Signed-off-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
>> ---
>>  criu/include/sysctl.h |  1 +
>>  criu/sysctl.c         | 21 +++++++++++++++++----
>>  2 files changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/criu/include/sysctl.h b/criu/include/sysctl.h
>> index b949a40..3611ef1 100644
>> --- a/criu/include/sysctl.h
>> +++ b/criu/include/sysctl.h
>> @@ -35,5 +35,6 @@ enum {
>>   * Some entries might be missing mark them as optional.
>>   */
>>  #define CTL_FLAGS_OPTIONAL	1
>> +#define CTL_FLAGS_HAS		2
>>
>>  #endif /* __CR_SYSCTL_H__ */
>> diff --git a/criu/sysctl.c b/criu/sysctl.c
>> index 21ae4ce..60e52db 100644
>> --- a/criu/sysctl.c
>> +++ b/criu/sysctl.c
>> @@ -303,8 +303,13 @@ static int __userns_sysctl_op(void *arg, int proc_fd, pid_t pid)
>>  		close(nsfd);
>>
>>  		for (i = 0; i < userns_req->nr_req; i++) {
>> -			if (do_sysctl_op(fds[i], reqs[i], op) < 0)
>> -				exit(1);
>> +			if (do_sysctl_op(fds[i], reqs[i], op) < 0) {
>> +				if (errno != EIO || !(req->flags & CTL_FLAGS_OPTIONAL))
>> +					exit(1);
>> +			} else {
>> +				/* mark sysctl in question exists */
>> +				req->flags |= CTL_FLAGS_HAS;
>> +			}
>>  		}
>>
>>  		exit(0);
>> @@ -372,8 +377,16 @@ static int __nonuserns_sysctl_op(struct sysctl_req *req, size_t nr_req, int op)
>>  		}
>>
>>  		ret = do_sysctl_op(fd, req, op);
>> -		if (ret)
>> -			goto out;
>> +		if (ret) {
>> +			if (errno != EIO || !(req->flags & CTL_FLAGS_OPTIONAL)) {
>> +				close(fd);
>> +				goto out;
>> +			}
>> +		} else {
>> +			/* mark sysctl in question exists */
>> +			req->flags |= CTL_FLAGS_HAS;
>> +		}
>> +
>>  		close(fd);
>>  		req++;
>>  	}
>>
>