)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"bfa1cfa638d2434a93257ba7891d1703111ac5d9","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Correct az property of instance when specifying force host/node"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently, the server creation will succeed when specifying force_host"},{"line_number":10,"context_line":"with an invalid availibility zone name, but which will cause failure"},{"line_number":11,"context_line":"when subsequent action(e.g. resize) is performed."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This patch fix this problem by correcting the availability_zone property"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"7aaa499b_cc23e204","line":10,"updated":"2015-04-13 03:25:39.000000000","message":"\"but\" is not needed","commit_id":"e27e810a7f47b69e51502d86a08fd450f35ee079"}],"nova/compute/api.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"727fa89056d67392a4b0ffac5a3e1c3c52ecfc07","unresolved":false,"context_lines":[{"line_number":793,"context_line":"                True)"},{"line_number":794,"context_line":"            az_dict \u003d dict(az_list)"},{"line_number":795,"context_line":""},{"line_number":796,"context_line":"            if availability_zone not in az_dict.iterkeys():"},{"line_number":797,"context_line":"                msg \u003d _(\u0027The requested availability zone is not available\u0027)"},{"line_number":798,"context_line":"                raise exception.InvalidRequest(msg)"},{"line_number":799,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1a6ced46_4f6afcee","line":796,"updated":"2015-03-26 09:47:29.000000000","message":"should we consider to use six.iterkeys here?","commit_id":"8455179f59a4c8bfe1bff02e83afad72e47c8213"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"59510c8a923fb0dc02afa9781410ffa62e4ded11","unresolved":false,"context_lines":[{"line_number":793,"context_line":"                True)"},{"line_number":794,"context_line":"            az_dict \u003d dict(az_list)"},{"line_number":795,"context_line":""},{"line_number":796,"context_line":"            if availability_zone not in az_dict.iterkeys():"},{"line_number":797,"context_line":"                msg \u003d _(\u0027The requested availability zone is not available\u0027)"},{"line_number":798,"context_line":"                raise exception.InvalidRequest(msg)"},{"line_number":799,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1a6ced46_6a0a467a","line":796,"in_reply_to":"1a6ced46_4f6afcee","updated":"2015-03-26 10:05:39.000000000","message":"oh, you\u0027re right, nice catch, I\u0027ll fix ASAP.","commit_id":"8455179f59a4c8bfe1bff02e83afad72e47c8213"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"ca01b8ead99098b5629d458a33d0a64af30c46b3","unresolved":false,"context_lines":[{"line_number":569,"context_line":"        #             via az using az:host:node. It might be nice to expose an"},{"line_number":570,"context_line":"        #             api to specify specific hosts to force onto, but for"},{"line_number":571,"context_line":"        #             now it just supports this legacy hack."},{"line_number":572,"context_line":"        # NOTE(deva): It is also possible to specify az::node, in which case"},{"line_number":573,"context_line":"        #             the host manager will determine the correct host."},{"line_number":574,"context_line":"        forced_host \u003d None"},{"line_number":575,"context_line":"        forced_node \u003d None"}],"source_content_type":"text/x-python","patch_set":6,"id":"baa041b7_8b793596","line":572,"updated":"2015-04-02 13:03:01.000000000","message":"It is currently allowed to specify node while omitting host when you specify az as this note says.","commit_id":"f073732b2ea2b2243a9baa8962537e76fffd6f92"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"03c7d8f9576764ca7f0b2a6e96acfada5a5408cb","unresolved":false,"context_lines":[{"line_number":797,"context_line":"                msg \u003d _(\u0027The requested availability zone is not available\u0027)"},{"line_number":798,"context_line":"                raise exception.InvalidRequest(msg)"},{"line_number":799,"context_line":""},{"line_number":800,"context_line":"            if forced_host and forced_host not in az_dict.get("},{"line_number":801,"context_line":"                    availability_zone):"},{"line_number":802,"context_line":"                msg \u003d _(\u0027The requested forced host does not belong to given \u0027"},{"line_number":803,"context_line":"                        \u0027availability zone\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa963961_05de2d6d","line":800,"updated":"2015-03-27 11:25:32.000000000","message":"There is one more case that\u0027s still not handled. It is possible to specify a forced node and no forced_host like az::node and this will bypass this check.\n\nOne way to solve this would be to lookup the host for that node and use that as forced_host. This could be done separately, I just wanted to bring it to attention.","commit_id":"f073732b2ea2b2243a9baa8962537e76fffd6f92"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"bf6f12e54681d0125d64080123fd635cd742ffef","unresolved":false,"context_lines":[{"line_number":797,"context_line":"                msg \u003d _(\u0027The requested availability zone is not available\u0027)"},{"line_number":798,"context_line":"                raise exception.InvalidRequest(msg)"},{"line_number":799,"context_line":""},{"line_number":800,"context_line":"            if forced_host and forced_host not in az_dict.get("},{"line_number":801,"context_line":"                    availability_zone):"},{"line_number":802,"context_line":"                msg \u003d _(\u0027The requested forced host does not belong to given \u0027"},{"line_number":803,"context_line":"                        \u0027availability zone\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"baa041b7_2de23604","line":800,"in_reply_to":"baa041b7_167548ca","updated":"2015-04-02 13:54:05.000000000","message":"another think is we didn\u0027t allow the user specify a force_node without a force_host.","commit_id":"f073732b2ea2b2243a9baa8962537e76fffd6f92"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"ca01b8ead99098b5629d458a33d0a64af30c46b3","unresolved":false,"context_lines":[{"line_number":797,"context_line":"                msg \u003d _(\u0027The requested availability zone is not available\u0027)"},{"line_number":798,"context_line":"                raise exception.InvalidRequest(msg)"},{"line_number":799,"context_line":""},{"line_number":800,"context_line":"            if forced_host and forced_host not in az_dict.get("},{"line_number":801,"context_line":"                    availability_zone):"},{"line_number":802,"context_line":"                msg \u003d _(\u0027The requested forced host does not belong to given \u0027"},{"line_number":803,"context_line":"                        \u0027availability zone\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"baa041b7_167548ca","line":800,"in_reply_to":"baa041b7_7204ab7d","updated":"2015-04-02 13:03:01.000000000","message":"I don\u0027t think we should hide anything from the user, rather inform if what they ask for is ambiguous. Since specifying az::node is perfectly valid (see above comment) I think we should handle it. One way to solve #1 could be:\n\nQuery db for compute nodes with hypervisor_hostname \u003d node. Check all the returned compute_node.host values against the specified az. If the result is one unique host, all is fine. If none or more than one, throw InvalidRequest with appropriate message.","commit_id":"f073732b2ea2b2243a9baa8962537e76fffd6f92"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"71cdcc667449db492a1728bb6676ea9d9829d1b1","unresolved":false,"context_lines":[{"line_number":797,"context_line":"                msg \u003d _(\u0027The requested availability zone is not available\u0027)"},{"line_number":798,"context_line":"                raise exception.InvalidRequest(msg)"},{"line_number":799,"context_line":""},{"line_number":800,"context_line":"            if forced_host and forced_host not in az_dict.get("},{"line_number":801,"context_line":"                    availability_zone):"},{"line_number":802,"context_line":"                msg \u003d _(\u0027The requested forced host does not belong to given \u0027"},{"line_number":803,"context_line":"                        \u0027availability zone\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"baa041b7_7204ab7d","line":800,"in_reply_to":"da9b358b_e29f6d4e","updated":"2015-04-02 03:24:44.000000000","message":"hi, Hans, thanks for your reply. I see your concern. but the problems here have 2-fold:\n\n* the node name is not unique across availability zones. so the user may be confused if there are more than 1 node with the same node name in az, what will happen if he/she specify az::nodename?\n* so, the workaround i could think of is override the az property of vm in the nova-compute layer as what jichen suggested before. but the issue #1 is still need to be solved.\n\nso, i want to hear your opinion again, thanks!","commit_id":"f073732b2ea2b2243a9baa8962537e76fffd6f92"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"732000c160356c7b64a05c32a4ad4e3d6828bb58","unresolved":false,"context_lines":[{"line_number":797,"context_line":"                msg \u003d _(\u0027The requested availability zone is not available\u0027)"},{"line_number":798,"context_line":"                raise exception.InvalidRequest(msg)"},{"line_number":799,"context_line":""},{"line_number":800,"context_line":"            if forced_host and forced_host not in az_dict.get("},{"line_number":801,"context_line":"                    availability_zone):"},{"line_number":802,"context_line":"                msg \u003d _(\u0027The requested forced host does not belong to given \u0027"},{"line_number":803,"context_line":"                        \u0027availability zone\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"da9b358b_e29f6d4e","line":800,"in_reply_to":"da9b358b_f2918453","updated":"2015-04-01 08:04:24.000000000","message":"Sorry if I was unclear, but that was not the case I was thinking of. Let me rephrase. If az and node is given with the boot request, the scheduler will figure out the correct host. Everything will boot fine but you might end up in the exact same situation that you are trying to solve, if the host picked by the scheduler based on node is not in the given az.","commit_id":"f073732b2ea2b2243a9baa8962537e76fffd6f92"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"1bd5642198bd894735b49a4e034f3b3ba852990d","unresolved":false,"context_lines":[{"line_number":797,"context_line":"                msg \u003d _(\u0027The requested availability zone is not available\u0027)"},{"line_number":798,"context_line":"                raise exception.InvalidRequest(msg)"},{"line_number":799,"context_line":""},{"line_number":800,"context_line":"            if forced_host and forced_host not in az_dict.get("},{"line_number":801,"context_line":"                    availability_zone):"},{"line_number":802,"context_line":"                msg \u003d _(\u0027The requested forced host does not belong to given \u0027"},{"line_number":803,"context_line":"                        \u0027availability zone\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"da9b358b_f2918453","line":800,"in_reply_to":"fa963961_05de2d6d","updated":"2015-04-01 03:16:09.000000000","message":"thanks Hans for your suggestion. we just can\u0027t do everything in the api layer. specifically, the \u0027node\u0027 will be validated in nova-compute, an exception will be raised if the node doesn\u0027t belong to the host, vm creation will fail. But it\u0027s not what I want to solve in this patch, because if you don\u0027t check the az, no exception will be raised, everything looks well, but it will cause subsequent errors.","commit_id":"f073732b2ea2b2243a9baa8962537e76fffd6f92"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"d08db510d1418c8e83e29ad05fc14c288e8da644","unresolved":false,"context_lines":[{"line_number":801,"context_line":"                    availability_zone):"},{"line_number":802,"context_line":"                msg \u003d _(\u0027The requested forced host does not belong to given \u0027"},{"line_number":803,"context_line":"                        \u0027availability zone\u0027)"},{"line_number":804,"context_line":"                raise exception.InvalidRequest(msg)"},{"line_number":805,"context_line":""},{"line_number":806,"context_line":"        if instance_type[\u0027disabled\u0027]:"},{"line_number":807,"context_line":"            raise exception.FlavorNotFound(flavor_id\u003dinstance_type[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa963961_4781e049","line":804,"updated":"2015-03-27 08:35:09.000000000","message":"this whole function is used for validation, \nI think make line 789 - 804 a helpful function \nlike _valid_availability_zone make sense","commit_id":"f073732b2ea2b2243a9baa8962537e76fffd6f92"},{"author":{"_account_id":6835,"name":"Kun Huang","email":"gareth@openstacker.org","username":"academicgareth"},"change_message_id":"639523214e666bc79040fa29615af24acc8f1c06","unresolved":false,"context_lines":[{"line_number":801,"context_line":"                    availability_zone):"},{"line_number":802,"context_line":"                msg \u003d _(\u0027The requested forced host does not belong to given \u0027"},{"line_number":803,"context_line":"                        \u0027availability zone\u0027)"},{"line_number":804,"context_line":"                raise exception.InvalidRequest(msg)"},{"line_number":805,"context_line":""},{"line_number":806,"context_line":"        if instance_type[\u0027disabled\u0027]:"},{"line_number":807,"context_line":"            raise exception.FlavorNotFound(flavor_id\u003dinstance_type[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa963961_928de833","line":804,"in_reply_to":"fa963961_4781e049","updated":"2015-03-27 09:17:05.000000000","message":"But the parent function is already _validate_and_build_base_options. It\u0027s not a good case here semantically. Personally I prefer to accepte those None(s) in unit test a bit more.","commit_id":"f073732b2ea2b2243a9baa8962537e76fffd6f92"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"ab19346440cf0a908f359851df42369516d72404","unresolved":false,"context_lines":[{"line_number":801,"context_line":"                    availability_zone):"},{"line_number":802,"context_line":"                msg \u003d _(\u0027The requested forced host does not belong to given \u0027"},{"line_number":803,"context_line":"                        \u0027availability zone\u0027)"},{"line_number":804,"context_line":"                raise exception.InvalidRequest(msg)"},{"line_number":805,"context_line":""},{"line_number":806,"context_line":"        if instance_type[\u0027disabled\u0027]:"},{"line_number":807,"context_line":"            raise exception.FlavorNotFound(flavor_id\u003dinstance_type[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa963961_b2310c0e","line":804,"in_reply_to":"fa963961_928de833","updated":"2015-03-27 09:28:52.000000000","message":"sorry but I don\u0027t think so \nmaybe it\u0027s not a big deal from functionality point of view\nbut from code style point, I still prefer to split it to a helper function","commit_id":"f073732b2ea2b2243a9baa8962537e76fffd6f92"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"d4b633fe7d61577976377fc567db2ce3bd98a3b8","unresolved":false,"context_lines":[{"line_number":791,"context_line":"                context.elevated(),"},{"line_number":792,"context_line":"                True,"},{"line_number":793,"context_line":"                True)"},{"line_number":794,"context_line":"            az_dict \u003d dict(az_list)"},{"line_number":795,"context_line":""},{"line_number":796,"context_line":"            if availability_zone not in six.iterkeys(az_dict):"},{"line_number":797,"context_line":"                msg \u003d _(\u0027The requested availability zone is not available\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"baa041b7_c33152a4","line":794,"updated":"2015-04-02 17:12:06.000000000","message":"IMHO you should put a comment explaining that the list is made from tuple items so that works","commit_id":"af7cdf5ac75e84c68c729084d39254a1ccfe6333"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"d4b633fe7d61577976377fc567db2ce3bd98a3b8","unresolved":false,"context_lines":[{"line_number":793,"context_line":"                True)"},{"line_number":794,"context_line":"            az_dict \u003d dict(az_list)"},{"line_number":795,"context_line":""},{"line_number":796,"context_line":"            if availability_zone not in six.iterkeys(az_dict):"},{"line_number":797,"context_line":"                msg \u003d _(\u0027The requested availability zone is not available\u0027)"},{"line_number":798,"context_line":"                raise exception.InvalidRequest(msg)"},{"line_number":799,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"baa041b7_e3d33637","line":796,"updated":"2015-04-02 17:12:06.000000000","message":"not necessary, you can directly call :\n\n  if az not in az_dict:\n    do..","commit_id":"af7cdf5ac75e84c68c729084d39254a1ccfe6333"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"d4b633fe7d61577976377fc567db2ce3bd98a3b8","unresolved":false,"context_lines":[{"line_number":800,"context_line":"            if forced_host and forced_host not in az_dict.get("},{"line_number":801,"context_line":"                    availability_zone):"},{"line_number":802,"context_line":"                msg \u003d _(\u0027The requested forced host does not belong to given \u0027"},{"line_number":803,"context_line":"                        \u0027availability zone\u0027)"},{"line_number":804,"context_line":"                raise exception.InvalidRequest(msg)"},{"line_number":805,"context_line":""},{"line_number":806,"context_line":"        if instance_type[\u0027disabled\u0027]:"}],"source_content_type":"text/x-python","patch_set":7,"id":"baa041b7_c38dd232","line":803,"updated":"2015-04-02 17:12:06.000000000","message":"Unfortunately, we are now in StringFreeze so this change can\u0027t be merged for Kilo because you\u0027re are changing the semantics of a translated string.\n\nSee https://wiki.openstack.org/wiki/StringFreeze for details","commit_id":"af7cdf5ac75e84c68c729084d39254a1ccfe6333"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"b546c67c0a9b36377f5964065cc3450fb186b053","unresolved":false,"context_lines":[{"line_number":800,"context_line":"            if forced_host and forced_host not in az_dict.get("},{"line_number":801,"context_line":"                    availability_zone):"},{"line_number":802,"context_line":"                msg \u003d _(\u0027The requested forced host does not belong to given \u0027"},{"line_number":803,"context_line":"                        \u0027availability zone\u0027)"},{"line_number":804,"context_line":"                raise exception.InvalidRequest(msg)"},{"line_number":805,"context_line":""},{"line_number":806,"context_line":"        if instance_type[\u0027disabled\u0027]:"}],"source_content_type":"text/x-python","patch_set":7,"id":"9aa53dc9_5382bb01","line":803,"in_reply_to":"baa041b7_c38dd232","updated":"2015-04-07 12:02:11.000000000","message":"thanks, sylvain. for this bug, i think it\u0027s probably acceptable that we override the availability_zone property in nova-compute in the forced host. what do you think?","commit_id":"af7cdf5ac75e84c68c729084d39254a1ccfe6333"}],"nova/compute/resource_tracker.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"3b67ebd70e65475e2452c2f14dbe0c2bfe781e9e","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        \"\"\""},{"line_number":219,"context_line":"        # correct the availability_zone property in case of specifying invalid"},{"line_number":220,"context_line":"        # az when booting instance with force host"},{"line_number":221,"context_line":"        az \u003d availability_zones.get_host_availability_zone(context, self.host)"},{"line_number":222,"context_line":"        values \u003d {\u0027host\u0027: self.host, \u0027node\u0027: self.nodename,"},{"line_number":223,"context_line":"                  \u0027availability_zone\u0027: az, \u0027launched_on\u0027: self.host}"},{"line_number":224,"context_line":"        self.conductor_api.instance_update(context, instance_ref[\u0027uuid\u0027],"}],"source_content_type":"text/x-python","patch_set":10,"id":"7aaa499b_753fa3d6","line":221,"updated":"2015-04-14 22:40:31.000000000","message":"Well, I really hate to see the RT instanciating the host and node fields when booting, while it\u0027s done in the compute.manager for the other boot cases (like migrating) but it sounds the AZ update has to be done here :(","commit_id":"e27e810a7f47b69e51502d86a08fd450f35ee079"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"5c35477627c4dea5e75641302b44c86444d93cac","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        \"\"\""},{"line_number":219,"context_line":"        # correct the availability_zone property in case of specifying invalid"},{"line_number":220,"context_line":"        # az when booting instance with force host"},{"line_number":221,"context_line":"        az \u003d availability_zones.get_host_availability_zone(context, self.host)"},{"line_number":222,"context_line":"        values \u003d {\u0027host\u0027: self.host, \u0027node\u0027: self.nodename,"},{"line_number":223,"context_line":"                  \u0027availability_zone\u0027: az, \u0027launched_on\u0027: self.host}"},{"line_number":224,"context_line":"        self.conductor_api.instance_update(context, instance_ref[\u0027uuid\u0027],"}],"source_content_type":"text/x-python","patch_set":10,"id":"7aaa499b_68a0e543","line":221,"in_reply_to":"7aaa499b_753fa3d6","updated":"2015-04-15 01:24:55.000000000","message":"hi, sylvain, yes, the workaroud may not be so graceful, on OpenStack China Hackahthon hold by Huawei and Intel in these days, we(engineers com from both companies) talked about this deeply. If we validate the az in the nova-api layer, it\u0027ll break the backward compatibility in terms of api bahaviour,which may confuse users. The possible impact of this patch is, user may see the wrong az in the api response when booting instance, but after a little while, it\u0027s changed internally. Maybe we could document this bahaviour. Your thoughts?","commit_id":"e27e810a7f47b69e51502d86a08fd450f35ee079"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"d861e4e00adabff7d198842978a3fb3e914bb7bd","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        instance_ref[\u0027host\u0027] \u003d self.host"},{"line_number":227,"context_line":"        instance_ref[\u0027launched_on\u0027] \u003d self.host"},{"line_number":228,"context_line":"        instance_ref[\u0027node\u0027] \u003d self.nodename"},{"line_number":229,"context_line":"        instance_ref[\u0027availability_zone\u0027] \u003d az"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    @utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE)"},{"line_number":232,"context_line":"    def abort_instance_claim(self, context, instance):"}],"source_content_type":"text/x-python","patch_set":11,"id":"dac4157f_cd084430","line":229,"updated":"2015-04-21 11:55:55.000000000","message":"IMHO https://review.openstack.org/172906 should address that issue","commit_id":"bb6b4fc74ca95a53fd74ab937a2a8d08319a45c1"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"15ce6ef1b94b63e102a4d44a3e56ad709823bd5f","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        instance_ref[\u0027host\u0027] \u003d self.host"},{"line_number":227,"context_line":"        instance_ref[\u0027launched_on\u0027] \u003d self.host"},{"line_number":228,"context_line":"        instance_ref[\u0027node\u0027] \u003d self.nodename"},{"line_number":229,"context_line":"        instance_ref[\u0027availability_zone\u0027] \u003d az"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    @utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE)"},{"line_number":232,"context_line":"    def abort_instance_claim(self, context, instance):"}],"source_content_type":"text/x-python","patch_set":11,"id":"1aca2d91_bc7220ac","line":229,"in_reply_to":"bab921fb_39d05953","updated":"2015-05-06 07:24:55.000000000","message":"I think you misunderstood me. While I thought a few comments before that my change was the only necessary, I changed my mind to accept your proposal since you were focusing on a separate thing (here, the thing that the user can provide a fake AZ without feedback loop in Nova)\n\nSo, please consider my -1 not as an equivalent -2 but really \"yes, I appreciate your change but I think something is not good to be merged now\".\n\nSo, back to the problem I mentioned in my last comment, I think that the main problem related to your problem (ie. defined AZ when using force_hosts) is that _handle_availability_zone() has a different behaviour based on if there was a pattern like \"az:host:node\" or just a string with no delimiters.\nThat behavioural change just creates confusion and leads to the change you propose. While I think your change can fix the issue, it doesn\u0027t fix the root cause.\n\nSo, that\u0027s why I propose that you just modify the return value of  _handle_availability_zone() by giving the tuple (default_az, hosts, nodes) since the AvailabilityZoneFilter is not called if force_hosts are present (we\u0027re forcing to hosts, we are not calling the filters)\n\nExhibit here :\nhttps://github.com/openstack/nova/blob/0913e799e9ce3138235f5ea6f80159f468ad2aaa/nova/scheduler/host_manager.py#L511-L514","commit_id":"bb6b4fc74ca95a53fd74ab937a2a8d08319a45c1"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"f97727491ef22a007105195139348aa3a9352290","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        instance_ref[\u0027host\u0027] \u003d self.host"},{"line_number":227,"context_line":"        instance_ref[\u0027launched_on\u0027] \u003d self.host"},{"line_number":228,"context_line":"        instance_ref[\u0027node\u0027] \u003d self.nodename"},{"line_number":229,"context_line":"        instance_ref[\u0027availability_zone\u0027] \u003d az"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    @utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE)"},{"line_number":232,"context_line":"    def abort_instance_claim(self, context, instance):"}],"source_content_type":"text/x-python","patch_set":11,"id":"dac4157f_f492879e","line":229,"in_reply_to":"dac4157f_605912fe","updated":"2015-04-22 10:10:43.000000000","message":"but, CONF.default_az here is also useless :(","commit_id":"bb6b4fc74ca95a53fd74ab937a2a8d08319a45c1"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"c69d214c13c3b4f630c63574bc4c0510c8ac01cf","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        instance_ref[\u0027host\u0027] \u003d self.host"},{"line_number":227,"context_line":"        instance_ref[\u0027launched_on\u0027] \u003d self.host"},{"line_number":228,"context_line":"        instance_ref[\u0027node\u0027] \u003d self.nodename"},{"line_number":229,"context_line":"        instance_ref[\u0027availability_zone\u0027] \u003d az"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    @utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE)"},{"line_number":232,"context_line":"    def abort_instance_claim(self, context, instance):"}],"source_content_type":"text/x-python","patch_set":11,"id":"dac4157f_605912fe","line":229,"in_reply_to":"dac4157f_650a4423","updated":"2015-04-22 10:07:27.000000000","message":"Fair point but I think something is wrong with your approach.\n\nnova.compute.api::_handle_availability_zone() just takes the az:host:node string from the availability zone option and decouple all fields.\nhttps://github.com/openstack/nova/blob/master/nova/compute/api.py#L1086-L1088\n\nAs the FilterScheduler is just bypassing the filters when there are force_hosts and force_nodes, the AvailabilityZoneFilter is not checked so the AZ field is just useless\nhttps://github.com/openstack/nova/blob/master/nova/scheduler/host_manager.py#L511-L514\n\nBy saying that, I think we just need to make sure that _handle_availability_zone() returns (CONF.default_az, forced_hosts, forced_nodes) because we just don\u0027t care about the given AZ as it\u0027s just useless","commit_id":"bb6b4fc74ca95a53fd74ab937a2a8d08319a45c1"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"4b250e6e3cb0705572a2199326e5442c151d357b","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        instance_ref[\u0027host\u0027] \u003d self.host"},{"line_number":227,"context_line":"        instance_ref[\u0027launched_on\u0027] \u003d self.host"},{"line_number":228,"context_line":"        instance_ref[\u0027node\u0027] \u003d self.nodename"},{"line_number":229,"context_line":"        instance_ref[\u0027availability_zone\u0027] \u003d az"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    @utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE)"},{"line_number":232,"context_line":"    def abort_instance_claim(self, context, instance):"}],"source_content_type":"text/x-python","patch_set":11,"id":"dac4157f_650a4423","line":229,"in_reply_to":"dac4157f_cd084430","updated":"2015-04-22 09:22:06.000000000","message":"hi, Sylvain, IMHO, i don\u0027t think that patch you memtioned could solve the problem I met. 172906 just ensure the right az when user perform \u0027nova show instance_id\u0027, but the az property remains wrong in the database.","commit_id":"bb6b4fc74ca95a53fd74ab937a2a8d08319a45c1"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"ccd3c490f44e52f06a8cbcb714151fd547ebeb83","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        instance_ref[\u0027host\u0027] \u003d self.host"},{"line_number":227,"context_line":"        instance_ref[\u0027launched_on\u0027] \u003d self.host"},{"line_number":228,"context_line":"        instance_ref[\u0027node\u0027] \u003d self.nodename"},{"line_number":229,"context_line":"        instance_ref[\u0027availability_zone\u0027] \u003d az"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    @utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE)"},{"line_number":232,"context_line":"    def abort_instance_claim(self, context, instance):"}],"source_content_type":"text/x-python","patch_set":11,"id":"bab921fb_39d05953","line":229,"in_reply_to":"dac4157f_d4778338","updated":"2015-04-23 05:16:54.000000000","message":"hi, Sylvain. hmm...sorry, I\u0027m afraid I don\u0027t agree with you, for some reasons:\n\n* if the az parameter is not provided, the system can give it a default value(i.e. default_az), so the scheduler filter only pick hosts belong to the default_az. the CONF.default_az does take effect here.\n* however, if user speicify az:force_host, the az may be the wrong one, no matter it\u0027s CONF.default_az or not. why not just let it go, and correct it when the right host is choosen? give it CONF.default_az doesn\u0027t make any sense here.\n\nthe problem you try to solve in your patch is to ensure user can see the right az by performing \u0027nova show\u0027, but the actual az property of instance may still be inconsistent in the dabatase, it\u0027s the very thing I am trying to solve through my patch.\n\nthoughts?\n\nP.S. what do you think if we bring this problem together with yours to the mail list, to get more feedback from the broad community? I have sent some emails before, but didn\u0027t get many core members\u0027 attention. Could you help do that, again? or, we can talk about it in the next nova irc meeting.","commit_id":"bb6b4fc74ca95a53fd74ab937a2a8d08319a45c1"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b0bb6f5d0facf89880ae1bd951782831e8a3bf4d","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        instance_ref[\u0027host\u0027] \u003d self.host"},{"line_number":227,"context_line":"        instance_ref[\u0027launched_on\u0027] \u003d self.host"},{"line_number":228,"context_line":"        instance_ref[\u0027node\u0027] \u003d self.nodename"},{"line_number":229,"context_line":"        instance_ref[\u0027availability_zone\u0027] \u003d az"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    @utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE)"},{"line_number":232,"context_line":"    def abort_instance_claim(self, context, instance):"}],"source_content_type":"text/x-python","patch_set":11,"id":"dac4157f_d4778338","line":229,"in_reply_to":"dac4157f_f492879e","updated":"2015-04-22 10:17:22.000000000","message":"Why ? If you\u0027re looking at _handle_availability_zone() it just returns by default CONF.default_az if no forced hosts are specified. \n\nThe logic is just the same : if we have forced hosts, we don\u0027t care about the AZ.","commit_id":"bb6b4fc74ca95a53fd74ab937a2a8d08319a45c1"}],"nova/tests/unit/compute/test_compute.py":[{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"6b33ce03e04f7456c70459fd2388e6c0149776ce","unresolved":false,"context_lines":[{"line_number":10708,"context_line":"                 \"network:validate_networks\": []}"},{"line_number":10709,"context_line":"        self.policy.set_rules(rules)"},{"line_number":10710,"context_line":""},{"line_number":10711,"context_line":"        # NOTE(xylan): availability zone must exsit according to the"},{"line_number":10712,"context_line":"        # bug #1431194"},{"line_number":10713,"context_line":"        admin_context \u003d context.get_admin_context()"},{"line_number":10714,"context_line":"        service \u003d objects.Service(admin_context)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_3ed2a634","line":10711,"updated":"2015-03-18 09:05:06.000000000","message":"nit: s/exsit/exist","commit_id":"d7cc38b1ae9d2573adfe5b10781d908a0ec0336e"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"03a9b4316bbe6c5b6da2968ba60af6e3157cc938","unresolved":false,"context_lines":[{"line_number":10778,"context_line":"        aggregate.name \u003d \u0027fake_aggregate\u0027"},{"line_number":10779,"context_line":"        aggregate.metadata \u003d {\u0027availability_zone\u0027: \u0027zone\u0027}"},{"line_number":10780,"context_line":"        aggregate.create()"},{"line_number":10781,"context_line":"        aggregate.add_host(\u0027host\u0027)"},{"line_number":10782,"context_line":""},{"line_number":10783,"context_line":"        self.compute_api.create(self.context, None, \u00271\u0027,"},{"line_number":10784,"context_line":"                                availability_zone\u003d\u0027zone:host\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa963961_f8a42e09","line":10781,"updated":"2015-03-26 17:32:12.000000000","message":"can we mock it ? I think it\u0027s too heavy \n\nalso, I think your case should use another test case","commit_id":"f073732b2ea2b2243a9baa8962537e76fffd6f92"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"771d8d60ab71aed69b32ef36de1c31d0ba087e8a","unresolved":false,"context_lines":[{"line_number":10778,"context_line":"        aggregate.name \u003d \u0027fake_aggregate\u0027"},{"line_number":10779,"context_line":"        aggregate.metadata \u003d {\u0027availability_zone\u0027: \u0027zone\u0027}"},{"line_number":10780,"context_line":"        aggregate.create()"},{"line_number":10781,"context_line":"        aggregate.add_host(\u0027host\u0027)"},{"line_number":10782,"context_line":""},{"line_number":10783,"context_line":"        self.compute_api.create(self.context, None, \u00271\u0027,"},{"line_number":10784,"context_line":"                                availability_zone\u003d\u0027zone:host\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa963961_411d5372","line":10781,"in_reply_to":"fa963961_f8a42e09","updated":"2015-03-27 04:58:08.000000000","message":"I just keep consistent with the other tests. What\u0027s that mean by \u0027use another test case\u0027?","commit_id":"f073732b2ea2b2243a9baa8962537e76fffd6f92"}],"nova/tests/unit/compute/test_compute_api.py":[{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"03a9b4316bbe6c5b6da2968ba60af6e3157cc938","unresolved":false,"context_lines":[{"line_number":2777,"context_line":"                          self.context, None, None, None, None, None, None,"},{"line_number":2778,"context_line":"                          None, None, None, None, None, \u0027invalid_az\u0027, None,"},{"line_number":2779,"context_line":"                          None, None, None, None, None, None, None, None,"},{"line_number":2780,"context_line":"                          None, None)"},{"line_number":2781,"context_line":""},{"line_number":2782,"context_line":"    @mock.patch(\u0027nova.availability_zones.get_availability_zones\u0027,"},{"line_number":2783,"context_line":"                return_value\u003d[(\u0027az1\u0027, (\u0027host1\u0027)), (\u0027az2\u0027, (\u0027host2\u0027))])"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa963961_18d59242","line":2780,"updated":"2015-03-26 17:32:12.000000000","message":"instead of writing so many None\n\ncan you make availability zone check a helper function and \ntest with it ?","commit_id":"f073732b2ea2b2243a9baa8962537e76fffd6f92"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"771d8d60ab71aed69b32ef36de1c31d0ba087e8a","unresolved":false,"context_lines":[{"line_number":2777,"context_line":"                          self.context, None, None, None, None, None, None,"},{"line_number":2778,"context_line":"                          None, None, None, None, None, \u0027invalid_az\u0027, None,"},{"line_number":2779,"context_line":"                          None, None, None, None, None, None, None, None,"},{"line_number":2780,"context_line":"                          None, None)"},{"line_number":2781,"context_line":""},{"line_number":2782,"context_line":"    @mock.patch(\u0027nova.availability_zones.get_availability_zones\u0027,"},{"line_number":2783,"context_line":"                return_value\u003d[(\u0027az1\u0027, (\u0027host1\u0027)), (\u0027az2\u0027, (\u0027host2\u0027))])"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa963961_5cff7c01","line":2780,"in_reply_to":"fa963961_18d59242","updated":"2015-03-27 04:58:08.000000000","message":"I know what you mean, and I admit it\u0027s a ugly way. but IMHO, adding a helper function(to generate these params) only for the two unit tests does not make much sense.","commit_id":"f073732b2ea2b2243a9baa8962537e76fffd6f92"}]}