Add methods for share instances in Share API

This commit adds a possibility to create/delete
share instances and allow/disable access to individual
share instances through Share API. It's required minimum
in Manila core to implement share migration functionality.

Also this commit updates methods in Share RPC API,
Share manager, Scheduler RPC API and Share manager
to maintain consistency.

Partially implements bp share-instances

Change-Id: I3af93071b5851944b59a3c5b1a0c2296e1267bfe

manila/db/api.py

@@ -287,18 +287,51 @@ def share_instance_get(context, instance_id, with_share_data=False):
                                    with_share_data=with_share_data)
 
 
+def share_instance_create(context, share_id, values):
+    """Create new share instance."""
+    return IMPL.share_instance_create(context, share_id, values)
+
+
+def share_instance_delete(context, instance_id):
+    """Delete share instance."""
+    return IMPL.share_instance_delete(context, instance_id)
+
+
 def share_instance_update(context, instance_id, values, with_share_data=False):
     """Update share instance fields."""
     return IMPL.share_instance_update(context, instance_id, values,
                                       with_share_data=with_share_data)
 
 
+def share_instances_get_all_by_share_server(context, share_server_id):
+    """Returns all share instances with given share_server_id."""
+    return IMPL.share_instances_get_all_by_share_server(context,
+                                                        share_server_id)
+
+
+def share_instances_get_all_by_host(context, host):
+    """Returns all share instances with given host."""
+    return IMPL.share_instances_get_all_by_host(context, host)
+
+
+def share_instances_get_all_by_share_network(context, share_network_id):
+    """Returns list of shares that belong to given share network."""
+    return IMPL.share_instances_get_all_by_share_network(context,
+                                                         share_network_id)
+
+
+def share_instances_get_all_by_share(context, share_id):
+    """Returns list of shares that belong to given share."""
+    return IMPL.share_instances_get_all_by_share_network(context, share_id)
+
+
 ###################
 
 
-def share_create(context, values):
+def share_create(context, values, create_share_instance=True):
     """Create new share."""
-    return IMPL.share_create(context, values)
+    return IMPL.share_create(context, values,
+                             create_share_instance=create_share_instance)
 
 
 def share_data_get_for_project(context, project_id, session=None):
@@ -323,11 +356,6 @@ def share_get_all(context, filters=None, sort_key=None, sort_dir=None):
     )
 
 
-def share_instances_get_all_by_host(context, host):
-    """Returns all share instances with given host."""
-    return IMPL.share_instances_get_all_by_host(context, host)
-
-
 def share_get_all_by_project(context, project_id, filters=None,
                              is_public=False, sort_key=None, sort_dir=None):
     """Returns all shares with given project ID."""
@@ -337,12 +365,6 @@ def share_get_all_by_project(context, project_id, filters=None,
     )
 
 
-def share_instances_get_all_by_share_network(context, share_network_id):
-    """Returns list of shares that belong to given share network."""
-    return IMPL.share_instances_get_all_by_share_network(context,
-                                                         share_network_id)
-
-
 def share_get_all_by_share_server(context, share_server_id, filters=None,
                                   sort_key=None, sort_dir=None):
     """Returns all shares with given share server ID."""
@@ -366,12 +388,17 @@ def share_access_create(context, values):
 
 
 def share_access_get(context, access_id):
-    """Allow access to share."""
+    """Get share access rule."""
     return IMPL.share_access_get(context, access_id)
 
 
+def share_instance_access_get(context, access_id, instance_id):
+    """Get access rule mapping for share instance."""
+    return IMPL.share_instance_access_get(context, access_id, instance_id)
+
+
 def share_access_get_all_for_share(context, share_id):
-    """Allow access to share."""
+    """Get all access rules for given share."""
     return IMPL.share_access_get_all_for_share(context, share_id)
 
 
@@ -387,9 +414,14 @@ def share_access_delete(context, access_id):
     return IMPL.share_access_delete(context, access_id)
 
 
-def share_access_update(context, access_id, values):
+def share_instance_access_delete(context, mapping_id):
-    """Update access record."""
+    """Deny access to share instance."""
-    return IMPL.share_access_update(context, access_id, values)
+    return IMPL.share_instance_access_delete(context, mapping_id)
+
+
+def share_instance_access_update_state(context, mapping_id, state):
+    """Update state of access rule mapping."""
+    return IMPL.share_instance_access_update_state(context, mapping_id, state)
 
 
 ####################

manila/db/migrations/alembic/versions/579c267fbb4d_add_share_instances_access_map.py

