
To clone a VM, try the following command: POST /vms/<vm-name>/clone. This is a work in progress! It's not finished yet and there are bugs! I'm only sending it now to get early feedback. I'm aware of the following issues: - BUG: cannot delete a recently cloned VM. This is because the clone function doesn't refresh the VM storage pools. I already started fixing this. - The storage volumes handling is not implemented. The current implementation should only work for DIR and NFS (well, it doesn't work completely because of the bug described above...). - The mockmodel implementation is missing. - The tests are missing. Crístian Viana (5): Make function "randomMAC" public Register Kimchi's namespace when updating XML Allow to update XML attribute in "xml_item_update" Render different types of data in generate_action_handler Add feature to clone VMs docs/API.md | 5 ++ src/kimchi/control/base.py | 37 ++++++++++---- src/kimchi/control/host.py | 18 ++----- src/kimchi/control/vms.py | 1 + src/kimchi/i18n.py | 1 + src/kimchi/model/vmifaces.py | 18 +++---- src/kimchi/model/vms.py | 116 ++++++++++++++++++++++++++++++++++++++++++- src/kimchi/xmlutils.py | 15 ++++-- 8 files changed, 172 insertions(+), 39 deletions(-) -- 1.9.3

A function from a different module might require a new random MAC so the function "randomMAC", defined inside another function in VMIfacesModel, should be declared in a public scope. Move the definition of "randomMAC" to outside its outer function in order to make it public. Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/model/vmifaces.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/kimchi/model/vmifaces.py b/src/kimchi/model/vmifaces.py index 66b3827..721bf14 100644 --- a/src/kimchi/model/vmifaces.py +++ b/src/kimchi/model/vmifaces.py @@ -38,13 +38,6 @@ class VMIfacesModel(object): return macs def create(self, vm, params): - def randomMAC(): - mac = [0x52, 0x54, 0x00, - random.randint(0x00, 0x7f), - random.randint(0x00, 0xff), - random.randint(0x00, 0xff)] - return ':'.join(map(lambda x: "%02x" % x, mac)) - conn = self.conn.get() networks = conn.listNetworks() + conn.listDefinedNetworks() @@ -59,11 +52,10 @@ class VMIfacesModel(object): macs = (iface.mac.get('address') for iface in self.get_vmifaces(vm, self.conn)) - mac = randomMAC() while True: + mac = VMIfacesModel.random_mac() if mac not in macs: break - mac = randomMAC() children = [E.mac(address=mac)] ("network" in params.keys() and @@ -86,6 +78,14 @@ class VMIfacesModel(object): return root.devices.findall("interface") + @staticmethod + def random_mac(): + mac = [0x52, 0x54, 0x00, + random.randint(0x00, 0x7f), + random.randint(0x00, 0xff), + random.randint(0x00, 0xff)] + return ':'.join(map(lambda x: u'%02x' % x, mac)) + class VMIfaceModel(object): def __init__(self, **kargs): -- 1.9.3

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 10/17/2014 05:26 AM, Crístian Viana wrote:
A function from a different module might require a new random MAC so the function "randomMAC", defined inside another function in VMIfacesModel, should be declared in a public scope.
Move the definition of "randomMAC" to outside its outer function in order to make it public.
Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/model/vmifaces.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/kimchi/model/vmifaces.py b/src/kimchi/model/vmifaces.py index 66b3827..721bf14 100644 --- a/src/kimchi/model/vmifaces.py +++ b/src/kimchi/model/vmifaces.py @@ -38,13 +38,6 @@ class VMIfacesModel(object): return macs
def create(self, vm, params): - def randomMAC(): - mac = [0x52, 0x54, 0x00, - random.randint(0x00, 0x7f), - random.randint(0x00, 0xff), - random.randint(0x00, 0xff)] - return ':'.join(map(lambda x: "%02x" % x, mac)) - conn = self.conn.get() networks = conn.listNetworks() + conn.listDefinedNetworks()
@@ -59,11 +52,10 @@ class VMIfacesModel(object): macs = (iface.mac.get('address') for iface in self.get_vmifaces(vm, self.conn))
- mac = randomMAC() while True: + mac = VMIfacesModel.random_mac() if mac not in macs: break - mac = randomMAC()
children = [E.mac(address=mac)] ("network" in params.keys() and @@ -86,6 +78,14 @@ class VMIfacesModel(object):
return root.devices.findall("interface")
+ @staticmethod + def random_mac(): + mac = [0x52, 0x54, 0x00, + random.randint(0x00, 0x7f), + random.randint(0x00, 0xff), + random.randint(0x00, 0xff)] + return ':'.join(map(lambda x: u'%02x' % x, mac)) +
class VMIfaceModel(object): def __init__(self, **kargs):

A domain's metadata content has a namespace assigned to it but that value is removed when the XML is updated using the function "kimchi.xmlutils.xml_item_update" because it doesn't know about the namespaces. Register the namespace "kimchi" with the appropriate value when updating XML using the function "kimchi.xmlutils.xml_item_update". Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/xmlutils.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/kimchi/xmlutils.py b/src/kimchi/xmlutils.py index 00a9d55..8b19a20 100644 --- a/src/kimchi/xmlutils.py +++ b/src/kimchi/xmlutils.py @@ -20,9 +20,10 @@ import libxml2 from lxml import objectify - from xml.etree import ElementTree +import kimchi.model + def xpath_get_text(xml, expr): doc = libxml2.parseDoc(xml) @@ -34,6 +35,9 @@ def xpath_get_text(xml, expr): def xml_item_update(xml, xpath, value): + ElementTree.register_namespace(kimchi.model.utils.KIMCHI_NAMESPACE, + kimchi.model.utils.KIMCHI_META_URL) + root = ElementTree.fromstring(xml) item = root.find(xpath) item.text = value -- 1.9.3

On 10/17/2014 05:26 AM, Crístian Viana wrote:
A domain's metadata content has a namespace assigned to it but that value is removed when the XML is updated using the function "kimchi.xmlutils.xml_item_update" because it doesn't know about the namespaces.
Register the namespace "kimchi" with the appropriate value when updating XML using the function "kimchi.xmlutils.xml_item_update".
Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/xmlutils.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/kimchi/xmlutils.py b/src/kimchi/xmlutils.py index 00a9d55..8b19a20 100644 --- a/src/kimchi/xmlutils.py +++ b/src/kimchi/xmlutils.py @@ -20,9 +20,10 @@ import libxml2 from lxml import objectify
- from xml.etree import ElementTree
+import kimchi.model +
Instead of importing the whole module, import only the constants you need here.
def xpath_get_text(xml, expr): doc = libxml2.parseDoc(xml) @@ -34,6 +35,9 @@ def xpath_get_text(xml, expr):
def xml_item_update(xml, xpath, value): + ElementTree.register_namespace(kimchi.model.utils.KIMCHI_NAMESPACE, + kimchi.model.utils.KIMCHI_META_URL) + root = ElementTree.fromstring(xml) item = root.find(xpath) item.text = value

On 21-10-2014 16:58, Aline Manera wrote:
+import kimchi.model +
Instead of importing the whole module, import only the constants you need here.
If I import anything longer than kimchi.model.* (e.g. kimchi.model.utils), I get a cyclic import error. And in order to actually solve that, we'd have to do a big refactoring on the code. Importing kimchi.model seemed much simpler for now. Here's the error I get when I import only the constants instead of the whole module: $ git diff diff --git a/src/kimchi/xmlutils/utils.py b/src/kimchi/xmlutils/utils.py index fcb6b14..d40664e 100644 --- a/src/kimchi/xmlutils/utils.py +++ b/src/kimchi/xmlutils/utils.py @@ -22,7 +22,7 @@ from lxml import objectify from xml.etree import ElementTree -import kimchi.model +from kimchi.model.utils import KIMCHI_NAMESPACE, KIMCHI_META_URL def xpath_get_text(xml, expr): @@ -35,8 +35,7 @@ def xpath_get_text(xml, expr): def xml_item_update(xml, xpath, value, attr=None): - ElementTree.register_namespace(kimchi.model.utils.KIMCHI_NAMESPACE, - kimchi.model.utils.KIMCHI_META_URL) + ElementTree.register_namespace(KIMCHI_NAMESPACE, KIMCHI_META_URL) root = ElementTree.fromstring(xml) item = root.find(xpath) $ sudo src/kimchid Traceback (most recent call last): File "src/kimchid", line 28, in <module> import kimchi.server File "/home/vianac/LTC/kimchi/src/kimchi/server.py", line 26, in <module> from kimchi import auth File "/home/vianac/LTC/kimchi/src/kimchi/auth.py", line 33, in <module> from kimchi import template File "/home/vianac/LTC/kimchi/src/kimchi/template.py", line 25, in <module> from kimchi.config import paths File "/home/vianac/LTC/kimchi/src/kimchi/config.py", line 30, in <module> from kimchi.xmlutils.utils import xpath_get_text File "/home/vianac/LTC/kimchi/src/kimchi/xmlutils/utils.py", line 25, in <module> from kimchi.model.utils import KIMCHI_NAMESPACE, KIMCHI_META_URL File "/home/vianac/LTC/kimchi/src/kimchi/model/utils.py", line 21, in <module> from kimchi.exception import OperationFailed File "/home/vianac/LTC/kimchi/src/kimchi/exception.py", line 25, in <module> from kimchi.template import get_lang, validate_language ImportError: cannot import name get_lang I had to do this on patch 5/5 as well, for the same reason. If you have a better suggestion (which does not require refactoring most classes), please let me know.

On 10/21/2014 05:17 PM, Crístian Viana wrote:
On 21-10-2014 16:58, Aline Manera wrote:
+import kimchi.model +
Instead of importing the whole module, import only the constants you need here.
If I import anything longer than kimchi.model.* (e.g. kimchi.model.utils), I get a cyclic import error. And in order to actually solve that, we'd have to do a big refactoring on the code. Importing kimchi.model seemed much simpler for now.
Here's the error I get when I import only the constants instead of the whole module:
$ git diff diff --git a/src/kimchi/xmlutils/utils.py b/src/kimchi/xmlutils/utils.py index fcb6b14..d40664e 100644 --- a/src/kimchi/xmlutils/utils.py +++ b/src/kimchi/xmlutils/utils.py @@ -22,7 +22,7 @@ from lxml import objectify
from xml.etree import ElementTree
-import kimchi.model +from kimchi.model.utils import KIMCHI_NAMESPACE, KIMCHI_META_URL
def xpath_get_text(xml, expr): @@ -35,8 +35,7 @@ def xpath_get_text(xml, expr):
def xml_item_update(xml, xpath, value, attr=None): - ElementTree.register_namespace(kimchi.model.utils.KIMCHI_NAMESPACE, - kimchi.model.utils.KIMCHI_META_URL) + ElementTree.register_namespace(KIMCHI_NAMESPACE, KIMCHI_META_URL)
root = ElementTree.fromstring(xml) item = root.find(xpath)
$ sudo src/kimchid Traceback (most recent call last): File "src/kimchid", line 28, in <module> import kimchi.server File "/home/vianac/LTC/kimchi/src/kimchi/server.py", line 26, in <module> from kimchi import auth File "/home/vianac/LTC/kimchi/src/kimchi/auth.py", line 33, in <module> from kimchi import template File "/home/vianac/LTC/kimchi/src/kimchi/template.py", line 25, in <module> from kimchi.config import paths File "/home/vianac/LTC/kimchi/src/kimchi/config.py", line 30, in <module> from kimchi.xmlutils.utils import xpath_get_text File "/home/vianac/LTC/kimchi/src/kimchi/xmlutils/utils.py", line 25, in <module> from kimchi.model.utils import KIMCHI_NAMESPACE, KIMCHI_META_URL File "/home/vianac/LTC/kimchi/src/kimchi/model/utils.py", line 21, in <module> from kimchi.exception import OperationFailed File "/home/vianac/LTC/kimchi/src/kimchi/exception.py", line 25, in <module> from kimchi.template import get_lang, validate_language ImportError: cannot import name get_lang
I had to do this on patch 5/5 as well, for the same reason.
If you have a better suggestion (which does not require refactoring most classes), please let me know.
Hmm... got it I think it can be solved when all XML manipulation is on xmlutils module. But keep it as you did by now.

The function "xml_item_update" only updates the content of any given tag, filtered by an XPath expression. However, one might want to update one of the tag's attributes instead of its content. Add an optional parameter to "xml_item_update" which sets the attribute name to be updated; if not set, the function will behave as before, i.e. update the tag's content. Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/xmlutils.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/kimchi/xmlutils.py b/src/kimchi/xmlutils.py index 8b19a20..ee56203 100644 --- a/src/kimchi/xmlutils.py +++ b/src/kimchi/xmlutils.py @@ -34,13 +34,18 @@ def xpath_get_text(xml, expr): return ret -def xml_item_update(xml, xpath, value): +def xml_item_update(xml, xpath, value, attr=None): ElementTree.register_namespace(kimchi.model.utils.KIMCHI_NAMESPACE, kimchi.model.utils.KIMCHI_META_URL) root = ElementTree.fromstring(xml) item = root.find(xpath) - item.text = value + + if attr is None: + item.text = value + else: + item.set(attr, value) + return ElementTree.tostring(root, encoding="utf-8") -- 1.9.3

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 10/17/2014 05:26 AM, Crístian Viana wrote:
The function "xml_item_update" only updates the content of any given tag, filtered by an XPath expression. However, one might want to update one of the tag's attributes instead of its content.
Add an optional parameter to "xml_item_update" which sets the attribute name to be updated; if not set, the function will behave as before, i.e. update the tag's content.
Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/xmlutils.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/kimchi/xmlutils.py b/src/kimchi/xmlutils.py index 8b19a20..ee56203 100644 --- a/src/kimchi/xmlutils.py +++ b/src/kimchi/xmlutils.py @@ -34,13 +34,18 @@ def xpath_get_text(xml, expr): return ret
-def xml_item_update(xml, xpath, value): +def xml_item_update(xml, xpath, value, attr=None): ElementTree.register_namespace(kimchi.model.utils.KIMCHI_NAMESPACE, kimchi.model.utils.KIMCHI_META_URL)
root = ElementTree.fromstring(xml) item = root.find(xpath) - item.text = value + + if attr is None: + item.text = value + else: + item.set(attr, value) + return ElementTree.tostring(root, encoding="utf-8")

The function "generate_action_handler" creates a function which handles a string being returned by the underlying action function. However, in some cases, another type of element might be returned. The action "swupdate", for example, doesn't use "generate_action_handler" because it returns a Task instead of a string. Make the function "generate_action_handler" more modular so it can process different types of data. Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/control/base.py | 37 +++++++++++++++++++++++++++---------- src/kimchi/control/host.py | 18 +++--------------- 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index 1eb10bd..60db1df 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -65,6 +65,29 @@ class Resource(object): raise cherrypy.HTTPRedirect(self.uri_fmt % tuple(uri_params), code) def generate_action_handler(self, action_name, action_args=None): + def _render_element(self, ident): + self._redirect(ident) + uri_params = [] + for arg in self.model_args: + if arg is None: + arg = '' + uri_params.append(urllib2.quote(arg.encode('utf-8'), + safe="")) + raise internal_redirect(self.uri_fmt % tuple(uri_params)) + + return self._generate_action_handler_base(action_name, _render_element, + action_args) + + def generate_action_handler_task(self, action_name, action_args=None): + def _render_task(self, task): + cherrypy.response.status = 202 + return kimchi.template.render('Task', task) + + return self._generate_action_handler_base(action_name, _render_task, + action_args) + + def _generate_action_handler_base(self, action_name, render_fn, + action_args=None): def wrapper(*args, **kwargs): validate_method(('POST'), self.role_key, self.admin_methods) try: @@ -76,16 +99,10 @@ class Resource(object): if action_args is not None: request = parse_request() model_args.extend(request[key] for key in action_args) - fn = getattr(self.model, model_fn(self, action_name)) - ident = fn(*model_args) - self._redirect(ident) - uri_params = [] - for arg in self.model_args: - if arg is None: - arg = '' - uri_params.append(urllib2.quote(arg.encode('utf-8'), - safe="")) - raise internal_redirect(self.uri_fmt % tuple(uri_params)) + + action_fn = getattr(self.model, model_fn(self, action_name)) + action_result = action_fn(*model_args) + return render_fn(self, action_result) except MissingParameter, e: raise cherrypy.HTTPError(400, e.message) except InvalidParameter, e: diff --git a/src/kimchi/control/host.py b/src/kimchi/control/host.py index 7bcae72..4362da7 100644 --- a/src/kimchi/control/host.py +++ b/src/kimchi/control/host.py @@ -17,12 +17,9 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA -import cherrypy - from kimchi.control.base import Collection, Resource, SimpleCollection -from kimchi.control.utils import UrlSubNode, validate_method -from kimchi.exception import OperationFailed, NotFoundError -from kimchi.template import render +from kimchi.control.utils import UrlSubNode +from kimchi.exception import NotFoundError @UrlSubNode('host', True) @@ -41,16 +38,7 @@ class Host(Resource): self.repositories = Repositories(self.model) self.users = Users(self.model) self.groups = Groups(self.model) - - @cherrypy.expose - def swupdate(self): - validate_method(('POST'), self.role_key, self.admin_methods) - try: - task = self.model.host_swupdate() - cherrypy.response.status = 202 - return render("Task", task) - except OperationFailed, e: - raise cherrypy.HTTPError(500, e.message) + self.swupdate = self.generate_action_handler_task('swupdate') @property def data(self): -- 1.9.3

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 10/17/2014 05:26 AM, Crístian Viana wrote:
The function "generate_action_handler" creates a function which handles a string being returned by the underlying action function. However, in some cases, another type of element might be returned. The action "swupdate", for example, doesn't use "generate_action_handler" because it returns a Task instead of a string.
Make the function "generate_action_handler" more modular so it can process different types of data.
Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/control/base.py | 37 +++++++++++++++++++++++++++---------- src/kimchi/control/host.py | 18 +++--------------- 2 files changed, 30 insertions(+), 25 deletions(-)
diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index 1eb10bd..60db1df 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -65,6 +65,29 @@ class Resource(object): raise cherrypy.HTTPRedirect(self.uri_fmt % tuple(uri_params), code)
def generate_action_handler(self, action_name, action_args=None): + def _render_element(self, ident): + self._redirect(ident) + uri_params = [] + for arg in self.model_args: + if arg is None: + arg = '' + uri_params.append(urllib2.quote(arg.encode('utf-8'), + safe="")) + raise internal_redirect(self.uri_fmt % tuple(uri_params)) + + return self._generate_action_handler_base(action_name, _render_element, + action_args) + + def generate_action_handler_task(self, action_name, action_args=None): + def _render_task(self, task): + cherrypy.response.status = 202 + return kimchi.template.render('Task', task) + + return self._generate_action_handler_base(action_name, _render_task, + action_args) + + def _generate_action_handler_base(self, action_name, render_fn, + action_args=None): def wrapper(*args, **kwargs): validate_method(('POST'), self.role_key, self.admin_methods) try: @@ -76,16 +99,10 @@ class Resource(object): if action_args is not None: request = parse_request() model_args.extend(request[key] for key in action_args) - fn = getattr(self.model, model_fn(self, action_name)) - ident = fn(*model_args) - self._redirect(ident) - uri_params = [] - for arg in self.model_args: - if arg is None: - arg = '' - uri_params.append(urllib2.quote(arg.encode('utf-8'), - safe="")) - raise internal_redirect(self.uri_fmt % tuple(uri_params)) + + action_fn = getattr(self.model, model_fn(self, action_name)) + action_result = action_fn(*model_args) + return render_fn(self, action_result) except MissingParameter, e: raise cherrypy.HTTPError(400, e.message) except InvalidParameter, e: diff --git a/src/kimchi/control/host.py b/src/kimchi/control/host.py index 7bcae72..4362da7 100644 --- a/src/kimchi/control/host.py +++ b/src/kimchi/control/host.py @@ -17,12 +17,9 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-import cherrypy - from kimchi.control.base import Collection, Resource, SimpleCollection -from kimchi.control.utils import UrlSubNode, validate_method -from kimchi.exception import OperationFailed, NotFoundError -from kimchi.template import render +from kimchi.control.utils import UrlSubNode +from kimchi.exception import NotFoundError
@UrlSubNode('host', True) @@ -41,16 +38,7 @@ class Host(Resource): self.repositories = Repositories(self.model) self.users = Users(self.model) self.groups = Groups(self.model) - - @cherrypy.expose - def swupdate(self): - validate_method(('POST'), self.role_key, self.admin_methods) - try: - task = self.model.host_swupdate() - cherrypy.response.status = 202 - return render("Task", task) - except OperationFailed, e: - raise cherrypy.HTTPError(500, e.message) + self.swupdate = self.generate_action_handler_task('swupdate')
@property def data(self):

Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- docs/API.md | 5 ++ src/kimchi/control/vms.py | 1 + src/kimchi/i18n.py | 1 + src/kimchi/model/vms.py | 116 +++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 121 insertions(+), 2 deletions(-) diff --git a/docs/API.md b/docs/API.md index 92fbbd5..db8aab0 100644 --- a/docs/API.md +++ b/docs/API.md @@ -133,6 +133,11 @@ the following general conventions: risk of data loss caused by reset without the guest OS shutdown. * connect: Prepare the connection for spice or vnc +* clone: Create a new VM identical to this VM. The new VM's name, UUID and + network MAC addresses will be generated automatically. Each existing + disks will be copied to a new volume in the same storage pool. This + action returns a Task. + ### Sub-resource: Virtual Machine Screenshot **URI:** /vms/*:name*/screenshot diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py index 88d8a81..a1589ef 100644 --- a/src/kimchi/control/vms.py +++ b/src/kimchi/control/vms.py @@ -46,6 +46,7 @@ class VM(Resource): self.shutdown = self.generate_action_handler('shutdown') self.reset = self.generate_action_handler('reset') self.connect = self.generate_action_handler('connect') + self.clone = self.generate_action_handler_task('clone') @property def data(self): diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 75fb076..5ac65ef 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -101,6 +101,7 @@ messages = { "KCHVM0030E": _("Unable to get access metadata of virtual machine %(name)s. Details: %(err)s"), "KCHVM0031E": _("The guest console password must be a string."), "KCHVM0032E": _("The life time for the guest console password must be a number."), + "KCHVM0033E": _("Virtual machine '%(name)s' must be stopped before cloning it."), "KCHVMHDEV0001E": _("VM %(vmid)s does not contain directly assigned host device %(dev_name)s."), "KCHVMHDEV0002E": _("The host device %(dev_name)s is not allowed to directly assign to VM."), diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 58686cd..5143c2a 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -22,6 +22,8 @@ import lxml.etree as ET from lxml import etree, objectify import os import random +import re +import shutil import string import time import uuid @@ -30,19 +32,21 @@ from xml.etree import ElementTree import libvirt from cherrypy.process.plugins import BackgroundTask +import kimchi.model from kimchi import vnc from kimchi import xmlutils from kimchi.config import READONLY_POOL_TYPE from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import NotFoundError, OperationFailed from kimchi.model.config import CapabilitiesModel +from kimchi.model.tasks import TaskModel from kimchi.model.templates import TemplateModel from kimchi.model.utils import get_vm_name from kimchi.model.utils import get_metadata_node from kimchi.model.utils import set_metadata_node from kimchi.screenshot import VMScreenshot -from kimchi.utils import import_class, kimchi_log, run_setfacl_set_attr -from kimchi.utils import template_name_from_uri +from kimchi.utils import add_task, import_class, kimchi_log +from kimchi.utils import run_setfacl_set_attr, template_name_from_uri from kimchi.xmlutils import xpath_get_text @@ -64,6 +68,13 @@ VM_LIVE_UPDATE_PARAMS = {} stats = {} +XPATH_DISK_SOURCE = "/domain/devices/disk[@device='disk']/source/@file" +XPATH_INTERFACE_MAC = "/domain/devices/interface[@type='network']/mac/@address" +XPATH_DISK_BY_FILE = "./devices/disk[@device='disk']/source[@file='%s']" +XPATH_INTERFACE_BY_ADDRESS = "./devices/interface[@type='network']/"\ + "mac[@address='%s']" + + class VMsModel(object): def __init__(self, **kargs): self.conn = kargs['conn'] @@ -245,6 +256,8 @@ class VMModel(object): self.vmscreenshot = VMScreenshotModel(**kargs) self.users = import_class('kimchi.model.host.UsersModel')(**kargs) self.groups = import_class('kimchi.model.host.GroupsModel')(**kargs) + self.vms = VMsModel(**kargs) + self.task = TaskModel(**kargs) def update(self, name, params): dom = self.get_vm(name, self.conn) @@ -252,6 +265,105 @@ class VMModel(object): self._live_vm_update(dom, params) return dom.name().decode('utf-8') + def clone(self, name): + name = name.decode('utf-8') + + # VM must be shutoff in order to clone it + info = self.lookup(name) + if info['state'] != 'shutoff': + raise InvalidParameter('KCHVM0033E', {'name': name}) + + # find out the next available clone number for that VM: + # if any VM named "<name>-clone<number>" is found, use the + # maximum "number" + 1; else, use 1. + max_num = 0 + re_group_num = 'num' + re_cloned_vm = re.compile(u'%s-clone(?P<%s>\d+)' % + (name, re_group_num)) + + all_vms = self.vms.get_list() + + for v in all_vms: + match = re_cloned_vm.match(v) + if match is not None: + max_num = max(max_num, int(match.group(re_group_num))) + + new_name = u'%s-clone%d' % (name, max_num + 1) + + # create a task with the actual clone function + taskid = add_task(u'/vms/%s' % new_name, self._do_clone, self.objstore, + {'name': name, 'new_name': new_name}) + + return self.task.lookup(taskid) + + def _do_clone(self, cb, params): + name = params['name'] + new_name = params['new_name'] + conn = self.conn.get() + + # keep track of the new disks so we can remove them later should an + # error occur + new_disk_paths = [] + + try: + # fetch base XML + cb('reading source VM XML') + dom = conn.lookupByName(name) + xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_SECURE).decode('utf-8') + + # update name + cb('updating VM name') + xml = xmlutils.xml_item_update(xml, './name', new_name) + + # update UUID + cb('updating VM UUID') + new_uuid = unicode(uuid.uuid4()) + xml = xmlutils.xml_item_update(xml, './uuid', new_uuid) + + # update MAC addresses + cb('updating VM MAC addresses...') + mac_addresses = xmlutils.xpath_get_text(xml, XPATH_INTERFACE_MAC) + for i, mac in enumerate(mac_addresses): + while True: + new_mac = kimchi.model.vmifaces.VMIfacesModel.random_mac() + if new_mac not in mac_addresses: + mac_addresses[i] = new_mac + break + + xpath_expr = XPATH_INTERFACE_BY_ADDRESS % mac + xml = xmlutils.xml_item_update(xml, xpath_expr, new_mac, + 'address') + + # copy disks + disk_paths = xmlutils.xpath_get_text(xml, XPATH_DISK_SOURCE) + for i, path in enumerate(disk_paths): + cb('copying VM disk \'%s\'' % path) + dir = os.path.dirname(path) + file_ext = os.path.splitext(path)[1] + new_path = os.path.join(dir, u'%s-%d.%s' % (new_uuid, i, + file_ext)) + shutil.copy(path, new_path) + xml = xmlutils.xml_item_update(xml, XPATH_DISK_BY_FILE % path, + new_path, 'file') + new_disk_paths.append(new_path) + + # create new guest + cb('defining new VM') + conn.defineXML(xml) + + cb('OK', True) + except Exception, e: + # remove newly created disks + for path in new_disk_paths: + try: + os.remove(path) + except OSError, e_remove: + kimchi_log.error('error removing new disk \'%s\': %s' % + (path, e_remove.message)) + + kimchi_log.error('error cloning VM \'%s\': %s' % (name, e.message)) + raise + def _build_access_elem(self, users, groups): access = E.access() for user in users: -- 1.9.3

Crístian, have you considered using virt-clone? http://linux.die.net/man/1/virt-clone It is a tool that seems to be widely supported by the distros and it could simplify your code at the cost of depending on yet another command line utility. On 10/17/2014 05:26 AM, Crístian Viana wrote:
Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- docs/API.md | 5 ++ src/kimchi/control/vms.py | 1 + src/kimchi/i18n.py | 1 + src/kimchi/model/vms.py | 116 +++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 121 insertions(+), 2 deletions(-)
diff --git a/docs/API.md b/docs/API.md index 92fbbd5..db8aab0 100644 --- a/docs/API.md +++ b/docs/API.md @@ -133,6 +133,11 @@ the following general conventions: risk of data loss caused by reset without the guest OS shutdown. * connect: Prepare the connection for spice or vnc
+* clone: Create a new VM identical to this VM. The new VM's name, UUID and + network MAC addresses will be generated automatically. Each existing + disks will be copied to a new volume in the same storage pool. This + action returns a Task. + ### Sub-resource: Virtual Machine Screenshot
**URI:** /vms/*:name*/screenshot diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py index 88d8a81..a1589ef 100644 --- a/src/kimchi/control/vms.py +++ b/src/kimchi/control/vms.py @@ -46,6 +46,7 @@ class VM(Resource): self.shutdown = self.generate_action_handler('shutdown') self.reset = self.generate_action_handler('reset') self.connect = self.generate_action_handler('connect') + self.clone = self.generate_action_handler_task('clone')
@property def data(self): diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 75fb076..5ac65ef 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -101,6 +101,7 @@ messages = { "KCHVM0030E": _("Unable to get access metadata of virtual machine %(name)s. Details: %(err)s"), "KCHVM0031E": _("The guest console password must be a string."), "KCHVM0032E": _("The life time for the guest console password must be a number."), + "KCHVM0033E": _("Virtual machine '%(name)s' must be stopped before cloning it."),
"KCHVMHDEV0001E": _("VM %(vmid)s does not contain directly assigned host device %(dev_name)s."), "KCHVMHDEV0002E": _("The host device %(dev_name)s is not allowed to directly assign to VM."), diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 58686cd..5143c2a 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -22,6 +22,8 @@ import lxml.etree as ET from lxml import etree, objectify import os import random +import re +import shutil import string import time import uuid @@ -30,19 +32,21 @@ from xml.etree import ElementTree import libvirt from cherrypy.process.plugins import BackgroundTask
+import kimchi.model from kimchi import vnc from kimchi import xmlutils from kimchi.config import READONLY_POOL_TYPE from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import NotFoundError, OperationFailed from kimchi.model.config import CapabilitiesModel +from kimchi.model.tasks import TaskModel from kimchi.model.templates import TemplateModel from kimchi.model.utils import get_vm_name from kimchi.model.utils import get_metadata_node from kimchi.model.utils import set_metadata_node from kimchi.screenshot import VMScreenshot -from kimchi.utils import import_class, kimchi_log, run_setfacl_set_attr -from kimchi.utils import template_name_from_uri +from kimchi.utils import add_task, import_class, kimchi_log +from kimchi.utils import run_setfacl_set_attr, template_name_from_uri from kimchi.xmlutils import xpath_get_text
@@ -64,6 +68,13 @@ VM_LIVE_UPDATE_PARAMS = {} stats = {}
+XPATH_DISK_SOURCE = "/domain/devices/disk[@device='disk']/source/@file" +XPATH_INTERFACE_MAC = "/domain/devices/interface[@type='network']/mac/@address" +XPATH_DISK_BY_FILE = "./devices/disk[@device='disk']/source[@file='%s']" +XPATH_INTERFACE_BY_ADDRESS = "./devices/interface[@type='network']/"\ + "mac[@address='%s']" + + class VMsModel(object): def __init__(self, **kargs): self.conn = kargs['conn'] @@ -245,6 +256,8 @@ class VMModel(object): self.vmscreenshot = VMScreenshotModel(**kargs) self.users = import_class('kimchi.model.host.UsersModel')(**kargs) self.groups = import_class('kimchi.model.host.GroupsModel')(**kargs) + self.vms = VMsModel(**kargs) + self.task = TaskModel(**kargs)
def update(self, name, params): dom = self.get_vm(name, self.conn) @@ -252,6 +265,105 @@ class VMModel(object): self._live_vm_update(dom, params) return dom.name().decode('utf-8')
+ def clone(self, name): + name = name.decode('utf-8') + + # VM must be shutoff in order to clone it + info = self.lookup(name) + if info['state'] != 'shutoff': + raise InvalidParameter('KCHVM0033E', {'name': name}) + + # find out the next available clone number for that VM: + # if any VM named "<name>-clone<number>" is found, use the + # maximum "number" + 1; else, use 1. + max_num = 0 + re_group_num = 'num' + re_cloned_vm = re.compile(u'%s-clone(?P<%s>\d+)' % + (name, re_group_num)) + + all_vms = self.vms.get_list() + + for v in all_vms: + match = re_cloned_vm.match(v) + if match is not None: + max_num = max(max_num, int(match.group(re_group_num))) + + new_name = u'%s-clone%d' % (name, max_num + 1) + + # create a task with the actual clone function + taskid = add_task(u'/vms/%s' % new_name, self._do_clone, self.objstore, + {'name': name, 'new_name': new_name}) + + return self.task.lookup(taskid) + + def _do_clone(self, cb, params): + name = params['name'] + new_name = params['new_name'] + conn = self.conn.get() + + # keep track of the new disks so we can remove them later should an + # error occur + new_disk_paths = [] + + try: + # fetch base XML + cb('reading source VM XML') + dom = conn.lookupByName(name) + xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_SECURE).decode('utf-8') + + # update name + cb('updating VM name') + xml = xmlutils.xml_item_update(xml, './name', new_name) + + # update UUID + cb('updating VM UUID') + new_uuid = unicode(uuid.uuid4()) + xml = xmlutils.xml_item_update(xml, './uuid', new_uuid) + + # update MAC addresses + cb('updating VM MAC addresses...') + mac_addresses = xmlutils.xpath_get_text(xml, XPATH_INTERFACE_MAC) + for i, mac in enumerate(mac_addresses): + while True: + new_mac = kimchi.model.vmifaces.VMIfacesModel.random_mac() + if new_mac not in mac_addresses: + mac_addresses[i] = new_mac + break + + xpath_expr = XPATH_INTERFACE_BY_ADDRESS % mac + xml = xmlutils.xml_item_update(xml, xpath_expr, new_mac, + 'address') + + # copy disks + disk_paths = xmlutils.xpath_get_text(xml, XPATH_DISK_SOURCE) + for i, path in enumerate(disk_paths): + cb('copying VM disk \'%s\'' % path) + dir = os.path.dirname(path) + file_ext = os.path.splitext(path)[1] + new_path = os.path.join(dir, u'%s-%d.%s' % (new_uuid, i, + file_ext)) + shutil.copy(path, new_path) + xml = xmlutils.xml_item_update(xml, XPATH_DISK_BY_FILE % path, + new_path, 'file') + new_disk_paths.append(new_path) + + # create new guest + cb('defining new VM') + conn.defineXML(xml) + + cb('OK', True) + except Exception, e: + # remove newly created disks + for path in new_disk_paths: + try: + os.remove(path) + except OSError, e_remove: + kimchi_log.error('error removing new disk \'%s\': %s' % + (path, e_remove.message)) + + kimchi_log.error('error cloning VM \'%s\': %s' % (name, e.message)) + raise + def _build_access_elem(self, users, groups): access = E.access() for user in users:

On 17-10-2014 14:38, Daniel H Barboza wrote:
Crístian, have you considered using virt-clone?
http://linux.die.net/man/1/virt-clone
It is a tool that seems to be widely supported by the distros and it could simplify your code at the cost of depending on yet another command line utility.
Yes, I have, but I don't think it adds enough value. If we had used virt-clone now in this patchset, the only function which would be simplified is _do_clone (PATCH 5/5), which is the function that actually clones the VM. All other changes would still be needed. And the most complex part of this patchset (IMO), which I haven't sent yet and is still to be implemented, is how to deal with the different storage pools and take decisions on how to clone automatically taking into account the context of Kimchi (e.g. clone to the pool 'default' when there's not enough space on the original one). By default, virt-clone always tries to create new files in the same directories as the existing disks and it fails if that's not possible. I am aware of the virt-clone flag "-f", which is how we can specify manually where the new disks will be saved. But if we still need to write our own logic to find out where the new disks will be saved and set them by using "-f", virt-clone itself will only perform the following operations: duplicate the original VM's XML descriptor, set the new VM name, UUID and MAC addresses, and cp the disk files specified by us. I don't think that depending on a package to run it as an external process is worth it only so that it can read an XML file and update a couple of tags.

On 10/17/2014 05:26 AM, Crístian Viana wrote:
Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- docs/API.md | 5 ++ src/kimchi/control/vms.py | 1 + src/kimchi/i18n.py | 1 + src/kimchi/model/vms.py | 116 +++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 121 insertions(+), 2 deletions(-)
diff --git a/docs/API.md b/docs/API.md index 92fbbd5..db8aab0 100644 --- a/docs/API.md +++ b/docs/API.md @@ -133,6 +133,11 @@ the following general conventions: risk of data loss caused by reset without the guest OS shutdown. * connect: Prepare the connection for spice or vnc
+* clone: Create a new VM identical to this VM. The new VM's name, UUID and + network MAC addresses will be generated automatically. Each existing + disks will be copied to a new volume in the same storage pool. This + action returns a Task.
It is good to mention here too the "default" pool fallback.
+ ### Sub-resource: Virtual Machine Screenshot
**URI:** /vms/*:name*/screenshot diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py index 88d8a81..a1589ef 100644 --- a/src/kimchi/control/vms.py +++ b/src/kimchi/control/vms.py @@ -46,6 +46,7 @@ class VM(Resource): self.shutdown = self.generate_action_handler('shutdown') self.reset = self.generate_action_handler('reset') self.connect = self.generate_action_handler('connect') + self.clone = self.generate_action_handler_task('clone')
@property def data(self): diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 75fb076..5ac65ef 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -101,6 +101,7 @@ messages = { "KCHVM0030E": _("Unable to get access metadata of virtual machine %(name)s. Details: %(err)s"), "KCHVM0031E": _("The guest console password must be a string."), "KCHVM0032E": _("The life time for the guest console password must be a number."), + "KCHVM0033E": _("Virtual machine '%(name)s' must be stopped before cloning it."),
"KCHVMHDEV0001E": _("VM %(vmid)s does not contain directly assigned host device %(dev_name)s."), "KCHVMHDEV0002E": _("The host device %(dev_name)s is not allowed to directly assign to VM."), diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 58686cd..5143c2a 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -22,6 +22,8 @@ import lxml.etree as ET from lxml import etree, objectify import os import random +import re +import shutil import string import time import uuid @@ -30,19 +32,21 @@ from xml.etree import ElementTree import libvirt from cherrypy.process.plugins import BackgroundTask
+import kimchi.model from kimchi import vnc from kimchi import xmlutils from kimchi.config import READONLY_POOL_TYPE from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import NotFoundError, OperationFailed from kimchi.model.config import CapabilitiesModel +from kimchi.model.tasks import TaskModel from kimchi.model.templates import TemplateModel from kimchi.model.utils import get_vm_name from kimchi.model.utils import get_metadata_node from kimchi.model.utils import set_metadata_node from kimchi.screenshot import VMScreenshot -from kimchi.utils import import_class, kimchi_log, run_setfacl_set_attr -from kimchi.utils import template_name_from_uri +from kimchi.utils import add_task, import_class, kimchi_log +from kimchi.utils import run_setfacl_set_attr, template_name_from_uri from kimchi.xmlutils import xpath_get_text
@@ -64,6 +68,13 @@ VM_LIVE_UPDATE_PARAMS = {} stats = {}
+XPATH_DISK_SOURCE = "/domain/devices/disk[@device='disk']/source/@file" +XPATH_INTERFACE_MAC = "/domain/devices/interface[@type='network']/mac/@address" +XPATH_DISK_BY_FILE = "./devices/disk[@device='disk']/source[@file='%s']" +XPATH_INTERFACE_BY_ADDRESS = "./devices/interface[@type='network']/"\ + "mac[@address='%s']" + + class VMsModel(object): def __init__(self, **kargs): self.conn = kargs['conn'] @@ -245,6 +256,8 @@ class VMModel(object): self.vmscreenshot = VMScreenshotModel(**kargs) self.users = import_class('kimchi.model.host.UsersModel')(**kargs) self.groups = import_class('kimchi.model.host.GroupsModel')(**kargs) + self.vms = VMsModel(**kargs) + self.task = TaskModel(**kargs)
def update(self, name, params): dom = self.get_vm(name, self.conn) @@ -252,6 +265,105 @@ class VMModel(object): self._live_vm_update(dom, params) return dom.name().decode('utf-8')
+ def clone(self, name): + name = name.decode('utf-8') + + # VM must be shutoff in order to clone it + info = self.lookup(name) + if info['state'] != 'shutoff': + raise InvalidParameter('KCHVM0033E', {'name': name}) + + # find out the next available clone number for that VM: + # if any VM named "<name>-clone<number>" is found, use the + # maximum "number" + 1; else, use 1. + max_num = 0 + re_group_num = 'num' + re_cloned_vm = re.compile(u'%s-clone(?P<%s>\d+)' % + (name, re_group_num)) + + all_vms = self.vms.get_list() + + for v in all_vms: + match = re_cloned_vm.match(v) + if match is not None: + max_num = max(max_num, int(match.group(re_group_num))) + + new_name = u'%s-clone%d' % (name, max_num + 1) + + # create a task with the actual clone function + taskid = add_task(u'/vms/%s' % new_name, self._do_clone, self.objstore, + {'name': name, 'new_name': new_name}) + + return self.task.lookup(taskid) + + def _do_clone(self, cb, params): + name = params['name'] + new_name = params['new_name'] + conn = self.conn.get() + + # keep track of the new disks so we can remove them later should an + # error occur + new_disk_paths = [] + + try: + # fetch base XML + cb('reading source VM XML') + dom = conn.lookupByName(name) + xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_SECURE).decode('utf-8') + + # update name + cb('updating VM name') + xml = xmlutils.xml_item_update(xml, './name', new_name) + + # update UUID + cb('updating VM UUID') + new_uuid = unicode(uuid.uuid4()) + xml = xmlutils.xml_item_update(xml, './uuid', new_uuid) + + # update MAC addresses + cb('updating VM MAC addresses...') + mac_addresses = xmlutils.xpath_get_text(xml, XPATH_INTERFACE_MAC) + for i, mac in enumerate(mac_addresses): + while True: + new_mac = kimchi.model.vmifaces.VMIfacesModel.random_mac() + if new_mac not in mac_addresses: + mac_addresses[i] = new_mac + break + + xpath_expr = XPATH_INTERFACE_BY_ADDRESS % mac + xml = xmlutils.xml_item_update(xml, xpath_expr, new_mac, + 'address') + + # copy disks + disk_paths = xmlutils.xpath_get_text(xml, XPATH_DISK_SOURCE) + for i, path in enumerate(disk_paths): + cb('copying VM disk \'%s\'' % path) + dir = os.path.dirname(path) + file_ext = os.path.splitext(path)[1] + new_path = os.path.join(dir, u'%s-%d.%s' % (new_uuid, i, + file_ext)) + shutil.copy(path, new_path) + xml = xmlutils.xml_item_update(xml, XPATH_DISK_BY_FILE % path, + new_path, 'file') + new_disk_paths.append(new_path) + + # create new guest + cb('defining new VM') + conn.defineXML(xml) + + cb('OK', True)
You also need to add a new entry on objectstore for this new VM. The data should be a copy of the original VM with the updated values (name, uuid)
+ except Exception, e: + # remove newly created disks + for path in new_disk_paths: + try: + os.remove(path) + except OSError, e_remove: + kimchi_log.error('error removing new disk \'%s\': %s' % + (path, e_remove.message)) + + kimchi_log.error('error cloning VM \'%s\': %s' % (name, e.message)) + raise + def _build_access_elem(self, users, groups): access = E.access() for user in users:

On 21-10-2014 17:07, Aline Manera wrote:
It is good to mention here too the "default" pool fallback.
ACK
You also need to add a new entry on objectstore for this new VM. The data should be a copy of the original VM with the updated values (name, uuid)
OK, I'll look into that.
participants (3)
-
Aline Manera
-
Crístian Viana
-
Daniel H Barboza