Merge lp:~jaypipes/glance/checksum into lp:~glance-coresec/glance/cactus-trunk

[Jay Pipes]

Add checksumming capabilities to Glance

Adds checksumming to Glance.

When adding an image (or uploading an image during PUT operations),
you may now supply an optional X-Image-Meta-Checksum header. When
storing the uploaded image, the backend image stores now are required
to return a checksum of the data they just stored. The optional
X-Image-Meta-Checksum header is compared against this generated checksum
and returns a 409 Bad Request if there is a mismatch.

The ETag header is now properly set to the image's checksum now
for all GET /images/<ID>, HEAD /images/<ID>, POST /images and
PUT /images/<ID> operations.

Adds unit tests verifying the checksumming behaviour in the API, and
in the Swift and Filesystem backend stores.

NOTE: This does not include the DB migration script. Separate bug will be filed for that.

[Chuck Thier (cthier)]

The swift parts look reasonable to me. The only other thing that I might add is to clarify in the docs that the checksum is an md5 checksum, otherwise people may try to use CRC.

review: Approve

[Jay Pipes (jaypipes)]

thx Chuck. Made a note in the docs about the checksum being an MD5 checksum.

[lp:~jaypipes/glance/checksum]

81. By Jay Pipes on 2011-03-08

Make it clear that the checksum is an MD5 checksum in docs.

[Rick Harris (rconradharris)]

Nice patch.

Unfortunately with the state of Nova <=> Glance trunks at the moment, I wasn't able to test functionally. That said, this seems pretty safe to go ahead and merge, and if we have issues, we can fix them along with all the other fixes that are in progress at the moment :)

> 110 + checksum = Column(String(32))

I assume the answer is YAGNI, but I'll ask it anyway: Will we ever allow SHA1 or SHA256?

Would it make sense to make this a String(64) or, heck, a String(255) for flexibility?

review: Approve

[Jay Pipes (jaypipes)]

> > 110 + checksum = Column(String(32))
>
> I assume the answer is YAGNI, but I'll ask it anyway: Will we ever allow SHA1
> or SHA256?
>
> Would it make sense to make this a String(64) or, heck, a String(255) for
> flexibility?

Hmm, good question. Not sure, really. I suppose we could expand it in the future if we decide its a needed feature.

[Cory Wright (corywright)]

I noticed that there is no migration script to add the checksum column. Trunk is already missing a migration for disk_format/container_format, but we probably shouldn't get in the habit of merging to trunk without migrations, since that effectively leaves trunk broken. However, the tests seem to run fine without the migration.

I tried merging trunk into this branch to test these changes with the glance cli tool, but there were conflicts so you may want to merge trunk once more.

Otherwise, this looks good to me. I'm going to mark it as Needs Fixing because of the missing migration. If you think it's ok to merge this to trunk without the migration then let me know and I'll mark this approved.

review: Needs Fixing

[Jay Pipes (jaypipes)]

> I noticed that there is no migration script to add the checksum column. Trunk
> is already missing a migration for disk_format/container_format, but we
> probably shouldn't get in the habit of merging to trunk without migrations,
> since that effectively leaves trunk broken. However, the tests seem to run
> fine without the migration.
>
> I tried merging trunk into this branch to test these changes with the glance
> cli tool, but there were conflicts so you may want to merge trunk once more.
>
> Otherwise, this looks good to me. I'm going to mark it as Needs Fixing
> because of the missing migration. If you think it's ok to merge this to trunk
> without the migration then let me know and I'll mark this approved.

Agreed, though I did note this in the merge proposal commit message:

"NOTE: This does not include the DB migration script. Separate bug will be filed for that."

But, I'll work on a migrate script after merging trunk again. Thanks, Cory!

[Cory Wright (corywright)]

You sure did mention it, sorry about that. I read the commit message yesterday when I first looked at this and forgot.

[Sandy Walsh (sandy-walsh)]

Hmm, I got bitten by this migration bug yesterday ... can't this be put in this branch to keep trunk happy?

[Jay Pipes (jaypipes)]

> Hmm, I got bitten by this migration bug yesterday ... can't this be put in
> this branch to keep trunk happy?

? This hasn't been merged yet, so I'm unclear how this could have bitten you :)

FYI, I've now figured out the migrations stuff. Full steam ahead today on getting the image format migration bug done and getting this branch merged with a migration file.

[lp:~jaypipes/glance/checksum]

82. By Jay Pipes on 2011-03-23

Merge trunk and resolve conflicts

83. By Jay Pipes on 2011-03-23

Merge bug730213 (Migration fixes and tests)

[Sandy Walsh (sandy-walsh)]

sorry, it was a nova issue. _get_kernel_ramdisk_from_image checks 'disk_format' != 'ami'

I guess that just stresses the importance of having the migrations in place with the branch :)

Minor tweak ... ping me and I'll approve immediately after

278 missing _() ?

review: Needs Fixing

[Jay Pipes (jaypipes)]

On Wed, Mar 23, 2011 at 10:26 AM, Sandy Walsh <email address hidden> wrote:
> Review: Needs Fixing
> sorry, it was a nova issue. _get_kernel_ramdisk_from_image checks 'disk_format' != 'ami'
>
> I guess that just stresses the importance of having the migrations in place with the branch :)

Ya, I'm currently working on the migration script. should be done shortly...

> Minor tweak ... ping me and I'll approve immediately after
>
> 278 missing _() ?

Glance doesn't have i18n. Yet :)

[lp:~jaypipes/glance/checksum]

84. By Jay Pipes on 2011-03-23

Merge fix from bug730213

85. By Jay Pipes on 2011-03-23

Add migration script for checksum column

[Sandy Walsh (sandy-walsh)]

ah! haha

review: Approve

[lp:~jaypipes/glance/checksum]

86. By Jay Pipes on 2011-03-25

Merge trunk

87. By Jay Pipes on 2011-03-25

Fix up test case after merging in bug fixes from trunk... expected results were incorrect in curl test

88. By Jay Pipes on 2011-03-25

Uhhhm, stop_servers() should stop servers, not start them! Thanks to Cory for uncovering this copy/paste fail.

Unmerged revisions

=== modified file 'doc/source/glanceapi.rst'
--- doc/source/glanceapi.rst	2011-03-05 17:04:43 +0000
+++ doc/source/glanceapi.rst	2011-03-23 15:06:49 +0000
@@ -67,6 +67,7 @@
      'disk_format': 'vhd',
      'container_format': 'ovf',
      'size': '5368709120',
+     'checksum': 'c2e5db72bd7fd153f53ede5da5a06de3',
      'location': 'swift://account:key/container/image.tar.gz.0',
      'created_at': '2010-02-03 09:34:01',
      'updated_at': '2010-02-03 09:34:01',
@@ -89,6 +90,8 @@
   The `properties` field is a mapping of free-form key/value pairs that
   have been saved with the image metadata
 
+  The `checksum` field is an MD5 checksum of the image file data
+
 
 Requesting Detailed Metadata on a Specific Image
 ------------------------------------------------
@@ -116,6 +119,7 @@
   x-image-meta-disk-format      vhd
   x-image-meta-container-format ovf
   x-image-meta-size             5368709120
+  x-image-meta-checksum         c2e5db72bd7fd153f53ede5da5a06de3
   x-image-meta-location         swift://account:key/container/image.tar.gz.0
   x-image-meta-created_at       2010-02-03 09:34:01
   x-image-meta-updated_at       2010-02-03 09:34:01
@@ -137,6 +141,9 @@
   that have been saved with the image metadata. The key is the string
   after `x-image-meta-property-` and the value is the value of the header
 
+  The response's `ETag` header will always be equal to the
+  `x-image-meta-checksum` value
+
 
 Retrieving a Virtual Machine Image
 ----------------------------------
@@ -166,6 +173,7 @@
   x-image-meta-disk-format      vhd
   x-image-meta-container-format ovf
   x-image-meta-size             5368709120
