[Kimchi-devel] [PATCH] Centralize graphics information
Aline Manera
alinefm at linux.vnet.ibm.com
Mon Aug 25 14:47:27 UTC 2014
On 08/25/2014 04:22 AM, Royce Lv wrote:
> On 2014年08月25日 06:43, Aline Manera wrote:
>> 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))
> What if user just want to extend expire time of the passwd without
> setting a password?
Thanks for pointing it out, Royce.
In this patch I've just wanted to reorganized the data structure.
But I will fix it in a separated patch and send a V2
>> + 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')
>
More information about the Kimchi-devel
mailing list