Browse Source

util: add qemu_write_pidfile()

There are variants of qemu_create_pidfile() in qemu-pr-helper and
qemu-ga. Let's have a common implementation in libqemuutil.

The code is initially based from pr-helper write_pidfile(), with
various improvements and suggestions from Daniel Berrangé:

  QEMU will leave the pidfile existing on disk when it exits which
  initially made me think it avoids the deletion race. The app
  managing QEMU, however, may well delete the pidfile after it has
  seen QEMU exit, and even if the app locks the pidfile before
  deleting it, there is still a race.

  eg consider the following sequence

        QEMU 1        libvirtd        QEMU 2

  1.    lock(pidfile)

  2.    exit()

  3.                 open(pidfile)

  4.                 lock(pidfile)

  5.                                  open(pidfile)

  6.                 unlink(pidfile)

  7.                 close(pidfile)

  8.                                  lock(pidfile)

  IOW, at step 8 the new QEMU has successfully acquired the lock, but
  the pidfile no longer exists on disk because it was deleted after
  the original QEMU exited.

  While we could just say no external app should ever delete the
  pidfile, I don't think that is satisfactory as people don't read
  docs, and admins don't like stale pidfiles being left around on
  disk.

  To make this robust, I think we might want to copy libvirt's
  approach to pidfile acquisition which runs in a loop and checks that
  the file on disk /after/ acquiring the lock matches the file that
  was locked. Then we could in fact safely let QEMU delete its own
  pidfiles on clean exit..

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <2018083114.14736-2-marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
tags/v3.1.0-rc0
Marc-André Lureau 9 months ago
parent
commit
9e6bdef224
8 changed files with 114 additions and 131 deletions
  1. 2
    1
      include/qemu/osdep.h
  2. 0
    24
      os-posix.c
  3. 0
    25
      os-win32.c
  4. 10
    44
      qga/main.c
  5. 5
    35
      scsi/qemu-pr-helper.c
  6. 68
    0
      util/oslib-posix.c
  7. 27
    0
      util/oslib-win32.c
  8. 2
    2
      vl.c

+ 2
- 1
include/qemu/osdep.h View File

@@ -448,7 +448,8 @@ bool qemu_has_ofd_lock(void);
448 448
 #define FMT_pid "%d"
449 449
 #endif
450 450
 
451
-int qemu_create_pidfile(const char *filename);
451
+bool qemu_write_pidfile(const char *pidfile, Error **errp);
452
+
452 453
 int qemu_get_thread_id(void);
453 454
 
454 455
 #ifndef CONFIG_IOVEC

+ 0
- 24
os-posix.c View File

@@ -344,30 +344,6 @@ void os_set_line_buffering(void)
344 344
     setvbuf(stdout, NULL, _IOLBF, 0);
345 345
 }
346 346
 
347
-int qemu_create_pidfile(const char *filename)
348
-{
349
-    char buffer[128];
350
-    int len;
351
-    int fd;
352
-
353
-    fd = qemu_open(filename, O_RDWR | O_CREAT, 0600);
354
-    if (fd == -1) {
355
-        return -1;
356
-    }
357
-    if (lockf(fd, F_TLOCK, 0) == -1) {
358
-        close(fd);
359
-        return -1;
360
-    }
361
-    len = snprintf(buffer, sizeof(buffer), FMT_pid "\n", getpid());
362
-    if (write(fd, buffer, len) != len) {
363
-        close(fd);
364
-        return -1;
365
-    }
366
-
367
-    /* keep pidfile open & locked forever */
368
-    return 0;
369
-}
370
-
371 347
 bool is_daemonized(void)
