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

Re: [libvirt] [PATCH] Adding filesystem mount support for openVZ



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 :|


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