linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] nfs-blkmapd: PID file read by systemd failed
@ 2022-10-18  3:05 zhanchengbin
  2022-10-21  3:13 ` zhanchengbin
  2022-10-24 17:25 ` Steve Dickson
  0 siblings, 2 replies; 4+ messages in thread
From: zhanchengbin @ 2022-10-18  3:05 UTC (permalink / raw)
  To: steved; +Cc: linux-nfs, liuzhiqiang26, linfeilong

When started nfs-blkmap.service, the PID file can't be opened, The
cause is that the child process does not create the PID file before
the systemd reads the PID file.
Adding "ExecStartPost=/bin/sleep 0.1" to
/usr/lib/systemd/system/nfs-blkmap.service will probably solve this
problem, However, there is no guarantee that the above solutions are
effective under high cpu pressure.So replace the daemon function with
the fork function, and put the behavior of creating the PID file in
the parent process to solve the above problems.

Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com>
Reviewed-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
---
V1->V2:
  2.27.0 -> 2.33.0

  utils/blkmapd/device-discovery.c | 48 +++++++++++++++++++++-----------
  1 file changed, 32 insertions(+), 16 deletions(-)

diff --git a/utils/blkmapd/device-discovery.c 
b/utils/blkmapd/device-discovery.c
index 49935c2e..dcced5c3 100644
--- a/utils/blkmapd/device-discovery.c
+++ b/utils/blkmapd/device-discovery.c
@@ -507,28 +507,44 @@ int main(int argc, char **argv)
  	if (fg) {
  		openlog("blkmapd", LOG_PERROR, 0);
  	} else {
-		if (daemon(0, 0) != 0) {
-			fprintf(stderr, "Daemonize failed\n");
+		pid_t pid = fork();
+		if (pid < 0) {
+			fprintf(stderr, "fork error\n");
  			exit(1);
+		} else if (pid != 0) {
+			pidfd = open(PID_FILE, O_WRONLY | O_CREAT, 0644);
+			if (pidfd < 0) {
+				fprintf(stderr, "Create pid file %s failed\n", PID_FILE);
+				exit(1);
+			}
+
+			if (lockf(pidfd, F_TLOCK, 0) < 0) {
+				fprintf(stderr, "Already running; Exiting!");
+				close(pidfd);
+				exit(1);
+			}
+			if (ftruncate(pidfd, 0) < 0)
+				fprintf(stderr, "ftruncate on %s failed: m\n", PID_FILE);
+			sprintf(pidbuf, "%d\n", pid);
+			if (write(pidfd, pidbuf, strlen(pidbuf)) != (ssize_t)strlen(pidbuf))
+				fprintf(stderr, "write on %s failed: m\n", PID_FILE);
+			exit(0);
  		}

-		openlog("blkmapd", LOG_PID, 0);
-		pidfd = open(PID_FILE, O_WRONLY | O_CREAT, 0644);
-		if (pidfd < 0) {
-			BL_LOG_ERR("Create pid file %s failed\n", PID_FILE);
-			exit(1);
+		(void)setsid();
+		if (chdir("/")) {
+			fprintf(stderr, "chdir error\n");
  		}
+		int fd = open("/dev/null", O_RDWR, 0);
+		if (fd >= 0) {
+		    (void)dup2(fd, STDIN_FILENO);
+		    (void)dup2(fd, STDOUT_FILENO);
+		    (void)dup2(fd, STDERR_FILENO);

-		if (lockf(pidfd, F_TLOCK, 0) < 0) {
-			BL_LOG_ERR("Already running; Exiting!");
-			close(pidfd);
-			exit(1);
+		    (void)close(fd);
  		}
-		if (ftruncate(pidfd, 0) < 0)
-			BL_LOG_WARNING("ftruncate on %s failed: m\n", PID_FILE);
-		sprintf(pidbuf, "%d\n", getpid());
-		if (write(pidfd, pidbuf, strlen(pidbuf)) != (ssize_t)strlen(pidbuf))
-			BL_LOG_WARNING("write on %s failed: m\n", PID_FILE);
+
+		openlog("blkmapd", LOG_PID, 0);
  	}

  	signal(SIGINT, sig_die);
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] nfs-blkmapd: PID file read by systemd failed
  2022-10-18  3:05 [PATCH v2] nfs-blkmapd: PID file read by systemd failed zhanchengbin
@ 2022-10-21  3:13 ` zhanchengbin
  2022-10-24 17:25 ` Steve Dickson
  1 sibling, 0 replies; 4+ messages in thread
From: zhanchengbin @ 2022-10-21  3:13 UTC (permalink / raw)
  To: steved; +Cc: linux-nfs, liuzhiqiang26, linfeilong

ping...

On 2022/10/18 11:05, zhanchengbin wrote:
> When started nfs-blkmap.service, the PID file can't be opened, The
> cause is that the child process does not create the PID file before
> the systemd reads the PID file.
> Adding "ExecStartPost=/bin/sleep 0.1" to
> /usr/lib/systemd/system/nfs-blkmap.service will probably solve this
> problem, However, there is no guarantee that the above solutions are
> effective under high cpu pressure.So replace the daemon function with
> the fork function, and put the behavior of creating the PID file in
> the parent process to solve the above problems.
> 
> Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com>
> Reviewed-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
> ---
> V1->V2:
>   2.27.0 -> 2.33.0
> 
>   utils/blkmapd/device-discovery.c | 48 +++++++++++++++++++++-----------
>   1 file changed, 32 insertions(+), 16 deletions(-)
> 
> diff --git a/utils/blkmapd/device-discovery.c 
> b/utils/blkmapd/device-discovery.c
> index 49935c2e..dcced5c3 100644
> --- a/utils/blkmapd/device-discovery.c
> +++ b/utils/blkmapd/device-discovery.c
> @@ -507,28 +507,44 @@ int main(int argc, char **argv)
>       if (fg) {
>           openlog("blkmapd", LOG_PERROR, 0);
>       } else {
> -        if (daemon(0, 0) != 0) {
> -            fprintf(stderr, "Daemonize failed\n");
> +        pid_t pid = fork();
> +        if (pid < 0) {
> +            fprintf(stderr, "fork error\n");
>               exit(1);
> +        } else if (pid != 0) {
> +            pidfd = open(PID_FILE, O_WRONLY | O_CREAT, 0644);
> +            if (pidfd < 0) {
> +                fprintf(stderr, "Create pid file %s failed\n", PID_FILE);
> +                exit(1);
> +            }
> +
> +            if (lockf(pidfd, F_TLOCK, 0) < 0) {
> +                fprintf(stderr, "Already running; Exiting!");
> +                close(pidfd);
> +                exit(1);
> +            }
> +            if (ftruncate(pidfd, 0) < 0)
> +                fprintf(stderr, "ftruncate on %s failed: m\n", PID_FILE);
> +            sprintf(pidbuf, "%d\n", pid);
> +            if (write(pidfd, pidbuf, strlen(pidbuf)) != 
> (ssize_t)strlen(pidbuf))
> +                fprintf(stderr, "write on %s failed: m\n", PID_FILE);
> +            exit(0);
>           }
> 
> -        openlog("blkmapd", LOG_PID, 0);
> -        pidfd = open(PID_FILE, O_WRONLY | O_CREAT, 0644);
> -        if (pidfd < 0) {
> -            BL_LOG_ERR("Create pid file %s failed\n", PID_FILE);
> -            exit(1);
> +        (void)setsid();
> +        if (chdir("/")) {
> +            fprintf(stderr, "chdir error\n");
>           }
> +        int fd = open("/dev/null", O_RDWR, 0);
> +        if (fd >= 0) {
> +            (void)dup2(fd, STDIN_FILENO);
> +            (void)dup2(fd, STDOUT_FILENO);
> +            (void)dup2(fd, STDERR_FILENO);
> 
> -        if (lockf(pidfd, F_TLOCK, 0) < 0) {
> -            BL_LOG_ERR("Already running; Exiting!");
> -            close(pidfd);
> -            exit(1);
> +            (void)close(fd);
>           }
> -        if (ftruncate(pidfd, 0) < 0)
> -            BL_LOG_WARNING("ftruncate on %s failed: m\n", PID_FILE);
> -        sprintf(pidbuf, "%d\n", getpid());
> -        if (write(pidfd, pidbuf, strlen(pidbuf)) != 
> (ssize_t)strlen(pidbuf))
> -            BL_LOG_WARNING("write on %s failed: m\n", PID_FILE);
> +
> +        openlog("blkmapd", LOG_PID, 0);
>       }
> 
>       signal(SIGINT, sig_die);

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] nfs-blkmapd: PID file read by systemd failed
  2022-10-18  3:05 [PATCH v2] nfs-blkmapd: PID file read by systemd failed zhanchengbin
  2022-10-21  3:13 ` zhanchengbin
@ 2022-10-24 17:25 ` Steve Dickson
  2022-10-28  3:20   ` zhanchengbin
  1 sibling, 1 reply; 4+ messages in thread
From: Steve Dickson @ 2022-10-24 17:25 UTC (permalink / raw)
  To: zhanchengbin; +Cc: linux-nfs, liuzhiqiang26, linfeilong

Hello,

This still does not apply.. at all.. but
did take a look.

On 10/17/22 11:05 PM, zhanchengbin wrote:
> When started nfs-blkmap.service, the PID file can't be opened, The
> cause is that the child process does not create the PID file before
> the systemd reads the PID file.
> Adding "ExecStartPost=/bin/sleep 0.1" to
> /usr/lib/systemd/system/nfs-blkmap.service will probably solve this
> problem, However, there is no guarantee that the above solutions are
> effective under high cpu pressure.So replace the daemon function with
> the fork function, and put the behavior of creating the PID file in
> the parent process to solve the above problems.
> 
> Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com>
> Reviewed-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
> ---
> V1->V2:
>   2.27.0 -> 2.33.0
> 
>   utils/blkmapd/device-discovery.c | 48 +++++++++++++++++++++-----------
>   1 file changed, 32 insertions(+), 16 deletions(-)
> 
> diff --git a/utils/blkmapd/device-discovery.c 
> b/utils/blkmapd/device-discovery.c
> index 49935c2e..dcced5c3 100644
> --- a/utils/blkmapd/device-discovery.c
> +++ b/utils/blkmapd/device-discovery.c
> @@ -507,28 +507,44 @@ int main(int argc, char **argv)
>       if (fg) {
>           openlog("blkmapd", LOG_PERROR, 0);
>       } else {
> -        if (daemon(0, 0) != 0) {
> -            fprintf(stderr, "Daemonize failed\n");
> +        pid_t pid = fork();
> +        if (pid < 0) {
> +            fprintf(stderr, "fork error\n");
use BL_LOG_ERR("fork error: %s\n", strerror(errno)) or
something similar which tells why the fork failed.

>               exit(1);
> +        } else if (pid != 0) {
> +            pidfd = open(PID_FILE, O_WRONLY | O_CREAT, 0644);
> +            if (pidfd < 0) {
> +                fprintf(stderr, "Create pid file %s failed\n", PID_FILE);
Same thing here... it is good you are showing the failed file
but you are not showing my it failed.

> +                exit(1);
> +            }
> +
> +            if (lockf(pidfd, F_TLOCK, 0) < 0) {
> +                fprintf(stderr, "Already running; Exiting!");
> +                close(pidfd);
> +                exit(1);
> +            }
> +            if (ftruncate(pidfd, 0) < 0)
> +                fprintf(stderr, "ftruncate on %s failed: m\n", PID_FILE);
> +            sprintf(pidbuf, "%d\n", pid);
> +            if (write(pidfd, pidbuf, strlen(pidbuf)) != 
> (ssize_t)strlen(pidbuf))
> +                fprintf(stderr, "write on %s failed: m\n", PID_FILE);
> +            exit(0);
>           }
> 
> -        openlog("blkmapd", LOG_PID, 0);
> -        pidfd = open(PID_FILE, O_WRONLY | O_CREAT, 0644);
> -        if (pidfd < 0) {
> -            BL_LOG_ERR("Create pid file %s failed\n", PID_FILE);
> -            exit(1);
> +        (void)setsid();
> +        if (chdir("/")) {
> +            fprintf(stderr, "chdir error\n");
>           }
> +        int fd = open("/dev/null", O_RDWR, 0);
> +        if (fd >= 0) {
> +            (void)dup2(fd, STDIN_FILENO);
> +            (void)dup2(fd, STDOUT_FILENO);
> +            (void)dup2(fd, STDERR_FILENO);
not clear this is necessary but it is
probably the safest since that's what
daemon() does.

steved.
> 
> -        if (lockf(pidfd, F_TLOCK, 0) < 0) {
> -            BL_LOG_ERR("Already running; Exiting!");
> -            close(pidfd);
> -            exit(1);
> +            (void)close(fd);
>           }
> -        if (ftruncate(pidfd, 0) < 0)
> -            BL_LOG_WARNING("ftruncate on %s failed: m\n", PID_FILE);
> -        sprintf(pidbuf, "%d\n", getpid());
> -        if (write(pidfd, pidbuf, strlen(pidbuf)) != 
> (ssize_t)strlen(pidbuf))
> -            BL_LOG_WARNING("write on %s failed: m\n", PID_FILE);
> +
> +        openlog("blkmapd", LOG_PID, 0);
>       }
> 
>       signal(SIGINT, sig_die);


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] nfs-blkmapd: PID file read by systemd failed
  2022-10-24 17:25 ` Steve Dickson
@ 2022-10-28  3:20   ` zhanchengbin
  0 siblings, 0 replies; 4+ messages in thread
From: zhanchengbin @ 2022-10-28  3:20 UTC (permalink / raw)
  To: Steve Dickson; +Cc: linux-nfs, liuzhiqiang26, linfeilong

On 2022/10/25 1:25, Steve Dickson wrote:
> Hello,
> 
> This still does not apply.. at all.. but
> did take a look.
> 
> On 10/17/22 11:05 PM, zhanchengbin wrote:
>> When started nfs-blkmap.service, the PID file can't be opened, The
>> cause is that the child process does not create the PID file before
>> the systemd reads the PID file.
>> Adding "ExecStartPost=/bin/sleep 0.1" to
>> /usr/lib/systemd/system/nfs-blkmap.service will probably solve this
>> problem, However, there is no guarantee that the above solutions are
>> effective under high cpu pressure.So replace the daemon function with
>> the fork function, and put the behavior of creating the PID file in
>> the parent process to solve the above problems.
>>
>> Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com>
>> Reviewed-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
>> ---
>> V1->V2:
>>   2.27.0 -> 2.33.0
>>
>>   utils/blkmapd/device-discovery.c | 48 +++++++++++++++++++++-----------
>>   1 file changed, 32 insertions(+), 16 deletions(-)
>>
>> diff --git a/utils/blkmapd/device-discovery.c 
>> b/utils/blkmapd/device-discovery.c
>> index 49935c2e..dcced5c3 100644
>> --- a/utils/blkmapd/device-discovery.c
>> +++ b/utils/blkmapd/device-discovery.c
>> @@ -507,28 +507,44 @@ int main(int argc, char **argv)
>>       if (fg) {
>>           openlog("blkmapd", LOG_PERROR, 0);
>>       } else {
>> -        if (daemon(0, 0) != 0) {
>> -            fprintf(stderr, "Daemonize failed\n");
>> +        pid_t pid = fork();
>> +        if (pid < 0) {
>> +            fprintf(stderr, "fork error\n");
> use BL_LOG_ERR("fork error: %s\n", strerror(errno)) or
> something similar which tells why the fork failed.
I'm considering if openlog will have problems after fork, I'll try to
fix this and push the patch for the next version, thx.
> 
>>               exit(1);
>> +        } else if (pid != 0) {
>> +            pidfd = open(PID_FILE, O_WRONLY | O_CREAT, 0644);
>> +            if (pidfd < 0) {
>> +                fprintf(stderr, "Create pid file %s failed\n", 
>> PID_FILE);
> Same thing here... it is good you are showing the failed file
> but you are not showing my it failed.
> 
>> +                exit(1);
>> +            }
>> +
>> +            if (lockf(pidfd, F_TLOCK, 0) < 0) {
>> +                fprintf(stderr, "Already running; Exiting!");
>> +                close(pidfd);
>> +                exit(1);
>> +            }
>> +            if (ftruncate(pidfd, 0) < 0)
>> +                fprintf(stderr, "ftruncate on %s failed: m\n", 
>> PID_FILE);
>> +            sprintf(pidbuf, "%d\n", pid);
>> +            if (write(pidfd, pidbuf, strlen(pidbuf)) != 
>> (ssize_t)strlen(pidbuf))
>> +                fprintf(stderr, "write on %s failed: m\n", PID_FILE);
>> +            exit(0);
>>           }
>>
>> -        openlog("blkmapd", LOG_PID, 0);
>> -        pidfd = open(PID_FILE, O_WRONLY | O_CREAT, 0644);
>> -        if (pidfd < 0) {
>> -            BL_LOG_ERR("Create pid file %s failed\n", PID_FILE);
>> -            exit(1);
>> +        (void)setsid();
>> +        if (chdir("/")) {
>> +            fprintf(stderr, "chdir error\n");
>>           }
>> +        int fd = open("/dev/null", O_RDWR, 0);
>> +        if (fd >= 0) {
>> +            (void)dup2(fd, STDIN_FILENO);
>> +            (void)dup2(fd, STDOUT_FILENO);
>> +            (void)dup2(fd, STDERR_FILENO);
> not clear this is necessary but it is
> probably the safest since that's what
> daemon() does.I think it's necessary,because similar processing is in the daemon
function.

  -zhanchengbin
> 
> steved.
>>
>> -        if (lockf(pidfd, F_TLOCK, 0) < 0) {
>> -            BL_LOG_ERR("Already running; Exiting!");
>> -            close(pidfd);
>> -            exit(1);
>> +            (void)close(fd);
>>           }
>> -        if (ftruncate(pidfd, 0) < 0)
>> -            BL_LOG_WARNING("ftruncate on %s failed: m\n", PID_FILE);
>> -        sprintf(pidbuf, "%d\n", getpid());
>> -        if (write(pidfd, pidbuf, strlen(pidbuf)) != 
>> (ssize_t)strlen(pidbuf))
>> -            BL_LOG_WARNING("write on %s failed: m\n", PID_FILE);
>> +
>> +        openlog("blkmapd", LOG_PID, 0);
>>       }
>>
>>       signal(SIGINT, sig_die);
> 
> .

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-10-28  3:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-18  3:05 [PATCH v2] nfs-blkmapd: PID file read by systemd failed zhanchengbin
2022-10-21  3:13 ` zhanchengbin
2022-10-24 17:25 ` Steve Dickson
2022-10-28  3:20   ` zhanchengbin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).