Merge lp:~cbehrens/nova/servers-search into lp:~hudson-openstack/nova/trunk

[Chris Behrens]

This adds the servers search capabilities defined in the OS API v1.1 spec.. and more for admins.

For users, flavor=, image=, status=, and name= can be specified. name= supports regular expression matching.
Most other options are ignored. (things outside of the spec like 'recurse_zones' and 'reservation_id' still work, also)

If admin_api is enabled and context is an admin: along with the above, one can specify ip= and ip6= which will do regular expression matching. Also, any other 'Instance' column name can be specified, so you can do regexp matching there as well. Unknown Instance columns are ignored.

Also fixes up fixed_ip=, making a 404 returned vs a 500 error... and handling this properly with zone recursion as well.

[Brian Lamar (blamar)]

This seems like a pretty inefficient way to do filtering if all we're looking for is wildcard matching. Why return everything when SQLAlchemy should successfully provide filtering across multiple backends? Can this be done using something like...

query.filter(Instance.name.like('%test%'))

...rather than returning each row and doing a regex?

review: Needs Fixing

[Jay Pipes (jaypipes)]

Hi Chris!

Few little comments...

190 + # See if a valid search option was passed in.
191 + # Ignore unknown search options for possible forward compatability.
192 + # Raise an exception if more than 1 search option is specified
193 + found_opt = None
194 + for opt in exclusive_opts:
195 + v = search_opts.get(opt, None)
196 + if v:
197 + if found_opt is None:
198 + found_opt = opt
199 + else:
200 + LOG.error(_("More than 1 mutually exclusive "
201 + "search option specified (%(found_opt)s and "
202 + "%(opt)s were both specified") % locals())
203 + raise exception.InvalidInput(reason=_(
204 + "More than 1 mutually exclusive "
205 + "search option specified (%(found_opt)s and "
206 + "%(opt)s were both specified") % locals())

Might be more succinctly written as:

found_opt = None
found_opts = [opt for opt in exclusive_opts
              if search_opts.get(opt, None)]
if len(found_opts) > 1:
    found_opt_str = ", ".join(found_opts)
    msg = _("More than 1 mutually exclusive "
            "search option specified: %(found_opt_str)s")
            % locals())
    logger.error(msg)
    raise exception.InvalidInput(reason=msg)

if found_opts:
    found_opt = found_opts[0]

I recognize (and agree with your decision) not to do regexp matching via the database. Not only is it not portable, it's not any more efficient to do that at the database level (still requires a scan of all pre-restricted rows anyway...).

However this method signature:

+def instance_get_all_by_name_regexp(context, ipv6_regexp):

Looked strange, perhaps a copy/paste error? Should ipv6_regexp be name_regexp?

Cheers!
jay

review: Needs Fixing

[Brian Lamar (blamar)]

> I recognize (and agree with your decision) not to do regexp matching via the database. Not only is
> it not portable, it's not any more efficient to do that at the database level (still requires a
> scan of all pre-restricted rows anyway...).

Regular expressions are more expensive than LIKE matches (which in their own right, are pretty expensive). Do we really want operators doing complex regexs? At that point we should be putting our data into a purpose-built search indexing solution like Lucene/Solr/ElasticSearch/Sphinx because that's what they're good at.

[Jay Pipes (jaypipes)]

> > I recognize (and agree with your decision) not to do regexp matching via the
> database. Not only is
> > it not portable, it's not any more efficient to do that at the database
> level (still requires a
> > scan of all pre-restricted rows anyway...).
>
> Regular expressions are more expensive than LIKE matches (which in their own
> right, are pretty expensive).

Actually, this is incorrect. LIKE '%something%' and column REGEXP 'someregexp' will produce identical query execution plans. The complexity of the REGEXP determines whether or not a simple string match such as '%something%' would be computationally more expensive to execute per row than a compiled regexp match.

> Do we really want operators doing complex
> regexs? At that point we should be putting our data into a purpose-built
> search indexing solution like Lucene/Solr/ElasticSearch/Sphinx because that's
> what they're good at.

Lucene/Solar/ElasticSearch/Sphinx are fulltext indexing technologies. What's happening here is looking for a particular pattern in a short string. The solution presented here is flexible enough to query for various IP(v6) and name patterns without having to set up a separate fulltext indexing server for this kind of thing, which I think would be overboard.

I understand your concern about the regexp inefficiency. Just saying that it's not that much less efficient than doing a REGEXP or LIKE '%something%' expression in SQL. The same loop and match process is occurring in Python code versus C code. The problem is that not all DBs support the REGEXP operator...

Just my two cents,
-jay

[Chris Behrens (cbehrens)]

Brian:

Yeah, I'm aware of everything in your first comment. I started out using 'like', but I ditched it for a few reasons.

1) Some of the things I'm matching against are not stored in the database. IPv6 addresses and the instance 'name', in particular. So, I'd have to build search functions in python that can parse '%..%'. That.. or I'd end up supporting regular expressions for some queries and the sql 'like' format for others.
2) Related to #1 in a way, what if the backend ends up not being SQL at all?
3) 'like' does a full table scan anyway and regular expressions are a little more flexible.

That said, using 'like' is going to be more efficient because I think it'll result in less 'join's. But, really, there's way too many joins in all of the previously existing instance getting code anyway. I duplicated what was used previously, but I don't think all of them are necessary. Someone really needs to go through and do an audit on all of these DB calls and find a way to not have to join stuff that isn't going to be used by the caller. I'd rather that I not have to be done as a part of this merge... as that'll be a huge task. :)

[Chris Behrens (cbehrens)]

Jay: Ah, thanks! I'll get those fixed up.

[Brian Waldon (bcwaldon)]

Is it necessary to have a separate db api method for each query string parameter? This seems like it could cause unnecessary growth in that module.

This doesn't seem to match the latest v1.1 spec. Is there a reason for the divergence?

review: Needs Information

[Chris Behrens (cbehrens)]

> Is it necessary to have a separate db api method for each query string
> parameter? This seems like it could cause unnecessary growth in that module.
>
> This doesn't seem to match the latest v1.1 spec. Is there a reason for the
> divergence?

Will check the spec.

As far as the separate db api methods... just to clarify... did you mean in db/api or actually in compute/api? Or both? Mostly everything can use the search "by_column" method I added. That tries to generically support columns by doing a 'getattr' on the Instance class for the column value to match. But, searching by 'ip', 'ipv6', and 'name' had to be separate because:

1) 'ip' matches against the FixedIp and FloatingIP tables, instead..
2) 'ipv6' isn't stored in the database
3) 'name' isn't stored in the database either... it's a @property of the class and a getattr() doesn't return the value directly... it returns a Property instance IIRC.

[Brian Lamar (blamar)]

> > > I recognize (and agree with your decision) not to do regexp matching via
> the
> > database. Not only is
> > > it not portable, it's not any more efficient to do that at the database
> > level (still requires a
> > > scan of all pre-restricted rows anyway...).
> >
> > Regular expressions are more expensive than LIKE matches (which in their own
> > right, are pretty expensive).
>
> Actually, this is incorrect. LIKE '%something%' and column REGEXP 'someregexp'
> will produce identical query execution plans. The complexity of the REGEXP
> determines whether or not a simple string match such as '%something%' would be
> computationally more expensive to execute per row than a compiled regexp
> match.
>
> > Do we really want operators doing complex
> > regexs? At that point we should be putting our data into a purpose-built
> > search indexing solution like Lucene/Solr/ElasticSearch/Sphinx because
> that's
> > what they're good at.
>
> Lucene/Solar/ElasticSearch/Sphinx are fulltext indexing technologies. What's
> happening here is looking for a particular pattern in a short string. The
> solution presented here is flexible enough to query for various IP(v6) and
> name patterns without having to set up a separate fulltext indexing server for
> this kind of thing, which I think would be overboard.
>
> I understand your concern about the regexp inefficiency. Just saying that it's
> not that much less efficient than doing a REGEXP or LIKE '%something%'
> expression in SQL. The same loop and match process is occurring in Python code
> versus C code. The problem is that not all DBs support the REGEXP operator...
>
> Just my two cents,
> -jay

Wow, just gonna have to agree to disagree then... I'm really positive it's going to be tons more efficient to do simple wildcard matching vs. fully-fledged perl-compatible regular expressions. I'm all about not putting specialized code into projects and I see this as being search-specialized code. I'm going to bow out now because I don't want to further this discussion on a merge-prop (sorry Chris!!).

> 1) Some of the things I'm matching against are not stored in the database. IPv6 addresses and the
> instance 'name', in particular. So, I'd have to build search functions in python that can parse
> '%..%'. That.. or I'd end up supporting regular expressions for some queries and the sql 'like'
> format for others.

Yeah, I can't believe that v6 address aren't stored in the DB. :) Good point.

> 2) Related to #1 in a way, what if the backend ends up not being SQL at all?

Heh, good luck with this one, but the point I'm trying to make is compatible with this as well. I use ElasticSearch with some of my CouchDB projects and there are tons of indexers for all types of datastores.

> 3) 'like' does a full table scan anyway and regular expressions are a little more flexible.

Absolutely, I just feel that we should be leaving the database to do the data-storage, a search-indexer to do searching, and Python to tie everything together.

Going to abstain from this merge prop, my apologies for cluttering it with conversations that most assuredly should have happened at the Blueprint level.

review: Abstain

[Chris Behrens (cbehrens)]

blamar: No problem, I understand. And FWIW, it's not 'pcre'.. it's just the python 're' module.. only slightly better performance-wise, probably. :)

[Chris Behrens (cbehrens)]

Ok. Per discussion with bcwaldon, I'm going to make the "name" query string option map to searching by display name. The spec sys name=serverName, and I got clarification that serverName means what the user sees... which is the Instance.display_name.

The spec doesn't mention 'instance_name' (Instance.name property in nova), but admins will want that. So, I'll make "instance_name" option map to what "name" did, which was search by this instance name. instance_name will have admin_api and admin context checking.

I'll remove searching by 'server_name='. It's always NULL in my tables, anyway, and it's not clear what this is really supposed to be used for (I haven't looked in the code, honestly). It's not mentioned in the spec, and it doesn't seem to be an immediate need... so...

[Chris Behrens (cbehrens)]

After reviewing section 4.1.1 in the v1.1 API spec regarding 'list', I went ahead and added some of the other search options like 'image', 'flavor', and status... along with making the previous changes that were discussed. Ie:

name matches display_name
instance_name is admin API/admin context only
I also made ip and ip6 admin-only due to it not being in the spec.
Unit tests have been added for OS API

[Brian Waldon (bcwaldon)]

A branch recently went in that replaced all returns of fault-wrapped webob exceptions with raising of non-wrapped webob exceptions. Can you convert any of the "return faults.Fault(...)" to "raise ..."?

