)]}'
{"nova/objects/compute_node.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6d499d16f5fb3d99d32e5e36e3e992ec5c8d7e72","unresolved":false,"context_lines":[{"line_number":54,"context_line":"    # Version 1.14: Added cpu_allocation_ratio and ram_allocation_ratio"},{"line_number":55,"context_line":"    # Version 1.15: Added uuid"},{"line_number":56,"context_line":"    # Version 1.16: Added disk_allocation_ratio"},{"line_number":57,"context_line":"    VERSION \u003d \u00271.16\u0027"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    fields \u003d {"},{"line_number":60,"context_line":"        \u0027id\u0027: fields.IntegerField(read_only\u003dTrue),"}],"source_content_type":"text/x-python","patch_set":4,"id":"9aed3d3a_600da2e7","line":57,"updated":"2016-02-29 18:58:30.000000000","message":"feels like we should bump the version here? even if not required?","commit_id":"a841eeb6e976a1a7036623128a983951b3a2817c"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6d499d16f5fb3d99d32e5e36e3e992ec5c8d7e72","unresolved":false,"context_lines":[{"line_number":240,"context_line":"        # NOTE(danms): Remove this conditional load (and remove uuid from"},{"line_number":241,"context_line":"        # the list of special_cases above) once we\u0027re in Newton and have"},{"line_number":242,"context_line":"        # enforced that all UUIDs in the database are not NULL."},{"line_number":243,"context_line":"        if db_compute.get(\u0027uuid\u0027) and \u0027uuid\u0027 not in compute:"},{"line_number":244,"context_line":"            compute.uuid \u003d db_compute[\u0027uuid\u0027]"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"        compute.obj_reset_changes()"}],"source_content_type":"text/x-python","patch_set":4,"id":"9aed3d3a_7de6e915","line":243,"updated":"2016-02-29 18:58:30.000000000","message":"hmm, I don\u0027t get this bit yet.","commit_id":"a841eeb6e976a1a7036623128a983951b3a2817c"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6d499d16f5fb3d99d32e5e36e3e992ec5c8d7e72","unresolved":false,"context_lines":[{"line_number":329,"context_line":"            objects.InventoryList.get_all_by_resource_provider_uuid("},{"line_number":330,"context_line":"                self._context, self.uuid)"},{"line_number":331,"context_line":"        if not inventory_list:"},{"line_number":332,"context_line":"            return False"},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"        for inventory in inventory_list:"},{"line_number":335,"context_line":"            if inventory.resource_class \u003d\u003d fields.ResourceClass.VCPU:"}],"source_content_type":"text/x-python","patch_set":4,"id":"9aed3d3a_60f12201","line":332,"updated":"2016-02-29 18:58:30.000000000","message":"maybe just do the create here?","commit_id":"a841eeb6e976a1a7036623128a983951b3a2817c"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6d499d16f5fb3d99d32e5e36e3e992ec5c8d7e72","unresolved":false,"context_lines":[{"line_number":345,"context_line":""},{"line_number":346,"context_line":"            if key in updates:"},{"line_number":347,"context_line":"                inventory.total \u003d getattr(self, key)"},{"line_number":348,"context_line":"                updates[key] \u003d 0"},{"line_number":349,"context_line":""},{"line_number":350,"context_line":"                inventory.save()"},{"line_number":351,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":4,"id":"9aed3d3a_e01ed2b9","line":348,"updated":"2016-02-29 18:58:30.000000000","message":"hmm, seems like we keep updating the compute node table here. do we not want \"del updates[key]\" instead?\n\nIf we did that, we drop that column sooner, I think, if we wanted.\n\nI guess the first _create_inventory could zero out the compute node values?\n\nMaybe that more confusing in the end?","commit_id":"a841eeb6e976a1a7036623128a983951b3a2817c"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6d499d16f5fb3d99d32e5e36e3e992ec5c8d7e72","unresolved":false,"context_lines":[{"line_number":356,"context_line":"        This is only ever called once, either for the first time when a compute"},{"line_number":357,"context_line":"        is created, or after an upgrade where the required services have"},{"line_number":358,"context_line":"        reached the required version."},{"line_number":359,"context_line":"        \"\"\""},{"line_number":360,"context_line":"        rp \u003d objects.ResourceProvider(context\u003dself._context, uuid\u003dself.uuid)"},{"line_number":361,"context_line":"        rp.create()"},{"line_number":362,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9aed3d3a_20d27a5c","line":359,"updated":"2016-02-29 18:58:30.000000000","message":"probably want to log the creation.","commit_id":"a841eeb6e976a1a7036623128a983951b3a2817c"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6d499d16f5fb3d99d32e5e36e3e992ec5c8d7e72","unresolved":false,"context_lines":[{"line_number":382,"context_line":"                                reserved\u003d0,  # FIXME"},{"line_number":383,"context_line":"                                min_unit\u003d1,"},{"line_number":384,"context_line":"                                max_unit\u003d1,"},{"line_number":385,"context_line":"                                step_size\u003d32,  # Sane?"},{"line_number":386,"context_line":"                                allocation_ratio\u003d1.0)  # FIXME"},{"line_number":387,"context_line":"        mem.create()"},{"line_number":388,"context_line":"        updates[\u0027memory_mb\u0027] \u003d 0"}],"source_content_type":"text/x-python","patch_set":4,"id":"9aed3d3a_7d5d4946","line":385,"updated":"2016-02-29 18:58:30.000000000","message":"maybe 1 is fine?","commit_id":"a841eeb6e976a1a7036623128a983951b3a2817c"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6d499d16f5fb3d99d32e5e36e3e992ec5c8d7e72","unresolved":false,"context_lines":[{"line_number":399,"context_line":"        disk.create()"},{"line_number":400,"context_line":"        updates[\u0027local_gb\u0027] \u003d 0"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"        # ...etc"},{"line_number":403,"context_line":""},{"line_number":404,"context_line":"    @base.remotable"},{"line_number":405,"context_line":"    def create(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"9aed3d3a_60a3e200","line":402,"updated":"2016-02-29 18:58:30.000000000","message":"right, I guess you effectively also need to call _update_inventory here? at least given the usage pattern below?\n\nI assume the constraints will stop two of these happing at the same time and causing issues.","commit_id":"a841eeb6e976a1a7036623128a983951b3a2817c"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6d499d16f5fb3d99d32e5e36e3e992ec5c8d7e72","unresolved":false,"context_lines":[{"line_number":409,"context_line":"        updates \u003d self.obj_get_changes()"},{"line_number":410,"context_line":"        if \u0027uuid\u0027 not in updates:"},{"line_number":411,"context_line":"            updates[\u0027uuid\u0027] \u003d uuidutils.generate_uuid()"},{"line_number":412,"context_line":"            self.uuid \u003d updates[\u0027uuid\u0027]"},{"line_number":413,"context_line":""},{"line_number":414,"context_line":"        self._convert_stats_to_db_format(updates)"},{"line_number":415,"context_line":"        self._convert_host_ip_to_db_format(updates)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9aed3d3a_7d0b892a","line":412,"updated":"2016-02-29 18:58:30.000000000","message":"do we need to reset the dirty bit after that?","commit_id":"a841eeb6e976a1a7036623128a983951b3a2817c"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"3b5b9e3f6560c4ca40260dbcd8a7f3526b45cde4","unresolved":false,"context_lines":[{"line_number":350,"context_line":"                inventory.save()"},{"line_number":351,"context_line":"        return True"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"    def _create_inventory(self, updates):"},{"line_number":354,"context_line":"        \"\"\"Create the initial inventory objects for this compute node."},{"line_number":355,"context_line":""},{"line_number":356,"context_line":"        This is only ever called once, either for the first time when a compute"}],"source_content_type":"text/x-python","patch_set":5,"id":"7af24918_33deac6d","line":353,"updated":"2016-03-02 12:16:29.000000000","message":"Idea, is this something we could call in the resource tracker, so we use the CONF values (reserved RAM and Disk values) for this code in that context, then send all that data in the object, in its new form, to be saved into the DB? Or will that not really work?","commit_id":"3ab687e03596676f58e0a17a212a7e7f307f99ab"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"f1176d7b2965fa561ece42e3d101300898faa147","unresolved":false,"context_lines":[{"line_number":367,"context_line":"                                resource_provider\u003drp,"},{"line_number":368,"context_line":"                                resource_class\u003dfields.ResourceClass.VCPU,"},{"line_number":369,"context_line":"                                total\u003dself.vcpus,"},{"line_number":370,"context_line":"                                reserved\u003d0,  # FIXME"},{"line_number":371,"context_line":"                                min_unit\u003d1,"},{"line_number":372,"context_line":"                                max_unit\u003d1,"},{"line_number":373,"context_line":"                                step_size\u003d1,"}],"source_content_type":"text/x-python","patch_set":5,"id":"7af24918_7befb8b0","line":370,"range":{"start_line":370,"start_character":32,"end_line":370,"end_character":42},"updated":"2016-03-01 13:36:05.000000000","message":"This can be 0 because currently there are no cpu reservation AFAIK,","commit_id":"3ab687e03596676f58e0a17a212a7e7f307f99ab"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"57265c98e91892973338986f260696e1ca374a3b","unresolved":false,"context_lines":[{"line_number":367,"context_line":"                                resource_provider\u003drp,"},{"line_number":368,"context_line":"                                resource_class\u003dfields.ResourceClass.VCPU,"},{"line_number":369,"context_line":"                                total\u003dself.vcpus,"},{"line_number":370,"context_line":"                                reserved\u003d0,  # FIXME"},{"line_number":371,"context_line":"                                min_unit\u003d1,"},{"line_number":372,"context_line":"                                max_unit\u003d1,"},{"line_number":373,"context_line":"                                step_size\u003d1,"}],"source_content_type":"text/x-python","patch_set":5,"id":"7af24918_c72b46ea","line":370,"range":{"start_line":370,"start_character":32,"end_line":370,"end_character":42},"in_reply_to":"7af24918_7befb8b0","updated":"2016-03-02 11:35:02.000000000","message":"+1","commit_id":"3ab687e03596676f58e0a17a212a7e7f307f99ab"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"004079c59158c167a31a4e730695948c74ca0f7f","unresolved":false,"context_lines":[{"line_number":367,"context_line":"                                resource_provider\u003drp,"},{"line_number":368,"context_line":"                                resource_class\u003dfields.ResourceClass.VCPU,"},{"line_number":369,"context_line":"                                total\u003dself.vcpus,"},{"line_number":370,"context_line":"                                reserved\u003d0,  # FIXME"},{"line_number":371,"context_line":"                                min_unit\u003d1,"},{"line_number":372,"context_line":"                                max_unit\u003d1,"},{"line_number":373,"context_line":"                                step_size\u003d1,"}],"source_content_type":"text/x-python","patch_set":5,"id":"7af24918_33d22c01","line":370,"range":{"start_line":370,"start_character":32,"end_line":370,"end_character":42},"in_reply_to":"7af24918_c72b46ea","updated":"2016-03-02 12:50:25.000000000","message":"Yeah, maybe adding a better comment note would be helpful like \u0027FIXME(danms): We don\u0027t propose atm to set a reserved value for CPU usage\u0027","commit_id":"3ab687e03596676f58e0a17a212a7e7f307f99ab"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"f1176d7b2965fa561ece42e3d101300898faa147","unresolved":false,"context_lines":[{"line_number":371,"context_line":"                                min_unit\u003d1,"},{"line_number":372,"context_line":"                                max_unit\u003d1,"},{"line_number":373,"context_line":"                                step_size\u003d1,"},{"line_number":374,"context_line":"                                allocation_ratio\u003d1.0)  # FIXME"},{"line_number":375,"context_line":"        cpu.create()"},{"line_number":376,"context_line":"        updates[\u0027vcpus\u0027] \u003d 0"},{"line_number":377,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7af24918_fb10e8aa","line":374,"updated":"2016-03-01 13:36:05.000000000","message":"self.cpu_allocation_ratio","commit_id":"3ab687e03596676f58e0a17a212a7e7f307f99ab"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"004079c59158c167a31a4e730695948c74ca0f7f","unresolved":false,"context_lines":[{"line_number":371,"context_line":"                                min_unit\u003d1,"},{"line_number":372,"context_line":"                                max_unit\u003d1,"},{"line_number":373,"context_line":"                                step_size\u003d1,"},{"line_number":374,"context_line":"                                allocation_ratio\u003d1.0)  # FIXME"},{"line_number":375,"context_line":"        cpu.create()"},{"line_number":376,"context_line":"        updates[\u0027vcpus\u0027] \u003d 0"},{"line_number":377,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7af24918_531c18c1","line":374,"in_reply_to":"7af24918_874adec3","updated":"2016-03-02 12:50:25.000000000","message":"So, I think there is no problem to get the value from the field, I already make sure in Mitaka that this is either sourced from the local value in the compute_nodes table, or got from the conductor\u0027s nova.conf\n\nThat works for RAM and CPU (those were implemented in Liberty so we\u0027re 100% sure that all supported computes are running this) and for Disk, this was done during Mitaka, so we have the backwards compat by using the conductor\u0027s value as said.","commit_id":"3ab687e03596676f58e0a17a212a7e7f307f99ab"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"57265c98e91892973338986f260696e1ca374a3b","unresolved":false,"context_lines":[{"line_number":371,"context_line":"                                min_unit\u003d1,"},{"line_number":372,"context_line":"                                max_unit\u003d1,"},{"line_number":373,"context_line":"                                step_size\u003d1,"},{"line_number":374,"context_line":"                                allocation_ratio\u003d1.0)  # FIXME"},{"line_number":375,"context_line":"        cpu.create()"},{"line_number":376,"context_line":"        updates[\u0027vcpus\u0027] \u003d 0"},{"line_number":377,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7af24918_874adec3","line":374,"in_reply_to":"7af24918_fb10e8aa","updated":"2016-03-02 11:35:02.000000000","message":"oh, true, we have those now, at least once the upgrade is complete, which is when we start doing these moves.","commit_id":"3ab687e03596676f58e0a17a212a7e7f307f99ab"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"f1176d7b2965fa561ece42e3d101300898faa147","unresolved":false,"context_lines":[{"line_number":379,"context_line":"                                resource_provider\u003drp,"},{"line_number":380,"context_line":"                                resource_class\u003dfields.ResourceClass.MEMORY_MB,"},{"line_number":381,"context_line":"                                total\u003dself.memory_mb,"},{"line_number":382,"context_line":"                                reserved\u003d0,  # FIXME"},{"line_number":383,"context_line":"                                min_unit\u003d1,"},{"line_number":384,"context_line":"                                max_unit\u003d1,"},{"line_number":385,"context_line":"                                step_size\u003d32,  # Sane?"}],"source_content_type":"text/x-python","patch_set":5,"id":"7af24918_bbbae0be","line":382,"updated":"2016-03-01 13:36:05.000000000","message":"This field should be set in the related resource tracker of nova-compute service. Only that compute node knows the actual reservation value, i.e. it\u0027s CONF.reserved_host_memory_mb of that compute node.","commit_id":"3ab687e03596676f58e0a17a212a7e7f307f99ab"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"004079c59158c167a31a4e730695948c74ca0f7f","unresolved":false,"context_lines":[{"line_number":379,"context_line":"                                resource_provider\u003drp,"},{"line_number":380,"context_line":"                                resource_class\u003dfields.ResourceClass.MEMORY_MB,"},{"line_number":381,"context_line":"                                total\u003dself.memory_mb,"},{"line_number":382,"context_line":"                                reserved\u003d0,  # FIXME"},{"line_number":383,"context_line":"                                min_unit\u003d1,"},{"line_number":384,"context_line":"                                max_unit\u003d1,"},{"line_number":385,"context_line":"                                step_size\u003d32,  # Sane?"}],"source_content_type":"text/x-python","patch_set":5,"id":"7af24918_be1283b2","line":382,"in_reply_to":"7af24918_6ae31934","updated":"2016-03-02 12:50:25.000000000","message":"Yeah, that\u0027s a very good point, nice catch Yingxin.\n\nSo, the issue is since save() and create() are remotable methods, that code will be run by the conductor service, not the compute service. \n\nSo, there are 2 options there :\n- either assume that our cloud\u0027s nova.conf is consistent (at least by a relnote explaining that) for those values across all nodes and get the value by import the opt - but that means we loose the ability to set it per compute node (and I guess it\u0027s already used that way)\n- or, do the necessary glue in this object facade by adding the fields, set those and save() those in the RT so that it\u0027s possible to get those here","commit_id":"3ab687e03596676f58e0a17a212a7e7f307f99ab"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"57265c98e91892973338986f260696e1ca374a3b","unresolved":false,"context_lines":[{"line_number":379,"context_line":"                                resource_provider\u003drp,"},{"line_number":380,"context_line":"                                resource_class\u003dfields.ResourceClass.MEMORY_MB,"},{"line_number":381,"context_line":"                                total\u003dself.memory_mb,"},{"line_number":382,"context_line":"                                reserved\u003d0,  # FIXME"},{"line_number":383,"context_line":"                                min_unit\u003d1,"},{"line_number":384,"context_line":"                                max_unit\u003d1,"},{"line_number":385,"context_line":"                                step_size\u003d32,  # Sane?"}],"source_content_type":"text/x-python","patch_set":5,"id":"7af24918_6ae31934","line":382,"in_reply_to":"7af24918_bbbae0be","updated":"2016-03-02 11:35:02.000000000","message":"You are correct, we actually need to run this in the context of the compute node :(\n\nI have a feeling we need to migrate reserved_host_disk_mb and reserved_host_memory_mb to be updated in a similar way to we are doing for: cpu_allocation_ratio, etc.\n\nI guess we need a patch before this one, to start sending that data into the compute node? ew, thats messy.","commit_id":"3ab687e03596676f58e0a17a212a7e7f307f99ab"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"f1176d7b2965fa561ece42e3d101300898faa147","unresolved":false,"context_lines":[{"line_number":381,"context_line":"                                total\u003dself.memory_mb,"},{"line_number":382,"context_line":"                                reserved\u003d0,  # FIXME"},{"line_number":383,"context_line":"                                min_unit\u003d1,"},{"line_number":384,"context_line":"                                max_unit\u003d1,"},{"line_number":385,"context_line":"                                step_size\u003d32,  # Sane?"},{"line_number":386,"context_line":"                                allocation_ratio\u003d1.0)  # FIXME"},{"line_number":387,"context_line":"        mem.create()"}],"source_content_type":"text/x-python","patch_set":5,"id":"7af24918_5b775cd4","line":384,"range":{"start_line":384,"start_character":32,"end_line":384,"end_character":42},"updated":"2016-03-01 13:36:05.000000000","message":"Maybe it\u0027s good to set no limits here, default to \u0027unlimited\u0027 or adding a configuration with a default value in compute service.","commit_id":"3ab687e03596676f58e0a17a212a7e7f307f99ab"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"0cacc758208d9fc735674f6cccaf31b912a9ace0","unresolved":false,"context_lines":[{"line_number":382,"context_line":"                                reserved\u003d0,  # FIXME"},{"line_number":383,"context_line":"                                min_unit\u003d1,"},{"line_number":384,"context_line":"                                max_unit\u003d1,"},{"line_number":385,"context_line":"                                step_size\u003d32,  # Sane?"},{"line_number":386,"context_line":"                                allocation_ratio\u003d1.0)  # FIXME"},{"line_number":387,"context_line":"        mem.create()"},{"line_number":388,"context_line":"        updates[\u0027memory_mb\u0027] \u003d 0"}],"source_content_type":"text/x-python","patch_set":5,"id":"7af24918_ed867112","line":385,"updated":"2016-03-01 12:08:56.000000000","message":"Would 1 make more sense? It doesn\u0027t mean anything at this stage does it, but if someone expects it to be less than 32 later down the road and they never updated their inventories they may be surprised by this limitation and get unexpected failures.","commit_id":"3ab687e03596676f58e0a17a212a7e7f307f99ab"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"004079c59158c167a31a4e730695948c74ca0f7f","unresolved":false,"context_lines":[{"line_number":382,"context_line":"                                reserved\u003d0,  # FIXME"},{"line_number":383,"context_line":"                                min_unit\u003d1,"},{"line_number":384,"context_line":"                                max_unit\u003d1,"},{"line_number":385,"context_line":"                                step_size\u003d32,  # Sane?"},{"line_number":386,"context_line":"                                allocation_ratio\u003d1.0)  # FIXME"},{"line_number":387,"context_line":"        mem.create()"},{"line_number":388,"context_line":"        updates[\u0027memory_mb\u0027] \u003d 0"}],"source_content_type":"text/x-python","patch_set":5,"id":"7af24918_5e2b7f94","line":385,"in_reply_to":"7af24918_6db5f32f","updated":"2016-03-02 12:50:25.000000000","message":"Agreed with John, we don\u0027t currently use that so using some value without explaining why sounds a bit bad for me.\n\nUsing 1 for now seems just a default value that doesn\u0027t throw anyone under the bus.","commit_id":"3ab687e03596676f58e0a17a212a7e7f307f99ab"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6839ba2c2314dec9c97ccd02942aa0f022a0b105","unresolved":false,"context_lines":[{"line_number":382,"context_line":"                                reserved\u003d0,  # FIXME"},{"line_number":383,"context_line":"                                min_unit\u003d1,"},{"line_number":384,"context_line":"                                max_unit\u003d1,"},{"line_number":385,"context_line":"                                step_size\u003d32,  # Sane?"},{"line_number":386,"context_line":"                                allocation_ratio\u003d1.0)  # FIXME"},{"line_number":387,"context_line":"        mem.create()"},{"line_number":388,"context_line":"        updates[\u0027memory_mb\u0027] \u003d 0"}],"source_content_type":"text/x-python","patch_set":5,"id":"7af24918_6db5f32f","line":385,"in_reply_to":"7af24918_8a520112","updated":"2016-03-02 11:41:37.000000000","message":"I think our best option is to set this to 1, for now.\n\nWe can do a deprecation cycle on it, if we decide thats a silly idea, but changing to 32 now seems like its asking for trouble.","commit_id":"3ab687e03596676f58e0a17a212a7e7f307f99ab"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"377c385f59fe2f62bace920c044c25098f0fa206","unresolved":false,"context_lines":[{"line_number":382,"context_line":"                                reserved\u003d0,  # FIXME"},{"line_number":383,"context_line":"                                min_unit\u003d1,"},{"line_number":384,"context_line":"                                max_unit\u003d1,"},{"line_number":385,"context_line":"                                step_size\u003d32,  # Sane?"},{"line_number":386,"context_line":"                                allocation_ratio\u003d1.0)  # FIXME"},{"line_number":387,"context_line":"        mem.create()"},{"line_number":388,"context_line":"        updates[\u0027memory_mb\u0027] \u003d 0"}],"source_content_type":"text/x-python","patch_set":5,"id":"7af24918_8a520112","line":385,"in_reply_to":"7af24918_c996e693","updated":"2016-03-02 00:24:28.000000000","message":"Not usually, no, but I might think it would make sense to request a VM somewhere between 32 and 64MB. If we set step_size to 32 we can\u0027t do that.\n\nMy issue here is that we are setting a default by fiat during a live migration. That we are setting it is good, we need to do that, but we want to do it in a way that is going to be least surprising to people who are upgrading their nodes. As I understand things, step_size is a new concept. We don\u0027t want its introduction to break existing workflows.\n\nTherefore it _might_ make sense (the might is why I phrased my original comment as a question) for us to make step_size something that would have no impact. Then people who want to start using the functionality can update the inventories for their nodes under their own steam after they\u0027ve thought about what their good default is, rather than us trying to guess.\n\nUnless, of course, I\u0027ve completely scuppered my mental model of step_size. We did go round in circles about it.","commit_id":"3ab687e03596676f58e0a17a212a7e7f307f99ab"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"3a088bc6ff41be0c0a892925c6de5e9a505a2dc7","unresolved":false,"context_lines":[{"line_number":382,"context_line":"                                reserved\u003d0,  # FIXME"},{"line_number":383,"context_line":"                                min_unit\u003d1,"},{"line_number":384,"context_line":"                                max_unit\u003d1,"},{"line_number":385,"context_line":"                                step_size\u003d32,  # Sane?"},{"line_number":386,"context_line":"                                allocation_ratio\u003d1.0)  # FIXME"},{"line_number":387,"context_line":"        mem.create()"},{"line_number":388,"context_line":"        updates[\u0027memory_mb\u0027] \u003d 0"}],"source_content_type":"text/x-python","patch_set":5,"id":"7af24918_c996e693","line":385,"in_reply_to":"7af24918_ed867112","updated":"2016-03-01 21:24:35.000000000","message":"Chris: would you think it sane to request a VM with 7MB of RAM?","commit_id":"3ab687e03596676f58e0a17a212a7e7f307f99ab"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"f1176d7b2965fa561ece42e3d101300898faa147","unresolved":false,"context_lines":[{"line_number":383,"context_line":"                                min_unit\u003d1,"},{"line_number":384,"context_line":"                                max_unit\u003d1,"},{"line_number":385,"context_line":"                                step_size\u003d32,  # Sane?"},{"line_number":386,"context_line":"                                allocation_ratio\u003d1.0)  # FIXME"},{"line_number":387,"context_line":"        mem.create()"},{"line_number":388,"context_line":"        updates[\u0027memory_mb\u0027] \u003d 0"},{"line_number":389,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7af24918_5b0c7c42","line":386,"updated":"2016-03-01 13:36:05.000000000","message":"self.ram_allocation_ratio","commit_id":"3ab687e03596676f58e0a17a212a7e7f307f99ab"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"f1176d7b2965fa561ece42e3d101300898faa147","unresolved":false,"context_lines":[{"line_number":391,"context_line":"                                resource_provider\u003drp,"},{"line_number":392,"context_line":"                                resource_class\u003dfields.ResourceClass.DISK_GB,"},{"line_number":393,"context_line":"                                total\u003dself.local_gb,"},{"line_number":394,"context_line":"                                reserved\u003d0,  # FIXME"},{"line_number":395,"context_line":"                                min_unit\u003d1,"},{"line_number":396,"context_line":"                                max_unit\u003d1,"},{"line_number":397,"context_line":"                                step_size\u003d1,"}],"source_content_type":"text/x-python","patch_set":5,"id":"7af24918_fef736f3","line":394,"updated":"2016-03-01 13:36:05.000000000","message":"Also, it\u0027s CONF.reserved_host_disk_mb/1024 in the specific compute node.","commit_id":"3ab687e03596676f58e0a17a212a7e7f307f99ab"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"f1176d7b2965fa561ece42e3d101300898faa147","unresolved":false,"context_lines":[{"line_number":395,"context_line":"                                min_unit\u003d1,"},{"line_number":396,"context_line":"                                max_unit\u003d1,"},{"line_number":397,"context_line":"                                step_size\u003d1,"},{"line_number":398,"context_line":"                                allocation_ratio\u003d1.0)  # FIXME"},{"line_number":399,"context_line":"        disk.create()"},{"line_number":400,"context_line":"        updates[\u0027local_gb\u0027] \u003d 0"},{"line_number":401,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7af24918_be300ea0","line":398,"updated":"2016-03-01 13:36:05.000000000","message":"self.disk_allocation_ratio","commit_id":"3ab687e03596676f58e0a17a212a7e7f307f99ab"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"0cacc758208d9fc735674f6cccaf31b912a9ace0","unresolved":false,"context_lines":[{"line_number":418,"context_line":"            self._create_inventory(updates)"},{"line_number":419,"context_line":""},{"line_number":420,"context_line":"        db_compute \u003d db.compute_node_create(self._context, updates)"},{"line_number":421,"context_line":"        # NOTE(danms): compute_node_update() operates on (and returns) the"},{"line_number":422,"context_line":"        # compute node model only. We need to get the full inventory-based"},{"line_number":423,"context_line":"        # result in order to satisfy _from_db_compute(). So, we do a double"},{"line_number":424,"context_line":"        # query here. This can be removed in Newton once we\u0027re sure that all"}],"source_content_type":"text/x-python","patch_set":5,"id":"7af24918_6dd8a11e","line":421,"updated":"2016-03-01 12:08:56.000000000","message":"create?","commit_id":"3ab687e03596676f58e0a17a212a7e7f307f99ab"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"9fa41f965895a86dade95d118f04bdb38ec41f26","unresolved":false,"context_lines":[{"line_number":386,"context_line":"                                reserved\u003d0,  # FIXME"},{"line_number":387,"context_line":"                                min_unit\u003d1,"},{"line_number":388,"context_line":"                                max_unit\u003d1,"},{"line_number":389,"context_line":"                                step_size\u003d32,  # Sane?"},{"line_number":390,"context_line":"                                allocation_ratio\u003d1.0)  # FIXME"},{"line_number":391,"context_line":"        mem.create()"},{"line_number":392,"context_line":"        updates[\u0027memory_mb\u0027] \u003d 0"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba0121b8_866956c1","line":389,"updated":"2016-03-29 17:18:22.000000000","message":"See the comments on patchset 5 for why this should be 1.","commit_id":"6e6db68eca54607ee467b204cbb0db20a0196f91"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bfc4511198fb997b751e145cd0ddf3d17937fa85","unresolved":false,"context_lines":[{"line_number":386,"context_line":"                                reserved\u003d0,  # FIXME"},{"line_number":387,"context_line":"                                min_unit\u003d1,"},{"line_number":388,"context_line":"                                max_unit\u003d1,"},{"line_number":389,"context_line":"                                step_size\u003d32,  # Sane?"},{"line_number":390,"context_line":"                                allocation_ratio\u003d1.0)  # FIXME"},{"line_number":391,"context_line":"        mem.create()"},{"line_number":392,"context_line":"        updates[\u0027memory_mb\u0027] \u003d 0"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba0121b8_8008132f","line":389,"in_reply_to":"ba0121b8_866956c1","updated":"2016-03-29 20:52:10.000000000","message":"I really think that if you\u0027re cutting up your hosts so small that you\u0027re quibbling over 32MB, you\u0027re probably looking for a different technology than openstack, or cloud, or .. anything we work on here :)\n\nBut, sure.","commit_id":"6e6db68eca54607ee467b204cbb0db20a0196f91"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"9fa41f965895a86dade95d118f04bdb38ec41f26","unresolved":false,"context_lines":[{"line_number":422,"context_line":"            self._create_inventory(updates)"},{"line_number":423,"context_line":""},{"line_number":424,"context_line":"        db_compute \u003d db.compute_node_create(self._context, updates)"},{"line_number":425,"context_line":"        # NOTE(danms): compute_node_update() operates on (and returns) the"},{"line_number":426,"context_line":"        # compute node model only. We need to get the full inventory-based"},{"line_number":427,"context_line":"        # result in order to satisfy _from_db_compute(). So, we do a double"},{"line_number":428,"context_line":"        # query here. This can be removed in Newton once we\u0027re sure that all"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba0121b8_46a61e11","line":425,"updated":"2016-03-29 17:18:22.000000000","message":"If this is making reference to the method just called, then it is wrong. If it is not then it is confusing.","commit_id":"6e6db68eca54607ee467b204cbb0db20a0196f91"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bfc4511198fb997b751e145cd0ddf3d17937fa85","unresolved":false,"context_lines":[{"line_number":422,"context_line":"            self._create_inventory(updates)"},{"line_number":423,"context_line":""},{"line_number":424,"context_line":"        db_compute \u003d db.compute_node_create(self._context, updates)"},{"line_number":425,"context_line":"        # NOTE(danms): compute_node_update() operates on (and returns) the"},{"line_number":426,"context_line":"        # compute node model only. We need to get the full inventory-based"},{"line_number":427,"context_line":"        # result in order to satisfy _from_db_compute(). So, we do a double"},{"line_number":428,"context_line":"        # query here. This can be removed in Newton once we\u0027re sure that all"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba0121b8_e0590f5c","line":425,"in_reply_to":"ba0121b8_46a61e11","updated":"2016-03-29 20:52:10.000000000","message":"Yeah I imagine I was thinking about models.ComputeNode.update() or some such.","commit_id":"6e6db68eca54607ee467b204cbb0db20a0196f91"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"be3374babececc6af62fc0e8a63ccf9b025e4aac","unresolved":false,"context_lines":[{"line_number":325,"context_line":"                                                            related_binaries)"},{"line_number":326,"context_line":"        return min_ver \u003e\u003d required_version"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"    def _update_inventory(self, updates):"},{"line_number":329,"context_line":"        # NOTE(danms): Until we remove the columns from compute_nodes,"},{"line_number":330,"context_line":"        # we need to constantly strip out resource updates"},{"line_number":331,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_99f1eed0","line":328,"updated":"2016-04-04 11:27:26.000000000","message":"Nit: Maybe this should be _update_and_save_inventory, so its clear this does the DB update? It took two reads for me to spot that for some reason, just thinking out loud.","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8e9e24c048eb9f95532109dc0d7c65040cdab469","unresolved":false,"context_lines":[{"line_number":325,"context_line":"                                                            related_binaries)"},{"line_number":326,"context_line":"        return min_ver \u003e\u003d required_version"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"    def _update_inventory(self, updates):"},{"line_number":329,"context_line":"        # NOTE(danms): Until we remove the columns from compute_nodes,"},{"line_number":330,"context_line":"        # we need to constantly strip out resource updates"},{"line_number":331,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_b21de4bf","line":328,"in_reply_to":"9a061dce_99f1eed0","updated":"2016-04-04 13:45:30.000000000","message":"\"update and save\" is redundant to me. Personally, I think update is right here (obviously). If you\u0027d rather save then that\u0027s fine, but \"update and save\" seems as redundant as \"update and update\"...","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3194293239fef69fbd42b1e132bfdb1b04303a0d","unresolved":false,"context_lines":[{"line_number":325,"context_line":"                                                            related_binaries)"},{"line_number":326,"context_line":"        return min_ver \u003e\u003d required_version"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"    def _update_inventory(self, updates):"},{"line_number":329,"context_line":"        # NOTE(danms): Until we remove the columns from compute_nodes,"},{"line_number":330,"context_line":"        # we need to constantly strip out resource updates"},{"line_number":331,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_bc480e5d","line":328,"in_reply_to":"9a061dce_b21de4bf","updated":"2016-04-04 16:22:21.000000000","message":"Isn\u0027t this what\u0027s called _migrate_inventory in the spec?\n\nhttps://review.openstack.org/#/c/300175/2/specs/newton/approved/compute-node-inventory-newton.rst@152","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"226cdd5b76b9c3c10fff3df7b4eb9ebd357c08e1","unresolved":false,"context_lines":[{"line_number":325,"context_line":"                                                            related_binaries)"},{"line_number":326,"context_line":"        return min_ver \u003e\u003d required_version"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"    def _update_inventory(self, updates):"},{"line_number":329,"context_line":"        # NOTE(danms): Until we remove the columns from compute_nodes,"},{"line_number":330,"context_line":"        # we need to constantly strip out resource updates"},{"line_number":331,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_2a6aabb3","line":328,"in_reply_to":"9a061dce_bc480e5d","updated":"2016-04-05 19:13:11.000000000","message":"No, because we call this on every save from here on out. Update is the right word, IMHO.","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6784a275f95a0a2b7a904192fc73aa3c23b195fa","unresolved":false,"context_lines":[{"line_number":352,"context_line":"                updates[key] \u003d 0"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"                inventory.save()"},{"line_number":355,"context_line":"        return True"},{"line_number":356,"context_line":""},{"line_number":357,"context_line":"    def _create_inventory(self, updates):"},{"line_number":358,"context_line":"        \"\"\"Create the initial inventory objects for this compute node."}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_d3ce9082","line":355,"updated":"2016-04-04 17:05:06.000000000","message":"What if we didn\u0027t hit vcpu/ram/disk in the loop above, we just continue\u0027ed on all, we\u0027d return True here even though we didn\u0027t do anything. Probably unlikely, but we should probably check that?","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"662230059467976e33a446331b6cd0d27a25f307","unresolved":false,"context_lines":[{"line_number":352,"context_line":"                updates[key] \u003d 0"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"                inventory.save()"},{"line_number":355,"context_line":"        return True"},{"line_number":356,"context_line":""},{"line_number":357,"context_line":"    def _create_inventory(self, updates):"},{"line_number":358,"context_line":"        \"\"\"Create the initial inventory objects for this compute node."}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_f51b4bc8","line":355,"in_reply_to":"9a061dce_d3ce9082","updated":"2016-04-06 13:43:15.000000000","message":"No, I don\u0027t think so. We don\u0027t have to update these values every time, so I\u0027m not sure why running to the end here is a failure. The return value is to signify that we updated the inventories if they existed. If we returned False here then we would run create_inventory again even though we don\u0027t need to, if we had no updates for the resources right?","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"da2ff13a0db5149430396a45f1108fd5774f3494","unresolved":false,"context_lines":[{"line_number":352,"context_line":"                updates[key] \u003d 0"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"                inventory.save()"},{"line_number":355,"context_line":"        return True"},{"line_number":356,"context_line":""},{"line_number":357,"context_line":"    def _create_inventory(self, updates):"},{"line_number":358,"context_line":"        \"\"\"Create the initial inventory objects for this compute node."}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_ac44e01a","line":355,"in_reply_to":"9a061dce_f51b4bc8","updated":"2016-04-06 16:42:32.000000000","message":"OK, I wasn\u0027t considering the tight-coupling with _create_inventory, so yeah, then it makes more sense. Maybe we should have a comment in this method explaining the return code.","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5b67c130d4f3b1e59608d395480e77334b424bdb","unresolved":false,"context_lines":[{"line_number":361,"context_line":"        is created, or after an upgrade where the required services have"},{"line_number":362,"context_line":"        reached the required version."},{"line_number":363,"context_line":"        \"\"\""},{"line_number":364,"context_line":"        rp \u003d objects.ResourceProvider(context\u003dself._context, uuid\u003dself.uuid)"},{"line_number":365,"context_line":"        rp.create()"},{"line_number":366,"context_line":""},{"line_number":367,"context_line":"        # NOTE(danms): Until we remove the columns from compute_nodes,"},{"line_number":368,"context_line":"        # we need to constantly strip out resource updates"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_77ffb134","line":365,"range":{"start_line":364,"start_character":8,"end_line":365,"end_character":19},"updated":"2016-04-04 17:00:09.000000000","message":"OK, so this is step 3 in the spec:\n\nhttps://review.openstack.org/#/c/300175/2/specs/newton/approved/compute-node-inventory-newton.rst@160\n\nCool.","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5b67c130d4f3b1e59608d395480e77334b424bdb","unresolved":false,"context_lines":[{"line_number":369,"context_line":""},{"line_number":370,"context_line":"        cpu \u003d objects.Inventory(context\u003dself._context,"},{"line_number":371,"context_line":"                                resource_provider\u003drp,"},{"line_number":372,"context_line":"                                resource_class\u003dfields.ResourceClass.VCPU,"},{"line_number":373,"context_line":"                                total\u003dself.vcpus,"},{"line_number":374,"context_line":"                                reserved\u003d0,"},{"line_number":375,"context_line":"                                min_unit\u003d1,"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_f3b6343c","line":372,"range":{"start_line":372,"start_character":32,"end_line":372,"end_character":72},"updated":"2016-04-04 17:00:09.000000000","message":"I guess the spec is outdated, since it says this should be an id to a resource class entry in the database,\n\nhttps://review.openstack.org/#/c/300175/2/specs/newton/approved/compute-node-inventory-newton.rst@171\n\n\"Grab the resource class identifier for CPU from the\n`resource_classes` table (see `resource-classes` blueprint).\"","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"226cdd5b76b9c3c10fff3df7b4eb9ebd357c08e1","unresolved":false,"context_lines":[{"line_number":369,"context_line":""},{"line_number":370,"context_line":"        cpu \u003d objects.Inventory(context\u003dself._context,"},{"line_number":371,"context_line":"                                resource_provider\u003drp,"},{"line_number":372,"context_line":"                                resource_class\u003dfields.ResourceClass.VCPU,"},{"line_number":373,"context_line":"                                total\u003dself.vcpus,"},{"line_number":374,"context_line":"                                reserved\u003d0,"},{"line_number":375,"context_line":"                                min_unit\u003d1,"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_b54e1c6d","line":372,"range":{"start_line":372,"start_character":32,"end_line":372,"end_character":72},"in_reply_to":"9a061dce_f3b6343c","updated":"2016-04-05 19:13:11.000000000","message":"Yeah, Jay originally spec\u0027d that we\u0027d store an arbitrary mapping in the DB, but we decided not to do that in Bristol.","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f379acd6610ceef3ed0b91f4ecd88e9d529dd35d","unresolved":false,"context_lines":[{"line_number":375,"context_line":"                                min_unit\u003d1,"},{"line_number":376,"context_line":"                                max_unit\u003d1,"},{"line_number":377,"context_line":"                                step_size\u003d1,"},{"line_number":378,"context_line":"                                allocation_ratio\u003d1.0)"},{"line_number":379,"context_line":"        cpu.create()"},{"line_number":380,"context_line":"        updates[\u0027vcpus\u0027] \u003d 0"},{"line_number":381,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_190cbe93","line":378,"updated":"2016-04-04 11:29:52.000000000","message":"should be self.cpu_allocation_ratio I think?","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ea40f62e8a38edb7267da45f869f81cfc59f9c4b","unresolved":false,"context_lines":[{"line_number":383,"context_line":"                                resource_provider\u003drp,"},{"line_number":384,"context_line":"                                resource_class\u003dfields.ResourceClass.MEMORY_MB,"},{"line_number":385,"context_line":"                                total\u003dself.memory_mb,"},{"line_number":386,"context_line":"                                reserved\u003d0,"},{"line_number":387,"context_line":"                                min_unit\u003d1,"},{"line_number":388,"context_line":"                                max_unit\u003d1,"},{"line_number":389,"context_line":"                                step_size\u003d1,"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_19a5de03","line":386,"updated":"2016-04-04 11:16:51.000000000","message":"From previous patches, there is concern about this ignoring the CONF.reserved_host_memory_mb value that could be set on the compute node.","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f379acd6610ceef3ed0b91f4ecd88e9d529dd35d","unresolved":false,"context_lines":[{"line_number":387,"context_line":"                                min_unit\u003d1,"},{"line_number":388,"context_line":"                                max_unit\u003d1,"},{"line_number":389,"context_line":"                                step_size\u003d1,"},{"line_number":390,"context_line":"                                allocation_ratio\u003d1.0)"},{"line_number":391,"context_line":"        mem.create()"},{"line_number":392,"context_line":"        updates[\u0027memory_mb\u0027] \u003d 0"},{"line_number":393,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_59263617","line":390,"updated":"2016-04-04 11:29:52.000000000","message":"as above, but ram_","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5b67c130d4f3b1e59608d395480e77334b424bdb","unresolved":false,"context_lines":[{"line_number":392,"context_line":"        updates[\u0027memory_mb\u0027] \u003d 0"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"        disk \u003d objects.Inventory(context\u003dself._context,"},{"line_number":395,"context_line":"                                resource_provider\u003drp,"},{"line_number":396,"context_line":"                                resource_class\u003dfields.ResourceClass.DISK_GB,"},{"line_number":397,"context_line":"                                total\u003dself.local_gb,"},{"line_number":398,"context_line":"                                reserved\u003d0,"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_3749b98c","line":395,"updated":"2016-04-04 17:00:09.000000000","message":"Alignment nit.","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5b67c130d4f3b1e59608d395480e77334b424bdb","unresolved":false,"context_lines":[{"line_number":393,"context_line":""},{"line_number":394,"context_line":"        disk \u003d objects.Inventory(context\u003dself._context,"},{"line_number":395,"context_line":"                                resource_provider\u003drp,"},{"line_number":396,"context_line":"                                resource_class\u003dfields.ResourceClass.DISK_GB,"},{"line_number":397,"context_line":"                                total\u003dself.local_gb,"},{"line_number":398,"context_line":"                                reserved\u003d0,"},{"line_number":399,"context_line":"                                min_unit\u003d1,"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_9320989d","line":396,"range":{"start_line":396,"start_character":68,"end_line":396,"end_character":75},"updated":"2016-04-04 17:00:09.000000000","message":"The spec says:\n\nhttps://review.openstack.org/#/c/300175/2/specs/newton/approved/compute-node-inventory-newton.rst@197\n\n\"For the DISK resource class, do not perform the INSERT if the compute node uses shared storage for the ephemeral disks.\"\n\nI don\u0027t see that check happening here.","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"da2ff13a0db5149430396a45f1108fd5774f3494","unresolved":false,"context_lines":[{"line_number":393,"context_line":""},{"line_number":394,"context_line":"        disk \u003d objects.Inventory(context\u003dself._context,"},{"line_number":395,"context_line":"                                resource_provider\u003drp,"},{"line_number":396,"context_line":"                                resource_class\u003dfields.ResourceClass.DISK_GB,"},{"line_number":397,"context_line":"                                total\u003dself.local_gb,"},{"line_number":398,"context_line":"                                reserved\u003d0,"},{"line_number":399,"context_line":"                                min_unit\u003d1,"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_2c68d086","line":396,"range":{"start_line":396,"start_character":68,"end_line":396,"end_character":75},"in_reply_to":"9a061dce_551af77d","updated":"2016-04-06 16:42:32.000000000","message":"OK, that makes sense, but we should probably have a TODO in here then for the shared storage stuff to come later.","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cc3cca07097beb9145b84174bf8bdfea04e3a616","unresolved":false,"context_lines":[{"line_number":393,"context_line":""},{"line_number":394,"context_line":"        disk \u003d objects.Inventory(context\u003dself._context,"},{"line_number":395,"context_line":"                                resource_provider\u003drp,"},{"line_number":396,"context_line":"                                resource_class\u003dfields.ResourceClass.DISK_GB,"},{"line_number":397,"context_line":"                                total\u003dself.local_gb,"},{"line_number":398,"context_line":"                                reserved\u003d0,"},{"line_number":399,"context_line":"                                min_unit\u003d1,"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_551af77d","line":396,"range":{"start_line":396,"start_character":68,"end_line":396,"end_character":75},"in_reply_to":"9a061dce_9320989d","updated":"2016-04-06 13:52:08.000000000","message":"So, I\u0027m not sure that (a) we need to do that here and (b) that we\u0027re ready for it.\n\nIf the compute node is reporting disk, I think we need to record it as inventory. What we need to do is get the compute node to stop reporting disk that it doesn\u0027t have locally, since that will be reported by an external entity. We should get jay to comment here.\n\nIf we didn\u0027t do this, we wouldn\u0027t be able to transition to this in the immediate term, using these inventory records immediately while we transition to externally-reported values for shared resources.","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ea40f62e8a38edb7267da45f869f81cfc59f9c4b","unresolved":false,"context_lines":[{"line_number":395,"context_line":"                                resource_provider\u003drp,"},{"line_number":396,"context_line":"                                resource_class\u003dfields.ResourceClass.DISK_GB,"},{"line_number":397,"context_line":"                                total\u003dself.local_gb,"},{"line_number":398,"context_line":"                                reserved\u003d0,"},{"line_number":399,"context_line":"                                min_unit\u003d1,"},{"line_number":400,"context_line":"                                max_unit\u003d1,"},{"line_number":401,"context_line":"                                step_size\u003d1,"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_39ac22ed","line":398,"updated":"2016-04-04 11:16:51.000000000","message":"As above, but the reserved disk value.","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f379acd6610ceef3ed0b91f4ecd88e9d529dd35d","unresolved":false,"context_lines":[{"line_number":399,"context_line":"                                min_unit\u003d1,"},{"line_number":400,"context_line":"                                max_unit\u003d1,"},{"line_number":401,"context_line":"                                step_size\u003d1,"},{"line_number":402,"context_line":"                                allocation_ratio\u003d1.0)"},{"line_number":403,"context_line":"        disk.create()"},{"line_number":404,"context_line":"        updates[\u0027local_gb\u0027] \u003d 0"},{"line_number":405,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_39238207","line":402,"updated":"2016-04-04 11:29:52.000000000","message":"as above but disk","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"472edf1a66165f962db6fdab4d83481d0ba3445e","unresolved":false,"context_lines":[{"line_number":424,"context_line":"        db_compute \u003d db.compute_node_create(self._context, updates)"},{"line_number":425,"context_line":"        # NOTE(danms): compute_node_create() operates on (and returns) the"},{"line_number":426,"context_line":"        # compute node model only. We need to get the full inventory-based"},{"line_number":427,"context_line":"        # result in order to satisfy _from_db_compute(). So, we do a double"},{"line_number":428,"context_line":"        # query here. This can be removed in Newton once we\u0027re sure that all"},{"line_number":429,"context_line":"        # compute nodes are inventory-based"},{"line_number":430,"context_line":"        db_compute \u003d db.compute_node_get(self._context, db_compute[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_535e00ea","line":427,"range":{"start_line":427,"start_character":37,"end_line":427,"end_character":53},"updated":"2016-04-04 17:02:26.000000000","message":"You mean _from_db_object?","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"226cdd5b76b9c3c10fff3df7b4eb9ebd357c08e1","unresolved":false,"context_lines":[{"line_number":424,"context_line":"        db_compute \u003d db.compute_node_create(self._context, updates)"},{"line_number":425,"context_line":"        # NOTE(danms): compute_node_create() operates on (and returns) the"},{"line_number":426,"context_line":"        # compute node model only. We need to get the full inventory-based"},{"line_number":427,"context_line":"        # result in order to satisfy _from_db_compute(). So, we do a double"},{"line_number":428,"context_line":"        # query here. This can be removed in Newton once we\u0027re sure that all"},{"line_number":429,"context_line":"        # compute nodes are inventory-based"},{"line_number":430,"context_line":"        db_compute \u003d db.compute_node_get(self._context, db_compute[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_95515811","line":427,"range":{"start_line":427,"start_character":37,"end_line":427,"end_character":53},"in_reply_to":"9a061dce_535e00ea","updated":"2016-04-05 19:13:11.000000000","message":"Yes","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"472edf1a66165f962db6fdab4d83481d0ba3445e","unresolved":false,"context_lines":[{"line_number":448,"context_line":"        db_compute \u003d db.compute_node_update(self._context, self.id, updates)"},{"line_number":449,"context_line":"        # NOTE(danms): compute_node_update() operates on (and returns) the"},{"line_number":450,"context_line":"        # compute node model only. We need to get the full inventory-based"},{"line_number":451,"context_line":"        # result in order to satisfy _from_db_compute(). So, we do a double"},{"line_number":452,"context_line":"        # query here. This can be removed in Newton once we\u0027re sure that all"},{"line_number":453,"context_line":"        # compute nodes are inventory-based"},{"line_number":454,"context_line":"        db_compute \u003d db.compute_node_get(self._context, self.id)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_935898fc","line":451,"range":{"start_line":451,"start_character":37,"end_line":451,"end_character":53},"updated":"2016-04-04 17:02:26.000000000","message":"_from_db_object?","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f0c39041165b5b3377a806e54cd916742e931994","unresolved":false,"context_lines":[{"line_number":208,"context_line":"            setattr(compute, key, value)"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"        for key in (\u0027vcpus\u0027, \u0027local_gb\u0027, \u0027memory_mb\u0027):"},{"line_number":211,"context_line":"            inv_key \u003d \u0027inv_%s\u0027 % key"},{"line_number":212,"context_line":"            if inv_key in db_compute and db_compute[inv_key] is not None:"},{"line_number":213,"context_line":"                setattr(compute, key, db_compute[inv_key])"},{"line_number":214,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a061dce_0ccd0c66","line":211,"range":{"start_line":211,"start_character":23,"end_line":211,"end_character":27},"updated":"2016-04-06 16:46:33.000000000","message":"What sets this key with the special \u0027inv_\u0027 prefix in the compute_nodes table?","commit_id":"75204187ea0796e4eb32e1823a47353445031e25"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9a4ad1df3bc32ef77a54a0f372d00e75ad29dc02","unresolved":false,"context_lines":[{"line_number":208,"context_line":"            setattr(compute, key, value)"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"        for key in (\u0027vcpus\u0027, \u0027local_gb\u0027, \u0027memory_mb\u0027):"},{"line_number":211,"context_line":"            inv_key \u003d \u0027inv_%s\u0027 % key"},{"line_number":212,"context_line":"            if inv_key in db_compute and db_compute[inv_key] is not None:"},{"line_number":213,"context_line":"                setattr(compute, key, db_compute[inv_key])"},{"line_number":214,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a061dce_2cee10b4","line":211,"range":{"start_line":211,"start_character":23,"end_line":211,"end_character":27},"in_reply_to":"9a061dce_0ccd0c66","updated":"2016-04-06 16:47:48.000000000","message":"Oh I guess the mega join in _compute_node_select.","commit_id":"75204187ea0796e4eb32e1823a47353445031e25"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0e9e8a84162f1ee590e6bfeca03905c373afc925","unresolved":false,"context_lines":[{"line_number":238,"context_line":"        # NOTE(danms): Remove this conditional load (and remove uuid from"},{"line_number":239,"context_line":"        # the list of special_cases above) once we\u0027re in Newton and have"},{"line_number":240,"context_line":"        # enforced that all UUIDs in the database are not NULL."},{"line_number":241,"context_line":"        if db_compute.get(\u0027uuid\u0027) and \u0027uuid\u0027 not in compute:"},{"line_number":242,"context_line":"            compute.uuid \u003d db_compute[\u0027uuid\u0027]"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"        compute.obj_reset_changes()"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a061dce_a70fdb86","line":241,"updated":"2016-04-06 17:12:10.000000000","message":"This seems like a separate cleanup that could be split out, but I\u0027m not sure.","commit_id":"75204187ea0796e4eb32e1823a47353445031e25"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"889a5e3d4e36f16b297f91faa75f8b4ce1af373b","unresolved":false,"context_lines":[{"line_number":238,"context_line":"        # NOTE(danms): Remove this conditional load (and remove uuid from"},{"line_number":239,"context_line":"        # the list of special_cases above) once we\u0027re in Newton and have"},{"line_number":240,"context_line":"        # enforced that all UUIDs in the database are not NULL."},{"line_number":241,"context_line":"        if db_compute.get(\u0027uuid\u0027) and \u0027uuid\u0027 not in compute:"},{"line_number":242,"context_line":"            compute.uuid \u003d db_compute[\u0027uuid\u0027]"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"        compute.obj_reset_changes()"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a061dce_07616705","line":241,"in_reply_to":"9a061dce_a70fdb86","updated":"2016-04-06 17:14:53.000000000","message":"We can actually remove this anyway per the comment above I think.","commit_id":"75204187ea0796e4eb32e1823a47353445031e25"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5f1cf3e3cb9a0f9eba7935b1e565dfb8423f8786","unresolved":false,"context_lines":[{"line_number":321,"context_line":"                                                            related_binaries)"},{"line_number":322,"context_line":"        return min_ver \u003e\u003d required_version"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"    def _update_inventory(self, updates):"},{"line_number":325,"context_line":"        # NOTE(danms): Until we remove the columns from compute_nodes,"},{"line_number":326,"context_line":"        # we need to constantly strip out resource updates"},{"line_number":327,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9a061dce_2ced70ef","line":324,"updated":"2016-04-06 16:44:18.000000000","message":"Per the discussion in the previous patch set, I think it might be useful to have a comment explaining what this method is used for and how the return value should be used, i.e. coupling with _create_inventory.","commit_id":"75204187ea0796e4eb32e1823a47353445031e25"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"889a5e3d4e36f16b297f91faa75f8b4ce1af373b","unresolved":false,"context_lines":[{"line_number":321,"context_line":"                                                            related_binaries)"},{"line_number":322,"context_line":"        return min_ver \u003e\u003d required_version"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"    def _update_inventory(self, updates):"},{"line_number":325,"context_line":"        # NOTE(danms): Until we remove the columns from compute_nodes,"},{"line_number":326,"context_line":"        # we need to constantly strip out resource updates"},{"line_number":327,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9a061dce_2702ab0b","line":324,"in_reply_to":"9a061dce_2ced70ef","updated":"2016-04-06 17:14:53.000000000","message":"Sure.","commit_id":"75204187ea0796e4eb32e1823a47353445031e25"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5c285b1330caf6e9f3cd4398f6f1c71dbcf01c94","unresolved":false,"context_lines":[{"line_number":373,"context_line":"                                step_size\u003d1,"},{"line_number":374,"context_line":"                                allocation_ratio\u003dself.cpu_allocation_ratio)"},{"line_number":375,"context_line":"        cpu.create()"},{"line_number":376,"context_line":"        updates[\u0027vcpus\u0027] \u003d 0"},{"line_number":377,"context_line":""},{"line_number":378,"context_line":"        mem \u003d objects.Inventory(context\u003dself._context,"},{"line_number":379,"context_line":"                                resource_provider\u003drp,"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a061dce_47c35f38","line":376,"range":{"start_line":376,"start_character":8,"end_line":376,"end_character":28},"updated":"2016-04-06 17:07:58.000000000","message":"So when you say \u0027strip out\u0027 above, I was thinking that meant actually popping the values from updates dict. Are the values supposed to be deltas and that\u0027s why it\u0027s reset to 0?","commit_id":"75204187ea0796e4eb32e1823a47353445031e25"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"889a5e3d4e36f16b297f91faa75f8b4ce1af373b","unresolved":false,"context_lines":[{"line_number":373,"context_line":"                                step_size\u003d1,"},{"line_number":374,"context_line":"                                allocation_ratio\u003dself.cpu_allocation_ratio)"},{"line_number":375,"context_line":"        cpu.create()"},{"line_number":376,"context_line":"        updates[\u0027vcpus\u0027] \u003d 0"},{"line_number":377,"context_line":""},{"line_number":378,"context_line":"        mem \u003d objects.Inventory(context\u003dself._context,"},{"line_number":379,"context_line":"                                resource_provider\u003drp,"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a061dce_a7151b4a","line":376,"range":{"start_line":376,"start_character":8,"end_line":376,"end_character":28},"in_reply_to":"9a061dce_47c35f38","updated":"2016-04-06 17:14:53.000000000","message":"We need to zero them so we know they have been migrated. They\u0027re not deltas.","commit_id":"75204187ea0796e4eb32e1823a47353445031e25"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6565cde46bdb2cc5fb92e6f3d8d83fb594ea0c97","unresolved":false,"context_lines":[{"line_number":373,"context_line":"                                step_size\u003d1,"},{"line_number":374,"context_line":"                                allocation_ratio\u003dself.cpu_allocation_ratio)"},{"line_number":375,"context_line":"        cpu.create()"},{"line_number":376,"context_line":"        updates[\u0027vcpus\u0027] \u003d 0"},{"line_number":377,"context_line":""},{"line_number":378,"context_line":"        mem \u003d objects.Inventory(context\u003dself._context,"},{"line_number":379,"context_line":"                                resource_provider\u003drp,"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a061dce_a63d1c91","line":376,"range":{"start_line":376,"start_character":8,"end_line":376,"end_character":28},"in_reply_to":"9a061dce_63e01697","updated":"2016-04-06 17:36:44.000000000","message":"In this code, we know that things are new by the non-null ness of the inv_ fields in from_db_object. However, later, we will need a blocker migration and a migration to drop these columns. Those migrations will need to make a simple assertion that all these columns are zero, meaning it\u0027s safe to move on.","commit_id":"75204187ea0796e4eb32e1823a47353445031e25"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4617d78b0bcadaad9fe7dd80953c5f5cb2a0f6ae","unresolved":false,"context_lines":[{"line_number":373,"context_line":"                                step_size\u003d1,"},{"line_number":374,"context_line":"                                allocation_ratio\u003dself.cpu_allocation_ratio)"},{"line_number":375,"context_line":"        cpu.create()"},{"line_number":376,"context_line":"        updates[\u0027vcpus\u0027] \u003d 0"},{"line_number":377,"context_line":""},{"line_number":378,"context_line":"        mem \u003d objects.Inventory(context\u003dself._context,"},{"line_number":379,"context_line":"                                resource_provider\u003drp,"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a061dce_60711f44","line":376,"range":{"start_line":376,"start_character":8,"end_line":376,"end_character":28},"in_reply_to":"9a061dce_a63d1c91","updated":"2016-04-06 21:49:52.000000000","message":"OK, \"However, later, we will need a blocker migration and a migration to drop these columns. Those migrations will need to make a simple assertion that all these columns are zero, meaning it\u0027s safe to move on.\" was what I was assuming was the case, i.e. 0 is more like a sentinel value that we use later. I just wasn\u0027t sure because of the comments about stripping out updates. Maybe we put a comment on that somewhere?","commit_id":"75204187ea0796e4eb32e1823a47353445031e25"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"913c87f1a5ade026e3cf24d33f5f5f8a26ec9de5","unresolved":false,"context_lines":[{"line_number":373,"context_line":"                                step_size\u003d1,"},{"line_number":374,"context_line":"                                allocation_ratio\u003dself.cpu_allocation_ratio)"},{"line_number":375,"context_line":"        cpu.create()"},{"line_number":376,"context_line":"        updates[\u0027vcpus\u0027] \u003d 0"},{"line_number":377,"context_line":""},{"line_number":378,"context_line":"        mem \u003d objects.Inventory(context\u003dself._context,"},{"line_number":379,"context_line":"                                resource_provider\u003drp,"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a061dce_63e01697","line":376,"range":{"start_line":376,"start_character":8,"end_line":376,"end_character":28},"in_reply_to":"9a061dce_a7151b4a","updated":"2016-04-06 17:26:36.000000000","message":"But where is the logic that checks to see if the values are 0 and uses that as the indicator that they are migrated? Or is that a later change?","commit_id":"75204187ea0796e4eb32e1823a47353445031e25"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c576817c3d0c42253a5e173a172fd67d19fe3872","unresolved":false,"context_lines":[{"line_number":379,"context_line":"                                resource_provider\u003drp,"},{"line_number":380,"context_line":"                                resource_class\u003dfields.ResourceClass.MEMORY_MB,"},{"line_number":381,"context_line":"                                total\u003dself.memory_mb,"},{"line_number":382,"context_line":"                                reserved\u003d0,"},{"line_number":383,"context_line":"                                min_unit\u003d1,"},{"line_number":384,"context_line":"                                max_unit\u003d1,"},{"line_number":385,"context_line":"                                step_size\u003d1,"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a061dce_f17b5126","line":382,"updated":"2016-04-06 16:25:36.000000000","message":"In the last patch, John asked: \n\n From previous patches, there is concern about this ignoring the\n CONF.reserved_host_memory_mb","commit_id":"75204187ea0796e4eb32e1823a47353445031e25"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c576817c3d0c42253a5e173a172fd67d19fe3872","unresolved":false,"context_lines":[{"line_number":389,"context_line":""},{"line_number":390,"context_line":"        disk \u003d objects.Inventory(context\u003dself._context,"},{"line_number":391,"context_line":"                                 resource_provider\u003drp,"},{"line_number":392,"context_line":"                                 resource_class\u003dfields.ResourceClass.DISK_GB,"},{"line_number":393,"context_line":"                                 total\u003dself.local_gb,"},{"line_number":394,"context_line":"                                 reserved\u003d0,"},{"line_number":395,"context_line":"                                 min_unit\u003d1,"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a061dce_51c31d38","line":392,"updated":"2016-04-06 16:25:36.000000000","message":"In a previous patch, Matt said:\n\n The spec says:\n https://review.openstack.org/#/c/300175/2/specs/newton/approved/compute-node-inventory-newton.rst@197\n \"For the DISK resource class, do not perform the INSERT if the \n compute node uses shared storage for the ephemeral disks.\"\n I don\u0027t see that check happening here.\n\nand I said:\n\n So, I\u0027m not sure that (a) we need to do that here and (b) that we\u0027re \n ready for it. If the compute node is reporting disk, I think we need to\n record it as inventory. What we need to do is get the compute node to\n stop reporting disk that it doesn\u0027t have locally, since that will be\n reported by an external entity. We should get jay to comment here.\n If we didn\u0027t do this, we wouldn\u0027t be able to transition to this in the\n immediate term, using these inventory records immediately while we\n transition to externally-reported values for shared resources.","commit_id":"75204187ea0796e4eb32e1823a47353445031e25"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5f1cf3e3cb9a0f9eba7935b1e565dfb8423f8786","unresolved":false,"context_lines":[{"line_number":389,"context_line":""},{"line_number":390,"context_line":"        disk \u003d objects.Inventory(context\u003dself._context,"},{"line_number":391,"context_line":"                                 resource_provider\u003drp,"},{"line_number":392,"context_line":"                                 resource_class\u003dfields.ResourceClass.DISK_GB,"},{"line_number":393,"context_line":"                                 total\u003dself.local_gb,"},{"line_number":394,"context_line":"                                 reserved\u003d0,"},{"line_number":395,"context_line":"                                 min_unit\u003d1,"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a061dce_eccd3886","line":392,"in_reply_to":"9a061dce_51c31d38","updated":"2016-04-06 16:44:18.000000000","message":"That makes more sense to me, but I think we should have a TODO in here to adjust this later for the shared storage case.","commit_id":"75204187ea0796e4eb32e1823a47353445031e25"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5c285b1330caf6e9f3cd4398f6f1c71dbcf01c94","unresolved":false,"context_lines":[{"line_number":443,"context_line":"                self._create_inventory(updates)"},{"line_number":444,"context_line":"        db_compute \u003d db.compute_node_update(self._context, self.id, updates)"},{"line_number":445,"context_line":"        # NOTE(danms): compute_node_update() operates on (and returns) the"},{"line_number":446,"context_line":"        # compute node model only. We need to get the full inventory-based"},{"line_number":447,"context_line":"        # result in order to satisfy _from_db_object(). So, we do a double"},{"line_number":448,"context_line":"        # query here. This can be removed in Newton once we\u0027re sure that all"},{"line_number":449,"context_line":"        # compute nodes are inventory-based"},{"line_number":450,"context_line":"        db_compute \u003d db.compute_node_get(self._context, self.id)"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a061dce_47751f30","line":447,"range":{"start_line":446,"start_character":35,"end_line":447,"end_character":55},"updated":"2016-04-06 17:07:58.000000000","message":"Ah and this is what does the mega join in _compute_node_select which gives us the special inv_ keys that are used in _from_db_object(), now I\u0027m catching on.","commit_id":"75204187ea0796e4eb32e1823a47353445031e25"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ecc2904d52cc995977e4263f9d978f0df2f81c99","unresolved":false,"context_lines":[{"line_number":304,"context_line":"                                                            related_binaries)"},{"line_number":305,"context_line":"        return min_ver \u003e\u003d required_version"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"    def _update_inventory(self, updates):"},{"line_number":308,"context_line":"        # NOTE(danms): Here we update our inventory records with our"},{"line_number":309,"context_line":"        # resource information. Since this information is prepared in"},{"line_number":310,"context_line":"        # updates against our older compute_node columns, we need to"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a1b2928_63608c36","line":307,"updated":"2016-04-11 14:29:40.000000000","message":"Please add a docstring explaining what the updates parameter is and that this method inline updates that parameter\u0027s values.","commit_id":"b4e1989ae687e7467d179a3c230d948aba7247fe"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ecc2904d52cc995977e4263f9d978f0df2f81c99","unresolved":false,"context_lines":[{"line_number":352,"context_line":""},{"line_number":353,"context_line":"        # NOTE(danms): Until we remove the columns from compute_nodes,"},{"line_number":354,"context_line":"        # we need to constantly zero out each value in our updates to"},{"line_number":355,"context_line":"        # signal that we wrote the value into inventory instead."},{"line_number":356,"context_line":""},{"line_number":357,"context_line":"        cpu \u003d objects.Inventory(context\u003dself._context,"},{"line_number":358,"context_line":"                                resource_provider\u003drp,"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a1b2928_43659043","line":355,"updated":"2016-04-11 14:29:40.000000000","message":"I think this comment above belongs somewhere else? I think it belongs above where _update_inventory() is called, no?","commit_id":"b4e1989ae687e7467d179a3c230d948aba7247fe"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9fa44f4453d4f853548e4e91355e2fbd91ca3fd2","unresolved":false,"context_lines":[{"line_number":352,"context_line":""},{"line_number":353,"context_line":"        # NOTE(danms): Until we remove the columns from compute_nodes,"},{"line_number":354,"context_line":"        # we need to constantly zero out each value in our updates to"},{"line_number":355,"context_line":"        # signal that we wrote the value into inventory instead."},{"line_number":356,"context_line":""},{"line_number":357,"context_line":"        cpu \u003d objects.Inventory(context\u003dself._context,"},{"line_number":358,"context_line":"                                resource_provider\u003drp,"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a1b2928_63636c1b","line":355,"in_reply_to":"7a1b2928_43659043","updated":"2016-04-11 14:31:50.000000000","message":"No, it belongs here (and in update inventory), per previous reviews. We\u0027re zeroing them in both places (see L367 for example).","commit_id":"b4e1989ae687e7467d179a3c230d948aba7247fe"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"67d83dbd7e60e2d1c743272f5e207c344f96d7f8","unresolved":false,"context_lines":[{"line_number":370,"context_line":"                                resource_provider\u003drp,"},{"line_number":371,"context_line":"                                resource_class\u003dfields.ResourceClass.MEMORY_MB,"},{"line_number":372,"context_line":"                                total\u003dself.memory_mb,"},{"line_number":373,"context_line":"                                reserved\u003d0,"},{"line_number":374,"context_line":"                                min_unit\u003d1,"},{"line_number":375,"context_line":"                                max_unit\u003d1,"},{"line_number":376,"context_line":"                                step_size\u003d1,"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a1b2928_ba376de2","line":373,"range":{"start_line":373,"start_character":32,"end_line":373,"end_character":40},"updated":"2016-04-11 13:41:03.000000000","message":"Guess we still need to answer:\n\n \"From previous patches, there is concern about this ignoring the\n CONF.reserved_host_memory_mb\" ?","commit_id":"b4e1989ae687e7467d179a3c230d948aba7247fe"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"610de45cf1e8d4c1674143546117406413239d3d","unresolved":false,"context_lines":[{"line_number":370,"context_line":"                                resource_provider\u003drp,"},{"line_number":371,"context_line":"                                resource_class\u003dfields.ResourceClass.MEMORY_MB,"},{"line_number":372,"context_line":"                                total\u003dself.memory_mb,"},{"line_number":373,"context_line":"                                reserved\u003d0,"},{"line_number":374,"context_line":"                                min_unit\u003d1,"},{"line_number":375,"context_line":"                                max_unit\u003d1,"},{"line_number":376,"context_line":"                                step_size\u003d1,"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a1b2928_7d9e6f57","line":373,"range":{"start_line":373,"start_character":32,"end_line":373,"end_character":40},"in_reply_to":"7a1b2928_ba376de2","updated":"2016-04-11 13:59:08.000000000","message":"My comment from before was roughly:\n\nI think that the compute nodes are reporting a total that includes the reserved amount removed. That means we won\u0027t actually have trouble with this right now for these values, although they will be represented a little differently. For other things (external disk pools for example) the reserved/total values will be right and then potentially later we can have the compute nodes doing the inventory updates themselves and include the proper reserved/total amounts.","commit_id":"b4e1989ae687e7467d179a3c230d948aba7247fe"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"89642927513be904f10420636ac4468ddd4cdd20","unresolved":false,"context_lines":[{"line_number":436,"context_line":"        if self._should_manage_inventory():"},{"line_number":437,"context_line":"            if not self._update_inventory(updates):"},{"line_number":438,"context_line":"                # NOTE(danms): This only happens once"},{"line_number":439,"context_line":"                self._create_inventory(updates)"},{"line_number":440,"context_line":"        db_compute \u003d db.compute_node_update(self._context, self.id, updates)"},{"line_number":441,"context_line":"        # NOTE(danms): compute_node_update() operates on (and returns) the"},{"line_number":442,"context_line":"        # compute node model only. We need to get the full inventory-based"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a1b2928_fd48bf5d","line":439,"updated":"2016-04-11 13:53:13.000000000","message":"Would be good to have a test in https://review.openstack.org/#/c/279313/16/nova/tests/functional/db/test_compute_node.py for this case where the service version is 10 so we should manage inventory, but the inventory records don\u0027t exist yet, so this creates them.","commit_id":"b4e1989ae687e7467d179a3c230d948aba7247fe"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"36e1f0f9afed027f5eb32869624359efa8ed0763","unresolved":false,"context_lines":[{"line_number":436,"context_line":"        if self._should_manage_inventory():"},{"line_number":437,"context_line":"            if not self._update_inventory(updates):"},{"line_number":438,"context_line":"                # NOTE(danms): This only happens once"},{"line_number":439,"context_line":"                self._create_inventory(updates)"},{"line_number":440,"context_line":"        db_compute \u003d db.compute_node_update(self._context, self.id, updates)"},{"line_number":441,"context_line":"        # NOTE(danms): compute_node_update() operates on (and returns) the"},{"line_number":442,"context_line":"        # compute node model only. We need to get the full inventory-based"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a1b2928_fda71f4d","line":439,"in_reply_to":"7a1b2928_fd48bf5d","updated":"2016-04-11 14:04:40.000000000","message":"Done","commit_id":"b4e1989ae687e7467d179a3c230d948aba7247fe"}],"nova/objects/service.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"004079c59158c167a31a4e730695948c74ca0f7f","unresolved":false,"context_lines":[{"line_number":70,"context_line":"    # Version 8: Add live_migration_abort in the compute_rpc"},{"line_number":71,"context_line":"    {\u0027compute_rpc\u0027: \u00274.10\u0027},"},{"line_number":72,"context_line":"    # Version 9: Compute node conversion to Inventories"},{"line_number":73,"context_line":"    {\u0027compute_rpc\u0027: \u00274.10\u0027},"},{"line_number":74,"context_line":")"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7af24918_196e3196","line":73,"updated":"2016-03-02 12:50:25.000000000","message":"Okay, so you want to signal which compute nodes embed the inventories side of that, so you can do the check against multiple versions, right?\nSo, you want to make sure that it works also for continuous deployers ? Just asking that becuase Version 8 was also a Mitaka thing, so you\u0027re sure that the minimum version is still correct.","commit_id":"3ab687e03596676f58e0a17a212a7e7f307f99ab"}],"nova/tests/functional/db/test_compute_node.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3aa90e7b97ceb4755723d5a5dd78e0e7dd4e2eb","unresolved":false,"context_lines":[{"line_number":30,"context_line":"                                      free_disk_gb\u003d0, hypervisor_type\u003d\u0027danvm\u0027,"},{"line_number":31,"context_line":"                                      hypervisor_version\u003d1, cpu_info\u003d\u0027barf\u0027)"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    @mock.patch(\u0027nova.objects.Service.get_minimum_version_multi\u0027)"},{"line_number":34,"context_line":"    def test_create_creates_inventories(self, mock_minver):"},{"line_number":35,"context_line":"        mock_minver.return_value \u003d 10"},{"line_number":36,"context_line":"        self.cn.create()"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_f3aab46f","line":33,"updated":"2016-04-04 17:08:06.000000000","message":"Do we even need to mock this out? I mean, won\u0027t we already be running this test on the latest service version already?","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"da2ff13a0db5149430396a45f1108fd5774f3494","unresolved":false,"context_lines":[{"line_number":30,"context_line":"                                      free_disk_gb\u003d0, hypervisor_type\u003d\u0027danvm\u0027,"},{"line_number":31,"context_line":"                                      hypervisor_version\u003d1, cpu_info\u003d\u0027barf\u0027)"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    @mock.patch(\u0027nova.objects.Service.get_minimum_version_multi\u0027)"},{"line_number":34,"context_line":"    def test_create_creates_inventories(self, mock_minver):"},{"line_number":35,"context_line":"        mock_minver.return_value \u003d 10"},{"line_number":36,"context_line":"        self.cn.create()"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_0c998cc4","line":33,"in_reply_to":"9a061dce_593eb18c","updated":"2016-04-06 16:42:32.000000000","message":"Sorry, I just saw functional test and assumed services were running, because the API samples ones and some other functional regression tests start up the services.","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cc3cca07097beb9145b84174bf8bdfea04e3a616","unresolved":false,"context_lines":[{"line_number":30,"context_line":"                                      free_disk_gb\u003d0, hypervisor_type\u003d\u0027danvm\u0027,"},{"line_number":31,"context_line":"                                      hypervisor_version\u003d1, cpu_info\u003d\u0027barf\u0027)"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    @mock.patch(\u0027nova.objects.Service.get_minimum_version_multi\u0027)"},{"line_number":34,"context_line":"    def test_create_creates_inventories(self, mock_minver):"},{"line_number":35,"context_line":"        mock_minver.return_value \u003d 10"},{"line_number":36,"context_line":"        self.cn.create()"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a061dce_593eb18c","line":33,"in_reply_to":"9a061dce_f3aab46f","updated":"2016-04-06 13:52:08.000000000","message":"We do because we don\u0027t have any services actually started, which means we don\u0027t have any service records in the database, which means this call would default to returning zero, which indicates \"all services are older than the point at which we added SERVICE_VERSION\".","commit_id":"7bff1349629af9d3dd2a74ce3dceb42f063b855a"}]}