Re: [libvirt] [PATCH] OpenVZ xml refactoring

2008-07-29 Thread Daniel P. Berrange 
On Mon, Jul 28, 2008 at 10:12:49AM -0400, Daniel Veillard wrote:
 On Mon, Jul 28, 2008 at 05:38:47PM +0400, Evgeniy Sokolov wrote:
  
  On Fri, Jul 25, 2008 at 04:44:09PM +0400, Evgeniy Sokolov wrote:
In general that looks way cleaner to me,
  I will give it a few nmore days and apply, unless you suggest another 
  version,
  
  fixed patch is attached.
 
   Okay, I applied and commited this because it enforces the transition to
 the new XML format for OpenVZ and any such change should be done as soon
 as possible. But Dan's point remain, we need to transition to the new 
 reading routines, and virDomainNetDefParseXML will have to be made static
 again when done. But as I understand you agree with this so it's just an
 intermediate state of the code :-)

This patch doesn't work or compile because it is missing an argument
to virXPathNodeSet(). Please make sure you're developing against the
latest CVS checkout of libvirt when submitting patches, and run the
configure script with the '--enable-compile-warnings=error' argument
the catch this sort of problem before submission.

Here is the nightly build failure in question:

http://builder.virt-manager.org/logs/modules/libvirt--devel-build-output.log

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] [PATCH] OpenVZ xml refactoring

2008-07-29 Thread Daniel Veillard 
On Tue, Jul 29, 2008 at 08:53:27AM +0100, Daniel P. Berrange wrote:
 On Mon, Jul 28, 2008 at 10:12:49AM -0400, Daniel Veillard wrote:
  On Mon, Jul 28, 2008 at 05:38:47PM +0400, Evgeniy Sokolov wrote:
   
   On Fri, Jul 25, 2008 at 04:44:09PM +0400, Evgeniy Sokolov wrote:
 In general that looks way cleaner to me,
   I will give it a few nmore days and apply, unless you suggest another 
   version,
   
   fixed patch is attached.
  
Okay, I applied and commited this because it enforces the transition to
  the new XML format for OpenVZ and any such change should be done as soon
  as possible. But Dan's point remain, we need to transition to the new 
  reading routines, and virDomainNetDefParseXML will have to be made static
  again when done. But as I understand you agree with this so it's just an
  intermediate state of the code :-)
 
 This patch doesn't work or compile because it is missing an argument
 to virXPathNodeSet(). Please make sure you're developing against the
 latest CVS checkout of libvirt when submitting patches, and run the
 configure script with the '--enable-compile-warnings=error' argument
 the catch this sort of problem before submission.

  Humpf ... the problem is that I ran autogen between testing both versions
of the patch and the --with-openvz vanished.
  Fixing in CVs, it's trivial, but also activating OpenVZ and LXC support
by default, there is no good reason to not do so at this point.

Sorry,

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard  | virtualization library  http://libvirt.org/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

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

Re: [libvirt] [PATCH] OpenVZ xml refactoring

2008-07-28 Thread Evgeniy Sokolov 



On Fri, Jul 25, 2008 at 04:44:09PM +0400, Evgeniy Sokolov wrote:

Patch switch OpenVZ XML format to generic.
main changes:
- I used generic virDomainNetDef to define network in container.
  And wrote function to apply virDomainNetDef for container.
  Method virDomainNetDefParseXML is public now.
- tag container is changed to devices
- changed format of tag filesystem
- added processing vcpu tag.


  Generally looks fine to me, just a few remarks


