[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCHv2 1/2] openvz: convert popen to virCommand



popen must be matched with pclose (not fclose), or it will leak
resources.  Furthermore, it is a lousy interface when it comes to
signal handling.  We're much better off using our decent command
wrapper.  Note that virCommand guarantees that VIR_FREE(outbuf) is
both required and safe to call, whether virCommandRun succeeded or
failed.

* src/openvz/openvz_conf.c (openvzLoadDomains, openvzGetVEID):
Replace popen with virCommand usage.
---

v2: avoid memory leak on failure in openvzGetVEID

This patch is awaiting review on these prerequisites:
https://www.redhat.com/archives/libvir-list/2010-December/msg00318.html
https://www.redhat.com/archives/libvir-list/2010-December/msg00321.html

 src/openvz/openvz_conf.c |   55 ++++++++++++++++++++++++---------------------
 1 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index 863af93..800c1f4 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -51,6 +51,7 @@
 #include "util.h"
 #include "nodeinfo.h"
 #include "files.h"
+#include "command.h"

 #define VIR_FROM_THIS VIR_FROM_OPENVZ

@@ -433,26 +434,26 @@ openvzFreeDriver(struct openvz_driver *driver)


 int openvzLoadDomains(struct openvz_driver *driver) {
-    FILE *fp;
     int veid, ret;
     char status[16];
     char uuidstr[VIR_UUID_STRING_BUFLEN];
     virDomainObjPtr dom = NULL;
     char temp[50];
+    char *outbuf = NULL;
+    char *line;
+    virCommandPtr cmd = NULL;

     if (openvzAssignUUIDs() < 0)
         return -1;

-    if ((fp = popen(VZLIST " -a -ovpsid,status -H 2>/dev/null", "r")) == NULL) {
-        openvzError(VIR_ERR_INTERNAL_ERROR, "%s", _("popen failed"));
-        return -1;
-    }
-
-    while (!feof(fp)) {
-        if (fscanf(fp, "%d %s\n", &veid, status) != 2) {
-            if (feof(fp))
-                break;
+    cmd = virCommandNewArgList(VZLIST, "-a", "-ovpsid,status", "-H", NULL);
+    virCommandSetOutputBuffer(cmd, &outbuf);
+    if (virCommandRun(cmd, NULL) < 0)
+        goto cleanup;

+    line = *outbuf ? outbuf : NULL;
+    while (line) {
+        if (sscanf(line, "%d %s\n", &veid, status) != 2) {
             openvzError(VIR_ERR_INTERNAL_ERROR, "%s",
                         _("Failed to parse vzlist output"));
             goto cleanup;
@@ -526,9 +527,14 @@ int openvzLoadDomains(struct openvz_driver *driver) {

         virDomainObjUnlock(dom);
         dom = NULL;
+
+        line = strchr(line, '\n');
+        if (line)
+            line++;
     }

-    VIR_FORCE_FCLOSE(fp);
+    virCommandFree(cmd);
+    VIR_FREE(outbuf);

     return 0;

@@ -536,7 +542,8 @@ int openvzLoadDomains(struct openvz_driver *driver) {
     virReportOOMError();

  cleanup:
-    VIR_FORCE_FCLOSE(fp);
+    virCommandFree(cmd);
+    VIR_FREE(outbuf);
     if (dom)
         virDomainObjUnref(dom);
     return -1;
@@ -978,27 +985,23 @@ static int openvzAssignUUIDs(void)
  */

 int openvzGetVEID(const char *name) {
-    char *cmd;
+    virCommandPtr cmd;
+    char *outbuf;
     int veid;
-    FILE *fp;
     bool ok;

-    if (virAsprintf(&cmd, "%s %s -ovpsid -H", VZLIST, name) < 0) {
-        openvzError(VIR_ERR_INTERNAL_ERROR, "%s",
-                    _("virAsprintf failed"));
+    cmd = virCommandNewArgList(VZLIST, name, "-ovpsid", "-H", NULL);
+    virCommandSetOutputBuffer(cmd, &outbuf);
+    if (virCommandRun(cmd, NULL) < 0) {
+        virCommandFree(cmd);
+        VIR_FREE(outbuf);
         return -1;
     }

-    fp = popen(cmd, "r");
-    VIR_FREE(cmd);
-
-    if (fp == NULL) {
-        openvzError(VIR_ERR_INTERNAL_ERROR, "%s", _("popen failed"));
-        return -1;
-    }
+    virCommandFree(cmd);
+    ok = sscanf(outbuf, "%d\n", &veid) == 1;
+    VIR_FREE(outbuf);

-    ok = fscanf(fp, "%d\n", &veid ) == 1;
-    VIR_FORCE_FCLOSE(fp);
     if (ok && veid >= 0)
         return veid;

-- 
1.7.3.2


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]