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

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.