+  x-image-meta-checksum         c2e5db72bd7fd153f53ede5da5a06de3
   x-image-meta-location         swift://account:key/container/image.tar.gz.0
   x-image-meta-created_at       2010-02-03 09:34:01
   x-image-meta-updated_at       2010-02-03 09:34:01
@@ -190,6 +198,9 @@
   The response's `Content-Length` header shall be equal to the value of
   the `x-image-meta-size` header
 
+  The response's `ETag` header will always be equal to the
+  `x-image-meta-checksum` value
+
   The image data itself will be the body of the HTTP response returned
   from the request, which will have content-type of
   `application/octet-stream`.
@@ -284,6 +295,15 @@
   When not present, Glance will calculate the image's size based on the size
   of the request body.
 
+* ``x-image-meta-checksum``
+
+  This header is optional. When present it shall be the expected **MD5**
+  checksum of the image file data.
+
+  When present, Glance will verify the checksum generated from the backend
+  store when storing your image against this value and return a 
+  **400 Bad Request** if the values do not match.
+
 * ``x-image-meta-is-public``
 
   This header is optional.
 
=== modified file 'glance/client.py'
--- glance/client.py	2011-03-06 17:02:44 +0000
+++ glance/client.py	2011-03-23 15:06:49 +0000
@@ -142,7 +142,10 @@
                 c.request(method, action, body, headers)
             res = c.getresponse()
             status_code = self.get_status_code(res)
-            if status_code == httplib.OK:
+            if status_code in (httplib.OK,
+                               httplib.CREATED,
+                               httplib.ACCEPTED,
+                               httplib.NO_CONTENT):
                 return res
             elif status_code == httplib.UNAUTHORIZED:
                 raise exception.NotAuthorized
 
=== modified file 'glance/registry/db/api.py'
--- glance/registry/db/api.py	2011-03-14 19:10:24 +0000
+++ glance/registry/db/api.py	2011-03-23 15:06:49 +0000
@@ -43,7 +43,7 @@
 
 IMAGE_ATTRS = BASE_MODEL_ATTRS | set(['name', 'status', 'size',
                                       'disk_format', 'container_format',
-                                      'is_public', 'location'])
+                                      'is_public', 'location', 'checksum'])
 
 CONTAINER_FORMATS = ['ami', 'ari', 'aki', 'bare', 'ovf']
 DISK_FORMATS = ['ami', 'ari', 'aki', 'vhd', 'vmdk', 'raw', 'qcow2', 'vdi']
 
=== added file 'glance/registry/db/migrate_repo/versions/003_add_disk_format.py'
--- glance/registry/db/migrate_repo/versions/003_add_disk_format.py	1970-01-01 00:00:00 +0000
+++ glance/registry/db/migrate_repo/versions/003_add_disk_format.py	2011-03-23 15:06:49 +0000
@@ -0,0 +1,147 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2011 OpenStack LLC.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from migrate.changeset import *
+from sqlalchemy import *
+from sqlalchemy.sql import and_, not_
+
+from glance.registry.db.migrate_repo.schema import (
+    Boolean, DateTime, Integer, String, Text, from_migration_import)
+
+
+def get_images_table(meta):
+    """
+    Returns the Table object for the images table that
+    corresponds to the images table definition of this version.
+    """
+    images = Table('images', meta,
+        Column('id', Integer(), primary_key=True, nullable=False),
+        Column('name', String(255)),
+        Column('disk_format', String(20)),
+        Column('container_format', String(20)),
+        Column('size', Integer()),
+        Column('status', String(30), nullable=False),
+        Column('is_public', Boolean(), nullable=False, default=False,
+               index=True),
+        Column('location', Text()),
+        Column('created_at', DateTime(), nullable=False),
+        Column('updated_at', DateTime()),
+        Column('deleted_at', DateTime()),
+        Column('deleted', Boolean(), nullable=False, default=False,
+               index=True),
+        mysql_engine='InnoDB',
+        useexisting=True)
+
+    return images
+
+
+def get_image_properties_table(meta):
+    """
+    No changes to the image properties table from 002...
+    """
+    (define_image_properties_table,) = from_migration_import(
+        '002_add_image_properties_table', ['define_image_properties_table'])
+
+    image_properties = define_image_properties_table(meta)
+    return image_properties
+
+
+def upgrade(migrate_engine):
+    meta = MetaData()
+    meta.bind = migrate_engine
+    (define_images_table,) = from_migration_import(
+        '001_add_images_table', ['define_images_table'])
+    (define_image_properties_table,) = from_migration_import(
+        '002_add_image_properties_table', ['define_image_properties_table'])
+
+    conn = migrate_engine.connect()
+    images = define_images_table(meta)
+    image_properties = define_image_properties_table(meta)
+
+    # Steps to take, in this order:
+    # 1) Move the existing type column from Image into
+    #    ImageProperty for all image records that have a non-NULL
+    #    type column
+    # 2) Drop the type column in images
+    # 3) Add the new columns to images
+
+    # The below wackiness correlates to the following ANSI SQL:
+    #   SELECT images.* FROM images
+    #   LEFT JOIN image_properties
+    #   ON images.id = image_properties.image_id
+    #   AND image_properties.key = 'type'
+    #   WHERE image_properties.image_id IS NULL
+    #   AND images.type IS NOT NULL
+    #
+    # which returns all the images that have a type set
+    # but that DO NOT yet have an image_property record
+    # with key of type.
+    sel = select([images], from_obj=[
+        images.outerjoin(image_properties,
+                         and_(images.c.id == image_properties.c.image_id,
+                              image_properties.c.key == 'type'))]).where(
+            and_(image_properties.c.image_id == None,
+                 images.c.type != None))
+    image_records = conn.execute(sel).fetchall()
+    property_insert = image_properties.insert()
+    for record in image_records:
+        conn.execute(property_insert,
+                     image_id=record.id,
+                     key='type',
+                     created_at=record.created_at,
+                     deleted=False,
+                     value=record.type)
+
+    disk_format = Column('disk_format', String(20))
+    disk_format.create(images)
+    container_format = Column('container_format', String(20))
+    container_format.create(images)
+
+    images.columns['type'].drop()
+    conn.close()
+
+
+def downgrade(migrate_engine):
+    meta = MetaData()
+    meta.bind = migrate_engine
+
+    # Steps to take, in this order:
+    # 1) Add type column back to Image
+    # 2) Move the existing type properties from ImageProperty into
+    #    Image.type
+    # 3) Drop the disk_format and container_format columns in Image
+
+    conn = migrate_engine.connect()
+    images = get_images_table(meta)
+    image_properties = get_image_properties_table(meta)
+
+    type_col = Column('type', String(30))
+    type_col.create(images)
+
+    sel = select([image_properties]).where(image_properties.c.key == 'type')
+    type_property_records = conn.execute(sel).fetchall()
+    for record in type_property_records:
+        upd = images.update().where(
+            images.c.id == record.image_id).values(type=record.value)
+        conn.execute(upd)
+        dlt = image_properties.delete().where(
+            image_properties.c.image_id == record.image_id)
+        conn.execute(dlt)
+    conn.close()
+
+    images.columns['disk_format'].drop()
+    images.columns['container_format'].drop()
 
