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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx

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@xxxxxxxxxx>  2009-09-22 12:19:09
Committer: Daniel Veillard <veillard@xxxxxxxxxx> 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@xxxxxxxxxxxx  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]