[Kimchi-devel] [PATCH 5/5] Add/remove users and groups to VMs

Aline Manera alinefm at linux.vnet.ibm.com
Tue Mar 4 13:27:28 UTC 2014


On 02/28/2014 03:40 PM, Crístian Viana wrote:
> To update (add/remove) the users and groups of a virtual machine, you
> should use the REST command:
>
> /vms/<vm_name> PUT "{ 'users': [ 'user1', 'user2' ],
>                        'groups': [ 'group1', 'group2' ] }"
>
> Users and groups associated with a virtual machine must be valid when they are
> added to the VM. Such verification is not done when they are removed from the
> system. This means that virtual machines may contain outdated information about
> users and groups.
>
> Signed-off-by: Crístian Viana <vianac at linux.vnet.ibm.com>
> ---
>   docs/API.md               |  2 ++
>   src/kimchi/control/vms.py |  2 +-
>   src/kimchi/i18n.py        |  2 ++
>   src/kimchi/model/vms.py   | 29 ++++++++++++++++++++++++++---
>   tests/test_model.py       | 40 ++++++++++++++++++++++++++++++++++++++++
>   5 files changed, 71 insertions(+), 4 deletions(-)
>
> diff --git a/docs/API.md b/docs/API.md
> index 8520abf..a6c2b65 100644
> --- a/docs/API.md
> +++ b/docs/API.md
> @@ -103,6 +103,8 @@ the following general conventions:
>   * **DELETE**: Remove the Virtual Machine
>   * **PUT**: update the parameters of existed VM
>       * name: New name for this VM (only applied for shutoff VM)
> +    * users: New list of system users.
> +    * groups: New list of system groups.
>   * **POST**: *See Virtual Machine Actions*
>   
>   **Actions (POST):**
> diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py
> index c2a96c0..b0cf2e0 100644
> --- a/src/kimchi/control/vms.py
> +++ b/src/kimchi/control/vms.py
> @@ -32,7 +32,7 @@ class VMs(Collection):
>   class VM(Resource):
>       def __init__(self, model, ident):
>           super(VM, self).__init__(model, ident)
> -        self.update_params = ["name"]
> +        self.update_params = ["name", "users", "groups"]
>           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 f6bce8c..94a02f5 100644
> --- a/src/kimchi/i18n.py
> +++ b/src/kimchi/i18n.py
> @@ -80,6 +80,8 @@ messages = {
>       "KCHVM0023E": _("User name must be a string"),
>       "KCHVM0024E": _("Group names list must be an array"),
>       "KCHVM0025E": _("Group name must be a string"),
> +    "KCHVM0026E": _("User %(user)s does not exist"),
> +    "KCHVM0027E": _("Group %(group)s does not exist"),
>   
>       "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/model/vms.py b/src/kimchi/model/vms.py
> index bedf660..4f4f572 100644
> --- a/src/kimchi/model/vms.py
> +++ b/src/kimchi/model/vms.py
> @@ -29,6 +29,7 @@ from lxml import etree
>   
>   from kimchi import vnc
>   from kimchi import xmlutils
> +from kimchi.auth import Group, User
>   from kimchi.exception import InvalidOperation, InvalidParameter
>   from kimchi.exception import MissingParameter, NotFoundError, OperationFailed
>   from kimchi.model.config import CapabilitiesModel
> @@ -260,10 +261,24 @@ class VMModel(object):
>   
>           old_xml = new_xml = dom.XMLDesc(0)
>   
> +        users = _get_vm_metadata(old_xml, "users")
> +        groups = _get_vm_metadata(old_xml, "groups")
> +
>           for key, val in params.items():
> -            if key in VM_STATIC_UPDATE_PARAMS:
> -                xpath = VM_STATIC_UPDATE_PARAMS[key]
> -                new_xml = xmlutils.xml_item_update(new_xml, xpath, val)
> +            if key == 'users':
> +                for user in val:
> +                    if not User(user).exists():
> +                        raise OperationFailed("KCHVM0026E", {'user': user})
> +                users = val
> +            elif key == 'groups':
> +                for group in val:
> +                    if not Group(group).exists():
> +                        raise OperationFailed("KCHVM0027E", {'group': group})
> +                groups = val
> +            else:
> +                if key in VM_STATIC_UPDATE_PARAMS:
> +                    xpath = VM_STATIC_UPDATE_PARAMS[key]
> +                    new_xml = xmlutils.xml_item_update(new_xml, xpath, val)
>   
>           try:
>               if 'name' in params:
> @@ -274,6 +289,14 @@ class VMModel(object):
>                       dom.undefine()
>               conn = self.conn.get()
>               dom = conn.defineXML(new_xml)

Don't you need to undefine the vm first and then create the new one with 
the updated xml?
 From the code, the vm will be undefine only when update its name


> +            metadata_xml = """
> +                <access>
> +                    <users>%s</users>
> +                    <groups>%s</groups>
> +                </access>
> +            """ % (str(users), str(groups))
> +            dom.setMetadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT, metadata_xml,
> +                            KIMCHI_NS_TAG, KIMCHI_NS_URI)
>           except libvirt.libvirtError as e:
>               dom = conn.defineXML(old_xml)

