7:44 a.m.
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.
11:23 a.m.
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="10000"/>
+ <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/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
8:38 a.m.
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="10000"/>
> + <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.
9:12 a.m.
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/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
2:53 a.m.
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 :|
3:43 a.m.
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/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
11:30 a.m.
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 -0000 1.5
+++ src/domain_conf.c 25 Jul 2008 12:40:17 -0000
@@ -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 -0000 1.1
+++ src/domain_conf.h 25 Jul 2008 12:40:17 -0000
@@ -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 -0000 1.9
+++ src/openvz_conf.h 25 Jul 2008 12:40:17 -0000
@@ -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 :|
5:58 a.m.
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 -0000 1.5
> +++ src/domain_conf.c 25 Jul 2008 12:40:17 -0000
> @@ -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 -0000 1.1
> +++ src/domain_conf.h 25 Jul 2008 12:40:17 -0000
> @@ -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..
But to use
virDomainDeviceDefParse we need before convert xmlNodePtr to
xml-string, or not?
>
> 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
Yes, you are right.
It is half of port and intermediate state. But it support generic xml
format.
I propouse to completely switch to generic
virDomainDefPtr/virDomainObjPtr in nearest future.
I will correct defects from Daniel Veillard and send new patch soon.
5990
days inactive
5994
days old
7 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Evgeniy Sokolov