I think we may want to take the approach of ignoring unsupported query params rather than raising an exception (that's how Glance does it). We should be liberal in what we accept. A lot of the _get_items / _items code can get cleaned back up if we don't need to explicitly check filter params. Same goes for check_option_permissions.

Power states belong to the compute layer, and server statuses belong to the OSAPI. I think we should remove any OSAPI status-related code from the power_states module and place it in either api.openstack.common or api.openstack.servers. What do you think?

What is the 'fresh' query filter? I just started seeing that in novaclient requests.

Are there future plans to implement changes-since? I can see it is an accepted filter, just not respected.

In the DB API, why is there a _get_all_by_column_regexp and a _get_all_by_instance_name? Can't we use get_all_by_column_regexp to accomplish the name filter?

Can you change _get_all_by_column to _get_all_by_column_value? The former doesn't clearly say you are checking the value in a specific column when there is another function with column_regexp.

I don't see it necessary to list OSAPI-specific filters in the compute api. Can we fix that known_params?

I really want to be able to sort by multiple parameters, and I think most people using the API would expect to be able to do that. How hard would it be to support this?

Don't get me wrong here, I'm really excited to get this feature in. Having done this in Glance already, I know this isn't a small undertaking :)

review: Needs Fixing

[Chris Behrens (cbehrens)]

> A branch recently went in that replaced all returns of fault-wrapped webob
> exceptions with raising of non-wrapped webob exceptions. Can you convert any
> of the "return faults.Fault(...)" to "raise ..."?

Aha.. that explains something. :) Sure.

> I think we may want to take the approach of ignoring unsupported query params
> rather than raising an exception (that's how Glance does it). We should be
> liberal in what we accept. A lot of the _get_items / _items code can get
> cleaned back up if we don't need to explicitly check filter params. Same goes
> for check_option_permissions.

Ok. I was thinking about v1.1 and trying to 'guess' at what I should do here based on the possible response codes to the query. I agree with what you say, though. I'd rather allow and ignore search options we don't understand. That will clean things up slightly. I still need to check for permissions on certain search items that should be admin-api only, but I can cut the 2 lists of options down to 1.. (list of options that require admin)

>
> Power states belong to the compute layer, and server statuses belong to the
> OSAPI. I think we should remove any OSAPI status-related code from the
> power_states module and place it in either api.openstack.common or
> api.openstack.servers. What do you think?

I had the same debate. I was going to move it to api.openstack.common.. until I realized that compute.api needs to send the 'status' to search by to child zones. This means that compute.api would end up making a call into api.openstack.common to convert a power state to the status to search for.. and that didn't feel quite right. I think I can solve that differently. I can make the OS API pass the 'status' to search for through to compute.api (along with 'state' which is the translation of status to power state). compute.api will ignore 'status' and use 'state' but it'll pass 'status' on to novaclient to talk to zones.

That or I could leave it the way it is. :) Changing it probably makes sense so there's clear separation.

>
> What is the 'fresh' query filter? I just started seeing that in novaclient
> requests.

Honestly, I have no clue. :-/ But I know that novaclient sends it. I figured it was v1.0 equiv of 'changes-since' that's in v1.1.

>
> Are there future plans to implement changes-since? I can see it is an accepted
> filter, just not respected.

Yeah, good question. I'm already going beyond the scope of the story that was given to us for the sprint, and that one seemed a little more difficult. Wasn't sure if compute.api should filter that.. or let OS API filter on the return to the client. I thought maybe titan would have a better idea of how that should be done. ;)

>
> In the DB API, why is there a _get_all_by_column_regexp and a
> _get_all_by_instance_name? Can't we use get_all_by_column_regexp to accomplish
> the name filter?

Let me double check that. I saw a problem early on with this, because 'name' is a @property in the Instance class and I seemed to be getting back a property object with getattr. I bet I was doing testing with a class and not an instance of a class at the time, however. I bet this can be consolidated.

> ...

[Brian Waldon (bcwaldon)]

> > Power states belong to the compute layer, and server statuses belong to the
> > OSAPI. I think we should remove any OSAPI status-related code from the
> > power_states module and place it in either api.openstack.common or
> > api.openstack.servers. What do you think?
>
> I had the same debate. I was going to move it to api.openstack.common.. until
> I realized that compute.api needs to send the 'status' to search by to child
> zones. This means that compute.api would end up making a call into
> api.openstack.common to convert a power state to the status to search for..
> and that didn't feel quite right. I think I can solve that differently. I
> can make the OS API pass the 'status' to search for through to compute.api
> (along with 'state' which is the translation of status to power state).
> compute.api will ignore 'status' and use 'state' but it'll pass 'status' on to
> novaclient to talk to zones.
>
> That or I could leave it the way it is. :) Changing it probably makes sense
> so there's clear separation.

Yeah, I really don't want the compute api (the power states module) to depend on concepts defined in the openstack api.

> > Are there future plans to implement changes-since? I can see it is an
> accepted
> > filter, just not respected.
>
> Yeah, good question. I'm already going beyond the scope of the story that was
> given to us for the sprint, and that one seemed a little more difficult.
> Wasn't sure if compute.api should filter that.. or let OS API filter on the
> return to the client. I thought maybe titan would have a better idea of how
> that should be done. ;)

We got it :)

> > I don't see it necessary to list OSAPI-specific filters in the compute api.
> > Can we fix that known_params?
>
> Well, I need a table to map the search parameter to the function to call with
> its arguments. In the initial merge prop, I didn't have this as I used
> getattr(self, '_get_all_by_%s' % opt_name).. and called that function. The
> problem is that the generic 'column' ones need an extra argument. The
> original merge prop had a list of those search options that could be done
> generically. I could change it back... but it'd also require moving all of
> the _get_all_by* calls back outside of 'def get_all' so I could use
> getattr(self, ..) again.
>
> Hm.. did that make sense? We can chat on IRC to clarify if needed.

What bothers me is the inclusion of non-filter query params (like changes-since). If those would just go away I would be more okay with it.

> > I really want to be able to sort by multiple parameters, and I think most
> > people using the API would expect to be able to do that. How hard would it
> be
> > to support this?
>
> Yeah, I hear you. The story I was given says to throw an error if multiple
> are given, so that's the main reason why I didn't code support for it. :) My
> instinct is to say it's rather difficult... but when I really start to think
> about it, I'm not sure it's so bad. I can think about it some more while
> doing the other fixups.

I don't mean to toot my own horn here, but look at glance and see if that approach would work.

[Chris Behrens (cbehrens)]

Alright. This is almost a re-write since the original merge prop, now. I updated the description up at the top to better reflect what this ends up adding.

Fire away.. :)

[Ed Leafe (ed-leafe)]

Only got through the first 700 lines of the diff so far, but here's what I have:

Lines 134 -143 can be more efficiently written:

def power_states_from_status(status):
    """Map the server status string to a list of power states"""
    low_status = status.strip().lower()
    power_states = [val for key, val in _STATUS_MAP.iteritems()
      if key.lower() == low_status
      and val is not None]
    return power_states
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

The method '_check_servers_options()' would be better named something like '_remove_invalid_options()' so that the fact that it modifies search_opts is more clearly expressed.
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

Line 643: What is this supposed to be doing? Did you maybe mean to copy the 'filters' dict to another name?
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

Lines 660 -664 can be more efficiently written:

for filter_name in query_filters:
    query_prefix = _exact_match_filter(query_prefix, filter_name,
            filters.pop(filter_name))

[Chris Behrens (cbehrens)]

Ed:

Ok re: _check_servers_options().

643: I meant to make a copy of it to use going forward.. I just reassigned it to same variable name. I'm making a copy because I'm modifying it and I can't have it affect the caller.

660-664: Good call... makes sense.

[Chris Behrens (cbehrens)]

Updated. Added a comment around #643 to clarify the .copy().

[Brian Waldon (bcwaldon)]

This looks great, Mr. Behrens. I like the db-api implementation a lot more. I've got just a few comments:

I noticed you linked a lazy load bug to this. Can you explain how this branch addresses that?

164: Instead of using an instance variable, can you implement this with constant tuples that are passed into _remove_invalid_options as an 'allowed_search_options' argument?

217: I'm not crazy about this method name. How about just _get_servers?
215: This is only called once, so does it make sense to break it out into its own method? I think it all fits under _servers_from_request (or whatever its new name might be ;)

392: There's a pattern here that is repeated a few times. Maybe you can create a mapping dictionary for converting key names

419: This behavior will also happen through OSAPI, right? I'm not so sure we want to support special cases like this. It's a totally valid request to filter by fixed_ip and name where no results are returned.

480: This behavior seems odd. I'm thinking we should just return an empty list. What do you think?

[Chris Behrens (cbehrens)]

lazy load: The fix is actually described in the thread with the bug, but: OS API accesses Instance['virtual_interfaces']['network'], but instance_get_all() does not joinedload('virtual_interfaces.network'). When the sqlalchemy session becomes detached, OS API cannot do the query for 'network' when it's accessed. My new DB API routine joins it. I didn't add it to any other queries, because the OS API may be the only place where it's needed.

164: Sure

217: Ok.

215: I'd originally broken it out because I split the caller into the 1.0/1.1 Controllers, so it had 2 references. I'll merge it back in, since that's not the case anymore.

392: I didn't use a mapping table because the flipping needs to happen in a certain order (since
i'm renaming name -> display_name, but instance_name to name. Doing those in reverse order would clobber the 'name' if both exist in the query). But I know how I can solve that.

419: Yeah, I did this because I'm not sure 'fixed_ip' should be used via OS API. One should use 'ip' instead. If it's for EC2 only, fixed_ip this way is a lot more efficient query since it looks up via FixedIps table directly with no joins to find the entry. Decisions like these are difficult, as I'm not entirely sure what's most desirable to the community...but generally there is some sort of reason to the method of my madness. :) I'm cool with changing it.

480: I feel this way, too.. I was just trying to match the original behavior in single zone. I guess if EC2 should really return an error in this case, the error return should happen in the ec2 controller if it gets an empty list back.

[Chris Behrens (cbehrens)]

Ok, I think all of those issues are addressed.

[Brian Waldon (bcwaldon)]

Thanks for addressing those issues. Code looks good to me, now, but I'm seeing one test failure:

