[libvirt] [PATCH] Adding filesystem mount support for openVZ
Hi everyone, This patch is to allow using the "mount" type in the "filesystem" tag for OpenVZ domains. Example: ... <filesystem type='mount'> <source dir='/path/to/filesystem/directory/' /> <target dir='/path/to/pivot/root/' /> </filesystem> ... This is my first patch to an external project, so don't spare me if I got things wrong :) Also, I'm curious for suggestions as to how I could allow for the target not to be specified in the XML. Because in this case OpenVZ just makes a temporary pivot root in "/var/lib/vz/root/" and that is probably sufficient for most people, who might not want to have to explicitly create a pivot root somewhere, just for mounting the filesystem while it's running. I was thinking either allow for the target tag not to be specified, or add an "auto" attribute to target. Which one sounds better ? Thanks, Florian
On Wed, Mar 11, 2009 at 05:42:20PM +0100, Florian Vichot wrote:
Hi everyone,
This patch is to allow using the "mount" type in the "filesystem" tag for OpenVZ domains.
Example: ... <filesystem type='mount'> <source dir='/path/to/filesystem/directory/' /> <target dir='/path/to/pivot/root/' /> </filesystem> ...
This is my first patch to an external project, so don't spare me if I got things wrong :)
Also, I'm curious for suggestions as to how I could allow for the target not to be specified in the XML. Because in this case OpenVZ just makes a temporary pivot root in "/var/lib/vz/root/" and that is probably sufficient for most people, who might not want to have to explicitly create a pivot root somewhere, just for mounting the filesystem while it's running.
Actually the <target dir="..."> means something a little different. This refers to the mount point *within* the container. So for the root filesystem of the container it will be <target dir='/' />. I believe OpenVZ only allows you to setup the root filesystem within the container, so effectively you'd never need worry about anything other than the one <target dir='/' />. IMHO there is no need to include the temporary pivot root location in the XML, just leave it on default. As an example, of why we have this, the LXC container driver, allows for specifying multiple filesystems for containers. So you could setup lots of containers each with their own private root fileystem, and then give them all the same /home, eg, in guest 1 you'd have <filesystem type='mount'> <source dir='/path/to/root/filesystem/guest1/' /> <target dir='/' /> </filesystem> <filesystem type='mount'> <source dir='/path/to/shared/home' /> <target dir='/home' /> </filesystem> While in guest 2 you'd have <filesystem type='mount'> <source dir='/path/to/root/filesystem/guest2/' /> <target dir='/' /> </filesystem> <filesystem type='mount'> <source dir='/path/to/shared/home' /> <target dir='/home' /> </filesystem>
diff --git a/src/openvz_conf.c b/src/openvz_conf.c index ff3d607..33fb259 100644 --- a/src/openvz_conf.c +++ b/src/openvz_conf.c @@ -314,6 +314,41 @@ error: }
+/* utility function to replace 'from' by 'to' in 'str' */ +static char* +openvz_replace(const char* str, + const char* from, + int to) { + char tmp[4096]; + char* offset = tmp; + int len = strlen(str); + int fromLen = strlen(from); + int r,i,maxcmp,left = sizeof(tmp); + + if(!from) + return NULL; + + for(i = 0; i < len && left; ++i) { + /* compare first caracters to limit useless full comparisons */ + if(*from == str[i]) { + maxcmp = (fromLen > len-i) ? len-i : fromLen; + if(strncmp(from, str+i, maxcmp) == 0) { + r = snprintf(offset, left, "%d", to); + offset += r; + i += fromLen - 1; + continue; + } + } + *offset++ = str[i]; + left = sizeof(tmp) - (offset-tmp); + } + + tmp[sizeof(tmp)-left] = '\0'; + + return strdup(tmp); +}
In this function, can you use STREQLEN(from, str+i, maxcmp) instead of strncmp == 0. Also, I think it'd be clearer to avoid using the pre-declared 'char tmp[4096]' and instead make use of our automatic grow-on-demand string buffer code. Just add #include "buf.h" Declare it with: virBuffer buf = VIR_BUFFER_INITIALIZER; And then use virBufferAdd to append to it When you're all done, use virBufferError() to check if there was an OOM condition and return an error. If OK,then use virBufferContentAndReset() to get the internal char * - no need to strdup() that - you own the pointer at that point.
@@ -343,7 +378,42 @@ openvzReadFSConf(virConnectPtr conn, goto no_memory; def->fss[def->nfss++] = fs; fs = NULL; + } else { + /* OSTEMPLATE was not found, VE was booted from a private dir directly */ + ret = openvzReadConfigParam(veid, "VE_PRIVATE", temp, sizeof(temp)); + if (ret <= 0) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("Cound not read 'VE_PRIVATE' from config for container %d"), + veid); + goto error; + } + + if (VIR_ALLOC(fs) < 0) + goto no_memory; + + fs->type = VIR_DOMAIN_FS_TYPE_MOUNT; + fs->src = openvz_replace(temp, "$VEID", veid); + + if (fs->src == NULL) + goto no_memory; + + ret = openvzReadConfigParam(veid, "VE_ROOT", temp, sizeof(temp)); + if (ret <= 0) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("Cound not read 'VE_ROOT' from config for container %d"), + veid); + goto error; + } + + fs->dst = openvz_replace(temp, "$VEID", veid);
+ if (fs->dst == NULL) + goto no_memory; + + if (VIR_REALLOC_N(def->fss, def->nfss + 1) < 0) + goto no_memory; + def->fss[def->nfss++] = fs; + fs = NULL; }
return 0; @@ -597,6 +667,81 @@ openvzReadConfigParam(int vpsid ,const char * param, char *value, int maxlen) return ret ; }
+static int +openvz_copyfile(char* from_path, char* to_path) +{ + char line[PATH_MAX]; + int fd, copy_fd; + int bytes_read; + + fd = open(from_path, O_RDONLY); + if (fd == -1) + return -1; + copy_fd = open(to_path, O_WRONLY | O_CREAT | O_TRUNC, 0644); + if (copy_fd == -1) { + close(fd); + return -1; + } + + while(1) { + if (openvz_readline(fd, line, sizeof(line)) <= 0) + break; + + bytes_read = strlen(line); + if (safewrite(copy_fd, line, bytes_read) != bytes_read) + goto error; + } + + if (close(fd) < 0) + goto error; + fd = -1; + if (close(copy_fd) < 0) + goto error; + copy_fd = -1; + + return 0; + +error: + if (fd != -1) + close(fd); + if (copy_fd != -1) + close(copy_fd); + return -1; +} + +/* +* Copy the default config to the VE conf file +* return: -1 - error +* 0 - OK +*/ +int +openvzCopyDefaultConfig(int vpsid) +{ + char * confdir; + char default_conf_file[PATH_MAX], conf_file[PATH_MAX]; + + confdir = openvzLocateConfDir(); + if (confdir == NULL) + return -1; + + if (snprintf(default_conf_file, PATH_MAX, "%s/%s", + confdir, VZ_DEFAULT_CONF_FILE) >= PATH_MAX) + { + VIR_FREE(confdir); + return -1; + }
Could you make use of virAsprintf() here instead of statically declaring a variable with PATH_MAX. I know we use PATH_MXA all over the place in libvirt already, but we need to try to get out of that habit where practical, to avoid wasting too much stack.
+ + VIR_FREE(confdir); + + if (openvzLocateConfFile(vpsid, conf_file, PATH_MAX, "conf")<0) + return -1; + + if (openvz_copyfile(default_conf_file, conf_file)<0) + return -1; + + return 0; +} + /* Locate config file of container
Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Hi Florian,
Hi everyone,
This patch is to allow using the "mount" type in the "filesystem" tag for OpenVZ domains.
Example: ... <filesystem type='mount'> <source dir='/path/to/filesystem/directory/' /> <target dir='/path/to/pivot/root/' /> </filesystem> ...
This is my first patch to an external project, so don't spare me if I got things wrong :)
Also, I'm curious for suggestions as to how I could allow for the target not to be specified in the XML. Because in this case OpenVZ just makes a temporary pivot root in "/var/lib/vz/root/" and that is probably sufficient for most people, who might not want to have to explicitly create a pivot root somewhere, just for mounting the filesystem while it's running.
I was thinking either allow for the target tag not to be specified, or add an "auto" attribute to target. Which one sounds better ?
Thanks, Florian
diff --git a/src/openvz_conf.c b/src/openvz_conf.c index ff3d607..33fb259 100644 --- a/src/openvz_conf.c +++ b/src/openvz_conf.c @@ -314,6 +314,41 @@ error: } +/* utility function to replace 'from' by 'to' in 'str' */ +static char* +openvz_replace(const char* str, + const char* from, + int to) { + char tmp[4096]; + char* offset = tmp; + int len = strlen(str); + int fromLen = strlen(from); + int r,i,maxcmp,left = sizeof(tmp); + + if(!from) + return NULL; + + for(i = 0; i < len && left; ++i) { + /* compare first caracters to limit useless full comparisons */ + if(*from == str[i]) { + maxcmp = (fromLen > len-i) ? len-i : fromLen; + if(strncmp(from, str+i, maxcmp) == 0) { + r = snprintf(offset, left, "%d", to); + offset += r; + i += fromLen - 1; + continue; + } + } + *offset++ = str[i]; + left = sizeof(tmp) - (offset-tmp); + } search can be simplified by strstr(str, from) + + tmp[sizeof(tmp)-left] = '\0'; + + return strdup(tmp); +} + + diff --git a/src/openvz_conf.h b/src/openvz_conf.h index 8e02056..da9bfb4 100644 --- a/src/openvz_conf.h +++ b/src/openvz_conf.h @@ -42,6 +42,7 @@ enum { OPENVZ_WARN, OPENVZ_ERR }; /* OpenVZ commands - Replace with wrapper scripts later? */ #define VZLIST "/usr/sbin/vzlist" #define VZCTL "/usr/sbin/vzctl" +#define VZ_DEFAULT_CONF_FILE "ve-vps.basic.conf-sample" really, default config is specified in /etc/sysconfig/vz # grep -R CONFIGFILE /etc/sysconfig/vz CONFIGFILE="vps.basic" config file is calculating by snprintf(path, sizeof(path), VPS_CONF_DIR "ve-%s.conf-sample", config); #define VZCTL_BRIDGE_MIN_VERSION ((3 * 1000 * 1000) + (0 * 1000) + 22 + 1)
Hi Evgeniy,
really, default config is specified in /etc/sysconfig/vz
# grep -R CONFIGFILE /etc/sysconfig/vz CONFIGFILE="vps.basic"
config file is calculating by snprintf(path, sizeof(path), VPS_CONF_DIR "ve-%s.conf-sample", config);
Actually, vz.conf is in /etc/vz/ on my machine (Ubuntu Hardy). Isn't /etc/vz/ standard ? If not, do you have any idea on how I could locate vz.conf reliably (or get the value of CONFIGFILE reliably) ? Thanks, Florian
Hi Evgeniy,
really, default config is specified in /etc/sysconfig/vz
# grep -R CONFIGFILE /etc/sysconfig/vz CONFIGFILE="vps.basic"
config file is calculating by snprintf(path, sizeof(path), VPS_CONF_DIR "ve-%s.conf-sample", config);
Actually, vz.conf is in /etc/vz/ on my machine (Ubuntu Hardy). Isn't /etc/vz/ standard ? If not, do you have any idea on how I could locate vz.conf reliably (or get the value of CONFIGFILE reliably) ?
Yes, /etc/vz/vz.conf is standard config. Better to use /etc/vz. I am accustomed to use old path /etc/sysconfig/vz, it is symlink to /etc/vz/vz.conf now.
Hi everyone, Here is the new patch, with the requested modifications: - The target tag is now set to '/', and not used to specify the pivot root location. - Search and replace uses virBuffer and strstr() - Use of virAsprintf when possible, instead of stack arrays. - Default config file name is read from /etc/vz/vz.conf Thanks for the comments and explanations, Florian
Florian Vichot a écrit :
Hi everyone,
Here is the new patch, with the requested modifications: - The target tag is now set to '/', and not used to specify the pivot root location. - Search and replace uses virBuffer and strstr() - Use of virAsprintf when possible, instead of stack arrays. - Default config file name is read from /etc/vz/vz.conf
Thanks for the comments and explanations, Florian
------------------------------------------------------------------------
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Hello, As I've not received any comments or follow ups on this patch, I'm just making sure it's not forgotten :) Thanks, Florian
On Fri, Mar 13, 2009 at 12:06:12PM +0100, Florian Vichot wrote:
Hi everyone,
Here is the new patch, with the requested modifications: - The target tag is now set to '/', and not used to specify the pivot root location. - Search and replace uses virBuffer and strstr() - Use of virAsprintf when possible, instead of stack arrays. - Default config file name is read from /etc/vz/vz.conf
I think this looks good now, so ACK to this patch unless Evgeniy has further suggestions from an OpenVZ point of view. I'll commit this tomorrow if no one objects... Daniel
diff --git a/src/openvz_conf.c b/src/openvz_conf.c index ff3d607..c5f4a14 100644 --- a/src/openvz_conf.c +++ b/src/openvz_conf.c @@ -192,7 +192,7 @@ openvzReadNetworkConf(virConnectPtr conn, * IP_ADDRESS="1.1.1.1 1.1.1.2" * splited IPs by space */ - ret = openvzReadConfigParam(veid, "IP_ADDRESS", temp, sizeof(temp)); + ret = openvzReadVPSConfigParam(veid, "IP_ADDRESS", temp, sizeof(temp)); if (ret < 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("Cound not read 'IP_ADDRESS' from config for container %d"), @@ -224,7 +224,7 @@ openvzReadNetworkConf(virConnectPtr conn, *NETIF="ifname=eth10,mac=00:18:51:C1:05:EE,host_ifname=veth105.10,host_mac=00:18:51:8F:D9:F3" *devices splited by ';' */ - ret = openvzReadConfigParam(veid, "NETIF", temp, sizeof(temp)); + ret = openvzReadVPSConfigParam(veid, "NETIF", temp, sizeof(temp)); if (ret < 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("Cound not read 'NETIF' from config for container %d"), @@ -314,15 +314,46 @@ error: }
+/* utility function to replace 'from' by 'to' in 'str' */ +static char* +openvz_replace(const char* str, + const char* from, + const char* to) { + const char* offset = NULL; + const char* str_start = str; + int to_len = strlen(to); + int from_len = strlen(from); + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if(!from) + return NULL; + + while((offset = strstr(str_start, from))) + { + virBufferAdd(&buf, str_start, offset-str_start); + virBufferAdd(&buf, to, to_len); + str_start = offset + from_len; + } + + virBufferAdd(&buf, str_start, strlen(str_start)); + + if(virBufferError(&buf)) + return NULL; + + return virBufferContentAndReset(&buf); +} + + static int openvzReadFSConf(virConnectPtr conn, virDomainDefPtr def, int veid) { int ret; virDomainFSDefPtr fs = NULL; - char temp[4096]; + char* veid_str = NULL; + char temp[100];
- ret = openvzReadConfigParam(veid, "OSTEMPLATE", temp, sizeof(temp)); + ret = openvzReadVPSConfigParam(veid, "OSTEMPLATE", temp, sizeof(temp)); if (ret < 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("Cound not read 'OSTEMPLATE' from config for container %d"), @@ -334,18 +365,38 @@ openvzReadFSConf(virConnectPtr conn,
fs->type = VIR_DOMAIN_FS_TYPE_TEMPLATE; fs->src = strdup(temp); - fs->dst = strdup("/"); + } else { + /* OSTEMPLATE was not found, VE was booted from a private dir directly */ + ret = openvzReadVPSConfigParam(veid, "VE_PRIVATE", temp, sizeof(temp)); + if (ret <= 0) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("Cound not read 'VE_PRIVATE' from config for container %d"), + veid); + goto error; + }
- if (fs->src == NULL || fs->dst == NULL) + if (VIR_ALLOC(fs) < 0) goto no_memory;
- if (VIR_REALLOC_N(def->fss, def->nfss + 1) < 0) - goto no_memory; - def->fss[def->nfss++] = fs; - fs = NULL; + if(virAsprintf(&veid_str, "%d", veid) < 0) + goto error; + + fs->type = VIR_DOMAIN_FS_TYPE_MOUNT; + fs->src = openvz_replace(temp, "$VEID", veid_str);
+ VIR_FREE(veid_str); }
+ fs->dst = strdup("/"); + + if (fs->src == NULL || fs->dst == NULL) + goto no_memory; + + if (VIR_REALLOC_N(def->fss, def->nfss + 1) < 0) + goto no_memory; + def->fss[def->nfss++] = fs; + fs = NULL; + return 0; no_memory: virReportOOMError(conn); @@ -432,7 +483,7 @@ int openvzLoadDomains(struct openvz_driver *driver) { if (!(dom->def->os.init = strdup("/sbin/init"))) goto no_memory;
- ret = openvzReadConfigParam(veid, "CPUS", temp, sizeof(temp)); + ret = openvzReadVPSConfigParam(veid, "CPUS", temp, sizeof(temp)); if (ret < 0) { openvzError(NULL, VIR_ERR_INTERNAL_ERROR, _("Cound not read config for container %d"), @@ -485,26 +536,23 @@ openvzGetNodeCPUs(void) return nodeinfo.cpus; }
-int -openvzWriteConfigParam(int vpsid, const char *param, const char *value) +static int +openvzWriteConfigParam(const char * conf_file, const char *param, const char *value) { - char conf_file[PATH_MAX]; - char temp_file[PATH_MAX]; + char * temp_file = NULL; + int fd = -1, temp_fd = -1; char line[PATH_MAX] ; - int fd, temp_fd;
- if (openvzLocateConfFile(vpsid, conf_file, PATH_MAX, "conf")<0) - return -1; - if (openvzLocateConfFile(vpsid, temp_file, PATH_MAX, "tmp")<0) + if (virAsprintf(&temp_file, "%s.tmp", conf_file)<0) return -1;
fd = open(conf_file, O_RDONLY); if (fd == -1) - return -1; + goto error; temp_fd = open(temp_file, O_WRONLY | O_CREAT | O_TRUNC, 0644); if (temp_fd == -1) { close(fd); - return -1; + goto error; }
while(1) { @@ -541,30 +589,32 @@ error: close(fd); if (temp_fd != -1) close(temp_fd); - unlink(temp_file); + if(temp_file) + unlink(temp_file); + VIR_FREE(temp_file); return -1; }
-/* -* 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) +openvzWriteVPSConfigParam(int vpsid, const char *param, const char *value) +{ + char conf_file[PATH_MAX]; + + if (openvzLocateConfFile(vpsid, conf_file, PATH_MAX, "conf")<0) + return -1; + + return openvzWriteConfigParam(conf_file, param, value); +} + +static int +openvzReadConfigParam(const char * conf_file ,const char * param, char *value, int maxlen) { - char conf_file[PATH_MAX] ; char line[PATH_MAX] ; int ret, found = 0; int fd ; char * sf, * token; char *saveptr = NULL;
- if (openvzLocateConfFile(vpsid, conf_file, PATH_MAX, "conf")<0) - return -1; - value[0] = 0;
fd = open(conf_file, O_RDONLY); @@ -597,6 +647,103 @@ openvzReadConfigParam(int vpsid ,const char * param, char *value, int maxlen) return ret ; }
+/* +* Read parameter from container config +* sample: 133, "OSTEMPLATE", value, 1024 +* return: -1 - error +* 0 - don't found +* 1 - OK +*/ +int +openvzReadVPSConfigParam(int vpsid ,const char * param, char *value, int maxlen) +{ + char conf_file[PATH_MAX] ; + + if (openvzLocateConfFile(vpsid, conf_file, PATH_MAX, "conf")<0) + return -1; + + return openvzReadConfigParam(conf_file, param, value, maxlen); +} + +static int +openvz_copyfile(char* from_path, char* to_path) +{ + char line[PATH_MAX]; + int fd, copy_fd; + int bytes_read; + + fd = open(from_path, O_RDONLY); + if (fd == -1) + return -1; + copy_fd = open(to_path, O_WRONLY | O_CREAT | O_TRUNC, 0644); + if (copy_fd == -1) { + close(fd); + return -1; + } + + while(1) { + if (openvz_readline(fd, line, sizeof(line)) <= 0) + break; + + bytes_read = strlen(line); + if (safewrite(copy_fd, line, bytes_read) != bytes_read) + goto error; + } + + if (close(fd) < 0) + goto error; + fd = -1; + if (close(copy_fd) < 0) + goto error; + copy_fd = -1; + + return 0; + +error: + if (fd != -1) + close(fd); + if (copy_fd != -1) + close(copy_fd); + return -1; +} + +/* +* Copy the default config to the VE conf file +* return: -1 - error +* 0 - OK +*/ +int +openvzCopyDefaultConfig(int vpsid) +{ + char * confdir = NULL; + char * default_conf_file = NULL; + char configfile_value[PATH_MAX]; + char conf_file[PATH_MAX]; + int ret = -1; + + if(openvzReadConfigParam(VZ_CONF_FILE, "CONFIGFILE", configfile_value, PATH_MAX) < 0) + goto cleanup; + + confdir = openvzLocateConfDir(); + if (confdir == NULL) + goto cleanup; + + if (virAsprintf(&default_conf_file, "%s/ve-%s.conf-sample", confdir, configfile_value) < 0) + goto cleanup; + + if (openvzLocateConfFile(vpsid, conf_file, PATH_MAX, "conf")<0) + goto cleanup; + + if (openvz_copyfile(default_conf_file, conf_file)<0) + goto cleanup; + + ret = 0; +cleanup: + VIR_FREE(confdir); + VIR_FREE(default_conf_file); + return ret; +} + /* Locate config file of container * return -1 - error * 0 - OK diff --git a/src/openvz_conf.h b/src/openvz_conf.h index 8e02056..00e18b4 100644 --- a/src/openvz_conf.h +++ b/src/openvz_conf.h @@ -42,6 +42,7 @@ enum { OPENVZ_WARN, OPENVZ_ERR }; /* OpenVZ commands - Replace with wrapper scripts later? */ #define VZLIST "/usr/sbin/vzlist" #define VZCTL "/usr/sbin/vzctl" +#define VZ_CONF_FILE "/etc/vz/vz.conf"
#define VZCTL_BRIDGE_MIN_VERSION ((3 * 1000 * 1000) + (0 * 1000) + 22 + 1)
@@ -56,8 +57,9 @@ struct openvz_driver { int openvz_readline(int fd, char *ptr, int maxlen); int openvzExtractVersion(virConnectPtr conn, struct openvz_driver *driver); -int openvzReadConfigParam(int vpsid ,const char * param, char *value, int maxlen); -int openvzWriteConfigParam(int vpsid, const char *param, const char *value); +int openvzReadVPSConfigParam(int vpsid ,const char * param, char *value, int maxlen); +int openvzWriteVPSConfigParam(int vpsid, const char *param, const char *value); +int openvzCopyDefaultConfig(int vpsid); virCapsPtr openvzCapsInit(void); int openvzLoadDomains(struct openvz_driver *driver); void openvzFreeDriver(struct openvz_driver *driver); diff --git a/src/openvz_driver.c b/src/openvz_driver.c index d6c4362..ac2c089 100644 --- a/src/openvz_driver.c +++ b/src/openvz_driver.c @@ -132,19 +132,9 @@ static int openvzDomainDefineCmd(virConnectPtr conn, ADD_ARG_LIT("create"); ADD_ARG_LIT(vmdef->name);
- if (vmdef->nfss) { - if (vmdef->fss[0]->type != VIR_DOMAIN_FS_TYPE_TEMPLATE) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - "%s", _("only filesystem templates are supported")); - return -1; - } - - if (vmdef->nfss > 1) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - "%s", _("only one filesystem supported")); - return -1; - } - + if (vmdef->nfss == 1 && + vmdef->fss[0]->type == VIR_DOMAIN_FS_TYPE_TEMPLATE) + { ADD_ARG_LIT("--ostemplate"); ADD_ARG_LIT(vmdef->fss[0]->src); } @@ -166,6 +156,77 @@ static int openvzDomainDefineCmd(virConnectPtr conn, }
+static int openvzSetInitialConfig(virConnectPtr conn, + virDomainDefPtr vmdef) +{ + int ret = -1; + int vpsid; + char * confdir = NULL; + const char *prog[OPENVZ_MAX_ARG]; + prog[0] = NULL; + + if (vmdef->nfss > 1) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("only one filesystem supported")); + goto cleanup; + } + + if (vmdef->nfss == 1 && + vmdef->fss[0]->type != VIR_DOMAIN_FS_TYPE_TEMPLATE && + vmdef->fss[0]->type != VIR_DOMAIN_FS_TYPE_MOUNT) + { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("filesystem is not of type 'template' or 'mount'")); + goto cleanup; + } + + + if (vmdef->nfss == 1 && + vmdef->fss[0]->type == VIR_DOMAIN_FS_TYPE_MOUNT) + { + + if(virStrToLong_i(vmdef->name, NULL, 10, &vpsid) < 0) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("Could not convert domain name to VEID")); + goto cleanup; + } + + if (openvzCopyDefaultConfig(vpsid) < 0) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("Could not copy default config")); + goto cleanup; + } + + if (openvzWriteVPSConfigParam(vpsid, "VE_PRIVATE", vmdef->fss[0]->src) < 0) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("Could not set the source dir for the filesystem")); + goto cleanup; + } + } + else + { + if (openvzDomainDefineCmd(conn, prog, OPENVZ_MAX_ARG, vmdef) < 0) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("Error creating command for container")); + goto cleanup; + } + + if (virRun(conn, prog, NULL) < 0) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("Could not exec %s"), VZCTL); + goto cleanup; + } + } + + ret = 0; + +cleanup: + VIR_FREE(confdir); + cmdExecFree(prog); + return ret; +} + + static virDomainPtr openvzDomainLookupByID(virConnectPtr conn, int id) { struct openvz_driver *driver = conn->privateData; @@ -447,7 +508,7 @@ openvzGenerateContainerVethName(int veid) char temp[1024];
/* try to get line "^NETIF=..." from config */ - if ( (ret = openvzReadConfigParam(veid, "NETIF", temp, sizeof(temp))) <= 0) { + if ( (ret = openvzReadVPSConfigParam(veid, "NETIF", temp, sizeof(temp))) <= 0) { snprintf(temp, sizeof(temp), "eth0"); } else { char *saveptr; @@ -630,7 +691,7 @@ openvzDomainSetNetworkConfig(virConnectPtr conn, if (driver->version < VZCTL_BRIDGE_MIN_VERSION && def->nnets) { param = virBufferContentAndReset(&buf); if (param) { - if (openvzWriteConfigParam(strtoI(def->name), "NETIF", param) < 0) { + if (openvzWriteVPSConfigParam(strtoI(def->name), "NETIF", param) < 0) { VIR_FREE(param); openvzError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("cannot replace NETIF config")); @@ -656,8 +717,6 @@ openvzDomainDefineXML(virConnectPtr conn, const char *xml) virDomainDefPtr vmdef = NULL; virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; - const char *prog[OPENVZ_MAX_ARG]; - prog[0] = NULL;
openvzDriverLock(driver); if ((vmdef = virDomainDefParseString(conn, driver->caps, xml, @@ -680,20 +739,14 @@ openvzDomainDefineXML(virConnectPtr conn, const char *xml) goto cleanup; vmdef = NULL;
- if (openvzDomainDefineCmd(conn, prog, OPENVZ_MAX_ARG, vm->def) < 0) { + if (openvzSetInitialConfig(conn, vm->def) < 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, - "%s", _("Error creating command for container")); + "%s", _("Error creating intial configuration")); goto cleanup; }
//TODO: set quota
- if (virRun(conn, prog, NULL) < 0) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("Could not exec %s"), VZCTL); - goto cleanup; - } - if (openvzSetDefinedUUID(strtoI(vm->def->name), vm->def->uuid) < 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("Could not set UUID")); @@ -717,7 +770,6 @@ openvzDomainDefineXML(virConnectPtr conn, const char *xml)
cleanup: virDomainDefFree(vmdef); - cmdExecFree(prog); if (vm) virDomainObjUnlock(vm); openvzDriverUnlock(driver); @@ -733,8 +785,6 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml, virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; const char *progstart[] = {VZCTL, "--quiet", "start", PROGRAM_SENTINAL, NULL}; - const char *progcreate[OPENVZ_MAX_ARG]; - progcreate[0] = NULL;
openvzDriverLock(driver); if ((vmdef = virDomainDefParseString(conn, driver->caps, xml, @@ -756,15 +806,9 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml, goto cleanup; vmdef = NULL;
- if (openvzDomainDefineCmd(conn, progcreate, OPENVZ_MAX_ARG, vm->def) < 0) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - "%s", _("Error creating command for container")); - goto cleanup; - } - - if (virRun(conn, progcreate, NULL) < 0) { + if (openvzSetInitialConfig(conn, vm->def) < 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("Could not exec %s"), VZCTL); + "%s", _("Error creating intial configuration")); goto cleanup; }
@@ -803,7 +847,6 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml,
cleanup: virDomainDefFree(vmdef); - cmdExecFree(progcreate); if (vm) virDomainObjUnlock(vm); openvzDriverUnlock(driver); @@ -940,7 +983,7 @@ openvzDomainGetAutostart(virDomainPtr dom, int *autostart) goto cleanup; }
- if (openvzReadConfigParam(strtoI(vm->def->name), "ONBOOT", value, sizeof(value)) < 0) { + if (openvzReadVPSConfigParam(strtoI(vm->def->name), "ONBOOT", value, sizeof(value)) < 0) { openvzError(dom->conn, VIR_ERR_INTERNAL_ERROR, "%s", _("Could not read container config")); goto cleanup;
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Fri, Mar 13, 2009 at 12:06:12PM +0100, Florian Vichot wrote:
Hi everyone,
Here is the new patch, with the requested modifications: - The target tag is now set to '/', and not used to specify the pivot root location. - Search and replace uses virBuffer and strstr() - Use of virAsprintf when possible, instead of stack arrays. - Default config file name is read from /etc/vz/vz.conf
Thanks for the comments and explanations,
Thanks for you contribution, I have committed this patch now. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (3)
-
Daniel P. Berrange -
Evgeniy Sokolov -
Florian Vichot