[libvirt] Re: OpenVZ : The restriction of domain name should be addressed

[Chris Lalancette]

Yuji NISHIDA wrote:
> Hi Daniel
> 
> Fixed patch according to your comments in openvzDomainDefineCmd func.
> 
> --- a/src/openvz_driver.c
> +++ b/src/openvz_driver.c
> @@ -101,6 +101,9 @@ static int openvzDomainDefineCmd(virConnectPtr conn,
>                                   virDomainDefPtr vmdef)
> {
>      int narg;
> +    int veid;
> +    int max_veid;
> +    FILE *fp;
> 
>      for (narg = 0; narg < maxarg; narg++)
>          args[narg] = NULL;
> @@ -130,6 +133,38 @@ static int openvzDomainDefineCmd(virConnectPtr  
> conn,
>      ADD_ARG_LIT(VZCTL);
>      ADD_ARG_LIT("--quiet");
>      ADD_ARG_LIT("create");
> +
> +    if ((fp = popen(VZLIST " -a -ovpsid -H 2>/dev/null", "r")) ==  
> NULL) {
> +        openvzError(NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("popen  
> failed"));
> +        return -1;
> +    }
> +    max_veid = 0;
> +    while(!feof(fp)) {
> +        if (fscanf(fp, "%d\n", &veid ) != 1) {
> +            if (feof(fp))
> +                break;
> +
> +            openvzError(NULL, VIR_ERR_INTERNAL_ERROR,
> +                        "%s", _("Failed to parse vzlist output"));
> +            goto cleanup;
> +        }
> +        if(veid > max_veid){
> +            max_veid = veid;
> +        }
> +    }
> +    fclose(fp);
> +
> +    if(max_veid == 0){
> +        max_veid = 100;
> +    }else{
> +        max_veid++;
> +    }

You might want to add a comment saying that vpsid's below 100 are reserved for
OpenVZ internal use; otherwise, it looks like an odd place to begin numbering.

> +
> +    char str_id[10];
> +    sprintf( str_id, "%d", max_veid++ );

You'll want to use snprintf here, like:

snprintf(str_id, sizeof(str_id), "%d", max_veid++);

(bear with me on this part, since I don't know much about OpenVZ).

Besides that, though, I'm not sure you necessarily want to do it like this,
since you aren't really tracking the ID's properly.  The problem I see is that
if you do it like this, start the container, and then do "virsh dumpxml
<openvz>", you won't see the ID in the output XML, like you do for the other
drivers.  Is that intentional?  If not, I think you'll want to store the id in
the virDomainDef->id member so that the information will be properly printed to
the user.

-- 
Chris Lalancette