======================================================================
FAIL: test_get_servers_allows_status_v1_1 (nova.tests.api.openstack.test_servers.ServersTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/brian.waldon/valhalla/nova/servers-search/nova/tests/api/openstack/test_servers.py", line 1184, in test_get_servers_allows_status_v1_1
    self.assertEqual(res.status_int, 200)
AssertionError: 500 != 200
-------------------- >> begin captured logging << --------------------

...

(nova.api.openstack): TRACE: File "/Users/brian.waldon/valhalla/nova/servers-search/nova/api/openstack/servers.py", line 56, in index
(nova.api.openstack): TRACE: servers = self._get_servers(req, is_detail=False)
(nova.api.openstack): TRACE: File "/Users/brian.waldon/valhalla/nova/servers-search/nova/api/openstack/servers.py", line 126, in _get_servers
(nova.api.openstack): TRACE: context, search_opts=search_opts)
(nova.api.openstack): TRACE: File "/Users/brian.waldon/valhalla/nova/servers-search/nova/tests/api/openstack/test_servers.py", line 1174, in fake_get_all
(nova.api.openstack): TRACE: [power_state.RUNNING, power_state.BLOCKED])
(nova.api.openstack): TRACE: File "/System/Library/Frameworks/Python.framework/Versions/2.6/lib/python2.6/unittest.py", line 350, in failUnlessEqual
(nova.api.openstack): TRACE: (msg or '%r != %r' % (first, second))
(nova.api.openstack): TRACE: AssertionError: [2, 1] != [1, 2]
(nova.api.openstack): TRACE:
--------------------- >> end captured logging << ---------------------

----------------------------------------------------------------------

[Rick Harris (rconradharris)]

Tests pass (for me), and code looks good. Really nice work here Chris.

review: Approve

[Chris Behrens (cbehrens)]

bcwaldon: Issue is this:

[15:51:38] <comstud> 1173 self.assertEqual(search_opts['state'],
[15:51:39] <comstud> 1174 [power_state.RUNNING,
                     power_state.BLOCKED])

The list is in reverse order in your test and I'm assuming order here. I've just wrapped those lists with set() so it does a set comparison. Try now. :)

[Brian Waldon (bcwaldon)]

Yep, all good now. Thank you, Dr. Behrens!

review: Approve

[OpenStack Infra (hudson-openstack)]

Attempt to merge into lp:nova failed due to conflicts:

text conflict in nova/api/ec2/cloud.py
text conflict in nova/tests/test_metadata.py

[Chris Behrens (cbehrens)]

Merged trunk and resolved 2 conflicts. Need the approves again.

=== modified file 'nova/api/ec2/cloud.py'
--- nova/api/ec2/cloud.py	2011-08-09 12:47:47 +0000
+++ nova/api/ec2/cloud.py	2011-08-09 20:27:18 +0000
@@ -213,8 +213,9 @@
 
     def _get_mpi_data(self, context, project_id):
         result = {}
+        search_opts = {'project_id': project_id}
         for instance in self.compute_api.get_all(context,
-                                                 project_id=project_id):
+                search_opts=search_opts):
             if instance['fixed_ips']:
                 line = '%s slots=%d' % (instance['fixed_ips'][0]['address'],
                                         instance['vcpus'])
@@ -264,8 +265,13 @@
 
     def get_metadata(self, address):
         ctxt = context.get_admin_context()
-        instance_ref = self.compute_api.get_all(ctxt, fixed_ip=address)
-        if instance_ref is None:
+        search_opts = {'fixed_ip': address}
+        try:
+            instance_ref = self.compute_api.get_all(ctxt,
+                    search_opts=search_opts)
+        except exception.NotFound:
+            instance_ref = None
+        if not instance_ref:
             return None
 
         # This ensures that all attributes of the instance
@@ -1086,11 +1092,16 @@
         return result
 
     def describe_instances(self, context, **kwargs):
-        return self._format_describe_instances(context, **kwargs)
+        # Optional DescribeInstances argument
+        instance_id = kwargs.get('instance_id', None)
+        return self._format_describe_instances(context,
+                instance_id=instance_id)
 
     def describe_instances_v6(self, context, **kwargs):
-        kwargs['use_v6'] = True
-        return self._format_describe_instances(context, **kwargs)
+        # Optional DescribeInstancesV6 argument
+        instance_id = kwargs.get('instance_id', None)
+        return self._format_describe_instances(context,
+                instance_id=instance_id, use_v6=True)
 
     def _format_describe_instances(self, context, **kwargs):
         return {'reservationSet': self._format_instances(context, **kwargs)}
@@ -1152,7 +1163,8 @@
         result['groupSet'] = CloudController._convert_to_set(
             security_group_names, 'groupId')
 
-    def _format_instances(self, context, instance_id=None, **kwargs):
+    def _format_instances(self, context, instance_id=None, use_v6=False,
+            **search_opts):
         # TODO(termie): this method is poorly named as its name does not imply
         #               that it will be making a variety of database calls
         #               rather than simply formatting a bunch of instances that
@@ -1163,11 +1175,17 @@
             instances = []
             for ec2_id in instance_id:
                 internal_id = ec2utils.ec2_id_to_id(ec2_id)
-                instance = self.compute_api.get(context,
-                                                instance_id=internal_id)
+                try:
+                    instance = self.compute_api.get(context, internal_id)
+                except exception.NotFound:
+                    continue
                 instances.append(instance)
         else:
-            instances = self.compute_api.get_all(context, **kwargs)
+            try:
+                instances = self.compute_api.get_all(context,
+                        search_opts=search_opts)
+            except exception.NotFound:
+                instances = []
         for instance in instances:
             if not context.is_admin:
                 if instance['image_ref'] == str(FLAGS.vpn_image_id):
@@ -1189,7 +1207,7 @@
                 fixed_addr = fixed['address']
                 if fixed['floating_ips']:
                     floating_addr = fixed['floating_ips'][0]['address']
-                if fixed['network'] and 'use_v6' in kwargs:
+                if fixed['network'] and use_v6:
                     i['dnsNameV6'] = ipv6.to_global(
                         fixed['network']['cidr_v6'],
                         fixed['virtual_interface']['address'],
@@ -1326,7 +1344,7 @@
                                   'AvailabilityZone'),
             block_device_mapping=kwargs.get('block_device_mapping', {}))
         return self._format_run_instances(context,
-                                          instances[0]['reservation_id'])
+                reservation_id=instances[0]['reservation_id'])
 
     def _do_instance(self, action, context, ec2_id):
         instance_id = ec2utils.ec2_id_to_id(ec2_id)

=== modified file 'nova/api/openstack/common.py'
--- nova/api/openstack/common.py	2011-08-09 16:33:13 +0000
+++ nova/api/openstack/common.py	2011-08-09 20:27:18 +0000
@@ -27,6 +27,7 @@
 from nova import log as logging
 from nova import quota
 from nova.api.openstack import wsgi
+from nova.compute import power_state as compute_power_state
 
 
 LOG = logging.getLogger('nova.api.openstack.common')
@@ -37,6 +38,38 @@
 XML_NS_V11 = 'http://docs.openstack.org/compute/api/v1.1'
 
 
+_STATUS_MAP = {
+    None: 'BUILD',
+    compute_power_state.NOSTATE: 'BUILD',
+    compute_power_state.RUNNING: 'ACTIVE',
+    compute_power_state.BLOCKED: 'ACTIVE',
+    compute_power_state.SUSPENDED: 'SUSPENDED',
+    compute_power_state.PAUSED: 'PAUSED',
+    compute_power_state.SHUTDOWN: 'SHUTDOWN',
+    compute_power_state.SHUTOFF: 'SHUTOFF',
+    compute_power_state.CRASHED: 'ERROR',
+    compute_power_state.FAILED: 'ERROR',
+    compute_power_state.BUILDING: 'BUILD',
+}
+
+
+def status_from_power_state(power_state):
+    """Map the power state to the server status string"""
+    return _STATUS_MAP[power_state]
+
+
+def power_states_from_status(status):
+    """Map the server status string to a list of power states"""
+    power_states = []
+    for power_state, status_map in _STATUS_MAP.iteritems():
+        # Skip the 'None' state
+        if power_state is None:
+            continue
+        if status.lower() == status_map.lower():
+            power_states.append(power_state)
+    return power_states
+
+
 def get_pagination_params(request):
     """Return marker, limit tuple from request.
 

=== modified file 'nova/api/openstack/servers.py'
--- nova/api/openstack/servers.py	2011-08-09 16:33:13 +0000
+++ nova/api/openstack/servers.py	2011-08-09 20:27:18 +0000
@@ -44,7 +44,7 @@
 
 
 class Controller(object):
-    """ The Server API controller for the OpenStack API """
+    """ The Server API base controller class for the OpenStack API """
 
     def __init__(self):
         self.compute_api = compute.API()
@@ -53,17 +53,21 @@
     def index(self, req):
         """ Returns a list of server names and ids for a given user """
         try:
-            servers = self._items(req, is_detail=False)
+            servers = self._get_servers(req, is_detail=False)
         except exception.Invalid as err:
             return exc.HTTPBadRequest(explanation=str(err))
+        except exception.NotFound:
+            return exc.HTTPNotFound()
         return servers
 
     def detail(self, req):
         """ Returns a list of server details for a given user """
         try:
-            servers = self._items(req, is_detail=True)
+            servers = self._get_servers(req, is_detail=True)
         except exception.Invalid as err:
             return exc.HTTPBadRequest(explanation=str(err))
+        except exception.NotFound as err:
+            return exc.HTTPNotFound()
         return servers
 
     def _build_view(self, req, instance, is_detail=False):
@@ -75,22 +79,55 @@
     def _action_rebuild(self, info, request, instance_id):
         raise NotImplementedError()
 
-    def _items(self, req, is_detail):
-        """Returns a list of servers for a given user.
-
-        builder - the response model builder
+    def _get_servers(self, req, is_detail):
+        """Returns a list of servers, taking into account any search
+        options specified.
         """
-        query_str = req.str_GET
-        reservation_id = query_str.get('reservation_id')
-        project_id = query_str.get('project_id')
-        fixed_ip = query_str.get('fixed_ip')
-        recurse_zones = utils.bool_from_str(query_str.get('recurse_zones'))
+
+        search_opts = {}
+        search_opts.update(req.str_GET)
+
+        context = req.environ['nova.context']
+        remove_invalid_options(context, search_opts,
+                self._get_server_search_options())
+
+        # Convert recurse_zones into a boolean
+        search_opts['recurse_zones'] = utils.bool_from_str(
+                search_opts.get('recurse_zones', False))
+
+        # If search by 'status', we need to convert it to 'state'
+        # If the status is unknown, bail.
+        # Leave 'state' in search_opts so compute can pass it on to
+        # child zones..
+        if 'status' in search_opts:
+            status = search_opts['status']
+            search_opts['state'] = common.power_states_from_status(status)
+            if len(search_opts['state']) == 0:
+                reason = _('Invalid server status: %(status)s') % locals()
+                LOG.error(reason)
+                raise exception.InvalidInput(reason=reason)
+
+        # By default, compute's get_all() will return deleted instances.
+        # If an admin hasn't specified a 'deleted' search option, we need
+        # to filter out deleted instances by setting the filter ourselves.
+        # ... Unless 'changes-since' is specified, because 'changes-since'
+        # should return recently deleted images according to the API spec.
+
+        if 'deleted' not in search_opts:
+            # Admin hasn't specified deleted filter
+            if 'changes-since' not in search_opts:
+                # No 'changes-since', so we need to find non-deleted servers
+                search_opts['deleted'] = False
+            else:
+                # This is the default, but just in case..
+                search_opts['deleted'] = True
+
         instance_list = self.compute_api.get_all(
-                req.environ['nova.context'],
-                reservation_id=reservation_id,
-                project_id=project_id,
-                fixed_ip=fixed_ip,
-                recurse_zones=recurse_zones)
+                context, search_opts=search_opts)
+
+        # FIXME(comstud): 'changes-since' is not fully implemented.  Where
+        # should this be filtered?
+
         limited_list = self._limit_items(instance_list, req)
         servers = [self._build_view(req, inst, is_detail)['server']
                 for inst in limited_list]
