)]}'
{"nova/tests/virt/vmwareapi/test_vmwareapi_vm_util.py":[{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"77f20ec5c3f4c2f48f132f861e0e244b023ea374","unresolved":false,"context_lines":[{"line_number":50,"context_line":"        super(VMwareVMUtilTestCase, self).tearDown()"},{"line_number":51,"context_line":"        fake.reset()"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def test_compare_datastores(self):"},{"line_number":54,"context_line":"        ds1 \u003d vm_util.DSRecord(datastore\u003dNone, name\u003d\u0027ds1\u0027, capacity\u003d1024,"},{"line_number":55,"context_line":"                               freespace\u003d1024, host_count\u003d1)"},{"line_number":56,"context_line":"        ds2 \u003d vm_util.DSRecord(datastore\u003dNone, name\u003d\u0027ds2\u0027, capacity\u003d1024,"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2F12s%3D","line":53,"updated":"2014-01-07 20:21:01.000000000","message":"good","commit_id":"6b8be54ad6c8f82a1d026fbf73b03983e7d67e54"}],"nova/virt/vmwareapi/vm_util.py":[{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"21f52a73d21d31f66bb567acacd94fcfcc64c746","unresolved":false,"context_lines":[{"line_number":870,"context_line":"        return ds2"},{"line_number":871,"context_line":"    if ds1.freespace \u003e ds2.freespace:"},{"line_number":872,"context_line":"        return ds1"},{"line_number":873,"context_line":"    return ds2"},{"line_number":874,"context_line":""},{"line_number":875,"context_line":""},{"line_number":876,"context_line":"def _get_datastore_ref_and_name(data_stores, datastore_regex\u003dNone):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAT3%2F%2F0HA%3D","line":873,"updated":"2013-12-30 23:59:45.000000000","message":"This method bothers me. I feel like you should be able to write something much simpler to accomplish the same task.\n\nif ds1.host_count \u003d\u003d ds2.host_count:\n    # return the ds with the larger freespace\n    ds \u003d ds1 if ds1.freespace \u003e\u003d ds2.freespace else ds2\n    return ds\nelse:\n    # return the ds with the largest host_count (we know\n    # the two cannot be equal because of above statement)\n    ds \u003d ds1 if ds1.host_count \u003e ds2.host_count else ds2\n    return ds\n\nsee: \nhttp://docs.python.org/3/reference/expressions.html#conditional-expressions\n\nThis uses a ternary operator which some people don\u0027t like but I feel it\u0027s actually clearer in this case.","commit_id":"dabb868a0ead18c1eca2ba1685fd853ca447b99e"},{"author":{"_account_id":9172,"name":"Radoslav Gerganov","email":"rgerganov@vmware.com","username":"rgerganov"},"change_message_id":"d0f84f42f8ae70d4d4b879c267ec2673b45da90b","unresolved":false,"context_lines":[{"line_number":870,"context_line":"        return ds2"},{"line_number":871,"context_line":"    if ds1.freespace \u003e ds2.freespace:"},{"line_number":872,"context_line":"        return ds1"},{"line_number":873,"context_line":"    return ds2"},{"line_number":874,"context_line":""},{"line_number":875,"context_line":""},{"line_number":876,"context_line":"def _get_datastore_ref_and_name(data_stores, datastore_regex\u003dNone):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAUH%2F%2F4W0%3D","line":873,"in_reply_to":"AAAAT3%2F%2F0HA%3D","updated":"2014-01-07 09:42:25.000000000","message":"I prefer my version because it is easy to add more comparison criteria without nesting if statements. If you really dislike it, I will change it with ternary operators.\n\nI think both versions are simple enough and don\u0027t need any comments (IMO comments hurt readability in such cases)","commit_id":"dabb868a0ead18c1eca2ba1685fd853ca447b99e"},{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"21f52a73d21d31f66bb567acacd94fcfcc64c746","unresolved":false,"context_lines":[{"line_number":903,"context_line":"        ds_type \u003d propdict[\u0027summary.type\u0027]"},{"line_number":904,"context_line":"        ds_name \u003d propdict[\u0027summary.name\u0027]"},{"line_number":905,"context_line":"        host \u003d propdict.get(\u0027host\u0027)"},{"line_number":906,"context_line":"        host_count \u003d len(host.DatastoreHostMount) if host else 1"},{"line_number":907,"context_line":"        if ((ds_type \u003d\u003d \u0027VMFS\u0027 or ds_type \u003d\u003d \u0027NFS\u0027) and"},{"line_number":908,"context_line":"                propdict[\u0027summary.accessible\u0027]):"},{"line_number":909,"context_line":"            if datastore_regex is None or datastore_regex.match(ds_name):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAT3%2F%2F0Go%3D","line":906,"updated":"2013-12-30 23:59:45.000000000","message":"good use of a ternary operator, so, I know you are familiar with them.","commit_id":"dabb868a0ead18c1eca2ba1685fd853ca447b99e"},{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"77f20ec5c3f4c2f48f132f861e0e244b023ea374","unresolved":false,"context_lines":[{"line_number":874,"context_line":"        return ds2"},{"line_number":875,"context_line":"    if ds1.freespace \u003e ds2.freespace:"},{"line_number":876,"context_line":"        return ds1"},{"line_number":877,"context_line":"    return ds2"},{"line_number":878,"context_line":""},{"line_number":879,"context_line":""},{"line_number":880,"context_line":"def _get_datastore_ref_and_name(data_stores, datastore_regex\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2F13Y%3D","line":877,"updated":"2014-01-07 20:21:01.000000000","message":"I think you have a problem... its more visible if you rewrite the method one of these ways...\n\nconsider:\n\nif ds1.host_count \u003d\u003d ds2.host_count:\n    # same number of hosts, compare freespace\n    if ds1.freespace \u003e ds2.freespace:\n        return ds1\n    else:\n        return ds2\n\n# otherwise largest host count wins\nif ds1.host_count \u003e ds2.host_count:\n    return ds1\nreturn ds2\n\nOr:\nif ds1.host_count \u003d\u003d ds2.host_count:\n    ds \u003d ds1 if ds1.freespace \u003e ds2.freespace else ds2\n    return ds\nif ds1.host_count \u003e ds2.host_count:\n    return ds1\nreturn ds2\n\n... at some point you may have a large amount of freespace that is completely unused. The comparison will not take into account the datastore\u0027s freespace until you find two datastores with the same number of attached hosts.","commit_id":"6b8be54ad6c8f82a1d026fbf73b03983e7d67e54"},{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"e706ffcdccdb45d6e089322c082d716e16a95de7","unresolved":false,"context_lines":[{"line_number":874,"context_line":"        return ds2"},{"line_number":875,"context_line":"    if ds1.freespace \u003e ds2.freespace:"},{"line_number":876,"context_line":"        return ds1"},{"line_number":877,"context_line":"    return ds2"},{"line_number":878,"context_line":""},{"line_number":879,"context_line":""},{"line_number":880,"context_line":"def _get_datastore_ref_and_name(data_stores, datastore_regex\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2FyCg%3D","line":877,"in_reply_to":"AAAAUH%2F%2F0E0%3D","updated":"2014-01-08 14:46:51.000000000","message":"Yes. 2 tiers, a \u0027shared\u0027 tier then an \u0027available\u0027 tier. But as I think about this there are other problems... we need to validate the selection of Host and the selection of Datastore ... we need to be sure that the two work together!\n\nThat means we need to have something that tells us \u0027is this datastore accessible from that host\u0027 on some level and *that* needs to be part of the selection criterion.","commit_id":"6b8be54ad6c8f82a1d026fbf73b03983e7d67e54"},{"author":{"_account_id":8027,"name":"Vui Lam","email":"vui@vmware.com","username":"vui"},"change_message_id":"315d17a6d04e81a875c7adf33eebdfe242d68e44","unresolved":false,"context_lines":[{"line_number":874,"context_line":"        return ds2"},{"line_number":875,"context_line":"    if ds1.freespace \u003e ds2.freespace:"},{"line_number":876,"context_line":"        return ds1"},{"line_number":877,"context_line":"    return ds2"},{"line_number":878,"context_line":""},{"line_number":879,"context_line":""},{"line_number":880,"context_line":"def _get_datastore_ref_and_name(data_stores, datastore_regex\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2F0E0%3D","line":877,"in_reply_to":"AAAAUH%2F%2F13Y%3D","updated":"2014-01-08 06:41:15.000000000","message":"Nice catch and good point. \n\nIn general favoring higher host_count is advisable, but it should not be at the detriment of picking a ds with insufficient space for the current operation. But without actually supplying to the ds picking algorithm the amount of storage space we need, we can\u0027t figure out whether a datastore is viable or not (*).\n\nIf I understand your suggestion, it is to pick the among preferred ds with the most freespace, and by preferred you mean host_count \u003e 1. This effectively divides the ds into two tiers, shared vs non-shared (instead of multiple groups by host count in the current patch), and we pick from the shared tier until we can\u0027t. However, we can still run into the same problem (*) where we are not sure when we need to drop to the non-shared tier. That said, this approach will minimize the effect of (*) (e.g. if all datastores are shared, we will be able to effectively use the storage available among them regardless of how \u0027shared\u0027 the datastores are)","commit_id":"6b8be54ad6c8f82a1d026fbf73b03983e7d67e54"},{"author":{"_account_id":7629,"name":"Shawn Hartsock","email":"hartsock@acm.org","username":"hartsocks"},"change_message_id":"05660dc09cafa5f395fe80c8034421d4c1a0dd0a","unresolved":false,"context_lines":[{"line_number":874,"context_line":"        return ds2"},{"line_number":875,"context_line":"    if ds1.freespace \u003e ds2.freespace:"},{"line_number":876,"context_line":"        return ds1"},{"line_number":877,"context_line":"    return ds2"},{"line_number":878,"context_line":""},{"line_number":879,"context_line":""},{"line_number":880,"context_line":"def _get_datastore_ref_and_name(data_stores, datastore_regex\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2BuAU%3D","line":877,"in_reply_to":"AAAAUH%2F%2Fm0o%3D","updated":"2014-01-22 20:36:39.000000000","message":"From this method _compare_datastores(ds1, ds2) we determine the relative \"fitness\" of two datastores using insufficient information. The result could be surprising as the datastore selection will spontaneously fail even though the scheduler believes there is enough space on the cluster for the operation to complete.\n\nIn short:\n_compare_datastores(ds1, ds2) -\u003e does not have enough information to make an intelligent decision.","commit_id":"6b8be54ad6c8f82a1d026fbf73b03983e7d67e54"},{"author":{"_account_id":9172,"name":"Radoslav Gerganov","email":"rgerganov@vmware.com","username":"rgerganov"},"change_message_id":"7b5d07604f7fd5f091251735ee78ceb8a3197529","unresolved":false,"context_lines":[{"line_number":874,"context_line":"        return ds2"},{"line_number":875,"context_line":"    if ds1.freespace \u003e ds2.freespace:"},{"line_number":876,"context_line":"        return ds1"},{"line_number":877,"context_line":"    return ds2"},{"line_number":878,"context_line":""},{"line_number":879,"context_line":""},{"line_number":880,"context_line":"def _get_datastore_ref_and_name(data_stores, datastore_regex\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2Fm0o%3D","line":877,"in_reply_to":"AAAAUH%2F%2FyCg%3D","updated":"2014-01-10 15:03:18.000000000","message":"If we provision on a cluster why we need this check? We select a datastore among the datastores which are accessible to the cluster. I am not aware of any host selection but maybe I am missing something. Shawn, could you please elaborate?","commit_id":"6b8be54ad6c8f82a1d026fbf73b03983e7d67e54"},{"author":{"_account_id":8027,"name":"Vui Lam","email":"vui@vmware.com","username":"vui"},"change_message_id":"315d17a6d04e81a875c7adf33eebdfe242d68e44","unresolved":false,"context_lines":[{"line_number":878,"context_line":""},{"line_number":879,"context_line":""},{"line_number":880,"context_line":"def _get_datastore_ref_and_name(data_stores, datastore_regex\u003dNone):"},{"line_number":881,"context_line":"    # selects the datastore with the most freespace"},{"line_number":882,"context_line":"    \"\"\"Find a usable datastore in a given RetrieveResult object."},{"line_number":883,"context_line":""},{"line_number":884,"context_line":"    :param data_stores: a RetrieveResult object from vSphere API call"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2F0Fc%3D","line":881,"updated":"2014-01-08 06:41:15.000000000","message":"This comment should be updated to match the behavior.","commit_id":"6b8be54ad6c8f82a1d026fbf73b03983e7d67e54"}]}