=== added file 'glance/registry/db/migrate_repo/versions/003_sqlite_downgrade.sql'
--- glance/registry/db/migrate_repo/versions/003_sqlite_downgrade.sql	1970-01-01 00:00:00 +0000
+++ glance/registry/db/migrate_repo/versions/003_sqlite_downgrade.sql	2011-03-23 15:06:49 +0000
@@ -0,0 +1,58 @@
+BEGIN;
+
+/* Make changes to the base images table */
+CREATE TEMPORARY TABLE images_backup (
+	id INTEGER NOT NULL, 
+	name VARCHAR(255), 
+	size INTEGER, 
+	status VARCHAR(30) NOT NULL, 
+	is_public BOOLEAN NOT NULL, 
+	location TEXT, 
+	created_at DATETIME NOT NULL, 
+	updated_at DATETIME, 
+	deleted_at DATETIME, 
+	deleted BOOLEAN NOT NULL, 
+	PRIMARY KEY (id)
+);
+
+INSERT INTO images_backup
+SELECT id, name, size, status, is_public, location, created_at, updated_at, deleted_at, deleted
+FROM images;
+
+DROP TABLE images;
+
+CREATE TABLE images (
+	id INTEGER NOT NULL, 
+	name VARCHAR(255), 
+	size INTEGER, 
+	type VARCHAR(30),
+	status VARCHAR(30) NOT NULL, 
+	is_public BOOLEAN NOT NULL, 
+	location TEXT, 
+	created_at DATETIME NOT NULL, 
+	updated_at DATETIME, 
+	deleted_at DATETIME, 
+	deleted BOOLEAN NOT NULL, 
+	PRIMARY KEY (id), 
+	CHECK (is_public IN (0, 1)), 
+	CHECK (deleted IN (0, 1))
+);
+CREATE INDEX ix_images_deleted ON images (deleted);
+CREATE INDEX ix_images_is_public ON images (is_public);
+
+INSERT INTO images (id, name, size, status, is_public, location, created_at, updated_at, deleted_at, deleted)
+SELECT id, name, size, status, is_public, location, created_at, updated_at, deleted_at, deleted
+FROM images_backup;
+
+DROP TABLE images_backup;
+
+/* Re-insert the type values from the temp table */
+UPDATE images
+SET type = (SELECT value FROM image_properties WHERE image_id = images.id AND key = 'type')
+WHERE EXISTS (SELECT * FROM image_properties WHERE image_id = images.id AND key = 'type');
+
+/* Remove the type properties from the image_properties table */
+DELETE FROM image_properties
+WHERE key = 'type';
+
+COMMIT;
 
=== added file 'glance/registry/db/migrate_repo/versions/003_sqlite_upgrade.sql'
--- glance/registry/db/migrate_repo/versions/003_sqlite_upgrade.sql	1970-01-01 00:00:00 +0000
+++ glance/registry/db/migrate_repo/versions/003_sqlite_upgrade.sql	2011-03-23 15:06:49 +0000
@@ -0,0 +1,64 @@
+BEGIN TRANSACTION;
+
+/* Move type column from base images table
+ * to be records in image_properties table */
+CREATE TEMPORARY TABLE tmp_type_records (id INTEGER NOT NULL, type VARCHAR(30) NOT NULL);
+INSERT INTO tmp_type_records
+SELECT id, type
+FROM images
+WHERE type IS NOT NULL;
+
+REPLACE INTO image_properties
+(image_id, key, value, created_at, deleted)
+SELECT id, 'type', type, date('now'), 0
+FROM tmp_type_records;
+
+DROP TABLE tmp_type_records;
+
+/* Make changes to the base images table */
+CREATE TEMPORARY TABLE images_backup (
+	id INTEGER NOT NULL, 
+	name VARCHAR(255), 
+	size INTEGER, 
+	status VARCHAR(30) NOT NULL, 
+	is_public BOOLEAN NOT NULL, 
+	location TEXT, 
+	created_at DATETIME NOT NULL, 
+	updated_at DATETIME, 
+	deleted_at DATETIME, 
+	deleted BOOLEAN NOT NULL, 
+	PRIMARY KEY (id)
+);
+
+INSERT INTO images_backup
+SELECT id, name, size, status, is_public, location, created_at, updated_at, deleted_at, deleted
+FROM images;
+
+DROP TABLE images;
+
+CREATE TABLE images (
+	id INTEGER NOT NULL, 
+	name VARCHAR(255), 
+	size INTEGER, 
+	status VARCHAR(30) NOT NULL, 
+	is_public BOOLEAN NOT NULL, 
+	location TEXT, 
+	created_at DATETIME NOT NULL, 
+	updated_at DATETIME, 
+	deleted_at DATETIME, 
+	deleted BOOLEAN NOT NULL, 
+	disk_format VARCHAR(20),
+	container_format VARCHAR(20),
+	PRIMARY KEY (id), 
+	CHECK (is_public IN (0, 1)), 
+	CHECK (deleted IN (0, 1))
+);
+CREATE INDEX ix_images_deleted ON images (deleted);
+CREATE INDEX ix_images_is_public ON images (is_public);
+
+INSERT INTO images (id, name, size, status, is_public, location, created_at, updated_at, deleted_at, deleted)
+SELECT id, name, size, status, is_public, location, created_at, updated_at, deleted_at, deleted
+FROM images_backup;
+
+DROP TABLE images_backup;
+COMMIT;
 
=== added file 'glance/registry/db/migrate_repo/versions/004_add_checksum.py'
--- glance/registry/db/migrate_repo/versions/004_add_checksum.py	1970-01-01 00:00:00 +0000
+++ glance/registry/db/migrate_repo/versions/004_add_checksum.py	2011-03-23 15:06:49 +0000
@@ -0,0 +1,80 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2011 OpenStack LLC.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from migrate.changeset import *
+from sqlalchemy import *
+from sqlalchemy.sql import and_, not_
+
+from glance.registry.db.migrate_repo.schema import (
+    Boolean, DateTime, Integer, String, Text, from_migration_import)
+
+
+def get_images_table(meta):
+    """
+    Returns the Table object for the images table that
+    corresponds to the images table definition of this version.
+    """
+    images = Table('images', meta,
+        Column('id', Integer(), primary_key=True, nullable=False),
+        Column('name', String(255)),
+        Column('disk_format', String(20)),
+        Column('container_format', String(20)),
+        Column('size', Integer()),
+        Column('status', String(30), nullable=False),
+        Column('is_public', Boolean(), nullable=False, default=False,
+               index=True),
+        Column('location', Text()),
+        Column('created_at', DateTime(), nullable=False),
+        Column('updated_at', DateTime()),
+        Column('deleted_at', DateTime()),
+        Column('deleted', Boolean(), nullable=False, default=False,
+               index=True),
+        Column('checksum', String(32)),
+        mysql_engine='InnoDB',
+        useexisting=True)
+
+    return images
+
+
+def get_image_properties_table(meta):
+    """
+    No changes to the image properties table from 002...
+    """
+    (define_image_properties_table,) = from_migration_import(
+        '002_add_image_properties_table', ['define_image_properties_table'])
+
+    image_properties = define_image_properties_table(meta)
+    return image_properties
+
+
+def upgrade(migrate_engine):
+    meta = MetaData()
+    meta.bind = migrate_engine
+
+    images = get_images_table(meta)
+
+    checksum = Column('checksum', String(32))
+    checksum.create(images)
+
+
+def downgrade(migrate_engine):
+    meta = MetaData()
+    meta.bind = migrate_engine
+
+    images = get_images_table(meta)
+
+    images.columns['checksum'].drop()
 
=== modified file 'glance/registry/db/migration.py'
--- glance/registry/db/migration.py	2011-03-08 19:51:25 +0000
+++ glance/registry/db/migration.py	2011-03-23 15:06:49 +0000
@@ -38,7 +38,7 @@
     :param options: options dict
     :retval version number
     """
-    repo_path = _find_migrate_repo()
+    repo_path = get_migrate_repo_path()
     sql_connection = options['sql_connection']
     try:
         return versioning_api.db_version(sql_connection, repo_path)
@@ -56,7 +56,7 @@
     :retval version number
     """
     db_version(options)  # Ensure db is under migration control
