[libvirt] [PATCH] Domain snapshot RNG and tests.
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- docs/schemas/domainsnapshot.rng | 53 ++++++++++++++++++++ tests/Makefile.am | 6 ++- tests/domainsnapshotschematest | 10 ++++ tests/domainsnapshotxml2xmlin/description_only.xml | 3 + tests/domainsnapshotxml2xmlin/empty.xml | 1 + .../name_and_description.xml | 4 ++ tests/domainsnapshotxml2xmlin/name_only.xml | 3 + tests/domainsnapshotxml2xmlout/all_parameters.xml | 13 +++++ tests/domainsnapshotxml2xmlout/noparent.xml | 9 +++ .../noparent_nodescription.xml | 7 +++ .../noparent_nodescription_noactive.xml | 6 ++ 11 files changed, 114 insertions(+), 1 deletions(-) create mode 100644 docs/schemas/domainsnapshot.rng create mode 100755 tests/domainsnapshotschematest create mode 100644 tests/domainsnapshotxml2xmlin/description_only.xml create mode 100644 tests/domainsnapshotxml2xmlin/empty.xml create mode 100644 tests/domainsnapshotxml2xmlin/name_and_description.xml create mode 100644 tests/domainsnapshotxml2xmlin/name_only.xml create mode 100644 tests/domainsnapshotxml2xmlout/all_parameters.xml create mode 100644 tests/domainsnapshotxml2xmlout/noparent.xml create mode 100644 tests/domainsnapshotxml2xmlout/noparent_nodescription.xml create mode 100644 tests/domainsnapshotxml2xmlout/noparent_nodescription_noactive.xml diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng new file mode 100644 index 0000000..86bab0b --- /dev/null +++ b/docs/schemas/domainsnapshot.rng @@ -0,0 +1,53 @@ +<!-- A Relax NG schema for the libvirt domain snapshot properties XML format --> +<grammar xmlns="http://relaxng.org/ns/structure/1.0"> + <start> + <ref name='domainsnapshot'/> + </start> + + <define name='domainsnapshot'> + <element name='domainsnapshot'> + <interleave> + <optional> + <element name='name'> + <text/> + </element> + </optional> + <optional> + <element name='description'> + <text/> + </element> + </optional> + <optional> + <element name='state'> + <text/> + </element> + </optional> + <optional> + <element name='creationTime'> + <text/> + </element> + </optional> + <optional> + <element name='active'> + <text/> + </element> + </optional> + <optional> + <element name='domain'> + <element name='uuid'> + <text/> + </element> + </element> + </optional> + <optional> + <element name='parent'> + <element name='name'> + <text/> + </element> + </element> + </optional> + </interleave> + </element> + </define> + +</grammar> diff --git a/tests/Makefile.am b/tests/Makefile.am index c5e52e3..caf8cd0 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -76,6 +76,9 @@ EXTRA_DIST = \ nwfilterxml2xmlout \ nwfilterxml2xmlin \ nwfilterschematest \ + domainsnapshotschematest \ + domainsnapshotxml2xmlout \ + domainsnapshotxml2xmlin \ $(patsubst %,qemuhelpdata/%,$(qemuhelpdata)) noinst_PROGRAMS = virshtest conftest \ @@ -123,7 +126,8 @@ test_scripts = \ storagevolschematest \ domainschematest \ nodedevschematest \ - nwfilterschematest + nwfilterschematest \ + domainsnapshotschematest if WITH_LIBVIRTD test_scripts += \ diff --git a/tests/domainsnapshotschematest b/tests/domainsnapshotschematest new file mode 100755 index 0000000..1bdc539 --- /dev/null +++ b/tests/domainsnapshotschematest @@ -0,0 +1,10 @@ +#!/bin/sh + +: ${srcdir=.} +. $srcdir/test-lib.sh +. $abs_srcdir/schematestutils.sh + +DIRS="domainsnapshotxml2xmlin domainsnapshotxml2xmlout" +SCHEMA="domainsnapshot.rng" + +check_schema "$DIRS" "$SCHEMA" diff --git a/tests/domainsnapshotxml2xmlin/description_only.xml b/tests/domainsnapshotxml2xmlin/description_only.xml new file mode 100644 index 0000000..b76fca2 --- /dev/null +++ b/tests/domainsnapshotxml2xmlin/description_only.xml @@ -0,0 +1,3 @@ +<domainsnapshot> + <description>My description</description> +</domainsnapshot> diff --git a/tests/domainsnapshotxml2xmlin/empty.xml b/tests/domainsnapshotxml2xmlin/empty.xml new file mode 100644 index 0000000..f2bcfbe --- /dev/null +++ b/tests/domainsnapshotxml2xmlin/empty.xml @@ -0,0 +1 @@ +<domainsnapshot/> diff --git a/tests/domainsnapshotxml2xmlin/name_and_description.xml b/tests/domainsnapshotxml2xmlin/name_and_description.xml new file mode 100644 index 0000000..90ce774 --- /dev/null +++ b/tests/domainsnapshotxml2xmlin/name_and_description.xml @@ -0,0 +1,4 @@ +<domainsnapshot> + <name>snap1</name> + <description>A longer description!</description> +</domainsnapshot> diff --git a/tests/domainsnapshotxml2xmlin/name_only.xml b/tests/domainsnapshotxml2xmlin/name_only.xml new file mode 100644 index 0000000..8123a02 --- /dev/null +++ b/tests/domainsnapshotxml2xmlin/name_only.xml @@ -0,0 +1,3 @@ +<domainsnapshot> + <name>snapshot1</name> +</domainsnapshot> diff --git a/tests/domainsnapshotxml2xmlout/all_parameters.xml b/tests/domainsnapshotxml2xmlout/all_parameters.xml new file mode 100644 index 0000000..ed4a600 --- /dev/null +++ b/tests/domainsnapshotxml2xmlout/all_parameters.xml @@ -0,0 +1,13 @@ +<domainsnapshot> + <name>my snap name</name> + <description>!@#$%^</description> + <parent> + <name>earlier_snap</name> + </parent> + <state>running</state> + <creationTime>1272917631</creationTime> + <domain> + <uuid>9d37b878-a7cc-9f9a-b78f-49b3abad25a8</uuid> + </domain> + <active>1</active> +</domainsnapshot> diff --git a/tests/domainsnapshotxml2xmlout/noparent.xml b/tests/domainsnapshotxml2xmlout/noparent.xml new file mode 100644 index 0000000..cbac0d8 --- /dev/null +++ b/tests/domainsnapshotxml2xmlout/noparent.xml @@ -0,0 +1,9 @@ +<domainsnapshot> + <name>my snap name</name> + <description>!@#$%^</description> + <state>running</state> + <creationTime>1272917631</creationTime> + <domain> + <uuid>9d37b878-a7cc-9f9a-b78f-49b3abad25a8</uuid> + </domain> +</domainsnapshot> diff --git a/tests/domainsnapshotxml2xmlout/noparent_nodescription.xml b/tests/domainsnapshotxml2xmlout/noparent_nodescription.xml new file mode 100644 index 0000000..0de202d --- /dev/null +++ b/tests/domainsnapshotxml2xmlout/noparent_nodescription.xml @@ -0,0 +1,7 @@ +<domainsnapshot> + <name>my snap name</name> + <description>!@#$%^</description> + <state>running</state> + <creationTime>1272917631</creationTime> + <active>1</active> +</domainsnapshot> diff --git a/tests/domainsnapshotxml2xmlout/noparent_nodescription_noactive.xml b/tests/domainsnapshotxml2xmlout/noparent_nodescription_noactive.xml new file mode 100644 index 0000000..25b60f3 --- /dev/null +++ b/tests/domainsnapshotxml2xmlout/noparent_nodescription_noactive.xml @@ -0,0 +1,6 @@ +<domainsnapshot> + <name>my snap name</name> + <description>!@#$%^</description> + <state>running</state> + <creationTime>1272917631</creationTime> +</domainsnapshot> -- 1.6.6.1
This involved a few fixes. To start with, an virDomainSnapshot object is really tied to a domain, not a connection, so we have to generate a slightly different object so that we can get at self._dom for the object. Next, we had to "dummy" up an override piece of XML with a bogus argument that the function doesn't actually take. That's so that the generator places virDomainRevertToSnapshot underneath the correct class (namely, the virDomain class). Finally, we had to hand-implement the virDomainRevertToSnapshot implementation, ignoring the bogus pointer we are being passed. With all of this in place, I was able to successfully take a snapshot and revert to it using only the Python bindings. Signed-off-by: Chris Lalancette <clalance@redhat.com> --- python/generator.py | 26 +++++++++++++++++++++++--- python/libvirt-override-api.xml | 7 +++++++ python/libvirt-override.c | 23 +++++++++++++++++++++++ 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/python/generator.py b/python/generator.py index a243c82..d876df6 100755 --- a/python/generator.py +++ b/python/generator.py @@ -331,7 +331,7 @@ skip_impl = ( 'virNodeListDevices', 'virNodeDeviceListCaps', 'virConnectBaselineCPU', - 'virDomainSnapshotListNames', + 'virDomainRevertToSnapshot', ) @@ -385,6 +385,10 @@ skip_function = ( "virStorageVolGetConnect", ) +function_skip_index_one = ( + "virDomainRevertToSnapshot", +) + def print_function_wrapper(name, output, export, include): global py_types @@ -688,9 +692,13 @@ classes_destructors = { } class_skip_connect_impl = { - "virConnect" : True + "virConnect" : True, + "virDomainSnapshot": True, } +class_domain_impl = { + "virDomainSnapshot": True, +} functions_noexcept = { 'virDomainGetID': True, @@ -986,7 +994,7 @@ def buildWrappers(): info = (0, func, name, ret, args, file) function_classes[classe].append(info) elif name[0:3] == "vir" and len(args) >= 2 and args[1][1] == type \ - and file != "python_accessor": + and file != "python_accessor" and not name in function_skip_index_one: found = 1 func = nameFixup(name, classe, type, file) info = (1, func, name, ret, args, file) @@ -1128,6 +1136,8 @@ def buildWrappers(): "virStorageVol", "virNodeDevice", "virSecret","virStream", "virNWFilter" ]: classes.write(" def __init__(self, conn, _obj=None):\n") + elif classname in [ 'virDomainSnapshot' ]: + classes.write(" def __init__(self, dom, _obj=None):\n") else: classes.write(" def __init__(self, _obj=None):\n") if reference_keepers.has_key(classname): @@ -1142,6 +1152,8 @@ def buildWrappers(): classes.write(" self._conn = conn\n" + \ " if not isinstance(conn, virConnect):\n" + \ " self._conn = conn._conn\n") + elif classname in [ "virDomainSnapshot" ]: + classes.write(" self._dom = dom\n") classes.write(" if _obj != None:self._o = _obj;return\n") classes.write(" self._o = None\n\n"); destruct=None @@ -1158,6 +1170,10 @@ def buildWrappers(): classes.write(" def connect(self):\n") classes.write(" return self._conn\n\n") + if class_domain_impl.has_key(classname): + classes.write(" def domain(self):\n") + classes.write(" return self._dom\n\n") + flist = function_classes[classname] flist.sort(functionCompare) oldfile = "" @@ -1252,6 +1268,10 @@ def buildWrappers(): classes.write( " if ret is None:raise libvirtError('%s() failed', vol=self)\n" % (name)) + elif classname == "virDomainSnapshot": + classes.write( + " if ret is None:raise libvirtError('%s() failed', dom=self._dom)\n" % + (name)) else: classes.write( " if ret is None:raise libvirtError('%s() failed')\n" % diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 9ba8e4e..be28b40 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -277,5 +277,12 @@ <arg name='flags' type='unsigned int' info='flags, curently unused'/> <return type='str *' info='the list of Names of None in case of error'/> </function> + <function name='virDomainRevertToSnapshot' file='python'> + <info>revert the domain to the given snapshot</info> + <arg name='dom' type='virDomainPtr' info='dummy domain pointer'/> + <arg name='snap' type='virDomainSnapshotPtr' info='pointer to the snapshot'/> + <arg name='flags' type='unsigned int' info='flags, curently unused'/> + <return type='int' info="0 on success, -1 on error"/> + </function> </symbols> </api> diff --git a/python/libvirt-override.c b/python/libvirt-override.c index c9721f7..ad55940 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -988,6 +988,28 @@ libvirt_virDomainSnapshotListNames(PyObject *self ATTRIBUTE_UNUSED, } static PyObject * +libvirt_virDomainRevertToSnapshot(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) { + int c_retval; + virDomainSnapshotPtr snap; + PyObject *pyobj_snap; + PyObject *pyobj_dom; + int flags; + + if (!PyArg_ParseTuple(args, (char *)"OOi:virDomainRevertToSnapshot", &pyobj_dom, &pyobj_snap, &flags)) + return(NULL); + snap = (virDomainSnapshotPtr) PyvirDomainSnapshot_Get(pyobj_snap); + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virDomainRevertToSnapshot(snap, flags); + LIBVIRT_END_ALLOW_THREADS; + if (c_retval < 0) + return VIR_PY_INT_FAIL; + + return PyInt_FromLong(c_retval); +} + +static PyObject * libvirt_virDomainGetInfo(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { PyObject *py_retval; int c_retval; @@ -3527,6 +3549,7 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virConnectBaselineCPU", libvirt_virConnectBaselineCPU, METH_VARARGS, NULL}, {(char *) "virDomainGetJobInfo", libvirt_virDomainGetJobInfo, METH_VARARGS, NULL}, {(char *) "virDomainSnapshotListNames", libvirt_virDomainSnapshotListNames, METH_VARARGS, NULL}, + {(char *) "virDomainRevertToSnapshot", libvirt_virDomainRevertToSnapshot, METH_VARARGS, NULL}, {NULL, NULL, 0, NULL} }; -- 1.6.6.1
On 05/20/2010 11:14 AM, Chris Lalancette wrote:
This involved a few fixes. To start with, an virDomainSnapshot object is really tied to a domain, not a connection, so we have to generate a slightly different object so that we can get at self._dom for the object.
Next, we had to "dummy" up an override piece of XML with a bogus argument that the function doesn't actually take. That's so that the generator places virDomainRevertToSnapshot underneath the correct class (namely, the virDomain class).
Finally, we had to hand-implement the virDomainRevertToSnapshot implementation, ignoring the bogus pointer we are being passed.
With all of this in place, I was able to successfully take a snapshot and revert to it using only the Python bindings.
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- python/generator.py | 26 +++++++++++++++++++++++--- python/libvirt-override-api.xml | 7 +++++++ python/libvirt-override.c | 23 +++++++++++++++++++++++ 3 files changed, 53 insertions(+), 3 deletions(-)
diff --git a/python/generator.py b/python/generator.py index a243c82..d876df6 100755 --- a/python/generator.py +++ b/python/generator.py @@ -331,7 +331,7 @@ skip_impl = ( 'virNodeListDevices', 'virNodeDeviceListCaps', 'virConnectBaselineCPU', - 'virDomainSnapshotListNames', + 'virDomainRevertToSnapshot', )
@@ -385,6 +385,10 @@ skip_function = ( "virStorageVolGetConnect", )
+function_skip_index_one = ( + "virDomainRevertToSnapshot", +) +
def print_function_wrapper(name, output, export, include): global py_types @@ -688,9 +692,13 @@ classes_destructors = { }
class_skip_connect_impl = { - "virConnect" : True + "virConnect" : True, + "virDomainSnapshot": True, }
+class_domain_impl = { + "virDomainSnapshot": True, +}
functions_noexcept = { 'virDomainGetID': True, @@ -986,7 +994,7 @@ def buildWrappers(): info = (0, func, name, ret, args, file) function_classes[classe].append(info) elif name[0:3] == "vir" and len(args) >= 2 and args[1][1] == type \ - and file != "python_accessor": + and file != "python_accessor" and not name in function_skip_index_one: found = 1 func = nameFixup(name, classe, type, file) info = (1, func, name, ret, args, file) @@ -1128,6 +1136,8 @@ def buildWrappers(): "virStorageVol", "virNodeDevice", "virSecret","virStream", "virNWFilter" ]: classes.write(" def __init__(self, conn, _obj=None):\n") + elif classname in [ 'virDomainSnapshot' ]: + classes.write(" def __init__(self, dom, _obj=None):\n") else: classes.write(" def __init__(self, _obj=None):\n") if reference_keepers.has_key(classname): @@ -1142,6 +1152,8 @@ def buildWrappers(): classes.write(" self._conn = conn\n" + \ " if not isinstance(conn, virConnect):\n" + \ " self._conn = conn._conn\n") + elif classname in [ "virDomainSnapshot" ]: + classes.write(" self._dom = dom\n") classes.write(" if _obj != None:self._o = _obj;return\n") classes.write(" self._o = None\n\n"); destruct=None @@ -1158,6 +1170,10 @@ def buildWrappers(): classes.write(" def connect(self):\n") classes.write(" return self._conn\n\n")
+ if class_domain_impl.has_key(classname): + classes.write(" def domain(self):\n") + classes.write(" return self._dom\n\n") + flist = function_classes[classname] flist.sort(functionCompare) oldfile = "" @@ -1252,6 +1268,10 @@ def buildWrappers(): classes.write( " if ret is None:raise libvirtError('%s() failed', vol=self)\n" % (name)) + elif classname == "virDomainSnapshot": + classes.write( + " if ret is None:raise libvirtError('%s() failed', dom=self._dom)\n" % + (name)) else: classes.write( " if ret is None:raise libvirtError('%s() failed')\n" % diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 9ba8e4e..be28b40 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -277,5 +277,12 @@ <arg name='flags' type='unsigned int' info='flags, curently unused'/> <return type='str *' info='the list of Names of None in case of error'/> </function> + <function name='virDomainRevertToSnapshot' file='python'> + <info>revert the domain to the given snapshot</info> + <arg name='dom' type='virDomainPtr' info='dummy domain pointer'/> + <arg name='snap' type='virDomainSnapshotPtr' info='pointer to the snapshot'/> + <arg name='flags' type='unsigned int' info='flags, curently unused'/> + <return type='int' info="0 on success, -1 on error"/> + </function> </symbols> </api> diff --git a/python/libvirt-override.c b/python/libvirt-override.c index c9721f7..ad55940 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -988,6 +988,28 @@ libvirt_virDomainSnapshotListNames(PyObject *self ATTRIBUTE_UNUSED, }
static PyObject * +libvirt_virDomainRevertToSnapshot(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) { + int c_retval; + virDomainSnapshotPtr snap; + PyObject *pyobj_snap; + PyObject *pyobj_dom; + int flags; + + if (!PyArg_ParseTuple(args, (char *)"OOi:virDomainRevertToSnapshot", &pyobj_dom, &pyobj_snap, &flags)) + return(NULL); + snap = (virDomainSnapshotPtr) PyvirDomainSnapshot_Get(pyobj_snap); + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virDomainRevertToSnapshot(snap, flags); + LIBVIRT_END_ALLOW_THREADS; + if (c_retval < 0) + return VIR_PY_INT_FAIL; + + return PyInt_FromLong(c_retval); +} + +static PyObject * libvirt_virDomainGetInfo(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { PyObject *py_retval; int c_retval; @@ -3527,6 +3549,7 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virConnectBaselineCPU", libvirt_virConnectBaselineCPU, METH_VARARGS, NULL}, {(char *) "virDomainGetJobInfo", libvirt_virDomainGetJobInfo, METH_VARARGS, NULL}, {(char *) "virDomainSnapshotListNames", libvirt_virDomainSnapshotListNames, METH_VARARGS, NULL}, + {(char *) "virDomainRevertToSnapshot", libvirt_virDomainRevertToSnapshot, METH_VARARGS, NULL}, {NULL, NULL, 0, NULL} };
ACK, change looks fine. Might be worth checking the python bindings diff though, the generator is fragile and changes can sometimes muck up existing APIs. - Cole
On 05/20/2010 12:34 PM, Cole Robinson wrote:
On 05/20/2010 11:14 AM, Chris Lalancette wrote:
This involved a few fixes. To start with, an virDomainSnapshot object is really tied to a domain, not a connection, so we have to generate a slightly different object so that we can get at self._dom for the object.
Next, we had to "dummy" up an override piece of XML with a bogus argument that the function doesn't actually take. That's so that the generator places virDomainRevertToSnapshot underneath the correct class (namely, the virDomain class).
Finally, we had to hand-implement the virDomainRevertToSnapshot implementation, ignoring the bogus pointer we are being passed.
With all of this in place, I was able to successfully take a snapshot and revert to it using only the Python bindings.
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- python/generator.py | 26 +++++++++++++++++++++++--- python/libvirt-override-api.xml | 7 +++++++ python/libvirt-override.c | 23 +++++++++++++++++++++++ 3 files changed, 53 insertions(+), 3 deletions(-)
diff --git a/python/generator.py b/python/generator.py index a243c82..d876df6 100755 --- a/python/generator.py +++ b/python/generator.py @@ -331,7 +331,7 @@ skip_impl = ( 'virNodeListDevices', 'virNodeDeviceListCaps', 'virConnectBaselineCPU', - 'virDomainSnapshotListNames', + 'virDomainRevertToSnapshot', )
@@ -385,6 +385,10 @@ skip_function = ( "virStorageVolGetConnect", )
+function_skip_index_one = ( + "virDomainRevertToSnapshot", +) +
def print_function_wrapper(name, output, export, include): global py_types @@ -688,9 +692,13 @@ classes_destructors = { }
class_skip_connect_impl = { - "virConnect" : True + "virConnect" : True, + "virDomainSnapshot": True, }
+class_domain_impl = { + "virDomainSnapshot": True, +}
functions_noexcept = { 'virDomainGetID': True, @@ -986,7 +994,7 @@ def buildWrappers(): info = (0, func, name, ret, args, file) function_classes[classe].append(info) elif name[0:3] == "vir" and len(args) >= 2 and args[1][1] == type \ - and file != "python_accessor": + and file != "python_accessor" and not name in function_skip_index_one: found = 1 func = nameFixup(name, classe, type, file) info = (1, func, name, ret, args, file) @@ -1128,6 +1136,8 @@ def buildWrappers(): "virStorageVol", "virNodeDevice", "virSecret","virStream", "virNWFilter" ]: classes.write(" def __init__(self, conn, _obj=None):\n") + elif classname in [ 'virDomainSnapshot' ]: + classes.write(" def __init__(self, dom, _obj=None):\n") else: classes.write(" def __init__(self, _obj=None):\n") if reference_keepers.has_key(classname): @@ -1142,6 +1152,8 @@ def buildWrappers(): classes.write(" self._conn = conn\n" + \ " if not isinstance(conn, virConnect):\n" + \ " self._conn = conn._conn\n") + elif classname in [ "virDomainSnapshot" ]: + classes.write(" self._dom = dom\n") classes.write(" if _obj != None:self._o = _obj;return\n") classes.write(" self._o = None\n\n"); destruct=None @@ -1158,6 +1170,10 @@ def buildWrappers(): classes.write(" def connect(self):\n") classes.write(" return self._conn\n\n")
+ if class_domain_impl.has_key(classname): + classes.write(" def domain(self):\n") + classes.write(" return self._dom\n\n") + flist = function_classes[classname] flist.sort(functionCompare) oldfile = "" @@ -1252,6 +1268,10 @@ def buildWrappers(): classes.write( " if ret is None:raise libvirtError('%s() failed', vol=self)\n" % (name)) + elif classname == "virDomainSnapshot": + classes.write( + " if ret is None:raise libvirtError('%s() failed', dom=self._dom)\n" % + (name)) else: classes.write( " if ret is None:raise libvirtError('%s() failed')\n" % diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 9ba8e4e..be28b40 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -277,5 +277,12 @@ <arg name='flags' type='unsigned int' info='flags, curently unused'/> <return type='str *' info='the list of Names of None in case of error'/> </function> + <function name='virDomainRevertToSnapshot' file='python'> + <info>revert the domain to the given snapshot</info> + <arg name='dom' type='virDomainPtr' info='dummy domain pointer'/> + <arg name='snap' type='virDomainSnapshotPtr' info='pointer to the snapshot'/> + <arg name='flags' type='unsigned int' info='flags, curently unused'/> + <return type='int' info="0 on success, -1 on error"/> + </function> </symbols> </api> diff --git a/python/libvirt-override.c b/python/libvirt-override.c index c9721f7..ad55940 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -988,6 +988,28 @@ libvirt_virDomainSnapshotListNames(PyObject *self ATTRIBUTE_UNUSED, }
static PyObject * +libvirt_virDomainRevertToSnapshot(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) { + int c_retval; + virDomainSnapshotPtr snap; + PyObject *pyobj_snap; + PyObject *pyobj_dom; + int flags; + + if (!PyArg_ParseTuple(args, (char *)"OOi:virDomainRevertToSnapshot", &pyobj_dom, &pyobj_snap, &flags)) + return(NULL); + snap = (virDomainSnapshotPtr) PyvirDomainSnapshot_Get(pyobj_snap); + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virDomainRevertToSnapshot(snap, flags); + LIBVIRT_END_ALLOW_THREADS; + if (c_retval < 0) + return VIR_PY_INT_FAIL; + + return PyInt_FromLong(c_retval); +} + +static PyObject * libvirt_virDomainGetInfo(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { PyObject *py_retval; int c_retval; @@ -3527,6 +3549,7 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virConnectBaselineCPU", libvirt_virConnectBaselineCPU, METH_VARARGS, NULL}, {(char *) "virDomainGetJobInfo", libvirt_virDomainGetJobInfo, METH_VARARGS, NULL}, {(char *) "virDomainSnapshotListNames", libvirt_virDomainSnapshotListNames, METH_VARARGS, NULL}, + {(char *) "virDomainRevertToSnapshot", libvirt_virDomainRevertToSnapshot, METH_VARARGS, NULL}, {NULL, NULL, 0, NULL} };
ACK, change looks fine. Might be worth checking the python bindings diff though, the generator is fragile and changes can sometimes muck up existing APIs.
Excellent thought. I did that across libvirt.c, libvirt-export.c, libvirt.h and libvirt.py, and the only changes were the ones I expected. Thanks for the heads up. I've now pushed this patch. -- Chris Lalancette
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- src/qemu/qemu_driver.c | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f2139c0..6df2356 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6100,8 +6100,7 @@ static int qemudDomainRestore(virConnectPtr conn, if (!(vm = virDomainAssignDef(driver->caps, &driver->domains, def, true))) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("failed to assign new VM")); + /* virDomainAssignDef already set the error */ goto cleanup; } def = NULL; @@ -9909,8 +9908,7 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn, if (!(vm = virDomainAssignDef(driver->caps, &driver->domains, def, true))) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("failed to assign new VM")); + /* virDomainAssignDef already set the error */ goto cleanup; } def = NULL; @@ -10140,8 +10138,7 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn, if (!(vm = virDomainAssignDef(driver->caps, &driver->domains, def, true))) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("failed to assign new VM")); + /* virDomainAssignDef already set the error */ goto cleanup; } def = NULL; -- 1.6.6.1
On 05/20/2010 11:14 AM, Chris Lalancette wrote:
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- src/qemu/qemu_driver.c | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f2139c0..6df2356 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6100,8 +6100,7 @@ static int qemudDomainRestore(virConnectPtr conn, if (!(vm = virDomainAssignDef(driver->caps, &driver->domains, def, true))) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("failed to assign new VM")); + /* virDomainAssignDef already set the error */ goto cleanup; } def = NULL; @@ -9909,8 +9908,7 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn, if (!(vm = virDomainAssignDef(driver->caps, &driver->domains, def, true))) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("failed to assign new VM")); + /* virDomainAssignDef already set the error */ goto cleanup; } def = NULL; @@ -10140,8 +10138,7 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn, if (!(vm = virDomainAssignDef(driver->caps, &driver->domains, def, true))) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("failed to assign new VM")); + /* virDomainAssignDef already set the error */ goto cleanup; } def = NULL;
ACK to the change, but I don't think it needs a comment, I think all virDomain functions that can fail report their error. - Cole
On 05/20/2010 12:24 PM, Cole Robinson wrote:
On 05/20/2010 11:14 AM, Chris Lalancette wrote:
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- src/qemu/qemu_driver.c | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f2139c0..6df2356 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6100,8 +6100,7 @@ static int qemudDomainRestore(virConnectPtr conn, if (!(vm = virDomainAssignDef(driver->caps, &driver->domains, def, true))) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("failed to assign new VM")); + /* virDomainAssignDef already set the error */ goto cleanup; } def = NULL; @@ -9909,8 +9908,7 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn, if (!(vm = virDomainAssignDef(driver->caps, &driver->domains, def, true))) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("failed to assign new VM")); + /* virDomainAssignDef already set the error */ goto cleanup; } def = NULL; @@ -10140,8 +10138,7 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn, if (!(vm = virDomainAssignDef(driver->caps, &driver->domains, def, true))) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("failed to assign new VM")); + /* virDomainAssignDef already set the error */ goto cleanup; } def = NULL;
ACK to the change, but I don't think it needs a comment, I think all virDomain functions that can fail report their error.
That's true. The thing is that there isn't a clear policy on this, so I like to put the comments in to remind me when I'm looking back at the code later (and wondering why we aren't setting the error). But I'm fine either way. -- Chris Lalancette
On 05/20/2010 12:24 PM, Cole Robinson wrote:
On 05/20/2010 11:14 AM, Chris Lalancette wrote:
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- src/qemu/qemu_driver.c | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f2139c0..6df2356 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6100,8 +6100,7 @@ static int qemudDomainRestore(virConnectPtr conn, if (!(vm = virDomainAssignDef(driver->caps, &driver->domains, def, true))) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("failed to assign new VM")); + /* virDomainAssignDef already set the error */ goto cleanup; } def = NULL; @@ -9909,8 +9908,7 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn, if (!(vm = virDomainAssignDef(driver->caps, &driver->domains, def, true))) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("failed to assign new VM")); + /* virDomainAssignDef already set the error */ goto cleanup; } def = NULL; @@ -10140,8 +10138,7 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn, if (!(vm = virDomainAssignDef(driver->caps, &driver->domains, def, true))) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("failed to assign new VM")); + /* virDomainAssignDef already set the error */ goto cleanup; } def = NULL;
ACK to the change, but I don't think it needs a comment, I think all virDomain functions that can fail report their error.
Oops, I had forgotten to push this one. I pushed it with the comments for now; if people are really opposed, we can strip them out later. -- Chris Lalancette
This patch is essentially a revert of commit: 180ca598c4517012014d226c78068d4b38ff9e24 The problem ends up being that virGetHostname is *too* clever, and is causing migration problems. In particular, on machines with dynamic networks, the hostname of the machine generally gets mapped to 127.0.0.1 in /etc/hosts. This means that virGetHostname will do gethostname (and get back something like foo.example.com), then resolve it to 127.0.0.1, and then pass that back to the source of the destination, which is bogus. By being less clever and just using gethostname, we avoid the problem. Signed-off-by: Chris Lalancette <clalance@redhat.com> --- .x-sc_prohibit_gethostname | 2 -- Makefile.am | 1 - cfg.mk | 5 ----- src/qemu/qemu_driver.c | 12 ++++++------ 4 files changed, 6 insertions(+), 14 deletions(-) delete mode 100644 .x-sc_prohibit_gethostname diff --git a/.x-sc_prohibit_gethostname b/.x-sc_prohibit_gethostname deleted file mode 100644 index e7acb03..0000000 --- a/.x-sc_prohibit_gethostname +++ /dev/null @@ -1,2 +0,0 @@ -^src/util/util\.c$ -^ChangeLog-old$ diff --git a/Makefile.am b/Makefile.am index 286b13b..9f2ae9e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -26,7 +26,6 @@ EXTRA_DIST = \ .x-sc_m4_quote_check \ .x-sc_prohibit_asprintf \ .x-sc_prohibit_gethostby \ - .x-sc_prohibit_gethostname \ .x-sc_prohibit_gettext_noop \ .x-sc_prohibit_have_config_h \ .x-sc_prohibit_HAVE_MBRTOWC \ diff --git a/cfg.mk b/cfg.mk index 1ef73e2..451ce37 100644 --- a/cfg.mk +++ b/cfg.mk @@ -253,11 +253,6 @@ sc_prohibit_readlink: halt='use virFileResolveLink, not readlink' \ $(_sc_search_regexp) -sc_prohibit_gethostname: - @prohibit='gethostname *\(' \ - halt='use virGetHostname, not gethostname' \ - $(_sc_search_regexp) - sc_prohibit_gettext_noop: @prohibit='gettext_noop *\(' \ halt='use N_, not gettext_noop' \ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6df2356..54703dd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10022,12 +10022,11 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn, virDomainDefPtr def = NULL; virDomainObjPtr vm = NULL; int this_port; - char *hostname; + char hostname[HOST_NAME_MAX+1]; char migrateFrom [64]; const char *p; virDomainEventPtr event = NULL; int ret = -1; - int internalret; *uri_out = NULL; @@ -10062,8 +10061,11 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn, if (port == QEMUD_MIGRATION_NUM_PORTS) port = 0; /* Get hostname */ - if ((hostname = virGetHostnameLocalhost(0)) == NULL) + if (gethostname(hostname, HOST_NAME_MAX) < 0) { + virReportSystemError(errno, "%s", + _("failed to determine host name")); goto cleanup; + } /* XXX this really should have been a properly well-formed * URI, but we can't add in tcp:// now without breaking @@ -10071,9 +10073,7 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn, * new targets accept both syntaxes though. */ /* Caller frees */ - internalret = virAsprintf(uri_out, "tcp:%s:%d", hostname, this_port); - VIR_FREE(hostname); - if (internalret < 0) { + if (virAsprintf(uri_out, "tcp:%s:%d", hostname, this_port) < 0) { virReportOOMError(); goto cleanup; } -- 1.6.6.1
On Thu, May 20, 2010 at 11:14:05AM -0400, Chris Lalancette wrote:
This patch is essentially a revert of commit: 180ca598c4517012014d226c78068d4b38ff9e24
The problem ends up being that virGetHostname is *too* clever, and is causing migration problems. In particular, on machines with dynamic networks, the hostname of the machine generally gets mapped to 127.0.0.1 in /etc/hosts. This means that virGetHostname will do gethostname (and get back something like foo.example.com), then resolve it to 127.0.0.1, and then pass that back to the source of the destination, which is bogus. By being less clever and just using gethostname, we avoid the problem.
Why don't we fix virGetHostname(), so all callers benefit instead. The idea behind calling getaddrinfo() on the result of gethostname() was to ensure you get the FQDN, instead of just a plain hostname. I'd say we could be more clever in that respect, eg hostname = gethostname() if (STRPREFIX(hostname, "localhost")) { ...report error... } if (strchr(hostname, '.')) return hostname; ...canonicalize hostname... if (STRPEFIX(newhostname, "localhost")) return hostname So we'd normally be returning the plain result of gethostname() unless we saw it needed canonicalization, and if canonicalization results in it turning into 'localhost', just use the non-canonical hostname instead Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On 05/20/2010 11:14 AM, Chris Lalancette wrote:
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- docs/schemas/domainsnapshot.rng | 53 ++++++++++++++++++++ tests/Makefile.am | 6 ++- tests/domainsnapshotschematest | 10 ++++ tests/domainsnapshotxml2xmlin/description_only.xml | 3 + tests/domainsnapshotxml2xmlin/empty.xml | 1 + .../name_and_description.xml | 4 ++ tests/domainsnapshotxml2xmlin/name_only.xml | 3 + tests/domainsnapshotxml2xmlout/all_parameters.xml | 13 +++++ tests/domainsnapshotxml2xmlout/noparent.xml | 9 +++ .../noparent_nodescription.xml | 7 +++ .../noparent_nodescription_noactive.xml | 6 ++ 11 files changed, 114 insertions(+), 1 deletions(-) create mode 100644 docs/schemas/domainsnapshot.rng create mode 100755 tests/domainsnapshotschematest create mode 100644 tests/domainsnapshotxml2xmlin/description_only.xml create mode 100644 tests/domainsnapshotxml2xmlin/empty.xml create mode 100644 tests/domainsnapshotxml2xmlin/name_and_description.xml create mode 100644 tests/domainsnapshotxml2xmlin/name_only.xml create mode 100644 tests/domainsnapshotxml2xmlout/all_parameters.xml create mode 100644 tests/domainsnapshotxml2xmlout/noparent.xml create mode 100644 tests/domainsnapshotxml2xmlout/noparent_nodescription.xml create mode 100644 tests/domainsnapshotxml2xmlout/noparent_nodescription_noactive.xml
Looks okay to me. ACK - Cole
On 05/20/2010 12:19 PM, Cole Robinson wrote:
On 05/20/2010 11:14 AM, Chris Lalancette wrote:
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- docs/schemas/domainsnapshot.rng | 53 ++++++++++++++++++++ tests/Makefile.am | 6 ++- tests/domainsnapshotschematest | 10 ++++ tests/domainsnapshotxml2xmlin/description_only.xml | 3 + tests/domainsnapshotxml2xmlin/empty.xml | 1 + .../name_and_description.xml | 4 ++ tests/domainsnapshotxml2xmlin/name_only.xml | 3 + tests/domainsnapshotxml2xmlout/all_parameters.xml | 13 +++++ tests/domainsnapshotxml2xmlout/noparent.xml | 9 +++ .../noparent_nodescription.xml | 7 +++ .../noparent_nodescription_noactive.xml | 6 ++ 11 files changed, 114 insertions(+), 1 deletions(-) create mode 100644 docs/schemas/domainsnapshot.rng create mode 100755 tests/domainsnapshotschematest create mode 100644 tests/domainsnapshotxml2xmlin/description_only.xml create mode 100644 tests/domainsnapshotxml2xmlin/empty.xml create mode 100644 tests/domainsnapshotxml2xmlin/name_and_description.xml create mode 100644 tests/domainsnapshotxml2xmlin/name_only.xml create mode 100644 tests/domainsnapshotxml2xmlout/all_parameters.xml create mode 100644 tests/domainsnapshotxml2xmlout/noparent.xml create mode 100644 tests/domainsnapshotxml2xmlout/noparent_nodescription.xml create mode 100644 tests/domainsnapshotxml2xmlout/noparent_nodescription_noactive.xml
Looks okay to me. ACK
Thanks, I've pushed this patch now. -- Chris Lalancette
participants (3)
-
Chris Lalancette -
Cole Robinson -
Daniel P. Berrange