)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"84a0db3dd1ff0a0dcf6e1c626dc6addeed7777d7","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Sahid Orentino Ferdjaoui \u003csahid.ferdjaoui@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-04-28 08:06:51 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"libvirt: introduces a Guest to wrap arround virConnect"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Introduces object Guest to wrap arround virConnect object"},{"line_number":10,"context_line":"and provide a higher level API."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"7ad32947_f76893bc","line":7,"updated":"2015-04-28 12:50:50.000000000","message":"s/virConnect/virDomain/","commit_id":"dd756fbff376a3a97ef5f390dbae6ef8a60918d4"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"84a0db3dd1ff0a0dcf6e1c626dc6addeed7777d7","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"libvirt: introduces a Guest to wrap arround virConnect"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Introduces object Guest to wrap arround virConnect object"},{"line_number":10,"context_line":"and provide a higher level API."},{"line_number":11,"context_line":"With the introduction of this new class also update _create_domain"},{"line_number":12,"context_line":"logic and update tests."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"7ad32947_779b63ca","line":9,"updated":"2015-04-28 12:50:50.000000000","message":"s/virConnect/virDomain","commit_id":"dd756fbff376a3a97ef5f390dbae6ef8a60918d4"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"42a0bc233952aaae8474ceab1330f13c7671b9b0","unresolved":false,"context_lines":[{"line_number":18,"context_line":"libvirt/driver.py and libvirt/test_driver.py since several"},{"line_number":19,"context_line":"parts of the code from thoce files will be moved in"},{"line_number":20,"context_line":"guests.py and test_guest.py"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I2d193ae44a4ac48ad863d237172cf2c1da05e3e9"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"7a016987_c23a4acd","line":21,"updated":"2015-05-27 11:33:49.000000000","message":"As Sean Dague mentioned on patch set 3, there should be a replicable reference to: \n\n   \"high level sketch of the libvirt cleanups for the cycle, \n    possibly an ML thread or etherpad, just to make reviewing \n    them have a global context.\"","commit_id":"a37f24912e56669863ca3e008a8ef9643f9935f0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4f6db45cc7a4bc6bd0b9341b87eb7af5945cf936","unresolved":false,"context_lines":[{"line_number":18,"context_line":"libvirt/driver.py and libvirt/test_driver.py since several"},{"line_number":19,"context_line":"parts of the code from thoce files will be moved in"},{"line_number":20,"context_line":"guests.py and test_guest.py"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I2d193ae44a4ac48ad863d237172cf2c1da05e3e9"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"7a016987_f6db93eb","line":21,"in_reply_to":"7a016987_bb93c6f9","updated":"2015-05-28 14:58:47.000000000","message":"Talked to sahid in IRC and it sounds like this is a separate refactor and is not explicitly tied to the admin password blueprint.","commit_id":"a37f24912e56669863ca3e008a8ef9643f9935f0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"175a8086f3e3803fd8f5f47ce88a78d787dec050","unresolved":false,"context_lines":[{"line_number":18,"context_line":"libvirt/driver.py and libvirt/test_driver.py since several"},{"line_number":19,"context_line":"parts of the code from thoce files will be moved in"},{"line_number":20,"context_line":"guests.py and test_guest.py"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I2d193ae44a4ac48ad863d237172cf2c1da05e3e9"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"7a016987_bb93c6f9","line":21,"in_reply_to":"7a016987_c23a4acd","updated":"2015-05-28 14:43:18.000000000","message":"Is this actually just tied to blueprint libvirt-set-admin-password ? If so, that\u0027s not approved so we shouldn\u0027t be ramming this all in until that\u0027s approved.","commit_id":"a37f24912e56669863ca3e008a8ef9643f9935f0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"23e487a67c210d40aab2a332771ea2ca6a7b0936","unresolved":false,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Note about Copyrights, they have been copied from"},{"line_number":18,"context_line":"libvirt/driver.py and libvirt/test_driver.py since several"},{"line_number":19,"context_line":"parts of the code from thoce files will be moved in"},{"line_number":20,"context_line":"guests.py and test_guest.py"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I2d193ae44a4ac48ad863d237172cf2c1da05e3e9"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":16,"id":"5afe65bd_8f95bde5","line":19,"updated":"2015-06-02 12:40:26.000000000","message":"s/thoce/those","commit_id":"3fc3b9447129c9789becfe2e0bceb48e3371c59e"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"71bf6146ed08a1e6ddb21b8b4896d76ab7ffd3b8","unresolved":false,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Note about Copyrights, they have been copied from"},{"line_number":18,"context_line":"libvirt/driver.py and libvirt/test_driver.py since several"},{"line_number":19,"context_line":"parts of the code from thoce files will be moved in"},{"line_number":20,"context_line":"guests.py and test_guest.py"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I2d193ae44a4ac48ad863d237172cf2c1da05e3e9"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":16,"id":"5afe65bd_b5f20799","line":19,"in_reply_to":"5afe65bd_8f95bde5","updated":"2015-06-02 14:01:27.000000000","message":"Done","commit_id":"3fc3b9447129c9789becfe2e0bceb48e3371c59e"}],"nova/tests/unit/virt/libvirt/test_guest.py":[{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"add3620d00867a3cd654f6ac59bbd0f3cf2c7ccb","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#    Copyright 2010 OpenStack Foundation"},{"line_number":2,"context_line":"#    Copyright 2012 University Of Minho"},{"line_number":3,"context_line":"#    Copyright 2014-2015 Red Hat, Inc"},{"line_number":4,"context_line":"#"},{"line_number":5,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":6,"context_line":"#    not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_24c3ee8a","line":3,"range":{"start_line":3,"start_character":5,"end_line":3,"end_character":14},"updated":"2015-05-21 16:20:07.000000000","message":"Surely this copyright statement is wrong? I can\u0027t see where this code is pulled out of another file?","commit_id":"e8b11dad23574a285b221642230d1e53bba96c9b"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"641fb63d75c1d8fef6fd1adf67ed6d418f88585c","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#    Copyright 2010 OpenStack Foundation"},{"line_number":2,"context_line":"#    Copyright 2012 University Of Minho"},{"line_number":3,"context_line":"#    Copyright 2014-2015 Red Hat, Inc"},{"line_number":4,"context_line":"#"},{"line_number":5,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":6,"context_line":"#    not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_a71ec06d","line":3,"in_reply_to":"7a016987_24c3ee8a","updated":"2015-05-21 17:20:29.000000000","message":"Hum yes...","commit_id":"e8b11dad23574a285b221642230d1e53bba96c9b"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"42a0bc233952aaae8474ceab1330f13c7671b9b0","unresolved":false,"context_lines":[{"line_number":31,"context_line":"CONF \u003d cfg.CONF"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"class GuestTestCase(test.NoDBTestCase):"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def setUp(self):"},{"line_number":37,"context_line":"        super(GuestTestCase, self).setUp()"}],"source_content_type":"text/x-python","patch_set":14,"id":"7a016987_0e02d309","line":34,"updated":"2015-05-27 11:33:49.000000000","message":"This test case is missing a test for the public \"get_interfaces\" method of the guest. Ideally for \"enable_hairpin\" as well.","commit_id":"a37f24912e56669863ca3e008a8ef9643f9935f0"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"5d5f1312921e26baaf32dd8703fd37ebc15d9c2f","unresolved":false,"context_lines":[{"line_number":31,"context_line":"CONF \u003d cfg.CONF"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"class GuestTestCase(test.NoDBTestCase):"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def setUp(self):"},{"line_number":37,"context_line":"        super(GuestTestCase, self).setUp()"}],"source_content_type":"text/x-python","patch_set":14,"id":"7a016987_c4532116","line":34,"in_reply_to":"7a016987_0e02d309","updated":"2015-05-29 07:24:18.000000000","message":"Ok I will add them","commit_id":"a37f24912e56669863ca3e008a8ef9643f9935f0"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"e6e754d83c20ddd4c2a0f8d05ef47b079fe9cd50","unresolved":false,"context_lines":[{"line_number":4310,"context_line":"        else:"},{"line_number":4311,"context_line":"            guest \u003d libvirt_guest.Guest(domain)"},{"line_number":4312,"context_line":""},{"line_number":4313,"context_line":"        if power_on or pause:"},{"line_number":4314,"context_line":"            guest.launch(pause\u003dpause)"},{"line_number":4315,"context_line":""},{"line_number":4316,"context_line":"        if not utils.is_neutron():"}],"source_content_type":"text/x-python","patch_set":10,"id":"1aca2d91_37cb9555","line":4313,"updated":"2015-05-07 09:33:36.000000000","message":"It looks like this could just be \u0027if power_on\u0027 - the previous code didn\u0027t check the pause flag.","commit_id":"3f0300a689a49c50534c87844bf4f16c68daa64e"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"02a243f2bc96bc45402e8d758d2d072ba375b28f","unresolved":false,"context_lines":[{"line_number":4310,"context_line":"        else:"},{"line_number":4311,"context_line":"            guest \u003d libvirt_guest.Guest(domain)"},{"line_number":4312,"context_line":""},{"line_number":4313,"context_line":"        if power_on or pause:"},{"line_number":4314,"context_line":"            guest.launch(pause\u003dpause)"},{"line_number":4315,"context_line":""},{"line_number":4316,"context_line":"        if not utils.is_neutron():"}],"source_content_type":"text/x-python","patch_set":10,"id":"1aca2d91_f74e3dc0","line":4313,"in_reply_to":"1aca2d91_37cb9555","updated":"2015-05-07 09:37:14.000000000","message":"Yes i intentionally changed the condition since if we want to use the parameter pause we need to power on.","commit_id":"3f0300a689a49c50534c87844bf4f16c68daa64e"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"8c9eca9cda48573d27a90ad87aa3672da14293fb","unresolved":false,"context_lines":[{"line_number":4286,"context_line":"            self._create_domain_cleanup_lxc(instance)"},{"line_number":4287,"context_line":""},{"line_number":4288,"context_line":"    def _create_domain("},{"line_number":4289,"context_line":"            self, xml\u003dNone, domain\u003dNone, pause\u003dFalse, power_on\u003dTrue):"},{"line_number":4290,"context_line":"        \"\"\"Create a domain."},{"line_number":4291,"context_line":""},{"line_number":4292,"context_line":"        Either domain or xml must be passed in. If both are passed, then"}],"source_content_type":"text/x-python","patch_set":12,"id":"9a0c5dc1_331773ef","line":4289,"updated":"2015-05-12 08:16:54.000000000","message":"The parameters should start on the same line as the method name, should they?","commit_id":"f1333eefbd9224149d31d30f0a5fbb2173b24a87"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"23e487a67c210d40aab2a332771ea2ca6a7b0936","unresolved":false,"context_lines":[{"line_number":4321,"context_line":"            self._create_domain_cleanup_lxc(instance)"},{"line_number":4322,"context_line":""},{"line_number":4323,"context_line":"    def _create_domain("},{"line_number":4324,"context_line":"            self, xml\u003dNone, domain\u003dNone, pause\u003dFalse, power_on\u003dTrue):"},{"line_number":4325,"context_line":"        \"\"\"Create a domain."},{"line_number":4326,"context_line":""},{"line_number":4327,"context_line":"        Either domain or xml must be passed in. If both are passed, then"}],"source_content_type":"text/x-python","patch_set":16,"id":"5afe65bd_58578d02","line":4324,"updated":"2015-06-02 12:40:26.000000000","message":"nit: you could have left the original line 4332 alone and just added a newline with:\n\n pause\u003dTrue):\n\nappended to the end of the existing kwargs.","commit_id":"3fc3b9447129c9789becfe2e0bceb48e3371c59e"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"71bf6146ed08a1e6ddb21b8b4896d76ab7ffd3b8","unresolved":false,"context_lines":[{"line_number":4321,"context_line":"            self._create_domain_cleanup_lxc(instance)"},{"line_number":4322,"context_line":""},{"line_number":4323,"context_line":"    def _create_domain("},{"line_number":4324,"context_line":"            self, xml\u003dNone, domain\u003dNone, pause\u003dFalse, power_on\u003dTrue):"},{"line_number":4325,"context_line":"        \"\"\"Create a domain."},{"line_number":4326,"context_line":""},{"line_number":4327,"context_line":"        Either domain or xml must be passed in. If both are passed, then"}],"source_content_type":"text/x-python","patch_set":16,"id":"5afe65bd_b590c796","line":4324,"in_reply_to":"5afe65bd_58578d02","updated":"2015-06-02 14:01:27.000000000","message":"Yes.. I like to indent by this way I will address your comment in the next PS","commit_id":"3fc3b9447129c9789becfe2e0bceb48e3371c59e"}],"nova/virt/libvirt/guest.py":[{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"a93b97ff900354e165c2cac798364b02ab141dbd","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        self._guest \u003d guest"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def __call__(self, *args, **kwargs):"},{"line_number":40,"context_line":"        raise NotImplementedError()"}],"source_content_type":"text/x-python","patch_set":1,"id":"3ab451fb_7d6241da","line":40,"updated":"2015-04-17 10:00:14.000000000","message":"I was thinking of a slightly different approach to this problem.  As you know we recently introduced the Host class which wraps around the libvirt virConnect object and provides a slightly higher level API for things Nova needs todo with it.\n\nI was thinking we\u0027d introduce a Guest class which wraps around the libvirt virDomain object. That would have various helper methods, eg a \u0027shutdown\u0027 method which takes the code from driver._destroy, a \u0027get_serial_ports method, etc, etc. Basically any code in driver.py that uses a virDomain python object should really call into a Guest.nnnn method. So that we end up with  driver.py not directly invoking any libvirt APIs - all the libvirt APIs are encapsulated in Guest or Host objects.  Finally, the list_instance_domains methods in Host, would want to be updated to return our new Guest objects, rather than directly returning virDomain objects.\n\nNow, I\u0027ve two thoughts about what lives as a _get_guest_config method in the driver.py file. One idea would for that to just move into Guest object as a  get_config() method marked as @staticmethod. The other idea would be to move it into the designer.py class as a set of file level methods there. This would avoid Guest object getting too large, so I\u0027d have a slight preference towards the second idea currently.\n\nAs for your Plugin class, I\u0027m not really convinced it buys us anything - we\u0027re never going to expose the ability for externally written plugins, so I think it is simpler to just keep the get_guest_*_config methods as they are written today. eg we do just a plain move of the code from driver.py to designer.py.\n\nAll in all though,  I agree with the desire to simplfy driver.py by moving the guest related methods out of it.","commit_id":"dfcac42c824294003a80086f10bc0facc8b686fe"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"5d3ab254edf5cb6f34e779f8ed68d7c1a94eb42a","unresolved":false,"context_lines":[{"line_number":54,"context_line":"    def _get_connection(self):"},{"line_number":55,"context_line":"        return self._host.get_connection()"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    _conn \u003d property(_get_connection)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def create_domain(self, xml\u003dNone, domain\u003dNone,"},{"line_number":60,"context_line":"                      instance\u003dNone, launch_flags\u003d0, power_on\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":3,"id":"bab921fb_76d0cc1f","line":57,"updated":"2015-04-24 12:47:31.000000000","message":"You\u0027ve designed the Guest class here as a singleton. I really feel it should be setup as a wrapper around the libvirt.Domain object - in the same way that Host is a wrapper around libvirt.Connection.\n\nSo I\u0027d expect the \u0027domain\u0027 to be passed into the constructor here. There would be no need for a _get_connection() method, as every non-static method would operate on the \u0027self._domain\u0027  not the connection.","commit_id":"95d857324f602f430f7fb007533fde8a4c8c6036"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"5d3ab254edf5cb6f34e779f8ed68d7c1a94eb42a","unresolved":false,"context_lines":[{"line_number":57,"context_line":"    _conn \u003d property(_get_connection)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def create_domain(self, xml\u003dNone, domain\u003dNone,"},{"line_number":60,"context_line":"                      instance\u003dNone, launch_flags\u003d0, power_on\u003dTrue):"},{"line_number":61,"context_line":"        \"\"\"Create a domain."},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"        Either domain or xml must be passed in. If both are passed, then"}],"source_content_type":"text/x-python","patch_set":3,"id":"bab921fb_7697ac48","line":60,"updated":"2015-04-24 12:47:31.000000000","message":"I\u0027d expect this to be @staticmethod or @classmethod, have the Host instance passed in and return a Guest instance on success.","commit_id":"95d857324f602f430f7fb007533fde8a4c8c6036"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"5d3ab254edf5cb6f34e779f8ed68d7c1a94eb42a","unresolved":false,"context_lines":[{"line_number":96,"context_line":"                          run_as_root\u003dTrue,"},{"line_number":97,"context_line":"                          check_exit_code\u003d[0, 1])"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def _get_interfaces(self, xml):"},{"line_number":100,"context_line":"        \"\"\"Note that this function takes a domain xml."},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        Returns a list of all network interfaces for this instance."}],"source_content_type":"text/x-python","patch_set":3,"id":"bab921fb_36c6d448","line":99,"updated":"2015-04-24 12:47:31.000000000","message":"No need to pass XML into this - I\u0027d expect  self._domain.XMLDesc() to be called. Though we might in fact consider calling   self._domain.XMLDesc() once, creating a nova.virt.libvirt.config.LibvirtConfigGuest class and then using that, instead of doing XPath queries","commit_id":"95d857324f602f430f7fb007533fde8a4c8c6036"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"84a0db3dd1ff0a0dcf6e1c626dc6addeed7777d7","unresolved":false,"context_lines":[{"line_number":50,"context_line":"            libvirt \u003d importutils.import_module(\u0027libvirt\u0027)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        self._domain \u003d domain"},{"line_number":53,"context_line":"        self._host \u003d host"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    @property"},{"line_number":56,"context_line":"    def _xml(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"7ad32947_77824354","line":53,"updated":"2015-04-28 12:50:50.000000000","message":"We shouldn\u0027t need to keep a reference to the host object here. Only the @classmethod should need it and they can accept it as a parameter.","commit_id":"dd756fbff376a3a97ef5f390dbae6ef8a60918d4"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"2fc606ee4cd358065a7c707a631e1cef84398ab4","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        :param flags: In (VIR_DOMAIN_START_PAUSED |"},{"line_number":83,"context_line":"                          VIR_DOMAIN_START_AUTODESTROY |"},{"line_number":84,"context_line":"                          VIR_DOMAIN_START_BYPASS_CACHE |"},{"line_number":85,"context_line":"                          VIR_DOMAIN_START_FORCE_BOOT)"},{"line_number":86,"context_line":"        \"\"\""},{"line_number":87,"context_line":"        try:"},{"line_number":88,"context_line":"            return self._domain.createWithFlags(flags)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3acd31a7_c2c9e127","line":85,"updated":"2015-05-05 15:53:09.000000000","message":"Instead of expecting callers to pass in a set of libvirt constants, I think it\u0027d be cleaner separation if we just had a named parameter for the only flag we care about using. eg\nget rid of flags\u003d0 and add paused\u003dFalse. This method can then set the correct libvirt flag to match","commit_id":"09719ed8fa2e0a0b48a9e7287965322e6f52c18f"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"dd126de04a43ea8e2bd1159330be67935cf2e380","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        :param flags: In (VIR_DOMAIN_START_PAUSED |"},{"line_number":83,"context_line":"                          VIR_DOMAIN_START_AUTODESTROY |"},{"line_number":84,"context_line":"                          VIR_DOMAIN_START_BYPASS_CACHE |"},{"line_number":85,"context_line":"                          VIR_DOMAIN_START_FORCE_BOOT)"},{"line_number":86,"context_line":"        \"\"\""},{"line_number":87,"context_line":"        try:"},{"line_number":88,"context_line":"            return self._domain.createWithFlags(flags)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1aca2d91_f49e6406","line":85,"in_reply_to":"3acd31a7_c2c9e127","updated":"2015-05-06 12:39:46.000000000","message":"Sure, that will be better yes.","commit_id":"09719ed8fa2e0a0b48a9e7287965322e6f52c18f"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"8c9eca9cda48573d27a90ad87aa3672da14293fb","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        return self._domain.XMLDesc(0)"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    def __str__(self):"},{"line_number":59,"context_line":"        return encodeutils.safe_decode(self._xml)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    @classmethod"},{"line_number":62,"context_line":"    def create(cls, xml, host):"}],"source_content_type":"text/x-python","patch_set":12,"id":"9a0c5dc1_13d34f3a","line":59,"updated":"2015-05-12 08:16:54.000000000","message":"This approach will bind the \"human readable representation\nof an object\" to the domain logic which will cause trouble\n(in my experience). I really strongly advise to not do that.\nIn review 181969 you used that line because of that:\n\n    xml \u003d str(self._host.get_domain(instance))\n\nIn Exceptions and logs this will look messy. An xpatch query\nof a few selected attributes would be more useful here,\nagain, when one has readability has mind. I quote [1] for\nthe use of \"__str__\": \n    \n    \u003e\u003e to compute the “informal” string representation of an object \u003c\u003c\n\n[1] https://docs.python.org/2/reference/datamodel.html#object.__str__","commit_id":"f1333eefbd9224149d31d30f0a5fbb2173b24a87"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"7652581d908e8047a8424ed3a8f24a8a99e3de85","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        return self._domain.XMLDesc(0)"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    def __str__(self):"},{"line_number":59,"context_line":"        return encodeutils.safe_decode(self._xml)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    @classmethod"},{"line_number":62,"context_line":"    def create(cls, xml, host):"}],"source_content_type":"text/x-python","patch_set":12,"id":"9a0c5dc1_c9aaa885","line":59,"in_reply_to":"9a0c5dc1_13d34f3a","updated":"2015-05-12 09:08:59.000000000","message":"More importantly, I think this will end up spamming the logs too much - the XML doc for a guest is *very* large and so we\u0027ll only want this logged in specific cases. I think it is better if we do stringification to just include the name, uuid and id unique identifiers.  BTW, you should override __repr__ rather than __str__ IIRC.","commit_id":"f1333eefbd9224149d31d30f0a5fbb2173b24a87"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"8c9eca9cda48573d27a90ad87aa3672da14293fb","unresolved":false,"context_lines":[{"line_number":107,"context_line":"                          self, errors\u003d\u0027ignore\u0027)"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"    def get_interfaces(self):"},{"line_number":110,"context_line":"        \"\"\"Returns a list of all network interfaces for this domain.\"\"\""},{"line_number":111,"context_line":"        doc \u003d None"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":12,"id":"9a0c5dc1_93e73f12","line":110,"updated":"2015-05-12 08:16:54.000000000","message":"Do we really want to return the etree XML nodes in a public method? If the return values are needed by other modules I\u0027d like to see them as python objects. The return type should then be named in the docstring as well. If network interfaces are only needed in the guest module, the method should be private.","commit_id":"f1333eefbd9224149d31d30f0a5fbb2173b24a87"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"7652581d908e8047a8424ed3a8f24a8a99e3de85","unresolved":false,"context_lines":[{"line_number":107,"context_line":"                          self, errors\u003d\u0027ignore\u0027)"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"    def get_interfaces(self):"},{"line_number":110,"context_line":"        \"\"\"Returns a list of all network interfaces for this domain.\"\"\""},{"line_number":111,"context_line":"        doc \u003d None"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":12,"id":"9a0c5dc1_c98188fd","line":110,"in_reply_to":"9a0c5dc1_93e73f12","updated":"2015-05-12 09:08:59.000000000","message":"IIUC this is only in fact returning the interface name strings, not elements nodes.","commit_id":"f1333eefbd9224149d31d30f0a5fbb2173b24a87"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"df28e00c4df9835f1ee4786d63a80512dd2c9ebf","unresolved":false,"context_lines":[{"line_number":107,"context_line":"                          self, errors\u003d\u0027ignore\u0027)"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"    def get_interfaces(self):"},{"line_number":110,"context_line":"        \"\"\"Returns a list of all network interfaces for this domain.\"\"\""},{"line_number":111,"context_line":"        doc \u003d None"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":12,"id":"9a0c5dc1_c943c86b","line":110,"in_reply_to":"9a0c5dc1_c98188fd","updated":"2015-05-12 09:20:41.000000000","message":"Yes, you are right. The return value is a list of strings which happen to be interface names (I misunderstood that in the first place). I usually tend to name such methods like \"get_interfaces_names\" or something like that. Because from the current naming I would expect a complex structure with a variety of attributes.","commit_id":"f1333eefbd9224149d31d30f0a5fbb2173b24a87"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"add3620d00867a3cd654f6ac59bbd0f3cf2c7ccb","unresolved":false,"context_lines":[{"line_number":5,"context_line":"# Copyright (c) 2011 Piston Cloud Computing, Inc"},{"line_number":6,"context_line":"# Copyright (c) 2012 University Of Minho"},{"line_number":7,"context_line":"# Copyright (c) 2013 Hewlett-Packard Development Company, L.P."},{"line_number":8,"context_line":"# Copyright (c) 2015 Red Hat, Inc"},{"line_number":9,"context_line":"#"},{"line_number":10,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":11,"context_line":"#    not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_04beb2fe","line":8,"updated":"2015-05-21 16:20:07.000000000","message":"Ditto on the possibly wrong copyright?","commit_id":"e8b11dad23574a285b221642230d1e53bba96c9b"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"641fb63d75c1d8fef6fd1adf67ed6d418f88585c","unresolved":false,"context_lines":[{"line_number":5,"context_line":"# Copyright (c) 2011 Piston Cloud Computing, Inc"},{"line_number":6,"context_line":"# Copyright (c) 2012 University Of Minho"},{"line_number":7,"context_line":"# Copyright (c) 2013 Hewlett-Packard Development Company, L.P."},{"line_number":8,"context_line":"# Copyright (c) 2015 Red Hat, Inc"},{"line_number":9,"context_line":"#"},{"line_number":10,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":11,"context_line":"#    not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_6746887c","line":8,"in_reply_to":"7a016987_04beb2fe","updated":"2015-05-21 17:20:29.000000000","message":"What do you mean? some of the code has been taken from the driver libvirt like enable_hairpin or get_interfaces. \n\nWhat should I do ?","commit_id":"e8b11dad23574a285b221642230d1e53bba96c9b"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"42a0bc233952aaae8474ceab1330f13c7671b9b0","unresolved":false,"context_lines":[{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        :param pause: Indicates whether to start and pause the guest"},{"line_number":100,"context_line":"        \"\"\""},{"line_number":101,"context_line":"        flags \u003d pause and libvirt.VIR_DOMAIN_START_PAUSED or 0"},{"line_number":102,"context_line":"        try:"},{"line_number":103,"context_line":"            return self._domain.createWithFlags(flags)"},{"line_number":104,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":14,"id":"7a016987_d7b8ce77","line":101,"updated":"2015-05-27 11:33:49.000000000","message":"My boolean algebra is a bit rusty, but could it be that the \"or 0\" will not have any effect? I tried this https://www.wolframalpha.com/input/?i\u003dP+and+Q+or+0 to get a truth table.","commit_id":"a37f24912e56669863ca3e008a8ef9643f9935f0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"23e487a67c210d40aab2a332771ea2ca6a7b0936","unresolved":false,"context_lines":[{"line_number":47,"context_line":""},{"line_number":48,"context_line":"        global libvirt"},{"line_number":49,"context_line":"        if libvirt is None:"},{"line_number":50,"context_line":"            libvirt \u003d importutils.import_module(\u0027libvirt\u0027)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        self._domain \u003d domain"},{"line_number":53,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"5afe65bd_3dbba374","line":50,"updated":"2015-06-02 12:40:26.000000000","message":"This seems like it would be extremely inefficient to be importing a module on creation of every Guest instance...especially considering the *only* use of the libvirt variable in this entire module is to reference a constant (libvirt.VIR_DOMAIN_START_PAUSED).\n\nWhy not just do the import_module() once in the global scope?","commit_id":"3fc3b9447129c9789becfe2e0bceb48e3371c59e"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"71bf6146ed08a1e6ddb21b8b4896d76ab7ffd3b8","unresolved":false,"context_lines":[{"line_number":47,"context_line":""},{"line_number":48,"context_line":"        global libvirt"},{"line_number":49,"context_line":"        if libvirt is None:"},{"line_number":50,"context_line":"            libvirt \u003d importutils.import_module(\u0027libvirt\u0027)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        self._domain \u003d domain"},{"line_number":53,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"5afe65bd_b5db271b","line":50,"in_reply_to":"5afe65bd_3dbba374","updated":"2015-06-02 14:01:27.000000000","message":"Yes, we want a way to set the global variable \"libvirt\" after we have imported the module to explicitly set fakelibvirt. The problem is if we put that block in the module level it will be evaluated when the module will be imported without let to us a chance to set fakelibvirt.\n\nI least it is my understand but please let me know if you have better idea?","commit_id":"3fc3b9447129c9789becfe2e0bceb48e3371c59e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"23e487a67c210d40aab2a332771ea2ca6a7b0936","unresolved":false,"context_lines":[{"line_number":84,"context_line":"        :returns guest.Guest: Guest ready to be launched"},{"line_number":85,"context_line":"        \"\"\""},{"line_number":86,"context_line":"        try:"},{"line_number":87,"context_line":"            # TODO(sahid): Host.write_instance_config should returns"},{"line_number":88,"context_line":"            # an instance of Guest"},{"line_number":89,"context_line":"            domain \u003d host.write_instance_config(xml)"},{"line_number":90,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":16,"id":"5afe65bd_783c8993","line":87,"updated":"2015-06-02 12:40:26.000000000","message":"nit: s/returns/return","commit_id":"3fc3b9447129c9789becfe2e0bceb48e3371c59e"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"71bf6146ed08a1e6ddb21b8b4896d76ab7ffd3b8","unresolved":false,"context_lines":[{"line_number":84,"context_line":"        :returns guest.Guest: Guest ready to be launched"},{"line_number":85,"context_line":"        \"\"\""},{"line_number":86,"context_line":"        try:"},{"line_number":87,"context_line":"            # TODO(sahid): Host.write_instance_config should returns"},{"line_number":88,"context_line":"            # an instance of Guest"},{"line_number":89,"context_line":"            domain \u003d host.write_instance_config(xml)"},{"line_number":90,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":16,"id":"5afe65bd_75048f7d","line":87,"in_reply_to":"5afe65bd_783c8993","updated":"2015-06-02 14:01:27.000000000","message":"Done","commit_id":"3fc3b9447129c9789becfe2e0bceb48e3371c59e"}]}