-    repo_path = _find_migrate_repo()
+    repo_path = get_migrate_repo_path()
     sql_connection = options['sql_connection']
     version_str = version or 'latest'
     logger.info("Upgrading %(sql_connection)s to version %(version_str)s" %
@@ -72,7 +72,7 @@
     :retval version number
     """
     db_version(options)  # Ensure db is under migration control
-    repo_path = _find_migrate_repo()
+    repo_path = get_migrate_repo_path()
     sql_connection = options['sql_connection']
     logger.info("Downgrading %(sql_connection)s to version %(version)s" %
                 locals())
@@ -98,7 +98,7 @@
 
     :param options: options dict
     """
-    repo_path = _find_migrate_repo()
+    repo_path = get_migrate_repo_path()
     sql_connection = options['sql_connection']
     return versioning_api.version_control(sql_connection, repo_path)
 
@@ -117,7 +117,7 @@
     upgrade(options, version=version)
 
 
-def _find_migrate_repo():
+def get_migrate_repo_path():
     """Get the path for the migrate repository."""
     path = os.path.join(os.path.abspath(os.path.dirname(__file__)),
                         'migrate_repo')
 
=== modified file 'glance/registry/db/models.py'
--- glance/registry/db/models.py	2011-03-05 17:04:43 +0000
+++ glance/registry/db/models.py	2011-03-23 15:06:49 +0000
@@ -104,6 +104,7 @@
     status = Column(String(30), nullable=False)
     is_public = Column(Boolean, nullable=False, default=False)
     location = Column(Text)
+    checksum = Column(String(32))
 
 
 class ImageProperty(BASE, ModelBase):
 
=== modified file 'glance/registry/server.py'
--- glance/registry/server.py	2011-02-25 16:52:12 +0000
+++ glance/registry/server.py	2011-03-23 15:06:49 +0000
@@ -14,8 +14,9 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
+
 """
-Parllax Image controller
+Reference implementation registry server WSGI controller
 """
 
 import json
@@ -31,6 +32,10 @@
 
 logger = logging.getLogger('glance.registry.server')
 
+DISPLAY_FIELDS_IN_INDEX = ['id', 'name', 'size',
+                           'disk_format', 'container_format',
+                           'checksum']
+
 
 class Controller(wsgi.Controller):
     """Controller for the reference implementation registry server"""
@@ -49,16 +54,24 @@
 
         Where image_list is a sequence of mappings::
 
-            {'id': image_id, 'name': image_name}
+            {
+            'id': <ID>,
+            'name': <NAME>,
+            'size': <SIZE>,
+            'disk_format': <DISK_FORMAT>,
+            'container_format': <CONTAINER_FORMAT>,
+            'checksum': <CHECKSUM>
+            }
 
         """
         images = db_api.image_get_all_public(None)
-        image_dicts = [dict(id=i['id'],
-                            name=i['name'],
-                            disk_format=i['disk_format'],
-                            container_format=i['container_format'],
-                            size=i['size']) for i in images]
-        return dict(images=image_dicts)
+        results = []
+        for image in images:
+            result = {}
+            for field in DISPLAY_FIELDS_IN_INDEX:
+                result[field] = image[field]
+            results.append(result)
+        return dict(images=results)
 
     def detail(self, req):
         """Return detailed information for all public, non-deleted images
 
=== modified file 'glance/server.py'
--- glance/server.py	2011-03-09 00:07:44 +0000
+++ glance/server.py	2011-03-23 15:06:49 +0000
@@ -29,6 +29,7 @@
 
 """
 
+import httplib
 import json
 import logging
 import sys
@@ -68,8 +69,8 @@
         GET /images/<ID> -- Return image data for image with id <ID>
         POST /images -- Store image data and return metadata about the
                         newly-stored image
-        PUT /images/<ID> -- Update image metadata (not image data, since
-                            image data is immutable once stored)
+        PUT /images/<ID> -- Update image metadata and/or upload image
+                            data for a previously-reserved image
         DELETE /images/<ID> -- Delete the image with id <ID>
     """
 
@@ -82,6 +83,9 @@
 
             * id -- The opaque image identifier
             * name -- The name of the image
+            * disk_format -- The disk image format
+            * container_format -- The "container" format of the image
+            * checksum -- MD5 checksum of the image data
             * size -- Size of image data in bytes
 
         :param request: The WSGI/Webob Request object
@@ -92,6 +96,7 @@
                  'name': <NAME>,
                  'disk_format': <DISK_FORMAT>,
                  'container_format': <DISK_FORMAT>,
+                 'checksum': <CHECKSUM>
                  'size': <SIZE>}, ...
             ]}
         """
@@ -111,6 +116,7 @@
                  'size': <SIZE>,
                  'disk_format': <DISK_FORMAT>,
                  'container_format': <CONTAINER_FORMAT>,
+                 'checksum': <CHECKSUM>,
                  'store': <STORE>,
                  'status': <STATUS>,
                  'created_at': <TIMESTAMP>,
@@ -136,6 +142,8 @@
 
         res = Response(request=req)
         utils.inject_image_meta_into_headers(res, image)
+        res.headers.add('Location', "/images/%s" % id)
+        res.headers.add('ETag', image['checksum'])
 
         return req.get_response(res)
 
@@ -165,6 +173,8 @@
         res = Response(app_iter=image_iterator(),
                        content_type="text/plain")
         utils.inject_image_meta_into_headers(res, image)
+        res.headers.add('Location', "/images/%s" % id)
+        res.headers.add('ETag', image['checksum'])
         return req.get_response(res)
 
     def _reserve(self, req):
@@ -225,36 +235,45 @@
 
         store = self.get_store_or_400(req, store_name)
 
-        image_meta['status'] = 'saving'
         image_id = image_meta['id']
-        logger.debug("Updating image metadata for image %s"
+        logger.debug("Setting image %s to status 'saving'"
                      % image_id)
-        registry.update_image_metadata(self.options,
-                                       image_meta['id'],
-                                       image_meta)
-
+        registry.update_image_metadata(self.options, image_id,
+                                       {'status': 'saving'})
         try:
             logger.debug("Uploading image data for image %(image_id)s "
                          "to %(store_name)s store" % locals())
-            location, size = store.add(image_meta['id'],
-                                       req.body_file,
-                                       self.options)
-            # If size returned from store is different from size
-            # already stored in registry, update the registry with
-            # the new size of the image
-            if image_meta.get('size', 0) != size:
-                image_meta['size'] = size
-                logger.debug("Updating image metadata for image %s"
-                             % image_id)
-                registry.update_image_metadata(self.options,
-                                               image_meta['id'],
-                                               image_meta)
+            location, size, checksum = store.add(image_meta['id'],
+                                                 req.body_file,
+                                                 self.options)
+
+            # Verify any supplied checksum value matches checksum
+            # returned from store when adding image
+            supplied_checksum = image_meta.get('checksum')
+            if supplied_checksum and supplied_checksum != checksum:
+                msg = ("Supplied checksum (%(supplied_checksum)s) and "
+                       "checksum generated from uploaded image "
+                       "(%(checksum)s) did not match. Setting image "
+                       "status to 'killed'.") % locals()
+                self._safe_kill(req, image_meta)
+                raise HTTPBadRequest(msg, content_type="text/plain",
+                                     request=req)
+
+            # Update the database with the checksum returned
+            # from the backend store
+            logger.debug("Updating image %(image_id)s data. "
+                         "Checksum set to %(checksum)s, size set "
+                         "to %(size)d" % locals())
+            registry.update_image_metadata(self.options, image_id,
+                                           {'checksum': checksum,
+                                            'size': size})
+
             return location
         except exception.Duplicate, e:
             logger.error("Error adding image to store: %s", str(e))
             raise HTTPConflict(str(e), request=req)
 
-    def _activate(self, req, image_meta, location):
+    def _activate(self, req, image_id, location):
         """
         Sets the image status to `active` and the image's location
         attribute.
@@ -263,25 +282,25 @@
         :param image_meta: Mapping of metadata about image
         :param location: Location of where Glance stored this image
         """
+        image_meta = {}
         image_meta['location'] = location
         image_meta['status'] = 'active'
-        registry.update_image_metadata(self.options,
-                                       image_meta['id'],
+        return registry.update_image_metadata(self.options,
+                                       image_id,
                                        image_meta)
 
-    def _kill(self, req, image_meta):
+    def _kill(self, req, image_id):
         """
         Marks the image status to `killed`
 
         :param request: The WSGI/Webob Request object