@@ -0,0 +1,105 @@
+# 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.
+
+"""add_share_instances_access_map
+
+Revision ID: 579c267fbb4d
+Revises: 5077ffcc5f1c
+Create Date: 2015-08-19 07:51:52.928542
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '579c267fbb4d'
+down_revision = '5077ffcc5f1c'
+
+from alembic import op
+from sqlalchemy import Column, DateTime, ForeignKey, String
+from oslo_utils import uuidutils
+
+from manila.db.migrations import utils
+
+
+def upgrade():
+    """Create 'share_instance_access_map' table and move 'state' column."""
+
+    instance_access_table = op.create_table(
+        'share_instance_access_map',
+        Column('created_at', DateTime),
+        Column('updated_at', DateTime),
+        Column('deleted_at', DateTime),
+        Column('deleted', String(length=36), default='False'),
+        Column('id', String(length=36), primary_key=True, nullable=False),
+        Column('share_instance_id', String(length=36),
+               ForeignKey('share_instances.id', name="siam_instance_fk")),
+        Column('access_id', String(length=36),
+               ForeignKey('share_access_map.id', name="siam_access_fk")),
+        Column('state', String(length=255)),
+        mysql_engine='InnoDB',
+        mysql_charset='utf8')
+
+    # NOTE(u_glide): Move all states from 'share_access_map'
+    # to 'share_instance_access_map'
+    instance_access_mappings = []
+    connection = op.get_bind()
+    access_table = utils.load_table('share_access_map', connection)
+    instances_table = utils.load_table('share_instances', connection)
+
+    for access_rule in connection.execute(access_table.select()):
+        instances_query = instances_table.select().where(
+            instances_table.c.share_id == access_rule.share_id
+        )
+
+        for instance in connection.execute(instances_query):
+            instance_access_mappings.append({
+                'created_at': access_rule.created_at,
+                'updated_at': access_rule.updated_at,
+                'deleted_at': access_rule.deleted_at,
+                'deleted': access_rule.deleted,
+                'id': uuidutils.generate_uuid(),
+                'share_instance_id': instance.id,
+                'access_id': access_rule.id,
+                'state': access_rule.state,
+            })
+    op.bulk_insert(instance_access_table, instance_access_mappings)
+    op.drop_column('share_access_map', 'state')
+
+
+def downgrade():
+    """Remove 'share_instance_access_map' table and add 'state' column back.
+
+    This method can lead to data loss because only first state is saved in
+    share_access_map table.
+    """
+    op.add_column('share_access_map', Column('state', String(length=255)))
+
+    # NOTE(u_glide): Move all states from 'share_instance_access_map'
+    # to 'share_access_map'
+    connection = op.get_bind()
+    access_table = utils.load_table('share_access_map', connection)
+    instance_access_table = utils.load_table('share_instance_access_map',
+                                             connection)
+
+    for access_rule in connection.execute(access_table.select()):
+        access_mapping = connection.execute(
+            instance_access_table.select().where(
+                instance_access_table.c.deleted == "False").where(
+                instance_access_table.c.access_id == access_rule['id'])
+        ).first()
+
+        op.execute(
+            access_table.update().where(
+                access_table.c.id == access_rule['id']
+            ).values({'state': access_mapping['state']})
+        )
+
+    op.drop_table('share_instance_access_map')

manila/db/sqlalchemy/api.py

@@ -1115,7 +1115,13 @@ def extract_snapshot_instance_values(values):
 
 
 @require_context
-def share_instance_create(context, share_id, values, session):
+def share_instance_create(context, share_id, values):
+    session = get_session()
+    with session.begin():
+        return _share_instance_create(context, share_id, values, session)
+
+
+def _share_instance_create(context, share_id, values, session):
     if not values.get('id'):
         values['id'] = uuidutils.generate_uuid()
     values.update({'share_id': share_id})
@@ -1171,6 +1177,26 @@ def share_instance_get(context, share_instance_id, session=None,
     return result
 
 
+@require_context
+def share_instance_delete(context, instance_id, session=None):
+    if session is None:
+        session = get_session()
+
+    with session.begin():
+        instance_ref = share_instance_get(context, instance_id,
+                                          session=session)
+        instance_ref.soft_delete(session=session, update_status=True)
+
+        session.query(models.ShareInstanceExportLocations).filter_by(
+            share_instance_id=instance_id).soft_delete()
+
+        share = share_get(context, instance_ref['share_id'], session=session)
+
+        if len(share.instances) == 0:
+            share.soft_delete(session=session)
+            share_access_delete_all_by_share(context, share['id'])
+
+
 @require_admin_context
 def share_instances_get_all_by_host(context, host):
     """Retrieves all share instances hosted on a host."""
@@ -1196,6 +1222,28 @@ def share_instances_get_all_by_share_network(context, share_network_id):
     return result
 
 
+@require_context
+def share_instances_get_all_by_share_server(context, share_server_id):
+    """Returns list of share instance with given share server."""
+    result = (
+        model_query(context, models.ShareInstance).filter(
+            models.ShareInstance.share_server_id == share_server_id,
+        ).all()
+    )
+    return result
+
+
+@require_context
+def share_instances_get_all_by_share(context, share_id):
+    """Returns list of share instances that belong to given share."""
+    result = (
+        model_query(context, models.ShareInstance).filter(
+            models.ShareInstance.share_id == share_id,
+        ).all()
+    )
+    return result
+
+
 ################
 
 
@@ -1233,7 +1281,7 @@ def share_create(context, values, create_share_instance=True):
         share_ref.save(session=session)
 
         if create_share_instance:
-            share_instance_create(context, share_ref['id'],
+            _share_instance_create(context, share_ref['id'],
                                    share_instance_values, session=session)
 
         # NOTE(u_glide): Do so to prevent errors with relationships
@@ -1394,32 +1442,68 @@ def share_delete(context, share_id):
 
     with session.begin():
         share_ref = share_get(context, share_id, session)
+
+        if len(share_ref.instances) > 0:
+            msg = _("Share %(id)s has %(count)s share instances.") % {
+                'id': share_id, 'count': len(share_ref.instances)}
+            raise exception.InvalidShare(msg)
+
         share_ref.soft_delete(session=session)
-        share_ref.instance.soft_delete(session=session, update_status=True)
 
         session.query(models.ShareMetadata).\
             filter_by(share_id=share_id).soft_delete()
-        session.query(models.ShareInstanceExportLocations).\
-            filter_by(share_instance_id=share_ref.instance['id']).soft_delete()
 
 
 ###################
 
 
-def _share_access_get_query(context, session, values):
+def _share_access_get_query(context, session, values, read_deleted='no'):
     """Get access record."""
-    query = model_query(context, models.ShareAccessMapping, session=session)
+    query = model_query(context, models.ShareAccessMapping, session=session,
+                        read_deleted=read_deleted)
     return query.filter_by(**values)
 
 
+def _share_instance_access_query(context, session, access_id,
+                                 instance_id=None):
+    filters = {'access_id': access_id}
+
+    if instance_id is not None:
+        filters.update({'share_instance_id': instance_id})
+
+    return model_query(context, models.ShareInstanceAccessMapping,
+                       session=session).filter_by(**filters)
+
+
 @require_context
 def share_access_create(context, values):
     values = ensure_model_dict_has_id(values)
     session = get_session()
     with session.begin():
         access_ref = models.ShareAccessMapping()
+        state = values.pop('state', None)
         access_ref.update(values)
         access_ref.save(session=session)
+
+        parent_share = share_get(context, values['share_id'], session=session)
+
+        for instance in parent_share.instances:
+            vals = {
+                'share_instance_id': instance['id'],
+                'access_id': access_ref['id'],
+            }
+            if state is not None:
+                vals.update({'state': state})
+
+            _share_instance_access_create(vals, session)
+
+    return share_access_get(context, access_ref['id'])
+
+
+def _share_instance_access_create(values, session):
+    access_ref = models.ShareInstanceAccessMapping()
+    access_ref.update(ensure_model_dict_has_id(values))
+    access_ref.save(session=session)
     return access_ref
 
 
@@ -1427,8 +1511,22 @@ def share_access_create(context, values):
 def share_access_get(context, access_id):
     """Get access record."""
     session = get_session()
-    access = _share_access_get_query(context, session,
+
-                                     {'id': access_id}).first()
+    access = _share_access_get_query(
+        context, session, {'id': access_id}).first()
+    if access:
+        return access
+    else:
+        raise exception.NotFound()
+
+
+@require_context
+def share_instance_access_get(context, access_id, instance_id):
+    """Get access record."""
+    session = get_session()
+
+    access = _share_instance_access_query(context, session, access_id,
+                                          instance_id).first()
     if access:
         return access
     else:
@@ -1442,6 +1540,10 @@ def share_access_get_all_for_share(context, share_id):
                                    {'share_id': share_id}).all()
 
 
+def _share_instance_access_get_all(context, access_id, session):
+    return _share_instance_access_query(context, session, access_id).all()
+
+
 @require_context
 def share_access_get_all_by_type_and_access(context, share_id, access_type,
                                             access):
@@ -1455,22 +1557,65 @@ def share_access_get_all_by_type_and_access(context, share_id, access_type,
 @require_context
 def share_access_delete(context, access_id):
     session = get_session()
+
     with session.begin():
+        mappings = _share_instance_access_get_all(context, access_id, session)
+
+        if len(mappings) > 0:
+            msg = (_("Access rule %s has mappings"
+                     " to share instances.") % access_id)
+            raise exception.InvalidShareAccess(msg)
+
         session.query(models.ShareAccessMapping).\
             filter_by(id=access_id).soft_delete(update_status=True,
                                                 status_field_name='state')
 
 
 @require_context
-@oslo_db_api.wrap_db_retry(max_retries=5, retry_on_deadlock=True)
+def share_access_delete_all_by_share(context, share_id):
-def share_access_update(context, access_id, values):
     session = get_session()
     with session.begin():
-        access = _share_access_get_query(context, session, {'id': access_id})
+        session.query(models.ShareAccessMapping). \
-        access = access.one()
+            filter_by(share_id=share_id).soft_delete(update_status=True,
-        access.update(values)
+                                                     status_field_name='state')
-        access.save(session=session)
+
-        return access
+
+@require_context
+def share_instance_access_delete(context, mapping_id):
+    session = get_session()
+    with session.begin():
+
+        mapping = session.query(models.ShareInstanceAccessMapping).\
+            filter_by(id=mapping_id).first()
+
+        if not mapping:
+            exception.NotFound()
+
+        mapping.soft_delete(session, update_status=True,
+                            status_field_name='state')
+
+        other_mappings = _share_instance_access_get_all(
+            context, mapping['access_id'], session)
+
+        # NOTE(u_glide): Remove access rule if all mappings were removed.
+        if len(other_mappings) == 0:
+            (
+                session.query(models.ShareAccessMapping)
+                .filter_by(id=mapping['access_id'])
+                .soft_delete(update_status=True, status_field_name='state')
+            )
+
+
+@require_context
+@oslo_db_api.wrap_db_retry(max_retries=5, retry_on_deadlock=True)
+def share_instance_access_update_state(context, mapping_id, state):
+    session = get_session()
+    with session.begin():
+        mapping = session.query(models.ShareInstanceAccessMapping).\
+            filter_by(id=mapping_id).first()
+        mapping.update({'state': state})
+        mapping.save(session=session)
+        return mapping
 
 
 ###################

manila/db/sqlalchemy/models.py

@@ -373,24 +373,76 @@ class ShareMetadata(BASE, ManilaBase):
 
 
 class ShareAccessMapping(BASE, ManilaBase):
-    """Represents access to NFS."""
+    """Represents access to share."""
+    __tablename__ = 'share_access_map'
+
+    @property
+    def state(self):
+        state = ShareInstanceAccessMapping.STATE_NEW
+
+        if len(self.instance_mappings) > 0:
+            state = ShareInstanceAccessMapping.STATE_ACTIVE
+            priorities = ShareInstanceAccessMapping.STATE_PRIORITIES
+
+            for mapping in self.instance_mappings:
+                priority = priorities.get(
+                    mapping['state'], ShareInstanceAccessMapping.STATE_ERROR)
+
+                if priority > priorities.get(state):
+                    state = mapping['state']
+
+                if state == ShareInstanceAccessMapping.STATE_ERROR:
+                    break
+
+        return state
+
+    id = Column(String(36), primary_key=True)
+    deleted = Column(String(36), default='False')
+    share_id = Column(String(36), ForeignKey('shares.id'))
+    access_type = Column(String(255))
+    access_to = Column(String(255))
+
+    access_level = Column(Enum(*constants.ACCESS_LEVELS),
+                          default=constants.ACCESS_LEVEL_RW)
+
+    instance_mappings = orm.relationship(
+        "ShareInstanceAccessMapping",
+        lazy='immediate',
+        primaryjoin=(
+            'and_('
+            'ShareAccessMapping.id == '
+            'ShareInstanceAccessMapping.access_id, '
+            'ShareInstanceAccessMapping.deleted == "False")'
+        )
+    )
+
+
+class ShareInstanceAccessMapping(BASE, ManilaBase):
+    """Represents access to individual share instances."""
     STATE_NEW = constants.STATUS_NEW
     STATE_ACTIVE = constants.STATUS_ACTIVE
     STATE_DELETING = constants.STATUS_DELETING
     STATE_DELETED = constants.STATUS_DELETED
     STATE_ERROR = constants.STATUS_ERROR
 
-    __tablename__ = 'share_access_map'
+    # NOTE(u_glide): State with greatest priority becomes a state of access
+    # rule
+    STATE_PRIORITIES = {
+        STATE_ACTIVE: 0,
+        STATE_NEW: 1,
+        STATE_DELETED: 2,
+        STATE_DELETING: 3,
+        STATE_ERROR: 4
+    }
+
+    __tablename__ = 'share_instance_access_map'
     id = Column(String(36), primary_key=True)
     deleted = Column(String(36), default='False')
-    share_id = Column(String(36), ForeignKey('shares.id'))
+    share_instance_id = Column(String(36), ForeignKey('share_instances.id'))
-    access_type = Column(String(255))
+    access_id = Column(String(36), ForeignKey('share_access_map.id'))
-    access_to = Column(String(255))
     state = Column(Enum(STATE_NEW, STATE_ACTIVE,
                         STATE_DELETING, STATE_DELETED, STATE_ERROR),
                    default=STATE_NEW)
-    access_level = Column(Enum(*constants.ACCESS_LEVELS),
-                          default=constants.ACCESS_LEVEL_RW)
 
 
 class ShareSnapshot(BASE, ManilaBase):

manila/exception.py

@@ -363,6 +363,10 @@ class InvalidShare(Invalid):
     message = _("Invalid share: %(reason)s.")
 
 
+class InvalidShareInstance(Invalid):
+    message = _("Invalid share instance: %(reason)s.")
+
+
 class ManageInvalidShare(InvalidShare):
     message = _("Manage existing share failed due to "
                 "invalid share: %(reason)s")

manila/scheduler/chance.py

@@ -66,7 +66,11 @@ class ChanceScheduler(driver.Scheduler):
         snapshot_id = request_spec['snapshot_id']
 
         updated_share = driver.share_update_db(context, share_id, host)
-        self.share_rpcapi.create_share(context, updated_share, host,
+        self.share_rpcapi.create_share_instance(
+            context,
+            updated_share.instance,
+            host,
             request_spec,
             filter_properties,
-                                       snapshot_id)
+            snapshot_id
+        )

manila/scheduler/filter_scheduler.py

@@ -102,10 +102,12 @@ class FilterScheduler(driver.Scheduler):
         # context is not serializable
         filter_properties.pop('context', None)
 
-        self.share_rpcapi.create_share(context, updated_share, host,
+        self.share_rpcapi.create_share_instance(
+            context, updated_share.instance, host,
             request_spec=request_spec,
             filter_properties=filter_properties,
-                                       snapshot_id=snapshot_id)
+            snapshot_id=snapshot_id
+        )
 
     def _schedule_share(self, context, request_spec, filter_properties=None):
         """Returns a list of hosts that meet the required specs.

manila/scheduler/manager.py

@@ -46,7 +46,7 @@ CONF.register_opt(scheduler_driver_opt)
 class SchedulerManager(manager.Manager):
     """Chooses a host to create shares."""
 
-    RPC_API_VERSION = '1.1'
+    RPC_API_VERSION = '1.2'
 
     def __init__(self, scheduler_driver=None, service_name=None,
                  *args, **kwargs):
@@ -76,8 +76,8 @@ class SchedulerManager(manager.Manager):
                                                 host,
                                                 capabilities)
 
-    def create_share(self, context, topic, share_id, snapshot_id=None,
+    def create_share_instance(self, context, request_spec=None,
-                     request_spec=None, filter_properties=None):
+                              filter_properties=None):
         try:
             self.driver.schedule_create_share(context, request_spec,
                                               filter_properties)

manila/scheduler/rpcapi.py

@@ -32,26 +32,25 @@ class SchedulerAPI(object):
 
         1.0 - Initial version.
         1.1 - Add get_pools method