@@ -506,6 +543,7 @@
 
 
 class ControllerV10(Controller):
+    """v1.0 OpenStack API controller"""
 
     @scheduler_api.redirect_handler
     def delete(self, req, id):
@@ -568,8 +606,13 @@
         """ Determine the admin password for a server on creation """
         return self.helper._get_server_admin_password_old_style(server)
 
+    def _get_server_search_options(self):
+        """Return server search options allowed by non-admin"""
+        return 'reservation_id', 'fixed_ip', 'name', 'recurse_zones'
+
 
 class ControllerV11(Controller):
+    """v1.1 OpenStack API controller"""
 
     @scheduler_api.redirect_handler
     def delete(self, req, id):
@@ -742,6 +785,11 @@
         """ Determine the admin password for a server on creation """
         return self.helper._get_server_admin_password_new_style(server)
 
+    def _get_server_search_options(self):
+        """Return server search options allowed by non-admin"""
+        return ('reservation_id', 'name', 'recurse_zones',
+                'status', 'image', 'flavor', 'changes-since')
+
 
 class HeadersSerializer(wsgi.ResponseHeadersSerializer):
 
@@ -920,3 +968,18 @@
     deserializer = wsgi.RequestDeserializer(body_deserializers)
 
     return wsgi.Resource(controller, deserializer, serializer)
+
+
+def remove_invalid_options(context, search_options, allowed_search_options):
+    """Remove search options that are not valid for non-admin API/context"""
+    if FLAGS.allow_admin_api and context.is_admin:
+        # Allow all options
+        return
+    # Otherwise, strip out all unknown options
+    unknown_options = [opt for opt in search_options
+            if opt not in allowed_search_options]
+    unk_opt_str = ", ".join(unknown_options)
+    log_msg = _("Removing options '%(unk_opt_str)s' from query") % locals()
+    LOG.debug(log_msg)
+    for opt in unknown_options:
+        search_options.pop(opt, None)

=== modified file 'nova/api/openstack/views/servers.py'
--- nova/api/openstack/views/servers.py	2011-07-29 16:28:02 +0000
+++ nova/api/openstack/views/servers.py	2011-08-09 20:27:18 +0000
@@ -20,7 +20,6 @@
 import os
 
 from nova import exception
-from nova.compute import power_state
 import nova.compute
 import nova.context
 from nova.api.openstack import common
@@ -61,24 +60,11 @@
 
     def _build_detail(self, inst):
         """Returns a detailed model of a server."""
-        power_mapping = {
-            None: 'BUILD',
-            power_state.NOSTATE: 'BUILD',
-            power_state.RUNNING: 'ACTIVE',
-            power_state.BLOCKED: 'ACTIVE',
-            power_state.SUSPENDED: 'SUSPENDED',
-            power_state.PAUSED: 'PAUSED',
-            power_state.SHUTDOWN: 'SHUTDOWN',
-            power_state.SHUTOFF: 'SHUTOFF',
-            power_state.CRASHED: 'ERROR',
-            power_state.FAILED: 'ERROR',
-            power_state.BUILDING: 'BUILD',
-        }
 
         inst_dict = {
             'id': inst['id'],
             'name': inst['display_name'],
-            'status': power_mapping[inst.get('state')]}
+            'status': common.status_from_power_state(inst.get('state'))}
 
         ctxt = nova.context.get_admin_context()
         compute_api = nova.compute.API()

=== modified file 'nova/compute/api.py'
--- nova/compute/api.py	2011-08-09 15:27:56 +0000
+++ nova/compute/api.py	2011-08-09 20:27:18 +0000
@@ -19,6 +19,7 @@
 """Handles all requests relating to instances (guest vms)."""
 
 import eventlet
+import novaclient
 import re
 import time
 