-        :param image_meta: Mapping of metadata about image
+        :param image_id: Opaque image identifier
         """
-        image_meta['status'] = 'killed'
         registry.update_image_metadata(self.options,
-                                       image_meta['id'],
-                                       image_meta)
+                                       image_id,
+                                       {'status': 'killed'})
 
-    def _safe_kill(self, req, image_meta):
+    def _safe_kill(self, req, image_id):
         """
         Mark image killed without raising exceptions if it fails.
 
@@ -289,12 +308,13 @@
         not raise itself, rather it should just log its error.
 
         :param request: The WSGI/Webob Request object
+        :param image_id: Opaque image identifier
         """
         try:
-            self._kill(req, image_meta)
+            self._kill(req, image_id)
         except Exception, e:
             logger.error("Unable to kill image %s: %s",
-                          image_meta['id'], repr(e))
+                          image_id, repr(e))
 
     def _upload_and_activate(self, req, image_meta):
         """
@@ -304,13 +324,16 @@
 
         :param request: The WSGI/Webob Request object
         :param image_meta: Mapping of metadata about image
+
+        :retval Mapping of updated image data
         """
         try:
+            image_id = image_meta['id']
             location = self._upload(req, image_meta)
-            self._activate(req, image_meta, location)
+            return self._activate(req, image_id, location)
         except:  # unqualified b/c we're re-raising it
             exc_type, exc_value, exc_traceback = sys.exc_info()
-            self._safe_kill(req, image_meta)
+            self._safe_kill(req, image_id)
             # NOTE(sirp): _safe_kill uses httplib which, in turn, uses
             # Eventlet's GreenSocket. Eventlet subsequently clears exceptions
             # by calling `sys.exc_clear()`.
@@ -318,8 +341,8 @@
             # This is why we can't use a raise with no arguments here: our
             # exception context was destroyed by Eventlet. To work around
             # this, we need to 'memorize' the exception context, and then
-            # re-raise using 3-arg form after Eventlet has run
-            raise exc_type, exc_value, exc_traceback
+            # re-raise here.
+            raise exc_type(exc_traceback)
 
     def create(self, req):
         """
@@ -354,19 +377,21 @@
                 image data.
         """
         image_meta = self._reserve(req)
+        image_id = image_meta['id']
 
         if utils.has_body(req):
-            self._upload_and_activate(req, image_meta)
+            image_meta = self._upload_and_activate(req, image_meta)
         else:
             if 'x-image-meta-location' in req.headers:
                 location = req.headers['x-image-meta-location']
-                self._activate(req, image_meta, location)
+                image_meta = self._activate(req, image_id, location)
 
         # APP states we should return a Location: header with the edit
         # URI of the resource newly-created.
         res = Response(request=req, body=json.dumps(dict(image=image_meta)),
-                       content_type="text/plain")
-        res.headers.add('Location', "/images/%s" % image_meta['id'])
+                       status=httplib.CREATED, content_type="text/plain")
+        res.headers.add('Location', "/images/%s" % image_id)
+        res.headers.add('ETag', image_meta['checksum'])
 
         return req.get_response(res)
 
@@ -393,9 +418,14 @@
                                                         id,
                                                         new_image_meta)
             if has_body:
-                self._upload_and_activate(req, image_meta)
+                image_meta = self._upload_and_activate(req, image_meta)
 
-            return dict(image=image_meta)
+            res = Response(request=req,
+                           body=json.dumps(dict(image=image_meta)),
+                           content_type="text/plain")
+            res.headers.add('Location', "/images/%s" % id)
+            res.headers.add('ETag', image_meta['checksum'])
+            return res
         except exception.Invalid, e:
             msg = ("Failed to update image metadata. Got error: %(e)s"
                    % locals())
 
=== modified file 'glance/store/__init__.py'
--- glance/store/__init__.py	2011-02-24 02:50:24 +0000
+++ glance/store/__init__.py	2011-03-23 15:06:49 +0000
@@ -141,40 +141,3 @@
     authurl = "https://%s" % '/'.join(path_parts)
 
     return user, key, authurl, container, obj
-
-
-def add_options(parser):
-    """
-    Adds any configuration options that each store might
-    have.
-
-    :param parser: An optparse.OptionParser object
-    :retval None
-    """
-    # TODO(jaypipes): Remove these imports...
-    from glance.store.http import HTTPBackend
-    from glance.store.s3 import S3Backend
-    from glance.store.swift import SwiftBackend
-    from glance.store.filesystem import FilesystemBackend
-
-    help_text = "The following configuration options are specific to the "\
-                "Glance image store."
-
-    DEFAULT_STORE_CHOICES = ['file', 'swift', 's3']
-    group = optparse.OptionGroup(parser, "Image Store Options", help_text)
-    group.add_option('--default-store', metavar="STORE",
-                     default="file",
-                     choices=DEFAULT_STORE_CHOICES,
-                     help="The backend store that Glance will use to store "
-                     "virtual machine images to. Choices: ('%s') "
-                     "Default: %%default" % "','".join(DEFAULT_STORE_CHOICES))
-
-    backend_classes = [FilesystemBackend,
-                       HTTPBackend,
-                       SwiftBackend,
-                       S3Backend]
-    for backend_class in backend_classes:
-        if hasattr(backend_class, 'add_options'):
-            backend_class.add_options(group)
-
-    parser.add_option_group(group)
 
=== modified file 'glance/store/filesystem.py'
--- glance/store/filesystem.py	2011-02-27 20:54:29 +0000
+++ glance/store/filesystem.py	2011-03-23 15:06:49 +0000
@@ -19,6 +19,7 @@
 A simple filesystem-backed store
 """
 
+import hashlib
 import logging
 import os
 import urlparse
@@ -110,9 +111,10 @@
         :param data: The image data to write, as a file-like object
         :param options: Conf mapping
 