Same problem I mentioned above.
As you don't undefine the original VM, when entering here we will get an 
error:
libvirtError: operation failed: domain 'пeω-∨м' is already defined with 
uuid cf2043b8-0234-4ded-896d-26ebbaed5be0


>               raise OperationFailed("KCHVM0008E", {'name': dom.name(),
> diff --git a/tests/test_model.py b/tests/test_model.py
> index 6d2927a..af52eed 100644
> --- a/tests/test_model.py
> +++ b/tests/test_model.py
> @@ -18,9 +18,11 @@
>   # 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 grp
>   import os
>   import platform
>   import psutil
> +import pwd
>   import shutil
>   import tempfile
>   import threading
> @@ -577,6 +579,44 @@ class ModelTests(unittest.TestCase):
>               rollback.prependDefer(self._rollback_wrapper, inst.vm_delete,
>                                     u'пeω-∨м')
>   
> +            # change only VM users - groups are not changed (default is empty)
> +            users = ['root']
> +            inst.vm_update(u'пeω-∨м', {'users': users})
> +            self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users'])
> +            self.assertEquals([], inst.vm_lookup(u'пeω-∨м')['groups'])
> +
> +            # change only VM groups - users are not changed (default is empty)
> +            groups = ['wheel']
> +            inst.vm_update(u'пeω-∨м', {'groups': groups})
> +            self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users'])
> +            self.assertEquals(groups, inst.vm_lookup(u'пeω-∨м')['groups'])
> +
> +            # change VM users and groups by adding a new element to each one
> +            users.append(pwd.getpwuid(os.getuid()).pw_name)
> +            groups.append(grp.getgrgid(os.getgid()).gr_name)
> +            inst.vm_update(u'пeω-∨м', {'users': users, 'groups': groups})
> +            self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users'])
> +            self.assertEquals(groups, inst.vm_lookup(u'пeω-∨м')['groups'])
> +
> +            # change VM users (wrong value) and groups
> +            # when an error occurs, everything fails and nothing is changed
> +            self.assertRaises(OperationFailed, inst.vm_update, u'пeω-∨м',
> +                              {'users': ['userdoesnotexist'], 'groups': []})
> +            self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users'])
> +            self.assertEquals(groups, inst.vm_lookup(u'пeω-∨м')['groups'])
> +
> +            # change VM users and groups (wrong value)
> +            # when an error occurs, everything fails and nothing is changed
> +            self.assertRaises(OperationFailed, inst.vm_update, u'пeω-∨м',
> +                              {'users': [], 'groups': ['groupdoesnotexist']})
> +            self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users'])
> +            self.assertEquals(groups, inst.vm_lookup(u'пeω-∨м')['groups'])
> +
> +            # change VM users and groups by removing all elements
> +            inst.vm_update(u'пeω-∨м', {'users': [], 'groups': []})
> +            self.assertEquals([], inst.vm_lookup(u'пeω-∨м')['users'])
> +            self.assertEquals([], inst.vm_lookup(u'пeω-∨м')['groups'])
> +
>       @unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
>       def test_network(self):
>           inst = model.Model('qemu:///system', self.tmp_store)




More information about the Kimchi-devel mailing list