[Kimchi-devel] [PATCH] Centralize graphics information

Aline Manera alinefm at linux.vnet.ibm.com
Sun Aug 24 22:43:24 UTC 2014


Recently the ticket information was added to the vm to report the
graphics password and its expiration time.
That way, graphics information was found in 2 different data structuce:
some information under "graphics" and other ones under "ticket".

   {
    "graphics":{
      "type":"vnc",
      "port":null,
      "listen":"127.0.0.1"
    },
    "ticket":{
      "passwd":null,
      "expire":null
    },
    "name":"Ubuntu14.04",
    "uuid":"8522d82d-d51b-436f-88ff-aff1fed27613",
    "access":"full",
    "state":"shutoff",
    "memory":1024
    ...
   }

As "ticket" data structure only contains graphics information it makes
more sense to centralize it under "graphics" and also use the same
terminology used by libvirt (passwd and passwdValidTo)

    "graphics":{
      "type":"vnc",
      "port":null,
      "listen":"127.0.0.1"
      "passwd":null,
      "passwdValidTo":null
    },

Signed-off-by: Aline Manera <alinefm at linux.vnet.ibm.com>
---
 docs/API.md               |  14 +++---
 src/kimchi/API.json       |  12 ++---
 src/kimchi/control/vms.py |   2 +-
 src/kimchi/i18n.py        |   4 +-
 src/kimchi/mockmodel.py   |  38 +++++++++-------
 src/kimchi/model/vms.py   | 112 ++++++++++++++++++++++------------------------
 tests/test_mockmodel.py   |   2 +-
 tests/test_model.py       |  19 ++++----
 tests/test_rest.py        |  13 +++---
 9 files changed, 111 insertions(+), 105 deletions(-)

diff --git a/docs/API.md b/docs/API.md
index d75c55f..7d5e804 100644
--- a/docs/API.md
+++ b/docs/API.md
@@ -94,9 +94,8 @@ the following general conventions:
         * port: The real port number of the graphics, vnc or spice. Users
                 can use this port to connect to the vm with general vnc/spice
                 clients.
-    * ticket: A ticket represents credentials to access VM.
-        * password: the password of a ticket.
-        * expire: lifetime of a ticket.
+        * passwd: console password
+        * passwdValidTo: lifetime for the console password.
     * users: A list of system users who have permission to access the VM.
       Default is: empty (i.e. only root-users may access).
     * groups: A list of system groups whose users have permission to access
@@ -110,9 +109,12 @@ the following general conventions:
             will take effect in next reboot)
     * memory: New amount of memory (MB) for this VM (if VM is running, new
               value will take effect in next reboot)
-    * ticket: A ticket represents credentials to access VM.
-        * password *(optional)*: the password of a ticket.
-        * expire *(optional)*: lifetime of a ticket.
+    * graphics: A dict to show detail of VM graphics.
+        * passwd *(optional)*: console password. When omitted a random password
+                               willbe generated.
+        * passwdValidTo *(optional)*: lifetime for the console password. When
+                                      omitted the password will be valid just
+                                      for 30 seconds.
 
 * **POST**: *See Virtual Machine Actions*
 
diff --git a/src/kimchi/API.json b/src/kimchi/API.json
index c3fc5e3..67612f2 100644
--- a/src/kimchi/API.json
+++ b/src/kimchi/API.json
@@ -235,17 +235,17 @@
                         "error": "KCHVM0026E"
                     }
                 },