-        :retval Tuple with (location, size)
-                The location that was written, with file:// scheme prepended
-                and the size in bytes of the data written
+        :retval Tuple with (location, size, checksum)
+                The location that was written, with file:// scheme prepended,
+                the size in bytes of the data written, and the checksum of
+                the image added.
         """
         datadir = options['filesystem_store_datadir']
 
@@ -127,6 +129,7 @@
             raise exception.Duplicate("Image file %s already exists!"
                                       % filepath)
 
+        checksum = hashlib.md5()
         bytes_written = 0
         with open(filepath, 'wb') as f:
             while True:
@@ -134,23 +137,11 @@
                 if not buf:
                     break
                 bytes_written += len(buf)
+                checksum.update(buf)
                 f.write(buf)
 
-        logger.debug("Wrote %(bytes_written)d bytes to %(filepath)s"
-                     % locals())
-        return ('file://%s' % filepath, bytes_written)
-
-    @classmethod
-    def add_options(cls, parser):
-        """
-        Adds specific configuration options for this store
-
-        :param parser: An optparse.OptionParser object
-        :retval None
-        """
-
-        parser.add_option('--filesystem-store-datadir', metavar="DIR",
-                          default="/var/lib/glance/images/",
-                          help="Location to write image data. This directory "
-                          "should be writeable by the user that runs the "
-                          "glance-api program. Default: %default")
+        checksum_hex = checksum.hexdigest()
+
+        logger.debug("Wrote %(bytes_written)d bytes to %(filepath)s with "
+                     "checksum %(checksum_hex)s" % locals())
+        return ('file://%s' % filepath, bytes_written, checksum_hex)
 
=== modified file 'glance/store/swift.py'
--- glance/store/swift.py	2011-03-16 16:45:01 +0000
+++ glance/store/swift.py	2011-03-23 15:06:49 +0000
@@ -161,7 +161,7 @@
             # header keys are lowercased by Swift
             if 'content-length' in resp_headers:
                 size = int(resp_headers['content-length'])
-            return (location, size)
+            return (location, size, obj_etag)
         except swift_client.ClientException, e:
             if e.http_status == httplib.CONFLICT:
                 raise exception.Duplicate("Swift already has an image at "
 
=== modified file 'tests/stubs.py'
--- tests/stubs.py	2011-03-14 19:10:24 +0000
+++ tests/stubs.py	2011-03-23 15:06:49 +0000
@@ -282,6 +282,7 @@
                 'updated_at': datetime.datetime.utcnow(),
                 'deleted_at': None,
                 'deleted': False,
+                'checksum': None,
                 'size': 13,
                 'location': "swift://user:passwd@acct/container/obj.tar.0",
              'properties': [{'key': 'type',
@@ -297,6 +298,7 @@
                 'updated_at': datetime.datetime.utcnow(),
                 'deleted_at': None,
                 'deleted': False,
+                'checksum': None,
                 'size': 19,
                 'location': "file:///tmp/glance-tests/2",
                 'properties': []}]
@@ -316,6 +318,7 @@
             glance.registry.db.api.validate_image(values)
 
             values['size'] = values.get('size', 0)
+            values['checksum'] = values.get('checksum')
             values['deleted'] = False
             values['properties'] = values.get('properties', {})
             values['created_at'] = datetime.datetime.utcnow()
@@ -348,6 +351,7 @@
             copy_image.update(values)
             glance.registry.db.api.validate_image(copy_image)
             props = []
+            orig_properties = image['properties']
 
             if 'properties' in values.keys():
                 for k, v in values['properties'].items():
@@ -360,7 +364,8 @@
                     p['deleted_at'] = None
                     props.append(p)
 
-            values['properties'] = props
+            orig_properties = orig_properties + props
+            values['properties'] = orig_properties
 
             image.update(values)
             return image
 
=== modified file 'tests/unit/test_api.py'
--- tests/unit/test_api.py	2011-03-14 19:10:24 +0000
+++ tests/unit/test_api.py	2011-03-23 15:06:49 +0000
@@ -15,8 +15,9 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import hashlib
+import httplib
 import os
-import httplib
 import json
 import unittest
 
@@ -52,7 +53,9 @@
 
         """
         fixture = {'id': 2,
-                   'name': 'fake image #2'}
+                   'name': 'fake image #2',
+                   'size': 19,
+                   'checksum': None}
         req = webob.Request.blank('/')
         res = req.get_response(self.api)
         res_dict = json.loads(res.body)
@@ -70,7 +73,9 @@
 
         """
         fixture = {'id': 2,
-                   'name': 'fake image #2'}
+                   'name': 'fake image #2',
+                   'size': 19,
+                   'checksum': None}
         req = webob.Request.blank('/images')
         res = req.get_response(self.api)
         res_dict = json.loads(res.body)
@@ -90,6 +95,8 @@
         fixture = {'id': 2,
                    'name': 'fake image #2',
                    'is_public': True,
+                   'size': 19,
+                   'checksum': None,
                    'disk_format': 'vhd',
                    'container_format': 'ovf',
                    'status': 'active'}
@@ -396,7 +403,7 @@
         for k, v in fixture_headers.iteritems():
             req.headers[k] = v
         res = req.get_response(self.api)
-        self.assertEquals(res.status_int, httplib.OK)
+        self.assertEquals(res.status_int, httplib.CREATED)
 
         res_body = json.loads(res.body)['image']
         self.assertEquals('queued', res_body['status'])
@@ -431,7 +438,7 @@
         req.headers['Content-Type'] = 'application/octet-stream'
         req.body = "chunk00000remainder"
         res = req.get_response(self.api)
-        self.assertEquals(res.status_int, 200)
+        self.assertEquals(res.status_int, httplib.CREATED)
 
         res_body = json.loads(res.body)['image']
         self.assertEquals(res_body['location'],
@@ -445,6 +452,97 @@
                         "res.headerlist = %r" % res.headerlist)
         self.assertTrue('/images/3' in res.headers['location'])
 
+    def test_image_is_checksummed(self):
+        """Test that the image contents are checksummed properly"""
+        fixture_headers = {'x-image-meta-store': 'file',
+                           'x-image-meta-disk-format': 'vhd',
+                           'x-image-meta-container-format': 'ovf',
+                           'x-image-meta-name': 'fake image #3'}
+        image_contents = "chunk00000remainder"
+        image_checksum = hashlib.md5(image_contents).hexdigest()
+
+        req = webob.Request.blank("/images")
+        req.method = 'POST'
+        for k, v in fixture_headers.iteritems():
+            req.headers[k] = v
+
+        req.headers['Content-Type'] = 'application/octet-stream'
+        req.body = image_contents
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, httplib.CREATED)
+
+        res_body = json.loads(res.body)['image']
+        self.assertEquals(res_body['location'],
+                          'file:///tmp/glance-tests/3')
+        self.assertEquals(image_checksum, res_body['checksum'],
+                          "Mismatched checksum. Expected %s, got %s" %
+                          (image_checksum, res_body['checksum']))
+
+    def test_etag_equals_checksum_header(self):
+        """Test that the ETag header matches the x-image-meta-checksum"""
+        fixture_headers = {'x-image-meta-store': 'file',
+                           'x-image-meta-disk-format': 'vhd',
+                           'x-image-meta-container-format': 'ovf',
+                           'x-image-meta-name': 'fake image #3'}
+        image_contents = "chunk00000remainder"
+        image_checksum = hashlib.md5(image_contents).hexdigest()
+
+        req = webob.Request.blank("/images")
+        req.method = 'POST'
+        for k, v in fixture_headers.iteritems():
+            req.headers[k] = v
+
+        req.headers['Content-Type'] = 'application/octet-stream'
+        req.body = image_contents
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, httplib.CREATED)
+
+        # HEAD the image and check the ETag equals the checksum header...
+        expected_headers = {'x-image-meta-checksum': image_checksum,
+                            'etag': image_checksum}
+        req = webob.Request.blank("/images/3")
+        req.method = 'HEAD'
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, 200)
+
+        for key in expected_headers.keys():
+            self.assertTrue(key in res.headers,
+                            "required header '%s' missing from "
+                            "returned headers" % key)
+        for key, value in expected_headers.iteritems():
+            self.assertEquals(value, res.headers[key])
+
+    def test_bad_checksum_kills_image(self):
+        """Test that the image contents are checksummed properly"""
+        image_contents = "chunk00000remainder"
+        bad_checksum = hashlib.md5("invalid").hexdigest()
+        fixture_headers = {'x-image-meta-store': 'file',
+                           'x-image-meta-disk-format': 'vhd',
+                           'x-image-meta-container-format': 'ovf',
+                           'x-image-meta-name': 'fake image #3',
+                           'x-image-meta-checksum': bad_checksum}
+
+        req = webob.Request.blank("/images")
+        req.method = 'POST'
+        for k, v in fixture_headers.iteritems():
+            req.headers[k] = v
+
+        req.headers['Content-Type'] = 'application/octet-stream'
+        req.body = image_contents
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, webob.exc.HTTPBadRequest.code)
+
+        # Test the image was killed...
+        expected_headers = {'x-image-meta-id': '3',
+                            'x-image-meta-status': 'killed'}
+        req = webob.Request.blank("/images/3")
+        req.method = 'HEAD'
+        res = req.get_response(self.api)
+        self.assertEquals(res.status_int, 200)
+
+        for key, value in expected_headers.iteritems():
+            self.assertEquals(value, res.headers[key])
+
     def test_image_meta(self):
         """Test for HEAD /images/<ID>"""
         expected_headers = {'x-image-meta-id': '2',
 
=== modified file 'tests/unit/test_filesystem_store.py'
--- tests/unit/test_filesystem_store.py	2011-02-27 20:54:29 +0000
+++ tests/unit/test_filesystem_store.py	2011-03-23 15:06:49 +0000
@@ -18,6 +18,7 @@
 """Tests the filesystem backend store"""
 
 import StringIO
+import hashlib
 import unittest
 import urlparse
 
@@ -27,6 +28,11 @@
 from glance.store.filesystem import FilesystemBackend, ChunkedFile
 from tests import stubs
 
+FILESYSTEM_OPTIONS = {
+    'verbose': True,
+    'debug': True,
+    'filesystem_store_datadir': stubs.FAKE_FILESYSTEM_ROOTDIR}
+
 
 class TestFilesystemBackend(unittest.TestCase):
 
@@ -75,17 +81,17 @@
         expected_image_id = 42
         expected_file_size = 1024 * 5  # 5K
         expected_file_contents = "*" * expected_file_size
+        expected_checksum = hashlib.md5(expected_file_contents).hexdigest()
         expected_location = "file://%s/%s" % (stubs.FAKE_FILESYSTEM_ROOTDIR,
                                               expected_image_id)
         image_file = StringIO.StringIO(expected_file_contents)
-        options = {'verbose': True,
-                   'debug': True,
-                   'filesystem_store_datadir': stubs.FAKE_FILESYSTEM_ROOTDIR}
 
-        location, size = FilesystemBackend.add(42, image_file, options)
+        location, size, checksum = FilesystemBackend.add(42, image_file,
+                                                         FILESYSTEM_OPTIONS)
 
         self.assertEquals(expected_location, location)
         self.assertEquals(expected_file_size, size)
+        self.assertEquals(expected_checksum, checksum)
 
         url_pieces = urlparse.urlparse("file:///tmp/glance-tests/42")
         new_image_file = FilesystemBackend.get(url_pieces)
@@ -110,7 +116,7 @@
                    'filesystem_store_datadir': stubs.FAKE_FILESYSTEM_ROOTDIR}
         self.assertRaises(exception.Duplicate,
                           FilesystemBackend.add,
-                          2, image_file, options)
+                          2, image_file, FILESYSTEM_OPTIONS)
 
     def test_delete(self):
         """
 