+        1.2 - Introduce Share Instances:
+            Replace create_share() - > create_share_instance()
     '''
 
-    RPC_API_VERSION = '1.1'
+    RPC_API_VERSION = '1.2'
 
     def __init__(self):
         super(SchedulerAPI, self).__init__()
         target = messaging.Target(topic=CONF.scheduler_topic,
                                   version=self.RPC_API_VERSION)
-        self.client = rpc.get_client(target, version_cap='1.1')
+        self.client = rpc.get_client(target, version_cap='1.2')
 
-    def create_share(self, ctxt, topic, share_id, snapshot_id=None,
+    def create_share_instance(self, ctxt, request_spec=None,
-                     request_spec=None, filter_properties=None):
+                              filter_properties=None):
         request_spec_p = jsonutils.to_primitive(request_spec)
-        cctxt = self.client.prepare(version='1.0')
+        cctxt = self.client.prepare(version='1.2')
         return cctxt.cast(
             ctxt,
-            'create_share',
+            'create_share_instance',
-            topic=topic,
-            share_id=share_id,
-            snapshot_id=snapshot_id,
             request_spec=request_spec_p,
             filter_properties=filter_properties,
         )

manila/scheduler/simple.py

@@ -59,8 +59,9 @@ class SimpleScheduler(chance.ChanceScheduler):
             if not utils.service_is_up(service):
                 raise exception.WillNotSchedule(host=host)
             updated_share = driver.share_update_db(context, share_id, host)
-            self.share_rpcapi.create_share(context,
+            self.share_rpcapi.create_share_instance(
-                                           updated_share,
+                context,
+                updated_share.instance,
                 host,
                 request_spec,
                 None,
@@ -80,8 +81,9 @@ class SimpleScheduler(chance.ChanceScheduler):
             if utils.service_is_up(service) and not service['disabled']:
                 updated_share = driver.share_update_db(context, share_id,
                                                        service['host'])
-                self.share_rpcapi.create_share(context,
+                self.share_rpcapi.create_share_instance(
-                                               updated_share,
+                    context,
+                    updated_share.instance,
                     service['host'],
                     request_spec,
                     None,

manila/share/api.py

@@ -2,6 +2,7 @@
 # Administrator of the National Aeronautics and Space Administration.
 # All Rights Reserved.
 # Copyright (c) 2015 Tom Barron.  All rights reserved.
+# Copyright (c) 2015 Mirantis Inc.
 #
 #    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
@@ -38,7 +39,6 @@ from manila import policy
 from manila import quota
 from manila.scheduler import rpcapi as scheduler_rpcapi
 from manila.share import rpcapi as share_rpcapi
-from manila.share import share_types
 
 share_api_opts = [
     cfg.BoolOpt('use_scheduler_creating_share_from_snapshot',
@@ -108,9 +108,6 @@ class API(base.Base):
             source_share = self.db.share_get(context, snapshot['share_id'])
             if share_type is None:
                 share_type_id = source_share['share_type_id']
-                if share_type_id is not None:
-                    share_type = share_types.get_share_type(context,
-                                                            share_type_id)
             else:
                 share_type_id = share_type['id']
                 if share_type_id != source_share['share_type_id']:
@@ -172,11 +169,8 @@ class API(base.Base):
                    'user_id': context.user_id,
                    'project_id': context.project_id,
                    'snapshot_id': snapshot_id,
-                   'share_network_id': share_network_id,
                    'availability_zone': availability_zone,
                    'metadata': metadata,
-                   'status': constants.STATUS_CREATING,
-                   'scheduled_at': timeutils.utcnow(),
                    'display_name': name,
                    'display_description': description,
                    'share_proto': share_proto,
@@ -185,7 +179,8 @@ class API(base.Base):
                    }
 
         try:
-            share = self.db.share_create(context, options)
+            share = self.db.share_create(context, options,
+                                         create_share_instance=False)
             QUOTAS.commit(context, reservations)
         except Exception:
             with excutils.save_and_reraise_exception():
@@ -194,43 +189,64 @@ class API(base.Base):
                 finally:
                     QUOTAS.rollback(context, reservations)
 
-        request_spec = {
+        host = None
-            'share_properties': options,
+        if snapshot and not CONF.use_scheduler_creating_share_from_snapshot:
-            'share_proto': share_proto,
-            'share_id': share['id'],
-            'snapshot_id': snapshot_id,
-            'share_type': share_type,
-        }
-        filter_properties = {}
-
-        if (snapshot and not CONF.use_scheduler_creating_share_from_snapshot):
             # Shares from snapshots with restriction - source host only.
             # It is common situation for different types of backends.
             host = snapshot['share']['host']
-            share = self.db.share_update(context, share['id'], {'host': host})
+
-            self.share_rpcapi.create_share(
+        self.create_instance(context, share, share_network_id=share_network_id,
-                context,
+                             host=host)
-                share,
-                host,
-                request_spec=request_spec,
-                filter_properties=filter_properties,
-                snapshot_id=snapshot_id,
-            )
-        else:
-            # Shares from scratch and from snapshots when source host is not
-            # the only allowed, it is possible, for example, in multibackend
-            # installation with Generic drivers only.
-            self.scheduler_rpcapi.create_share(
-                context,
-                CONF.share_topic,
-                share['id'],
-                snapshot_id,
-                request_spec=request_spec,
-                filter_properties=filter_properties,
-            )
 
         return share
 
+    def create_instance(self, context, share, share_network_id=None,
+                        host=None):
+        policy.check_policy(context, 'share', 'create')
+
+        share_instance = self.db.share_instance_create(
+            context, share['id'],
+            {
+                'share_network_id': share_network_id,
+                'status': constants.STATUS_CREATING,
+                'scheduled_at': timeutils.utcnow(),
+                'host': host or ''
+            }
+        )
+
+        share_dict = share.to_dict()
+        share_dict.update(
+            {'metadata': self.db.share_metadata_get(context, share['id'])}
+        )
+
+        share_type = None
+        if share['share_type_id']:
+            share_type = self.db.share_type_get(
+                context, share['share_type_id'])
+
+        request_spec = {
+            'share_properties': share_dict,
+            'share_proto': share['share_proto'],
+            'share_id': share['id'],
+            'snapshot_id': share['snapshot_id'],
+            'share_type': share_type,
+        }
+
+        if host:
+            self.share_rpcapi.create_share_instance(
+                context,
+                share_instance,
+                host,
+                request_spec=request_spec,
+                filter_properties={},
+                snapshot_id=share['snapshot_id'],
+            )
+        else:
+            # Create share instance from scratch or from snapshot could happen
+            # on hosts other than the source host.
+            self.scheduler_rpcapi.create_share_instance(
+                context, request_spec=request_spec, filter_properties={})
+
     def manage(self, context, share_data, driver_options):
         policy.check_policy(context, 'share', 'manage')
 
@@ -285,26 +301,13 @@ class API(base.Base):
     @policy.wrap_check_policy('share')
     def delete(self, context, share, force=False):
         """Delete share."""
+        share = self.db.share_get(context, share['id'])
         if context.is_admin and context.project_id != share['project_id']:
             project_id = share['project_id']
         else:
             project_id = context.project_id
 
         share_id = share['id']
-        if not share['host']:
-            try:
-                reservations = QUOTAS.reserve(context,
-                                              project_id=project_id,
-                                              shares=-1,
-                                              gigabytes=-share['size'])
-            except Exception:
-                reservations = None
-                LOG.exception(_LE("Failed to update quota for deleting share"))
-            self.db.share_delete(context.elevated(), share_id)
-
-            if reservations:
-                QUOTAS.commit(context, reservations, project_id=project_id)
-            return
 
         statuses = (constants.STATUS_AVAILABLE, constants.STATUS_ERROR)
         if not (force or share['status'] in statuses):
@@ -317,12 +320,43 @@ class API(base.Base):
             msg = _("Share still has %d dependent snapshots") % len(snapshots)
             raise exception.InvalidShare(reason=msg)
 
-        now = timeutils.utcnow()
+        try:
-        share = self.db.share_update(
+            reservations = QUOTAS.reserve(context,
-            context, share_id, {'status': constants.STATUS_DELETING,
+                                          project_id=project_id,
-                                'terminated_at': now})
+                                          shares=-1,
+                                          gigabytes=-share['size'])
+        except Exception as e:
+            reservations = None
+            LOG.exception(
+                _LE("Failed to update quota for deleting share: %s"),
+                six.text_type(e)
+            )
 
-        self.share_rpcapi.delete_share(context, share)
+        for share_instance in share.instances:
+            if share_instance['host']:
+                self.delete_instance(context, share_instance, force=force)
+            else:
+                self.db.share_instance_delete(context, share_instance['id'])
+
+        if reservations:
+            QUOTAS.commit(context, reservations, project_id=project_id)
+
+    def delete_instance(self, context, share_instance, force=False):
+        policy.check_policy(context, 'share', 'delete')
+
+        statuses = (constants.STATUS_AVAILABLE, constants.STATUS_ERROR)
+        if not (force or share_instance['status'] in statuses):
+            msg = _("Share instance status must be one of %(statuses)s") % {
+                "statuses": statuses}
+            raise exception.InvalidShareInstance(reason=msg)
+
+        share_instance = self.db.share_instance_update(
+            context, share_instance['id'],
+            {'status': constants.STATUS_DELETING,
+             'terminated_at': timeutils.utcnow()}
+        )
+
+        self.share_rpcapi.delete_share_instance(context, share_instance)
 
         # NOTE(u_glide): 'updated_at' timestamp is used to track last usage of
         # share server. This is required for automatic share servers cleanup
@@ -330,16 +364,17 @@ class API(base.Base):
         # doesn't have shares (unused). We do this update only on share
         # deletion because share server with shares cannot be deleted, so no
         # need to do this update on share creation or any other share operation
-        if share['share_server_id']:
+        if share_instance['share_server_id']:
             self.db.share_server_update(
                 context,
-                share['share_server_id'],
+                share_instance['share_server_id'],
                 {'updated_at': timeutils.utcnow()})
 
     def delete_share_server(self, context, server):
         """Delete share server."""
         policy.check_policy(context, 'share_server', 'delete', server)
-        shares = self.db.share_get_all_by_share_server(context, server['id'])
+        shares = self.db.share_instances_get_all_by_share_server(context,
+                                                                 server['id'])
         if shares:
             raise exception.ShareServerInUse(share_server_id=server['id'])
         # NOTE(vponomaryov): There is no share_server status update here,
@@ -560,13 +595,11 @@ class API(base.Base):
     def allow_access(self, ctx, share, access_type, access_to,
                      access_level=None):
         """Allow access to share."""
-        if not share['host']:
+        policy.check_policy(ctx, 'share', 'allow_access')
-            msg = _("Share host is None")
+        share = self.db.share_get(ctx, share['id'])
-            raise exception.InvalidShare(reason=msg)
         if share['status'] != constants.STATUS_AVAILABLE:
             msg = _("Share status must be %s") % constants.STATUS_AVAILABLE
             raise exception.InvalidShare(reason=msg)
-        policy.check_policy(ctx, 'share', 'allow_access')
         values = {
             'share_id': share['id'],
             'access_type': access_type,
@@ -582,27 +615,39 @@ class API(base.Base):
             msg = _("Invalid share access level: %s.") % access_level
             raise exception.InvalidShareAccess(reason=msg)
         access = self.db.share_access_create(ctx, values)
-        self.share_rpcapi.allow_access(ctx, share, access)
+
+        for share_instance in share.instances:
+            self.allow_access_to_instance(ctx, share_instance, access)
+
         return access
 
+    def allow_access_to_instance(self, context, share_instance, access):
+        policy.check_policy(context, 'share', 'allow_access')
+
+        if not share_instance['host']:
+            msg = _("Invalid share instance host: %s") % share_instance['host']
+            raise exception.InvalidShareInstance(reason=msg)
+
+        self.share_rpcapi.allow_access(context, share_instance, access)
+
     def deny_access(self, ctx, share, access):
         """Deny access to share."""
         policy.check_policy(ctx, 'share', 'deny_access')
         # First check state of the target share
-        if not share['host']:
+        share = self.db.share_get(ctx, share['id'])
-            msg = _("Share host is None")
+        if not (share.instances and share.instance['host']):
+            msg = _("Share doesn't have any instances")
             raise exception.InvalidShare(reason=msg)
         if share['status'] != constants.STATUS_AVAILABLE:
             msg = _("Share status must be %s") % constants.STATUS_AVAILABLE
             raise exception.InvalidShare(reason=msg)
 
         # Then check state of the access rule
-        if access['state'] == access.STATE_ERROR:
+        if access['state'] == constants.STATUS_ERROR:
             self.db.share_access_delete(ctx, access["id"])
-        elif access['state'] == access.STATE_ACTIVE:
+        elif access['state'] == constants.STATUS_ACTIVE:
-            self.db.share_access_update(ctx, access["id"],
+            for share_instance in share.instances:
-                                        {'state': access.STATE_DELETING})
+                self.deny_access_to_instance(ctx, share_instance, access)
-            self.share_rpcapi.deny_access(ctx, share, access)
         else:
             msg = _("Access policy should be %(active)s or in %(error)s "
                     "state") % {"active": constants.STATUS_ACTIVE,
@@ -610,6 +655,21 @@ class API(base.Base):
             raise exception.InvalidShareAccess(reason=msg)
             # update share state and send message to manager
 
+    def deny_access_to_instance(self, context, share_instance, access):
+        policy.check_policy(context, 'share', 'deny_access')
+
+        if not share_instance['host']:
+            msg = _("Invalid share instance host: %s") % share_instance['host']
+            raise exception.InvalidShareInstance(reason=msg)
+
+        access_mapping = self.db.share_instance_access_get(
+            context, access['id'], share_instance['id'])
+        self.db.share_instance_access_update_state(
+            context, access_mapping['id'],
+            access_mapping.STATE_DELETING)
+
+        self.share_rpcapi.deny_access(context, share_instance, access)
+
     def access_get_all(self, context, share):
         """Returns all access rules for share."""
         policy.check_policy(context, 'share', 'access_get_all')

manila/share/manager.py

@@ -94,7 +94,7 @@ QUOTAS = quota.QUOTAS
 class ShareManager(manager.SchedulerDependentManager):
     """Manages NAS storages."""
 
-    RPC_API_VERSION = '1.3'
+    RPC_API_VERSION = '1.4'
 
     def __init__(self, share_driver=None, service_name=None, *args, **kwargs):
         """Load the driver from args, or from flags."""
@@ -188,9 +188,9 @@ class ShareManager(manager.SchedulerDependentManager):
                     ctxt, share_instance['id'], export_locations)
 
             rules = self.db.share_access_get_all_for_share(
-                ctxt, share_instance['id'])
+                ctxt, share_instance['share_id'])
             for access_ref in rules:
-                if access_ref['state'] != access_ref.STATE_ACTIVE:
+                if access_ref['state'] != constants.STATUS_ACTIVE:
                     continue
 
                 try:
@@ -343,31 +343,32 @@ class ShareManager(manager.SchedulerDependentManager):
 
         return _provide_share_server_for_share()
 
-    def _get_share_server(self, context, share):
+    def _get_share_server(self, context, share_instance):
-        if share['share_server_id']:
+        if share_instance['share_server_id']:
             return self.db.share_server_get(
-                context, share['share_server_id'])
+                context, share_instance['share_server_id'])
         else:
             return None
 
     def _get_share_instance(self, context, share):
-        return self.db.share_instance_get(
+        if isinstance(share, six.string_types):
-            context, share.instance['id'], with_share_data=True)
+            id = share
+        else:
+            id = share.instance['id']
+        return self.db.share_instance_get(context, id, with_share_data=True)
 
-    def create_share(self, context, share_id, request_spec=None,
+    def create_share_instance(self, context, share_instance_id,
-                     filter_properties=None, snapshot_id=None):
+                              request_spec=None, filter_properties=None,
-        """Creates a share."""
+                              snapshot_id=None):
+        """Creates a share instance."""
         context = context.elevated()
-        if filter_properties is None:
-            filter_properties = {}
 
-        share_ref = self.db.share_get(context, share_id)
+        share_instance = self._get_share_instance(context, share_instance_id)
-        share_instance = self._get_share_instance(context, share_ref)
         share_network_id = share_instance.get('share_network_id', None)
 
         if share_network_id and not self.driver.driver_handles_share_servers:
-            self.db.share_update(
+            self.db.share_instance_update(
-                context, share_id, {'status': constants.STATUS_ERROR})
+                context, share_instance_id, {'status': constants.STATUS_ERROR})
             raise exception.ManilaException(
                 "Driver does not expect share-network to be provided "
                 "with current configuration.")
@@ -390,9 +391,11 @@ class ShareManager(manager.SchedulerDependentManager):
             except Exception:
                 with excutils.save_and_reraise_exception():
                     LOG.error(_LE("Failed to get share server"
-                                  " for share creation."))
+                                  " for share instance creation."))
-                    self.db.share_update(context, share_id,
+                    self.db.share_instance_update(
-                                         {'status': constants.STATUS_ERROR})
+                        context, share_instance_id,
+                        {'status': constants.STATUS_ERROR}
+                    )
         else:
             share_server = None
 
@@ -410,7 +413,8 @@ class ShareManager(manager.SchedulerDependentManager):
 
         except Exception as e:
             with excutils.save_and_reraise_exception():
-                LOG.error(_LE("Share %s failed on creation."), share_id)
+                LOG.error(_LE("Share instance %s failed on creation."),
+                          share_instance_id)
                 detail_data = getattr(e, 'detail_data', {})
 
                 def get_export_location(details):
@@ -425,17 +429,21 @@ class ShareManager(manager.SchedulerDependentManager):
                     self.db.share_export_locations_update(
                         context, share_instance['id'], export_locations)
                 else:
-                    LOG.warning(_LW('Share information in exception '
+                    LOG.warning(_LW('Share instance information in exception '
                                     'can not be written to db because it '
                                     'contains %s and it is not a dictionary.'),
                                 detail_data)
-                self.db.share_update(
+                self.db.share_instance_update(
-                    context, share_id, {'status': constants.STATUS_ERROR})
+                    context, share_instance_id,
+                    {'status': constants.STATUS_ERROR}
+                )
         else:
-            LOG.info(_LI("Share created successfully."))
+            LOG.info(_LI("Share instance created successfully."))
-            self.db.share_update(context, share_id,
+            self.db.share_instance_update(
+                context, share_instance_id,
                 {'status': constants.STATUS_AVAILABLE,
-                                  'launched_at': timeutils.utcnow()})
+                 'launched_at': timeutils.utcnow()}
+            )
 
     def manage_share(self, context, share_id, driver_options):
         context = context.elevated()
@@ -547,7 +555,7 @@ class ShareManager(manager.SchedulerDependentManager):
         if self.configuration.safe_get('unmanage_remove_access_rules'):
             try:
                 self._remove_share_access_rules(context, share_ref,
-                                                share_server)
+                                                share_instance, share_server)
             except Exception as e:
                 share_manage_set_error_status(
                     _LE("Can not remove access rules of share: %s."), e)
@@ -557,46 +565,32 @@ class ShareManager(manager.SchedulerDependentManager):
                              {'status': constants.STATUS_UNMANAGED,
                               'deleted': True})
 
-    def delete_share(self, context, share_id):
+    def delete_share_instance(self, context, share_instance_id):
-        """Delete a share."""
+        """Delete a share instance."""
         context = context.elevated()
-        share_ref = self.db.share_get(context, share_id)
+        share_instance = self._get_share_instance(context, share_instance_id)
-        share_instance = self._get_share_instance(context, share_ref)
+        share = self.db.share_get(context, share_instance['share_id'])
-        share_server = self._get_share_server(context, share_ref)
+        share_server = self._get_share_server(context, share_instance)
-
-        if context.project_id != share_ref['project_id']:
-            project_id = share_ref['project_id']
-        else:
-            project_id = context.project_id
 
         try:
-            self._remove_share_access_rules(context, share_ref, share_server)
+            self._remove_share_access_rules(context, share, share_instance,
+                                            share_server)
             self.driver.delete_share(context, share_instance,
                                      share_server=share_server)
         except Exception:
             with excutils.save_and_reraise_exception():
-                self.db.share_update(
+                self.db.share_instance_update(
                     context,
-                    share_id,
+                    share_instance_id,
                     {'status': constants.STATUS_ERROR_DELETING})
-        try:
-            reservations = QUOTAS.reserve(context,
-                                          project_id=project_id,
-                                          shares=-1,
-                                          gigabytes=-share_ref['size'])
-        except Exception:
-            reservations = None
-            LOG.exception(_LE("Failed to update usages deleting share"))
 
-        self.db.share_delete(context, share_id)
+        self.db.share_instance_delete(context, share_instance_id)
-        LOG.info(_LI("Share %s: deleted successfully."), share_ref['name'])
+        LOG.info(_LI("Share instance %s: deleted successfully."),
-
+                 share_instance_id)
-        if reservations:
-            QUOTAS.commit(context, reservations, project_id=project_id)
 
         if CONF.delete_share_server_with_last_share:
-            share_server = self._get_share_server(context, share_ref)
+            share_server = self._get_share_server(context, share_instance)
-            if share_server and not share_server.share_instances:
+            if share_server and len(share_server.share_instances) == 0:
                 LOG.debug("Scheduled deletion of share-server "
                           "with id '%s' automatically by "
                           "deletion of last share.", share_server['id'])
@@ -616,12 +610,14 @@ class ShareManager(manager.SchedulerDependentManager):
         for server in servers:
             self.delete_share_server(ctxt, server)
 
-    def _remove_share_access_rules(self, context, share_ref, share_server):
+    def _remove_share_access_rules(self, context, share_ref, share_instance,
+                                   share_server):
         rules = self.db.share_access_get_all_for_share(
             context, share_ref['id'])
 
         for access_ref in rules:
-            self._deny_access(context, access_ref, share_ref, share_server)
+            self._deny_access(context, access_ref,
+                              share_instance, share_server)
 
     def create_snapshot(self, context, share_id, snapshot_id):
         """Create snapshot for share."""
@@ -701,44 +697,47 @@ class ShareManager(manager.SchedulerDependentManager):
             if reservations:
                 QUOTAS.commit(context, reservations, project_id=project_id)
 
-    def allow_access(self, context, access_id):
+    def allow_access(self, context, share_instance_id, access_id):
-        """Allow access to some share."""
+        """Allow access to some share instance."""
+        access_mapping = self.db.share_instance_access_get(context, access_id,
+                                                           share_instance_id)
+
+        if access_mapping['state'] != access_mapping.STATE_NEW:
+            return
+
         try:
             access_ref = self.db.share_access_get(context, access_id)
-            share_ref = self.db.share_get(context, access_ref['share_id'])
+            share_instance = self.db.share_instance_get(
-            share_server = self._get_share_server(context,
+                context, share_instance_id, with_share_data=True)
-                                                  share_ref)
+            share_server = self._get_share_server(context, share_instance)
-            if access_ref['state'] == access_ref.STATE_NEW:
+            self.driver.allow_access(context, share_instance, access_ref,
-                share_instance = self._get_share_instance(context, share_ref)
-                self.driver.allow_access(context, share_instance,
-                                         access_ref,
                                      share_server=share_server)
-                self.db.share_access_update(
+            self.db.share_instance_access_update_state(
-                    context, access_id, {'state': access_ref.STATE_ACTIVE})
+                context, access_mapping['id'], access_mapping.STATE_ACTIVE)
         except Exception:
             with excutils.save_and_reraise_exception():
-                self.db.share_access_update(
+                self.db.share_instance_access_update_state(
-                    context, access_id, {'state': access_ref.STATE_ERROR})
+                    context, access_mapping['id'], access_mapping.STATE_ERROR)
 
-    def deny_access(self, context, access_id):
+    def deny_access(self, context, share_instance_id, access_id):
         """Deny access to some share."""
         access_ref = self.db.share_access_get(context, access_id)
-        share_ref = self.db.share_get(context, access_ref['share_id'])
+        share_instance = self.db.share_instance_get(
-        share_server = self._get_share_server(context,
+            context, share_instance_id, with_share_data=True)
-                                              share_ref)
+        share_server = self._get_share_server(context, share_instance)
-        self._deny_access(context, access_ref, share_ref, share_server)
+        self._deny_access(context, access_ref, share_instance, share_server)
 
-    def _deny_access(self, context, access_ref, share_ref, share_server):
+    def _deny_access(self, context, access_ref, share_instance, share_server):
-        access_id = access_ref['id']
+        access_mapping = self.db.share_instance_access_get(
-        share_instance = self._get_share_instance(context, share_ref)
+            context, access_ref['id'], share_instance['id'])
         try:
             self.driver.deny_access(context, share_instance, access_ref,
                                     share_server=share_server)
+            self.db.share_instance_access_delete(context, access_mapping['id'])
         except Exception:
             with excutils.save_and_reraise_exception():
-                self.db.share_access_update(
+                self.db.share_instance_access_update_state(
-                    context, access_id, {'state': access_ref.STATE_ERROR})
+                    context, access_mapping['id'], access_mapping.STATE_ERROR)
-        self.db.share_access_delete(context, access_id)
 
     @periodic_task.periodic_task(spacing=CONF.periodic_interval)
     def _report_driver_status(self, context):
@@ -920,7 +919,8 @@ class ShareManager(manager.SchedulerDependentManager):
             # this method starts executing when amount of dependent shares
             # has been changed.
             server_id = share_server['id']
-            shares = self.db.share_get_all_by_share_server(context, server_id)
+            shares = self.db.share_instances_get_all_by_share_server(
+                context, server_id)
 
             if shares:
                 raise exception.ShareServerInUse(share_server_id=server_id)

manila/share/rpcapi.py

@@ -27,7 +27,7 @@ CONF = cfg.CONF
 
 
 class ShareAPI(object):
-    '''Client side of the share rpc API.
+    """Client side of the share rpc API.
 
     API version history:
 
@@ -35,7 +35,11 @@ class ShareAPI(object):
         1.1 - Add manage_share() and unmanage_share() methods
         1.2 - Add extend_share() method
         1.3 - Add shrink_share() method
-    '''
+        1.4 - Introduce Share Instances:
+            create_share() -> create_share_instance()
+            delete_share() -> delete_share_instance()
+            Add share_instance argument to allow_access() & deny_access()
+    """
 
     BASE_RPC_API_VERSION = '1.0'
 
