Re: [libvirt] Re: OpenVZ : The restriction of domain name should be addressed
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 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Re: OpenVZ : The restriction of domain name should be addressed
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
nish...@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
", 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 2009-09-22 12:19:09
Committer: Daniel Veillard 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/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Re: OpenVZ : The restriction of domain name should be addressed
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
> ", 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 2009-09-22 12:19:09
Committer: Daniel Veillard 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/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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
", 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
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Re: OpenVZ : The restriction of domain name should be addressed
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
nish...@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 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Re: OpenVZ : The restriction of domain name should be addressed
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 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Re: OpenVZ : The restriction of domain name should be addressed
On Fri, Aug 21, 2009 at 11:08:05AM +0900, Yuji NISHIDA wrote:
>
>>> 2009/7/24 Daniel P. Berrange
>
We should make use of this --name parameter then - I guess it didn't
exist when we first wrote the driver. It is useful to users to have
separate ID vs Name parameters - and in fact the current reusing of
'ID' for the name, causes a little confusion in virsh's lookup
routines
because it can't tell whether the parameter its given is a name or
an
ID, since they are identical.
>>>
>>> There is still a question of how to specify both a name and CTID in
>>> XML
>>> description.
>>> By default, CTID can be obtained as openvzGimmeFirstUnusedCTID(), but
>>> actually
>>> I think that there exists a number of persons interested in giving
>>> CTIDs
>>> manually.
>>
>> Well, can be used for CTID remaining for
>> alphabetical domain name?
>
>
> I worte a small patch and tried with following XML setting.
>
> ### Patch ###
> --- a/src/openvz_driver.c
> +++ b/src/openvz_driver.c
> @@ -130,9 +130,6 @@
> ADD_ARG_LIT(VZCTL);
> ADD_ARG_LIT("--quiet");
> ADD_ARG_LIT("create");
> -ADD_ARG_LIT(vmdef->id);
> -
> -ADD_ARG_LIT("--name");
> ADD_ARG_LIT(vmdef->name);
>
> ### XML ###
>
> abc
>
> I found the type of id was identified as number( obj->type ==
> XPATH_NUMBER ) in virXPathLongBase,
> but it is clearly string before converted.
> I think correct path is to go in the first if context( obj->type ==
> XPATH_STRING ) and run strtol.
the id of domain must be a number, that's a requirement of libvirt
so when parsing we expect a number value.
> Then I tried with following XML.
>
> ### XML ###
>
> abc
>
> I got following error.
>
> AttValue: " or ' expected
yes, that's not XML ! you need ' or " around attributes, normal too.
> I'm not sure from which function should return this result.
I'm not sure what you're trying to do but as is the behaviour of
libvirt looks fine from your report.
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
