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

Re: [libvirt] [PATCH] SetAutostart and GetAutostart in openvz driver




I realize you are just following the existing pattern used in OpenVZ
driver, but this piece of code is horrible. sprintf'ing into a string, then parsing that string and turning it back into a list of argv[] strings, with no escaping of special characters,
or quoting. eg if the vm name had a space in it it'll mis-parse it.
In fact, names of VMs are always numbers in OpenVZ.

Just declare the command argv straight into a char*[], eg

  const char *prog[] = {
     VZCTL,
     "set",
     vm->vmdef->name,
     "--onboot",
     autostart ? "yes" : "no",
     "--save"
  }
  ret = virExec(conn, prog, &pid, -1, &outfd, &errfd)

See the storage_backend_logical.c file for examples of this kind of approach

We should put other uses of convCmdbufExec() on the TODO list for removal
in the future.
Ok. Moreover, many code is needed to rewrite.

Daniel, thanks for review!

fixed patch is attached.
Index: src/openvz_conf.c
===================================================================
RCS file: /data/cvs/libvirt/src/openvz_conf.c,v
retrieving revision 1.26
diff -u -p -r1.26 openvz_conf.c
--- src/openvz_conf.c	9 Jul 2008 11:55:27 -0000	1.26
+++ src/openvz_conf.c	9 Jul 2008 14:23:18 -0000
@@ -603,6 +603,63 @@ error:
     return NULL;
 }
 
+/*
+* Read parameter from container config
+* sample: 133, "OSTEMPLATE", value, 1024
+* return: -1 - error
+*	   0 - don't found
+*          1 - OK
+*/
+int
+openvzReadConfigParam(int vpsid ,const char * param, char *value, int maxlen)
+{
+    char conf_file[PATH_MAX] ;
+    char line[PATH_MAX] ;
+    int ret, found = 0;
+    char * conf_dir;
+    int fd ;
+    char * sf, * token;
+    char *saveptr = NULL;
+
+        
+    conf_dir = openvzLocateConfDir();
+    if (conf_dir == NULL)
+        return -1;
+
+    if (snprintf(conf_file, PATH_MAX,"%s/%d.conf",conf_dir,vpsid) >= PATH_MAX) 
+        return -1;
+
+    VIR_FREE(conf_dir);
+
+    value[0] = 0;
+
+    fd = open(conf_file, O_RDONLY);
+    if (fd == -1) 
+        return -1;
+
+    while(1) {
+        ret = openvz_readline(fd, line, sizeof(line));
+        if(ret <= 0)
+            break;
+        saveptr = NULL;
+        if (STREQLEN(line, param, strlen(param))) { 
+            sf = line;
+            sf += strlen(param);
+            if (sf[0] == '=' && (token = strtok_r(sf,"\"\t=\n", &saveptr)) != NULL) {
+                strncpy(value, token, maxlen) ;
+                value[maxlen-1] = '\0';
+                found = 1;
+            }
+	}
+    }
+    close(fd);
+
+    if (ret == 0 && found)
+        ret = 1;
+
+    return ret ;
+}
+
 static char
 *openvzLocateConfDir(void)
 {
@@ -680,6 +737,8 @@ openvzGetVPSUUID(int vpsid, char *uuidst
             break;
         }
     }
+    close(fd);
+
     return 0;
 }
 
Index: src/openvz_conf.h
===================================================================
RCS file: /data/cvs/libvirt/src/openvz_conf.h,v
retrieving revision 1.7
diff -u -p -r1.7 openvz_conf.h
--- src/openvz_conf.h	9 Jul 2008 11:55:27 -0000	1.7
+++ src/openvz_conf.h	9 Jul 2008 14:23:18 -0000
@@ -112,6 +112,7 @@ openvzIsActiveVM(struct openvz_vm *vm)
 
 void error (virConnectPtr conn, virErrorNumber code, const char *fmt, ...);
 int openvz_readline(int fd, char *ptr, int maxlen);
+int openvzReadConfigParam(int vpsid ,const char * param, char *value, int maxlen);
 struct openvz_vm *openvzFindVMByID(const struct openvz_driver *driver, int id);
 struct openvz_vm *openvzFindVMByUUID(const struct openvz_driver *driver,
                                             const unsigned char *uuid);
Index: src/openvz_driver.c
===================================================================
RCS file: /data/cvs/libvirt/src/openvz_driver.c,v
retrieving revision 1.24
diff -u -p -r1.24 openvz_driver.c
--- src/openvz_driver.c	9 Jul 2008 11:55:27 -0000	1.24
+++ src/openvz_driver.c	9 Jul 2008 14:23:18 -0000
@@ -547,6 +547,54 @@ bail_out5:
     return ret;
 }
 
+static int
+openvzDomainSetAutostart(virDomainPtr dom, int autostart)
+{
+    virConnectPtr conn= dom->conn;
+    struct openvz_driver *driver = (struct openvz_driver *) conn->privateData;
+    struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid);
+    const char *prog[] = { VZCTL, "set", vm->vmdef->name, 
+                           "--onboot", autostart ? "yes" : "no", 
+                           "--save", NULL };
+
+    if (!vm) {
+        error(conn, VIR_ERR_INVALID_DOMAIN, _("no domain with matching uuid"));
+        return -1;
+    }
+
+    if (virRun(conn, (char **)prog, NULL) < 0) {
+        error(conn, VIR_ERR_INTERNAL_ERROR, _("Could not exec %s"), VZCTL);
+        return -1;
+    }
+
+    return 0;
+}
+
+static int
+openvzDomainGetAutostart(virDomainPtr dom, int *autostart)
+{
+    virConnectPtr conn= dom->conn;
+    struct openvz_driver *driver = (struct openvz_driver *) conn->privateData;
+    struct openvz_vm *vm = openvzFindVMByUUID(driver, dom->uuid);
+    char value[1024];
+
+    if (!vm) {
+        error(conn, VIR_ERR_INVALID_DOMAIN, _("no domain with matching uuid"));
+        return -1;
+    }
+
+    if (openvzReadConfigParam(vm->vpsid , "ONBOOT", value, sizeof(value)) < 0) {
+        error(conn, VIR_ERR_INTERNAL_ERROR, _("Cound not read container config"));
+        return -1;
+    }
+
+    *autostart = 0;
+    if (STREQ(value,"yes"))
+        *autostart = 1; 
+
+    return 0;
+}
+
 static const char *openvzProbe(void)
 {
 #ifdef __linux__
@@ -748,8 +796,8 @@ static virDriver openvzDriver = {
     openvzDomainUndefine, /* domainUndefine */
     NULL, /* domainAttachDevice */
     NULL, /* domainDetachDevice */
-    NULL, /* domainGetAutostart */
-    NULL, /* domainSetAutostart */
+    openvzDomainGetAutostart, /* domainGetAutostart */
+    openvzDomainSetAutostart, /* domainSetAutostart */
     NULL, /* domainGetSchedulerType */
     NULL, /* domainGetSchedulerParameters */
     NULL, /* domainSetSchedulerParameters */

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