@@ -43,18 +47,18 @@ class ShareAPI(object):
         super(ShareAPI, self).__init__()
         target = messaging.Target(topic=CONF.share_topic,
                                   version=self.BASE_RPC_API_VERSION)
-        self.client = rpc.get_client(target, version_cap='1.3')
+        self.client = rpc.get_client(target, version_cap='1.4')
 
-    def create_share(self, ctxt, share, host,
+    def create_share_instance(self, ctxt, share_instance, host,
                               request_spec, filter_properties,
                               snapshot_id=None):
         new_host = utils.extract_host(host)
-        cctxt = self.client.prepare(server=new_host, version='1.0')
+        cctxt = self.client.prepare(server=new_host, version='1.4')
         request_spec_p = jsonutils.to_primitive(request_spec)
         cctxt.cast(
             ctxt,
-            'create_share',
+            'create_share_instance',
-            share_id=share['id'],
+            share_instance_id=share_instance['id'],
             request_spec=request_spec_p,
             filter_properties=filter_properties,
             snapshot_id=snapshot_id,
@@ -73,10 +77,11 @@ class ShareAPI(object):
         cctxt = self.client.prepare(server=host, version='1.1')
         cctxt.cast(ctxt, 'unmanage_share', share_id=share['id'])
 
-    def delete_share(self, ctxt, share):
+    def delete_share_instance(self, ctxt, share_instance):
-        host = utils.extract_host(share['host'])
+        host = utils.extract_host(share_instance['host'])
-        cctxt = self.client.prepare(server=host, version='1.0')
+        cctxt = self.client.prepare(server=host, version='1.4')
-        cctxt.cast(ctxt, 'delete_share', share_id=share['id'])
+        cctxt.cast(ctxt, 'delete_share_instance',
+                   share_instance_id=share_instance['id'])
 
     def delete_share_server(self, ctxt, share_server):
         host = utils.extract_host(share_server['host'])
@@ -98,15 +103,19 @@ class ShareAPI(object):
         cctxt = self.client.prepare(server=new_host)
         cctxt.cast(ctxt, 'delete_snapshot', snapshot_id=snapshot['id'])
 
-    def allow_access(self, ctxt, share, access):
+    def allow_access(self, ctxt, share_instance, access):
-        host = utils.extract_host(share['host'])
+        host = utils.extract_host(share_instance['host'])
-        cctxt = self.client.prepare(server=host, version='1.0')
+        cctxt = self.client.prepare(server=host, version='1.4')
-        cctxt.cast(ctxt, 'allow_access', access_id=access['id'])
+        cctxt.cast(ctxt, 'allow_access',
+                   share_instance_id=share_instance['id'],
+                   access_id=access['id'])
 
-    def deny_access(self, ctxt, share, access):
+    def deny_access(self, ctxt, share_instance, access):
-        host = utils.extract_host(share['host'])
+        host = utils.extract_host(share_instance['host'])
-        cctxt = self.client.prepare(server=host, version='1.0')
+        cctxt = self.client.prepare(server=host, version='1.4')
-        cctxt.cast(ctxt, 'deny_access', access_id=access['id'])
+        cctxt.cast(ctxt, 'deny_access',
+                   share_instance_id=share_instance['id'],
+                   access_id=access['id'])
 
     def publish_service_capabilities(self, ctxt):
         cctxt = self.client.prepare(fanout=True, version='1.0')

manila/tests/db/sqlalchemy/test_api.py

@@ -77,11 +77,60 @@ class GenericDatabaseAPITestCase(test.TestCase):
         share = db_utils.create_share()
         share_access = db_utils.create_access(share_id=share['id'])
 
+        db_api.share_instance_access_delete(
+            self.ctxt, share_access.instance_mappings[0].id)
         db_api.share_access_delete(self.ctxt, share_access.id)
         self.assertRaises(exception.NotFound, db_api.share_access_get,
                           self.ctxt, share_access.id)
 
 
+@ddt.ddt
+class ShareAccessDatabaseAPITestCase(test.TestCase):
+
+    def setUp(self):
+        """Run before each test."""
+        super(ShareAccessDatabaseAPITestCase, self).setUp()
+        self.ctxt = context.get_admin_context()
+
+    @ddt.data(
+        {'statuses': (constants.STATUS_ACTIVE, constants.STATUS_ACTIVE,
+                      constants.STATUS_ACTIVE),
+         'valid': constants.STATUS_ACTIVE},
+        {'statuses': (constants.STATUS_ACTIVE, constants.STATUS_ACTIVE,
+                      constants.STATUS_NEW),
+         'valid': constants.STATUS_NEW},
+        {'statuses': (constants.STATUS_ACTIVE, constants.STATUS_ACTIVE,
+                      constants.STATUS_ERROR),
+         'valid': constants.STATUS_ERROR},
+        {'statuses': (constants.STATUS_DELETING, constants.STATUS_DELETED,
+                      constants.STATUS_ERROR),
+         'valid': constants.STATUS_ERROR},
+        {'statuses': (constants.STATUS_DELETING, constants.STATUS_DELETED,
+                      constants.STATUS_ACTIVE),
+         'valid': constants.STATUS_DELETING},
+        {'statuses': (constants.STATUS_DELETED, constants.STATUS_DELETED,
+                      constants.STATUS_DELETED),
+         'valid': constants.STATUS_DELETED},
+    )
+    @ddt.unpack
+    def test_share_access_state(self, statuses, valid):
+        share = db_utils.create_share()
+        db_utils.create_share_instance(share_id=share['id'])
+        db_utils.create_share_instance(share_id=share['id'])
+
+        share = db_api.share_get(self.ctxt, share['id'])
+        access = db_utils.create_access(state=constants.STATUS_ACTIVE,
+                                        share_id=share['id'])
+
+        for index, mapping in enumerate(access.instance_mappings):
+            db_api.share_instance_access_update_state(
+                self.ctxt, mapping['id'], statuses[index])
+
+        access = db_api.share_access_get(self.ctxt, access['id'])
+
+        self.assertEqual(valid, access.state)
+
+
 @ddt.ddt
 class ShareDatabaseAPITestCase(test.TestCase):
 
@@ -129,6 +178,14 @@ class ShareDatabaseAPITestCase(test.TestCase):
         self.assertEqual(1, len(actual_result))
         self.assertEqual(share['id'], actual_result[0].id)
 
+    def test_share_instance_delete_with_share(self):
+        share = db_utils.create_share()
+
+        db_api.share_instance_delete(self.ctxt, share.instance['id'])
+
+        self.assertRaises(exception.NotFound, db_api.share_get,
+                          self.ctxt, share['id'])
+
     @ddt.data('host', 'availability_zone')
     def test_share_get_all_sort_by_share_instance_fields(self, sort_key):
         shares = [db_utils.create_share(**{sort_key: n, 'size': 1})

manila/tests/db_utils.py

@@ -41,18 +41,37 @@ def create_share(**kwargs):
         'project_id': 'fake',
         'metadata': {'fake_key': 'fake_value'},
         'availability_zone': 'fake_availability_zone',
-        'status': "creating",
+        'status': constants.STATUS_CREATING,
         'host': 'fake_host'
     }
     return _create_db_row(db.share_create, share, kwargs)
 
 
+def create_share_instance(**kwargs):
+    """Create a share instance object."""
+    instance = {
+        'host': 'fake',
+        'status': constants.STATUS_CREATING,
+    }
+    instance.update(kwargs)
+
+    return db.share_instance_create(context.get_admin_context(),
+                                    kwargs.pop('share_id'), kwargs)
+
+
 def create_snapshot(**kwargs):
     """Create a snapshot object."""
+    with_share = kwargs.pop('with_share', False)
+
+    share = None
+    if with_share:
+        share = create_share(status=constants.STATUS_AVAILABLE,
+                             size=kwargs.get('size', 0))
+
     snapshot = {
         'share_proto': "NFS",
         'size': 0,
-        'share_id': None,
+        'share_id': share['id'] if with_share else None,
         'user_id': 'fake',
         'project_id': 'fake',
         'status': 'creating'
@@ -66,7 +85,6 @@ def create_access(**kwargs):
         'access_type': 'fake_type',
         'access_to': 'fake_IP',
         'share_id': None,
-        'state': 'new',
     }
     return _create_db_row(db.share_access_create, access, kwargs)
 

manila/tests/scheduler/test_rpcapi.py

@@ -80,15 +80,12 @@ class SchedulerRpcAPITestCase(test.TestCase):
                                  capabilities='fake_capabilities',
                                  fanout=True)
 
-    def test_create_share(self):
+    def test_create_share_instance(self):
-        self._test_scheduler_api('create_share',
+        self._test_scheduler_api('create_share_instance',
                                  rpc_method='cast',
-                                 topic='topic',
-                                 share_id='share_id',
-                                 snapshot_id='snapshot_id',
                                  request_spec='fake_request_spec',
                                  filter_properties='filter_properties',
-                                 version='1.0')
+                                 version='1.2')
 
     def test_get_pools(self):
         self._test_scheduler_api('get_pools',

manila/tests/scheduler/test_scheduler.py

@@ -29,6 +29,7 @@ from manila.scheduler import manager
 from manila.scheduler import simple
 from manila.share import rpcapi as share_rpcapi
 from manila import test
+from manila.tests import db_utils
 from manila import utils
 
 CONF = cfg.CONF
@@ -83,16 +84,14 @@ class SchedulerManagerTestCase(test.TestCase):
             raise exception.NoValidHost(reason="")
 
         fake_share_id = 1
-        topic = 'fake_topic'
+
-        share_id = fake_share_id
         request_spec = {'share_id': fake_share_id}
         with mock.patch.object(self.manager.driver,
                                'schedule_create_share',
                                mock.Mock(side_effect=raise_no_valid_host)):
             self.mock_object(manager.LOG, 'error')
-            self.manager.create_share(self.context, topic, share_id,
+            self.manager.create_share_instance(
-                                      request_spec=request_spec,
+                self.context, request_spec=request_spec, filter_properties={})
-                                      filter_properties={})
             db.share_update.assert_called_once_with(
                 self.context, fake_share_id, {'status': 'error'})
             self.manager.driver.schedule_create_share.assert_called_once_with(
@@ -209,19 +208,18 @@ class SimpleSchedulerSharesTestCase(test.TestCase):
             'share_id': share_id,
             'share_properties': fake_share,
         }
-        with mock.patch.object(db, 'service_get_all_share_sorted',
+        self.mock_object(db, 'service_get_all_share_sorted',
-                               mock.Mock(return_value=fake_result)):
+                         mock.Mock(return_value=fake_result))
-            with mock.patch.object(driver, 'share_update_db',
+        self.mock_object(driver, 'share_update_db',
-                                   mock.Mock(return_value=fake_share)):
+                         mock.Mock(return_value=db_utils.create_share()))
+
         self.driver.schedule_create_share(self.context,
                                           fake_request_spec, {})
-                utils.service_is_up.assert_called_once_with(
+        utils.service_is_up.assert_called_once_with(utils.IsAMatcher(dict))
-                    utils.IsAMatcher(dict))
         db.service_get_all_share_sorted.assert_called_once_with(
             utils.IsAMatcher(context.RequestContext))
         driver.share_update_db.assert_called_once_with(
-                    utils.IsAMatcher(context.RequestContext),
+            utils.IsAMatcher(context.RequestContext), share_id, 'fake_host1')
-                    share_id, 'fake_host1')
 
     def test_create_share_if_services_not_available(self):
         share_id = 'fake'
@@ -278,10 +276,11 @@ class SimpleSchedulerSharesTestCase(test.TestCase):
             'share_id': share_id,
             'share_properties': fake_share,
         }
