1:40 a.m.
Hi Daniel,
I didn't realize that I even did not follow the manner of XML.
I have worked with this problem and got a small patch to handle "ID"
in OpenVZ functionality.
I found it is working well with the XML script included "ID" in domain
tag.
I am concerned that I had to edit the common file domain_conf.c.
I still believe I should keep it away from this problem for
compatibility with the others.
How do you think can I avoid this?
diff --git a/src/domain_conf.c b/src/domain_conf.c
index 5ae0775..f74961f 100644
--- a/src/domain_conf.c
+++ b/src/domain_conf.c
@@ -2492,7 +2492,7 @@ static virDomainDefPtr virDomainDefParseXML
(virConnectPtr conn,
return NULL;
}
- if (!(flags & VIR_DOMAIN_XML_INACTIVE))
+ // if (!(flags & VIR_DOMAIN_XML_INACTIVE))
if((virXPathLong(conn, "string(./@id)", ctxt, &id)) < 0)
id = -1;
def->id = (int)id;
diff --git a/src/openvz_conf.c b/src/openvz_conf.c
index be94b9e..d23f173 100644
--- a/src/openvz_conf.c
+++ b/src/openvz_conf.c
@@ -425,17 +425,18 @@ int openvzLoadDomains(struct openvz_driver
*driver) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
virDomainObjPtr dom = NULL;
char temp[50];
+ char name[64];
if (openvzAssignUUIDs() < 0)
return -1;
- if ((fp = popen(VZLIST " -a -ovpsid,status -H 2>/dev/null",
"r"))
== NULL) {
+ if ((fp = popen(VZLIST " -a -ovpsid,name,status -H 2>/dev/null",
"r")) == NULL) {
openvzError(NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("popen
failed"));
return -1;
}
while(!feof(fp)) {
- if (fscanf(fp, "%d %s¥n", &veid, status) != 2) {
+ if (fscanf(fp, "%d %s¥n", &veid, name, status) != 3) {
if (feof(fp))
break;
@@ -465,7 +466,7 @@ int openvzLoadDomains(struct openvz_driver
*driver) {
dom->pid = veid;
dom->def->id = dom->state == VIR_DOMAIN_SHUTOFF ? -1 : veid;
- if (virAsprintf(&dom->def->name, "%i", veid) < 0)
+ if (virAsprintf(&dom->def->name, "%s", name) < 0)
goto no_memory;
openvzGetVPSUUID(veid, uuidstr, sizeof(uuidstr));
diff --git a/src/openvz_driver.c b/src/openvz_driver.c
index a8c24ba..c0c1e0f 100644
--- a/src/openvz_driver.c
+++ b/src/openvz_driver.c
@@ -101,6 +101,7 @@ static int openvzDomainDefineCmd(virConnectPtr conn,
virDomainDefPtr vmdef)
{
int narg;
+ char str_id[10];
for (narg = 0; narg < maxarg; narg++)
args[narg] = NULL;
@@ -130,6 +131,11 @@ static int openvzDomainDefineCmd(virConnectPtr
conn,
ADD_ARG_LIT(VZCTL);
ADD_ARG_LIT("--quiet");
ADD_ARG_LIT("create");
+
+ sprintf( str_id, "%d", vmdef->id );
+ ADD_ARG_LIT(str_id);
+
+ ADD_ARG_LIT("--name");
ADD_ARG_LIT(vmdef->name);
if (vmdef->nfss == 1 &&
@@ -1229,7 +1235,11 @@ static int openvzListDefinedDomains
(virConnectPtr conn,
char vpsname[32];
char buf[32];
char *endptr;
- const char *cmd[] = {VZLIST, "-ovpsid", "-H", "-S",
NULL};
+ const char *cmd[] = {VZLIST, "-oname", "-H", "-S",
NULL};
+ int cnt = 0;
+ char name_buf[32];
+
+
/* the -S options lists only stopped domains */
ret = virExec(conn, cmd, NULL, NULL,
@@ -1241,14 +1251,14 @@ static int openvzListDefinedDomains
(virConnectPtr conn,
}
while(got < nnames){
- ret = openvz_readline(outfd, buf, 32);
+ ret = openvz_readline(outfd, buf, 64);
if(!ret) break;
- if (virStrToLong_i(buf, &endptr, 10, &veid) < 0) {
- openvzError(conn, VIR_ERR_INTERNAL_ERROR,
- _("Could not parse VPS ID %s"), buf);
- continue;
+ cnt = 0;
+ while( buf[cnt] != ' ' ){
+ name_buf[cnt] = buf[cnt];
+ cnt++;
}
- snprintf(vpsname, sizeof(vpsname), "%d", veid);
+ snprintf(vpsname, sizeof(vpsname), "%d", name_buf);
if (!(names[got] = strdup(vpsname)))
goto no_memory;
got ++;
--
1.5.2.2
-----
Yuji Nishida
nishidy(a)nict.go.jp
4:59 a.m.
On Tue, Sep 15, 2009 at 03:40:09PM +0900, Yuji NISHIDA wrote:
Hi Daniel,
I didn't realize that I even did not follow the manner of XML.
I have worked with this problem and got a small patch to handle "ID"
in OpenVZ functionality.
I found it is working well with the XML script included "ID" in domain
tag.
I am concerned that I had to edit the common file domain_conf.c.
I still believe I should keep it away from this problem for
compatibility with the others.
How do you think can I avoid this?
The 'id' value is not intended to be settable by the end user,
which is why the domain_conf.c parser does not parse it by
default.
So for your usage in OpenVZ, you'll need to auto-assign an 'id'
value when creating a new guest. eg, get a list of existing
OpenVZ guest 'id' values and then pick the first one which is
not used.
diff --git a/src/openvz_driver.c b/src/openvz_driver.c
index a8c24ba..c0c1e0f 100644
--- a/src/openvz_driver.c
+++ b/src/openvz_driver.c
@@ -130,6 +131,11 @@ static int openvzDomainDefineCmd(virConnectPtr
conn,
ADD_ARG_LIT(VZCTL);
ADD_ARG_LIT("--quiet");
ADD_ARG_LIT("create");
+
+ sprintf( str_id, "%d", vmdef->id );
+ ADD_ARG_LIT(str_id);
+
+ ADD_ARG_LIT("--name");
ADD_ARG_LIT(vmdef->name);
This is where you need to pull in an auto-assigned ID value
instead of using vmdef->id.
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 :|
5:25 a.m.
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++;
+ }
+
+ char str_id[10];
+ sprintf( str_id, "%d", max_veid++ );
+ ADD_ARG_LIT(str_id);
+
+ ADD_ARG_LIT("--name");
ADD_ARG_LIT(vmdef->name);
if (vmdef->nfss == 1 &&
@@ -151,6 +186,11 @@ static int openvzDomainDefineCmd(virConnectPtr
conn,
openvzError(conn, VIR_ERR_INTERNAL_ERROR,
_("Could not put argument to %s"), VZCTL);
return -1;
+
+ cleanup:
+ fclose(fp);
+ return -1;
+
#undef ADD_ARG
#undef ADD_ARG_LIT
}
-----
Yuji Nishida
nishidy(a)nict.go.jp
On 2009/09/15, at 18:59, Daniel P. Berrange wrote:
On Tue, Sep 15, 2009 at 03:40:09PM +0900, Yuji NISHIDA wrote:
> Hi Daniel,
>
> I didn't realize that I even did not follow the manner of XML.
> I have worked with this problem and got a small patch to handle "ID"
> in OpenVZ functionality.
> I found it is working well with the XML script included "ID" in
> domain
> tag.
>
> I am concerned that I had to edit the common file domain_conf.c.
> I still believe I should keep it away from this problem for
> compatibility with the others.
> How do you think can I avoid this?
The 'id' value is not intended to be settable by the end user,
which is why the domain_conf.c parser does not parse it by
default.
So for your usage in OpenVZ, you'll need to auto-assign an 'id'
value when creating a new guest. eg, get a list of existing
OpenVZ guest 'id' values and then pick the first one which is
not used.
> diff --git a/src/openvz_driver.c b/src/openvz_driver.c
> index a8c24ba..c0c1e0f 100644
> --- a/src/openvz_driver.c
> +++ b/src/openvz_driver.c
> @@ -130,6 +131,11 @@ static int openvzDomainDefineCmd(virConnectPtr
> conn,
> ADD_ARG_LIT(VZCTL);
> ADD_ARG_LIT("--quiet");
> ADD_ARG_LIT("create");
> +
> + sprintf( str_id, "%d", vmdef->id );
> + ADD_ARG_LIT(str_id);
> +
> + ADD_ARG_LIT("--name");
> ADD_ARG_LIT(vmdef->name);
This is where you need to pull in an auto-assigned ID value
instead of using vmdef->id.
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 :|
3:22 a.m.
New subject: [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.
--
Chris Lalancette
9:45 a.m.
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(a)nict.go.jp> 2009-09-22 12:19:09
Committer: Daniel Veillard <veillard(a)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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
2:08 a.m.
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(a)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(a)nict.go.jp> 2009-09-22 12:19:09
Committer: Daniel Veillard <veillard(a)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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
2:52 a.m.
Yuji NISHIDA wrote:
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.
Right, the patch looks good to correct the two smaller issues that I pointed
out, but you still have the problem of not tracking the ID's properly. I guess
that will have to be done separately.
--
Chris Lalancette
5537
days inactive
5550
days old
6 comments
4 participants
participants (4)
-
Chris Lalancette -
Daniel P. Berrange -
Daniel Veillard -
Yuji NISHIDA