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

[Yuji NISHIDA]

Thanks, Chris and Daniel

I corrected the code that I posted here according to your comments.
Chris, I now need to handle openvz containers by character(name) not  
integer(id) at all.

diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index 54bcaa9..3b8505d 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -156,12 +156,13 @@ openvzDomainDefineCmd(virConnectPtr conn,
      fclose(fp);

      if (max_veid == 0) {
-        max_veid = 100;
+        /* OpenVZ reserves the IDs ranging from 0 to 100 */
+        max_veid = 101;
      } else {
          max_veid++;
      }

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

      ADD_ARG_LIT("--name");
-- 
1.5.2.2


-----
Yuji Nishida
nishidy at nict.go.jp

On 2009/09/23, at 23:45, Daniel Veillard wrote:

> On Wed, Sep 23, 2009 at 10:22:51AM +0200, Chris Lalancette wrote:
>> 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.
>
>  good point.
>
>>> +
>>> +    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.
>
>  I actually applied that patch on monday (after a couple of cleanups)
> and apparently my reply mail is part of the set that got lost :-(
>     Author: Yuji NISHIDA <nishidy at nict.go.jp>  2009-09-22 12:19:09
>     Committer: Daniel Veillard <veillard at redhat.com>  2009-09-22  
> 12:19:09
>     0c85095e46f3aba09ac401f559b76df0b0bea998
>
> the snprintf wasn't looking critical because I don't think a %d can
> generate more than 9 characters, but you're right in the absolute :-)
>
> Daniel
>
> -- 
> Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
> daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
> http://veillard.com/ | virtualization library  http://libvirt.org/