-        with mock.patch.object(db, 'service_get_all_share_sorted',
+        self.mock_object(db, 'service_get_all_share_sorted',
-                               mock.Mock(return_value=fake_result)):
+                         mock.Mock(return_value=fake_result))
-            with mock.patch.object(driver, 'share_update_db',
+        self.mock_object(driver, 'share_update_db',
-                                   mock.Mock(return_value=fake_share)):
+                         mock.Mock(return_value=db_utils.create_share()))
+
         self.driver.schedule_create_share(self.context,
                                           fake_request_spec, {})
         utils.service_is_up.assert_called_once_with(fake_service_1)
@@ -303,16 +302,16 @@ class SimpleSchedulerSharesTestCase(test.TestCase):
             'share_id': share_id,
             'share_properties': fake_share,
         }
-        with mock.patch.object(db, 'service_get_by_args',
+        self.mock_object(db, 'service_get_by_args',
-                               mock.Mock(return_value='fake_service')):
+                         mock.Mock(return_value='fake_service'))
-            with mock.patch.object(driver, 'share_update_db',
+        self.mock_object(driver, 'share_update_db',
-                                   mock.Mock(return_value=fake_share)):
+                         mock.Mock(return_value=db_utils.create_share()))
+
         self.driver.schedule_create_share(self.admin_context,
                                           fake_request_spec, {})
         utils.service_is_up.assert_called_once_with('fake_service')
         db.service_get_by_args.assert_called_once_with(
-                    utils.IsAMatcher(context.RequestContext),
+            utils.IsAMatcher(context.RequestContext), 'fake', 'manila-share')
-                    'fake', 'manila-share')
         driver.share_update_db.assert_called_once_with(
             utils.IsAMatcher(context.RequestContext), share_id, 'fake')
 

