[CRIU] [PATCH v2] util: disable log service in cr_system_userns if error fd was -1

[Stanislav Kinsburskiy]

23.04.2016 01:06, Andrew Vagin пишет:
> On Fri, Apr 22, 2016 at 07:44:24PM +0300, Stanislav Kinsburskiy wrote:
>> In case of error fd was negative, log_fd is _moved_ as STDERR for the child.
>> After move, log_fd is closed, and thus log service is broken and any error
>> prints will disappear.
> The bug is that we move log_fd into STDERR. If out is negative,
> we will move logfd into STDOUT and your patch doesn't handle this case.
>
> I think we should not move logfd anywhere. Pls, look at my patch.

I'm ok with your patch,

>> This patch disables log service, thus redirecting error output to STDERR
>> (which is log fd in this case).
>>
>> Calling of log fini is:
>> 1) to early: reopen_fd_as_nocheck will fail
>> 2) to much: close_safe will print error message, because log fd was already
>> closed
>>
>> v2:
>> log_disable helper introduced
>>
>> Signed-off-by: Stanislav Kinsburskiy <skinsbursky at virtuozzo.com>
>> ---
>>   criu/include/log.h |    1 +
>>   criu/log.c         |    5 +++++
>>   criu/util.c        |   17 ++++++++++++++---
>>   3 files changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/criu/include/log.h b/criu/include/log.h
>> index fe53a7c..7ecc3fe 100644
>> --- a/criu/include/log.h
>> +++ b/criu/include/log.h
>> @@ -6,6 +6,7 @@
>>   #include "criu-log.h"
>>   
>>   extern int log_init(const char *output);
>> +extern void log_disable(void);
>>   extern void log_fini(void);
>>   extern int log_init_by_pid(void);
>>   extern void log_closedir(void);
>> diff --git a/criu/log.c b/criu/log.c
>> index 6c5d807..1992e3a 100644
>> --- a/criu/log.c
>> +++ b/criu/log.c
>> @@ -134,6 +134,11 @@ int log_init_by_pid(void)
>>   	return log_init(path);
>>   }
>>   
>> +void log_disable(void)
>> +{
>> +	disable_service_fd(LOG_FD_OFF);
>> +}
>> +
>>   void log_fini(void)
>>   {
>>   	close_service_fd(LOG_FD_OFF);
>> diff --git a/criu/util.c b/criu/util.c
>> index dae6031..216efb6 100644
>> --- a/criu/util.c
>> +++ b/criu/util.c
>> @@ -451,6 +451,17 @@ int criu_get_image_dir(void)
>>   	return get_service_fd(IMG_FD_OFF);
>>   }
>>   
>> +int disable_service_fd(enum sfd_type type)
>> +{
>> +	int fd;
>> +
>> +	fd = get_service_fd(type);
>> +	if (fd < 0)
>> +		return 0;
>> +
>> +	clear_bit(type, sfd_map);
>> +	return 0;
>> +}
>>   int close_service_fd(enum sfd_type type)
>>   {
>>   	int fd;
>> @@ -641,9 +652,6 @@ int cr_system_userns(int in, int out, int err, char *cmd,
>>   		    move_img_fd(&err, STDIN_FILENO))
>>   			goto out_chld;
>>   
>> -		if (stop_log_fd)
>> -			log_fini();
>> -
>>   		if (in < 0) {
>>   			close(STDIN_FILENO);
>>   		} else {
>> @@ -660,6 +668,9 @@ int cr_system_userns(int in, int out, int err, char *cmd,
>>   		if (reopen_fd_as_nocheck(STDERR_FILENO, err))
>>   			goto out_chld;
>>   
>> +		if (stop_log_fd)
>> +			log_disable();
>> +
>>   		execvp(cmd, argv);
>>   
>>   		pr_perror("exec failed");
>>
>> _______________________________________________
>> CRIU mailing list
>> CRIU at openvz.org
>> https://lists.openvz.org/mailman/listinfo/criu