=== added file 'tests/unit/test_migrations.conf'
--- tests/unit/test_migrations.conf	1970-01-01 00:00:00 +0000
+++ tests/unit/test_migrations.conf	2011-03-23 15:06:49 +0000
@@ -0,0 +1,5 @@
+[DEFAULT]
+# Set up any number of migration data stores you want, one
+# The "name" used in the test is the config variable key.
+sqlite=sqlite:///test_migrations.db
+mysql=mysql://root:@localhost/test_migrations
 
=== modified file 'tests/unit/test_migrations.py'
--- tests/unit/test_migrations.py	2011-03-16 16:11:56 +0000
+++ tests/unit/test_migrations.py	2011-03-23 15:06:49 +0000
@@ -15,39 +15,241 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+"""
+Tests for database migrations. This test case reads the configuration
+file /tests/unit/test_migrations.conf for database connection settings
+to use in the tests. For each connection found in the config file,
+the test case runs a series of test cases to ensure that migrations work
+properly both upgrading and downgrading, and that no data loss occurs
+if possible.
+"""
+
+import ConfigParser
+import datetime
 import os
 import unittest
-
+import urlparse
+
+from migrate.versioning.repository import Repository
+from sqlalchemy import *
+from sqlalchemy.pool import NullPool
+
+from glance.common import exception
 import glance.registry.db.migration as migration_api
-import glance.registry.db.api as api
-import glance.common.config as config
+from tests.unit.test_misc import execute
 
 
 class TestMigrations(unittest.TestCase):
+
     """Test sqlalchemy-migrate migrations"""
 
+    TEST_DATABASES = {}
+    CONFIG_FILE_PATH = os.path.join('tests', 'unit',
+                                    'test_migrations.conf')
+    REPOSITORY_PATH = os.path.join('glance', 'registry', 'db', 'migrate_repo')
+    REPOSITORY = Repository(REPOSITORY_PATH)
+
+    def __init__(self, *args, **kwargs):
+        super(TestMigrations, self).__init__(*args, **kwargs)
+
     def setUp(self):
-        self.db_path = "glance_test_migration.sqlite"
-        sql_connection = os.environ.get('GLANCE_SQL_CONNECTION',
-                                        "sqlite:///%s" % self.db_path)
-
-        self.options = dict(sql_connection=sql_connection,
-                            verbose=False)
-        config.setup_logging(self.options, {})
+        # Load test databases from the config file. Only do this
+        # once. No need to re-run this on each test...
+        if not TestMigrations.TEST_DATABASES:
+            if os.path.exists(TestMigrations.CONFIG_FILE_PATH):
+                cp = ConfigParser.RawConfigParser()
+                try:
+                    cp.read(TestMigrations.CONFIG_FILE_PATH)
+                    defaults = cp.defaults()
+                    for key, value in defaults.items():
+                        TestMigrations.TEST_DATABASES[key] = value
+                except ConfigParser.ParsingError, e:
+                    print ("Failed to read test_migrations.conf config file. "
+                           "Got error: %s" % e)
+
+        self.engines = {}
+        for key, value in TestMigrations.TEST_DATABASES.items():
+            self.engines[key] = create_engine(value, poolclass=NullPool)
+
+        # We start each test case with a completely blank slate.
+        self._reset_databases()
 
     def tearDown(self):