manila/tests/share/test_manager.py

@@ -37,21 +37,6 @@ from manila.tests import utils as test_utils
 from manila import utils
 
 
-class FakeAccessRule(object):
-
-    def __init__(self, **kwargs):
-        self.STATE_ACTIVE = constants.STATUS_ACTIVE
-        self.STATE_NEW = constants.STATUS_NEW
-        self.STATE_ERROR = constants.STATUS_ERROR
-        self.access_type = 'fake_type'
-        self.id = 'fake_id'
-        for k, v in kwargs.items():
-            setattr(self, k, v)
-
-    def __getitem__(self, item):
-        return getattr(self, item)
-
-
 @ddt.ddt
 class ShareManagerTestCase(test.TestCase):
 
@@ -98,6 +83,30 @@ class ShareManagerTestCase(test.TestCase):
         self.share_manager.driver.check_for_setup_error.\
             assert_called_once_with()
 
+    def _setup_init_mocks(self, setup_access_rules=True):
+        instances = [
+            db_utils.create_share(id='fake_id_1',
+                                  status=constants.STATUS_AVAILABLE,
+                                  display_name='fake_name_1').instance,
+            db_utils.create_share(id='fake_id_2',
+                                  status=constants.STATUS_ERROR,
+                                  display_name='fake_name_2').instance,
+            db_utils.create_share(id='fake_id_3',
+                                  status=constants.STATUS_AVAILABLE,
+                                  display_name='fake_name_3').instance,
+        ]
+        if not setup_access_rules:
+            return instances
+
+        rules = [
+            db_utils.create_access(state=constants.STATUS_ACTIVE,
+                                   share_id='fake_id_1'),
+            db_utils.create_access(state=constants.STATUS_ERROR,
+                                   share_id='fake_id_3'),
+        ]
+
+        return instances, rules
+
     def test_init_host_with_shares_and_rules(self):
 
         # initialisation of test data
@@ -105,24 +114,14 @@ class ShareManagerTestCase(test.TestCase):
             raise exception.ShareAccessExists(
                 access_type='fake_access_type', access='fake_access')
 
-        shares = [
+        instances, rules = self._setup_init_mocks()
-            {'id': 'fake_id_1', 'status': constants.STATUS_AVAILABLE},
-            {'id': 'fake_id_2',
-             'status': constants.STATUS_ERROR,
-             'name': 'fake_name_2'},
-            {'id': 'fake_id_3', 'status': 'fake', 'name': 'fake_name_3'},
-        ]
-        rules = [
-            FakeAccessRule(state=constants.STATUS_ACTIVE),
-            FakeAccessRule(state=constants.STATUS_ERROR),
-        ]
         fake_export_locations = ['fake/path/1', 'fake/path']
         share_server = 'fake_share_server_type_does_not_matter'
         self.mock_object(self.share_manager.db,
                          'share_instances_get_all_by_host',
-                         mock.Mock(return_value=shares))
+                         mock.Mock(return_value=instances))
         self.mock_object(self.share_manager.db, 'share_instance_get',
-                         mock.Mock(side_effect=shares))
+                         mock.Mock(side_effect=[instances[0], instances[2]]))
         self.mock_object(self.share_manager.db,
                          'share_export_locations_update')
         self.mock_object(self.share_manager.driver, 'ensure_share',
@@ -146,51 +145,55 @@ class ShareManagerTestCase(test.TestCase):
             assert_called_once_with(utils.IsAMatcher(context.RequestContext),
                                     self.share_manager.host)
         exports_update = self.share_manager.db.share_export_locations_update
-        exports_update.assert_called_once_with(
+        exports_update.assert_has_calls([
-            mock.ANY, 'fake_id_1', fake_export_locations)
+            mock.call(mock.ANY, instances[0]['id'], fake_export_locations),
+            mock.call(mock.ANY, instances[2]['id'], fake_export_locations)
+        ])
         self.share_manager.driver.do_setup.assert_called_once_with(
             utils.IsAMatcher(context.RequestContext))
         self.share_manager.driver.check_for_setup_error.\
             assert_called_once_with()
-        self.share_manager._ensure_share_instance_has_pool.\
+        self.share_manager._ensure_share_instance_has_pool.assert_has_calls([
-            assert_called_once_with(utils.IsAMatcher(context.RequestContext),
+            mock.call(utils.IsAMatcher(context.RequestContext), instances[0]),
-                                    shares[0])
+            mock.call(utils.IsAMatcher(context.RequestContext), instances[2]),
-        self.share_manager._get_share_server.assert_called_once_with(
+        ])
-            utils.IsAMatcher(context.RequestContext), shares[0])
+        self.share_manager._get_share_server.assert_has_calls([
-        self.share_manager.driver.ensure_share.assert_called_once_with(
+            mock.call(utils.IsAMatcher(context.RequestContext), instances[0]),
-            utils.IsAMatcher(context.RequestContext), shares[0],
+            mock.call(utils.IsAMatcher(context.RequestContext), instances[2]),
-            share_server=share_server)
+        ])
-        self.share_manager.db.share_access_get_all_for_share.\
+        self.share_manager.driver.ensure_share.assert_has_calls([
-            assert_called_once_with(
+            mock.call(utils.IsAMatcher(context.RequestContext), instances[0],
-                utils.IsAMatcher(context.RequestContext), shares[0]['id'])
+                      share_server=share_server),
+            mock.call(utils.IsAMatcher(context.RequestContext), instances[2],
+                      share_server=share_server),
+        ])
+        self.share_manager.db.share_access_get_all_for_share.assert_has_calls([
+            mock.call(utils.IsAMatcher(context.RequestContext),
+                      instances[0]['share_id']),
+            mock.call(utils.IsAMatcher(context.RequestContext),
+                      instances[2]['share_id']),
+        ])
         self.share_manager.publish_service_capabilities.\
             assert_called_once_with(
                 utils.IsAMatcher(context.RequestContext))
-        self.share_manager.driver.allow_access.assert_called_once_with(
+        self.share_manager.driver.allow_access.assert_has_calls([
-            utils.IsAMatcher(context.RequestContext), shares[0], rules[0],
+            mock.call(mock.ANY, instances[0], rules[0],
-            share_server=share_server)
+                      share_server=share_server),
+            mock.call(mock.ANY, instances[2], rules[0],
+                      share_server=share_server),
+        ])
 
     def test_init_host_with_exception_on_ensure_share(self):
         def raise_exception(*args, **kwargs):
             raise exception.ManilaException(message="Fake raise")
 
-        shares = [
+        instances = self._setup_init_mocks(setup_access_rules=False)
-            {'id': 'fake_id_1',
-             'status': constants.STATUS_AVAILABLE,
-             'name': 'fake_name_1'},
-            {'id': 'fake_id_2',
-             'status': constants.STATUS_ERROR,
-             'name': 'fake_name_2'},
-            {'id': 'fake_id_3',
-             'status': constants.STATUS_AVAILABLE,
-             'name': 'fake_name_3'},
-        ]
         share_server = 'fake_share_server_type_does_not_matter'
         self.mock_object(self.share_manager.db,
                          'share_instances_get_all_by_host',
-                         mock.Mock(return_value=shares))
+                         mock.Mock(return_value=instances))
         self.mock_object(self.share_manager.db, 'share_instance_get',
-                         mock.Mock(side_effect=[shares[0], shares[2]]))
+                         mock.Mock(side_effect=[instances[0], instances[2]]))
         self.mock_object(self.share_manager.driver, 'ensure_share',
                          mock.Mock(side_effect=raise_exception))
         self.mock_object(self.share_manager, '_ensure_share_instance_has_pool')
@@ -211,17 +214,17 @@ class ShareManagerTestCase(test.TestCase):
             utils.IsAMatcher(context.RequestContext))
         self.share_manager.driver.check_for_setup_error.assert_called_with()
         self.share_manager._ensure_share_instance_has_pool.assert_has_calls([
-            mock.call(utils.IsAMatcher(context.RequestContext), shares[0]),
+            mock.call(utils.IsAMatcher(context.RequestContext), instances[0]),
-            mock.call(utils.IsAMatcher(context.RequestContext), shares[2]),
+            mock.call(utils.IsAMatcher(context.RequestContext), instances[2]),
         ])
         self.share_manager._get_share_server.assert_has_calls([
-            mock.call(utils.IsAMatcher(context.RequestContext), shares[0]),
+            mock.call(utils.IsAMatcher(context.RequestContext), instances[0]),
-            mock.call(utils.IsAMatcher(context.RequestContext), shares[2]),
+            mock.call(utils.IsAMatcher(context.RequestContext), instances[2]),
         ])
         self.share_manager.driver.ensure_share.assert_has_calls([
-            mock.call(utils.IsAMatcher(context.RequestContext), shares[0],
+            mock.call(utils.IsAMatcher(context.RequestContext), instances[0],
                       share_server=share_server),
-            mock.call(utils.IsAMatcher(context.RequestContext), shares[2],
+            mock.call(utils.IsAMatcher(context.RequestContext), instances[2],
                       share_server=share_server),
         ])
         self.share_manager.publish_service_capabilities.\
@@ -229,34 +232,20 @@ class ShareManagerTestCase(test.TestCase):
                 utils.IsAMatcher(context.RequestContext))
         manager.LOG.info.assert_called_once_with(
             mock.ANY,
-            {'id': shares[1]['id'], 'status': shares[1]['status']},
+            {'id': instances[1]['id'], 'status': instances[1]['status']},
         )
 
     def test_init_host_with_exception_on_rule_access_allow(self):
         def raise_exception(*args, **kwargs):
             raise exception.ManilaException(message="Fake raise")
 
-        shares = [
+        instances, rules = self._setup_init_mocks()
-            {'id': 'fake_id_1',
-             'status': constants.STATUS_AVAILABLE,
-             'name': 'fake_name_1'},
-            {'id': 'fake_id_2',
-             'status': constants.STATUS_ERROR,
-             'name': 'fake_name_2'},
-            {'id': 'fake_id_3',
-             'status': constants.STATUS_AVAILABLE,
-             'name': 'fake_name_3'},
-        ]
-        rules = [
-            FakeAccessRule(state=constants.STATUS_ACTIVE),
-            FakeAccessRule(state=constants.STATUS_ERROR),
-        ]
         share_server = 'fake_share_server_type_does_not_matter'
         self.mock_object(self.share_manager.db,
                          'share_instances_get_all_by_host',
-                         mock.Mock(return_value=shares))
+                         mock.Mock(return_value=instances))
         self.mock_object(self.share_manager.db, 'share_instance_get',
-                         mock.Mock(side_effect=[shares[0], shares[2]]))
+                         mock.Mock(side_effect=[instances[0], instances[2]]))
         self.mock_object(self.share_manager.driver, 'ensure_share',
                          mock.Mock(return_value=None))
         self.mock_object(self.share_manager, '_ensure_share_instance_has_pool')