@@ -712,59 +713,84 @@
         """
         return self.get(context, instance_id)
 
-    def get_all(self, context, project_id=None, reservation_id=None,
-                fixed_ip=None, recurse_zones=False):
+    def get_all(self, context, search_opts=None):
         """Get all instances filtered by one of the given parameters.
 
         If there is no filter and the context is an admin, it will retreive
         all instances in the system.
         """
 
-        if reservation_id is not None:
+        if search_opts is None:
+            search_opts = {}
+
+        LOG.debug(_("Searching by: %s") % str(search_opts))
+
+        # Fixups for the DB call
+        filters = {}
+
+        def _remap_flavor_filter(flavor_id):
+            instance_type = self.db.instance_type_get_by_flavor_id(
+                    context, flavor_id)
+            filters['instance_type_id'] = instance_type['id']
+
+        def _remap_fixed_ip_filter(fixed_ip):
+            # Turn fixed_ip into a regexp match. Since '.' matches
+            # any character, we need to use regexp escaping for it.
+            filters['ip'] = '^%s$' % fixed_ip.replace('.', '\\.')
+
+        # search_option to filter_name mapping.
+        filter_mapping = {
+                'image': 'image_ref',
+                'name': 'display_name',
+                'instance_name': 'name',
+                'recurse_zones': None,
+                'flavor': _remap_flavor_filter,
+                'fixed_ip': _remap_fixed_ip_filter}
+
+        # copy from search_opts, doing various remappings as necessary
+        for opt, value in search_opts.iteritems():
+            # Do remappings.
+            # Values not in the filter_mapping table are copied as-is.
+            # If remapping is None, option is not copied
+            # If the remapping is a string, it is the filter_name to use
+            try:
+                remap_object = filter_mapping[opt]
+            except KeyError:
+                filters[opt] = value
+            else:
+                if remap_object:
+                    if isinstance(remap_object, basestring):
+                        filters[remap_object] = value
+                    else:
+                        remap_object(value)
+
+        recurse_zones = search_opts.get('recurse_zones', False)
+        if 'reservation_id' in filters:
             recurse_zones = True
-            instances = self.db.instance_get_all_by_reservation(
-                                    context, reservation_id)
-        elif fixed_ip is not None:
-            try:
-                instances = self.db.fixed_ip_get_instance(context, fixed_ip)
-            except exception.FloatingIpNotFound, e:
-                if not recurse_zones:
-                    raise
-                instances = None
-        elif project_id or not context.is_admin:
-            if not context.project_id:
-                instances = self.db.instance_get_all_by_user(
-                    context, context.user_id)
-            else:
-                if project_id is None:
-                    project_id = context.project_id
-                instances = self.db.instance_get_all_by_project(
-                    context, project_id)
-        else:
-            instances = self.db.instance_get_all(context)
 
-        if instances is None:
-            instances = []
-        elif not isinstance(instances, list):
-            instances = [instances]
+        instances = self.db.instance_get_all_by_filters(context, filters)
 
         if not recurse_zones:
             return instances
 
+        # Recurse zones.  Need admin context for this.  Send along
+        # the un-modified search options we received..
         admin_context = context.elevated()
         children = scheduler_api.call_zone_method(admin_context,
                 "list",
+                errors_to_ignore=[novaclient.exceptions.NotFound],
                 novaclient_collection_name="servers",
-                reservation_id=reservation_id,
-                project_id=project_id,
-                fixed_ip=fixed_ip,
-                recurse_zones=True)
+                search_opts=search_opts)
 
         for zone, servers in children:
+            # 'servers' can be None if a 404 was returned by a zone
+            if servers is None:
+                continue
             for server in servers:
                 # Results are ready to send to user. No need to scrub.
                 server._info['_is_precooked'] = True
                 instances.append(server._info)
+
         return instances
 
     def _cast_compute_message(self, method, context, instance_id, host=None,

=== modified file 'nova/db/api.py'
--- nova/db/api.py	2011-08-08 21:33:03 +0000
+++ nova/db/api.py	2011-08-09 20:27:18 +0000
@@ -387,15 +387,6 @@
     return IMPL.fixed_ip_get_by_virtual_interface(context, vif_id)
 
 
-def fixed_ip_get_instance(context, address):
-    """Get an instance for a fixed ip by address."""
-    return IMPL.fixed_ip_get_instance(context, address)
-
-
-def fixed_ip_get_instance_v6(context, address):
-    return IMPL.fixed_ip_get_instance_v6(context, address)
-
-
 def fixed_ip_get_network(context, address):
     """Get a network for a fixed ip by address."""
     return IMPL.fixed_ip_get_network(context, address)
@@ -500,6 +491,11 @@
     return IMPL.instance_get_all(context)
 
 
+def instance_get_all_by_filters(context, filters):
+    """Get all instances that match all filters."""
+    return IMPL.instance_get_all_by_filters(context, filters)
+
+
 def instance_get_active_by_window(context, begin, end=None):
     """Get instances active during a certain time window."""
     return IMPL.instance_get_active_by_window(context, begin, end)
@@ -521,10 +517,20 @@
 
 
 def instance_get_all_by_reservation(context, reservation_id):
-    """Get all instance belonging to a reservation."""
+    """Get all instances belonging to a reservation."""
     return IMPL.instance_get_all_by_reservation(context, reservation_id)
 
 
+def instance_get_by_fixed_ip(context, address):
+    """Get an instance for a fixed ip by address."""
+    return IMPL.instance_get_by_fixed_ip(context, address)
+
+
+def instance_get_by_fixed_ipv6(context, address):
+    """Get an instance for a fixed ip by IPv6 address."""
+    return IMPL.instance_get_by_fixed_ipv6(context, address)
+
+
 def instance_get_fixed_addresses(context, instance_id):
     """Get the fixed ip address of an instance."""
     return IMPL.instance_get_fixed_addresses(context, instance_id)

=== modified file 'nova/db/sqlalchemy/api.py'
--- nova/db/sqlalchemy/api.py	2011-08-09 15:27:56 +0000
+++ nova/db/sqlalchemy/api.py	2011-08-09 20:27:18 +0000
@@ -18,6 +18,7 @@
 """
 Implementation of SQLAlchemy backend.
 """
+import re
 import warnings
 
 from nova import block_device
@@ -829,28 +830,6 @@
     return rv
 
 
-@require_context
-def fixed_ip_get_instance(context, address):
-    fixed_ip_ref = fixed_ip_get_by_address(context, address)
-    return fixed_ip_ref.instance
-
-
-@require_context
-def fixed_ip_get_instance_v6(context, address):
-    session = get_session()
-
-    # convert IPv6 address to mac
-    mac = ipv6.to_mac(address)
-
-    # get virtual interface
-    vif_ref = virtual_interface_get_by_address(context, mac)
-
-    # look up instance based on instance_id from vif row
-    result = session.query(models.Instance).\
-                     filter_by(id=vif_ref['instance_id'])
-    return result
-
-
 @require_admin_context
 def fixed_ip_get_network(context, address):
     fixed_ip_ref = fixed_ip_get_by_address(context, address)
@@ -1169,6 +1148,114 @@
                    all()
 
 
+@require_context
+def instance_get_all_by_filters(context, filters):
+    """Return instances that match all filters.  Deleted instances
+    will be returned by default, unless there's a filter that says
+    otherwise"""
+
+    def _regexp_filter_by_ipv6(instance, filter_re):
+        for interface in instance['virtual_interfaces']:
+            fixed_ipv6 = interface.get('fixed_ipv6')
+            if fixed_ipv6 and filter_re.match(fixed_ipv6):
+                return True
+        return False
+
+    def _regexp_filter_by_ip(instance, filter_re):
+        for interface in instance['virtual_interfaces']:
+            for fixed_ip in interface['fixed_ips']:
+                if not fixed_ip or not fixed_ip['address']:
+                    continue
+                if filter_re.match(fixed_ip['address']):
+                    return True
+                for floating_ip in fixed_ip.get('floating_ips', []):
+                    if not floating_ip or not floating_ip['address']:
+                        continue
+                    if filter_re.match(floating_ip['address']):
+                        return True
+        return False
+
+    def _regexp_filter_by_column(instance, filter_name, filter_re):
+        try:
+            v = getattr(instance, filter_name)
+        except AttributeError:
+            return True
+        if v and filter_re.match(str(v)):
+            return True
+        return False
+
+    def _exact_match_filter(query, column, value):
+        """Do exact match against a column.  value to match can be a list
+        so you can match any value in the list.
+        """
+        if isinstance(value, list):
+            column_attr = getattr(models.Instance, column)
+            return query.filter(column_attr.in_(value))
+        else:
+            filter_dict = {}
+            filter_dict[column] = value
+            return query.filter_by(**filter_dict)
+
+    session = get_session()
+    query_prefix = session.query(models.Instance).\
+                   options(joinedload_all('fixed_ips.floating_ips')).\
+                   options(joinedload_all('virtual_interfaces.network')).\
+                   options(joinedload_all(
+                           'virtual_interfaces.fixed_ips.floating_ips')).\
+                   options(joinedload('security_groups')).\
+                   options(joinedload_all('fixed_ips.network')).\
+                   options(joinedload('metadata')).\
+                   options(joinedload('instance_type'))
+
+    # Make a copy of the filters dictionary to use going forward, as we'll
+    # be modifying it and we shouldn't affect the caller's use of it.
+    filters = filters.copy()
+
+    if not context.is_admin:
+        # If we're not admin context, add appropriate filter..
+        if context.project_id:
+            filters['project_id'] = context.project_id
+        else:
+            filters['user_id'] = context.user_id
+
+    # Filters for exact matches that we can do along with the SQL query...
+    # For other filters that don't match this, we will do regexp matching
+    exact_match_filter_names = ['project_id', 'user_id', 'image_ref',
+            'state', 'instance_type_id', 'deleted']
+
+    query_filters = [key for key in filters.iterkeys()
+            if key in exact_match_filter_names]
+
+    for filter_name in query_filters:
+        # Do the matching and remove the filter from the dictionary
+        # so we don't try it again below..
+        query_prefix = _exact_match_filter(query_prefix, filter_name,
+                filters.pop(filter_name))
+
+    instances = query_prefix.all()
+
+    if not instances:
+        return []
+
+    # Now filter on everything else for regexp matching..
+    # For filters not in the list, we'll attempt to use the filter_name
+    # as a column name in Instance..
+    regexp_filter_funcs = {'ip6': _regexp_filter_by_ipv6,
+            'ip': _regexp_filter_by_ip}
+
+    for filter_name in filters.iterkeys():
+        filter_func = regexp_filter_funcs.get(filter_name, None)
+        filter_re = re.compile(str(filters[filter_name]))
+        if filter_func:
+            filter_l = lambda instance: filter_func(instance, filter_re)
+        else:
+            filter_l = lambda instance: _regexp_filter_by_column(instance,
+                    filter_name, filter_re)
+        instances = filter(filter_l, instances)
+
+    return instances
+
+
 @require_admin_context
 def instance_get_active_by_window(context, begin, end=None):
     """Return instances that were continuously active over the given window"""
@@ -1237,30 +1324,48 @@
 @require_context
 def instance_get_all_by_reservation(context, reservation_id):
     session = get_session()
+    query = session.query(models.Instance).\
+                    filter_by(reservation_id=reservation_id).\
+                    options(joinedload_all('fixed_ips.floating_ips')).\
+                    options(joinedload('virtual_interfaces')).\
+                    options(joinedload('security_groups')).\
+                    options(joinedload_all('fixed_ips.network')).\
+                    options(joinedload('metadata')).\
+                    options(joinedload('instance_type'))
 
     if is_admin_context(context):
-        return session.query(models.Instance).\
-                       options(joinedload_all('fixed_ips.floating_ips')).\
-                       options(joinedload('virtual_interfaces')).\
-                       options(joinedload('security_groups')).\
-                       options(joinedload_all('fixed_ips.network')).\
-                       options(joinedload('metadata')).\
-                       options(joinedload('instance_type')).\
-                       filter_by(reservation_id=reservation_id).\
-                       filter_by(deleted=can_read_deleted(context)).\
-                       all()
+        return query.\
+                filter_by(deleted=can_read_deleted(context)).\
+                all()
     elif is_user_context(context):
-        return session.query(models.Instance).\
-                       options(joinedload_all('fixed_ips.floating_ips')).\
-                       options(joinedload('virtual_interfaces')).\
-                       options(joinedload('security_groups')).\
-                       options(joinedload_all('fixed_ips.network')).\
-                       options(joinedload('metadata')).\
-                       options(joinedload('instance_type')).\
-                       filter_by(project_id=context.project_id).\
-                       filter_by(reservation_id=reservation_id).\
-                       filter_by(deleted=False).\
-                       all()
+        return query.\
+                filter_by(project_id=context.project_id).\
+                filter_by(deleted=False).\
+                all()
+
+
+@require_context
+def instance_get_by_fixed_ip(context, address):
+    """Return instance ref by exact match of FixedIP"""
+    fixed_ip_ref = fixed_ip_get_by_address(context, address)
+    return fixed_ip_ref.instance
+
+
+@require_context
+def instance_get_by_fixed_ipv6(context, address):
+    """Return instance ref by exact match of IPv6"""
+    session = get_session()
+
+    # convert IPv6 address to mac
+    mac = ipv6.to_mac(address)
+
+    # get virtual interface
+    vif_ref = virtual_interface_get_by_address(context, mac)
+
+    # look up instance based on instance_id from vif row
+    result = session.query(models.Instance).\
+                     filter_by(id=vif_ref['instance_id'])
+    return result
 
 
 @require_admin_context
@@ -1302,7 +1407,7 @@
         network_refs = network_get_all_by_instance(context, instance_id)
         # compile a list of cidr_v6 prefixes sorted by network id
         prefixes = [ref.cidr_v6 for ref in
-                    sorted(network_refs, key=lambda ref: ref.id)]
+                sorted(network_refs, key=lambda ref: ref.id)]
         # get vifs associated with instance
         vif_refs = virtual_interface_get_by_instance(context, instance_ref.id)
         # compile list of the mac_addresses for vifs sorted by network id

=== modified file 'nova/db/sqlalchemy/models.py'
--- nova/db/sqlalchemy/models.py	2011-08-03 11:31:10 +0000
+++ nova/db/sqlalchemy/models.py	2011-08-09 20:27:18 +0000
@@ -180,6 +180,7 @@
     image_ref = Column(String(255))
     kernel_id = Column(String(255))
     ramdisk_id = Column(String(255))
+    server_name = Column(String(255))
 
 #    image_ref = Column(Integer, ForeignKey('images.id'), nullable=True)
 #    kernel_id = Column(Integer, ForeignKey('images.id'), nullable=True)

=== modified file 'nova/tests/api/openstack/fakes.py'
--- nova/tests/api/openstack/fakes.py	2011-08-02 22:57:24 +0000
+++ nova/tests/api/openstack/fakes.py	2011-08-09 20:27:18 +0000
@@ -71,14 +71,18 @@
     return self.application
 
 
-def wsgi_app(inner_app10=None, inner_app11=None, fake_auth=True):
+def wsgi_app(inner_app10=None, inner_app11=None, fake_auth=True,
+        fake_auth_context=None):
     if not inner_app10:
         inner_app10 = openstack.APIRouterV10()
     if not inner_app11:
         inner_app11 = openstack.APIRouterV11()
 
     if fake_auth:
-        ctxt = context.RequestContext('fake', 'fake')
+        if fake_auth_context is not None:
+            ctxt = fake_auth_context
+        else:
+            ctxt = context.RequestContext('fake', 'fake')
         api10 = openstack.FaultWrapper(wsgi.InjectContext(ctxt,
               limits.RateLimitingMiddleware(inner_app10)))
         api11 = openstack.FaultWrapper(wsgi.InjectContext(ctxt,

=== modified file 'nova/tests/api/openstack/test_servers.py'
--- nova/tests/api/openstack/test_servers.py	2011-08-09 04:47:16 +0000
+++ nova/tests/api/openstack/test_servers.py	2011-08-09 20:27:18 +0000
@@ -236,7 +236,8 @@
         fakes.stub_out_key_pair_funcs(self.stubs)
         fakes.stub_out_image_service(self.stubs)
         self.stubs.Set(utils, 'gen_uuid', fake_gen_uuid)
-        self.stubs.Set(nova.db.api, 'instance_get_all', return_servers)
+        self.stubs.Set(nova.db.api, 'instance_get_all_by_filters',
+                return_servers)
         self.stubs.Set(nova.db.api, 'instance_get', return_server_by_id)
         self.stubs.Set(nova.db, 'instance_get_by_uuid',
                        return_server_by_uuid)
@@ -1098,6 +1099,277 @@
         self.assertEqual(res.status_int, 400)
         self.assertTrue(res.body.find('marker param') > -1)
 
+    def test_get_servers_with_bad_option_v1_0(self):
+        # 1.0 API ignores unknown options
+        def fake_get_all(compute_self, context, search_opts=None):
+            return [stub_instance(100)]
+
+        self.stubs.Set(nova.compute.API, 'get_all', fake_get_all)
+
+        req = webob.Request.blank('/v1.0/servers?unknownoption=whee')
+        res = req.get_response(fakes.wsgi_app())
+        self.assertEqual(res.status_int, 200)
+        servers = json.loads(res.body)['servers']
+        self.assertEqual(len(servers), 1)
+        self.assertEqual(servers[0]['id'], 100)
+
+    def test_get_servers_with_bad_option_v1_1(self):
+        # 1.1 API also ignores unknown options
+        def fake_get_all(compute_self, context, search_opts=None):
+            return [stub_instance(100)]
+
+        self.stubs.Set(nova.compute.API, 'get_all', fake_get_all)
+
+        req = webob.Request.blank('/v1.1/servers?unknownoption=whee')
+        res = req.get_response(fakes.wsgi_app())
+        self.assertEqual(res.status_int, 200)
+        servers = json.loads(res.body)['servers']
+        self.assertEqual(len(servers), 1)
+        self.assertEqual(servers[0]['id'], 100)
+
+    def test_get_servers_allows_image_v1_1(self):
+        def fake_get_all(compute_self, context, search_opts=None):
+            self.assertNotEqual(search_opts, None)
+            self.assertTrue('image' in search_opts)
+            self.assertEqual(search_opts['image'], '12345')
+            return [stub_instance(100)]
+
+        self.stubs.Set(nova.compute.API, 'get_all', fake_get_all)
+        self.flags(allow_admin_api=False)
+
+        req = webob.Request.blank('/v1.1/servers?image=12345')
+        res = req.get_response(fakes.wsgi_app())
+        # The following assert will fail if either of the asserts in
+        # fake_get_all() fail
+        self.assertEqual(res.status_int, 200)
+        servers = json.loads(res.body)['servers']
+        self.assertEqual(len(servers), 1)
+        self.assertEqual(servers[0]['id'], 100)
+
+    def test_get_servers_allows_flavor_v1_1(self):
+        def fake_get_all(compute_self, context, search_opts=None):
+            self.assertNotEqual(search_opts, None)
+            self.assertTrue('flavor' in search_opts)
+            # flavor is an integer ID
+            self.assertEqual(search_opts['flavor'], '12345')
+            return [stub_instance(100)]
+
+        self.stubs.Set(nova.compute.API, 'get_all', fake_get_all)
+        self.flags(allow_admin_api=False)
+
+        req = webob.Request.blank('/v1.1/servers?flavor=12345')
+        res = req.get_response(fakes.wsgi_app())
+        # The following assert will fail if either of the asserts in
+        # fake_get_all() fail
+        self.assertEqual(res.status_int, 200)
+        servers = json.loads(res.body)['servers']
+        self.assertEqual(len(servers), 1)
+        self.assertEqual(servers[0]['id'], 100)
+
+    def test_get_servers_allows_status_v1_1(self):
+        def fake_get_all(compute_self, context, search_opts=None):
+            self.assertNotEqual(search_opts, None)
+            self.assertTrue('state' in search_opts)
+            self.assertEqual(set(search_opts['state']),
+                    set([power_state.RUNNING, power_state.BLOCKED]))
+            return [stub_instance(100)]
+
+        self.stubs.Set(nova.compute.API, 'get_all', fake_get_all)
+        self.flags(allow_admin_api=False)
+
+        req = webob.Request.blank('/v1.1/servers?status=active')
+        res = req.get_response(fakes.wsgi_app())
+        # The following assert will fail if either of the asserts in
+        # fake_get_all() fail
+        self.assertEqual(res.status_int, 200)
+        servers = json.loads(res.body)['servers']
+        self.assertEqual(len(servers), 1)
+        self.assertEqual(servers[0]['id'], 100)
+
+    def test_get_servers_invalid_status_v1_1(self):
+        """Test getting servers by invalid status"""
+
+        self.flags(allow_admin_api=False)
+
+        req = webob.Request.blank('/v1.1/servers?status=running')
+        res = req.get_response(fakes.wsgi_app())
+        # The following assert will fail if either of the asserts in
+        # fake_get_all() fail
+        self.assertEqual(res.status_int, 400)
+        self.assertTrue(res.body.find('Invalid server status') > -1)
+
+    def test_get_servers_allows_name_v1_1(self):
+        def fake_get_all(compute_self, context, search_opts=None):
+            self.assertNotEqual(search_opts, None)
+            self.assertTrue('name' in search_opts)
+            self.assertEqual(search_opts['name'], 'whee.*')
+            return [stub_instance(100)]
+
+        self.stubs.Set(nova.compute.API, 'get_all', fake_get_all)
+        self.flags(allow_admin_api=False)
+
+        req = webob.Request.blank('/v1.1/servers?name=whee.*')
+        res = req.get_response(fakes.wsgi_app())
+        # The following assert will fail if either of the asserts in
+        # fake_get_all() fail
+        self.assertEqual(res.status_int, 200)
+        servers = json.loads(res.body)['servers']
+        self.assertEqual(len(servers), 1)
+        self.assertEqual(servers[0]['id'], 100)
+
+    def test_get_servers_unknown_or_admin_options1(self):
+        """Test getting servers by admin-only or unknown options.
+        This tests when admin_api is off.  Make sure the admin and
+        unknown options are stripped before they get to
+        compute_api.get_all()
+        """
+
+        self.flags(allow_admin_api=False)
+
+        def fake_get_all(compute_self, context, search_opts=None):
+            self.assertNotEqual(search_opts, None)
+            # Allowed by user
+            self.assertTrue('name' in search_opts)
+            self.assertTrue('status' in search_opts)
+            # Allowed only by admins with admin API on
+            self.assertFalse('ip' in search_opts)
+            self.assertFalse('unknown_option' in search_opts)
+            return [stub_instance(100)]
+
+        self.stubs.Set(nova.compute.API, 'get_all', fake_get_all)
+
+        query_str = "name=foo&ip=10.*&status=active&unknown_option=meow"
+        req = webob.Request.blank('/v1.1/servers?%s' % query_str)
+        # Request admin context
+        context = nova.context.RequestContext('testuser', 'testproject',
+                is_admin=True)
+        res = req.get_response(fakes.wsgi_app(fake_auth_context=context))
+        # The following assert will fail if either of the asserts in
+        # fake_get_all() fail
+        self.assertEqual(res.status_int, 200)
+        servers = json.loads(res.body)['servers']
+        self.assertEqual(len(servers), 1)
+        self.assertEqual(servers[0]['id'], 100)
+
+    def test_get_servers_unknown_or_admin_options2(self):
+        """Test getting servers by admin-only or unknown options.
+        This tests when admin_api is on, but context is a user.
+        Make sure the admin and unknown options are stripped before
+        they get to compute_api.get_all()
+        """
+
+        self.flags(allow_admin_api=True)
+
+        def fake_get_all(compute_self, context, search_opts=None):
+            self.assertNotEqual(search_opts, None)
+            # Allowed by user
+            self.assertTrue('name' in search_opts)
+            self.assertTrue('status' in search_opts)
+            # Allowed only by admins with admin API on
+            self.assertFalse('ip' in search_opts)
+            self.assertFalse('unknown_option' in search_opts)
+            return [stub_instance(100)]
+
+        self.stubs.Set(nova.compute.API, 'get_all', fake_get_all)
+
+        query_str = "name=foo&ip=10.*&status=active&unknown_option=meow"
+        req = webob.Request.blank('/v1.1/servers?%s' % query_str)
+        # Request admin context
+        context = nova.context.RequestContext('testuser', 'testproject',
+                is_admin=False)
+        res = req.get_response(fakes.wsgi_app(fake_auth_context=context))
+        # The following assert will fail if either of the asserts in
+        # fake_get_all() fail
+        self.assertEqual(res.status_int, 200)
+        servers = json.loads(res.body)['servers']
+        self.assertEqual(len(servers), 1)
+        self.assertEqual(servers[0]['id'], 100)
+
+    def test_get_servers_unknown_or_admin_options3(self):
+        """Test getting servers by admin-only or unknown options.
+        This tests when admin_api is on and context is admin.
+        All options should be passed through to compute_api.get_all()
+        """
+
+        self.flags(allow_admin_api=True)
+
+        def fake_get_all(compute_self, context, search_opts=None):
+            self.assertNotEqual(search_opts, None)
+            # Allowed by user
+            self.assertTrue('name' in search_opts)
+            self.assertTrue('status' in search_opts)
+            # Allowed only by admins with admin API on
+            self.assertTrue('ip' in search_opts)
+            self.assertTrue('unknown_option' in search_opts)
+            return [stub_instance(100)]
+
+        self.stubs.Set(nova.compute.API, 'get_all', fake_get_all)
+
+        query_str = "name=foo&ip=10.*&status=active&unknown_option=meow"
+        req = webob.Request.blank('/v1.1/servers?%s' % query_str)
+        # Request admin context
+        context = nova.context.RequestContext('testuser', 'testproject',
+                is_admin=True)
+        res = req.get_response(fakes.wsgi_app(fake_auth_context=context))
+        # The following assert will fail if either of the asserts in
+        # fake_get_all() fail
+        self.assertEqual(res.status_int, 200)
+        servers = json.loads(res.body)['servers']
+        self.assertEqual(len(servers), 1)
+        self.assertEqual(servers[0]['id'], 100)
+
+    def test_get_servers_admin_allows_ip_v1_1(self):
+        """Test getting servers by ip with admin_api enabled and
+        admin context
+        """
+        self.flags(allow_admin_api=True)
+
+        def fake_get_all(compute_self, context, search_opts=None):
+            self.assertNotEqual(search_opts, None)
+            self.assertTrue('ip' in search_opts)
+            self.assertEqual(search_opts['ip'], '10\..*')
+            return [stub_instance(100)]
+
+        self.stubs.Set(nova.compute.API, 'get_all', fake_get_all)
+
+        req = webob.Request.blank('/v1.1/servers?ip=10\..*')
+        # Request admin context
+        context = nova.context.RequestContext('testuser', 'testproject',
+                is_admin=True)
+        res = req.get_response(fakes.wsgi_app(fake_auth_context=context))
+        # The following assert will fail if either of the asserts in
+        # fake_get_all() fail
+        self.assertEqual(res.status_int, 200)
+        servers = json.loads(res.body)['servers']
+        self.assertEqual(len(servers), 1)
+        self.assertEqual(servers[0]['id'], 100)
+
+    def test_get_servers_admin_allows_ip6_v1_1(self):
+        """Test getting servers by ip6 with admin_api enabled and
+        admin context
+        """
+        self.flags(allow_admin_api=True)
+
+        def fake_get_all(compute_self, context, search_opts=None):
+            self.assertNotEqual(search_opts, None)
+            self.assertTrue('ip6' in search_opts)
+            self.assertEqual(search_opts['ip6'], 'ffff.*')
+            return [stub_instance(100)]
+
+        self.stubs.Set(nova.compute.API, 'get_all', fake_get_all)
+
+        req = webob.Request.blank('/v1.1/servers?ip6=ffff.*')
+        # Request admin context
+        context = nova.context.RequestContext('testuser', 'testproject',
+                is_admin=True)
+        res = req.get_response(fakes.wsgi_app(fake_auth_context=context))
+        # The following assert will fail if either of the asserts in
+        # fake_get_all() fail
+        self.assertEqual(res.status_int, 200)
+        servers = json.loads(res.body)['servers']
+        self.assertEqual(len(servers), 1)
+        self.assertEqual(servers[0]['id'], 100)
+
     def _setup_for_create_instance(self):
         """Shared implementation for tests below that create instance"""
         def instance_create(context, inst):
@@ -1665,6 +1937,7 @@
     def test_get_all_server_details_v1_0(self):
         req = webob.Request.blank('/v1.0/servers/detail')
         res = req.get_response(fakes.wsgi_app())
+        self.assertEqual(res.status_int, 200)
         res_dict = json.loads(res.body)
 
         for i, s in enumerate(res_dict['servers']):
@@ -1720,7 +1993,7 @@
             return [stub_instance(i, 'fake', 'fake', None, None, i % 2)
                     for i in xrange(5)]
 
-        self.stubs.Set(nova.db.api, 'instance_get_all_by_project',
+        self.stubs.Set(nova.db.api, 'instance_get_all_by_filters',
             return_servers_with_host)
 
         req = webob.Request.blank('/v1.0/servers/detail')

=== modified file 'nova/tests/test_compute.py'
--- nova/tests/test_compute.py	2011-08-05 05:06:22 +0000
+++ nova/tests/test_compute.py	2011-08-09 20:27:18 +0000
@@ -26,6 +26,7 @@
 from nova import context
 from nova import db
 from nova.db.sqlalchemy import models
+from nova.db.sqlalchemy import api as sqlalchemy_api
 from nova import exception
 from nova import flags
 import nova.image.fake
@@ -73,8 +74,11 @@
 
         self.stubs.Set(nova.image.fake._FakeImageService, 'show', fake_show)
 
-    def _create_instance(self, params={}):
+    def _create_instance(self, params=None):
         """Create a test instance"""
+
+        if params is None:
+            params = {}
         inst = {}
         inst['image_ref'] = 1
         inst['reservation_id'] = 'r-fakeres'
@@ -864,6 +868,458 @@
         self.assertEqual(len(instances), 1)
         self.assertEqual(power_state.SHUTOFF, instances[0]['state'])
 
+    def test_get_all_by_name_regexp(self):
+        """Test searching instances by name (display_name)"""
+        c = context.get_admin_context()
+        instance_id1 = self._create_instance({'display_name': 'woot'})
+        instance_id2 = self._create_instance({
+                'display_name': 'woo',
+                'id': 20})
+        instance_id3 = self._create_instance({
+                'display_name': 'not-woot',
+                'id': 30})
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'name': 'woo.*'})
+        self.assertEqual(len(instances), 2)
+        instance_ids = [instance.id for instance in instances]
+        self.assertTrue(instance_id1 in instance_ids)
+        self.assertTrue(instance_id2 in instance_ids)
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'name': 'woot.*'})
+        instance_ids = [instance.id for instance in instances]
+        self.assertEqual(len(instances), 1)
+        self.assertTrue(instance_id1 in instance_ids)
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'name': '.*oot.*'})
+        self.assertEqual(len(instances), 2)
+        instance_ids = [instance.id for instance in instances]
+        self.assertTrue(instance_id1 in instance_ids)
+        self.assertTrue(instance_id3 in instance_ids)
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'name': 'n.*'})
+        self.assertEqual(len(instances), 1)
+        instance_ids = [instance.id for instance in instances]
+        self.assertTrue(instance_id3 in instance_ids)
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'name': 'noth.*'})
+        self.assertEqual(len(instances), 0)
+
+        db.instance_destroy(c, instance_id1)
+        db.instance_destroy(c, instance_id2)
+        db.instance_destroy(c, instance_id3)
+
+    def test_get_all_by_instance_name_regexp(self):
+        """Test searching instances by name"""
+        self.flags(instance_name_template='instance-%d')
+
+        c = context.get_admin_context()
+        instance_id1 = self._create_instance()
+        instance_id2 = self._create_instance({'id': 2})
+        instance_id3 = self._create_instance({'id': 10})
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'instance_name': 'instance.*'})
+        self.assertEqual(len(instances), 3)
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'instance_name': '.*\-\d$'})
+        self.assertEqual(len(instances), 2)
+        instance_ids = [instance.id for instance in instances]
+        self.assertTrue(instance_id1 in instance_ids)
+        self.assertTrue(instance_id2 in instance_ids)
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'instance_name': 'i.*2'})
+        self.assertEqual(len(instances), 1)
+        self.assertEqual(instances[0].id, instance_id2)
+
+        db.instance_destroy(c, instance_id1)
+        db.instance_destroy(c, instance_id2)
+        db.instance_destroy(c, instance_id3)
+
+    def test_get_by_fixed_ip(self):
+        """Test getting 1 instance by Fixed IP"""
+        c = context.get_admin_context()
+        instance_id1 = self._create_instance()
+        instance_id2 = self._create_instance({'id': 20})
+        instance_id3 = self._create_instance({'id': 30})
+
+        vif_ref1 = db.virtual_interface_create(c,
+                {'address': '12:34:56:78:90:12',
+                 'instance_id': instance_id1,
+                 'network_id': 1})
+        vif_ref2 = db.virtual_interface_create(c,
+                {'address': '90:12:34:56:78:90',
+                 'instance_id': instance_id2,
+                 'network_id': 1})
+
+        db.fixed_ip_create(c,
+                {'address': '1.1.1.1',
+                 'instance_id': instance_id1,
+                 'virtual_interface_id': vif_ref1['id']})
+        db.fixed_ip_create(c,
+                {'address': '1.1.2.1',
+                 'instance_id': instance_id2,
+                 'virtual_interface_id': vif_ref2['id']})
+
+        # regex not allowed
+        instances = self.compute_api.get_all(c,
+                search_opts={'fixed_ip': '.*'})
+        self.assertEqual(len(instances), 0)
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'fixed_ip': '1.1.3.1'})
+        self.assertEqual(len(instances), 0)
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'fixed_ip': '1.1.1.1'})
+        self.assertEqual(len(instances), 1)
+        self.assertEqual(instances[0].id, instance_id1)
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'fixed_ip': '1.1.2.1'})
+        self.assertEqual(len(instances), 1)
+        self.assertEqual(instances[0].id, instance_id2)
+
+        db.virtual_interface_delete(c, vif_ref1['id'])
+        db.virtual_interface_delete(c, vif_ref2['id'])
+        db.instance_destroy(c, instance_id1)
+        db.instance_destroy(c, instance_id2)
+
+    def test_get_all_by_ip_regexp(self):
+        """Test searching by Floating and Fixed IP"""
+        c = context.get_admin_context()
+        instance_id1 = self._create_instance({'display_name': 'woot'})
+        instance_id2 = self._create_instance({
+                'display_name': 'woo',
+                'id': 20})
+        instance_id3 = self._create_instance({
+                'display_name': 'not-woot',
+                'id': 30})
+
+        vif_ref1 = db.virtual_interface_create(c,
+                {'address': '12:34:56:78:90:12',
+                 'instance_id': instance_id1,
+                 'network_id': 1})
+        vif_ref2 = db.virtual_interface_create(c,
+                {'address': '90:12:34:56:78:90',
+                 'instance_id': instance_id2,
+                 'network_id': 1})
+        vif_ref3 = db.virtual_interface_create(c,
+                {'address': '34:56:78:90:12:34',
+                 'instance_id': instance_id3,
+                 'network_id': 1})
+
+        db.fixed_ip_create(c,
+                {'address': '1.1.1.1',
+                 'instance_id': instance_id1,
+                 'virtual_interface_id': vif_ref1['id']})
+        db.fixed_ip_create(c,
+                {'address': '1.1.2.1',
+                 'instance_id': instance_id2,
+                 'virtual_interface_id': vif_ref2['id']})
+        fix_addr = db.fixed_ip_create(c,
+                {'address': '1.1.3.1',
+                 'instance_id': instance_id3,
+                 'virtual_interface_id': vif_ref3['id']})
+        fix_ref = db.fixed_ip_get_by_address(c, fix_addr)
+        flo_ref = db.floating_ip_create(c,
+                {'address': '10.0.0.2',
+                'fixed_ip_id': fix_ref['id']})
+
+        # ends up matching 2nd octet here.. so all 3 match
+        instances = self.compute_api.get_all(c,
+                search_opts={'ip': '.*\.1'})
+        self.assertEqual(len(instances), 3)
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'ip': '1.*'})
+        self.assertEqual(len(instances), 3)
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'ip': '.*\.1.\d+$'})
+        self.assertEqual(len(instances), 1)
+        instance_ids = [instance.id for instance in instances]
+        self.assertTrue(instance_id1 in instance_ids)
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'ip': '.*\.2.+'})
+        self.assertEqual(len(instances), 1)
+        self.assertEqual(instances[0].id, instance_id2)
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'ip': '10.*'})
+        self.assertEqual(len(instances), 1)
+        self.assertEqual(instances[0].id, instance_id3)
+
+        db.virtual_interface_delete(c, vif_ref1['id'])
+        db.virtual_interface_delete(c, vif_ref2['id'])
+        db.virtual_interface_delete(c, vif_ref3['id'])
+        db.floating_ip_destroy(c, '10.0.0.2')
+        db.instance_destroy(c, instance_id1)
+        db.instance_destroy(c, instance_id2)
+        db.instance_destroy(c, instance_id3)
+
+    def test_get_all_by_ipv6_regexp(self):
+        """Test searching by IPv6 address"""
+
+        c = context.get_admin_context()
+        instance_id1 = self._create_instance({'display_name': 'woot'})
+        instance_id2 = self._create_instance({
+                'display_name': 'woo',
+                'id': 20})
+        instance_id3 = self._create_instance({
+                'display_name': 'not-woot',
+                'id': 30})
+
+        vif_ref1 = db.virtual_interface_create(c,
+                {'address': '12:34:56:78:90:12',
+                 'instance_id': instance_id1,
+                 'network_id': 1})
+        vif_ref2 = db.virtual_interface_create(c,
+                {'address': '90:12:34:56:78:90',
+                 'instance_id': instance_id2,
+                 'network_id': 1})
+        vif_ref3 = db.virtual_interface_create(c,
+                {'address': '34:56:78:90:12:34',
+                 'instance_id': instance_id3,
+                 'network_id': 1})
+
+        # This will create IPv6 addresses of:
+        # 1: fd00::1034:56ff:fe78:9012
+        # 20: fd00::9212:34ff:fe56:7890
+        # 30: fd00::3656:78ff:fe90:1234
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'ip6': '.*1034.*'})
+        self.assertEqual(len(instances), 1)
+        self.assertEqual(instances[0].id, instance_id1)
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'ip6': '^fd00.*'})
+        self.assertEqual(len(instances), 3)
+        instance_ids = [instance.id for instance in instances]
+        self.assertTrue(instance_id1 in instance_ids)
+        self.assertTrue(instance_id2 in instance_ids)
+        self.assertTrue(instance_id3 in instance_ids)
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'ip6': '^.*12.*34.*'})
+        self.assertEqual(len(instances), 2)
+        instance_ids = [instance.id for instance in instances]
+        self.assertTrue(instance_id2 in instance_ids)
+        self.assertTrue(instance_id3 in instance_ids)
+
+        db.virtual_interface_delete(c, vif_ref1['id'])
+        db.virtual_interface_delete(c, vif_ref2['id'])
+        db.virtual_interface_delete(c, vif_ref3['id'])
+        db.instance_destroy(c, instance_id1)
+        db.instance_destroy(c, instance_id2)
+        db.instance_destroy(c, instance_id3)
+
+    def test_get_all_by_multiple_options_at_once(self):
+        """Test searching by multiple options at once"""
+        c = context.get_admin_context()
+        instance_id1 = self._create_instance({'display_name': 'woot'})
+        instance_id2 = self._create_instance({
+                'display_name': 'woo',
+                'id': 20})
+        instance_id3 = self._create_instance({
+                'display_name': 'not-woot',
+                'id': 30})
+
+        vif_ref1 = db.virtual_interface_create(c,
+                {'address': '12:34:56:78:90:12',
+                 'instance_id': instance_id1,
+                 'network_id': 1})
+        vif_ref2 = db.virtual_interface_create(c,
+                {'address': '90:12:34:56:78:90',
+                 'instance_id': instance_id2,
+                 'network_id': 1})
+        vif_ref3 = db.virtual_interface_create(c,
+                {'address': '34:56:78:90:12:34',
+                 'instance_id': instance_id3,
+                 'network_id': 1})
+
+        db.fixed_ip_create(c,
+                {'address': '1.1.1.1',
+                 'instance_id': instance_id1,
+                 'virtual_interface_id': vif_ref1['id']})
+        db.fixed_ip_create(c,
+                {'address': '1.1.2.1',
+                 'instance_id': instance_id2,
+                 'virtual_interface_id': vif_ref2['id']})
+        fix_addr = db.fixed_ip_create(c,
+                {'address': '1.1.3.1',
+                 'instance_id': instance_id3,
+                 'virtual_interface_id': vif_ref3['id']})
+        fix_ref = db.fixed_ip_get_by_address(c, fix_addr)
+        flo_ref = db.floating_ip_create(c,
+                {'address': '10.0.0.2',
+                'fixed_ip_id': fix_ref['id']})
+
+        # ip ends up matching 2nd octet here.. so all 3 match ip
+        # but 'name' only matches one
+        instances = self.compute_api.get_all(c,
+                search_opts={'ip': '.*\.1', 'name': 'not.*'})
+        self.assertEqual(len(instances), 1)
+        self.assertEqual(instances[0].id, instance_id3)
+
+        # ip ends up matching any ip with a '2' in it.. so instance
+        # 2 and 3.. but name should only match #2
+        # but 'name' only matches one
+        instances = self.compute_api.get_all(c,
+                search_opts={'ip': '.*2', 'name': '^woo.*'})
+        self.assertEqual(len(instances), 1)
+        self.assertEqual(instances[0].id, instance_id2)
+
+        # same as above but no match on name (name matches instance_id1
+        # but the ip query doesn't
+        instances = self.compute_api.get_all(c,
+                search_opts={'ip': '.*2.*', 'name': '^woot.*'})
+        self.assertEqual(len(instances), 0)
+
+        # ip matches all 3... ipv6 matches #2+#3...name matches #3
+        instances = self.compute_api.get_all(c,
+                search_opts={'ip': '.*\.1',
+                             'name': 'not.*',
+                             'ip6': '^.*12.*34.*'})
+        self.assertEqual(len(instances), 1)
+        self.assertEqual(instances[0].id, instance_id3)
+
+        db.virtual_interface_delete(c, vif_ref1['id'])
+        db.virtual_interface_delete(c, vif_ref2['id'])
+        db.virtual_interface_delete(c, vif_ref3['id'])
+        db.floating_ip_destroy(c, '10.0.0.2')
+        db.instance_destroy(c, instance_id1)
+        db.instance_destroy(c, instance_id2)
+        db.instance_destroy(c, instance_id3)
+
+    def test_get_all_by_image(self):
+        """Test searching instances by image"""
+
+        c = context.get_admin_context()
+        instance_id1 = self._create_instance({'image_ref': '1234'})
+        instance_id2 = self._create_instance({
+            'id': 2,
+            'image_ref': '4567'})
+        instance_id3 = self._create_instance({
+            'id': 10,
+            'image_ref': '4567'})
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'image': '123'})
+        self.assertEqual(len(instances), 0)
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'image': '1234'})
+        self.assertEqual(len(instances), 1)
+        self.assertEqual(instances[0].id, instance_id1)
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'image': '4567'})
+        self.assertEqual(len(instances), 2)
+        instance_ids = [instance.id for instance in instances]
+        self.assertTrue(instance_id2 in instance_ids)
+        self.assertTrue(instance_id3 in instance_ids)
+
+        # Test passing a list as search arg
+        instances = self.compute_api.get_all(c,
+                search_opts={'image': ['1234', '4567']})
+        self.assertEqual(len(instances), 3)
+
+        db.instance_destroy(c, instance_id1)
+        db.instance_destroy(c, instance_id2)
+        db.instance_destroy(c, instance_id3)
+
+    def test_get_all_by_flavor(self):
+        """Test searching instances by image"""
+
+        c = context.get_admin_context()
+        instance_id1 = self._create_instance({'instance_type_id': 1})
+        instance_id2 = self._create_instance({
+                'id': 2,
+                'instance_type_id': 2})
+        instance_id3 = self._create_instance({
+                'id': 10,
+                'instance_type_id': 2})
+
+        # NOTE(comstud): Migrations set up the instance_types table
+        # for us.  Therefore, we assume the following is true for
+        # these tests:
+        # instance_type_id 1 == flavor 3
+        # instance_type_id 2 == flavor 1
+        # instance_type_id 3 == flavor 4
+        # instance_type_id 4 == flavor 5
+        # instance_type_id 5 == flavor 2
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'flavor': 5})
+        self.assertEqual(len(instances), 0)
+
+        self.assertRaises(exception.FlavorNotFound,
+                self.compute_api.get_all,
+                c, search_opts={'flavor': 99})
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'flavor': 3})
+        self.assertEqual(len(instances), 1)
+        self.assertEqual(instances[0].id, instance_id1)
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'flavor': 1})
+        self.assertEqual(len(instances), 2)
+        instance_ids = [instance.id for instance in instances]
+        self.assertTrue(instance_id2 in instance_ids)
+        self.assertTrue(instance_id3 in instance_ids)
+
+        db.instance_destroy(c, instance_id1)
+        db.instance_destroy(c, instance_id2)
+        db.instance_destroy(c, instance_id3)
+
+    def test_get_all_by_state(self):
+        """Test searching instances by state"""
+
+        c = context.get_admin_context()
+        instance_id1 = self._create_instance({'state': power_state.SHUTDOWN})
+        instance_id2 = self._create_instance({
+                'id': 2,
+                'state': power_state.RUNNING})
+        instance_id3 = self._create_instance({
+                'id': 10,
+                'state': power_state.RUNNING})
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'state': power_state.SUSPENDED})
+        self.assertEqual(len(instances), 0)
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'state': power_state.SHUTDOWN})
+        self.assertEqual(len(instances), 1)
+        self.assertEqual(instances[0].id, instance_id1)
+
+        instances = self.compute_api.get_all(c,
+                search_opts={'state': power_state.RUNNING})
+        self.assertEqual(len(instances), 2)
+        instance_ids = [instance.id for instance in instances]
+        self.assertTrue(instance_id2 in instance_ids)
+        self.assertTrue(instance_id3 in instance_ids)
+
+        # Test passing a list as search arg
+        instances = self.compute_api.get_all(c,
+                search_opts={'state': [power_state.SHUTDOWN,
+                        power_state.RUNNING]})
+        self.assertEqual(len(instances), 3)
+
+        db.instance_destroy(c, instance_id1)
+        db.instance_destroy(c, instance_id2)
+        db.instance_destroy(c, instance_id3)
+
     @staticmethod
     def _parse_db_block_device_mapping(bdm_ref):
         attr_list = ('delete_on_termination', 'device_name', 'no_device',

=== modified file 'nova/tests/test_metadata.py'
--- nova/tests/test_metadata.py	2011-07-23 07:57:04 +0000
+++ nova/tests/test_metadata.py	2011-08-09 20:27:18 +0000
@@ -43,17 +43,21 @@
                          'reservation_id': 'r-xxxxxxxx',
                          'user_data': '',
                          'image_ref': 7,
+                         'fixed_ips': [],
                          'root_device_name': '/dev/sda1',
                          'hostname': 'test'})
 
         def instance_get(*args, **kwargs):
             return self.instance
 
+        def instance_get_list(*args, **kwargs):
+            return [self.instance]
+
         def floating_get(*args, **kwargs):
             return '99.99.99.99'
 
         self.stubs.Set(api, 'instance_get', instance_get)
-        self.stubs.Set(api, 'fixed_ip_get_instance', instance_get)
+        self.stubs.Set(api, 'instance_get_all_by_filters', instance_get_list)
         self.stubs.Set(api, 'instance_get_floating_address', floating_get)
         self.app = metadatarequesthandler.MetadataRequestHandler()