-        api.configure_db(self.options)
-        api.unregister_models()
-
-    def test_db_sync_downgrade_then_upgrade(self):
-        migration_api.db_sync(self.options)
-
-        latest = migration_api.db_version(self.options)
-
-        migration_api.downgrade(self.options, latest - 1)
-        cur_version = migration_api.db_version(self.options)
-        self.assertEqual(cur_version, latest - 1)
-
-        migration_api.upgrade(self.options, cur_version + 1)
-        cur_version = migration_api.db_version(self.options)
-        self.assertEqual(cur_version, latest)
+        # We destroy the test data store between each test case,
+        # and recreate it, which ensures that we have no side-effects
+        # from the tests
+        self._reset_databases()
+
+    def _reset_databases(self):
+        for key, engine in self.engines.items():
+            conn_string = TestMigrations.TEST_DATABASES[key]
+            conn_pieces = urlparse.urlparse(conn_string)
+            if conn_string.startswith('sqlite'):
+                # We can just delete the SQLite database, which is
+                # the easiest and cleanest solution
+                db_path = conn_pieces.path.strip('/')
+                if os.path.exists(db_path):
+                    os.unlink(db_path)
+                # No need to recreate the SQLite DB. SQLite will
+                # create it for us if it's not there...
+            elif conn_string.startswith('mysql'):
+                # We can execute the MySQL client to destroy and re-create
+                # the MYSQL database, which is easier and less error-prone
+                # than using SQLAlchemy to do this via MetaData...trust me.
+                database = conn_pieces.path.strip('/')
+                loc_pieces = conn_pieces.netloc.split('@')
+                host = loc_pieces[1]
+                auth_pieces = loc_pieces[0].split(':')
+                user = auth_pieces[0]
+                password = ""
+                if len(auth_pieces) > 1:
+                    if auth_pieces[1].strip():
+                        password = "-p%s" % auth_pieces[1]
+                sql = ("drop database if exists %(database)s; "
+                       "create database %(database)s;") % locals()
+                cmd = ("mysql -u%(user)s %(password)s -h%(host)s "
+                       "-e\"%(sql)s\"") % locals()
+                exitcode, out, err = execute(cmd)
+                self.assertEqual(0, exitcode)
+
+    def test_walk_versions(self):
+        """
+        Walks all version scripts for each tested database, ensuring
+        that there are no errors in the version scripts for each engine
+        """
+        for key, engine in self.engines.items():
+            options = {'sql_connection': TestMigrations.TEST_DATABASES[key]}
+            self._walk_versions(options)
+
+    def _walk_versions(self, options):
+        # Determine latest version script from the repo, then
+        # upgrade from 1 through to the latest, with no data
+        # in the databases. This just checks that the schema itself
+        # upgrades successfully.
+
+        # Assert we are not under version control...
+        self.assertRaises(exception.DatabaseMigrationError,
+                          migration_api.db_version,
+                          options)
+        # Place the database under version control
+        migration_api.version_control(options)
+
+        cur_version = migration_api.db_version(options)
+        self.assertEqual(0, cur_version)
+
+        for version in xrange(1, TestMigrations.REPOSITORY.latest + 1):
+            migration_api.upgrade(options, version)
+            cur_version = migration_api.db_version(options)
+            self.assertEqual(cur_version, version)
+
+        # Now walk it back down to 0 from the latest, testing
+        # the downgrade paths.
+        for version in reversed(
+            xrange(0, TestMigrations.REPOSITORY.latest)):
+            migration_api.downgrade(options, version)
+            cur_version = migration_api.db_version(options)
+            self.assertEqual(cur_version, version)
+
+    def test_no_data_loss_2_to_3_to_2(self):
+        """
+        Here, we test that in the case when we moved a column "type" from the
+        base images table to be records in the image_properties table, that
+        we don't lose any data during the migration. Similarly, we test that
+        on downgrade, we don't lose any data, as the records are moved from
+        the image_properties table back into the base image table.
+        """
+        for key, engine in self.engines.items():
+            options = {'sql_connection': TestMigrations.TEST_DATABASES[key]}
+            self._no_data_loss_2_to_3_to_2(engine, options)
+
+    def _no_data_loss_2_to_3_to_2(self, engine, options):
+        migration_api.version_control(options)
+        migration_api.upgrade(options, 2)
+
+        cur_version = migration_api.db_version(options)
+        self.assertEquals(2, cur_version)
+
+        # We are now on version 2. Check that the images table does
+        # not contain the type column...
+
+        images_table = Table('images', MetaData(), autoload=True,
+                             autoload_with=engine)
+
+        image_properties_table = Table('image_properties', MetaData(),
+                                       autoload=True,
+                                       autoload_with=engine)
+
+        self.assertTrue('type' in images_table.c,
+                        "'type' column found in images table columns! "
+                        "images table columns: %s"
+                        % images_table.c.keys())
+
+        conn = engine.connect()
+        sel = select([func.count("*")], from_obj=[images_table])
+        orig_num_images = conn.execute(sel).scalar()
+        sel = select([func.count("*")], from_obj=[image_properties_table])
+        orig_num_image_properties = conn.execute(sel).scalar()
+
+        now = datetime.datetime.now()
+        inserter = images_table.insert()
+        conn.execute(inserter, [
+                {'deleted': False, 'created_at': now,
+                 'updated_at': now, 'type': 'kernel',
+                 'status': 'active', 'is_public': True},
+                {'deleted': False, 'created_at': now,
+                 'updated_at': now, 'type': 'ramdisk',
+                 'status': 'active', 'is_public': True}])
+
+        sel = select([func.count("*")], from_obj=[images_table])
+        num_images = conn.execute(sel).scalar()
+        self.assertEqual(orig_num_images + 2, num_images)
+        conn.close()
+
+        # Now let's upgrade to 3. This should move the type column
+        # to the image_properties table as type properties.
+
+        migration_api.upgrade(options, 3)
+
+        cur_version = migration_api.db_version(options)
+        self.assertEquals(3, cur_version)
+
+        images_table = Table('images', MetaData(), autoload=True,
+                             autoload_with=engine)
+
+        self.assertTrue('type' not in images_table.c,
+                        "'type' column not found in images table columns! "
+                        "images table columns reported by metadata: %s\n"
+                        % images_table.c.keys())
+
+        image_properties_table = Table('image_properties', MetaData(),
+                                       autoload=True,
+                                       autoload_with=engine)
+
+        conn = engine.connect()
+        sel = select([func.count("*")], from_obj=[image_properties_table])
+        num_image_properties = conn.execute(sel).scalar()
+        self.assertEqual(orig_num_image_properties + 2, num_image_properties)
+        conn.close()
+
+        # Downgrade to 2 and check that the type properties were moved
+        # to the main image table
+
+        migration_api.downgrade(options, 2)
+
+        images_table = Table('images', MetaData(), autoload=True,
+                             autoload_with=engine)
+
+        self.assertTrue('type' in images_table.c,
+                        "'type' column found in images table columns! "
+                        "images table columns: %s"
+                        % images_table.c.keys())
+
+        image_properties_table = Table('image_properties', MetaData(),
+                                       autoload=True,
+                                       autoload_with=engine)
+
+        conn = engine.connect()
+        sel = select([func.count("*")], from_obj=[image_properties_table])
+        last_num_image_properties = conn.execute(sel).scalar()
+
+        self.assertEqual(num_image_properties - 2, last_num_image_properties)
 
=== modified file 'tests/unit/test_swift_store.py'
--- tests/unit/test_swift_store.py	2011-03-16 16:45:01 +0000
+++ tests/unit/test_swift_store.py	2011-03-23 15:06:49 +0000
@@ -70,15 +70,20 @@
             if hasattr(contents, 'read'):
                 fixture_object = StringIO.StringIO()
                 chunk = contents.read(SwiftBackend.CHUNKSIZE)
+                checksum = hashlib.md5()
                 while chunk:
                     fixture_object.write(chunk)
+                    checksum.update(chunk)
                     chunk = contents.read(SwiftBackend.CHUNKSIZE)
+                etag = checksum.hexdigest()
             else:
                 fixture_object = StringIO.StringIO(contents)
+                etag = hashlib.md5(fixture_object.getvalue()).hexdigest()
+            read_len = fixture_object.len
             fixture_objects[fixture_key] = fixture_object
             fixture_headers[fixture_key] = {
-                'content-length': fixture_object.len,
-                'etag': hashlib.md5(fixture_object.read()).hexdigest()}
+                'content-length': read_len,
+                'etag': etag}
             return fixture_headers[fixture_key]['etag']
         else:
             msg = ("Object PUT failed - Object with key %s already exists"
@@ -226,8 +231,9 @@
     def test_add(self):
         """Test that we can add an image via the swift backend"""
         expected_image_id = 42
-        expected_swift_size = 1024 * 5  # 5K
+        expected_swift_size = FIVE_KB
         expected_swift_contents = "*" * expected_swift_size
+        expected_checksum = hashlib.md5(expected_swift_contents).hexdigest()
         expected_location = format_swift_location(
             SWIFT_OPTIONS['swift_store_user'],
             SWIFT_OPTIONS['swift_store_key'],
@@ -236,10 +242,12 @@
             expected_image_id)
         image_swift = StringIO.StringIO(expected_swift_contents)
 
-        location, size = SwiftBackend.add(42, image_swift, SWIFT_OPTIONS)
+        location, size, checksum = SwiftBackend.add(42, image_swift,
+                                                    SWIFT_OPTIONS)
 
         self.assertEquals(expected_location, location)
         self.assertEquals(expected_swift_size, size)
+        self.assertEquals(expected_checksum, checksum)
 
         url_pieces = urlparse.urlparse(expected_location)
         new_image_swift = SwiftBackend.get(url_pieces)
@@ -280,8 +288,9 @@
         options['swift_store_create_container_on_put'] = 'True'
         options['swift_store_container'] = 'noexist'
         expected_image_id = 42
-        expected_swift_size = 1024 * 5  # 5K
+        expected_swift_size = FIVE_KB
         expected_swift_contents = "*" * expected_swift_size
+        expected_checksum = hashlib.md5(expected_swift_contents).hexdigest()
         expected_location = format_swift_location(
             options['swift_store_user'],
             options['swift_store_key'],
@@ -290,10 +299,12 @@
             expected_image_id)
         image_swift = StringIO.StringIO(expected_swift_contents)
 
-        location, size = SwiftBackend.add(42, image_swift, options)
+        location, size, checksum = SwiftBackend.add(42, image_swift,
+                                                    options)
 
         self.assertEquals(expected_location, location)
         self.assertEquals(expected_swift_size, size)
+        self.assertEquals(expected_checksum, checksum)
 
         url_pieces = urlparse.urlparse(expected_location)
         new_image_swift = SwiftBackend.get(url_pieces)