@@ -282,17 +271,17 @@ class ShareManagerTestCase(test.TestCase):
             utils.IsAMatcher(context.RequestContext))
         self.share_manager.driver.check_for_setup_error.assert_called_with()
         self.share_manager._ensure_share_instance_has_pool.assert_has_calls([
-            mock.call(utils.IsAMatcher(context.RequestContext), shares[0]),
+            mock.call(utils.IsAMatcher(context.RequestContext), instances[0]),
-            mock.call(utils.IsAMatcher(context.RequestContext), shares[2]),
+            mock.call(utils.IsAMatcher(context.RequestContext), instances[2]),
         ])
         self.share_manager._get_share_server.assert_has_calls([
-            mock.call(utils.IsAMatcher(context.RequestContext), shares[0]),
+            mock.call(utils.IsAMatcher(context.RequestContext), instances[0]),
-            mock.call(utils.IsAMatcher(context.RequestContext), shares[2]),
+            mock.call(utils.IsAMatcher(context.RequestContext), instances[2]),
         ])
         self.share_manager.driver.ensure_share.assert_has_calls([
-            mock.call(utils.IsAMatcher(context.RequestContext), shares[0],
+            mock.call(utils.IsAMatcher(context.RequestContext), instances[0],
                       share_server=share_server),
-            mock.call(utils.IsAMatcher(context.RequestContext), shares[2],
+            mock.call(utils.IsAMatcher(context.RequestContext), instances[2],
                       share_server=share_server),
         ])
         self.share_manager.publish_service_capabilities.\
@@ -300,12 +289,12 @@ class ShareManagerTestCase(test.TestCase):
                 utils.IsAMatcher(context.RequestContext))
         manager.LOG.info.assert_called_once_with(
             mock.ANY,
-            {'id': shares[1]['id'], 'status': shares[1]['status']},
+            {'id': instances[1]['id'], 'status': instances[1]['status']},
         )
         self.share_manager.driver.allow_access.assert_has_calls([
-            mock.call(utils.IsAMatcher(context.RequestContext), shares[0],
+            mock.call(utils.IsAMatcher(context.RequestContext), instances[0],
                       rules[0], share_server=share_server),
-            mock.call(utils.IsAMatcher(context.RequestContext), shares[2],
+            mock.call(utils.IsAMatcher(context.RequestContext), instances[2],
                       rules[0], share_server=share_server),
         ])
         manager.LOG.error.assert_has_calls([
@@ -313,7 +302,7 @@ class ShareManagerTestCase(test.TestCase):
             mock.call(mock.ANY, mock.ANY),
         ])
 
-    def test_create_share_from_snapshot_with_server(self):
+    def test_create_share_instance_from_snapshot_with_server(self):
         """Test share can be created from snapshot if server exists."""
         network = db_utils.create_share_network()
         server = db_utils.create_share_server(
@@ -326,8 +315,8 @@ class ShareManagerTestCase(test.TestCase):
         snapshot = db_utils.create_snapshot(share_id=parent_share['id'])
         snapshot_id = snapshot['id']
 
-        self.share_manager.create_share(self.context, share_id,
+        self.share_manager.create_share_instance(
-                                        snapshot_id=snapshot_id)
+            self.context, share.instance['id'], snapshot_id=snapshot_id)
         self.assertEqual(share_id, db.share_get(context.get_admin_context(),
                          share_id).id)
 
@@ -335,7 +324,7 @@ class ShareManagerTestCase(test.TestCase):
         self.assertEqual(shr['status'], constants.STATUS_AVAILABLE)
         self.assertEqual(shr['share_server_id'], server['id'])
 
-    def test_create_share_from_snapshot_with_server_not_found(self):
+    def test_create_share_instance_from_snapshot_with_server_not_found(self):
         """Test creation from snapshot fails if server not found."""
         parent_share = db_utils.create_share(share_network_id='net-id',
                                              share_server_id='fake-id')
@@ -345,24 +334,24 @@ class ShareManagerTestCase(test.TestCase):
         snapshot_id = snapshot['id']
 
         self.assertRaises(exception.ShareServerNotFound,
-                          self.share_manager.create_share,
+                          self.share_manager.create_share_instance,
                           self.context,
-                          share_id,
+                          share.instance['id'],
                           snapshot_id=snapshot_id
                           )
 
         shr = db.share_get(self.context, share_id)
         self.assertEqual(shr['status'], constants.STATUS_ERROR)
 
-    def test_create_share_from_snapshot(self):
+    def test_create_share_instance_from_snapshot(self):
         """Test share can be created from snapshot."""
         share = db_utils.create_share()
         share_id = share['id']
         snapshot = db_utils.create_snapshot(share_id=share_id)
         snapshot_id = snapshot['id']
 
-        self.share_manager.create_share(self.context, share_id,
+        self.share_manager.create_share_instance(
-                                        snapshot_id=snapshot_id)
+            self.context, share.instance['id'], snapshot_id=snapshot_id)
         self.assertEqual(share_id, db.share_get(context.get_admin_context(),
                          share_id).id)
 
@@ -439,7 +428,7 @@ class ShareManagerTestCase(test.TestCase):
             utils.IsAMatcher(models.ShareSnapshotInstance),
             share_server=None)
 
-    def test_delete_share_if_busy(self):
+    def test_delete_share_instance_if_busy(self):
         """Test snapshot could not be deleted if busy."""
 
         def _raise_share_snapshot_is_busy(self, *args, **kwargs):
@@ -460,30 +449,34 @@ class ShareManagerTestCase(test.TestCase):
             utils.IsAMatcher(models.ShareSnapshotInstance),
             share_server=None)
 
-    def test_create_share_with_share_network_driver_not_handles_servers(self):
+    def test_create_share_instance_with_share_network_dhss_false(self):
         manager.CONF.set_default('driver_handles_share_servers', False)
         self.mock_object(
             self.share_manager.driver.configuration, 'safe_get',
             mock.Mock(return_value=False))
-        share_id = 'fake_share_id'
         share_network_id = 'fake_sn'
+        share_instance = db_utils.create_share(
+            share_network_id=share_network_id).instance
         self.mock_object(
-            self.share_manager.db, 'share_get',
+            self.share_manager.db, 'share_instance_get',
-            mock.Mock(return_value=db_utils.create_share(
+            mock.Mock(return_value=share_instance))
-                share_network_id=share_network_id)))
+        self.mock_object(self.share_manager.db, 'share_instance_update')
-        self.mock_object(self.share_manager.db, 'share_update')
 
         self.assertRaises(
             exception.ManilaException,
-            self.share_manager.create_share, self.context, share_id)
+            self.share_manager.create_share_instance, self.context,
+            share_instance['id'])
 
-        self.share_manager.db.share_get.assert_called_once_with(
+        self.share_manager.db.share_instance_get.assert_called_once_with(
-            utils.IsAMatcher(context.RequestContext), share_id)
+            utils.IsAMatcher(context.RequestContext),
-        self.share_manager.db.share_update.assert_called_once_with(
+            share_instance['id'],
-            utils.IsAMatcher(context.RequestContext), share_id,
+            with_share_data=True
+        )
+        self.share_manager.db.share_instance_update.assert_called_once_with(
+            utils.IsAMatcher(context.RequestContext), share_instance['id'],
             {'status': constants.STATUS_ERROR})
 
-    def test_create_share_with_share_network_server_not_exists(self):
+    def test_create_share_instance_with_share_network_server_not_exists(self):
         """Test share can be created without share server."""
 
         share_net = db_utils.create_share_network()
@@ -499,11 +492,12 @@ class ShareManagerTestCase(test.TestCase):
         self.share_manager.driver.create_share = mock.Mock(
             return_value='fake_location')
         self.share_manager._setup_server = fake_setup_server
-        self.share_manager.create_share(self.context, share_id)
+        self.share_manager.create_share_instance(self.context,
+                                                 share.instance['id'])
         self.assertEqual(share_id, db.share_get(context.get_admin_context(),
                          share_id).id)
 
-    def test_create_share_with_share_network_server_creation_failed(self):
+    def test_create_share_instance_with_share_network_server_fail(self):
         fake_share = db_utils.create_share(share_network_id='fake_sn_id',
                                            size=1)
         fake_server = {
@@ -512,10 +506,10 @@ class ShareManagerTestCase(test.TestCase):
         }
         self.mock_object(db, 'share_server_create',
                          mock.Mock(return_value=fake_server))
-        self.mock_object(db, 'share_update',
+        self.mock_object(db, 'share_instance_update',
-                         mock.Mock(return_value=fake_share))
+                         mock.Mock(return_value=fake_share.instance))
-        self.mock_object(db, 'share_get',
+        self.mock_object(db, 'share_instance_get',
-                         mock.Mock(return_value=fake_share))
+                         mock.Mock(return_value=fake_share.instance))
 
         def raise_share_server_not_found(*args, **kwargs):
             raise exception.ShareServerNotFound(
@@ -532,9 +526,9 @@ class ShareManagerTestCase(test.TestCase):
 
         self.assertRaises(
             exception.ManilaException,
-            self.share_manager.create_share,
+            self.share_manager.create_share_instance,
             self.context,
-            fake_share['id'],
+            fake_share.instance['id'],
         )
         db.share_server_get_all_by_host_and_share_net_valid.\
             assert_called_once_with(
@@ -544,31 +538,31 @@ class ShareManagerTestCase(test.TestCase):
             )
         db.share_server_create.assert_called_once_with(
             utils.IsAMatcher(context.RequestContext), mock.ANY)
-        db.share_update.assert_has_calls([
+        db.share_instance_update.assert_has_calls([
             mock.call(
                 utils.IsAMatcher(context.RequestContext),
-                fake_share['id'],
+                fake_share.instance['id'],
                 {'status': constants.STATUS_ERROR},
             )
         ])
         self.share_manager._setup_server.assert_called_once_with(
             utils.IsAMatcher(context.RequestContext), fake_server)
 
-    def test_create_share_with_share_network_not_found(self):
+    def test_create_share_instance_with_share_network_not_found(self):
         """Test creation fails if share network not found."""
 
         share = db_utils.create_share(share_network_id='fake-net-id')
         share_id = share['id']
         self.assertRaises(
             exception.ShareNetworkNotFound,
-            self.share_manager.create_share,
+            self.share_manager.create_share_instance,
             self.context,
-            share_id
+            share.instance['id']
         )
         shr = db.share_get(self.context, share_id)
         self.assertEqual(shr['status'], constants.STATUS_ERROR)
 
-    def test_create_share_with_share_network_server_exists(self):
+    def test_create_share_instance_with_share_network_server_exists(self):
         """Test share can be created with existing share server."""
         share_net = db_utils.create_share_network()
         share = db_utils.create_share(share_network_id=share_net['id'])
@@ -583,7 +577,8 @@ class ShareManagerTestCase(test.TestCase):
             share_srv
         )
         self.share_manager.driver = driver_mock
-        self.share_manager.create_share(self.context, share_id)
+        self.share_manager.create_share_instance(self.context,
+                                                 share.instance['id'])
         self.assertFalse(self.share_manager.driver.setup_network.called)
         self.assertEqual(share_id, db.share_get(context.get_admin_context(),
                          share_id).id)
@@ -595,7 +590,7 @@ class ShareManagerTestCase(test.TestCase):
         self.assertEqual(1, len(shr['export_locations']))
 
     @ddt.data('export_location', 'export_locations')
-    def test_create_share_with_error_in_driver(self, details_key):
+    def test_create_share_instance_with_error_in_driver(self, details_key):
         """Test db updates if share creation fails in driver."""
         share = db_utils.create_share()
         share_id = share['id']
@@ -605,15 +600,15 @@ class ShareManagerTestCase(test.TestCase):
         self.share_manager.driver.create_share.side_effect = e
         self.assertRaises(
             exception.ManilaException,
-            self.share_manager.create_share,
+            self.share_manager.create_share_instance,
             self.context,
-            share_id
+            share.instance['id']
         )
         self.assertTrue(self.share_manager.driver.create_share.called)
         shr = db.share_get(self.context, share_id)
         self.assertEqual(some_data, shr['export_location'])
 
-    def test_create_share_with_server_created(self):
+    def test_create_share_instance_with_server_created(self):
         """Test share can be created and share server is created."""
         share_net = db_utils.create_share_network()
         share = db_utils.create_share(share_network_id=share_net['id'])
@@ -630,7 +625,8 @@ class ShareManagerTestCase(test.TestCase):
         self.mock_object(self.share_manager, '_setup_server',
                          mock.Mock(return_value=fake_server))
 
-        self.share_manager.create_share(self.context, share_id)
+        self.share_manager.create_share_instance(self.context,
+                                                 share.instance['id'])
 
         self.assertEqual(share_id, db.share_get(context.get_admin_context(),
                          share_id).id)
@@ -642,7 +638,7 @@ class ShareManagerTestCase(test.TestCase):
         self.share_manager._setup_server.assert_called_once_with(
             utils.IsAMatcher(context.RequestContext), fake_server)
 
-    def test_create_delete_share_error(self):
+    def test_create_delete_share_instance_error(self):
         """Test share can be created and deleted with error."""
 
         def _raise_not_found(self, *args, **kwargs):
@@ -656,16 +652,16 @@ class ShareManagerTestCase(test.TestCase):
         share = db_utils.create_share()
         share_id = share['id']
         self.assertRaises(exception.NotFound,
-                          self.share_manager.create_share,
+                          self.share_manager.create_share_instance,
                           self.context,
-                          share_id)
+                          share.instance['id'])
 
         shr = db.share_get(self.context, share_id)
         self.assertEqual(shr['status'], constants.STATUS_ERROR)
         self.assertRaises(exception.NotFound,
-                          self.share_manager.delete_share,
+                          self.share_manager.delete_share_instance,
                           self.context,
-                          share_id)
+                          share.instance['id'])
 
         shr = db.share_get(self.context, share_id)
         self.assertEqual(shr['status'], constants.STATUS_ERROR_DELETING)
@@ -1002,7 +998,7 @@ class ShareManagerTestCase(test.TestCase):
         self.share_manager.driver.unmanage.\
             assert_called_once_with(mock.ANY)
         self.share_manager._remove_share_access_rules.assert_called_once_with(
-            mock.ANY, mock.ANY, mock.ANY
+            mock.ANY, mock.ANY, mock.ANY, mock.ANY
         )
         self.share_manager.db.share_update.assert_called_once_with(
             mock.ANY, share_id,
@@ -1013,31 +1009,31 @@ class ShareManagerTestCase(test.TestCase):
                          'share_access_get_all_for_share',
                          mock.Mock(return_value=['fake_ref', 'fake_ref2']))
         self.mock_object(self.share_manager, '_deny_access')
-        share_ref = {'id': 'fake_id'}
+        share_ref = db_utils.create_share()
         share_server = 'fake'
 
-        self.share_manager._remove_share_access_rules(self.context,
+        self.share_manager._remove_share_access_rules(
-                                                      share_ref, share_server)
+            self.context, share_ref, share_ref.instance, share_server)
 
         self.share_manager.db.share_access_get_all_for_share.\
             assert_called_once_with(mock.ANY, share_ref['id'])
         self.assertEqual(2, self.share_manager._deny_access.call_count)
 
-    def test_delete_share_share_server_not_found(self):
+    def test_delete_share_instance_share_server_not_found(self):
         share_net = db_utils.create_share_network()
         share = db_utils.create_share(share_network_id=share_net['id'],
                                       share_server_id='fake-id')
 
-        share_id = share['id']
         self.assertRaises(
             exception.ShareServerNotFound,
-            self.share_manager.delete_share,
+            self.share_manager.delete_share_instance,
             self.context,
-            share_id
+            share.instance['id']
         )
 
     @ddt.data(True, False)
-    def test_delete_share_last_on_server_with_sec_services(self, with_details):
+    def test_delete_share_instance_last_on_srv_with_sec_service(
+            self, with_details):
         share_net = db_utils.create_share_network()
         sec_service = db_utils.create_security_service(
             share_network_id=share_net['id'])
@@ -1056,18 +1052,18 @@ class ShareManagerTestCase(test.TestCase):
                 context.get_admin_context(), share_srv['id'], backend_details)
         share = db_utils.create_share(share_network_id=share_net['id'],
                                       share_server_id=share_srv['id'])
