Merge lp:~markwash/nova/lp727225 into lp:~hudson-openstack/nova/trunk

[Mark Washenberger]

Fix lp727225 by adding support for personality files to the openstack api.

Description:

This merge adds support for personality files to the openstack api. It leverages previous work which added this functionality to the compute api, compute manager, and xen api.

Fix lp727225 by adding support for personality files to the openstack api.

[Mark Washenberger (markwash)]

Sorry, maybe some of this should have gone in the original description. . .

Description:

This merge adds support for personality files to the openstack api. It leverages previous work which added this functionality to the compute api, compute manager, and xen api.

Stuff you should know up front:

- deserialization
  I had to add a custom deserializer for server create requests. This deserializer is only called if the content type is application/xml. Adding this was the least intrusive way for me to get around the complete lack of support for slightly more complicated xml handling in the default serializer.
- variable name change
  I changed onset_files to personality_files in the compute api at a few layers. I did this because the compute_api handles the quota enforcement, but unfortunately right now quota error messages appear to bubble up directly into the os api http error responses. It would be confusing for an os api user to give an invalid personality and get an error about onset_files.
- changes to base64 handling
  Previously there was a safety function that was designed to ensure that a string is base64 encoded, and encode it if it is not. In fact this function is quite deceptive. As an example, for '/etc/passwd' it would return 'L2V0Yy9wYXNzd2Q=', but for '/etc/sudoers' it would return '/etc/sudoers', because the latter is already a valid base64 string. To remedy this problem, I removed the function and replaced its usage with stronger function contracts.
- additional flags
  To support the quota on personality files, file paths, and file contents, I added a few new flags to nova/quota.py.

[Christopher MacGown (0x44)]

Needs a docstring in _check_personality_file_quota otherwise will approve

[Mark Washenberger (markwash)]

Added a missing docstring to my branch

[Eric Day (eday)]