+/* Parse filesystem section
+Sample:
+filesystem type=template
+  source name=fedora-core-5-i386/
+  quota type=size max=1/
+  quota type=inodes max=100/
+/filesystem
+*/
+static int openvzParseDomainFS(virConnectPtr conn,
+   struct openvz_fs_def *fs,
+   xmlXPathContextPtr ctxt)
+{
+xmlNodePtr cur, obj;
+char *type;
+
+obj = virXPathNode(/domain/devices/filesystem[1], ctxt);
+if (obj == NULL) {
+openvzError(conn, VIR_ERR_INTERNAL_ERROR,
+   _(missing filesystem tag));
+return -1;
+}


  hum, maybe use virXPathNodeSet and checking you won't get more than one ?

good idea! done



 /* Extract domain uuid */
-obj = xmlXPathEval(BAD_CAST string(/domain/uuid[1]), ctxt);
-if ((obj == NULL) || (obj-type != XPATH_STRING) ||
-(obj-stringval == NULL) || (obj-stringval[0] == 0)) {
+prop = virXPathString(string(./uuid[1]), ctxt);
+if (!prop) {
 int err;


  Hum, if you start using relative XPath queries like that it's a good idea
to make sure ctxt-node is set to the node you're starting the lookup from
I don't see that in the patch, maybe I missed it...

Yes, you found bug. Fixed now. Thanks!



+} else {
+if (virUUIDParse(prop, def-uuid)  0) {
+openvzError(conn, VIR_ERR_INTERNAL_ERROR,
+_(malformed uuid element));
+goto bail_out;
+}
+VIR_FREE(prop);


[...]

+//TODO: processing NAT and phisical device


  typo physical :-)

  In general that looks way cleaner to me,
I will give it a few nmore days and apply, unless you suggest another version,


fixed patch is attached.

Index: src/domain_conf.c
===
RCS file: /data/cvs/libvirt/src/domain_conf.c,v
retrieving revision 1.5
diff -u -p -r1.5 domain_conf.c
--- src/domain_conf.c	19 Jul 2008 07:42:34 -	1.5
+++ src/domain_conf.c	28 Jul 2008 13:20:10 -
@@ -633,7 +633,7 @@ static void virDomainNetRandomMAC(virDom
  * @param node XML nodeset to parse for net definition
  * @return 0 on success, -1 on failure
  */
-static virDomainNetDefPtr
+virDomainNetDefPtr
 virDomainNetDefParseXML(virConnectPtr conn,
 xmlNodePtr node) {
 virDomainNetDefPtr def;
Index: src/domain_conf.h
===
RCS file: /data/cvs/libvirt/src/domain_conf.h,v
retrieving revision 1.1
diff -u -p -r1.1 domain_conf.h
--- src/domain_conf.h	11 Jul 2008 16:23:36 -	1.1
+++ src/domain_conf.h	28 Jul 2008 13:20:10 -
@@ -472,6 +472,9 @@ int virDomainLoadAllConfigs(virConnectPt
 int virDomainDeleteConfig(virConnectPtr conn,
   virDomainObjPtr dom);
 
+virDomainNetDefPtr virDomainNetDefParseXML(virConnectPtr conn,
+xmlNodePtr node);
+
 VIR_ENUM_DECL(virDomainVirt)
 VIR_ENUM_DECL(virDomainBoot)
 VIR_ENUM_DECL(virDomainFeature)
Index: src/openvz_conf.c
===
RCS file: /data/cvs/libvirt/src/openvz_conf.c,v
retrieving revision 1.30
diff -u -p -r1.30 openvz_conf.c
--- src/openvz_conf.c	21 Jul 2008 13:34:19 -	1.30
+++ src/openvz_conf.c	28 Jul 2008 13:20:12 -
@@ -56,6 +56,8 @@
 #include buf.h
 #include memory.h
 #include util.h
+#include xml.h
+#include domain_conf.h
 
 static char *openvzLocateConfDir(void);
 static struct openvz_vm_def *openvzParseXML(virConnectPtr conn, xmlDocPtr xml);
@@ -136,6 +138,34 @@ strtoI(const char *str)
 return val;
 }
 
+/* function checks MAC address is empty
+   return 0 - empty
+  1 - not
+*/
+int openvzCheckEmptyMac(const unsigned char *mac)
+{
+int i;
+for (i = 0; i  VIR_DOMAIN_NET_MAC_SIZE; i++)
+if (mac[i] != 0x00)
+return 1;
+
+return 0;
+}
+
+/* convert mac address to string
+   return pointer to string or NULL
+*/
+char *openvzMacToString(const unsigned char *mac)
+{
+char str[20];
+if (snprintf(str, 18, %02X:%02X:%02X:%02X:%02X:%02X,
+  mac[0], mac[1], mac[2],
+  mac[3], mac[4], mac[5]) = 18)
+return NULL;
+
+return strdup(str);
+}
+
 void
 openvzRemoveInactiveVM(struct openvz_driver *driver, struct openvz_vm *vm)
 {
@@ -148,30 +178,7 @@ void
 openvzFreeVMDef(struct openvz_vm_def *def)
 {
 if (def) {
-struct ovz_quota *quota = def-fs.quota;
-struct ovz_ip *ip = def-net.ips;
-

Re: [libvirt] [PATCH] OpenVZ xml refactoring

2008-07-28 Thread Daniel Veillard 
On Mon, Jul 28, 2008 at 05:38:47PM +0400, Evgeniy Sokolov wrote:
 
 On Fri, Jul 25, 2008 at 04:44:09PM +0400, Evgeniy Sokolov wrote:
   In general that looks way cleaner to me,
 I will give it a few nmore days and apply, unless you suggest another 
 version,
 
 fixed patch is attached.

  Okay, I applied and commited this because it enforces the transition to
the new XML format for OpenVZ and any such change should be done as soon
as possible. But Dan's point remain, we need to transition to the new 
reading routines, and virDomainNetDefParseXML will have to be made static
again when done. But as I understand you agree with this so it's just an
intermediate state of the code :-) 

  thanks a lot !

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard  | virtualization library  http://libvirt.org/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

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

[libvirt] [PATCH] OpenVZ xml refactoring

2008-07-25 Thread Evgeniy Sokolov 

Patch switch OpenVZ XML format to generic.
main changes:
- I used generic virDomainNetDef to define network in container.
  And wrote function to apply virDomainNetDef for container.
  Method virDomainNetDefParseXML is public now.
- tag container is changed to devices
- changed format of tag filesystem
- added processing vcpu tag.
Index: src/domain_conf.c
===
RCS file: /data/cvs/libvirt/src/domain_conf.c,v
retrieving revision 1.5
diff -u -p -r1.5 domain_conf.c
--- src/domain_conf.c	19 Jul 2008 07:42:34 -	1.5
+++ src/domain_conf.c	25 Jul 2008 12:40:17 -
@@ -633,7 +633,7 @@ static void virDomainNetRandomMAC(virDom
  * @param node XML nodeset to parse for net definition
  * @return 0 on success, -1 on failure
  */
-static virDomainNetDefPtr
+virDomainNetDefPtr
 virDomainNetDefParseXML(virConnectPtr conn,
 xmlNodePtr node) {
 virDomainNetDefPtr def;
Index: src/domain_conf.h
===
RCS file: /data/cvs/libvirt/src/domain_conf.h,v
retrieving revision 1.1
diff -u -p -r1.1 domain_conf.h
--- src/domain_conf.h	11 Jul 2008 16:23:36 -	1.1
+++ src/domain_conf.h	25 Jul 2008 12:40:17 -
@@ -472,6 +472,9 @@ int virDomainLoadAllConfigs(virConnectPt
 int virDomainDeleteConfig(virConnectPtr conn,
   virDomainObjPtr dom);
 
+virDomainNetDefPtr virDomainNetDefParseXML(virConnectPtr conn,
+xmlNodePtr node);
+
 VIR_ENUM_DECL(virDomainVirt)
 VIR_ENUM_DECL(virDomainBoot)
 VIR_ENUM_DECL(virDomainFeature)
Index: src/openvz_conf.c
===
RCS file: /data/cvs/libvirt/src/openvz_conf.c,v
retrieving revision 1.30
diff -u -p -r1.30 openvz_conf.c
--- src/openvz_conf.c	21 Jul 2008 13:34:19 -	1.30
+++ src/openvz_conf.c	25 Jul 2008 12:40:17 -
@@ -56,6 +56,8 @@
 #include buf.h
 #include memory.h
 #include util.h
+#include xml.h
+#include domain_conf.h
 
 static char *openvzLocateConfDir(void);
 static struct openvz_vm_def *openvzParseXML(virConnectPtr conn, xmlDocPtr xml);
@@ -136,6 +138,34 @@ strtoI(const char *str)
 return val;
 }
 
+/* function checks MAC address is empty
+   return 0 - empty
+  1 - not
+*/
+int openvzCheckEmptyMac(const unsigned char *mac)
+{
+int i;
+for (i = 0; i  VIR_DOMAIN_NET_MAC_SIZE; i++)
+if (mac[i] != 0x00)
+return 1;
+
+return 0;
+}
+
+/* convert mac address to string
+   return pointer to string or NULL
+*/
+char *openvzMacToString(const unsigned char *mac)
+{
+char str[20];
+if (snprintf(str, 18, %02X:%02X:%02X:%02X:%02X:%02X,
+  mac[0], mac[1], mac[2],
+  mac[3], mac[4], mac[5]) = 18)
+return NULL;
+
+return strdup(str);
+}
+
 void
 openvzRemoveInactiveVM(struct openvz_driver *driver, struct openvz_vm *vm)
 {
@@ -148,30 +178,7 @@ void
 openvzFreeVMDef(struct openvz_vm_def *def)
 {
 if (def) {
-struct ovz_quota *quota = def-fs.quota;
-struct ovz_ip *ip = def-net.ips;
-struct ovz_ns *ns = def-net.ns;
-
-while (quota) {
-struct ovz_quota *prev = quota;
-
-quota = quota-next;
-VIR_FREE(prev);
-}
-while (ip) {
-struct ovz_ip *prev = ip;
-
-ip = ip-next;
-VIR_FREE(prev);
-}
-while (ns) {
-struct ovz_ns *prev = ns;
-
-ns = ns-next;
-VIR_FREE(prev);
-}
-
-VIR_FREE(def);
+virDomainNetDefFree(def-net);
 }
 }
 
@@ -285,6 +292,74 @@ struct openvz_vm_def
 return def;
 }
 
+/* Parse filesystem section
+Sample:
+filesystem type=template
+  source name=fedora-core-5-i386/
+  quota type=size max=1/
+  quota type=inodes max=100/
+/filesystem
+*/
+static int openvzParseDomainFS(virConnectPtr conn,
+   struct openvz_fs_def *fs,
+   xmlXPathContextPtr ctxt)
+{
+xmlNodePtr cur, obj;
+char *type;
+
+obj = virXPathNode(/domain/devices/filesystem[1], ctxt);
+if (obj == NULL) {
+openvzError(conn, VIR_ERR_INTERNAL_ERROR,
+   _(missing filesystem tag));
+return -1;
+}
+
+/*check template type*/
+type = virXMLPropString(obj, type);
+if (!type) {
+ openvzError(conn, VIR_ERR_INTERNAL_ERROR,
+  _(missing type attribute));
+ return -1;
+}
+
+if (STRNEQ(type, template)) {
+ openvzError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(Unknown type attribute %s), type);
+ return -1;
+}
+VIR_FREE(type);
+
+cur = obj-children;
+while(cur != NULL)
+{
+if (cur-type == XML_ELEMENT_NODE) {
+if (xmlStrEqual(cur-name, BAD_CAST source)) {
+ char * name =  virXMLPropString(cur,

Re: [libvirt] [PATCH] OpenVZ xml refactoring

2008-07-25 Thread Daniel Veillard 
On Fri, Jul 25, 2008 at 04:44:09PM +0400, Evgeniy Sokolov wrote:
 Patch switch OpenVZ XML format to generic.
 main changes:
 - I used generic virDomainNetDef to define network in container.
   And wrote function to apply virDomainNetDef for container.
   Method virDomainNetDefParseXML is public now.
 - tag container is changed to devices
 - changed format of tag filesystem
 - added processing vcpu tag.

  Generally looks fine to me, just a few remarks

 +/* Parse filesystem section
 +Sample:
 +filesystem type=template
 +  source name=fedora-core-5-i386/
 +  quota type=size max=1/
 +  quota type=inodes max=100/
 +/filesystem
 +*/
 +static int openvzParseDomainFS(virConnectPtr conn,
 +   struct openvz_fs_def *fs,
 +   xmlXPathContextPtr ctxt)
 +{
 +xmlNodePtr cur, obj;
 +char *type;
 +
 +obj = virXPathNode(/domain/devices/filesystem[1], ctxt);
 +if (obj == NULL) {
 +openvzError(conn, VIR_ERR_INTERNAL_ERROR,
 +   _(missing filesystem tag));
 +return -1;
 +}

  hum, maybe use virXPathNodeSet and checking you won't get more than one ?

  /* Extract domain uuid */
 -obj = xmlXPathEval(BAD_CAST string(/domain/uuid[1]), ctxt);
 -if ((obj == NULL) || (obj-type != XPATH_STRING) ||
 -(obj-stringval == NULL) || (obj-stringval[0] == 0)) {
 +prop = virXPathString(string(./uuid[1]), ctxt);
 +if (!prop) {
  int err;

  Hum, if you start using relative XPath queries like that it's a good idea
to make sure ctxt-node is set to the node you're starting the lookup from
I don't see that in the patch, maybe I missed it...

 +} else {
 +if (virUUIDParse(prop, def-uuid)  0) {
 +openvzError(conn, VIR_ERR_INTERNAL_ERROR,
 +_(malformed uuid element));
 +goto bail_out;
 +}
 +VIR_FREE(prop);

[...]
 +//TODO: processing NAT and phisical device

  typo physical :-)

  In general that looks way cleaner to me,
I will give it a few nmore days and apply, unless you suggest another version,

   thanks !

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard  | virtualization library  http://libvirt.org/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

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

Re: [libvirt] [PATCH] OpenVZ xml refactoring

2008-07-25 Thread Daniel P. Berrange 
On Fri, Jul 25, 2008 at 04:44:09PM +0400, Evgeniy Sokolov wrote:
 Patch switch OpenVZ XML format to generic.
 main changes:
 - I used generic virDomainNetDef to define network in container.
   And wrote function to apply virDomainNetDef for container.
   Method virDomainNetDefParseXML is public now.
 - tag container is changed to devices
 - changed format of tag filesystem
 - added processing vcpu tag.

 Index: src/domain_conf.c
 ===
 RCS file: /data/cvs/libvirt/src/domain_conf.c,v
 retrieving revision 1.5
 diff -u -p -r1.5 domain_conf.c
 --- src/domain_conf.c 19 Jul 2008 07:42:34 -  1.5
 +++ src/domain_conf.c 25 Jul 2008 12:40:17 -
 @@ -633,7 +633,7 @@ static void virDomainNetRandomMAC(virDom
   * @param node XML nodeset to parse for net definition
   * @return 0 on success, -1 on failure
   */
 -static virDomainNetDefPtr
 +virDomainNetDefPtr
  virDomainNetDefParseXML(virConnectPtr conn,
  xmlNodePtr node) {
  virDomainNetDefPtr def;
 Index: src/domain_conf.h
 ===
 RCS file: /data/cvs/libvirt/src/domain_conf.h,v
 retrieving revision 1.1
 diff -u -p -r1.1 domain_conf.h
 --- src/domain_conf.h 11 Jul 2008 16:23:36 -  1.1
 +++ src/domain_conf.h 25 Jul 2008 12:40:17 -
 @@ -472,6 +472,9 @@ int virDomainLoadAllConfigs(virConnectPt
  int virDomainDeleteConfig(virConnectPtr conn,
virDomainObjPtr dom);
  
 +virDomainNetDefPtr virDomainNetDefParseXML(virConnectPtr conn,
 +xmlNodePtr node);
 +

This is not needed - we already export a generic virDOmainDeviceDefParse
method, which allows parsing of an individual device of any type. That
said, I don't think you need to use that either..

 diff -u -p -r1.9 openvz_conf.h
 --- src/openvz_conf.h 10 Jul 2008 12:21:09 -  1.9
 +++ src/openvz_conf.h 25 Jul 2008 12:40:17 -
 @@ -29,6 +29,7 @@
  #define OPENVZ_CONF_H
  
  #include openvz_driver.h
 +#include domain_conf.h
  
  enum { OPENVZ_WARN, OPENVZ_ERR };
  
 @@ -61,33 +62,16 @@ struct vps_props {
  
  struct openvz_fs_def {
  char tmpl[OPENVZ_TMPL_MAX];
 -struct ovz_quota *quota;
 -};
 -
 -struct ovz_ip {
 -char ip[OPENVZ_IP_MAX];
 -char netmask[OPENVZ_IP_MAX];
 -struct ovz_ip *next;
 -};
 -
 -struct ovz_ns {
 -char ip[OPENVZ_IP_MAX];
 -struct ovz_ns *next;
 -};
 -
 -struct openvz_net_def {
 -char hostname[OPENVZ_HOSTNAME_MAX];
 -char def_gw[OPENVZ_IP_MAX];
 -struct ovz_ip *ips;
 -struct ovz_ns *ns;
 +long int disksize, diskinodes;
  };
  
  struct openvz_vm_def {
  char name[OPENVZ_NAME_MAX];
  unsigned char uuid[VIR_UUID_BUFLEN];
  char profile[OPENVZ_PROFILE_MAX];
 +unsigned long vcpus;
  struct openvz_fs_def fs;
 -struct openvz_net_def net;
 +virDomainNetDefPtr net;
  };

Rather than doing this 50/50  port, we should just switch over
to the virDomainDefPtr/virDomainObjPtr objects entirely and
do away with openvz_vm and openvz_vm_def objects completely.

This will require the addition of my filesystem patch to the
generic parser, but that's less effort that maintaining the
custom parser in openvz code. This will allowing 95% of the
openvz_conf.c file to go away entirely

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