-        share_id = share['id']
         self.share_manager.driver = mock.Mock()
         manager.CONF.delete_share_server_with_last_share = True
 
-        self.share_manager.delete_share(self.context, share_id)
+        self.share_manager.delete_share_instance(self.context,
+                                                 share.instance['id'])
 
         self.share_manager.driver.teardown_server.assert_called_once_with(
             server_details=backend_details,
             security_services=[jsonutils.loads(
                 backend_details['security_service_ldap'])])
 
-    def test_delete_share_last_on_server(self):
+    def test_delete_share_instance_last_on_server(self):
         share_net = db_utils.create_share_network()
         share_srv = db_utils.create_share_server(
             share_network_id=share_net['id'],
@@ -1076,16 +1072,15 @@ class ShareManagerTestCase(test.TestCase):
         share = db_utils.create_share(share_network_id=share_net['id'],
                                       share_server_id=share_srv['id'])
 
-        share_id = share['id']
-
         self.share_manager.driver = mock.Mock()
         manager.CONF.delete_share_server_with_last_share = True
-        self.share_manager.delete_share(self.context, share_id)
+        self.share_manager.delete_share_instance(self.context,
+                                                 share.instance['id'])
         self.share_manager.driver.teardown_server.assert_called_once_with(
             server_details=share_srv.get('backend_details'),
             security_services=[])
 
-    def test_delete_share_last_on_server_deletion_disabled(self):
+    def test_delete_share_instance_last_on_server_deletion_disabled(self):
         share_net = db_utils.create_share_network()
         share_srv = db_utils.create_share_server(
             share_network_id=share_net['id'],
@@ -1094,13 +1089,13 @@ class ShareManagerTestCase(test.TestCase):
         share = db_utils.create_share(share_network_id=share_net['id'],
                                       share_server_id=share_srv['id'])
 
-        share_id = share['id']
         manager.CONF.delete_share_server_with_last_share = False
         self.share_manager.driver = mock.Mock()
-        self.share_manager.delete_share(self.context, share_id)
+        self.share_manager.delete_share_instance(self.context,
+                                                 share.instance['id'])
         self.assertFalse(self.share_manager.driver.teardown_network.called)
 
-    def test_delete_share_not_last_on_server(self):
+    def test_delete_share_instance_not_last_on_server(self):
         share_net = db_utils.create_share_network()
         share_srv = db_utils.create_share_server(
             share_network_id=share_net['id'],
@@ -1110,11 +1105,11 @@ class ShareManagerTestCase(test.TestCase):
                                       share_server_id=share_srv['id'])
         db_utils.create_share(share_network_id=share_net['id'],
                               share_server_id=share_srv['id'])
-        share_id = share['id']
 
         manager.CONF.delete_share_server_with_last_share = True
         self.share_manager.driver = mock.Mock()
-        self.share_manager.delete_share(self.context, share_id)
+        self.share_manager.delete_share_instance(self.context,
+                                                 share.instance['id'])
         self.assertFalse(self.share_manager.driver.teardown_network.called)
 
     def test_allow_deny_access(self):
@@ -1123,14 +1118,12 @@ class ShareManagerTestCase(test.TestCase):
         share_id = share['id']
         access = db_utils.create_access(share_id=share_id)
         access_id = access['id']
-        self.share_manager.allow_access(self.context, access_id)
+        self.share_manager.allow_access(self.context, share.instance['id'],
+                                        access_id)
         self.assertEqual('active', db.share_access_get(self.context,
                                                        access_id).state)
 
-        self.share_manager.deny_access(self.context, access_id)
+        self.share_manager.deny_access(self.context, share.instance['id'],
-        self.assertRaises(exception.NotFound,
-                          db.share_access_get,
-                          self.context,
                                        access_id)
 
     def test_allow_deny_access_error(self):
@@ -1155,6 +1148,7 @@ class ShareManagerTestCase(test.TestCase):
         self.assertRaises(exception.NotFound,
                           self.share_manager.allow_access,
                           self.context,
+                          share.instance['id'],
                           access_id)
 
         acs = db.share_access_get(self.context, access_id)
@@ -1163,6 +1157,7 @@ class ShareManagerTestCase(test.TestCase):
         self.assertRaises(exception.NotFound,
                           self.share_manager.deny_access,
                           self.context,
+                          share.instance['id'],
                           access_id)
 
         acs = db.share_access_get(self.context, access_id)
@@ -1400,14 +1395,14 @@ class ShareManagerTestCase(test.TestCase):
             self.context, share_server['id']
         )
 
-    def test_ensure_share_has_pool_with_only_host(self):
+    def test_ensure_share_instance_has_pool_with_only_host(self):
         fake_share = {
             'status': constants.STATUS_AVAILABLE, 'host': 'host1', 'id': 1}
         host = self.share_manager._ensure_share_instance_has_pool(
             context.get_admin_context(), fake_share)
         self.assertIsNone(host)
 
-    def test_ensure_share_has_pool_with_full_pool_name(self):
+    def test_ensure_share_instance_has_pool_with_full_pool_name(self):
         fake_share = {'host': 'host1#pool0', 'id': 1,
                       'status': constants.STATUS_AVAILABLE}
         fake_share_expected_value = 'pool0'
@@ -1415,7 +1410,7 @@ class ShareManagerTestCase(test.TestCase):
             context.get_admin_context(), fake_share)
         self.assertEqual(fake_share_expected_value, host)
 
-    def test_ensure_share_has_pool_unable_to_fetch_share(self):
+    def test_ensure_share_instance_has_pool_unable_to_fetch_share(self):
         fake_share = {'host': 'host@backend', 'id': 1,
                       'status': constants.STATUS_AVAILABLE}
         with mock.patch.object(self.share_manager.driver, 'get_pool',

manila/tests/share/test_rpcapi.py

@@ -65,11 +65,13 @@ class ShareRpcAPITestCase(test.TestCase):
             share = expected_msg['share']
             del expected_msg['share']
             expected_msg['share_id'] = share['id']
+        if 'share_instance' in expected_msg:
+            share_instance = expected_msg.pop('share_instance', None)
+            expected_msg['share_instance_id'] = share_instance['id']
         if 'access' in expected_msg:
             access = expected_msg['access']
             del expected_msg['access']
             expected_msg['access_id'] = access['id']
-            del expected_msg['share_id']
         if 'host' in expected_msg:
             del expected_msg['host']
         if 'snapshot' in expected_msg:
@@ -81,6 +83,8 @@ class ShareRpcAPITestCase(test.TestCase):
             host = kwargs['host']
         elif 'share_server' in kwargs:
             host = kwargs['share_server']['host']
+        elif 'share_instance' in kwargs:
+            host = kwargs['share_instance']['host']
         else:
             host = kwargs['share']['host']
         target['server'] = host
@@ -113,30 +117,34 @@ class ShareRpcAPITestCase(test.TestCase):
         for kwarg, value in six.iteritems(self.fake_kwargs):
             self.assertEqual(value, expected_msg[kwarg])
 
-    def test_create_share(self):
+    def test_create_share_instance(self):
-        self._test_share_api('create_share',
+        self._test_share_api('create_share_instance',
                              rpc_method='cast',
-                             share=self.fake_share,
+                             version='1.4',
+                             share_instance=self.fake_share,
                              host='fake_host1',
                              snapshot_id='fake_snapshot_id',
                              filter_properties=None,
                              request_spec=None)
 
-    def test_delete_share(self):
+    def test_delete_share_instance(self):
-        self._test_share_api('delete_share',
+        self._test_share_api('delete_share_instance',
                              rpc_method='cast',
-                             share=self.fake_share)
+                             version='1.4',
+                             share_instance=self.fake_share)
 
     def test_allow_access(self):
         self._test_share_api('allow_access',
                              rpc_method='cast',
-                             share=self.fake_share,
+                             version='1.4',
+                             share_instance=self.fake_share,
                              access=self.fake_access)
 
     def test_deny_access(self):
         self._test_share_api('deny_access',
                              rpc_method='cast',
-                             share=self.fake_share,
+                             version='1.4',
+                             share_instance=self.fake_share,
                              access=self.fake_access)
 
     def test_create_snapshot(self):