-                "ticket": {
-                    "description": "A ticket represents credentials to access VM",
+                "graphics": {
+                    "description": "Graphics information from guest",
                     "type": "object",
                     "properties": {
-                        "password": {
-                            "description": "the password of a ticket.",
+                        "passwd": {
+                            "description": "New graphics password.",
                             "type": "string",
                             "error": "KCHVM0031E"
                         },
-                        "size": {
-                            "description": "lifetime of a ticket.",
+                        "passwdValidTo": {
+                            "description": "Life time for the graphics password.",
                             "type": "number",
                             "error": "KCHVM0032E"
                         }
diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py
index 6632e52..88d8a81 100644
--- a/src/kimchi/control/vms.py
+++ b/src/kimchi/control/vms.py
@@ -36,7 +36,7 @@ class VM(Resource):
         super(VM, self).__init__(model, ident)
         self.role_key = 'guests'
         self.update_params = ["name", "users", "groups", "cpus", "memory",
-                              "ticket"]
+                              "graphics"]
         self.screenshot = VMScreenShot(model, ident)
         self.uri_fmt = '/vms/%s'
         for ident, node in sub_nodes.items():
diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
index 2eae7e8..c276b38 100644
--- a/src/kimchi/i18n.py
+++ b/src/kimchi/i18n.py
@@ -94,8 +94,8 @@ messages = {
     "KCHVM0028E": _("Group(s) '%(groups)s' do not exist"),
     "KCHVM0029E": _("Unable to shutdown virtual machine %(name)s. Details: %(err)s"),
     "KCHVM0030E": _("Unable to get access metadata of virtual machine %(name)s. Details: %(err)s"),
-    "KCHVM0031E": _("password of a vm ticket must be a string."),
-    "KCHVM0032E": _("expire of a vm ticket must be a number."),
+    "KCHVM0031E": _("The guest console password must be a string."),
+    "KCHVM0032E": _("The life time for the guest console password must be a number."),
 
     "KCHVMIF0001E": _("Interface %(iface)s does not exist in virtual machine %(name)s"),
     "KCHVMIF0002E": _("Network %(network)s specified for virtual machine %(name)s does not exist"),
diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py
index fc474de..1322def 100644
--- a/src/kimchi/mockmodel.py
+++ b/src/kimchi/mockmodel.py
@@ -91,15 +91,6 @@ class MockModel(object):
     def _static_vm_update(self, dom, params):
         state = dom.info['state']
 
-        if 'ticket' in params:
-            ticket = params.pop('ticket')
-            passwd = ticket.get('passwd')
-            dom.ticket["passwd"] = passwd if passwd is not None else "".join(
-                random.sample(string.ascii_letters + string.digits, 8))
-            expire = ticket.get('expire')
-            dom.ticket["ValidTo"] = expire if expire is None else round(
-                time.time()) + expire
-
         for key, val in params.items():
             if key == 'name':
                 if state == 'running' or params['name'] in self.vms_get_list():
@@ -126,7 +117,21 @@ class MockModel(object):
             dom.info[key] = val
 
     def _live_vm_update(self, dom, params):
-        pass
+        if 'graphics' not in params:
+            return
+
+        graphics = params.pop('graphics')
+        passwd = graphics.get('passwd')
+        if passwd is None:
+            passwd = "".join(random.sample(string.ascii_letters +
+                                           string.digits, 8))
+
+        expire = graphics.get('passwdValidTo')
+        if expire is not None:
+            expire = round(time.time()) + expire
+
+        dom.info['graphics']["passwd"] = passwd
+        dom.info['graphics']["passwdValidTo"] = expire
 
     def vm_update(self, name, params):
         dom = self._get_vm(name)
@@ -141,10 +146,11 @@ class MockModel(object):
             vm.info['screenshot'] = self.vmscreenshot_lookup(name)
         else:
             vm.info['screenshot'] = None
-        vm.info['ticket'] = {"passwd": vm.ticket["passwd"]}
-        validTo = vm.ticket["ValidTo"]
-        vm.info['ticket']["expire"] = (validTo - round(time.time())
-                                       if validTo is not None else None)
+
+        validTo = vm.info['graphics']['passwdValidTo']
+        validTo = (validTo - round(time.time()) if validTo is not None
+                   else None)
+        vm.info['graphics']['passwdValidTo'] = validTo
         return vm.info
 
     def vm_delete(self, name):
@@ -1050,7 +1056,6 @@ class MockVM(object):
         self.networks = template_info['networks']
         ifaces = [MockVMIface(net) for net in self.networks]
         self.storagedevices = {}
-        self.ticket = {"passwd": "123456", "ValidTo": None}
         self.ifaces = dict([(iface.info['mac'], iface) for iface in ifaces])
 
         stats = {'cpu_utilization': 20,
@@ -1066,7 +1071,8 @@ class MockVM(object):
                      'cpus': self.cpus,
                      'icon': None,
                      'graphics': {'type': 'vnc', 'listen': '127.0.0.1',
-                                  'port': None},
+                                  'port': None, 'passwd': '123456',
+                                  'passwdValidTo': None},
                      'users': ['user1', 'user2', 'root'],
                      'groups': ['group1', 'group2', 'admin'],
                      'access': 'full'
diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py
index 476e4ac..0a05b76 100644
--- a/src/kimchi/model/vms.py
+++ b/src/kimchi/model/vms.py
@@ -251,12 +251,6 @@ class VMModel(object):
         self.groups = import_class('kimchi.model.host.GroupsModel')(**kargs)
 
     def update(self, name, params):
-        if 'ticket' in params:
-            password = params['ticket'].get("passwd")
-            password = password if password is not None else "".join(
-                random.sample(string.ascii_letters + string.digits, 8))
-            params['ticket']['passwd'] = password
-
         dom = self.get_vm(name, self.conn)
         dom = self._static_vm_update(dom, params)
         self._live_vm_update(dom, params)
@@ -315,14 +309,15 @@ class VMModel(object):
         os_elem = E.os({"distro": distro, "version": version})
         set_metadata_node(dom, os_elem)
 
-    def _set_ticket(self, xml, params, flag=0):
+    def _set_graphics_passwd(self, xml, params, flag=0):
         DEFAULT_VALID_TO = 30
         password = params.get("passwd")
-        expire = params.get("expire")
+        expire = params.get("passwdValidTo")
         root = objectify.fromstring(xml)
         graphic = root.devices.find("graphics")
         if graphic is None:
             return None
+
         graphic.attrib['passwd'] = password
         to = graphic.attrib.get('passwdValidTo')
         if to is not None:
@@ -337,27 +332,27 @@ class VMModel(object):
 
         return root if flag == 0 else graphic
 
-    def _set_persistent_ticket(self, dom, xml, params):
-        if 'ticket' not in params:
-            # store the password for copy
-            ticket = self._get_ticket(dom)
-            if ticket['passwd'] is None:
-                return xml
-            else:
-                params['ticket'] = ticket
-        node = self._set_ticket(xml, params['ticket'])
-        return xml if node is None else ET.tostring(node, encoding="utf-8")
+    def _update_graphics(self, dom, xml, params):
+        if 'graphics' not in params:
+            return xml
 
-    def _set_live_ticket(self, dom, params):
-        if 'ticket' not in params:
-            return
-        if dom.isActive():
-            xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_SECURE)
-            flag = libvirt.VIR_DOMAIN_XML_SECURE
-            node = self._set_ticket(xml, params['ticket'], flag)
-            if node is not None:
-                dom.updateDeviceFlags(etree.tostring(node),
-                                      libvirt.VIR_DOMAIN_AFFECT_LIVE)
+        password = params['graphics'].get("passwd")
+        if password is None:
+            password = "".join(random.sample(string.ascii_letters +
+                                             string.digits, 8))
+        params['graphics']['passwd'] = password
+
+        if not dom.isActive():
+            node = self._set_graphics_passwd(xml, params['graphics'])
+            return xml if node is None else ET.tostring(node, encoding="utf-8")
+
+        xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_SECURE)
+        flag = libvirt.VIR_DOMAIN_XML_SECURE
+        node = self._set_graphics_passwd(xml, params['graphics'], flag)
+        if node is not None:
+            dom.updateDeviceFlags(etree.tostring(node),
+                                  libvirt.VIR_DOMAIN_AFFECT_LIVE)
+        return xml
 
     def _static_vm_update(self, dom, params):
         state = DOM_STATE_MAP[dom.info()[0]]
@@ -374,7 +369,7 @@ class VMModel(object):
                 xpath = VM_STATIC_UPDATE_PARAMS[key]
                 new_xml = xmlutils.xml_item_update(new_xml, xpath, val)
 
-        new_xml = self._set_persistent_ticket(dom, new_xml, params)
+        new_xml = self._update_graphics(dom, new_xml, params)
 
         conn = self.conn.get()
         try:
@@ -397,35 +392,19 @@ class VMModel(object):
 
     def _live_vm_update(self, dom, params):
         self._vm_update_access_metadata(dom, params)
-        self._set_live_ticket(dom, params)
 
     def _has_video(self, dom):
         dom = ElementTree.fromstring(dom.XMLDesc(0))
         return dom.find('devices/video') is not None
 
-    def _get_ticket(self, dom):
-        xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_SECURE)
-        root = objectify.fromstring(xml)
-        graphic = root.devices.find("graphics")
-        if graphic is None:
-            return {"passwd": None, "expire": None}
-        passwd = graphic.attrib.get('passwd')
-        ticket = {"passwd": passwd}
-        valid_to = graphic.attrib.get('passwdValidTo')
-        ticket['expire'] = None
-        if valid_to is not None:
-            to = time.mktime(time.strptime(valid_to, '%Y-%m-%dT%H:%M:%S'))
-            ticket['expire'] = to - time.mktime(time.gmtime())
-
-        return ticket
-
     def lookup(self, name):
         dom = self.get_vm(name, self.conn)
         info = dom.info()
         state = DOM_STATE_MAP[info[0]]
         screenshot = None
+        # (type, listen, port, passwd, passwdValidTo)
         graphics = self._vm_get_graphics(name)
-        graphics_type, graphics_listen, graphics_port = graphics
+        graphics_port = graphics[2]
         graphics_port = graphics_port if state == 'running' else None
         try:
             if state == 'running' and self._has_video(dom):
@@ -465,10 +444,12 @@ class VMModel(object):
                 'cpus': info[3],
                 'screenshot': screenshot,
                 'icon': icon,
-                'graphics': {"type": graphics_type,
-                             "listen": graphics_listen,
-                             "port": graphics_port},
-                'ticket': self._get_ticket(dom),
+                # (type, listen, port, passwd, passwdValidTo)
+                'graphics': {"type": graphics[0],
+                             "listen": graphics[1],
+                             "port": graphics_port,
+                             "passwd": graphics[3],
+                             "passwdValidTo": graphics[4]},
                 'users': users,
                 'groups': groups,
                 'access': 'full'
@@ -567,23 +548,38 @@ class VMModel(object):
 
     def _vm_get_graphics(self, name):
         dom = self.get_vm(name, self.conn)
-        xml = dom.XMLDesc(0)
+        xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_SECURE)
+
         expr = "/domain/devices/graphics/@type"
         res = xmlutils.xpath_get_text(xml, expr)
         graphics_type = res[0] if res else None
+
         expr = "/domain/devices/graphics/@listen"
         res = xmlutils.xpath_get_text(xml, expr)
         graphics_listen = res[0] if res else None
-        graphics_port = None
+
+        graphics_port = graphics_passwd = graphics_passwdValidTo = None
         if graphics_type:
-            expr = "/domain/devices/graphics[@type='%s']/@port" % graphics_type
-            res = xmlutils.xpath_get_text(xml, expr)
+            expr = "/domain/devices/graphics[@type='%s']/@port"
+            res = xmlutils.xpath_get_text(xml, expr % graphics_type)
             graphics_port = int(res[0]) if res else None
-        return graphics_type, graphics_listen, graphics_port
+
+            expr = "/domain/devices/graphics[@type='%s']/@passwd"
+            res = xmlutils.xpath_get_text(xml, expr % graphics_type)
+            graphics_passwd = res[0] if res else None
+
+            expr = "/domain/devices/graphics[@type='%s']/@passwdValidTo"
+            res = xmlutils.xpath_get_text(xml, expr % graphics_type)
+            if res:
+                to = time.mktime(time.strptime(res[0], '%Y-%m-%dT%H:%M:%S'))
+                graphics_passwdValidTo = to - time.mktime(time.gmtime())
+
+        return (graphics_type, graphics_listen, graphics_port,
+                graphics_passwd, graphics_passwdValidTo)
 
     def connect(self, name):
-        graphics = self._vm_get_graphics(name)
-        graphics_type, graphics_listen, graphics_port = graphics
+        # (type, listen, port, passwd, passwdValidTo)
+        graphics_port = self._vm_get_graphics(name)[2]
         if graphics_port is not None:
             vnc.add_proxy_token(name, graphics_port)
         else:
diff --git a/tests/test_mockmodel.py b/tests/test_mockmodel.py
index b7e6a48..8de7ce9 100644
--- a/tests/test_mockmodel.py
+++ b/tests/test_mockmodel.py
@@ -143,7 +143,7 @@ class MockModelTests(unittest.TestCase):
 
         keys = set(('name', 'state', 'stats', 'uuid', 'memory', 'cpus',
                     'screenshot', 'icon', 'graphics', 'users', 'groups',
-                    'access', 'ticket'))
+                    'access'))
 
         stats_keys = set(('cpu_utilization',
                           'net_throughput', 'net_throughput_peak',
diff --git a/tests/test_model.py b/tests/test_model.py
index f5b13d0..5090395 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -65,7 +65,7 @@ class ModelTests(unittest.TestCase):
 
         keys = set(('name', 'state', 'stats', 'uuid', 'memory', 'cpus',
                     'screenshot', 'icon', 'graphics', 'users', 'groups',
-                    'access', 'ticket'))
+                    'access'))
 
         stats_keys = set(('cpu_utilization',
                           'net_throughput', 'net_throughput_peak',
@@ -764,24 +764,25 @@ class ModelTests(unittest.TestCase):
             vms = inst.vms_get_list()
             self.assertTrue('kimchi-vm1' in vms)
 
-            # update vm ticket when vm is not running
+            # update vm graphics when vm is not running
             inst.vm_update(u'kimchi-vm1',
-                           {"ticket": {"passwd": "123456"}})
+                           {"graphics": {"passwd": "123456"}})
 
             inst.vm_start('kimchi-vm1')
             rollback.prependDefer(self._rollback_wrapper, inst.vm_poweroff,
                                   'kimchi-vm1')
 
             vm_info = inst.vm_lookup(u'kimchi-vm1')
-            self.assertEquals('123456', vm_info['ticket']["passwd"])
-            self.assertEquals(None, vm_info['ticket']["expire"])
+            self.assertEquals('123456', vm_info['graphics']["passwd"])
+            self.assertEquals(None, vm_info['graphics']["passwdValidTo"])
 
-            # update vm ticket when vm is running
+            # update vm graphics when vm is running
             inst.vm_update(u'kimchi-vm1',
-                           {"ticket": {"passwd": "abcdef", "expire": 20}})
+                           {"graphics": {"passwd": "abcdef",
+                                         "passwdValidTo": 20}})
             vm_info = inst.vm_lookup(u'kimchi-vm1')
