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