372 348
 {
373 349
     return daemonize;

+ 0
- 25
os-win32.c View File

@@ -97,28 +97,3 @@ int os_parse_cmd_args(int index, const char *optarg)
97 97
 {
98 98
     return -1;
99 99
 }
100
-
101
-int qemu_create_pidfile(const char *filename)
102
-{
103
-    char buffer[128];
104
-    int len;
105
-    HANDLE file;
106
-    OVERLAPPED overlap;
107
-    BOOL ret;
108
-    memset(&overlap, 0, sizeof(overlap));
109
-
110
-    file = CreateFile(filename, GENERIC_WRITE, FILE_SHARE_READ, NULL,
111
-                      OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
112
-
113
-    if (file == INVALID_HANDLE_VALUE) {
114
-        return -1;
115
-    }
116
-    len = snprintf(buffer, sizeof(buffer), "%d\n", getpid());
117
-    ret = WriteFile(file, (LPCVOID)buffer, (DWORD)len,
118
-                    NULL, &overlap);
119
-    CloseHandle(file);
120
-    if (ret == 0) {
121
-        return -1;
122
-    }
123
-    return 0;
124
-}

+ 10
- 44
qga/main.c View File

@@ -340,46 +340,6 @@ static FILE *ga_open_logfile(const char *logfile)
340 340
     return f;
341 341
 }
342 342
 
343
-#ifndef _WIN32
344
-static bool ga_open_pidfile(const char *pidfile)
345
-{
346
-    int pidfd;
347
-    char pidstr[32];
348
-
349
-    pidfd = qemu_open(pidfile, O_CREAT|O_WRONLY, S_IRUSR|S_IWUSR);
350
-    if (pidfd == -1 || lockf(pidfd, F_TLOCK, 0)) {
351
-        g_critical("Cannot lock pid file, %s", strerror(errno));
352
-        if (pidfd != -1) {
353
-            close(pidfd);
354
-        }
355
-        return false;
356
-    }
357
-
358
-    if (ftruncate(pidfd, 0)) {
359
-        g_critical("Failed to truncate pid file");
360
-        goto fail;
361
-    }
362
-    snprintf(pidstr, sizeof(pidstr), "%d\n", getpid());
363
-    if (write(pidfd, pidstr, strlen(pidstr)) != strlen(pidstr)) {
364
-        g_critical("Failed to write pid file");
365
-        goto fail;
366
-    }
367
-
368
-    /* keep pidfile open & locked forever */
369
-    return true;
370
-
371
-fail:
372
-    unlink(pidfile);
373
-    close(pidfd);
374
-    return false;
375
-}
376
-#else /* _WIN32 */
377
-static bool ga_open_pidfile(const char *pidfile)
378
-{
379
-    return true;
380
-}
381
-#endif
382
-
383 343
 static gint ga_strcmp(gconstpointer str1, gconstpointer str2)
384 344
 {
385 345
     return strcmp(str1, str2);
@@ -479,8 +439,11 @@ void ga_unset_frozen(GAState *s)
479 439
     ga_enable_logging(s);
480 440
     g_warning("logging re-enabled due to filesystem unfreeze");
481 441
     if (s->deferred_options.pid_filepath) {
482
-        if (!ga_open_pidfile(s->deferred_options.pid_filepath)) {
483
-            g_warning("failed to create/open pid file");
442
+        Error *err = NULL;
443
+
444
+        if (!qemu_write_pidfile(s->deferred_options.pid_filepath, &err)) {
445
+            g_warning("%s", error_get_pretty(err));
446
+            error_free(err);
484 447
         }
485 448
         s->deferred_options.pid_filepath = NULL;
486 449
     }
@@ -515,8 +478,11 @@ static void become_daemon(const char *pidfile)
515 478
     }
516 479
 
517 480
     if (pidfile) {
518
-        if (!ga_open_pidfile(pidfile)) {
519
-            g_critical("failed to create pidfile");
481
+        Error *err = NULL;
482
+
483
+        if (!qemu_write_pidfile(pidfile, &err)) {
484
+            g_critical("%s", error_get_pretty(err));
485
+            error_free(err);
520 486
             exit(EXIT_FAILURE);
521 487
         }
522 488
     }

+ 5
- 35
scsi/qemu-pr-helper.c View File

@@ -117,39 +117,6 @@ QEMU_COPYRIGHT "\n"
117 117
     , name);
118 118
 }
119 119
 
120
-static void write_pidfile(void)
121
-{
122
-    int pidfd;
123
-    char pidstr[32];
124
-
125
-    pidfd = qemu_open(pidfile, O_CREAT|O_WRONLY, S_IRUSR|S_IWUSR);
126
-    if (pidfd == -1) {
127
-        error_report("Cannot open pid file, %s", strerror(errno));
128
-        exit(EXIT_FAILURE);
129
-    }
130
-
131
-    if (lockf(pidfd, F_TLOCK, 0)) {
132
-        error_report("Cannot lock pid file, %s", strerror(errno));
133
-        goto fail;
134
-    }
135
-    if (ftruncate(pidfd, 0)) {
136
-        error_report("Failed to truncate pid file");
137
-        goto fail;
138
-    }
139
-
140
-    snprintf(pidstr, sizeof(pidstr), "%d\n", getpid());
141
-    if (write(pidfd, pidstr, strlen(pidstr)) != strlen(pidstr)) {
142
-        error_report("Failed to write pid file");
143
-        goto fail;
144
-    }
145
-    return;
146
-
147
-fail:
148
-    unlink(pidfile);
149
-    close(pidfd);
150
-    exit(EXIT_FAILURE);
151
-}
152
-
153 120
 /* SG_IO support */
154 121
 
155 122
 typedef struct PRHelperSGIOData {
@@ -1080,8 +1047,11 @@ int main(int argc, char **argv)
1080 1047
         }
1081 1048
     }
1082 1049
 