-            self.assertEquals('abcdef', vm_info['ticket']["passwd"])
-            self.assertGreaterEqual(20, vm_info['ticket']['expire'])
+            self.assertEquals('abcdef', vm_info['graphics']["passwd"])
+            self.assertGreaterEqual(20, vm_info['graphics']['passwdValidTo'])
 
             info = inst.vm_lookup('kimchi-vm1')
             self.assertEquals('running', info['state'])
diff --git a/tests/test_rest.py b/tests/test_rest.py
index 2117399..0bf5997 100644
--- a/tests/test_rest.py
+++ b/tests/test_rest.py
@@ -221,20 +221,21 @@ class RestTests(unittest.TestCase):
         resp = self.request('/vms/vm-1', req, 'PUT')
         self.assertEquals(200, resp.status)
 
-        req = json.dumps({"ticket": {'passwd': "abcdef"}})
+        req = json.dumps({"graphics": {'passwd': "abcdef"}})
         resp = self.request('/vms/vm-1', req, 'PUT')
         info = json.loads(resp.read())
-        self.assertEquals('abcdef', info["ticket"]["passwd"])
-        self.assertEquals(None, info["ticket"]["expire"])
+        self.assertEquals('abcdef', info["graphics"]["passwd"])
+        self.assertEquals(None, info["graphics"]["passwdValidTo"])
 
         resp = self.request('/vms/vm-1/poweroff', '{}', 'POST')
         self.assertEquals(200, resp.status)
 
-        req = json.dumps({"ticket": {'passwd': "123456", "expire": 20}})
+        req = json.dumps({"graphics": {'passwd': "123456",
+                                       'passwdValidTo': 20}})
         resp = self.request('/vms/vm-1', req, 'PUT')
         info = json.loads(resp.read())
-        self.assertEquals('123456', info["ticket"]["passwd"])
-        self.assertGreaterEqual(20, info["ticket"]["expire"])
+        self.assertEquals('123456', info["graphics"]["passwd"])
+        self.assertGreaterEqual(20, info["graphics"]["passwdValidTo"])
 
         req = json.dumps({'name': 12})
         resp = self.request('/vms/vm-1', req, 'PUT')
-- 
1.9.3




More information about the Kimchi-devel mailing list