There is some inconsistency in naming with "personality" and "onset". The nova/api/* can be whatever the external API uses, but within nova/compute/* and nova/db/* the name should be the same. For example, nova/compute/api.py uses personality and nova/compute/manager.py uses onset.

review: Needs Fixing

[Mark Washenberger (markwash)]

> There is some inconsistency in naming with "personality" and "onset". The
> nova/api/* can be whatever the external API uses, but within nova/compute/*
> and nova/db/* the name should be the same. For example, nova/compute/api.py
> uses personality and nova/compute/manager.py uses onset.

Thanks for having a look Eric.

I'm definitely a bit nervous about the naming change. Let me put forward my reasoning, and then hopefully I can beg a little more guidance about how to address this.

The reason why I changed the naming down into the compute layer is because the text of the QuotaErrors associated with onset/personality files is what is actually used in the http error message. So, if I keep the name onset_files and make it consistent throughout the compute service layer, then when a user submits an invalid number of personality files, his error message will complain about too many onset files.

I would also suggest that the only place where onset_files remains in the compute manager is in instance_ref, which is actually coming out of the data layer. Therefore, the compute layer could be seen as a sort of adapter on top of the data layer for this particular naming issue.

Here are a couple of ways I could solve this differently:

1) make the quota error text say "personality" but change the variable names back to onset_files. This would leave a different kind of inconsistency in the compute service layer.
2) create new quotaerror exceptions that are specific to personality files, and catch them in the os api. The only drawback I see here is that I don't like to get trigger happy with exception types.
3) figure out some fancy way of using internationalization?

Anyone got any suggestions? I want to do the right thing here.

[Eric Day (eday)]

We should probably be raising an exception with data limits but no text from nova/compute/* and let the API handle creating a message. This allows different APIs to do whatever they need with the exception and data, especially since not all APIs will want to even report a text message in the future. This allows us to keep names consistent within API boundaries.

As far as if we use "personality" or "onset", I don't really care (although both are not immediately obvious that they are injected files), I just think it's important to have consistency. :)

[Mark Washenberger (markwash)]

Updated to return to consistent use of onset_files. Handles http error codes in the os api by checking the quota error code and raising a new http exception if it is a quota error where the message won't make sense to an os api client.

[Eric Day (eday)]

141: Any way to make this generic and plug into the common deserializer in some way? Possibly passing options to trigger specific actions for certain elements?

243: any reason you assign the result of quota check (or why quota check returns anything)? this should always be a RO operation, no?

Other than that, looks good to me!

review: Needs Information

[Mark Washenberger (markwash)]

> 141: Any way to make this generic and plug into the common deserializer in
> some way? Possibly passing options to trigger specific actions for certain
> elements?

I think it would require replacing the common serializer in order to make this work. In fact, I'd like to see serialization become more of an api- or controller-specific activity. However, it seems out of scope for this particular merge.

> 243: any reason you assign the result of quota check (or why quota check
> returns anything)? this should always be a RO operation, no?

I thought I was being consistent with other places in that function--but looking again I don't think so anymore. So I've refactored it in the latest commit.

Thanks!

[lp:~markwash/nova/lp727225]

791. By Mark Washenberger on 2011-03-15

merge lp:nova

792. By Mark Washenberger on 2011-03-15

refactor onset_files quota checking

[Jay Pipes (jaypipes)]

Hi!

Good stuff :)

As mentioned on IRC, I'd suggest a global search and replace:

s/onset_files/injected_files/g

104 + expl = 'Bad personality format: missing %s' % key
107 + raise exc.HTTPBadRequest(explanation='Bad personality format')
111 + msg = 'Personality content for %s cannot be decoded' % path
121 + expl = "Personality file limit exceeded"
124 + expl = "Personality file path too long"
127 + expl = "Personality file content too long"

Please i18n all those! If you're unclear on how to do that, let us know.

309 + return int(FLAGS.quota_max_onset_files)
314 + return int(FLAGS.quota_max_onset_file_content_bytes)
319 + return int(FLAGS.quota_max_onset_file_path_bytes)

You used flags.DEFINE_integer, so the int()'s above are redundant. Not sure any of those functions are really necessary?

Throughout the test cases in TestServerCreateRequestXMLDeserializer, you put backslashes on the end of text lines, when the text is encloses in """'s. This is incorrect and these need to be removed. As an example:

364 + def test_minimal_request(self):
365 + serial_request = """
366 +<server xmlns="http://docs.rackspacecloud.com/servers/api/v1.0"\
367 + name="new-server-test" imageId="1" flavorId="1"/>"""
368 + request = self.deserializer.deserialize(serial_request)
369 + expected = {"server": {
370 + "name": "new-server-test",
371 + "imageId": "1",
372 + "flavorId": "1",
373 + }}
374 + self.assertEquals(request, expected)

Should be:

def test_minimal_request(self):
     serial_request = """
<server xmlns="http://docs.rackspacecloud.com/servers/api/v1.0"
name="new-server-test" imageId="1" flavorId="1"/>"""
    request = self.deserializer.deserialize(serial_request)
    expected = {"server": {
        "name": "new-server-test",
        "imageId": "1",
        "flavorId": "1"}}
    self.assertEquals(request, expected)

This is important, because currently, you are testing an invalid XML string (having a \ between attributes and in other places, having a \ between element tags) and the serializer tests are apparently passing.

507 + def test_request_with_metadata_missing_key(self):
508 + serial_request = """
509 +<server xmlns="http://docs.rackspacecloud.com/servers/api/v1.0"\
510 + name="new-server-test" imageId="1" flavorId="1">\
511 +<metadata><meta>beta</meta></metadata></server>"""
512 + request = self.deserializer.deserialize(serial_request)
513 + expected = {"": "beta"}
514 + self.assertEquals(request["server"]["metadata"], expected)

The test case above technically passes. However, try adding in an additional <meta>gamma</meta> and see if the test passes ;)

Generally, when testing the parsing/deserialization of lists/dicts, you will want to add at least one test that has >1 element in the list being tested, and check that tests with one element work the same as tests with >1 element. Please feel free to add a test case for each of these cases:

1) >1 metadata element with no key
2) >1 injected file in the personality
3) >1 personality in the server

I think you may find some interesting results :)

The tests in TestServerInstanceCreation look excellent. Nice work on those!

Cheers!
jay

review: Needs Fixing

[lp:~markwash/nova/lp727225]

793. By Mark Washenberger on 2011-03-15

s/onset_files/injected_files/g

794. By Mark Washenberger on 2011-03-15

internationalization

795. By Mark Washenberger on 2011-03-15

pep8

796. By Mark Washenberger on 2011-03-15

ignore differently-named nodes in personality and metadata parsing

797. By Mark Washenberger on 2011-03-15

merge lp:nova

798. By Mark Washenberger on 2011-03-15

pep8

[Mark Washenberger (markwash)]

Thanks for the suggestions Jay.

I went ahead and renamed onset_files everywhere to injected_files. I also added internationalization to the strings you identified, and couldn't find any more that needed it.

I pulled the int cast off of the flags, but I decided to go ahead and keep the quota module functions. The don't do much at this time, but it would be better for other parts of the code not to directly access the flags.

I removed most of the backslashes in the xml tests and was pleased to find that my parser did just fine. However, inside personality and metadata extraction, it needed a little "ignore unknown" love, so I added that. I preserved the backslashes in one test, because as far as I know there cannot be newlines in the base64 contents.

I added a few more tests to the deserialization, according to your requests. Everything worked out fine.

Thanks again.

[Jay Pipes (jaypipes)]

Excellent work, Mark. Thanks for those additional tests and cleanups.

review: Approve

[Eric Day (eday)]

I'm good if folks are comfortable with accepting the one-off deserialization for now. We should address the serialization issue in another patch soon (possibly after proposal to rework plus a ML or summit discussion).

review: Approve

[Brian Lamar (blamar)]

Looks good to us....Mark.

review: Approve

[lp:~markwash/nova/lp727225]

799. By Mark Washenberger on 2011-03-16

merge lp:nova and resolve conflicts

[Jay Pipes (jaypipes)]

approved again after merge/resolve.

review: Approve

=== modified file 'Authors'
--- Authors	2011-03-11 08:54:08 +0000
+++ Authors	2011-03-16 19:19:09 +0000
@@ -40,6 +40,7 @@
 Kevin L. Mitchell <[e-mail]>
 Koji Iida <[e-mail]>
 Lorin Hochstein <[e-mail]>
+Mark Washenberger <[e-mail]>
 Masanori Itoh <[e-mail]>
 Matt Dietz <[e-mail]>
 Michael Gundlach <[e-mail]>

=== modified file 'nova/api/openstack/servers.py'
--- nova/api/openstack/servers.py	2011-03-15 02:57:30 +0000
+++ nova/api/openstack/servers.py	2011-03-16 19:19:09 +0000
@@ -13,9 +13,11 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import base64
 import hashlib
 import json
 import traceback
+from xml.dom import minidom
 
 from webob import exc
 
@@ -141,7 +143,7 @@
 
     def create(self, req):
         """ Creates a new server for a given user """
-        env = self._deserialize(req.body, req.get_content_type())
+        env = self._deserialize_create(req)
         if not env:
             return faults.Fault(exc.HTTPUnprocessableEntity())
 
@@ -170,18 +172,24 @@
             for k, v in env['server']['metadata'].items():
                 metadata.append({'key': k, 'value': v})
 
-        instances = self.compute_api.create(
-            context,
-            instance_types.get_by_flavor_id(env['server']['flavorId']),
-            image_id,
-            kernel_id=kernel_id,
-            ramdisk_id=ramdisk_id,
-            display_name=env['server']['name'],
-            display_description=env['server']['name'],
-            key_name=key_name,
-            key_data=key_data,
-            metadata=metadata,
-            onset_files=env.get('onset_files', []))
+        personality = env['server'].get('personality', [])
+        injected_files = self._get_injected_files(personality)
+
+        try:
+            instances = self.compute_api.create(
+                context,
+                instance_types.get_by_flavor_id(env['server']['flavorId']),
+                image_id,
+                kernel_id=kernel_id,
+                ramdisk_id=ramdisk_id,
+                display_name=env['server']['name'],
+                display_description=env['server']['name'],
+                key_name=key_name,
+                key_data=key_data,
+                metadata=metadata,
+                injected_files=injected_files)
+        except QuotaError as error:
+            self._handle_quota_error(error)
 
         server = _translate_keys(instances[0])
         password = "%s%s" % (server['server']['name'][:4],
@@ -191,6 +199,61 @@
                                             password)
         return server
 
+    def _deserialize_create(self, request):
+        """
+        Deserialize a create request
+
+        Overrides normal behavior in the case of xml content
+        """
+        if request.content_type == "application/xml":
+            deserializer = ServerCreateRequestXMLDeserializer()
+            return deserializer.deserialize(request.body)
+        else:
+            return self._deserialize(request.body, request.get_content_type())
+
+    def _get_injected_files(self, personality):
+        """
+        Create a list of injected files from the personality attribute
+
+        At this time, injected_files must be formatted as a list of
+        (file_path, file_content) pairs for compatibility with the
+        underlying compute service.
+        """
+        injected_files = []
+        for item in personality:
+            try:
+                path = item['path']
+                contents = item['contents']
+            except KeyError as key:
+                expl = _('Bad personality format: missing %s') % key
+                raise exc.HTTPBadRequest(explanation=expl)
+            except TypeError:
+                expl = _('Bad personality format')
+                raise exc.HTTPBadRequest(explanation=expl)
+            try:
+                contents = base64.b64decode(contents)
+            except TypeError:
+                expl = _('Personality content for %s cannot be decoded') % path
+                raise exc.HTTPBadRequest(explanation=expl)
+            injected_files.append((path, contents))
+        return injected_files
+
+    def _handle_quota_errors(self, error):
+        """
+        Reraise quota errors as api-specific http exceptions
+        """
+        if error.code == "OnsetFileLimitExceeded":
+            expl = _("Personality file limit exceeded")
+            raise exc.HTTPBadRequest(explanation=expl)
+        if error.code == "OnsetFilePathLimitExceeded":
+            expl = _("Personality file path too long")
+            raise exc.HTTPBadRequest(explanation=expl)
+        if error.code == "OnsetFileContentLimitExceeded":
+            expl = _("Personality file content too long")
+            raise exc.HTTPBadRequest(explanation=expl)
+        # if the original error is okay, just reraise it
+        raise error
+
     def update(self, req, id):
         """ Updates the server name or password """
         if len(req.body) == 0:
@@ -480,3 +543,79 @@
                 _("Ramdisk not found for image %(image_id)s") % locals())
 
         return kernel_id, ramdisk_id
+
+
+class ServerCreateRequestXMLDeserializer(object):
+    """
+    Deserializer to handle xml-formatted server create requests.
+
+    Handles standard server attributes as well as optional metadata
+    and personality attributes
+    """
+
+    def deserialize(self, string):
+        """Deserialize an xml-formatted server create request"""
+        dom = minidom.parseString(string)
+        server = self._extract_server(dom)
+        return {'server': server}
+
+    def _extract_server(self, node):
+        """Marshal the server attribute of a parsed request"""
+        server = {}
+        server_node = self._find_first_child_named(node, 'server')
+        for attr in ["name", "imageId", "flavorId"]:
+            server[attr] = server_node.getAttribute(attr)
+        metadata = self._extract_metadata(server_node)
+        if metadata is not None:
+            server["metadata"] = metadata
+        personality = self._extract_personality(server_node)
+        if personality is not None:
+            server["personality"] = personality
+        return server
+
+    def _extract_metadata(self, server_node):
+        """Marshal the metadata attribute of a parsed request"""
+        metadata_node = self._find_first_child_named(server_node, "metadata")
+        if metadata_node is None:
+            return None
+        metadata = {}
+        for meta_node in self._find_children_named(metadata_node, "meta"):
+            key = meta_node.getAttribute("key")
+            metadata[key] = self._extract_text(meta_node)
+        return metadata
+
+    def _extract_personality(self, server_node):
+        """Marshal the personality attribute of a parsed request"""
+        personality_node = \
+                self._find_first_child_named(server_node, "personality")
+        if personality_node is None:
+            return None
+        personality = []
+        for file_node in self._find_children_named(personality_node, "file"):
+            item = {}
+            if file_node.hasAttribute("path"):
+                item["path"] = file_node.getAttribute("path")
+            item["contents"] = self._extract_text(file_node)
+            personality.append(item)
+        return personality
+
+    def _find_first_child_named(self, parent, name):
+        """Search a nodes children for the first child with a given name"""
+        for node in parent.childNodes:
+            if node.nodeName == name:
+                return node
+        return None
+
+    def _find_children_named(self, parent, name):
+        """Return all of a nodes children who have the given name"""
+        for node in parent.childNodes:
+            if node.nodeName == name:
+                yield node
+
+    def _extract_text(self, node):
+        """Get the text field contained by the given node"""
+        if len(node.childNodes) == 1:
+            child = node.childNodes[0]
+            if child.nodeType == child.TEXT_NODE:
+                return child.nodeValue
+        return ""

=== modified file 'nova/compute/api.py'
--- nova/compute/api.py	2011-03-14 17:59:41 +0000
+++ nova/compute/api.py	2011-03-16 19:19:09 +0000
@@ -80,13 +80,32 @@
                         topic,
                         {"method": "get_network_topic", "args": {'fake': 1}})
 
+    def _check_injected_file_quota(self, context, injected_files):
+        """
+        Enforce quota limits on injected files
+
+        Raises a QuotaError if any limit is exceeded
+        """
+        if injected_files is None:
+            return
+        limit = quota.allowed_injected_files(context)
+        if len(injected_files) > limit:
+            raise quota.QuotaError(code="OnsetFileLimitExceeded")
+        path_limit = quota.allowed_injected_file_path_bytes(context)
+        content_limit = quota.allowed_injected_file_content_bytes(context)
+        for path, content in injected_files:
+            if len(path) > path_limit:
+                raise quota.QuotaError(code="OnsetFilePathLimitExceeded")
+            if len(content) > content_limit:
+                raise quota.QuotaError(code="OnsetFileContentLimitExceeded")
+
     def create(self, context, instance_type,
                image_id, kernel_id=None, ramdisk_id=None,
                min_count=1, max_count=1,
                display_name='', display_description='',
                key_name=None, key_data=None, security_group='default',
                availability_zone=None, user_data=None, metadata=[],
-               onset_files=None):
+               injected_files=None):
         """Create the number of instances requested if quota and
         other arguments check out ok."""
 
@@ -124,6 +143,8 @@
                 LOG.warn(msg)
                 raise quota.QuotaError(msg, "MetadataLimitExceeded")
 
+        self._check_injected_file_quota(context, injected_files)
+
         image = self.image_service.show(context, image_id)
 
         os_type = None
@@ -225,7 +246,7 @@
                       "args": {"topic": FLAGS.compute_topic,
                                "instance_id": instance_id,
                                "availability_zone": availability_zone,
-                               "onset_files": onset_files}})
+                               "injected_files": injected_files}})
 
         for group_id in security_groups:
             self.trigger_security_group_members_refresh(elevated, group_id)

=== modified file 'nova/compute/manager.py'
--- nova/compute/manager.py	2011-03-10 04:30:52 +0000
+++ nova/compute/manager.py	2011-03-16 19:19:09 +0000
@@ -34,7 +34,6 @@
                   :func:`nova.utils.import_object`
 """
 
-import base64
 import datetime
 import os
 import random
@@ -180,7 +179,7 @@
         """Launch a new instance with specified options."""
         context = context.elevated()
         instance_ref = self.db.instance_get(context, instance_id)
-        instance_ref.onset_files = kwargs.get('onset_files', [])
+        instance_ref.injected_files = kwargs.get('injected_files', [])
         if instance_ref['name'] in self.driver.list_instances():
             raise exception.Error(_("Instance has already been created"))
         LOG.audit(_("instance %s: starting..."), instance_id,
@@ -359,15 +358,10 @@
             LOG.warn(_('trying to inject a file into a non-running '
                     'instance: %(instance_id)s (state: %(instance_state)s '
                     'expected: %(expected_state)s)') % locals())
-        # Files/paths *should* be base64-encoded at this point, but
-        # double-check to make sure.
-        b64_path = utils.ensure_b64_encoding(path)
-        b64_contents = utils.ensure_b64_encoding(file_contents)
-        plain_path = base64.b64decode(b64_path)
         nm = instance_ref['name']
-        msg = _('instance %(nm)s: injecting file to %(plain_path)s') % locals()
+        msg = _('instance %(nm)s: injecting file to %(path)s') % locals()
         LOG.audit(msg)
-        self.driver.inject_file(instance_ref, b64_path, b64_contents)
+        self.driver.inject_file(instance_ref, path, file_contents)
 
     @exception.wrap_exception
     @checks_instance_lock

=== modified file 'nova/db/sqlalchemy/models.py'
--- nova/db/sqlalchemy/models.py	2011-03-14 17:59:41 +0000
+++ nova/db/sqlalchemy/models.py	2011-03-16 19:19:09 +0000
@@ -161,7 +161,7 @@
 class Instance(BASE, NovaBase):
     """Represents a guest vm."""
     __tablename__ = 'instances'
-    onset_files = []
+    injected_files = []
 
     id = Column(Integer, primary_key=True, autoincrement=True)
 

=== modified file 'nova/quota.py'
--- nova/quota.py	2011-02-17 23:00:18 +0000
+++ nova/quota.py	2011-03-16 19:19:09 +0000
@@ -37,6 +37,12 @@
                      'number of floating ips allowed per project')
 flags.DEFINE_integer('quota_metadata_items', 128,
                      'number of metadata items allowed per instance')
+flags.DEFINE_integer('quota_max_injected_files', 5,
+                     'number of injected files allowed')
+flags.DEFINE_integer('quota_max_injected_file_content_bytes', 10 * 1024,
+                     'number of bytes allowed per injected file')
+flags.DEFINE_integer('quota_max_injected_file_path_bytes', 255,
+                     'number of bytes allowed per injected file path')
 
 
 def get_quota(context, project_id):
@@ -46,6 +52,7 @@
             'gigabytes': FLAGS.quota_gigabytes,
             'floating_ips': FLAGS.quota_floating_ips,
             'metadata_items': FLAGS.quota_metadata_items}
+
     try:
         quota = db.quota_get(context, project_id)
         for key in rval.keys():
@@ -106,6 +113,21 @@
     return min(num_metadata_items, num_allowed_metadata_items)
 
 
+def allowed_injected_files(context):
+    """Return the number of injected files allowed"""
+    return FLAGS.quota_max_injected_files
+
+
+def allowed_injected_file_content_bytes(context):
+    """Return the number of bytes allowed per injected file content"""
+    return FLAGS.quota_max_injected_file_content_bytes
+
+
+def allowed_injected_file_path_bytes(context):
+    """Return the number of bytes allowed in an injected file path"""
+    return FLAGS.quota_max_injected_file_path_bytes
+
+
 class QuotaError(exception.ApiError):
     """Quota Exceeeded"""
     pass

=== modified file 'nova/tests/api/openstack/fakes.py'
--- nova/tests/api/openstack/fakes.py	2011-03-16 19:02:17 +0000
+++ nova/tests/api/openstack/fakes.py	2011-03-16 19:19:09 +0000
@@ -69,8 +69,6 @@
 @webob.dec.wsgify
 def fake_wsgi(self, req):
     req.environ['nova.context'] = context.RequestContext(1, 1)
-    if req.body:
-        req.environ['inst_dict'] = json.loads(req.body)
     return self.application
 
 

=== modified file 'nova/tests/api/openstack/test_servers.py'
--- nova/tests/api/openstack/test_servers.py	2011-03-16 18:49:25 +0000
+++ nova/tests/api/openstack/test_servers.py	2011-03-16 19:19:09 +0000
@@ -15,8 +15,11 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import base64
 import datetime
 import json
+import unittest
+from xml.dom import minidom
 
 import stubout
 import webob
@@ -607,5 +610,529 @@
         res = req.get_response(fakes.wsgi_app())
         self.assertEqual(res.status_int, 400)
 
+
+class TestServerCreateRequestXMLDeserializer(unittest.TestCase):
+
+    def setUp(self):
+        self.deserializer = servers.ServerCreateRequestXMLDeserializer()
+
+    def test_minimal_request(self):
+        serial_request = """
+<server xmlns="http://docs.rackspacecloud.com/servers/api/v1.0"
+ name="new-server-test" imageId="1" flavorId="1"/>"""
+        request = self.deserializer.deserialize(serial_request)
+        expected = {"server": {
+                "name": "new-server-test",
+                "imageId": "1",
+                "flavorId": "1",
+                }}
+        self.assertEquals(request, expected)
+
+    def test_request_with_empty_metadata(self):
+        serial_request = """
+<server xmlns="http://docs.rackspacecloud.com/servers/api/v1.0"
+ name="new-server-test" imageId="1" flavorId="1">
+    <metadata/>
+</server>"""
+        request = self.deserializer.deserialize(serial_request)
+        expected = {"server": {
+                "name": "new-server-test",
+                "imageId": "1",
+                "flavorId": "1",
+                "metadata": {},
+                }}
+        self.assertEquals(request, expected)
+
+    def test_request_with_empty_personality(self):
+        serial_request = """
+<server xmlns="http://docs.rackspacecloud.com/servers/api/v1.0"
+ name="new-server-test" imageId="1" flavorId="1">
+    <personality/>
+</server>"""
+        request = self.deserializer.deserialize(serial_request)
+        expected = {"server": {
+                "name": "new-server-test",
+                "imageId": "1",
+                "flavorId": "1",
+                "personality": [],
+                }}
+        self.assertEquals(request, expected)
+
+    def test_request_with_empty_metadata_and_personality(self):
+        serial_request = """
+<server xmlns="http://docs.rackspacecloud.com/servers/api/v1.0"
+ name="new-server-test" imageId="1" flavorId="1">
+    <metadata/>
+    <personality/>
+</server>"""
+        request = self.deserializer.deserialize(serial_request)
+        expected = {"server": {
+                "name": "new-server-test",
+                "imageId": "1",
+                "flavorId": "1",
+                "metadata": {},
+                "personality": [],
+                }}
+        self.assertEquals(request, expected)
+
+    def test_request_with_empty_metadata_and_personality_reversed(self):
+        serial_request = """
+<server xmlns="http://docs.rackspacecloud.com/servers/api/v1.0"
+ name="new-server-test" imageId="1" flavorId="1">
+    <personality/>
+    <metadata/>
+</server>"""
+        request = self.deserializer.deserialize(serial_request)
+        expected = {"server": {
+                "name": "new-server-test",
+                "imageId": "1",
+                "flavorId": "1",
+                "metadata": {},
+                "personality": [],
+                }}
+        self.assertEquals(request, expected)
+
+    def test_request_with_one_personality(self):
+        serial_request = """
+<server xmlns="http://docs.rackspacecloud.com/servers/api/v1.0"
+ name="new-server-test" imageId="1" flavorId="1">
+    <personality>
+        <file path="/etc/conf">aabbccdd</file>
+    </personality>
+</server>"""
+        request = self.deserializer.deserialize(serial_request)
+        expected = [{"path": "/etc/conf", "contents": "aabbccdd"}]
+        self.assertEquals(request["server"]["personality"], expected)
+
+    def test_request_with_two_personalities(self):
+        serial_request = """
+<server xmlns="http://docs.rackspacecloud.com/servers/api/v1.0"
+ name="new-server-test" imageId="1" flavorId="1">
+<personality><file path="/etc/conf">aabbccdd</file>
+<file path="/etc/sudoers">abcd</file></personality></server>"""
+        request = self.deserializer.deserialize(serial_request)
+        expected = [{"path": "/etc/conf", "contents": "aabbccdd"},
+                    {"path": "/etc/sudoers", "contents": "abcd"}]
+        self.assertEquals(request["server"]["personality"], expected)
+
+    def test_request_second_personality_node_ignored(self):
+        serial_request = """
+<server xmlns="http://docs.rackspacecloud.com/servers/api/v1.0"
+ name="new-server-test" imageId="1" flavorId="1">
+    <personality>
+        <file path="/etc/conf">aabbccdd</file>
+    </personality>
+    <personality>
+        <file path="/etc/ignoreme">anything</file>
+    </personality>
+</server>"""
+        request = self.deserializer.deserialize(serial_request)
+        expected = [{"path": "/etc/conf", "contents": "aabbccdd"}]
+        self.assertEquals(request["server"]["personality"], expected)
+
+    def test_request_with_one_personality_missing_path(self):
+        serial_request = """
+<server xmlns="http://docs.rackspacecloud.com/servers/api/v1.0"
+ name="new-server-test" imageId="1" flavorId="1">
+<personality><file>aabbccdd</file></personality></server>"""
+        request = self.deserializer.deserialize(serial_request)
+        expected = [{"contents": "aabbccdd"}]
+        self.assertEquals(request["server"]["personality"], expected)
+
+    def test_request_with_one_personality_empty_contents(self):
+        serial_request = """
+<server xmlns="http://docs.rackspacecloud.com/servers/api/v1.0"
+ name="new-server-test" imageId="1" flavorId="1">
+<personality><file path="/etc/conf"></file></personality></server>"""
+        request = self.deserializer.deserialize(serial_request)
+        expected = [{"path": "/etc/conf", "contents": ""}]
+        self.assertEquals(request["server"]["personality"], expected)
+
+    def test_request_with_one_personality_empty_contents_variation(self):
+        serial_request = """
+<server xmlns="http://docs.rackspacecloud.com/servers/api/v1.0"
+ name="new-server-test" imageId="1" flavorId="1">
+<personality><file path="/etc/conf"/></personality></server>"""
+        request = self.deserializer.deserialize(serial_request)
+        expected = [{"path": "/etc/conf", "contents": ""}]
+        self.assertEquals(request["server"]["personality"], expected)
+
+    def test_request_with_one_metadata(self):
+        serial_request = """
+<server xmlns="http://docs.rackspacecloud.com/servers/api/v1.0"
+ name="new-server-test" imageId="1" flavorId="1">
+    <metadata>
+        <meta key="alpha">beta</meta>
+    </metadata>
+</server>"""
+        request = self.deserializer.deserialize(serial_request)
+        expected = {"alpha": "beta"}
+        self.assertEquals(request["server"]["metadata"], expected)
+
+    def test_request_with_two_metadata(self):
+        serial_request = """
+<server xmlns="http://docs.rackspacecloud.com/servers/api/v1.0"
+ name="new-server-test" imageId="1" flavorId="1">
+    <metadata>
+        <meta key="alpha">beta</meta>
+        <meta key="foo">bar</meta>
+    </metadata>
+</server>"""
+        request = self.deserializer.deserialize(serial_request)
+        expected = {"alpha": "beta", "foo": "bar"}
+        self.assertEquals(request["server"]["metadata"], expected)
+
+    def test_request_with_metadata_missing_value(self):
+        serial_request = """
+<server xmlns="http://docs.rackspacecloud.com/servers/api/v1.0"
+ name="new-server-test" imageId="1" flavorId="1">
+    <metadata>
+        <meta key="alpha"></meta>
+    </metadata>
+</server>"""
+        request = self.deserializer.deserialize(serial_request)
+        expected = {"alpha": ""}
+        self.assertEquals(request["server"]["metadata"], expected)
+
+    def test_request_with_two_metadata_missing_value(self):
+        serial_request = """
+<server xmlns="http://docs.rackspacecloud.com/servers/api/v1.0"
+ name="new-server-test" imageId="1" flavorId="1">
+    <metadata>
+        <meta key="alpha"/>
+        <meta key="delta"/>
+    </metadata>
+</server>"""
+        request = self.deserializer.deserialize(serial_request)
+        expected = {"alpha": "", "delta": ""}
+        self.assertEquals(request["server"]["metadata"], expected)
+
+    def test_request_with_metadata_missing_key(self):
+        serial_request = """
+<server xmlns="http://docs.rackspacecloud.com/servers/api/v1.0"
+ name="new-server-test" imageId="1" flavorId="1">
+    <metadata>
+        <meta>beta</meta>
+    </metadata>
+</server>"""
+        request = self.deserializer.deserialize(serial_request)
+        expected = {"": "beta"}
+        self.assertEquals(request["server"]["metadata"], expected)
+
+    def test_request_with_two_metadata_missing_key(self):
+        serial_request = """
+<server xmlns="http://docs.rackspacecloud.com/servers/api/v1.0"
+ name="new-server-test" imageId="1" flavorId="1">
+    <metadata>
+        <meta>beta</meta>
+        <meta>gamma</meta>
+    </metadata>
+</server>"""
+        request = self.deserializer.deserialize(serial_request)
+        expected = {"": "gamma"}
+        self.assertEquals(request["server"]["metadata"], expected)
+
+    def test_request_with_metadata_duplicate_key(self):
+        serial_request = """
+<server xmlns="http://docs.rackspacecloud.com/servers/api/v1.0"
+ name="new-server-test" imageId="1" flavorId="1">
+    <metadata>
+        <meta key="foo">bar</meta>
+        <meta key="foo">baz</meta>
+    </metadata>
+</server>"""
+        request = self.deserializer.deserialize(serial_request)
+        expected = {"foo": "baz"}
+        self.assertEquals(request["server"]["metadata"], expected)
+
+    def test_canonical_request_from_docs(self):
+        serial_request = """
+<server xmlns="http://docs.rackspacecloud.com/servers/api/v1.0"
+ name="new-server-test" imageId="1" flavorId="1">
+    <metadata>
+        <meta key="My Server Name">Apache1</meta>
+    </metadata>
+    <personality>
+        <file path="/etc/banner.txt">\
+ICAgICAgDQoiQSBjbG91ZCBkb2VzIG5vdCBrbm93IHdoeSBp\
+dCBtb3ZlcyBpbiBqdXN0IHN1Y2ggYSBkaXJlY3Rpb24gYW5k\
+IGF0IHN1Y2ggYSBzcGVlZC4uLkl0IGZlZWxzIGFuIGltcHVs\
+c2lvbi4uLnRoaXMgaXMgdGhlIHBsYWNlIHRvIGdvIG5vdy4g\
+QnV0IHRoZSBza3kga25vd3MgdGhlIHJlYXNvbnMgYW5kIHRo\
+ZSBwYXR0ZXJucyBiZWhpbmQgYWxsIGNsb3VkcywgYW5kIHlv\
+dSB3aWxsIGtub3csIHRvbywgd2hlbiB5b3UgbGlmdCB5b3Vy\
+c2VsZiBoaWdoIGVub3VnaCB0byBzZWUgYmV5b25kIGhvcml6\
+b25zLiINCg0KLVJpY2hhcmQgQmFjaA==</file>
+    </personality>
+</server>"""
+        expected = {"server": {
+            "name": "new-server-test",
+            "imageId": "1",
+            "flavorId": "1",
+            "metadata": {
+                "My Server Name": "Apache1",
+            },
+            "personality": [
+                {
+                    "path": "/etc/banner.txt",
+                    "contents": """\
+ICAgICAgDQoiQSBjbG91ZCBkb2VzIG5vdCBrbm93IHdoeSBp\
+dCBtb3ZlcyBpbiBqdXN0IHN1Y2ggYSBkaXJlY3Rpb24gYW5k\
+IGF0IHN1Y2ggYSBzcGVlZC4uLkl0IGZlZWxzIGFuIGltcHVs\
+c2lvbi4uLnRoaXMgaXMgdGhlIHBsYWNlIHRvIGdvIG5vdy4g\
+QnV0IHRoZSBza3kga25vd3MgdGhlIHJlYXNvbnMgYW5kIHRo\
+ZSBwYXR0ZXJucyBiZWhpbmQgYWxsIGNsb3VkcywgYW5kIHlv\
+dSB3aWxsIGtub3csIHRvbywgd2hlbiB5b3UgbGlmdCB5b3Vy\
+c2VsZiBoaWdoIGVub3VnaCB0byBzZWUgYmV5b25kIGhvcml6\
+b25zLiINCg0KLVJpY2hhcmQgQmFjaA==""",
+                },
+            ],
+        }}
+        request = self.deserializer.deserialize(serial_request)
+        self.assertEqual(request, expected)
+
+
+class TestServerInstanceCreation(test.TestCase):
+
+    def setUp(self):
+        super(TestServerInstanceCreation, self).setUp()
+        self.stubs = stubout.StubOutForTesting()
+        fakes.FakeAuthManager.auth_data = {}
+        fakes.FakeAuthDatabase.data = {}
+        fakes.stub_out_auth(self.stubs)
+        fakes.stub_out_key_pair_funcs(self.stubs)
+        self.allow_admin = FLAGS.allow_admin_api
+
+    def tearDown(self):
+        self.stubs.UnsetAll()
+        FLAGS.allow_admin_api = self.allow_admin
+        super(TestServerInstanceCreation, self).tearDown()
+
+    def _setup_mock_compute_api_for_personality(self):
+
+        class MockComputeAPI(object):
+
+            def __init__(self):
+                self.injected_files = None
+
+            def create(self, *args, **kwargs):
+                if 'injected_files' in kwargs:
+                    self.injected_files = kwargs['injected_files']
+                else:
+                    self.injected_files = None
+                return [{'id': '1234', 'display_name': 'fakeinstance'}]
+
+            def set_admin_password(self, *args, **kwargs):
+                pass
+
+        def make_stub_method(canned_return):
+            def stub_method(*args, **kwargs):
+                return canned_return
+            return stub_method
+
+        compute_api = MockComputeAPI()
+        self.stubs.Set(nova.compute, 'API', make_stub_method(compute_api))
+        self.stubs.Set(nova.api.openstack.servers.Controller,
+            '_get_kernel_ramdisk_from_image', make_stub_method((1, 1)))
+        self.stubs.Set(nova.api.openstack.common,
+            'get_image_id_from_image_hash', make_stub_method(2))
+        return compute_api
+
+    def _create_personality_request_dict(self, personality_files):
+        server = {}
+        server['name'] = 'new-server-test'
+        server['imageId'] = 1
+        server['flavorId'] = 1
+        if personality_files is not None:
+            personalities = []
+            for path, contents in personality_files:
+                personalities.append({'path': path, 'contents': contents})
+            server['personality'] = personalities
+        return {'server': server}
+
+    def _get_create_request_json(self, body_dict):
+        req = webob.Request.blank('/v1.0/servers')
+        req.content_type = 'application/json'
+        req.method = 'POST'
+        req.body = json.dumps(body_dict)
+        return req
+
+    def _run_create_instance_with_mock_compute_api(self, request):
+        compute_api = self._setup_mock_compute_api_for_personality()
+        response = request.get_response(fakes.wsgi_app())
+        return compute_api, response
+
+    def _format_xml_request_body(self, body_dict):
+        server = body_dict['server']
+        body_parts = []
+        body_parts.extend([
+            '<?xml version="1.0" encoding="UTF-8"?>',
+            '<server xmlns="http://docs.rackspacecloud.com/servers/api/v1.0"',
+            ' name="%s" imageId="%s" flavorId="%s">' % (
+                    server['name'], server['imageId'], server['flavorId'])])
+        if 'metadata' in server:
+            metadata = server['metadata']
+            body_parts.append('<metadata>')
+            for item in metadata.iteritems():
+                body_parts.append('<meta key="%s">%s</meta>' % item)
+            body_parts.append('</metadata>')
+        if 'personality' in server:
+            personalities = server['personality']
+            body_parts.append('<personality>')
+            for file in personalities:
+                item = (file['path'], file['contents'])
+                body_parts.append('<file path="%s">%s</file>' % item)
+            body_parts.append('</personality>')
+        body_parts.append('</server>')
+        return ''.join(body_parts)
+
+    def _get_create_request_xml(self, body_dict):
+        req = webob.Request.blank('/v1.0/servers')
+        req.content_type = 'application/xml'
+        req.accept = 'application/xml'
+        req.method = 'POST'
+        req.body = self._format_xml_request_body(body_dict)
+        return req
+
+    def _create_instance_with_personality_json(self, personality):
+        body_dict = self._create_personality_request_dict(personality)
+        request = self._get_create_request_json(body_dict)
+        compute_api, response = \
+            self._run_create_instance_with_mock_compute_api(request)
+        return request, response, compute_api.injected_files
+
+    def _create_instance_with_personality_xml(self, personality):
+        body_dict = self._create_personality_request_dict(personality)
+        request = self._get_create_request_xml(body_dict)
+        compute_api, response = \
+            self._run_create_instance_with_mock_compute_api(request)
+        return request, response, compute_api.injected_files
+
+    def test_create_instance_with_no_personality(self):
+        request, response, injected_files = \
+                self._create_instance_with_personality_json(personality=None)
+        self.assertEquals(response.status_int, 200)
+        self.assertEquals(injected_files, [])
+
+    def test_create_instance_with_no_personality_xml(self):
+        request, response, injected_files = \
+                self._create_instance_with_personality_xml(personality=None)
+        self.assertEquals(response.status_int, 200)
+        self.assertEquals(injected_files, [])
+
+    def test_create_instance_with_personality(self):
+        path = '/my/file/path'
+        contents = '#!/bin/bash\necho "Hello, World!"\n'
+        b64contents = base64.b64encode(contents)
+        personality = [(path, b64contents)]
+        request, response, injected_files = \
+            self._create_instance_with_personality_json(personality)
+        self.assertEquals(response.status_int, 200)
+        self.assertEquals(injected_files, [(path, contents)])
+
+    def test_create_instance_with_personality_xml(self):
+        path = '/my/file/path'
+        contents = '#!/bin/bash\necho "Hello, World!"\n'
+        b64contents = base64.b64encode(contents)
+        personality = [(path, b64contents)]
+        request, response, injected_files = \
+            self._create_instance_with_personality_xml(personality)
+        self.assertEquals(response.status_int, 200)
+        self.assertEquals(injected_files, [(path, contents)])
+
+    def test_create_instance_with_personality_no_path(self):
+        personality = [('/remove/this/path',
+            base64.b64encode('my\n\file\ncontents'))]
+        body_dict = self._create_personality_request_dict(personality)
+        del body_dict['server']['personality'][0]['path']
+        request = self._get_create_request_json(body_dict)
+        compute_api, response = \
+            self._run_create_instance_with_mock_compute_api(request)
+        self.assertEquals(response.status_int, 400)
+        self.assertEquals(compute_api.injected_files, None)
+
+    def _test_create_instance_with_personality_no_path_xml(self):
+        personality = [('/remove/this/path',
+            base64.b64encode('my\n\file\ncontents'))]
+        body_dict = self._create_personality_request_dict(personality)
+        request = self._get_create_request_xml(body_dict)
+        request.body = request.body.replace(' path="/remove/this/path"', '')
+        compute_api, response = \
+            self._run_create_instance_with_mock_compute_api(request)
+        self.assertEquals(response.status_int, 400)
+        self.assertEquals(compute_api.injected_files, None)
+
+    def test_create_instance_with_personality_no_contents(self):
+        personality = [('/test/path',
+            base64.b64encode('remove\nthese\ncontents'))]
+        body_dict = self._create_personality_request_dict(personality)
+        del body_dict['server']['personality'][0]['contents']
+        request = self._get_create_request_json(body_dict)
+        compute_api, response = \
+            self._run_create_instance_with_mock_compute_api(request)
+        self.assertEquals(response.status_int, 400)
+        self.assertEquals(compute_api.injected_files, None)
+
+    def test_create_instance_with_personality_not_a_list(self):
+        personality = [('/test/path', base64.b64encode('test\ncontents\n'))]
+        body_dict = self._create_personality_request_dict(personality)
+        body_dict['server']['personality'] = \
+            body_dict['server']['personality'][0]
+        request = self._get_create_request_json(body_dict)
+        compute_api, response = \
+            self._run_create_instance_with_mock_compute_api(request)
+        self.assertEquals(response.status_int, 400)
+        self.assertEquals(compute_api.injected_files, None)
+
+    def test_create_instance_with_personality_with_non_b64_content(self):
+        path = '/my/file/path'
+        contents = '#!/bin/bash\necho "Oh no!"\n'
+        personality = [(path, contents)]
+        request, response, injected_files = \
+            self._create_instance_with_personality_json(personality)
+        self.assertEquals(response.status_int, 400)
+        self.assertEquals(injected_files, None)
+
+    def test_create_instance_with_three_personalities(self):
+        files = [
+            ('/etc/sudoers', 'ALL ALL=NOPASSWD: ALL\n'),
+            ('/etc/motd', 'Enjoy your root access!\n'),
+            ('/etc/dovecot.conf', 'dovecot\nconfig\nstuff\n'),
+            ]
+        personality = []
+        for path, content in files:
+            personality.append((path, base64.b64encode(content)))
+        request, response, injected_files = \
+            self._create_instance_with_personality_json(personality)
+        self.assertEquals(response.status_int, 200)
+        self.assertEquals(injected_files, files)
+
+    def test_create_instance_personality_empty_content(self):
+        path = '/my/file/path'
+        contents = ''
+        personality = [(path, contents)]
+        request, response, injected_files = \
+            self._create_instance_with_personality_json(personality)
+        self.assertEquals(response.status_int, 200)
+        self.assertEquals(injected_files, [(path, contents)])
+
+    def test_create_instance_admin_pass_json(self):
+        request, response, dummy = \
+            self._create_instance_with_personality_json(None)
+        self.assertEquals(response.status_int, 200)
+        response = json.loads(response.body)
+        self.assertTrue('adminPass' in response['server'])
+        self.assertTrue(response['server']['adminPass'].startswith('fake'))
+
+    def test_create_instance_admin_pass_xml(self):
+        request, response, dummy = \
+            self._create_instance_with_personality_xml(None)
+        self.assertEquals(response.status_int, 200)
+        dom = minidom.parseString(response.body)
+        server = dom.childNodes[0]
+        self.assertEquals(server.nodeName, 'server')
+        self.assertTrue(server.getAttribute('adminPass').startswith('fake'))
+
+
 if __name__ == "__main__":
     unittest.main()

=== modified file 'nova/tests/test_quota.py'
--- nova/tests/test_quota.py	2011-03-07 01:25:01 +0000
+++ nova/tests/test_quota.py	2011-03-16 19:19:09 +0000
@@ -33,6 +33,12 @@
 
 
 class QuotaTestCase(test.TestCase):
+
+    class StubImageService(object):
+
+        def show(self, *args, **kwargs):
+            return {"properties": {}}
+
     def setUp(self):
         super(QuotaTestCase, self).setUp()
         self.flags(connection_type='fake',
@@ -193,3 +199,67 @@
                                             instance_type='m1.small',
                                             image_id='fake',
                                             metadata=metadata)
+
+    def test_allowed_injected_files(self):
+        self.assertEqual(
+                quota.allowed_injected_files(self.context),
+                FLAGS.quota_max_injected_files)
+
+    def _create_with_injected_files(self, files):
+        api = compute.API(image_service=self.StubImageService())
+        api.create(self.context, min_count=1, max_count=1,
+                instance_type='m1.small', image_id='fake',
+                injected_files=files)
+
+    def test_no_injected_files(self):
+        api = compute.API(image_service=self.StubImageService())
+        api.create(self.context, instance_type='m1.small', image_id='fake')
+
+    def test_max_injected_files(self):
+        files = []
+        for i in xrange(FLAGS.quota_max_injected_files):
+            files.append(('/my/path%d' % i, 'config = test\n'))
+        self._create_with_injected_files(files)  # no QuotaError
+
+    def test_too_many_injected_files(self):
+        files = []
+        for i in xrange(FLAGS.quota_max_injected_files + 1):
+            files.append(('/my/path%d' % i, 'my\ncontent%d\n' % i))
+        self.assertRaises(quota.QuotaError,
+                          self._create_with_injected_files, files)
+
+    def test_allowed_injected_file_content_bytes(self):
+        self.assertEqual(
+                quota.allowed_injected_file_content_bytes(self.context),
+                FLAGS.quota_max_injected_file_content_bytes)
+
+    def test_max_injected_file_content_bytes(self):
+        max = FLAGS.quota_max_injected_file_content_bytes
+        content = ''.join(['a' for i in xrange(max)])
+        files = [('/test/path', content)]
+        self._create_with_injected_files(files)  # no QuotaError
+
+    def test_too_many_injected_file_content_bytes(self):
+        max = FLAGS.quota_max_injected_file_content_bytes
+        content = ''.join(['a' for i in xrange(max + 1)])
+        files = [('/test/path', content)]
+        self.assertRaises(quota.QuotaError,
+                          self._create_with_injected_files, files)
+
+    def test_allowed_injected_file_path_bytes(self):
+        self.assertEqual(
+                quota.allowed_injected_file_path_bytes(self.context),
+                FLAGS.quota_max_injected_file_path_bytes)
+
+    def test_max_injected_file_path_bytes(self):
+        max = FLAGS.quota_max_injected_file_path_bytes
+        path = ''.join(['a' for i in xrange(max)])
+        files = [(path, 'config = quotatest')]
+        self._create_with_injected_files(files)  # no QuotaError
+
+    def test_too_many_injected_file_path_bytes(self):
+        max = FLAGS.quota_max_injected_file_path_bytes
+        path = ''.join(['a' for i in xrange(max + 1)])
+        files = [(path, 'config = quotatest')]
+        self.assertRaises(quota.QuotaError,
+                          self._create_with_injected_files, files)

=== modified file 'nova/utils.py'
--- nova/utils.py	2011-03-15 18:24:07 +0000
+++ nova/utils.py	2011-03-16 19:19:09 +0000
@@ -538,18 +538,6 @@
     return wrap
 
 
-def ensure_b64_encoding(val):
-    """Safety method to ensure that values expected to be base64-encoded
-    actually are. If they are, the value is returned unchanged. Otherwise,
-    the encoded value is returned.
-    """
-    try:
-        dummy = base64.decode(val)
-        return val
-    except TypeError:
-        return base64.b64encode(val)
-
-
 def get_from_path(items, path):
     """ Returns a list of items matching the specified path.  Takes an
     XPath-like expression e.g. prop1/prop2/prop3, and for each item in items,

=== modified file 'nova/virt/xenapi/vmops.py'
--- nova/virt/xenapi/vmops.py	2011-03-12 00:00:34 +0000
+++ nova/virt/xenapi/vmops.py	2011-03-16 19:19:09 +0000
@@ -19,6 +19,7 @@
 Management class for VM-related functions (spawn, reboot, etc).
 """
 
+import base64
 import json
 import M2Crypto
 import os
@@ -136,19 +137,20 @@
         LOG.info(_('Spawning VM %(instance_name)s created %(vm_ref)s.')
                  % locals())
 
-        def _inject_onset_files():
-            onset_files = instance.onset_files
-            if onset_files:
+        def _inject_files():
+            injected_files = instance.injected_files
+            if injected_files:
                 # Check if this is a JSON-encoded string and convert if needed.
-                if isinstance(onset_files, basestring):
+                if isinstance(injected_files, basestring):
                     try:
-                        onset_files = json.loads(onset_files)
+                        injected_files = json.loads(injected_files)
                     except ValueError:
-                        LOG.exception(_("Invalid value for onset_files: '%s'")
-                                % onset_files)
-                        onset_files = []
+                        LOG.exception(
+                            _("Invalid value for injected_files: '%s'")
+                                % injected_files)
+                        injected_files = []
                 # Inject any files, if specified
-                for path, contents in instance.onset_files:
+                for path, contents in instance.injected_files:
                     LOG.debug(_("Injecting file path: '%s'") % path)
                     self.inject_file(instance, path, contents)
         # NOTE(armando): Do we really need to do this in virt?
@@ -164,7 +166,7 @@
                 if state == power_state.RUNNING:
                     LOG.debug(_('Instance %s: booted'), instance_name)
                     timer.stop()
-                    _inject_onset_files()
+                    _inject_files()
                     return True
             except Exception, exc:
                 LOG.warn(exc)
@@ -408,17 +410,16 @@
             raise RuntimeError(resp_dict['message'])
         return resp_dict['message']
 
-    def inject_file(self, instance, b64_path, b64_contents):
+    def inject_file(self, instance, path, contents):
         """Write a file to the VM instance. The path to which it is to be
-        written and the contents of the file need to be supplied; both should
+        written and the contents of the file need to be supplied; both will
         be base64-encoded to prevent errors with non-ASCII characters being
         transmitted. If the agent does not support file injection, or the user
         has disabled it, a NotImplementedError will be raised.
         """
-        # Files/paths *should* be base64-encoded at this point, but
-        # double-check to make sure.
-        b64_path = utils.ensure_b64_encoding(b64_path)
-        b64_contents = utils.ensure_b64_encoding(b64_contents)
+        # Files/paths must be base64-encoded for transmission to agent
+        b64_path = base64.b64encode(path)
+        b64_contents = base64.b64encode(contents)
 
         # Need to uniquely identify this request.
         transaction_id = str(uuid.uuid4())