1083
-    if (daemonize || pidfile_specified)
1084
-        write_pidfile();
1050
+    if ((daemonize || pidfile_specified) &&
1051
+        !qemu_write_pidfile(pidfile, &local_err)) {
1052
+        error_report_err(local_err);
1053
+        exit(EXIT_FAILURE);
1054
+    }
1085 1055
 
1086 1056
 #ifdef CONFIG_LIBCAP
1087 1057
     if (drop_privileges() < 0) {

+ 68
- 0
util/oslib-posix.c View File

@@ -88,6 +88,74 @@ int qemu_daemon(int nochdir, int noclose)
88 88
     return daemon(nochdir, noclose);
89 89
 }
90 90
 
91
+bool qemu_write_pidfile(const char *path, Error **errp)
92
+{
93
+    int fd;
94
+    char pidstr[32];
95
+
96
+    while (1) {
97
+        struct stat a, b;
98
+
99
+        fd = qemu_open(path, O_CREAT | O_WRONLY, S_IRUSR | S_IWUSR);
100
+        if (fd == -1) {
101
+            error_setg_errno(errp, errno, "Cannot open pid file");
102
+            return false;
103
+        }
104
+
105
+        if (fstat(fd, &b) < 0) {
106
+            error_setg_errno(errp, errno, "Cannot stat file");
107
+            goto fail_close;
108
+        }
109
+
110
+        if (lockf(fd, F_TLOCK, 0) < 0) {
111
+            error_setg_errno(errp, errno, "Cannot lock pid file");
112
+            goto fail_close;
113
+        }
114
+
115
+        /*
116
+         * Now make sure the path we locked is the same one that now
117
+         * exists on the filesystem.
118
+         */
119
+        if (stat(path, &a) < 0) {
120
+            /*
121
+             * PID file disappeared, someone else must be racing with
122
+             * us, so try again.
123
+             */
124
+            close(fd);
125
+            continue;
126
+        }
127
+
128
+        if (a.st_ino == b.st_ino) {
129
+            break;
130
+        }
131
+
132
+        /*
133
+         * PID file was recreated, someone else must be racing with
134
+         * us, so try again.
135
+         */
136
+        close(fd);
137
+    }
138
+
139
+    if (ftruncate(fd, 0) < 0) {
140
+        error_setg_errno(errp, errno, "Failed to truncate pid file");
141
+        goto fail_unlink;
142
+    }
143
+
144
+    snprintf(pidstr, sizeof(pidstr), FMT_pid "\n", getpid());
145
+    if (write(fd, pidstr, strlen(pidstr)) != strlen(pidstr)) {
146
+        error_setg(errp, "Failed to write pid file");
147
+        goto fail_unlink;
148
+    }
149
+
150
+    return true;
151
+
152
+fail_unlink:
153
+    unlink(path);
154
+fail_close:
155
+    close(fd);
156
+    return false;
157
+}
158
+
91 159
 void *qemu_oom_check(void *ptr)
92 160
 {
93 161
     if (ptr == NULL) {

+ 27
- 0
util/oslib-win32.c View File

@@ -776,3 +776,30 @@ ssize_t qemu_recvfrom_wrap(int sockfd, void *buf, size_t len, int flags,
776 776
     }
777 777
     return ret;
778 778
 }
779
+
780
+bool qemu_write_pidfile(const char *filename, Error **errp)
781
+{
782
+    char buffer[128];
783
+    int len;
784
+    HANDLE file;
785
+    OVERLAPPED overlap;
786
+    BOOL ret;
787
+    memset(&overlap, 0, sizeof(overlap));
788
+
789
+    file = CreateFile(filename, GENERIC_WRITE, FILE_SHARE_READ, NULL,
790
+                      OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
791
+
792
+    if (file == INVALID_HANDLE_VALUE) {
793
+        error_setg(errp, "Failed to create PID file");
794
+        return false;
795
+    }
796
+    len = snprintf(buffer, sizeof(buffer), FMT_pid "\n", (pid_t)getpid());
797
+    ret = WriteFile(file, (LPCVOID)buffer, (DWORD)len,
798
+                    NULL, &overlap);
799
+    CloseHandle(file);
800
+    if (ret == 0) {
801
+        error_setg(errp, "Failed to write PID file");
802
+        return false;
803
+    }
804
+    return true;
805
+}

+ 2
- 2
vl.c View File

@@ -3906,8 +3906,8 @@ int main(int argc, char **argv, char **envp)
3906 3906
     os_daemonize();
3907 3907
     rcu_disable_atfork();
3908 3908
 
3909
-    if (pid_file && qemu_create_pidfile(pid_file) != 0) {
3910
-        error_report("could not acquire pid file: %s", strerror(errno));
3909
+    if (pid_file && !qemu_write_pidfile(pid_file, &err)) {
3910
+        error_reportf_err(err, "cannot create PID file: ");
3911 3911
         exit(1);
3912 3912
     }
3913 3913
 

Loading…
Cancel
Save